[Koha-bugs] [Bug 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Katrin Fischerchanged: What|Removed |Added Resolution|--- |FIXED Status|Pushed to Master|RESOLVED --- Comment #37 from Katrin Fischer --- Thx Tomas! This won't get ported back to 16.11.x as it is an enhancement. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #36 from Tomás Cohen Arazi--- (In reply to Katrin Fischer from comment #35) > Hm, expecting explosions... should this be in stable? I don't think so! Unless it fixes something, but it is too early to say it doesn't break things. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Katrin Fischerchanged: What|Removed |Added CC||katrin.fisc...@bsz-bw.de --- Comment #35 from Katrin Fischer --- Hm, expecting explosions... should this be in stable? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Tomás Cohen Arazichanged: What|Removed |Added Blocks||18473 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18473 [Bug 18473] Search broken due to bug in dates handling -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Kyle M Hallchanged: What|Removed |Added CC||k...@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #34 from Kyle M Hall --- Pushed to master for 17.05, thanks Marcel, Jonathan! -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #33 from Jonathan Druart--- I am expecting explosions with these patches but that will help to discover hidden bugs. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Jonathan Druartchanged: What|Removed |Added Attachment #62168|0 |1 is obsolete|| Attachment #62169|0 |1 is obsolete|| --- Comment #30 from Jonathan Druart --- Created attachment 62408 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62408=edit Bug 17502: Add type check to output_pref This patch makes the following changes: [1] In Koha/DateUtils.pm, sub output_pref: Add a test if $dt is really a DateTime before calling method ymd. Preventing a crash like: Can't locate object method "ymd" via package "dateonly". See also BZ 17502/15822. [2] Adds a few unit tests in t/DateUtils.t. Test plan: [1] Run the adjusted unit test t/DateUtils.t Signed-off-by: Marcel de Rooy Signed-off-by: Josef Moravec Signed-off-by: Jonathan Druart -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Jonathan Druartchanged: What|Removed |Added 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 http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #31 from Jonathan Druart--- Created attachment 62409 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62409=edit Bug 17502: Throw some exceptions in output_pref Test plan: Run the adjusted t/DateUtils.t Signed-off-by: Josef Moravec NOTE: This patch is amended in QA. The exceptions are moved from a separate module to the general section of Exceptions.pm. Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #32 from Jonathan Druart--- Created attachment 62410 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62410=edit Bug 17502: Add info when throwing the exception Signed-off-by: Jonathan Druart -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Marcel de Rooychanged: What|Removed |Added Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Marcel de Rooychanged: What|Removed |Added Attachment #61518|0 |1 is obsolete|| Attachment #61519|0 |1 is obsolete|| --- Comment #28 from Marcel de Rooy --- Created attachment 62168 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62168=edit Bug 17502: Add type check to output_pref This patch makes the following changes: [1] In Koha/DateUtils.pm, sub output_pref: Add a test if $dt is really a DateTime before calling method ymd. Preventing a crash like: Can't locate object method "ymd" via package "dateonly". See also BZ 17502/15822. [2] Adds a few unit tests in t/DateUtils.t. Test plan: [1] Run the adjusted unit test t/DateUtils.t Signed-off-by: Marcel de Rooy Signed-off-by: Josef Moravec -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #29 from Marcel de Rooy--- Created attachment 62169 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62169=edit Bug 17502: Throw some exceptions in output_pref Test plan: Run the adjusted t/DateUtils.t Signed-off-by: Josef Moravec NOTE: This patch is amended in QA. The exceptions are moved from a separate module to the general section of Exceptions.pm. Signed-off-by: Marcel de Rooy -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #27 from Tomás Cohen Arazi--- (In reply to Marcel de Rooy from comment #25) > (In reply to Jonathan Druart from comment #23) > > The classes of the exceptions should not be named with the module they are > > raised from. > > Hmm. Just following what we already did (and approved). > Like: > > package Koha::Patron::Modification; > use Koha::Exceptions::Patron::Modification; > Koha::Exceptions::Patron::Modification::DuplicateVerificationToken->throw > === > package Koha::Exceptions::Patron::Modification; > > 'Koha::Exceptions::Patron::Modification::DuplicateVerificationToken' => { > isa => 'Koha::Exceptions::Patron::Modification', > description => "The verification token given already exists" > }, > === > > Please explain. > Should we formulate a coding guideline here? We might need to. I think the thing with the exceptions you add is: - There's no need to have them tied to output_pref, they could jus tbe top-level for the DateUtils namespace. Remember when you catch them, you have the context in which it happened and can build the needed actions. - The ones you add should be general exceptions: WrongParameters, MissingParameters, InvalidParamType. Keep in mind that you can use them like this: Koha::Exceptions::InvalidParamType->throw('DateTime object expected'); Koha::Exceptions::WrongParameters->throw('Conflicting parameters dt and str passed'); instead of baking a too specific exception. Regarding the Patron modification ones, I found that patron modification tokens were too specific, and they deserved its own exception. But it might be subject to future changes if someone raises it. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Marcel de Rooychanged: What|Removed |Added Status|Signed Off |Failed QA --- Comment #26 from Marcel de Rooy --- (In reply to Jonathan Druart from comment #24) > Moreover I do not know if it is very safe to explode if output_pref is > called without a defined variable. > I can easily imagine some places where output_pref($var) is called with $var > undefined. Agreed. Changing status. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #25 from Marcel de Rooy--- (In reply to Jonathan Druart from comment #23) > The classes of the exceptions should not be named with the module they are > raised from. Hmm. Just following what we already did (and approved). Like: package Koha::Patron::Modification; use Koha::Exceptions::Patron::Modification; Koha::Exceptions::Patron::Modification::DuplicateVerificationToken->throw === package Koha::Exceptions::Patron::Modification; 'Koha::Exceptions::Patron::Modification::DuplicateVerificationToken' => { isa => 'Koha::Exceptions::Patron::Modification', description => "The verification token given already exists" }, === Please explain. Should we formulate a coding guideline here? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #24 from Jonathan Druart--- Moreover I do not know if it is very safe to explode if output_pref is called without a defined variable. I can easily imagine some places where output_pref($var) is called with $var undefined. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #23 from Jonathan Druart--- Hi Marcel, The classes of the exceptions should not be named with the module they are raised from. Also I do not think we need 1 class per exception. If you want to raise an exception because the API has not been used correctly I guess we need (or have already?) a generic exception for it. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #22 from Marcel de Rooy--- (In reply to Josef Moravec from comment #21) > Signed-off-by: Josef Moravec Thanks Josef -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Josef Moravecchanged: What|Removed |Added Attachment #61510|0 |1 is obsolete|| --- Comment #21 from Josef Moravec --- Created attachment 61519 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61519=edit [SIGNED-OFF] Bug 17502: Throw some exceptions in output_pref This patch adds Koha/Exceptions/DateUtils.pm. It contains now 4 exceptions for output_pref: [1] default general exception, [2] conflicting parameters (dt and str), [3] invalid date string, [4] no date time object. Test plan: Run the adjusted t/DateUtils.t Signed-off-by: Josef Moravec -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Josef Moravecchanged: 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 http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Josef Moravecchanged: What|Removed |Added Attachment #61458|0 |1 is obsolete|| --- Comment #20 from Josef Moravec --- Created attachment 61518 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61518=edit [SIGNED-OFF] Bug 17502: Add type check to output_pref This patch makes the following changes: [1] In Koha/DateUtils.pm, sub output_pref: Add a test if $dt is really a DateTime before calling method ymd. Preventing a crash like: Can't locate object method "ymd" via package "dateonly". See also BZ 17502/15822. [2] Adds a few unit tests in t/DateUtils.t. Test plan: [1] Run the adjusted unit test t/DateUtils.t Signed-off-by: Josef Moravec -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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 17502] Add type check to output_pref and use exceptions
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 Marcel de Rooychanged: What|Removed |Added Summary|Add type check to |Add type check to |output_pref |output_pref and use ||exceptions -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://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/