The goal of this enhancement is to allow libraries to make adjustments to invoices based on feedback form a vendor (credit for returned books, damaged books, or fees for extra charges etc.)
Created attachment 66475 [details] [review] Bug 19166 - Database changes
Created attachment 66476 [details] [review] Bug 19166 - Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct
This needs tests and more work (rollover etc.) but I wanted to get any initial opinions on the intended functionality. Probably should be Koha::Acquisitions::Invoices::Adjustments
Hi Nick, I know it's a somewhat early draft, hope the comments are still helpful: - Maybe the reason should be an authorised values list. Or a list in combination with a notes field? - I am not sure about the use case for encumber now/encumber later. Do you have a bit more information about this requirement maybe? (please take a look at my comments and tests on bug 18999) - If I choose "encumber while open" - should it show up as "spent"? And otherwise as "ordered"? It only seems to be taken into account on the acq start page right now when the checkbox is checked. - Budget should be fund in the form (the code is confusing here as budget and fund were switched in the GUI after the new acq module was written). - I think a delete link or button might be nicer than the checkbox - or have both in a nice table. - Adjustments can be stored currently without a fund, I think this needs some validation. - Adjustments can be positive or negative, right? (credit (-) and additional shipping/processing costs (+)?) So far this seems to work. - Format prices nicely using the TT plugin everywhere. - For the rollover things should be alright for the open orders as the new feature should not take effect there. But moving remaining funds over would have to take the actual amount into account.
One more addition: This is a much asked for and needed feature - please keep it going!
Looking at bug 10412: Could we imagine a use case where the credit note is independent from an invoice?
(In reply to Katrin Fischer from comment #4) > I know it's a somewhat early draft, hope the comments are still helpful: Yes! Good to get feedback early :-) > - Maybe the reason should be an authorised values list. Or a list in > combination with a notes field? The first I planned as a later enhancement, but the second is interesting, I will take a look > - I am not sure about the use case for encumber now/encumber later. Do you > have a bit more information about this requirement maybe? (please take a > look at my comments and tests on bug 18999) Yes, 'encumber while open' decide if your credits affect the invoice when open, or only when closed. If 'encumber while open' is checked then adjustments should show in 'ordered' until invoice is closed - once it is closed they move to 'spent' If not 'encumber while open' the adjustments have no effect on 'ordered' while invoice is open and move to 'spent' when closed > - Budget should be fund in the form Yup, will fix > - I think a delete link or button might be nicer than the checkbox - or have > both in a nice table. I am working on improving the GUI > - Adjustments can be stored currently without a fund, I think this needs > some validation. This was to mirror 'Shipping cost' which may not have a fund, it seems the value is not deducted when this is used? It can be removed if not necessary > - Adjustments can be positive or negative, right? (credit (-) and additional > shipping/processing costs (+)?) So far this seems to work. Yes, the intention is to allow corrections of any kind to the invoice > - Format prices nicely using the TT plugin everywhere. Will fix > - For the rollover things should be alright for the open orders as the new > feature should not take effect there. But moving remaining funds over would > have to take the actual amount into account. Hmmm...as I understand the process a rollover only moves unreceived orders - we are dealing with things at the invoice level - any invoices/adjustments should be closed and affect the old budget only - if you have a remaning credit from an invoice that should end up in the 'move remaining funds' and the adjustments should be taken into account there. I think that should work right now > This is a much asked for and needed feature - please keep it going! Agreed :-) > Looking at bug 10412: Could we imagine a use case where the credit note is independent from an invoice? I think these achieve similar ends by different means - an invoice with adjustments and no orders seems equivalent to a free standing credit - it seemed logical to have an invoice for any monetary adjustment and if that bug is resurrected it could be done without the option to link to invoices and be compatible with this.
In an EDI environment credit note messages are a type of invoice message, and we'd have to drive the creation of th Adjustment from there. With that case in mind (but also with manual cases) It would be useful to have a field similar to the invoicenumber in the aqinvoice table to record the suppliers document number (mandatory in edifact where its a max 35 character field)
Created attachment 67599 [details] [review] Bug 19166 - Database changes
Created attachment 67600 [details] [review] Bug 19166 - Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Conflicts: C4/Budgets.pm
Created attachment 67601 [details] [review] Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green
Created attachment 67602 [details] [review] Bug 19166: DO NOT PUSH - Schema updates Included to ease testing :-)
(In reply to Colin Campbell from comment #8) > In an EDI environment credit note messages are a type of invoice message, > and we'd have to drive the creation of th Adjustment from there. With that > case in mind (but also with manual cases) It would be useful to have a field > similar to the invoicenumber in the aqinvoice table to record the suppliers > document number > (mandatory in edifact where its a max 35 character field) Do you want this field simply added to the DB changes for future use? Does it need to be displayed/editable?
Sorry, tests are failing: t/db_dependent/Budgets.t .. 2/145 # Failed test 'total ordered price is 20' # at t/db_dependent/Budgets.t line 475. # got: '26' # expected: '20.000000' t/db_dependent/Budgets.t .. 99/145 # Looks like you failed 1 test of 145. t/db_dependent/Budgets.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/145 subtests Test Summary Report ------------------- t/db_dependent/Budgets.t (Wstat: 256 Tests: 145 Failed: 1) Failed test: 65 Non-zero exit status: 1 Files=1, Tests=145, 4 wallclock secs ( 0.04 usr 0.00 sys + 3.57 cusr 0.36 csys = 3.97 CPU) Result: FAIL
Created attachment 67890 [details] [review] Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green
I want this so much to work! I've been asked for this many many times! But I got a software error. BIx::Class::Storage::DBI::_dbh_execute(): Table 'koha_master_dev_inlibro.invoice_adjustments' doesn't exist at /inlibro/git/koha-master-dev-inlibro/Koha/Object.pm line 121 Plus the prove -v ... returned red. DBD::mysql::st execute failed: Table 'koha_master_dev_inlibro.invoice_adjustments' doesn't exist [for Statement "SELECT `me`.`adjustment_id`, `me`.`invoiceid`, `me`.`adjustment`, `me`.`reason`, `me`.`note`, `me`.`budget_id`, `me`.`encumber_open`, `me`.`timestamp` FROM `invoice_adjustments` `me` JOIN `aqinvoices` `invoiceid` ON `invoiceid`.`invoiceid` = `me`.`invoiceid` WHERE ( ( `budget_id` = ? AND `closedate` IS NOT NULL ) )" with ParamValues: 0='6'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1765. DBIx::Class::Storage::DBI::_dbh_execute(): Table 'koha_master_dev_inlibro.invoice_adjustments' doesn't exist at /inlibro/git/koha-master-dev-inlibro/Koha/Objects.pm line 209 # Looks like your test exited with 255 just after 60. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 85/145 subtests Test Summary Report ------------------- t/db_dependent/Budgets.t (Wstat: 65280 Tests: 60 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 145 tests but ran 60. Files=1, Tests=60, 1 wallclock secs ( 0.02 usr 0.00 sys + 1.49 cusr 0.08 csys = 1.59 CPU) Result: FAIL
(In reply to Caroline Cyr La Rose from comment #16) > I want this so much to work! I've been asked for this many many times! > > But I got a software error. > > BIx::Class::Storage::DBI::_dbh_execute(): Table > 'koha_master_dev_inlibro.invoice_adjustments' doesn't exist at > /inlibro/git/koha-master-dev-inlibro/Koha/Object.pm line 121 > > Plus the prove -v ... returned red. HI Caroline, Did you run the database updates? It looks like the tables introduced in this patchset have not been created.
I have applied the database changes on my kohadevbox and I get this from `prove -v t/db_dependent/Budgets.t` (first 128 tests of the 145 are OK, so looks like test 129 is blowing up, so I won't include all the output for brevity here): ok 127 - MoveOrders updated the budget amount with the previous unspent budget (for budget budget_2) ok 128 - MoveOrders updated the budget amount with the previous unspent budget (for budget budget_21) DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`account_offsets`, CONSTRAINT `account_offsets_ibfk_t` FOREIGN KEY (`type`) REFERENCES `account_offset_types` (`type`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO `account_offsets` ( `amount`, `debit_id`, `type`) VALUES ( ?, ?, ? )" with ParamValues: 0=173079442325239000000.000000, 1='1', 2='Manual Debit'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. DBIx::Class::Storage::DBI::_dbh_execute(): Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`account_offsets`, CONSTRAINT `account_offsets_ibfk_t` FOREIGN KEY (`type`) REFERENCES `account_offset_types` (`type`) ON DELETE CASCADE ON UPDATE CASCADE) at /home/vagrant/kohaclone/Koha/Object.pm line 122 # Looks like your test exited with 255 just after 128. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 17/145 subtests Test Summary Report ------------------- t/db_dependent/Budgets.t (Wstat: 65280 Tests: 128 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 145 tests but ran 128. Files=1, Tests=128, 5 wallclock secs ( 0.04 usr 0.02 sys + 3.98 cusr 0.44 csys = 4.48 CPU) Result: FAIL
Created attachment 70413 [details] [review] Bug 19166 - Database changes
Created attachment 70414 [details] [review] Bug 19166 - Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Conflicts: C4/Budgets.pm
Created attachment 70415 [details] [review] Bug 19166: DO NOT PUSH - Schema updates Included to ease testing :-)
Created attachment 70416 [details] [review] Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green
conflicts ~/d/koha ❯❯❯ git bz apply 19166 Bug 19166 - Add the ability to add adjustments to an invoice 70413 - Bug 19166 - Database changes 70414 - Bug 19166 - Add the ability to add adjustments to an invoice 70415 - Bug 19166: DO NOT PUSH - Schema updates 70416 - Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 19166 - Database changes Using index info to reconstruct a base tree... M installer/data/mysql/kohastructure.sql Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/kohastructure.sql Applying: Bug 19166 - Add the ability to add adjustments to an invoice Using index info to reconstruct a base tree... M acqui/invoice.pl M acqui/ordered.pl M acqui/spent.pl M koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt M koha-tmpl/intranet-tmpl/prog/en/modules/acqui/ordered.tt M koha-tmpl/intranet-tmpl/prog/en/modules/acqui/spent.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/spent.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/ordered.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt Auto-merging acqui/spent.pl Auto-merging acqui/ordered.pl Auto-merging acqui/invoice.pl Applying: Bug 19166: DO NOT PUSH - Schema updates Using index info to reconstruct a base tree... M Koha/Schema/Result/Aqbudget.pm M Koha/Schema/Result/Aqinvoice.pm Falling back to patching base and 3-way merge... Auto-merging Koha/Schema/Result/Aqinvoice.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Aqinvoice.pm Auto-merging Koha/Schema/Result/Aqbudget.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Aqbudget.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 19166: DO NOT PUSH - Schema updates The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-19166-DO-NOT-PUSH---Schema-updates-Wq9yGZ.patch
Created attachment 72702 [details] [review] Bug 19166 - Database changes
Created attachment 72703 [details] [review] Bug 19166 - Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Conflicts: C4/Budgets.pm
Created attachment 72704 [details] [review] Bug 19166: DO NOT PUSH - Schema updates Included to ease testing :-)
Created attachment 72705 [details] [review] Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green
I have very strange results on the acqui-home table. I put a order with 2 titles of each 20€ tax excluded with 1.10 GST I received my first title with a shipment cost of 15€ and add a negative adjustement (-10) on this one. When I added the adjustment, I didn't check "Encumber while invoice open" but it was checked when I save it. Adjustements are added to ordered orders but the sums in acqui-home page are wrong. Ordered : 26,10 Title 2 = 21,10 € Adjustement = -10 -> should be 11,10 € Spent : 36,10 Title 1 = 21,10 Shipment cost = 15 ->ok
Created attachment 72744 [details] [review] Bug 19116: Get correct value for encumbering when open for new lines
Created attachment 72978 [details] [review] Bug 19166: (follow-up) Shipment cost is always added to 'spent' Two koha professionals agreed, also it is current behvaiour so this patch removes a change
What is the authorized value category for the "reason" column?
Created attachment 73481 [details] [review] Bug 19166: Database changes
Created attachment 73482 [details] [review] Bug 19166: Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Conflicts: C4/Budgets.pm
Created attachment 73483 [details] [review] Bug 19166: DO NOT PUSH - Schema updates Included to ease testing :-)
Created attachment 73484 [details] [review] Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green
Created attachment 73485 [details] [review] Bug 19166: Get correct value for encumbering when open for new lines
Created attachment 73486 [details] [review] Bug 19166: (follow-up) Shipment cost is always added to 'spent' Two koha professionals agreed, also it is current behvaiour so this patch removes a change
Created attachment 73487 [details] [review] Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes This patch adds the ADJ_REASON authorised value category tot he atomic update, and fixes code to display a hint of no reasons are defined other minor updates to remove warns in logs To test: Apply patches Run updates Check authorised value categories to see ADJ_REASON exists Add an adjustment, not you have no reaosn drop down Note there is a hint if you hover Add a value to ADJ_REASON add another adjustment, note you can now add reasons (or not)
(In reply to Caroline Cyr La Rose from comment #31) > What is the authorized value category for the "reason" column? It is ADJ_REASON - I now add this in the updates and fixed the hint that displays so the user knows where to add reasons
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74742 [details] [review] Bug 19166: Database changes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74743 [details] [review] Bug 19166: Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Conflicts: C4/Budgets.pm Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74744 [details] [review] Bug 19166: DO NOT PUSH - Schema updates Included to ease testing :-) Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74745 [details] [review] Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74746 [details] [review] Bug 19166: Get correct value for encumbering when open for new lines https://bugs.koha-community.org/show_bug.cgi?id=19166 Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74747 [details] [review] Bug 19166: (follow-up) Shipment cost is always added to 'spent' Two koha professionals agreed, also it is current behvaiour so this patch removes a change Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 74748 [details] [review] Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes This patch adds the ADJ_REASON authorised value category tot he atomic update, and fixes code to display a hint of no reasons are defined other minor updates to remove warns in logs To test: Apply patches Run updates Check authorised value categories to see ADJ_REASON exists Add an adjustment, not you have no reaosn drop down Note there is a hint if you hover Add a value to ADJ_REASON add another adjustment, note you can now add reasons (or not) Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Hi Nick, some complaints in the QA tools, especially this one: FAIL installer/data/mysql/kohastructure.sql FAIL charset_collate The table invoice_adjustments does not have the current charset collate (see bug 18336) OK git manipulation Can you please take a look?
Created attachment 74843 [details] [review] Bug 19166: Correct charset collate
Thx Jonathan!
Nick, The GUI could still needs some improvement, but I think this could be done in a separate bug report, as logic and calculations appear to work ok. Some notes: I'd remove the reason column when the authorised category is not set up with values and remove the 'none' it displays now. Also, you show one line for each adjustment on an invoice in the spent and ordered tables. I'd either summarize them (one line for each invoice) or give more detail (reason and note). For an easier to handle table, maybe 1 is the better option. I'd move the save button from the Invoice closer to the invoice detail box to make it clearer that it only applies to this part of the page. The 2 separate save buttons are not ideal. A long note won't be displayed nicely in the notes field.
Created attachment 74893 [details] [review] Bug 19166: Database changes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74894 [details] [review] Bug 19166: Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Conflicts: C4/Budgets.pm Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74895 [details] [review] Bug 19166: DO NOT PUSH - Schema updates Included to ease testing :-) Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74896 [details] [review] Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74897 [details] [review] Bug 19166: Get correct value for encumbering when open for new lines https://bugs.koha-community.org/show_bug.cgi?id=19166 Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74898 [details] [review] Bug 19166: (follow-up) Shipment cost is always added to 'spent' Two koha professionals agreed, also it is current behvaiour so this patch removes a change Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74899 [details] [review] Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes This patch adds the ADJ_REASON authorised value category tot he atomic update, and fixes code to display a hint of no reasons are defined other minor updates to remove warns in logs To test: Apply patches Run updates Check authorised value categories to see ADJ_REASON exists Add an adjustment, not you have no reaosn drop down Note there is a hint if you hover Add a value to ADJ_REASON add another adjustment, note you can now add reasons (or not) Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 74900 [details] [review] Bug 19166: Correct charset collate Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 75014 [details] [review] Bug 19166: Follow-up - Reformat the new adjustments markup This follow-up makes some changes to the way invoice adjustments are displayed and added: - Move this section into a <fieldset> to set it off visually from the other sections of the page. - Take the "add adjustment" fields out of the table and display them in a standard way. - Hide the "add adjustment" fields by default, shown via "add an adjustment" link. - Add deletion confirmation to adjustment deletions. To test, add, delete, and change invoice adjustments as instructed in the original test plan.
Created attachment 75043 [details] [review] Bug 19166: (follow-up) Reformat the new adjustments markup This follow-up makes some changes to the way invoice adjustments are displayed and added: - Move this section into a <fieldset> to set it off visually from the other sections of the page. - Take the "add adjustment" fields out of the table and display them in a standard way. - Hide the "add adjustment" fields by default, shown via "add an adjustment" link. - Add deletion confirmation to adjustment deletions. To test, add, delete, and change invoice adjustments as instructed in the original test plan. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
QA Comment: Acquisitions is one of the few places where we're fairly consistent with table names (i.e. they have a prepended aq_).. did we want to follow that convention through here too?
Encumber is a really odd term.. I'm wondering where it came from.. even as a native english speaker I can't entirely work out what it means in this context.
I take that last comment back.. once I found the right accounting book to read it made a lot more sense.. though I'm not sure how well it translates to credits.. 'encumbered' tends to mean 'committed to paying', but a credit it sorta the opposite.
I agree this should be moved to Koha::Acquisitions::Invoices::Adjustments, InvoiceAdjustments without the prepended Acquisitions could well be confused with Accounts processing in the future...
When running the QA scripts I'm seeing a number of warnings: Argument "" isn't numeric in numeric gt (>) at /home/martin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/ordered.tt line 85. Argument "" isn't numeric in numeric gt (>) at /home/martin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt line 162. Argument "" isn't numeric in numeric gt (>) at /home/martin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/spent.tt line 85. Argument "" isn't numeric in numeric gt (>) at /home/martin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/spent.tt line 103. And also some POD warnings too.
Created attachment 76994 [details] [review] Bug 19166: Database changes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76995 [details] [review] Bug 19166: Database changes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76996 [details] [review] Bug 19166: Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Conflicts: C4/Budgets.pm Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76997 [details] [review] Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76998 [details] [review] Bug 19166: Get correct value for encumbering when open for new lines https://bugs.koha-community.org/show_bug.cgi?id=19166 Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 76999 [details] [review] Bug 19166: (follow-up) Shipment cost is always added to 'spent' Two koha professionals agreed, also it is current behvaiour so this patch removes a change Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77000 [details] [review] Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes This patch adds the ADJ_REASON authorised value category tot he atomic update, and fixes code to display a hint of no reasons are defined other minor updates to remove warns in logs To test: Apply patches Run updates Check authorised value categories to see ADJ_REASON exists Add an adjustment, not you have no reaosn drop down Note there is a hint if you hover Add a value to ADJ_REASON add another adjustment, note you can now add reasons (or not) Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77001 [details] [review] Bug 19166: Correct charset collate Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77002 [details] [review] Bug 19166: (follow-up) Reformat the new adjustments markup This follow-up makes some changes to the way invoice adjustments are displayed and added: - Move this section into a <fieldset> to set it off visually from the other sections of the page. - Take the "add adjustment" fields out of the table and display them in a standard way. - Hide the "add adjustment" fields by default, shown via "add an adjustment" link. - Add deletion confirmation to adjustment deletions. To test, add, delete, and change invoice adjustments as instructed in the original test plan. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 77003 [details] [review] Bug 19166: (follow-up) Adjust table and files and QA issues Move to aqinvoice_adjustment Move to Koha::Acquisition::Invoice::Adjustments Test if variable exists before count
Created attachment 77004 [details] [review] Bug 19166: DO NOT PUSH - Schema updates
QA tool will warn of deleted/created in same patchset - pfft - can squash on push It also complains of pod on the Schema files - I think is a red herring TT warns cleared
Created attachment 77104 [details] [review] Bug 19166: Database changes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77105 [details] [review] Bug 19166: Database changes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77106 [details] [review] Bug 19166: Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77107 [details] [review] Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77108 [details] [review] Bug 19166: Get correct value for encumbering when open for new lines https://bugs.koha-community.org/show_bug.cgi?id=19166 Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77109 [details] [review] Bug 19166: (follow-up) Shipment cost is always added to 'spent' Two koha professionals agreed, also it is current behvaiour so this patch removes a change Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77110 [details] [review] Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes This patch adds the ADJ_REASON authorised value category tot he atomic update, and fixes code to display a hint of no reasons are defined other minor updates to remove warns in logs To test: Apply patches Run updates Check authorised value categories to see ADJ_REASON exists Add an adjustment, not you have no reaosn drop down Note there is a hint if you hover Add a value to ADJ_REASON add another adjustment, note you can now add reasons (or not) Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77111 [details] [review] Bug 19166: Correct charset collate Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77112 [details] [review] Bug 19166: (follow-up) Reformat the new adjustments markup This follow-up makes some changes to the way invoice adjustments are displayed and added: - Move this section into a <fieldset> to set it off visually from the other sections of the page. - Take the "add adjustment" fields out of the table and display them in a standard way. - Hide the "add adjustment" fields by default, shown via "add an adjustment" link. - Add deletion confirmation to adjustment deletions. To test, add, delete, and change invoice adjustments as instructed in the original test plan. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 77113 [details] [review] Bug 19166: (follow-up) Adjust table and files and QA issues Move to aqinvoice_adjustment Move to Koha::Acquisition::Invoice::Adjustments Test if variable exists before count
Created attachment 77114 [details] [review] Bug 19166: DO NOT PUSH Schema updates
Created attachment 77115 [details] [review] Bug 19166: Database changes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77116 [details] [review] Bug 19166: Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77117 [details] [review] Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77118 [details] [review] Bug 19166: Get correct value for encumbering when open for new lines https://bugs.koha-community.org/show_bug.cgi?id=19166 Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77119 [details] [review] Bug 19166: (follow-up) Shipment cost is always added to 'spent' Two koha professionals agreed, also it is current behvaiour so this patch removes a change Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77120 [details] [review] Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes This patch adds the ADJ_REASON authorised value category tot he atomic update, and fixes code to display a hint of no reasons are defined other minor updates to remove warns in logs To test: Apply patches Run updates Check authorised value categories to see ADJ_REASON exists Add an adjustment, not you have no reaosn drop down Note there is a hint if you hover Add a value to ADJ_REASON add another adjustment, note you can now add reasons (or not) Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77121 [details] [review] Bug 19166: Correct charset collate Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 77122 [details] [review] Bug 19166: (follow-up) Reformat the new adjustments markup This follow-up makes some changes to the way invoice adjustments are displayed and added: - Move this section into a <fieldset> to set it off visually from the other sections of the page. - Take the "add adjustment" fields out of the table and display them in a standard way. - Hide the "add adjustment" fields by default, shown via "add an adjustment" link. - Add deletion confirmation to adjustment deletions. To test, add, delete, and change invoice adjustments as instructed in the original test plan. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 77123 [details] [review] Bug 19166: (follow-up) Adjust table and files and QA issues Move to aqinvoice_adjustment Move to Koha::Acquisition::Invoice::Adjustments Test if variable exists before count
Created attachment 77124 [details] [review] Bug 19166: DO NOT PUSH Schema updates
Created attachment 77125 [details] [review] Bug 19166: Database changes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77126 [details] [review] Bug 19166: Add the ability to add adjustments to an invoice This patchset adds the ability to add adjustments to an invoice, one can provide a reason, an adjustment amount, select a budget, and choose whether to encumber the funds before the invoice is closed or not To test: 1 - Create a new invoice with or without a shipping cost 2 - Note there are no existing adjustments 3 - Add an adjustment 4 - Submit the form withno changes, nothing happens 5 - Update the adjustment you created, ensure changes are saved but no extra adjustment created 6 - Add another invoice prodiving only reason or amount (you can have 0 value adjustments) 7 - Verify the adjustment total at bottom is correct 8 - Recieve some orders 9 - Verify totals are correct Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77127 [details] [review] Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered To test: 1. prove -v t/db_dependent/Budgets.t 2. Should return green Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77128 [details] [review] Bug 19166: Get correct value for encumbering when open for new lines https://bugs.koha-community.org/show_bug.cgi?id=19166 Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77129 [details] [review] Bug 19166: (follow-up) Shipment cost is always added to 'spent' Two koha professionals agreed, also it is current behvaiour so this patch removes a change Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77130 [details] [review] Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes This patch adds the ADJ_REASON authorised value category tot he atomic update, and fixes code to display a hint of no reasons are defined other minor updates to remove warns in logs To test: Apply patches Run updates Check authorised value categories to see ADJ_REASON exists Add an adjustment, not you have no reaosn drop down Note there is a hint if you hover Add a value to ADJ_REASON add another adjustment, note you can now add reasons (or not) Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77131 [details] [review] Bug 19166: Correct charset collate Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77132 [details] [review] Bug 19166: (follow-up) Reformat the new adjustments markup This follow-up makes some changes to the way invoice adjustments are displayed and added: - Move this section into a <fieldset> to set it off visually from the other sections of the page. - Take the "add adjustment" fields out of the table and display them in a standard way. - Hide the "add adjustment" fields by default, shown via "add an adjustment" link. - Add deletion confirmation to adjustment deletions. To test, add, delete, and change invoice adjustments as instructed in the original test plan. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77133 [details] [review] Bug 19166: (follow-up) Adjust table and files and QA issues Move to aqinvoice_adjustment Move to Koha::Acquisition::Invoice::Adjustments Test if variable exists before count Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 77134 [details] [review] Bug 19166: DO NOT PUSH Schema updates Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Code all looks good now, tests all pass and qa script is happy. All my testing seems solid too, passing QA.
Awesome work all! Pushed to master for 18.11
Created attachment 77329 [details] [review] Bug 19166: Remove 'IF NOT EXISTS' in kohastructure The table is removed just before, this clause is useless. It also fixes a test: t/db_dependent/Creators/Lib.t .. 1/645 # Failed test 'get_table_names return all tables matching' # at t/db_dependent/Creators/Lib.t line 1258. # Structures begin differing at: # $got->[10] = 'aqinvoice_adjustments' # $expected->[10] = 'aqinvoices' # Looks like you failed 1 test of 645.
Follow-up pushed to master. Gracias, Jonathan!
Enhancement, will not be backported to 18.05.x series.
Maybe we could promote this to 'new feature' to hightlight in release notes?
*** Bug 10412 has been marked as a duplicate of this bug. ***