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.

Sign up for GitHub

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

Jump to bottom

Do not observe the same model more than once. Fixes #35. #36

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

Do not observe the same model more than once. Fixes #35. #36

wants to merge 2 commits into from

Conversation

anthonyjsmith
Copy link

  • This was happening when a subclass of an observed class was created.
  • The subclass was being observed twice.
  • I've changed one of the tests to catch this.

@anthonyjsmith
Do not observe the same model more than once. Fixes rails#35.
ce9e6a6 
* This was happening when a subclass of an observed class was created.
* The subclass was being observed twice.
* I've changed one of the tests to catch this.
@anthonyjsmith
Add subclasses to observed_classes when observer is instantiated. For r…
8080eea 
…ails#35.

* Subclasses were being observed (via add_observer) but not added to
  observed_classes
* Added call to observe() in initialize to update observed_classes
* Moved searching of descendants to initialize, so the comment "Start observing
  the declared classes and their subclasses." will be correct for all subclasses
  of ActiveModel::Observer (not just ActiveRecord::Observer)
* Removed ActiveRecord::Observer#observed_classes, as descendants are now
  searched on initialize (this was the main cause of rails#35)
* No need for models.uniq! in #observe (undoing previous commit)
* Improved tests to better cover uniqueness of observed_classes
* Tests don't work with minitest 5.8.0; require < 5
@DesertSnow
Copy link

Any updates on this issue?

@cfis
Copy link

cfis commented Jan 26, 2017

Agreed, would be nice to have this fixed.

@anthonyjsmith anthonyjsmith mentioned this pull request May 21, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@anthonyjsmith @DesertSnow @cfis