[Bug 15985] New: Include transacting library in fines
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Bug ID: 15985 Summary: Include transacting library in fines Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: nengard@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com 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 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Depends on| |14826 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Steps 1 through 3 are completed, only step 4 remains! Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14826 [Bug 14826] Resurrect account offsets table -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 58157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58157&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Academy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Jason Robb <jrobb@sekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Jason Robb <jrobb@sekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58157|0 |1 is obsolete| | --- Comment #3 from Jason Robb <jrobb@sekls.org> --- Created attachment 58163 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58163&action=edit [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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Status|Signed Off |BLOCKED --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I don't understand why this is signed off while the dependencies do not apply. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|Academy | CC| |chris@bigballofwax.co.nz -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Lisette <lisetteslatah@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lisetteslatah@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |josef.moravec@gmail.com |y.org | CC| |josef.moravec@gmail.com Status|Signed Off |Patch doesn't apply --- Comment #5 from Josef Moravec <josef.moravec@gmail.com> --- Please rebase. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58163|0 |1 is obsolete| | --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 71165 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=71165&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #7 from Josef Moravec <josef.moravec@gmail.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #71165|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 72611 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72611&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 72612 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72612&action=edit Bug 15985: Add new method Koha::Account::Line::checkout -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #10 from Josef Moravec <josef.moravec@gmail.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #11 from Josef Moravec <josef.moravec@gmail.com> --- This should be rebased and adapted on top of bug 19489 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |14825 CC| |martin.renvoize@ptfs-europe | |.com Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14825 [Bug 14825] Accounts Rewrite Omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- +1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |19489 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19489 [Bug 19489] Detailed description of charges on Accounting tab -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72611|0 |1 is obsolete| | Attachment #72612|0 |1 is obsolete| | --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 90317 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90317&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 90318 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90318&action=edit Bug 15985: Add new method Koha::Account::Line::checkout -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Might this be a good opportunity to introduce a proper foreign key constraint at the DB level for issues.branchcode and old_issues.branchcode? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=22683 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #15)
Might this be a good opportunity to introduce a proper foreign key constraint at the DB level for issues.branchcode and old_issues.branchcode?
Sounds like a capital idea! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm resurrecting bug 22421 for the FK piece at the minute.. may also be nice to have bug 22683 feed into this too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 90364 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90364&action=edit Bug 15985: Add foreign key for issues tables -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90364|0 |1 is obsolete| | --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Comment on attachment 90364 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90364 Bug 15985: Add foreign key for issues tables We should move this patch to its' own bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |23066 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23066 [Bug 23066] Add foreign key for issues tables to branches table for branchcodes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |katrin.fischer@bsz-bw.de --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Kyle, the unit tests pass, but the QA script has some complaints: FAIL Koha/Checkout.pm OK critic OK forbidden patterns OK git manipulation FAIL pod *** ERROR: Unknown command 'head' in file Koha/Checkout.pm FAIL pod coverage POD is missing for 'library' OK spelling OK valid FAIL Koha/Old/Checkout.pm OK critic OK forbidden patterns OK git manipulation OK pod OK pod coverage OK spelling FAIL valid Inconsistent hierarchy during C3 merge of class 'Koha::Old::Checkout': current merge results [ Koha::Old::Checkout, ] merging failed on 'Koha::Checkout' FAIL koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt FAIL filters missing_filter at line 88 ( [% line.checkout.library.branchname %]) OK forbidden patterns OK git manipulation OK js_in_body OK spelling OK tt_valid OK valid_template Can you please check? Also a question for testing: are overdues the only fines where the library will show? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 90478 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=90478&action=edit Bug 15985: (QA Follow up) Fix issues found by koha-qa.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- I'm unable to resolve the qa failure FAIL valid Inconsistent hierarchy during C3 merge of class 'Koha::Old::Checkout': current merge results [ Koha::Old::Checkout, ] merging failed on 'Koha::Checkout' without removing the 'use Modern::Perl', which of course causes a different qa failure. I believe we can safely ignore this one. The actual dependency tree is clear and simple. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #23 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 1. Changes are made in Koha/Schema/Result/OldIssue.pm, above the "DO NOT MODIFY THIS OR ANYTHING ABOVE!" line. Is it expected? Did you forget to attached the DB changes patch? If you wanted to add it after that line, why not adding the constraint at DB level? Why not for Issue.pm? 2. -use base qw(Koha::Object); +use base qw(Koha::Checkout); Is this really needed? Why? If so I think it must be moved to its own bug report, explaining why we need it and what it will bring. Also tests must be provided for such changes. Note that both t/db_dependent/Koha/Account/Lines.t and t/db_dependent/Koha/Checkouts.t are passing without that change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19066 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|14826 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14826 [Bug 14826] Store account offsets -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90317|0 |1 is obsolete| | --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 97177 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97177&action=edit 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> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90318|0 |1 is obsolete| | --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 97178 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97178&action=edit Bug 15985: Add new method Koha::Account::Line::checkout -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #90478|0 |1 is obsolete| | --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 97179 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97179&action=edit Bug 15985: (QA Follow up) Fix issues found by koha-qa.pl Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 97180 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=97180&action=edit Bug 15985: (QA follow-up) Revert to Koha::Object bases Being pragmatic to try and move this bug on, I've reverted the changed base class for the Old:: classes and cloned the required code from the corresponding Object classes. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Rebased, Cleaned up for QA complaints and Signed off.. Trying to be pragmatic to get the feature in and then we'll handle the further cleanup of login in followup bugs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|josef.moravec@gmail.com |testopia@bugs.koha-communit | |y.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This mostly looks good. I'm adding some more tests for full coverage and also changing some bits based on later guidelines as this dev was written a while ago. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97177|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97178|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97179|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #97180|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #30 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 99821 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99821&action=edit 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> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #31 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 99822 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99822&action=edit Bug 15985: Add new method Koha::Account::Line::checkout -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #32 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 99823 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99823&action=edit Bug 15985: (QA Follow up) Fix issues found by koha-qa.pl Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #33 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 99824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99824&action=edit Bug 15985: (QA follow-up) Revert to Koha::Object bases Being pragmatic to try and move this bug on, I've reverted the changed base class for the Old:: classes and cloned the required code from the corresponding Object classes. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #34 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 99825 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99825&action=edit Bug 15985: Add more tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #35 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- There is not FK on old_issues.branchcode, I think that should be taken into account in Koha::Old::Issue->library Also, POD and tests missing for Koha::Old::Issue->claim_returned -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #36 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #23)
1. Changes are made in Koha/Schema/Result/OldIssue.pm, above the "DO NOT MODIFY THIS OR ANYTHING ABOVE!" line. Is it expected? Did you forget to attached the DB changes patch? If you wanted to add it after that line, why not adding the constraint at DB level?
Why not for Issue.pm?
2. -use base qw(Koha::Object); +use base qw(Koha::Checkout);
Is this really needed? Why? If so I think it must be moved to its own bug report, explaining why we need it and what it will bring. Also tests must be provided for such changes.
Note that both t/db_dependent/Koha/Account/Lines.t and t/db_dependent/Koha/Checkouts.t are passing without that change.
And the 2 points here are been ignored. Please comment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #37 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #36)
(In reply to Jonathan Druart from comment #23)
1. Changes are made in Koha/Schema/Result/OldIssue.pm, above the "DO NOT MODIFY THIS OR ANYTHING ABOVE!" line. Is it expected? Did you forget to attached the DB changes patch? If you wanted to add it after that line, why not adding the constraint at DB level?
Why not for Issue.pm?
We don't want to delete old issues just because a branch was removed. They are often needed for reporting purposes. It is presumed that current checkouts will be resolved before deleting a branch. That being said, it should be below the DO NOT line. That could be a rebasing accident.
2. -use base qw(Koha::Object); +use base qw(Koha::Checkout);
Is this really needed? Why? If so I think it must be moved to its own bug report, explaining why we need it and what it will bring. Also tests must be provided for such changes.
Note that both t/db_dependent/Koha/Account/Lines.t and t/db_dependent/Koha/Checkouts.t are passing without that change.
The allows old checkouts objects to inherit the same methods the checkouts object has, without the need to duplicate all the code. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99821|0 |1 is obsolete| | Attachment #99822|0 |1 is obsolete| | Attachment #99823|0 |1 is obsolete| | Attachment #99824|0 |1 is obsolete| | Attachment #99825|0 |1 is obsolete| | --- Comment #38 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103152 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103152&action=edit 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> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #39 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103153 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103153&action=edit Bug 15985: Add new method Koha::Account::Line::checkout -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #40 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103154 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103154&action=edit Bug 15985: (QA Follow up) Fix issues found by koha-qa.pl Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #41 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103155 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103155&action=edit Bug 15985: (QA follow-up) Revert to Koha::Object bases Being pragmatic to try and move this bug on, I've reverted the changed base class for the Old:: classes and cloned the required code from the corresponding Object classes. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #42 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103156 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103156&action=edit Bug 15985: Add more tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #43 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103157&action=edit Bug 15985: Move dbic relationship to correct part of file -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA Summary|Include transacting library |Include checkout library in |in fines |fines --- Comment #44 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 1) It looks like we are still stuck with this one: FAIL Koha/Old/Checkout.pm FAIL valid Inconsistent hierarchy during C3 merge of class 'Koha::Old::Checkout': current merge results [ Koha::Old::Checkout, ] merging failed on 'Koha::Checkout' I have to admit, I am not sure what it means - is there a chance we could resolve or at least explain it? 2) =head3 claim_returned This seems unrelated to the patch - could it have slipped in? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #45 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 3) The column seems to be missing from column configuration (no change to .yml and I can't spot it. 4) I'd like to see it moved before the issue date added by bug 24995. At the moment it's between Account type and description and I feel like we should keep those together at the beginning of the table. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #46 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 104491 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104491&action=edit Bug 15985: Move new column to the right of checkout date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #47 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 104492 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104492&action=edit Bug 15985: Add to columns configuration -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #48 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Katrin Fischer from comment #44)
1) It looks like we are still stuck with this one:
FAIL Koha/Old/Checkout.pm FAIL valid Inconsistent hierarchy during C3 merge of class 'Koha::Old::Checkout': current merge results [ Koha::Old::Checkout, ] merging failed on 'Koha::Checkout'
I have to admit, I am not sure what it means - is there a chance we could resolve or at least explain it?
2) =head3 claim_returned
This seems unrelated to the patch - could it have slipped in?
Hi Kyle, could you check these out too please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #49 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 105144 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105144&action=edit Bug 15985: Fix QA script issues * Use original base for old checkout objects * test rs for results before returning a patron object -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_11_target -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Signed Off --- Comment #50 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- There is no QA stamp here, back to NQA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #51 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- The +=head3 claim_returned is still included in this patch set. Please have a look and remove it or explain please. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 --- Comment #52 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 105398 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105398&action=edit Bug 15985: Remove Koha::Old::Checkout::claim_returned -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #53 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Kyle, I am sorry, but this doesn't quite work as expected. Possibly the table structure changed a bit since this was originally written. 1) Total columns at the end of table are off by at least one column: Outstanding credits could be applied: Total due if credit applied: 2) Hiding columns in column configuration doesn't work. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_11_target | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15985 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |DUPLICATE Status|Failed QA |RESOLVED --- Comment #54 from Tomás Cohen Arazi <tomascohen@gmail.com> --- With Kyle's blessing, I started a new bug that will cleanly implement the same this patchset does, but easier to apply and understand. *** This bug has been marked as a duplicate of bug 26327 *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org