datatracker icon indicating copy to clipboard operation
datatracker copied to clipboard

fix: display document expiration notification in UTC

Open meadmaker opened this issue 2 years ago • 12 comments

The document expiration mail displays the date of the upcoming document expiration, but different timezones makes that date ambituous. Instead, clearly show that as a timestamp in UTC.

Fixes https://github.com/ietf-tools/datatracker/issues/1825

meadmaker avatar Nov 04 '23 15:11 meadmaker

@meadmaker - please start using this for your commit messages: https://www.conventionalcommits.org/en/v1.0.0/

rjsparks avatar Nov 05 '23 09:11 rjsparks

Codecov Report

Merging #6600 (5dfbaab) into main (d246879) will increase coverage by 0.01%. Report is 19 commits behind head on main. The diff coverage is 96.00%.

:exclamation: Current head 5dfbaab differs from pull request most recent head 602515c. Consider uploading reports for the commit 602515c to get more accurate results

@@            Coverage Diff             @@
##             main    #6600      +/-   ##
==========================================
+ Coverage   88.85%   88.86%   +0.01%     
==========================================
  Files         284      284              
  Lines       40253    40275      +22     
==========================================
+ Hits        35765    35791      +26     
+ Misses       4488     4484       -4     
Files Coverage Δ
ietf/doc/expire.py 95.31% <100.00%> (ø)
ietf/doc/urls_review.py 100.00% <ø> (ø)
ietf/review/models.py 92.85% <100.00%> (+0.88%) :arrow_up:
ietf/doc/views_review.py 95.14% <94.73%> (-0.02%) :arrow_down:

... and 3 files with indirect coverage changes

codecov[bot] avatar Nov 05 '23 09:11 codecov[bot]

@larseggert - Can we by fiat declare draft expiration happens at UTC midnight, or do we need more process first?

rjsparks avatar Nov 06 '23 10:11 rjsparks

I think IESG can declare.

How easy would it be to do "anywhere on Earth"?

larseggert avatar Nov 06 '23 10:11 larseggert

Anywhere on earth essentially means "international date line" instead of "Greenwich Meridian". It is easy to do, but it runs counter to the previous expressed goal to do everything in UTC.

rjsparks avatar Nov 06 '23 11:11 rjsparks

On 2023-11-06, at 11:11, Jennifer Richards @.***> wrote:

Thinking about it now, I think it'll be midnight at the start of the doc.expires date, which might be incorrect. Maybe it needs to be +datetime.timedelta(days=1).

This.

If my driving license expires on 2023-12-24, I expect still to be able to drive on 2023-12-24.

Grüße, Carsten

cabo avatar Nov 06 '23 11:11 cabo

My takeaways here are:

  • Remove the extraneous timezone conversion, as identified by @jennifer-richards
  • Make the timestamp accurate
    • Earlier in the file, the logic includes the "+1 day" calculation that @jennifer-richards suggested. I'll adopt that one.

I'm going to go work on that, and submit it to this issue. I'm going to look for opportunities to make the expiration of documents more explicit, though - we've already found at least six reasonable interpretations for when that could be (beginning of date and end of date, crossed with PDT, UTC, and International Dateline). I wonder whether it would be better to convert the date into a datestamp, in order to make that explicit.

meadmaker avatar Nov 08 '23 20:11 meadmaker

I wonder whether it would be better to convert the date into a datestamp, in order to make that explicit.

I should have said timestamp instead of datestamp. Oops.

I just examined the doc_document table, and the expires column is already a timestamp data type. It looks like that is set in ietf/submit/utils.py for regular submission and ietf/doc/views_draft.py for document resurrection by the Secretariat. I expect that if I can direct those two code paths through a single function to compute the expiration timestamp correctly, then all the other calculations can be converted to simple value display instead.

meadmaker avatar Nov 08 '23 20:11 meadmaker

This lost attention during the transition period. Lets see if it can either be finished or turned back into a clarified issue. cc: @jennifer-richards

rjsparks avatar Sep 04 '24 17:09 rjsparks

I believe the main hold-up is deciding what is the desired functionality.

Two parts:

  1. what time of day is the expiration
  2. where on Earth do you specify that time

I think 1) is "end of the expiration date" - at least, @cabo and I seem to agree.

For 2), UTC or "anywhere on earth" have been suggested.

The current implementation, if I understand it, will expire a document at the stroke of midnight UTC at the start of the expiration date. I think that leaves everyone unhappy, because we either want 12 hours later (for the end of day "anywhere on earth" standard, neglecting the Millenium Islands time zone) or 24 hours later (for end of day UTC)

jennifer-richards avatar Sep 04 '24 20:09 jennifer-richards

I agree with 1 and 2. We should build a reusable component to do that calculation. Is this PR a sufficient basis to work with to shift to that point, or should we approach the codebase again more generally?

rjsparks avatar Sep 04 '24 20:09 rjsparks

I'd say this is a good place to start.

jennifer-richards avatar Sep 05 '24 13:09 jennifer-richards