[Bug 32357] New: Set borrower_message_preferences.days_in_advance default to NULL
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Bug ID: 32357 Summary: Set borrower_message_preferences.days_in_advance default to NULL Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Database Assignee: koha-bugs@lists.koha-community.org Reporter: lari.taskula@hypernova.fi QA Contact: testopia@bugs.koha-community.org days_in_advance currently defaults to 0, but it should default to NULL because all but one entry in message_attribute has takes_days=1 making it the exception and not the default. Current column default 0 is already ignored by C4::Members::Messaging::SetMessagingPreference, see https://github.com/Koha-Community/Koha/blob/8c367775470fcecdda54e28270b05d92... , so we should also make NULL default at database level instead. Blocking Bug 18595 (because without this change some tests there will fail unless we replicate the misuse of default values) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 --- Comment #1 from Lari Taskula <lari.taskula@hypernova.fi> --- (In reply to Lari Taskula from comment #0)
because all but one entry in message_attribute has takes_days=1 making it the exception and not the default.
I of course meant to say that only one entry in message_attribute has takes_days=1 making it the exception and not the default. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18595 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18595 [Bug 18595] Move C4::Members::Messaging to Koha namespace -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 --- Comment #2 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 144307 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144307&action=edit Bug 32357: Set borrower_message_preferences.days_in_advance default to NULL -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 --- Comment #3 from Lari Taskula <lari.taskula@hypernova.fi> --- Not sure how to test it thoroughly, but any case using SetMessagingPreference() should not be affected as mentioned in comment #1 - as far as I understand only direct SQL INSERT to borrower_message_preferences and inserts with DBIx object (or TestBuilder) may cause regression. I found five tests making such insertions using $ grep -rn 'BorrowerMessagePreference' and $ grep -rn 'borrower_message_preferences' t/db_dependent/cronjobs/advance_notices_digest.t t/db_dependent/Letters/TemplateToolkit.t t/db_dependent/Koha/CurbsidePickups.t t/db_dependent/Koha/Patrons.t t/db_dependent/Patron/Messaging.t I'll add these to the patch description. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144307|0 |1 is obsolete| | --- Comment #4 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 144309 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144309&action=edit Bug 32357: Set borrower_message_preferences.days_in_advance default to NULL To test: 1. prove t/db_dependent/cronjobs/advance_notices_digest.t 2. prove t/db_dependent/Letters/TemplateToolkit.t 3. prove t/db_dependent/Koha/CurbsidePickups.t 4. prove t/db_dependent/Koha/Patrons.t 5. prove t/db_dependent/Patron/Messaging.t -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #5 from David Nind <david@davidnind.com> --- Tests pass both before and after the patch is applied. However, I don't think the databaseupdate updates existing installations. I had to do a reset_all to replicate a new installation (using koha-testing-docker). Not that I really know how all this should work! Testing notes: 1. Access the database: koha-mysql kohadev 2. Show the current database configuration - the default before the patch is applied for days_in_advance is 0. SQL query: show columns from borrower_message_preferences; 3. Apply the patch. 4. Update the database (updatedatabase). 5. Repeat steps 1-2 - the default for days_in_advance should now be NULL.[1] 6. Tests should pass: . prove t/db_dependent/cronjobs/advance_notices_digest.t . prove t/db_dependent/Letters/TemplateToolkit.t . prove t/db_dependent/Koha/CurbsidePickups.t . prove t/db_dependent/Koha/Patrons.t . prove t/db_dependent/Patron/Messaging.t [1] The updatedatabase command didn't update the existing installation, I had to do a reset_all to get the database to update. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |lari.taskula@hypernova.fi |ity.org | --- Comment #6 from David Nind <david@davidnind.com> --- I've changed the assignee. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144309|0 |1 is obsolete| | --- Comment #7 from Lari Taskula <lari.taskula@hypernova.fi> --- Created attachment 146475 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146475&action=edit Bug 32357: Set borrower_message_preferences.days_in_advance default to NULL To test: 1. prove t/db_dependent/cronjobs/advance_notices_digest.t 2. prove t/db_dependent/Letters/TemplateToolkit.t 3. prove t/db_dependent/Koha/CurbsidePickups.t 4. prove t/db_dependent/Koha/Patrons.t 5. prove t/db_dependent/Patron/Messaging.t -- You are receiving this mail because: You are watching all bug changes.
However, I don't think the databaseupdate updates existing installations. I had to do a reset_all to replicate a new installation (using koha-testing-docker). True! I didn't know but there seems to be a difference in atomicupdate filenames ending between .perl and .pl, as seen on C4::Installer::run_atomic_updates(). I renamed it to *.pl and at least on my
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 --- Comment #8 from Lari Taskula <lari.taskula@hypernova.fi> --- (In reply to David Nind from comment #5) old dev installation it now applies with the updatedatabase script. Maybe someone knows why this is? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146475|0 |1 is obsolete| | --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 149223 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=149223&action=edit Bug 32357: Set borrower_message_preferences.days_in_advance default to NULL To test: 1. prove t/db_dependent/cronjobs/advance_notices_digest.t 2. prove t/db_dependent/Letters/TemplateToolkit.t 3. prove t/db_dependent/Koha/CurbsidePickups.t 4. prove t/db_dependent/Koha/Patrons.t 5. prove t/db_dependent/Patron/Messaging.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 --- Comment #10 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Access the database: koha-mysql kohadev 2. Show the current database configuration - the default before the patch is applied for days_in_advance is 0. SQL query: show columns from borrower_message_preferences; 3. Apply the patch. 4. Update the database (updatedatabase). 5. Repeat steps 1-2 - the default for days_in_advance should now be NULL. 6. Tests should pass: . prove t/db_dependent/cronjobs/advance_notices_digest.t . prove t/db_dependent/Letters/TemplateToolkit.t . prove t/db_dependent/Koha/CurbsidePickups.t . prove t/db_dependent/Koha/Patrons.t . prove t/db_dependent/Patron/Messaging.t Note: the database update now updates the existing installation. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #149223|0 |1 is obsolete| | --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 150405 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150405&action=edit Bug 32357: Set borrower_message_preferences.days_in_advance default to NULL To test: 1. prove t/db_dependent/cronjobs/advance_notices_digest.t 2. prove t/db_dependent/Letters/TemplateToolkit.t 3. prove t/db_dependent/Koha/CurbsidePickups.t 4. prove t/db_dependent/Koha/Patrons.t 5. prove t/db_dependent/Patron/Messaging.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00 |23.05.00,22.11.06 released in| | Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Many hands makes light work, thankyou everyone! Pushed to 22.11.x for the next release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32357 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes the default release notes| |value in the database for | |the 'Days in advance' field | |for patron messaging | |preferences so that it | |defaults to NULL instead of | |0 | |(borrower_message_preferenc | |es table and the | |days_in_advance field). -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org