Created attachment 12754 [details] [review] Delete serials no more missing from missinglist With manual history disabled in subscription history section; that is when you have choosen to automatically record missing issues in the missinglist box, a serial prevously marked as missing was not deleted if received at the end or if its status was changed to 'claimed or 'late'.
Could you please explain how to test this?
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
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?
Created attachment 14657 [details] [review] Bug 8896: Delete no more missing serials from missinglist Well,I used a smartmatch in my code that didn't match (I think it works only from perl 5.10 and on). I replace it by a grep and it's ok now. 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 must be equal to 0 in sql subscription table) Save and go to Edit serial. Say you think that you will never receive the number N 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
Created attachment 15436 [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, it will be deleted from missinglist. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
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.
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 :)
Created attachment 17275 [details] [review] Bug 8896: Delete serials no more missing from missinglist I take into account in this new patch, the new possibility of reverting to expected status for Test plan see Comment 2
QA: Working on this now..
Created attachment 17362 [details] [review] Bug 8896 QA Followup for regex Making sure that the regex does not kill more than it should. Here are my test results when removing serialseq 3 :) 3 => 3 => 3; => 1;3 => 1 1; 3 => 1 1; 3; => 1 3;1 => 1 3; 1 => 1 3 ;1 => 1 3 ; 1 => 1 1;3;31 => 1;31 1;3 ;31 => 1;31 1;3; 31 => 1;31 1; 3;31 => 1;31 2;3;31;; ;,, ;;,, ,;,;,;4 => 2;31;4 Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 17363 [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.
Created attachment 17364 [details] [review] Bug 8896 QA Followup for regex Making sure that the regex does not kill more than it should. Here are my test results when removing serialseq 3 :) 3 => 3 => 3; => 1;3 => 1 1; 3 => 1 1; 3; => 1 3;1 => 1 3; 1 => 1 3 ;1 => 1 3 ; 1 => 1 1;3;31 => 1;31 1;3 ;31 => 1;31 1;3; 31 => 1;31 1; 3;31 => 1;31 2;3;31;; ;,, ;;,, ,;,;,;4 => 2;31;4 Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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
Please wait: still found something else..
Created attachment 17365 [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.
Created attachment 17366 [details] [review] Bug 8896 QA Followup for regex Making sure that the regex does not kill more than it should. Here are my test results when removing serialseq 3 :) 3 => 3 => 3; => 1;3 => 1 1; 3 => 1 1; 3; => 1 3;1 => 1 3; 1 => 1 3 ;1 => 1 3 ; 1 => 1 1;3;31 => 1;31 1;3 ;31 => 1;31 1;3; 31 => 1;31 1; 3;31;3A => 1;31;3A 2;3;31;; ;,, ;;,, ,;,;,;4 => 2;31;4 Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(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
With this patch, the missing list ends up screwed up: Vol;1;No;4;Vol;1;No;9
Created attachment 17577 [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>
(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.
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.
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>
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>
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.
Pushed to 3.10.x will be in 3.10.6