Bug 24842

Summary: Automatic serials missing list feeding does not work when the numbering sequence includes regexp specific characters
Product: Koha Reporter: Koha Team University Lyon 3 <koha>
Component: SerialsAssignee: Koha Team University Lyon 3 <koha>
Status: RESOLVED DUPLICATE QA Contact: Testopia <testopia>
Severity: normal    
Priority: P2 CC: claudie.tregouet, colin.campbell, fridolin.somers, jonathan.druart, katrin.fischer, koha
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24903
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 24842 Regexp issue create doubles in the serial missing list
Bug 24842 Regexp issue create doubles in the serial missing list
Bug 24842 regexp issue create serial missing list doubles

Description Koha Team University Lyon 3 2020-03-10 14:20:35 UTC
We can encounter this issue, when we choose to automatically feed the serials missing list of a subscription (Manual history is uncheck in the subcription planning form).
When setting or removing the missing status to a serial issue in the staff interface, the process in the code of adding the issue numering sequence or removing it from  the missinglist is based on some regular expression.
It's not uncommon that librarians use characters like parenthesis in this sequence, for exemple to add year detail :
No 236 (2019)
We can even fall upon + or * characters.
The consequence is that they are interpreted by the regexp as capturing characters rather then litterals and this causes the process to fail. The result is the creation of multiple doubles in the missing list.

This patch aims to solve the problem

Test plan :

To go faster, choose an existing subscription, click on "Edit/Edit as new(duplicate)"
Uncheck Manual history and go to next page
Choose whatever simple config for this new subscription
Exemple :
Frequency : 1/month
Subscription length : 12
Subscription start date : 01/01/2020
Subscription end date : nothing
Numbering pattern : Volume, number
Last value 
    volume :2019
    number : 1

Click on "Hide advanced pattern" then "Modify pattern"
In the "Numbering formula pattern" zone, add parenthesis around {X}:
   Vol({X}), No {Y}
or an other critical character like + :
   Vol ({X}), No {Y} + suppl.
Fullfill the "Pattern name" : MyTest
Save as new pattern
Test prediction pattern
Save subscription

1) Receive one or two issues on this subscription
2) For the next expected one, set to any missing status rather than arrived one
3) Go to check the missing list in the summary tab of subscription :
   you can see that the issue had not been added to the list.

Install the patch

1) go back to Receive page and set the expected issue as missing.
2) go back to check the missing list : now the issue number is in the list
3) go back to Receive page and set the last missing issue as arrived.
4) go back to missing list : the issue had been removed.
Comment 1 Koha Team University Lyon 3 2020-03-10 14:33:57 UTC
Created attachment 100462 [details] [review]
Bug 24842 Regexp issue create doubles in the serial missing list
Comment 2 Koha Team University Lyon 3 2020-03-10 14:38:09 UTC
I have to add that this bug is already present in 18.11 and maybe in previuous ones

Olivier Crouzet
Comment 3 Koha Team University Lyon 3 2020-03-10 15:13:45 UTC
Sorry, an error in my test plan

NEW TEST PLAN :

Test plan :

To go faster, choose an existing subscription, click on "Edit/Edit as new(duplicate)"
Uncheck Manual history and go to next page
Choose whatever simple config for this new subscription
Exemple :
Frequency : 1/month
Subscription length : 12
Subscription start date : 01/01/2020
Subscription end date : nothing
Numbering pattern : Volume, number
Last value 
    volume :2019
    number : 1

Click on "Hide advanced pattern" then "Modify pattern"
In the "Numbering formula pattern" zone, add parenthesis around {X}:
   Vol({X}), No {Y}
or an other critical character like + :
   Vol ({X}), No {Y} + suppl.
Fullfill the "Pattern name" : MyTest
Save as new pattern
Test prediction pattern
Save subscription

1) Receive one or two issues on this subscription
2) For the next expected one, set to any missing status rather than arrived one
3) Modify the missing status to another missing status.
3) Go to check the missing list in the summary tab of subscription :
   the issue had been added to the list. You could think, it's ok but the reason is that the list was empt.
4) If you go back to set the same issue to another type of missing status, you will discover that it had been added a second time to the list.
4) if you set the missing status to "arrived", you will see that the sequences had not been removed from the missing list.

Install the patch

1) go back to Receive page and set the expected issue as missing.
2) go back to check the missing list : now the issue number is in the list
3) go back to Receive page and set the last missing issue as arrived.
4) go back to missing list : the issue had been removed

Olivier
Comment 4 Koha Team University Lyon 3 2020-03-12 13:49:49 UTC
Created attachment 100635 [details] [review]
Bug 24842 Regexp issue create doubles in the serial missing list

the "delete issue" aspect (sub DelIssue) of the process was still uncorrectly handled. I synchronized it with the rest.
... and redo my test plan again (...)

FINAL TEST PLAN (I hope so !) :
-------------------------------
To go faster, choose an existing subscription, click on "Edit/Edit as new(duplicate)"
Uncheck Manual history and go to next page
Choose whatever simple config for this new subscription
Exemple :
Frequency : 1/month
Subscription length : 12
Subscription start date : 01/01/2020
Subscription end date : nothing
Numbering pattern : Volume, number
Last value 
    volume :2019
    number : 1

Click on "Hide advanced pattern" then "Modify pattern"
In the "Numbering formula pattern" zone, add parenthesis around {X}:
   Vol({X}), No {Y}
or an other critical character like + :
   Vol ({X}), No {Y} + suppl.
Fullfill the "Pattern name" : MyTest
Save as new pattern
Test prediction pattern
Save subscription

1) Receive one or two issues on this subscription
2) For the next expected one, set to any missing status rather than arrived one
3) Go to check the missing list in the summary tab of subscription :
   the issue had been added to the list.
4) If you go back to set the same issue to another type of missing status, you will discover that it had been added a second time to the list.
5) if you set the missing status to "arrived", you will see that the sequences had not been removed from the missing list.

Install the patch
clear manually the missing list from added issues.
redo 1 to 5 steps
You 'll see that issues are correctly added (and only one time) when tey have to and correctly repoved.

If you delete an issue with missing status, you'll see that it's deleted from the list as well


Olivier Crouzet
Comment 5 Claudie Trégouët 2020-04-10 12:05:03 UTC
Created attachment 102702 [details] [review]
Bug 24842 regexp issue create serial missing list doubles

Signed-off-by: Claudie Trégouët <claudie.tregouet@biblibre.com>
Comment 6 Katrin Fischer 2020-04-12 22:29:16 UTC
I think this might be fixed by bug 24903 that I just passed QA - could you check and see if it helps your cause? It moves the logic into a new internal method, which looks like a good approach to me.

First tests are looking good.
Comment 7 Koha Team University Lyon 3 2020-06-29 16:53:06 UTC
(In reply to Katrin Fischer from comment #6)
> I think this might be fixed by bug 24903 that I just passed QA - could you
> check and see if it helps your cause? It moves the logic into a new internal
> method, which looks like a good approach to me.
> 
> First tests are looking good.

Hey Katrin,

At a first glance, it could help but it deals only with parenthesis case. I already fell upon sequences that were including  + and * characters (my colleagues are very creative) and I don't despair of finding out some $ one day.
I saw the comment "maybe extend in future" so I hope we are already in the future...

Olivier Crouzet
Comment 8 Jonathan Druart 2020-10-12 15:20:11 UTC
Olivier, could you write a unit test for this change?
Comment 9 Koha Team University Lyon 3 2020-10-15 14:22:06 UTC
(In reply to Jonathan Druart from comment #8)
> Olivier, could you write a unit test for this change?

Sorry, Jonathan but unit test is a thing I do not know to do and I fear I have no time to dig into at the moment.

Olivier.
Comment 10 Jonathan Druart 2021-02-10 07:21:53 UTC
This has been fixed by bug 24903.

*** This bug has been marked as a duplicate of bug 24903 ***