ubccr / coldfront

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

Refactor allocation approval. #484

Closed aebruno closed 1 year ago

aebruno commented 1 year ago

This commit refactors the allocation approval logic in the following ways:

  1. Removes the AllocationActivateRequestView and AllocationDenyRequestView. These views are currently handled via GET requests and because they modify the database they really should be POST requests. The logic in these views is also a duplicate of what is already contained in the AllocationDetailView.

  2. Modify AllocationDetailView to handle the various actions of update, approve, auto-approve and deny allocations. These are defined as follows:

    • update = user clicks the "update" button on the allocation detail page with the intent of updating the allocation data.

    • approve = user clicks the "approve" button on the allocation detail page. In addition to updating the allocation data this will set the status to "Active" and set the end date equal to now + ALLOCATION_DEFAULT_ALLOCATION_LENGTH days This also properly fixes #433.

    • auto-approve = user clicks the "approve" button from the allocation request list page. This will set the status to "Active" and set the end date equal to now + ALLOCATION_DEFAULT_ALLOCATION_LENGTH days. It will not update any other data on the allocation.

    • deny = user clicks the "deny" button on the allocation detail page. This will set the status to "Denied" and remove the start/end dates.

  3. On the allocation request list page, the "approve" button now submits a POST request instead of a get request to "auto-approve" the allocation (see above logic). Also the button label has been changed to "Approve" to be consistent with the allocation detail page.

aebruno commented 1 year ago

This broke part of the allocation change request approval process.

Nothing in this PR touched the allocation change request approval process. It appears this was broken before so would rather fix that in a separate PR. Can you confirm you see this behavior in the main branch? If so, we can fix in another PR.

dsajdak commented 1 year ago

I can confirm this behavior is found in the main branch so I'll approve this PR and we can fix separately.