Bug 18889 - Add barcode to 'Pay Fines' tab in patrons payment page
Summary: Add barcode to 'Pay Fines' tab in patrons payment page
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P1 - high enhancement with 2 votes (vote)
Assignee: Alex Buckley
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-03 09:31 UTC by vimal kumar
Modified: 2019-03-13 21:47 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Fines (70.20 KB, image/png)
2017-07-03 09:31 UTC, vimal kumar
Details
Bug 18889 - Storing and display of barcode with fines in intranet (5.26 KB, patch)
2017-11-27 00:25 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18889 - Storing and display of barcode with fines in intranet (5.22 KB, patch)
2018-03-28 03:25 UTC, Alex Buckley
Details | Diff | Splinter Review
Patch applies cleanly (34.87 KB, image/png)
2018-03-28 03:49 UTC, Alex Buckley
Details
Circled barcode in the pay fines table (21.24 KB, image/png)
2018-03-28 03:49 UTC, Alex Buckley
Details
Bug 18889: Simpler counter-patch (1.78 KB, patch)
2018-04-02 01:33 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 18889: Simpler counter-patch (1.90 KB, patch)
2018-05-09 02:38 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18889: Simpler counter-patch (1.96 KB, patch)
2018-05-09 03:01 UTC, Amit Gupta
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description vimal kumar 2017-07-03 09:31:01 UTC
Created attachment 64772 [details]
Fines

Dear Friends,
Very useful to see the item number (barcode) along with bibliographic details in fines page. If barcode appears along with book details easy identify the specific book. For example, in law libraries books with the same title by many authors (e.g. Constitution of India). Please see the screenshot.
Comment 1 Alex Buckley 2017-11-27 00:25:35 UTC
Created attachment 69352 [details] [review]
Bug 18889 - Storing and display of barcode with fines in intranet

Test plan:
1. Create a manual invoice fine making sure you enter in a barcode value

2. Go to the 'Pay fines' tab and notice that the barcode value is not
displayed

3. Apply patch

4. Update your database by running the updatedatabase.pl script in
koha-shell

This will run the atomicupdate file
bug_18889_added_barcode_field_to_accountlines.sql which contains the
database changes

5. Update your DBIx by running the command (in the Koha root directory):
misc/devel/update_dbix_class_files.pl --db_name=<dbname> --db_user=<dbuser> --db_pass=<dbpass>

6. Repeat step 1 and 2 and notice that there is a new front end column
in the 'Pay fines' page which displays the item barcode

Sponsored-By: Catalyst IT
Comment 2 David Bourgault 2017-12-15 21:28:21 UTC
Patch had a minor conflict when applying.

When following test plan I got the following:

Software error:

DBIC result _type  isn't of the _type Category at members/moremember.pl line 138.
Comment 3 Alex Buckley 2018-03-28 03:25:49 UTC
Created attachment 73362 [details] [review]
Bug 18889 - Storing and display of barcode with fines in intranet

Test plan:
1. Create a manual invoice fine making sure you enter in a barcode value

2. Go to the 'Pay fines' tab and notice that the barcode value is not
displayed

3. Apply patch

4. Update your database by running the updatedatabase.pl script in
koha-shell

This will run the atomicupdate file
bug_18889_added_barcode_field_to_accountlines.sql which contains the
database changes

5. Update your DBIx by running the command (in the Koha root directory):
misc/devel/update_dbix_class_files.pl --db_name=<dbname> --db_user=<dbuser> --db_pass=<dbpass>

6. Repeat step 1 and 2 and notice that there is a new front end column
in the 'Pay fines' page which displays the item barcode

Sponsored-By: Catalyst IT
Comment 4 Alex Buckley 2018-03-28 03:49:06 UTC
Created attachment 73365 [details]
Patch applies cleanly
Comment 5 Alex Buckley 2018-03-28 03:49:42 UTC
Created attachment 73366 [details]
Circled barcode in the pay fines table
Comment 6 Alex Buckley 2018-03-28 03:55:48 UTC
I have recorded some screencast videos showing the behaviour of the bug before applying the patch and following the test plan after applying the patch and running steps 4 and 5. If you are testing this and are confused about the test plan I will happily email these videos to you.
Comment 7 M. Tompsett 2018-04-02 01:32:25 UTC
Comment on attachment 73362 [details] [review]
Bug 18889 - Storing and display of barcode with fines in intranet

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

::: C4/Accounts.pm
@@ +281,4 @@
>              date              => \'NOW()',
>              amount            => $amount,
>              description       => $desc,
> +            barcode           => $barcode,

Why pass barcode when the itemnumber is available?
Comment 8 M. Tompsett 2018-04-02 01:33:07 UTC
Created attachment 73519 [details] [review]
Bug 18889: Simpler counter-patch

There is no need to add barcode into the accountlines
table, as a relationship via itemnumber can easily look it up.
Comment 9 M. Tompsett 2018-04-02 01:39:55 UTC
Counter Patch Test Plan
------------------------
1) Add a manual fine with a barcode to a member.
2) Look at the pay invoice tab.
   -- no barcode shown.
3) apply counter patch only patch
   -- and no updatedatabase or rebuilding tables required!
4) restart_all (silly plack caching)
5) refresh the pay invoice tab.
   -- barcode now displayed.
6) run koha qa test tools.
Comment 10 Séverine Queune 2018-04-20 08:36:50 UTC
The 'Simpler counter-patch' doesn't apply.

> Applying: Bug 18889: Simpler counter-patch
> Using index info to reconstruct a base tree...
> M	koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
> M	members/pay.pl
> Falling back to patching base and 3-way merge...
> error: Your local changes to the following files would be overwritten by merge:
> 	members/pay.pl
> Please, commit your changes or stash them before you can merge.
> Aborting
> Failed to merge in the changes.
> Patch failed at 0001 Bug 18889: Simpler counter-patch
> The copy of the patch that failed is found in:
>    /var/repositories/koha/.git/rebase-apply/patch

Barcode display is good applying only the first patch 'Storing and display of barcode with fines in intranet'.

I'm not sure if I can sign or failed it because of the conflict.
Comment 11 Séverine Queune 2018-04-27 12:43:30 UTC
Should title be "Add barcode to 'Fines' tab in biblio description" or something like that ?
Comment 12 M. Tompsett 2018-04-30 18:24:31 UTC
(In reply to Séverine Queune from comment #10)
> The 'Simpler counter-patch' doesn't apply.

It does by itself. That's the point of the counter-patch.
Comment 13 M. Tompsett 2018-04-30 18:27:25 UTC
In order to apply just one or the other.

On a kohadevbox:
    git checkout -b bug_18889_t1 origin/master
    git bz apply 18889
    i
    -- that's interactive, and then you put # at the beginning of the
       line you do not want

I'll leave the setting of titles up to someone else.
Comment 14 Alex Buckley 2018-05-09 02:38:53 UTC
Created attachment 75197 [details] [review]
Bug 18889: Simpler counter-patch

There is no need to add barcode into the accountlines
table, as a relationship via itemnumber can easily look it up.

Followed test plan, works as described. Passes QA test tool

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 15 Amit Gupta 2018-05-09 03:01:06 UTC
Created attachment 75198 [details] [review]
Bug 18889: Simpler counter-patch

Works OK.
Comment 16 Jonathan Druart 2018-05-30 16:38:58 UTC
1. Please adjust commit title and body
2. Table footer is broken - https://screenshots.firefox.com/26EsdTn15j1x9k4R/pro.kohadev.org
3. Why a new column? Maybe it would make more sense to have it in the first column, close to the biblio's title?

Failing QA for 1 and 2, Discussion for 3.
Comment 17 Hayley Mapley 2019-03-13 00:42:13 UTC
(In reply to Jonathan Druart from comment #16)
> 1. Please adjust commit title and body
> 2. Table footer is broken -
> https://screenshots.firefox.com/26EsdTn15j1x9k4R/pro.kohadev.org
> 3. Why a new column? Maybe it would make more sense to have it in the first
> column, close to the biblio's title?
> 
> Failing QA for 1 and 2, Discussion for 3.

The current master has a barcode column in the Patrons 'Make a payment' tab. Does anything need to be done further for this bug or shall I mark it resolved?
Comment 18 Katrin Fischer 2019-03-13 07:01:26 UTC
(In reply to Hayley Mapley from comment #17)
> (In reply to Jonathan Druart from comment #16)
> > 1. Please adjust commit title and body
> > 2. Table footer is broken -
> > https://screenshots.firefox.com/26EsdTn15j1x9k4R/pro.kohadev.org
> > 3. Why a new column? Maybe it would make more sense to have it in the first
> > column, close to the biblio's title?
> > 
> > Failing QA for 1 and 2, Discussion for 3.
> 
> The current master has a barcode column in the Patrons 'Make a payment' tab.
> Does anything need to be done further for this bug or shall I mark it
> resolved?

You are right, it looks like it has been resolved already. It's also visible on the transactions tab. In this case you can mark "RESOLVED FIXED". If you are aware of the other bug that fixed it, you could also link them or 'Mark as duplicate'.
Comment 19 Hayley Mapley 2019-03-13 20:02:36 UTC
(In reply to Katrin Fischer from comment #18)
> (In reply to Hayley Mapley from comment #17)
> > (In reply to Jonathan Druart from comment #16)
> > > 1. Please adjust commit title and body
> > > 2. Table footer is broken -
> > > https://screenshots.firefox.com/26EsdTn15j1x9k4R/pro.kohadev.org
> > > 3. Why a new column? Maybe it would make more sense to have it in the first
> > > column, close to the biblio's title?
> > > 
> > > Failing QA for 1 and 2, Discussion for 3.
> > 
> > The current master has a barcode column in the Patrons 'Make a payment' tab.
> > Does anything need to be done further for this bug or shall I mark it
> > resolved?
> 
> You are right, it looks like it has been resolved already. It's also visible
> on the transactions tab. In this case you can mark "RESOLVED FIXED". If you
> are aware of the other bug that fixed it, you could also link them or 'Mark
> as duplicate'.

Thank you for all of your help Katrin on the various bugs you've commented on, it's very useful as a newbie to learn the best practices for the process :)
Comment 20 Katrin Fischer 2019-03-13 21:47:35 UTC
> 
> Thank you for all of your help Katrin on the various bugs you've commented
> on, it's very useful as a newbie to learn the best practices for the process
> :)

Please feel free to catch me on IRC too if there are any questions! :)