[Koha-bugs] [Bug 17502] Add type check to output_pref and use exceptions

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Katrin Fischer  changed:

   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

[bugzilla-daemon]

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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Katrin Fischer  changed:

   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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Tomás Cohen Arazi  changed:

   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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Kyle M Hall  changed:

   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

[bugzilla-daemon]

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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Jonathan Druart  changed:

   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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Jonathan Druart  changed:

   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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Marcel de Rooy  changed:

   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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Marcel de Rooy  changed:

   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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Marcel de Rooy  changed:

   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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Josef Moravec  changed:

   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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Josef Moravec  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
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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Josef Moravec  changed:

   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

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

Marcel de Rooy  changed:

   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/