Rename subscriptions to notification_subscriptions

[apuntovanini]

Problem or use case

Out target model has already associations with subscriptions, subscriber.rb uses same name to access notifications to which a target is subscribed to. This create an overlap between the associations, and fails.

Expected solution

I'd probably suggest to rename notification subscriptions association to notification_subscriptions, or to allow clients to define the name of the association

Alternatives

None

What do you think @simukappu

[simukappu]

Hi @apuntovanini Thank you for your feedback, and sorry for my late reply.

I think your suggestion make sense. We probably should allow subscriber model to define the name of the association. But I'm not sure how we reproduce this issue and how we confirm if it resolved. Can you create simple test case for this issue?

[apuntovanini]

Hello @simukappu! Any reasoning about this?

[simukappu]

Hi @apuntovanini Sorry for my late reply. Thank you for creating test PR, and I understood you’ve created fork for this patch. I will investigate an impact of renaming the association using existing tests, and will consider if put this on master branch. I would like to continue to open this issue for a while.

[apuntovanini]

Sure thing, in the fork master branch I already migrated tests and methods to the new naming (would be a breaking che change tough). If you want I can draft a PR and see if that works for you

[simukappu]

@apuntovanini If you could draft a PR, it would be helpful. Can you create it?

[apuntovanini]

Sure, did you take a look at https://github.com/uidu-org/activity_notification? I renamed the association there, fixed the tests, but couldn't make it configurable. I'll open a PR so you can take a look!

[simukappu]

Thank you for your PR. I've checked it. I understand the issue and request, but it seems to be difficult to make this subscription association name configurable. The changes of this association name and method name will be breaking changes for current working applications with this gem. I will keep this issue opened, and wait for more feedback about this issue. Thank you!