[Bug 11846] New: Correct database update for debarments with 9999-12-31
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Bug ID: 11846 Summary: Correct database update for debarments with 9999-12-31 Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Database Assignee: gmcharlt@gmail.com Reporter: fridolyn.somers@biblibre.com QA Contact: testopia@bugs.koha-community.org The borrowers with infinite debarments have borrowers.debarred with '9999-12-31'. Database update of Bug 2720 3.13.00.035 contains : INSERT INTO borrower_debarments ( borrowernumber, expiration, comment ) SELECT borrowernumber, debarred, debarredcomment FROM borrowers WHERE debarred IS NOT NULL For borrowers where borrowers.debarred is '9999-12-31', this value is copied into borrower_debarments.expiration. This is not correct because borrower_debarments.expiration must be NULL for infinite debarments. Set as major because it creates an heavy load on pages displaying this infinite date (using the TT plugin KohaDates). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 --- Comment #1 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 25637 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25637&action=edit Bug 11846 - Correct database update for debarments with 9999-12-31 The borrowers with infinite debarments have borrowers.debarred with '9999-12-31'. Database update for this bug contained : INSERT INTO borrower_debarments ( borrowernumber, expiration, comment ) SELECT borrowernumber, debarred, debarredcomment FROM borrowers WHERE debarred IS NOT NULL For borrowers where borrowers.debarred is '9999-12-31', this value is copied into borrower_debarments.expiration. This is not correct because borrower_debarments.expiration must be NULL for infinite debarments. This patch corrects update 3.13.00.035 and also adds an update for databases already updated. Test plan : - Use a database with verison 3.13.00.000 - Set a borrower as restricted for ever (leave until empty) - Use sources to master + patch - Perform updatedatabase - Look at borrower details, tab "Restrictions" => Without patch, you see Expiration 31/12/9999 => With patch, you see Expiration Infinite -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |2720 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- I'm just wondering why even fix the old one? It requires more testing to prove it works. If it upgrades incorrectly (because you upgrade an older one), the current one will fix it. If it already upgraded incorrectly (because you upgrade after the back patch fix), the current one will fix it. But because of the back patch fix portion, you need to test the upgrading prior to the back patch fix portion. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #3 from M. Tompsett <mtompset@hotmail.com> --- You confused debarred (from borrowers) with expiration (from borrower_debarments). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 --- Comment #4 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- (In reply to M. Tompsett from comment #3)
You confused debarred (from borrowers) with expiration (from borrower_debarments). Oh. Is the patch wrong or is only the upgrade text wrong ?
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Comment on attachment 25637 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25637 Bug 11846 - Correct database update for debarments with 9999-12-31 Review of attachment 25637: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=11846&attachment=25637) ----------------------------------------------------------------- ::: installer/data/mysql/updatedatabase.pl @@ +8019,5 @@
+$DBversion = "3.15.00.XXX"; +if ( CheckVersion($DBversion) ) { + $dbh->do(q{ + UPDATE borrower_debarments SET debarred = NULL WHERE debarred = '9999-12-31'
This line is not right, as there is no debarred as a date in borrower_debarments. Please fix and test. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- Thank you for clarifying the problem, Katrin. That line. :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 --- Comment #7 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Indeed. I correct my patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25637|0 |1 is obsolete| | --- Comment #8 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Created attachment 26542 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26542&action=edit Bug 11846 - Correct database update for debarments with 9999-12-31 The borrowers with infinite debarments have borrowers.debarred with '9999-12-31'. Database update for this bug contained : INSERT INTO borrower_debarments ( borrowernumber, expiration, comment ) SELECT borrowernumber, debarred, debarredcomment FROM borrowers WHERE debarred IS NOT NULL For borrowers where borrowers.debarred is '9999-12-31', this value is copied into borrower_debarments.expiration. This is not correct because borrower_debarments.expiration must be NULL for infinite debarments. This patch corrects update 3.13.00.035 and also adds an update for databases already updated. Test plan : - Use a database with verison 3.13.00.000 - Set a borrower as restricted for ever (leave until empty) - Use sources to master + patch - Perform updatedatabase - Look at borrower details, tab "Restrictions" => Without patch, you see Expiration 31/12/9999 => With patch, you see Expiration Infinite -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Thanks for the fix. However, you still didn't address my question in comment 2. "why even fix the old one?" If it upgrades incorrectly (because you upgrade an older one), the current one will fix it. If it already upgraded incorrectly (because you upgrade after the back patch fix), the current one will fix it. I see no reason to change historical versions. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Is there a reason not to fix the old update? :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Assignee|gmcharlt@gmail.com |fridolyn.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 --- Comment #11 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #10)
Is there a reason not to fix the old update? :)
I agree that we can fix older updates. And in this case testing the older dbrev is more or less a theoretical problem. (By resetting Version and adding one SetVersion line in updatedatabase, we can achieve the same result.) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26542|0 |1 is obsolete| | --- Comment #12 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 27079 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27079&action=edit Bug 11846 - Correct database update for debarments with 9999-12-31 The borrowers with infinite debarments have borrowers.debarred with '9999-12-31'. Database update for this bug contained : INSERT INTO borrower_debarments ( borrowernumber, expiration, comment ) SELECT borrowernumber, debarred, debarredcomment FROM borrowers WHERE debarred IS NOT NULL For borrowers where borrowers.debarred is '9999-12-31', this value is copied into borrower_debarments.expiration. This is not correct because borrower_debarments.expiration must be NULL for infinite debarments. This patch corrects update 3.13.00.035 and also adds an update for databases already updated. Test plan : - Use a database with version 3.13.00.000 - Set a borrower as restricted for ever (leave until empty) - Use sources to master + patch - Perform updatedatabase - Look at borrower details, tab "Restrictions" => Without patch, you see Expiration 31/12/9999 => With patch, you see Expiration Infinite Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> I tested both db revisions by resetting Version of my database (and adding a SetVersion after the revised older dbrev). This was possible since the debarred field still exists in borrowers. (I have my doubts about that, but that goes outside the scope of this report.) Also note that routine _UpdateBorrowerDebarmentFlags puts back 9999-12-31 into borrowers for indefinite debarments (which looks not very consistent). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> ---
I tested both db revisions by resetting Version of my database (and adding a SetVersion after the revised older dbrev). This was possible since the debarred field still exists in borrowers. (I have my doubts about that, but that goes outside the scope of this report.)
Yes, borrower.debarred was retained for both speed and the fact that we wouldn't need to modify all the code that checks this field.
Also note that routine _UpdateBorrowerDebarmentFlags puts back 9999-12-31 into borrowers for indefinite debarments (which looks not very consistent).
Agreed, but again this is due to the extended feature being "backwards compatible" with the original debarment system. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27079|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 27093 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27093&action=edit Bug 11846 - Correct database update for debarments with 9999-12-31 The borrowers with infinite debarments have borrowers.debarred with '9999-12-31'. Database update for this bug contained : INSERT INTO borrower_debarments ( borrowernumber, expiration, comment ) SELECT borrowernumber, debarred, debarredcomment FROM borrowers WHERE debarred IS NOT NULL For borrowers where borrowers.debarred is '9999-12-31', this value is copied into borrower_debarments.expiration. This is not correct because borrower_debarments.expiration must be NULL for infinite debarments. This patch corrects update 3.13.00.035 and also adds an update for databases already updated. Test plan : - Use a database with version 3.13.00.000 - Set a borrower as restricted for ever (leave until empty) - Use sources to master + patch - Perform updatedatabase - Look at borrower details, tab "Restrictions" => Without patch, you see Expiration 31/12/9999 => With patch, you see Expiration Infinite Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> I tested both db revisions by resetting Version of my database (and adding a SetVersion after the revised older dbrev). This was possible since the debarred field still exists in borrowers. (I have my doubts about that, but that goes outside the scope of this report.) Also note that routine _UpdateBorrowerDebarmentFlags puts back 9999-12-31 into borrowers for indefinite debarments (which looks not very consistent). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |gmcharlt@gmail.com --- Comment #15 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Fridolin! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11846 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #16 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Patch pushed to 3.14.x, will be in 3.14.06 (creates version 3.14.05.001) -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org