[Bug 17680] New: C4::Circulation - Replace GetItemIssue with Koha::Issues
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Bug ID: 17680 Summary: C4::Circulation - Replace GetItemIssue with Koha::Issues Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17677 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17677 [Bug 17677] Move C4::Circulation code to the Koha namespace -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=17680 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57787 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57787&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls C4::Circulation::GetItemIssue returned all the issue and item informations for a given issue. Moveover it also did some date manipulations. Most of the time this subroutine was called, there additional information were useless as the caller usually just needed the basic issue's infos 'from the issue table). This first patch updates the simple calls, ie. the ones that just need the issue's infomations. Test plan: The following operations should success: - transfer a book - create a rule for on-site checkouts and confirm that a patron cannot check more items out that it's defined in the rule. - Renew an issue using ILSDI - Using SIP confirm that you are able to see your issues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57788&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 57789 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57789&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |17689, 17679 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17679 [Bug 17679] C4::Circulation - Remove unused GetItemIssues https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17689 [Bug 17689] Add the Koha::Issue->is_overdue method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Aleisha Amohia <aleishaamohia@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleishaamohia@hotmail.com Status|Needs Signoff |Patch doesn't apply --- Comment #4 from Aleisha Amohia <aleishaamohia@hotmail.com> --- Patch does not apply: Applying: Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls fatal: sha1 information is lacking or useless (C4/Circulation.pm). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com --- Comment #5 from Josef Moravec <josef.moravec@gmail.com> --- (In reply to Aleisha Amohia from comment #4)
Patch does not apply:
Applying: Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls fatal: sha1 information is lacking or useless (C4/Circulation.pm).
Did you apply the dependencies first? With dependencies, I was able apply these patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57789|0 |1 is obsolete| | --- Comment #6 from Alex Buckley <alexbuckley@catalyst.net.nz> --- Created attachment 57847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57847&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alexbuckley@catalyst.net.nz 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=17680 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57787|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=17680 Alex Buckley <alexbuckley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57788|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=17680 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Attachment #57787|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57788|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- This patch set needs updated to use Koha::Checkouts -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|C4::Circulation - Replace |C4::Circulation - Replace |GetItemIssue with |GetItemIssue with |Koha::Issues |Koha::Checkouts -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57787|0 |1 is obsolete| | Attachment #57788|0 |1 is obsolete| | Attachment #57847|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58431 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58431&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls C4::Circulation::GetItemIssue returned all the issue and item informations for a given issue. Moveover it also did some date manipulations. Most of the time this subroutine was called, there additional information were useless as the caller usually just needed the basic issue's infos 'from the issue table). This first patch updates the simple calls, ie. the ones that just need the issue's infomations. Test plan: The following operations should success: - transfer a book - create a rule for on-site checkouts and confirm that a patron cannot check more items out that it's defined in the rule. - Renew an issue using ILSDI - Using SIP confirm that you are able to see your issues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58432 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58432&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58433 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58433&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Bug 17680 depends on bug 17689, which changed state. Bug 17689 Summary: Add the Koha::Issue->is_overdue method https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17689 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58432|0 |1 is obsolete| | --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58501 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58501&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Bug 17680 depends on bug 17679, which changed state. Bug 17679 Summary: C4::Circulation - Remove unused GetItemIssues https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17679 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #12 from Josef Moravec <josef.moravec@gmail.com> --- could you rebase please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58431|0 |1 is obsolete| | Attachment #58433|0 |1 is obsolete| | Attachment #58501|0 |1 is obsolete| | --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58918 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58918&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls C4::Circulation::GetItemIssue returned all the issue and item informations for a given issue. Moveover it also did some date manipulations. Most of the time this subroutine was called, there additional information were useless as the caller usually just needed the basic issue's infos 'from the issue table). This first patch updates the simple calls, ie. the ones that just need the issue's infomations. Test plan: The following operations should success: - transfer a book - create a rule for on-site checkouts and confirm that a patron cannot check more items out that it's defined in the rule. - Renew an issue using ILSDI - Using SIP confirm that you are able to see your issues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58919 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58919&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #15 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 58920 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58920&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #58918|0 |1 is obsolete| | Attachment #58919|0 |1 is obsolete| | Attachment #58920|0 |1 is obsolete| | --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61761 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61761&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls C4::Circulation::GetItemIssue returned all the issue and item informations for a given issue. Moveover it also did some date manipulations. Most of the time this subroutine was called, there additional information were useless as the caller usually just needed the basic issue's infos 'from the issue table). This first patch updates the simple calls, ie. the ones that just need the issue's infomations. Test plan: The following operations should success: - transfer a book - create a rule for on-site checkouts and confirm that a patron cannot check more items out that it's defined in the rule. - Renew an issue using ILSDI - Using SIP confirm that you are able to see your issues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61762 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61762&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 61763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61763&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Just a rebase. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17829 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17829 [Bug 17829] Move GetMember to Koha::Patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch --- Comment #20 from Marc Véron <veron@veron.ch> --- First two patches apply cleanly, with 3rd patch (complex calls) I get: fatal: sha1 information is lacking or useless (C4/Circulation.pm). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61763|0 |1 is obsolete| | --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 62034 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62034&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #22 from Marc Véron <veron@veron.ch> --- Typo in 3rd patch, circ/returns.pl , line 314: my $date_due_dt = dt_strom_string( $issue->date_due, 'sql' ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62034|0 |1 is obsolete| | --- Comment #23 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 62086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62086&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #24 from Marc Véron <veron@veron.ch> --- Worked through test plans from comments #1 - #3, everything OK, will sign off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61761|0 |1 is obsolete| | --- Comment #25 from Marc Véron <veron@veron.ch> --- Created attachment 62102 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62102&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls C4::Circulation::GetItemIssue returned all the issue and item informations for a given issue. Moveover it also did some date manipulations. Most of the time this subroutine was called, there additional information were useless as the caller usually just needed the basic issue's infos 'from the issue table). This first patch updates the simple calls, ie. the ones that just need the issue's infomations. Test plan: The following operations should success: - transfer a book - create a rule for on-site checkouts and confirm that a patron cannot check more items out that it's defined in the rule. - Renew an issue using ILSDI - Using SIP confirm that you are able to see your issues Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #61762|0 |1 is obsolete| | --- Comment #26 from Marc Véron <veron@veron.ch> --- Created attachment 62103 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62103&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62086|0 |1 is obsolete| | --- Comment #27 from Marc Véron <veron@veron.ch> --- Created attachment 62104 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62104&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch 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=17680 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62102|0 |1 is obsolete| | Attachment #62103|0 |1 is obsolete| | Attachment #62104|0 |1 is obsolete| | --- Comment #28 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64257 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64257&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls C4::Circulation::GetItemIssue returned all the issue and item informations for a given issue. Moveover it also did some date manipulations. Most of the time this subroutine was called, there additional information were useless as the caller usually just needed the basic issue's infos 'from the issue table). This first patch updates the simple calls, ie. the ones that just need the issue's infomations. Test plan: The following operations should success: - transfer a book - create a rule for on-site checkouts and confirm that a patron cannot check more items out that it's defined in the rule. - Renew an issue using ILSDI - Using SIP confirm that you are able to see your issues Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #29 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64258 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64258&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #30 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64259 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64259&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #31 from Nick Clemens <nick@bywatersolutions.com> --- looking here - SIP item_information request failed for me in testing (worked on master) - will try again tomorrow -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #32 from Nick Clemens <nick@bywatersolutions.com> --- SIP fails on the date, this needs to be converted to a string for returning Checking in an overdue item fails to calculate the fine Test fails: t/db_dependent/Circulation.t .. 44/95 # Failed test 'Can auto renew, OPACFineNoRenewals=10, patron has 10' # at t/db_dependent/Circulation.t line 671. # got: 'auto_too_much_oweing' # expected: 'auto_renew' # Looks like you failed 1 test of 6. I think there are spots where the date isn't being converted from a DateTime object when needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #33 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64284 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64284&action=edit Bug 17680: Fix regression - is_overdue must be called is_overdue must be called even if there is not dropbox date (!) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #34 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Nick Clemens from comment #32)
SIP fails on the date, this needs to be converted to a string for returning
Any chances you could try to fix this one?
Checking in an overdue item fails to calculate the fine
Ha, terrible! Will be fixed with the last patch, thanks for catching that.
Test fails: t/db_dependent/Circulation.t .. 44/95 # Failed test 'Can auto renew, OPACFineNoRenewals=10, patron has 10' # at t/db_dependent/Circulation.t line 671. # got: 'auto_too_much_oweing' # expected: 'auto_renew' # Looks like you failed 1 test of 6.
I manage to recreate that with prove t/db_dependent/Circulation* but then the test failed on master too. Sounds like a master bug, depending on data.
I think there are spots where the date isn't being converted from a DateTime object when needed
Did you spot where? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=18802 --- Comment #35 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #34)
Test fails: t/db_dependent/Circulation.t .. 44/95 # Failed test 'Can auto renew, OPACFineNoRenewals=10, patron has 10' # at t/db_dependent/Circulation.t line 671. # got: 'auto_too_much_oweing' # expected: 'auto_renew' # Looks like you failed 1 test of 6.
I manage to recreate that with prove t/db_dependent/Circulation* but then the test failed on master too. Sounds like a master bug, depending on data.
It depends on the value of finesMode. I have opened bug 18802. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED Status|ASSIGNED |Needs Signoff Status|Needs Signoff |Signed Off Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Signed Off |Patch doesn't apply --- Comment #36 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64337 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64337&action=edit Bug 17680: Add few tests for AddReturn when overdue To make sure the last patch fixes the issue --- Comment #37 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Code looks ok, but I cannot apply the 'complex calls' patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64259|0 |1 is obsolete| | Attachment #64284|0 |1 is obsolete| | Attachment #64337|0 |1 is obsolete| | --- Comment #38 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64805&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #39 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64806&action=edit Bug 17680: Fix regression - is_overdue must be called is_overdue must be called even if there is not dropbox date (!) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #40 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64807 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64807&action=edit Bug 17680: Add few tests for AddReturn when overdue To make sure the last patch fixes the issue -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #41 from Tomás Cohen Arazi <tomascohen@gmail.com> --- wihtout the patches: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Circulation/Returns.t t/db_dependent/Circulation/Returns.t .. ok All tests successful. Files=1, Tests=4, 2 wallclock secs ( 0.02 usr 0.01 sys + 2.09 cusr 0.35 csys = 2.47 CPU) Result: PASS with the patches: kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Circulation/Returns.t t/db_dependent/Circulation/Returns.t .. 1/4 # Failed test 'No account lines should exist on old issue_id' # at t/db_dependent/Circulation/Returns.t line 311. # got: '1' # expected: '0' # Failed test 'Two account lines should exist on new issue_id' # at t/db_dependent/Circulation/Returns.t line 314. # got: '1' # expected: '2' # Looks like you failed 2 tests of 4. t/db_dependent/Circulation/Returns.t .. 4/4 # Failed test 'Handle ids duplication' # at t/db_dependent/Circulation/Returns.t line 318. # Looks like you failed 1 test of 4. t/db_dependent/Circulation/Returns.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/4 subtests Test Summary Report ------------------- t/db_dependent/Circulation/Returns.t (Wstat: 256 Tests: 4 Failed: 1) Failed test: 4 Non-zero exit status: 1 Files=1, Tests=4, 3 wallclock secs ( 0.02 usr 0.01 sys + 2.14 cusr 0.33 csys = 2.50 CPU) Result: FAIL kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #42 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64833&action=edit Bug 17680: Fix conflict with bug 18651 $issue is now a Koha::Checkout, not a hashref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #43 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #41) Thanks for catching that, fixed! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64257|0 |1 is obsolete| | Attachment #64258|0 |1 is obsolete| | Attachment #64805|0 |1 is obsolete| | Attachment #64806|0 |1 is obsolete| | Attachment #64807|0 |1 is obsolete| | Attachment #64833|0 |1 is obsolete| | --- Comment #44 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64866 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64866&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls C4::Circulation::GetItemIssue returned all the issue and item informations for a given issue. Moveover it also did some date manipulations. Most of the time this subroutine was called, there additional information were useless as the caller usually just needed the basic issue's infos 'from the issue table). This first patch updates the simple calls, ie. the ones that just need the issue's infomations. Test plan: The following operations should success: - transfer a book - create a rule for on-site checkouts and confirm that a patron cannot check more items out that it's defined in the rule. - Renew an issue using ILSDI - Using SIP confirm that you are able to see your issues Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #45 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64867 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64867&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #46 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64868 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64868&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #47 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64869 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64869&action=edit Bug 17680: Fix regression - is_overdue must be called is_overdue must be called even if there is not dropbox date (!) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #48 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64870 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64870&action=edit Bug 17680: Add few tests for AddReturn when overdue To make sure the last patch fixes the issue -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #49 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 64871 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64871&action=edit Bug 17680: Fix conflict with bug 18651 $issue is now a Koha::Checkout, not a hashref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64866|0 |1 is obsolete| | --- Comment #50 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 64925 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64925&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, simple calls C4::Circulation::GetItemIssue returned all the issue and item informations for a given issue. Moveover it also did some date manipulations. Most of the time this subroutine was called, there additional information were useless as the caller usually just needed the basic issue's infos 'from the issue table). This first patch updates the simple calls, ie. the ones that just need the issue's infomations. Test plan: The following operations should success: - transfer a book - create a rule for on-site checkouts and confirm that a patron cannot check more items out that it's defined in the rule. - Renew an issue using ILSDI - Using SIP confirm that you are able to see your issues Followed test plan, works as expected Signed-off-by: Marc Véron <veron@veron.ch> 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=17680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64867|0 |1 is obsolete| | --- Comment #51 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 64926 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64926&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue This patch simply removes the C4::Circulation::GetItemIssue subroutine Test plan: At this point, `git grep GetItemIssue` should not return any occurrence in the codebase Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marc Véron <veron@veron.ch> 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=17680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64868|0 |1 is obsolete| | --- Comment #52 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 64927 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64927&action=edit Bug 17680: C4::Circulation - Remove GetItemIssue, complex calls There are a few calls to GetItemIssue where it's not as easy to make sure everything will be fine just replacing the calls with a Koha::Issues->find - In AddReturn the overdue flag is used (that's why this patch depends on bug 17689) - In CanBookBeRenewed, as well as the overdue flag the dates converted to DateTime were used. It's now our job to convert them when we need them. - Same in AddRenewal but we also call _CalculateAndUpdateFine, so we need to update the variables in this subroutine. Note that, prior to this patch, AddReturn returned the GetItemIssue hashref in the $iteminformation. Most of the time this variable is not used, I have found only 1 place where it's used: circ/returns.pl TODO: In this script we should call ->is_overdue instead of the DateTime->compare calls Test plan: All the circulation tests must pass (it's how I have caught the specific cases). Do some checkins/checkouts/renewal and focus on the due date Signed-off-by: Marc Véron <veron@veron.ch> 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=17680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64869|0 |1 is obsolete| | --- Comment #53 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 64928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64928&action=edit Bug 17680: Fix regression - is_overdue must be called is_overdue must be called even if there is not dropbox date (!) 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=17680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64870|0 |1 is obsolete| | --- Comment #54 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 64929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64929&action=edit Bug 17680: Add few tests for AddReturn when overdue To make sure the last patch fixes the issue 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=17680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #64871|0 |1 is obsolete| | --- Comment #55 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 64930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64930&action=edit Bug 17680: Fix conflict with bug 18651 $issue is now a Koha::Checkout, not a hashref 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=17680 --- Comment #56 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 64931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=64931&action=edit Bug 17680: Remove Koha::Upload use incorrectly added 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=17680 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Version|unspecified |master --- Comment #57 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I tested this patchset on the UI. It looks trivial in most of the cases. I haven't tested ILS-DI and SIP beyond the included unit tests, but the changes seems pretty straight-forward, and tests pass of course. The QA script is happy too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #58 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 17.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED CC| |fridolin.somers@biblibre.co | |m --- Comment #59 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Enhancement not pushed to 17.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #60 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #57)
I tested this patchset on the UI. It looks trivial in most of the cases. I haven't tested ILS-DI and SIP beyond the included unit tests, but the changes seems pretty straight-forward, and tests pass of course.
I think I found a problem in SIP/ILS/Item. It fails silently on dt_from_string when checking in an item. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #61 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 65302 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65302&action=edit Bug 17680: [QA Follow-up] dt_from_string failing silently This is a trivial fix, but finding the cause for a failing checkin in SIPServer surely was not. Without this simple change, I could not checkin an item while the SIP server just closed the connection and syslog contained nothing useful. The error can be easily reproduced without this patch by: use Modern::Perl; use C4::SIP::ILS::Item; my $item = C4::SIP::ILS::Item->new('some_barcode'); where some_barcode should refer to an item checked out. You should trigger the error (with your due date): 2017-07-28 23:59:00 is not an object at line 3. Why the ILS/Item.pm module still compiles without a warning, is not clear though. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED |--- -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=17680 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> 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=17680 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18996 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18996 [Bug 18996] SIP sets ok flag to true for refused checkin for data corruption -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |colin.campbell@ptfs-europe. | |com --- Comment #62 from Colin Campbell <colin.campbell@ptfs-europe.com> --- (In reply to Marcel de Rooy from comment #61)
Created attachment 65302 [details] [review] Bug 17680: [QA Follow-up] dt_from_string failing silently
This is a trivial fix, but finding the cause for a failing checkin in SIPServer surely was not. Without this simple change, I could not checkin an item while the SIP server just closed the connection and syslog contained nothing useful.
The error can be easily reproduced without this patch by: use Modern::Perl; use C4::SIP::ILS::Item; my $item = C4::SIP::ILS::Item->new('some_barcode'); where some_barcode should refer to an item checked out. You should trigger the error (with your due date): 2017-07-28 23:59:00 is not an object at line 3.
Why the ILS/Item.pm module still compiles without a warning, is not clear though.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Nice find - these are painful to locate. Suspect that the reason there is no compile time warning is because ILS::Item is using autoloading so that the routine gets resolved at runtime. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #65302|0 |1 is obsolete| | --- Comment #63 from Colin Campbell <colin.campbell@ptfs-europe.com> --- Created attachment 65310 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=65310&action=edit Bug 17680 [QA Follow-up] dt_from_string failing silently Added independent sign off to last patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #64 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Would be great to have the SIP tests moved to t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #65 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Last patch pushed to master for 17.11. Thanks Marcel and Colin! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #66 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Colin Campbell from comment #62)
Suspect that the reason there is no compile time warning is because ILS::Item is using autoloading so that the routine gets resolved at runtime.
Yes, nice effects of autoloading of course. We could improve that some day ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 --- Comment #67 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #64)
Would be great to have the SIP tests moved to t
Will try to add at least a test for checkin on 18996. Bit by bit extending what we have in t/db_dependent/SIP. I am not that comfortable with C4/SIP/t and suspect that it does not create its own data too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED --- Comment #68 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Enhancement not pushed to 17.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |21020 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21020 [Bug 21020] Return branch not set for transfer when using SIP -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17680 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21020 Depends on|21020 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21020 [Bug 21020] Return branch not set for transfer when using SIP -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org