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

Audited gem is partially incompatible with Rails 7.2.x and the Rails team insist you fix it #725

Open
pond opened this issue Aug 26, 2024 · 8 comments · May be fixed by #726
Open

Audited gem is partially incompatible with Rails 7.2.x and the Rails team insist you fix it #725

pond opened this issue Aug 26, 2024 · 8 comments · May be fixed by #726

Comments

@pond
Copy link

pond commented Aug 26, 2024

Please see rails/rails#52715 - this looks more to me like a Rails bug than an Audited gem bug, which is why I've filed the report over there. There's a small chance, though, that it's some issue with the inner workings of Audited.

Issue posted here really as an FYI, but of course, feel free to close if you're confident that it's a Rails problem.

@pond
Copy link
Author

pond commented Aug 27, 2024

BUMP To my absolute horror the Rails maintainers have closed the issue saying it must be fixed in the gem, even though I can't see how you could possibly do that.

This is an urgent issue. Audited is incompatible with Rails 7.2 at this time, for certain use cases that worked fine for many years prior.

@pond pond changed the title Audited gem *may* be partially incompatible with Rails 7.2.x. Audited gem is partially incompatible with Rails 7.2.x and the Rails team insist you fix it Aug 27, 2024
@pond
Copy link
Author

pond commented Aug 27, 2024

(Edited) Rails maintainers have clarified their position and point to documentation that indicates Audited may have been relying on undefined behaviour previously.

https://api.rubyonrails.org/classes/ActiveRecord/Inheritance/ClassMethods.html

If you are using inheritance with Active Record and don’t want a class to be considered as part of the STI hierarchy, you must set this to true.

...they recommend defining inherited in the Audit::Audited class and setting self to abstract therein.

If you're happy with that approach, I am happy to do that work and send a PR for the change.

@byroot
Copy link

byroot commented Aug 27, 2024

and the Rails team insist you fix it

FYI, we don't insist anything and this sort of phrasing isn't a good way to get project maintainers help.

@pond
Copy link
Author

pond commented Aug 27, 2024

@byroot I don't care how my words land, I just want this fixed, and I'm happy to do the work to do so, whichever project that lands in. My numerous PRs to fix open source projects prior speak for themselves. I contribute to this community routinely.

I do not have time for politics.

@byroot
Copy link

byroot commented Aug 27, 2024

I don't care how my words land,

Well you should, because I was very close to just ignore you on the other issue because of your tone.

And I could fix your issue in 2 minutes or even give you a one line snippet you could include in an initializer that would fix your issue. But since you don't care about how your words land, I guess I don't care if your problem is fixed.

@pond
Copy link
Author

pond commented Aug 27, 2024

Well you should, because I was very close to just ignore you on the other issue because of your tone.

The other issue being rails/rails#52715, which you closed as "won't fix", saying that the gem must do it, but that's not "insisting"? What word would you use for that, then?

because of your tone.

Exactly which part of which comment, please? A link would help. Would be this be the part where I produced a tight replication test case, explained everything in detail in a very well formatted issue precisely to the Rails guidelines required template? Or was it in rails/rails#52715 (comment), where I hoped for a resolution and conversed genially? Or would it be after you closed the issue saying you were not going to do anything on the Rails side?

And I could fix your issue in 2 minutes

If you mean a work-around, it's clearly trivial to add:

  if Rails.gem_version < Gem::Version.new('7.2.2')
    ::Audited::Audit.class_eval do
      self.table_name = 'audit_trails'
    end
  end

...in config/initializers/audited.rb, which is what I did as soon as the issue came to light. But this does not help other community members, which is what we're here for right now.

I do not have time for politics.

@pond
Copy link
Author

pond commented Aug 27, 2024

Oh and incidentally - and apologies, I should have said this much sooner - could we perhaps take this off the Audited gem's issue?! I'm sure the maintainers don't want to plough through this ridiculous bickering.

You can contact me on ahodgkin@rowing.org.uk per public profile, or I can fork this gem and you can continue in an issue on the fork should you want the discourse to be public.

@pond
Copy link
Author

pond commented Aug 28, 2024

After trying many approaches, I decided there was no good way to automate this and it essentially cannot be fixed. See #726. This describes three coherent solution options (out of many intermediate hacks not covered!) all of which have problems - a maintainer might in fact decide that one of those is acceptable, but otherwise, a warning in README.md is all we have left.

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 a pull request may close this issue.

4 participants
@byroot @pond and others