Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: permission check for approve Agreement page #2846

Merged
merged 17 commits into from
Sep 27, 2024
Merged

fix: permission check for approve Agreement page #2846

merged 17 commits into from
Sep 27, 2024

Conversation

fpigeonjr
Copy link
Contributor

@fpigeonjr fpigeonjr commented Sep 23, 2024 •
edited
Loading

What changed

  • protect the Approve Agreement page from hackerz
  • adds vscode settings for python tests
  • adds roles for cypress e2e tests

Issue

#2833

How to test

  1. create some change requests
  2. try and view the approve page logged in as admin
  3. should fail
  4. try and view the approve page logged in as division director
  5. will work

Screenshots

Screen.Recording.2024-09-25.at.3.12.13.PM.mov

Definition of Done Checklist

  • OESA: Code refactored for clarity
  • OESA: Dependency rules followed
  • Automated unit tests updated and passed
  • Automated integration tests updated and passed
  • Automated quality tests updated and passed
  • Automated load tests updated and passed
  • Automated a11y tests updated and passed
  • Automated security tests updated and passed
  • 90%+ Code coverage achieved
  • [-] Form validations updated

@fpigeonjr fpigeonjr self-assigned this Sep 23, 2024
@fpigeonjr fpigeonjr added the bug Something isn't working label Sep 23, 2024
@fpigeonjr
Copy link
Contributor Author

fpigeonjr commented Sep 23, 2024 •
edited
Loading

artifacts from collab time on 9/23/24:

SCR-20240923-nbza SCR-20240923-ooua

@fpigeonjr
Copy link
Contributor Author

uh-oh
SCR-20240924-pocu

@fpigeonjr fpigeonjr changed the title wip: permission check feat: permission check for approve Agreement page Sep 25, 2024
@fpigeonjr fpigeonjr marked this pull request as ready for review September 25, 2024 21:21
@fpigeonjr fpigeonjr changed the title feat: permission check for approve Agreement page fix: permission check for approve Agreement page Sep 25, 2024
fpigeonjr
fpigeonjr commented Sep 26, 2024
View reviewed changes
frontend/src/pages/agreements/approve/ApproveAgreement.hooks.js
@@ -169,6 +173,25 @@ const useApproveAgreement = () => {
const budgetLinesToPlannedMessages = useChangeRequestsForBudgetLines(budgetLinesInReview, BLI_STATUS.PLANNED);
const budgetLinesToExecutingMessages = useChangeRequestsForBudgetLines(budgetLinesInReview, BLI_STATUS.EXECUTING);

// NOTE: Permission checks
const is2849Ready = false; // feature flag for 2849 readiness
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 where is the best place for feature-flags?

@fpigeonjr fpigeonjr mentioned this pull request Sep 27, 2024
Merged
10 tasks
@fpigeonjr fpigeonjr merged commit 20ecb16 into main Sep 27, 2024
40 checks passed
@fpigeonjr fpigeonjr deleted the OPS-2833/add_permission_check branch September 27, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rajohnson90 rajohnson90 rajohnson90 approved these changes

@johndeange johndeange johndeange approved these changes

@maiyerlee maiyerlee Awaiting requested review from maiyerlee

@devops-chris devops-chris Awaiting requested review from devops-chris

@Santi-3rd Santi-3rd Awaiting requested review from Santi-3rd

Assignees

@fpigeonjr fpigeonjr

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fpigeonjr @johndeange @rajohnson90