Skip to content

Conversation

@lorint
Copy link

@lorint lorint commented Jun 5, 2018

Transitioning away from using base_class in order to support versioned sub-classes that utilise STI. The other half of this fix is in PR #1108 in PaperTrail.

@westonganger
Copy link
Owner

@Iorint now that the PT core fix got merged. Could you please revisit this PR and update as required. Thanks

@lorint
Copy link
Author

lorint commented Jul 31, 2018

Could you please revisit this PR and update as required. Thanks

Certainly. Will have some spare cycles later today to encapsulate the approach we've taken for has_many and has_many_through. Later in the week can do the belongs_to, and then finalise the other two next week.

Stay tuned :)

@jaredbeck
Copy link
Collaborator

paper-trail-gem/paper_trail#1108 didn't work out. Instead, we'll add an optional column item_subtype to our versions table(s), per paper-trail-gem/paper_trail#1143

The addition of this column will be mentioned under the "Fixed" section of the PT 10.0.0 changelog, and (for lack of a better place) users will be directed to this PR for further information about PT-AT's adoption of this new column.

@lorint, Sean, and I believe that PT-AT can adopt this column and fix paper-trail-gem/paper_trail#594 with changes like the following:

# paper_trail_association_tracking/reifiers/has_one.rb
# def load_versions(assoc, model, transaction_id, version_at, base_class_name)
            where("#{version_table_name}.item_type = ?", base_class_name).
+           where("#{version_table_name}.item_subtype = ?", assoc.class_name).
            where("created_at >= ? OR transaction_id = ?", version_at, transaction_id).

@westonganger
Copy link
Owner

Hey @lorint just checking in. Any chance to revisit this PR and get it up to date?

@westonganger westonganger force-pushed the master branch 3 times, most recently from bdf0933 to 5326acb Compare April 19, 2020 15:48
@westonganger westonganger force-pushed the master branch 2 times, most recently from a2eec6c to d6668e2 Compare October 22, 2020 04:40
@westonganger
Copy link
Owner

Closing due to inactivity.

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