issue with releasing the refetch behaviour change - specifically for concurrent editing of pages in similar locations with different users, there is the risk of race conditions. E.g. User A and B both want to modify folder 1; User A edits a page in folder 1, but before he submits the operation, user B deletes the folder - we suspect this will cause a floating page in the deleted folder 1 that is inaccessible via CMS. This issue is already present in production currently, but it's mostly mitigated by our refetch behaviour, because retrieving updated data on refocus will immediately stop users from making these changes to recently updated files/folders.
Closes [insert issue #]
Solution
Our proposed solution involves adding guards to create and update operations in the base GithubService - other operations (e.g. deleting an already removed file) will trigger an error from Github itself already, so only create and update endpoints have the potential to cause errors. We would have to make an additional query for each create and update endpoint to check that the file/directory still exists before we can make the change, so this increases the number of API calls we're making, but we feel that this is still an overall improvement over refetching on focus since there are a lot more GET queries than CREATE/UPDATE. We can monitor the overall change in token usage after pushing all the changes to see if this will cause any issues
Tests
To be honest, quite worried about this PR since the surface area of bugs is quite large. Manual testing done so far:
1) CRUD operations for files,folders
2) unable to replicate bug in the problem stated in this PR
3) Creating resource rooms folders
4) Creating more deeply nested media folders
5) Moving pages
Please feel free to put any notes about flows that I might have missed out here!
seaerchin
commented
1 year ago
Copying from @alexanderleegs's write up in slack:
Problem
issue with releasing the refetch behaviour change - specifically for concurrent editing of pages in similar locations with different users, there is the risk of race conditions. E.g. User A and B both want to modify folder 1; User A edits a page in folder 1, but before he submits the operation, user B deletes the folder - we suspect this will cause a floating page in the deleted folder 1 that is inaccessible via CMS. This issue is already present in production currently, but it's mostly mitigated by our refetch behaviour, because retrieving updated data on refocus will immediately stop users from making these changes to recently updated files/folders.
Closes [insert issue #]
Solution
Our proposed solution involves adding guards to create and update operations in the base GithubService - other operations (e.g. deleting an already removed file) will trigger an error from Github itself already, so only create and update endpoints have the potential to cause errors. We would have to make an additional query for each create and update endpoint to check that the file/directory still exists before we can make the change, so this increases the number of API calls we're making, but we feel that this is still an overall improvement over refetching on focus since there are a lot more GET queries than CREATE/UPDATE. We can monitor the overall change in token usage after pushing all the changes to see if this will cause any issues
Tests
To be honest, quite worried about this PR since the surface area of bugs is quite large. Manual testing done so far: 1) CRUD operations for files,folders 2) unable to replicate bug in the problem stated in this PR 3) Creating resource rooms folders 4) Creating more deeply nested media folders 5) Moving pages
Please feel free to put any notes about flows that I might have missed out here!