Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for belongs_to whodunnit #1481

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

barodeur
Copy link

@barodeur barodeur commented Jul 4, 2024

Thank you really much for PaperTrail.

I ran into a bug while using PaperTrail in a specific way:

  • I'm using custom version classes
  • I'm using Active Record instances as whodunnit values to enforce foreign keys
    • instead of t.string :whodunnit I have t.references :whodunnit, foreign_key: {to_table: :users}
    • class CustomVersion < PaperTrailVersion
        belongs_to :whodunnit, class_name: "User"
      end

I really loved that this worked out of the box.
But I eventually ran into a weird behavior:

  • Given that the current PaperTrail.request.whodunnit is an instance of a User
  • When using PaperTrail.request(whodunnit:) with a block
  • Then the value set back on PaperTrail.request.whodunnit is a dup (new record) of the initial user instance.

This is particularly bad when creating another version right after because that new User dup will now be saved and duplicated.

I did my best to add a test to highlight the problem with a new test. I understand that using an AR model as whodunnit value is not necessarily supported, but the proposed changed should not break the API.

Thank you

Check the following boxes:

  • 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.

@barodeur barodeur marked this pull request as ready for review July 4, 2024 21:10
@barodeur barodeur changed the title Belongs to whodunnit Support for belongs_to whodunnit Jul 4, 2024
Copy link

github-actions bot commented Oct 3, 2024

This PR has been automatically marked as stale due to inactivity.
The resources of our volunteers are limited.
If this is something you are committed to continue working on, please address any concerns raised by review and/or ping us again.
Thank you for all your contributions.

@github-actions github-actions bot added the Stale label Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant