Bug 14870

Summary: Delete C4/Dates.pm from System
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: hector.hecaxmmx, josef.moravec, katrin.fischer, martin.renvoize, tomascohen, ztajoli
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13813
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14060
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15087
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15076
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 14873, 14879, 14890, 14903, 14916, 14918, 14920, 14922, 14923, 14924, 14925, 14926, 14928, 14931, 14946, 14949, 14950, 14951, 14954, 14956, 14960, 14961, 14965, 14969, 14977, 14982, 14985, 14995, 14996, 14997, 14998    
Bug Blocks: 12072    
Attachments: Bug 14870 - Remove C4/Dates.pm from Koha
Bug 14870 - Remove t/Dates.t from Koha
Bug 14870 - Remove C4::Dates from plack.psgi and koha.psgi
Bug 14870: (followup) Remove stray C4::Dates from circ/returns.pl
Bug 14870: (followup) Remove stray C4::Dates from reports files
Bug 14870: (followup) Remove mention of C4::Dates from C4/Debug.pm
Bug 14969: (followup) Remove stray C4::Dates
Bug 14870: (followup) Remove stray C4::Dates
Bug 14870: (followup) Remove superfluous C4::Dates from fix_accountlines_rmdupfines_bug8253.pl
[SIGNED OFF] remove C4/Dates.pm from Koha
[SIGNED OFF] remove C4/Dates.pm from Koha
[SIGNED OFF] Bug 14870 - Remove t/Dates.t from Koha
[SIGNED OFF] Remove C4::Dates from plack.psgi and koha.psgi
[SIGNED OFF] Bug 14870: (followup) Remove stray C4::Dates from circ/returns.pl
[SIGNED OFF] Bug 14870: (followup) Remove stray C4::Dates from reports files
[SIGNED OFF] Bug 14870: (followup) Remove mention of C4::Dates from C4/Debug.pm
[SIGNED OFF] Bug 14870: (followup) Remove stray C4::Dates
[PASSED QA] Bug 14870: (followup) Remove superfluous C4::Dates from fix_accountlines_rmdupfines_bug8253.pl
Bug 14870: QA follow-up - fixing tiny typo
[PASSED QA] Bug 14870: Remove C4/Dates.pm from Koha
[PASSED QA] Bug 14870: Remove t/Dates.t from Koha
[PASSED QA] Bug 14870: Remove C4::Dates from plack.psgi and koha.psgi
[PASSED QA] Bug 14870: (followup) Remove stray C4::Dates from circ/returns.pl
[PASSED QA] Bug 14870: (followup) Remove stray C4::Dates from reports files
[PASSED QA] Bug 14870: (followup) Remove mention of C4::Dates from C4/Debug.pm
[PASSED QA] Bug 14870: (followup) Remove stray C4::Dates
[PASSED QA] Bug 14870: (followup) Remove superfluous C4::Dates from fix_accountlines_rmdupfines_bug8253.pl
[PASSED QA] Bug 14870: QA follow-up - fixing tiny typo

Description Marc Véron 2015-09-22 14:00:36 UTC
Bug to remove file C4/Dates.pm from system.

With Bug 13813 we removed C4::Dates from a lot of files. However, there are many files left before we can delete C4/Dates.pm and resolve issues like Bug 12072

I think we should break down the task of weeding out C4::Dates to several Bugs that block this one.


Files with C4::Dates (extracted from: git grep C4::Dates)

C4/Calendar.pm
C4/Circulation.pm
C4/Debug.pm
C4/Members.pm
C4/Reports/Guided.pm
C4/Reserves.pm
C4/Search.pm
C4/Serials.pm
C4/Utils/DataTables.pm
C4/Utils/DataTables/Members.pm
C4/VirtualShelves/Page.pm
acqui/histsearch.pl
acqui/invoice.pl
acqui/invoices.pl
acqui/orderreceive.pl
acqui/parcel.pl
acqui/parcels.pl
acqui/pdfformat/layout2pages.pm
acqui/pdfformat/layout2pagesde.pm
acqui/pdfformat/layout3pages.pm
acqui/pdfformat/layout3pagesfr.pm
acqui/transferorder.pl
admin/aqbudgets.pl
admin/aqcontract.pl
admin/aqplan.pl
admin/categorie.pl
admin/smart-rules.pl
catalogue/detail.pl
cataloguing/additem.pl
circ/bookcount.pl
circ/circulation.pl
circ/overdue.pl
circ/pendingreserves.pl
circ/reserveratios.pl
circ/returns.pl
debian/templates/plack.psgi
installer/data/mysql/updatedatabase.pli
labels/label-item-search.pl
members/memberentry.pl
members/moremember.pl
members/printfeercpt.pl
misc/cronjobs/batch_anonymise.pl
misc/cronjobs/cleanup_database.pl
misc/cronjobs/gather_print_notices.pl
misc/cronjobs/overdue_notices.pl
misc/cronjobs/serialsUpdate.pl
misc/cronjobs/staticfines.pl
misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl
misc/maintenance/fix_accountlines_date.pl
misc/plack/koha.psgi
opac/opac-alert-subscribe.pl
opac/opac-ics.pl
opac/opac-memberentry.pl
opac/opac-reserve.pl
opac/opac-search-history.pl
opac/opac-serial-issues.pl
opac/opac-showreviews.pl
opac/opac-suggestions.pl
plugins/plugins-home.pl
plugins/run.pl
reports/acquisitions_stats.pl
reports/bor_issues_top.pl
reports/borrowers_out.pl
reports/borrowers_stats.pl
reports/cat_issues_top.pl
reports/dictionary.pl
reports/guided_reports.pl
reports/issues_avg_stats.pl
reports/issues_stats.pl
reports/itemslost.pl
reports/reservereport.pl
reports/reserves_stats.pl
reports/serials_stats.pl
reserve/request.pl
serials/acqui-search-result.pl
serials/checkexpiration.pl
serials/routing-preview.pl
serials/routing.pl
serials/serial-issues.pl
serials/serials-collection.pl
serials/serials-edit.pl
serials/showpredictionpattern.pl
serials/subscription-add.pl
serials/subscription-detail.pl
serials/subscription-history.pl
serials/subscription-renew.pl
t/Dates.t
t/db_dependent/Budgets.t
t/db_dependent/Holds.t
t/db_dependent/Koha_template_plugin_KohaDates.t
t/db_dependent/Log.t
t/db_dependent/NewsChannels.t
tags/review.pl
tools/batchMod.pl
tools/exceptionHolidays.pl
tools/export.pl
tools/holidays.pl
tools/import_borrowers.pl
tools/newHolidays.pl
Comment 1 Marc Véron 2015-10-10 10:05:49 UTC
With all the blocking Bugs pushed (latest is Bug 14998), the only remaining files should be:

To be removed:

C4/Dates.pm
t/Dates.t

To be changed: 

debian/templates/plack.psgi
misc/plack/koha.psgi
Comment 2 Marc Véron 2015-10-23 07:52:56 UTC
17 patches left to sign-off as of today.
Comment 3 Marc Véron 2015-10-25 19:40:50 UTC
13 patches left to sign-off as of today
Comment 4 Marc Véron 2015-10-30 08:21:47 UTC
As of today 
- 7 patches left to sign-off
- 1 signed off
Comment 5 Marc Véron 2015-11-04 22:59:55 UTC
As of today depending on:

Need Sign-off:
14954
14946
14960
14969

Signed-off
14985
14965
14998
14956

I switch to "Needs sign-off"
Comment 6 Héctor Eduardo Castro Avalos 2015-11-06 16:39:41 UTC Comment hidden (obsolete)
Comment 7 Héctor Eduardo Castro Avalos 2015-11-06 16:41:38 UTC
Hi Marc

I attached the patch for the near future.

Regards
Comment 8 Marc Véron 2015-11-07 05:40:57 UTC
Hi Héctor,

Thanks for the patch for removing C4/Dates.pm

With all the patches applied, we can finally use the new date format from Bug 12072

However, before we remove C4/Dates.pm we have to make sure that the file really no longer is used or referenced.

See my comment #1:
> To be removed:
> 
> C4/Dates.pm
> t/Dates.t
> 
> To be changed: 
> 
> debian/templates/plack.psgi
> misc/plack/koha.psgi

Additionally, we should do a final global search for C4::Dates.
Comment 9 Marc Véron 2015-11-07 05:54:22 UTC
As of today, with following patches applied:
14960 (nso)
14969 (nqa)
14985 (nqa)
14954 (nqa)
14946 (nqa)
14998 (nqa)

Rusults from: git grep 'C4::Dates':

Form comment #1:
C4/Dates.pm (remove)
t/Dates.t (remove)

debian/templates/plack.psgi (change to remove C4::Dates)
misc/plack/koha.psgi (change to remove C4::Dates)

Some stray C4::Dates found in:

C4/Debug.pm (follow up)
C4/Overdues.pm (follow up)
C4/Serials.pm (follow up)
circ/returns.pl (follow up)
reports/borrowers_stats.pl (follow up)
reports/guided_reports.pl (follow up)


t/DateUtils.t (OK, it's in a comment only)

misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl (remove? follow up?)
Comment 10 Héctor Eduardo Castro Avalos 2015-11-07 16:57:03 UTC Comment hidden (obsolete)
Comment 11 Héctor Eduardo Castro Avalos 2015-11-07 17:04:05 UTC
Hi Marc.
Please help me out with koha.psgi and plack.psgi I don't want to screw up.
Comment 12 Tomás Cohen Arazi 2015-11-07 17:58:48 UTC
(In reply to Héctor Eduardo Castro Avalos from comment #11)
> Hi Marc.
> Please help me out with koha.psgi and plack.psgi I don't want to screw up.

Just remove it. It is only used for preloading it.
Comment 13 Héctor Eduardo Castro Avalos 2015-11-08 03:50:13 UTC Comment hidden (obsolete)
Comment 14 Héctor Eduardo Castro Avalos 2015-11-08 04:01:15 UTC
(In reply to Tomás Cohen Arazi from comment #12)

> Just remove it. It is only used for preloading it.

Done. But I place instead Koha::DateUtils, correct me if I'm wrong.
Comment 15 Marc Véron 2015-11-08 09:17:45 UTC
(In reply to Marc Véron from comment #9)
(...)

> Some stray C4::Dates found in:
(...)
> C4/Serials.pm (follow up)
(...)
Fixed in follow up to Bug 14969
Comment 16 Marc Véron 2015-11-08 09:21:02 UTC
I will fix some remaining stray C4::Dates in follow-ups and/or a separate bug.

Some stray C4::Dates found in:

C4/Debug.pm (follow up)
C4/Overdues.pm (follow up)
circ/returns.pl (follow up)
reports/borrowers_stats.pl (follow up)
reports/guided_reports.pl (follow up)

t/DateUtils.t (OK, it's in a comment only)

misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl (remove? follow up?)


Switching to "Needs sign-off"
Comment 17 Marc Véron 2015-11-08 21:12:33 UTC
C4/Overdues.pm fixed in followup in Bug 14985
Comment 18 Marc Véron 2015-11-08 21:30:53 UTC Comment hidden (obsolete)
Comment 19 Marc Véron 2015-11-08 22:30:54 UTC Comment hidden (obsolete)
Comment 20 Marc Véron 2015-11-08 22:36:27 UTC Comment hidden (obsolete)
Comment 21 Marc Véron 2015-11-12 16:42:15 UTC
Still applies.
Comment 22 Katrin Fischer 2015-11-17 23:59:56 UTC
It appears a few are missing. I tested this on top of:
14954
14946

I found those references outside the release notes:
C4/Serials.pm:$planneddate is a C4::Dates object.
misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl:use C4::Dates;
serials/acqui-search-result.pl:use C4::Dates qw/format_date/;
t/DateUtils.t:# C4::Dates allowed 00th of the month

2 of those appear to be comments - but please check.
Comment 23 Katrin Fischer 2015-11-18 00:40:40 UTC
Looked a bit further:

1) C4/Serials.pm:$planneddate is a C4::Dates object.
- POD, should be corrected

2) misc/maintenance/fix_accountlines_rmdupfines_bug8253.pl:use C4::Dates;
- Covered by bug 15076. We need to make a decision there.

3) serials/acqui-search-result.pl:use C4::Dates qw/format_date/;
- This one needs to be fixed, has a few date_format.

4) t/DateUtils.t:# C4::Dates allowed 00th of the month
- I think that can probably be ignored

Looking for format_date I find one more ocurrence:

5) C4/Log.pm:        $data->{day} = format_date($data->{timestamp});
- Looks like this was missed.

format_date_in_iso:

6) C4/Items.pm:        $datelastseen = format_date_in_iso($datelastseen);  

7) t/db_dependent/Items/GetItemsForInventory.t:        $datelastseen = format_date_in_iso($datelastseen);

8) tools/koha-news.pl:             #$new->{'expirationdate'}=format_date_in_iso($new->{'expirationdate'});
Comment 24 Marc Véron 2015-11-18 08:22:28 UTC Comment hidden (obsolete)
Comment 25 Marc Véron 2015-11-18 08:30:02 UTC Comment hidden (obsolete)
Comment 26 Zeno Tajoli 2015-11-18 09:44:04 UTC
Hi, I found that in reports/guided_reports.pl C4:Dates is still present:


359 # If the range values are dates
360  if ($fromvalue =~ C4::Dates->regexp('syspref') && $tovalue =~   C4::Dates->regexp('syspref')) {
 361                 $fromvalue = C4::Dates->new($fromvalue)->output("iso");
 362                 $tovalue = C4::Dates->new($tovalue)->output("iso");
 363             }
Comment 27 Zeno Tajoli 2015-11-18 09:46:50 UTC
Hi, I found that in reports/guided_reports.pl C4:Dates is still present.

From lines 359 to 363:

# If the range values are dates
if ($fromvalue =~ C4::Dates->regexp('syspref') && $tovalue =~  C4::Dates->regexp('syspref')) {
    $fromvalue = C4::Dates->new($fromvalue)->output("iso");
    $tovalue = C4::Dates->new($tovalue)->output("iso");
}

A new bug or you fix here ?
Comment 28 Marc Véron 2015-11-18 10:20:29 UTC
Hi Zeno, thakns for testing.

I can not find C4::Dates in reports/guided_reports.pl

Git grep 'C4::Dates' only gives results only in misc/release_notes/release_notes_3_22_0.txt

I applyed the patches after a recent git pull:
HEAD is now at 87e7d55 Bug 15199: t/db_dependent/Review.t should not depend on existing data
Comment 29 Marc Véron 2015-11-18 10:24:24 UTC
Switching back to NSO
Comment 30 Marc Véron 2015-11-18 10:58:27 UTC Comment hidden (obsolete)
Comment 31 Héctor Eduardo Castro Avalos 2015-11-18 16:05:52 UTC
(In reply to Zeno Tajoli from comment #27)
> Hi, I found that in reports/guided_reports.pl C4:Dates is still present.
> 
> From lines 359 to 363:
> 
> # If the range values are dates
> if ($fromvalue =~ C4::Dates->regexp('syspref') && $tovalue =~ 
> C4::Dates->regexp('syspref')) {
>     $fromvalue = C4::Dates->new($fromvalue)->output("iso");
>     $tovalue = C4::Dates->new($tovalue)->output("iso");
> }
> 
> A new bug or you fix here ?

A previous attachment in this bug fixed it up <http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44625&action=diff>
Comment 32 Katrin Fischer 2015-11-19 13:53:28 UTC Comment hidden (obsolete)
Comment 33 Katrin Fischer 2015-11-19 13:54:05 UTC Comment hidden (obsolete)
Comment 34 Katrin Fischer 2015-11-19 13:54:09 UTC Comment hidden (obsolete)
Comment 35 Katrin Fischer 2015-11-19 13:54:12 UTC Comment hidden (obsolete)
Comment 36 Katrin Fischer 2015-11-19 13:54:16 UTC Comment hidden (obsolete)
Comment 37 Katrin Fischer 2015-11-19 13:54:20 UTC Comment hidden (obsolete)
Comment 38 Katrin Fischer 2015-11-19 13:54:23 UTC Comment hidden (obsolete)
Comment 39 Katrin Fischer 2015-11-19 13:54:27 UTC Comment hidden (obsolete)
Comment 40 Katrin Fischer 2015-11-19 13:54:30 UTC Comment hidden (obsolete)
Comment 41 Katrin Fischer 2015-11-19 13:54:34 UTC Comment hidden (obsolete)
Comment 42 Martin Renvoize 2015-11-19 14:01:45 UTC
Created attachment 45007 [details] [review]
[PASSED QA] Bug 14870: Remove C4/Dates.pm from Koha

http://bugs.koha-community.org/show_bug.cgi?id=14870
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 43 Martin Renvoize 2015-11-19 14:01:48 UTC
Created attachment 45008 [details] [review]
[PASSED QA] Bug 14870: Remove t/Dates.t from Koha

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Martin Renvoize 2015-11-19 14:01:52 UTC
Created attachment 45009 [details] [review]
[PASSED QA] Bug 14870: Remove C4::Dates from plack.psgi and koha.psgi

Remove C4::Dates from .psgi files from preloaded modules, placing
Koha::DateUtils instead

http://bugs.koha-community.org/show_bug.cgi?id=14870
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Martin Renvoize 2015-11-19 14:01:56 UTC
Created attachment 45010 [details] [review]
[PASSED QA] Bug 14870: (followup) Remove stray C4::Dates from circ/returns.pl

This patch removes 1 remaining / stray occurence of C4::Dates from
circ/returns.pl

To test:
- Apply patch
- Go to Home > Circulation > Check in
- Verfiy that check-ins with overriding return dates work as before.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize 2015-11-19 14:01:59 UTC
Created attachment 45011 [details] [review]
[PASSED QA] Bug 14870: (followup) Remove stray C4::Dates from reports files

This patch removes stray C4::Dates from following files:

reports/borrowers_stats.pl
To test: Verify that C4::Dates is not used in the file

reports/guided_reports.pl:
To test:
- Run a saved report with date input and with date range input, e.g.
- Verify that you are asked for date range or date and that the results
  are the same as without patch

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2015-11-19 14:02:02 UTC
Created attachment 45012 [details] [review]
[PASSED QA] Bug 14870: (followup) Remove mention of C4::Dates from C4/Debug.pm

C4::Dates is mentioned as example in C4/Debug.pm. This patch removes it.
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 48 Martin Renvoize 2015-11-19 14:02:06 UTC
Created attachment 45013 [details] [review]
[PASSED QA] Bug 14870: (followup) Remove stray C4::Dates

(See comment #23)

This patch removes stray C4::Dates (date_fromat...) from
C4/Items.pm
C4/Log.pm
C4/Serials.pm
serials/acqui-search-result.pl
t/DateUtils.t
t/db_dependent/Items/GetItemsForInventory.t
tools/koha-news.pl

Some of them were inside comments etc.

To test:
- git grep 'C4::Dates' should give no result
- git grep 'format_d' should give no result
  Exception: in one cron job there exists an own sub format_date, and occurences not
             related to C4::Dates
- Search for regressions

http://bugs.koha-community.org/show_bug.cgi?id=14870
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
serials/acqui-search-results.pl looks like it should be revisited,
containing code that might not be needed. Searching a vendor
in serials still works witout a problem.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 49 Martin Renvoize 2015-11-19 14:02:09 UTC
Created attachment 45014 [details] [review]
[PASSED QA] Bug 14870: (followup) Remove superfluous C4::Dates from fix_accountlines_rmdupfines_bug8253.pl

This patch removes C4::Dates from fix_accountlines_rmdupfines_bug8253.pl

To test:
- Apply patch
- Check code for uses of C4::Dates (date_format)

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 50 Martin Renvoize 2015-11-19 14:02:13 UTC
Created attachment 45015 [details] [review]
[PASSED QA] Bug 14870: QA follow-up - fixing tiny typo

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 51 Martin Renvoize 2015-11-19 14:07:05 UTC
In the words of cait: Die C4::Dates, die!

Code all checks out, tests all pass and my testing also showed nothing untoward.. passing!
Comment 52 Tomás Cohen Arazi 2015-11-19 23:18:12 UTC
Patches pushed to master.

Thanks Hector and Marc!