In email, render dates with an explicit timezone offset
ClosedPublic
Actions

Authored by epriestley on Jul 13 2016, 2:52 PM.

Details

Reviewers

chad

Maniphest Tasks

T10633: Render timezones explicitly in email

Commits

rP26c6f64fd469: In email, render dates with an explicit timezone offset

Summary

Fixes T10633. When generating email about a transaction which adjusts a date, render the offset explicitly (like "UTC-7").

This makes it more clear in cases like this:

mail is being sent to multiple users, and not necessarily using the viewer's settings;
you get some mail while travelling and aren't sure which timezone setting it generated under.

Test Plan

Rendered in text mode, saw UTC offset.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 39174.Jul 13 2016, 2:52 PM

epriestley retitled this revision from to In email, render dates with an explicit timezone offset.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T10633: Render timezones explicitly in email.

epriestley mentioned this in T10633: Render timezones explicitly in email.Jul 13 2016, 2:54 PM

Slightly smaller change.

chad accepted this revision.Jul 13 2016, 3:00 PM

chad edited edge metadata.

This revision is now accepted and ready to land.Jul 13 2016, 3:00 PM

Closed by commit rP26c6f64fd469: In email, render dates with an explicit timezone offset (authored by epriestley, committed by epriestley). · Explain WhyJul 13 2016, 3:32 PM

This revision was automatically updated to reflect the committed changes.

I have a feeling we will soon want to separate "plain-text email" and "html email" as rendering targets, along with "web-html"; So maybe have a isMailMode() instead here, to get the right meaning in this iteration.

Yeah -- we currently never render mail transactions as HTML, but could easily start now that we send HTML mail by default and that seems to be sticking.

This runs into T10363, too -- when publishing into other systems we probably should render this extra context, so maybe the check is really something like isDestinedForARemoteSystem().

avivey mentioned this in D17063: Introduce ModularTransactionType::isRenderingTargetExternal.Dec 16 2016, 12:15 AM

avivey mentioned this in rP8b7e99f68c97: Introduce ModularTransactionType::isRenderingTargetExternal.Dec 16 2016, 12:52 AM