Supplementary to Bug 5021 : dates are also incorrectly formatted in serials edit if not defined. Apply similar processing in display. In test version (for 3.4) this should be handled for all cases in one place rather than the current format ad hoc all over the place
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