Bug 14985

Summary: Remove C4::Dates from 6 files in folder C4/*.pm
Product: Koha Reporter: Marc Véron <veron>
Component: Architecture, internals, and plumbingAssignee: Marc Véron <veron>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: brendan, jonathan.druart, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 15166    
Bug Blocks: 14870    
Attachments: Remove C4::Dates from files in folder C4/*.pm (part one)
Remove C4::Dates from files in folder C4/*.pm (part one)
Bug 14985: (followup) Remove eval if dates come from database
Bug 14985: (followup) Remove C4::Dates from C4/Overdues.pm
Remove C4::Dates from files in folder C4/*.pm (part one)
Bug 14985: (followup) Remove eval if dates come from database
Bug 14985: (followup) Remove C4::Dates from C4/Overdues.pm
Bug 14985: Remove C4::Dates from files in folder C4/*.pm (part one)
Bug 14985: Fix regression on patron search
Bug 14985: Simplify the date management in AddReserve
zsh:1: command not found: q
Bug 14985: dbms expects a iso formatted date

Description Marc Véron 2015-10-08 20:59:50 UTC
This patch removes C4::Dates from following files in folder C4:

- C4/Members.pm
- C4/Reserves.pm
- C4/Search.pm
- C4/Utils/DataTables.pm
- C4/Utils/DataTables/Members.pm
- C4/VirtualShelves/Page.pm
Comment 1 Marc Véron 2015-10-08 21:05:33 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-10-23 09:33:20 UTC
Still applies
Comment 3 Marc Véron 2015-10-29 14:10:58 UTC
Still applies
Comment 4 Marc Véron 2015-11-04 22:09:48 UTC
Hmm, signed-off but no signed-off patch attached?
Comment 5 Brendan Gallagher 2015-11-04 22:26:49 UTC
Marc - my fault - got distracted half way through the sign-off - I will send a patch up in a minute
Comment 6 Brendan Gallagher 2015-11-04 22:29:06 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2015-11-06 11:00:17 UTC
Marc, we are using eval compulsively.
It should only be used for user inputs, not when the data comes from the DB.
Comment 8 Marc Véron 2015-11-06 11:14:24 UTC
(In reply to Jonathan Druart from comment #7)
> Marc, we are using eval compulsively.
> It should only be used for user inputs, not when the data comes from the DB.

Jonathan, maybe I was to anxious / wanted to be on the "save side".
Does it harm functionality and/or does it slow down to much everything?
Comment 9 Jonathan Druart 2015-11-06 11:28:30 UTC
(In reply to Marc Véron from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > Marc, we are using eval compulsively.
> > It should only be used for user inputs, not when the data comes from the DB.
> 
> Jonathan, maybe I was to anxious / wanted to be on the "save side".
> Does it harm functionality and/or does it slow down to much everything?

It could hide future bugs
Comment 10 Marc Véron 2015-11-06 13:23:47 UTC Comment hidden (obsolete)
Comment 11 Marc Véron 2015-11-08 19:39:30 UTC Comment hidden (obsolete)
Comment 12 Marc Véron 2015-11-10 13:30:23 UTC Comment hidden (obsolete)
Comment 13 Marc Véron 2015-11-10 13:32:09 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2015-11-10 13:32:56 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2015-11-11 12:08:33 UTC
Created attachment 44739 [details] [review]
Bug 14985: Remove C4::Dates from files in folder C4/*.pm (part one)

This patch removes C4::Dates from following files in folder C4:

- C4/Members.pm
- C4/Reserves.pm
- C4/Search.pm
- C4/Utils/DataTables.pm
- C4/Utils/DataTables/Members.pm
- C4/VirtualShelves/Page.pm

To test:
 -run tests as appropriate,
- have a close look at the code changes
- try to find regressions

http://bugs.koha-community.org/show_bug.cgi?id=14985

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Bug 14985: (followup) Remove eval if dates come from database

This patch removes some evals from date-formatting where the dates come
from the database.

See comments #7 - #9

Additionaly, C4/VirtualShelves/Page.pm is removed from the patches (obsolete).

Bug 14985: (followup) Remove C4::Dates from C4/Overdues.pm

Ths patch removes a stray C4::Dates from C4/Overdues.pm

- To test got to a patron who has overdues
  (Home > Circulation > Checkouts > [Patron])
- Print overdues

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Jonathan Druart 2015-11-11 12:08:38 UTC
Created attachment 44740 [details] [review]
Bug 14985: Fix regression on patron search

This patch fixes 2 major errors introduced by last patch:
1/ Undefined subroutine &C4::Utils::DataTables::Members::dt_from_string
called
when searching for patrons
2/ Display the dates on the patron search in the pref format, not iso

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2015-11-11 12:08:41 UTC
Created attachment 44741 [details] [review]
Bug 14985: Simplify the date management in AddReserve

The code is much more readable using the str param of output_pref

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Jonathan Druart 2015-11-11 12:09:31 UTC
Marc, I have squashed your 3 patches to make the log more readable. There were too many changes for the same lines.
Comment 19 Tomás Cohen Arazi 2015-11-11 14:07:45 UTC
Please, check this patchset breaks:

  $ prove t/db_dependent/Holds.t
Comment 20 Jonathan Druart 2015-11-11 14:27:06 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2015-11-11 14:27:41 UTC
I missed this one!
Comment 22 Jonathan Druart 2015-11-11 14:27:52 UTC
Created attachment 44765 [details] [review]
Bug 14985: dbms expects a iso formatted date
Comment 23 Tomás Cohen Arazi 2015-11-11 18:08:59 UTC
It fails now, on a different way but it fails.
Comment 24 Marc Véron 2015-11-11 21:04:15 UTC
Test passes OK with patch applied on top of Bug 15166
Comment 25 Tomás Cohen Arazi 2015-11-13 14:34:23 UTC
Patches pushed to master.

Thanks Marc and Jonathan!