[Bug 29083] New: Update article requests-related Koha::Patron methods to use relationships
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Bug ID: 29083 Summary: Update article requests-related Koha::Patron methods to use relationships Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org -- 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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com, | |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27945 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 [Bug 27945] Limit the number of active article requests per patron category -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Added bug 27945 as dependency because it touches the same tests file. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch 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=29083 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125160 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125160&action=edit Bug 29083: Unit tests This patch adds missing tests for Koha::Patron->article_requests and moves (and extends) tests for 'article_requests_current' and 'article_requests_finished' that were originally in ArticleRequests.t into Koha/Patron.t as we now do. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125161 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125161&action=edit Bug 29083: Update article requests-related Koha::Patron methods to use relationships This patch makes Koha::Patron->article_requests use the underlying DBIC relationship and _new_from_dbic instead of a plain search. It also refactors 'article_requests_current' and 'article_requests_finished' to use ->article_requests, as well as the new methods introduced by bug 29082 for filtering. No behavior change should take place. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/ArticleRequests.t => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29082 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29082 [Bug 29082] Add filtering methods to Koha::ArticleRequests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125162 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125162&action=edit Bug 29083: (QA follow-up) Remove unused param 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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29084 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 [Bug 29084] Update article requests-related Koha::Biblio methods to use relationships -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28684 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|27945 |27947 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27945 [Bug 27945] Limit the number of active article requests per patron category https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27947 [Bug 27947] Add default cancellation reasons to article requests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125186 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125186&action=edit Bug 29083: Fix OPAC listing of article requests This patch makes the OPAC template reuse a precalculated value for the active article requests for the patron (and its count). The original code relied on the methods returning a list, which is not the case for _new_from_dbic until bug 28883 is pushed. This patch fixes that. Note: there was an odd behavior when ArticleRequests was enabled but no active article requests were present: the tab wasn't rendered but the 'empty table' with the 'You have no article requests currently.' message was displayed below the Checkouts tab. I'm not sure that was caused by this patches, or other. Fixed on this patch. To test: 1. In the OPAC, go to 'your summary' => FAIL: Things don't show for article requests 2. Add some article requests and repeat 1 => FAIL: Something's wrong there 3. Apply this patch and repeat 1 => Yes! Things show correctly! 4. Cancel all your article requests => SUCCESS: Things render as they should 5. Re-enter the 'your summary' page (to force re-rendering) => SUCCESS: Things render correctly for empty article requests 6. Sign off :-D 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=29083 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|27947 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27947 [Bug 27947] Add default cancellation reasons to article requests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125160|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125240&action=edit Bug 29083: Unit tests This patch adds missing tests for Koha::Patron->article_requests and moves (and extends) tests for 'article_requests_current' and 'article_requests_finished' that were originally in ArticleRequests.t into Koha/Patron.t as we now do. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! 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=29083 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125161|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125241 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125241&action=edit Bug 29083: Update article requests-related Koha::Patron methods to use relationships This patch makes Koha::Patron->article_requests use the underlying DBIC relationship and _new_from_dbic instead of a plain search. It also refactors 'article_requests_current' and 'article_requests_finished' to use ->article_requests, as well as the new methods introduced by bug 29082 for filtering. No behavior change should take place. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/ArticleRequests.t => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D 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=29083 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125162|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125242 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125242&action=edit Bug 29083: (QA follow-up) Remove unused param Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=29083 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125186|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125243 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125243&action=edit Bug 29083: Fix OPAC listing of article requests This patch makes the OPAC template reuse a precalculated value for the active article requests for the patron (and its count). The original code relied on the methods returning a list, which is not the case for _new_from_dbic until bug 28883 is pushed. This patch fixes that. Note: there was an odd behavior when ArticleRequests was enabled but no active article requests were present: the tab wasn't rendered but the 'empty table' with the 'You have no article requests currently.' message was displayed below the Checkouts tab. I'm not sure that was caused by this patches, or other. Fixed on this patch. To test: 1. In the OPAC, go to 'your summary' => FAIL: Things don't show for article requests 2. Add some article requests and repeat 1 => FAIL: Something's wrong there 3. Apply this patch and repeat 1 => Yes! Things show correctly! 4. Cancel all your article requests => SUCCESS: Things render as they should 5. Re-enter the 'your summary' page (to force re-rendering) => SUCCESS: Things render correctly for empty article requests 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> 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=29083 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice clear refactor.. signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Nick Clemens <nick@bywatersolutions.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=29083 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125240|0 |1 is obsolete| | Attachment #125241|0 |1 is obsolete| | Attachment #125242|0 |1 is obsolete| | Attachment #125243|0 |1 is obsolete| | --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125268 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125268&action=edit Bug 29083: Unit tests This patch adds missing tests for Koha::Patron->article_requests and moves (and extends) tests for 'article_requests_current' and 'article_requests_finished' that were originally in ArticleRequests.t into Koha/Patron.t as we now do. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125269 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125269&action=edit Bug 29083: Update article requests-related Koha::Patron methods to use relationships This patch makes Koha::Patron->article_requests use the underlying DBIC relationship and _new_from_dbic instead of a plain search. It also refactors 'article_requests_current' and 'article_requests_finished' to use ->article_requests, as well as the new methods introduced by bug 29082 for filtering. No behavior change should take place. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/ArticleRequests.t => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125270 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125270&action=edit Bug 29083: (QA follow-up) Remove unused param Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125271 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125271&action=edit Bug 29083: Fix OPAC listing of article requests This patch makes the OPAC template reuse a precalculated value for the active article requests for the patron (and its count). The original code relied on the methods returning a list, which is not the case for _new_from_dbic until bug 28883 is pushed. This patch fixes that. Note: there was an odd behavior when ArticleRequests was enabled but no active article requests were present: the tab wasn't rendered but the 'empty table' with the 'You have no article requests currently.' message was displayed below the Checkouts tab. I'm not sure that was caused by this patches, or other. Fixed on this patch. To test: 1. In the OPAC, go to 'your summary' => FAIL: Things don't show for article requests 2. Add some article requests and repeat 1 => FAIL: Something's wrong there 3. Apply this patch and repeat 1 => Yes! Things show correctly! 4. Cancel all your article requests => SUCCESS: Things render as they should 5. Re-enter the 'your summary' page (to force re-rendering) => SUCCESS: Things render correctly for empty article requests 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125268|0 |1 is obsolete| | --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125640 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125640&action=edit Bug 29083: Unit tests This patch adds missing tests for Koha::Patron->article_requests and moves (and extends) tests for 'article_requests_current' and 'article_requests_finished' that were originally in ArticleRequests.t into Koha/Patron.t as we now do. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125269|0 |1 is obsolete| | --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125641 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125641&action=edit Bug 29083: Update article requests-related Koha::Patron methods to use relationships This patch makes Koha::Patron->article_requests use the underlying DBIC relationship and _new_from_dbic instead of a plain search. It also refactors 'article_requests_current' and 'article_requests_finished' to use ->article_requests, as well as the new methods introduced by bug 29082 for filtering. No behavior change should take place. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/ArticleRequests.t => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125270|0 |1 is obsolete| | --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125642 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125642&action=edit Bug 29083: (QA follow-up) Remove unused param Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125271|0 |1 is obsolete| | --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125643 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125643&action=edit Bug 29083: Fix OPAC listing of article requests This patch makes the OPAC template reuse a precalculated value for the active article requests for the patron (and its count). The original code relied on the methods returning a list, which is not the case for _new_from_dbic until bug 28883 is pushed. This patch fixes that. Note: there was an odd behavior when ArticleRequests was enabled but no active article requests were present: the tab wasn't rendered but the 'empty table' with the 'You have no article requests currently.' message was displayed below the Checkouts tab. I'm not sure that was caused by this patches, or other. Fixed on this patch. To test: 1. In the OPAC, go to 'your summary' => FAIL: Things don't show for article requests 2. Add some article requests and repeat 1 => FAIL: Something's wrong there 3. Apply this patch and repeat 1 => Yes! Things show correctly! 4. Cancel all your article requests => SUCCESS: Things render as they should 5. Re-enter the 'your summary' page (to force re-rendering) => SUCCESS: Things render correctly for empty article requests 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Same as bug 29084 comment 8. I don't think we need $patron->article_requests_finished, callers can $patron->article_requests->filter_by_finished (or ->finished as the filter_by methods are not correctly named in this module). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #19)
Same as bug 29084 comment 8. I don't think we need $patron->article_requests_finished, callers can $patron->article_requests->filter_by_finished (or ->finished as the filter_by methods are not correctly named in this module).
Until we sort bug 28883 and friends, can we keep the methods and allow this bug? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- We don't need to wait for bug 28883, we have a workaround. Having scalar in include files it not horrible IMO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #21)
We don't need to wait for bug 28883, we have a workaround. Having scalar in include files it not horrible IMO.
Smaken verschillen. But you could also simply call your method outside the template and pass in the results.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #23 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #22)
(In reply to Jonathan Druart from comment #21)
We don't need to wait for bug 28883, we have a workaround. Having scalar in include files it not horrible IMO.
Smaken verschillen. But you could also simply call your method outside the template and pass in the results..
Yes but you duplicate the code. It's not nice when you need to add it to 10 controllers. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125833&action=edit Bug 29083: Remove article_requests_finished and article_requests_current This patch removes those methods that are not really needed. Templates are adjusted to use the expected combination of ->article_requests->filter_by_current. To test: 1. Apply this patch 2. Visit a patron with article requests => SUCCESS: All works 3. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass, less tests. 4. Sign off :-D 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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Passed QA --- Comment #25 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Done. Went the Context.Scalar way for now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #26 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Rebased. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125640|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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125641|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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125642|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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125643|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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125833|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=29083 --- Comment #27 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125834 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125834&action=edit Bug 29083: Unit tests This patch adds missing tests for Koha::Patron->article_requests and moves (and extends) tests for 'article_requests_current' and 'article_requests_finished' that were originally in ArticleRequests.t into Koha/Patron.t as we now do. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 --- Comment #28 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125835 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125835&action=edit Bug 29083: Update article requests-related Koha::Patron methods to use relationships This patch makes Koha::Patron->article_requests use the underlying DBIC relationship and _new_from_dbic instead of a plain search. It also refactors 'article_requests_current' and 'article_requests_finished' to use ->article_requests, as well as the new methods introduced by bug 29082 for filtering. No behavior change should take place. To test: 1. Apply the unit tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/ArticleRequests.t => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 --- Comment #29 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125836 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125836&action=edit Bug 29083: (QA follow-up) Remove unused param Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 --- Comment #30 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125837 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125837&action=edit Bug 29083: Fix OPAC listing of article requests This patch makes the OPAC template reuse a precalculated value for the active article requests for the patron (and its count). The original code relied on the methods returning a list, which is not the case for _new_from_dbic until bug 28883 is pushed. This patch fixes that. Note: there was an odd behavior when ArticleRequests was enabled but no active article requests were present: the tab wasn't rendered but the 'empty table' with the 'You have no article requests currently.' message was displayed below the Checkouts tab. I'm not sure that was caused by this patches, or other. Fixed on this patch. To test: 1. In the OPAC, go to 'your summary' => FAIL: Things don't show for article requests 2. Add some article requests and repeat 1 => FAIL: Something's wrong there 3. Apply this patch and repeat 1 => Yes! Things show correctly! 4. Cancel all your article requests => SUCCESS: Things render as they should 5. Re-enter the 'your summary' page (to force re-rendering) => SUCCESS: Things render correctly for empty article requests 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <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=29083 --- Comment #31 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125838 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125838&action=edit Bug 29083: Remove article_requests_finished and article_requests_current This patch removes those methods that are not really needed. Templates are adjusted to use the expected combination of ->article_requests->filter_by_current. To test: 1. Apply this patch 2. Visit a patron with article requests => SUCCESS: All works 3. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass, less tests. 4. Sign off :-D 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=29083 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125838|0 |1 is obsolete| | --- Comment #32 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125840 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125840&action=edit Bug 29083: Remove article_requests_finished and article_requests_current This patch removes those methods that are not really needed. Templates are adjusted to use the expected combination of ->article_requests->filter_by_current. To test: 1. Apply this patch 2. Visit a patron with article requests => SUCCESS: All works 3. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t => SUCCESS: Tests pass, less tests. 4. Sign off :-D 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=29083 --- Comment #33 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Patches will be squashed when pushed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #34 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 125862 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125862&action=edit Bug 29083: Don't pass _count var -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #35 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #34)
Created attachment 125862 [details] [review] Bug 29083: Don't pass _count var
Something wrong with size? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #36 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.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=29083 --- Comment #37 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #35)
(In reply to Jonathan Druart from comment #34)
Created attachment 125862 [details] [review] [review] Bug 29083: Don't pass _count var
Something wrong with size?
It was used twice, not sure it was worth calculating twice. Don't you care about global warming? LOL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 --- Comment #38 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I am expecting it to be cached at some point at lower level (dbix or dbms). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 Bug 29083 depends on bug 29082, which changed state. Bug 29082 Summary: Add filtering methods to Koha::ArticleRequests https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29082 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=29083 Kyle M Hall <kyle@bywatersolutions.com> changed: 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=29083 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |Provides a small release notes| |performance enhancement and | |allows prefetching and | |embedding to work in the | |API -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org