Bug 19166 - Add the ability to add adjustments to an invoice
Summary: Add the ability to add adjustments to an invoice
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature with 2 votes (vote)
Target Milestone: ---
Assignee: Nick Clemens
QA Contact: Martin Renvoize
URL:
Whiteboard:
Keywords: Manual, release-notes-needed
Depends on:
Blocks:
 
Reported: 2017-08-23 12:43 UTC by Nick Clemens
Modified: 2018-11-14 15:24 UTC (History)
20 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
This enhancement to acquisitions allows libraries to record adjustments to invoices. These may be based on feedback from a vendor, for example, a credit for returned books or damaged books, or a debit for extra service charges etc.


Attachments
Bug 19166 - Database changes (3.28 KB, patch)
2017-08-25 13:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Add the ability to add adjustments to an invoice (16.44 KB, patch)
2017-08-25 13:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Database changes (3.42 KB, patch)
2017-10-04 13:26 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Add the ability to add adjustments to an invoice (23.05 KB, patch)
2017-10-04 13:26 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered (4.92 KB, patch)
2017-10-04 13:26 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH - Schema updates (5.83 KB, patch)
2017-10-04 13:26 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered (5.19 KB, patch)
2017-10-10 16:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Database changes (3.42 KB, patch)
2018-01-10 18:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Add the ability to add adjustments to an invoice (23.15 KB, patch)
2018-01-10 18:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH - Schema updates (5.83 KB, patch)
2018-01-10 18:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered (5.19 KB, patch)
2018-01-10 18:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Database changes (3.43 KB, patch)
2018-03-12 15:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Add the ability to add adjustments to an invoice (23.18 KB, patch)
2018-03-12 15:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH - Schema updates (4.85 KB, patch)
2018-03-12 15:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166 - Unit tests for GetBudgetSpent and GetBudget ordered (5.19 KB, patch)
2018-03-12 15:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19116: Get correct value for encumbering when open for new lines (1.71 KB, patch)
2018-03-13 09:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Shipment cost is always added to 'spent' (1.93 KB, patch)
2018-03-15 16:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Database changes (3.42 KB, patch)
2018-03-30 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Add the ability to add adjustments to an invoice (23.17 KB, patch)
2018-03-30 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH - Schema updates (4.85 KB, patch)
2018-03-30 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered (5.19 KB, patch)
2018-03-30 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Get correct value for encumbering when open for new lines (1.71 KB, patch)
2018-03-30 13:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Shipment cost is always added to 'spent' (1.93 KB, patch)
2018-03-30 13:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes (4.81 KB, patch)
2018-03-30 13:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Database changes (3.57 KB, patch)
2018-04-23 15:53 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19166: Add the ability to add adjustments to an invoice (23.25 KB, patch)
2018-04-23 15:53 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH - Schema updates (5.00 KB, patch)
2018-04-23 15:53 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered (5.33 KB, patch)
2018-04-23 15:53 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19166: Get correct value for encumbering when open for new lines (1.86 KB, patch)
2018-04-23 15:53 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19166: (follow-up) Shipment cost is always added to 'spent' (2.08 KB, patch)
2018-04-23 15:53 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes (5.01 KB, patch)
2018-04-23 15:54 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 19166: Correct charset collate (2.04 KB, patch)
2018-04-25 12:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19166: Database changes (3.63 KB, patch)
2018-04-27 06:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: Add the ability to add adjustments to an invoice (23.30 KB, patch)
2018-04-27 06:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH - Schema updates (5.05 KB, patch)
2018-04-27 06:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered (5.39 KB, patch)
2018-04-27 06:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: Get correct value for encumbering when open for new lines (1.92 KB, patch)
2018-04-27 06:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: (follow-up) Shipment cost is always added to 'spent' (2.14 KB, patch)
2018-04-27 06:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes (5.07 KB, patch)
2018-04-27 06:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: Correct charset collate (2.91 KB, patch)
2018-04-27 06:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: Follow-up - Reformat the new adjustments markup (18.46 KB, patch)
2018-05-03 14:55 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19166: (follow-up) Reformat the new adjustments markup (18.52 KB, patch)
2018-05-03 22:03 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19166: Database changes (3.63 KB, patch)
2018-07-16 14:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Database changes (3.63 KB, patch)
2018-07-16 14:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Add the ability to add adjustments to an invoice (23.30 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered (5.39 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Get correct value for encumbering when open for new lines (1.92 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Shipment cost is always added to 'spent' (2.14 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes (5.07 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Correct charset collate (2.91 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Reformat the new adjustments markup (18.71 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Adjust table and files and QA issues (22.93 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH - Schema updates (3.47 KB, patch)
2018-07-16 14:29 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Database changes (3.63 KB, patch)
2018-07-19 13:11 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Database changes (3.63 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Add the ability to add adjustments to an invoice (28.93 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered (5.39 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Get correct value for encumbering when open for new lines (1.92 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Shipment cost is always added to 'spent' (2.14 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes (5.07 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Correct charset collate (2.91 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Reformat the new adjustments markup (18.71 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Adjust table and files and QA issues (22.93 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH Schema updates (3.47 KB, patch)
2018-07-19 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Database changes (3.63 KB, patch)
2018-07-19 13:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Add the ability to add adjustments to an invoice (23.62 KB, patch)
2018-07-19 13:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered (5.39 KB, patch)
2018-07-19 13:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Get correct value for encumbering when open for new lines (1.92 KB, patch)
2018-07-19 13:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Shipment cost is always added to 'spent' (2.14 KB, patch)
2018-07-19 13:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes (5.07 KB, patch)
2018-07-19 13:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Correct charset collate (2.91 KB, patch)
2018-07-19 13:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Reformat the new adjustments markup (18.71 KB, patch)
2018-07-19 13:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: (follow-up) Adjust table and files and QA issues (17.63 KB, patch)
2018-07-19 13:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH Schema updates (3.47 KB, patch)
2018-07-19 13:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19166: Database changes (3.73 KB, patch)
2018-07-19 13:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: Add the ability to add adjustments to an invoice (23.69 KB, patch)
2018-07-19 13:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: Unit tests for GetBudgetSpent and GetBudget ordered (5.45 KB, patch)
2018-07-19 13:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: Get correct value for encumbering when open for new lines (1.99 KB, patch)
2018-07-19 13:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: (follow-up) Shipment cost is always added to 'spent' (2.21 KB, patch)
2018-07-19 13:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: (follow-up) Add ADJ_REASON auhtorised value category and minor fixes (5.15 KB, patch)
2018-07-19 13:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: Correct charset collate (3.00 KB, patch)
2018-07-19 13:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: (follow-up) Reformat the new adjustments markup (18.78 KB, patch)
2018-07-19 13:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: (follow-up) Adjust table and files and QA issues (17.88 KB, patch)
2018-07-19 13:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: DO NOT PUSH Schema updates (3.53 KB, patch)
2018-07-19 13:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19166: Remove 'IF NOT EXISTS' in kohastructure (1.35 KB, patch)
2018-07-31 11:31 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2017-08-23 12:43:42 UTC
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.)
Comment 1 Nick Clemens 2017-08-25 13:14:12 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2017-08-25 13:14:15 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2017-08-25 13:17:58 UTC
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
Comment 4 Katrin Fischer 2017-09-20 21:00:40 UTC
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.
Comment 5 Katrin Fischer 2017-09-20 21:01:36 UTC
One more addition: This is a much asked for and needed feature - please keep it going!
Comment 6 Katrin Fischer 2017-09-20 21:16:04 UTC
Looking at bug 10412: 
Could we imagine a use case where the credit note is independent from an invoice?
Comment 7 Nick Clemens 2017-09-26 10:33:30 UTC
(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.
Comment 8 Colin Campbell 2017-09-27 11:08:50 UTC
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)
Comment 9 Nick Clemens 2017-10-04 13:26:26 UTC Comment hidden (obsolete)
Comment 10 Nick Clemens 2017-10-04 13:26:30 UTC Comment hidden (obsolete)
Comment 11 Nick Clemens 2017-10-04 13:26:34 UTC Comment hidden (obsolete)
Comment 12 Nick Clemens 2017-10-04 13:26:38 UTC Comment hidden (obsolete)
Comment 13 Nick Clemens 2017-10-04 13:28:02 UTC
(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?
Comment 14 Katrin Fischer 2017-10-07 18:31:22 UTC
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
Comment 15 Nick Clemens 2017-10-10 16:51:54 UTC Comment hidden (obsolete)
Comment 16 Caroline Cyr La Rose 2017-11-03 20:39:21 UTC
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
Comment 17 Nick Clemens 2017-11-04 02:01:03 UTC
(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.
Comment 18 Jon Knight 2017-12-18 16:56:08 UTC
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
Comment 19 Nick Clemens 2018-01-10 18:34:53 UTC Comment hidden (obsolete)
Comment 20 Nick Clemens 2018-01-10 18:34:57 UTC Comment hidden (obsolete)
Comment 21 Nick Clemens 2018-01-10 18:35:01 UTC Comment hidden (obsolete)
Comment 22 Nick Clemens 2018-01-10 18:35:05 UTC Comment hidden (obsolete)
Comment 23 Victor Grousset/tuxayo 2018-03-12 15:15:32 UTC
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
Comment 24 Nick Clemens 2018-03-12 15:18:28 UTC Comment hidden (obsolete)
Comment 25 Nick Clemens 2018-03-12 15:18:34 UTC Comment hidden (obsolete)
Comment 26 Nick Clemens 2018-03-12 15:18:39 UTC Comment hidden (obsolete)
Comment 27 Nick Clemens 2018-03-12 15:18:44 UTC Comment hidden (obsolete)
Comment 28 Koha Team Lyon 3 2018-03-12 16:20:05 UTC
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
Comment 29 Nick Clemens 2018-03-13 09:18:21 UTC Comment hidden (obsolete)
Comment 30 Nick Clemens 2018-03-15 16:22:56 UTC Comment hidden (obsolete)
Comment 31 Caroline Cyr La Rose 2018-03-23 19:22:17 UTC
What is the authorized value category for the "reason" column?
Comment 32 Nick Clemens 2018-03-30 13:41:45 UTC Comment hidden (obsolete)
Comment 33 Nick Clemens 2018-03-30 13:41:50 UTC Comment hidden (obsolete)
Comment 34 Nick Clemens 2018-03-30 13:41:55 UTC Comment hidden (obsolete)
Comment 35 Nick Clemens 2018-03-30 13:41:59 UTC Comment hidden (obsolete)
Comment 36 Nick Clemens 2018-03-30 13:42:06 UTC Comment hidden (obsolete)
Comment 37 Nick Clemens 2018-03-30 13:42:10 UTC Comment hidden (obsolete)
Comment 38 Nick Clemens 2018-03-30 13:42:14 UTC Comment hidden (obsolete)
Comment 39 Nick Clemens 2018-03-30 13:44:02 UTC
(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
Comment 40 sandboxes@biblibre.com 2018-04-23 15:52:12 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 41 sandboxes@biblibre.com 2018-04-23 15:53:36 UTC Comment hidden (obsolete)
Comment 42 sandboxes@biblibre.com 2018-04-23 15:53:40 UTC Comment hidden (obsolete)
Comment 43 sandboxes@biblibre.com 2018-04-23 15:53:45 UTC Comment hidden (obsolete)
Comment 44 sandboxes@biblibre.com 2018-04-23 15:53:50 UTC Comment hidden (obsolete)
Comment 45 sandboxes@biblibre.com 2018-04-23 15:53:54 UTC Comment hidden (obsolete)
Comment 46 sandboxes@biblibre.com 2018-04-23 15:53:58 UTC Comment hidden (obsolete)
Comment 47 sandboxes@biblibre.com 2018-04-23 15:54:03 UTC Comment hidden (obsolete)
Comment 48 Katrin Fischer 2018-04-25 06:10:12 UTC
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?
Comment 49 Jonathan Druart 2018-04-25 12:56:09 UTC Comment hidden (obsolete)
Comment 50 Katrin Fischer 2018-04-27 05:42:47 UTC
Thx Jonathan!
Comment 51 Katrin Fischer 2018-04-27 06:19:52 UTC
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.
Comment 52 Katrin Fischer 2018-04-27 06:23:50 UTC
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>
Comment 53 Katrin Fischer 2018-04-27 06:23:55 UTC
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>
Comment 54 Katrin Fischer 2018-04-27 06:24:01 UTC
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>
Comment 55 Katrin Fischer 2018-04-27 06:24:06 UTC
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>
Comment 56 Katrin Fischer 2018-04-27 06:24:11 UTC
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>
Comment 57 Katrin Fischer 2018-04-27 06:24:17 UTC
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>
Comment 58 Katrin Fischer 2018-04-27 06:24:22 UTC
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>
Comment 59 Katrin Fischer 2018-04-27 06:24:27 UTC
Created attachment 74900 [details] [review]
Bug 19166: Correct charset collate

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 60 Owen Leonard 2018-05-03 14:55:52 UTC
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.
Comment 61 Katrin Fischer 2018-05-03 22:03:02 UTC
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>
Comment 62 Martin Renvoize 2018-06-29 14:00:04 UTC
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?
Comment 63 Martin Renvoize 2018-06-29 15:44:51 UTC
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.
Comment 64 Martin Renvoize 2018-06-29 15:55:11 UTC
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.
Comment 65 Martin Renvoize 2018-07-03 07:30:51 UTC
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...
Comment 66 Martin Renvoize 2018-07-03 08:09:30 UTC
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.
Comment 67 Nick Clemens 2018-07-16 14:28:02 UTC
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>
Comment 68 Nick Clemens 2018-07-16 14:28:59 UTC
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>
Comment 69 Nick Clemens 2018-07-16 14:29:06 UTC
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>
Comment 70 Nick Clemens 2018-07-16 14:29:12 UTC
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>
Comment 71 Nick Clemens 2018-07-16 14:29:17 UTC
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>
Comment 72 Nick Clemens 2018-07-16 14:29:24 UTC
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>
Comment 73 Nick Clemens 2018-07-16 14:29:30 UTC
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>
Comment 74 Nick Clemens 2018-07-16 14:29:37 UTC
Created attachment 77001 [details] [review]
Bug 19166: Correct charset collate

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 75 Nick Clemens 2018-07-16 14:29:44 UTC
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>
Comment 76 Nick Clemens 2018-07-16 14:29:50 UTC
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
Comment 77 Nick Clemens 2018-07-16 14:29:56 UTC
Created attachment 77004 [details] [review]
Bug 19166: DO NOT PUSH - Schema updates
Comment 78 Nick Clemens 2018-07-16 14:31:16 UTC
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
Comment 79 Nick Clemens 2018-07-19 13:11:14 UTC
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>
Comment 80 Nick Clemens 2018-07-19 13:12:03 UTC
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>
Comment 81 Nick Clemens 2018-07-19 13:12:09 UTC
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>
Comment 82 Nick Clemens 2018-07-19 13:12:14 UTC
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>
Comment 83 Nick Clemens 2018-07-19 13:12:21 UTC
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>
Comment 84 Nick Clemens 2018-07-19 13:12:26 UTC
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>
Comment 85 Nick Clemens 2018-07-19 13:12:32 UTC
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>
Comment 86 Nick Clemens 2018-07-19 13:12:38 UTC
Created attachment 77111 [details] [review]
Bug 19166: Correct charset collate

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 87 Nick Clemens 2018-07-19 13:12:44 UTC
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>
Comment 88 Nick Clemens 2018-07-19 13:12:51 UTC
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
Comment 89 Nick Clemens 2018-07-19 13:12:56 UTC
Created attachment 77114 [details] [review]
Bug 19166: DO NOT PUSH Schema updates
Comment 90 Nick Clemens 2018-07-19 13:30:26 UTC
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>
Comment 91 Nick Clemens 2018-07-19 13:30:32 UTC
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>
Comment 92 Nick Clemens 2018-07-19 13:30:38 UTC
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>
Comment 93 Nick Clemens 2018-07-19 13:30:44 UTC
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>
Comment 94 Nick Clemens 2018-07-19 13:30:50 UTC
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>
Comment 95 Nick Clemens 2018-07-19 13:30:56 UTC
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>
Comment 96 Nick Clemens 2018-07-19 13:31:02 UTC
Created attachment 77121 [details] [review]
Bug 19166: Correct charset collate

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 97 Nick Clemens 2018-07-19 13:31:08 UTC
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>
Comment 98 Nick Clemens 2018-07-19 13:31:14 UTC
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
Comment 99 Nick Clemens 2018-07-19 13:31:20 UTC
Created attachment 77124 [details] [review]
Bug 19166: DO NOT PUSH Schema updates
Comment 100 Martin Renvoize 2018-07-19 13:49:37 UTC
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>
Comment 101 Martin Renvoize 2018-07-19 13:49:43 UTC
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>
Comment 102 Martin Renvoize 2018-07-19 13:49:49 UTC
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>
Comment 103 Martin Renvoize 2018-07-19 13:49:54 UTC
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>
Comment 104 Martin Renvoize 2018-07-19 13:50:00 UTC
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>
Comment 105 Martin Renvoize 2018-07-19 13:50:06 UTC
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>
Comment 106 Martin Renvoize 2018-07-19 13:50:12 UTC
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>
Comment 107 Martin Renvoize 2018-07-19 13:50:19 UTC
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>
Comment 108 Martin Renvoize 2018-07-19 13:50:25 UTC
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>
Comment 109 Martin Renvoize 2018-07-19 13:50:31 UTC
Created attachment 77134 [details] [review]
Bug 19166: DO NOT PUSH Schema updates

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 110 Martin Renvoize 2018-07-19 13:51:33 UTC
Code all looks good now, tests all pass and qa script is happy.  All my testing seems solid too, passing QA.
Comment 111 Nick Clemens 2018-07-19 17:36:03 UTC
Awesome work all!

Pushed to master for 18.11
Comment 112 Jonathan Druart 2018-07-31 11:31:52 UTC
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.
Comment 113 Tomás Cohen Arazi 2018-07-31 13:46:08 UTC
Follow-up pushed to master. Gracias, Jonathan!
Comment 114 Martin Renvoize 2018-08-02 14:16:00 UTC
Enhancement, will not be backported to 18.05.x series.
Comment 115 Katrin Fischer 2018-09-24 05:33:24 UTC
Maybe we could promote this to 'new feature' to hightlight in release notes?