Skip to content

Support alternative paper_trail versions association name#3354

Merged
mshibuya merged 1 commit into
railsadminteam:masterfrom
fuegas:support-alternative-paper-trail-versions-association
Jun 29, 2021
Merged

Support alternative paper_trail versions association name#3354
mshibuya merged 1 commit into
railsadminteam:masterfrom
fuegas:support-alternative-paper-trail-versions-association

Conversation

@fuegas

@fuegas fuegas commented Apr 22, 2021

Copy link
Copy Markdown
Contributor

When using PaperTrail as an auditing gem, you can specify an alternative versions association by defining the versions hash in the has_paper_trail call. Initialization of PaperTrail (per model) stores the name of the association (since paper-trail-gem/paper_trail@3032b53, 23rd of July 2011) in versions_association_name. Using this method we can retrieve the history of a model when it uses a different name for the versions association.

For example, using the following configuration won't break the auditing_adapter with this change:

has_paper_trail(
  version: :paper_trail_version,
  versions: {
    name: :paper_trail_versions,
  },
)

current_page = page.presence || '1'

versions = object.nil? ? versions_for_model(model) : object.versions
versions = object.nil? ? versions_for_model(model) : object.send(model.model.versions_association_name)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe .versions_association_name is public

Suggested change
versions = object.nil? ? versions_for_model(model) : object.send(model.model.versions_association_name)
versions = object.nil? ? versions_for_model(model) : object.public_send(model.model.versions_association_name)

Also is this covered by any existing tests?

@codealchemy codealchemy Apr 29, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sidenote - this solves the same problem as #3269 with a slightly different approach (IIRC object.version_association_name will execute a SQL query, while calling from the model does not)

@fuegas fuegas Apr 29, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is public indeed, I've changed it to public_send (in 09449a6 to keep it squashed). I used send to keep it in line with the other send usage in the file.

There was no test covering the versions usage, only one regarding the user used for the audit.

When using PaperTrail as an auditing gem, you can specify an alternative versions association by defining the `versions` hash in the `has_paper_trail` call. Initialization of PaperTrail (per model) stores the name of the association (since paper-trail-gem/paper_trail@3032b53, 23rd of July 2011) in `versions_association_name`. Using this method we can retrieve the history of a model when it uses a different name for the versions association.

For example, using the following configuration won't break the auditing_adapter with this change:
```ruby
has_paper_trail(
  version: :paper_trail_version,
  versions: {
    name: :paper_trail_versions,
  },
)
```
@fuegas fuegas force-pushed the support-alternative-paper-trail-versions-association branch from cec972c to 09449a6 Compare April 29, 2021 16:58
@fuegas

fuegas commented May 6, 2021

Copy link
Copy Markdown
Contributor Author

@codealchemy would you consider either of proposed solutions? 🙂

@codealchemy

Copy link
Copy Markdown
Contributor

@codealchemy would you consider either of proposed solutions? 🙂

I'm afraid anything beyond providing feedback here is out of my hands cc: @mshibuya

@fuegas

fuegas commented Jun 17, 2021

Copy link
Copy Markdown
Contributor Author

Hi @mshibuya, any chance you might have some time to look at this (small) change? 😄

mshibuya added a commit that referenced this pull request Jun 29, 2021
mshibuya added a commit that referenced this pull request Jun 29, 2021
mshibuya added a commit that referenced this pull request Jun 29, 2021
@mshibuya mshibuya merged commit f78f060 into railsadminteam:master Jun 29, 2021
@mshibuya

Copy link
Copy Markdown
Member

Thanks!

@fuegas fuegas deleted the support-alternative-paper-trail-versions-association branch June 29, 2021 11:30
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.

3 participants