-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes UI infinite polling after image delete #4389
Conversation
Recognises soft deleted as a deleted. Returns promise to that it can be waited on.
34fc99d
to
db7c10c
Compare
Thanks! I've tested locally and this looks to be working well. I think we have the same problem on the "undelete" button too though - maybe we can add the same logic there too? https://github.com/guardian/grid/blob/main/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js |
Looking again at this, I think there's a different logic problem here, the undelete button is polling until it finds the image has been deleted, where it should probably be polling until it finds the image has been undeleted. I'll merge this and look at that in a different PR. |
Seen on cropper (created by @tonytw1 and merged by @andrew-nowak 7 minutes and 33 seconds ago) Please check your changes! |
Seen on metadata-editor (created by @tonytw1 and merged by @andrew-nowak 7 minutes and 41 seconds ago) Please check your changes! |
Seen on leases, usage (created by @tonytw1 and merged by @andrew-nowak 7 minutes and 54 seconds ago) Please check your changes! |
Seen on image-loader, kahuna (created by @tonytw1 and merged by @andrew-nowak 7 minutes and 54 seconds ago) Please check your changes! |
Seen on leases, usage (created by @tonytw1 and merged by @andrew-nowak 7 minutes and 54 seconds ago) Please check your changes! |
Seen on auth (created by @tonytw1 and merged by @andrew-nowak 7 minutes and 55 seconds ago) Please check your changes! |
Seen on leases, usage (created by @tonytw1 and merged by @andrew-nowak 8 minutes ago) Please check your changes! |
Seen on leases (created by @tonytw1 and merged by @andrew-nowak 8 minutes ago) Please check your changes! |
Seen on leases, usage (created by @tonytw1 and merged by @andrew-nowak 8 minutes ago) Please check your changes! |
Seen on usage (created by @tonytw1 and merged by @andrew-nowak 8 minutes and 4 seconds ago) Please check your changes! |
|
What does this change?
After an image is deleted, Kahuna with poll endlessly looking for the deleted image to be removed from the API.
As soft deleted images do not disappear from the API, there is not way for this loop to end other than when the page is next reloaded.
Stopped the infinite polling by updating the polling function to recognise soft deleted as deleted.
Also returns the polling promise rather than void from
pollDeleted
.The existing code appears to have been dropping straight instantly, without waiting for the poll to complete.
Delete from list view UI now behaves as expected; there is a noticeable pause then the image is removed from the view.
Delete from single image screen may be slightly better.
This delete triggers a search refresh to remove the deleted image and is probably catching stale Elastic eventually consistent search results most of the time. The deleted image will still be visible in the search results most of the time as per what currently happens.
I think this PR is a net improvement.
How should a reviewer test this change?
How can success be measured?
Who should look at this?
Tested? Documented?