Bug 21584 - Wrong offset type for Lost Item
Summary: Wrong offset type for Lost Item
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 2696 20978
Blocks: 13098 21715
  Show dependency treegraph
 
Reported: 2018-10-16 18:13 UTC by Tomás Cohen Arazi
Modified: 2019-10-14 19:56 UTC (History)
5 users (show)

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


Attachments
Bug 21584: Wrong offset type for Lost Item (1.80 KB, patch)
2018-10-16 18:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21584: Wrong offset type for Lost Item (1.92 KB, patch)
2018-10-24 05:41 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21584: Wrong offset type for Lost Item (2.00 KB, patch)
2018-10-24 10:21 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2018-10-16 18:13:24 UTC
Bug 20978 tried to add a single source of truth for this values, but an error was introduced.
Comment 1 Tomás Cohen Arazi 2018-10-16 18:25:53 UTC
Created attachment 80676 [details] [review]
Bug 21584: Wrong offset type for Lost Item

Bug 2696 introduced account_offset_type.inc but 'Lost Item Return' doesn't match what is added on the database. Moreover, bug 20978 added references to this wrong value too in Koha::Account

This patch performs a trivial fix for this.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Pierre-Marc Thibault 2018-10-19 18:39:38 UTC
Can we have a test plan to validate that both changes are effective?
Comment 3 Tomás Cohen Arazi 2018-10-23 15:17:53 UTC
The original bug is in:

koha-tmpl/intranet-tmpl/prog/en/includes/account_offset_type.inc

it is not easy to 'reproduce', as the code that should generate credit return offsets is not working properly (bug 13098). This is my attempt:

- View the following file: installer/data/mysql/account_offset_types.sql
  $ less installer/data/mysql/account_offset_types.sql
- Notice there's 'Lost Item' in line 4
- View koha-tmpl/intranet-tmpl/prog/en/includes/account_offset_type.inc
  $ less koha-tmpl/intranet-tmpl/prog/en/includes/account_offset_type.inc
- Notice it maps the values from the account_offset_types.sql file into english descriptions.
=> FAIL: There's no mapping for 'Lost Item', but there's a mapping for 'Lost Item Return', which doesn't really exist in account_offset_types.sql
- Apply the patch
=> SUCCESS: The value <-> description mapping in account_offset_type.inc makes sense now. It respects the values that would be stored on the DB if the feature worked correctly.
=> BONUS: Koha/Account.pm gets fixed to generate the right value too. Notice there's a hash with mappings for some keywords into both account_offset_types and account types.
Comment 4 Alex Buckley 2018-10-24 05:41:44 UTC
Created attachment 81048 [details] [review]
Bug 21584: Wrong offset type for Lost Item

Bug 2696 introduced account_offset_type.inc but 'Lost Item Return' doesn't match what is added on the database. Moreover, bug 20978 added references to this wrong value too in Koha::Account

This patch performs a trivial fix for this.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Patch matches what was outlined in test plan, also passes qa test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 5 Kyle M Hall 2018-10-24 10:21:28 UTC
Created attachment 81057 [details] [review]
Bug 21584: Wrong offset type for Lost Item

Bug 2696 introduced account_offset_type.inc but 'Lost Item Return' doesn't match what is added on the database. Moreover, bug 20978 added references to this wrong value too in Koha::Account

This patch performs a trivial fix for this.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Patch matches what was outlined in test plan, also passes qa test tool

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Nick Clemens 2018-10-25 19:09:02 UTC
Awesome work all!

Pushed to master for 18.11
Comment 7 Martin Renvoize 2018-11-06 10:29:15 UTC
Causation bug 20978 not in 18.05.x series.