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

[WIP] When a record is deleted it should use unscoped association calls #4

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

Conversation

vkmita
Copy link

@vkmita vkmita commented May 9, 2014

@libo @dasch @bquorning @pdeuter

I think this addition is useful, for example, you can retrieve a deleted category, and gracefully get the default_translation without having to do something like this:

>> DC::Content.with_deleted { DC::Translation.with_deleted { category.default_translation.title } }
  DC::Content Load (0.2ms)  SELECT `dc_contents`.* FROM `dc_contents` WHERE `dc_contents`.`source_id` = 10017 AND `dc_contents`.`source_type` = 'Category' LIMIT 1
  DC::Translation Load (0.3ms)  SELECT `dc_translations`.* FROM `dc_translations` WHERE `dc_translations`.`id` = 10112 LIMIT 1
=> "Using Help Center"

Instead we can simply do:

>> category.default_translation.title
  DC::Content Load (0.2ms)  SELECT `dc_contents`.* FROM `dc_contents` WHERE `dc_contents`.`source_id` = 10017 AND `dc_contents`.`source_type` = 'Category' LIMIT 1
  DC::Translation Load (0.3ms)  SELECT `dc_translations`.* FROM `dc_translations` WHERE `dc_translations`.`id` = 10112 LIMIT 1
=> "Using Help Center"

The thing I'm having trouble with is the timing of redefining the association methods. If I do a class_eval at the time the module is included it doesn't work because it seems like Rails is redefining the has_many associations after I initially redefine them. The only solution I came up with for that was overriding the methods in an after_initialize callback, and simply marking which classes were overridden so we don't do it twice.

What do you guys think?

@vkmita
Copy link
Author

vkmita commented May 9, 2014

Hmm I just saw #3 I think my approach still works with Rails4??

next unless association.klass.include?(Restorable)
associate_method(association)
end
Restorable.associated_classes << self.class
Copy link
Contributor

Choose a reason for hiding this comment

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

This is updating global state :-/ I'm not quite sure what this is supposed to do – do you want to track which AR classes include Restorable? If so, there are simpler ways of doing it.

Copy link
Author

Choose a reason for hiding this comment

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

I had trouble tapping into the a time after AR defined the association methods. My solution was to do it in an after_initialize callback because I knew AR wouldn't redefine them again at that point. And because I don't want to redefine the association methods after each initialization, I kept track of the classes where I redefined the.

Does that make sense?

@dasch
Copy link
Contributor

dasch commented Sep 23, 2014

@vkmita can you rebase this on master?

@pdeuter
Copy link

pdeuter commented Sep 23, 2014

hi @dasch ... we cannot rely on @vkmita to work further on this PR. Victor is working on another team now.
This PR really needs your expertise. Can you take over this PR?

@vkmita vkmita force-pushed the vkmita/associate-with-deleted branch from edef496 to 5f4800f Compare September 23, 2014 18:06
@mriddle mriddle added the teams/ruby-core Ensures ruby-core team are notified of activity over in #ruby-core-team-github label Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
teams/ruby-core Ensures ruby-core team are notified of activity over in #ruby-core-team-github
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants