Don't show edited timestamps if annotations haven't been updated #2690
Conversation
|
@robertknight I hand-edited my local DB to reproduce this issue for development. Let me know if you'd like to reproduce locally and/or would be assisted by any instructions. |
Codecov Report
@@ Coverage Diff @@
## master #2690 +/- ##
=======================================
Coverage 97.73% 97.73%
=======================================
Files 199 199
Lines 7506 7518 +12
Branches 1647 1651 +4
=======================================
+ Hits 7336 7348 +12
Misses 170 170
Continue to review full report at Codecov.
|
|
CI coverage fail because I touched tests in |
|
Converting to draft while I make sure that my |
lyzadanger
force-pushed
the
fix-edited-timestamp
branch
from
1bc46d7 to
8989935
Compare
|
Updated. Addressing this also cleared up the code coverage fail. |
There was a problem hiding this comment.
Hi Lyza,
The high-level change makes sense, I had some recommendations around caching the hasBeenEdited calculation, streamlining/simplifying the hasBeenEdited implementation and updating some weird variable names in the tests while you're modernizing that code.
lyzadanger
force-pushed
the
fix-edited-timestamp
branch
from
8989935 to
a5fd5a7
Compare
Previously, the `AnnotationHeader` component checked the equivalency of `annotation.created` and `annotation.updated` to determine if an annotation had been updated subsequent to its creation. This is problematic if an annotation has a very small difference between its `created` and `updated` dates. `created` and `updated` are returned by the API as ISO-8601 date strings with microsecond resolution. These days, annotations created by the `h` service will have exactly equivalent `created` and `updated` timestamps (assuming they haven't actually been edited subsequently, of course). In the past, however, annotations often had negligible (sub-second) differences in their `created` and `updated` dates. This resulted in an edited timestamp misleadingly appearing in the UI for older annotations. These changes add a `hasBeenEdited` function to the `annotation-metadata` util module that considers an annotation edited only if `created` and `updated` differ by at least 2 seconds. Tests for `annotation-metadata` have also been modernized somewhat here. Fixes #2684
lyzadanger
force-pushed
the
fix-edited-timestamp
branch
from
a5fd5a7 to
870a607
Compare
| assert.isFalse(annotationMetadata.hasBeenEdited(annotation)); | ||
| }); | ||
|
|
||
| it('should return false if created and updated are different but problematic', () => { |
There was a problem hiding this comment.
| it('should return false if created and updated are different but problematic', () => { | |
| it('should return false if created and updated are different but cannot be parsed', () => { |
I think it is useful to be more specific about what "problematic" means here.
Previously, the
AnnotationHeadercomponent checked the equivalencyof
annotation.createdandannotation.updatedto determine if anannotation had been updated subsequent to its creation. This is
problematic if an annotation has a very small difference between its
createdandupdateddates.createdandupdatedare returned by the API as ISO-8601 date stringswith microsecond resolution. These days, annotations created by the
hservice will have exactly equivalent
createdandupdatedtimestamps(assuming they haven't actually been edited subsequently, of course). In
the past, however, annotations often had negligible (sub-second)
differences in their
createdandupdateddates. This resulted inan edited timestamp misleadingly appearing in the UI for older
annotations.
These changes add a
hasBeenEditedfunction to theannotation-metadatautil module that considers an annotation editedonly if
createdandupdateddiffer by at least 2 seconds.Tests for
annotation-metadatahave also been modernized somewhat here. (This makes the diff look way bigger than it "is").Fixes #2684