-
Notifications
You must be signed in to change notification settings - Fork 24
JM: Capture sector "Other" free-text as records; curate (promote/keep/dismiss) #1939
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,96 @@ | ||||||
| class OtherResponsesController < ApplicationController | ||||||
| before_action :set_other_response, only: :update | ||||||
|
|
||||||
| # Review page: the same free-text "Other" sector value typed across many | ||||||
| # people, grouped with a count so a curator can decide what to promote. | ||||||
| def index | ||||||
| authorize! | ||||||
| @status_filter = params[:status].presence_in(OtherResponse::VISIBLE_STATUSES) | ||||||
| statuses = @status_filter ? [ @status_filter ] : OtherResponse::VISIBLE_STATUSES | ||||||
| responses = OtherResponse | ||||||
| .sectors.where(status: statuses) | ||||||
| .includes(:person) | ||||||
|
|
||||||
| @groups = responses.group_by(&:normalized_text).map do |_normalized, rows| | ||||||
| { | ||||||
| display_text: rows.first.text, | ||||||
| normalized_text: rows.first.normalized_text, | ||||||
| count: rows.size, | ||||||
| status_counts: rows.each_with_object(Hash.new(0)) { |r, h| h[r.status] += 1 } | ||||||
| } | ||||||
| end.sort_by { |group| [ -group[:count], group[:display_text].downcase ] } | ||||||
|
|
||||||
| @sectors = Sector.excluding_other.order(:name) | ||||||
| end | ||||||
|
|
||||||
| # 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? | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| status = params[:status] | ||||||
| unless %w[kept dismissed].include?(status) | ||||||
| return redirect_to other_responses_path, alert: "Choose keep or dismiss." | ||||||
| end | ||||||
|
|
||||||
| scope = OtherResponse.sectors.where(status: OtherResponse::VISIBLE_STATUSES) | ||||||
| .where(normalized_text: OtherResponse.normalize(params[:normalized_text])) | ||||||
| count = scope.count | ||||||
| scope.find_each { |response| response.update!(status: status) } | ||||||
|
|
||||||
| verb = status == "kept" ? "Kept" : "Dismissed" | ||||||
| redirect_to other_responses_path(status: params[:return_status].presence), | ||||||
| status: :see_other, notice: "#{verb} #{count} response(s)." | ||||||
| end | ||||||
|
|
||||||
| # Curate a single response β the profile-edit "Γ" dismisses, and the review | ||||||
| # page can keep an individual person's response. | ||||||
| def update | ||||||
| authorize! @other_response | ||||||
| status = params.dig(:other_response, :status) | ||||||
| @other_response.update!(status: status) if OtherResponse::STATUSES.include?(status) | ||||||
|
|
||||||
| if params[:return_to] == "person_edit" | ||||||
| redirect_to edit_person_path(@other_response.person), status: :see_other | ||||||
| else | ||||||
| redirect_to other_responses_path, status: :see_other | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| # Promote every non-dismissed person who typed this value into a real Sector | ||||||
| # tag β mapping to an existing sector or minting a new (published) one β and | ||||||
| # mark those responses promoted so they stop showing as free-text chips. | ||||||
| def promote | ||||||
| authorize! to: :promote? | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| 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 | ||||||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π€ From Claude: |
||||||
| .where(normalized_text: OtherResponse.normalize(params[:normalized_text])) | ||||||
|
|
||||||
| responses.includes(:person).find_each do |response| | ||||||
| response.person.tag_sectors(primary_ids: [], additional_ids: [ sector.id ]) | ||||||
| response.update!(status: "promoted", promotable: sector) | ||||||
| end | ||||||
|
|
||||||
| redirect_to other_responses_path, status: :see_other, | ||||||
| notice: "Promoted #{responses.size} response(s) to β#{sector.name}β." | ||||||
| end | ||||||
|
|
||||||
| private | ||||||
|
|
||||||
| def set_other_response | ||||||
| @other_response = OtherResponse.find(params[:id]) | ||||||
| end | ||||||
|
|
||||||
| # The promote target: an existing sector by id, or a newly minted published one | ||||||
| # from a typed name. Returns nil when neither was supplied. | ||||||
| def target_sector | ||||||
| if params[:sector_id].present? | ||||||
| Sector.find_by(id: params[:sector_id]) | ||||||
| elsif params[:new_sector_name].present? | ||||||
| Sector.find_or_create_by!(name: params[:new_sector_name].strip) do |sector| | ||||||
| sector.published = true | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| class OtherResponse < ApplicationRecord | ||
| # The free-text "Other" a person typed on a tag-backed form question. Only | ||
| # sectors are wired up today; the `kind` column leaves room for the identical | ||
| # workshop-setting responses to move here later. | ||
| KINDS = %w[sector].freeze | ||
|
|
||
| # A response starts life as `pending` (awaiting a curator's decision) and is | ||
| # then either promoted into a real tag, kept as a free-text chip, or dismissed | ||
| # (hidden from the person's profile and edit form). | ||
| STATUSES = %w[pending kept promoted dismissed].freeze | ||
|
|
||
| # Statuses that still surface as an "(other)" chip on the person's pages. | ||
| VISIBLE_STATUSES = %w[pending kept].freeze | ||
|
|
||
| belongs_to :person | ||
| belongs_to :promotable, polymorphic: true, optional: true | ||
| belongs_to :source_form_answer, class_name: "FormAnswer", optional: true | ||
|
|
||
| before_validation :set_normalized_text | ||
|
|
||
| validates :text, presence: true | ||
| validates :kind, inclusion: { in: KINDS } | ||
| validates :status, inclusion: { in: STATUSES } | ||
| validates :normalized_text, uniqueness: { scope: [ :person_id, :kind ] } | ||
|
|
||
| scope :sectors, -> { where(kind: "sector") } | ||
| scope :visible, -> { where(status: VISIBLE_STATUSES) } | ||
| scope :pending, -> { where(status: "pending") } | ||
| scope :promotable_now, -> { where.not(status: "dismissed") } | ||
|
|
||
| # Case/whitespace-insensitive key used both for the unique index and for | ||
| # grouping the same typed value across many people on the review page. | ||
| def self.normalize(value) | ||
| value.to_s.strip.downcase | ||
| end | ||
|
|
||
| def dismiss! | ||
| update!(status: "dismissed") | ||
| end | ||
|
|
||
| def keep! | ||
| update!(status: "kept") | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def set_normalized_text | ||
| self.normalized_text = self.class.normalize(text) | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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?. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 promote? | ||
| admin? | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -422,6 +422,7 @@ def invoice_requested? | |
| def create_form_submission(person) | ||
| submission = FormSubmission.create!(person: person, form: @form, event: @event) | ||
| save_form_answers(submission) | ||
| capture_other_sector_responses(submission) | ||
| submission | ||
| end | ||
|
|
||
|
|
@@ -430,9 +431,30 @@ def update_form_submission(person) | |
| record.event = @event | ||
| end | ||
| save_form_answers(submission) | ||
| capture_other_sector_responses(submission) | ||
| submission | ||
| end | ||
|
|
||
| # Materialize the free-text "Other" sector answers as OtherResponse records so | ||
| # 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) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π€ From Claude: Capture reuses |
||
| submission.form_answers | ||
| .joins(:form_field) | ||
| .where(form_fields: { field_identifier: FormField::SECTOR_FIELD_IDENTIFIERS }) | ||
| .each do |answer| | ||
| OtherOption.texts(answer.submitted_answer).each do |text| | ||
| submission.person.other_responses.find_or_create_by!( | ||
| kind: "sector", normalized_text: OtherResponse.normalize(text) | ||
| ) do |response| | ||
| response.text = text | ||
| response.source_form_answer = answer | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def save_form_answers(submission) | ||
| @form_params.each do |field_id, raw_value| | ||
| field = @form.form_fields.find_by(id: field_id) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| <% content_for(:page_bg_class, "admin-only bg-blue-100") %> | ||
| <%= link_to sectors_path, class: "inline-flex items-center gap-1.5 text-sm text-gray-500 hover:text-gray-700 px-2 py-1 mb-2" do %> | ||
| <i class="fa-solid fa-arrow-left text-xs"></i> Sectors | ||
| <% end %> | ||
| <div class="<%= DomainTheme.bg_class_for(:sectors) %> border border-gray-200 rounded-xl shadow p-6"> | ||
| <div class="space-y-6"> | ||
| <!-- Header --> | ||
| <div class="flex items-center justify-between mb-6"> | ||
| <div> | ||
| <h1 class="text-2xl font-semibold text-gray-900">Review βOtherβ sectors</h1> | ||
| <p class="text-sm text-gray-600 mt-1"> | ||
| Free-text sectors people typed on registration forms, grouped by value. | ||
| Promote a recurring one into a real sector tag for everyone who typed it. | ||
| </p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- Status filter --> | ||
| <div class="flex items-center gap-2 text-sm"> | ||
| <% [ [ "All", nil ], [ "Pending", "pending" ], [ "Kept", "kept" ] ].each do |label, value| %> | ||
| <% active = @status_filter == value %> | ||
| <%= link_to label, other_responses_path(status: value), | ||
| class: "px-3 py-1 rounded-full #{active ? "bg-gray-800 text-white" : "bg-white text-gray-600 border border-gray-300 hover:bg-gray-100"}" %> | ||
| <% end %> | ||
| </div> | ||
|
|
||
| <div class="rounded-xl bg-white p-6"> | ||
| <% if @groups.any? %> | ||
| <table class="w-full border-collapse border border-gray-200"> | ||
| <thead class="bg-gray-100"> | ||
| <tr> | ||
| <th class="px-4 py-2 text-left text-sm font-semibold text-gray-700">Response</th> | ||
| <th class="px-4 py-2 text-center text-sm font-semibold text-gray-700 w-24">People</th> | ||
| <th class="px-4 py-2 text-left text-sm font-semibold text-gray-700 w-1/2">Promote to a sector</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| <% @groups.each do |group| %> | ||
| <tr class="border-t border-gray-200 align-top"> | ||
| <td class="px-4 py-3 text-sm text-gray-800"> | ||
| <span class="font-medium"><%= group[:display_text] %></span> | ||
| <% if group[:status_counts]["kept"].to_i.positive? %> | ||
| <span class="ml-1 text-xs text-gray-400">(<%= group[:status_counts]["kept"] %> kept)</span> | ||
| <% end %> | ||
| </td> | ||
| <td class="px-4 py-3 text-center text-sm text-gray-700"><%= group[:count] %></td> | ||
| <td class="px-4 py-3"> | ||
| <%= form_with url: promote_other_responses_path, method: :post, | ||
| class: "flex flex-wrap items-center gap-2" do %> | ||
| <%= hidden_field_tag :normalized_text, group[:normalized_text] %> | ||
| <%= select_tag :sector_id, | ||
| options_from_collection_for_select(@sectors, :id, :name), | ||
| include_blank: "Existing sectorβ¦", | ||
| class: "rounded-md border-gray-300 text-sm" %> | ||
| <span class="text-xs text-gray-400">or</span> | ||
| <%= text_field_tag :new_sector_name, group[:display_text], | ||
| placeholder: "New sector name", | ||
| class: "rounded-md border-gray-300 text-sm" %> | ||
| <%= submit_tag "Promote", class: "btn btn-primary-outline text-sm", | ||
| data: { turbo_confirm: "Promote β#{group[:display_text]}β for all #{group[:count]} people?" } %> | ||
| <% end %> | ||
| <div class="flex items-center gap-3 mt-2 text-sm"> | ||
| <%= button_to "Keep all", curate_other_responses_path(normalized_text: group[:normalized_text], status: "kept", return_status: @status_filter), | ||
| class: "text-gray-500 hover:text-gray-700" %> | ||
| <%= button_to "Dismiss all", curate_other_responses_path(normalized_text: group[:normalized_text], status: "dismissed", return_status: @status_filter), | ||
| class: "text-gray-500 hover:text-red-600", | ||
| form: { data: { turbo_confirm: "Hide β#{group[:display_text]}β from all #{group[:count]} profiles?" } } %> | ||
| </div> | ||
| </td> | ||
| </tr> | ||
| <% end %> | ||
| </tbody> | ||
| </table> | ||
| <% else %> | ||
| <p class="text-gray-500 italic">No βOtherβ sector responses to review.</p> | ||
| <% end %> | ||
| </div> | ||
| </div> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| <%# Chips for a person's free-text "Other" sector responses (OtherResponse | ||
| records). Read-only on the profile; on the edit form (dismissable: true) | ||
| each chip carries an Γ that dismisses it (hides it from the profile/edit). | ||
| Renders bare chips β the caller supplies the surrounding flex container. %> | ||
| <% responses ||= [] %> | ||
| <% dismissable = local_assigns.fetch(:dismissable, false) %> | ||
| <% responses.each do |response| %> | ||
| <span class="inline-flex items-center rounded-md border border-gray-300 bg-white text-gray-600 px-3 py-1 text-sm font-medium" | ||
| title="Other response entered during registration"> | ||
| <%= response.text %> | ||
| <span class="ml-1 text-xs text-gray-400">(other)</span> | ||
| <% if dismissable %> | ||
| <%= link_to "Γ", | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π€ From Claude: Dismiss is a |
||
| other_response_path(response, other_response: { status: "dismissed" }, return_to: "person_edit"), | ||
| data: { turbo_method: :patch }, | ||
| class: "ml-2 -mr-1 px-1 leading-none text-gray-400 hover:text-red-600", | ||
| title: "Hide this response from the profile" %> | ||
| <% end %> | ||
| </span> | ||
| <% end %> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| class CreateOtherResponses < ActiveRecord::Migration[8.1] | ||
| # Materializes the free-text "Other" answers people type on tag-backed form | ||
| # questions (sectors today; workshop settings could follow via `kind`). These | ||
| # can't be Sector/Category records, so they were previously derived on the fly | ||
| # from form answers. Capturing them as records lets a curator promote a | ||
| # recurring value into a real tag, keep it as a free-text chip, or dismiss it. | ||
| def up | ||
| return if table_exists?(:other_responses) | ||
|
|
||
| create_table :other_responses do |t| | ||
| t.references :person, null: false, foreign_key: true | ||
| t.string :kind, null: false | ||
| t.string :text, null: false | ||
| t.string :normalized_text, null: false | ||
| t.string :status, null: false, default: "pending" | ||
| t.references :promotable, polymorphic: true, null: true | ||
| t.references :source_form_answer, null: true, foreign_key: { to_table: :form_answers } | ||
| t.timestamps | ||
| end | ||
|
|
||
| add_index :other_responses, [ :person_id, :kind, :normalized_text ], | ||
| unique: true, name: "index_other_responses_on_person_kind_text" | ||
| end | ||
|
|
||
| def down | ||
| drop_table :other_responses, if_exists: true | ||
| end | ||
| end |
There was a problem hiding this comment.
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_STATUSESrows for thisnormalized_text), mirroring howpromotefans out across everyone who typed it.return_statusround-trips the active filter so the queue stays put after the action.