Bug 22993 - Messaging preferences not set for patrons imported through API
Summary: Messaging preferences not set for patrons imported through API
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-27 14:01 UTC by Hans Pålsson
Modified: 2022-03-28 09:57 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.04,21.05.13,20.11.18


Attachments
Bug 22993: Unit tests (2.56 KB, patch)
2022-02-25 20:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22993: Handle default messaging preferences during patron creation from API (1.45 KB, patch)
2022-02-25 20:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22993: Unit tests (2.61 KB, patch)
2022-03-03 19:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22993: Handle default messaging preferences during patron creation from API (1.50 KB, patch)
2022-03-03 19:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22993: Unit tests (2.66 KB, patch)
2022-03-03 19:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 22993: Handle default messaging preferences during patron creation from API (1.56 KB, patch)
2022-03-03 19:43 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[20.11] Bug 22993: Handle default messaging preferences during patron creation through API (1.16 KB, patch)
2022-03-28 09:29 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Hans Pålsson 2019-05-27 14:01:35 UTC
It appears that from 18.11.03 (maybe earlier), patrons that are created/imported through the API does not get messaging preferences set as specified in the system preferences ("Default messaging preferences for this patron category"). This should work in the same way that messaging preferences are set for patrons created in the GUI.
Comment 1 Nick Clemens 2022-02-25 20:07:03 UTC
Created attachment 131131 [details] [review]
Bug 22993: Unit tests
Comment 2 Nick Clemens 2022-02-25 20:07:06 UTC
Created attachment 131132 [details] [review]
Bug 22993: Handle default messaging preferences during patron creation from API

TO test:
1 - Set default messaging preferences for a patron category
2 - Create a patron in that category using the API
3 - Verify messaging preferences are not set
4 - Apply patch, restart all
5 - Create another patron
6 - Verify messaging preferences are correctly set
7 - prove -v t/db_dependent/api/v1/patrons.t
Comment 3 Kyle M Hall 2022-03-03 19:34:58 UTC
Created attachment 131358 [details] [review]
Bug 22993: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Kyle M Hall 2022-03-03 19:35:09 UTC
Created attachment 131359 [details] [review]
Bug 22993: Handle default messaging preferences during patron creation from API

TO test:
1 - Set default messaging preferences for a patron category
2 - Create a patron in that category using the API
3 - Verify messaging preferences are not set
4 - Apply patch, restart all
5 - Create another patron
6 - Verify messaging preferences are correctly set
7 - prove -v t/db_dependent/api/v1/patrons.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Tomás Cohen Arazi 2022-03-03 19:43:01 UTC
Created attachment 131361 [details] [review]
Bug 22993: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2022-03-03 19:43:06 UTC
Created attachment 131362 [details] [review]
Bug 22993: Handle default messaging preferences during patron creation from API

TO test:
1 - Set default messaging preferences for a patron category
2 - Create a patron in that category using the API
3 - Verify messaging preferences are not set
4 - Apply patch, restart all
5 - Create another patron
6 - Verify messaging preferences are correctly set
7 - prove -v t/db_dependent/api/v1/patrons.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2022-03-03 19:45:14 UTC
Well done. Only members/memberentry.pl uses handle_form_action, the rest of the tools that add patrons do what Nick added to the API controller.
Comment 8 Fridolin Somers 2022-03-09 09:07:00 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 9 Kyle M Hall 2022-03-11 11:47:57 UTC
Pushed to 21.11.x for 21.11.04
Comment 10 Andrew Fuerste-Henry 2022-03-24 18:07:27 UTC
Pushed to 21.05.x for 21.05.13
Comment 11 Victor Grousset/tuxayo 2022-03-27 00:33:07 UTC
Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed.
Comment 12 Katrin Fischer 2022-03-27 00:50:40 UTC
Hi Victor, we applied this manually to our 20.11.16 branch, it's working and needed. Just had to move it a little as there was a tiny conflict with some additional code in master.
Comment 13 Victor Grousset/tuxayo 2022-03-28 09:01:25 UTC
I don't see a simple way to solve the conflict. Can you attach your 20.11.x version here?

Here is the conflict I get. We can see that there is another patch missing for 20.11.x that changes stuffs and this ticket just adds the block with if EnhancedMessagingPreferences
The 3 sections are:
<<<<<<<
the code on 20.11.x before the patch
|||||||
the code on 21.05.x before the patch
=======
the code on 21.05.x after the patch
>>>>>>>


<<<<<<< HEAD
        $c->res->headers->location( $c->req->url->to_string . '/' . $patron->borrowernumber );
        return $c->render(
            status  => 201,
            openapi => $patron->to_api
||||||| parent of ced32711bb (Bug 22993: Handle default messaging preferences during patron creation from API)
                my $body = $c->validation->param('body');

                my $extended_attributes = delete $body->{extended_attributes} // [];

                my $patron = Koha::Patron->new_from_api($body)->store;
                $patron->extended_attributes(
                    [
                        map { { code => $_->{type}, attribute => $_->{value} } }
                          @$extended_attributes
                    ]
                );

                $c->res->headers->location($c->req->url->to_string . '/' . $patron->borrowernumber);
                return $c->render(
                    status  => 201,
                    openapi => $patron->to_api
                );
            }
=======
                my $body = $c->validation->param('body');

                my $extended_attributes = delete $body->{extended_attributes} // [];

                my $patron = Koha::Patron->new_from_api($body)->store;
                $patron->extended_attributes(
                    [
                        map { { code => $_->{type}, attribute => $_->{value} } }
                          @$extended_attributes
                    ]
                );
                if ( C4::Context->preference('EnhancedMessagingPreferences') ) {
                    C4::Members::Messaging::SetMessagingPreferencesFromDefaults(
                        {
                            borrowernumber => $patron->borrowernumber,
                            categorycode   => $patron->categorycode,
                        }
                    );
                }

                $c->res->headers->location($c->req->url->to_string . '/' . $patron->borrowernumber);
                return $c->render(
                    status  => 201,
                    openapi => $patron->to_api
                );
            }
>>>>>>> ced32711bb (Bug 22993: Handle default messaging preferences during patron creation from API)
Comment 14 Victor Grousset/tuxayo 2022-03-28 09:24:27 UTC
the changes are due to bug 28002
Comment 15 Katrin Fischer 2022-03-28 09:29:43 UTC
Created attachment 132300 [details] [review]
[20.11] Bug 22993: Handle default messaging preferences during patron creation through API

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 16 Katrin Fischer 2022-03-28 09:30:53 UTC
Hi Victor, I have attached the patch from our branch. We might want to fix the author still.
Comment 17 Victor Grousset/tuxayo 2022-03-28 09:52:48 UTC
Ok, the code is independent. Moving the new if block in the old code worked :)

Backported: Pushed to 20.11.x branch for 20.11.18
Comment 18 Victor Grousset/tuxayo 2022-03-28 09:54:38 UTC
Mid air collision!
Thanks, after doublechecking, my solution is the same as yours so it will work :D
Comment 19 Katrin Fischer 2022-03-28 09:57:17 UTC
(In reply to Victor Grousset/tuxayo from comment #18)
> Mid air collision!
> Thanks, after doublechecking, my solution is the same as yours so it will
> work :D

That's the important bit :)