diff --git a/AGENTS.md b/AGENTS.md index a14ae0cfa5..99a3e191e6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -143,7 +143,7 @@ This codebase (Rails 8.1) | `SectorsTaggable` | Enforces a single primary sector for sector-tagged owners | | `TagFilterable` | Scope-based filtering by tag names | | `Trendable` | Trending metrics tracking | -| `UserStampable` | Stamps `updated_by_id` from `Current.user` on every write (no-op without the column) | +| `UserStampable` | Stamps `created_by_id`/`updated_by_id` from `Current.user` (no-op without the columns) | | `WindowsTypeFilterable` | Filter by WindowsType association | ## Controllers diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 93928cb6d8..11f5fb0edf 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -13,7 +13,6 @@ def index def create authorize! @comment = @commentable.comments.build(comment_params) - @comment.created_by = current_user @comment.updated_by = current_user if @comment.save diff --git a/app/controllers/community_news_controller.rb b/app/controllers/community_news_controller.rb index 10ce42cfe0..433d67dc00 100644 --- a/app/controllers/community_news_controller.rb +++ b/app/controllers/community_news_controller.rb @@ -61,7 +61,6 @@ def edit def create @community_news = CommunityNews.new(community_news_params) - @community_news.created_by = current_user authorize! @community_news success = false diff --git a/app/controllers/event_registrations_controller.rb b/app/controllers/event_registrations_controller.rb index 3dccb72f18..fdec9e354b 100644 --- a/app/controllers/event_registrations_controller.rb +++ b/app/controllers/event_registrations_controller.rb @@ -79,8 +79,6 @@ def create def update authorize! @event_registration @event_registration.assign_attributes(event_registration_update_params) - @event_registration.comments.select(&:new_record?).each { |c| c.created_by = current_user; c.updated_by = current_user } - @event_registration.comments.select { |c| c.persisted? && c.body_changed? }.each { |c| c.updated_by = current_user } # Inline-logged notifications are addressed to the registrant. recipient_email = @event_registration.registrant&.preferred_email.presence || "n/a" diff --git a/app/controllers/events/registrations_controller.rb b/app/controllers/events/registrations_controller.rb index 082ddf4779..b1c24ee508 100644 --- a/app/controllers/events/registrations_controller.rb +++ b/app/controllers/events/registrations_controller.rb @@ -170,7 +170,6 @@ def create_person_for_current_user first_name: current_user.first_name, last_name: current_user.last_name, email: current_user.email, - created_by: current_user, updated_by: current_user ) current_user.update!(person: person) diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index daa1c6052b..9064a58bb3 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -315,7 +315,6 @@ def send_reminder def create authorize! @event = Event.new(event_params) - @event.created_by ||= current_user success = false diff --git a/app/controllers/grants_controller.rb b/app/controllers/grants_controller.rb index d2ed0a6a94..33b1cce3eb 100644 --- a/app/controllers/grants_controller.rb +++ b/app/controllers/grants_controller.rb @@ -48,7 +48,6 @@ def edit def create @grant = Grant.new(grant_params) - @grant.created_by = current_user @grant.updated_by = current_user authorize! @grant diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index a5c7c1e17d..4d6eddcd19 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -127,8 +127,6 @@ def create def update authorize! @organization @organization.assign_attributes(organization_params) - @organization.comments.select(&:new_record?).each { |c| c.created_by = current_user; c.updated_by = current_user } - @organization.comments.select { |c| c.persisted? && c.body_changed? }.each { |c| c.updated_by = current_user } if @organization.save assign_associations(@organization) if params.dig(:organization, :category_ids) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 1928782486..7c8a19ec8f 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -224,8 +224,6 @@ def update attrs = person_params reject_locked_license_changes!(attrs) @person.assign_attributes(attrs) - @person.comments.select(&:new_record?).each { |c| c.created_by = current_user; c.updated_by = current_user } - @person.comments.select { |c| c.persisted? && c.body_changed? }.each { |c| c.updated_by = current_user } # Inline-logged notifications are addressed to the person. recipient_email = @person.preferred_email.presence || "n/a" diff --git a/app/controllers/resources_controller.rb b/app/controllers/resources_controller.rb index deb644e4d7..b213861531 100644 --- a/app/controllers/resources_controller.rb +++ b/app/controllers/resources_controller.rb @@ -103,7 +103,6 @@ def create def update @resource = Resource.find(params[:id]) authorize! @resource - @resource.created_by ||= current_user success = false Resource.transaction do @@ -192,7 +191,7 @@ def resource_params def load_forms form = @resource.form if form - @user_form = Report.new(created_by: current_user, owner: @resource) + @user_form = Report.new(owner: @resource) form.form_fields.where(status: 1).order(:position).each do |field| @user_form.report_form_field_answers.build(form_field: field) end diff --git a/app/controllers/scholarships_controller.rb b/app/controllers/scholarships_controller.rb index 7cdda55792..51aa7a4911 100644 --- a/app/controllers/scholarships_controller.rb +++ b/app/controllers/scholarships_controller.rb @@ -82,7 +82,6 @@ def update authorize! @scholarship @scholarship.assign_attributes(scholarship_params) - attribute_comment_authorship # Inline-logged notifications are addressed to the scholarship recipient. recipient_email = @scholarship.recipient&.preferred_email.presence || "n/a" @@ -212,16 +211,4 @@ def scholarship_params notifications_attributes: [ :id, :channel, :sender_id, :email_subject, :email_body_text, :noticeable_type, :noticeable_id, :_destroy ] ) end - - # Stamp authorship on comments edited through the scholarship form: author + editor - # on new ones, editor on existing ones whose body changed. - def attribute_comment_authorship - @scholarship.comments.select(&:new_record?).each do |c| - c.created_by = current_user - c.updated_by = current_user - end - @scholarship.comments.select { |c| c.persisted? && c.body_changed? }.each do |c| - c.updated_by = current_user - end - end end diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 34b618fabf..eac0333344 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -63,7 +63,6 @@ def edit def create @story = Story.new(story_params.except(:category_ids, :sector_ids)) - @story.created_by = current_user authorize! @story success = false diff --git a/app/controllers/story_ideas_controller.rb b/app/controllers/story_ideas_controller.rb index 3d40639a1a..f43e339133 100644 --- a/app/controllers/story_ideas_controller.rb +++ b/app/controllers/story_ideas_controller.rb @@ -33,7 +33,6 @@ def edit def create @story_idea = StoryIdea.new(story_idea_params.except(:category_ids, :sector_ids)) - @story_idea.created_by = current_user @story_idea.updated_by = current_user authorize! @story_idea diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 574ad17dc2..38a11cb90c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -99,7 +99,6 @@ def create # assign person person_id = params[:person_id].presence || params.dig(:user, :person_id).presence @user.person = Person.find(person_id) if person_id - @user.created_by = current_user @user.updated_by = current_user if @user.save @@ -134,8 +133,6 @@ def update @user.assign_attributes(user_params.except(:password, :password_confirmation)) @user.updated_by = current_user - @user.comments.select(&:new_record?).each { |c| c.created_by = current_user; c.updated_by = current_user } - @user.comments.select { |c| c.persisted? && c.body_changed? }.each { |c| c.updated_by = current_user } # Suppress Devise's automatic reconfirmation email so the interstitial can control it @user.skip_confirmation_notification! diff --git a/app/controllers/workshops_controller.rb b/app/controllers/workshops_controller.rb index 0650ef5286..de316651bb 100644 --- a/app/controllers/workshops_controller.rb +++ b/app/controllers/workshops_controller.rb @@ -82,7 +82,7 @@ def new @workshop = WorkshopFromIdeaService.new(@workshop_idea, user: current_user).call authorize! @workshop else - @workshop = Workshop.new(created_by: current_user) + @workshop = Workshop.new authorize! @workshop end set_form_variables @@ -156,8 +156,6 @@ def update success = false @workshop.assign_attributes(workshop_params) - @workshop.comments.select(&:new_record?).each { |c| c.created_by = current_user; c.updated_by = current_user } - @workshop.comments.select { |c| c.persisted? && c.body_changed? }.each { |c| c.updated_by = current_user } Workshop.transaction do if @workshop.save diff --git a/app/models/concerns/user_stampable.rb b/app/models/concerns/user_stampable.rb index ddc587e1bf..4798be5d02 100644 --- a/app/models/concerns/user_stampable.rb +++ b/app/models/concerns/user_stampable.rb @@ -1,25 +1,41 @@ module UserStampable extend ActiveSupport::Concern - # Stamps updated_by_id from Current.user on every write, so the last editor is - # recorded on the record itself. (created_by_id is set at create time by the - # controllers; only updated_by_id was going unset on updates.) Included on - # ApplicationRecord; the guard makes it a no-op for tables without the column. Runs - # on before_validation so it satisfies a required belongs_to :updated_by before the - # presence check. + # Stamps created_by_id / updated_by_id from Current.user so attribution lives on the + # record itself instead of being assigned by hand in every controller. Included on + # ApplicationRecord; the guards make it a no-op for tables without the columns. Runs + # on before_validation so it satisfies a required belongs_to before the presence + # check. included do - before_validation :stamp_updated_by + before_validation :stamp_user_columns end private - def stamp_updated_by + def stamp_user_columns user = Current.user return unless user + + stamp_created_by(user) + stamp_updated_by(user) + end + + # Only at creation, and always the authenticated actor — created_by is not a + # user-supplied value, so a mass-assigned created_by_id must not override it. A + # later editor never reaches this (guarded on new_record?), so it can't claim it. + def stamp_created_by(user) + return unless new_record? && has_attribute?(:created_by_id) + + self.created_by_id = user.id + end + + def stamp_updated_by(user) return unless has_attribute?(:updated_by_id) # Skip when the caller set updated_by_id explicitly (respect it), and when nothing # else changed (don't turn a no-op save into a write / spurious update event). + # A save whose only changes are to associations leaves the record itself unchanged, + # so those paths still assign updated_by by hand. return if updated_by_id_changed? return unless new_record? || changed? diff --git a/spec/models/concerns/user_stampable_spec.rb b/spec/models/concerns/user_stampable_spec.rb index e0fb3ee44b..f0383bcf48 100644 --- a/spec/models/concerns/user_stampable_spec.rb +++ b/spec/models/concerns/user_stampable_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" RSpec.describe UserStampable, type: :model do - # Banner carries both stamp columns and a required belongs_to :created_by. + # Banner carries both stamp columns, each a required belongs_to. let(:creator) { create(:user) } let(:editor) { create(:user) } @@ -10,26 +10,27 @@ def with_current(user, &block) end describe "on create" do - it "stamps updated_by from Current.user" do - banner = with_current(creator) { Banner.create!(content: "Hi", show: true, created_by: creator) } + it "stamps created_by and updated_by from Current.user" do + banner = with_current(creator) { Banner.create!(content: "Hi", show: true) } + expect(banner.created_by).to eq(creator) expect(banner.updated_by).to eq(creator) end - it "satisfies a required belongs_to :updated_by without an explicit assignment" do - expect { with_current(creator) { Banner.create!(content: "Hi", show: true, created_by: creator) } } + it "satisfies required belongs_to stamps without an explicit assignment" do + expect { with_current(creator) { Banner.create!(content: "Hi", show: true) } } .not_to raise_error end - it "leaves created_by to the caller" do + it "forces created_by to Current.user, ignoring a mass-assigned value" do banner = with_current(creator) { Banner.create!(content: "Hi", show: true, created_by: editor) } - expect(banner.created_by).to eq(editor) + expect(banner.created_by).to eq(creator) end end describe "on update" do - let!(:banner) { with_current(creator) { Banner.create!(content: "Hi", show: true, created_by: creator) } } + let!(:banner) { with_current(creator) { Banner.create!(content: "Hi", show: true) } } it "stamps updated_by with the current editor without touching created_by" do with_current(editor) { banner.update!(content: "Edited") } @@ -58,6 +59,7 @@ def with_current(user, &block) Banner.create!(content: "Hi", show: true, created_by: creator, updated_by: creator) end + expect(banner.created_by).to eq(creator) expect(banner.updated_by).to eq(creator) end end