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

1034 replace staffaccount #1039

Merged

kasugaijin merged 31 commits into main from 1034-replace-staffaccount

Oct 20, 2024

Collaborator

mononoken commented Oct 7, 2024 •

edited

Loading

🔗 Issue

✍️ Description

This finishes up the Person refactorings from the conference, and it finishes the removal of the "accounts" objects. It changes all logic that was using StaffAccounts to use the authorization layer instead (which uses roles, permissions, and ActionPolicy).

I did a few refactorings to simplify logic while removing the account logic, but otherwise most behavior should be the same.

I did change:

On the adoptable pets show page, for staff, instead of not showing anything at all in the adopt button section, I added a link to go to the staff pet page for that pet. Simple QOL change. See screenshot below.
I made Person required for User now. I think all the User records should already have Person IDs so hopefully no problems will occur.

This may cause some conflicts with other open PRs like @MooseCowBear's #997, so we may want to hold off merging this until the former PRs get merged.

📷 Screenshots/Demos

Embedded link for staff on adoptable pet page to get to the pet's staff page

mononoken requested review from jmilljr24, MooseCowBear and kasugaijin

October 7, 2024 01:04

coalest reviewed

View reviewed changes

app/policies/application_policy.rb Outdated Show resolved Hide resolved

app/controllers/organizations/staff/staff_controller.rb Outdated Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

app/controllers/organizations/adopter_fosterer/adopter_applications_controller.rb Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

app/controllers/organizations/staff/staff_controller.rb Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

config/routes.rb Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

app/models/staff_account.rb Outdated Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

app/models/user.rb Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

app/models/user.rb Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

app/views/organizations/adoptable_pets/show.html.erb Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

app/views/organizations/staff/staff/update_activation.turbo_stream.erb Show resolved Hide resolved

kasugaijin reviewed

View reviewed changes

Collaborator

kasugaijin left a comment

Looking good @mononoken just a couple comments!

kasugaijin mentioned this pull request

Clicking pet when viewing adoption applications as adopter fosterer leads to unauthorized error #1056

Open

mononoken force-pushed the 1034-replace-staffaccount branch 3 times, most recently from 39b3ec0 to 5063aa0 Compare

October 13, 2024 04:49

Collaborator Author

mononoken commented Oct 13, 2024

Thanks for the comments, y'all! I just worked through all the conflicts from the rebase and will get to your comments hopefully tomorrow.

mononoken added 12 commits

October 19, 2024 17:05


          Remove staff account relation with organization

10899f8


          Copy deactivated_at logic from StaffAccount to User

8dc6ac2


          Create UserPolicy in lieu of StaffAccountPolicy

6b3c1f7


          Refactor verify_active_staff to remove staff_account logic

bff2b13


          Create UserPolicy test

ca2dedd


          Refactor the staff index route action logic


          Remove unused routes deactivation routes

ad1df91


          Change http request type from post to put for accuracy

3f00023


          Remove useless staff_account conditional

97af368


          Refactor deactivation logic

42ba895


          Refactor staff_account conditionals

d456d65


          Refactor organization_staff class method for role logic

6e635f2

mononoken added 19 commits

October 19, 2024 17:05


          Remove staff_account logic from service

30c6c4c


          Add organization to role assignment in service

81bd008


          Remove more staff_account references

15ca1bb


          Drop staff_accounts

d30cb77


          Refactor the conditionals on the adoptable pets page

fcc157c


          Fix localization ref

6bdadd0


          Remove optional Person from User

7685d1d


          Fail loudly on activation changes

f3cf17d


          Fix this test implementation to actually test OrganizationPolicy

1484d50


          Fix incorrect usage of tenancy in test

eccba8b


          Remove unused instance variables

06026a2


          Remove unnecessary organization method as AdopterApp has org association

365e8a5


          Define organization for verify_active_staff check

0a68b7f


          Move staff? check to user model

5c4c1e3


          Move role method to authorizable concern

5b7c412


          Add flash message for updating activation

9758b38


          Remove old references to staff_account

b86774b


          Loudly fail activation updates

35d0728


          Replace staff_account activation logic

f9488c3

mononoken force-pushed the 1034-replace-staffaccount branch from 81c23a7 to f9488c3 Compare

October 20, 2024 01:26

mononoken requested review from kasugaijin and coalest

October 20, 2024 01:30

Collaborator Author

mononoken commented Oct 20, 2024

Think I got all the comments addressed and resolved new merge conflicts. Lmk if I missed anything.

kasugaijin approved these changes

View reviewed changes

Collaborator

kasugaijin left a comment

Thanks @mononoken ! lgmt.

kasugaijin merged commit 6966f22 into main

5 checks passed

kasugaijin deleted the 1034-replace-staffaccount branch

October 20, 2024 20:27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet