Bug 14998 - Remove C4::Dates from installer and maintenance folders
Summary: Remove C4::Dates from installer and maintenance folders
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:
Blocks: 14870
  Show dependency treegraph
 
Reported: 2015-10-10 09:36 UTC by Marc Véron
Modified: 2016-06-21 21:39 UTC (History)
3 users (show)

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


Attachments
Bug 14998 - Remove C4::Dates from installer and maintenance folders (3.92 KB, patch)
2015-10-10 09:48 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14998 - (followup) Remove file fix_accountlines_date.pl (6.06 KB, patch)
2015-10-27 15:43 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14998 - Remove C4::Dates from installer and maintenance folders (4.12 KB, patch)
2015-11-04 15:23 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14998 - Remove C4::Dates from installer and maintenance folders (4.19 KB, patch)
2015-11-11 12:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14998: Restore previous behavior (1.67 KB, patch)
2015-11-11 12:44 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-10 09:36:57 UTC
This patch removes C4::Dates from files:

- installer/data/mysql/updatedatabase.pl
- misc/maintenance/fix_accountlines_date.pl
Comment 1 Marc Véron 2015-10-10 09:48:39 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-10-23 09:37:54 UTC
Still applies
Comment 3 Marc Véron 2015-10-27 15:43:06 UTC Comment hidden (obsolete)
Comment 4 M. Tompsett 2015-10-28 23:37:50 UTC
(In reply to Marc Véron from comment #3)
> - Verify that the file is no longer used

While it isn't used as far as I can tell. Does a date get placed in description still? If so, this script may still be handy to keep.
Comment 5 Marc Véron 2015-10-29 07:45:14 UTC
(In reply to M. Tompsett from comment #4)
> (In reply to Marc Véron from comment #3)
> > - Verify that the file is no longer used
> 
> While it isn't used as far as I can tell. Does a date get placed in
> description still? If so, this script may still be handy to keep.

In the first patch I removed C4::Dates from 
misc/maintenance/fix_accountlines_date.pl

So it does no harm to keep the file. 

I obsolete the second patch and open a new bug for removing. That can be discussed later. The main thing now bring the C4::Dates removal to an end.
Comment 6 Frédéric Demians 2015-11-04 15:23:35 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2015-11-06 10:55:56 UTC
I don't want to spent too much time on this script, but I don't think it will work as before.

  C4::Dates->new($1, 'us')->output()
and
  output_pref( { dt => dt_from_string( $1 ), dateonly => 1, dateformat => 'us' }
are different I think, it should be
  output_pref( { dt => dt_from_string( $1, 'us' ), dateonly => 1 }

I would prefer to see this script deleted :)
Comment 8 Marc Véron 2015-11-10 15:47:00 UTC
(In reply to Jonathan Druart from comment #7)
> I don't want to spent too much time on this script, but I don't think it
> will work as before.
> 
>   C4::Dates->new($1, 'us')->output()
> and
>   output_pref( { dt => dt_from_string( $1 ), dateonly => 1, dateformat =>
> 'us' }
> are different I think, it should be
>   output_pref( { dt => dt_from_string( $1, 'us' ), dateonly => 1 }
> 
> I would prefer to see this script deleted :)

Delete +1
Comment 9 Marc Véron 2015-11-10 22:01:03 UTC
See Bug 15076 - Remove file misc/maintenance/fix_accountlines_date.pl
Comment 10 Jonathan Druart 2015-11-11 12:43:58 UTC
Created attachment 44745 [details] [review]
Bug 14998 - Remove C4::Dates from installer and maintenance folders

- installer/data/mysql/updatedatabase.pl
- misc/maintenance/fix_accountlines_date.pl

To test for updatedatabase.pl

- perl installer/data/mysql/updatedatabase.pl
- review code. Difficult to test, because the code affects
  $DBversion = '3.03.00.035'

To test for misc/maintenance/fix_accountlines_date.pl
 - review code
 - The file seems to be a file used for an update, could it be obsolete?
   If yes, remove it in a follow-up

Signed-off-by: Frederic Demians <f.demians@tamil.fr>
  Code OK. Tested updatedatabase.pl with an old DB backup: OK.
  fix_accountlines_date.pl still working after the patch. Could be removed from
  master IMO.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2015-11-11 12:44:01 UTC
Created attachment 44746 [details] [review]
Bug 14998: Restore previous behavior

We want to convert a us or metric date formatted to the format defined in
the pref.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2015-11-11 12:44:32 UTC
Continue without waiting for 15076.
Comment 13 Tomás Cohen Arazi 2015-11-11 14:11:44 UTC
Patches pushed to master.

Thanks Marc!