The use of C4::Dates module is deprecated since June 2014. It has been superseded by Koha::DateUtils. See: http://wiki.koha-community.org/wiki/Coding_Guidelines Some functionality should be moved to Koha::DateUtils and it should be removed step by step from the system.
Created attachment 36827 [details] [review] Bug 13813 - Remove deprecated C4::Dates from opac/opac-showreviews.pl To test: This patch removes C4::Dates from opac/opac-showreviews.pl and treats the RSS time stamps the same way as in opac/opac-search.pl To test: - Enable display of 'Recent comments' in OPAC (Syspref OpacShowRecentComments) - In OPAC, go to Home > Recent comments - Test RSS feed - Apply patch - Test RSS feed again, it should work as before
Note: Patch "Bug 13813 - Remove deprecated C4::Dates from opac/opac-showreviews.pl" does not depend from Bug 13601
Created attachment 36828 [details] [review] Bug 13813 - Remove C4::Dates from members/readingrec.pl This patch removes depraceted C4::Dates from members/readingrec.pl To test: Apply patch Go to a patron's cirulation history Verify that circulation history behaves as before.
Created attachment 36829 [details] [review] Bug 13813 - Remove C4::Dates from C4/Items.pm This patch removes depraceted C4::Dates from C4/Items.pm To test: Apply patch To test: Add modify items. Checkout added item. Verify that item handling works as before.
Created attachment 36830 [details] [review] Bug 13813 - Remove C4::Dates from overdue_notices.pl This patch removes depraceted C4::Dates from misc/cronjobs/overdue_notices.pl To test: Apply patch Run misc/cronjobs/overdue_notices.pl Verify that dates appear in notices as before (following syspref 'dateformat')
Created attachment 36831 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm This patch removes depraceted C4::Dates from C4/Acquisition.pm To test: Apply patch Go to Home > Acquisitions Verify that receiving orders works as before (Receiving orders only affected by this patch)
Created attachment 36832 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from C4/HoldsQueue.pm To test: Apply patch Make sure that C4::Dates is not used in C4/HoldsQueue.pm
Created attachment 36835 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from Koha/Borrower/Files.pm To test: Apply patch Make sure that uploading arbitrary files for a patron works as before (Set syspref EnableBorrowerFiles for testing )
Created attachment 36836 [details] [review] Bug 13813 - Remove C4::Dates from PDF templates in acquisitions To test: Apply patch In Acquisitions, go to a vendor's "Basket grouping" Find a basket and print order Change PDF templates using syspref OrderPdfFormat and test for - English 2-page - English 3-page - French 3-page - German 2-page
Created attachment 36839 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from Koha/Misc/Files.pm To test: Apply patch Make sure that C4::Dates is not used in the code Set syspref AcqEnableFiles to enable Go to Home > Acquisitions > Invoices Find an invoice, go to Details, Manage Invoice files, make sure that uploading / displaying files works as before
Created attachment 36845 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from tags/list.pl To test: Apply patch Verify in code that C4::Dates is not necessary in tags/list.pl Set sysprefs to TagsEnabled and TagsModeration Got to Home > Tools > Tags In table of tags, click on a Term You should get a page Home > Tools > Tags > Results for [Term]
Note: Previous patches are independent from each other. More patches to come will need changes to Koha/DateUtils.pm (reason for dependency form Bug 13601
Created attachment 36850 [details] [review] Bug 13813 - Remove C4::Dates from C4/Members.pm This patch removes C4::Dates from C4/Members.pm Note: This patch can be tested independently from other patches in this Bug, but it depends on patches from Bug 13601 To test: Make sure that patches from 13601 are applied Apply patch Check changes in code Make sure that following functionality works as before: - Add a patron - Renew patrons - Go to checkout page and create message for patron
Created attachment 36851 [details] [review] Bug 13813 - Remove C4::Dates from members/readingrec.pl This patch removes depraceted C4::Dates from members/readingrec.pl To test: Apply patch Go to a patron's cirulation history Verify that circulation history behaves as before.
Created attachment 36855 [details] [review] Bug 13813 - Remove C4::Dates from members/readingrec.pl This patch removes depraceted C4::Dates from members/readingrec.pl To test: Apply patch Go to a patron's cirulation history Verify that circulation history behaves as before.
Created attachment 36857 [details] [review] Bug 13813 - Remove C4::Dates from C4/Items.pm This patch removes depraceted C4::Dates from C4/Items.pm To test: Apply patch To test: Add modify items. Checkout added item. Verify that item handling works as before.
Created attachment 36858 [details] [review] Bug 13813 - Remove C4::Dates from C4/Items.pm This patch removes depraceted C4::Dates from C4/Items.pm To test: Apply patch To test: Add modify items. Checkout added item. Verify that item handling works as before.
Created attachment 36859 [details] [review] Bug 13813 - Remove C4::Dates from C4/Members.pm This patch removes C4::Dates from C4/Members.pm Note: This patch can be tested independently from other patches in this Bug, but it depends on patches from Bug 13601 To test: Make sure that patches from 13601 are applied Apply patch Check changes in code Make sure that following functionality works as before: - Add a patron - Renew patrons - Go to checkout page and create message for patron
Created attachment 36862 [details] [review] Bug 13813 - Remove C4::Dates from C4/Barcodes/annual.pm To test: Set syspref autoBarcode to <year>-001, <year>-002 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate.
Created attachment 36863 [details] [review] Bug 13813 - Remove C4::Dates from C4/Barcodes/hbyymmincr.pm To test: Set syspref autoBarcode to <branchcode>yymm0001 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate.
Created attachment 36864 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from C4/Barcodes.pm To test: Apply patch Verify that C4::Dates is not used in code.
Created attachment 36865 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from C4/Biblio.pm To test: Apply patch Verify that C4::Dates is not used in code.
Created attachment 36870 [details] [review] Bug 13813 - Remove C4::Dates from C4/Log.pm To test: Apply patch Go to Home > Tools > Logs Search logs with different from- and to-dates, verify that date range displays as appropriate.
Created attachment 36871 [details] [review] Bug 13813 - Remove C4::Dates from C4/NewsChannels.pm To test: Apply patch Create some news for OPAC and staff client (Home > Tools > News) Verify that dates appear as appropriate Verify that Publicaten Date (Creation Date) and Expiration Date work as expected. Note: Inconsistency in wording for creation date / publication date should be managed in separate Bug 13834
Created attachment 36872 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from C4/Output.pm To test: Apply patch Verify that C4::Dates is not used in code
Created attachment 36878 [details] [review] Bug 13813 - Remove C4::Dates from C4/Suggestions.pm To test: Apply patch Go to Home > Acquisitions > Suggestions management Verify in "Filter by" that filter works for Suggestion information / dates
Created attachment 36879 [details] [review] Bug 13813 - Remove C4::Dates from C4/VirtualShelves/Page.pm To test: Apply patch In staff client, go to Home > Lists Verify that list of lists displays as before In OPAC, go to Lists Verify that Public and Private Lists display as before Note: 'dateadded' is not displayed in lists of lists.
Created attachment 36880 [details] [review] Bug 13813 - Remove C4::Dates from members/printinvoice.pl To test: Apply patch Go to a patrons Account In table of fines / transactions, column 'Print', click on 'Print' of a transaction Verify that the date displays correctly on the invoice.
Created attachment 36881 [details] [review] Bug 13813 - Remove C4::Dates from members/printfeercpt.pl To test: Apply patch Go to a patrons Account In table of fines / transactions, column 'Print', click on 'Print' of a payment transaction ("Payment, thanks") Verify that the date displays correctly on the receipt.
Created attachment 36882 [details] [review] Bug 13813 - Remove C4::Dates from circ/bookcount.pl To test: Apply patch Search a biblio with items that are or have been checked out Go to "Items" (moredetail.pl) Locate "Total checkouts" of an item and click 'View item's checkout hsitory' Verify date and time in column "Last seen"
Created attachment 36883 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from members/notices.pl To test: Apply patch Verify that C4::Dates is not used in members/notices.pl
Created attachment 36884 [details] [review] Bug 13813 - Remove C4::Dates from members/guarantor_search.pl To test: Edit a patron's detail Add or modify guarantor information Do a search in popup "Search for guarantor" Verify that dates ob birth display correctly in list
Created attachment 36885 [details] [review] Bug 13813 - Remove C4::Dates from members/moremember.pl To test: Apply patch Go to a patron's Details page (moremember.pl) Verify that dates display correctly (birth date, registration, espiration, manual restriction with date)
Created attachment 36886 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-messaging.pl To test: Apply patch Verify that C4::Dates is not used in code
Created attachment 36887 [details] [review] Remove superfluous use 'C4::Dates' from opac/opac-privacy.pl To test: Apply patch Verify that C4::Dates is not used in code
Created attachment 36888 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-ratings.pl To test: Apply patch Verify that C4::Dates is not used in code
Created attachment 36889 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-search-history.pl To test: Apply patch Verify that C4::Dates is not used in code Enable display of reading history in OPAC (syspref opacreadinghistory) Diesplay reading history in OPAC, should behave as before
Created attachment 36890 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-search-history.pl To test: Apply patch Verify that C4::Dates is not used in code Enable display of search history in OPAC (syspref EnableOpacSearchHistory) Diesplay search history in OPAC, should behave as before
Created attachment 36892 [details] [review] Bug 13813 - Remove C4::Dates from opac/opac-user.pl To test: Apply patch Manually restrict a patron (with end date) Check in to OPAC with this patron's username/password Verify that message about restrictions displays restriction date correctly in tab "Your summary" (opac-user.pl)
Created attachment 36893 [details] [review] Bug 13813 - Remove C4::Dates from patroncards/members-search.pl To test: Apply patch In staffclient go to Home > Tools > Patron card creator Create new batch or edit existing one Click button "Add Item(s)and perform a search in pop-up "Patron search" (patroncards/members-search.pl) Verify that dates in column "Expires on" display correctly.
Created attachment 36894 [details] [review] Bug 13813 - Remove C4::Dates from reserve/request.pl To test: Apply patch Search a biblio in staff client and place hold. This will take you to reserve/request.pl Add / Suspend / Unsuspend / hold(s) Verify that all dates behave and display OK
Created attachment 36895 [details] [review] Bug 13813 - Remove C4::Dates from circ/waitingreserves.pl To test: Go to Home > Circulation > Holds awaiting pickup (Circulation Rports - Holds awaiting pickup) Verify that date in page title appears correcty
Created attachment 36896 [details] [review] Bug 13813 - Remove C4::Dates from circ/circ/transferstoreceive.pl To test: Go to Home > Circulation > Transfers to your library (Circulation Reports - Transfers to receive) Verify that date in page title appears correcty w
Created attachment 36897 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/barcode.pl To test: Set syspref autoBarcode to <year>-001, <year>-002 and <branchcode>yymm001 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate.
Created attachment 36898 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/barcode_manual.pl To test: Apply patch Set the plugin for 952$p (or UNIMARC equivalent to items.barcode) for one of your frameworks to barcode_manual.pl instead of barcode.pl Set syspref autoBarcode to <year>-001, <year>-002 and <branchcode>yymm001 Edit items or add item, verify that barcodes are filled in as appropriate by clicking on the value builder link (...) (Note: Description of barcode_manual.pl in Bug 8524)
Created attachment 36899 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/dateaccessioned.pl To test: Apply patch Edit an item Verifiy that autofilling date in d - Date acquired works as before.
Created attachment 36900 [details] [review] Bug 13813 - Remove C4::Dates from tools/koha-news.pl To test: Apply patch Go to Home > Tools > News Verify that dates appear as expected in date columns Create and edit news, verify dates behave as expected.
Created attachment 36901 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-search-history.pl To test: Apply patch Verify that C4::Dates is not used in code Go to ome > Tools > Logs Display logs with from and to dates Verify that search range behaves as expected. ' (Note: In table 'XXXX lines found', date and time always appear as timestamp as before)
Created attachment 36902 [details] [review] Bug 13813 - Remove C4::Dates from tools/scheduler.pl To test: Apply patch Go to Home > Tools > Task scheduler Add new tasks Verify that tasks are properly added
So far, deprecated C4::Date is removed from 40+ files I would appreciate if somebody could start signing off. I added a patch and a test plan for every file. Most of the changes and tests ar rather trivial. Please be aware that for parts of the patches depend on Bug 13601. There are still about 90 files left with C4::Date - it still will be al long way until we can have more international date formats (like dd.mm.yyyy - blocked Bug 12072).
Comment on attachment 36884 [details] [review] Bug 13813 - Remove C4::Dates from members/guarantor_search.pl Obsoleted because no longer necessary.
Created attachment 37227 [details] [review] Bug 13813 - Remove C4::Dates from reserve/request.pl To test: Apply patch Search a biblio in staff client and place hold. This will take you to reserve/request.pl Add / Suspend / Unsuspend / hold(s) Verify that all dates behave and display OK Retested after 3-way-merge. / MV
Comment on attachment 37227 [details] [review] Bug 13813 - Remove C4::Dates from reserve/request.pl Obsoleted because of merge conflicts. Will add new patch later. /MV
Comment on attachment 36830 [details] [review] Bug 13813 - Remove C4::Dates from overdue_notices.pl Obsoleted because not sure if three-was-merge will break things.
Minor since I suppose it existed before: FAIL cataloguing/value_builder/dateaccessioned.pl FAIL forbidden patterns forbidden pattern: tab char (line 54) FAIL members/moremember.pl FAIL forbidden patterns forbidden pattern: tab char (line 136) FAIL C4/Barcodes/hbyymmincr.pm FAIL forbidden patterns forbidden pattern: tab char (line 76) forbidden pattern: tab char (line 106) FAIL C4/Barcodes/annual.pm FAIL forbidden patterns forbidden pattern: tab char (line 84) forbidden pattern: tab char (line 63) forbidden pattern: tab char (line 49) forbidden pattern: tab char (line 46) forbidden pattern: tab char (line 52) FAIL C4/Items.pm FAIL forbidden patterns forbidden pattern: trailing space char (line 650) FAIL members/readingrec.pl FAIL forbidden patterns forbidden pattern: trailing space char (line 81) Could you explain why you replaced - my $rsstimestamp = C4::Dates->new($result->{datereviewed},"iso"); - my $rsstimestamp_output = $rsstimestamp->output("rfc822"); - $result->{timestamp} = $rsstimestamp_output; + $result->{timestamp} = strftime("%Y-%m-%dT%H:%M:%S-00:00", gmtime); and - $datereceived = C4::Dates->output('iso') unless $datereceived; + $datereceived = output_pref({ dt=>DateTime->now(), dateonly=>1, otputpref=>'iso' }) unless $datereceived; I would use Koha::DateUtils everywhere.
Hi Jonathan, Regarding forbidden chars like tabs: I did not clean up existing things. Regarding your questions: I propose to obsolete the following patches and do them in a next step: "Remove deprecated C4::Dates from opac/opac-showreviews.pl" "Remove C4::Dates from C4/Acquisition.pm" If there are more patches where you have questions or concerns about please do not hesitate to obsolete them. My aim is to have done the "easy ones" in a first round.
Comment on attachment 36831 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm Review of attachment 36831 [details] [review]: ----------------------------------------------------------------- ::: C4/Acquisition.pm @@ +1399,4 @@ > my $order_vendornote = $params->{order_vendornote}; > > my $dbh = C4::Context->dbh; > + $datereceived = output_pref({ dt=>DateTime->now(), dateonly=>1, otputpref=>'iso' }) unless $datereceived; otputpref should be dateformat :)
Comment on attachment 36836 [details] [review] Bug 13813 - Remove C4::Dates from PDF templates in acquisitions Review of attachment 36836 [details] [review]: ----------------------------------------------------------------- ::: acqui/pdfformat/layout2pages.pm @@ +171,4 @@ > $text->text($basketgroup->{'id'}); > > # print the date > + my $today = output_pref({ dt=>DateTime->now(), dateonly=>1 }); Should be output_pref({ dt => dt_from_string, dateonly => 1 });
Comment on attachment 36831 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm Review of attachment 36831 [details] [review]: ----------------------------------------------------------------- ::: C4/Acquisition.pm @@ +27,5 @@ > use C4::Biblio; > use C4::Contract; > use C4::Debug; > use C4::Templates qw(gettemplate); > +use Koha::DateUtils; Hum, why?
(In reply to Marc Véron from comment #56) > Hi Jonathan, > > Regarding forbidden chars like tabs: I did not clean up existing things. > > Regarding your questions: > > I propose to obsolete the following patches and do them in a next step: > > "Remove deprecated C4::Dates from opac/opac-showreviews.pl" > "Remove C4::Dates from C4/Acquisition.pm" > > If there are more patches where you have questions or concerns about please > do not hesitate to obsolete them. My aim is to have done the "easy ones" in > a first round. No, keep it. Koha::DateUtils does not support rfc822 format, so I am fine for using strftime.
Created attachment 37730 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm This patch removes depraceted C4::Dates from C4/Acquisition.pm To test: Apply patch Go to Home > Acquisitions Verify that receiving orders works as before (Receiving orders only affected by this patch) Amended because of mistake (otputpref should be dateformat) / MV
Created attachment 37731 [details] [review] Bug 13813 - Remove C4::Dates from PDF templates in acquisitions To test: Apply patch In Acquisitions, go to a vendor's "Basket grouping" Find a basket and print order Change PDF templates using syspref OrderPdfFormat and test for - English 2-page - English 3-page - French 3-page - German 2-page Amended following comment #58 /MV
Created attachment 37732 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm This patch removes depraceted C4::Dates from C4/Acquisition.pm To test: Apply patch Go to Home > Acquisitions Verify that receiving orders works as before (Receiving orders only affected by this patch) Amended because of mistake (otputpref should be dateformat), See comment #57 / MV Amended because of "use Koha::DateUtils;" instead of original, See comment #59 /MV
Comment on attachment 37732 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm Sorry, something went wrong with this amendement, re-activating previous patch.
Created attachment 37736 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm This patch removes depraceted C4::Dates from C4/Acquisition.pm To test: Apply patch Go to Home > Acquisitions Verify that receiving orders works as before (Receiving orders only affected by this patch) Amended because of mistake (otputpref should be dateformat), see comment #57/ MV Amended because of "use Koha::DateUtils;" instead of original, See comment #59 /MV
Created attachment 37737 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm This patch removes depraceted C4::Dates from C4/Acquisition.pm To test: Apply patch Go to Home > Acquisitions Verify that receiving orders works as before (Receiving orders only affected by this patch) Amended because of mistake (otputpref should be dateformat), see comment #57/ MV Amended because of "use Koha::DateUtils;" instead of original, See comment #59 /MV
Created attachment 37746 [details] [review] Bug 13813 - Remove C4::Dates from circ/circ/transferstoreceive.pl To test: Go to Home > Circulation > Transfers to your library (Circulation Reports - Transfers to receive) Verify that date in page title appears correcty Amendend following comment #58 /MV
Created attachment 37748 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/barcode_manual.pl To test: Apply patch Set the plugin for 952$p (or UNIMARC equivalent to items.barcode) for one of your frameworks to barcode_manual.pl instead of barcode.pl Set syspref autoBarcode to <year>-001, <year>-002 and <branchcode>yymm001 Edit items or add item, verify that barcodes are filled in as appropriate by clicking on the value builder link (...) (Note: Description of barcode_manual.pl in Bug 8524) Amended following comment #58 /MV
Created attachment 37749 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/dateaccessioned.pl To test: Apply patch Edit an item Verifiy that autofilling date in d - Date acquired works as before. Amended following comment #58 /MV
Created attachment 37753 [details] [review] Bug 13813 - Remove C4::Dates from members/readingrec.pl This patch removes depraceted C4::Dates from members/readingrec.pl To test: Apply patch Go to a patron's cirulation history Verify that circulation history behaves as before. Amended following comment #58 /MV
Created attachment 37754 [details] [review] Bug 13813 - Remove C4::Dates from C4/Items.pm This patch removes depraceted C4::Dates from C4/Items.pm To test: Apply patch To test: Add modify items. Checkout added item. Verify that item handling works as before. Amended following comment #58 / MV
Created attachment 37757 [details] [review] Bug 13813 - Remove C4::Dates from C4/Members.pm This patch removes C4::Dates from C4/Members.pm Note: This patch can be tested independently from other patches in this Bug, but it depends on patches from Bug 13601 To test: Make sure that patches from 13601 are applied Apply patch Check changes in code Make sure that following functionality works as before: - Add a patron - Renew patrons - Go to checkout page and create message for patron Amended following comment #58 / MV There is one more DateTime->now() left on line 1825, this is not related to my changes.
Created attachment 37762 [details] [review] Bug 13813 - Remove C4::Dates from C4/Barcodes/annual.pm To test: Set syspref autoBarcode to <year>-001, <year>-002 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate. Amended following comment #58
Created attachment 37763 [details] [review] Bug 13813 - Remove C4::Dates from C4/Barcodes/hbyymmincr.pm To test: Set syspref autoBarcode to <branchcode>yymm0001 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate. Amended following comment #58 /mv
Created attachment 37764 [details] [review] Bug 13813 - Remove C4::Dates from C4/Suggestions.pm To test: Apply patch Go to Home > Acquisitions > Suggestions management Verify in "Filter by" that filter works for Suggestion information / dates Amended following comment #58 /mv
Created attachment 37765 [details] [review] Bug 13813 - Remove C4::Dates from members/moremember.pl To test: Apply patch Go to a patron's Details page (moremember.pl) Verify that dates display correctly (birth date, registration, espiration, manual restriction with date) Amended following comment #58 /mv
Created attachment 37766 [details] [review] Bug 13813 - Remove C4::Dates from circ/waitingreserves.pl To test: Go to Home > Circulation > Holds awaiting pickup (Circulation Rports - Holds awaiting pickup) Verify that date in page title appears correcty Amended following comment #58
Created attachment 37767 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/barcode.pl To test: Set syspref autoBarcode to <year>-001, <year>-002 and <branchcode>yymm001 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate. Amended following comment #58 /mv
Created attachment 37768 [details] [review] Bug 13813 - Remove C4::Dates from PDF templates in acquisitions To test: Apply patch In Acquisitions, go to a vendor's "Basket grouping" Find a basket and print order Change PDF templates using syspref OrderPdfFormat and test for - English 2-page - English 3-page - French 3-page - German 2-page Amended following comment #58 /MV
Created attachment 37770 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm This patch removes depraceted C4::Dates from C4/Acquisition.pm To test: Apply patch Go to Home > Acquisitions Verify that receiving orders works as before (Receiving orders only affected by this patch) Amended because of mistake (otputpref should be dateformat), see comment #57/ MV Amended because of "use Koha::DateUtils;" instead of original, See comment #59 /MV Amended following comment #58
Comment on attachment 36893 [details] [review] Bug 13813 - Remove C4::Dates from patroncards/members-search.pl CONFLICT (modify/delete): patroncards/members-search.pl deleted in HEAD and modified in Bug 13813
Created attachment 37810 [details] temporary tests Marc, If I am not wrong, these tests reflect what you are doing in the patches. If you have a look, you will see there are some mistakes. For instance output_pref( { dt => dt_from_string($iso), dateformat => 'iso', dateonly => 1 } ); won't return the same as C4::Dates->new( $iso, 'iso' )->output("iso"); But output_pref( { dt => dt_from_string({ dt => $iso, dateformat => 'iso' }), dateformat => 'iso', dateonly => 1 } ); will :) You should try to list exhaustively all existing patterns of C4::Dates you will replace and try to find their correct replacements.
Created attachment 37828 [details] [review] Bug 13813 - Remove C4::Dates from circ/waitingreserves.pl To test: Go to Home > Circulation > Holds awaiting pickup (Circulation Rports - Holds awaiting pickup) Verify that date in page title appears correcty Amended following comment #58 Changed output_pref to format 'iso'
Created attachment 37835 [details] [review] Bug 13813 - Remove C4::Dates from members/readingrec.pl This patch removes depraceted C4::Dates from members/readingrec.pl To test: Apply patch Go to a patron's cirulation history Verify that circulation history behaves as before. Amended following comment #58 /MV Amended to fix output_pref (dateonly added)
Created attachment 37839 [details] [review] Bug 13813 - Remove C4::Dates from members/printfeercpt.pl To test: Apply patch Go to a patrons Account In table of fines / transactions, column 'Print', click on 'Print' of a payment transaction ("Payment, thanks") Verify that the date displays correctly on the receipt. Amended for proper input for dt_from_string - (is 'iso')
Created attachment 37840 [details] [review] Bug 13813 - Remove C4::Dates from members/readingrec.pl This patch removes depraceted C4::Dates from members/readingrec.pl To test: Apply patch Go to a patron's cirulation history Verify that circulation history behaves as before. Amended following comment #58 /MV Amended to fix output_pref (dateonly added)
Created attachment 37841 [details] [review] Bug 13813 - Remove C4::Dates from members/printfeercpt.pl To test: Apply patch Go to a patrons Account In table of fines / transactions, column 'Print', click on 'Print' of a payment transaction ("Payment, thanks") Verify that the date displays correctly on the receipt. Amended for proper input for dt_from_string - (is 'iso')
Created attachment 37842 [details] [review] Bug 13813 - Remove C4::Dates from members/printfeercpt.pl To test: Apply patch Go to a patrons Account In table of fines / transactions, column 'Print', click on 'Print' of a payment transaction ("Payment, thanks") Verify that the date displays correctly on the receipt. Amended for proper input for dt_from_string - (is 'iso')
Created attachment 37843 [details] [review] Bug 13813 - Remove C4::Dates from members/printinvoice.pl To test: Apply patch Go to a patrons Account In table of fines / transactions, column 'Print', click on 'Print' of a transaction Verify that the date displays correctly on the invoice. Amended for proper format in dt_from_string (is 'iso')
(In reply to Jonathan Druart from comment #82) > Created attachment 37810 [details] > temporary tests > > Marc, > If I am not wrong, these tests reflect what you are doing in the patches. > If you have a look, you will see there are some mistakes. (...) > You should try to list exhaustively all existing patterns of C4::Dates you > will replace and try to find their correct replacements. Jonathan, Thank you very much for your help, your advice and for the idea with the test file. I used it as a base to go systematically through all attachments (and amended some of them). I tested the patches one by one in the staff client as well and hope now that this first patch set is really ready for sign-off. Marc
Comment on attachment 37757 [details] [review] Bug 13813 - Remove C4::Dates from C4/Members.pm Review of attachment 37757 [details] [review]: ----------------------------------------------------------------- ::: C4/Members.pm @@ +2363,4 @@ > my @results; > > while ( my $data = $sth->fetchrow_hashref ) { > + $data->{message_date_formatted} = output_pref({ dt => dt_from_string( $data->{message_date} ), dateformat => 'iso', dateonly => 1 }); We don't want an iso formatted date here.
Comment on attachment 36882 [details] [review] Bug 13813 - Remove C4::Dates from circ/bookcount.pl Review of attachment 36882 [details] [review]: ----------------------------------------------------------------- ::: circ/bookcount.pl @@ +86,4 @@ > biblioitemnumber => $bi, > homebranch => $homebranch, > holdingbranch => $holdingbranch, > + lastdate => $lastdate ? output_pref({ dt => dt_from_string( $lastdate ), dateformat => 'iso', dateonly => 1 }) : 0, Not correct, we want a formatted date. @@ +177,4 @@ > my ($date) = @_; > $date or return; > return ( > + output_pref({ dt => dt_from_string( $date, 'iso' ), dateonly => 1 }), This is correct.
Comment on attachment 36900 [details] [review] Bug 13813 - Remove C4::Dates from tools/koha-news.pl Review of attachment 36900 [details] [review]: ----------------------------------------------------------------- ::: tools/koha-news.pl @@ +40,5 @@ > my $id = $cgi->param('id'); > my $title = $cgi->param('title'); > my $new = $cgi->param('new'); > +my $expirationdate = output_pref({ dt => dt_from_string( $cgi->param('expirationdate') ), dateformat => 'iso', dateonly => 1 }); > +my $timestamp = output_pref({ dt => dt_from_string( $cgi->param('timestamp') ), dateformat => 'iso', dateonly => 1 }); Not sure about this but it does not seem to break anything.
Created attachment 37853 [details] [review] Bug 13813 - Remove deprecated C4::Dates from opac/opac-showreviews.pl To test: This patch removes C4::Dates from opac/opac-showreviews.pl and treats the RSS time stamps the same way as in opac/opac-search.pl To test: - Enable display of 'Recent comments' in OPAC (Syspref OpacShowRecentComments) - In OPAC, go to Home > Recent comments - Test RSS feed - Apply patch - Test RSS feed again, it should work as before Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37854 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from C4/HoldsQueue.pm To test: Apply patch Make sure that C4::Dates is not used in C4/HoldsQueue.pm Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37855 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from Koha/Borrower/Files.pm To test: Apply patch Make sure that uploading arbitrary files for a patron works as before (Set syspref EnableBorrowerFiles for testing ) Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37856 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from Koha/Misc/Files.pm To test: Apply patch Make sure that C4::Dates is not used in the code Set syspref AcqEnableFiles to enable Go to Home > Acquisitions > Invoices Find an invoice, go to Details, Manage Invoice files, make sure that uploading / displaying files works as before Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37857 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from tags/list.pl To test: Apply patch Verify in code that C4::Dates is not necessary in tags/list.pl Set sysprefs to TagsEnabled and TagsModeration Got to Home > Tools > Tags In table of tags, click on a Term You should get a page Home > Tools > Tags > Results for [Term] Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37858 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from C4/Barcodes.pm To test: Apply patch Verify that C4::Dates is not used in code. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37859 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from C4/Biblio.pm To test: Apply patch Verify that C4::Dates is not used in code. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37860 [details] [review] Bug 13813 - Remove C4::Dates from C4/Log.pm To test: Apply patch Go to Home > Tools > Logs Search logs with different from- and to-dates, verify that date range displays as appropriate. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37861 [details] [review] Bug 13813 - Remove C4::Dates from C4/NewsChannels.pm To test: Apply patch Create some news for OPAC and staff client (Home > Tools > News) Verify that dates appear as appropriate Verify that Publicaten Date (Creation Date) and Expiration Date work as expected. Note: Inconsistency in wording for creation date / publication date should be managed in separate Bug 13834 Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37862 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from C4/Output.pm To test: Apply patch Verify that C4::Dates is not used in code Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37863 [details] [review] Bug 13813 - Remove C4::Dates from C4/VirtualShelves/Page.pm To test: Apply patch In staff client, go to Home > Lists Verify that list of lists displays as before In OPAC, go to Lists Verify that Public and Private Lists display as before Note: 'dateadded' is not displayed in lists of lists. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37864 [details] [review] Bug 13813 - Remove C4::Dates from circ/bookcount.pl To test: Apply patch Search a biblio with items that are or have been checked out Go to "Items" (moredetail.pl) Locate "Total checkouts" of an item and click 'View item's checkout hsitory' Verify date and time in column "Last seen" Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37865 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from members/notices.pl To test: Apply patch Verify that C4::Dates is not used in members/notices.pl Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37866 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-messaging.pl To test: Apply patch Verify that C4::Dates is not used in code Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37867 [details] [review] Remove superfluous use 'C4::Dates' from opac/opac-privacy.pl To test: Apply patch Verify that C4::Dates is not used in code http://bugs.koha-community.org/show_bug.cgi?id=13813 Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37868 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-ratings.pl To test: Apply patch Verify that C4::Dates is not used in code Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37869 [details] [review] Bug 13813 - Remove C4::Dates from opac/opac-user.pl To test: Apply patch Manually restrict a patron (with end date) Check in to OPAC with this patron's username/password Verify that message about restrictions displays restriction date correctly in tab "Your summary" (opac-user.pl) Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37870 [details] [review] Bug 13813 - Remove C4::Dates from tools/koha-news.pl To test: Apply patch Go to Home > Tools > News Verify that dates appear as expected in date columns Create and edit news, verify dates behave as expected. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37871 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-search-history.pl To test: Apply patch Verify that C4::Dates is not used in code Go to ome > Tools > Logs Display logs with from and to dates Verify that search range behaves as expected. ' (Note: In table 'XXXX lines found', date and time always appear as timestamp as before) Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37872 [details] [review] Bug 13813 - Remove C4::Dates from tools/scheduler.pl To test: Apply patch Go to Home > Tools > Task scheduler Add new tasks Verify that tasks are properly added Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37873 [details] [review] Bug 13813 - Remove C4::Dates from circ/circ/transferstoreceive.pl To test: Go to Home > Circulation > Transfers to your library (Circulation Reports - Transfers to receive) Verify that date in page title appears correcty Amendend following comment #58 /MV Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37874 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/barcode_manual.pl To test: Apply patch Set the plugin for 952$p (or UNIMARC equivalent to items.barcode) for one of your frameworks to barcode_manual.pl instead of barcode.pl Set syspref autoBarcode to <year>-001, <year>-002 and <branchcode>yymm001 Edit items or add item, verify that barcodes are filled in as appropriate by clicking on the value builder link (...) (Note: Description of barcode_manual.pl in Bug 8524) Amended following comment #58 /MV Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37875 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/dateaccessioned.pl To test: Apply patch Edit an item Verifiy that autofilling date in d - Date acquired works as before. Amended following comment #58 /MV Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37876 [details] [review] Bug 13813 - Remove C4::Dates from C4/Items.pm This patch removes depraceted C4::Dates from C4/Items.pm To test: Apply patch To test: Add modify items. Checkout added item. Verify that item handling works as before. Amended following comment #58 / MV Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37877 [details] [review] Bug 13813 - Remove C4::Dates from C4/Members.pm This patch removes C4::Dates from C4/Members.pm Note: This patch can be tested independently from other patches in this Bug, but it depends on patches from Bug 13601 To test: Make sure that patches from 13601 are applied Apply patch Check changes in code Make sure that following functionality works as before: - Add a patron - Renew patrons - Go to checkout page and create message for patron Amended following comment #58 / MV There is one more DateTime->now() left on line 1825, this is not related to my changes. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37878 [details] [review] Bug 13813 - Remove C4::Dates from C4/Barcodes/annual.pm To test: Set syspref autoBarcode to <year>-001, <year>-002 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate. Amended following comment #58 Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37879 [details] [review] Bug 13813 - Remove C4::Dates from C4/Barcodes/hbyymmincr.pm To test: Set syspref autoBarcode to <branchcode>yymm0001 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate. Amended following comment #58 /mv Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37880 [details] [review] Bug 13813 - Remove C4::Dates from C4/Suggestions.pm To test: Apply patch Go to Home > Acquisitions > Suggestions management Verify in "Filter by" that filter works for Suggestion information / dates Amended following comment #58 /mv Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37881 [details] [review] Bug 13813 - Remove C4::Dates from members/moremember.pl To test: Apply patch Go to a patron's Details page (moremember.pl) Verify that dates display correctly (birth date, registration, espiration, manual restriction with date) Amended following comment #58 /mv Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37882 [details] [review] Bug 13813 - Remove C4::Dates from cataloguing/value_builder/barcode.pl To test: Set syspref autoBarcode to <year>-001, <year>-002 and <branchcode>yymm001 Edit items or add item, click in empty barcode field, verify that barcodes are filled in as appropriate. Amended following comment #58 /mv Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37883 [details] [review] Bug 13813 - Remove C4::Dates from PDF templates in acquisitions To test: Apply patch In Acquisitions, go to a vendor's "Basket grouping" Find a basket and print order Change PDF templates using syspref OrderPdfFormat and test for - English 2-page - English 3-page - French 3-page - German 2-page Amended following comment #58 /MV Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37884 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm This patch removes depraceted C4::Dates from C4/Acquisition.pm To test: Apply patch Go to Home > Acquisitions Verify that receiving orders works as before (Receiving orders only affected by this patch) Amended because of mistake (otputpref should be dateformat), see comment #57/ MV Amended because of "use Koha::DateUtils;" instead of original, See comment #59 /MV Amended following comment #58 Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37885 [details] [review] Bug 13813 - Remove C4::Dates from circ/waitingreserves.pl To test: Go to Home > Circulation > Holds awaiting pickup (Circulation Rports - Holds awaiting pickup) Verify that date in page title appears correcty Amended following comment #58 Changed output_pref to format 'iso' Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37886 [details] [review] Bug 13813 - Remove C4::Dates from members/readingrec.pl This patch removes depraceted C4::Dates from members/readingrec.pl To test: Apply patch Go to a patron's cirulation history Verify that circulation history behaves as before. Amended following comment #58 /MV Amended to fix output_pref (dateonly added) Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37887 [details] [review] Bug 13813 - Remove C4::Dates from members/printfeercpt.pl To test: Apply patch Go to a patrons Account In table of fines / transactions, column 'Print', click on 'Print' of a payment transaction ("Payment, thanks") Verify that the date displays correctly on the receipt. Amended for proper input for dt_from_string - (is 'iso') Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37888 [details] [review] Bug 13813 - Remove C4::Dates from members/printinvoice.pl To test: Apply patch Go to a patrons Account In table of fines / transactions, column 'Print', click on 'Print' of a transaction Verify that the date displays correctly on the invoice. Amended for proper format in dt_from_string (is 'iso') Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37889 [details] [review] Bug 13813 - (follow-up) Remove C4::Dates from C4/Members.pm message_date_formatted is only used once in the template. Previous patch does not formatted correctly the date (was in iso). The easier for me was to use the KohaDates TT plugin Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37890 [details] [review] Bug 13813 - (follow-up) Remove C4::Dates from circ/bookcount.pl Previous patch was wrong for the lastdate format. We want a date formatted following the syspref value, not an iso formatted date. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 37891 [details] [review] Bug 13813: Fix compilation errors Bareword "dt_from_string" not allowed while "strict subs" in use Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
To be honest, I did not check all pages. But looking at the code it should be all right now. I tried some pages and dates look correctly formatted.
(In reply to Jonathan Druart from comment #93) > Comment on attachment 36900 [details] [review] [review] > Bug 13813 - Remove C4::Dates from tools/koha-news.pl > > Review of attachment 36900 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: tools/koha-news.pl > @@ +40,5 @@ > > my $id = $cgi->param('id'); > > my $title = $cgi->param('title'); > > my $new = $cgi->param('new'); > > +my $expirationdate = output_pref({ dt => dt_from_string( $cgi->param('expirationdate') ), dateformat => 'iso', dateonly => 1 }); > > +my $timestamp = output_pref({ dt => dt_from_string( $cgi->param('timestamp') ), dateformat => 'iso', dateonly => 1 }); > > Not sure about this but it does not seem to break anything. For QAer, have a look at this. I don't see the timestamp param set in the template. On the list view, before the patch, I got: koha-news.pl: Usage: POSIX::strftime(fmt, sec, min, hour, mday, mon, year, wday = -1, yday = -1, isdst = -1) at /home/koha/src/C4/Dates.pm line 187. koha-news.pl: ...caught at /home/koha/src/tools/koha-news.pl line 45.
Something else: I did not try but... C4::Dates was (is) very efficient to format a date from an iso date. Now we have to call dt_from_string and output_pref. Maybe we should introduce a new subroutine without calls to DateTime to do this job.
(In reply to Jonathan Druart from comment #135) > Something else: I did not try but... C4::Dates was (is) very efficient to > format a date from an iso date. > Now we have to call dt_from_string and output_pref. > Maybe we should introduce a new subroutine without calls to DateTime to do > this job. % more t.pl use C4::Dates qw(format_date); my $iso_date = '2015-04-10 09:30:03'; for ( 1 .. 10000 ) { format_date($iso_date); } % time perl t.pl perl t.pl 0.78s user 0.01s system 99% cpu 0.795 total % time perl t.pl perl t.pl 0.74s user 0.01s system 99% cpu 0.749 total % time perl t.pl perl t.pl 0.71s user 0.02s system 99% cpu 0.735 total % more t.pl use Koha::DateUtils; my $iso_date = '2015-04-10 09:30:03'; for ( 1 .. 10000 ) { output_pref({ dt => dt_from_string( $iso_date, 'iso' ), dateonly => 1 }); } % time perl t.pl perl t.pl 2.40s user 0.02s system 100% cpu 2.424 total % time perl t.pl perl t.pl 2.42s user 0.02s system 99% cpu 2.442 total % time perl t.pl perl t.pl 2.43s user 0.01s system 100% cpu 2.439 total
(In reply to Jonathan Druart from comment #135) > Something else: I did not try but... C4::Dates was (is) very efficient to > format a date from an iso date. > Now we have to call dt_from_string and output_pref. > Maybe we should introduce a new subroutine without calls to DateTime to do > this job. I was thinking about adding new functions to Koha::DateUtils mostly to streamline the transition process to Koha::DateUtils (easier and more human readable calls) but I was afraid that that would slow down the changes. (I'm eager to get Bug 12072 resolved.) I think if calls to dt_from_string and output_pref turn up to be to time costly e.g. in big loops we can resolve this in a follow up.
Patches still apply on current master
Created attachment 39666 [details] [review] Bug 13813 - Remove C4::Dates from C4/Acquisition.pm This patch removes depraceted C4::Dates from C4/Acquisition.pm To test: Apply patch Go to Home > Acquisitions Verify that receiving orders works as before (Receiving orders only affected by this patch) Amended because of mistake (otputpref should be dateformat), see comment #57/ MV Amended because of "use Koha::DateUtils;" instead of original, See comment #59 /MV Amended following comment #58 Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Amended to resolv conflict while applying / MV
The number of patches this report touches does not justify the trivial patch status imo btw..
(In reply to Marcel de Rooy from comment #140) > The number of patches this report touches does not justify the trivial patch > status imo btw.. patches -> files OR touches -> includes :)
Comment on attachment 37868 [details] [review] Bug 13813 - Remove superfluous use 'C4::Dates' from opac/opac-ratings.pl Obsoleted because int hte meantime the line use C4::Dates qw(format_date) was removed.
Created attachment 40894 [details] [review] Bug 13813 - Remove C4::Dates from circ/circ/transferstoreceive.pl To test: Go to Home > Circulation > Transfers to your library (Circulation Reports - Transfers to receive) Verify that date in page title appears correcty Amendend following comment #58 /MV Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> It ssemed that the amendent did not work out. Amended again following comment #58 / MV
Still applies. Still blocks Bug 12072: New dateformat dd.mm.yyyy
(In reply to Marc Véron from comment #144) > Still applies. > Still blocks Bug 12072: New dateformat dd.mm.yyyy Will have a look now. But the number of patches here is scary.. Do not know if I can finish this in one session.
Marc: Why do you add MARC::Record in Acquisition.pm?
(In reply to Marcel de Rooy from comment #145) > (In reply to Marc Véron from comment #144) > > Still applies. > > Still blocks Bug 12072: New dateformat dd.mm.yyyy > > Will have a look now. But the number of patches here is scary.. > Do not know if I can finish this in one session. It's almost always the same changes. See comments 134-136
(In reply to Marcel de Rooy from comment #146) > Marc: Why do you add MARC::Record in Acquisition.pm? It's a bad merge conflict, in the local branch I have, there is only -use C4::Dates qw(format_date format_date_in_iso); use MARC::Record; Now there is use C4::Dates qw(format_date format_date_in_iso); +use MARC::Record; It should be -use C4::Dates qw(format_date format_date_in_iso); -use MARC::Record; :)
(In reply to Jonathan Druart from comment #148) > It should be > > -use C4::Dates qw(format_date format_date_in_iso); > -use MARC::Record; > > :) Well no, it should be -use C4::Dates qw(format_date format_date_in_iso);
Apart from the previous question, here are the results of the Dutch jury: =head2 DelItem @@ -2128,7 +2128,7 @@ C<items.withdrawn> sub _set_defaults_for_add { my $item = shift; - $item->{dateaccessioned} ||= C4::Dates->new->output('iso'); + $item->{dateaccessioned} || output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }); $item->{$_} ||= 0 for (qw( notforloan damaged itemlost withdrawn)); } OOPS: You lost an important character || instead of ||= !!!!!!! C4/Members: $data->{message_date_formatted} = $d->output; [ no longer added ] GetMessages Not sure if you also resolved all calls of this sub accordingly. Could you confirm? C4/VirtualShelves - $this_item->{'dateadded'} = format_date( $this_item->{'dateadded'} ); + $this_item->{'dateadded'} = output_pref({ dt => dt_from_string( $this_item->{'dateadded'} ), dateformat => 'iso', dateonly => 1 }); No iso ? circ.bookcount - lastdate => $lastdate ? format_date($lastdate) : 0, + lastdate => $lastdate ? output_pref({ dt => dt_from_string( $lastdate, 'iso' ), dateonly => 1 }) : 0, No iso? - format_date($date), + output_pref({ dt => dt_from_string( $date, 'iso' ), dateonly => 1 }), Again. moremember + $template->param( 'userdebarreddate' => output_pref({ dt => dt_from_string( $debar, 'iso' ), dateonly => 1 }) ); Looks different members/printfeercpt.pl - my %row = ( 'date' => format_date($accts->[$i]{'date'}), + my %row = ( 'date' => output_pref({ dt => dt_from_string( $accts->[$i]{'date'}, 'iso' ), dateonly => 1 }), No iso? members/printinvoice.pl Again opac-showreviews.pl + timestamp => strftime("%Y-%m-%dT%H:%M:%S-00:00", gmtime), Could you confirm this is the same as rfc822 Since I see it more, would this be something for the KohaDates module too ?
Created attachment 42052 [details] [review] Bug 13813 - [Follow-up] Remove C4::Dates from C4/Acquisition.pm Fix badly merged lines in C4/Acquisition.pm See comments #146-149
Hi Marcel, Thanks a lot for QAing. I really appreciate. Question: Can we remove the 'problematic' patches from this huge patch set and then do a second round (maybe with a new bug number) after the 'unproblematic' patches have been pushed? Marc
(In reply to Marc Véron from comment #152) > Hi Marcel, > > Thanks a lot for QAing. I really appreciate. > > Question: > Can we remove the 'problematic' patches from this huge patch set and then do > a second round (maybe with a new bug number) after the 'unproblematic' > patches have been pushed? > > Marc It seems to that your corrections should not be that hard or time consuming. I would favor to reduce the number of patches here. There is no particular need to add a patch for each module or script. But of course, I have no objection if you move some patches elsewhere. Please set back to SO if you are ready..
Comment on attachment 37853 [details] [review] Bug 13813 - Remove deprecated C4::Dates from opac/opac-showreviews.pl Removed - Will be done in "second round"
Comment on attachment 37887 [details] [review] Bug 13813 - Remove C4::Dates from members/printfeercpt.pl Removed - will be done in "second round"
Comment on attachment 37881 [details] [review] Bug 13813 - Remove C4::Dates from members/moremember.pl Removed - will be done in "second round"
Comment on attachment 37864 [details] [review] Bug 13813 - Remove C4::Dates from circ/bookcount.pl Removed - will be done in "second round"
Comment on attachment 37890 [details] [review] Bug 13813 - (follow-up) Remove C4::Dates from circ/bookcount.pl Removed - will be done in "second round"
Comment on attachment 37863 [details] [review] Bug 13813 - Remove C4::Dates from C4/VirtualShelves/Page.pm Removed - Will be done in "second round"
Comment on attachment 37877 [details] [review] Bug 13813 - Remove C4::Dates from C4/Members.pm Removed - Will be done in "second round"
Comment on attachment 37889 [details] [review] Bug 13813 - (follow-up) Remove C4::Dates from C4/Members.pm Removed - will be done in "second round"
Created attachment 42175 [details] [review] [Follow-up] Bug 13813 - Remove C4::Dates from C4/Items.pm Fixed missing character (see comment #150)
Created attachment 42274 [details] [review] Bug 13813: Remove deprecated module C4::Dates from system Test plan: See Bugzilla. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment on attachment 37810 [details] temporary tests I ran this test, but it failed. Illegal date specified (year = 2014, month = 31, day = 01) at dates_compare.t line 17 The 'month' parameter ("31") to DateTime::new did not pass the 'an integer between 1 and 12' callback
Second QA round: circ/transferstoreceive.pl - show_date => format_date(C4::Dates->today('iso')), + show_date => output_pref({ dt => dt_from_string, dateonly => 1 }), No iso? Probably I did not catch this one in the first round :) circ/waitingreserves.pl - show_date => format_date(C4::Dates->today('iso')), + show_date => output_pref({ dt => dt_from_string, dateonly => 1 }), Same. members/printinvoice.pl You did still include this line, as marked in the "first round". FQA
Created attachment 42286 [details] [review] [Follow-up] Bug 13813 - Remove deprecated module C4::Dates from system Changes for QA comment #165 circ/transferstoreceive.pl 'iso' added circ/waitingreserves.pl 'iso' added members/printinvoice.pl 'iso' removed
Created attachment 42292 [details] [review] Bug 13813: Remove deprecated module C4::Dates from system Test plan: See Bugzilla. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 42293 [details] [review] Bug 13813 - Remove deprecated module C4::Dates from system Changes for QA comment #165 circ/transferstoreceive.pl 'iso' added circ/waitingreserves.pl 'iso' added members/printinvoice.pl 'iso' removed Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 42294 [details] [review] Bug 13813: [QA Follow-up] Fix some tabs for qa tools FAIL C4/Barcodes/annual.pm FAIL forbidden patterns forbidden pattern: tab char (line 86) forbidden pattern: tab char (line 51) FAIL C4/Barcodes/hbyymmincr.pm FAIL forbidden patterns forbidden pattern: tab char (line 108) forbidden pattern: tab char (line 78) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Looks good to me now. Did a full code review twice: found some small issues in both rounds. Tested only some locations in the interface with all three settings of dateformat. Added a follow-up to satisfy qa tools (trivial tabs). This kind of change needs some broader exposure by an early push.
Patches pushed to master. Thanks Marc! Clean it aaaaaaaaaall!
Comment on attachment 42292 [details] [review] Bug 13813: Remove deprecated module C4::Dates from system Review of attachment 42292 [details] [review]: ----------------------------------------------------------------- ::: C4/Suggestions.pm @@ +179,5 @@ > my $to = $field . "_to"; > if ( $suggestion->{$from} || $suggestion->{$to} ) { > push @query, qq{ AND suggestions.$field BETWEEN ? AND ? }; > push @sql_params, > + output_pref({ dt => dt_from_string( $suggestion->{$from} ), dateformat => 'iso', dateonly => 1 }) || '0000-00-00'; This is wrong. Please provide a follow-up. Before this patch the "from" date was 0000-00-00 if not filled, now it's today.
I think last comment is fixed with bug 15090.