usnistgov / NEMO

NEMO is a laboratory logistics web application. Use it to schedule reservations, control tool access, track maintenance issues, and more.
Other
135 stars 63 forks source link

Staff Assistance Requests #222

Open abuckles-uci opened 4 months ago

abuckles-uci commented 4 months ago

Hello @rptmat57! This is the initial code for "staff assistance requests". It's essentially a duplicate of the buddy request system, but without needing to specify a start, end, or area. It is only a description of the issue the user wants assistance with. There's some more that I would like to add, however I would like to get your opinion if this is a good fit (and general code review).

Thanks! Aaron

Closes #186

abuckles-uci commented 4 months ago

image

rptmat57 commented 4 months ago

Hi and thank you for this PR draft. I like it and I think it would be a great addition to NEMO.

A few things:

  1. Please follow the instructions for installing dev-tools and pre commit hooks: https://github.com/usnistgov/NEMO/wiki/Development-Setup#set-up-pre-commit-hooks and then run it on all the files so your changed files are formatted correctly
  2. The staff assistance feature would need to be enabled explicitly, following the same idea as the customization "adjustment_requests_enabled"
  3. There are a few copy-paste errors from the buddy view, so make sure to double check and also add new css classes for staff assistance.
  4. You can remove the preference "email_new_staff_assistance_request_reply" since in this case the creator is the only user and he will always need to be notified
abuckles-uci commented 4 months ago

Thank you for reviewing! I'll implement these changes

abuckles-uci commented 4 months ago

Hi @rptmat57,

I've implemented the requested changes and finished up on adding a button to resolve requests. I made it so requests can only be resolved by staff and only if there is at least one reply. In addition, users can now only see their own requests, unless they are staff in which they see all.

The UI is pretty rough still, so I was hoping I could get some suggestions:

  1. In the image below you can see that the top request scrolls to the right instead of wrapping. This is the same as the buddy systems. Should this be fixed?
  2. I think it would be useful to filter out resolved requests. Currently I'm showing them with a [Resolved] badge below all unresolved requests.
  3. I'm not totally sure how the notification system works, I haven't looked into it enough. I made the "expiration" 30 days like other requests, but I'm not sure if there's a better number for this.
  4. On the same note as notifications, how does the email situation look? In my opinion it would be the requestor and all staff. In addition, how can I test emails locally? I was hoping I could get some assistance here.

Thanks! Aaron

image

rptmat57 commented 4 months ago

Hi Aaron,

  1. Yes, it should probably be fixed
  2. I would have a separate section with resolved requests, a bit like we have for approved adjustment/area access requests
  3. 30 days is good
  4. Requestor and staff sounds good, you can test emails by going to Detailed administration -> Email logs
uci-oit-or commented 4 months ago

Hi @rptmat57,

I've implemented the collapsible section for resolved requests. I've also added the ability for requests to be reopened by staff (in case of mistake). I fixed the word-wrap for both staff assistance requests and buddy requests. I still haven't tested the emails yet, but will do so when I have some more time.

Something I've noticed is that regular, non-staff users will see a badge if another user creates a request, even though they can't see the request themselves (as intended). Is this something that should be fixed? Would it require modifying how the notification system works?

image

I still need to do more testing, but please let me know if you catch anything in the code. Thanks!

-Aaron

Screenshot of UI: image

abuckles-uci commented 4 months ago

Hi @rptmat57,

I committed your suggested change, so the notifications appear to be working as intended now.

For the emails, I added a new email that sends to all staff when a staff assistance request is created. The replies still work the same (send to the request creator and other repliers). I have a couple of questions:

  1. The replies use the email_send_staff_assistance_request_replies user preference. Do we want new staff assistance requests to use this same preference or always send to all staff?
  2. Do we want an email to the request creator when their request is resolved/reopened?
  3. I also hid the "New Request" button for staff, since this feature is meant for non-staff. Is this how you want it?

Thanks, Aaron Buckles

abuckles-uci commented 4 months ago

Hi @rptmat57,

Have you had a chance to review this PR yet?

Thanks, Aaron Buckles

abuckles-uci commented 4 months ago

Thanks, it looks great! Outside of my inline comments, there are a few general changes I would like to request:

  1. Please merge with 6.0.0.dev (there are new recent changes)
  2. Also, I would prefer the notification (badge) for staff to only go away when the request is resolved
  3. Lastly, when requests are resolved, let display them as a table (similar to adjustment requests, with replies smaller etc.)

Sounds good, thank you for the feedback. I'll work on implementing these sometime next week hopefully.

abuckles-uci commented 3 months ago

Thanks, it looks great! Outside of my inline comments, there are a few general changes I would like to request:

  1. Please merge with 6.0.0.dev (there are new recent changes)
  2. Also, I would prefer the notification (badge) for staff to only go away when the request is resolved
  3. Lastly, when requests are resolved, let display them as a table (similar to adjustment requests, with replies smaller etc.)

Hi @rptmat57, how does this look?

image

abuckles-uci commented 3 months ago

Hi @rptmat57,

I made the requested changes, so this should be ready to be reviewed again.

Thank, Aaron

rptmat57 commented 3 months ago

thanks, I'll try to take a look this week

abuck351 commented 2 months ago

Hi @rptmat57, have you had a chance to review this PR yet? Thanks!

abuck351 commented 1 month ago

Hi @rptmat57

Sorry to keep bumping this PR. Do you have any feedback? Thanks!

abuckles-uci commented 2 weeks ago

Hi @rptmat57,

Are there any updates for getting this pull request merged? Does anything need to be changed?

Thank you, Aaron