Conversation
maebeale
force-pushed
the
maebeale/person-data-change-requests
branch
from
September 20, 2026 23:25
51c5056 to
88e0a12
Compare
maebeale
marked this pull request as ready for review
September 21, 2026 02:29
maebeale
force-pushed
the
maebeale/person-data-change-requests
branch
from
September 21, 2026 02:31
88e0a12 to
901891a
Compare
maebeale
force-pushed
the
maebeale/person-data-change-requests
branch
from
September 21, 2026 04:37
882d077 to
f446e5b
Compare
jmilljr24
reviewed
Sep 21, 2026
| # and affiliations are rendered read-only for owners, who can only *request* | ||
| # changes to them. Drop those from a non-admin owner's submission so a crafted | ||
| # request can't slip past. No-op for admins, who edit them directly. | ||
| def reject_owner_locked_changes!(attrs) |
Collaborator
There was a problem hiding this comment.
Can we use action policy scoped params?
Comment on lines
+114
to
+120
| format.turbo_stream do | ||
| render turbo_stream: turbo_stream.replace( | ||
| ActionView::RecordIdentifier.dom_id(@request), | ||
| partial: "profile_change_requests/request", | ||
| locals: { request: @request } | ||
| ) | ||
| end |
Collaborator
There was a problem hiding this comment.
Suggested change
| format.turbo_stream do | |
| render turbo_stream: turbo_stream.replace( | |
| ActionView::RecordIdentifier.dom_id(@request), | |
| partial: "profile_change_requests/request", | |
| locals: { request: @request } | |
| ) | |
| end | |
| format.turbo_stream |
If you create a turbo_stream.erb dom_id is available without calling ActionView....` and its a bit more idiomatic
Collaborator
There was a problem hiding this comment.
Also you will want to add flash.now if you want the same notice that you are using for html response
| # Owner self-service profile editing is staged behind this flag so it can be | ||
| # trialed in staging before PersonPolicy#edit? flips from admin-only to | ||
| # admin-or-owner at profile launch. Default off; set OWNER_PROFILE_EDIT=true. | ||
| def self.owner_editing_enabled? |
Collaborator
There was a problem hiding this comment.
I'm not apposed to using an env but just a thought.
We have !Rails.env.production? for enable? on membership. We could do the same for this. On less thing to do on DO
maebeale
force-pushed
the
maebeale/person-data-change-requests
branch
7 times, most recently
from
September 30, 2026 11:22
1d12886 to
2488053
Compare
Collaborator
Author
|
@jmilljr24 i think i've addressed all of your comments |
Non-admin owners can't edit their primary email, organization name, or affiliation details, but had no way to ask for a correction. Add a reusable "Contact us to request a change" affordance (prefilled through the existing ContactUs → admin notification channel) next to those read-only fields, and stage owner self-service profile editing behind Person.owner_editing_enabled? (OWNER_PROFILE_EDIT) so it can be trialed in staging before the profile-launch policy flip. The controller strips the locked fields from an owner's submission as a server-side backstop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the email-only "request a change" links with a ProfileChangeRequest record so admins can act on requests in-app instead of only over email. Owners submit a per-field request from their profile; submitting still creates the admin + submitter notification email thread, now pointing at the structured record. Admins work a "Changes requested" queue (linked from the admin home and surfaced on each person's edit page): Approve auto-applies where it can (primary email via the existing confirmation flow, organization rename), otherwise Update manually + Mark as resolved, or Decline. Status is pending/resolved/declined with a resolution_method (approved/manual). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round of improvements to the change-request feature: - Notify the requester when a request is resolved or declined, with an optional reviewer note (required-in-spirit on decline) shown to them. - One open request per (person, field): the "Request a change" link re-opens the pending one to edit, and owners can edit their own pending request. - Snapshot the target organization/affiliation so a later profile change can't move the target; org rename now shows a blast-radius confirm. - Apply guards: reject an email already used by another account, and no-op (resolve without re-sending) when the value already matches. - Admin queue is now a lazy turbo-frame with status filtering; Approve / Decline / Mark-as-resolved answer with turbo-streams so the row updates in place instead of a full reload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Aligns with the See Other convention from #2536, picked up in the rebase onto main, so a Turbo form submission advances instead of re-rendering. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Affiliation requests were a single free-text box. Now the requester selects which affiliation (or "a new one"), picks what should change (title/role, dates, organization, remove, add, other), and gives details — so staff get a specific, structured request instead of prose to parse. - affiliation_id target (validated to belong to the person); admin "Update manually" deep-links to that affiliation, and the card shows which one. - One open request per affiliation (not just per field), so two affiliations can be flagged at once; the pending-state link still applies to the single-target email/org fields. - requested_value is now required for every field (the new value / the category). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Affiliation requests now capture the concrete new value per category, so Approve applies them like the email/org requests instead of always needing a manual edit: - Category-driven form (reuses the conditional-fields Stimulus controller): title → text; dates → two date fields; organization → new name; add a new affiliation → org search (remote-select) + title + dates; remove → none; other → details. - Approve applies title/dates/org-rename/remove(end-date+inactivate)/add-new; "Other" stays manual. Org rename keeps the blast-radius confirm. - proposed_* columns store the new value; irrelevant sibling values are cleared before save; per-category presence validations. - Admin card shows the proposed "New value"; conditional-fields now accepts a comma-separated show-when so one field can serve several categories. Also: status chips moved to the left of affiliation rows; "Remove this affiliation" wording; line break in the request intro. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses review feedback: affiliation change requests had no visible pending indicator like email/org. Show a per-affiliation "Change requested" chip on the row that has an open request (linking to edit it), and a note by the request links for a pending add-new/other request that targets no specific affiliation.
- Route the change-request notification noticeable to the admin queue so the comms feed no longer dead-ends (it has no show route). - Drop redundant `to:` from authorize! calls (rule matches the action); add an edit? alias to update? so edit/update infer cleanly. - Use redirect_back_or_to (redirect_back is soft-deprecated). - Answer the row actions with a turbo_stream template (idiomatic dom_id via turbo_stream.replace @request) and show the notice via flash.now.
Owner self-service editing now keys off !Rails.env.production? (like Membership.enabled?) rather than the OWNER_PROFILE_EDIT env var — one less DigitalOcean setting, and it's on in dev/staging/test. Addresses review feedback; drops the env var and its .env.sample entry. Because it's now on in test, people authorization specs assert owners can edit, and the policy spec stubs owner_editing_enabled? false for the disabled cases.
Addresses review feedback ('use action policy scoped params'). Move the person
permit list into PersonPolicy#params_filter and drop it from the controller;
non-admin owners have the admin-only fields (primary email, affiliations)
stripped there instead of the hand-rolled reject_owner_locked_changes!. Matches
the params_filter pattern already used by EventPolicy and MembershipPolicy.
With owner editing on outside production, an owner viewing their own workshop log / variation idea sees the author credit as an edit link (person_edit_button → edit_person_path) rather than a plain profile link. Update the two show specs to assert the edit link.
maebeale
force-pushed
the
maebeale/person-data-change-requests
branch
from
September 30, 2026 17:54
3cd8fa0 to
8bb75a1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


🤖 suggested review level: 5 Inspect 🔬 new model + migrations, admin apply-on-approve logic, notification wiring, lazy queue + turbo-stream actions
Facilitators can't edit their primary email, organization name, or affiliation details — this gives them a way to request those corrections and gives admins an in-app queue to act on, instead of the changes living only in an email.
Owner side
ProfileChangeRequestand the admin + submitter notification email thread.Admin side
/profile_change_requests) — lazy turbo-frame with status filter, linked from the admin home (pending count) and surfaced on each person's edit page. Cards show the target affiliation.ProfileChangeRequests::Apply), Update manually (deep-links to the exact affiliation), Mark as resolved, Decline. Resolve/Decline carry a reviewer note.profile_change_reviewed).Safeguards
requested_valuerequired on every request.Notes for reviewers
OWNER_PROFILE_EDITflag; decoupling that entry point is deferred to the profile-launch work.🤖 Generated with Claude Code