Bug 14985 - Remove C4::Dates from 6 files in folder C4/*.pm
Summary: Remove C4::Dates from 6 files in folder C4/*.pm
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marc Véron
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 15166
Blocks: 14870
  Show dependency treegraph
 
Reported: 2015-10-08 20:59 UTC by Marc Véron
Modified: 2016-06-21 21:39 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Remove C4::Dates from files in folder C4/*.pm (part one) (9.79 KB, patch)
2015-10-08 21:05 UTC, Marc Véron
Details | Diff | Splinter Review
Remove C4::Dates from files in folder C4/*.pm (part one) (9.85 KB, patch)
2015-11-04 22:29 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 14985: (followup) Remove eval if dates come from database (3.42 KB, patch)
2015-11-06 13:23 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14985: (followup) Remove C4::Dates from C4/Overdues.pm (1.13 KB, patch)
2015-11-08 19:39 UTC, Marc Véron
Details | Diff | Splinter Review
Remove C4::Dates from files in folder C4/*.pm (part one) (8.48 KB, patch)
2015-11-10 13:30 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14985: (followup) Remove eval if dates come from database (3.42 KB, patch)
2015-11-10 13:32 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14985: (followup) Remove C4::Dates from C4/Overdues.pm (1.13 KB, patch)
2015-11-10 13:32 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14985: Remove C4::Dates from files in folder C4/*.pm (part one) (9.73 KB, patch)
2015-11-11 12:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14985: Fix regression on patron search (1.65 KB, patch)
2015-11-11 12:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14985: Simplify the date management in AddReserve (2.01 KB, patch)
2015-11-11 12:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
zsh:1: command not found: q (1.74 KB, patch)
2015-11-11 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14985: dbms expects a iso formatted date (1.74 KB, patch)
2015-11-11 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!