Bug 12100 - Messaging preferences did not save Days in Advance
Summary: Messaging preferences did not save Days in Advance
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-17 11:30 UTC by Marcel de Rooy
Modified: 2014-12-07 20:07 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12100: Messaging preferences did not save Days in Advance (3.28 KB, patch)
2014-04-17 11:40 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[Signed-off] Bug 12100: Messaging preferences did not save Days in Advance (3.42 KB, patch)
2014-04-21 17:58 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 12100: Messaging preferences did not save Days in Advance (3.38 KB, patch)
2014-04-25 13:46 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2014-04-17 11:30:32 UTC
If you have enhanced messaging preference, the Days in Advance combo value (in Patron Messaging Preferences) is saved in the database but not retrieved when you have not enabled the Email checkbox next to it.

This comes from a JOIN that should be a LEFT JOIN.
Comment 1 Marcel de Rooy 2014-04-17 11:40:42 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2014-04-21 17:58:27 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2014-04-25 13:46:38 UTC
Created attachment 27627 [details] [review]
[PASSED QA] Bug 12100: Messaging preferences did not save Days in Advance

If you have enhanced messaging preference, the Days in Advance combo
value (in Patron Messaging Preferences) is saved in the database but
not retrieved when you have not enabled the Email checkbox next to it.

This patch does the following:
[1] It replaces a JOIN by a LEFT JOIN that is the actual reason of the
    problem described.
[2] Removes a FIXME by saving a hardcoded 30 into a constant.
[3] Fixes a typo in the neighborhood.
[4] Removes a superfluous comma in the map statement.
[5] Simplifies code for the selected field of the days combo. It should
    just be a boolean. The text selected="selected" is in the template.

Test plan:
[1] Enable enhanced messaging preferences.
[2] Fill in Days in advance for Advance notice but uncheck Email.
[3] Save the preferences.
[4] The member home screen does not display the number of days (until you
    decide to apply this patch :)

Followed test plan. Works as expected.
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Galen Charlton 2014-04-28 21:37:03 UTC
Pushed to master, along with a follow-up that fixes a regression on bug 5603 introduced by the main patch.

Thanks, Marcel!