DRAFT Post mortem for annotations issue #34
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bz2
approved these changes
Feb 29, 2024
Comment on lines
+15
to
+24
| * -22 (Thursday) 18:25 after discussion with MP, AL confirms his understanding of auto-save | ||
| * When the user loads the platform in their browser it makes a GET request to the annotations API to get their current comments. | ||
| * From this point on, comment state is tracked internally to the app, with no further GET request to the server. | ||
| * The app has a boolean that says "Has my comments state changed since the last time I pushed comments to the server y/n"? | ||
| * This boolean is checked every 15 seconds. | ||
| * If the boolean is true (i.e. comments have been made or updated), a POST request is made to the annotations API to save the user's new comments state as a new annotations bag object. | ||
| * The boolean is then set back to false. We think this might happen regardless of whether or not the POST request to save the new comments state was actually successful, and there are no retries in the event of failure. | ||
| * Therefore, it is possible that if a user loses internet connection while they're making comments, and doesn't re-establish that connection until 15 seconds after they've made their last comment, all comments made within that period would not be saved. | ||
| * If they then closed/navigated away from the site without making further changes to their comments, the comments would be lost. | ||
| * -22 (Thursday) 18:44 MP add that as soon as an edit is made we create a promise on a (30 not 15, it turns out) timeout, and any future edits cancel out the old one and make a new one however, we do also have a visibilitychange listener that fires immediately if you eg tab away. There is no re-fire on hgttp error or timeout, which there would ideally be" |
There was a problem hiding this comment.
I'd suggest collapsing this entry to just one short bit about cause speculation for timeline parsability. Can move the details about being unsure on behaviour/edge cases to they whys block.
There was a problem hiding this comment.
I am not sure how best to do that @bz2
|
|
||
| 2023-02 (all times UTC) | ||
|
|
||
| * -19 (Monday) 15:24 CLient user begins recording comments using annotation feature - records 63 comments, can only see record of 45 comments after page refresh |
There was a problem hiding this comment.
I'd generally assign an initial to the specific client, as it can be unclear with multiple correspondence. There's also a facts-at-the-time vs known-at-the time thing.
| * -23 (Friday) 16:57 User loses comments a third time. Consultant updates second report and provides records of discussion with user. | ||
| * -24 (Saturday) 14:19 RT QA is able to manually reproduce error in Google Chrome with varying number of limts on comments (35 - 45) depending on the map and comment length - AL is able to establish that limit is 64kb. Error is not reproducable in Firefox. Error is reproducable in Safari. | ||
| * -26 (Monday) 10:29 MP is able to implement fix to staging | ||
| * -27 (Tuesday) ? Fix is deployed to production. Some limitations remain - Google Chrome is only saving every 30 seconds, and closing the broswer tab during the save can cause comments to be lost. |
There was a problem hiding this comment.
From heroku deploy logs:
=== shared-meaning Releases - Current: v155
v155 Deplo… 2024/02/29 15:44:44 +0000 (~ 7h ago)
v154 Deplo… 2024/02/27 13:09:33 +0000
Worth including the Thursday release too.
|
@bz2 - I can't merge in this repo. I don't think there are any further changes I can make. Please can you take this to completion? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Annotations feature did not record all comments provided for client - post mortem of issue and response
https://visual-meaning.atlassian.net/browse/SMP-2222