Printing room access page

[Gunvor4]

Proposed changes

This PR is mainly to help the printing team. Right now, they get a lot of emails from people who have attended a 3D printing course, but forgot to bring their card to the course. When this happens, people have to send their EM number to the printing team, and the printing team need to add it to the model Printer3DCourse manually. When this PR is merged, people can add their EM number themselves via this page. The page also lets users update their card information, and request access to the printing room if they have lost it. When users add or alter their EM number via this page, it is added to the User model, but if the EM number is changed for an instance in the User model, and the same user is added to or already exists in the Printer3DCourse model, the EM number is updated there as well.

Areas to review closely

Deployment notes

The title of this page comes from a contentbox. After deployment, the contentbox on the page has to be edited, to give the page a title and to give the user information about how to get access to the 3D-printing room.
The FAQ page (https://makentnu.no/en/faq/) and the 3D printing room paragraph on the About page (https://makentnu.no/en/makerspace/) should also be updated to include links to the new 3D printing room access page.

Checklist

(If any of the points are not relevant, mark them as checked)

• Remembered to run the makemigrations, makemessages and compilemessages management commands and committed any changes that should be included in this PR
• Created tests that fail without the changes, if relevant/possible
• Manually tested that the website UI works as intended with different device layouts
  • (Most common is to test with typical screen sizes for mobile (320-425 px), tablet (768 px) and desktop (1024+ px), which can easily be done with your browser's dev tools)
• Manually tested that everything works as intended when logged in as different users locally
  • (This can be e.g. anonymous users (i.e. not being logged in), "normal" non-member users, members of different committees, and superusers)
• Made sure that your code conforms to the code style guides
  • (It's not intended that you read through this whole document, but that you get yourself an overview over its contents, and that you use it as a reference guide / checklist while taking a second look at your code before opening a pull request)
• Attempted to minimize the number of common code smells
  • (See the comment for the previous checkbox)
• Added sufficient documentation - e.g. as comments, docstrings or in the README, if suitable
• Added your changes to the "Unreleased" section of the changelog - mainly the changes that are of particular interest to users and/or developers, if any
• Added a "Deployment notes" section above, if anything out of the ordinary should be done when deploying these changes to the server
• Structured your commits reasonably
  • (Some tips on this can be found in "Write Better Commits, Build Better Projects" and in the code style guides)
  • (Restructuring commits after they've already been committed, can be done using e.g. interactive rebase; see the Git docs on rewriting history)

[codecov]

Codecov Report

Merging #699 (a824720) into dev (6f55b7d) will decrease coverage by 0.20%.
The diff coverage is 59.52%.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #699      +/-   ##
==========================================
- Coverage   88.16%   87.97%   -0.20%     
==========================================
  Files         152      152              
  Lines        6187     6228      +41     
==========================================
+ Hits         5455     5479      +24     
- Misses        732      749      +17     

Files	Coverage Δ
src/makerspace/forms.py	100.00% <100.00%> (ø)
src/makerspace/urls.py	100.00% <ø> (ø)
src/makerspace/views.py	78.20% <48.48%> (-21.80%)	⬇️

[ddabble]

I personally don't really have any context for why this PR was made - which I would need to be able to review e.g. whether the view code does what it should do - so could you update the description with the motivation behind this PR, like what problem it's trying to solve and why? 🙂

[Gunvor4]

I personally don't really have any context for why this PR was made - which I would need to be able to review e.g. whether the view code does what it should do - so could you update the description with the motivation behind this PR, like what problem it's trying to solve and why? 🙂

Sure. This PR is mainly to help the printing team. Right now, they get a lot of emails from people who have attended a 3D printing course, but forgot to bring their card to the course. When this happens, people have to send their EM number to the printing team, and the printing team need to add it to the model Printer3DCourse manually. When this PR is merged, people can add their EM number themselves via this page. The page also lets users update their card information, and request access to the printing room if they have lost it. When users add or alter their EM number via this page, it is added to the User model, but if the EM number is changed for an instance in the User model, and the same user is added to or already exists in the Printer3DCourse model, the EM number is updated is updated there as well.

[ddabble]

Alright 👌 Feel free to update the PR description (i.e. the "comment" at the top of this page, with "Proposed changes" etc.) with the same info - which could potentially be useful for future reference, so that people don't have to scroll down here to find that info 😊 (The same goes for screenshots :))

but if the EM number is changed for an instance in the User model, and the same user is added to or already exists in the Printer3DCourse model, the EM number is updated is updated there as well

Btw, I can't seem to find what part of the added code that does this.. Could you give me a pointer? :)

[ddabble]

This looks useful! 😊 Before reviewing the code more detailed, I'd like to address some (functional) design issues:

1. First off, I think we should hide the card registration page from people who have not attended a 3D printing course, as I don't think it's a good idea to potentially give people the impression that that form might be the starting point for gaining access, as opposed to through a course event.

   Also, letting users who have not taken a 3D printing course register their card number, may cause trouble/confusion when members of the printer team register the card number for that user after it's already been set - at least as long as the course registration form works the way it currently does..

2. I don't think many people will find the page when it's under "Makerverkstedet" in the header, for a couple reasons:

   1. Naively implementing the above point 1 would make the added "Printing room access" button only visible to people after they've attended a course. However, adding UI elements to a sub-menu after users have had the chance to get familiar with the UI, is generally a bad idea, as they would have no reason to reexplore a sub-menu that they've already categorized in their heads as being pretty much entirely unrelated to 3D printing - as the "Makerverkstedet" sub-menu currently is.
   2. I think people looking for this functionality (i.e. "I want access to the 3D printing room") will either look for things related to 3D printing - which in practice is only on the "Reservations" (machine list) page, especially for users who are experienced with that page - or they will look for things related to "me" - which would lead them to the user dropdown and then probably "Profile". (I think "Reservations" is most likely, though.)

   So, I think a better solution is to instead add the form - or a link to it - on both the "Reservations" page and the "Profile" page; more on that in point 3 below.

   • New users who are unfamiliar with the website are usually much more incentivized to explore it, and are therefore likely to find the access form through either the "Reservations" page, the profile page, the FAQ page or the "About" (Makerverkstedet) page (see the suggestion at the bottom of this review comment). And there is also definitely a point to be made that we should limit the number of buttons in the main navigation menus that are overly specific - which I feel this access form kind of is. In other words, elements that are only relevant in certain situations - like losing access to the 3D printing room - should not be in the main navigation menu.

3. The added functionality does indeed pertain to the physical makerspace, but I don't think it belongs in the makerspace app (it's absolutely fair to criticize the naming of that app - as well as make_queue 😅), which is indicated by the fact that the form and view code doesn't reference anything in that app, but instead references code in the make_queue and users apps. Instead, I think the form should be split in two, and:

   1. The code related to the card number can be moved to users, and the card number field moved to a small form on the profile page - which I think is a more natural placement for it. This allows users to check and update their card number at any time (not only when re-requesting access), and helps establish the profile page as the "hub" for the info we have linked to each user (like the already present course completion mini table, and a list of available reservation quotas - that I've written code for locally 🤠).

      • (This might add an extra reason for us to potentially consider changing the name of the "Profile" button, but that's for another time :))
      • When a user updates their card number, their course registration should automatically change status to a new status option (e.g. named REREQUEST; see the point below), so that the printer team members know that they should re-send the user's card number to NTNU.

   2. The code related to requesting access can be moved to make_queue, and the form can just consist of the radio buttons and a submit button; on success, the form can change the user's course registration status to a new option named e.g. REREQUEST. This separate option will help the printer team members understand what actually happened, in contrast to setting the status to REGISTERED as the form currently does - which has the potential to cause confusion.

      Lastly, the access form can probably stay on its own page that's linked to from the profile page, and a new button can be added at the top of the "Reservations" / machine list page (above the two blue buttons) linking to the profile page, with the label "Info linked to you"/"Info knyttet til deg" or something similar. I think the placement and label of this new button will make most/many of the users already familiar with the website curious enough to click it, which will then introduce them to the card number field and the link/button to the access form.

4. I don't think it's necessary to add a content box for the access form page, as there's not a lot of text necessary to explain its usage, and because I think it's unlikely to need updating as readily as the other content boxes do.

5. Instead of the ?success=true query parameter, use Django's messages framework (see how Printer3DCourseCreateView and its printer_3d_course_create.html template do it, or MemberStatusUpdateView and the member_list.html template). This avoids adding unnecessary elements to the URL, and will likely make the code slightly less complex and less dependent on JavaScript.

---

Lastly, a suggestion to add to the "Deployment notes" section of the PR description:

After deployment, one or more entries should be added to the FAQ page that explain how a user (who has taken a 3D printing course) can see/update their card number and re-request access to the 3D printing room, as well as providing links to the access form and the card number form on the profile page. The 3D printing room paragraph on the "About" page under "Makerverkstedet" can also be updated to link to the aforementioned form pages.

[Gunvor4]

Alright 👌 Feel free to update the PR description (i.e. the "comment" at the top of this page, with "Proposed changes" etc.) with the same info - which could potentially be useful for future reference, so that people don't have to scroll down here to find that info 😊 (The same goes for screenshots :))

but if the EM number is changed for an instance in the User model, and the same user is added to or already exists in the Printer3DCourse model, the EM number is updated is updated there as well

Btw, I can't seem to find what part of the added code that does this.. Could you give me a pointer? :)

Sure, I've now updated the PR description, and moved the picture there.
About the code that updates the EM number on the Printer3DCourse model when it is changed in the User model, and vice versa, this functionality was already there. What this PR does is merely to give users a possibility to change their EM number themselves, instead of mailing it to the 3D printing team

[Gunvor4]

This looks useful! 😊 Before reviewing the code more detailed, I'd like to address some (functional) design issues:

1. First off, I think we should hide the card registration page from people who have not attended a 3D printing course, as I don't think it's a good idea to potentially give people the impression that that form might be the starting point for gaining access, as opposed to through a course event.

This is why I 1. want people to log in before they can see this page, 2. only let people add their EM number to their instance in the User model (which means that they can not add themselves to the Printer3DCourse model. They still have to take the 3D printing course and be added to that model by the 3D printing team), and 3. included a content box, so we can give them information about how this works, and also link to 3D printing courses on the event page. That being said - I agree with your comment further down that it might make sense to move this page from the main header. The way it is now - people might stumble upon it, and mistakenly think that they can get access to the printing room without taking a course.

Also, letting users who have not taken a 3D printing course register their card number, may cause trouble/confusion when members of the printer team register the card number for that user after it's already been set - at least as long as the course registration form works the way it currently does.

When this page is up and running, the printing team doesn't have to collect EM numbers from course participants anymore. They can just refer everyone who attends a course to this page. If the user has already added his or her EM number via this page when someone from the 3D printing team adds them to the Printer3DCourse model, the user's EM number will show up once the user has been added. And if the user is added to the Printer3DCourse model before he or she adds his/her EM number, this field will be empty until they add it. And only users who have taken the printing course and added their EM number will be sent to "NTNU Vakt og Service", so the same way as it currently works. The only change is that the users can add their own EM numbers, and that someone from the printing team doesn't have to do it for them.

2. I don't think many people will find the page when it's under "Makerverkstedet" in the header, for a couple reasons:

   1. Naively implementing the above point 1 would make the added "Printing room access" button only visible to people after they've attended a course. However, adding UI elements to a sub-menu after users have had the chance to get familiar with the UI, is generally a bad idea, as they would have no reason to reexplore a sub-menu that they've already categorized in their heads as being pretty much entirely unrelated to 3D printing - as the "Makerverkstedet" sub-menu currently is.

Sure, I agree with this.

2. I think people looking for this functionality (i.e. "I want access to the 3D printing room") will either look for things related to 3D printing - which in practice is only on the "Reservations" (machine list) page, especially for users who are experienced with that page - or they will look for things related to "me" - which would lead them to the user dropdown and then probably "Profile". (I think "Reservations" is most likely, though.)
   So, I think a better solution is to instead add the form - or a link to it - on both the "Reservations" page and the "Profile" page; more on that in point 3 below.

Sure, I can remove it from the header, and add it to the "reservations" page and the "profile" page instead

• New users who are unfamiliar with the website are usually much more incentivized to explore it, and are therefore likely to find the access form through either the "Reservations" page, the profile page, the FAQ page or the "About" (Makerverkstedet) page (see the suggestion at the bottom of this review comment). And there is also definitely a point to be made that we should limit the number of buttons in the main navigation menus that are overly specific - which I feel this access form kind of is. In other words, elements that are only relevant in certain situations - like losing access to the 3D printing room - should not be in the main navigation menu.

Yup, I agree. I've also added the point about including links on the "About" page under "Makerverkstedet", and the "FAQ" page to "Deployment notes" in the PR description.

3. The added functionality does indeed pertain to the physical makerspace, but I don't think it belongs in the makerspace app (it's absolutely fair to criticize the naming of that app - as well as make_queue 😅), which is indicated by the fact that the form and view code doesn't reference anything in that app, but instead references code in the make_queue and users apps. Instead, I think the form should be split in two, and:

   1. The code related to the card number can be moved to users, and the card number field moved to a small form on the profile page - which I think is a more natural placement for it. This allows users to check and update their card number at any time (not only when re-requesting access), and helps establish the profile page as the "hub" for the info we have linked to each user (like the already present course completion mini table, and a list of available reservation quotas - that I've written code for locally 🤠).

      • (This might add an extra reason for us to potentially consider changing the name of the "Profile" button, but that's for another time :))

I agree - the app names make_queue and makerspace are a bit confusing. I don't mind moving the code to another app, though. If we can agree that the profile page is a more logical starting point for getting access to the printing room, couldn't I simply move everything to the users app?

  * When a user updates their card number, their course registration should automatically change status to a new status option (e.g. named `REREQUEST`; see the point below), so that the printer team members know that they should re-send the user's card number to NTNU.

I considered adding a new status option, and I also asked the 3D printing team about this. They did not feel it was necessary, since they already have a routine of sending all users with an EM number having the status REGISTERED to "NTNU Vakt og Service". So a new status option, according to them, will be redundant.

2. The code related to requesting access can be moved to make_queue, and the form can just consist of the radio buttons and a submit button; on success, the form can change the user's course registration status to a new option named e.g. REREQUEST. This separate option will help the printer team members understand what actually happened, in contrast to setting the status to REGISTERED as the form currently does - which has the potential to cause confusion.

Again, I talked to the printing team about this, and with their current routine for sending users to "NTNU Vakt og Service", they felt it would be more confusing with a new status option. Also, I'm a bit confused to where you want me to move my code. Is the make_queue app or the user app the better place for it?

  Lastly, the access form can probably stay on its own page that's linked to from the profile page, and a new button can be added at the top of the "Reservations" / machine list page (above the two blue buttons) linking to the profile page, with the label "Info linked to you"/"Info knyttet til deg" or something similar. I think the placement and label of this new button will make most/many of the users already familiar with the website curious enough to click it, which will then introduce them to the card number field and the link/button to the access form.

Sounds good, I'll change this.

4. I don't think it's necessary to add a content box for the access form page, as there's not a lot of text necessary to explain its usage, and because I think it's unlikely to need updating as readily as the other content boxes do.

Some members of the printing team wanted this, which is why I added it. I do agree, however, that the info on this page probably don't need to be updated very often. Still - I felt like simply including it in the template would make it look a bit cluttered, which is why I added a separate content box.

5. Instead of the ?success=true query parameter, use Django's messages framework (see how Printer3DCourseCreateView and its printer_3d_course_create.html template do it, or MemberStatusUpdateView and the member_list.html template). This avoids adding unnecessary elements to the URL, and will likely make the code slightly less complex and less dependent on JavaScript.

Ok, I haven't used this before, but I can look into it. I'll ask if I need any help ;-)

Lastly, a suggestion to add to the "Deployment notes" section of the PR description:

After deployment, one or more entries should be added to the FAQ page that explain how a user (who has taken a 3D printing course) can see/update their card number and re-request access to the 3D printing room, as well as providing links to the access form and the card number form on the profile page. The 3D printing room paragraph on the "About" page under "Makerverkstedet" can also be updated to link to the aforementioned form pages.

Done!