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

Add comments subscriptions to core #7857

Closed
jdalsem opened this issue Feb 4, 2015 · 14 comments
Closed

Add comments subscriptions to core #7857

jdalsem opened this issue Feb 4, 2015 · 14 comments

Comments

@jdalsem
Copy link
Member

jdalsem commented Feb 4, 2015

There are currently 2 plugins around that provide a very basic feature, and are both very well maintained.

https://github.com/AU-Landing-Project/comment_tracker

  • Subscribes commenters to an entity so they receive notifications of new comments.

https://github.com/ColdTrick/content_subscriptions

  • Automaticly follow a discussion or content on your first comment
  • Subscribe to a discussion/content to get notifications about new comments

Both projects almost provide identical features related to getting notifications of new comments on a certain piece of content (and automatically subscribing to those notifications if you comment).

I think this is such a valuable feature that we should add it to core.

Questions:

  • as a separate plugin, as a part of notifications or as part of core (build into comments or subscriptions lib)
  • Elgg 2.0 or 1.11?

cc @Elgg/owners @Elgg/contributors

@juho-jaakkola
Copy link
Member

I've been deliberately holding this back until we overhaul the subscriptions system. I created a new issue for that: #7859

@mrclay
Copy link
Member

mrclay commented Feb 4, 2015

I'm not intimately familiar with either in their current state, though I've hacked on one. My biggest concern was that the "auto-subscribe" feature subscribed users to all their active notification methods. I also auto-subscribe on likes (walking up if it's a comment). Are either/both of these handling these?

@mrclay
Copy link
Member

mrclay commented Feb 4, 2015

See also #7863

@mrclay
Copy link
Member

mrclay commented Feb 4, 2015

I see content_subscriptions does handle all notification handlers as well as the weird way discussions are stored.

An important part of these would be providing a UI to see what you're subscribe to. I think comment_tracker has that.

Let's say we add this as a plugin. What happens if a site already has one of these enabled? I could see adding this in 1.x as long as we don't enable by default. I think the individual plugins should be responsible for migrating to the chosen format.

@mrclay
Copy link
Member

mrclay commented Feb 4, 2015

I wouldn't mind creating an Elgg/content_subscribe repo, adding you all as contributors and we start merging these two (including scripts to migrate from either plugin). /cc @beck24

@beck24
Copy link
Member

beck24 commented Feb 5, 2015

That sounds good to me, to be honest the comment tracker upgrade has been entirely the work of @juho-jaakkola - all credit for that goes to him. I would certainly like to help see the functionality become part of core though.

@mrclay
Copy link
Member

mrclay commented Feb 10, 2015

Upon closer inspection I see content_subscriptions directly uses elgg_add_subscription() to subscribe the user to all available handlers, whereas comment_tracker instead queries its own relationship to provide Elgg with the subscribed users. What are the pros/cons of each method?

@juho-jaakkola
Copy link
Member

In comment_tracker the idea is that new notification methods are enabled by default. Only the ones explicitly blocked are not being used.

@mrclay
Copy link
Member

mrclay commented Feb 10, 2015

And notification methods change, new user-item relationships don't need to be added/removed.

@jdalsem
Copy link
Member Author

jdalsem commented Feb 10, 2015

in both situations you will receive notifications on all services, but comment_tracker has the benefit (or the downside) of future services, that are introduced after you subscribed, automatically being included as a notification service.

Downside is you need to provide your own mechanisme of providing the subscribers as where content_subscriptions can rely on default Elgg to handle those...

@jdalsem
Copy link
Member Author

jdalsem commented Feb 17, 2015

Assigning this to 2.0 as it is a killer feature

@jdalsem jdalsem added this to the Elgg 2.0.x milestone Feb 17, 2015
@ewinslow ewinslow removed this from the Elgg 2.0.x milestone May 30, 2015
@ewinslow
Copy link
Contributor

Killer feature perhaps but not a blocker for 2.0 since it shouldn't be breaking, amirite?

@jdalsem
Copy link
Member Author

jdalsem commented Jun 1, 2015

urrite

@jdalsem jdalsem added this to the Elgg 3.1 milestone Apr 24, 2019
@jdalsem jdalsem modified the milestones: Elgg 3.1, Elgg 3.2 Jul 12, 2019
@jdalsem jdalsem modified the milestones: Elgg 3.2, Elgg 3.3 Oct 22, 2019
@jdalsem jdalsem modified the milestones: Elgg 3.3, Elgg 4.0.x Jan 22, 2020
@jdalsem
Copy link
Member Author

jdalsem commented Apr 9, 2021

Added in #13408

@jdalsem jdalsem closed this as completed Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants