[Bug 29084] New: Update article requests-related Koha::Biblio methods to use relationships
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Bug ID: 29084 Summary: Update article requests-related Koha::Biblio 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 As bug 29083 does for Koha::Patron, this bug will deal the with Koha::Biblio counterpart. -- 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=29084 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Depends on| |29083 Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 [Bug 29083] Update article requests-related Koha::Patron methods to use relationships -- 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=29084 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=29084 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125163 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125163&action=edit Bug 29084: Unit tests This patch adds missing tests for Koha::Biblio->article_requests and reorganizes (and extends) the tests for 'article_requests_current' and 'article_requests_finished' that were originally in ArticleRequests.t into Koha/Biblio.t as we do now. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 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=29084 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125164 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125164&action=edit Bug 29084: Update article requests-related Koha::Biblio 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/Biblio.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=29084 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29086 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29086 [Bug 29086] Reuse article request filtering methods in Biblio template plugin -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=29084 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125163|0 |1 is obsolete| | --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125244 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125244&action=edit Bug 29084: Unit tests This patch adds missing tests for Koha::Biblio->article_requests and reorganizes (and extends) the tests for 'article_requests_current' and 'article_requests_finished' that were originally in ArticleRequests.t into Koha/Biblio.t as we do now. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 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=29084 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125164|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125245 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125245&action=edit Bug 29084: Update article requests-related Koha::Biblio 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/Biblio.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=29084 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |martin.renvoize@ptfs-europe | |.com --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Another nice improvement.. no regressions found. Signing off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Failed QA --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- I followed the chain up till here and somewhere it goes wrong. But since I discover it here, I will blame this report :) I have a patron with 10 article requests. OPAC shows 10 using [% current_article_requests_count %] Fine! Intranet patron details shows None! What does it use? koha-tmpl/intranet-tmpl/prog/en/includes/patron-article-requests.inc [% IF patron.article_requests_current.count %] There is our famous bottleneck from bug 28883. TT runs the article_requests_current call in LIST context although you think that you are chaining. So the above count does NOT work ! If I do [% SET x = patron.article_requests_current %] [% x.size %] then I see that x contains 10 entries. The problem is somewhere earlier. But I wouldnt mind correcting it here, and not disturb all previous QAs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- sub article_requests_current { return $self->article_requests->filter_by_current; sub article_requests_finished { return $self->article_requests->filter_by_finished; How are they useful? Callers can call $biblio->article_requests->filter_by_* -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #8)
sub article_requests_current { return $self->article_requests->filter_by_current;
sub article_requests_finished { return $self->article_requests->filter_by_finished;
How are they useful? Callers can call $biblio->article_requests->filter_by_*
Agreed. My plan was to remove them. But it felt like it was better to keep them to prove no behavior change. And then remove them while refactoring the controllers. And why not just remove the controllers if I had the chance :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125244|0 |1 is obsolete| | --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125841 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125841&action=edit Bug 29084: Unit tests This patch adds missing tests for Koha::Biblio->article_requests and reorganizes (and extends) the tests for 'article_requests_current' and 'article_requests_finished' that were originally in ArticleRequests.t into Koha/Biblio.t as we do now. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/ArticleRequests.t \ t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 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=29084 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125245|0 |1 is obsolete| | --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125842 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125842&action=edit Bug 29084: Update article requests-related Koha::Biblio 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/Biblio.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=29084 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 125843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125843&action=edit Bug 29084: 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 biblio with article requests => SUCCESS: All works 3. Run: $ kshell k$ prove t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass, less tests. 4. 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=29084 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Went the Context.Scalar way on this one as well. Back to PQA. Please note that the unit tests need the latest follow-up I submitted on bug 27526. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Applying: Bug 29084: Unit tests error: sha1 information is lacking or useless (t/db_dependent/Koha/Biblio.t). error: could not build fake ancestor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #14)
Applying: Bug 29084: Unit tests error: sha1 information is lacking or useless (t/db_dependent/Koha/Biblio.t). error: could not build fake ancestor
You need the last patch from bug 27526. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 --- Comment #16 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=29084 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 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=29084 --- Comment #17 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=29084 Bug 29084 depends on bug 29083, which changed state. Bug 29083 Summary: Update article requests-related Koha::Patron methods to use relationships https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29083 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=29084 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29084 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