Summary: | claims management improvement | ||
---|---|---|---|
Product: | Koha | Reporter: | claire.hernandez <claire.hernandez> |
Component: | Acquisitions | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | corinnebulac, jonathan.druart, m.de.rooy, paul.poulain |
Version: | 3.8 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | Sponsored | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 7293 | ||
Bug Blocks: | 7163, 10556, 14535 | ||
Attachments: |
screenshot
Bug 7291: Adds new field aqbooksellers.deliverytime Bug 7291: Adds new field aqbooksellers.deliverytime Bug 7291: Adds new field aqbooksellers.deliverytime Bug 7291: Adds new field aqbooksellers.deliverytime csv file Bug 7291: Adds new field aqbooksellers.deliverytime Bug 7291: Followup: removes backquotes |
Description
claire.hernandez@biblibre.com
2011-12-02 09:30:03 UTC
Created attachment 7277 [details] screenshot Links to "check all/uncheck all" orders from a vendor were implemented with bug 5347. *** Bug 7293 has been marked as a duplicate of this bug. *** Created attachment 7622 [details] [review] Bug 7291: Adds new field aqbooksellers.deliverytime Bug 7291: Adds new field aqbooksellers.deliverytime New field deliverytime in aqbooksellers table. It is an estimated delivery time for orders (in days). You can set this delay on the supplier modification page. It is used in the late orders search. The order estimated date is the aqbasket.closedate + aqbooksellers.deliverytime If you set a delay, the query check if closedate <= today - delay If you set a "delivery time from" and a "delivery time to", the query check if $delivery_time_from <= aqbooksellers.deliverytime is not NULL and if closedate + deliverytime >= $delivery_time_to if there is not a time_to then $delivery_time_to = the current date. Hi Jonathan, I am sorry, this patch has a lot of conflicts now and will no longer apply. Could you please rebase and resubmit? Perhaps you will also want to make use of the TT plugin for dates. Created attachment 7800 [details] [review] Bug 7291: Adds new field aqbooksellers.deliverytime Rebased patch Created attachment 8074 [details] [review] Bug 7291: Adds new field aqbooksellers.deliverytime Comment on attachment 8074 [details] [review] Bug 7291: Adds new field aqbooksellers.deliverytime Rebased patch the filter on "Estimated Delivery date from: "/"to" doesnt't work Vendor -> OK Days ago -> OK Created attachment 8310 [details] [review] Bug 7291: Adds new field aqbooksellers.deliverytime Date to iso Created attachment 8332 [details]
csv file
this is still a problem. We have write delivery date as "AAAAMMJJ' if we want if it to work. Or the input form indicate "JJ/AA/MMMM' (ditto for the calendar help) I test on a sandbox and I can not reproduce the problem. Please, can you retest and tell me yours differents actions ? New tests -> OK QA comment: * patch does not apply, there are small merge problem * dates are formatted without the Date plugin, please fix ! + estimateddeliverydate => C4::Dates->new( $estimateddeliverydate, 'iso' )->output, * you use a MYSQLism for getting the current date. Instead of CURDATE( ) use CAST(now() AS date) Marking failed QA. The rest is OK, will mark passed QA once you've fixed those points Created attachment 8592 [details] [review] Bug 7291: Adds new field aqbooksellers.deliverytime Rebased patch. Remove warnings in tt file. Replace MYSQLism with standard format Replace date format QA comment: 1- there is a hardcoded variable inside SQL: - WHERE (closedate < DATE_SUB(CURDATE( ),INTERVAL $delay DAY) that is removed => +1 from QA point of view ! 2- the dates are displayed using kohaDates plugin => +1 A possible enhancement would be to highlight passed dates, but outside from a QA proposing passed QA, (not setting it as it's a BibLibre patch) Additional QA comment: I have no real objections for pushing this patch, but want to note: 1) Use of adddate, date_sub, ifnull: they are all mysql-isms I suspect that you could not avoid them here; and they are already present too. But what is the use of replacing curdate's and backquotes if we do not remove typical mysql date/time functions? But this is a discussion on itself, and should be held elsewhere. 2) BTW You add backtics in kohastructure with this patch (instead of removing them..) 3) Very small note: You edited the part with the cryptic test: not $delay =~ /^\d{1,3}$/ Actually, we do not want a delay>999 (for some reason). If you edit that line, please make the test faster and more readable with delay>999. Passed QA Created attachment 8809 [details] [review] Bug 7291: Followup: removes backquotes Hi Marcel, Thanks for your comment. 1,2/ This followup removes backquotes in kohastructure. I would like to remove these mysql specific functions, but there is not yet a way to do this in koha. 3/ the regexp does the same think as 999, even better because it does not warn if $delay is not an integer. I thing it is better, don't you think ? (In reply to comment #18) > Created attachment 8809 [details] [review] > Bug 7291: Followup: removes backquotes > 3/ the regexp does the same think as 999, even better because it does not > warn if $delay is not an integer. I thing it is better, don't you think ? I agree, as it avoid a risk of a SQL injection. patch pushed |