Bug 29182

Summary: ArticleRequest status changing methods calling SUPER::store
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, kyle, m.de.rooy, martin.renvoize, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00
Bug Depends on: 27944    
Bug Blocks: 29093    
Attachments: Bug 29182: Regression tests
Bug 29182: Make sure methods call the overloaded ->store
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store
Bug 29182: Regression tests
Bug 29182: Make sure methods call the overloaded ->store
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store
Bug 29182: (follow-up) Fix ArticleRequests.t
Bug 29182: Regression tests
Bug 29182: Make sure methods call the overloaded ->store
Bug 29182: (follow-up) Simplify Koha::ArticleRequest->store
Bug 29182: (follow-up) Fix ArticleRequests.t

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!