Plots2: Email digest follow-up changes list

Created on 30 Jan 2019  Â·  55Comments  Â·  Source: publiclab/plots2

Our daily email digest is now working! Now we have a few follow ups to do.

  • [x] Images seem broken
  • [x] dates on posts don't seem right (https://github.com/publiclab/plots2/pull/4851)
  • [x] daily digest is labeled weekly (https://github.com/publiclab/plots2/issues/4822) (solved in https://github.com/publiclab/plots2/pull/4955)
  • [x] some page width oddities on a smartphone
  • [x] images not loading https://github.com/publiclab/plots2/issues/5230
  • [x] item content is in Markdown and needs to be parsed to HTML
  • [x] looks like maybe updated wiki pages are included? For now let's just show notes (https://github.com/publiclab/plots2/pull/5568)
  • [x] are we sure the same posts aren't being sent out multiple times? #5907
  • [x] adding today's date to the heading title of the digest email https://github.com/publiclab/plots2/pull/5331/

Overall this is very exciting!

HTML break-me-up help wanted planning

Most helpful comment

Let's do responsive! Shouldn't we be able to use a layout similar to, or
even based on the same code as, the main PublicLab.org codebase? Thanks
Isha!

On Mon, Feb 4, 2019 at 2:05 PM Isha Gupta notifications@github.com wrote:

I think for the width related problems, we can do 2 things:

  1. Make the existing code responsive
  2. Make a new email format using tables and make that responsive

What's your take on this @ViditChitkara https://github.com/ViditChitkara
@jywarren https://github.com/jywarren ?

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

All 55 comments

@viditchitkara check it out!

screenshot_20190129-221111

screenshot_20190129-221118

Also linking with #2378 and with the browser based view of the digest at https://publiclab.org/subscriptions/digest

@jywarren I wanna work on this issue

That would be great! What part do you want to work on?

On Fri, Feb 1, 2019, 1:28 AM Arun Goel <[email protected] wrote:

@jywarren https://github.com/jywarren I wanna work on this issue

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

Wow, feels really good to see this working @jywarren. @arungoel123456 in my opinion you may start with dates and label of mail. This is what I feel would be a good starting point. It's totally up to you if you want to start with another. Ping me if you need any help with this.

@ViditChitkara @jywarren could I work on some other parts of this issue?

Hi @IshaGupta18, feel free to proceed. But, before working on a part please drop a message here to let others know, to avoid any conflict.

Also, @arungoel123456 and @IshaGupta18 you can discuss and divide parts here. Thanks!

Yes absoluely @gauravano. Thanks a lot! I actually found the places where the 'daily digest is labeled weekly' and 'dates on posts don't seem right' are mentioned. Maybe I can start with them, if @arungoel123456 hasn't already done that? Please let me know in either case.

I think that 'are we sure the same posts aren't being sent out multiple times?' would be a relatively harder one, because we'll have to keep a record of what content has already been sent in the previous day's mail and a mail should be sent on that day if and only is the content is slightly different from the one sent the day before. I have received I think 2-3 redundant mails for a tag.

@IshaGupta18 I would like to work on the date and label issue .... Sorry for the late reply

@ViditChitkara I will sure contact u if I need some help . Thanks for advising

Awesome folks, thanks! Yes Isha i agree on the redundancy issue - what we
can do is just be sure we're only reporting things that occurred in the
past day or week.

Also, things like wiki page edits and such are perhaps lower priority; we
could list them at the bottom in a section called "Active wiki pages"
without showing the full entry we do for notes; does that make sense?

On Mon, Feb 4, 2019 at 12:59 AM Arun Goel notifications@github.com wrote:

@ViditChitkara https://github.com/ViditChitkara I will sure contact u
if I need some help . Thanks for advising

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

@arungoel123456 kindly proceed then! @jywarren I think that could be great idea!

I think for the width related problems, we can do 2 things:

1) Make the existing code responsive
2) Make a new email format using tables and make that responsive

What's your take on this @ViditChitkara @jywarren ?

Let's do responsive! Shouldn't we be able to use a layout similar to, or
even based on the same code as, the main PublicLab.org codebase? Thanks
Isha!

On Mon, Feb 4, 2019 at 2:05 PM Isha Gupta notifications@github.com wrote:

I think for the width related problems, we can do 2 things:

  1. Make the existing code responsive
  2. Make a new email format using tables and make that responsive

What's your take on this @ViditChitkara https://github.com/ViditChitkara
@jywarren https://github.com/jywarren ?

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

Hey @jywarren how should I test my changes that I will make to make the email responsive?

@jywarren @ViditChitkara Images are working fine to some extent, like if you change your gmail
settings->images-> "always show", some of them show up, but not all, like the PL logo and default profile picture.

Hi, Isha - there's a "test digest" button on the profile, i don't know if
it will pop up an email for you if you click it, but give it a try?

The other option is that we had made this page as an "online" version in
order to test the digest display: https://publiclab.org/subscriptions/digest

If you can track down the template there, perhaps we could have that page
display the same HTML as the email will, so it can be used to test out the
digest display online as well?

On Wed, Feb 6, 2019 at 4:40 AM Isha Gupta notifications@github.com wrote:

@jywarren https://github.com/jywarren @ViditChitkara
https://github.com/ViditChitkara Images are working fine to some
extent, like if you change your gmail
settings->images-> "always show", some of them show up, but not all, like
the PL logo and default profile picture.

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

Hey sorry to bother you, I actually found a template in the /test/mailers/preview folder and I am trying to work on that! Thanks a lot for your help though!

I also wanted to let you know @jywarren that I will be having my mid semester exams during early mid-feb so I may not be as active, but I'll try to stay in touch as much as I can!

ok, good luck with the exams, Isha!

On Wed, Feb 6, 2019 at 10:50 AM Isha Gupta notifications@github.com wrote:

I also wanted to let you know @jywarren https://github.com/jywarren
that I will be having my mid semester exams during early mid-feb so I may
not be as active, but I'll try to stay in touch as much as I can!

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

Okay so I think I may have found a way to get rid of the page width problems, however, I am not too sure that if it will be considered as a very good way because it will require some refactoring of the existing code. Should I make a PR for it @jywarren ?

sure!

On Thu, Feb 7, 2019 at 3:21 PM Isha Gupta notifications@github.com wrote:

Okay so I think I may have found a way to get rid of the page width
problems, however, I am not too sure that if it will be considered as a
very good way because it will require some refactoring of the existing
code. Should I make a PR for it @jywarren https://github.com/jywarren ?

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

4780 is my PR in an attempt to fix width issues. @jywarren kindly have a look at it and let me know if there's anything else that can be done! Thanks a lot!

what all issues are solved ? Can u please tell

@jywarren what is the status of this issue ? Can u please tell

Width related problems have been solved, author's profile picture problem
has been solved and some work is being done on weekly or daily title, I and
@jywarren are discussing that. You can make a PR for the date issue if you
want to. Thanks a lot!

On Mon, Feb 11, 2019, 8:09 PM Arun Goel <[email protected] wrote:

@jywarren https://github.com/jywarren what is the status of this issue
? Can u please tell

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

OK , let me check the date issue. Thanks

Okay @jywarren , so I am seeing, on using byebug, that the main_image_id field is nil even for a post that contains an image, for example (this is just a test post I made)

#<Node nid: 49, vid: 49, type: "note", language: "", title: "try image", uid: 2, status: 1, created: 1549696643, changed: 1549696643, comment: 2, promote: 0, moderate: 0, sticky: 0, tnid: 0, translate: 0, cached_likes: 0, comments_count: 0, drupal_node_revisions_count: 1, path: "/notes/moderator/02-09-2019/try-image", main_image_id: nil, slug: "try-image", legacy_views: 0, views: 1>

So I think this is why the images of the post are not appearing. Any ideas?

4792 is my pr for why dates dont appear right in emails

4792 is my pr for why dates dont appear right in emails

Moved to #4793

@arungoel123456 , for your query in #4792 , https://github.com/publiclab/plots2/issues/4732#issuecomment-462351446 read this.

Also, me and @jywarren discussed about * are we sure the same posts aren't being sent out multiple times? and I believe about * looks like maybe updated wiki pages are included? If so we should present them a bit differently as well and I have asked him about https://github.com/publiclab/plots2/issues/4732#issuecomment-462359915 as well. So I think we should wait for his reply on the progress of things in the issue.

Re main image - try using this code here, which looks for a main image, and if it doesn't find one, actually tries to scrape out the first image in the post:

https://github.com/publiclab/plots2/blob/fe50428f4c56f12fcea92ae76343346d0421da70/app/views/notes/_notes.html.erb#L7-L11

Thanks!

Just responded in https://github.com/publiclab/plots2/pull/4780#discussion_r255303340, but this is looking great, and we can do the refactoring of weekly vs daily in a new issue!

4822 for the weekly vs daily issue!

4824 For dates to appear right In E-MAIL

@jywarren #4875 is the PR for fixing broken images! Let me know what else can be done! Thanks a ton!

Hi, thanks @IshaGupta18 - merged that one! Reopening for the remainder of this checklist! Thanks!

I added some notes and tips to https://github.com/publiclab/plots2/issues/4822 -- great!

And @arungoel123456 solved the date one in https://github.com/publiclab/plots2/pull/4851 -- great work!

Daily/weekly fix is live! Checked it off.

image

I noticed not all images are loading so I added a new item to the checklist above.

The image URLs were:

profile pic: http:///system/profile/photos/1/thumb/bio-jeff-warren.jpg

post picture: http:///system/images/photos/000/030/324/medium/IMG_20190320_162254_076.jpg

It looks like we're skipping the "publiclab.org" part in the template. We should be able to add in Rails.root in /app/views/mailers/...

Getting there!

Making new ones for Markdown and for the date in the title!

https://github.com/publiclab/plots2/issues/5340 and https://github.com/publiclab/plots2/issues/5341 !

Added: looks like maybe updated wiki pages are included? For now let's just show notes (#5568)

Okay, so I think there are a couple of more bugs with the email digest.

image

(Like the date problem and the links at the bottom)

I am gonna work on them as well as the ones you have added here @jywarren, just some refining is needed I think, and a little more integration with the parts we have broken up this issue into.

@jywarren #5638 is my PR for the broken date issue as shown above, kindly review and let me know if its good to go. Thanks!

So I have seen that all the links (after being updated to full path) are breaking somewhere and hence, both authors' and notes' images are being broken. Could someone tell me why we need to keep the full path because by keeping the original paths, the images seem to work just fine? Thanks!

Whoa, that's odd because i'm seeing digests like this:

image

Which is not perfect but better than what you seem to be receiving?

Yeah, I think so, but in my mail box, the links ( the "click here" links at the bottom) and the images seem to be broken:
image

What do you think should be done?

And also, I think we can open up an FTO for thedate in the heading checkpoint, because the PR #5331 is for the path links part of this issue. Please correct me if I am wrong?

hmm, this is so weird! Are you sure that email is from our production
server? Thanks Isha!!!

On Wed, May 1, 2019 at 10:28 AM Isha Gupta notifications@github.com wrote:

And also, I think we can open up an FTO for the date in the heading
checkpoint, because the PR #5331
https://github.com/publiclab/plots2/pull/5331 is for the path links
part of this issue. Please correct me if I am wrong?

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/4732#issuecomment-488297328,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAF6J6TNGSIZVFHSSB6QTDPTGSH5ANCNFSM4GTEYFJQ
.

This comes from the production right?
image

This is really weird.

Almost there now! I think we need to adjust image aspect ratio:

image

Also, i confirmed, posts are getting included day after day, so I'm not sure why but maybe it's sending for any revisions, or something?

OK, last thing -- this section fetches revisions by timestamp, which is why old posts are getting re-added to the digest when they are updated (i.e. their revision gets a new timestamp):

https://github.com/publiclab/plots2/blob/master/app/models/user.rb#L245-L260

I'm going to make a parameter to include revisions and default it to false.

OK! This should be completely finished when #5907 merges. Thanks all!!!!!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

noi5e picture noi5e  Â·  3Comments

shapironick picture shapironick  Â·  3Comments

first-timers[bot] picture first-timers[bot]  Â·  3Comments

grvsachdeva picture grvsachdeva  Â·  3Comments

ebarry picture ebarry  Â·  3Comments