|
Description
Katrin Fischer
2020-06-02 15:39:19 UTC
After looking at the current database schema, the new columns in aqorders could be named: * invoice_currency: defaults to aqbooksellers.invoiceprice, can be changed on receive. We already have the option to enter the price in foreign currency on receive, but it is not stored. (see also bug 24971) * invoice_unitprice: the unit price in foreign currency as listed on the invoice For further information see attached documentation. Test plan 1. Receive an order. 2. Mark checkbox ‚Change currency‘ 3. select currency from drop down (invoice currency) 4. enter invoice unitprice in foreign currency as listed on the invoice 5. Save invoice 6. Make sure that invoice_unitprice and invoice_currency are stored in the database table aqorders Created attachment 144618 [details]
Receive Order in acq / Additionally store actual cost in foreign currency
Receive Order in acq / Additionally store actual cost in foreign currency
Created attachment 145906 [details] [review] Bug 25655: DB changes Sponsored-by: The Research University in the Helmholtz Association (KIT) Created attachment 145907 [details] [review] Bug 25655: DBIC changes Sponsored-by: The Research University in the Helmholtz Association (KIT) Created attachment 145908 [details] [review] Bug 25655: REST API specs Sponsored-by: The Research University in the Helmholtz Association (KIT) Created attachment 145909 [details] [review] Bug 25655: Store actual cost in foreign currency and currency from the invoice Bug 24158 allows to calculate the actual cost on time of receive from a foreign currency. This new enhancement is storing the price and the currency in database, into two new columns invoice_unitprice an invoice_currency Test plan: Create a new order with several items to receive Receive one or more items, and tick "change currency" on the order receive page Select a currency and finish the receiving In DB notice that the price and the currency have been stored. Receive remaining items and confirm that the form is prefilled with previous price and currency Sponsored-by: The Research University in the Helmholtz Association (KIT) Created attachment 145910 [details] [review] Bug 25655: Save values from ModOrderReceive Sponsored-by: The Research University in the Helmholtz Association (KIT) Created attachment 145911 [details] [review] Bug 25655: Select currency and prefill unitprice with previous receipt In case of partial order we want to prefill the unit price and the currency with the value of the previous receipt. Sponsored-by: The Research University in the Helmholtz Association (KIT) Created attachment 145912 [details] [review] Bug 25655: Add tests And fix a bug they caught. We need to undef if a modification is made and invoice_currency is removed. Sponsored-by: The Research University in the Helmholtz Association (KIT) Created attachment 146082 [details] [review] Bug 25655: DB changes Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146084 [details] [review] Bug 25655: DBIC changes Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146085 [details] [review] Bug 25655: REST API specs Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146086 [details] [review] Bug 25655: Store actual cost in foreign currency and currency from the invoice Bug 24158 allows to calculate the actual cost on time of receive from a foreign currency. This new enhancement is storing the price and the currency in database, into two new columns invoice_unitprice an invoice_currency Test plan: Create a new order with several items to receive Receive one or more items, and tick "change currency" on the order receive page Select a currency and finish the receiving In DB notice that the price and the currency have been stored. Receive remaining items and confirm that the form is prefilled with previous price and currency Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146087 [details] [review] Bug 25655: Save values from ModOrderReceive Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146088 [details] [review] Bug 25655: Select currency and prefill unitprice with previous receipt In case of partial order we want to prefill the unit price and the currency with the value of the previous receipt. Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146089 [details] [review] Bug 25655: Add tests And fix a bug they caught. We need to undef if a modification is made and invoice_currency is removed. Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> This works well overall, unit tests and QA script pass. What I notice is an inconsistency: * listprice and currency are used for the original price on ordering. * invoice_unitprice and invoice_currency are used for the original price on receive. But: * listprice and currency are always set, even if the currency is the active currency. * invoice_unitprice and invoice_currency are NULL, when the currency matches the active currency. I see some merit in making the invoice_unitprice behave like the listprice: * It could make it easier to use these columns in reporting. * It could make the transition easier if the active currency changes (just had that happen for a library in Croatia) But maybe there are downsides as well or a reason to keep it like that. Michaela and Joubu, what do you think? Note: * 'Bug 24971 - Pre-select invoice currency from vendor on receive' is not fixed by this patch. The pre-selected currency is the active one. Created attachment 146478 [details] [review] Bug 25655: DB changes Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146479 [details] [review] Bug 25655: DBIC changes Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146480 [details] [review] Bug 25655: REST API specs Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146481 [details] [review] Bug 25655: Store actual cost in foreign currency and currency from the invoice Bug 24158 allows to calculate the actual cost on time of receive from a foreign currency. This new enhancement is storing the price and the currency in database, into two new columns invoice_unitprice an invoice_currency Test plan: Create a new order with several items to receive Receive one or more items, and tick "change currency" on the order receive page Select a currency and finish the receiving In DB notice that the price and the currency have been stored. Receive remaining items and confirm that the form is prefilled with previous price and currency Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146482 [details] [review] Bug 25655: Save values from ModOrderReceive Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146483 [details] [review] Bug 25655: Select currency and prefill unitprice with previous receipt In case of partial order we want to prefill the unit price and the currency with the value of the previous receipt. Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Created attachment 146484 [details] [review] Bug 25655: Add tests And fix a bug they caught. We need to undef if a modification is made and invoice_currency is removed. Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Last changes just were a rebase - my testing overlapped with some changes on master that made it necessary. Marked the important comment important... :) Created attachment 146658 [details] [review] Bug 25655: Store invoice price and currency even if no currency change We could need these info to be stored even if the currency is the same as the active one, for statistic purpose for instance. Created attachment 146660 [details] [review] Bug 25655: Update existing orders in DB Michaela, I've implemented Katrin's suggestion from comment 18. Could you confirm you are agreeing with it? And retest? :) (In reply to Jonathan Druart from comment #27) > > We could need these info to be stored even if the currency is the same > as the active one, for statistic purpose for instance. Good point! So yes, I agree :-) Created attachment 146818 [details] [review] Bug 25655: DB changes Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 146819 [details] [review] Bug 25655: DBIC changes Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 146820 [details] [review] Bug 25655: REST API specs Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 146821 [details] [review] Bug 25655: Store actual cost in foreign currency and currency from the invoice Bug 24158 allows to calculate the actual cost on time of receive from a foreign currency. This new enhancement is storing the price and the currency in database, into two new columns invoice_unitprice an invoice_currency Test plan: Create a new order with several items to receive Receive one or more items, and tick "change currency" on the order receive page Select a currency and finish the receiving In DB notice that the price and the currency have been stored. Receive remaining items and confirm that the form is prefilled with previous price and currency Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 146822 [details] [review] Bug 25655: Save values from ModOrderReceive Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 146823 [details] [review] Bug 25655: Select currency and prefill unitprice with previous receipt In case of partial order we want to prefill the unit price and the currency with the value of the previous receipt. Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 146824 [details] [review] Bug 25655: Add tests And fix a bug they caught. We need to undef if a modification is made and invoice_currency is removed. Sponsored-by: The Research University in the Helmholtz Association (KIT) Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 146825 [details] [review] Bug 25655: Store invoice price and currency even if no currency change We could need these info to be stored even if the currency is the same as the active one, for statistic purpose for instance. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 146826 [details] [review] Bug 25655: Update existing orders in DB Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> As this patch adds the column invoice_currency I don't think the last patch with the database update is required :) (In reply to Katrin Fischer from comment #40) > As this patch adds the column invoice_currency I don't think the last patch > with the database update is required :) Thinking a bit more about this and correcting myself: The new database update updates the old order lines, setting: * invoice_currency to active currency code * invoice_unitprice to unitprice This is not wrong, it will not affect any calculations and if you run reports, you'll always get results. Only thing maybe is that we don't know if they were actually paid in active currency... as we just didn't store this information before. So if you are running reports on how many invoices you have in foreign currencies etc. it could create a false impression. There is no perfect solution for the pre-patch order lines as we are missing information. We need to pick one solution or the other, I am OK with Joubu's pick. (In reply to Katrin Fischer from comment #42) > There is no perfect solution for the pre-patch order lines as we are missing > information. We need to pick one solution or the other, I am OK with Joubu's > pick. I would lean towards leaving older orders blank - we don't know if the active currency ever changed, or what was done. Leaving them blank and knowing that only the ones received after this patch are populated seems okay to me. Not blocking, but that would be my preference I am ok with whatever you decide! (In reply to Jonathan Druart from comment #44) > I am ok with whatever you decide! Can you please obsolete/amend things so it works as Nick proposed? Thanks! I am also OK with Nicks solution and I know the sponsoring library won't care as they will have this patch from the beginning :) Joubu, I think obsoleting the last patch should be enough, do you agree? (In reply to Katrin Fischer from comment #46) > > Joubu, I think obsoleting the last patch should be enough, do you agree? I thought the same but you suggested otherwise last week on IRC heh (In reply to Tomás Cohen Arazi from comment #47) > (In reply to Katrin Fischer from comment #46) > > > > Joubu, I think obsoleting the last patch should be enough, do you agree? > > I thought the same but you suggested otherwise last week on IRC heh I double checked now :) Last patch obsoleted. Pushed to master for 23.05. Nice work everyone, thanks! Enhancement - not backporting to 22.11.x. Setting to Needs documenting. Nice work everyone! Not sure what needs documenting here. It seems to be only backend changes. I'll close, but feel free to add details if the manual needs to be changed. |