[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Katrin Fischer changed: What|Removed |Added CC||martin.renvoize@ptfs-europe ||.com, ||n...@bywatersolutions.com --- Comment #15 from Katrin Fischer --- Calling for reinforcements to hopefully get another review/opinion. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 --- Comment #14 from Emmi Takkinen --- (In reply to Katrin Fischer from comment #13) > 1) The changes make sense, but I wondered if it wouldn't be safer to change > > sub _set_default_expirationdate { > my $self = shift; > > my $period = C4::Context->preference('DefaultHoldExpirationdatePeriod') > || 0; > my $timeunit = > > ? > > This would mean one change in one spot instead of changing 2. > > We could just "do onthing" if DefaultHoldExpirationdatePeriod is > undefined/empty. That's a good point, I'm just not sure if there's point to trigger sub _set_default_expirationdate at all. I mean, with this current patch we don't proceed to sub _set_default_expirationdate if DefaultHoldExpirationdatePeriod is not set. There's also less code to execute this way. But that's just my hunch :D If someone has any other points to this, please comment. > 2) For the note I suggest a tiny rephrase: > > If href="/cgi-bin/koha/admin/preferences. > pl?op=search=DefaultHoldExpirationdatePeriod">DefaultHoldExpirati > ondatePeriod is left blank default expiration date is not set. > > ..., the default expiration date is not set. > ..., no default expiration date is set. Makes sense. I'll adjust this after we decide what to do with point 1, since if we proceed with that, it would probably be good idea to rewrite these patches. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Katrin Fischer changed: What|Removed |Added QA Contact|katrin.fisc...@bsz-bw.de|testo...@bugs.koha-communit ||y.org -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Katrin Fischer changed: What|Removed |Added Status|Signed Off |Failed QA --- Comment #13 from Katrin Fischer --- 1) The changes make sense, but I wondered if it wouldn't be safer to change sub _set_default_expirationdate { my $self = shift; my $period = C4::Context->preference('DefaultHoldExpirationdatePeriod') || 0; my $timeunit = ? This would mean one change in one spot instead of changing 2. We could just "do onthing" if DefaultHoldExpirationdatePeriod is undefined/empty. 2) For the note I suggest a tiny rephrase: If DefaultHoldExpirationdatePeriod is left blank default expiration date is not set. ..., the default expiration date is not set. ..., no default expiration date is set. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Lucas Gass changed: What|Removed |Added Attachment #157069|0 |1 is obsolete|| --- Comment #12 from Lucas Gass --- Created attachment 157129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157129=edit Bug 29074: Prevent undef warning when DefaultHoldExpirationdate is read Test plan: Repeat test plan from first patch. Also prove t/db_dependent/Hold.t. Sponsored-by: Koha-Suomi Oy Signed-off-by: Lucas Gass -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Lucas Gass changed: What|Removed |Added Attachment #157068|0 |1 is obsolete|| --- Comment #11 from Lucas Gass --- Created attachment 157128 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157128=edit Bug 29074: Do not set default expiration date for hold if DefaultHoldExpirationdatePeriod is left blank If DefaultHoldExpirationdate is on and DefaultHoldExpirationdatePeriod is blank, the blank is treated as a zero and all holds are created with an expiration date of today. This patch changes expiration dates storing so that if DefaultHoldExpirationdatePeriod is left blank, expiration date is not set. Users are informed about this with a note in syspref. To test: 1. Set DefaultHoldExpirationdate on but leave DefaultHoldExpirationdatePeriod blank. 2. Add hold for patron. => Note that expiration date is set as today. 3. Apply this patch => Confirm following note is now added after sysprefs: "NOTE: If DefaultHoldExpirationdatePeriod is left blank default expiration date is not set." 4. Add new hold for patron. => Note that expiration date is not set. Also prove t/db_dependent/Hold.t. Sponsored-by: Koha-Suomi Oy Signed-off-by: stina Signed-off-by: Katrin Fischer Signed-off-by: Lucas Gass -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Lucas Gass changed: What|Removed |Added Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Emmi Takkinen changed: What|Removed |Added Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 --- Comment #10 from Emmi Takkinen --- Created attachment 157069 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157069=edit Bug 29074: Prevent undef warning when DefaultHoldExpirationdate is read Test plan: Repeat test plan from first patch. Also prove t/db_dependent/Hold.t. Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Emmi Takkinen changed: What|Removed |Added Attachment #154813|0 |1 is obsolete|| --- Comment #9 from Emmi Takkinen --- Created attachment 157068 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=157068=edit Bug 29074: Do not set default expiration date for hold if DefaultHoldExpirationdatePeriod is left blank If DefaultHoldExpirationdate is on and DefaultHoldExpirationdatePeriod is blank, the blank is treated as a zero and all holds are created with an expiration date of today. This patch changes expiration dates storing so that if DefaultHoldExpirationdatePeriod is left blank, expiration date is not set. Users are informed about this with a note in syspref. To test: 1. Set DefaultHoldExpirationdate on but leave DefaultHoldExpirationdatePeriod blank. 2. Add hold for patron. => Note that expiration date is set as today. 3. Apply this patch => Confirm following note is now added after sysprefs: "NOTE: If DefaultHoldExpirationdatePeriod is left blank default expiration date is not set." 4. Add new hold for patron. => Note that expiration date is not set. Also prove t/db_dependent/Hold.t. Sponsored-by: Koha-Suomi Oy Signed-off-by: stina Signed-off-by: Katrin Fischer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Tomás Cohen Arazi changed: What|Removed |Added Status|Passed QA |Failed QA CC||tomasco...@gmail.com --- Comment #8 from Tomás Cohen Arazi --- Should the condition be: && defined C4::Context->preference('DefaultHoldExpirationdatePeriod') && C4::Context->preference('DefaultHoldExpirationdatePeriod') ne '' otherwise it will log a warning if it is undef, each time the syspref is read. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Katrin Fischer changed: What|Removed |Added Attachment #154484|0 |1 is obsolete|| --- Comment #7 from Katrin Fischer --- Created attachment 154813 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154813=edit Bug 29074: Do not set default expiration date for hold if DefaultHoldExpirationdatePeriod is left blank If DefaultHoldExpirationdate is on and DefaultHoldExpirationdatePeriod is blank, the blank is treated as a zero and all holds are created with an expiration date of today. This patch changes expiration dates storing so that if DefaultHoldExpirationdatePeriod is left blank, expiration date is not set. Users are informed about this with a note in syspref. To test: 1. Set DefaultHoldExpirationdate on but leave DefaultHoldExpirationdatePeriod blank. 2. Add hold for patron. => Note that expiration date is set as today. 3. Apply this patch => Confirm following note is now added after sysprefs: "NOTE: If DefaultHoldExpirationdatePeriod is left blank default expiration date is not set." 4. Add new hold for patron. => Note that expiration date is not set. Also prove t/db_dependent/Hold.t. Sponsored-by: Koha-Suomi Oy Signed-off-by: stina Signed-off-by: Katrin Fischer -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Katrin Fischer changed: What|Removed |Added Patch complexity|--- |Small patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Katrin Fischer changed: What|Removed |Added QA Contact|testo...@bugs.koha-communit |katrin.fisc...@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Stina Hallin changed: What|Removed |Added Status|Needs Signoff |Signed Off CC||stina.hal...@ub.lu.se -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 ByWater Sandboxes changed: What|Removed |Added Attachment #144572|0 |1 is obsolete|| --- Comment #6 from ByWater Sandboxes --- Created attachment 154484 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154484=edit Bug 29074: Do not set default expiration date for hold if DefaultHoldExpirationdatePeriod is left blank If DefaultHoldExpirationdate is on and DefaultHoldExpirationdatePeriod is blank, the blank is treated as a zero and all holds are created with an expiration date of today. This patch changes expiration dates storing so that if DefaultHoldExpirationdatePeriod is left blank, expiration date is not set. Users are informed about this with a note in syspref. To test: 1. Set DefaultHoldExpirationdate on but leave DefaultHoldExpirationdatePeriod blank. 2. Add hold for patron. => Note that expiration date is set as today. 3. Apply this patch => Confirm following note is now added after sysprefs: "NOTE: If DefaultHoldExpirationdatePeriod is left blank default expiration date is not set." 4. Add new hold for patron. => Note that expiration date is not set. Also prove t/db_dependent/Hold.t. Sponsored-by: Koha-Suomi Oy Signed-off-by: stina -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Emmi Takkinen changed: What|Removed |Added Status|Failed QA |Needs Signoff --- Comment #5 from Emmi Takkinen --- (In reply to Emmi Takkinen from comment #4) > (In reply to Marius from comment #3) > > If I don't put the packet, with "DefaultHoldExpirationdate" on the set and > > with DefaultHoldExpirationdatePeriod blank, the reservation expiration date > > remains blank. > > This is odd, without using this patch if DefaultHoldExpirationdatePeriod is > blank, expiration date is set in database but it's not displayed in Koha. > > And even odder, this happens only if syspref dateformat is set as any other > than -mm-dd. This seems like an whole other bug to me. This happanes because expiration date input has attribute 'data-flatpickr-futuredate="true"'. When this is set, expiration dates set as today or in the past aren't displayed. Which can now happen if "DefaultHoldExpirationdate" is enabled and "DefaultHoldExpirationdatePeriod" is blank. Setting this bug back to "Needs Signoff". -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 --- Comment #4 from Emmi Takkinen --- (In reply to Marius from comment #3) > If I don't put the packet, with "DefaultHoldExpirationdate" on the set and > with DefaultHoldExpirationdatePeriod blank, the reservation expiration date > remains blank. This is odd, without using this patch if DefaultHoldExpirationdatePeriod is blank, expiration date is set in database but it's not displayed in Koha. And even odder, this happens only if syspref dateformat is set as any other than -mm-dd. This seems like an whole other bug to me. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Marius changed: What|Removed |Added Status|Needs Signoff |Failed QA CC||marius.mandre...@inlibro.co ||m --- Comment #3 from Marius --- If I don't put the packet, with "DefaultHoldExpirationdate" on the set and with DefaultHoldExpirationdatePeriod blank, the reservation expiration date remains blank. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Emmi Takkinen changed: What|Removed |Added Status|ASSIGNED|Needs Signoff -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 --- Comment #2 from Emmi Takkinen --- Created attachment 144572 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144572=edit Bug 29074: Do not set default expiration date for hold if DefaultHoldExpirationdatePeriod is left blank If DefaultHoldExpirationdate is on and DefaultHoldExpirationdatePeriod is blank, the blank is treated as a zero and all holds are created with an expiration date of today. This patch changes expiration dates storing so that if DefaultHoldExpirationdatePeriod is left blank, expiration date is not set. Users are informed about this with a note in syspref. To test: 1. Set DefaultHoldExpirationdate on but leave DefaultHoldExpirationdatePeriod blank. 2. Add hold for patron. => Note that expiration date is set as today. 3. Apply this patch => Confirm following note is now added after sysprefs: "NOTE: If DefaultHoldExpirationdatePeriod is left blank default expiration date is not set." 4. Add new hold for patron. => Note that expiration date is not set. Also prove t/db_dependent/Hold.t. Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 29074] DefaultHoldExpirationdatePeriod blank value interpreted as zero
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29074 Emmi Takkinen changed: What|Removed |Added Status|NEW |ASSIGNED CC||emmi.takki...@koha-suomi.fi Assignee|koha-b...@lists.koha-commun |emmi.takki...@koha-suomi.fi |ity.org | --- Comment #1 from Emmi Takkinen --- (In reply to Andrew Fuerste-Henry from comment #0) > If DefaultHoldExpirationdate is on and DefaultHoldExpirationdatePeriod is > blank, the blank is treated as a zero and all holds are created with an > expiration date of today. I'm not sure what the best answer is. We could: > - add some text to the syspref to make clear that a blank is treated as zero > - make it always save and display a zero when you try to save a blank > or > - make it treat a blank as disabled (and then add some text to the syspref > to tell people that) Hmm, I'd say either of two last options could be the way to go. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/