Skip to content

Frontend/staff post login#228

Merged
isabeleliassen merged 30 commits intocsg-org:developmentfrom
InspiringApps:frontend/staff-post-login
Oct 11, 2024
Merged

Frontend/staff post login#228
isabeleliassen merged 30 commits intocsg-org:developmentfrom
InspiringApps:frontend/staff-post-login

Conversation

@jsandoval81
Copy link
Collaborator

@jsandoval81 jsandoval81 commented Oct 3, 2024

Draft Todo

  1. Merge PR Feat/licensee login #216 into development (CSG)
    • This work is based on that branch
  2. Merge latest development into this branch & smoke test (IA)
  3. Publish this PR (IA)

Requirements List

  • None

Description List

  • Updated the App component initial auth handling
  • Added a CompactSelector component to allow staff users with access to multiple compacts to change compact views
  • Updated form input components to be configured with a no-spacing option
  • Updated the PageHeader component to use the new CompactSelector component
  • Updated the StateUpload component to present the state options from the user's permissions
  • Updated the Home page component to simplify its tasks now that the App component handles the initial auth
  • Updated the data layer (store & network area & local mocks) to include the staff user profile fetch
  • Updated the Logout test to comment out a test that is affected by Vue's deep reactivity - we'd like to eventually keep this, but it will require some deeper mocking than we currently have
  • Added a global store property for tracking the current AuthType
  • Updated the User model to make sure its permission compacts are fully transformed

Testing List

  • yarn test:unit:all should run without errors or warnings
  • yarn serve should run without errors or warnings
  • yarn build should run without errors or warnings
  • Code review

Closes #193

@jlkravitz
Copy link
Collaborator

(Feel free to ignore till Monday!)

@jsandoval81 If this is ready to review, mind merging in the latest development so that I can see the changes relative to that branch? Looks like there might be some merge conflicts, too.

@jsandoval81 jsandoval81 marked this pull request as ready for review October 10, 2024 15:20
@jsandoval81
Copy link
Collaborator Author

@jlkravitz This one is all caught up & ready for review.

@jlkravitz
Copy link
Collaborator

jlkravitz commented Oct 10, 2024

  • review the pull request to get oriented
    • read the description of the pull request, which should summarize the changes made
    • read through every task on the Scrum board that's encompassed by this pull request
    • read the description of the commits that comprise the pull request
  • skim all new code, in the context of existing code, looking for problems (knowing that the vast majority of new code will be covered by tests)
  • review all tests
    • methodically review all new tests for correctness, quality of naming
    • look at code coverage of tests
    • determine what code isn’t tested, review that rigorously
  • review documentation to ensure that it matches changes
  • provide comments on the pull request on GitHub, as necessary
    • for comments that are specific to a particular line of code, comment on those specific lines
    • for comments that are more general, attach the comment to a random line in README.md (as opposed to commenting on the pull request itself), to be able to use GitHub's ability to thread discussions on those comments
  • run a security audit of dependencies (e.g. npm audit and pip audit) to ensure that there are no vulnerabilities that will be deployed to production (as opposed to vulnerabilities that only have an impact on the development environment)

Copy link
Collaborator

@jlkravitz jlkravitz left a comment

Choose a reason for hiding this comment

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

looks good! few comments.

@jsandoval81 jsandoval81 requested a review from jlkravitz October 10, 2024 18:16
Copy link
Collaborator

@jlkravitz jlkravitz left a comment

Choose a reason for hiding this comment

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

@isabeleliassen good to go

@isabeleliassen isabeleliassen merged commit d98570b into csg-org:development Oct 11, 2024
@jsandoval81 jsandoval81 deleted the frontend/staff-post-login branch November 20, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

frontend implementation of staff login permissions handling

4 participants