Bug 15985 - Include transacting library in fines
Summary: Include transacting library in fines
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Josef Moravec
URL:
Whiteboard:
Keywords:
Depends on: 14826
Blocks:
  Show dependency treegraph
 
Reported: 2016-03-04 19:53 UTC by Nicole C. Engard
Modified: 2018-04-04 12:34 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 15985 - Include transacting library in fines (4.70 KB, patch)
2016-12-13 13:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Attachment to Bug 15985 - Include transacting library in fines (4.74 KB, patch)
2016-12-13 18:05 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 15985 - Include transacting library in fines (4.76 KB, patch)
2018-02-02 19:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15985 - Include transacting library in fines (4.62 KB, patch)
2018-03-09 15:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15985: Add new method Koha::Account::Line::checkout (5.29 KB, patch)
2018-03-09 15:44 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 Nicole C. Engard 2016-03-04 19:53:45 UTC
Work to be done:

1) Add column issue_id to accountlines table
2) Modify fines generation to set issue_id for fine upon generation
3) Modify updatedatabase.pl to insert issue_id in accountlines for currently accruing fines
4) On the pay fines page, use this data to display the library where the item was checked out from
Comment 1 Kyle M Hall 2016-09-06 14:59:38 UTC
Steps 1 through 3 are completed, only step 4 remains!
Comment 2 Kyle M Hall 2016-12-13 13:25:18 UTC
Created attachment 58157 [details] [review]
Bug 15985 - Include transacting library in fines

Some librarians would like to see the library an item was checked out
from in the fines table on the fines payment page.

Test Plan:
1) Apply this patch
2) Find a patron with overdue items that have fines
3) Browse to the pay fines page
4) Note the new "checked out from" column
Comment 3 Jason Robb 2016-12-13 18:05:14 UTC
Created attachment 58163 [details] [review]
[SIGNED-OFF] Attachment to Bug 15985 - Include transacting library in fines

Bug 15985 - Include transacting library in fines

Some librarians would like to see the library an item was checked out
from in the fines table on the fines payment page.

Test Plan:
1) Apply this patch
2) Find a patron with overdue items that have fines
3) Browse to the pay fines page
4) Note the new "checked out from" column

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 4 Jonathan Druart 2016-12-14 11:06:20 UTC
I don't understand why this is signed off while the dependencies do not apply.
Comment 5 Josef Moravec 2018-01-02 10:28:08 UTC
Please rebase.
Comment 6 Kyle M Hall 2018-02-02 19:31:07 UTC
Created attachment 71165 [details] [review]
Bug 15985 - Include transacting library in fines

Some librarians would like to see the library an item was checked out
from in the fines table on the fines payment page.

Test Plan:
1) Apply this patch
2) Find a patron with overdue items that have fines
3) Browse to the pay fines page
4) Note the new "checked out from" column

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 7 Josef Moravec 2018-02-05 08:38:16 UTC
Sorry Kyle, but not ready to pass QA yet:

first, thing reported by qa tools:
 FAIL	Koha/Checkout.pm
  FAIL	  pod
		 in file Koha/Checkout.pm
		Unknown command 'head'
		*** ERROR: 
   FAIL	  pod coverage
		POD is missing for 'library'


 FAIL	members/pay.pl
   FAIL	  valid
		members/pay.pl had compilation errors.
		Global symbol "$accountline" requires explicit package name 

Then:
When the item with fine is checked out, I can see the name of library, but when I check it in, it disappear. Shouldn't it be still visible?

In pay.pl you are doing this:
$account_line->{checkout}     = Koha::Checkouts->find($accountline->{issue_id});
Maybe be better could be to add new sub to Koha::Account::Line object. You can take care of issue/old_issue table relation.
Comment 8 Kyle M Hall 2018-03-09 15:44:02 UTC
Created attachment 72611 [details] [review]
Bug 15985 - Include transacting library in fines

Some librarians would like to see the library an item was checked out
from in the fines table on the fines payment page.

Test Plan:
1) Apply this patch
2) Find a patron with overdue items that have fines
3) Browse to the pay fines page
4) Note the new "checked out from" column

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 9 Kyle M Hall 2018-03-09 15:44:10 UTC
Created attachment 72612 [details] [review]
Bug 15985: Add new method Koha::Account::Line::checkout
Comment 10 Josef Moravec 2018-04-04 12:34:46 UTC
Kyle, I still found some issues with this:

1. reported by QA tools, not sure how to fix this..., :
 FAIL	Koha/Old/Checkout.pm
   FAIL	  valid
			merging failed on 'Koha::Checkout' 
			]
				Koha::Old::Checkout,
			current merge results [
		Inconsistent hierarchy during C3 merge of class 'Koha::Old::Checkout':


2. This still remains:
 FAIL	Koha/Checkout.pm
  FAIL	  pod
		 in file Koha/Checkout.pm
		Unknown command 'head'
		*** ERROR: 
   FAIL	  pod coverage
		POD is missing for 'library'

you should use =head3 I think

3. The code added to Koha/Schema/Result/OldIssue.pm should be after schema loader mark

4. Also, when you make Koha::Checkout base for Koha::Old::Checkout, you should also define relations 'borrower' and 'item' in Koha::Schema::OldIssue