Bug 29765

Summary: Make Koha::Patron->safe_to_delete return a Koha::Result::Boolean object
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart+koha, kyle
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29788
Change sponsored?: --- Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
22.05.00
Bug Depends on: 29741, 29746    
Bug Blocks: 29772    
Attachments: Bug 29765: Make Koha::Patron->safe_to_delete use Koha::Result::Boolean
Bug 29765: Adjust DELETE /patrons/:patron_id controller
Bug 29765: Adjust DELETE /patrons/:patron_id controller
Bug 29765: Make Koha::Patron->safe_to_delete use Koha::Result::Boolean
Bug 29765: Adjust DELETE /patrons/:patron_id controller
Bug 29765: Make Koha::Patron->safe_to_delete use Koha::Result::Boolean
Bug 29765: Adjust DELETE /patrons/:patron_id controller

Description Tomás Cohen Arazi 2021-12-23 13:07:02 UTC
With the purpose of highlighting the value of the Koha::Result::Boolean class, this bug will migrate the current implementation with string return values to one using Koha::Result::Boolean instead
Comment 1 Tomás Cohen Arazi 2021-12-23 14:54:27 UTC
Created attachment 128887 [details] [review]
Bug 29765: Make Koha::Patron->safe_to_delete use Koha::Result::Boolean

This patch makes the safe_to_delete method in Koha::Patron return this
new object type instead of a plain string. This way we have a consistent
way to deal with 'can_*'-like methods return values when feedback is
needed.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: The adjusted tests pass. Tests cover the boolean context
eval and also the carried messages, that include the same string code
that was returned originally.
3. Sign off :-D
Comment 2 Tomás Cohen Arazi 2021-12-23 14:54:31 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2021-12-27 10:25:46 UTC
Created attachment 128914 [details] [review]
Bug 29765: Adjust DELETE /patrons/:patron_id controller

This patch adjusts the only current place using the
Koha::Patron->safe_to_delete method to the new return value. There
should be no behavior change

To test:
1. Apply the previous patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> FAIL: Tests fail, because of the change, the controller is broken
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! No test tweak required, no behavior change
5. Sign off :-D
Comment 4 David Nind 2021-12-29 10:08:30 UTC
Created attachment 128928 [details] [review]
Bug 29765: Make Koha::Patron->safe_to_delete use Koha::Result::Boolean

This patch makes the safe_to_delete method in Koha::Patron return this
new object type instead of a plain string. This way we have a consistent
way to deal with 'can_*'-like methods return values when feedback is
needed.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: The adjusted tests pass. Tests cover the boolean context
eval and also the carried messages, that include the same string code
that was returned originally.
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2021-12-29 10:08:38 UTC
Created attachment 128929 [details] [review]
Bug 29765: Adjust DELETE /patrons/:patron_id controller

This patch adjusts the only current place using the
Koha::Patron->safe_to_delete method to the new return value. There
should be no behavior change

To test:
1. Apply the previous patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> FAIL: Tests fail, because of the change, the controller is broken
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! No test tweak required, no behavior change
5. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Jonathan Druart 2022-01-03 14:39:27 UTC
If we are using Koha::Result::Boolean for Koha::Patron->safe_to_delete we certainly need to implement it for Koha::Item->safe_to_delete as well (same time, same push).
Comment 7 Tomás Cohen Arazi 2022-01-03 15:11:09 UTC
(In reply to Jonathan Druart from comment #6)
> If we are using Koha::Result::Boolean for Koha::Patron->safe_to_delete we
> certainly need to implement it for Koha::Item->safe_to_delete as well (same
> time, same push).

Sure, if we have consensus on things, I can do it.
Comment 8 Jonathan Druart 2022-01-04 15:20:51 UTC
Created attachment 128999 [details] [review]
Bug 29765: Make Koha::Patron->safe_to_delete use Koha::Result::Boolean

This patch makes the safe_to_delete method in Koha::Patron return this
new object type instead of a plain string. This way we have a consistent
way to deal with 'can_*'-like methods return values when feedback is
needed.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: The adjusted tests pass. Tests cover the boolean context
eval and also the carried messages, that include the same string code
that was returned originally.
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Jonathan Druart 2022-01-04 15:20:55 UTC
Created attachment 129000 [details] [review]
Bug 29765: Adjust DELETE /patrons/:patron_id controller

This patch adjusts the only current place using the
Koha::Patron->safe_to_delete method to the new return value. There
should be no behavior change

To test:
1. Apply the previous patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> FAIL: Tests fail, because of the change, the controller is broken
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! No test tweak required, no behavior change
5. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Fridolin Somers 2022-01-05 23:35:32 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄