oaeproject / Hilary

Open Academic Environment (OAE) Back-End
http://www.oaeproject.org
Educational Community License v2.0
106 stars 96 forks source link

recent activity mail notifications time stamps #1270

Closed dehourican closed 8 years ago

dehourican commented 8 years ago

The email notification I received for the "new activity is waiting for you" has a different date than today. I received this notification a few minutes ago (12/10/15 @4:38 pm EDT) but the date states "2015-12-11 02:23:58 image Yet within the application the time states it was 17 minutes ago. image

sallakarppinen commented 8 years ago

To me this still looks like a timezone problem, although how that could've happened for two people in the same tenancy (and thus on the same timezone) I am not certain. Emails get the right timezone from the tenancy configuration separately from the activity view so currently my assumption is that for some reason the timezone received in the email was wrong (apparently one used in Pakistan and parts of Russia).

nicolaasmatthijs commented 8 years ago

@mrvisser : Would you be able to provide a second pair of eyes on this?

sallakarppinen commented 8 years ago

I looked at this a bit further and now I think it could actually be tz.js method getClosestSupportedTimezone, in particular this bit here (from line 84 onwards):

// Offsets are in minutes, 1440 is 24 hours
  var closest = {'delta': 1440, 'zone': null};
  _.each(railsOffsets, function(zone, offset) {
      var delta = Math.abs(offset - userOffset);
      if (delta < closest.delta) {
          closest = {
              'delta': delta,
              'zone': zone
          };
      }
});

If for some reason we failed to get the timezone previously (when comparing to railsOffset), we fall into this block which is using Math.abs. The offsets (in hours) for EDT and PKT are -5 and +5, respectively... Math.abs ignores the number's sign, so these would be considered equal. We only set one value as the zone, so whichever came up first would be the one we end up using. @nicolaasmatthijs , @mrvisser, does this sound plausible to you?

mrvisser commented 8 years ago

@sallakarppinen can you clarify where the relevant entry point to tz.js is in this case, from the OAE source?

sallakarppinen commented 8 years ago

The oae-ui API requires it and is used to render email templates, so my guess would be there. That's where it gets a bit muddy for me though, since I'm not sure which email templates we are actually using - I thought it'd be the ones in oae-activity/emailTemplates, but they specify a different date format.

nicolaasmatthijs commented 8 years ago

@mrvisser : Can you provide some input on Salla's question?

mrvisser commented 8 years ago

Sorry, @sallakarppinen . The ones in oae-activity/emailTemplates (mail.html.jst) should be the one it's using. What format discrepancy are you unsure about?

https://github.com/oaeproject/Hilary/blob/master/node_modules/oae-activity/emailTemplates/mail.html.jst#L479

sallakarppinen commented 8 years ago

The exact line you highlighted, actually... It specifies the date format as 'f'. The documentation on oae-ui api has this link for Globalize date formats, and on that page 'f' is defined as 'Long Date, Short Time', but the one we get in emails looks more like ISO 8601.

mrvisser commented 8 years ago

@sallakarppinen right, I was looking in timezone-js' formatting and couldn't find a reference for 'f'.

The offsets (in hours) for EDT and PKT are -5 and +5, respectively... Math.abs ignores the number's sign, so these would be considered equal. We only set one value as the zone, so whichever came up first would be the one we end up using. @nicolaasmatthijs , @mrvisser, does this sound plausible to you?

I think this block is trying to find the timezone with the closest offset to the user's offset. For example, if the user's offset was -5, when we compare with PKT, we get the delta of abs(5 - (-5)) = 10. Then when we arrive at EST, we have abs(-5 - (-5)) = 0, so it should properly choose EST (or the first timezone that had an equivalent).

On the other hand, if the reason that we're finding an offset that is not found in the railsOffsets is because of some bad data, that could cause some obscure timezone and offset choices, so maybe it's worth analyzing that data to see if there are any results that could break this area of the code.

FWIW, my best advice for trying to figure out if an email template is being used is to update the template, restart the server, then trigger an email and see what the content is. When email is in debug mode, they are printed to the console.

sallakarppinen commented 8 years ago

@mrvisser, thanks for the explanation, that makes sense. I tried updating the template and triggering an email, and the result is that it seems like mail.html.js is the template that's being used, but the date format is just completely different from the one in the docs... Will continue to look in that direction to figure out why and also try to dig up some data on this, although if it hasn't been reported since December I don't know how much luck I'll have.

EDIT: I've had a look at the logs from Dec and nothing jumped out at me, but then again eyeballing logs in an unfamiliar format is not the most reliable way to get information out from them. Will try again when filter-bunyan has the right permissions.

sallakarppinen commented 8 years ago

I'm going to close this as 'Cannot reproduce' since it seems like it hasn't happened again, there's nothing in the logs from December, and no obvious issues with the code.