Bug 33247 - Deleted authority still on results list
Summary: Deleted authority still on results list
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-16 15:11 UTC by Filipe Ribeiro
Modified: 2023-12-28 20:47 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.01,22.11.07


Attachments
Bug 33247: Flag the authority pending deletion in the result list (2.63 KB, patch)
2023-05-11 11:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33247: Flag the authority pending deletion in the result list (2.67 KB, patch)
2023-05-12 21:30 UTC, David Nind
Details | Diff | Splinter Review
Bug 33247: Flag the authority pending deletion in the result list (2.77 KB, patch)
2023-06-02 08:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33247: (QA follow-up) Remove pending deletion from results array (2.20 KB, patch)
2023-06-02 08:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Filipe Ribeiro 2023-03-16 15:11:52 UTC
After deleting an authority from authorities home, the page loaded after the operation still contains the deleted authority.

To reproduce:
1 - Go to authorities home and perform a search;
2 - Delete any authority;
3 - After the confirmation, the page is refreshed but still contains the deleted authority;

If you reload the page, that row disappear.
It seems that the authorities search is performed before the index is deleted.
Comment 1 Katrin Fischer 2023-04-16 10:36:26 UTC
I can confirm this behavior in current master. 

The index update usually happens almost immediately, but only almost and the page load/search appears to be a little faster.

I am really not sure how this could be helped. 

Maybe we could show a "success" dialog page first to gain some time, but then we lose a bit of usability. Waiting for the index also seems like a bad idea as it could be stuck for a little when a lot of operations need to be done. 

Maybe someone else has a good idea?
Comment 2 Jonathan Druart 2023-05-11 11:51:33 UTC
Created attachment 151070 [details] [review]
Bug 33247: Flag the authority pending deletion in the result list

When an authority is removed from the result list it is not removed from
the search engine yet.
This patch suggests to add a note in the summary column to tell the user
that the deletion is pending.

Test plan:
1 - Go to authorities home and perform a search;
2 - Delete any authority;
3 - After the confirmation, the page is refreshed but still contains the
deleted authority, with a red note explaining that the deletion is
pending.
Comment 3 Jonathan Druart 2023-05-11 11:52:10 UTC
Quick an dirty fix. Would it be enough?
Comment 4 Phil Ringnalda 2023-05-12 21:19:03 UTC
Enough for people who never delete more than one between index updates, maybe. I routinely have 5-10 deleted between updates multiple times a day.

We actually already have a dirty indication of deleted records if your search didn't specify an authority type, since in that case we can't look up the authority type to display it above the heading in the search results. So already-deleted headings are the ones with a slightly shorter row because they don't have the authority type part of the display.

Maybe try to look up the authority type even when it's specified in the search, and when the record isn't found, mark it as deleted/missing? I'd be surprised if that made a noticeable perf hit, what with everything else we have to do to assemble a page of search results.
Comment 5 David Nind 2023-05-12 21:30:48 UTC
Created attachment 151162 [details] [review]
Bug 33247: Flag the authority pending deletion in the result list

When an authority is removed from the result list it is not removed from
the search engine yet.
This patch suggests to add a note in the summary column to tell the user
that the deletion is pending.

Test plan:
1 - Go to authorities home and perform a search;
2 - Delete any authority;
3 - After the confirmation, the page is refreshed but still contains the
deleted authority, with a red note explaining that the deletion is
pending.

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Marcel de Rooy 2023-05-17 13:29:01 UTC
This does not cover cases where another user just removed an authority before it got reindexed. So it feels like a partial solution. Should we build the list from the database, or just live with the time frame between action and reindex?

Instead of marking it as pending deletion, I would suggest to not display it since it was deleted after all. Please provide feedback why the current solution is better than doing that.
Comment 7 Marcel de Rooy 2023-05-17 13:33:40 UTC
(In reply to Phil Ringnalda from comment #4)
> We actually already have a dirty indication of deleted records if your
> search didn't specify an authority type, since in that case we can't look up
> the authority type to display it above the heading in the search results. So
> already-deleted headings are the ones with a slightly shorter row because
> they don't have the authority type part of the display.

Do not understand this argument. I removed a PERSO_NAME record. It is marked with Pending deletion, but I still see the heading type Personal name. Or do you mean something else ?
Comment 8 Jonathan Druart 2023-05-17 13:54:17 UTC
(In reply to Marcel de Rooy from comment #6)
> Instead of marking it as pending deletion, I would suggest to not display it
> since it was deleted after all. Please provide feedback why the current
> solution is better than doing that.

Easy and simple to implement, that was my only motivation to try to resolve this bug.
Comment 9 Marcel de Rooy 2023-05-17 13:56:18 UTC
(In reply to Jonathan Druart from comment #8)
> (In reply to Marcel de Rooy from comment #6)
> > Instead of marking it as pending deletion, I would suggest to not display it
> > since it was deleted after all. Please provide feedback why the current
> > solution is better than doing that.
> 
> Easy and simple to implement, that was my only motivation to try to resolve
> this bug.

Would it be so hard to hide a row instead of marking it with an additional string then?
Comment 10 Phil Ringnalda 2023-05-17 20:39:08 UTC
(In reply to Marcel de Rooy from comment #7)
> (In reply to Phil Ringnalda from comment #4)
> > We actually already have a dirty indication of deleted records if your
> > search didn't specify an authority type, since in that case we can't look up
> > the authority type to display it above the heading in the search results. So
> > already-deleted headings are the ones with a slightly shorter row because
> > they don't have the authority type part of the display.
> 
> Do not understand this argument. I removed a PERSO_NAME record. It is marked
> with Pending deletion, but I still see the heading type Personal name. Or do
> you mean something else ?

Turns out not to be what I thought it was: rather than the result of hitting the db for the authority type, it must be the result of index differences (either a missing index, or speed at updating an index) between Zebra and Elasticsearch, since it's Zebra-only that the type in the Summary column disappears, and the one in the Heading type column switches to Default.
Comment 11 Marcel de Rooy 2023-06-02 08:39:55 UTC
Created attachment 151974 [details] [review]
Bug 33247: Flag the authority pending deletion in the result list

When an authority is removed from the result list it is not removed from
the search engine yet.
This patch suggests to add a note in the summary column to tell the user
that the deletion is pending.

Test plan:
1 - Go to authorities home and perform a search;
2 - Delete any authority;
3 - After the confirmation, the page is refreshed but still contains the
deleted authority, with a red note explaining that the deletion is
pending.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2023-06-02 08:39:57 UTC
Created attachment 151975 [details] [review]
Bug 33247: (QA follow-up) Remove pending deletion from results array

Both Elastic and Zebra search_auth_compat return a hash key
authid for a result.

Test plan:
Remove an authority that should be in your results list.
Note that it is no longer marked as pending, but just not displayed.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Jonathan Druart 2023-06-02 08:55:22 UTC
But then you refresh and the auth popup again (if index not updated already)? :)
Comment 14 Marcel de Rooy 2023-06-02 09:20:01 UTC
(In reply to Jonathan Druart from comment #13)
> But then you refresh and the auth popup again (if index not updated
> already)? :)

If you are fast enough :)
Comment 15 Tomás Cohen Arazi 2023-06-05 17:37:07 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 16 Tomás Cohen Arazi 2023-06-09 14:07:38 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 17 Matt Blenkinsop 2023-06-12 10:15:05 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x