ucb-rit / coldfront

HPC Resource Allocation System
https://coldfront.readthedocs.io
GNU General Public License v3.0
3 stars 3 forks source link

WIP Issue 399 part 2: MOU Generation #549

Closed Hzaakk closed 6 months ago

matthew-li commented 10 months ago

The number of service units requested in a purchase request don't appear in the admin view for reviewing/updating request details--unsure if this was the case before this PR, but it should be fixed.

matthew-li commented 10 months ago

For secure directory requests, the entries on the detail page should not read "Unsigned/Signed Memorandum..." but rather "Unsigned/Signed RUA", where RUA is unabbreviated.

matthew-li commented 10 months ago

I forget if we discussed this--should the unsigned/signed entries on the detail page be entirely hidden until the admin has confirmed details and notified the PI?

I want to avoid the case where the user downloads the file before an admin has updated it, and then the user uploads a signed version of the outdated file.

EDIT: Given that the checklist item says "enable/notify", I'd think it should be hidden.

matthew-li commented 10 months ago

The formatting of the header of the generated RUA looks off--I've sent you a screenshot. I downloaded it via the "Download" button on the detail page, with the same result across Chrome and Safari.

matthew-li commented 10 months ago

I received the following error when clicking on the "Manage" button for "Confirm or edit allowance details, and enable/notify the PI to sign the Memorandum of Understanding." for a secure directory request--are you able to reproduce this?

NoReverseMatch at /allocation/secure-dir-request/1/notify-pi/
Reverse for 'secure-dir-request-detail' with arguments '('',)' not found. 1 pattern(s) tried: ['allocation\\/secure\\-dir\\-request\\-detail\\/(?P<pk>[0-9]+)$']
Request Method: GET
Request URL:    https://scgup-dev.lbl.gov/allocation/secure-dir-request/1/notify-pi/

(This checklist step and the "Confirm signed..." step after it should also read RUA (unabbreviated) instead of MOU.)

Hzaakk commented 10 months ago

I forget if we discussed this--should the unsigned/signed entries on the detail page be entirely hidden until the admin has confirmed details and notified the PI?

I want to avoid the case where the user downloads the file before an admin has updated it, and then the user uploads a signed version of the outdated file.

EDIT: Given that the checklist item says "enable/notify", I'd think it should be hidden.

I made it so that it's hidden until the user is notified yeah

Hzaakk commented 10 months ago

The number of service units requested in a purchase request don't appear in the admin view for reviewing/updating request details--unsure if this was the case before this PR, but it should be fixed.

fixed it. The field was being popped for some reason

Hzaakk commented 10 months ago

I received the following error when clicking on the "Manage" button for "Confirm or edit allowance details, and enable/notify the PI to sign the Memorandum of Understanding." for a secure directory request--are you able to reproduce this?

NoReverseMatch at /allocation/secure-dir-request/1/notify-pi/
Reverse for 'secure-dir-request-detail' with arguments '('',)' not found. 1 pattern(s) tried: ['allocation\\/secure\\-dir\\-request\\-detail\\/(?P<pk>[0-9]+)$']
Request Method:   GET
Request URL:  https://scgup-dev.lbl.gov/allocation/secure-dir-request/1/notify-pi/

(This checklist step and the "Confirm signed..." step after it should also read RUA (unabbreviated) instead of MOU.)

Fixed, was caused due to forgetting to change a variable name from "addition_request" to "secure_dir_request" when refactoring

matthew-li commented 9 months ago

@Hzaakk The name of the new project PI isn't included in the notification email that the MOU is ready because the context variable pi_name should be to_name.

To avoid issues like this, and given that the three ...NotifyPIView classes are nearly-identical, I'd make a mixin or something to reduce redundant code.