Skip to content

Capture sector "Other" free-text as records; curate (promote/keep/dismiss)#1939

Open
maebeale wants to merge 11 commits into
mainfrom
maebeale/hide-sector-other-from-profile
Open

Capture sector "Other" free-text as records; curate (promote/keep/dismiss)#1939
maebeale wants to merge 11 commits into
mainfrom
maebeale/hide-sector-other-from-profile

Conversation

@maebeale

@maebeale maebeale commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

🤖 suggested review level: 5 Inspect 🔬 new polymorphic model + capture across 3 submission paths + org-type capture + admin curation reachable from each person's chips

Why

Free-text "Other" answers people type on forms were derived on the fly and shown as read-only chips — with nowhere to record a curation decision, and no way to hide one from the public profile. This captures the promotable-track "Other"s as records so admins can promote / keep / dismiss them, from the review queue or from a person's page.

What

  • OtherResponse model — polymorphic owner, because the answer's subject differs by question:
    • sector → owned by the Person; promotable into a Sector, shown on the profile/edit chip.
    • organization_type → owned by the Organization; stored now, not promotable until OrganizationType is a model (its promote button is hidden).
    • generic questions are not captured — that data stays searchable in the form answers. field_identifier is retained so any question can be switched on later.
    • Statuses: pendingpromoted / kept / dismissed.
  • Capture at submission time: person-owned "Other"s via shared OtherResponses::CaptureFromSubmission (registration, scholarship, bulk payment); org-type via PublicRegistration#sync_agency_type (where the org is known). De-duped per owner + question. No backfill.
  • Profile + edit show the sector chips; admins deep-link into the review queue from a chip (anchored, with a back link to the person) to promote/keep/dismiss; edit keeps a one-click × dismiss. Submission view untouched (audit record).
  • Review page (/other_responses): groups by kind/question with a count, filterable All / Pending / Kept. Promote (sectors only) maps a group to an existing sector or mints a new published one; Keep all / Dismiss all for any group (incl. org-type).

Decisions baked in

Tests

Model (kind derivation incl. org-type, polymorphic owner, promotability, anchors, uniqueness), capture service (person/sector), org-type capture in registration, profile/edit chip + dismiss + deep-link, review page (sector/generic/org-type groups, filter, curate by kind, promote incl. new-sector minting, eyebrow return).

Rebased on main

Resolved conflicts with #1872 (age-range editor); coexists with #1875 (org agency_type_other).

Reporting (event background page)

Pulling "Other" out of real sector tags left the background report with no "Other" signal. Restored in two roles, mirroring the profile (sector chips + separate (other) chips):

  • Aggregate "Other" slice in the All sectors chart (how many chose it), pinned last, drilling into those registrants. The Primary chart gets none — "Other" is an additional-sector answer only.
  • Separate "Other sector responses" detail card listing the actual typed values (grouped by normalized text, count each), each drilling into the registrants who wrote it. Kept out of the pie on purpose: they're un-curated free text, would double-count, and make a useless long-tail.

Copilot AI review requested due to automatic review settings July 5, 2026 01:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

# they can be curated (promoted/kept/dismissed). Reuses OtherOption.texts, the
# same extraction used to display them, and de-dupes on the person's normalized
# value so a repeat registration of the same text doesn't create a second row.
def capture_other_sector_responses(submission)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: Capture reuses OtherOption.texts (the same extraction the display used) and find_or_create_by on the person + normalized value, so a repeat registration of the same "Other" text doesn't create a second row or trip the unique index.

sector = target_sector
return redirect_to other_responses_path, alert: "Pick or name a sector to promote to." unless sector

responses = OtherResponse.sectors.promotable_now

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: promotable_now excludes dismissed — so "dismissed wins": promoting a value never re-tags someone who deliberately hid it.

<%= response.text %>
<span class="ml-1 text-xs text-gray-400">(other)</span>
<% if dismissable %>
<%= link_to "×",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: Dismiss is a link_to with turbo_method: :patch, not button_to — this chip lives inside the person edit <form>, and a nested <form> (what button_to emits) is invalid HTML.

@maebeale maebeale force-pushed the maebeale/hide-sector-other-from-profile branch from f22fbe8 to 1b3fb0f Compare July 5, 2026 02:58
Copilot AI review requested due to automatic review settings July 5, 2026 02:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.


# Bulk keep/dismiss every visible person who typed this value, from the review
# queue. Keep leaves it as a free-text chip; dismiss hides it from profiles.
def curate

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: Bulk keep/dismiss acts on the whole grouped value (all VISIBLE_STATUSES rows for this normalized_text), mirroring how promote fans out across everyone who typed it. return_status round-trips the active filter so the queue stays put after the action.

@maebeale maebeale marked this pull request as ready for review July 5, 2026 03:06
@maebeale maebeale changed the title Capture sector "Other" free-text as records; curate (promote/keep/dismiss) JM: Capture sector "Other" free-text as records; curate (promote/keep/dismiss) Jul 5, 2026
@maebeale

maebeale commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

what we have now on profile w form submission data is super brittle. this would allow us to capture all form 'other' answer data and decide if it gets promoted, wiped, or kept as auxiliary data.

@maebeale maebeale marked this pull request as draft July 5, 2026 03:09
# Bulk keep/dismiss every visible person who typed this value, from the review
# queue. Keep leaves it as a free-text chip; dismiss hides it from profiles.
def curate
authorize! to: :update?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

update? is not defined in other_response_policy.rb so it will default to application_policy. While this will work as it hits the admin check, we are explicit in every other policy instead of falling back to application_policy

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: Fixed in 65d48db → 8933c0a. other_response_policy.rb now spells out index?/update?/curate?/promote? explicitly (all admin?), and curate uses plain authorize! so it resolves curate? instead of borrowing update?. Nothing falls back to application_policy anymore.

Comment thread app/controllers/other_responses_controller.rb Outdated
Comment thread app/policies/other_response_policy.rb Outdated
@@ -0,0 +1,8 @@
class OtherResponsePolicy < ApplicationPolicy
# Curating free-text "Other" responses (reviewing, promoting, keeping,
# dismissing) is an admin-only task. index?/update? fall back to manage?.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't make sense. yes is falls back to manage in application policy but that is admin only as well

  def manage?
    admin?
  end

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: Agreed it was inconsistent — a lone promote? while everything else leaned on the inherited manage?. Rewrote the policy to define every rule the controller authorizes explicitly (index?/update?/curate?/promote?, each admin?), matching the other policies, and added non-admin rejection specs for curate and promote.

Copilot AI review requested due to automatic review settings July 8, 2026 01:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@maebeale maebeale marked this pull request as ready for review July 8, 2026 01:21
Copilot AI review requested due to automatic review settings July 8, 2026 01:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 8, 2026 13:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

# must not land in the person's review queue. Organization type is synced
# onto the org (PublicRegistration#sync_agency_type) and will get its own
# promotable path when OrganizationType becomes a model.
EXCLUDED_FIELD_IDENTIFIERS = %w[agency_type].freeze

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: agency_type is answered on the reg form and its answer IS saved into the person's form_answers (that's how #1875 reads it), so a naive "capture all fields" would slurp the org's "Other: …" into the person's review queue. Excluded here; it gets its own org-scoped, promotable path when OrganizationType lands.


# Group key: sector "Other"s share one bucket (kind), every other question is
# keyed by its field so distinct questions never merge.
def group_bucket(response)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: Sector "Other"s from different sector fields (additional_sectors vs the legacy service-area names) bucket together since they all promote into the one Sector catalog; every other question stays keyed by its own field so distinct questions never merge. curate/promote scope the same way (group_scope).

Copilot AI review requested due to automatic review settings July 8, 2026 14:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.


# Capture only the questions whose "Other" is (or will be) promotable — the
# rest stay searchable in the form answers.
def capturable?(field_identifier)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: Only promotable questions are captured — no point storing an "Other" that can never become a tag (it stays searchable in the form answers). field_identifier is still recorded, so enabling a new question later is just adding its kind to PROMOTABLE_KINDS + kind_for.

Copilot AI review requested due to automatic review settings July 8, 2026 14:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

# Materialize the org-type "Other" as an OtherResponse owned by the org, so it
# joins the curation queue alongside sector "Other"s. Not promotable yet (no
# OrganizationType model), but stored now so nothing is lost; de-duped per org.
def capture_organization_type_other(organization, text)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: Org-type "Other" is captured here rather than in CaptureFromSubmission because this is where the organization (the polymorphic owner) is known — the submission only knows the person. Stored now but promotable? == false, so the review queue shows keep/dismiss for it and hides Promote until OrganizationType exists.

Copilot AI review requested due to automatic review settings July 8, 2026 14:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 8, 2026 15:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@maebeale maebeale changed the title JM: Capture sector "Other" free-text as records; curate (promote/keep/dismiss) Capture sector "Other" free-text as records; curate (promote/keep/dismiss) Jul 9, 2026
maebeale and others added 10 commits July 9, 2026 19:43
Free-text "Other" sector values were derived on the fly from form answers and
shown as read-only chips with nowhere to hang a curation decision. Materialize
them as records so a curator can promote a recurring value into a real Sector,
keep it as a free-text chip, or dismiss it (hide from profile/edit).

Data layer only: migration, model, factory, specs. `kind` leaves room for the
identical workshop-setting responses to migrate here later.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Capture free-text "Other" sector answers as OtherResponse records at
  registration time (PublicRegistration), de-duped per person.
- Person#other_sector_responses now reads the visible (pending/kept) records;
  profile + edit render them via a records-based partial. The edit chip gains an
  × that dismisses (hides) a response. Submission view is untouched (audit).
- Admin review page (/other_responses) groups the same value across people with
  a count; promote maps them all to an existing sector or mints a new published
  one and tags every non-dismissed person. Reachable from the Sectors index.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The review page could only promote; add Keep all / Dismiss all bulk actions per
grouped value, and a status filter (All / Pending / Kept) so kept responses stay
visible in the queue but can be filtered out.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Justin <16829344+jmilljr24@users.noreply.github.com>
Per review: define index?/update?/curate?/promote? explicitly (all admin?)
rather than leaning on ApplicationPolicy's inherited manage? rule, matching how
the other policies are written. Controller uses plain authorize! so each action
resolves its own rule. Adds non-admin rejection specs for curate and promote.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Generalize the capture so any question's free-text "Other" is materialized as an
OtherResponse, on all three submission paths (registration, scholarship, bulk
payment) via a shared OtherResponses::CaptureFromSubmission service.

- OtherResponse gains field_identifier (which question) and derives kind:
  "sector" (promotable into a Sector, shown on the profile) or "generic"
  (auxiliary data, admin-review only). Unique per person + field + value.
- Org-owned agency_type "Other" is excluded — it belongs to the organization
  (synced by #1875) and will get its own promotable path when OrganizationType
  becomes a model.
- Review page groups by question (sectors bucketed together, each other question
  on its own), shows Promote only for sector groups; keep/dismiss for all.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reverse the "capture every question" scope: only questions whose "Other" can
become a real tag are stored (sectors today; org-type once OrganizationType is a
model). Every other "Other" stays searchable in the form answers rather than
duplicated here. field_identifier is kept so switching a new question on later is
a one-line change.

Also let admins curate from the person: the profile and edit sector "Other"
chips deep-link into the review queue (anchored to the row, with a back link to
the person), where they can promote/keep/dismiss. Edit keeps its one-click × too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
OtherResponse.owner is now polymorphic: a sector "Other" is owned by the Person
(promotable, shown on their profile), an organization-type "Other" is owned by
the Organization. Org-type is captured in sync_agency_type (where the org is
known), stored now but not promotable — its promote button is hidden until
OrganizationType is a model. The review queue groups by kind and labels the
org-type group; keep/dismiss work for it, promote stays sector-only.

Non-promotable "generic" questions are still not captured (searchable in form
answers). field_identifier is retained so any question can be switched on later.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The promote form wrapped its button onto a second line and pushed Keep all /
Dismiss all to a third. Put all actions in one flex row: the promote controls
stay together (constrained input widths, nowrap), with Keep all / Dismiss all
inline after them.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
"Other" is the free-text catch-all (captured as an OtherResponse), not a
selectable sector — but the person sector picker was the one selectable-sector
list built without excluding_other, so it could be added and saved as the literal
"Other" sector tag.

- People picker now uses excluding_other, like every other selectable list.
- tag_sectors (Person/Org) drops the Other sector id defensively.
- Profile + edit drop a stray legacy "Other" tag from the displayed sectors.
- Migration clears any Person/Organization "Other" taggings already saved.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 9, 2026 23:46
@maebeale maebeale force-pushed the maebeale/hide-sector-other-from-profile branch from 7d6b8d0 to 3645731 Compare July 9, 2026 23:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Pulling "Other" out of real sector tags left the background report with no
"Other" signal at all. Restore it in two roles: a single aggregate "Other"
slice in the All-sectors chart (how many chose it) and a separate detail card
of the actual typed values (what they wrote), each drilling into its
registrants. The primary chart gets no slice — "Other" is additional-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 13, 2026 19:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

<%# Free-text "Other" sectors ride along as one aggregate slice, pinned last (the
actual typed values get their own detail card below). The primary chart has no
such slice — "Other" is an additional-sector answer only. %>
<% sector_data += [ [ "Other", @dashboard.other_sector_response_count ] ] if @dashboard.other_sector_response_count.positive? %>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

🤖 From Claude: "Other" appears in two roles: one aggregate slice here (the count) and a separate detail card below (the typed values). Kept apart on purpose — folding un-curated free-text into the sector pie would double-count and make a count-1 long tail. Primary chart gets no slice since "Other" is additional-only.

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.

3 participants