Bug 8896 - Delete serials no more missing from missinglist
Summary: Delete serials no more missing from missinglist
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Koha Team Lyon 3
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks: 10092
  Show dependency treegraph
 
Reported: 2012-10-10 08:58 UTC by Koha Team Lyon 3
Modified: 2013-12-05 20:09 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Delete serials no more missing from missinglist (1.12 KB, patch)
2012-10-10 08:58 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 8896: Delete no more missing serials from missinglist (1.17 KB, patch)
2013-01-17 10:29 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 8896: Delete serials no more missing from missinglist (1.19 KB, patch)
2013-02-17 08:00 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8896: Delete serials no more missing from missinglist (1.20 KB, patch)
2013-04-08 09:31 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 8896 QA Followup for regex (1.47 KB, patch)
2013-04-11 10:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8896: Delete serials no more missing from missinglist (1.31 KB, patch)
2013-04-11 10:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8896 QA Followup for regex (1.47 KB, patch)
2013-04-11 10:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8896: Delete serials no more missing from missinglist (1.31 KB, patch)
2013-04-11 11:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8896 QA Followup for regex (1.49 KB, patch)
2013-04-11 11:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8896 QA Followup for regex (2.11 KB, patch)
2013-04-22 10:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8896: Delete serials no more missing from missinglist (1.33 KB, patch)
2013-05-03 13:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8896 QA Followup for regex (2.13 KB, patch)
2013-05-03 13:05 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Koha Team Lyon 3 2012-10-10 08:58:03 UTC Comment hidden (obsolete)
Comment 1 Jared Camins-Esakov 2012-11-10 13:57:51 UTC
Could you please explain how to test this?
Comment 2 Koha Team Lyon 3 2012-11-12 18:02:40 UTC
Test Plan :
Choose a serial to search in serial module
click on title link
click on Modify subscription and uncheck if necessary the 'manual history' checkbox in Subscription history part. ( => manualhistory=0 in sql subscription table)
Save and go to Edit serial.
Say you think that you will never receive the number X and out of despair set the status to 'missing'.
If you come back to Modify subscription, Subscription history section, you can now see that the number N had been added to Missing field and as this supplies the corresponding zone on the OPAC, you'll also find it in the detail record of catalogue.
Now imagine that you unexpectedly receive this serial the day after.
You go to Edit serial and set it as 'arrived'.
But number N remains in subscription Missing field and opac.

Then, apply the patch and redo the same manouvers.
You will see at the end, that the number N had been deleted from missing zone
Comment 3 Liz Rea 2012-12-27 22:57:35 UTC
I stepped through your test plan - 

From start to finish, with the patch on, I don't see the missing issue in the history, but I don't see it in the arrived list either - am I supposed to?
Comment 4 Koha Team Lyon 3 2013-01-17 10:29:36 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2013-02-17 08:00:28 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2013-03-26 15:34:46 UTC
What about this: I marked it back as Expected instead of Arrived, perhaps because Missing was just a wrong click. Now I see that it is still missing.
So do you agree that we are still missing something :) or do I overlook something?

Please clarify. Thanks.
Comment 7 Katrin Fischer 2013-03-26 18:42:16 UTC
The option to change status back to expected is new in master as of the hackfest in Marseille last week, so maybe this patch needs to be amended a bit :)
Comment 8 Koha Team Lyon 3 2013-04-08 09:31:00 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2013-04-11 10:33:43 UTC
QA: Working on this now..
Comment 10 Marcel de Rooy 2013-04-11 10:56:39 UTC Comment hidden (obsolete)
Comment 11 Marcel de Rooy 2013-04-11 10:56:51 UTC Comment hidden (obsolete)
Comment 12 Marcel de Rooy 2013-04-11 10:56:55 UTC Comment hidden (obsolete)
Comment 13 Marcel de Rooy 2013-04-11 11:00:00 UTC
QA Comment:

Thanks for adjusting. I unfortunately still had some issues:
1) Your regex removes $serialseq from the missing list. Say that it is 1. This will remove also all other ;1\d+ from the list like 10 or 11 etc.
I had some fun with regexes and sent a QA followup for them.

2) Could you send a followup for marking Not available and back? Please test. Had some funny effects with it:
Like Missing issues:5; not issued 5
Delete 5 now: 5 is removed from missinglist, but "; not issued 5" remains there.

Point 2 does not block this patch however.

No problems with qa tools.

Passed QA
Comment 14 Marcel de Rooy 2013-04-11 11:01:32 UTC
Please wait: still found something else..
Comment 15 Marcel de Rooy 2013-04-11 11:11:00 UTC Comment hidden (obsolete)
Comment 16 Marcel de Rooy 2013-04-11 11:11:04 UTC Comment hidden (obsolete)
Comment 17 Marcel de Rooy 2013-04-11 11:11:54 UTC
(In reply to comment #14)
> Please wait: still found something else..
Replaced \D in the adjusted regex by a better [,;\s]

Back to Passed QA
Comment 18 Jared Camins-Esakov 2013-04-22 01:12:19 UTC
With this patch, the missing list ends up screwed up: Vol;1;No;4;Vol;1;No;9
Comment 19 Marcel de Rooy 2013-04-22 10:16:20 UTC Comment hidden (obsolete)
Comment 20 Marcel de Rooy 2013-04-22 10:23:01 UTC
(In reply to comment #18)
> With this patch, the missing list ends up screwed up: Vol;1;No;4;Vol;1;No;9

And call it QA followup :)
Good catch! I sobered (English?) the followup; it just looks at the ; (colons) and does not touch the commas anymore.
Could not resist however to touch two index expressions. Replaced by the same regex for consistency.
There is even more in this code. But I started a new report to get even more consistency. Bug 10092 handles updating the receivedlist similarly.

The new report still needs a sign-off. Could anyone of you take care of that?

I am setting the status of this report back to Signed off. But propose that somebody else passes QA now. Thanks.
Comment 21 Jonathan Druart 2013-05-03 13:04:02 UTC
QA comment:

The patches works as expected.
But it does not change the data in the DB, so the issue still exists until user change status for a missing serial.
It's too bad not to deal with existing data.
I pass QA this patch until it fixes a bug but it would be great to have a followup.

Marked as Passed QA.
Comment 22 Jonathan Druart 2013-05-03 13:05:36 UTC
Created attachment 17927 [details] [review]
Bug 8896: Delete serials no more missing from missinglist

When manual history is disabled in subscription history section
if a serial has been previously set as missing and is received
or set as expected, late or claimed, it will be deleted from missinglist

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Regex needs a followup. More comments on Bugzilla.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 23 Jonathan Druart 2013-05-03 13:05:40 UTC
Created attachment 17928 [details] [review]
Bug 8896 QA Followup for regex

Making sure that the regex does not kill more than it should.
Amended: does now only look at separating colons(;) not commas(,).
Amended: two index expressions in direct context replaced by same regex for
consistency.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 24 Jared Camins-Esakov 2013-05-06 12:52:38 UTC
This patch has been pushed to master and 3.12.x.

I was unable to test this, because between the revised serials history patches and previously applying these patches the Serials module in my test installation is wholly unusable. I will revert from 3.12.x at the first complaint, but I did not want to bump it to 3.14 if that isn't necessary.
Comment 25 Chris Cormack 2013-05-12 03:09:35 UTC
Pushed to 3.10.x will be in 3.10.6