Bug 5026 - Undefined dates not formatted in serials update
Summary: Undefined dates not formatted in serials update
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: rel_3_2
Hardware: All All
: PATCH-Sent (DO NOT USE) normal (vote)
Assignee: Colin Campbell
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-16 13:21 UTC by Colin Campbell
Modified: 2012-10-25 23:04 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed Patch (1.25 KB, patch)
2010-07-16 13:30 UTC, Colin Campbell
Details | Diff | Splinter Review
Updated fuller patch (9.25 KB, patch)
2010-07-16 15:42 UTC, Colin Campbell
Details | Diff | Splinter Review
Proposed Patch (9.47 KB, patch)
2010-12-21 10:21 UTC, Colin Campbell
Details | Diff | Splinter Review
Revised Patch (10.01 KB, patch)
2011-01-13 14:17 UTC, Colin Campbell
Details | Diff | Splinter Review
Patch (version for 3.2) (10.10 KB, patch)
2011-01-13 16:14 UTC, Colin Campbell
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2010-07-16 13:21:39 UTC
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
Comment 1 Colin Campbell 2010-07-16 13:30:36 UTC Comment hidden (obsolete)
Comment 2 Colin Campbell 2010-07-16 14:22:32 UTC
Patch withdrawn. Needs a couple of other cases dealt with
Comment 3 Colin Campbell 2010-07-16 15:42:13 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2010-12-20 12:17:40 UTC
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
Comment 5 Colin Campbell 2010-12-21 10:21:38 UTC Comment hidden (obsolete)
Comment 6 MJ Ray (software.coop) 2011-01-11 12:58:20 UTC
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
Comment 7 Marcel de Rooy 2011-01-13 11:21:33 UTC
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?
Comment 8 Colin Campbell 2011-01-13 14:17:22 UTC
Created attachment 2984 [details] [review]
Revised Patch

Revised patch which addresses some issues in the original patch
Comment 9 Colin Campbell 2011-01-13 14:31:53 UTC
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.
Comment 10 Colin Campbell 2011-01-13 16:14:20 UTC
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
Comment 11 Marcel de Rooy 2011-01-17 08:20:43 UTC
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.
Comment 12 Chris Cormack 2011-01-17 10:01:55 UTC
Pushed to master, please test