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
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
17 patches left to sign-off as of today.
13 patches left to sign-off as of today
As of today - 7 patches left to sign-off - 1 signed off
As of today depending on: Need Sign-off: 14954 14946 14960 14969 Signed-off 14985 14965 14998 14956 I switch to "Needs sign-off"
Created attachment 44595 [details] [review] Bug 14870 - Remove C4/Dates.pm from Koha Remove C4/Dates.pm form Koha
Hi Marc I attached the patch for the near future. Regards
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.
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?)
Created attachment 44612 [details] [review] Bug 14870 - Remove t/Dates.t from Koha
Hi Marc. Please help me out with koha.psgi and plack.psgi I don't want to screw up.
(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.
Created attachment 44613 [details] [review] Bug 14870 - Remove C4::Dates from plack.psgi and koha.psgi Remove C4::Dates from .psgi files from preloaded modules, placing Koha::DateUtils instead
(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.
(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
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"
C4/Overdues.pm fixed in followup in Bug 14985
Created attachment 44623 [details] [review] 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.
Created attachment 44625 [details] [review] 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
Created attachment 44626 [details] [review] 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.
Still applies.
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.
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'});
Created attachment 44943 [details] [review] Bug 14969: (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
Created attachment 44944 [details] [review] 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
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 }
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 ?
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
Switching back to NSO
Created attachment 44954 [details] [review] 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)
(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>
Created attachment 44997 [details] [review] [SIGNED OFF] 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>
Created attachment 44998 [details] [review] [SIGNED OFF] 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>
Created attachment 44999 [details] [review] [SIGNED OFF] Bug 14870 - Remove t/Dates.t from Koha Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 45000 [details] [review] [SIGNED OFF] 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>
Created attachment 45001 [details] [review] [SIGNED OFF] 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>
Created attachment 45002 [details] [review] [SIGNED OFF] 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>
Created attachment 45003 [details] [review] [SIGNED OFF] 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>
Created attachment 45004 [details] [review] [SIGNED OFF] 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.
Created attachment 45005 [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>
Created attachment 45006 [details] [review] Bug 14870: QA follow-up - fixing tiny typo
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>
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>
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>
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>
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>
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>
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>
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>
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>
In the words of cait: Die C4::Dates, die! Code all checks out, tests all pass and my testing also showed nothing untoward.. passing!
Patches pushed to master. Thanks Hector and Marc!