Summary: | Undefined dates not formatted in serials update | ||
---|---|---|---|
Product: | Koha | Reporter: | Colin Campbell <colin.campbell> |
Component: | Serials | Assignee: | Colin Campbell <colin.campbell> |
Status: | CLOSED FIXED | QA Contact: | Bugs List <koha-bugs> |
Severity: | normal | ||
Priority: | PATCH-Sent (DO NOT USE) | CC: | chris, m.de.rooy |
Version: | rel_3_2 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Proposed Patch
Updated fuller patch Proposed Patch Revised Patch Patch (version for 3.2) |
Description
Colin Campbell
2010-07-16 13:21:39 UTC
Created attachment 2380 [details] [review] Proposed Patch Patch withdrawn. Needs a couple of other cases dealt with Created attachment 2381 [details] [review] Updated fuller patch Patch fails at current master. HEAD is now at 3c19f22 Merge remote branch 'kc/new/enh/bug_5283' into kcmaster error: patch failed: serials/serials-edit.pl:90 error: serials/serials-edit.pl: patch does not apply Created attachment 2898 [details] [review] Proposed Patch Patch rebased against current master This bug is mentioned in: Bug 5026 Undefined dates formatted poorly in serials-edit http://lists.koha-community.org/pipermail/koha-patches/2010-December/013450.html Thanks for updating the patch. Testing these changes seemed fine, but it is hard to say if you cover all changes. Did some code checking too on these changes and have four questions. Serials.pm 1) Line 734 $line->{planneddate} should be $line->{$datefield} ? serials-edit.pl 2) Line 99 If you assign to @serialsid in the for loop, you only keep the last results. There was a push to this array in the old code. Looks like a bug? 3) Line 119 This was already, but why is debug 1 ? 4) Line 376 A former comment stated that the sort was needed. Sure that you could remove it? Created attachment 2984 [details] [review] Revised Patch Revised patch which addresses some issues in the original patch Thanks for spotting those issues. I'm sending an amended patch. I've fixed the typos in 1 & 2 As for 3 most/all the serials cgi scripts set debug to 1 but looking at the called routine that parameter appears not to be examined. I'll look further but I think we can remove these. I'll generate a bug/patch for that. 4. Yes - the comment was a bit opaque in not saying why. In fact there are two redirects to serials-collections.pl and one sorted and one didn't. What I've done in the revised patch is move the sort to serials-collection so that the fact that the list is sorted is apparent where it is used and consistent. I'll take a closer look at serials-collection.pl, its not as clear as it could be. Created attachment 2988 [details] [review] Patch (version for 3.2) Patch did not cleanly apply against 3.2.x have added a version that does Tested revised patch on master. Works OK. Just a note: Still see some XXX's in date fields on serials-collection.pl. Originate probably from routine PrepareSerialsData in Serials.pm. Pushed to master, please test |