Bug 15878 - C4::Barcodes::hbyymmincr inccorectly calculates max and should warn when no branchcode present
Summary: C4::Barcodes::hbyymmincr inccorectly calculates max and should warn when no b...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 16561
Blocks: 14752
  Show dependency treegraph
 
Reported: 2016-02-22 13:49 UTC by Nick Clemens
Modified: 2016-12-05 21:25 UTC (History)
4 users (show)

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


Attachments
Bug 15878 - Updated unit tests for hbyymmincr barcodes (6.71 KB, patch)
2016-05-13 18:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15878 - Updated unit tests for hbyymmincr barcodes (6.71 KB, patch)
2016-05-13 18:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15878 - C4::Barcodes::hbyymmincr inccorectly calculates max and should warn when no branchcode present (2.01 KB, patch)
2016-05-13 18:06 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15878 - Updated unit tests for hbyymmincr barcodes (6.79 KB, patch)
2016-05-19 14:31 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15878 - C4::Barcodes::hbyymmincr inccorectly calculates max and should warn when no branchcode present (2.17 KB, patch)
2016-05-19 14:31 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 15878 - QA Followup (1.28 KB, patch)
2016-05-21 23:43 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15878 - Updated unit tests for hbyymmincr barcodes (6.73 KB, patch)
2016-05-22 00:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15878 - C4::Barcodes::hbyymmincr inccorectly calculates max and should warn when no branchcode present (2.16 KB, patch)
2016-05-22 00:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15878 - QA Followup (1.68 KB, patch)
2016-05-22 00:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15878 - Updated unit tests for hbyymmincr barcodes (6.87 KB, patch)
2016-05-22 07:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15878 - C4::Barcodes::hbyymmincr inccorectly calculates max and should warn when no branchcode present (2.23 KB, patch)
2016-05-22 07:25 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 2016-02-22 13:49:23 UTC
When creating a new hbyymmincr barcode no branch is used if none is passed in.  If the format requires hb=homebranch it shouldn't be able to be created without one.

To recreate:
1 - prove -v t/db_dependent/Barcodes.t
2 - observe that in output above no hb component is found in the generated barcodes

This works in cataloguing/value_builder/barcode.pl because a branchcode is passed
Comment 1 Nick Clemens 2016-02-22 13:52:00 UTC
db_max for this format also has a similar problem to 'annual' in bug 15877
Comment 2 Nick Clemens 2016-05-13 18:06:05 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2016-05-13 18:06:37 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens 2016-05-13 18:06:42 UTC Comment hidden (obsolete)
Comment 5 Bernardo Gonzalez Kriegel 2016-05-19 14:31:40 UTC Comment hidden (obsolete)
Comment 6 Bernardo Gonzalez Kriegel 2016-05-19 14:31:53 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-05-21 12:19:57 UTC
Comment on attachment 51624 [details] [review]
[SIGNED-OFF] Bug 15878 - Updated unit tests for hbyymmincr barcodes

Review of attachment 51624 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/Barcodes.t
@@ +84,4 @@
>  };
>  
>  
> +$schema->storage->txn_begin;

There is an issue (existed before this patch), the transaction should begin in the subtests.

@@ +96,5 @@
> +            source => 'Branch',
> +            branchcode => 'LETTERS',
> +            });
> +
> +    my $branchcode_1 = "LETT";

LETT vs LETTERS?
$barcode_branch is never used later.

@@ +97,5 @@
> +            branchcode => 'LETTERS',
> +            });
> +
> +    my $branchcode_1 = "LETT";
> +    print "$branchcode_1\n";

Unused debug statement.
Comment 8 Nick Clemens 2016-05-21 23:43:03 UTC Comment hidden (obsolete)
Comment 9 Nick Clemens 2016-05-22 00:20:32 UTC Comment hidden (obsolete)
Comment 10 Nick Clemens 2016-05-22 00:20:50 UTC Comment hidden (obsolete)
Comment 11 Nick Clemens 2016-05-22 00:20:55 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-05-22 07:25:00 UTC
Created attachment 51694 [details] [review]
Bug 15878 - Updated unit tests for hbyymmincr barcodes

Test plan in second patch

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 15878 - QA Followup

* Remove debug statment
* Move transcation start
* Remove unused testbuilder object

Rebased on top of 16561

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2016-05-22 07:25:07 UTC
Created attachment 51695 [details] [review]
Bug 15878 - C4::Barcodes::hbyymmincr inccorectly calculates max and should warn when no branchcode present

Test plan:

1 - prove t/db_dependent/Barcodes.t
2 - All should pass
3 - Apply first patch (unit tests update)
    4 - Tests should fail on values and warnings
    5 - Apply second patch
    6 - All tests should now pass

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described
Removed tab on line 47 of C4/Barcodes/hbyymmincr.pm
No more qa errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2016-05-22 07:25:42 UTC
I have squashed 1st & 3rd for the readability.
Thanks for the quick patch Nick!
Comment 15 Kyle M Hall 2016-05-23 17:49:55 UTC
Pushed to master for 16.05, thanks Nick!
Comment 16 Julian Maurice 2016-05-25 09:07:27 UTC
Patches pushed to 3.22.x, will be in 3.22.7