From 3d33f862a2a191d5696b7544453794c4d5c9724b Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 4 Aug 2026 10:10:58 -0400 Subject: [PATCH 01/12] Make the profile the source of truth for author credit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Name display was asked twice with two value sets: once on the person profile (people index) and once per item (every credit), so the same facilitator could render three different ways with no single place to fix it. Point credits at the profile instead. Anonymity stays a per-item latch — a person may want four stories credited and the fifth not, and nothing should be able to de-anonymize an item that was submitted anonymously. Co-Authored-By: Claude --- app/models/community_news.rb | 6 +- app/models/concerns/author_creditable.rb | 139 ++++++++++-------- app/models/person.rb | 31 +++- app/models/story.rb | 7 +- app/models/story_idea.rb | 2 - app/models/workshop_idea.rb | 2 - app/models/workshop_variation_idea.rb | 2 - ...add_author_credit_preferences_to_people.rb | 18 +++ db/schema.rb | 2 + 9 files changed, 133 insertions(+), 76 deletions(-) create mode 100644 db/migrate/20260804140743_add_author_credit_preferences_to_people.rb diff --git a/app/models/community_news.rb b/app/models/community_news.rb index b75a120b8a..3f00786c39 100644 --- a/app/models/community_news.rb +++ b/app/models/community_news.rb @@ -45,9 +45,11 @@ def missing_author_label # SearchCop include SearchCop search_scope :search do - attributes :title, :published, person_first: "people.first_name", person_last: "people.last_name" + attributes :title, :published - scope { join_rich_texts.left_joins(:author) } + # Author names are deliberately not indexed here — see Story. Person-name + # search goes through `by_credited_person_name`, which honors the preference. + scope { join_rich_texts } attributes action_text_body: "action_text_rich_texts.plain_text_body" end diff --git a/app/models/concerns/author_creditable.rb b/app/models/concerns/author_creditable.rb index 1585dddba1..951911feee 100644 --- a/app/models/concerns/author_creditable.rb +++ b/app/models/concerns/author_creditable.rb @@ -1,15 +1,16 @@ module AuthorCreditable extend ActiveSupport::Concern + # How a credit renders comes from the credited person's profile, not from the record. + # `author_credit_preference` is retained as the record of what the submitter consented + # to at submission time, and is human-editable only on the author credit divergences + # page. It no longer drives display — with one exception: a stored "anonymous" is + # always honored, because anonymity is inherently per-item (a person may want four + # stories credited and the fifth not) and because nothing should be able to + # de-anonymize an item that was submitted anonymously. AUTHOR_CREDIT_PREFERENCES = %w[full_name first_name_last_initial first_name_only last_name_only anonymous].freeze - IDEA_FORM_OPTIONS = { - "I would like my full name published with the story" => "full_name", - "I would like my first name and last initial published" => "first_name_last_initial", - "I would like only my first name published" => "first_name_only", - "I would like only my last name published" => "last_name_only", - "I do not want my name published with my story" => "anonymous" - }.freeze + ANONYMOUS = "anonymous" ADMIN_FORM_OPTIONS = { "Full name" => "full_name", @@ -20,11 +21,9 @@ module AuthorCreditable }.freeze included do - # Admin-created records default a blank preference to full_name (in the UI and - # on save) — no data backfill. Public-submission models call - # `require_author_credit_preference` instead, to force a conscious choice. - attribute :author_credit_preference, :string, default: "full_name" - before_validation :apply_default_author_credit_preference + # Snapshot the credited person's profile preference on create, so the column keeps + # recording consent-at-submission without a human ever picking it. + before_create :snapshot_author_credit_preference validates :author_credit_preference, inclusion: { in: AUTHOR_CREDIT_PREFERENCES }, allow_blank: true # Filter to content explicitly authored by a person (belongs_to :author); @@ -57,16 +56,15 @@ def legacy_author_name_text nil end - # Display string for the credited author, honoring the credit preference. - # Precedence: an explicit "anonymous" preference always renders "Anonymous"; - # then the primary author person, then the legacy free-text name, then the + # Display string for the credited author, formatted by that person's profile. + # Precedence: the primary author person, then the legacy free-text name, then the # creating user's person, then `missing_author_label`. def author_credit - return "Anonymous" if author_credit_preference == "anonymous" person = primary_author_person - return format_person_credit(person) if person + return credit_for(person) if person return legacy_author_name_text if legacy_author_name_text.present? - format_person_credit(created_by&.person) + creator = created_by&.person + creator ? credit_for(creator) : missing_author_label end # The person the credit should link to, or nil when the credit must not resolve @@ -75,8 +73,23 @@ def author_credit # never declared authorship (and the record isn't listed on their profile # either). Anonymous never links. def author_credit_person - return nil if author_credit_preference == "anonymous" - primary_author_person + person = primary_author_person + person && !credit_anonymous?(person) ? person : nil + end + + # Anonymity is a one-way latch: the profile can set it, the record can set it, + # and neither can strip it from the other. + def credit_anonymous?(person) + person.contributions_anonymous? || author_credit_preference == ANONYMOUS + end + + # True when the stored consent snapshot no longer agrees with the credited + # person's current profile — surfaced as a warning on the record's form and as a + # row on the author credit divergences page. + def author_credit_diverged? + return false if author_credit_preference.blank? + person = author_person + person.present? && author_credit_preference != person.effective_author_credit_preference end # Shown when there is no credited person or legacy name. Overridable per model @@ -85,44 +98,19 @@ def missing_author_label "Anonymous" end - # Default an unset preference to "full_name" (so legacy rows normalize on save, - # no backfill) — unless the model requires an explicit choice. - def apply_default_author_credit_preference - return if self.class.require_author_credit_preference? - self.author_credit_preference = "full_name" if author_credit_preference.blank? + def snapshot_author_credit_preference + # Promotion services copy the originating idea's snapshot forward — keep it. + return if author_credit_preference.present? + person = primary_author_person || created_by&.person + self.author_credit_preference = person.effective_author_credit_preference if person end - # Formats a person's name per the credit preference, falling back to - # `missing_author_label` when the person or the requested name part is missing. - private def format_person_credit(person) - case author_credit_preference - when "first_name_last_initial" - first = person&.first_name - first.present? ? "#{first} #{person.last_name&.first}." : missing_author_label - when "first_name_only" - person&.first_name.presence || missing_author_label - when "last_name_only" - person&.last_name.presence || missing_author_label - else # full_name — the default, and the fallback for any unknown value - person&.full_name.presence || missing_author_label - end + private def credit_for(person) + return "Anonymous" if credit_anonymous?(person) + person.name.presence || missing_author_label end class_methods do - # Require an explicit credit choice rather than defaulting to full_name — for - # public submissions where the preference is a privacy decision (the submitter - # must not be silently opted into publishing their full name). Used by the - # *_idea models. - def require_author_credit_preference - @require_author_credit_preference = true - attribute :author_credit_preference, :string, default: nil - validates :author_credit_preference, presence: true - end - - def require_author_credit_preference? - @require_author_credit_preference == true - end - # Legacy free-text columns (fully qualified, e.g. "resources.legacy_author_name") # that also hold an author's name. Overridden per model that has one. def legacy_author_name_columns @@ -138,8 +126,8 @@ def by_credited_person_name(query) sanitized = query.to_s.strip.gsub(/\s+/, "") return none if sanitized.blank? - clauses = credited_person_aliases.flat_map { |a| person_name_match_clauses(a) } - clauses += legacy_author_name_columns.map { |col| "LOWER(REPLACE(#{col}, ' ', '')) LIKE :name" } + clauses = credited_person_aliases.map { |a| credited_person_match_sql(a) } + clauses += legacy_author_name_columns.map { |col| legacy_author_name_match_sql(col) } joins(credited_person_join_sql).where(clauses.join(" OR "), name: "%#{sanitized}%") end @@ -189,13 +177,40 @@ def coalesced_author_arel(field, ascending) ascending ? node.asc : node.desc end - def person_name_match_clauses(sql_alias) - [ - "LOWER(REPLACE(CONCAT(#{sql_alias}.first_name, #{sql_alias}.last_name), ' ', '')) LIKE :name", - "LOWER(REPLACE(CONCAT(#{sql_alias}.last_name, #{sql_alias}.first_name), ' ', '')) LIKE :name", - "LOWER(REPLACE(#{sql_alias}.first_name, ' ', '')) LIKE :name", - "LOWER(REPLACE(#{sql_alias}.last_name, ' ', '')) LIKE :name" - ] + # Match only on the name parts the credit actually displays, so search can't + # surface what the credit hides: an anonymous credit matches nothing, a + # "first name only" credit isn't findable by last name, and a "first name, + # last initial" credit matches the initial rather than the whole last name. + def credited_person_match_sql(sql_alias) + first = "#{sql_alias}.first_name" + last = "#{sql_alias}.last_name" + preference = "COALESCE(#{sql_alias}.display_name_preference, 'full_name')" + + by_preference = { + "full_name" => [ "CONCAT(#{first}, #{last})", "CONCAT(#{last}, #{first})", first, last ], + "first_name_last_initial" => [ "CONCAT(#{first}, LEFT(#{last}, 1))", first ], + "first_name_only" => [ first ], + "last_name_only" => [ last ] + }.map do |value, expressions| + "(#{preference} = '#{value}' AND (#{expressions.map { |e| name_like(e) }.join(' OR ')}))" + end + + "(#{sql_alias}.contributions_anonymous = FALSE AND #{not_anonymous_sql} AND (#{by_preference.join(' OR ')}))" + end + + # Legacy free-text author names have no person, so only the record's own + # anonymity applies. + def legacy_author_name_match_sql(column) + "(#{not_anonymous_sql} AND #{name_like(column)})" + end + + def not_anonymous_sql + "(#{table_name}.author_credit_preference IS NULL OR " \ + "#{table_name}.author_credit_preference <> '#{AuthorCreditable::ANONYMOUS}')" + end + + def name_like(expression) + "LOWER(REPLACE(#{expression}, ' ', '')) LIKE :name" end end end diff --git a/app/models/person.rb b/app/models/person.rb index e401863896..ee5eb8e208 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -71,6 +71,20 @@ class Person < ApplicationRecord CONTACT_TYPES = [ "work", "personal" ].freeze validates :email_type, inclusion: { in: %w[work personal] }, allow_blank: true validates :email_2_type, inclusion: { in: %w[work personal] }, allow_blank: true + + # How this person's name is formatted wherever it appears. Anonymity is not one of + # these — it's the separate `contributions_anonymous` flag, because a person still + # has to be listed *somehow* on the people index. + DISPLAY_NAME_PREFERENCES = %w[full_name first_name_last_initial first_name_only last_name_only].freeze + + DISPLAY_NAME_PREFERENCE_LABELS = { + "full_name" => "First and last name", + "first_name_last_initial" => "First name and last initial", + "first_name_only" => "First name only", + "last_name_only" => "Last name only" + }.freeze + + validates :display_name_preference, inclusion: { in: DISPLAY_NAME_PREFERENCES }, allow_blank: true # Mirrors SectorsTaggable's single-primary rule for age ranges — the chip # editor's single-star JS is the first line of defense, this guards imports, # the console, and bad form posts. Person-only: organizations aggregate @@ -206,21 +220,30 @@ def mailing_list_consented=(value) end end + # The single name formatter for this person — drives the people index, the profile + # header, and every author credit on content they've shared. def name case display_name_preference - when "full_name" - full_name when "first_name_last_initial" - "#{first_name} #{last_name.first}" + initial = last_name&.first + initial.present? ? "#{first_name} #{initial}." : first_name.to_s when "first_name_only" first_name when "last_name_only" last_name - else + else # full_name — the default, and the fallback for any unknown value full_name end end + # How this person is credited on content they've shared. Anonymity is a separate + # axis from the name format: it suppresses author credits without affecting how + # they're listed on the people index. See AuthorCreditable. + def effective_author_credit_preference + return "anonymous" if contributions_anonymous? + display_name_preference.presence || "full_name" + end + def full_name "#{first_name} #{last_name}" end diff --git a/app/models/story.rb b/app/models/story.rb index 187717903c..d72d96cc67 100644 --- a/app/models/story.rb +++ b/app/models/story.rb @@ -50,10 +50,13 @@ class Story < ApplicationRecord search_scope :search do attributes all: [ :title, :published ] attributes :title, :published - attributes person_first: "people.first_name", person_last: "people.last_name" options :all, type: :text, default: true, default_operator: :or - scope { join_rich_texts.left_joins(created_by: :person) } + # Author names are deliberately not indexed here. `by_credited_person_name` is + # the only person-name search path, because it honors the credit preference — + # indexing people.first_name/last_name would let a full-text query surface a + # credit that renders "Anonymous". + scope { join_rich_texts } attributes action_text_body: "action_text_rich_texts.plain_text_body" options :action_text_body, type: :text, default: true, default_operator: :or end diff --git a/app/models/story_idea.rb b/app/models/story_idea.rb index 907c34a8f5..1e2a708aea 100644 --- a/app/models/story_idea.rb +++ b/app/models/story_idea.rb @@ -1,7 +1,5 @@ class StoryIdea < ApplicationRecord include AuthorCreditable - # Public submission: the submitter must choose how they're credited. - require_author_credit_preference include SearchCop search_scope :search do attributes :title, :body diff --git a/app/models/workshop_idea.rb b/app/models/workshop_idea.rb index 243e3083b3..a0d794c9ff 100644 --- a/app/models/workshop_idea.rb +++ b/app/models/workshop_idea.rb @@ -1,7 +1,5 @@ class WorkshopIdea < ApplicationRecord include AuthorCreditable - # Public submission: the submitter must choose how they're credited. - require_author_credit_preference belongs_to :created_by, class_name: "User" belongs_to :updated_by, class_name: "User" diff --git a/app/models/workshop_variation_idea.rb b/app/models/workshop_variation_idea.rb index 993b498dcc..9cedd1e280 100644 --- a/app/models/workshop_variation_idea.rb +++ b/app/models/workshop_variation_idea.rb @@ -1,7 +1,5 @@ class WorkshopVariationIdea < ApplicationRecord include AuthorCreditable - # Public submission: the submitter must choose how they're credited. - require_author_credit_preference include SearchCop search_scope :search do attributes :name, :body diff --git a/db/migrate/20260804140743_add_author_credit_preferences_to_people.rb b/db/migrate/20260804140743_add_author_credit_preferences_to_people.rb new file mode 100644 index 0000000000..fff9282b0a --- /dev/null +++ b/db/migrate/20260804140743_add_author_credit_preferences_to_people.rb @@ -0,0 +1,18 @@ +class AddAuthorCreditPreferencesToPeople < ActiveRecord::Migration[8.0] + def up + unless column_exists?(:people, :contributions_anonymous) + add_column :people, :contributions_anonymous, :boolean, default: false, null: false + end + + # Stamped when an admin resolves this person on the author credit divergences + # page, so a deliberate divergence stops reappearing on the worklist. + unless column_exists?(:people, :author_credit_reconciled_at) + add_column :people, :author_credit_reconciled_at, :datetime + end + end + + def down + remove_column :people, :contributions_anonymous, if_exists: true + remove_column :people, :author_credit_reconciled_at, if_exists: true + end +end diff --git a/db/schema.rb b/db/schema.rb index b96f6d27d3..ee1521a41a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1005,9 +1005,11 @@ end create_table "people", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| + t.datetime "author_credit_reconciled_at" t.string "best_time_to_call" t.text "bio" t.boolean "blog_contributor", default: false, null: false + t.boolean "contributions_anonymous", default: false, null: false t.datetime "created_at", null: false t.integer "created_by_id" t.date "date_of_birth" From b283190f34777c6ce5015e56bf17826b94975e59 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 4 Aug 2026 10:36:48 -0400 Subject: [PATCH 02/12] Reconcile stored credit snapshots from one admin page Strip the per-item credit select from all 8 forms and show a warning only where the stored snapshot disagrees with the profile. The stored column stays as the consent record, editable in one place. Search now honors the preference too: an anonymous credit matches nothing, first_name_only isn't findable by last name, and first_name_last_initial matches only the initial. Dropped the person_first/person_last SearchCop attributes that bypassed this. Co-Authored-By: Claude --- AGENTS.md | 7 +- .../author_credit_divergences_controller.rb | 62 +++++++ app/controllers/community_news_controller.rb | 2 +- app/controllers/people_controller.rb | 2 +- app/controllers/resources_controller.rb | 2 +- app/controllers/stories_controller.rb | 2 +- app/controllers/story_ideas_controller.rb | 2 +- app/controllers/workshop_ideas_controller.rb | 3 +- .../workshop_variation_ideas_controller.rb | 3 +- .../workshop_variations_controller.rb | 3 +- app/controllers/workshops_controller.rb | 3 +- app/decorators/resource_decorator.rb | 8 - app/helpers/admin_cards_helper.rb | 1 + .../author_credit_divergences_helper.rb | 15 ++ app/models/workshop.rb | 4 - app/models/workshop_idea.rb | 9 +- .../author_credit_divergence_policy.rb | 13 ++ .../author_credit_divergence_query.rb | 97 +++++++++++ .../_filter_fields.html.erb | 5 + .../_filters.html.erb | 36 ++++ .../_results_skeleton.html.erb | 9 + ...author_credit_divergences_results.html.erb | 112 +++++++++++++ .../author_credit_divergences/index.html.erb | 22 +++ app/views/community_news/_form.html.erb | 8 +- app/views/people/_form.html.erb | 14 +- app/views/resources/_form.html.erb | 10 +- .../shared/_author_credit_preview.html.erb | 14 ++ .../shared/_author_credit_status.html.erb | 7 + .../shared/_author_credit_warning.html.erb | 23 +++ app/views/stories/_form.html.erb | 24 +-- app/views/story_ideas/_form.html.erb | 12 +- app/views/story_ideas/show.html.erb | 5 +- app/views/workshop_ideas/_form.html.erb | 8 +- app/views/workshop_ideas/show.html.erb | 5 +- .../workshop_variation_ideas/_form.html.erb | 8 +- .../workshop_variation_ideas/index.html.erb | 9 +- .../workshop_variation_ideas/show.html.erb | 5 +- app/views/workshop_variations/_form.html.erb | 10 +- app/views/workshops/_form.html.erb | 8 +- config/routes.rb | 6 + db/seeds/dev/people_profiles.rb | 4 + db/seeds/dev/workshops.rb | 2 +- spec/factories/people.rb | 4 + spec/factories/story_ideas.rb | 1 - spec/factories/workshop_ideas.rb | 1 - spec/factories/workshop_variation_ideas.rb | 1 - spec/factories/workshop_variations.rb | 1 - spec/models/community_news_spec.rb | 54 +++--- spec/models/person_spec.rb | 44 ++++- .../author_credit_divergences_spec.rb | 104 ++++++++++++ .../author_credit_divergences_routing_spec.rb | 19 +++ .../author_credit_divergence_query_spec.rb | 93 +++++++++++ .../shared_examples/author_creditable.rb | 156 +++++++++++++----- .../community_news/edit.html.erb_spec.rb | 1 - .../views/community_news/new.html.erb_spec.rb | 1 - spec/views/page_bg_class_alignment_spec.rb | 1 + spec/views/stories/edit.html.erb_spec.rb | 1 - spec/views/stories/new.html.erb_spec.rb | 5 +- spec/views/story_ideas/edit.html.erb_spec.rb | 1 - 59 files changed, 877 insertions(+), 215 deletions(-) create mode 100644 app/controllers/author_credit_divergences_controller.rb create mode 100644 app/helpers/author_credit_divergences_helper.rb create mode 100644 app/policies/author_credit_divergence_policy.rb create mode 100644 app/services/author_credit_divergence_query.rb create mode 100644 app/views/author_credit_divergences/_filter_fields.html.erb create mode 100644 app/views/author_credit_divergences/_filters.html.erb create mode 100644 app/views/author_credit_divergences/_results_skeleton.html.erb create mode 100644 app/views/author_credit_divergences/author_credit_divergences_results.html.erb create mode 100644 app/views/author_credit_divergences/index.html.erb create mode 100644 app/views/shared/_author_credit_preview.html.erb create mode 100644 app/views/shared/_author_credit_status.html.erb create mode 100644 app/views/shared/_author_credit_warning.html.erb create mode 100644 spec/requests/author_credit_divergences_spec.rb create mode 100644 spec/routing/author_credit_divergences_routing_spec.rb create mode 100644 spec/services/author_credit_divergence_query_spec.rb diff --git a/AGENTS.md b/AGENTS.md index f1c3d8d953..3e444aa138 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,9 +60,9 @@ This codebase (Rails 8.1) | `app/controllers/` | Rails controllers (admin/, events/) | ~78 files | | `app/views/` | ERB templates | ~745 files | | `app/decorators/` | Draper decorators for view logic | ~40 files | -| `app/policies/` | ActionPolicy authorization rules | ~55 files | +| `app/policies/` | ActionPolicy authorization rules | ~61 files | | `app/presenters/` | Presentation objects | 6 files | -| `app/helpers/` | View helpers | ~31 files | +| `app/helpers/` | View helpers | ~32 files | | `app/mailers/` | ActionMailer classes | 5 files | | `app/inputs/` | Custom SimpleForm inputs | 1 file | @@ -133,7 +133,7 @@ This codebase (Rails 8.1) |---|---| | `AgeGroupTaggable` | Splits AgeRange category taggings into primary/additional via `categorizable_items.is_primary` (Person, Organization) | | `AhoyTrackable` | Event tracking integration | -| `AuthorCreditable` | Author attribution | +| `AuthorCreditable` | Author attribution. Credits are formatted by the credited **person's profile** (`Person#display_name_preference`), not by the record. The record's `author_credit_preference` is the consent snapshot taken at create time and is human-editable only on the author credit divergences page — it no longer drives display, except `"anonymous"`, which is always honored (anonymity is a one-way latch: profile or record can set it, neither can strip it) | | `Featureable` | `featured`, `publicly_featured` scopes | | `Mentioner` | ActionText @mention extraction and grouping | | `NameFilterable` | Name-based filtering | @@ -208,6 +208,7 @@ action, or `authorize! :workshop, to: :summary?`). - `WorkshopSearchService` — Complex filtering, sorting, pagination with ActionPolicy - `WorkshopFromIdeaService` — Converts WorkshopIdea to Workshop with asset migration - `WorkshopVariationFromIdeaService` — Variation creation from ideas +- `AuthorCreditDivergenceQuery` — Finds content whose stored `author_credit_preference` no longer matches the credited person's profile, grouped by person, for the admin reconciliation page. `MODEL_NAMES` doubles as the allowlist for the `type` param (never constantize a raw param) - `TaggingSearchService` — Search and filter tagging data - `PersonFromUserService` — Create Person from User account - `PersonCommentAggregator` — Unifies every comment connected to a person (their profile, event registrations, scholarships, CE registrations, topic subscriptions, and user account) into one newest-first `Comment` relation for the aggregated `/people/:id/all_comments` page diff --git a/app/controllers/author_credit_divergences_controller.rb b/app/controllers/author_credit_divergences_controller.rb new file mode 100644 index 0000000000..daffcfc93f --- /dev/null +++ b/app/controllers/author_credit_divergences_controller.rb @@ -0,0 +1,62 @@ +class AuthorCreditDivergencesController < ApplicationController + before_action :authorize_page + + FILTER_KEYS = %i[person_id type preference include_reconciled].freeze + + def index + @groups = AuthorCreditDivergenceQuery.new(**filters.symbolize_keys).call + + return unless turbo_frame_request? + render :author_credit_divergences_results + end + + # Resolve a whole person: point their profile at one preference and stamp them + # reconciled so a deliberate divergence stops reappearing on the worklist. + def update_person + person = Person.find(params[:id]) + person.assign_attributes(person_params) + person.author_credit_reconciled_at = Time.current + person.updated_by = current_user + + if person.save + redirect_to author_credit_divergences_path(filters), notice: "Updated credit preferences for #{person.full_name}." + else + redirect_to author_credit_divergences_path(filters), alert: person.errors.full_messages.to_sentence + end + end + + # Resolve a single item by rewriting its stored consent snapshot. Setting + # "anonymous" here is a live override that suppresses that item's credit alone; + # any other value only re-records history (see AuthorCreditable). + def update_item + model = AuthorCreditDivergenceQuery.model_for(params[:record_type]) + return redirect_to(author_credit_divergences_path(filters), alert: "Unknown record type.") unless model + + record = model.find(params[:record_id]) + record.author_credit_preference = params[:author_credit_preference] + record.updated_by = current_user if record.respond_to?(:updated_by=) + + if record.save + redirect_to author_credit_divergences_path(filters), notice: "Updated credit for #{model.name.underscore.humanize.downcase} ##{record.id}." + else + redirect_to author_credit_divergences_path(filters), alert: record.errors.full_messages.to_sentence + end + end + + private + + def authorize_page + authorize! :author_credit_divergence, to: :"#{action_name}?", with: AuthorCreditDivergencePolicy + end + + def person_params + params.require(:person).permit(:display_name_preference, :contributions_anonymous) + end + + # Carried through every redirect so the admin lands back on the same filtered list. + # The write actions deliberately name their own params `id` / `record_type` / + # `record_id` so a record identifier can never be mistaken for a filter. + def filters + params.permit(*FILTER_KEYS).to_h.compact_blank + end +end diff --git a/app/controllers/community_news_controller.rb b/app/controllers/community_news_controller.rb index 10ce42cfe0..3e35d01edb 100644 --- a/app/controllers/community_news_controller.rb +++ b/app/controllers/community_news_controller.rb @@ -146,7 +146,7 @@ def community_news_params :title, :rhino_body, :published, :featured, :publicly_visible, :publicly_featured, :reference_url, :youtube_url, :organization_id, - :author_id, :author_credit_preference, :created_by_id, :updated_by_id, + :author_id, :created_by_id, :updated_by_id, category_ids: [], sector_ids: [], primary_asset_attributes: [ :id, :file, :_destroy ], diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index f7ed091694..4162a5198e 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -525,8 +525,8 @@ def person_params :mailing_list_consented, :bio, :shoutout_text, :notes, :display_name_preference, + :contributions_anonymous, :pronouns, - :profile_show_name_preference, :profile_is_searchable, :profile_show_pronouns, :profile_show_credentials, diff --git a/app/controllers/resources_controller.rb b/app/controllers/resources_controller.rb index deb644e4d7..37cd5e4b4b 100644 --- a/app/controllers/resources_controller.rb +++ b/app/controllers/resources_controller.rb @@ -180,7 +180,7 @@ def resource_params params.require(:resource).permit( :rhino_body, :kind, :male, :female, :title, :featured, :published, :publicly_visible, :publicly_featured, :hidden_from_search, - :agency, :author_id, :author_credit_preference, :filemaker_code, :windows_type_id, :position, + :agency, :author_id, :filemaker_code, :windows_type_id, :position, primary_asset_attributes: [ :id, :file, :_destroy ], downloadable_asset_attributes: [ :id, :file, :_destroy ], gallery_assets_attributes: [ :id, :file, :_destroy ], diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index c680f545ec..0148819e30 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -211,7 +211,7 @@ def story_params params.require(:story).permit( :title, :rhino_body, :featured, :published, :publicly_visible, :publicly_featured, :youtube_url, :website_url, :windows_type_id, :organization_id, :workshop_id, :external_workshop_title, - :author_id, :updated_by_id, :story_idea_id, :spotlighted_facilitator_id, :author_credit_preference, + :author_id, :updated_by_id, :story_idea_id, :spotlighted_facilitator_id, category_ids: [], sector_ids: [], primary_asset_attributes: [ :id, :file, :_destroy ], diff --git a/app/controllers/story_ideas_controller.rb b/app/controllers/story_ideas_controller.rb index 60c9d34197..cd35032f98 100644 --- a/app/controllers/story_ideas_controller.rb +++ b/app/controllers/story_ideas_controller.rb @@ -146,7 +146,7 @@ def set_story_idea def story_idea_params params.require(:story_idea).permit( :title, :rhino_body, :youtube_url, - :permission_given, :author_credit_preference, :promoted_to_story, + :permission_given, :windows_type_id, :organization_id, :workshop_id, :external_workshop_title, :created_by_id, :updated_by_id, category_ids: [], diff --git a/app/controllers/workshop_ideas_controller.rb b/app/controllers/workshop_ideas_controller.rb index 11d8058246..5cd8ecdb0f 100644 --- a/app/controllers/workshop_ideas_controller.rb +++ b/app/controllers/workshop_ideas_controller.rb @@ -111,8 +111,7 @@ def set_workshop_idea # Strong parameters def workshop_idea_params params.require(:workshop_idea).permit( - :title, :staff_notes, :author_credit_preference, - :created_by_id, :updated_by_id, :windows_type_id, + :title, :staff_notes, :created_by_id, :updated_by_id, :windows_type_id, :time_closing, :time_creation, :time_demonstration, :time_hours, :time_intro, :time_minutes, :time_opening, :time_opening_circle, :time_warm_up, diff --git a/app/controllers/workshop_variation_ideas_controller.rb b/app/controllers/workshop_variation_ideas_controller.rb index 3ade7ecfd7..9d9e6b16f0 100644 --- a/app/controllers/workshop_variation_ideas_controller.rb +++ b/app/controllers/workshop_variation_ideas_controller.rb @@ -117,8 +117,7 @@ def set_form_variables def workshop_variation_idea_params params.require(:workshop_variation_idea).permit( :name, :rhino_body, :youtube_url, - :permission_given, :author_credit_preference, - :organization_id, :windows_type_id, :workshop_id, :created_by_id, :updated_by_id, + :permission_given, :organization_id, :windows_type_id, :workshop_id, :created_by_id, :updated_by_id, primary_asset_attributes: [ :id, :file, :_destroy ], gallery_assets_attributes: [ :id, :file, :_destroy ] ) diff --git a/app/controllers/workshop_variations_controller.rb b/app/controllers/workshop_variations_controller.rb index 9f42ac9ffe..964ba8fa4b 100644 --- a/app/controllers/workshop_variations_controller.rb +++ b/app/controllers/workshop_variations_controller.rb @@ -132,8 +132,7 @@ def set_form_variables def workshop_variation_params params.require(:workshop_variation).permit( [ :name, :rhino_body, :published, :publicly_visible, :position, :youtube_url, :author_id, - :organization_id, :workshop_id, :workshop_variation_idea_id, :author_credit_preference, - :windows_type_id, + :organization_id, :workshop_id, :workshop_variation_idea_id, :windows_type_id, primary_asset_attributes: [ :id, :file, :_destroy ], gallery_assets_attributes: [ :id, :file, :_destroy ] ] diff --git a/app/controllers/workshops_controller.rb b/app/controllers/workshops_controller.rb index 0650ef5286..e4fe284207 100644 --- a/app/controllers/workshops_controller.rb +++ b/app/controllers/workshops_controller.rb @@ -226,8 +226,7 @@ def log_workshop_error(action, error) def workshop_params params.require(:workshop).permit( :title, :featured, :published, - :full_name, :author_id, :windows_type_id, :workshop_idea_id, :author_credit_preference, - :month, :year, + :full_name, :author_id, :windows_type_id, :workshop_idea_id, :month, :year, :publicly_visible, :publicly_featured, diff --git a/app/decorators/resource_decorator.rb b/app/decorators/resource_decorator.rb index 172ac2fa97..f156774631 100644 --- a/app/decorators/resource_decorator.rb +++ b/app/decorators/resource_decorator.rb @@ -12,10 +12,6 @@ def kind_display kind == "Scholarship" ? "Scholar-ship" : (kind.present? ? kind.titleize : "Resource") end - def truncated_author - h.truncate author_credit, length: 20 - end - def truncated_title h.truncate title, length: 25 end @@ -36,10 +32,6 @@ def breadcrumbs "#{type_link} >> #{title}".html_safe end - def author_full_name - author_credit - end - def display_date created_at.strftime("%B %Y") end diff --git a/app/helpers/admin_cards_helper.rb b/app/helpers/admin_cards_helper.rb index e0f48fdee7..605bda026c 100644 --- a/app/helpers/admin_cards_helper.rb +++ b/app/helpers/admin_cards_helper.rb @@ -76,6 +76,7 @@ def deprecated_data_cards def additional_data_cards [ custom_card("Allocations", allocations_path, icon: "📤", color: :sky, intensity: 100), + custom_card("Author credit divergences", author_credit_divergences_path, icon: "✍️", color: :sky, intensity: 100), disabled_card("Bulk payments", icon: "💳"), custom_card("Event registrations", event_registrations_path, icon: "🎟️", color: :sky, intensity: 100), custom_card("Forms", forms_path, icon: "📋", color: :sky, intensity: 100), diff --git a/app/helpers/author_credit_divergences_helper.rb b/app/helpers/author_credit_divergences_helper.rb new file mode 100644 index 0000000000..3c3dc75e54 --- /dev/null +++ b/app/helpers/author_credit_divergences_helper.rb @@ -0,0 +1,15 @@ +module AuthorCreditDivergencesHelper + # The 8 AuthorCreditable models label their content differently (title vs name). + def divergence_record_title(record) + record.try(:title).presence || record.try(:name).presence || "##{record.id}" + end + + # The suggestion is the most restrictive preference across the person's content, + # but `anonymous` isn't a name format — it's the separate checkbox — so fall back + # to the profile's current format when that's what was suggested. + def suggested_display_name_preference(group) + suggested = group.suggested_preference + return suggested if Person::DISPLAY_NAME_PREFERENCES.include?(suggested) + group.person.display_name_preference.presence || "full_name" + end +end diff --git a/app/models/workshop.rb b/app/models/workshop.rb index 7d6028788c..5e0e7a7bf3 100644 --- a/app/models/workshop.rb +++ b/app/models/workshop.rb @@ -200,10 +200,6 @@ def missing_author_label "AWBW Facilitator" end - def author_name - author_person&.full_name.presence || full_name.presence - end - def date if month.present? && year.present? Date.new(year.to_i, month.to_i).strftime("%B %Y") diff --git a/app/models/workshop_idea.rb b/app/models/workshop_idea.rb index a0d794c9ff..dbb6b53be2 100644 --- a/app/models/workshop_idea.rb +++ b/app/models/workshop_idea.rb @@ -73,9 +73,12 @@ class WorkshopIdea < ApplicationRecord # Scopes scope :title, ->(title) { where("workshop_ideas.title like ?", "%#{ title }%") } - scope :author_name, ->(author_name) { joins(:created_by). - where("users.first_name like ? or users.last_name like ? or users.email like ?", - "%#{author_name}%", "%#{author_name}%", "%#{author_name}%") } + # Goes through by_credited_person_name so the filter honors the credit preference — + # matching users.first_name/last_name/email directly would surface ideas whose + # credit renders "Anonymous". + scope :author_name, ->(author_name) { + where(id: by_credited_person_name(author_name).select("workshop_ideas.id")) + } def self.search(params) results = is_a?(ActiveRecord::Relation) ? self : all diff --git a/app/policies/author_credit_divergence_policy.rb b/app/policies/author_credit_divergence_policy.rb new file mode 100644 index 0000000000..5d78b597cf --- /dev/null +++ b/app/policies/author_credit_divergence_policy.rb @@ -0,0 +1,13 @@ +class AuthorCreditDivergencePolicy < ApplicationPolicy + def index? + admin? + end + + def update_person? + admin? + end + + def update_item? + admin? + end +end diff --git a/app/services/author_credit_divergence_query.rb b/app/services/author_credit_divergence_query.rb new file mode 100644 index 0000000000..1a07a6fa64 --- /dev/null +++ b/app/services/author_credit_divergence_query.rb @@ -0,0 +1,97 @@ +# Finds content whose stored `author_credit_preference` no longer agrees with the +# credited person's profile, grouped by person so an admin can resolve a whole +# person at once. +# +# The comparison runs in Ruby rather than SQL because it walks the author fallback +# chain (explicit author, then the creating user's person). The candidate set is +# small: the column was added with no default and no backfill, so most legacy rows +# are NULL and only the *_idea tables hold a real spread. +class AuthorCreditDivergenceQuery + # Every AuthorCreditable model. Doubles as the allowlist for the `type` param — + # never constantize a raw param. + MODEL_NAMES = %w[ + Story + StoryIdea + Workshop + WorkshopIdea + WorkshopVariation + WorkshopVariationIdea + Resource + CommunityNews + ].freeze + + # Which preference reveals the least, for suggesting a profile value that + # satisfies all of a person's items. + RESTRICTIVENESS = { + "anonymous" => 4, + "last_name_only" => 3, + "first_name_only" => 3, + "first_name_last_initial" => 2, + "full_name" => 1 + }.freeze + + Group = Struct.new(:person, :records, :suggested_preference, keyword_init: true) + + def self.model_for(type) + MODEL_NAMES.include?(type.to_s) ? type.to_s.constantize : nil + end + + def initialize(person_id: nil, type: nil, preference: nil, include_reconciled: false) + @person_id = person_id.presence + @type = type.presence + @preference = preference.presence + @include_reconciled = ActiveModel::Type::Boolean.new.cast(include_reconciled) + end + + # => [Group] sorted by the person's name + def call + diverged_records + .group_by(&:author_person) + .filter_map { |person, records| build_group(person, records) } + .sort_by { |group| group.person.full_name.to_s.downcase } + end + + private + + attr_reader :person_id, :type, :preference, :include_reconciled + + def models + return [ self.class.model_for(type) ].compact if type + MODEL_NAMES.map(&:constantize) + end + + def diverged_records + models.flat_map { |model| diverged_for(model) } + end + + # No person_id filter here — a record can be credited through `author_id` *or* + # through the creating user's person, so the filter has to run after the + # fallback chain resolves. See `build_group`. + def diverged_for(model) + scope = model.where.not(author_credit_preference: nil) + scope = scope.where(author_credit_preference: preference) if preference + scope.includes(includes_for(model)).select(&:author_credit_diverged?) + end + + def includes_for(model) + includes = [ { created_by: :person } ] + includes << :author if model.column_names.include?("author_id") + includes + end + + def build_group(person, records) + return nil if person.blank? + return nil if person_id && person.id != person_id.to_i + return nil if person.author_credit_reconciled_at.present? && !include_reconciled + + Group.new( + person: person, + records: records.sort_by { |record| [ record.class.name, record.id ] }, + suggested_preference: most_restrictive(records) + ) + end + + def most_restrictive(records) + records.map(&:author_credit_preference).max_by { |value| RESTRICTIVENESS.fetch(value, 0) } + end +end diff --git a/app/views/author_credit_divergences/_filter_fields.html.erb b/app/views/author_credit_divergences/_filter_fields.html.erb new file mode 100644 index 0000000000..d01ad02454 --- /dev/null +++ b/app/views/author_credit_divergences/_filter_fields.html.erb @@ -0,0 +1,5 @@ +<%# Carries the active filters through a save so the admin lands back on the same list. %> +<% AuthorCreditDivergencesController::FILTER_KEYS.each do |key| %> + <% next if params[key].blank? %> + <%= hidden_field_tag key, params[key], id: nil %> +<% end %> diff --git a/app/views/author_credit_divergences/_filters.html.erb b/app/views/author_credit_divergences/_filters.html.erb new file mode 100644 index 0000000000..f45f5bfc97 --- /dev/null +++ b/app/views/author_credit_divergences/_filters.html.erb @@ -0,0 +1,36 @@ +<%= form_with url: author_credit_divergences_path, method: :get, + data: { turbo_frame: "author_credit_divergences_results" }, + class: "flex flex-wrap items-end gap-3 mb-6" do |f| %> +
+ <%= f.label :type, "Content type", class: "block text-sm font-medium text-gray-700" %> + <%= f.select :type, + options_for_select(AuthorCreditDivergenceQuery::MODEL_NAMES.map { |name| [ name.underscore.humanize, name ] }, params[:type]), + { include_blank: "All types" }, + class: "mt-1 rounded-md border-gray-300 text-sm" %> +
+ +
+ <%= f.label :preference, "Stored preference", class: "block text-sm font-medium text-gray-700" %> + <%= f.select :preference, + options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, params[:preference]), + { include_blank: "Any preference" }, + class: "mt-1 rounded-md border-gray-300 text-sm" %> +
+ +
+ <%= f.label :person_id, "Person ID", class: "block text-sm font-medium text-gray-700" %> + <%= f.number_field :person_id, value: params[:person_id], placeholder: "Any person", + class: "mt-1 rounded-md border-gray-300 text-sm w-32" %> +
+ + + + <%= f.submit "Filter", class: "rounded-md bg-blue-600 px-4 py-2 text-sm font-medium text-white hover:bg-blue-700 cursor-pointer" %> + <%= link_to "Clear", author_credit_divergences_path, + data: { turbo_frame: "author_credit_divergences_results" }, + class: "text-sm text-gray-600 underline pb-2" %> +<% end %> diff --git a/app/views/author_credit_divergences/_results_skeleton.html.erb b/app/views/author_credit_divergences/_results_skeleton.html.erb new file mode 100644 index 0000000000..1d73a997c8 --- /dev/null +++ b/app/views/author_credit_divergences/_results_skeleton.html.erb @@ -0,0 +1,9 @@ +
+ <% 3.times do %> +
+
+
+
+
+ <% end %> +
diff --git a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb new file mode 100644 index 0000000000..aa1dc4f2ee --- /dev/null +++ b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb @@ -0,0 +1,112 @@ +<%= turbo_frame_tag :author_credit_divergences_results do %> + <% if @groups.empty? %> +
+ +

Nothing to reconcile.

+

Every stored credit preference matches its author's profile.

+
+ <% else %> +

+ <%= pluralize(@groups.size, "person") %> with diverging content. +

+ +
+ <% @groups.each do |group| %> + <% person = group.person %> +
+
+
+ <%= link_to person.full_name, person_path(person), class: "font-semibold text-gray-900 hover:underline" %> + + — profile currently credits as + <%= AuthorCreditable::ADMIN_FORM_OPTIONS.key(person.effective_author_credit_preference) %> + +
+ <% if person.author_credit_reconciled_at.present? %> + + Reconciled <%= person.author_credit_reconciled_at.strftime("%b %-d, %Y") %> + + <% end %> +
+ +
+ <%= form_with url: update_person_author_credit_divergences_path, method: :patch, + data: { turbo_frame: "_top" }, + class: "flex flex-wrap items-end gap-3" do |f| %> + <%= hidden_field_tag :id, person.id %> + <%= render "filter_fields" %> + +
+ <%= label_tag "person_display_name_preference_#{person.id}", "Set profile to", + class: "block text-xs font-medium text-gray-700" %> + <%= select_tag "person[display_name_preference]", + options_for_select(Person::DISPLAY_NAME_PREFERENCE_LABELS.invert.to_a, + suggested_display_name_preference(group)), + id: "person_display_name_preference_#{person.id}", + class: "mt-1 rounded-md border-gray-300 text-sm" %> +
+ + + + <%= submit_tag "Apply to profile", + class: "rounded-md bg-blue-600 px-3 py-2 text-sm font-medium text-white hover:bg-blue-700 cursor-pointer" %> + + Suggested: the most restrictive preference across their content. + + <% end %> +
+ + + + + + + + + + + + <% group.records.each do |record| %> + + + + + + + <% end %> + +
ContentTypeRenders asStored consent
+ <%= link_to divergence_record_title(record), polymorphic_path(record), + target: "_blank", rel: "noopener", + title: "Opens in a new tab", + class: "text-blue-700 hover:underline" %> + <%= record.class.name.underscore.humanize %><%= record.author_credit %> + <%= form_with url: update_item_author_credit_divergences_path, method: :patch, + data: { turbo_frame: "_top" }, + class: "flex items-center gap-2" do %> + <%= hidden_field_tag :record_type, record.class.name %> + <%= hidden_field_tag :record_id, record.id %> + <%= render "filter_fields" %> + <%= select_tag "author_credit_preference", + options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, record.author_credit_preference), + class: "rounded-md border-gray-300 text-sm" %> + <%= submit_tag "Save", + class: "rounded-md border border-gray-300 bg-white px-2 py-1 text-xs text-gray-700 hover:bg-gray-50 cursor-pointer" %> + <% end %> +
+

+ Only Anonymous changes what renders — the other values just + re-record what was consented to. +

+
+ <% end %> +
+ <% end %> +<% end %> diff --git a/app/views/author_credit_divergences/index.html.erb b/app/views/author_credit_divergences/index.html.erb new file mode 100644 index 0000000000..8b51a3e646 --- /dev/null +++ b/app/views/author_credit_divergences/index.html.erb @@ -0,0 +1,22 @@ +<% content_for(:page_bg_class, "admin-only bg-blue-100") %> +
+
+
+

Author credit divergences

+

+ Content whose stored credit preference no longer matches the author's profile. + Credits render using the profile, so these are historical records that need a + decision — except anonymous, which always wins and keeps that + item uncredited no matter what the profile says. +

+
+ + <%= render "filters" %> + + <% result_src = author_credit_divergences_path(request.query_parameters) %> + + <%= turbo_frame_tag :author_credit_divergences_results, src: result_src, data: { turbo: "temporary" } do %> + <%= render "results_skeleton" %> + <% end %> +
+
diff --git a/app/views/community_news/_form.html.erb b/app/views/community_news/_form.html.erb index cf39270a64..51a3c9ba1f 100644 --- a/app/views/community_news/_form.html.erb +++ b/app/views/community_news/_form.html.erb @@ -61,13 +61,7 @@ data: { controller: "remote-select", remote_select_model_value: "person" } } %> - <%= f.input :author_credit_preference, - as: :select, - label: "Author credit preference", - hint: "Controls how the author's name is displayed", - collection: AuthorCreditable::ADMIN_FORM_OPTIONS, - selected: f.object.author_credit_preference, - input_html: { class: "block w-full rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500 sm:text-sm" } %> + <%= render "shared/author_credit_warning", record: f.object %> <%= render "shared/form_image_fields", f: f, include_primary_asset: true %> diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index 6e8a453b08..e292e541cb 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -457,7 +457,7 @@ -
+
Profile display preferences
@@ -472,14 +472,14 @@ <%= f.input :display_name_preference, as: :select, - collection: [ - ["First and Last Name", "full_name"], - ["First Name and Last Initial", "first_name_last_initial"], - ["First Name Only", "first_name_only"], - ["Last Name Only", "last_name_only"] - ], + collection: Person::DISPLAY_NAME_PREFERENCE_LABELS.invert.to_a, + hint: "Applies everywhere this person's name appears, including author credits", selected: f.object.display_name_preference || "full_name" %> + <%= f.input :contributions_anonymous, + label: "Contributions show as anonymous", + hint: "Credits stories, workshops, variations and resources as \"Anonymous\". Does not change how they're listed on the people index." %> + <%= f.input :profile_show_credentials, label: "Show credentials" %> <%= f.input :profile_show_pronouns, label: "Show pronouns" %> diff --git a/app/views/resources/_form.html.erb b/app/views/resources/_form.html.erb index ee21d2d789..bdb02ea15a 100644 --- a/app/views/resources/_form.html.erb +++ b/app/views/resources/_form.html.erb @@ -79,15 +79,7 @@
- <%= f.input :author_credit_preference, - as: :select, - collection: AuthorCreditable::ADMIN_FORM_OPTIONS, - include_blank: "Select a preference", - selected: f.object.author_credit_preference || "full_name", - label: "Author credit preference", - hint: "Controls how the author’s name is displayed", - label_html: { class: "block font-medium mb-1 text-gray-700" }, - input_html: { class: "w-full rounded border-gray-300" } %> + <%= render "shared/author_credit_warning", record: f.object %>
diff --git a/app/views/shared/_author_credit_preview.html.erb b/app/views/shared/_author_credit_preview.html.erb new file mode 100644 index 0000000000..2007ce5fe3 --- /dev/null +++ b/app/views/shared/_author_credit_preview.html.erb @@ -0,0 +1,14 @@ +<%# Submitter-facing. A new record has no stored snapshot to diverge from, so this is + unconditional — it's the only place a submitter learns how they'll be credited. %> +<% person = current_user&.person %> +
+ <% if person %> + You'll be credited as <%= person.contributions_anonymous? ? "Anonymous" : person.name %>. + <% else %> + You'll be credited as Anonymous. + <% end %> +

+ This comes from your profile and applies to everything you share. + <%= link_to "Contact us", contact_us_path, class: "underline hover:text-gray-700" %> to change it. +

+
diff --git a/app/views/shared/_author_credit_status.html.erb b/app/views/shared/_author_credit_status.html.erb new file mode 100644 index 0000000000..29da8b7e77 --- /dev/null +++ b/app/views/shared/_author_credit_status.html.erb @@ -0,0 +1,7 @@ +<%# Idea forms are reached both by submitters (new) and admins (edit), so show the + submitter-facing preview on a new record and the divergence warning on a saved one. %> +<% if record.new_record? %> + <%= render "shared/author_credit_preview" %> +<% else %> + <%= render "shared/author_credit_warning", record: record %> +<% end %> diff --git a/app/views/shared/_author_credit_warning.html.erb b/app/views/shared/_author_credit_warning.html.erb new file mode 100644 index 0000000000..25d19d9789 --- /dev/null +++ b/app/views/shared/_author_credit_warning.html.erb @@ -0,0 +1,23 @@ +<%# Renders only when the stored consent snapshot disagrees with the author's profile. + Credits render from the profile, so on most records this is silent. %> +<% if record.persisted? && record.author_credit_diverged? %> + <% stored = AuthorCreditable::ADMIN_FORM_OPTIONS.key(record.author_credit_preference) %> + <% person = record.author_person %> + <% profile = AuthorCreditable::ADMIN_FORM_OPTIONS.key(person.effective_author_credit_preference) %> + + <% if record.author_credit_preference == AuthorCreditable::ANONYMOUS %> +
+ + Submitted anonymously. This item stays anonymous regardless of the profile setting. + <%= link_to "Reconcile", author_credit_divergences_path(person_id: person.id), + class: "underline hover:text-gray-700" %> +
+ <% else %> +
+ ⚠ Submitted as "<%= stored %>", but this profile is now set to "<%= profile %>". + This item is credited using the profile setting. + <%= link_to "Reconcile", author_credit_divergences_path(person_id: person.id), + class: "underline hover:text-amber-900" %> +
+ <% end %> +<% end %> diff --git a/app/views/stories/_form.html.erb b/app/views/stories/_form.html.erb index 167e59f429..2c0b669bcf 100644 --- a/app/views/stories/_form.html.erb +++ b/app/views/stories/_form.html.erb @@ -209,18 +209,7 @@
- <%= f.input :author_credit_preference, - as: :select, - collection: AuthorCreditable::ADMIN_FORM_OPTIONS, - prompt: "Select a preference", - selected: f.object.author_credit_preference || story_idea&.author_credit_preference, - label: "Author credit preference", - hint: "Controls how the author's name is displayed", - input_html: { - class: select_caret_class(blank: f.object.author_credit_preference.blank?), - style: custom_caret_style, - onchange: select_caret_onchange - } %> + <%= render "shared/author_credit_warning", record: f.object %>
@@ -243,15 +232,8 @@
Story idea author credit:
- <% if story_idea.created_by.person %> - <%= person_profile_button(story_idea.created_by.person, display_name: story_idea.author_credit, subtitle: story_idea.created_by.person.full_name) %> - <% else %> - <%= story_idea.author_credit %> - <% end %> -
- Story idea "author credit preference": -
-
<%= link_to story_idea.author_credit_preference, edit_story_idea_path(story_idea, anchor: "publish-preferences"), class: "hover:underline hover:text-blue-600" %>
+ <%= credited_author_link(story_idea) %> + <%= render "shared/author_credit_warning", record: story_idea %>
<% end %> diff --git a/app/views/story_ideas/_form.html.erb b/app/views/story_ideas/_form.html.erb index c4e15de103..73190b5f58 100644 --- a/app/views/story_ideas/_form.html.erb +++ b/app/views/story_ideas/_form.html.erb @@ -292,17 +292,7 @@
- <%= f.input :author_credit_preference, - as: :select, - required: true, - collection: AuthorCreditable::IDEA_FORM_OPTIONS, - prompt: "Select a preference", - selected: f.object.author_credit_preference, - input_html: { - class: select_caret_class(blank: f.object.author_credit_preference.blank?), - style: custom_caret_style, - onchange: select_caret_onchange - } %> + <%= render "shared/author_credit_status", record: f.object %> <%= f.input :youtube_url, as: :text, label: "YouTube link (optional)".html_safe, diff --git a/app/views/story_ideas/show.html.erb b/app/views/story_ideas/show.html.erb index caecbbe87d..46ca04a52a 100644 --- a/app/views/story_ideas/show.html.erb +++ b/app/views/story_ideas/show.html.erb @@ -42,10 +42,7 @@
Author credit: - <%= @story_idea.author_credit_preference&.humanize || "—" %> - <% if @story_idea.author_credit_preference.present? %> - (<%= @story_idea.author_credit %>) - <% end %> + <%= credited_author_link(@story_idea) %>
Permission given: diff --git a/app/views/workshop_ideas/_form.html.erb b/app/views/workshop_ideas/_form.html.erb index 4f3a623849..4996f80860 100644 --- a/app/views/workshop_ideas/_form.html.erb +++ b/app/views/workshop_ideas/_form.html.erb @@ -87,13 +87,7 @@ focus:border-blue-500 sm:text-sm" } %>
- <%= f.input :author_credit_preference, - as: :select, - collection: AuthorCreditable::IDEA_FORM_OPTIONS, - include_blank: "Select a preference", - selected: f.object.author_credit_preference, - label: "Author credit preference", - hint: "Controls how the author's name is displayed" %> + <%= render "shared/author_credit_status", record: f.object %>
diff --git a/app/views/workshop_ideas/show.html.erb b/app/views/workshop_ideas/show.html.erb index 24372358f9..6837a43c58 100644 --- a/app/views/workshop_ideas/show.html.erb +++ b/app/views/workshop_ideas/show.html.erb @@ -31,10 +31,7 @@
Author credit: - <%= @workshop_idea.author_credit_preference&.humanize || "—" %> - <% if @workshop_idea.author_credit_preference.present? %> - (<%= @workshop_idea.author_credit %>) - <% end %> + <%= credited_author_link(@workshop_idea) %>
Windows audience: diff --git a/app/views/workshop_variation_ideas/_form.html.erb b/app/views/workshop_variation_ideas/_form.html.erb index 16a1212a41..165b3dd3ea 100644 --- a/app/views/workshop_variation_ideas/_form.html.erb +++ b/app/views/workshop_variation_ideas/_form.html.erb @@ -103,13 +103,7 @@ <% end %>
- <%= f.input :author_credit_preference, - as: :select, - required: true, - collection: AuthorCreditable::IDEA_FORM_OPTIONS, - include_blank: "Select a preference", - selected: f.object.author_credit_preference, - input_html: { class: "block w-full h-10 rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500 sm:text-sm" } %> + <%= render "shared/author_credit_status", record: f.object %> <%= f.input :youtube_url, as: :text, label: "YouTube link (optional)".html_safe, diff --git a/app/views/workshop_variation_ideas/index.html.erb b/app/views/workshop_variation_ideas/index.html.erb index caed96eaea..314478f833 100644 --- a/app/views/workshop_variation_ideas/index.html.erb +++ b/app/views/workshop_variation_ideas/index.html.erb @@ -67,14 +67,7 @@ <% end %> - <% display_name = workshop_variation_idea.author_credit.presence || workshop_variation_idea.created_by&.name %> - <% if workshop_variation_idea.created_by&.person %> - <%= link_to display_name, - person_path(workshop_variation_idea.created_by.person), - class: "text-gray-500 hover:text-gray-700" %> - <% elsif display_name %> - <%= display_name %> - <% end %> + <%= credited_author_link(workshop_variation_idea, class: "text-gray-500 hover:text-gray-700") %> <% promoted_variation = workshop_variation_idea.workshop_variations.first %> diff --git a/app/views/workshop_variation_ideas/show.html.erb b/app/views/workshop_variation_ideas/show.html.erb index 0a70276b40..3b4a38c359 100644 --- a/app/views/workshop_variation_ideas/show.html.erb +++ b/app/views/workshop_variation_ideas/show.html.erb @@ -62,10 +62,7 @@
Author credit: - <%= @workshop_variation_idea.author_credit_preference&.humanize || "—" %> - <% if @workshop_variation_idea.author_credit_preference.present? %> - (<%= @workshop_variation_idea.author_credit %>) - <% end %> + <%= credited_author_link(@workshop_variation_idea) %>
Permission given: diff --git a/app/views/workshop_variations/_form.html.erb b/app/views/workshop_variations/_form.html.erb index b7c14b83c4..d448f7353d 100644 --- a/app/views/workshop_variations/_form.html.erb +++ b/app/views/workshop_variations/_form.html.erb @@ -90,15 +90,7 @@
- <%= f.input :author_credit_preference, - as: :select, - required: true, - collection: AuthorCreditable::ADMIN_FORM_OPTIONS, - include_blank: "Select a preference", - selected: f.object.author_credit_preference || "full_name", - label: "Author credit preference", - hint: "Controls how the author’s name is displayed", - input_html: { class: "h-10 rounded-md" } %> + <%= render "shared/author_credit_warning", record: f.object %> <% if allowed_to?(:manage?, WorkshopVariation) %>
diff --git a/app/views/workshops/_form.html.erb b/app/views/workshops/_form.html.erb index 0eba1a082f..d88df7e27c 100644 --- a/app/views/workshops/_form.html.erb +++ b/app/views/workshops/_form.html.erb @@ -71,13 +71,7 @@ data: { controller: "remote-select", remote_select_model_value: "person" } } %>
- <%= f.input :author_credit_preference, - as: :select, - collection: AuthorCreditable::ADMIN_FORM_OPTIONS, - include_blank: "Select a preference", - selected: f.object.author_credit_preference, - label: "Author credit preference", - hint: "Controls how the author's name is displayed" %> + <%= render "shared/author_credit_warning", record: f.object %>
<%= f.input :workshop_idea_id, diff --git a/config/routes.rb b/config/routes.rb index 7d0534d362..2f0f9b7aa1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -194,6 +194,12 @@ resource :invoice, only: [ :show ], module: :events get "form_submissions/:person_id", to: "events/form_submissions#show", as: :registrant_submissions end + resources :author_credit_divergences, only: :index do + collection do + patch :update_person + patch :update_item + end + end resources :people do collection do get :check_duplicates diff --git a/db/seeds/dev/people_profiles.rb b/db/seeds/dev/people_profiles.rb index be9aab16a8..c9c65c2890 100644 --- a/db/seeds/dev/people_profiles.rb +++ b/db/seeds/dev/people_profiles.rb @@ -121,6 +121,10 @@ email: data[:email], email_2: data[:email_2], profile_is_searchable: data[:searchable], + # Spread the credit preferences across seeded people so the author credit + # divergences page has something to triage in dev. + display_name_preference: Person::DISPLAY_NAME_PREFERENCES.sample, + contributions_anonymous: [ true, false, false, false ].sample, created_by: admin_user, updated_by: admin_user } diff --git a/db/seeds/dev/workshops.rb b/db/seeds/dev/workshops.rb index d71c19805e..0d4e6a95f6 100644 --- a/db/seeds/dev/workshops.rb +++ b/db/seeds/dev/workshops.rb @@ -541,7 +541,7 @@ position: var_data[:position], published: [ true, true, false ].sample, windows_type_id: windows_type_id, - author_credit_preference: "anonymous" + author_credit_preference: "anonymous" # a real anonymity latch, kept per-item ) variation.save! end diff --git a/spec/factories/people.rb b/spec/factories/people.rb index 30d1e116d0..0222e72497 100644 --- a/spec/factories/people.rb +++ b/spec/factories/people.rb @@ -6,6 +6,10 @@ first_name { Faker::Name.first_name.gsub("'", " ") } last_name { Faker::Name.last_name.gsub("'", " ") } + trait :anonymous_contributions do + contributions_anonymous { true } + end + trait :with_organization do after(:create) do |person| person.organizations << create(:organization) diff --git a/spec/factories/story_ideas.rb b/spec/factories/story_ideas.rb index 4d411b6c33..286e42f191 100644 --- a/spec/factories/story_ideas.rb +++ b/spec/factories/story_ideas.rb @@ -6,7 +6,6 @@ title { "My Title" } rhino_body { "

My Body

" } permission_given { true } - author_credit_preference { "full_name" } association :created_by, factory: :user association :updated_by, factory: :user diff --git a/spec/factories/workshop_ideas.rb b/spec/factories/workshop_ideas.rb index cdb54cd4ca..d612b33b58 100644 --- a/spec/factories/workshop_ideas.rb +++ b/spec/factories/workshop_ideas.rb @@ -18,7 +18,6 @@ instructions { "MyText" } optional_materials { "MyText" } notes { "MyText" } - author_credit_preference { "full_name" } association :created_by, factory: :user association :updated_by, factory: :user diff --git a/spec/factories/workshop_variation_ideas.rb b/spec/factories/workshop_variation_ideas.rb index 3fa7c4536d..4a995a89f9 100644 --- a/spec/factories/workshop_variation_ideas.rb +++ b/spec/factories/workshop_variation_ideas.rb @@ -4,7 +4,6 @@ rhino_body { "

This is a variation idea description

" } youtube_url { "https://www.youtube.com/watch?v=example" } permission_given { true } - author_credit_preference { "full_name" } association :workshop association :organization association :windows_type diff --git a/spec/factories/workshop_variations.rb b/spec/factories/workshop_variations.rb index e75cd1be46..1d9b50fe8f 100644 --- a/spec/factories/workshop_variations.rb +++ b/spec/factories/workshop_variations.rb @@ -4,7 +4,6 @@ association :windows_type sequence(:name) { |n| "Variation #{n}" } rhino_body { "

Variation details using CKEditor

" } - author_credit_preference { "full_name" } sequence(:position) { |n| n } published { false } diff --git a/spec/models/community_news_spec.rb b/spec/models/community_news_spec.rb index 2f7d2bceb2..5dbf937c10 100644 --- a/spec/models/community_news_spec.rb +++ b/spec/models/community_news_spec.rb @@ -58,22 +58,13 @@ end end + # Author names are no longer in the SearchCop index — see .search_by_params below, + # which ORs in `by_credited_person_name` so person search honors the credit + # preference. Indexing people.first_name/last_name here could not. context 'when searching by person name' do - it 'finds records by person first name' do - results = CommunityNews.search('John') - expect(results).to include(community_news_with_person) - expect(results).not_to include(community_news_without_person) - end - - it 'finds records by person last name' do - results = CommunityNews.search('Doe') - expect(results).to include(community_news_with_person) - expect(results).not_to include(community_news_without_person) - end - - it 'finds records by partial person name' do - results = CommunityNews.search('Joh') - expect(results).to include(community_news_with_person) + it 'does not match on the author name' do + expect(CommunityNews.search('John')).to be_empty + expect(CommunityNews.search('Doe')).to be_empty end end @@ -97,15 +88,36 @@ end context 'when searching with multiple terms' do - it 'finds records matching all terms across different fields' do - results = CommunityNews.search('John Breaking') + it 'finds records matching all terms across title and content' do + results = CommunityNews.search('Breaking technology') expect(results).to include(community_news_with_person) expect(results).not_to include(community_news_without_person) end - it 'finds records matching content and person name' do - results = CommunityNews.search('John technology') + # Deliberate tradeoff: an author name can no longer be one term of an AND + # query, because honoring the credit preference needs per-person branching + # that a flat SearchCop index can't express. + it 'does not combine an author name with a content term' do + expect(CommunityNews.search('John technology')).to be_empty + end + end + + context 'via search_by_params (the user-facing path)' do + it 'finds records by the credited author name' do + results = CommunityNews.search_by_params(query: 'John') expect(results).to include(community_news_with_person) + expect(results).not_to include(community_news_without_person) + end + + it 'stops matching once the author marks contributions anonymous' do + person.update!(contributions_anonymous: true) + expect(CommunityNews.search_by_params(query: 'John')).to be_empty + end + + it 'stops matching the last name when only the first name is credited' do + person.update!(display_name_preference: 'first_name_only') + expect(CommunityNews.search_by_params(query: 'John')).to include(community_news_with_person) + expect(CommunityNews.search_by_params(query: 'Doe')).to be_empty end end @@ -118,13 +130,13 @@ context 'AND operator behavior for multiple search terms' do it 'requires all search terms to match across different fields' do - results = CommunityNews.search('John Breaking') + results = CommunityNews.search('Breaking important') expect(results).to include(community_news_with_person) expect(results).not_to include(community_news_without_person) end it 'finds no results when terms match different records' do - results = CommunityNews.search('John Report') + results = CommunityNews.search('technology Report') expect(results).to be_empty end diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index ad9853d1ee..98ac4aa135 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -232,7 +232,7 @@ def term(cost_cents:, start_date: Date.current, subscription: nil) context "when display_name_preference is first_name_last_initial" do it "returns first name and last initial" do person.display_name_preference = "first_name_last_initial" - expect(person.name).to eq("Jane D") + expect(person.name).to eq("Jane D.") end end @@ -256,6 +256,48 @@ def term(cost_cents:, start_date: Date.current, subscription: nil) expect(person.name).to eq("Jane Doe") end end + + it "is unaffected by contributions_anonymous" do + person.display_name_preference = "full_name" + person.contributions_anonymous = true + expect(person.name).to eq("Jane Doe") + end + end + + describe "display_name_preference validation" do + it "accepts each allowed value" do + Person::DISPLAY_NAME_PREFERENCES.each do |value| + expect(build(:person, display_name_preference: value)).to be_valid + end + end + + it "allows blank" do + expect(build(:person, display_name_preference: nil)).to be_valid + end + + it "rejects anything else" do + person = build(:person, display_name_preference: "anonymous") + expect(person).not_to be_valid + expect(person.errors[:display_name_preference]).to be_present + end + end + + describe "#effective_author_credit_preference" do + let(:person) { build(:person, display_name_preference: "first_name_only") } + + it "is the display name preference by default" do + expect(person.effective_author_credit_preference).to eq("first_name_only") + end + + it "falls back to full_name when unset" do + person.display_name_preference = nil + expect(person.effective_author_credit_preference).to eq("full_name") + end + + it "is anonymous when contributions are anonymous, whatever the format" do + person.contributions_anonymous = true + expect(person.effective_author_credit_preference).to eq("anonymous") + end end describe "#published?" do diff --git a/spec/requests/author_credit_divergences_spec.rb b/spec/requests/author_credit_divergences_spec.rb new file mode 100644 index 0000000000..15d64f14ab --- /dev/null +++ b/spec/requests/author_credit_divergences_spec.rb @@ -0,0 +1,104 @@ +require "rails_helper" + +RSpec.describe "AuthorCreditDivergences", type: :request do + let(:admin) { create(:user, :admin) } + let(:regular_user) { create(:user) } + let(:author_user) { create(:user, :with_person) } + let(:person) { author_user.person } + + let!(:story) do + person.update!(display_name_preference: "first_name_only") + record = create(:story, created_by: author_user, author: person, author_credit_preference: nil) + person.update!(display_name_preference: "full_name") + record + end + + describe "GET /author_credit_divergences" do + it "requires an admin" do + sign_in regular_user + get author_credit_divergences_path + expect(response).not_to have_http_status(:ok) + end + + it "renders the page shell for an admin" do + sign_in admin + get author_credit_divergences_path + expect(response).to have_http_status(:ok) + expect(response.body).to include("Author credit divergences") + end + + it "renders just the results inside the turbo frame" do + sign_in admin + get author_credit_divergences_path, headers: { "Turbo-Frame" => "author_credit_divergences_results" } + expect(response).to have_http_status(:ok) + expect(response.body).to include(person.full_name) + expect(response.body).to include(story.title) + end + end + + describe "PATCH /author_credit_divergences/update_person" do + before { sign_in admin } + + it "updates the profile and stamps the person reconciled" do + patch update_person_author_credit_divergences_path, + params: { id: person.id, person: { display_name_preference: "first_name_only", contributions_anonymous: "0" } } + + expect(person.reload.display_name_preference).to eq("first_name_only") + expect(person.author_credit_reconciled_at).to be_present + end + + it "can mark contributions anonymous" do + patch update_person_author_credit_divergences_path, + params: { id: person.id, person: { display_name_preference: "full_name", contributions_anonymous: "1" } } + + expect(person.reload.contributions_anonymous).to be(true) + expect(story.reload.author_credit).to eq("Anonymous") + end + + it "carries the active filters through the redirect" do + patch update_person_author_credit_divergences_path, + params: { id: person.id, type: "Story", + person: { display_name_preference: "full_name", contributions_anonymous: "0" } } + + expect(response).to redirect_to(author_credit_divergences_path(type: "Story")) + end + + it "rejects a non-admin" do + sign_out admin + sign_in regular_user + patch update_person_author_credit_divergences_path, + params: { id: person.id, person: { display_name_preference: "first_name_only" } } + + expect(person.reload.display_name_preference).to eq("full_name") + end + end + + describe "PATCH /author_credit_divergences/update_item" do + before { sign_in admin } + + it "rewrites a single record's stored snapshot" do + patch update_item_author_credit_divergences_path, + params: { record_type: "Story", record_id: story.id, author_credit_preference: "full_name" } + + expect(story.reload.author_credit_preference).to eq("full_name") + end + + it "makes one item anonymous without touching the person's others" do + other = create(:story, created_by: author_user, author: person) + + patch update_item_author_credit_divergences_path, + params: { record_type: "Story", record_id: story.id, author_credit_preference: "anonymous" } + + expect(story.reload.author_credit).to eq("Anonymous") + expect(other.reload.author_credit).to eq(person.full_name) + end + + it "refuses a type outside the allowlist instead of constantizing it" do + patch update_item_author_credit_divergences_path, + params: { record_type: "User", record_id: admin.id, author_credit_preference: "anonymous" } + + expect(response).to redirect_to(author_credit_divergences_path) + expect(flash[:alert]).to eq("Unknown record type.") + end + end +end diff --git a/spec/routing/author_credit_divergences_routing_spec.rb b/spec/routing/author_credit_divergences_routing_spec.rb new file mode 100644 index 0000000000..984c747905 --- /dev/null +++ b/spec/routing/author_credit_divergences_routing_spec.rb @@ -0,0 +1,19 @@ +require "rails_helper" + +RSpec.describe AuthorCreditDivergencesController, type: :routing do + describe "routing" do + it "routes to #index" do + expect(get: "/author_credit_divergences").to route_to("author_credit_divergences#index") + end + + it "routes to #update_person" do + expect(patch: "/author_credit_divergences/update_person") + .to route_to("author_credit_divergences#update_person") + end + + it "routes to #update_item" do + expect(patch: "/author_credit_divergences/update_item") + .to route_to("author_credit_divergences#update_item") + end + end +end diff --git a/spec/services/author_credit_divergence_query_spec.rb b/spec/services/author_credit_divergence_query_spec.rb new file mode 100644 index 0000000000..b94d7c4cd6 --- /dev/null +++ b/spec/services/author_credit_divergence_query_spec.rb @@ -0,0 +1,93 @@ +require "rails_helper" + +RSpec.describe AuthorCreditDivergenceQuery do + let(:author_user) { create(:user, :with_person) } + let(:person) { author_user.person } + + # Snapshot "first_name_only", then move the profile so the two disagree. + def diverged_story + person.update!(display_name_preference: "first_name_only") + story = create(:story, created_by: author_user, author: person, author_credit_preference: nil) + person.update!(display_name_preference: "full_name") + story + end + + describe "#call" do + it "returns nothing when every snapshot matches its profile" do + person.update!(display_name_preference: "full_name") + create(:story, created_by: author_user, author: person, author_credit_preference: nil) + + expect(described_class.new.call).to be_empty + end + + it "groups diverging records under their credited person" do + story = diverged_story + + groups = described_class.new.call + + expect(groups.size).to eq(1) + expect(groups.first.person).to eq(person) + expect(groups.first.records).to include(story) + end + + it "finds records credited through the creating user's person, not just author_id" do + person.update!(display_name_preference: "first_name_only") + idea = create(:story_idea, created_by: author_user, author_credit_preference: nil) + person.update!(display_name_preference: "full_name") + + expect(described_class.new.call.first.records).to include(idea) + end + + it "ignores records with no stored snapshot" do + create(:story, created_by: author_user, author: person, author_credit_preference: nil) + Story.update_all(author_credit_preference: nil) + + expect(described_class.new.call).to be_empty + end + + it "suggests the most restrictive preference across the person's records" do + diverged_story + create(:story, created_by: author_user, author: person, author_credit_preference: "anonymous") + + expect(described_class.new.call.first.suggested_preference).to eq("anonymous") + end + end + + describe "filters" do + before { diverged_story } + + it "filters by person_id" do + expect(described_class.new(person_id: person.id).call.size).to eq(1) + expect(described_class.new(person_id: person.id + 9999).call).to be_empty + end + + it "filters by type" do + expect(described_class.new(type: "Story").call.size).to eq(1) + expect(described_class.new(type: "Resource").call).to be_empty + end + + it "filters by stored preference" do + expect(described_class.new(preference: "first_name_only").call.size).to eq(1) + expect(described_class.new(preference: "anonymous").call).to be_empty + end + + it "hides reconciled people unless asked for" do + person.update!(author_credit_reconciled_at: Time.current) + + expect(described_class.new.call).to be_empty + expect(described_class.new(include_reconciled: "1").call.size).to eq(1) + end + end + + describe ".model_for" do + it "resolves an allowlisted name" do + expect(described_class.model_for("Story")).to eq(Story) + end + + it "refuses anything else rather than constantizing it" do + expect(described_class.model_for("User")).to be_nil + expect(described_class.model_for("Kernel")).to be_nil + expect(described_class.model_for("NotAClass")).to be_nil + end + end +end diff --git a/spec/support/shared_examples/author_creditable.rb b/spec/support/shared_examples/author_creditable.rb index 4d8141bc03..bf49177476 100644 --- a/spec/support/shared_examples/author_creditable.rb +++ b/spec/support/shared_examples/author_creditable.rb @@ -4,68 +4,64 @@ let(:person) { author_user.person } let(:record) { create(factory, created_by: author_user) } - context "when author_credit_preference is full_name" do - it "returns the person's full name" do - record.update!(author_credit_preference: "full_name") + context "when the profile formats the name" do + it "returns the full name for full_name" do + person.update!(display_name_preference: "full_name") expect(record.author_credit).to eq(person.full_name) end - end - context "when author_credit_preference is first_name_last_initial" do - it "returns first name and last initial with period" do - record.update!(author_credit_preference: "first_name_last_initial") + it "returns first name and last initial with period for first_name_last_initial" do + person.update!(display_name_preference: "first_name_last_initial") expect(record.author_credit).to eq("#{person.first_name} #{person.last_name.first}.") end - end - context "when author_credit_preference is first_name_only" do - it "returns the person's first name" do - record.update!(author_credit_preference: "first_name_only") + it "returns the first name for first_name_only" do + person.update!(display_name_preference: "first_name_only") expect(record.author_credit).to eq(person.first_name) end - end - context "when author_credit_preference is last_name_only" do - it "returns the person's last name" do - record.update!(author_credit_preference: "last_name_only") + it "returns the last name for last_name_only" do + person.update!(display_name_preference: "last_name_only") expect(record.author_credit).to eq(person.last_name) end + + it "falls back to the full name when the profile has no preference" do + person.update!(display_name_preference: nil) + expect(record.author_credit).to eq(person.full_name) + end end - context "when author_credit_preference is anonymous" do - it "returns Anonymous" do - record.update!(author_credit_preference: "anonymous") + context "when the profile marks contributions anonymous" do + before { person.update!(contributions_anonymous: true) } + + it "returns Anonymous regardless of the name format" do + person.update!(display_name_preference: "full_name") expect(record.author_credit).to eq("Anonymous") end + + it "does not link the credit to a profile" do + expect(record.author_credit_person).to be_nil + end end - if described_class.require_author_credit_preference? - context "when the preference is unset (required)" do - it "is invalid without a credit preference" do - record.author_credit_preference = nil - expect(record).not_to be_valid - expect(record.errors[:author_credit_preference]).to be_present - end + context "when the record itself was submitted anonymously" do + before { record.update!(author_credit_preference: "anonymous") } + + it "stays anonymous even though the profile says otherwise" do + person.update!(display_name_preference: "full_name", contributions_anonymous: false) + expect(record.author_credit).to eq("Anonymous") + end - it "does not default new records" do - expect(described_class.new.author_credit_preference).to be_blank - end + it "does not link the credit to a profile" do + expect(record.author_credit_person).to be_nil end - else - context "when the preference is unset (defaulted)" do - it "defaults new records to full_name" do - expect(described_class.new.author_credit_preference).to eq("full_name") - end - - it "treats a blank preference as full_name at read time" do - record.author_credit_preference = nil - expect(record.author_credit).to eq(person.full_name) - end - - it "normalizes a blank preference to full_name on save (no backfill)" do - record.update!(author_credit_preference: nil) - expect(record.reload.author_credit_preference).to eq("full_name") - end + end + + context "when the stored preference is a name format" do + it "is ignored in favor of the profile" do + record.update!(author_credit_preference: "first_name_only") + person.update!(display_name_preference: "full_name") + expect(record.author_credit).to eq(person.full_name) end end @@ -87,20 +83,90 @@ end end + describe "the consent snapshot" do + let(:author_user) { create(:user, :with_person) } + let(:person) { author_user.person } + + it "records the profile's preference on create" do + person.update!(display_name_preference: "first_name_only") + record = create(factory, created_by: author_user, author_credit_preference: nil) + expect(record.reload.author_credit_preference).to eq("first_name_only") + end + + it "records anonymous when the profile suppresses credits" do + person.update!(contributions_anonymous: true) + record = create(factory, created_by: author_user, author_credit_preference: nil) + expect(record.reload.author_credit_preference).to eq("anonymous") + end + + it "does not overwrite a preference carried forward from an idea" do + record = create(factory, created_by: author_user, author_credit_preference: "last_name_only") + expect(record.reload.author_credit_preference).to eq("last_name_only") + end + + it "is left alone when the profile later changes, and reports the divergence" do + person.update!(display_name_preference: "first_name_only") + record = create(factory, created_by: author_user, author_credit_preference: nil) + + person.update!(display_name_preference: "full_name") + + expect(record.reload.author_credit_preference).to eq("first_name_only") + expect(record.author_credit_diverged?).to be(true) + expect(record.author_credit).to eq(person.full_name) + end + + it "reports no divergence when the snapshot matches the profile" do + person.update!(display_name_preference: "full_name") + record = create(factory, created_by: author_user, author_credit_preference: nil) + expect(record.author_credit_diverged?).to be(false) + end + end + describe ".by_credited_person_name" do let(:author_user) { create(:user, :with_person) } + let(:person) { author_user.person } let!(:record) { create(factory, created_by: author_user) } + before { person.update!(first_name: "Zephyrine", last_name: "Quixotel") } + it "matches the creating user's person by name" do - author_user.person.update!(first_name: "Zephyrine", last_name: "Quixotel") expect(described_class.by_credited_person_name("Zephyrine")).to include(record) expect(described_class.by_credited_person_name("Quixotel")).to include(record) end it "does not match an unrelated name" do - author_user.person.update!(first_name: "Zephyrine", last_name: "Quixotel") expect(described_class.by_credited_person_name("Nonexistententry")).not_to include(record) end + + it "matches nothing when the profile marks contributions anonymous" do + person.update!(contributions_anonymous: true) + expect(described_class.by_credited_person_name("Zephyrine")).not_to include(record) + expect(described_class.by_credited_person_name("Quixotel")).not_to include(record) + end + + it "matches nothing when the record was submitted anonymously" do + record.update!(author_credit_preference: "anonymous") + expect(described_class.by_credited_person_name("Zephyrine")).not_to include(record) + end + + it "does not match on last name when only the first name is shown" do + person.update!(display_name_preference: "first_name_only") + expect(described_class.by_credited_person_name("Zephyrine")).to include(record) + expect(described_class.by_credited_person_name("Quixotel")).not_to include(record) + end + + it "does not match on first name when only the last name is shown" do + person.update!(display_name_preference: "last_name_only") + expect(described_class.by_credited_person_name("Quixotel")).to include(record) + expect(described_class.by_credited_person_name("Zephyrine")).not_to include(record) + end + + it "matches only the initial when the last name is reduced to one" do + person.update!(display_name_preference: "first_name_last_initial") + expect(described_class.by_credited_person_name("Zephyrine")).to include(record) + expect(described_class.by_credited_person_name("ZephyrineQ")).to include(record) + expect(described_class.by_credited_person_name("Quixotel")).not_to include(record) + end end describe ".order_by_author" do diff --git a/spec/views/community_news/edit.html.erb_spec.rb b/spec/views/community_news/edit.html.erb_spec.rb index f8557da5af..6c65984a47 100644 --- a/spec/views/community_news/edit.html.erb_spec.rb +++ b/spec/views/community_news/edit.html.erb_spec.rb @@ -44,7 +44,6 @@ assert_select "select[name=?]", "community_news[author_id]" - assert_select "select[name=?]", "community_news[author_credit_preference]" assert_select "textarea[name=?]", "community_news[reference_url]" diff --git a/spec/views/community_news/new.html.erb_spec.rb b/spec/views/community_news/new.html.erb_spec.rb index 9b5e962916..a110886a37 100644 --- a/spec/views/community_news/new.html.erb_spec.rb +++ b/spec/views/community_news/new.html.erb_spec.rb @@ -40,7 +40,6 @@ assert_select "select[name=?]", "community_news[author_id]" - assert_select "select[name=?]", "community_news[author_credit_preference]" assert_select "textarea[name=?]", "community_news[reference_url]" diff --git a/spec/views/page_bg_class_alignment_spec.rb b/spec/views/page_bg_class_alignment_spec.rb index 7924fcd22a..dd4a64e68a 100644 --- a/spec/views/page_bg_class_alignment_spec.rb +++ b/spec/views/page_bg_class_alignment_spec.rb @@ -130,6 +130,7 @@ "app/views/event_registrations/index.html.erb" => "admin-only bg-blue-100", "app/views/forms/index.html.erb" => "admin-only bg-blue-100", "app/views/forms/show.html.erb" => "admin-only bg-blue-100", + "app/views/author_credit_divergences/index.html.erb" => "admin-only bg-blue-100", "app/views/notifications/index.html.erb" => "admin-only bg-blue-100", "app/views/notifications/new.html.erb" => "admin-only bg-blue-100", "app/views/organization_statuses/index.html.erb" => "admin-only bg-blue-100", diff --git a/spec/views/stories/edit.html.erb_spec.rb b/spec/views/stories/edit.html.erb_spec.rb index 3169e3fca1..87095efdcb 100644 --- a/spec/views/stories/edit.html.erb_spec.rb +++ b/spec/views/stories/edit.html.erb_spec.rb @@ -30,7 +30,6 @@ assert_select "textarea[name=?]", "story[youtube_url]" - assert_select "select[name=?]", "story[author_credit_preference]" assert_select "select[name=?]", "story[author_id]" end diff --git a/spec/views/stories/new.html.erb_spec.rb b/spec/views/stories/new.html.erb_spec.rb index bb3440a645..4e752fa903 100644 --- a/spec/views/stories/new.html.erb_spec.rb +++ b/spec/views/stories/new.html.erb_spec.rb @@ -128,11 +128,10 @@ expect(rendered).to include(story_idea.author_credit) end - it "displays story idea author credit preference" do + it "does not offer a per-item credit preference select" do render - expect(rendered).to include("author credit preference") - expect(rendered).to include(story_idea.author_credit_preference) + assert_select "select[name=?]", "story[author_credit_preference]", count: 0 end context "with sectors and categories from story idea" do diff --git a/spec/views/story_ideas/edit.html.erb_spec.rb b/spec/views/story_ideas/edit.html.erb_spec.rb index d87e97c297..4cf460e08f 100644 --- a/spec/views/story_ideas/edit.html.erb_spec.rb +++ b/spec/views/story_ideas/edit.html.erb_spec.rb @@ -28,7 +28,6 @@ assert_select "select[name=?]", "story_idea[workshop_id]" assert_select "input[name=?][type=?]", "story_idea[rhino_body]", "hidden" assert_select "textarea[name=?]", "story_idea[youtube_url]" - assert_select "select[name=?]", "story_idea[author_credit_preference]" end end From 836bf2abe53c1f1dc8180559a217ade1546db254 Mon Sep 17 00:00:00 2001 From: maebeale Date: Thu, 6 Aug 2026 14:19:47 -0400 Subject: [PATCH 03/12] Use the shared badge partial for the reconciled indicator Main introduced shared/_badge while this branch was in flight; the reconciled pill was hand-rolled. Co-Authored-By: Claude --- .../author_credit_divergences_results.html.erb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb index aa1dc4f2ee..a077df31cf 100644 --- a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb +++ b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb @@ -23,9 +23,10 @@
<% if person.author_credit_reconciled_at.present? %> - - Reconciled <%= person.author_credit_reconciled_at.strftime("%b %-d, %Y") %> - + <%= render "shared/badge", + label: "Reconciled #{person.author_credit_reconciled_at.strftime('%b %-d, %Y')}", + classes: "bg-gray-50 text-gray-600 border-gray-200", + icon: "fa-solid fa-circle-info" %> <% end %>
From 05dba59fb1c2968bb374b01b6711e49943440136 Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 8 Aug 2026 07:35:57 -0400 Subject: [PATCH 04/12] Group credit divergences into resolvable sections with a person search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sort each person's suggested preference into the worklist and name it inline (e.g. "Suggested: First name only — most restrictive across Maria Johnson's content") so the hint is specific to the person rather than a generic line. Add a person name-search picker to the filters and order groups by first then last name. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../author_credit_divergences_controller.rb | 25 +- .../author_credit_divergences_helper.rb | 15 ++ .../author_credit_divergence_policy.rb | 4 + .../author_credit_divergence_query.rb | 127 +++++++--- .../_assign_author_form.html.erb | 18 ++ .../_filters.html.erb | 76 +++--- .../_preference_group.html.erb | 93 ++++++++ .../_section_clear.html.erb | 10 + .../_unlinked_table.html.erb | 29 +++ ...author_credit_divergences_results.html.erb | 221 ++++++++++-------- .../author_credit_divergences/index.html.erb | 9 +- config/routes.rb | 1 + .../author_credit_divergences_spec.rb | 54 +++++ .../author_credit_divergences_routing_spec.rb | 5 + .../author_credit_divergence_query_spec.rb | 114 ++++++++- 15 files changed, 625 insertions(+), 176 deletions(-) create mode 100644 app/views/author_credit_divergences/_assign_author_form.html.erb create mode 100644 app/views/author_credit_divergences/_preference_group.html.erb create mode 100644 app/views/author_credit_divergences/_section_clear.html.erb create mode 100644 app/views/author_credit_divergences/_unlinked_table.html.erb diff --git a/app/controllers/author_credit_divergences_controller.rb b/app/controllers/author_credit_divergences_controller.rb index daffcfc93f..62fac3bbce 100644 --- a/app/controllers/author_credit_divergences_controller.rb +++ b/app/controllers/author_credit_divergences_controller.rb @@ -4,7 +4,7 @@ class AuthorCreditDivergencesController < ApplicationController FILTER_KEYS = %i[person_id type preference include_reconciled].freeze def index - @groups = AuthorCreditDivergenceQuery.new(**filters.symbolize_keys).call + @result = AuthorCreditDivergenceQuery.new(**filters.symbolize_keys).call return unless turbo_frame_request? render :author_credit_divergences_results @@ -43,6 +43,29 @@ def update_item end end + # Point a record at a real person. This is the fix for every section below the + # first: an author_id is the only credit path that follows the person's profile, + # links to it, and lists the record there. Once set, any legacy free-text name on + # the record stops being used. + def assign_author + model = AuthorCreditDivergenceQuery.model_for(params[:record_type]) + return redirect_to(author_credit_divergences_path(filters), alert: "Unknown record type.") unless model + + record = model.find(params[:record_id]) + person = Person.find_by(id: params[:author_id]) + return redirect_to(author_credit_divergences_path(filters), alert: "Choose a person to credit.") unless person + + record.author_id = person.id + record.updated_by = current_user if record.respond_to?(:updated_by=) + + if record.save + redirect_to author_credit_divergences_path(filters), + notice: "Credited #{model.name.underscore.humanize.downcase} ##{record.id} to #{person.full_name}." + else + redirect_to author_credit_divergences_path(filters), alert: record.errors.full_messages.to_sentence + end + end + private def authorize_page diff --git a/app/helpers/author_credit_divergences_helper.rb b/app/helpers/author_credit_divergences_helper.rb index 3c3dc75e54..03599144ea 100644 --- a/app/helpers/author_credit_divergences_helper.rb +++ b/app/helpers/author_credit_divergences_helper.rb @@ -4,6 +4,21 @@ def divergence_record_title(record) record.try(:title).presence || record.try(:name).presence || "##{record.id}" end + # An empty page means "nothing left to reconcile" only when nothing is filtered + # out — otherwise the congratulations would be reporting on the filter. + def divergence_filters_applied? + AuthorCreditDivergencesController::FILTER_KEYS.any? { |key| params[key].present? } + end + + # New tab rather than an eyebrow: these rows link to 8 different destinations, + # none of which carries a return_to today. + def divergence_record_link(record) + link_to divergence_record_title(record), polymorphic_path(record), + target: "_blank", rel: "noopener", + title: "Opens in a new tab", + class: "text-blue-700 hover:underline" + end + # The suggestion is the most restrictive preference across the person's content, # but `anonymous` isn't a name format — it's the separate checkbox — so fall back # to the profile's current format when that's what was suggested. diff --git a/app/policies/author_credit_divergence_policy.rb b/app/policies/author_credit_divergence_policy.rb index 5d78b597cf..52089688b6 100644 --- a/app/policies/author_credit_divergence_policy.rb +++ b/app/policies/author_credit_divergence_policy.rb @@ -10,4 +10,8 @@ def update_person? def update_item? admin? end + + def assign_author? + admin? + end end diff --git a/app/services/author_credit_divergence_query.rb b/app/services/author_credit_divergence_query.rb index 1a07a6fa64..65ae6e3e47 100644 --- a/app/services/author_credit_divergence_query.rb +++ b/app/services/author_credit_divergence_query.rb @@ -1,11 +1,19 @@ -# Finds content whose stored `author_credit_preference` no longer agrees with the -# credited person's profile, grouped by person so an admin can resolve a whole -# person at once. +# Everything on the author credit divergences page: content whose credit doesn't +# resolve cleanly through the credited person's profile. # -# The comparison runs in Ruby rather than SQL because it walks the author fallback -# chain (explicit author, then the creating user's person). The candidate set is -# small: the column was added with no default and no backfill, so most legacy rows -# are NULL and only the *_idea tables hold a real spread. +# Four sections, in the order an admin should work them. The first is about a +# preference that drifted; the other three are all forms of "this credit isn't +# coming from an author_id", which is the only path that links to a profile and +# lists the record on it. +# +# preference — stored consent snapshot no longer matches the profile +# legacy — credited by a free-text name column, no person at all +# creator — author_id is blank, so the credit falls back to the creator +# unattributed — nothing to credit; renders the model's missing_author_label +# +# Comparisons run in Ruby because they walk the author fallback chain. The +# candidate sets are small: the preference column was added with no default and +# no backfill, and the legacy columns only hold pre-person data. class AuthorCreditDivergenceQuery # Every AuthorCreditable model. Doubles as the allowlist for the `type` param — # never constantize a raw param. @@ -20,8 +28,10 @@ class AuthorCreditDivergenceQuery CommunityNews ].freeze + SECTIONS = %w[preference legacy creator unattributed].freeze + # Which preference reveals the least, for suggesting a profile value that - # satisfies all of a person's items. + # satisfies all of a person's content. RESTRICTIVENESS = { "anonymous" => 4, "last_name_only" => 3, @@ -30,7 +40,12 @@ class AuthorCreditDivergenceQuery "full_name" => 1 }.freeze - Group = Struct.new(:person, :records, :suggested_preference, keyword_init: true) + PersonGroup = Struct.new(:person, :records, :suggested_preference, keyword_init: true) + Result = Struct.new(:preference, :legacy, :creator, :unattributed, keyword_init: true) do + def empty? + preference.empty? && legacy.empty? && creator.empty? && unattributed.empty? + end + end def self.model_for(type) MODEL_NAMES.include?(type.to_s) ? type.to_s.constantize : nil @@ -43,12 +58,13 @@ def initialize(person_id: nil, type: nil, preference: nil, include_reconciled: f @include_reconciled = ActiveModel::Type::Boolean.new.cast(include_reconciled) end - # => [Group] sorted by the person's name def call - diverged_records - .group_by(&:author_person) - .filter_map { |person, records| build_group(person, records) } - .sort_by { |group| group.person.full_name.to_s.downcase } + Result.new( + preference: preference_groups, + legacy: legacy_records, + creator: creator_groups, + unattributed: unattributed_records + ) end private @@ -56,21 +72,18 @@ def call attr_reader :person_id, :type, :preference, :include_reconciled def models - return [ self.class.model_for(type) ].compact if type - MODEL_NAMES.map(&:constantize) + @models ||= type ? [ self.class.model_for(type) ].compact : MODEL_NAMES.map(&:constantize) end - def diverged_records - models.flat_map { |model| diverged_for(model) } + # Only models that actually have an author_id can be "missing" one — the idea + # models have no such column, so crediting through the creator is their normal + # and correct behavior, not something to resolve. + def authorable_models + models.select { |model| model.column_names.include?("author_id") } end - # No person_id filter here — a record can be credited through `author_id` *or* - # through the creating user's person, so the filter has to run after the - # fallback chain resolves. See `build_group`. - def diverged_for(model) - scope = model.where.not(author_credit_preference: nil) - scope = scope.where(author_credit_preference: preference) if preference - scope.includes(includes_for(model)).select(&:author_credit_diverged?) + def scoped(model) + model.includes(includes_for(model)) end def includes_for(model) @@ -79,19 +92,75 @@ def includes_for(model) includes end + # ── Section 1: the stored snapshot drifted from the profile ──────────────── + def preference_groups + records = models.flat_map do |model| + scope = scoped(model).where.not(author_credit_preference: nil) + scope = scope.where(author_credit_preference: preference) if preference + scope.select(&:author_credit_diverged?) + end + + group_by_person(records) + end + + # ── Section 2: credited by a free-text name, with no person behind it ────── + # The person filter can't apply here: these records have no credited person, + # which is the whole problem with them. + def legacy_records + return [] if preference || person_id + + authorable_models.flat_map do |model| + next [] if model.legacy_author_name_columns.empty? + sorted(scoped(model).where(author_id: nil).select { |record| record.legacy_author_name_text.present? }) + end + end + + # ── Section 3: author_id blank, so the credit falls back to the creator ──── + def creator_groups + records = authorable_models.flat_map do |model| + scoped(model) + .where(author_id: nil) + .select { |record| record.legacy_author_name_text.blank? && record.created_by&.person.present? } + end + + group_by_person(records) + end + + # ── Section 4: nothing to credit at all ──────────────────────────────────── + def unattributed_records + return [] if preference || person_id + + authorable_models.flat_map do |model| + sorted(scoped(model) + .where(author_id: nil) + .select { |record| record.legacy_author_name_text.blank? && record.created_by&.person.blank? }) + end + end + + def group_by_person(records) + records + .group_by(&:author_person) + .filter_map { |person, grouped| build_group(person, grouped) } + .sort_by { |group| [ group.person.first_name.to_s.downcase, group.person.last_name.to_s.downcase ] } + end + def build_group(person, records) return nil if person.blank? - return nil if person_id && person.id != person_id.to_i + return nil if person_id.present? && person.id != person_id.to_i return nil if person.author_credit_reconciled_at.present? && !include_reconciled - Group.new( + PersonGroup.new( person: person, - records: records.sort_by { |record| [ record.class.name, record.id ] }, + records: sorted(records), suggested_preference: most_restrictive(records) ) end + def sorted(records) + records.sort_by { |record| [ record.class.name, record.id ] } + end + def most_restrictive(records) - records.map(&:author_credit_preference).max_by { |value| RESTRICTIVENESS.fetch(value, 0) } + records.map(&:author_credit_preference).compact.max_by { |value| RESTRICTIVENESS.fetch(value, 0) } end end diff --git a/app/views/author_credit_divergences/_assign_author_form.html.erb b/app/views/author_credit_divergences/_assign_author_form.html.erb new file mode 100644 index 0000000000..66a562ad12 --- /dev/null +++ b/app/views/author_credit_divergences/_assign_author_form.html.erb @@ -0,0 +1,18 @@ +<%# Point one record at a real person. Locals: record (required), suggested (optional + Person to preselect, e.g. the creator on the "credited to the creator" section). %> +<% suggested = local_assigns[:suggested] %> +<%= form_with url: assign_author_author_credit_divergences_path, method: :patch, + data: { turbo_frame: "_top" }, + class: "flex items-center gap-2" do %> + <%= hidden_field_tag :record_type, record.class.name, id: nil %> + <%= hidden_field_tag :record_id, record.id, id: nil %> + <%= render "filter_fields" %> + <%= select_tag :author_id, + options_for_select(suggested ? [ [ suggested.full_name, suggested.id ] ] : [], suggested&.id), + include_blank: "Select a person", + class: "rounded-md border-gray-300 text-sm", + id: nil, + data: { controller: "remote-select", remote_select_model_value: "person" } %> + <%= submit_tag "Credit", + class: "rounded-md border border-gray-300 bg-white px-2 py-1 text-xs text-gray-700 hover:bg-gray-50 cursor-pointer" %> +<% end %> diff --git a/app/views/author_credit_divergences/_filters.html.erb b/app/views/author_credit_divergences/_filters.html.erb index f45f5bfc97..7a23b66be1 100644 --- a/app/views/author_credit_divergences/_filters.html.erb +++ b/app/views/author_credit_divergences/_filters.html.erb @@ -1,36 +1,48 @@ -<%= form_with url: author_credit_divergences_path, method: :get, - data: { turbo_frame: "author_credit_divergences_results" }, - class: "flex flex-wrap items-end gap-3 mb-6" do |f| %> -
- <%= f.label :type, "Content type", class: "block text-sm font-medium text-gray-700" %> - <%= f.select :type, - options_for_select(AuthorCreditDivergenceQuery::MODEL_NAMES.map { |name| [ name.underscore.humanize, name ] }, params[:type]), - { include_blank: "All types" }, - class: "mt-1 rounded-md border-gray-300 text-sm" %> -
+
+ <%# The collection controller auto-submits into the results frame, so filters + apply as you type/pick — no Filter button needed. %> + <%= form_with url: author_credit_divergences_path, method: :get, + data: { controller: "collection", turbo_frame: "author_credit_divergences_results" }, + html: { autocomplete: "off" }, + class: "flex flex-wrap items-end gap-3" do |f| %> +
+ + Filter +
-
- <%= f.label :preference, "Stored preference", class: "block text-sm font-medium text-gray-700" %> - <%= f.select :preference, - options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, params[:preference]), - { include_blank: "Any preference" }, - class: "mt-1 rounded-md border-gray-300 text-sm" %> -
+
+ <%= f.label :type, "Content type", class: "block text-sm font-medium text-gray-700" %> + <%= f.select :type, + options_for_select(AuthorCreditDivergenceQuery::MODEL_NAMES.map { |name| [ name.underscore.humanize, name ] }, params[:type]), + { include_blank: "All types" }, + class: "mt-1 rounded-md border-gray-300 text-sm" %> +
-
- <%= f.label :person_id, "Person ID", class: "block text-sm font-medium text-gray-700" %> - <%= f.number_field :person_id, value: params[:person_id], placeholder: "Any person", - class: "mt-1 rounded-md border-gray-300 text-sm w-32" %> -
+
+ <%= f.label :preference, "Stored preference", class: "block text-sm font-medium text-gray-700" %> + <%= f.select :preference, + options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, params[:preference]), + { include_blank: "Any preference" }, + class: "mt-1 rounded-md border-gray-300 text-sm" %> +
- +
+ <%= f.label :person_id, "Person", class: "block text-sm font-medium text-gray-700" %> + <%= select_tag :person_id, + options_for_select(Person.where(id: params[:person_id]).map { |person| [ person.full_name, person.id ] }, params[:person_id]), + include_blank: true, prompt: "Search for a person", + class: "mt-1 block w-full rounded-md border-gray-300 text-sm", + data: { controller: "remote-select", remote_select_model_value: "person" } %> +
- <%= f.submit "Filter", class: "rounded-md bg-blue-600 px-4 py-2 text-sm font-medium text-white hover:bg-blue-700 cursor-pointer" %> - <%= link_to "Clear", author_credit_divergences_path, - data: { turbo_frame: "author_credit_divergences_results" }, - class: "text-sm text-gray-600 underline pb-2" %> -<% end %> + + + <%= link_to "Clear", author_credit_divergences_path, + data: { action: "collection#clearAndSubmit" }, + class: "text-sm text-gray-600 underline pb-2" %> + <% end %> +
diff --git a/app/views/author_credit_divergences/_preference_group.html.erb b/app/views/author_credit_divergences/_preference_group.html.erb new file mode 100644 index 0000000000..0c221141a3 --- /dev/null +++ b/app/views/author_credit_divergences/_preference_group.html.erb @@ -0,0 +1,93 @@ +<% person = group.person %> +
+
+
+ <%= link_to person.full_name, person_path(person), class: "font-semibold text-gray-900 hover:underline" %> + + — profile currently credits as + <%= AuthorCreditable::ADMIN_FORM_OPTIONS.key(person.effective_author_credit_preference) %> + +
+ <% if person.author_credit_reconciled_at.present? %> + <%= render "shared/badge", + label: "Reconciled #{person.author_credit_reconciled_at.strftime('%b %-d, %Y')}", + classes: "bg-gray-50 text-gray-600 border-gray-200", + icon: "fa-solid fa-circle-info" %> + <% end %> +
+ + + + + + + + + + + + <% group.records.each do |record| %> + + + + + + + <% end %> + +
ContentTypeRenders asStored consent
<%= divergence_record_link(record) %><%= record.class.name.underscore.humanize %><%= record.author_credit %> + <%= form_with url: update_item_author_credit_divergences_path, method: :patch, + data: { turbo_frame: "_top" }, + class: "flex items-center gap-2" do %> + <%= hidden_field_tag :record_type, record.class.name, id: nil %> + <%= hidden_field_tag :record_id, record.id, id: nil %> + <%= render "filter_fields" %> + <%= select_tag "author_credit_preference", + options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, record.author_credit_preference), + id: nil, + class: "rounded-md border-gray-300 text-sm" %> + <%= submit_tag "Save", + class: "rounded-md border border-gray-300 bg-white px-2 py-1 text-xs text-gray-700 hover:bg-gray-50 cursor-pointer" %> + <% end %> +
+

+ Only Anonymous changes what renders — the other values just + re-record what was consented to. +

+ + <%# The action lands after the supportive data above: read the content, then apply. %> +
+ <%= form_with url: update_person_author_credit_divergences_path, method: :patch, + data: { turbo_frame: "_top" }, + class: "flex flex-wrap items-end gap-3" do %> + <%= hidden_field_tag :id, person.id, id: nil %> + <%= render "filter_fields" %> + +
+ <%= label_tag "person_display_name_preference_#{person.id}", "Set profile to", + class: "block text-xs font-medium text-gray-700" %> + <%= select_tag "person[display_name_preference]", + options_for_select(Person::DISPLAY_NAME_PREFERENCE_LABELS.invert.to_a, + suggested_display_name_preference(group)), + id: "person_display_name_preference_#{person.id}", + class: "mt-1 rounded-md border-gray-300 text-sm" %> +
+ + + + <%= submit_tag "Apply to profile", + class: "rounded-md bg-blue-600 px-3 py-2 text-sm font-medium text-white hover:bg-blue-700 cursor-pointer" %> + + Suggested: <%= AuthorCreditable::ADMIN_FORM_OPTIONS.key(group.suggested_preference) %> + — the most restrictive consent across <%= person.full_name %>'s content. + + <% end %> +
+
diff --git a/app/views/author_credit_divergences/_section_clear.html.erb b/app/views/author_credit_divergences/_section_clear.html.erb new file mode 100644 index 0000000000..62a838713d --- /dev/null +++ b/app/views/author_credit_divergences/_section_clear.html.erb @@ -0,0 +1,10 @@ +<%# All-clear note for one section. Locals: message (required), cleanup (optional — + the code that can now be retired, which is the real payoff of clearing it). %> +
+

+ <%= message %> +

+ <% if local_assigns[:cleanup].present? %> +

<%= cleanup %>

+ <% end %> +
diff --git a/app/views/author_credit_divergences/_unlinked_table.html.erb b/app/views/author_credit_divergences/_unlinked_table.html.erb new file mode 100644 index 0000000000..b94892b891 --- /dev/null +++ b/app/views/author_credit_divergences/_unlinked_table.html.erb @@ -0,0 +1,29 @@ +<%# Records with no author_id, each resolvable by crediting a person. Locals: + records (required), name_header (required), suggest_creator (required — + preselect the creating user's person in the picker). %> +
+ + + + + + + + + + + <% records.each do |record| %> + + + + + + + <% end %> + +
ContentType<%= name_header %>Credit to
<%= divergence_record_link(record) %><%= record.class.name.underscore.humanize %><%= record.author_credit %> + <%= render "assign_author_form", + record: record, + suggested: (record.created_by&.person if suggest_creator) %> +
+
diff --git a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb index a077df31cf..5d9da2d582 100644 --- a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb +++ b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb @@ -1,113 +1,138 @@ <%= turbo_frame_tag :author_credit_divergences_results do %> - <% if @groups.empty? %> + <% if @result.empty? && divergence_filters_applied? %>
- -

Nothing to reconcile.

-

Every stored credit preference matches its author's profile.

+ +

Nothing matches these filters.

+

+ Clear them to see whether anything is left to reconcile overall. +

+
+ <% elsif @result.empty? %> +
+ +

Every credit resolves through a profile.

+

+ Nothing left to reconcile in any section. Ask your developers to remove any unused + author credit code — the stored author_credit_preference columns, the + legacy free-text name columns, and the creator and missing-author fallbacks in + AuthorCreditable. +

<% else %> -

- <%= pluralize(@groups.size, "person") %> with diverging content. -

- -
- <% @groups.each do |group| %> - <% person = group.person %> -
-
-
- <%= link_to person.full_name, person_path(person), class: "font-semibold text-gray-900 hover:underline" %> - - — profile currently credits as - <%= AuthorCreditable::ADMIN_FORM_OPTIONS.key(person.effective_author_credit_preference) %> - -
- <% if person.author_credit_reconciled_at.present? %> - <%= render "shared/badge", - label: "Reconciled #{person.author_credit_reconciled_at.strftime('%b %-d, %Y')}", - classes: "bg-gray-50 text-gray-600 border-gray-200", - icon: "fa-solid fa-circle-info" %> + <%# Section counts up front so the admin can see the full scope and jump to any section. %> + <% overview = [ + { anchor: "section-preference", num: 1, label: "Preference drift", count: @result.preference.size, unit: "person" }, + { anchor: "section-legacy", num: 2, label: "Legacy name", count: @result.legacy.size, unit: "record" }, + { anchor: "section-creator", num: 3, label: "Creator fallback", count: @result.creator.size, unit: "person" }, + { anchor: "section-unattributed", num: 4, label: "No author", count: @result.unattributed.size, unit: "record" } + ] %> +
+ <% overview.each do |section| %> + <% clear = section[:count].zero? %> + <%= link_to "##{section[:anchor]}", + class: "block rounded-lg border p-3 transition-colors #{clear ? "border-green-200 bg-green-50 hover:bg-green-100" : "border-gray-200 bg-white shadow-sm hover:bg-gray-50"}" do %> +
+ <%= section[:num] %>. <%= section[:label] %> + <% if clear %> + <% end %>
+
"><%= section[:count] %>
+
<%= clear ? "clear" : "#{section[:unit].pluralize(section[:count])} to review" %>
+ <% end %> + <% end %> +
-
- <%= form_with url: update_person_author_credit_divergences_path, method: :patch, - data: { turbo_frame: "_top" }, - class: "flex flex-wrap items-end gap-3" do |f| %> - <%= hidden_field_tag :id, person.id %> - <%= render "filter_fields" %> +

+ Work these top to bottom. A record can appear in more than one section when it needs + more than one fix. +

-
- <%= label_tag "person_display_name_preference_#{person.id}", "Set profile to", - class: "block text-xs font-medium text-gray-700" %> - <%= select_tag "person[display_name_preference]", - options_for_select(Person::DISPLAY_NAME_PREFERENCE_LABELS.invert.to_a, - suggested_display_name_preference(group)), - id: "person_display_name_preference_#{person.id}", - class: "mt-1 rounded-md border-gray-300 text-sm" %> -
+ <%# ── 1. Stored consent snapshot drifted from the profile ───────────────── %> +
+

1. Preference no longer matches the profile

+

+ These are credited correctly — the profile formats them — but the stored record of + what the submitter consented to has since drifted. Confirm which one is right. +

- + <% if @result.preference.empty? %> + <%= render "section_clear", + message: "No preference divergences. Every stored consent snapshot matches its author's profile.", + cleanup: "Ask your developers to remove any unused author credit code." %> + <% else %> +

<%= pluralize(@result.preference.size, "person") %> to review.

+
+ <%= render partial: "preference_group", collection: @result.preference, as: :group %> +
+ <% end %> +
- <%= submit_tag "Apply to profile", - class: "rounded-md bg-blue-600 px-3 py-2 text-sm font-medium text-white hover:bg-blue-700 cursor-pointer" %> - - Suggested: the most restrictive preference across their content. - - <% end %> -
+ <%# ── 2. Credited by a legacy free-text name ────────────────────────────── %> +
+

2. Credited by a legacy name, not a person

+

+ Pre-person records whose credit comes from a free-text column + (workshops.full_name, resources.legacy_author_name). The name shown + obeys nobody's profile and links nowhere. Crediting a real person replaces it. +

+ + <% if @result.legacy.empty? %> + <%= render "section_clear", + message: "No legacy author names left. Every record is credited to a real person.", + cleanup: "Ask your developers to remove any unused full_name code — the workshops.full_name and resources.legacy_author_name columns, their legacy_author_name_columns / legacy_author_name_text overrides, and the legacy branches in AuthorCreditable." %> + <% else %> + <%= render "unlinked_table", records: @result.legacy, name_header: "Legacy name", suggest_creator: false %> + <% end %> +
- - - - - - - - - - - <% group.records.each do |record| %> - - - - - - - <% end %> - -
ContentTypeRenders asStored consent
- <%= link_to divergence_record_title(record), polymorphic_path(record), - target: "_blank", rel: "noopener", - title: "Opens in a new tab", - class: "text-blue-700 hover:underline" %> - <%= record.class.name.underscore.humanize %><%= record.author_credit %> - <%= form_with url: update_item_author_credit_divergences_path, method: :patch, - data: { turbo_frame: "_top" }, - class: "flex items-center gap-2" do %> - <%= hidden_field_tag :record_type, record.class.name %> - <%= hidden_field_tag :record_id, record.id %> - <%= render "filter_fields" %> - <%= select_tag "author_credit_preference", - options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, record.author_credit_preference), - class: "rounded-md border-gray-300 text-sm" %> - <%= submit_tag "Save", - class: "rounded-md border border-gray-300 bg-white px-2 py-1 text-xs text-gray-700 hover:bg-gray-50 cursor-pointer" %> - <% end %> -
-

- Only Anonymous changes what renders — the other values just - re-record what was consented to. -

+ <%# ── 3. author_id blank, credit falls back to the creator ──────────────── %> +
+

3. Credited to the creator, not an author

+

+ These have no author_id, so the credit falls back to whoever created the record. + The name is right and follows that person's profile, but it never links to them and the record + doesn't appear on their profile. Confirm the creator is the author, or pick someone else. + Idea records are excluded — they have no author_id, so the creator is their + correct and only credit path. +

+ + <% if @result.creator.empty? %> + <%= render "section_clear", + message: "No creator fallbacks. Every record names its author explicitly.", + cleanup: "Ask your developers to remove the created_by fallback in AuthorCreditable#author_credit." %> + <% else %> +

<%= pluralize(@result.creator.size, "person") %> to confirm.

+
+ <% @result.creator.each do |group| %> +
+
+ <%= link_to group.person.full_name, person_path(group.person), + class: "font-semibold text-gray-900 hover:underline" %> + — created <%= pluralize(group.records.size, "record") %> with no author set +
+ <%= render "unlinked_table", records: group.records, name_header: "Renders as", suggest_creator: true %> +
+ <% end %>
<% end %> -
+ + + <%# ── 4. Nothing to credit at all ───────────────────────────────────────── %> +
+

4. No author at all

+

+ No author, no legacy name, and no person behind the creating account, so these fall back to a + generic label like “AWBW Facilitator”. +

+ + <% if @result.unattributed.empty? %> + <%= render "section_clear", + message: "Nothing unattributed. Every record has someone to credit.", + cleanup: "Ask your developers to remove the missing_author_label fallbacks." %> + <% else %> + <%= render "unlinked_table", records: @result.unattributed, name_header: "Renders as", suggest_creator: false %> + <% end %> +
<% end %> <% end %> diff --git a/app/views/author_credit_divergences/index.html.erb b/app/views/author_credit_divergences/index.html.erb index 8b51a3e646..0938b8e64c 100644 --- a/app/views/author_credit_divergences/index.html.erb +++ b/app/views/author_credit_divergences/index.html.erb @@ -4,10 +4,11 @@

Author credit divergences

- Content whose stored credit preference no longer matches the author's profile. - Credits render using the profile, so these are historical records that need a - decision — except anonymous, which always wins and keeps that - item uncredited no matter what the profile says. + Content whose credit doesn't resolve cleanly through a person's profile — either the + stored preference drifted, or the credit isn't coming from an author_id at all. + An author_id is the only path that follows the person's profile, links to it, + and lists the record there. Note that anonymous always wins over the + profile and keeps that item uncredited.

diff --git a/config/routes.rb b/config/routes.rb index 2f0f9b7aa1..0bc4a800af 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -198,6 +198,7 @@ collection do patch :update_person patch :update_item + patch :assign_author end end resources :people do diff --git a/spec/requests/author_credit_divergences_spec.rb b/spec/requests/author_credit_divergences_spec.rb index 15d64f14ab..61a73dd5ae 100644 --- a/spec/requests/author_credit_divergences_spec.rb +++ b/spec/requests/author_credit_divergences_spec.rb @@ -73,6 +73,60 @@ end end + describe "PATCH /author_credit_divergences/assign_author" do + before { sign_in admin } + + let(:target) { create(:person, first_name: "Rosalind", last_name: "Franklin") } + + it "credits a legacy free-text record to a real person" do + workshop = create(:workshop, author: nil, full_name: "Marguerite Pre-Person") + + patch assign_author_author_credit_divergences_path, + params: { record_type: "Workshop", record_id: workshop.id, author_id: target.id } + + expect(workshop.reload.author).to eq(target) + expect(workshop.author_credit).to eq("Rosalind Franklin") + end + + it "credits a creator-fallback record so it links to the profile" do + story = create(:story, created_by: author_user, author: nil) + expect(story.author_credit_person).to be_nil + + patch assign_author_author_credit_divergences_path, + params: { record_type: "Story", record_id: story.id, author_id: person.id } + + expect(story.reload.author_credit_person).to eq(person) + end + + it "requires a person" do + story = create(:story, created_by: author_user, author: nil) + + patch assign_author_author_credit_divergences_path, + params: { record_type: "Story", record_id: story.id, author_id: "" } + + expect(flash[:alert]).to eq("Choose a person to credit.") + expect(story.reload.author).to be_nil + end + + it "refuses a type outside the allowlist" do + patch assign_author_author_credit_divergences_path, + params: { record_type: "User", record_id: admin.id, author_id: target.id } + + expect(flash[:alert]).to eq("Unknown record type.") + end + + it "rejects a non-admin" do + sign_out admin + sign_in regular_user + story = create(:story, created_by: author_user, author: nil) + + patch assign_author_author_credit_divergences_path, + params: { record_type: "Story", record_id: story.id, author_id: target.id } + + expect(story.reload.author).to be_nil + end + end + describe "PATCH /author_credit_divergences/update_item" do before { sign_in admin } diff --git a/spec/routing/author_credit_divergences_routing_spec.rb b/spec/routing/author_credit_divergences_routing_spec.rb index 984c747905..a08bcedb8d 100644 --- a/spec/routing/author_credit_divergences_routing_spec.rb +++ b/spec/routing/author_credit_divergences_routing_spec.rb @@ -11,6 +11,11 @@ .to route_to("author_credit_divergences#update_person") end + it "routes to #assign_author" do + expect(patch: "/author_credit_divergences/assign_author") + .to route_to("author_credit_divergences#assign_author") + end + it "routes to #update_item" do expect(patch: "/author_credit_divergences/update_item") .to route_to("author_credit_divergences#update_item") diff --git a/spec/services/author_credit_divergence_query_spec.rb b/spec/services/author_credit_divergence_query_spec.rb index b94d7c4cd6..2bf51098fd 100644 --- a/spec/services/author_credit_divergence_query_spec.rb +++ b/spec/services/author_credit_divergence_query_spec.rb @@ -17,13 +17,13 @@ def diverged_story person.update!(display_name_preference: "full_name") create(:story, created_by: author_user, author: person, author_credit_preference: nil) - expect(described_class.new.call).to be_empty + expect(described_class.new.call.preference).to be_empty end it "groups diverging records under their credited person" do story = diverged_story - groups = described_class.new.call + groups = described_class.new.call.preference expect(groups.size).to eq(1) expect(groups.first.person).to eq(person) @@ -35,21 +35,35 @@ def diverged_story idea = create(:story_idea, created_by: author_user, author_credit_preference: nil) person.update!(display_name_preference: "full_name") - expect(described_class.new.call.first.records).to include(idea) + expect(described_class.new.call.preference.first.records).to include(idea) end it "ignores records with no stored snapshot" do create(:story, created_by: author_user, author: person, author_credit_preference: nil) Story.update_all(author_credit_preference: nil) - expect(described_class.new.call).to be_empty + expect(described_class.new.call.preference).to be_empty + end + + it "orders groups by first name then last name" do + person.update!(first_name: "Zoe", last_name: "Adams") + diverged_story + early_user = create(:user, :with_person) + early_user.person.update!(first_name: "Ada", last_name: "Zimmerman") + early_user.person.update!(display_name_preference: "first_name_only") + create(:story, created_by: early_user, author: early_user.person, author_credit_preference: nil) + early_user.person.update!(display_name_preference: "full_name") + + names = described_class.new.call.preference.map { |group| group.person.first_name } + + expect(names).to eq(%w[Ada Zoe]) end it "suggests the most restrictive preference across the person's records" do diverged_story create(:story, created_by: author_user, author: person, author_credit_preference: "anonymous") - expect(described_class.new.call.first.suggested_preference).to eq("anonymous") + expect(described_class.new.call.preference.first.suggested_preference).to eq("anonymous") end end @@ -57,25 +71,101 @@ def diverged_story before { diverged_story } it "filters by person_id" do - expect(described_class.new(person_id: person.id).call.size).to eq(1) - expect(described_class.new(person_id: person.id + 9999).call).to be_empty + expect(described_class.new(person_id: person.id).call.preference.size).to eq(1) + expect(described_class.new(person_id: person.id + 9999).call.preference).to be_empty end it "filters by type" do - expect(described_class.new(type: "Story").call.size).to eq(1) - expect(described_class.new(type: "Resource").call).to be_empty + expect(described_class.new(type: "Story").call.preference.size).to eq(1) + expect(described_class.new(type: "Resource").call.preference).to be_empty end it "filters by stored preference" do - expect(described_class.new(preference: "first_name_only").call.size).to eq(1) - expect(described_class.new(preference: "anonymous").call).to be_empty + expect(described_class.new(preference: "first_name_only").call.preference.size).to eq(1) + expect(described_class.new(preference: "anonymous").call.preference).to be_empty end it "hides reconciled people unless asked for" do person.update!(author_credit_reconciled_at: Time.current) + expect(described_class.new.call.preference).to be_empty + expect(described_class.new(include_reconciled: "1").call.preference.size).to eq(1) + end + end + + describe "the legacy section" do + it "lists records credited by a free-text name with no person" do + workshop = create(:workshop, author: nil, full_name: "Marguerite Pre-Person") + + legacy = described_class.new.call.legacy + + expect(legacy).to include(workshop) + expect(workshop.author_credit).to eq("Marguerite Pre-Person") + end + + it "drops a record once a real author is credited" do + workshop = create(:workshop, author: nil, full_name: "Marguerite Pre-Person") + workshop.update!(author: person) + + expect(described_class.new.call.legacy).not_to include(workshop) + end + + it "excludes models that have no legacy column" do + create(:story, created_by: author_user, author: nil) + expect(described_class.new.call.legacy).to be_empty + end + end + + describe "the creator section" do + it "groups records whose author_id is blank under the creating person" do + story = create(:story, created_by: author_user, author: nil) + + groups = described_class.new.call.creator + + expect(groups.map(&:person)).to include(person) + expect(groups.find { |g| g.person == person }.records).to include(story) + end + + it "excludes records that already name an author" do + story = create(:story, created_by: author_user, author: person) + expect(described_class.new.call.creator.flat_map(&:records)).not_to include(story) + end + + it "excludes idea models, whose only credit path is the creator" do + idea = create(:story_idea, created_by: author_user) + expect(described_class.new.call.creator.flat_map(&:records)).not_to include(idea) + end + + it "excludes records covered by the legacy section instead" do + workshop = create(:workshop, created_by: author_user, author: nil, full_name: "Legacy Name") + expect(described_class.new.call.creator.flat_map(&:records)).not_to include(workshop) + end + end + + describe "the unattributed section" do + let(:personless_user) { create(:user, person: nil) } + + it "lists records with no author, no legacy name, and no creator person" do + story = create(:story, created_by: personless_user, author: nil) + + expect(described_class.new.call.unattributed).to include(story) + expect(story.author_credit).to eq(story.missing_author_label) + end + + it "drops a record once an author is credited" do + story = create(:story, created_by: personless_user, author: nil) + story.update!(author: person) + + expect(described_class.new.call.unattributed).not_to include(story) + end + end + + describe "#empty?" do + it "is true only when every section is clear" do expect(described_class.new.call).to be_empty - expect(described_class.new(include_reconciled: "1").call.size).to eq(1) + + create(:story, created_by: create(:user, person: nil), author: nil) + expect(described_class.new.call).not_to be_empty end end From f11e8e9b089d344fe3324ea960b14f5d35abb24a Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 8 Aug 2026 07:38:36 -0400 Subject: [PATCH 05/12] Snapshot in-progress work on profile-visibility-preferences Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 3e444aa138..55df1f3536 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -208,7 +208,7 @@ action, or `authorize! :workshop, to: :summary?`). - `WorkshopSearchService` — Complex filtering, sorting, pagination with ActionPolicy - `WorkshopFromIdeaService` — Converts WorkshopIdea to Workshop with asset migration - `WorkshopVariationFromIdeaService` — Variation creation from ideas -- `AuthorCreditDivergenceQuery` — Finds content whose stored `author_credit_preference` no longer matches the credited person's profile, grouped by person, for the admin reconciliation page. `MODEL_NAMES` doubles as the allowlist for the `type` param (never constantize a raw param) +- `AuthorCreditDivergenceQuery` — Backs the admin author credit divergences page. Returns four sections: `preference` (stored snapshot no longer matches the profile, grouped by person), `legacy` (credited by a free-text column — `workshops.full_name`, `resources.legacy_author_name`), `creator` (no `author_id`, so the credit falls back to the creating user's person — idea models excluded, since that's their only credit path), and `unattributed` (nothing to credit, renders `missing_author_label`). The last three all resolve by assigning an `author_id`, the only credit path that follows a profile and links to it. `MODEL_NAMES` doubles as the allowlist for the `type` param (never constantize a raw param) - `TaggingSearchService` — Search and filter tagging data - `PersonFromUserService` — Create Person from User account - `PersonCommentAggregator` — Unifies every comment connected to a person (their profile, event registrations, scholarships, CE registrations, topic subscriptions, and user account) into one newest-first `Comment` relation for the aggregated `/people/:id/all_comments` page From e54097fbe6f132851f7cd23d7170aa07ea00425c Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 8 Aug 2026 07:42:01 -0400 Subject: [PATCH 06/12] Restyle divergences filter bar to match the app's filter conventions Co-Authored-By: Claude Opus 4.8 (1M context) --- .../_filters.html.erb | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/app/views/author_credit_divergences/_filters.html.erb b/app/views/author_credit_divergences/_filters.html.erb index 7a23b66be1..5e3992464f 100644 --- a/app/views/author_credit_divergences/_filters.html.erb +++ b/app/views/author_credit_divergences/_filters.html.erb @@ -4,45 +4,43 @@ <%= form_with url: author_credit_divergences_path, method: :get, data: { controller: "collection", turbo_frame: "author_credit_divergences_results" }, html: { autocomplete: "off" }, - class: "flex flex-wrap items-end gap-3" do |f| %> -
- - Filter -
- -
- <%= f.label :type, "Content type", class: "block text-sm font-medium text-gray-700" %> + class: "flex flex-wrap items-end gap-6" do |f| %> +
+ <%= f.label :type, "Content type", class: "block text-xs font-semibold uppercase text-gray-500 tracking-wide mb-1" %> <%= f.select :type, options_for_select(AuthorCreditDivergenceQuery::MODEL_NAMES.map { |name| [ name.underscore.humanize, name ] }, params[:type]), { include_blank: "All types" }, - class: "mt-1 rounded-md border-gray-300 text-sm" %> + class: "w-full bg-white border border-gray-300 rounded-lg px-3 py-2 focus:ring-blue-500 focus:border-blue-500", + onchange: "this.form.requestSubmit()" %>
-
- <%= f.label :preference, "Stored preference", class: "block text-sm font-medium text-gray-700" %> +
+ <%= f.label :preference, "Stored preference", class: "block text-xs font-semibold uppercase text-gray-500 tracking-wide mb-1" %> <%= f.select :preference, options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, params[:preference]), { include_blank: "Any preference" }, - class: "mt-1 rounded-md border-gray-300 text-sm" %> + class: "w-full bg-white border border-gray-300 rounded-lg px-3 py-2 focus:ring-blue-500 focus:border-blue-500", + onchange: "this.form.requestSubmit()" %>
-
- <%= f.label :person_id, "Person", class: "block text-sm font-medium text-gray-700" %> +
+ <%= f.label :person_id, "Person", class: "block text-xs font-semibold uppercase text-gray-500 tracking-wide mb-1" %> <%= select_tag :person_id, options_for_select(Person.where(id: params[:person_id]).map { |person| [ person.full_name, person.id ] }, params[:person_id]), include_blank: true, prompt: "Search for a person", - class: "mt-1 block w-full rounded-md border-gray-300 text-sm", + class: "w-full bg-white border border-gray-300 rounded-lg px-3 py-2 focus:ring-blue-500 focus:border-blue-500", data: { controller: "remote-select", remote_select_model_value: "person" } %>
-
From 9d31510c58fa87b03af9ee1e593390b8503ab5d8 Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 8 Aug 2026 07:59:58 -0400 Subject: [PATCH 07/12] Open divergences person links in a new tab, like the content links Keeps the admin worklist open while checking a profile, matching the content links that already open in a new tab. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../author_credit_divergences/_preference_group.html.erb | 7 +++++-- .../author_credit_divergences_results.html.erb | 1 + app/views/shared/_author_credit_warning.html.erb | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/views/author_credit_divergences/_preference_group.html.erb b/app/views/author_credit_divergences/_preference_group.html.erb index 0c221141a3..72a3ceb041 100644 --- a/app/views/author_credit_divergences/_preference_group.html.erb +++ b/app/views/author_credit_divergences/_preference_group.html.erb @@ -1,8 +1,11 @@ <% person = group.person %> -
+<% highlighted = params[:highlight].to_s == person.id.to_s %> +
">
- <%= link_to person.full_name, person_path(person), class: "font-semibold text-gray-900 hover:underline" %> + <%= link_to person.full_name, person_path(person), + target: "_blank", rel: "noopener", title: "Opens in a new tab", + class: "font-semibold text-gray-900 hover:underline" %> — profile currently credits as <%= AuthorCreditable::ADMIN_FORM_OPTIONS.key(person.effective_author_credit_preference) %> diff --git a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb index 5d9da2d582..0d36c3a105 100644 --- a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb +++ b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb @@ -108,6 +108,7 @@
<%= link_to group.person.full_name, person_path(group.person), + target: "_blank", rel: "noopener", title: "Opens in a new tab", class: "font-semibold text-gray-900 hover:underline" %> — created <%= pluralize(group.records.size, "record") %> with no author set
diff --git a/app/views/shared/_author_credit_warning.html.erb b/app/views/shared/_author_credit_warning.html.erb index 25d19d9789..5c6aedae6f 100644 --- a/app/views/shared/_author_credit_warning.html.erb +++ b/app/views/shared/_author_credit_warning.html.erb @@ -9,14 +9,14 @@
Submitted anonymously. This item stays anonymous regardless of the profile setting. - <%= link_to "Reconcile", author_credit_divergences_path(person_id: person.id), + <%= link_to "Reconcile", author_credit_divergences_path(person_id: person.id, highlight: person.id, anchor: dom_id(person, :divergence)), class: "underline hover:text-gray-700" %>
<% else %>
⚠ Submitted as "<%= stored %>", but this profile is now set to "<%= profile %>". This item is credited using the profile setting. - <%= link_to "Reconcile", author_credit_divergences_path(person_id: person.id), + <%= link_to "Reconcile", author_credit_divergences_path(person_id: person.id, highlight: person.id, anchor: dom_id(person, :divergence)), class: "underline hover:text-amber-900" %>
<% end %> From 35719c3260c07eba48ba3fccd0b94451453e109f Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 8 Aug 2026 08:46:34 -0400 Subject: [PATCH 08/12] Report each legacy author column separately, with a suggested person Clearing a single column is what makes that column safe to drop, so each one gets its own subsection and its own all-clear note naming the field. Guess who each free-text name refers to so an admin confirms rather than looks every one up, resolved in one query for the page. Co-Authored-By: Claude --- .../author_credit_divergences_controller.rb | 9 +++ app/controllers/people_controller.rb | 19 +++++-- .../author_credit_divergences_helper.rb | 9 +++ app/models/concerns/author_creditable.rb | 3 + .../author_credit_divergence_query.rb | 57 +++++++++++++++++-- .../_legacy_group.html.erb | 19 +++++++ .../_preference_group.html.erb | 6 +- .../_unlinked_table.html.erb | 19 ++++--- ...author_credit_divergences_results.html.erb | 23 ++++---- app/views/people/_form.html.erb | 4 +- app/views/people/_show_card.html.erb | 12 +++- app/views/people/sections/_resources.html.erb | 3 +- app/views/people/sections/_stories.html.erb | 5 +- .../sections/_workshop_variations.html.erb | 3 +- app/views/people/sections/_workshops.html.erb | 3 +- .../author_credit_divergences_spec.rb | 19 +++++++ spec/requests/people_stories_section_spec.rb | 20 +++++++ .../requests/people_workshops_section_spec.rb | 31 ++++++++++ .../author_credit_divergence_query_spec.rb | 41 +++++++++++-- .../shared_examples/author_creditable.rb | 6 ++ 20 files changed, 269 insertions(+), 42 deletions(-) create mode 100644 app/views/author_credit_divergences/_legacy_group.html.erb diff --git a/app/controllers/author_credit_divergences_controller.rb b/app/controllers/author_credit_divergences_controller.rb index 62fac3bbce..f5e0ebbec8 100644 --- a/app/controllers/author_credit_divergences_controller.rb +++ b/app/controllers/author_credit_divergences_controller.rb @@ -33,6 +33,15 @@ def update_item return redirect_to(author_credit_divergences_path(filters), alert: "Unknown record type.") unless model record = model.find(params[:record_id]) + + # Anonymity is a one-way latch: clearing the snapshot of an item submitted + # anonymously would silently de-anonymize it. A deliberate re-credit still works + # by picking an explicit preference. + if params[:author_credit_preference].blank? && record.author_credit_preference == AuthorCreditable::ANONYMOUS + return redirect_to(author_credit_divergences_path(filters), + alert: "Can't clear the consent for an item submitted anonymously — pick an explicit preference instead.") + end + record.author_credit_preference = params[:author_credit_preference] record.updated_by = current_user if record.respond_to?(:updated_by=) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 4162a5198e..7efd7eb48d 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -52,24 +52,26 @@ def show when "workshops" # Credit the person for workshops they authored — not ones their user # merely created (created_by is a pure audit trail). - @workshops = @person.workshops_as_author.order(created_at: :desc).paginate(page: params[:page], per_page: per_page) + @workshops = visible_authored_content(@person.workshops_as_author).order(created_at: :desc).paginate(page: params[:page], per_page: per_page) render partial: "people/sections/workshops", locals: { person: @person, workshops: @workshops } when "workshop_variations" # Credit the person for variations they authored — not ones their user # merely entered (created_by is a pure audit trail). - @workshop_variations = @person.workshop_variations_as_author.order(created_at: :desc).paginate(page: params[:page], per_page: per_page) + @workshop_variations = visible_authored_content(@person.workshop_variations_as_author).order(created_at: :desc).paginate(page: params[:page], per_page: per_page) render partial: "people/sections/workshop_variations", locals: { person: @person, workshop_variations: @workshop_variations } when "stories" # Credit the person for stories they authored or were spotlighted in — # not ones their user merely entered (created_by is a pure audit trail). - story_ids = @person.stories_as_author.pluck(:id) + + # Spotlighted stories are always listed: the spotlight is a separate credit + # from authorship, so the anonymity flag doesn't apply to it. + story_ids = visible_authored_content(@person.stories_as_author).pluck(:id) + @person.stories_as_spotlighted_facilitator.pluck(:id) @stories = Story.where(id: story_ids).order(created_at: :desc).paginate(page: params[:page], per_page: per_page) render partial: "people/sections/stories", locals: { person: @person, stories: @stories } when "resources" # Credit the person for resources they authored — not ones their user # merely entered (created_by is a pure audit trail). - @resources = @person.resources_as_author.order(created_at: :desc).paginate(page: params[:page], per_page: per_page) + @resources = visible_authored_content(@person.resources_as_author).order(created_at: :desc).paginate(page: params[:page], per_page: per_page) render partial: "people/sections/resources", locals: { person: @person, resources: @resources } when "events" @event_registrations = @person.event_registrations.active.includes(:event).order("events.start_date DESC").references(:events).paginate(page: params[:page], per_page: per_page) @@ -297,6 +299,15 @@ def check_duplicates private + # Anonymously-credited content is listed on the profile only for the person + # themselves and admins — showing it to anyone else would tie an "Anonymous" + # credit back to a name. `contributions_anonymous` anonymizes every item at once; + # otherwise only the items whose stored consent is "anonymous" are hidden. + def visible_authored_content(scope) + return scope if allowed_to?(:manage?, Person) || current_user&.person_id == @person.id + return scope.none if @person.contributions_anonymous? + scope.where.not(author_credit_preference: AuthorCreditable::ANONYMOUS) + end def set_person @person = Person.find(params[:id]) diff --git a/app/helpers/author_credit_divergences_helper.rb b/app/helpers/author_credit_divergences_helper.rb index 03599144ea..009614c7a4 100644 --- a/app/helpers/author_credit_divergences_helper.rb +++ b/app/helpers/author_credit_divergences_helper.rb @@ -4,6 +4,15 @@ def divergence_record_title(record) record.try(:title).presence || record.try(:name).presence || "##{record.id}" end + # Wraps plain records for the shared assign-a-person table. Sections 3 and 4 have + # no free-text name to guess from, so the suggestion is passed in (the creator) or + # omitted entirely. + def assignable_rows(records, suggested_author: nil) + records.map do |record| + AuthorCreditDivergenceQuery::AssignableRow.new(record: record, suggested_author: suggested_author) + end + end + # An empty page means "nothing left to reconcile" only when nothing is filtered # out — otherwise the congratulations would be reporting on the filter. def divergence_filters_applied? diff --git a/app/models/concerns/author_creditable.rb b/app/models/concerns/author_creditable.rb index 951911feee..ebf27e86f7 100644 --- a/app/models/concerns/author_creditable.rb +++ b/app/models/concerns/author_creditable.rb @@ -24,6 +24,9 @@ module AuthorCreditable # Snapshot the credited person's profile preference on create, so the column keeps # recording consent-at-submission without a human ever picking it. before_create :snapshot_author_credit_preference + # A blank preference means "no per-item override, just follow the profile" — store + # it as nil so the divergence worklist (which excludes nil) never re-flags it. + normalizes :author_credit_preference, with: ->(value) { value.presence } validates :author_credit_preference, inclusion: { in: AUTHOR_CREDIT_PREFERENCES }, allow_blank: true # Filter to content explicitly authored by a person (belongs_to :author); diff --git a/app/services/author_credit_divergence_query.rb b/app/services/author_credit_divergence_query.rb index 65ae6e3e47..9f522cbdb8 100644 --- a/app/services/author_credit_divergence_query.rb +++ b/app/services/author_credit_divergence_query.rb @@ -41,9 +41,21 @@ class AuthorCreditDivergenceQuery }.freeze PersonGroup = Struct.new(:person, :records, :suggested_preference, keyword_init: true) + + # One per legacy column, kept even when empty so the page can congratulate each + # field separately — clearing a column is what makes it safe to drop. + LegacyGroup = Struct.new(:model, :column, :entries, keyword_init: true) do + def empty? = entries.empty? + def field = column.split(".").last + end + # A row an admin can resolve by picking a person, optionally pre-guessed. + AssignableRow = Struct.new(:record, :suggested_author, keyword_init: true) + Result = Struct.new(:preference, :legacy, :creator, :unattributed, keyword_init: true) do + def legacy_empty? = legacy.all?(&:empty?) + def empty? - preference.empty? && legacy.empty? && creator.empty? && unattributed.empty? + preference.empty? && legacy_empty? && creator.empty? && unattributed.empty? end end @@ -61,7 +73,7 @@ def initialize(person_id: nil, type: nil, preference: nil, include_reconciled: f def call Result.new( preference: preference_groups, - legacy: legacy_records, + legacy: legacy_groups, creator: creator_groups, unattributed: unattributed_records ) @@ -104,17 +116,50 @@ def preference_groups end # ── Section 2: credited by a free-text name, with no person behind it ────── - # The person filter can't apply here: these records have no credited person, - # which is the whole problem with them. - def legacy_records - return [] if preference || person_id + # One group per legacy column, so each field can be reported (and retired) + # on its own. The person filter can't apply here: these records have no + # credited person, which is the whole problem with them. + def legacy_groups + groups = authorable_models.flat_map do |model| + model.legacy_author_name_columns.map { |column| [ model, column ] } + end + + records = preference || person_id ? [] : legacy_candidates + suggestions = suggested_authors_for(records) + + groups.map do |model, column| + entries = records.select { |record| record.is_a?(model) }.map do |record| + AssignableRow.new(record: record, suggested_author: suggestions[normalized(record.legacy_author_name_text)]) + end + LegacyGroup.new(model: model, column: column, entries: entries) + end + end + def legacy_candidates authorable_models.flat_map do |model| next [] if model.legacy_author_name_columns.empty? sorted(scoped(model).where(author_id: nil).select { |record| record.legacy_author_name_text.present? }) end end + # Guess who each free-text name refers to, so an admin can confirm rather than + # look every one up. Matches on the whole name, or on first + last token, in one + # query for the whole page rather than one per row. + def suggested_authors_for(records) + names = records.filter_map { |record| record.legacy_author_name_text.presence }.uniq + return {} if names.empty? + + candidates = Person.where(last_name: names.flat_map { |name| name.split(/\s+/) }.uniq) + by_normalized_full_name = candidates.index_by { |person| normalized(person.full_name) } + + names.index_with { |name| by_normalized_full_name[normalized(name)] } + .transform_keys { |name| normalized(name) } + end + + def normalized(value) + value.to_s.downcase.gsub(/\s+/, "") + end + # ── Section 3: author_id blank, so the credit falls back to the creator ──── def creator_groups records = authorable_models.flat_map do |model| diff --git a/app/views/author_credit_divergences/_legacy_group.html.erb b/app/views/author_credit_divergences/_legacy_group.html.erb new file mode 100644 index 0000000000..5ec6eff981 --- /dev/null +++ b/app/views/author_credit_divergences/_legacy_group.html.erb @@ -0,0 +1,19 @@ +<%# One legacy free-text column. Reported separately from its siblings so clearing + a single column is a visible milestone — that's what makes it safe to drop. %> +
+

+ <%= group.model.name.underscore.humanize %> + <%= group.column %> +

+ + <% if group.empty? %> + <%= render "section_clear", + message: "No #{group.model.name.underscore.humanize.downcase} records are credited by #{group.field} any more.", + cleanup: "Ask your developers to remove the unused #{group.column} field." %> + <% else %> + <%= render "unlinked_table", + rows: group.entries, + name_header: "Legacy name", + suggestion_note: "Suggested: a person whose name matches the legacy text." %> + <% end %> +
diff --git a/app/views/author_credit_divergences/_preference_group.html.erb b/app/views/author_credit_divergences/_preference_group.html.erb index 72a3ceb041..a5ba5b3ec1 100644 --- a/app/views/author_credit_divergences/_preference_group.html.erb +++ b/app/views/author_credit_divergences/_preference_group.html.erb @@ -41,8 +41,12 @@ <%= hidden_field_tag :record_type, record.class.name, id: nil %> <%= hidden_field_tag :record_id, record.id, id: nil %> <%= render "filter_fields" %> + <%# "None" clears the snapshot so the item just follows the profile. Hidden + for anonymous items — clearing that value would de-anonymize them. %> + <% allow_clear = record.author_credit_preference != AuthorCreditable::ANONYMOUS %> <%= select_tag "author_credit_preference", options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, record.author_credit_preference), + include_blank: (allow_clear ? "None (follow profile)" : false), id: nil, class: "rounded-md border-gray-300 text-sm" %> <%= submit_tag "Save", @@ -82,7 +86,7 @@ group.suggested_preference == AuthorCreditable::ANONYMOUS || person.contributions_anonymous?, id: "person_contributions_anonymous_#{person.id}", class: "rounded border-gray-300" %> - Contributions show as anonymous + Anonymous contributions <%= submit_tag "Apply to profile", diff --git a/app/views/author_credit_divergences/_unlinked_table.html.erb b/app/views/author_credit_divergences/_unlinked_table.html.erb index b94892b891..d543cfc75a 100644 --- a/app/views/author_credit_divergences/_unlinked_table.html.erb +++ b/app/views/author_credit_divergences/_unlinked_table.html.erb @@ -1,6 +1,6 @@ <%# Records with no author_id, each resolvable by crediting a person. Locals: - records (required), name_header (required), suggest_creator (required — - preselect the creating user's person in the picker). %> + rows (required — objects responding to #record and #suggested_author), + name_header (required), suggestion_note (optional caption under the table). %>
@@ -12,18 +12,19 @@ - <% records.each do |record| %> + <% rows.each do |row| %> - - - + + + <% end %>
<%= divergence_record_link(record) %><%= record.class.name.underscore.humanize %><%= record.author_credit %><%= divergence_record_link(row.record) %><%= row.record.class.name.underscore.humanize %><%= row.record.author_credit %> - <%= render "assign_author_form", - record: record, - suggested: (record.created_by&.person if suggest_creator) %> + <%= render "assign_author_form", record: row.record, suggested: row.suggested_author %>
+ <% if local_assigns[:suggestion_note].present? %> +

<%= suggestion_note %>

+ <% end %>
diff --git a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb index 0d36c3a105..c4d5925867 100644 --- a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb +++ b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb @@ -22,7 +22,7 @@ <%# Section counts up front so the admin can see the full scope and jump to any section. %> <% overview = [ { anchor: "section-preference", num: 1, label: "Preference drift", count: @result.preference.size, unit: "person" }, - { anchor: "section-legacy", num: 2, label: "Legacy name", count: @result.legacy.size, unit: "record" }, + { anchor: "section-legacy", num: 2, label: "Legacy name", count: @result.legacy.sum { |g| g.entries.size }, unit: "record" }, { anchor: "section-creator", num: 3, label: "Creator fallback", count: @result.creator.size, unit: "person" }, { anchor: "section-unattributed", num: 4, label: "No author", count: @result.unattributed.size, unit: "record" } ] %> @@ -77,12 +77,12 @@ obeys nobody's profile and links nowhere. Crediting a real person replaces it.

- <% if @result.legacy.empty? %> + <%= render partial: "legacy_group", collection: @result.legacy, as: :group %> + + <% if @result.legacy_empty? %> <%= render "section_clear", - message: "No legacy author names left. Every record is credited to a real person.", - cleanup: "Ask your developers to remove any unused full_name code — the workshops.full_name and resources.legacy_author_name columns, their legacy_author_name_columns / legacy_author_name_text overrides, and the legacy branches in AuthorCreditable." %> - <% else %> - <%= render "unlinked_table", records: @result.legacy, name_header: "Legacy name", suggest_creator: false %> + message: "Every legacy name column is clear.", + cleanup: "Ask your developers to remove both unused fields and the legacy-name handling in AuthorCreditable that reads them." %> <% end %> @@ -112,7 +112,10 @@ class: "font-semibold text-gray-900 hover:underline" %> — created <%= pluralize(group.records.size, "record") %> with no author set
- <%= render "unlinked_table", records: group.records, name_header: "Renders as", suggest_creator: true %> + <%= render "unlinked_table", + rows: assignable_rows(group.records, suggested_author: group.person), + name_header: "Renders as", + suggestion_note: "Suggested: the person who created the record." %>
<% end %>
@@ -124,15 +127,15 @@

4. No author at all

No author, no legacy name, and no person behind the creating account, so these fall back to a - generic label like “AWBW Facilitator”. + generic placeholder like “AWBW Facilitator” or “AWBW Staff”.

<% if @result.unattributed.empty? %> <%= render "section_clear", message: "Nothing unattributed. Every record has someone to credit.", - cleanup: "Ask your developers to remove the missing_author_label fallbacks." %> + cleanup: "Ask your developers to remove the generic placeholder names, since no record falls back to one any more." %> <% else %> - <%= render "unlinked_table", records: @result.unattributed, name_header: "Renders as", suggest_creator: false %> + <%= render "unlinked_table", rows: assignable_rows(@result.unattributed), name_header: "Renders as" %> <% end %> <% end %> diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index e292e541cb..4745549abd 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -477,8 +477,8 @@ selected: f.object.display_name_preference || "full_name" %> <%= f.input :contributions_anonymous, - label: "Contributions show as anonymous", - hint: "Credits stories, workshops, variations and resources as \"Anonymous\". Does not change how they're listed on the people index." %> + label: "Anonymous contributions", + hint: "Author credit only" %> <%= f.input :profile_show_credentials, label: "Show credentials" %> diff --git a/app/views/people/_show_card.html.erb b/app/views/people/_show_card.html.erb index c3924e1f80..1e4c6edf69 100644 --- a/app/views/people/_show_card.html.erb +++ b/app/views/people/_show_card.html.erb @@ -2,8 +2,9 @@ <% record_title ||= record.title %> <% title_font_size ||= nil %> <% bookmarkable ||= record.object %> +<% anonymous ||= false %> -
+
">
<%= render "bookmarks/editable_bookmark_icon", resource: bookmarkable %> @@ -36,6 +37,15 @@
+ <% if anonymous %> + <%# Admin/owner-only marker: this credit renders "Anonymous" publicly, so + the item is hidden from everyone else. %> + + + Anonymous + + <% end %>
<%= link_to record.link_target, data: { turbo_frame: "_top"}, diff --git a/app/views/people/sections/_resources.html.erb b/app/views/people/sections/_resources.html.erb index d21e233c86..2323f89999 100644 --- a/app/views/people/sections/_resources.html.erb +++ b/app/views/people/sections/_resources.html.erb @@ -2,7 +2,8 @@ <% if resources.any? %>
<% resources.each do |resource| %> - <%= render "show_card", record: resource.decorate, title_font_size: "text-sm" %> + <%= render "show_card", record: resource.decorate, title_font_size: "text-sm", + anonymous: resource.credit_anonymous?(person) %> <% end %>
diff --git a/app/views/people/sections/_stories.html.erb b/app/views/people/sections/_stories.html.erb index c0608ad0f8..92ea0c3822 100644 --- a/app/views/people/sections/_stories.html.erb +++ b/app/views/people/sections/_stories.html.erb @@ -2,7 +2,10 @@ <% if stories.any? %>
<% stories.each do |story| %> - <%= render "show_card", record: story.decorate, title_font_size: "text-sm" %> + <%# Spotlight-only stories (person isn't the author) are never anonymized — + the anonymity flag governs authorship credit, not the spotlight. %> + <%= render "show_card", record: story.decorate, title_font_size: "text-sm", + anonymous: story.author_id == person.id && story.credit_anonymous?(person) %> <% end %>
diff --git a/app/views/people/sections/_workshop_variations.html.erb b/app/views/people/sections/_workshop_variations.html.erb index 086b1156de..a02b90f9fd 100644 --- a/app/views/people/sections/_workshop_variations.html.erb +++ b/app/views/people/sections/_workshop_variations.html.erb @@ -5,7 +5,8 @@ <%= render "show_card", record_title: "#{workshop_variation.name}
" + "WORKSHOP: #{workshop_variation.workshop.name}", - record: workshop_variation.decorate, title_font_size: "text-sm" %> + record: workshop_variation.decorate, title_font_size: "text-sm", + anonymous: workshop_variation.credit_anonymous?(person) %> <% end %>
diff --git a/app/views/people/sections/_workshops.html.erb b/app/views/people/sections/_workshops.html.erb index 70e4366c9f..4949d45e06 100644 --- a/app/views/people/sections/_workshops.html.erb +++ b/app/views/people/sections/_workshops.html.erb @@ -2,7 +2,8 @@ <% if workshops.any? %>
<% workshops.each do |workshop| %> - <%= render "show_card", record: workshop.decorate, title_font_size: "text-sm" %> + <%= render "show_card", record: workshop.decorate, title_font_size: "text-sm", + anonymous: workshop.credit_anonymous?(person) %> <% end %>
diff --git a/spec/requests/author_credit_divergences_spec.rb b/spec/requests/author_credit_divergences_spec.rb index 61a73dd5ae..5c5cdf7d23 100644 --- a/spec/requests/author_credit_divergences_spec.rb +++ b/spec/requests/author_credit_divergences_spec.rb @@ -137,6 +137,25 @@ expect(story.reload.author_credit_preference).to eq("full_name") end + it "clears the stored snapshot when set to blank, so the item just follows the profile" do + story.update_column(:author_credit_preference, "last_name_only") + + patch update_item_author_credit_divergences_path, + params: { record_type: "Story", record_id: story.id, author_credit_preference: "" } + + expect(story.reload.author_credit_preference).to be_nil + end + + it "refuses to clear the snapshot of an item submitted anonymously" do + story.update_column(:author_credit_preference, "anonymous") + + patch update_item_author_credit_divergences_path, + params: { record_type: "Story", record_id: story.id, author_credit_preference: "" } + + expect(story.reload.author_credit_preference).to eq("anonymous") + expect(flash[:alert]).to be_present + end + it "makes one item anonymous without touching the person's others" do other = create(:story, created_by: author_user, author: person) diff --git a/spec/requests/people_stories_section_spec.rb b/spec/requests/people_stories_section_spec.rb index 89b45b6503..3bd9eef8f6 100644 --- a/spec/requests/people_stories_section_spec.rb +++ b/spec/requests/people_stories_section_spec.rb @@ -38,4 +38,24 @@ def get_stories_section expect(response.body).not_to include("Only Created Story") end + + it "flags an anonymously-credited authored story" do + create(:story, :published, title: "Hush Story", + author: person, author_credit_preference: "anonymous") + + get_stories_section + + expect(response.body).to include("Hush Story") + expect(response.body).to include("Credited as Anonymous") + end + + it "never flags a spotlighted story, even when the person is anonymous" do + person.update!(contributions_anonymous: true) + create(:story, :published, title: "Spotlight Story", spotlighted_facilitator: person) + + get_stories_section + + expect(response.body).to include("Spotlight Story") + expect(response.body).not_to include("Credited as Anonymous") + end end diff --git a/spec/requests/people_workshops_section_spec.rb b/spec/requests/people_workshops_section_spec.rb index 1f33d22fdd..226ce4a3e5 100644 --- a/spec/requests/people_workshops_section_spec.rb +++ b/spec/requests/people_workshops_section_spec.rb @@ -29,4 +29,35 @@ def get_workshops_section expect(response.body).not_to include("Only Created Workshop") end + + it "flags an anonymously-credited authored workshop for an admin" do + create(:workshop, :published, title: "Hush Workshop", + author: person, author_credit_preference: "anonymous") + + get_workshops_section + + expect(response.body).to include("Hush Workshop") + expect(response.body).to include("Credited as Anonymous") + end + + it "does not flag a normally-credited workshop" do + create(:workshop, :published, title: "Loud Workshop", + author: person, author_credit_preference: "full_name") + + get_workshops_section + + expect(response.body).to include("Loud Workshop") + expect(response.body).not_to include("Credited as Anonymous") + end + + it "still shows the anonymous workshop, flagged, to the owner viewing their own profile" do + sign_in owner_user + create(:workshop, :published, title: "Hush Workshop", + author: person, author_credit_preference: "anonymous") + + get_workshops_section + + expect(response.body).to include("Hush Workshop") + expect(response.body).to include("Credited as Anonymous") + end end diff --git a/spec/services/author_credit_divergence_query_spec.rb b/spec/services/author_credit_divergence_query_spec.rb index 2bf51098fd..e0ed363411 100644 --- a/spec/services/author_credit_divergence_query_spec.rb +++ b/spec/services/author_credit_divergence_query_spec.rb @@ -94,12 +94,15 @@ def diverged_story end describe "the legacy section" do + def legacy_records_for(column) + group = described_class.new.call.legacy.find { |g| g.column == column } + group.entries.map(&:record) + end + it "lists records credited by a free-text name with no person" do workshop = create(:workshop, author: nil, full_name: "Marguerite Pre-Person") - legacy = described_class.new.call.legacy - - expect(legacy).to include(workshop) + expect(legacy_records_for("workshops.full_name")).to include(workshop) expect(workshop.author_credit).to eq("Marguerite Pre-Person") end @@ -107,12 +110,40 @@ def diverged_story workshop = create(:workshop, author: nil, full_name: "Marguerite Pre-Person") workshop.update!(author: person) - expect(described_class.new.call.legacy).not_to include(workshop) + expect(legacy_records_for("workshops.full_name")).not_to include(workshop) + end + + it "keeps a group per legacy column even when it is empty, so each can be retired" do + columns = described_class.new.call.legacy.map(&:column) + + expect(columns).to contain_exactly("workshops.full_name", "resources.legacy_author_name") + expect(described_class.new.call).to be_legacy_empty end it "excludes models that have no legacy column" do create(:story, created_by: author_user, author: nil) - expect(described_class.new.call.legacy).to be_empty + expect(described_class.new.call.legacy.flat_map(&:entries)).to be_empty + end + + it "suggests a person whose name matches the legacy text" do + match = create(:person, first_name: "Marguerite", last_name: "Duras") + workshop = create(:workshop, author: nil, full_name: "Marguerite Duras") + + entry = described_class.new.call.legacy + .find { |g| g.column == "workshops.full_name" } + .entries.find { |e| e.record == workshop } + + expect(entry.suggested_author).to eq(match) + end + + it "suggests nothing when no person matches" do + workshop = create(:workshop, author: nil, full_name: "Nobody Byanyname") + + entry = described_class.new.call.legacy + .find { |g| g.column == "workshops.full_name" } + .entries.find { |e| e.record == workshop } + + expect(entry.suggested_author).to be_nil end end diff --git a/spec/support/shared_examples/author_creditable.rb b/spec/support/shared_examples/author_creditable.rb index bf49177476..7f4f482bdb 100644 --- a/spec/support/shared_examples/author_creditable.rb +++ b/spec/support/shared_examples/author_creditable.rb @@ -104,6 +104,12 @@ expect(record.reload.author_credit_preference).to eq("last_name_only") end + it "normalizes a blank preference to nil so the record just follows the profile" do + record = create(factory, created_by: author_user, author_credit_preference: "full_name") + record.update!(author_credit_preference: "") + expect(record.reload.author_credit_preference).to be_nil + end + it "is left alone when the profile later changes, and reports the divergence" do person.update!(display_name_preference: "first_name_only") record = create(factory, created_by: author_user, author_credit_preference: nil) From be82e7ff67b98fce1ffbe334acc21373d082614a Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 8 Aug 2026 14:13:07 -0400 Subject: [PATCH 09/12] Update credit divergences in place over Turbo, not a full reload Every fix on the author credit divergences page (Save stored consent, Apply to profile, Credit to a person) did a full-page redirect. Respond with a Turbo Stream that re-renders just the results frame and flash, so the worklist updates in place with no page flip. Non-Turbo requests still redirect. Also widen the remote-select left padding so the search icon never overlaps the placeholder or selected value. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../author_credit_divergences_controller.rb | 35 ++++++++++++------- .../controllers/remote_select_controller.js | 2 +- .../_assign_author_form.html.erb | 1 - .../_preference_group.html.erb | 5 +-- .../divergence_change.turbo_stream.erb | 9 +++++ .../author_credit_divergences_spec.rb | 30 ++++++++++++++++ 6 files changed, 64 insertions(+), 18 deletions(-) create mode 100644 app/views/author_credit_divergences/divergence_change.turbo_stream.erb diff --git a/app/controllers/author_credit_divergences_controller.rb b/app/controllers/author_credit_divergences_controller.rb index f5e0ebbec8..cb2bf62b78 100644 --- a/app/controllers/author_credit_divergences_controller.rb +++ b/app/controllers/author_credit_divergences_controller.rb @@ -19,9 +19,9 @@ def update_person person.updated_by = current_user if person.save - redirect_to author_credit_divergences_path(filters), notice: "Updated credit preferences for #{person.full_name}." + render_divergence_change("Updated credit preferences for #{person.full_name}.", :notice) else - redirect_to author_credit_divergences_path(filters), alert: person.errors.full_messages.to_sentence + render_divergence_change(person.errors.full_messages.to_sentence, :alert) end end @@ -30,7 +30,7 @@ def update_person # any other value only re-records history (see AuthorCreditable). def update_item model = AuthorCreditDivergenceQuery.model_for(params[:record_type]) - return redirect_to(author_credit_divergences_path(filters), alert: "Unknown record type.") unless model + return render_divergence_change("Unknown record type.", :alert) unless model record = model.find(params[:record_id]) @@ -38,17 +38,16 @@ def update_item # anonymously would silently de-anonymize it. A deliberate re-credit still works # by picking an explicit preference. if params[:author_credit_preference].blank? && record.author_credit_preference == AuthorCreditable::ANONYMOUS - return redirect_to(author_credit_divergences_path(filters), - alert: "Can't clear the consent for an item submitted anonymously — pick an explicit preference instead.") + return render_divergence_change("Can't clear the consent for an item submitted anonymously — pick an explicit preference instead.", :alert) end record.author_credit_preference = params[:author_credit_preference] record.updated_by = current_user if record.respond_to?(:updated_by=) if record.save - redirect_to author_credit_divergences_path(filters), notice: "Updated credit for #{model.name.underscore.humanize.downcase} ##{record.id}." + render_divergence_change("Updated credit for #{model.name.underscore.humanize.downcase} ##{record.id}.", :notice) else - redirect_to author_credit_divergences_path(filters), alert: record.errors.full_messages.to_sentence + render_divergence_change(record.errors.full_messages.to_sentence, :alert) end end @@ -58,25 +57,37 @@ def update_item # the record stops being used. def assign_author model = AuthorCreditDivergenceQuery.model_for(params[:record_type]) - return redirect_to(author_credit_divergences_path(filters), alert: "Unknown record type.") unless model + return render_divergence_change("Unknown record type.", :alert) unless model record = model.find(params[:record_id]) person = Person.find_by(id: params[:author_id]) - return redirect_to(author_credit_divergences_path(filters), alert: "Choose a person to credit.") unless person + return render_divergence_change("Choose a person to credit.", :alert) unless person record.author_id = person.id record.updated_by = current_user if record.respond_to?(:updated_by=) if record.save - redirect_to author_credit_divergences_path(filters), - notice: "Credited #{model.name.underscore.humanize.downcase} ##{record.id} to #{person.full_name}." + render_divergence_change("Credited #{model.name.underscore.humanize.downcase} ##{record.id} to #{person.full_name}.", :notice) else - redirect_to author_credit_divergences_path(filters), alert: record.errors.full_messages.to_sentence + render_divergence_change(record.errors.full_messages.to_sentence, :alert) end end private + # Update in place: re-render the results frame and flash over Turbo so a save + # doesn't flip the whole page. Falls back to a redirect for non-Turbo requests. + def render_divergence_change(message, type) + respond_to do |format| + format.turbo_stream do + flash.now[type] = message + @result = AuthorCreditDivergenceQuery.new(**filters.symbolize_keys).call + render :divergence_change + end + format.html { redirect_to author_credit_divergences_path(filters), flash: { type => message } } + end + end + def authorize_page authorize! :author_credit_divergence, to: :"#{action_name}?", with: AuthorCreditDivergencePolicy end diff --git a/app/frontend/javascript/controllers/remote_select_controller.js b/app/frontend/javascript/controllers/remote_select_controller.js index e759e55596..ce69d403e7 100644 --- a/app/frontend/javascript/controllers/remote_select_controller.js +++ b/app/frontend/javascript/controllers/remote_select_controller.js @@ -80,7 +80,7 @@ export default class extends Controller { z-index: 1; } .remote-select-container .ts-control { - padding-left: 1.5rem !important; /* Make room for the search icon */ + padding-left: 2rem !important; /* Clear the search icon so it never overlaps the placeholder or value */ } .ts-control { border: none !important; diff --git a/app/views/author_credit_divergences/_assign_author_form.html.erb b/app/views/author_credit_divergences/_assign_author_form.html.erb index 66a562ad12..46ad7eed93 100644 --- a/app/views/author_credit_divergences/_assign_author_form.html.erb +++ b/app/views/author_credit_divergences/_assign_author_form.html.erb @@ -2,7 +2,6 @@ Person to preselect, e.g. the creator on the "credited to the creator" section). %> <% suggested = local_assigns[:suggested] %> <%= form_with url: assign_author_author_credit_divergences_path, method: :patch, - data: { turbo_frame: "_top" }, class: "flex items-center gap-2" do %> <%= hidden_field_tag :record_type, record.class.name, id: nil %> <%= hidden_field_tag :record_id, record.id, id: nil %> diff --git a/app/views/author_credit_divergences/_preference_group.html.erb b/app/views/author_credit_divergences/_preference_group.html.erb index a5ba5b3ec1..9d0e2bf9cb 100644 --- a/app/views/author_credit_divergences/_preference_group.html.erb +++ b/app/views/author_credit_divergences/_preference_group.html.erb @@ -36,7 +36,6 @@ <%= record.author_credit %> <%= form_with url: update_item_author_credit_divergences_path, method: :patch, - data: { turbo_frame: "_top" }, class: "flex items-center gap-2" do %> <%= hidden_field_tag :record_type, record.class.name, id: nil %> <%= hidden_field_tag :record_id, record.id, id: nil %> @@ -65,7 +64,6 @@ <%# The action lands after the supportive data above: read the content, then apply. %>
<%= form_with url: update_person_author_credit_divergences_path, method: :patch, - data: { turbo_frame: "_top" }, class: "flex flex-wrap items-end gap-3" do %> <%= hidden_field_tag :id, person.id, id: nil %> <%= render "filter_fields" %> @@ -91,9 +89,8 @@ <%= submit_tag "Apply to profile", class: "rounded-md bg-blue-600 px-3 py-2 text-sm font-medium text-white hover:bg-blue-700 cursor-pointer" %> - + Suggested: <%= AuthorCreditable::ADMIN_FORM_OPTIONS.key(group.suggested_preference) %> - — the most restrictive consent across <%= person.full_name %>'s content. <% end %>
diff --git a/app/views/author_credit_divergences/divergence_change.turbo_stream.erb b/app/views/author_credit_divergences/divergence_change.turbo_stream.erb new file mode 100644 index 0000000000..63d97dbb3f --- /dev/null +++ b/app/views/author_credit_divergences/divergence_change.turbo_stream.erb @@ -0,0 +1,9 @@ +<%# Re-render the results frame and flash in place, so saving a divergence fix + doesn't reload the whole page. Shared by update_item and update_person. %> +<%= turbo_stream.replace "author_credit_divergences_results" do %> + <%= render template: "author_credit_divergences/author_credit_divergences_results", formats: :html %> +<% end %> + +<%= turbo_stream.replace "flash_now" do %> + <%= render "shared/flash_messages" %> +<% end %> diff --git a/spec/requests/author_credit_divergences_spec.rb b/spec/requests/author_credit_divergences_spec.rb index 5c5cdf7d23..3c37fcbda6 100644 --- a/spec/requests/author_credit_divergences_spec.rb +++ b/spec/requests/author_credit_divergences_spec.rb @@ -55,6 +55,15 @@ expect(story.reload.author_credit).to eq("Anonymous") end + it "updates the results in place with a Turbo Stream instead of a full-page redirect" do + patch update_person_author_credit_divergences_path, + params: { id: person.id, person: { display_name_preference: "first_name_only", contributions_anonymous: "0" } }, + as: :turbo_stream + + expect(response.media_type).to eq(Mime[:turbo_stream]) + expect(response.body).to include("author_credit_divergences_results") + end + it "carries the active filters through the redirect" do patch update_person_author_credit_divergences_path, params: { id: person.id, type: "Story", @@ -78,6 +87,17 @@ let(:target) { create(:person, first_name: "Rosalind", last_name: "Franklin") } + it "updates the results in place with a Turbo Stream instead of a full-page redirect" do + workshop = create(:workshop, author: nil, full_name: "Marguerite Pre-Person") + + patch assign_author_author_credit_divergences_path, + params: { record_type: "Workshop", record_id: workshop.id, author_id: target.id }, + as: :turbo_stream + + expect(response.media_type).to eq(Mime[:turbo_stream]) + expect(response.body).to include("author_credit_divergences_results") + end + it "credits a legacy free-text record to a real person" do workshop = create(:workshop, author: nil, full_name: "Marguerite Pre-Person") @@ -137,6 +157,16 @@ expect(story.reload.author_credit_preference).to eq("full_name") end + it "updates the results in place with a Turbo Stream instead of a full-page redirect" do + patch update_item_author_credit_divergences_path, + params: { record_type: "Story", record_id: story.id, author_credit_preference: "full_name" }, + as: :turbo_stream + + expect(response.media_type).to eq(Mime[:turbo_stream]) + expect(response.body).to include("author_credit_divergences_results") + expect(response.body).to include("flash_now") + end + it "clears the stored snapshot when set to blank, so the item just follows the profile" do story.update_column(:author_credit_preference, "last_name_only") From d5271029084cc1b515aa4eaa53b4e0ce7f5c6313 Mon Sep 17 00:00:00 2001 From: maebeale Date: Sun, 9 Aug 2026 16:00:10 -0400 Subject: [PATCH 10/12] Suppress stored-anonymous over legacy names; harden the divergences page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - author_credit returns "Anonymous" for a stored-anonymous item even when it carries a legacy free-text name — that name belongs to no profile, so nothing else would suppress it. - Add a credited_openly scope so a NULL snapshot (which means "follow the profile") stays visible; a bare where.not would drop it, since NULL never compares unequal. Use it for a person's public authored content. - Build the divergences result only on the frame request, so the full page load stays cheap. - Under an active filter, an empty section is the filter's doing, not a milestone, so section-clear withholds the congratulations and cleanup note. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../author_credit_divergences_controller.rb | 6 ++-- app/controllers/people_controller.rb | 2 +- app/models/concerns/author_creditable.rb | 10 +++++++ .../_section_clear.html.erb | 28 +++++++++++++------ ...author_credit_divergences_results.html.erb | 3 +- spec/models/resource_spec.rb | 15 ++++++++++ spec/models/workshop_spec.rb | 15 ++++++++++ .../shared_examples/author_creditable.rb | 21 ++++++++++++++ 8 files changed, 87 insertions(+), 13 deletions(-) diff --git a/app/controllers/author_credit_divergences_controller.rb b/app/controllers/author_credit_divergences_controller.rb index cb2bf62b78..e9d343c6fd 100644 --- a/app/controllers/author_credit_divergences_controller.rb +++ b/app/controllers/author_credit_divergences_controller.rb @@ -3,10 +3,12 @@ class AuthorCreditDivergencesController < ApplicationController FILTER_KEYS = %i[person_id type preference include_reconciled].freeze + # The full page renders only the header, filters, and an empty results frame; + # the frame's src request builds the divergences. def index - @result = AuthorCreditDivergenceQuery.new(**filters.symbolize_keys).call - return unless turbo_frame_request? + + @result = AuthorCreditDivergenceQuery.new(**filters.symbolize_keys).call render :author_credit_divergences_results end diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 7efd7eb48d..864a05f346 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -306,7 +306,7 @@ def check_duplicates def visible_authored_content(scope) return scope if allowed_to?(:manage?, Person) || current_user&.person_id == @person.id return scope.none if @person.contributions_anonymous? - scope.where.not(author_credit_preference: AuthorCreditable::ANONYMOUS) + scope.credited_openly end def set_person diff --git a/app/models/concerns/author_creditable.rb b/app/models/concerns/author_creditable.rb index ebf27e86f7..b857a286b1 100644 --- a/app/models/concerns/author_creditable.rb +++ b/app/models/concerns/author_creditable.rb @@ -33,6 +33,13 @@ module AuthorCreditable # no-op when person_id is blank. Only models with an author_id column use it. scope :authored_by, ->(person_id) { where(author_id: person_id) if person_id.present? } + # Content whose credit isn't suppressed per item. A blank snapshot means "follow + # the profile", so those rows stay in — `where.not` on its own would drop them, + # since NULL never compares unequal in SQL. + scope :credited_openly, -> { + where(author_credit_preference: nil).or(where.not(author_credit_preference: ANONYMOUS)) + } + # Filter to content whose creating user belongs to a person. This is the only # authorship link the idea models have (they carry no author_id of their own), # and it's keyed on the person rather than a user id so it stays correct — and @@ -63,6 +70,9 @@ def legacy_author_name_text # Precedence: the primary author person, then the legacy free-text name, then the # creating user's person, then `missing_author_label`. def author_credit + # A stored "anonymous" outranks every source, including the legacy free-text + # name — that name belongs to no profile, so nothing else would suppress it. + return "Anonymous" if author_credit_preference == ANONYMOUS person = primary_author_person return credit_for(person) if person return legacy_author_name_text if legacy_author_name_text.present? diff --git a/app/views/author_credit_divergences/_section_clear.html.erb b/app/views/author_credit_divergences/_section_clear.html.erb index 62a838713d..832f2b06a4 100644 --- a/app/views/author_credit_divergences/_section_clear.html.erb +++ b/app/views/author_credit_divergences/_section_clear.html.erb @@ -1,10 +1,20 @@ <%# All-clear note for one section. Locals: message (required), cleanup (optional — - the code that can now be retired, which is the real payoff of clearing it). %> -
-

- <%= message %> -

- <% if local_assigns[:cleanup].present? %> -

<%= cleanup %>

- <% end %> -
+ the code that can now be retired, which is the real payoff of clearing it). + Under a filter an empty section is the filter's doing, not a milestone, so both + the congratulations and the cleanup instruction are withheld. %> +<% if divergence_filters_applied? %> +
+

+ Nothing in this section matches the current filters. +

+
+<% else %> +
+

+ <%= message %> +

+ <% if local_assigns[:cleanup].present? %> +

<%= cleanup %>

+ <% end %> +
+<% end %> diff --git a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb index c4d5925867..907443d51d 100644 --- a/app/views/author_credit_divergences/author_credit_divergences_results.html.erb +++ b/app/views/author_credit_divergences/author_credit_divergences_results.html.erb @@ -28,7 +28,8 @@ ] %>
<% overview.each do |section| %> - <% clear = section[:count].zero? %> + <%# A zero under a filter is the filter's doing, so it isn't reported as clear. %> + <% clear = section[:count].zero? && !divergence_filters_applied? %> <%= link_to "##{section[:anchor]}", class: "block rounded-lg border p-3 transition-colors #{clear ? "border-green-200 bg-green-50 hover:bg-green-100" : "border-gray-200 bg-white shadow-sm hover:bg-gray-50"}" do %>
diff --git a/spec/models/resource_spec.rb b/spec/models/resource_spec.rb index f391676042..6dea75f7c5 100644 --- a/spec/models/resource_spec.rb +++ b/spec/models/resource_spec.rb @@ -35,6 +35,21 @@ end end + describe "#author_credit with a legacy free-text name" do + let(:creator) { create(:user, :with_person) } + + it "uses the legacy name when no author is credited" do + resource = create(:resource, created_by: creator, author: nil, legacy_author_name: "Jane Legacy") + expect(resource.author_credit).to eq("Jane Legacy") + end + + it "stays anonymous rather than exposing the legacy name" do + resource = create(:resource, created_by: creator, author: nil, legacy_author_name: "Jane Legacy", + author_credit_preference: "anonymous") + expect(resource.author_credit).to eq("Anonymous") + end + end + describe 'validations' do # Requires associations for create it { should validate_presence_of(:title) } diff --git a/spec/models/workshop_spec.rb b/spec/models/workshop_spec.rb index 65fb785823..ee535d092a 100644 --- a/spec/models/workshop_spec.rb +++ b/spec/models/workshop_spec.rb @@ -111,6 +111,21 @@ end end + describe "#author_credit with a legacy free-text name" do + let(:creator) { create(:user, :with_person) } + + it "uses the legacy name when no author is credited" do + workshop = create(:workshop, created_by: creator, author: nil, full_name: "Jane Legacy") + expect(workshop.author_credit).to eq("Jane Legacy") + end + + it "stays anonymous rather than exposing the legacy name" do + workshop = create(:workshop, created_by: creator, author: nil, full_name: "Jane Legacy", + author_credit_preference: "anonymous") + expect(workshop.author_credit).to eq("Anonymous") + end + end + describe "#remote_search_label" do it "returns title with windows type short_name" do record = create(:workshop, title: "Art Therapy", windows_type: create(:windows_type, :children)) diff --git a/spec/support/shared_examples/author_creditable.rb b/spec/support/shared_examples/author_creditable.rb index 7f4f482bdb..1733c76156 100644 --- a/spec/support/shared_examples/author_creditable.rb +++ b/spec/support/shared_examples/author_creditable.rb @@ -128,6 +128,27 @@ end end + describe ".credited_openly" do + let(:author_user) { create(:user, :with_person) } + let!(:record) { create(factory, created_by: author_user, author_credit_preference: "full_name") } + + it "includes a record whose snapshot is a name format" do + expect(described_class.credited_openly).to include(record) + end + + it "excludes a record submitted anonymously" do + record.update!(author_credit_preference: "anonymous") + expect(described_class.credited_openly).not_to include(record) + end + + it "includes a record with no snapshot, which just follows the profile" do + # The un-backfilled state of every pre-callback row, and what clearing the + # snapshot on the divergences page writes back. + described_class.where(id: record.id).update_all(author_credit_preference: nil) + expect(described_class.credited_openly).to include(record) + end + end + describe ".by_credited_person_name" do let(:author_user) { create(:user, :with_person) } let(:person) { author_user.person } From b9867853ff5fccea7d01fbf58787d853dfe8defe Mon Sep 17 00:00:00 2001 From: maebeale Date: Sun, 9 Aug 2026 22:31:51 -0400 Subject: [PATCH 11/12] Credit only the author the record actually names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The divergences page couldn't finish its own job. An item whose stored consent was "anonymous" offered no way back to "follow the profile", so the only exit was picking a name format — re-recording a consent nobody gave. Per-item anonymity is the legacy state this page exists to drain, and the profile is the source of truth, so clearing it is now allowed on every row. The author picker fell back to the creator, so a workshop credited to the legacy name "Lisa Cohen" showed "Umberto User" in its own edit form — and the next save would have written that creator into author_id, destroying the legacy credit. The picker now reflects only the record's own author (still defaulting to the creator on new records) and names the legacy credit standing in when there isn't one. Sorting COALESCEd author, creator, legacy while display used author, legacy, creator, so legacy-credited rows sorted under a name they never show. A note under each author picker flags when the credited person's profile suppresses credits, since nothing on the record itself reveals that. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- .../author_credit_divergences_controller.rb | 10 +++----- app/helpers/application_helper.rb | 8 ++++++ app/models/concerns/author_creditable.rb | 13 ++++++---- .../_preference_group.html.erb | 7 +++--- app/views/community_news/_form.html.erb | 5 ++-- app/views/resources/_form.html.erb | 8 +++--- app/views/shared/_author_credit_note.html.erb | 25 +++++++++++++++++++ app/views/stories/_form.html.erb | 9 ++++--- app/views/workshop_variations/_form.html.erb | 5 ++-- app/views/workshops/_form.html.erb | 7 +++--- .../author_credit_divergences_spec.rb | 6 ++--- 12 files changed, 69 insertions(+), 36 deletions(-) create mode 100644 app/views/shared/_author_credit_note.html.erb diff --git a/AGENTS.md b/AGENTS.md index 55df1f3536..41ddd2a0c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -133,7 +133,7 @@ This codebase (Rails 8.1) |---|---| | `AgeGroupTaggable` | Splits AgeRange category taggings into primary/additional via `categorizable_items.is_primary` (Person, Organization) | | `AhoyTrackable` | Event tracking integration | -| `AuthorCreditable` | Author attribution. Credits are formatted by the credited **person's profile** (`Person#display_name_preference`), not by the record. The record's `author_credit_preference` is the consent snapshot taken at create time and is human-editable only on the author credit divergences page — it no longer drives display, except `"anonymous"`, which is always honored (anonymity is a one-way latch: profile or record can set it, neither can strip it) | +| `AuthorCreditable` | Author attribution. Credits are formatted by the credited **person's profile** (`Person#display_name_preference`), not by the record. The record's `author_credit_preference` is the consent snapshot taken at create time and is human-editable only on the author credit divergences page — it no longer drives display, except `"anonymous"`, which is always honored while set (either the profile or the record can make a credit anonymous, and neither strips the other's flag — only an admin clearing the record's snapshot on that page does) | | `Featureable` | `featured`, `publicly_featured` scopes | | `Mentioner` | ActionText @mention extraction and grouping | | `NameFilterable` | Name-based filtering | diff --git a/app/controllers/author_credit_divergences_controller.rb b/app/controllers/author_credit_divergences_controller.rb index e9d343c6fd..fc1a52b442 100644 --- a/app/controllers/author_credit_divergences_controller.rb +++ b/app/controllers/author_credit_divergences_controller.rb @@ -36,13 +36,9 @@ def update_item record = model.find(params[:record_id]) - # Anonymity is a one-way latch: clearing the snapshot of an item submitted - # anonymously would silently de-anonymize it. A deliberate re-credit still works - # by picking an explicit preference. - if params[:author_credit_preference].blank? && record.author_credit_preference == AuthorCreditable::ANONYMOUS - return render_divergence_change("Can't clear the consent for an item submitted anonymously — pick an explicit preference instead.", :alert) - end - + # Clearing an "anonymous" snapshot hands the item back to the profile, which may + # well credit it. That's the point: a per-item anonymous flag is the legacy state + # this page exists to drain, and the person's profile is the source of truth. record.author_credit_preference = params[:author_credit_preference] record.updated_by = current_user if record.respond_to?(:updated_by=) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f29302bd9d..966c1706a7 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -12,6 +12,14 @@ def credited_author_link(record, **link_options) end end + # The person an author picker should show. Only the record's own author counts — + # falling back to the creator would present a person nobody chose as the selected + # author, and saving the form would silently promote them over a legacy credit. + # New records still default to the current user, which is the documented behavior. + def author_picker_person(record) + record.author || (record.new_record? ? current_user&.person : nil) + end + # Tags an admin may use in a form field name / group header that should # render (rather than escape) on the public form. Block + inline formatting, # links, line breaks, and font sizing/coloring (via or inline style). diff --git a/app/models/concerns/author_creditable.rb b/app/models/concerns/author_creditable.rb index b857a286b1..04ca616fec 100644 --- a/app/models/concerns/author_creditable.rb +++ b/app/models/concerns/author_creditable.rb @@ -5,9 +5,9 @@ module AuthorCreditable # `author_credit_preference` is retained as the record of what the submitter consented # to at submission time, and is human-editable only on the author credit divergences # page. It no longer drives display — with one exception: a stored "anonymous" is - # always honored, because anonymity is inherently per-item (a person may want four - # stories credited and the fifth not) and because nothing should be able to - # de-anonymize an item that was submitted anonymously. + # always honored while it's set, because anonymity is inherently per-item (a person + # may want four stories credited and the fifth not). Only an admin clearing the + # snapshot on that page hands the item back to the profile. AUTHOR_CREDIT_PREFERENCES = %w[full_name first_name_last_initial first_name_only last_name_only anonymous].freeze ANONYMOUS = "anonymous" @@ -179,13 +179,16 @@ def credited_person_join_sql # Arel COALESCE over every credited person alias (and legacy name column), # so the ORDER BY carries no interpolated SQL. Aliases and column names come - # from model config / column_names, never user input. + # from model config / column_names, never user input. Ordered author → legacy → + # creator to match `author_credit`, so a row sorts under the name it displays. def coalesced_author_arel(field, ascending) - parts = credited_person_aliases.map { |sql_alias| Arel::Table.new(sql_alias)[field] } + parts = [] + parts << Arel::Table.new("credited_author")[field] if column_names.include?("author_id") parts += legacy_author_name_columns.map do |col| table, column = col.split(".") Arel::Table.new(table)[column] end + parts << Arel::Table.new("credited_creator")[field] node = Arel::Nodes::NamedFunction.new("COALESCE", parts) ascending ? node.asc : node.desc end diff --git a/app/views/author_credit_divergences/_preference_group.html.erb b/app/views/author_credit_divergences/_preference_group.html.erb index 9d0e2bf9cb..9cd146de51 100644 --- a/app/views/author_credit_divergences/_preference_group.html.erb +++ b/app/views/author_credit_divergences/_preference_group.html.erb @@ -40,12 +40,11 @@ <%= hidden_field_tag :record_type, record.class.name, id: nil %> <%= hidden_field_tag :record_id, record.id, id: nil %> <%= render "filter_fields" %> - <%# "None" clears the snapshot so the item just follows the profile. Hidden - for anonymous items — clearing that value would de-anonymize them. %> - <% allow_clear = record.author_credit_preference != AuthorCreditable::ANONYMOUS %> + <%# "None" clears the snapshot so the item just follows the profile — + offered on anonymous items too, since that's how they're reconciled. %> <%= select_tag "author_credit_preference", options_for_select(AuthorCreditable::ADMIN_FORM_OPTIONS.to_a, record.author_credit_preference), - include_blank: (allow_clear ? "None (follow profile)" : false), + include_blank: "None (follow profile)", id: nil, class: "rounded-md border-gray-300 text-sm" %> <%= submit_tag "Save", diff --git a/app/views/community_news/_form.html.erb b/app/views/community_news/_form.html.erb index 51a3c9ba1f..dd87bdd482 100644 --- a/app/views/community_news/_form.html.erb +++ b/app/views/community_news/_form.html.erb @@ -54,12 +54,13 @@ as: :select, label: "Author", required: true, - collection: f.object.author_person.present? ? [[ f.object.author_person.remote_search_label[:label], f.object.author_person.id ]] : [], - selected: f.object.author_id || current_user&.person_id, + collection: author_picker_person(f.object).present? ? [[ author_picker_person(f.object).remote_search_label[:label], author_picker_person(f.object).id ]] : [], + selected: author_picker_person(f.object)&.id, input_html: { required: true, class: "block w-full rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500 sm:text-sm", data: { controller: "remote-select", remote_select_model_value: "person" } } %> + <%= render "shared/author_credit_note", record: f.object %> <%= render "shared/author_credit_warning", record: f.object %>
diff --git a/app/views/resources/_form.html.erb b/app/views/resources/_form.html.erb index bdb02ea15a..3a1bb122b1 100644 --- a/app/views/resources/_form.html.erb +++ b/app/views/resources/_form.html.erb @@ -62,8 +62,8 @@
<%= f.input :author_id, as: :select, - collection: f.object.author_person.present? ? [[ f.object.author_person.remote_search_label[:label], f.object.author_person.id ]] : [], - selected: f.object.author_person&.id || current_user.person_id, + collection: author_picker_person(f.object).present? ? [[ author_picker_person(f.object).remote_search_label[:label], author_picker_person(f.object).id ]] : [], + selected: author_picker_person(f.object)&.id, include_blank: "Select an author", label: (f.object.author ? ( link_to "Resource author", @@ -73,9 +73,7 @@ input_html: { class: "w-full rounded border-gray-300", data: { controller: "remote-select", remote_select_model_value: "person" } } %> - <% if f.object.legacy_author_name.present? %> -

Legacy author credit: <%= f.object.legacy_author_name %>

- <% end %> + <%= render "shared/author_credit_note", record: f.object %>
diff --git a/app/views/shared/_author_credit_note.html.erb b/app/views/shared/_author_credit_note.html.erb new file mode 100644 index 0000000000..bff2d6dcdf --- /dev/null +++ b/app/views/shared/_author_credit_note.html.erb @@ -0,0 +1,25 @@ +<%# Sits under an author picker and says what the credit actually resolves to when + that isn't just the selected author: the person's profile suppresses credits, or + no author is set and a legacy free-text name is standing in. Locals: record. %> +<% person = author_picker_person(record) %> +<% if person&.contributions_anonymous? %> +

+ + + <%= person.full_name %>'s profile marks contributions anonymous, so this credit renders + “Anonymous” wherever it appears. + <%= link_to "Change it on their profile", edit_person_path(person, anchor: "profile-preferences"), + target: "_blank", rel: "noopener", title: "Opens in a new tab", + class: "underline hover:text-amber-900" %> + +

+<% elsif record.author.blank? && record.legacy_author_name_text.present? %> +

+ + + No author is set, so this is credited to the legacy name + “<%= record.legacy_author_name_text %>”, which links nowhere and follows nobody's + profile. Pick a person to replace it. + +

+<% end %> diff --git a/app/views/stories/_form.html.erb b/app/views/stories/_form.html.erb index 2c0b669bcf..d623c29ccd 100644 --- a/app/views/stories/_form.html.erb +++ b/app/views/stories/_form.html.erb @@ -240,12 +240,12 @@
- <% story_author_id = f.object.author_person&.id || current_user.person_id %> + <% story_author = author_picker_person(f.object) %> <%= f.input :author_id, - collection: f.object.author_person.present? ? [[ f.object.author_person.remote_search_label[:label], f.object.author_person.id ]] : [], + collection: story_author.present? ? [[ story_author.remote_search_label[:label], story_author.id ]] : [], prompt: "Select an author", - selected: story_author_id, - hint: "Defaults to the creator; change to credit someone else.", + selected: story_author&.id, + hint: "Defaults to the creator on a new story; change to credit someone else.", label: (f.object.author ? ( link_to "Story author", person_path(f.object.author), @@ -255,6 +255,7 @@ data: { controller: "remote-select", remote_select_model_value: "person" } } %> + <%= render "shared/author_credit_note", record: f.object %>
diff --git a/app/views/workshop_variations/_form.html.erb b/app/views/workshop_variations/_form.html.erb index d448f7353d..8128d6ed61 100644 --- a/app/views/workshop_variations/_form.html.erb +++ b/app/views/workshop_variations/_form.html.erb @@ -103,8 +103,8 @@ link_to "Variation author", person_path(f.object.author), class: "hover:underline") : "Variation author").html_safe, - collection: f.object.author_person.present? ? [[ f.object.author_person.remote_search_label[:label], f.object.author_person.id ]] : [], - selected: f.object.author_person&.id || current_user.person_id, + collection: author_picker_person(f.object).present? ? [[ author_picker_person(f.object).remote_search_label[:label], author_picker_person(f.object).id ]] : [], + selected: author_picker_person(f.object)&.id, include_blank: true, input_html: { data: { @@ -112,6 +112,7 @@ remote_select_model_value: "person" } } %> + <%= render "shared/author_credit_note", record: f.object %>
<% end %>
diff --git a/app/views/workshops/_form.html.erb b/app/views/workshops/_form.html.erb index d88df7e27c..0d21e3384f 100644 --- a/app/views/workshops/_form.html.erb +++ b/app/views/workshops/_form.html.erb @@ -62,14 +62,15 @@ link_to "Author", person_path(f.object.author), class: "hover:underline") : "Author").html_safe, - collection: f.object.author_person.present? ? [[ f.object.author_person.remote_search_label[:label], f.object.author_person.id ]] : [], - selected: (f.object.author_person&.id || current_user.person_id), - hint: "Credited author — any person, not just active users. Defaults to the creator; change to credit someone else.", + collection: author_picker_person(f.object).present? ? [[ author_picker_person(f.object).remote_search_label[:label], author_picker_person(f.object).id ]] : [], + selected: author_picker_person(f.object)&.id, + hint: "Credited author — any person, not just active users. Defaults to the creator on a new workshop; change to credit someone else.", input_html: { class: "block w-full rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500 sm:text-sm", data: { controller: "remote-select", remote_select_model_value: "person" } } %> + <%= render "shared/author_credit_note", record: f.object %>
<%= render "shared/author_credit_warning", record: f.object %>
diff --git a/spec/requests/author_credit_divergences_spec.rb b/spec/requests/author_credit_divergences_spec.rb index 3c37fcbda6..ead0383cda 100644 --- a/spec/requests/author_credit_divergences_spec.rb +++ b/spec/requests/author_credit_divergences_spec.rb @@ -176,14 +176,14 @@ expect(story.reload.author_credit_preference).to be_nil end - it "refuses to clear the snapshot of an item submitted anonymously" do + it "clears the snapshot of an item submitted anonymously, handing it to the profile" do story.update_column(:author_credit_preference, "anonymous") patch update_item_author_credit_divergences_path, params: { record_type: "Story", record_id: story.id, author_credit_preference: "" } - expect(story.reload.author_credit_preference).to eq("anonymous") - expect(flash[:alert]).to be_present + expect(story.reload.author_credit_preference).to be_nil + expect(story.author_credit).to eq(person.full_name) end it "makes one item anonymous without touching the person's others" do From 11fc6307c8f3dac1054ef82881a3f00ce3cb7e85 Mon Sep 17 00:00:00 2001 From: maebeale Date: Mon, 10 Aug 2026 16:52:29 -0400 Subject: [PATCH 12/12] Rename contributions_anonymous, and stop legacy credits resolving to the creator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The column reads better as the state it is than as a predicate on a collection, and it now matches the "Anonymous contributions" label already used in the profile form and the factory trait. The migration is unmerged, so it changes in place rather than stacking a rename on top of itself. A legacy free-text name follows nobody's profile, so a legacy-credited row now reports no governing person. It was being grouped on the divergences page under whoever happened to enter it, asserting that person's profile had drifted from a consent snapshot that never described them. Those rows have to be matched to a real person by hand against the legacy text — the creator is not a fallback for them. Community news author is optional on the model, so the form offers a blank option rather than forcing whoever edits one of the legacy authorless rows to credit somebody before they can save anything else. Co-Authored-By: Claude Opus 5 (1M context) --- .../author_credit_divergences_controller.rb | 2 +- app/controllers/people_controller.rb | 6 ++--- app/models/concerns/author_creditable.rb | 23 ++++++++++++++----- app/models/person.rb | 4 ++-- .../author_credit_divergence_query.rb | 2 +- .../_preference_group.html.erb | 8 +++---- app/views/community_news/_form.html.erb | 7 +++--- app/views/people/_form.html.erb | 2 +- app/views/shared/_author_credit_note.html.erb | 2 +- .../shared/_author_credit_preview.html.erb | 2 +- .../shared/_author_credit_warning.html.erb | 2 +- ...add_author_credit_preferences_to_people.rb | 6 ++--- db/schema.rb | 2 +- db/seeds/dev/people_profiles.rb | 2 +- spec/factories/people.rb | 2 +- spec/models/community_news_spec.rb | 2 +- spec/models/person_spec.rb | 6 ++--- spec/models/workshop_spec.rb | 9 ++++++++ .../author_credit_divergences_spec.rb | 10 ++++---- spec/requests/people_stories_section_spec.rb | 2 +- .../shared_examples/author_creditable.rb | 8 +++---- 21 files changed, 65 insertions(+), 44 deletions(-) diff --git a/app/controllers/author_credit_divergences_controller.rb b/app/controllers/author_credit_divergences_controller.rb index fc1a52b442..8a590d9122 100644 --- a/app/controllers/author_credit_divergences_controller.rb +++ b/app/controllers/author_credit_divergences_controller.rb @@ -91,7 +91,7 @@ def authorize_page end def person_params - params.require(:person).permit(:display_name_preference, :contributions_anonymous) + params.require(:person).permit(:display_name_preference, :anonymous_contributions) end # Carried through every redirect so the admin lands back on the same filtered list. diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 864a05f346..308ffb20ab 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -301,11 +301,11 @@ def check_duplicates # Anonymously-credited content is listed on the profile only for the person # themselves and admins — showing it to anyone else would tie an "Anonymous" - # credit back to a name. `contributions_anonymous` anonymizes every item at once; + # credit back to a name. `anonymous_contributions` anonymizes every item at once; # otherwise only the items whose stored consent is "anonymous" are hidden. def visible_authored_content(scope) return scope if allowed_to?(:manage?, Person) || current_user&.person_id == @person.id - return scope.none if @person.contributions_anonymous? + return scope.none if @person.anonymous_contributions? scope.credited_openly end @@ -536,7 +536,7 @@ def person_params :mailing_list_consented, :bio, :shoutout_text, :notes, :display_name_preference, - :contributions_anonymous, + :anonymous_contributions, :pronouns, :profile_is_searchable, :profile_show_pronouns, diff --git a/app/models/concerns/author_creditable.rb b/app/models/concerns/author_creditable.rb index 04ca616fec..6aa6d5c39c 100644 --- a/app/models/concerns/author_creditable.rb +++ b/app/models/concerns/author_creditable.rb @@ -93,15 +93,26 @@ def author_credit_person # Anonymity is a one-way latch: the profile can set it, the record can set it, # and neither can strip it from the other. def credit_anonymous?(person) - person.contributions_anonymous? || author_credit_preference == ANONYMOUS + person.anonymous_contributions? || author_credit_preference == ANONYMOUS end - # True when the stored consent snapshot no longer agrees with the credited - # person's current profile — surfaced as a warning on the record's form and as a - # row on the author credit divergences page. + # The person whose profile actually formats this credit. A legacy free-text name + # follows nobody's profile, so a legacy-credited record has no governing person even + # when it has a creator — those have to be matched to a real person by hand, not + # resolved to whoever happened to enter them. + def credit_governing_person + person = primary_author_person + return person if person + return nil if legacy_author_name_text.present? + created_by&.person + end + + # True when the stored consent snapshot no longer agrees with the profile that + # governs this credit — surfaced as a warning on the record's form and as a row on + # the author credit divergences page. def author_credit_diverged? return false if author_credit_preference.blank? - person = author_person + person = credit_governing_person person.present? && author_credit_preference != person.effective_author_credit_preference end @@ -211,7 +222,7 @@ def credited_person_match_sql(sql_alias) "(#{preference} = '#{value}' AND (#{expressions.map { |e| name_like(e) }.join(' OR ')}))" end - "(#{sql_alias}.contributions_anonymous = FALSE AND #{not_anonymous_sql} AND (#{by_preference.join(' OR ')}))" + "(#{sql_alias}.anonymous_contributions = FALSE AND #{not_anonymous_sql} AND (#{by_preference.join(' OR ')}))" end # Legacy free-text author names have no person, so only the record's own diff --git a/app/models/person.rb b/app/models/person.rb index ee5eb8e208..1c4ec3d444 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -73,7 +73,7 @@ class Person < ApplicationRecord validates :email_2_type, inclusion: { in: %w[work personal] }, allow_blank: true # How this person's name is formatted wherever it appears. Anonymity is not one of - # these — it's the separate `contributions_anonymous` flag, because a person still + # these — it's the separate `anonymous_contributions` flag, because a person still # has to be listed *somehow* on the people index. DISPLAY_NAME_PREFERENCES = %w[full_name first_name_last_initial first_name_only last_name_only].freeze @@ -240,7 +240,7 @@ def name # axis from the name format: it suppresses author credits without affecting how # they're listed on the people index. See AuthorCreditable. def effective_author_credit_preference - return "anonymous" if contributions_anonymous? + return "anonymous" if anonymous_contributions? display_name_preference.presence || "full_name" end diff --git a/app/services/author_credit_divergence_query.rb b/app/services/author_credit_divergence_query.rb index 9f522cbdb8..4002abb9a8 100644 --- a/app/services/author_credit_divergence_query.rb +++ b/app/services/author_credit_divergence_query.rb @@ -184,7 +184,7 @@ def unattributed_records def group_by_person(records) records - .group_by(&:author_person) + .group_by(&:credit_governing_person) .filter_map { |person, grouped| build_group(person, grouped) } .sort_by { |group| [ group.person.first_name.to_s.downcase, group.person.last_name.to_s.downcase ] } end diff --git a/app/views/author_credit_divergences/_preference_group.html.erb b/app/views/author_credit_divergences/_preference_group.html.erb index 9cd146de51..a2bc4f969e 100644 --- a/app/views/author_credit_divergences/_preference_group.html.erb +++ b/app/views/author_credit_divergences/_preference_group.html.erb @@ -78,10 +78,10 @@
diff --git a/app/views/community_news/_form.html.erb b/app/views/community_news/_form.html.erb index dd87bdd482..cdc03a67fa 100644 --- a/app/views/community_news/_form.html.erb +++ b/app/views/community_news/_form.html.erb @@ -50,14 +50,15 @@ selected: f.object.organization_id, input_html: { class: "block w-full rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500 sm:text-sm" } %> + <%# Optional on the model, so the form has to allow no author — the credit then + falls to the creator's person, or to "AWBW Staff" when there isn't one. %> <%= f.input :author_id, as: :select, label: "Author", - required: true, collection: author_picker_person(f.object).present? ? [[ author_picker_person(f.object).remote_search_label[:label], author_picker_person(f.object).id ]] : [], selected: author_picker_person(f.object)&.id, - input_html: { required: true, - class: "block w-full rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500 sm:text-sm", + include_blank: "Select an author", + input_html: { class: "block w-full rounded-md border-gray-300 shadow-sm focus:ring-blue-500 focus:border-blue-500 sm:text-sm", data: { controller: "remote-select", remote_select_model_value: "person" } } %> <%= render "shared/author_credit_note", record: f.object %> diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index 4745549abd..8b674f620b 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -476,7 +476,7 @@ hint: "Applies everywhere this person's name appears, including author credits", selected: f.object.display_name_preference || "full_name" %> - <%= f.input :contributions_anonymous, + <%= f.input :anonymous_contributions, label: "Anonymous contributions", hint: "Author credit only" %> diff --git a/app/views/shared/_author_credit_note.html.erb b/app/views/shared/_author_credit_note.html.erb index bff2d6dcdf..d133be0842 100644 --- a/app/views/shared/_author_credit_note.html.erb +++ b/app/views/shared/_author_credit_note.html.erb @@ -2,7 +2,7 @@ that isn't just the selected author: the person's profile suppresses credits, or no author is set and a legacy free-text name is standing in. Locals: record. %> <% person = author_picker_person(record) %> -<% if person&.contributions_anonymous? %> +<% if person&.anonymous_contributions? %>

diff --git a/app/views/shared/_author_credit_preview.html.erb b/app/views/shared/_author_credit_preview.html.erb index 2007ce5fe3..5372fd47de 100644 --- a/app/views/shared/_author_credit_preview.html.erb +++ b/app/views/shared/_author_credit_preview.html.erb @@ -3,7 +3,7 @@ <% person = current_user&.person %>

<% if person %> - You'll be credited as <%= person.contributions_anonymous? ? "Anonymous" : person.name %>. + You'll be credited as <%= person.anonymous_contributions? ? "Anonymous" : person.name %>. <% else %> You'll be credited as Anonymous. <% end %> diff --git a/app/views/shared/_author_credit_warning.html.erb b/app/views/shared/_author_credit_warning.html.erb index 5c6aedae6f..52cc57bb7f 100644 --- a/app/views/shared/_author_credit_warning.html.erb +++ b/app/views/shared/_author_credit_warning.html.erb @@ -2,7 +2,7 @@ Credits render from the profile, so on most records this is silent. %> <% if record.persisted? && record.author_credit_diverged? %> <% stored = AuthorCreditable::ADMIN_FORM_OPTIONS.key(record.author_credit_preference) %> - <% person = record.author_person %> + <% person = record.credit_governing_person %> <% profile = AuthorCreditable::ADMIN_FORM_OPTIONS.key(person.effective_author_credit_preference) %> <% if record.author_credit_preference == AuthorCreditable::ANONYMOUS %> diff --git a/db/migrate/20260804140743_add_author_credit_preferences_to_people.rb b/db/migrate/20260804140743_add_author_credit_preferences_to_people.rb index fff9282b0a..efc433525b 100644 --- a/db/migrate/20260804140743_add_author_credit_preferences_to_people.rb +++ b/db/migrate/20260804140743_add_author_credit_preferences_to_people.rb @@ -1,7 +1,7 @@ class AddAuthorCreditPreferencesToPeople < ActiveRecord::Migration[8.0] def up - unless column_exists?(:people, :contributions_anonymous) - add_column :people, :contributions_anonymous, :boolean, default: false, null: false + unless column_exists?(:people, :anonymous_contributions) + add_column :people, :anonymous_contributions, :boolean, default: false, null: false end # Stamped when an admin resolves this person on the author credit divergences @@ -12,7 +12,7 @@ def up end def down - remove_column :people, :contributions_anonymous, if_exists: true + remove_column :people, :anonymous_contributions, if_exists: true remove_column :people, :author_credit_reconciled_at, if_exists: true end end diff --git a/db/schema.rb b/db/schema.rb index ee1521a41a..7c2f31a314 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1005,11 +1005,11 @@ end create_table "people", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| + t.boolean "anonymous_contributions", default: false, null: false t.datetime "author_credit_reconciled_at" t.string "best_time_to_call" t.text "bio" t.boolean "blog_contributor", default: false, null: false - t.boolean "contributions_anonymous", default: false, null: false t.datetime "created_at", null: false t.integer "created_by_id" t.date "date_of_birth" diff --git a/db/seeds/dev/people_profiles.rb b/db/seeds/dev/people_profiles.rb index c9c65c2890..00b1396704 100644 --- a/db/seeds/dev/people_profiles.rb +++ b/db/seeds/dev/people_profiles.rb @@ -124,7 +124,7 @@ # Spread the credit preferences across seeded people so the author credit # divergences page has something to triage in dev. display_name_preference: Person::DISPLAY_NAME_PREFERENCES.sample, - contributions_anonymous: [ true, false, false, false ].sample, + anonymous_contributions: [ true, false, false, false ].sample, created_by: admin_user, updated_by: admin_user } diff --git a/spec/factories/people.rb b/spec/factories/people.rb index 0222e72497..35694d6d26 100644 --- a/spec/factories/people.rb +++ b/spec/factories/people.rb @@ -7,7 +7,7 @@ last_name { Faker::Name.last_name.gsub("'", " ") } trait :anonymous_contributions do - contributions_anonymous { true } + anonymous_contributions { true } end trait :with_organization do diff --git a/spec/models/community_news_spec.rb b/spec/models/community_news_spec.rb index 5dbf937c10..7fbc9826e6 100644 --- a/spec/models/community_news_spec.rb +++ b/spec/models/community_news_spec.rb @@ -110,7 +110,7 @@ end it 'stops matching once the author marks contributions anonymous' do - person.update!(contributions_anonymous: true) + person.update!(anonymous_contributions: true) expect(CommunityNews.search_by_params(query: 'John')).to be_empty end diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 98ac4aa135..dd722e6947 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -257,9 +257,9 @@ def term(cost_cents:, start_date: Date.current, subscription: nil) end end - it "is unaffected by contributions_anonymous" do + it "is unaffected by anonymous_contributions" do person.display_name_preference = "full_name" - person.contributions_anonymous = true + person.anonymous_contributions = true expect(person.name).to eq("Jane Doe") end end @@ -295,7 +295,7 @@ def term(cost_cents:, start_date: Date.current, subscription: nil) end it "is anonymous when contributions are anonymous, whatever the format" do - person.contributions_anonymous = true + person.anonymous_contributions = true expect(person.effective_author_credit_preference).to eq("anonymous") end end diff --git a/spec/models/workshop_spec.rb b/spec/models/workshop_spec.rb index ee535d092a..8a4e54bc13 100644 --- a/spec/models/workshop_spec.rb +++ b/spec/models/workshop_spec.rb @@ -124,6 +124,15 @@ author_credit_preference: "anonymous") expect(workshop.author_credit).to eq("Anonymous") end + + it "has no governing person, so the creator's profile can't be said to have drifted" do + creator.person.update!(display_name_preference: "first_name_only") + workshop = create(:workshop, created_by: creator, author: nil, full_name: "Jane Legacy", + author_credit_preference: "full_name") + + expect(workshop.credit_governing_person).to be_nil + expect(workshop.author_credit_diverged?).to be(false) + end end describe "#remote_search_label" do diff --git a/spec/requests/author_credit_divergences_spec.rb b/spec/requests/author_credit_divergences_spec.rb index ead0383cda..c31c8c41f1 100644 --- a/spec/requests/author_credit_divergences_spec.rb +++ b/spec/requests/author_credit_divergences_spec.rb @@ -41,7 +41,7 @@ it "updates the profile and stamps the person reconciled" do patch update_person_author_credit_divergences_path, - params: { id: person.id, person: { display_name_preference: "first_name_only", contributions_anonymous: "0" } } + params: { id: person.id, person: { display_name_preference: "first_name_only", anonymous_contributions: "0" } } expect(person.reload.display_name_preference).to eq("first_name_only") expect(person.author_credit_reconciled_at).to be_present @@ -49,15 +49,15 @@ it "can mark contributions anonymous" do patch update_person_author_credit_divergences_path, - params: { id: person.id, person: { display_name_preference: "full_name", contributions_anonymous: "1" } } + params: { id: person.id, person: { display_name_preference: "full_name", anonymous_contributions: "1" } } - expect(person.reload.contributions_anonymous).to be(true) + expect(person.reload.anonymous_contributions).to be(true) expect(story.reload.author_credit).to eq("Anonymous") end it "updates the results in place with a Turbo Stream instead of a full-page redirect" do patch update_person_author_credit_divergences_path, - params: { id: person.id, person: { display_name_preference: "first_name_only", contributions_anonymous: "0" } }, + params: { id: person.id, person: { display_name_preference: "first_name_only", anonymous_contributions: "0" } }, as: :turbo_stream expect(response.media_type).to eq(Mime[:turbo_stream]) @@ -67,7 +67,7 @@ it "carries the active filters through the redirect" do patch update_person_author_credit_divergences_path, params: { id: person.id, type: "Story", - person: { display_name_preference: "full_name", contributions_anonymous: "0" } } + person: { display_name_preference: "full_name", anonymous_contributions: "0" } } expect(response).to redirect_to(author_credit_divergences_path(type: "Story")) end diff --git a/spec/requests/people_stories_section_spec.rb b/spec/requests/people_stories_section_spec.rb index 3bd9eef8f6..efbe7d195a 100644 --- a/spec/requests/people_stories_section_spec.rb +++ b/spec/requests/people_stories_section_spec.rb @@ -50,7 +50,7 @@ def get_stories_section end it "never flags a spotlighted story, even when the person is anonymous" do - person.update!(contributions_anonymous: true) + person.update!(anonymous_contributions: true) create(:story, :published, title: "Spotlight Story", spotlighted_facilitator: person) get_stories_section diff --git a/spec/support/shared_examples/author_creditable.rb b/spec/support/shared_examples/author_creditable.rb index 1733c76156..2914f3c2b3 100644 --- a/spec/support/shared_examples/author_creditable.rb +++ b/spec/support/shared_examples/author_creditable.rb @@ -32,7 +32,7 @@ end context "when the profile marks contributions anonymous" do - before { person.update!(contributions_anonymous: true) } + before { person.update!(anonymous_contributions: true) } it "returns Anonymous regardless of the name format" do person.update!(display_name_preference: "full_name") @@ -48,7 +48,7 @@ before { record.update!(author_credit_preference: "anonymous") } it "stays anonymous even though the profile says otherwise" do - person.update!(display_name_preference: "full_name", contributions_anonymous: false) + person.update!(display_name_preference: "full_name", anonymous_contributions: false) expect(record.author_credit).to eq("Anonymous") end @@ -94,7 +94,7 @@ end it "records anonymous when the profile suppresses credits" do - person.update!(contributions_anonymous: true) + person.update!(anonymous_contributions: true) record = create(factory, created_by: author_user, author_credit_preference: nil) expect(record.reload.author_credit_preference).to eq("anonymous") end @@ -166,7 +166,7 @@ end it "matches nothing when the profile marks contributions anonymous" do - person.update!(contributions_anonymous: true) + person.update!(anonymous_contributions: true) expect(described_class.by_credited_person_name("Zephyrine")).not_to include(record) expect(described_class.by_credited_person_name("Quixotel")).not_to include(record) end