Skip to content

Fix spurious versions from touch events with no changes#1546

Open
mattmenefee wants to merge 1 commit intopaper-trail-gem:masterfrom
mattmenefee:fix-empty-touch-versions
Open

Fix spurious versions from touch events with no changes#1546
mattmenefee wants to merge 1 commit intopaper-trail-gem:masterfrom
mattmenefee:fix-empty-touch-versions

Conversation

@mattmenefee
Copy link
Copy Markdown

@mattmenefee mattmenefee commented Jan 23, 2026

Summary

Fixes #1465 — spurious empty update versions being created when using ActiveStorage.

The Problem

When has_one_attached (ActiveStorage) calls touch on the parent record after attaching a blob, Paper Trail creates an empty version with no meaningful object_changes. This happens because Events::Update#changed_notably? unconditionally returned true for touch events when changes_in_latest_version was empty.

Before this fix:

def changed_notably?
  if @is_touch && changes_in_latest_version.empty?
    true  # Always creates a version, even with no changes
  else
    super
  end
end

The Fix

By removing this override, we delegate to the base class implementation which correctly returns false when there are no notable changes tracked.

After this fix:

  • Touch events that update updated_at → version IS created (because Rails tracks the change)
  • Touch events with no tracked changes → version is NOT created (fixes spurious versions)

Breaking Change Notice

This is technically a breaking change for users who rely on the old behavior of always creating versions for touch events. If the maintainers prefer a backward-compatible approach, an alternative would be to add an opt-in configuration option:

# Alternative approach: add a configuration option
has_paper_trail skip_touch_with_no_changes: true

# Or globally:
PaperTrail.config.has_paper_trail_defaults = {
  skip_touch_with_no_changes: true
}

The implementation for the opt-in approach would be:

def changed_notably?
  if @is_touch && changes_in_latest_version.empty?
    !@record.paper_trail_options[:skip_touch_with_no_changes]
  else
    super
  end
end

Happy to implement this alternative if preferred.

Test Plan

  • Existing test suite passes (CI will verify)
  • Tests that expect touch to create versions may need updating if Rails doesn't track the updated_at change
  • Manual testing confirms ActiveStorage attachments no longer create spurious versions

Related


Contribution Checklist

  • Wrote good commit messages.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new
    code introduces user-observable changes.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.

Notes on unchecked items

  • Up-to-date with master: Will rebase if requested by maintainers.
  • Tests: The existing touch-related tests in the suite cover this behavior. The change removes an override that forced version creation; the base class logic (already tested) now handles touch events consistently. Happy to add a dedicated regression test for the ActiveStorage scenario if desired.
  • Changelog: This is a behavior change that warrants a changelog entry. Holding off until maintainers confirm whether this should land as a breaking change or behind a configuration option, which would affect the changelog section it belongs in.

Previously, `changed_notably?` in `Events::Update` returned `true` for
touch events even when `changes_in_latest_version` was empty. This
caused spurious version records to be created, particularly when using
ActiveStorage, which calls `touch` after attaching blobs.

By removing this override, we now delegate to the base class
implementation, which only creates versions when there are actual
notable changes tracked by Rails' dirty tracking.

Fixes paper-trail-gem#1465

# If it is a touch event, and changed are empty, it is assumed to be
# implicit `touch` mutation, and will a version is created.
# NOTE: We previously overrode `changed_notably?` here to return `true`
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably not necessary to retain this comment, so I can remove it after the changes are approved and before this is merged.

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.

Empty "update" versions when using active storage

1 participant