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

mark_as_unread! does not work for association collections #66

Closed
elvinaspredkelis opened this issue Nov 23, 2020 · 11 comments
Closed

mark_as_unread! does not work for association collections #66

elvinaspredkelis opened this issue Nov 23, 2020 · 11 comments
Assignees
Labels
enhancement New feature or request

Comments

@elvinaspredkelis
Copy link

elvinaspredkelis commented Nov 23, 2020

Hey!

This gem is a blessing and saved so much time already. I ran into this obscure use case where I'd need to mark a bunch of notifications as unread and the gem doesn't seem to support it (yet). 🤷‍♂️

Gem version: 1.2.19

Expected Behavior

Running user.notifications.mark_as_unread! should mark the notification records as not read.

Actual Behavior

Running user.notifications.mark_as_unread! throws an undefined method 'mark_as_unread!' for #<Notification::ActiveRecord_Associations_CollectionProxy:0x0000000000000> error.

Additional Information

  • Running user.notifications.mark_as_read! works as one would expect.
  • Documenting association methods would be a great add as well! 📚
@excid3
Copy link
Owner

excid3 commented Nov 23, 2020

I think this is a bug. The concern should define class_methods do and not module ClassMethods so that method is available.

@excid3
Copy link
Owner

excid3 commented Nov 23, 2020

Actually, seems to work without it. Added a test to confirm it was broken, but it passed.

Must be because the module name matches what Concern looks for.

@elvinaspredkelis
Copy link
Author

Indeed. Perhaps it makes sense to add a corresponding method there to mark multiple notifications unread too?

@excid3
Copy link
Owner

excid3 commented Nov 23, 2020

Like I mentioned, it's already there and has been for a while. https://github.com/excid3/noticed/blob/master/lib/noticed/model.rb#L18-L20

Here's the test I just added to confirm it works: 8f9d070

Your model includes the Noticed::Model module?

@elvinaspredkelis
Copy link
Author

elvinaspredkelis commented Nov 23, 2020

Haha, yep, like I stated in the "Additional Information", mark_as_read! does work as expected.

The opposite function mark_as_UNread! does not exist, however.

@excid3
Copy link
Owner

excid3 commented Nov 23, 2020

Ah! I read through too quickly. 😅

I blame it being Monday. I'll get that fixed shortly.

@elvinaspredkelis
Copy link
Author

Perfect, thank you! 👏

I suppose documenting methods for collections would be sweet as well. Might help out people who wouldn't test their luck out.

@excid3 excid3 added the enhancement New feature or request label Nov 23, 2020
@excid3 excid3 self-assigned this Nov 23, 2020
@excid3 excid3 closed this as completed in cfcad82 Nov 23, 2020
@excid3
Copy link
Owner

excid3 commented Nov 23, 2020

Released as v1.2.20

Also added to the readme with the other methods: https://github.com/excid3/noticed#-database-model

@excid3 excid3 reopened this Nov 23, 2020
@excid3 excid3 closed this as completed Nov 23, 2020
@ykamin-booth
Copy link

Hey, it seems that the mark_as_read! and mark_as_unread! methods no longer work for the association collections as of V2 with the new models. Is this intentional?

@chrisortman
Copy link

chrisortman commented Feb 8, 2024

@ykamin-booth is your association empty when you call it and get the error?

Edit: Scratch that, it seems that the bang variant does not work but mark_as_unread does

@excid3
Copy link
Owner

excid3 commented Feb 8, 2024

Adding that to the UPGRADE guide.

Since they do not raise errors, we removed the ! to be consistent with Rails save and save!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants