Bug 29086 - Reuse article request filtering methods in Biblio template plugin
Summary: Reuse article request filtering methods in Biblio template plugin
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 29084
Blocks: 27949
  Show dependency treegraph
 
Reported: 2021-09-22 19:48 UTC by Tomás Cohen Arazi
Modified: 2021-10-08 15:02 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00


Attachments
Bug 29086: Simplify querying active article requests (1.67 KB, patch)
2021-09-22 19:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29086: (follow-up) Remove unrequired Koha::ArticleRequest::Status uses (1.29 KB, patch)
2021-09-22 19:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29086: Simplify querying active article requests (1.74 KB, patch)
2021-09-24 13:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29086: (follow-up) Remove unrequired Koha::ArticleRequest::Status uses (1.36 KB, patch)
2021-09-24 13:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29086: Simplify querying active article requests (1.83 KB, patch)
2021-09-27 08:25 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29086: (follow-up) Remove unrequired Koha::ArticleRequest::Status uses (1.45 KB, patch)
2021-09-27 08:25 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2021-09-22 19:48:48 UTC
Introduced by bug 29082, and covered by tests, we should use those higher level methods instead of crafting queries in-place.
Comment 1 Tomás Cohen Arazi 2021-09-22 19:55:55 UTC
Created attachment 125175 [details] [review]
Bug 29086: Simplify querying active article requests

This patch makes a trivial replacement of a query for a higher level
method that does the same query.

To test:
1. Enable the ArticleRequests feature
2. Place some article requests on known biblios
3. Have some article requests be active on others finished (either
   completed or cancelled)
4. Go to the detail page for those records
=> SUCCESS: The active article requests count is correct!
5. Sign off :-D
Comment 2 Tomás Cohen Arazi 2021-09-22 19:56:00 UTC
Created attachment 125176 [details] [review]
Bug 29086: (follow-up) Remove unrequired Koha::ArticleRequest::Status uses

I noticed a couple places in which this library was included but not
used.

To test:
1. Apply this patch
2. Run:
   $ git grep 'Koha::ArticleRequest::Status'
=> SUCCESS: There aren't any calls to their defined constants in
Biblio.pm or Patrons.pm
3. Sign off :-D
Comment 3 David Nind 2021-09-23 03:39:38 UTC
Everything worked fine in the staff interface.

However, in the OPAC no items are shown for the patron (Your account > Your summary > Article requests tab (has no number in (), and not items listed. This may be one of the dependcies causing this...

Testing notes:
- remember to run updatedatabase (for other dependencies)
Comment 4 Tomás Cohen Arazi 2021-09-23 11:24:31 UTC
(In reply to David Nind from comment #3)
> Everything worked fine in the staff interface.
> 
> However, in the OPAC no items are shown for the patron (Your account > Your
> summary > Article requests tab (has no number in (), and not items listed.
> This may be one of the dependcies causing this...
> 
> Testing notes:
> - remember to run updatedatabase (for other dependencies)

Thanks for testing! Will check it ASAP.
Comment 5 Tomás Cohen Arazi 2021-09-24 12:25:54 UTC
(In reply to Tomás Cohen Arazi from comment #4)
> (In reply to David Nind from comment #3)
> > Everything worked fine in the staff interface.
> > 
> > However, in the OPAC no items are shown for the patron (Your account > Your
> > summary > Article requests tab (has no number in (), and not items listed.
> > This may be one of the dependcies causing this...
> > 
> > Testing notes:
> > - remember to run updatedatabase (for other dependencies)
> 
> Thanks for testing! Will check it ASAP.

Solved, on bug 29083.
Comment 6 Martin Renvoize 2021-09-24 13:08:23 UTC
Created attachment 125246 [details] [review]
Bug 29086: Simplify querying active article requests

This patch makes a trivial replacement of a query for a higher level
method that does the same query.

To test:
1. Enable the ArticleRequests feature
2. Place some article requests on known biblios
3. Have some article requests be active on others finished (either
   completed or cancelled)
4. Go to the detail page for those records
=> SUCCESS: The active article requests count is correct!
5. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2021-09-24 13:08:27 UTC
Created attachment 125247 [details] [review]
Bug 29086: (follow-up) Remove unrequired Koha::ArticleRequest::Status uses

I noticed a couple places in which this library was included but not
used.

To test:
1. Apply this patch
2. Run:
   $ git grep 'Koha::ArticleRequest::Status'
=> SUCCESS: There aren't any calls to their defined constants in
Biblio.pm or Patrons.pm
3. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2021-09-24 13:08:44 UTC
Works as expected, signing off
Comment 9 Marcel de Rooy 2021-09-27 08:25:21 UTC
Created attachment 125347 [details] [review]
Bug 29086: Simplify querying active article requests

This patch makes a trivial replacement of a query for a higher level
method that does the same query.

To test:
1. Enable the ArticleRequests feature
2. Place some article requests on known biblios
3. Have some article requests be active on others finished (either
   completed or cancelled)
4. Go to the detail page for those records
=> SUCCESS: The active article requests count is correct!
5. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2021-09-27 08:25:25 UTC
Created attachment 125348 [details] [review]
Bug 29086: (follow-up) Remove unrequired Koha::ArticleRequest::Status uses

I noticed a couple places in which this library was included but not
used.

To test:
1. Apply this patch
2. Run:
   $ git grep 'Koha::ArticleRequest::Status'
=> SUCCESS: There aren't any calls to their defined constants in
Biblio.pm or Patrons.pm
3. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2021-09-27 08:25:51 UTC
Note that the dependency needs a fix.
Comment 12 Jonathan Druart 2021-10-07 09:21:17 UTC
Pushed to master for 21.11, thanks to everybody involved!