Bug 29182 - ArticleRequest status changing methods calling SUPER::store
Summary: ArticleRequest status changing methods calling SUPER::store
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 27944
Blocks: 29093
  Show dependency treegraph
 
Reported: 2021-10-06 11:53 UTC by Tomás Cohen Arazi
Modified: 2022-06-06 20:24 UTC (History)
5 users (show)

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


Attachments
Bug 29182: Regression tests (2.57 KB, patch)
2021-10-06 12:04 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29182: Make sure methods call the overloaded ->store (1.57 KB, patch)
2021-10-06 12:04 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store (907 bytes, patch)
2021-10-06 12:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29182: Regression tests (2.67 KB, patch)
2021-10-06 12:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29182: Make sure methods call the overloaded ->store (1.67 KB, patch)
2021-10-06 12:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store (1005 bytes, patch)
2021-10-06 12:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29182: (follow-up) Fix ArticleRequests.t (958 bytes, patch)
2021-10-07 09:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29182: Regression tests (2.69 KB, patch)
2021-10-07 12:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29182: Make sure methods call the overloaded ->store (1.69 KB, patch)
2021-10-07 12:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store (1.00 KB, patch)
2021-10-07 12:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29182: (follow-up) Fix ArticleRequests.t (983 bytes, patch)
2021-10-07 12:13 UTC, Martin Renvoize
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-10-06 11:53:29 UTC
Some methods are still using $self->SUPER::store instead of the overloaded method, so when you create a new request, it doesn't get things like created_on set.
Comment 1 Tomás Cohen Arazi 2021-10-06 12:04:13 UTC
Created attachment 125779 [details] [review]
Bug 29182: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2021-10-06 12:04:18 UTC
Created attachment 125780 [details] [review]
Bug 29182: Make sure methods call the overloaded ->store

This patch addresses the issue of `request()` and `set_pending()`
calling $self->SUPER::store instead of the overloaded one.

This prevents created_on being set.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/ArticleRequest.t
=> FAIL: created_on is not set on two methods
3. Apply this patch
4. Repeat 2
=> SUCCESS: created_on is now set!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2021-10-06 12:11:07 UTC
Created attachment 125783 [details] [review]
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Marcel de Rooy 2021-10-06 12:11:41 UTC
Created attachment 125784 [details] [review]
Bug 29182: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2021-10-06 12:11:44 UTC
Created attachment 125785 [details] [review]
Bug 29182: Make sure methods call the overloaded ->store

This patch addresses the issue of `request()` and `set_pending()`
calling $self->SUPER::store instead of the overloaded one.

This prevents created_on being set.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/ArticleRequest.t
=> FAIL: created_on is not set on two methods
3. Apply this patch
4. Repeat 2
=> SUCCESS: created_on is now set!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2021-10-06 12:16:14 UTC
Created attachment 125786 [details] [review]
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2021-10-07 08:49:02 UTC
4/54 Can't call method "letter_code" on an undefined value at /kohadevbox/koha/t/db_dependent/ArticleRequests.t line 82.
# Looks like your test exited with 11 just after 4.
Comment 8 Marcel de Rooy 2021-10-07 09:57:27 UTC
Created attachment 125869 [details] [review]
Bug 29182: (follow-up) Fix ArticleRequests.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Martin Renvoize 2021-10-07 12:12:42 UTC
Created attachment 125871 [details] [review]
Bug 29182: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2021-10-07 12:12:46 UTC
Created attachment 125872 [details] [review]
Bug 29182: Make sure methods call the overloaded ->store

This patch addresses the issue of `request()` and `set_pending()`
calling $self->SUPER::store instead of the overloaded one.

This prevents created_on being set.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/ArticleRequest.t
=> FAIL: created_on is not set on two methods
3. Apply this patch
4. Repeat 2
=> SUCCESS: created_on is now set!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2021-10-07 12:12:58 UTC
Created attachment 125873 [details] [review]
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2021-10-07 12:13:02 UTC
Created attachment 125874 [details] [review]
Bug 29182: (follow-up) Fix ArticleRequests.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2021-10-07 12:16:38 UTC
All works well, QA scripts happy and Tests passing..

Passing QA.  It's great to see this area getting some attention and modernisation.
Comment 14 Jonathan Druart 2021-10-07 13:48:35 UTC
Pushed to master for 21.11, thanks to everybody involved!