Bug 8125 - SIP should send Zipcode with address
Summary: SIP should send Zipcode with address
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Colin Campbell
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-18 16:54 UTC by Liz Rea
Modified: 2013-12-03 13:22 UTC (History)
5 users (show)

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


Attachments
Bug 8125 - Adds zipcode to address output (1.31 KB, patch)
2012-05-18 16:56 UTC, Liz Rea
Details | Diff | Splinter Review
Proposed patch (1.89 KB, patch)
2012-05-22 10:20 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 8125 Sip addess should include all address fields (1.94 KB, patch)
2012-07-01 10:07 UTC, Chris Cormack
Details | Diff | Splinter Review
Rebased patch (1.90 KB, patch)
2012-12-17 17:50 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 8215 - Course Reserves - Omnibus - v3.12.05 (147.35 KB, patch)
2013-10-08 16:32 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 8215 - Course Reserves - Omnibus - v3.12.07 (142.88 KB, patch)
2013-12-03 13:22 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Liz Rea 2012-05-18 16:54:00 UTC
SIP does not currently send the zipcode in the address field, I think it should.
Comment 1 Liz Rea 2012-05-18 16:56:08 UTC Comment hidden (obsolete)
Comment 2 Colin Campbell 2012-05-21 16:36:12 UTC
The zip code is just part of the address in sip terms, but the patch as is does not do that it appends address to adr but it never appends zip you probably want two lines and make the second test on $adr && $zip the test (is there a string of greater than 0 length in both elsements?) is there to decide whether to add a space before appending the new element. I think

$adr .= ($adr && $address ) ? " $address" : $address;
$adr .= ($adr && $zip) ? " $zip" : $zip;

is what you want.
Comment 3 Colin Campbell 2012-05-22 10:20:22 UTC Comment hidden (obsolete)
Comment 4 Liz Rea 2012-05-23 18:29:12 UTC
you're right, we should stick it all in there. 

and thanks for being fast on your patch too.
Comment 5 Chris Cormack 2012-07-01 10:07:23 UTC Comment hidden (obsolete)
Comment 6 Paul Poulain 2012-07-06 16:39:24 UTC
QA comment: small patch, does the job.
The next if / if / else / ... of _get_address could have been written in a smaller number of lines, but that's not worth failing QA
Comment 7 Paul Poulain 2012-07-06 16:39:54 UTC
Chris, your decision, but maybe worth porting to 3.8
Comment 8 Colin Campbell 2012-12-04 16:36:12 UTC
This patch was marked as status 'pushed to Master' but its not actually been applied.
Comment 9 Paul Poulain 2012-12-14 16:25:06 UTC
(In reply to comment #8)
> This patch was marked as status 'pushed to Master' but its not actually been
> applied.
I confirm. I must have made a mistake... back to passed QA for Jared
Comment 10 Jared Camins-Esakov 2012-12-17 14:12:40 UTC
This patch does not apply, and I'm not quite sure how to resolve the conflict. Please attach a rebased patch and reset status to "Passed QA."
Comment 11 Colin Campbell 2012-12-17 17:50:48 UTC
Created attachment 14169 [details] [review]
Rebased patch

Rebased patch to fix problem caused by interim changes to indebtation (taps -> spaces)
Comment 12 Colin Campbell 2012-12-17 17:52:13 UTC
Status reset as per Jared's request
Comment 13 Jared Camins-Esakov 2012-12-19 13:01:30 UTC
This patch has been pushed to master.
Comment 14 Kyle M Hall (khall) 2013-10-08 16:32:10 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall (khall) 2013-12-03 13:22:01 UTC Comment hidden (obsolete)