Bug 35265 - Remove drag and drop in Elasticsearch mappings
Summary: Remove drag and drop in Elasticsearch mappings
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Martin Renvoize
URL:
Keywords: rel_24_05_candidate
Depends on:
Blocks:
 
Reported: 2023-11-06 06:41 UTC by Fridolin Somers
Modified: 2024-01-15 14:19 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This removes the ability to drag and drop the order of the bibliographic and authorities search fields (Administration > Catalog > Search engine configuration (Elasticsearch)). This was removed as the feature has no effect on the search results when using Elasticsearch or OpenSearch as the search engine.
Version(s) released in:
24.05.00,23.11.02


Attachments
Bug 35265: Remove drag and drop in Elasticsearch mappings (3.90 KB, patch)
2023-11-06 06:57 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 35265: Remove drag and drop in Elasticsearch mappings (3.95 KB, patch)
2023-11-06 23:45 UTC, David Nind
Details | Diff | Splinter Review
Bug 35265: Remove drag and drop in Elasticsearch mappings (4.01 KB, patch)
2023-11-17 12:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35265: Remove drag and drop in Elasticsearch mappings (3.59 KB, patch)
2023-12-13 07:34 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 35265: Remove drag and drop in Elasticsearch mappings (3.66 KB, patch)
2023-12-14 13:27 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 35265: Remove drag and drop in Elasticsearch mappings (3.72 KB, patch)
2024-01-06 21:53 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2023-11-06 06:41:59 UTC
Bug 18827 says ordering search fields in Elasticsearch mappings is useless.
We can remove it.
This may help to convert into a real DataTable Bug 20588
Comment 1 Fridolin Somers 2023-11-06 06:57:22 UTC
Created attachment 158493 [details] [review]
Bug 35265: Remove drag and drop in Elasticsearch mappings

Bug 18827 says ordering search fields in Elasticsearch mappings is useless.

This patch removes it.

Test plan :
1) Enable Elasticsearch
2) Go to Administration > Search engine configuration (Elasticsearch)
3) Go to tab 'Bibliographic records'
4) Check there is no Javscript error
5) Check you can't drag and drop lines anymore
Comment 2 Fridolin Somers 2023-11-06 06:58:20 UTC
After that we may remove lib/jquery/plugins/jquery.tablednd.js looks like it is only used there.
Comment 3 Owen Leonard 2023-11-06 11:50:54 UTC
I'm working on Bug 35249 which removes the tablednd plugin in favor of the DataTables "official" extension. It is still needed for "Facet order" table found at the bottom of the Bibliographic records tab.

Unless I'm wrong in thinking that table uses the row-reordering functionality?
Comment 4 David Nind 2023-11-06 23:45:18 UTC
Created attachment 158582 [details] [review]
Bug 35265: Remove drag and drop in Elasticsearch mappings

Bug 18827 says ordering search fields in Elasticsearch mappings is useless.

This patch removes it.

Test plan :
1) Enable Elasticsearch
2) Go to Administration > Search engine configuration (Elasticsearch)
3) Go to tab 'Bibliographic records'
4) Check there is no Javscript error
5) Check you can't drag and drop lines anymore

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Martin Renvoize 2023-11-08 12:45:37 UTC
Should there be any cleanup serverside for this?  The patch removes the ability to drag and drop sort in the UI, but I'm struggling to see where/how that was stored serverside.. we should clean that side up too if it's really not useful.
Comment 6 Owen Leonard 2023-11-08 13:05:06 UTC
Can someone please verify whether table row reordering is still needed for the "Facet order" table? If so the plugin should not be removed.
Comment 7 Martin Renvoize 2023-11-08 13:55:14 UTC
OK.. looking through the code I'm fairly certain now that the sort ordering was never really stored anywhere.. we just displayed in name alpha order always.
Comment 8 Martin Renvoize 2023-11-17 12:26:05 UTC
I grabbed Nick.. he thinks the Facet Order does still matter.

So we can move this bug forward as is.. but we'll want to tackle any change to the facet side in another bug.
Comment 9 Martin Renvoize 2023-11-17 12:33:37 UTC
(In reply to Martin Renvoize from comment #8)
> I grabbed Nick.. he thinks the Facet Order does still matter.
> 
> So we can move this bug forward as is.. but we'll want to tackle any change
> to the facet side in another bug.

In other words.. bug 35249 is still valid I'm afraid Owen :)
Comment 10 Martin Renvoize 2023-11-17 12:36:58 UTC
Created attachment 159083 [details] [review]
Bug 35265: Remove drag and drop in Elasticsearch mappings

Bug 18827 says ordering search fields in Elasticsearch mappings is useless.

This patch removes it.

Test plan :
1) Enable Elasticsearch
2) Go to Administration > Search engine configuration (Elasticsearch)
3) Go to tab 'Bibliographic records'
4) Check there is no Javscript error
5) Check you can't drag and drop lines anymore

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Owen Leonard 2023-11-17 15:11:12 UTC
(In reply to Martin Renvoize from comment #9)
> In other words.. bug 35249 is still valid I'm afraid Owen :)

That's fine... I was trying to wait for Bug 34913
Comment 12 Tomás Cohen Arazi 2023-11-22 12:52:01 UTC
String changes, deferring
Comment 13 Katrin Fischer 2023-12-12 11:18:43 UTC
(In reply to Martin Renvoize from comment #8)
> I grabbed Nick.. he thinks the Facet Order does still matter.
> 
> So we can move this bug forward as is.. but we'll want to tackle any change
> to the facet side in another bug.

Just double-checking: before this patch, we can re-order the facets and the changed order also was stored for me. This patch breaks this behaviour. Please confirm if this is indeed intended.
Comment 14 Martin Renvoize 2023-12-12 13:01:02 UTC
You're right.. I must have switched branches mid-QA session as I know I tested it.. but it's really clearly removed here.

Back to you Fridolin I'm afraid.
Comment 15 Fridolin Somers 2023-12-13 07:34:19 UTC
Created attachment 159781 [details] [review]
Bug 35265: Remove drag and drop in Elasticsearch mappings

Bug 18827 says ordering search fields in Elasticsearch mappings is useless.

This patch removes it.
Drag and drop is kept for facets.

Test plan :
1) Enable Elasticsearch
2) Go to Administration > Search engine configuration (Elasticsearch)
3) Go to tab 'Bibliographic records'
4) Check there is no Javscript error
5) Check you can't drag and drop lines anymore

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Fridolin Somers 2023-12-13 07:34:58 UTC
(In reply to Martin Renvoize from comment #14)
> You're right.. I must have switched branches mid-QA session as I know I
> tested it.. but it's really clearly removed here.
> 
> Back to you Fridolin I'm afraid.

Indeed :)

I've restored DND for facets.

Please re-test.
Comment 17 ByWater Sandboxes 2023-12-14 13:27:17 UTC
Created attachment 159835 [details] [review]
Bug 35265: Remove drag and drop in Elasticsearch mappings

Bug 18827 says ordering search fields in Elasticsearch mappings is useless.

This patch removes it.
Drag and drop is kept for facets.

Test plan :
1) Enable Elasticsearch
2) Go to Administration > Search engine configuration (Elasticsearch)
3) Go to tab 'Bibliographic records'
4) Check there is no Javscript error
5) Check you can't drag and drop lines anymore

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Barbara Petritsch <barbara.petritsch@wienmuseum.at>
Comment 18 Victor Grousset/tuxayo 2024-01-06 21:53:30 UTC
Created attachment 160605 [details] [review]
Bug 35265: Remove drag and drop in Elasticsearch mappings

Bug 18827 says ordering search fields in Elasticsearch mappings is useless.

This patch removes it.
Drag and drop is kept for facets.

Test plan :
1) Enable Elasticsearch
2) Go to Administration > Search engine configuration (Elasticsearch)
3) Go to tab 'Bibliographic records'
4) Check there is no Javscript error
5) Check you can't drag and drop lines anymore

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Barbara Petritsch <barbara.petritsch@wienmuseum.at>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 19 Victor Grousset/tuxayo 2024-01-06 21:55:52 UTC
Works (facets still ok, adding, removing, editing biblio and auth record mappings too), makes sense, QA script happy, code looks good, passing QA :)
Comment 20 Katrin Fischer 2024-01-11 14:49:54 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 21 Fridolin Somers 2024-01-15 14:19:40 UTC
Pushed to 23.11.x for 23.11.02