Record who created and updated incidents#105
Conversation
Stamp the acting user's name onto incidents and their timeline updates (created_by / updated_by) and show it on the admin incidents views. The public status page is unchanged and never shows an author. System-posted maintenance updates have no author. Also fixes the dummy app's persistent database migrations_paths so engine persistent migrations resolve from the engine root, mirroring the primary database's dual-path config.
There was a problem hiding this comment.
Pull request overview
This PR adds lightweight author attribution to incidents and incident timeline updates by stamping the acting user’s name into created_by / updated_by, displaying it in admin incident views, and ensuring the public status page does not reveal authors. It also fixes the dummy app’s migrations_paths for persistent migrations so engine migrations are found when running from the engine root.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Add persistent DB columns for
incidents.created_by,incidents.updated_by, andincident_updates.created_by, plus schema updates. - Stamp author fields from
Upright::Current.uservia model callbacks and render author info in admin incident UI (with “System” fallback for nil authors). - Fix dummy app persistent DB
migrations_pathsto include both engine-root and dummy-root paths; add tests covering author visibility and system maintenance behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/models/upright/incident_test.rb | Adds coverage for stamping behavior and system maintenance updates (but needs Current reset between tests). |
| test/integration/public/incidents_controller_test.rb | Ensures public incident detail never displays author (but uses assert_no_match incorrectly). |
| test/dummy/db/persistent_schema.rb | Updates persistent schema version and adds new author columns. |
| test/dummy/config/database.yml | Expands migrations_paths for the dummy persistent DB to resolve from engine root or dummy root. |
| db/persistent_migrate/20260709000001_add_created_by_to_incidents.rb | Adds persistent migration for new author columns. |
| app/views/upright/incidents/edit.html.erb | Displays update author on the incident timeline (admin). |
| app/views/upright/incidents/_row.html.erb | Displays incident “Declared by …” metadata in incident rows (admin). |
| app/models/upright/incident.rb | Stamps created_by and updated_by from Upright::Current.user. |
| app/models/upright/incident_update.rb | Stamps created_by on incident updates from Upright::Current.user. |
| app/assets/stylesheets/upright/incidents.css | Styles the new timeline author label. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| get upright.public_incident_path(incident) | ||
|
|
||
| assert_response :success | ||
| assert_no_match "Ada Lovelace", response.body |
There was a problem hiding this comment.
Fixed — using a regexp literal.
Move the "System" fallback for updates with no acting user out of the view and into IncidentUpdate. Exercise the behavior through the incident fixtures instead of building records inline.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
test/test_helper.rb:33
acting_assetsUpright::Current.userbut nothing clears it between tests. Since the new callbacks depend onUpright::Current.user, this can leak state across tests and make later incident creations get stamped unexpectedly. Clearing the current user in ateardownkeeps tests isolated.
include CurrentUserHelper
include IpApiHelper
include MtrHelper
include RailsEnvHelper
include SiteHelper
| get upright.public_incident_path(incident) | ||
|
|
||
| assert_response :success | ||
| assert_no_match "Lewis Buckley", response.body |
There was a problem hiding this comment.
Fixed — using a regexp literal.
| <li class="timeline__item"> | ||
| <span class="timeline__status"><%= update.status.humanize %></span> | ||
| <time class="timeline__time" datetime="<%= update.created_at.iso8601 %>"><%= update.created_at.strftime("%b %-d, %H:%M %Z") %></time> | ||
| <span class="timeline__author">by <%= update.created_by %></span> |
There was a problem hiding this comment.
Won't backfill — this system isn't live yet, so there are no pre-existing updates with a null author. Going forward every update is stamped on create (a name, or "System" when there's no acting user), so the timeline never renders an empty "by ".
|
|
||
| validates :status, presence: true | ||
|
|
||
| before_create { self.created_by ||= Upright::Current.user&.name || "System" } |
Stamps the acting user's name onto incidents and their timeline updates (
created_by/updated_by) and surfaces it on the admin/incidentsviews. The public status page never shows an author. Automated maintenance updates have no author.Also fixes the dummy app's persistent
migrations_pathsso engine persistent migrations resolve from the engine root (mirrors the primary DB's dual-path config); previouslyapp:db:migratesilently applied zero persistent migrations.