publiclab / plots2

a collaborative knowledge-exchange platform in Rails; we welcome first-time contributors! :balloon:
https://publiclab.org
GNU General Public License v3.0
961 stars 1.83k forks source link

wiki page edit email notifications (max 1 per hour) #396

Open jywarren opened 8 years ago

jywarren commented 8 years ago

The difficulty here is managing too many notifications for subsequent edits.

Note: we could start by only doing notifications if there's been no edits for over 1 hour.

If there's a second edit, we could also start a timed job on a wiki edit which executes 1 hour later, and collects all the subsequent edits.


Email notifications, similar to comment notifications, for when someone edits a wiki page (like this page: https://publiclab.org/wiki/gsoc-ideas). Subscription would happen under the "Follow" menu:

screenshot 2016-03-25 at 11 18 39 am

Code for that menu is here: https://github.com/publiclab/plots2/blob/master/app/views/like/_like.html.erb

A good template for this is one of these:

https://github.com/publiclab/plots2/blob/master/app/mailers/subscription_mailer.rb https://github.com/publiclab/plots2/blob/master/app/mailers/comment_mailer.rb

This'd be a little annoying on pages that get edited often, so we should check if an edit has happened in the past (maybe) 10 minutes, and send an email that "batches" all edits since the last time it could've been triggered. So instead of getting 10 emails that Alice has edited a page in 15 minutes, you'd get 2 at the most.

The email would include (potentially) the changes made (see the wiki diff display on the new dashboard: https://publiclab.org/dashboard2 (login required)): https://github.com/publiclab/plots2/blob/master/app/views/dashboard/_node_wiki.html.erb#L26-L34 and diffing code, in javascript, here: https://github.com/publiclab/plots2/blob/master/app/assets/javascripts/dashboard.js

Diffs look like this:

screenshot 2016-03-25 at 11 19 49 am

...and of course the username of the editor.

These could have the same "Look like spam?" links the research note notifications have: https://github.com/publiclab/plots2/blob/master/app/views/subscription_mailer/notify_node_creation.html.erb#L23

Recommend tackling basic notifications first, with no diffs, and opening a new issue to include diffs inline later.

This would be the preferred way to keep an eye out for spam on a page, where locking (#397) is a last resort.

StlMaris123 commented 7 years ago

Hello @jywarren, could this issue be broken down a little bit. looks a little bulky to me.

jywarren commented 7 years ago

I agree. Do you want to try to make a checklist in the comments here, and propose a way to break it up? I think perhaps just getting an experimental ActiveJob (ActionJob?) task running and passing all tests would be a good first step. Then one which sends an email regularly (although this could be annoying, but we could create a Gmail rule to filter it out) -- that way we can confirm that emails are being sent at all on a schedule.

Then we could move on to more advanced integration.

On Thu, May 11, 2017 at 2:53 PM, StellaMaris Njage <notifications@github.com

wrote:

Hello @jywarren https://github.com/jywarren, could this issue be broken down a little bit. looks a little bulky to me.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/396#issuecomment-300883779, or mute the thread https://github.com/notifications/unsubscribe-auth/AABfJ_WxQXZW0UiefcCZEY7O_IgiRqFKks5r41kmgaJpZM4Hr-xr .

StlMaris123 commented 7 years ago

Yes. I'll send the breakdown tomorrow morning.

On 11 May 2017 22:08, "Jeffrey Warren" notifications@github.com wrote:

I agree. Do you want to try to make a checklist in the comments here, and propose a way to break it up? I think perhaps just getting an experimental ActiveJob (ActionJob?) task running and passing all tests would be a good first step. Then one which sends an email regularly (although this could be annoying, but we could create a Gmail rule to filter it out) -- that way we can confirm that emails are being sent at all on a schedule.

Then we could move on to more advanced integration.

On Thu, May 11, 2017 at 2:53 PM, StellaMaris Njage < notifications@github.com

wrote:

Hello @jywarren https://github.com/jywarren, could this issue be broken down a little bit. looks a little bulky to me.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/396#issuecomment-300883779, or mute the thread https://github.com/notifications/unsubscribe- auth/AABfJ_WxQXZW0UiefcCZEY7O_IgiRqFKks5r41kmgaJpZM4Hr-xr .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/396#issuecomment-300887687, or mute the thread https://github.com/notifications/unsubscribe-auth/AMdR2VH8IhWWHgO9sqTUuLyf_VsqSO0yks5r41yWgaJpZM4Hr-xr .

StlMaris123 commented 7 years ago

We could divide them into two tasks:

  1. No edits for over an hour

    • Acceptance tests
    • Send only one email if no edits have occurred within an hour.
  2. Several subsequent edits

    • Acceptance tests
    • Send only one email for all subsequent edits

However, I just thought we could have one email once a wiki page is created. We could have two links:

One can keep editing the wiki page until finally contented the click submit final. That is is when guys get notified about the wiki updates. That way we don't have to worry about the number of edits performed. What do you think?

jywarren commented 7 years ago

Cool - i like the direction you're going.

Now that I think about it, the advantage to the number 1 is that we don't actually need ActionJob, because it'd be triggered by the edit itself.

I like the 2-stage edit sequence, but I think it'll be a bit more complex to implement, with an additional state needed for the post. Let's begin with your first task and think: where would this code go? The update action on wiki controller?

And could we break it up, like, could we have a node method like node.minutes_since_updated and see if it's larger than 60? Could we test that method?

Where would we put a test to ensure an email was sent upon the update method running?

Of course, I can help you find the answers to these, but this is the kind of "breaking it down into parts" i'd like to do. Can you try that in a bit more detail for the first item?

Thanks, Stella!

ujithaperera commented 7 years ago

Hi @StlMaris123,

BTW I am Ujitha Perera. I also agree with the @jywarren . I feel that first we should make sure that active Job is up and running. Since this is a new module to the code base, have to create new active Jobs without affecting to the current functionalities.

And since we are using rails 3.2 active jobs don't support by default. hence we have to add active jobs as a gem to the codebase and then the queuing process. Since our DB size is not small, definitely we may have to go with parallel processing. Perhaps we may able to run email batches within the rails app. But it would be a huge load for the rails app and other functionalities might not work within that email processing time. Hope @jywarren will explain the actual requirement.

@StlMaris123 are you familiar with active jobs and background jobs ? if not, installing Redis and scheduling background jobs would be a great hands on experience for you to begin with.

StlMaris123 commented 7 years ago

Hello Ujitha, I am not familiar but i will have a look at them.

On 13 May 2017 21:05, "Ujitha Perera" notifications@github.com wrote:

Hi @StlMaris123 https://github.com/stlmaris123,

BTW I am Ujitha Perera. I also agree with the @jywarren https://github.com/jywarren . I feel that first we should make sure that active Job is up and running. Since this is a new module to the code base, have to create new active Jobs without affecting to the current functionalities.

And since we are using rails 3.2 active jobs don't support by default. hence we have to add active jobs as a gem to the codebase and then the queuing process. Since our DB size is not small definitely we may have to go with parallel process. Perhaps we may able to run email batches within the rails app. But it would be a huge load for the rails app and other function might not work within that email processing time. Hope @jywarren https://github.com/jywarren will explain the actual requirement.

@StlMaris123 https://github.com/stlmaris123 are you familiar with active jobs and background jobs ? if not, installing Redis and scheduling background jobs would be a great hands on experience for you to begin with.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/396#issuecomment-301264524, or mute the thread https://github.com/notifications/unsubscribe-auth/AMdR2VGSpy3gphcHmjjpnx9tjd0Qr3Feks5r5fD8gaJpZM4Hr-xr .

StlMaris123 commented 7 years ago

@jywarren Please help me in breaking down the tasks into more distinct steps.

jywarren commented 7 years ago

Hi, @StlMaris123 - i'll reply to this soon, sorry i've been tied up!

jywarren commented 7 years ago

Hi, @StlMaris123 -- sorry for the delay but I've posted a proposed 2-phase plan to attempt the above as well as the next step. Check out #1421 and tell me what you think!

SidharthBansal commented 4 years ago

If @jywarren you think this is a minor project OR can be broken into 2-3 minor projects then we can include this as hall of fame task in GCI