[Bug 29886] New: Add Koha::Suggestions->search_limited
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Bug ID: 29886 Summary: Add Koha::Suggestions->search_limited 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 SearchSuggestion does filtering on IndependentBranches, etc. We need to implement Koha::Suggestions->search_limited that does that as well. -- 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=29886 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart+koha@gmail. |ity.org |com Blocks| |23991 Status|NEW |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23991 [Bug 23991] Move SearchSuggestion to Koha::Suggestions -- 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=29886 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129508 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129508&action=edit Bug 29886: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129509 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129509&action=edit Bug 29886: Add Koha::Suggestions->search_limited 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=29886 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff CC| |jonathan.druart+koha@gmail. | |com, | |kyle@bywatersolutions.com, | |nick@bywatersolutions.com Assignee|jonathan.druart+koha@gmail. |tomascohen@gmail.com |com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29888 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 David Nind <david@davidnind.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=29886 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129508|0 |1 is obsolete| | --- Comment #3 from David Nind <david@davidnind.com> --- Created attachment 129709 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129709&action=edit Bug 29886: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129509|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 129710 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129710&action=edit Bug 29886: Add Koha::Suggestions->search_limited Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #5 from David Nind <david@davidnind.com> --- Test plan used: 1. Create some suggestions for different libraries using the OPAC. 2. Log in as a staff user without super librarian privileges and view the suggestions - you should see suggestions for all libraries. 3. Enable the IndependentBranches system preference. 4. Repeat step 2 - you should only see suggestions for the staff user's library. 5. Run the tests - these should all pass: prove t/db_dependent/Koha/Suggestions.t 6. Apply the patch. 7. Repeat step 2-5 - there should be no change in behaviour, and things should work as expected. 8. Sign-off! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This is the third Koha::PluralClass::search_limited method we've introduced recently.. and they're all nearly the same. Should we not DRY this code out and adopt something similar to Koha::Objects::Limit::Library here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmm.. this is very different to our other search_limited methods.. Instead of using the Koha::Patron->libraries_where_can_see_patrons we're doing some special cases IndependantBranches stuff.. I'm wondering if we ought to be using the existing limitation method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- The 'C4::Context::only_my_library' call in the other search_limited form should catch the same IndependantBranches stuff as this does.. Given we have good tests written here, I'll try to adapt it and use the tests to prove the route. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Ack, more confusion here.. We are limiting by Suggestion branch.. which I think makes sense.. but could expose more than we're expecting (we could perhaps limit on suggester.branchcode to limit to only suggestions made by users from the branches you are allowed to see). ArticleRequests takes the above approach and limits by the requester.branchcode.. (but this could also be seen as a possible bug.. as you may want to see all article requests at the current branch.. regardless of who requested them and where they're based) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30019 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Those limits are inherited from C4::Suggestions::SearchSuggestion, which is to be removed on bug 23991. I stripped this from that bug report in order to backport it to 21.11, which implements /suggestions routes that really need this filter. If the logic is not correct, then it is not correct in Koha already. And we should probably discuss it in a follow-up bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129709|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 130294 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130294&action=edit Bug 29886: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> 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=29886 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129710|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 130295 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130295&action=edit Bug 29886: Add Koha::Suggestions->search_limited Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> 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=29886 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 130296 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130296&action=edit Bug 29886: (QA follow-up) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130296|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=29886 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Oops, didn't mean to upload that last one.. deprecating. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Right.. I decided to PQA this as it stands. It makes sense that we limit only by IndependantBranches, though I'd love to see it moved to utilise the library groups feature in the future. As for the different style of search_limited, it also makes sense here.. we're limiting the object by it's branch and the current users permission to see that branch.. it has nothing to do with the patrons attached to the suggestion (that should be getting handled at another level) I will open a bug to discuss how I believe the same method in ArticleRequests is wrong on this matter. So.. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30048 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.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=29886 --- Comment #16 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, 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=29886 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|22.05.00 |22.05.00,21.11.04 released in| | --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.11.x for 21.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.05.00,21.11.04 |22.05.00,21.11.04,21.05.13 released in| | CC| |andrew@bywatersolutions.com Status|Pushed to stable |Pushed to oldstable --- Comment #18 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.13 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29886 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Resolution|--- |FIXED Status|Pushed to oldstable |RESOLVED --- Comment #19 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org