Skip to content

Centralize created_by stamping in UserStampable - #2026

Draft
maebeale wants to merge 2 commits into
maebeale/updated-by-stampingfrom
maebeale/centralize-created-by-stamping
Draft

Centralize created_by stamping in UserStampable#2026
maebeale wants to merge 2 commits into
maebeale/updated-by-stampingfrom
maebeale/centralize-created-by-stamping

Conversation

@maebeale

@maebeale maebeale commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

🤖 suggested review level: 5 Inspect 🔬 changes an anti-forgery security invariant (created_by force) and removes manual attribution across ~14 controllers

Stacked on #2024 — review/merge that first (base is maebeale/updated-by-stamping, not main).

Why

created_by was force-assigned to current_user in ~14 controllers — partly boilerplate, but where created_by_id is mass-assignable it also served as an anti-forgery override (tested: "records the current user as created_by regardless of submitted value" in stories/workshops/community_news specs).

What

  • UserStampable now also stamps created_by_id on create, forcing Current.user (via before_validation) so a mass-assigned created_by_id can't override it. Only on new_record?, so a later editor never claims created_by.
  • Removed the redundant manual created_by = current_user assignments from the create actions.
  • Removed the per-controller comment authorship loops (workshops, scholarships, organizations, people, event_registrations, users). Comment#body is a plain column and comments don't permit the stamp columns as nested params, so the concern already forces created_by and stamps updated_by on both new and edited comments — the loops were dead code.

Deliberately left alone

  • Services (WorkshopFromIdeaService, WorkshopVariationFromIdeaService, ProcessConfirmation) keep their explicit user: argument — they can run where Current.user isn't set, and the value may legitimately be nil (public self-registration).
  • Parent updated_by on association-only saves (e.g. @user.updated_by = current_user on user update). The concern only stamps updated_by when the record's own columns change; adding/removing a nested record leaves the parent unchanged, so those still assign by hand.

Tests

  • user_stampable_spec updated for the force behavior; ran the full request-spec set for every touched controller — all green. Notably scholarships_spec still asserts comment.created_by == admin through the nested form, confirming the concern covers comments.

@maebeale
maebeale force-pushed the maebeale/updated-by-stamping branch from a8fe1bb to 2eb95b4 Compare July 30, 2026 01:03
@maebeale
maebeale force-pushed the maebeale/centralize-created-by-stamping branch from b5607ef to 290d74f Compare July 31, 2026 00:06
@maebeale
maebeale force-pushed the maebeale/updated-by-stamping branch from f5645fe to 326c1f0 Compare July 31, 2026 00:08
maebeale and others added 2 commits July 30, 2026 20:09
…er assignments

created_by was force-assigned to current_user in ~14 controllers, both as
boilerplate and (where created_by_id is mass-assignable) as an anti-forgery
override. Fold that into the concern: stamp created_by from Current.user on create,
forcing the authenticated actor so a submitted created_by_id can't override it.
Remove the now-redundant manual assignments. Services keep their explicit user:
argument (they may run where Current.user isn't set); updated_by manual lines stay
(they cover association-only saves the concern's changed? guard skips).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-controller comment loops hand-stamped created_by/updated_by on new and
edited comments. Comment#body is a plain column and comments don't permit the stamp
columns as nested params, so the concern already forces created_by and stamps
updated_by on both new and column-changed comments — the loops were dead code (and
one was left half-gutted after dropping its created_by assignment). Drop them; keep
the parent updated_by stamp on user update, which covers association-only saves the
concern's changed? guard skips.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@maebeale
maebeale force-pushed the maebeale/centralize-created-by-stamping branch from 290d74f to 8505f3b Compare July 31, 2026 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant