Bug 17605 - EDI should set currency in order record on creation
Summary: EDI should set currency in order record on creation
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Colin Campbell
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-10 10:35 UTC by Colin Campbell
Modified: 2017-12-07 22:20 UTC (History)
5 users (show)

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


Attachments
Proposed patch (1.22 KB, patch)
2016-11-10 10:44 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 17605 Add currency to orders generated by quotes (1.28 KB, patch)
2016-11-29 22:37 UTC, Alex Buckley
Details | Diff | Splinter Review
Patch against master (1.28 KB, patch)
2017-03-13 11:30 UTC, Colin Campbell
Details | Diff | Splinter Review
Patch against master (1.45 KB, patch)
2017-03-13 11:54 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 17605: [MASTER] Add currency to orders generated by quotes (1.55 KB, patch)
2017-03-13 12:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17605: [STABLE] Add currency to orders generated by quotes (1.38 KB, patch)
2017-03-13 12:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2016-11-10 10:35:59 UTC
When creating an aqorder for a quote orderline currency is not explicitly set. The process should set it to the appropriate currency for the vendor
Comment 1 Colin Campbell 2016-11-10 10:44:53 UTC
Created attachment 57421 [details] [review]
Proposed patch

Without the patch currency in created orders defaults to null - with patch they are set to the currency as specified in the vendor record for listprice
Comment 2 Alex Buckley 2016-11-29 22:37:19 UTC
Created attachment 57811 [details] [review]
Bug 17605 Add currency to orders generated by quotes

Sets the vendors currency in the edi generated order
The currency used is agreed between vendor and library
and the value in the quote is optional

Edifact potentially allows the currency to be overwitten
by specifying another currency in the CUX segment but
we know currently of no supplier doing this

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 3 Alex Buckley 2016-11-29 22:39:14 UTC
Signed off
Comment 4 Marcel de Rooy 2017-03-10 09:24:54 UTC
Note that we have a Koha::Object for Bookseller(s)
Comment 5 Colin Campbell 2017-03-10 12:47:38 UTC
(In reply to Marcel de Rooy from comment #4)
> Note that we have a Koha::Object for Bookseller(s)

By not using the Koha::Object this patch will apply to the stable banches 16.5 and 16.11 Booksellers.pm and hence the find method only present in master
Comment 6 Marcel de Rooy 2017-03-10 12:59:39 UTC
(In reply to Colin Campbell from comment #5)
> (In reply to Marcel de Rooy from comment #4)
> > Note that we have a Koha::Object for Bookseller(s)
> 
> By not using the Koha::Object this patch will apply to the stable banches
> 16.5 and 16.11 Booksellers.pm and hence the find method only present in
> master

Agreed. Sounds reasonable for the stable branches.
Since the patch only deals with two lines, you could also supply two versions?
Comment 7 Colin Campbell 2017-03-13 11:30:23 UTC
Created attachment 61027 [details] [review]
Patch against master

Previous version of patch required for 16.5 or 16.11 this version wraps the dbic calls in Koha::Object::Booksellers but requires dependencies currently only im master
Comment 8 Colin Campbell 2017-03-13 11:31:02 UTC
(In reply to Marcel de Rooy from comment #6)
> (In reply to Colin Campbell from comment #5)
> > (In reply to Marcel de Rooy from comment #4)
> > > Note that we have a Koha::Object for Bookseller(s)
> > 
> > By not using the Koha::Object this patch will apply to the stable banches
> > 16.5 and 16.11 Booksellers.pm and hence the find method only present in
> > master
> 
> Agreed. Sounds reasonable for the stable branches.
> Since the patch only deals with two lines, you could also supply two
> versions?

Done
Comment 9 Marcel de Rooy 2017-03-13 11:43:25 UTC
They are exactly the same?
Comment 10 Colin Campbell 2017-03-13 11:54:31 UTC
Created attachment 61028 [details] [review]
Patch against master

oops uploaded wrong version - corrected
Comment 11 Marcel de Rooy 2017-03-13 12:11:30 UTC
Created attachment 61029 [details] [review]
Bug 17605: [MASTER] Add currency to orders generated by quotes

Sets the vendors currency in the edi generated order
The currency used is agreed between vendor and library
and the value in the quote is optional

Edifact potentially allows the currency to be overwitten
by specifying another currency in the CUX segment but
we know currently of no supplier doing this

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2017-03-13 12:12:39 UTC
Created attachment 61030 [details] [review]
Bug 17605: [STABLE] Add currency to orders generated by quotes

Sets the vendors currency in the edi generated order
The currency used is agreed between vendor and library
and the value in the quote is optional

Edifact potentially allows the currency to be overwitten
by specifying another currency in the CUX segment but
we know currently of no supplier doing this

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2017-03-13 12:14:32 UTC
Changing enhancement to normal. Stable patch is provided for stable branches..
Looks good to me.
Just noting for the record that we do not have unit tests for Koha::EDI. Should not block this twoliner..

Passed QA
Comment 14 Brendan Gallagher 2017-03-23 15:48:10 UTC
Pushed the MASTER one to master.  should be in 17.05 ;)   Thanks 

Note we still have a STABLE on to take care of
Comment 15 Katrin Fischer 2017-03-26 21:08:45 UTC
Pushed the [STABLE] patch.

This patch has been pushed to 16.11.x and will be in 16.11.06.
Comment 16 Mason James 2017-05-03 03:46:53 UTC
Pushed to 16.05.x, for 16.05.12 release