Ops 3012/update change request filtering #3050
Conversation
fpigeonjr
added
the
tech-debt
Santi-3rd
requested review from
fpigeonjr,
rajohnson90,
johndeange and
maiyerlee
There was a problem hiding this comment.
Looks good @Santi-3rd. I see schema changes and would ask to update the OpenApi spec as part of this effort.
There was a problem hiding this comment.
Frank and I also talked and think that maybe the other uses of userDivisionId in the ApproveAgreement.hooks.js should be replaced with userId and the same check that we changed in the 'getInReviewChangeRequests' file. The concern was that if we were checking the ownership of the division is checked in different ways it could lead to unexpected inconsistencies.
| export type Portfolio = { | ||
| id: number; | ||
| name?: string; | ||
| abbreviation: string; | ||
| status?: string; | ||
| cans?: CAN[]; | ||
| division_id: number; | ||
| division?: Division; |
There was a problem hiding this comment.
Division is not optional. With the way the backfill works, if we have a division_id we will have a division. You should take out the question mark because any scenario where there isn't a division is just a data error.
|
Didn't we also discuss removing the unneeded |
|
Frank is right we also want to remove the changeRequestsForUser in the ChangeRequestsList component as well. |
… ChangeRequestsList
…thub.com/HHS/OPRE-OPS into OPS-3012/Update-Change-Request-Filtering
…e-Request-Filtering
| @@ -53,15 +53,15 @@ export function renderChangeValues(keyName, changeTo, oldCan = "", newCan = "") | |||
| /** | |||
| * Get change requests in review from budget lines. | |||
| * @param {BudgetLine[]} budgetLines - The budget lines. | |||
| * @param {number}[ userDivisionId] - The user division ID. | |||
| * @param {number}[ userId] - The user division ID. | |||
There was a problem hiding this comment.
update comment to user Id
|
🎉 This PR is included in version 1.15.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What changed
Change request filtering is now based on the division ID and the deputy division director's ID.
Issue
#3012
How to test
Run end-to-end tests
Definition of Done Checklist