Bug 19368 - Add syspref to ignore words when sorting serials
Summary: Add syspref to ignore words when sorting serials
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: David Bourgault
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-26 17:47 UTC by David Bourgault
Modified: 2018-04-19 16:27 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


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, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19368 - Correction on QA feedback (3.31 KB, patch)
2018-03-12 11:23 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19368 - Syspref now "SortIgnoreArticles", used in datatables.inc (7.19 KB, patch)
2018-03-12 11:23 UTC, sandboxes@biblibre.com
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, sandboxes@biblibre.com
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, sandboxes@biblibre.com
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

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
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
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
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)
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
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
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
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 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
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
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
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
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 sandboxes@biblibre.com 2018-03-12 11:22:11 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 34 sandboxes@biblibre.com 2018-03-12 11:23:09 UTC Comment hidden (obsolete)
Comment 35 sandboxes@biblibre.com 2018-03-12 11:23:13 UTC Comment hidden (obsolete)
Comment 36 sandboxes@biblibre.com 2018-03-12 11:23:16 UTC Comment hidden (obsolete)
Comment 37 sandboxes@biblibre.com 2018-03-12 11:23:20 UTC Comment hidden (obsolete)
Comment 38 sandboxes@biblibre.com 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
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
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
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
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
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 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...