<%= group.column %>
+ | Content | +Type | +Renders as | +Stored consent | +
|---|---|---|---|
| <%= divergence_record_link(record) %> | +<%= record.class.name.underscore.humanize %> | +<%= record.author_credit %> | ++ <%= form_with url: update_item_author_credit_divergences_path, method: :patch, + 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" %> + <%# "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", + 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. %> ++ <%= message %> +
+ <% if local_assigns[:cleanup].present? %> +<%= cleanup %>
+ <% end %> +| Content | +Type | +<%= name_header %> | +Credit to | +
|---|---|---|---|
| <%= divergence_record_link(row.record) %> | +<%= row.record.class.name.underscore.humanize %> | +<%= row.record.author_credit %> | ++ <%= render "assign_author_form", record: row.record, suggested: row.suggested_author %> + | +
<%= suggestion_note %>
+ <% end %> +Nothing matches these filters.
++ Clear them to see whether anything is left to reconcile overall. +
+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.
+
+ Work these top to bottom. A record can appear in more than one section when it needs + more than one fix. +
+ + <%# ── 1. Stored consent snapshot drifted from 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.
+
+ 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.
+
+ 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.
+
<%= pluralize(@result.creator.size, "person") %> to confirm.
++ No author, no legacy name, and no person behind the creating account, so these fall back to a + 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 generic placeholder names, since no record falls back to one any more." %> + <% else %> + <%= render "unlinked_table", rows: assignable_rows(@result.unattributed), name_header: "Renders as" %> + <% end %> +
+ 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.
+
+ 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. +
+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 de575e46b7..f5d7cb2e23 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..3c37fcbda6 --- /dev/null +++ b/spec/requests/author_credit_divergences_spec.rb @@ -0,0 +1,207 @@ +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 "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", + 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/assign_author" do + before { sign_in admin } + + 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") + + 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 } + + 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 "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") + + 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) + + 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/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/routing/author_credit_divergences_routing_spec.rb b/spec/routing/author_credit_divergences_routing_spec.rb new file mode 100644 index 0000000000..a08bcedb8d --- /dev/null +++ b/spec/routing/author_credit_divergences_routing_spec.rb @@ -0,0 +1,24 @@ +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 #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") + 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..e0ed363411 --- /dev/null +++ b/spec/services/author_credit_divergence_query_spec.rb @@ -0,0 +1,214 @@ +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.preference).to be_empty + end + + it "groups diverging records under their credited person" do + story = diverged_story + + groups = described_class.new.call.preference + + 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.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.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.preference.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.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.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.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 + 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") + + expect(legacy_records_for("workshops.full_name")).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(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.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 + + 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 + + create(:story, created_by: create(:user, person: nil), author: nil) + expect(described_class.new.call).not_to be_empty + 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..7f4f482bdb 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 "does not default new records" do - expect(described_class.new.author_credit_preference).to be_blank - end + 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 - 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 + + it "does not link the credit to a profile" do + expect(record.author_credit_person).to be_nil + 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,96 @@ 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 "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) + + 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 aa9a95bf20..a0686c38e0 100644 --- a/spec/views/page_bg_class_alignment_spec.rb +++ b/spec/views/page_bg_class_alignment_spec.rb @@ -124,6 +124,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