audited icon indicating copy to clipboard operation
audited copied to clipboard

Allow multiple associated audits

Open fatkodima opened this issue 7 years ago • 9 comments

This PR adds the ability to associate a model with multiple parent models. Something like the following:

class Mother < ActiveRecord::Base
  audited
  has_associated_audits
end

class Father < ActiveRecord::Base
  audited
  has_associated_audits
end

class Child < ActiveRecord::Base
  audited associated_with: [:mother, :father]
end

This introduces a bit of backwards incompatibility (like renaming associated to associates, for example) so I'm wondering how we should handle this: a) extensively deprecating (how? - also remains as a question then) or b) do not worry too much and release in next major version or c) other variant?

Related https://github.com/collectiveidea/audited/issues/337. Related https://github.com/collectiveidea/audited/issues/209. Related PR https://github.com/collectiveidea/audited/pull/342 - discussion on why this should be better to implement using separate table.

fatkodima avatar Jan 17 '18 02:01 fatkodima

Hey @fatkodima I just started using this gem and decided to use your PR as a starting point. Generally things are working!

I noticed that if I ask for associates, I get the full list of associated models, even those that didn't change with that audit. For instance, I have a Reservation that I've changed the house_id of from one house to another. The associates field returns the reservation's associated guest record even though it didn't change in addition to the new house record.

I also noticed the associated_audits method stayed the same...should that be like, associates_audits now?

zachfeldman avatar Jan 19 '18 18:01 zachfeldman

Hey, I've been rolling with this PR for a month or two and will get it into production soon. Could we possibly merge it into master?

zachfeldman avatar Mar 02 '18 00:03 zachfeldman

Hi there. Also interested in seeing this merged, aside from the conflicts is it just waiting for 5.0 to be ready?

misaka avatar Apr 10 '19 12:04 misaka

Hi, I am having an issue while setting up this gem with above branch. Application stack:

Rails: 5.2.0
Ruby: 2.5.1

I am getting following error while performing bundle install command after adding gem 'audited', git: 'https://github.com/fatkodima/audited.git', branch: 'multiple-assoc-audits' in Gemfile.

Bundler could not find compatible versions for gem "activerecord":
  In snapshot (Gemfile.lock):
    activerecord (= 5.2.3)

  In Gemfile:
    activerecord-postgis-adapter was resolved to 5.2.2, which depends on
      activerecord (~> 5.1)

    ancestry was resolved to 3.0.7, which depends on
      activerecord (>= 3.2.0)

    apartment-activejob was resolved to 0.0.1, which depends on
      apartment was resolved to 2.2.1, which depends on
        activerecord (< 6.0, >= 3.1.2)

    audited was resolved to 4.6.0, which depends on
      activerecord (< 5.2, >= 4.0)

    blamer was resolved to 4.1.0, which depends on
      activerecord

    paranoia was resolved to 2.4.2, which depends on
      activerecord (< 6.1, >= 4.0)

    rails (~> 5.2.0) was resolved to 5.2.3, which depends on
      activerecord (= 5.2.3)

    activerecord-postgis-adapter was resolved to 5.2.2, which depends on
      rgeo-activerecord (~> 6.0) was resolved to 6.2.0, which depends on
        activerecord (>= 5.0)

Running `bundle update` will rebuild your snapshot from scratch, using
only
the gems in your Gemfile, which may resolve the conflict.

I am trying to implement this gem but without this branch, it's no use for me. So, I like to ask in which branch this pull request will be merged and when are you guys planning to release a version including these changes.

Thank you

sumanawal avatar Jun 25 '19 05:06 sumanawal

I'd be willing to bet that in the vast majority of cases where there's a need for more than one associated, the need is specifically for 2 (supporting HABTM). If so, would it be better just to add 2 more fields to the audits table and allow a model to be associated with up to 2 parents?

markaschneider avatar May 04 '23 20:05 markaschneider

any progress on this?

mikeni avatar Jul 26 '23 18:07 mikeni

I am here. This was created quite a while ago 😅 If people will accept the PR, I will update it.

fatkodima avatar Jul 26 '23 18:07 fatkodima

+1 for getting some motion on this since I could really use it. I'm getting ready to start testing this solution. It's not exactly like what is being suggested here but could work for a through model having two belongs_to associations.

However I believe @fatkodima that the conflicts need to be resolved first before it can be merged.

silent-e avatar Dec 01 '23 21:12 silent-e

+1 this PR would be incredibly useful if accepted

@silent-e curious how you were able to adapt the HABTM solution for the simpler use case here?

gang-qiu avatar Dec 21 '23 14:12 gang-qiu