19368 – Add syspref to ignore words when sorting serials

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:	Main
Hardware:	All All

Importance:	P5 - low enhancement
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)
CC List:	14 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:
Circulation function:

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
Show Obsolete (61) View All Add an attachment (proposed patch, testcase, etc.)

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)

Created attachment 67373 [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..."

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)

Created attachment 67447 [details] [review]
Bug 19368 - Correction on QA feedback

- Removed '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

Comment 4 Alex Buckley 2017-10-04 00:25:33 UTC Comment hidden (obsolete)

Created attachment 67580 [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 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)

Created attachment 67612 [details] [review]
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc

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)

Created attachment 69005 [details] [review]
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc

Comment 12 David Bourgault 2017-11-07 18:34:01 UTC Comment hidden (obsolete)

Updated incorrect YAML preference file

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)

Created attachment 70051 [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 16 Charles Farmer 2017-12-21 15:12:19 UTC Comment hidden (obsolete)

Created attachment 70052 [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 17 Charles Farmer 2017-12-21 15:12:24 UTC Comment hidden (obsolete)

Created attachment 70053 [details] [review]
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc

Comment 18 Charles Farmer 2017-12-21 20:53:57 UTC Comment hidden (obsolete)

Necessary rebase

Comment 19 Charles Farmer 2017-12-22 14:37:52 UTC Comment hidden (obsolete)

Created attachment 70125 [details] [review]
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref

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)

Created attachment 71962 [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 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)

Created attachment 71964 [details] [review]
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc

Comment 24 Charles Farmer 2018-02-19 16:21:28 UTC Comment hidden (obsolete)

Created attachment 71965 [details] [review]
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref

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 on attachment 71962 [details] [review]
Bug 19368 - Ignore words when sorting serial search

You're comparing the stopwords to the _unaccented_ subscription title. This isn't mentioned in the SerialSortIgnoreWords description.

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)

Created attachment 72044 [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 29 Charles Farmer 2018-02-21 17:44:40 UTC Comment hidden (obsolete)

Created attachment 72045 [details] [review]
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc

Comment 30 Charles Farmer 2018-02-21 17:44:44 UTC Comment hidden (obsolete)

Created attachment 72046 [details] [review]
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref

Comment 31 Charles Farmer 2018-02-21 17:44:47 UTC Comment hidden (obsolete)

Created attachment 72047 [details] [review]
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title

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)

Created attachment 72655 [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 35 Biblibre Sandboxes 2018-03-12 11:23:13 UTC Comment hidden (obsolete)

Created attachment 72656 [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 36 Biblibre Sandboxes 2018-03-12 11:23:16 UTC Comment hidden (obsolete)

Created attachment 72657 [details] [review]
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Comment 37 Biblibre Sandboxes 2018-03-12 11:23:20 UTC Comment hidden (obsolete)

Created attachment 72658 [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 38 Biblibre Sandboxes 2018-03-12 11:23:23 UTC Comment hidden (obsolete)

Created attachment 72659 [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 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)

Created attachment 74262 [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 41 Charles Farmer 2018-04-16 19:39:36 UTC Comment hidden (obsolete)

Created attachment 74263 [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 42 Charles Farmer 2018-04-16 19:39:40 UTC Comment hidden (obsolete)

Created attachment 74264 [details] [review]
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Comment 43 Charles Farmer 2018-04-16 19:39:45 UTC Comment hidden (obsolete)

Created attachment 74265 [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 44 Charles Farmer 2018-04-16 19:39:49 UTC Comment hidden (obsolete)

Created attachment 74266 [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 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 (kidclamp) 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)

Created attachment 90915 [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 49 Nadine Pierre 2019-06-21 15:48:58 UTC Comment hidden (obsolete)

Created attachment 90916 [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 50 Nadine Pierre 2019-06-21 15:49:04 UTC Comment hidden (obsolete)

Created attachment 90918 [details] [review]
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Comment 51 Nadine Pierre 2019-06-21 15:49:10 UTC Comment hidden (obsolete)

Created attachment 90919 [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 52 Nadine Pierre 2019-06-21 15:49:17 UTC Comment hidden (obsolete)

Created attachment 90920 [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 53 Nadine Pierre 2019-06-21 15:49:24 UTC Comment hidden (obsolete)

Created attachment 90921 [details] [review]
Bug 19368 - Added missing closing bracket at the end of serials-search.pl

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)

Created attachment 113018 [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 57 Alexis Ripetti 2020-11-04 22:17:16 UTC Comment hidden (obsolete)

Created attachment 113019 [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 58 Alexis Ripetti 2020-11-04 22:17:24 UTC Comment hidden (obsolete)

Created attachment 113020 [details] [review]
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Comment 59 Alexis Ripetti 2020-11-04 22:17:30 UTC Comment hidden (obsolete)

Created attachment 113021 [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 60 Alexis Ripetti 2020-11-04 22:17:41 UTC Comment hidden (obsolete)

Created attachment 113022 [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 61 Alexis Ripetti 2020-11-04 22:17:47 UTC Comment hidden (obsolete)

Created attachment 113023 [details] [review]
Bug 19368 - Added missing closing bracket at the end of serials-search.pl

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)

Created attachment 113344 [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 65 Séverine Queune 2020-11-10 11:25:15 UTC Comment hidden (obsolete)

Created attachment 113345 [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 66 Séverine Queune 2020-11-10 11:25:20 UTC Comment hidden (obsolete)

Created attachment 113346 [details] [review]
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Comment 67 Séverine Queune 2020-11-10 11:25:24 UTC Comment hidden (obsolete)

Created attachment 113347 [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 68 Séverine Queune 2020-11-10 11:25:28 UTC Comment hidden (obsolete)

Created attachment 113348 [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 69 Séverine Queune 2020-11-10 11:25:32 UTC Comment hidden (obsolete)

Created attachment 113349 [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 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)

Created attachment 118963 [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

Comment 73 Alexis Ripetti 2021-03-29 15:06:02 UTC Comment hidden (obsolete)

Created attachment 118964 [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

Comment 74 Alexis Ripetti 2021-03-29 15:06:15 UTC Comment hidden (obsolete)

Created attachment 118965 [details] [review]
Bug 19368 - Syspref now SortIgnoreArticles, used in datatables.inc

Comment 75 Alexis Ripetti 2021-03-29 15:06:26 UTC Comment hidden (obsolete)

Created attachment 118966 [details] [review]
Bug 19368 - Removed the old preference 'SerialSortIgnoreWords' that was left hanging in serials.pref

Comment 76 Alexis Ripetti 2021-03-29 15:06:38 UTC Comment hidden (obsolete)

Created attachment 118967 [details] [review]
Bug 19368 - clearly state, in the pref's desc, that the comparison is done against unaccented title

Comment 77 Alexis Ripetti 2021-03-29 15:06:46 UTC Comment hidden (obsolete)

Created attachment 118968 [details] [review]
Bug 19368 - Added missing closing bracket at the end of serials-search.pl

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).