Bug 19368 - Add syspref to ignore words when sorting serials
Summary: Add syspref to ignore words when sorting serials
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Charles Farmer
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-26 17:47 UTC by David Bourgault
Modified: 2021-08-26 15:32 UTC (History)
15 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:


Attachments
Bug 19368 - Ignore words when sorting serial search (7.48 KB, patch)
2017-09-26 18:01 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.69 KB, patch)
2017-09-29 12:35 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (7.58 KB, patch)
2017-10-04 00:25 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.76 KB, patch)
2017-10-04 00:26 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc (6.15 KB, patch)
2017-10-04 17:57 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc (7.00 KB, patch)
2017-11-07 18:33 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (8.40 KB, patch)
2017-12-21 15:12 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.76 KB, patch)
2017-12-21 15:12 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc (7.00 KB, patch)
2017-12-21 15:12 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.22 KB, patch)
2017-12-22 14:37 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (7.43 KB, patch)
2018-02-19 16:21 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.16 KB, patch)
2018-02-19 16:21 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc (7.01 KB, patch)
2018-02-19 16:21 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.22 KB, patch)
2018-02-19 16:21 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (7.43 KB, patch)
2018-02-21 17:44 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.16 KB, patch)
2018-02-21 17:44 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc (7.04 KB, patch)
2018-02-21 17:44 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.22 KB, patch)
2018-02-21 17:44 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.33 KB, patch)
2018-02-21 17:44 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (7.58 KB, patch)
2018-03-12 11:23 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.31 KB, patch)
2018-03-12 11:23 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc (7.19 KB, patch)
2018-03-12 11:23 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.36 KB, patch)
2018-03-12 11:23 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.53 KB, patch)
2018-03-12 11:23 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (7.58 KB, patch)
2018-04-16 19:39 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.31 KB, patch)
2018-04-16 19:39 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc (6.98 KB, patch)
2018-04-16 19:39 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.36 KB, patch)
2018-04-16 19:39 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.53 KB, patch)
2018-04-16 19:39 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (14.47 KB, patch)
2019-06-21 15:48 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.29 KB, patch)
2019-06-21 15:48 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc (7.05 KB, patch)
2019-06-21 15:49 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.36 KB, patch)
2019-06-21 15:49 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.53 KB, patch)
2019-06-21 15:49 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 19368 - Added missing closing bracket at the end of serials-search.pl (631 bytes, patch)
2019-06-21 15:49 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (14.35 KB, patch)
2020-11-04 22:17 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.33 KB, patch)
2020-11-04 22:17 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc (7.95 KB, patch)
2020-11-04 22:17 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.20 KB, patch)
2020-11-04 22:17 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.54 KB, patch)
2020-11-04 22:17 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Added missing closing bracket at the end of serials-search.pl (631 bytes, patch)
2020-11-04 22:17 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (14.35 KB, patch)
2020-11-10 11:25 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.32 KB, patch)
2020-11-10 11:25 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc (7.95 KB, patch)
2020-11-10 11:25 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.20 KB, patch)
2020-11-10 11:25 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.54 KB, patch)
2020-11-10 11:25 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 19368 - Added missing closing bracket at the end of serials-search.pl (773 bytes, patch)
2020-11-10 11:25 UTC, Séverine Queune
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (14.35 KB, patch)
2021-03-29 15:05 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.33 KB, patch)
2021-03-29 15:06 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc (8.08 KB, patch)
2021-03-29 15:06 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.19 KB, patch)
2021-03-29 15:06 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.66 KB, patch)
2021-03-29 15:06 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Added missing closing bracket at the end of serials-search.pl (780 bytes, patch)
2021-03-29 15:06 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (14.46 KB, patch)
2021-08-25 20:44 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.33 KB, patch)
2021-08-25 20:44 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc (8.08 KB, patch)
2021-08-25 20:44 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.19 KB, patch)
2021-08-25 20:44 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.66 KB, patch)
2021-08-25 20:44 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Added missing closing bracket at the end of serials-search.pl (780 bytes, patch)
2021-08-25 20:45 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - (QA follow-up) Correction on QA feedback (12.04 KB, patch)
2021-08-25 20:45 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Deleting useless variables in datatables.inc (2.42 KB, patch)
2021-08-25 20:45 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Ignore words when sorting serial search (14.46 KB, patch)
2021-08-26 12:24 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.33 KB, patch)
2021-08-26 12:24 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc (8.08 KB, patch)
2021-08-26 12:24 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref (1.19 KB, patch)
2021-08-26 12:24 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title (1.66 KB, patch)
2021-08-26 12:25 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Added missing closing bracket at the end of serials-search.pl (780 bytes, patch)
2021-08-26 12:25 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - (QA follow-up) Correction on QA feedback (12.04 KB, patch)
2021-08-26 12:25 UTC, Alexis Ripetti
Details | Diff | Splinter Review
Bug 19368 - Deleting useless variables in datatables.inc (2.46 KB, patch)
2021-08-26 12:25 UTC, Alexis Ripetti
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Bourgault 2017-09-26 17:47:25 UTC
This is a functionality we offer some of our clients that we would like to push to the community.

It adds a system preference containing a list of words, which are ignored when sorting the serial search results table.
Comment 1 David Bourgault 2017-09-26 18:01:04 UTC Comment hidden (obsolete)
Comment 2 Josef Moravec 2017-09-29 12:15:50 UTC
Comment on attachment 67373 [details] [review]
Bug 19368 - Ignore words when sorting serial search

Review of attachment 67373 [details] [review]:
-----------------------------------------------------------------

Nice enhancement David.

Just few comments/issues, all looks easy to fix ;)

In update database file, don't use INSERT IGNORE, it is mysqlism. Better to make update files using perl skeleton and do needed db tests in the code.

::: installer/data/mysql/sysprefs.sql
@@ +475,4 @@
>  ('SelfCheckTimeout','120','','Define the number of seconds before the Web-based Self Checkout times out a patron','Integer'),
>  ('SeparateHoldings','0',NULL,'Separate current branch holdings from other holdings','YesNo'),
>  ('SeparateHoldingsBranch','homebranch','homebranch|holdingbranch','Branch used to separate holdings','Choice'),
> +('SerialSortIgnoreWords','des du de la le les leur d l an a the',NULL,'Words to ignore in serial search','Free'),

Your commit message says, it's empty  by default... which is good idea IMHO

::: koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt
@@ +38,4 @@
>        return confirm(_("Are you sure you want to reopen this subscription?"));
>      });
>   });
> +  $

typo?

::: serials/serials-search.pl
@@ +139,5 @@
> +    }
> +    $subsctitle =~ s/^\s+//;
> +    $sub->{titlesort} = ucfirst($subsctitle);
> +
> +    warn $subsctitle;

please, remove this warn
Comment 3 David Bourgault 2017-09-29 12:35:43 UTC Comment hidden (obsolete)
Comment 4 Alex Buckley 2017-10-04 00:25:33 UTC Comment hidden (obsolete)
Comment 5 Alex Buckley 2017-10-04 00:26:47 UTC
Created attachment 67581 [details] [review]
Bug 19368 - Correction on QA feedback

* I removed the IGNORE in the atomicupdate SQL. There is currently no other systempreference by that name in Master so that should work fine
- Removed leftover test values from syspref.sql
- Removed extra $ in serial-search.tt
- Removed leftover warn in serial-search.pl

Passes QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 6 Katrin Fischer 2017-10-04 05:45:35 UTC
I feel like this is trying to achieve something for one table, that we need a more general solution for. At the moment we already do something like this for other tables using a translatable string:

http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=koha-tmpl/intranet-tmpl/prog/en/includes/datatables.inc;h=72be3f10e834e9b478e1e66bbb60c3a9c39c91d4;hb=a1bf319829c9d44991daa2fcf2593420f8445c5b
var CONFIG_EXCLUDE_ARTICLES_FROM_SORT = _("a an the");

Maybe we should just move this string to be a system preference and then use it for serials too?
Comment 7 Katrin Fischer 2017-10-04 05:47:16 UTC
I think with the patch both methods would be used at the same time - anti-the and the new perl based method. 

Note: Example content of the pref should at least also contain English articles!
Comment 8 David Bourgault 2017-10-04 17:57:48 UTC Comment hidden (obsolete)
Comment 9 David Bourgault 2017-10-04 18:00:17 UTC
I updated the systempreference name to something more global (SortIgnoreWords) and updated datatables.inc to use it (opac and intranet).

Test plan is same as before, plus testing any table that uses datatables.inc.

Sponsored by : CCSR
Comment 10 Alex Buckley 2017-10-20 21:41:37 UTC
Hi David

The syspref SortIgnoreArticles is not being displaying in the Global System preferences module to fix this can you please add SortIgnoreArticles syspref to the admin.pref YAML file
Comment 11 David Bourgault 2017-11-07 18:33:24 UTC Comment hidden (obsolete)
Comment 12 David Bourgault 2017-11-07 18:34:01 UTC Comment hidden (obsolete)
Comment 13 Alex Buckley 2017-12-03 22:16:39 UTC
(Reply to David Bourgault in comment 12)

Yes I would have thought the serials.pref file would be the most suitable YAML file to be updated. 

I think I was wrong in comment 10.

Can you please add a patch to add SortIgnoreArticles to serials.pref
Comment 14 Katrin Fischer 2017-12-03 22:21:52 UTC
As this now not only works for Serials, but for datatables in general, I think maybe it would better fit on the 'staff client' tab? Hope I didn't misunderstand the change.
Comment 15 Charles Farmer 2017-12-21 15:12:14 UTC Comment hidden (obsolete)
Comment 16 Charles Farmer 2017-12-21 15:12:19 UTC Comment hidden (obsolete)
Comment 17 Charles Farmer 2017-12-21 15:12:24 UTC Comment hidden (obsolete)
Comment 18 Charles Farmer 2017-12-21 20:53:57 UTC Comment hidden (obsolete)
Comment 19 Charles Farmer 2017-12-22 14:37:52 UTC Comment hidden (obsolete)
Comment 20 Charles Farmer 2017-12-22 14:43:27 UTC
(In reply to Katrin Fischer from comment #14)
> As this now not only works for Serials, but for datatables in general, I
> think maybe it would better fit on the 'staff client' tab? Hope I didn't
> misunderstand the change.

The preference was recently moved from serials.pref to searching.pref to account, as you said, for the more 'global' impact the change has on datatables.

I have no qualms moving it though, if admin/preferences/searching.pref is not where you feel it should be.
Comment 21 Charles Farmer 2018-02-19 16:21:14 UTC Comment hidden (obsolete)
Comment 22 Charles Farmer 2018-02-19 16:21:18 UTC
Created attachment 71963 [details] [review]
Bug 19368 - Correction on QA feedback

* I removed the IGNORE in the atomicupdate SQL. There is currently no other systempreference by that name in Master so that should work fine
- Removed leftover test values from syspref.sql
- Removed extra $ in serial-search.tt
- Removed leftover warn in serial-search.pl

Passes QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 23 Charles Farmer 2018-02-19 16:21:23 UTC Comment hidden (obsolete)
Comment 24 Charles Farmer 2018-02-19 16:21:28 UTC Comment hidden (obsolete)
Comment 25 Charles Farmer 2018-02-19 16:22:39 UTC
Rebased on current master.
Comment 26 paxed 2018-02-20 06:29:24 UTC Comment hidden (obsolete)
Comment 27 Charles Farmer 2018-02-21 17:44:32 UTC
Created attachment 72043 [details] [review]
Bug 19368 - Ignore words when sorting serial search

This adds the 'SerialSortIgnoreWords' system preference.
It is empty by default, since words to ignore are very much language-specific.

The functionality is achieved by adding a hidden column to the table, and sorting by this column by default. The downfall of this solution is that if the user sorts by another column, they cannot go back to this sorting without refreshing the page.

To test:
0) Apply patch
1) Run updatedatabase.pl
2) Add test subscriptions if you have none.
    Make sure to have subscriptions starting with "The An A" or similar
3) Go to system preferences and edit the SerialSortIgnoreWords pref
    Add the words "The An A" or the ones you used in your subscriptions
4) Go to Serials
5) Search serials (empty field will return all serials)
6) Result table should be sorted ignoring leading "The An A..."

Followed test plan, patch works as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 28 Charles Farmer 2018-02-21 17:44:36 UTC Comment hidden (obsolete)
Comment 29 Charles Farmer 2018-02-21 17:44:40 UTC Comment hidden (obsolete)
Comment 30 Charles Farmer 2018-02-21 17:44:44 UTC Comment hidden (obsolete)
Comment 31 Charles Farmer 2018-02-21 17:44:47 UTC Comment hidden (obsolete)
Comment 32 Charles Farmer 2018-02-21 17:47:39 UTC
As per comment#26, I made sure to warn the end-user, in the pref's description, that the comparison was done against the unaccented title.

Rebased the rest on the current master.
Comment 33 Biblibre Sandboxes 2018-03-12 11:22:11 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 34 Biblibre Sandboxes 2018-03-12 11:23:09 UTC Comment hidden (obsolete)
Comment 35 Biblibre Sandboxes 2018-03-12 11:23:13 UTC Comment hidden (obsolete)
Comment 36 Biblibre Sandboxes 2018-03-12 11:23:16 UTC Comment hidden (obsolete)
Comment 37 Biblibre Sandboxes 2018-03-12 11:23:20 UTC Comment hidden (obsolete)
Comment 38 Biblibre Sandboxes 2018-03-12 11:23:23 UTC Comment hidden (obsolete)
Comment 39 Séverine Queune 2018-03-12 11:24:20 UTC
This patch works perfectly !
A detail : the syspref is "SortIgnoreArticles" and not "SerialSortIgnoreWords".
Thanks for this work !
Comment 40 Charles Farmer 2018-04-16 19:39:31 UTC Comment hidden (obsolete)
Comment 41 Charles Farmer 2018-04-16 19:39:36 UTC Comment hidden (obsolete)
Comment 42 Charles Farmer 2018-04-16 19:39:40 UTC Comment hidden (obsolete)
Comment 43 Charles Farmer 2018-04-16 19:39:45 UTC Comment hidden (obsolete)
Comment 44 Charles Farmer 2018-04-16 19:39:49 UTC Comment hidden (obsolete)
Comment 45 Charles Farmer 2018-04-16 19:44:30 UTC
New rebase.

For some reason, patch couldn't be applied on master and failed with the useful "error: could not build fake ancestor".
Comment 46 Nick Clemens 2018-04-19 14:11:33 UTC
I like this idea, but I think I am confused here. 

The pref now affects all datatables where we use anti-the, however, we still generate the extra column in serials-search and sort against that initially?

This code also seems to automatically strip articles before apostrophes - this works for some languages, but would it work for all? I don't know if the 'anti-the' plugin works with apostrophe'd articles, can you weigh in on that severine or someone else?

I find it confusing that the table is initially sorted by one column, then uses another if you click.  Adding this to datatables seems to allow sorting by a hidden column:
 { 'orderData': 2, 'targets': 0},
Comment 47 Séverine Queune 2018-04-19 16:27:01 UTC
Hi Nick !
I'm not sure I understood all your comments, but I've tested once more and here are my feedbacks.

(In reply to Nick Clemens from comment #46)
> The pref now affects all datatables where we use anti-the,
When you say 'all databases using anti-the', can you tell me which one are concernd ?
Do you have some ideas it could impact staff client and so where I should test ?
I've tried on the /cgi-bin/koha/circ/circulation.pl page for the issues-table : articles are ignored even when the syspref is empty.

> however, we still generate the extra column in serials-search and sort against that initially?
I don't understand that part.

> This code also seems to automatically strip articles before apostrophes -
> this works for some languages, but would it work for all? I don't know if
> the 'anti-the' plugin works with apostrophe'd articles, can you weigh in on
> that severine or someone else?
I tested with " L' " and " D' ", used very often in french and they are correctly ignore for sorting if I save them on the syspref.
Do you have ideas of what of strings / articles / apostrophes use could be a problem ?

> I find it confusing that the table is initially sorted by one column, then
> uses another if you click.  Adding this to datatables seems to allow sorting
> by a hidden column:
>  { 'orderData': 2, 'targets': 0},
I don't understand that part either...
Comment 48 Nadine Pierre 2019-06-21 15:48:50 UTC Comment hidden (obsolete)
Comment 49 Nadine Pierre 2019-06-21 15:48:58 UTC Comment hidden (obsolete)
Comment 50 Nadine Pierre 2019-06-21 15:49:04 UTC Comment hidden (obsolete)
Comment 51 Nadine Pierre 2019-06-21 15:49:10 UTC Comment hidden (obsolete)
Comment 52 Nadine Pierre 2019-06-21 15:49:17 UTC Comment hidden (obsolete)
Comment 53 Nadine Pierre 2019-06-21 15:49:24 UTC Comment hidden (obsolete)
Comment 54 Nadine Pierre 2019-06-21 15:50:37 UTC
Rebased on current master and added missing bracket after resolving conflicts.
Comment 55 Séverine Queune 2019-10-29 12:24:10 UTC
Sorry Nadine I didn't try sooner, the patch doesn’t apply on more time.

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 19368 - Ignore words when sorting serial search
Using index info to reconstruct a base tree...
M	installer/data/mysql/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref
M	koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-search.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/serials.pref
Auto-merging installer/data/mysql/sysprefs.sql
Failed to merge in the changes.
Patch failed at 0001 Bug 19368 - Ignore words when sorting serial search
Comment 56 Alexis Ripetti 2020-11-04 22:17:05 UTC Comment hidden (obsolete)
Comment 57 Alexis Ripetti 2020-11-04 22:17:16 UTC Comment hidden (obsolete)
Comment 58 Alexis Ripetti 2020-11-04 22:17:24 UTC Comment hidden (obsolete)
Comment 59 Alexis Ripetti 2020-11-04 22:17:30 UTC Comment hidden (obsolete)
Comment 60 Alexis Ripetti 2020-11-04 22:17:41 UTC Comment hidden (obsolete)
Comment 61 Alexis Ripetti 2020-11-04 22:17:47 UTC Comment hidden (obsolete)
Comment 62 Séverine Queune 2020-11-05 16:57:41 UTC
Now I test it one more time, I noticed a few things...

Unexpected characters like '' can't be ignored (we've got plenty of those in our titles because of some ugly imported datas...)

I now most of devs won't agree with me, but I think it could be great if this feature finally doesn't ignore accents. For example, the word 'Thé' in French means tea and if I have a subscription beginning with it ('Thé ou café ?' for instance), I don't want it to be ignored.
The same for A and À.

I also have a question about sorting a title like 'À l'abordage !' or 'À l'ombre de mon arbre' : if i set the syspref to ignore both à and l' , should the 2 articles be ignored or only the first one ? For now, only the first is ignores.

Except these 2 points, this patch is great and works as expected !
Comment 63 Blou 2020-11-09 20:56:23 UTC
Frankly, the accent part is arguable.  The main reason it's like that is because comparing accents is a nightmare.  They look the same, they sound the same, they ARE the same, except they are coded differently (UTF8 and friends) so the comparison fails and the patch is mosly useless in French and other accentuated languages.

As for the last question, there's (again) no perfect answer.  Right now it'd skip the À and keep the L'.

This is one of those patches that we'd like to go through because it offers an improvement on the current code for most users (and usages) in its current form.  Trying to solve the details will put it in In Discussion purgatory.
Comment 64 Séverine Queune 2020-11-10 11:25:11 UTC Comment hidden (obsolete)
Comment 65 Séverine Queune 2020-11-10 11:25:15 UTC Comment hidden (obsolete)
Comment 66 Séverine Queune 2020-11-10 11:25:20 UTC Comment hidden (obsolete)
Comment 67 Séverine Queune 2020-11-10 11:25:24 UTC Comment hidden (obsolete)
Comment 68 Séverine Queune 2020-11-10 11:25:28 UTC Comment hidden (obsolete)
Comment 69 Séverine Queune 2020-11-10 11:25:32 UTC Comment hidden (obsolete)
Comment 70 Séverine Queune 2020-11-10 11:28:04 UTC
I completely understand, I expected this kind of answer.
Just pointing some complementary points.

This patch behaves as it's expected and the information about accent is present in the syspref, so I don't see now reasons for not signing it :)

A last detail : syspref is now named 'SortIgnoreArticles' and not 'SerialSortIgnoreWords' as in the test plan.
Comment 71 Marcel de Rooy 2021-01-29 10:03:57 UTC
Applying: Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc
error: sha1 information is lacking or useless (installer/data/mysql/mandatory/sysprefs.sql).
Comment 72 Alexis Ripetti 2021-03-29 15:05:50 UTC Comment hidden (obsolete)
Comment 73 Alexis Ripetti 2021-03-29 15:06:02 UTC Comment hidden (obsolete)
Comment 74 Alexis Ripetti 2021-03-29 15:06:15 UTC Comment hidden (obsolete)
Comment 75 Alexis Ripetti 2021-03-29 15:06:26 UTC Comment hidden (obsolete)
Comment 76 Alexis Ripetti 2021-03-29 15:06:38 UTC Comment hidden (obsolete)
Comment 77 Alexis Ripetti 2021-03-29 15:06:46 UTC Comment hidden (obsolete)
Comment 78 Owen Leonard 2021-03-29 15:26:30 UTC
- Are the changes to datatables.inc necessary? I think this has made the one change you want (CONFIG_EXCLUDE_ARTICLES_FROM_SORT) obsolete:
https://git.koha-community.org/Koha-community/Koha/commit/0c9b39a66dff00d2d76effc42824c2051484f9c5

- Shouldn't the preference description omit the reference to serial search results since the its use is theoretically general?

- Is it necessary to replace [% INCLUDE subscriptions_table %] in serials-search.tt? Going back to all that markup after switching to an include seems like a step backwards.

- Don't forget to run the QA tool to check for other issues after making corrections.
Comment 79 Alexis Ripetti 2021-08-25 20:44:37 UTC
Created attachment 124090 [details] [review]
Bug 19368 - Ignore words when sorting serial search

This adds the 'SerialSortIgnoreWords' system preference.
It is empty by default, since words to ignore are very much language-specific.

The functionality is achieved by adding a hidden column to the table, and sorting by this column by default. The downfall of this solution is that if the user sorts by another column, they cannot go back to this sorting without refreshing the page.

To test:
0) Apply patch
1) Run updatedatabase.pl
2) Add test subscriptions if you have none.
    Make sure to have subscriptions starting with "The An A" or similar
3) Go to system preferences and edit the SortIgnoreArticles pref
    Add the words "The An A" or the ones you used in your subscriptions
4) Go to Serials
5) Search serials (empty field will return all serials)
6) Result table should be sorted ignoring leading "The An A..."

Followed test plan, patch works as described
Comment 80 Alexis Ripetti 2021-08-25 20:44:42 UTC
Created attachment 124091 [details] [review]
Bug 19368 - Correction on QA feedback

* I removed the IGNORE in the atomicupdate SQL. There is currently no other systempreference by that name in Master so that should work fine
- Removed leftover test values from syspref.sql
- Removed extra $ in serial-search.tt
- Removed leftover warn in serial-search.pl

Passes QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 81 Alexis Ripetti 2021-08-25 20:44:46 UTC
Created attachment 124092 [details] [review]
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 82 Alexis Ripetti 2021-08-25 20:44:51 UTC
Created attachment 124093 [details] [review]
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 83 Alexis Ripetti 2021-08-25 20:44:55 UTC
Created attachment 124094 [details] [review]
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 84 Alexis Ripetti 2021-08-25 20:45:01 UTC
Created attachment 124095 [details] [review]
Bug 19368 - Added missing closing bracket at the end of serials-search.pl

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 85 Alexis Ripetti 2021-08-25 20:45:06 UTC
Created attachment 124096 [details] [review]
Bug 19368 - (QA follow-up) Correction on QA feedback
Comment 86 Alexis Ripetti 2021-08-25 20:45:11 UTC
Created attachment 124097 [details] [review]
Bug 19368 - Deleting useless variables in datatables.inc

I tried to understand where it was used but I finally came to the conclusion that this is useless.
Comment 87 Alexis Ripetti 2021-08-25 20:49:56 UTC
(In reply to Owen Leonard from comment #78)
> - Are the changes to datatables.inc necessary? I think this has made the one
> change you want (CONFIG_EXCLUDE_ARTICLES_FROM_SORT) obsolete:
> https://git.koha-community.org/Koha-community/Koha/commit/
> 0c9b39a66dff00d2d76effc42824c2051484f9c5
> 
> - Shouldn't the preference description omit the reference to serial search
> results since the its use is theoretically general?
> 
> - Is it necessary to replace [% INCLUDE subscriptions_table %] in
> serials-search.tt? Going back to all that markup after switching to an
> include seems like a step backwards.
> 
> - Don't forget to run the QA tool to check for other issues after making
> corrections.

No they are not necessary. They have been removed.

We are only using this patch on the serials search. So for now this description is good.
Comment 88 Jonathan Druart 2021-08-26 08:00:30 UTC
Alexis, did you run the QA script on those patches?
Comment 89 Alexis Ripetti 2021-08-26 12:24:37 UTC
Created attachment 124104 [details] [review]
Bug 19368 - Ignore words when sorting serial search

This adds the 'SerialSortIgnoreWords' system preference.
It is empty by default, since words to ignore are very much language-specific.

The functionality is achieved by adding a hidden column to the table, and sorting by this column by default. The downfall of this solution is that if the user sorts by another column, they cannot go back to this sorting without refreshing the page.

To test:
0) Apply patch
1) Run updatedatabase.pl
2) Add test subscriptions if you have none.
    Make sure to have subscriptions starting with "The An A" or similar
3) Go to system preferences and edit the SerialSortIgnoreWords pref
    Add the words "The An A" or the ones you used in your subscriptions
4) Go to Serials
5) Search serials (empty field will return all serials)
6) Result table should be sorted ignoring leading "The An A..."

Followed test plan, patch works as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 90 Alexis Ripetti 2021-08-26 12:24:43 UTC
Created attachment 124105 [details] [review]
Bug 19368 - Correction on QA feedback

* I removed the IGNORE in the atomicupdate SQL. There is currently no other systempreference by that name in Master so that should work fine
- Removed leftover test values from syspref.sql
- Removed extra $ in serial-search.tt
- Removed leftover warn in serial-search.pl

Passes QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 91 Alexis Ripetti 2021-08-26 12:24:48 UTC
Created attachment 124106 [details] [review]
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 92 Alexis Ripetti 2021-08-26 12:24:53 UTC
Created attachment 124107 [details] [review]
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 93 Alexis Ripetti 2021-08-26 12:25:05 UTC
Created attachment 124108 [details] [review]
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 94 Alexis Ripetti 2021-08-26 12:25:21 UTC
Created attachment 124109 [details] [review]
Bug 19368 - Added missing closing bracket at the end of serials-search.pl

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 95 Alexis Ripetti 2021-08-26 12:25:25 UTC
Created attachment 124110 [details] [review]
Bug 19368 - (QA follow-up) Correction on QA feedback
Comment 96 Alexis Ripetti 2021-08-26 12:25:30 UTC
Created attachment 124111 [details] [review]
Bug 19368 - Deleting useless variables in datatables.inc

I tried to understand where it was used but I finally came to the conclusion that this is useless.
Comment 97 Alexis Ripetti 2021-08-26 12:25:54 UTC
(In reply to Jonathan Druart from comment #88)
> Alexis, did you run the QA script on those patches?

Yes I did.
I missed one.
I just fixed it. But the other one is just a git manipulation error and I don't know how to fix it.
There are some spelling errors too.
Comment 98 Jonathan Druart 2021-08-26 12:45:29 UTC
You need to amend the commits and fix their title:

% git rebase -i HEAD~X (with X the number of commit)
Replace "pick" with "r" (reword), save.
Comment 99 Jonathan Druart 2021-08-26 15:32:18 UTC
The way this enhancement is implemented sounds weird to me. Why are you using perl when it can be done using Datatables (client-side)?

Having the ability to define a list of word to ignore is great, but it can be done (at first glance) easily by extending the 'anti-the' plugin (and it will apply to *all* tables of Koha).