Bug 17499 - Koha objects for messaging preferences
Summary: Koha objects for messaging preferences
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lari Taskula
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 17505 18595
  Show dependency treegraph
 
Reported: 2016-10-26 11:53 UTC by Lari Taskula
Modified: 2023-11-30 16:14 UTC (History)
12 users (show)

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


Attachments
Bug 17499: Add Koha-objects for messaging preferences (20.89 KB, patch)
2017-05-12 13:25 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add useful Koha::Exceptions (1.52 KB, patch)
2017-05-12 13:25 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add basic validation for messaging preference (5.48 KB, patch)
2017-05-12 13:26 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Throw an exception on duplicate messaging preference (4.03 KB, patch)
2017-05-12 13:26 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add a method for getting messaging options (4.50 KB, patch)
2017-05-12 13:26 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add a method for setting default messaging preferences (8.17 KB, patch)
2017-05-12 13:26 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Improve object usability by enabling direct access to transport preferences (16.89 KB, patch)
2017-05-12 13:27 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Simplify searching by message_name (4.62 KB, patch)
2017-05-12 13:27 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (35.08 KB, patch)
2017-05-29 09:56 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add useful Koha::Exceptions (1.52 KB, patch)
2017-05-29 09:56 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add basic validation for messaging preference (8.36 KB, patch)
2017-05-29 09:56 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Throw an exception on duplicate messaging preference (5.80 KB, patch)
2017-05-29 09:56 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add a method for getting messaging options (4.63 KB, patch)
2017-05-29 09:57 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add a method for setting default messaging preferences (11.26 KB, patch)
2017-05-29 09:57 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Improve object usability by enabling direct access to transport preferences (17.65 KB, patch)
2017-05-29 09:58 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Simplify searching by message_name (5.63 KB, patch)
2017-05-29 09:58 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (35.21 KB, patch)
2017-05-29 12:26 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: Add useful Koha::Exceptions (1.65 KB, patch)
2017-05-29 12:27 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: Add basic validation for messaging preference (8.49 KB, patch)
2017-05-29 12:28 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: Throw an exception on duplicate messaging preference (5.93 KB, patch)
2017-05-29 12:29 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: Add a method for getting messaging options (4.77 KB, patch)
2017-05-29 12:30 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: Add a method for setting default messaging preferences (11.39 KB, patch)
2017-05-29 12:31 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: Improve object usability by enabling direct access to transport preferences (17.79 KB, patch)
2017-05-29 12:33 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: Simplify searching by message_name (5.83 KB, patch)
2017-05-29 12:34 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: (follow-up) Bugfix for validation (7.96 KB, patch)
2017-05-29 12:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Bugfix for validation (8.12 KB, patch)
2017-05-29 12:39 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: (follow-up) Bugfix for validation (8.90 KB, patch)
2017-05-29 12:40 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Bugfix for validation (9.17 KB, patch)
2017-05-29 13:03 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17499: (follow-up) Bugfix for validation, 2 (6.51 KB, patch)
2017-05-31 14:57 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Bugfix for validation, 2 (11.21 KB, patch)
2017-06-01 11:56 UTC, Lari Taskula
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: Add Koha-objects for messaging preferences (35.28 KB, patch)
2017-06-06 11:05 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: Add useful Koha::Exceptions (1.72 KB, patch)
2017-06-06 11:05 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: Add basic validation for messaging preference (8.56 KB, patch)
2017-06-06 11:05 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: Throw an exception on duplicate messaging preference (6.00 KB, patch)
2017-06-06 11:05 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: Add a method for getting messaging options (4.83 KB, patch)
2017-06-06 11:05 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: Add a method for setting default messaging preferences (11.46 KB, patch)
2017-06-06 11:05 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: Improve object usability by enabling direct access to transport preferences (17.85 KB, patch)
2017-06-06 11:05 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: Simplify searching by message_name (5.89 KB, patch)
2017-06-06 11:06 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: (follow-up) Bugfix for validation (9.24 KB, patch)
2017-06-06 11:06 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17499: (follow-up) Bugfix for validation, 2 (11.28 KB, patch)
2017-06-06 11:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add information on digest being the only option to get_options (1.30 KB, patch)
2017-07-05 09:42 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (84.39 KB, patch)
2017-09-01 06:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (80.72 KB, patch)
2017-12-01 14:29 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (80.73 KB, patch)
2017-12-01 14:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (80.79 KB, patch)
2017-12-01 20:27 UTC, Dominic Pichette
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (194.30 KB, patch)
2019-07-30 11:35 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (80.84 KB, patch)
2019-10-08 09:48 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.82 KB, patch)
2019-10-08 13:27 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.82 KB, patch)
2019-10-08 14:28 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.83 KB, patch)
2019-10-25 12:18 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.35 KB, patch)
2020-02-24 09:24 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.25 KB, patch)
2020-02-26 10:06 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.95 KB, patch)
2020-02-26 17:01 UTC, David Nind
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.39 KB, patch)
2020-02-26 17:01 UTC, David Nind
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.32 KB, patch)
2020-02-26 17:01 UTC, David Nind
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.89 KB, patch)
2020-04-22 09:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.39 KB, patch)
2020-04-22 09:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.29 KB, patch)
2020-04-22 09:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.95 KB, patch)
2020-07-08 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.46 KB, patch)
2020-07-08 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.36 KB, patch)
2020-07-08 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.05 KB, patch)
2020-07-08 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (3.97 KB, patch)
2020-07-08 12:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (84.10 KB, patch)
2020-07-30 22:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.51 KB, patch)
2020-07-30 22:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.43 KB, patch)
2020-07-30 22:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.11 KB, patch)
2020-07-30 22:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (4.03 KB, patch)
2020-07-30 22:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (24.46 KB, patch)
2020-11-04 15:55 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (24.41 KB, patch)
2020-11-04 16:06 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (25.85 KB, patch)
2020-11-05 17:34 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.45 KB, patch)
2020-11-05 18:31 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.37 KB, patch)
2020-11-17 00:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.45 KB, patch)
2020-11-17 00:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.46 KB, patch)
2020-11-18 01:17 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.45 KB, patch)
2020-11-18 01:17 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Validate phone and itiva transport types (7.65 KB, patch)
2020-11-18 04:24 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Contact information vs. mtt validation (10.74 KB, patch)
2020-11-18 04:24 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference (89.06 KB, patch)
2021-03-18 03:47 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor (3.31 KB, patch)
2021-03-18 03:47 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (84.01 KB, patch)
2021-04-12 19:36 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.51 KB, patch)
2021-04-12 19:36 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.41 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.11 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (4.03 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.48 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.45 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Validate phone and itiva transport types (7.63 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Contact information vs. mtt validation (10.72 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference (89.06 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor (3.30 KB, patch)
2021-04-12 19:37 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (84.14 KB, patch)
2021-04-12 19:50 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.56 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.48 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.16 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (4.07 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.56 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.49 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: (follow-up) Validate phone and itiva transport types (7.69 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: (follow-up) Contact information vs. mtt validation (10.78 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference (89.23 KB, patch)
2021-04-12 19:51 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor (3.36 KB, patch)
2021-04-12 19:52 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.73 KB, patch)
2022-10-06 12:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.56 KB, patch)
2022-10-06 12:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.46 KB, patch)
2022-10-06 12:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.16 KB, patch)
2022-10-06 12:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (4.07 KB, patch)
2022-10-06 12:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.53 KB, patch)
2022-10-06 12:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.49 KB, patch)
2022-10-06 12:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Validate phone and itiva transport types (7.68 KB, patch)
2022-10-06 12:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Contact information vs. mtt validation (10.77 KB, patch)
2022-10-06 12:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference (89.10 KB, patch)
2022-10-06 12:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor (3.35 KB, patch)
2022-10-06 12:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.75 KB, patch)
2023-02-11 14:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.56 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.46 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.16 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (4.07 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.53 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.49 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Validate phone and itiva transport types (7.68 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Contact information vs. mtt validation (10.77 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference (89.11 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor (3.35 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Replace Koha::MessageAttribute(s) with Koha::Patron::MessagePreference::Attribute(s) (4.58 KB, patch)
2023-02-11 14:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.75 KB, patch)
2023-06-01 10:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.56 KB, patch)
2023-06-01 10:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.46 KB, patch)
2023-06-01 10:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.16 KB, patch)
2023-06-01 10:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (4.07 KB, patch)
2023-06-01 10:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.53 KB, patch)
2023-06-01 10:04 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.49 KB, patch)
2023-06-01 10:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: (follow-up) Validate phone and itiva transport types (7.68 KB, patch)
2023-06-01 10:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: (follow-up) Contact information vs. mtt validation (10.77 KB, patch)
2023-06-01 10:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference (89.11 KB, patch)
2023-06-01 10:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor (3.35 KB, patch)
2023-06-01 10:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: Replace Koha::MessageAttribute(s) with Koha::Patron::MessagePreference::Attribute(s) (4.58 KB, patch)
2023-06-01 10:05 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.79 KB, patch)
2023-06-01 16:03 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.60 KB, patch)
2023-06-01 16:03 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.50 KB, patch)
2023-06-01 16:03 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.20 KB, patch)
2023-06-01 16:03 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (4.12 KB, patch)
2023-06-01 16:04 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.57 KB, patch)
2023-06-01 16:04 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.54 KB, patch)
2023-06-01 16:04 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: (follow-up) Validate phone and itiva transport types (7.72 KB, patch)
2023-06-01 16:04 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: (follow-up) Contact information vs. mtt validation (10.81 KB, patch)
2023-06-01 16:04 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference (89.16 KB, patch)
2023-06-01 16:04 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor (3.39 KB, patch)
2023-06-01 16:04 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: Replace Koha::MessageAttribute(s) with Koha::Patron::MessagePreference::Attribute(s) (4.63 KB, patch)
2023-06-01 16:04 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (83.88 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: Define koha_object(s)_class where needed (3.70 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: Replace test subroutines with build_object (14.60 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix Licence Statements (17.29 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: (QA follow-up) Fix POD errors (4.21 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: (follow-up) More explicit exceptions (26.67 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: (follow-up) Fix warning (1.63 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: (follow-up) Validate phone and itiva transport types (7.82 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: (follow-up) Contact information vs. mtt validation (10.91 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference (89.25 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor (3.49 KB, patch)
2023-09-08 08:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17499: Replace Koha::MessageAttribute(s) with Koha::Patron::MessagePreference::Attribute(s) (4.72 KB, patch)
2023-09-08 08:02 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 Lari Taskula 2016-10-26 11:53:27 UTC
Create Koha objects for messaging preferences. We need to create REST API endpoint for managing patron messaging preferences and this is the first step.
Comment 1 Lari Taskula 2017-05-12 13:25:38 UTC Comment hidden (obsolete)
Comment 2 Lari Taskula 2017-05-12 13:25:59 UTC Comment hidden (obsolete)
Comment 3 Lari Taskula 2017-05-12 13:26:11 UTC Comment hidden (obsolete)
Comment 4 Lari Taskula 2017-05-12 13:26:24 UTC Comment hidden (obsolete)
Comment 5 Lari Taskula 2017-05-12 13:26:35 UTC Comment hidden (obsolete)
Comment 6 Lari Taskula 2017-05-12 13:26:48 UTC Comment hidden (obsolete)
Comment 7 Lari Taskula 2017-05-12 13:27:00 UTC Comment hidden (obsolete)
Comment 8 Lari Taskula 2017-05-12 13:27:13 UTC Comment hidden (obsolete)
Comment 9 Lari Taskula 2017-05-29 09:56:14 UTC
Created attachment 63786 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences. This patch does not
add any extra logic into these objects.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*
Comment 10 Lari Taskula 2017-05-29 09:56:25 UTC
Created attachment 63787 [details] [review]
Bug 17499: Add useful Koha::Exceptions

This patch adds some basic Koha::Exceptions that will be useful.
Comment 11 Lari Taskula 2017-05-29 09:56:44 UTC
Created attachment 63788 [details] [review]
Bug 17499: Add basic validation for messaging preference

This patch adds simple validation for messaging preferences.

The validation includes
- check that only either borrowernumber or categorycode is given, but not both
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t
Comment 12 Lari Taskula 2017-05-29 09:56:58 UTC
Created attachment 63789 [details] [review]
Bug 17499: Throw an exception on duplicate messaging preference

When trying to add a duplicate messaging preference that has the same
borrowernumber or category, and message_attribute_id as another preference,
throw Koha::Exceptions::DuplicateObject.

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t
Comment 13 Lari Taskula 2017-05-29 09:57:10 UTC
Created attachment 63790 [details] [review]
Bug 17499: Add a method for getting messaging options

This patch adds a method for getting available messaging options.

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t
Comment 14 Lari Taskula 2017-05-29 09:57:36 UTC
Created attachment 63791 [details] [review]
Bug 17499: Add a method for setting default messaging preferences

This patch adds a method Koha::Patron::Message::Preference->new_from_default that
can be used to add category's default messaging preferences to patron for a given
message type.

Example call:

Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
});

Also adds a simple method for Koha::Patron, set_default_messaging_preferences.
Usage: $patron->set_default_messaging_preferences()

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t
Comment 15 Lari Taskula 2017-05-29 09:58:03 UTC
Created attachment 63792 [details] [review]
Bug 17499: Improve object usability by enabling direct access to transport preferences

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating.

This patch adds a feature for Koha::Patron::Message::Preference which enables
access to message transport preferences directly via this object. It allows us
to skip calls to Koha::Patron::Message::Transport::Preference(s) because we can
now get and set via K::P::M::Preference.

Get:
$preference->message_transport_types
Returns a hashref, where each key is stored transport type and value for the key
is letter code for the transport.

Set:
$preference->set({ message_transport_types => ['sms'] }) or
$preference->message_transport_types('email', 'sms') or
$preference->message_transport_types(['email', 'sms'])
Returns $self (Koha::Patron::Message::Preference object)

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t
Comment 16 Lari Taskula 2017-05-29 09:58:23 UTC
Created attachment 63793 [details] [review]
Bug 17499: Simplify searching by message_name

This patch adds optional search & find parameter message_name for
Koha::Patron::Message::Preferences->search and ->find. This simplifies object
usage by allowing us to skip joins or finding the attribute id ourselves.
Comment 17 Marc Véron 2017-05-29 12:24:44 UTC
Walking through 8 patches, following test plans...
1 OK
1-3 OK
1-4 OK
1-5 OK
1-6 OK
1-7 OK
1-8 OK - Test plan for 8 is the same: 
         prove -v t/db_dependent/Koha/Patron/Message/Preferences.t
Comment 18 Marc Véron 2017-05-29 12:26:25 UTC
Created attachment 63801 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences. This patch does not
add any extra logic into these objects.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 19 Marc Véron 2017-05-29 12:27:14 UTC
Created attachment 63802 [details] [review]
Bug 17499: Add useful Koha::Exceptions

This patch adds some basic Koha::Exceptions that will be useful.

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 20 Marc Véron 2017-05-29 12:28:08 UTC
Created attachment 63803 [details] [review]
Bug 17499: Add basic validation for messaging preference

This patch adds simple validation for messaging preferences.

The validation includes
- check that only either borrowernumber or categorycode is given, but not both
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 21 Marc Véron 2017-05-29 12:29:10 UTC
Created attachment 63804 [details] [review]
Bug 17499: Throw an exception on duplicate messaging preference

When trying to add a duplicate messaging preference that has the same
borrowernumber or category, and message_attribute_id as another preference,
throw Koha::Exceptions::DuplicateObject.

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 22 Marc Véron 2017-05-29 12:30:49 UTC
Created attachment 63805 [details] [review]
Bug 17499: Add a method for getting messaging options

This patch adds a method for getting available messaging options.

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 23 Marc Véron 2017-05-29 12:31:58 UTC
Created attachment 63806 [details] [review]
Bug 17499: Add a method for setting default messaging preferences

This patch adds a method Koha::Patron::Message::Preference->new_from_default that
can be used to add category's default messaging preferences to patron for a given
message type.

Example call:

Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
});

Also adds a simple method for Koha::Patron, set_default_messaging_preferences.
Usage: $patron->set_default_messaging_preferences()

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 24 Marc Véron 2017-05-29 12:33:09 UTC
Created attachment 63807 [details] [review]
Bug 17499: Improve object usability by enabling direct access to transport preferences

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating.

This patch adds a feature for Koha::Patron::Message::Preference which enables
access to message transport preferences directly via this object. It allows us
to skip calls to Koha::Patron::Message::Transport::Preference(s) because we can
now get and set via K::P::M::Preference.

Get:
$preference->message_transport_types
Returns a hashref, where each key is stored transport type and value for the key
is letter code for the transport.

Set:
$preference->set({ message_transport_types => ['sms'] }) or
$preference->message_transport_types('email', 'sms') or
$preference->message_transport_types(['email', 'sms'])
Returns $self (Koha::Patron::Message::Preference object)

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 25 Marc Véron 2017-05-29 12:34:40 UTC
Created attachment 63808 [details] [review]
Bug 17499: Simplify searching by message_name

This patch adds optional search & find parameter message_name for
Koha::Patron::Message::Preferences->search and ->find. This simplifies object
usage by allowing us to skip joins or finding the attribute id ourselves.

prove -v t/db_dependent/Koha/Patron/Message/Preferences.t turns green
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 26 Lari Taskula 2017-05-29 12:34:57 UTC
Created attachment 63809 [details] [review]
Bug 17499: (follow-up) Bugfix for validation

- Bugfix for days_in_advance and wants_digest validation. Have to check definedness
  of value instead the value itself.
- Fixes broken tests caused by above update.
- Adding a missing unit test for wants_digest validation
- Minor edits to exception messages.
- Throwing exception for invalid message_attribute_id. Incl. unit test
Comment 27 Marc Véron 2017-05-29 12:39:51 UTC
Created attachment 63810 [details] [review]
Bug 17499: (follow-up) Bugfix for validation

- Bugfix for days_in_advance and wants_digest validation. Have to check definedness
  of value instead the value itself.
- Fixes broken tests caused by above update.
- Adding a missing unit test for wants_digest validation
- Minor edits to exception messages.
- Throwing exception for invalid message_attribute_id. Incl. unit test

Unit test turns green.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 28 Lari Taskula 2017-05-29 12:40:24 UTC
Created attachment 63811 [details] [review]
Bug 17499: (follow-up) Bugfix for validation

- Bugfix for days_in_advance and wants_digest validation. Have to check definedness
  of value instead the value itself.
- Fixes broken tests caused by above update.
- Adding a missing unit test for wants_digest validation
- Minor edits to exception messages.
- Throwing exception for invalid message_attribute_id. Incl. unit test
Comment 29 Marc Véron 2017-05-29 13:03:28 UTC
Created attachment 63812 [details] [review]
Bug 17499: (follow-up) Bugfix for validation

- Bugfix for days_in_advance and wants_digest validation. Have to check definedness
  of value instead the value itself.
- Fixes broken tests caused by above update.
- Adding a missing unit test for wants_digest validation
- Minor edits to exception messages.
- Throwing exception for invalid message_attribute_id. Incl. unit test

Tested again, prove -v t/db_dependent/Koha/Patron/Message/Preferences.t turns green
prove -v t/db_dependent/Koha/Patron/Message/* turns green
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 30 Marc Véron 2017-05-29 13:05:40 UTC
Tested 
- editing messaging preferences in staff client and opac
- check out an item to have an email sent
All OK.

Signing off.
Comment 31 Lari Taskula 2017-05-29 13:21:01 UTC
(In reply to Marc Véron from comment #30)
> Tested 
> - editing messaging preferences in staff client and opac
> - check out an item to have an email sent
> All OK.
> 
> Signing off.
Please note that the actual move from C4::Members::Messaging to these Koha-objects is in Bug 18595. This Bug just introduces new Koha-objects for messaging preferences. This Bug doesn't yet use them in action.

I did it like this because personally I am just interested in deprecating the C4::Members::Messaging methods and instead having the Koha-objects to use with REST API.
Comment 32 Lari Taskula 2017-05-31 14:57:31 UTC
Created attachment 63878 [details] [review]
Bug 17499: (follow-up) Bugfix for validation, 2

Remove duplicate validation for digest availability. This is already checked in
$preference->validate().

Also prevents a crash when patron has messaging transport type selected for
a message for which it should not be available. This patch logs it with as
a warning, and only throws an exception when attempting to set it.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*
Comment 33 Lari Taskula 2017-05-31 15:27:22 UTC
Removing Needs Signoff status, I still noticed some bugs when using this together with REST API, will also attach tests for the latest patch.
Comment 34 Lari Taskula 2017-06-01 11:56:32 UTC
Created attachment 63895 [details] [review]
Bug 17499: (follow-up) Bugfix for validation, 2

Remove duplicate validation for digest availability. This is already checked in
$preference->validate().

Set values for current object before validating messaging transport types, not
the other way around as it was before this patch.

Also prevents a crash when patron has messaging transport type selected for
a message for which it should not be available. This patch logs it with as
a warning, and only throws an exception when attempting to set it.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*
Comment 35 Josef Moravec 2017-06-06 11:05:36 UTC
Created attachment 64035 [details] [review]
[SIGNED-OFF] Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences. This patch does not
add any extra logic into these objects.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 36 Josef Moravec 2017-06-06 11:05:40 UTC
Created attachment 64036 [details] [review]
[SIGNED-OFF] Bug 17499: Add useful Koha::Exceptions

This patch adds some basic Koha::Exceptions that will be useful.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 37 Josef Moravec 2017-06-06 11:05:43 UTC
Created attachment 64037 [details] [review]
[SIGNED-OFF] Bug 17499: Add basic validation for messaging preference

This patch adds simple validation for messaging preferences.

The validation includes
- check that only either borrowernumber or categorycode is given, but not both
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 38 Josef Moravec 2017-06-06 11:05:46 UTC
Created attachment 64038 [details] [review]
[SIGNED-OFF] Bug 17499: Throw an exception on duplicate messaging preference

When trying to add a duplicate messaging preference that has the same
borrowernumber or category, and message_attribute_id as another preference,
throw Koha::Exceptions::DuplicateObject.

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 39 Josef Moravec 2017-06-06 11:05:49 UTC
Created attachment 64039 [details] [review]
[SIGNED-OFF] Bug 17499: Add a method for getting messaging options

This patch adds a method for getting available messaging options.

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 40 Josef Moravec 2017-06-06 11:05:53 UTC
Created attachment 64040 [details] [review]
[SIGNED-OFF] Bug 17499: Add a method for setting default messaging preferences

This patch adds a method Koha::Patron::Message::Preference->new_from_default that
can be used to add category's default messaging preferences to patron for a given
message type.

Example call:

Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
});

Also adds a simple method for Koha::Patron, set_default_messaging_preferences.
Usage: $patron->set_default_messaging_preferences()

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 41 Josef Moravec 2017-06-06 11:05:56 UTC
Created attachment 64041 [details] [review]
[SIGNED-OFF] Bug 17499: Improve object usability by enabling direct access to transport preferences

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating.

This patch adds a feature for Koha::Patron::Message::Preference which enables
access to message transport preferences directly via this object. It allows us
to skip calls to Koha::Patron::Message::Transport::Preference(s) because we can
now get and set via K::P::M::Preference.

Get:
$preference->message_transport_types
Returns a hashref, where each key is stored transport type and value for the key
is letter code for the transport.

Set:
$preference->set({ message_transport_types => ['sms'] }) or
$preference->message_transport_types('email', 'sms') or
$preference->message_transport_types(['email', 'sms'])
Returns $self (Koha::Patron::Message::Preference object)

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 42 Josef Moravec 2017-06-06 11:06:00 UTC
Created attachment 64042 [details] [review]
[SIGNED-OFF] Bug 17499: Simplify searching by message_name

This patch adds optional search & find parameter message_name for
Koha::Patron::Message::Preferences->search and ->find. This simplifies object
usage by allowing us to skip joins or finding the attribute id ourselves.

prove -v t/db_dependent/Koha/Patron/Message/Preferences.t turns green
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 43 Josef Moravec 2017-06-06 11:06:04 UTC
Created attachment 64043 [details] [review]
[SIGNED-OFF] Bug 17499: (follow-up) Bugfix for validation

- Bugfix for days_in_advance and wants_digest validation. Have to check definedness
  of value instead the value itself.
- Fixes broken tests caused by above update.
- Adding a missing unit test for wants_digest validation
- Minor edits to exception messages.
- Throwing exception for invalid message_attribute_id. Incl. unit test

Tested again, prove -v t/db_dependent/Koha/Patron/Message/Preferences.t turns green
prove -v t/db_dependent/Koha/Patron/Message/* turns green
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 44 Josef Moravec 2017-06-06 11:06:08 UTC
Created attachment 64044 [details] [review]
[SIGNED-OFF] Bug 17499: (follow-up) Bugfix for validation, 2

Remove duplicate validation for digest availability. This is already checked in
$preference->validate().

Set values for current object before validating messaging transport types, not
the other way around as it was before this patch.

Also prevents a crash when patron has messaging transport type selected for
a message for which it should not be available. This patch logs it with as
a warning, and only throws an exception when attempting to set it.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 45 Lari Taskula 2017-07-05 09:42:51 UTC
Created attachment 64795 [details] [review]
Bug 17499: (follow-up) Add information on digest being the only option to get_options

If a library wants to force digest on a message type, add this information to
Koha::Patron::Message::Preferences->get_options.
Comment 46 Marcel de Rooy 2017-09-01 06:03:21 UTC
QA: Looking here now
Comment 47 Marcel de Rooy 2017-09-01 06:09:04 UTC
You add exceptions that are already there.

Koha::Exceptions::BadParameter
Koha::Exceptions::WrongParameter

Koha::Exceptions::ObjectNotFound
Koha::Exceptions::UnknownObject
Comment 48 Marcel de Rooy 2017-09-01 06:41:18 UTC
Created attachment 66686 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences. This patch does not
add any extra logic into these objects.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: Add useful Koha::Exceptions

This patch adds some basic Koha::Exceptions that will be useful.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: Add basic validation for messaging preference

This patch adds simple validation for messaging preferences.

The validation includes
- check that only either borrowernumber or categorycode is given, but not both
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: Throw an exception on duplicate messaging preference

When trying to add a duplicate messaging preference that has the same
borrowernumber or category, and message_attribute_id as another preference,
throw Koha::Exceptions::DuplicateObject.

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: Add a method for getting messaging options

This patch adds a method for getting available messaging options.

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: Add a method for setting default messaging preferences

This patch adds a method Koha::Patron::Message::Preference->new_from_default that
can be used to add category's default messaging preferences to patron for a given
message type.

Example call:

Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
});

Also adds a simple method for Koha::Patron, set_default_messaging_preferences.
Usage: $patron->set_default_messaging_preferences()

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: Improve object usability by enabling direct access to transport preferences

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating.

This patch adds a feature for Koha::Patron::Message::Preference which enables
access to message transport preferences directly via this object. It allows us
to skip calls to Koha::Patron::Message::Transport::Preference(s) because we can
now get and set via K::P::M::Preference.

Get:
$preference->message_transport_types
Returns a hashref, where each key is stored transport type and value for the key
is letter code for the transport.

Set:
$preference->set({ message_transport_types => ['sms'] }) or
$preference->message_transport_types('email', 'sms') or
$preference->message_transport_types(['email', 'sms'])
Returns $self (Koha::Patron::Message::Preference object)

To test:
1. Run t/db_dependent/Koha/Patron/Message/Preferences.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: Simplify searching by message_name

This patch adds optional search & find parameter message_name for
Koha::Patron::Message::Preferences->search and ->find. This simplifies object
usage by allowing us to skip joins or finding the attribute id ourselves.

prove -v t/db_dependent/Koha/Patron/Message/Preferences.t turns green
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: (follow-up) Bugfix for validation

- Bugfix for days_in_advance and wants_digest validation. Have to check definedness
  of value instead the value itself.
- Fixes broken tests caused by above update.
- Adding a missing unit test for wants_digest validation
- Minor edits to exception messages.
- Throwing exception for invalid message_attribute_id. Incl. unit test

Tested again, prove -v t/db_dependent/Koha/Patron/Message/Preferences.t turns green
prove -v t/db_dependent/Koha/Patron/Message/* turns green
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: (follow-up) Bugfix for validation, 2

Remove duplicate validation for digest availability. This is already checked in
$preference->validate().

Set values for current object before validating messaging transport types, not
the other way around as it was before this patch.

Also prevents a crash when patron has messaging transport type selected for
a message for which it should not be available. This patch logs it with as
a warning, and only throws an exception when attempting to set it.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 17499: (follow-up) Add information on digest being the only option to get_options

If a library wants to force digest on a message type, add this information to
Koha::Patron::Message::Preferences->get_options.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Squashed 11 commits during QA.
Comment 49 Marcel de Rooy 2017-09-01 06:42:37 UTC
QA Comment:
Looks good to me overall (see also comment47).
No blockers found.

Koha::Patron::Message::Preferences
sub find_with_message_name
 my $attr = Koha::Patron::Message::Attributes->find({
            message_name => $id->{'message_name'},
        });
Isnt this actually a search? Couldnt we have multiple results on name ?
Similar question for  search_with_message_name later

my $name = $transport->get_column('message_name');
Why get_column, not just message_name ?

+=head3 search
+
+Koha::Patron::Message::Preferences->search_with_message_name({
+    borrowernumber => 123,
+    message_name => 'Hold_Filled',
+});
Wrong POD header
Comment 50 Jonathan Druart 2017-09-01 14:11:54 UTC
(In reply to Marcel de Rooy from comment #49)
> Koha::Patron::Message::Preferences
> sub find_with_message_name
>  my $attr = Koha::Patron::Message::Attributes->find({
>             message_name => $id->{'message_name'},
>         });
> Isnt this actually a search? Couldnt we have multiple results on name ?
> Similar question for  search_with_message_name later

It's a unique key
  UNIQUE KEY `message_name` (`message_name`)


> my $name = $transport->get_column('message_name');
> Why get_column, not just message_name ?

It's a column from another table.
Comment 51 Jonathan Druart 2017-09-01 14:14:03 UTC
(In reply to Marcel de Rooy from comment #47)
> You add exceptions that are already there.
> 
> Koha::Exceptions::BadParameter
> Koha::Exceptions::WrongParameter
> 
> Koha::Exceptions::ObjectNotFound
> Koha::Exceptions::UnknownObject

Lari, please adjust.
Are you ok with the squash?
Comment 52 Lari Taskula 2017-12-01 14:29:59 UTC
Created attachment 69434 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.
Comment 53 Lari Taskula 2017-12-01 14:31:29 UTC
Created attachment 69435 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.
Comment 54 Lari Taskula 2017-12-01 14:35:46 UTC
(In reply to Marcel de Rooy from comment #49)
> QA Comment:
> Looks good to me overall (see also comment47).
Fixed.

> +=head3 search
> +
> +Koha::Patron::Message::Preferences->search_with_message_name({
> +    borrowernumber => 123,
> +    message_name => 'Hold_Filled',
> +});
> Wrong POD header
Fixed.

Also cleaned commit message. I set it to Needs Signoff again as I added a new test changed the functionality of $patron->set_default_messaging_preferences to delete any previous preferences (this is expected behaviour; before this fix it would throw an exception).
Comment 55 Dominic Pichette 2017-12-01 20:27:58 UTC
Created attachment 69458 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 56 Dominic Pichette 2017-12-01 20:28:13 UTC
prove t/db_dependent/Koha/Patron/Message/*
t/db_dependent/Koha/Patron/Message/Attributes.t ............. ok   
t/db_dependent/Koha/Patron/Message/Preferences.t ............ ok   
t/db_dependent/Koha/Patron/Message/Transport/Preferences.t .. ok   
t/db_dependent/Koha/Patron/Message/Transport/Types.t ........ ok   
t/db_dependent/Koha/Patron/Message/Transports.t ............. ok   
All tests successful.
Files=5, Tests=15,  9 wallclock secs ( 0.07 usr  0.01 sys +  5.05 cusr  0.20 csys =  5.33 CPU)
Result: PASS
Comment 57 Katrin Fischer 2018-03-12 14:48:20 UTC
Patch doesn't apply, please rebase!
Comment 58 Emmi Takkinen 2019-07-30 11:35:13 UTC
Created attachment 91864 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

Rebased patch.
Comment 59 Lari Taskula 2019-10-08 09:48:40 UTC
Created attachment 93863 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 60 Katrin Fischer 2019-10-08 11:26:52 UTC
Hi Lari,

thx for the update!

Is this related to https://wiki.koha-community.org/wiki/Patrons_messaging_preferences_endpoint_RFC and ready for voting? We got a meeting tomorrow and could add it to the agenda right away.

There was a bit of discussion on the wiki page - so please check the bottom of the page!
Comment 61 Lari Taskula 2019-10-08 11:35:30 UTC
(In reply to Katrin Fischer from comment #60)
> Hi Lari,
> 
> thx for the update!
> 
> Is this related to
> https://wiki.koha-community.org/wiki/
> Patrons_messaging_preferences_endpoint_RFC and ready for voting? We got a
> meeting tomorrow and could add it to the agenda right away.
> 
> There was a bit of discussion on the wiki page - so please check the bottom
> of the page!

Hi Katrin,

Yes, this is related to the endpoint. Current REST API implementation in Bug 17505 uses the Koha objects I provided here.

The idea is to add validations in messaging preference Koha objects and throw Koha::Exceptions when needed, so that we can easily catch any issues in REST API controllers.

The patch is now rebased on top of latest master branch.
Comment 62 Katrin Fischer 2019-10-08 11:37:15 UTC
Just realized, I got my bugs turned upside down - objects first, then the API. It might still get sense to get the API voted on early, so we can just push things through once ready. 

Hope this can get a sign-off soon!
Comment 63 Lari Taskula 2019-10-08 13:27:04 UTC
Created attachment 93865 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 64 Lari Taskula 2019-10-08 14:28:45 UTC
Created attachment 93872 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 65 Lari Taskula 2019-10-25 12:18:39 UTC
Created attachment 94748 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 66 Tomás Cohen Arazi 2019-11-12 18:20:16 UTC
Can we de-squash this huge patch into meaningful pieces with easy to follow details on the resulting implementation, please? I like the results :-D
Comment 67 David Nind 2020-02-17 10:14:04 UTC
What should the status of this be after Tomás's comment? (comment 66) Needs Signoff or Failed QA?

I tried to apply the patch and got this - no sure whether this means it has applied cleanly or note:

94748 - Bug 17499: Add Koha-objects for messaging preferences

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17499: Add Koha-objects for messaging preferences
Using index info to reconstruct a base tree...
M	Koha/Exceptions.pm
M	Koha/Patron.pm
Falling back to patching base and 3-way merge...
Auto-merging Koha/Patron.pm
Auto-merging Koha/Exceptions.pm

Also, running the test resulted in the tests failing:

prove t/db_dependent/Koha/Patron/Message/*
t/db_dependent/Koha/Patron/Message/Attributes.t ............. 1/2     # Looks like you planned 6 tests but ran 4.

#   Failed test 'Test Koha::Patron::Message::Attributes'
#   at t/db_dependent/Koha/Patron/Message/Attributes.t line 72.
Can't locate object method "_new_from_dbic" via package "Koha::MessageAttribute" (perhaps you forgot to load "Koha::MessageAttribute"?) at /kohadevbox/koha/Koha/Object.pm line 222.
# Looks like your test exited with 255 just after 2.
t/db_dependent/Koha/Patron/Message/Attributes.t ............. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 1/2 subtests 
t/db_dependent/Koha/Patron/Message/Preferences.t ............ 1/7         # Looks like you planned 3 tests but ran 2.

    #   Failed test 'Test for a patron'
    #   at t/db_dependent/Koha/Patron/Message/Preferences.t line 99.
    # Looks like you planned 2 tests but ran 1.
    # Looks like you failed 1 test of 1 run.

#   Failed test 'Test Koha::Patron::Message::Preferences'
#   at t/db_dependent/Koha/Patron/Message/Preferences.t line 125.
Can't locate object method "_new_from_dbic" via package "Koha::BorrowerMessagePreference" (perhaps you forgot to load "Koha::BorrowerMessagePreference"?) at /kohadevbox/koha/Koha/Object.pm line 222.
# Looks like your test exited with 255 just after 2.
t/db_dependent/Koha/Patron/Message/Preferences.t ............ Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 6/7 subtests 
t/db_dependent/Koha/Patron/Message/Transport/Preferences.t .. 1/2     # Looks like you planned 2 tests but ran 1.

#   Failed test 'Test Koha::Patron::Message::Transport::Preferences'
#   at t/db_dependent/Koha/Patron/Message/Transport/Preferences.t line 112.
Can't locate object method "_new_from_dbic" via package "Koha::BorrowerMessageTransportPreference" (perhaps you forgot to load "Koha::BorrowerMessageTransportPreference"?) at /kohadevbox/koha/Koha/Object.pm line 222.
# Looks like your test exited with 255 just after 2.
t/db_dependent/Koha/Patron/Message/Transport/Preferences.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 1/2 subtests 
t/db_dependent/Koha/Patron/Message/Transport/Types.t ........ 1/2     # Looks like you planned 2 tests but ran 1.

#   Failed test 'Test Koha::Patron::Message::Transport::Types'
#   at t/db_dependent/Koha/Patron/Message/Transport/Types.t line 52.
Can't locate object method "_new_from_dbic" via package "Koha::MessageTransportType" (perhaps you forgot to load "Koha::MessageTransportType"?) at /kohadevbox/koha/Koha/Object.pm line 222.
# Looks like your test exited with 255 just after 2.
t/db_dependent/Koha/Patron/Message/Transport/Types.t ........ Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 1/2 subtests 
t/db_dependent/Koha/Patron/Message/Transports.t ............. 1/2     # Looks like you planned 2 tests but ran 1.

#   Failed test 'Test Koha::Patron::Message::Transports'
#   at t/db_dependent/Koha/Patron/Message/Transports.t line 70.
Can't locate object method "_new_from_dbic" via package "Koha::MessageTransport" (perhaps you forgot to load "Koha::MessageTransport"?) at /kohadevbox/koha/Koha/Object.pm line 222.
# Looks like your test exited with 255 just after 2.
t/db_dependent/Koha/Patron/Message/Transports.t ............. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 1/2 subtests 

Test Summary Report
-------------------
t/db_dependent/Koha/Patron/Message/Attributes.t           (Wstat: 65280 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 255
t/db_dependent/Koha/Patron/Message/Preferences.t          (Wstat: 65280 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 7 tests but ran 2.
t/db_dependent/Koha/Patron/Message/Transport/Preferences.t (Wstat: 65280 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 255
t/db_dependent/Koha/Patron/Message/Transport/Types.t      (Wstat: 65280 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 255
t/db_dependent/Koha/Patron/Message/Transports.t           (Wstat: 65280 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 255
Files=5, Tests=10,  8 wallclock secs ( 0.04 usr  0.01 sys +  6.58 cusr  0.64 csys =  7.27 CPU)
Result: FAIL
Comment 68 David Nind 2020-02-17 10:15:25 UTC
Actually, I've answered my own question I think - I've run the tests and some failed, so back to Failed QA.
Comment 69 Emmi Takkinen 2020-02-24 09:24:02 UTC
Created attachment 99479 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Comment 70 Emmi Takkinen 2020-02-24 09:28:39 UTC
Tests should work now.

Keeping status as Failed QA since I'm also unsure what it should be after Tomás's comment (comment 66)
Comment 71 David Nind 2020-02-24 09:37:25 UTC
(In reply to Emmi Takkinen from comment #70)
> Tests should work now.
> 
> Keeping status as Failed QA since I'm also unsure what it should be after
> Tomás's comment (comment 66)

Can confirm, tests all pass now.
Comment 72 Jonathan Druart 2020-02-24 10:06:57 UTC
I had a very quick look at the patch. About the tests I would like to see the subroutines replaced with a call to $builder->build_object when it is the only thing they actually do.

Example:

sub build_a_test_transport_type {
    my $mtt = $builder->build({
        source => 'MessageTransportType' });

    return Koha::Patron::Message::Transport::Types->find(
        $mtt->{message_transport_type}
    );
}

can be replaced with:

  $builder->build_object({ class => 'Koha::Patron::Message::Transport::Types' });
Comment 73 Emmi Takkinen 2020-02-26 10:06:20 UTC
Created attachment 99634 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Comment 74 David Nind 2020-02-26 17:01:14 UTC
Created attachment 99668 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Nind <david@davidnind.com>
Comment 75 David Nind 2020-02-26 17:01:18 UTC
Created attachment 99669 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: David Nind <david@davidnind.com>
Comment 76 David Nind 2020-02-26 17:01:23 UTC
Created attachment 99670 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy

Signed-off-by: David Nind <david@davidnind.com>
Comment 77 Jonathan Druart 2020-04-22 09:25:14 UTC
Created attachment 103441 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: David Nind <david@davidnind.com>
Comment 78 Jonathan Druart 2020-04-22 09:25:18 UTC
Created attachment 103442 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy

Signed-off-by: David Nind <david@davidnind.com>
Comment 79 Jonathan Druart 2020-04-22 09:25:23 UTC
Created attachment 103443 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy

Signed-off-by: David Nind <david@davidnind.com>
Comment 80 Jonathan Druart 2020-04-22 09:31:53 UTC
Lari, I have rebased the patches.

Wondering about find_with_message_name

Koha::Patron::Message::Preferences->find_with_message_name({
    borrowernumber => 123,
    message_name => 'Hold_Filled',
});

Should not we prefer something like:

 $patron->messaging_preferences->filter_by_message_name('Hold_Filled');
or even:
 $patron->messaging_preferences->search({ message_name => 'Hold_Filled' });

ie why is it a class method?

Same for the setter, there is only 1 set_default_messaging_preferences, why cannot we add set messaging preferences directly from $patron?
Comment 81 Lari Taskula 2020-04-22 09:37:07 UTC
(In reply to Jonathan Druart from comment #80)
> Lari, I have rebased the patches.
> 
> Wondering about find_with_message_name
> 
> Koha::Patron::Message::Preferences->find_with_message_name({
>     borrowernumber => 123,
>     message_name => 'Hold_Filled',
> });
> 
> Should not we prefer something like:
> 
>  $patron->messaging_preferences->filter_by_message_name('Hold_Filled');
> or even:
>  $patron->messaging_preferences->search({ message_name => 'Hold_Filled' });
> 
> ie why is it a class method?
> 
> Same for the setter, there is only 1 set_default_messaging_preferences, why
> cannot we add set messaging preferences directly from $patron?

I don't think there is a reason why we shouldn't have getters & setters in Koha::Patron as well. The purpose of that method was just to convert human readable message name into an id number.
Comment 82 Katrin Fischer 2020-06-22 23:11:15 UTC
This needs a bit of work, there are lots of "red" entries when running the QA tools, but they should be easy to fix. Examples:

 FAIL	Koha/Patron/Message/Attributes.pm
   OK	  critic
   FAIL	  forbidden patterns
		forbidden pattern: Incorrect license statement (using postal address), may be a false positive if the file is coming from outside Koha (bug 24545) (line 18)
   OK	  git manipulation
   OK	  pod
   FAIL	  pod coverage
		POD is missing for 'object_class'

All the tests do pass for me.

Lari, could you take a look and fix these? I'll probably need another QA Team member to take a closer look at the code after that.
Comment 83 Lari Taskula 2020-06-24 08:22:11 UTC
(In reply to Katrin Fischer from comment #82)
> This needs a bit of work, there are lots of "red" entries when running the
> QA tools, but they should be easy to fix. Examples:
> 
>  FAIL	Koha/Patron/Message/Attributes.pm
>    OK	  critic
>    FAIL	  forbidden patterns
> 		forbidden pattern: Incorrect license statement (using postal address), may
> be a false positive if the file is coming from outside Koha (bug 24545)
> (line 18)
>    OK	  git manipulation
>    OK	  pod
>    FAIL	  pod coverage
> 		POD is missing for 'object_class'
> 
> All the tests do pass for me.
> 
> Lari, could you take a look and fix these? I'll probably need another QA
> Team member to take a closer look at the code after that.

Hi, thanks for the comment, however I'm currently unable to work with this so if anyone wants to step in and help, it would be greatly appreciated!
Comment 84 Katrin Fischer 2020-07-04 21:17:23 UTC
Moving out of the QA queue for now - hoping someone will rescue this one soon!
Comment 85 Martin Renvoize 2020-07-08 12:56:44 UTC
Created attachment 106653 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 86 Martin Renvoize 2020-07-08 12:56:48 UTC
Created attachment 106654 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 87 Martin Renvoize 2020-07-08 12:56:52 UTC
Created attachment 106655 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 88 Martin Renvoize 2020-07-08 12:56:56 UTC
Created attachment 106656 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 89 Martin Renvoize 2020-07-08 12:57:00 UTC
Created attachment 106657 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 90 Martin Renvoize 2020-07-08 13:00:13 UTC
Rescued.. I've sorted the licence issues and added some POD.. Remaining QA script failure are false negatives.

I've not QA'd as a whole as I'm not close enough to the area of code to really have an opinion whether it's good or not.. back to NQA
Comment 91 Katrin Fischer 2020-07-30 22:31:05 UTC
Created attachment 107612 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 92 Katrin Fischer 2020-07-30 22:31:10 UTC
Created attachment 107613 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 93 Katrin Fischer 2020-07-30 22:31:15 UTC
Created attachment 107614 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 94 Katrin Fischer 2020-07-30 22:31:21 UTC
Created attachment 107615 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 95 Katrin Fischer 2020-07-30 22:31:26 UTC
Created attachment 107616 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 96 Katrin Fischer 2020-07-30 22:34:23 UTC
Reported in 2016... feeling a little 'reckless' tonight and going to pass this. There have been quite some eyes on it already. QA script and plenty of included tests pass. I hope this will trigger work to resume on bug 18595 that makes use of these new objects so we can see them in action.
Comment 97 Jonathan Druart 2020-08-03 10:23:38 UTC
I have not found anything obvious that would reject this patch set. However we need to push it alongside with bug 18595.
Comment 98 Lari Taskula 2020-11-04 15:55:51 UTC Comment hidden (obsolete)
Comment 99 Lari Taskula 2020-11-04 16:06:06 UTC
Created attachment 113011 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 100 Lari Taskula 2020-11-04 16:07:50 UTC
As per https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18595#c114 , I have attached a patch that provides more explicit Koha::Exceptions and reset bug status from "Passed QA" back to "Needs Signoff" for that patch.
Comment 101 Lari Taskula 2020-11-05 17:34:35 UTC
Created attachment 113127 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 102 Lari Taskula 2020-11-05 18:31:22 UTC
Created attachment 113129 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland
Comment 103 Lari Taskula 2020-11-17 00:03:45 UTC
Created attachment 113686 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 104 Lari Taskula 2020-11-17 00:03:50 UTC
Created attachment 113687 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland
Comment 105 Lari Taskula 2020-11-18 01:17:01 UTC
Created attachment 113765 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 106 Lari Taskula 2020-11-18 01:17:08 UTC
Created attachment 113766 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland
Comment 107 Lari Taskula 2020-11-18 03:22:20 UTC
There are some issues.

1. Considering class naming - These patches add "Koha::Patron::Message::Preference::*", but to avoid confusion between "message_queue" and "messages" table, would Koha::Notice::Preference better match the purpose?

As far as I understand, message_queue and messaging preferences are related, but "messages" is something different as it is only delivered in the web browser in staff client and/or OPAC.

As of now:
- message_queue       = Koha::Notice::Messages
- messages            = Koha::Patron::Message
- message preferences = Koha::Patron::Message::Preference

Right now this class naming makes it seem as if these preferences have something to do with "messages" when in fact it is only related to "message_queue".

I believe the work on this Bug overlapped Bug 17959 so we ended up with this confusion.

Still, notices vs messages is pretty confusing. It seems "Notice" is being used for the messages that end up in message_queue. Since notices are related to messaging preferences, should we call it notice preferences instead? I've switched this Bug into "In Discussion" in order to solve this problem first.

2. There is some logic in C4/Reserves.pm, at the end of _koha_notify_reserve() that should be centralized to avoid code duplication. In C4::Reserves::_koha_notify_reserve() we have:
> next if (
>         ( $mtt eq 'email' and not $to_address ) # No email address
>      or ( $mtt eq 'sms'   and not $patron->smsalertnumber ) # No SMS number
>      or ( $mtt eq 'itiva' and C4::Context->preference('TalkingTechItivaPhoneNotification') ) # Notice is handled by TalkingTech_itiva_outbound.pl
>      or ( $mtt eq 'phone' and not $patron->phone ) # No phone number to call
> );

What this Bug currently adds is that messaging preferences can not be saved at all if the above check fails. The reason behind this was from our production experience - there were people constantly asking about undelivered notices even when the mtt was selected. Bug 18595 together with this tackle the issue with proper UI error messages.

I propose we centralize this logic into the preference class under a new method "mtt_deliverable($mtt[, $borrowernumber])".

3. Current patches do not check if "phone" and "itiva" can be used as message transport types. Those mtts are ignored here and in Bug 18595 altogether. I'll work on it.
Comment 108 Lari Taskula 2020-11-18 04:24:48 UTC
Created attachment 113767 [details] [review]
Bug 17499: (follow-up) Validate phone and itiva transport types

Message transport types 'phone' and 'itiva' were not validated unlike
'email' and 'sms'. This patch adds equivalent checks to the missing mtts.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 109 Lari Taskula 2020-11-18 04:24:52 UTC
Created attachment 113768 [details] [review]
Bug 17499: (follow-up) Contact information vs. mtt validation

This patch adds a subroutine that handles message transport type validation by
first checking related patron contact information.

As an example, if there is no email address, we shouldn't let email be selected
as a messaging transport type.

The reason to isolate logic into a separate subroutine is that C4/Reserves.pm
_koha_notify_reserve() also uses the same logic and we should not duplicate it.
C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in
Bug 18595.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 110 Joonas Kylmälä 2020-12-01 09:22:03 UTC
(In reply to Lari Taskula from comment #107)
> 1. Considering class naming - These patches add
> "Koha::Patron::Message::Preference::*", but to avoid confusion between
> "message_queue" and "messages" table, would Koha::Notice::Preference better
> match the purpose?
> 
> As far as I understand, message_queue and messaging preferences are related,
> but "messages" is something different as it is only delivered in the web
> browser in staff client and/or OPAC.
> 
> As of now:
> - message_queue       = Koha::Notice::Messages
> - messages            = Koha::Patron::Message
> - message preferences = Koha::Patron::Message::Preference
> 
> Right now this class naming makes it seem as if these preferences have
> something to do with "messages" when in fact it is only related to
> "message_queue".
> 
> I believe the work on this Bug overlapped Bug 17959 so we ended up with this
> confusion.
> 
> Still, notices vs messages is pretty confusing. It seems "Notice" is being
> used for the messages that end up in message_queue. Since notices are
> related to messaging preferences, should we call it notice preferences
> instead? I've switched this Bug into "In Discussion" in order to solve this
> problem first.

We use messaging preferences in the GUI so I would not switch to notice preference.

I see these message preferences as their own object tied to a patron (not to a patron's individual message). This would mean the name would be Koha::Patron::MessagePreference and Koha::Patron::MessagePreferences. Then this message preference is combination of a Attribute and Transports. 

Important thing to note, reading the code C4::Members::Messaging::GetMessagingPreferences it appears that one MessagePreference would correspond to one message_attribute. As far as I can tell the table message_attributes is incorrectly named / created, the columns should be in my opinion in the borrower_message_preferences table. So the final structure could be 

Koha::Patron::MessagePreference             (contains both borrower_message_preferences and message_attributes with JOIN)
Koha::Patron::MessagePreferences            (contains MessagePreference)
Koha::Patron::MessagePreference::Transport  (message_transports)
Koha::Patron::MessagePreference::Transports (contains all Transport of a MessagePreference)

The message_transports seems to contains also unrelated columns to transport types, namely the is_digest information which seems to be preference of the patron instead of something that is decided based on the transportation medium. But maybe you can fit this somehow nicely anyway? If it seems impossible we could restructure first the messaging preference related tables.

Also see comment #80 from Jonathan, I think it the $patron->messaging_preferences suggestion is a bit like what I have here but instead with an object. Using object allows now modifying all the message preference fields easily so I would go with the object.
Comment 111 Lari Taskula 2020-12-31 03:14:13 UTC
(In reply to Joonas Kylmälä from comment #110)
> I see these message preferences as their own object tied to a patron (not to
> a patron's individual message). This would mean the name would be Koha::Patron::MessagePreference and Koha::Patron::MessagePreferences.

Good point. That makes sense. Let's go with that.

> As far as I can
> tell the table message_attributes is incorrectly named / created, the
> columns should be in my opinion in the borrower_message_preferences table.
> ...
> Koha::Patron::MessagePreference             (contains both
> borrower_message_preferences and message_attributes with JOIN)

This is basically what the current patches do, but without a join. The Message::Preference object has an accessor called "message_name" even though it's not physically part of borrower_message_preferences table.

JOIN sounds interesting. But doesn't DBIx require you to use "table.column" key format when referencing to a column of a joined table? That's not very user friendly. Would it work with DBIx's "+as" option? Then we could get rid of the "find_with_message_name" method pointed out in comment #80 by Jonathan. I wonder if it's possible to define the JOIN in DBIx schema file so that we don't have to override search/find/whatever methods manually?

While there could be improvement at database level, imo that's outside of this Bug's scope. At database level, I don't think they should be merged either. We need message_attribute for database normalization.

borrower_message_preferences could also be improved by splitting it in two; borrower_message_preferences and category_message_preferences (the default category preferences). Anyways I think it's outside of the scope here.

> The message_transports seems to contains also unrelated columns to transport
> types, namely the is_digest information which seems to be preference of the
> patron instead of something that is decided based on the transportation
> medium. But maybe you can fit this somehow nicely anyway? If it seems
> impossible we could restructure first the messaging preference related
> tables.

"message_options" would better describe its current data. It should also have a primary key integer. Then borrower_message_preferences columns "message_attribute_id" and "wants_digest" could be replaced with a foreign key to "message_option_id". So, borrower_message_preference would simply be a reference to a borrowernumber and message_option_id (and additionally having the days_in_advance setting).

But your suggestion works as well. It's just a matter of selecting where the digest information should be at.

> Also see comment #80 from Jonathan, I think it the
> $patron->messaging_preferences suggestion is a bit like what I have here but
> instead with an object.

We can implement the Koha::Patron accessors from comment #80, I've nothing against that. But $patron->messaging_preferences would be an object (of MessagingPreferences class), I'm not sure what you mean.
Comment 112 Lari Taskula 2021-03-18 03:47:50 UTC
Created attachment 118452 [details] [review]
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference

Koha::Patron::Message refers to "messages" table that is used for circulation
messages. MessagePreference now refers to a messaging preference that is used
for enqueued notifications.

To test:
1. prove t/db_dependent/Koha/Patron/MessagePreference/Attributes.t
2. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Preferences.t
3. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Types.t
4. prove t/db_dependent/Koha/Patron/MessagePreference/Transports.t
5. prove t/db_dependent/Koha/Patron/MessagePreferences.t

Sponsored-by: The National Library of Finland
Comment 113 Lari Taskula 2021-03-18 03:47:56 UTC
Created attachment 118453 [details] [review]
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor

To test:
1. prove t/db_dependent/Koha/Patron.t

Sponsored-by: The National Library of Finland
Comment 114 Michal Denar 2021-04-09 19:15:22 UTC
Hi Lari,
can you update this patch on top of master?

Thank you.
Comment 115 Lari Taskula 2021-04-12 19:36:44 UTC
Created attachment 119470 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 116 Lari Taskula 2021-04-12 19:36:53 UTC
Created attachment 119471 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 117 Lari Taskula 2021-04-12 19:37:00 UTC
Created attachment 119472 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 118 Lari Taskula 2021-04-12 19:37:06 UTC
Created attachment 119473 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 119 Lari Taskula 2021-04-12 19:37:12 UTC
Created attachment 119474 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 120 Lari Taskula 2021-04-12 19:37:18 UTC
Created attachment 119475 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 121 Lari Taskula 2021-04-12 19:37:23 UTC
Created attachment 119476 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland
Comment 122 Lari Taskula 2021-04-12 19:37:29 UTC
Created attachment 119477 [details] [review]
Bug 17499: (follow-up) Validate phone and itiva transport types

Message transport types 'phone' and 'itiva' were not validated unlike
'email' and 'sms'. This patch adds equivalent checks to the missing mtts.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 123 Lari Taskula 2021-04-12 19:37:35 UTC
Created attachment 119478 [details] [review]
Bug 17499: (follow-up) Contact information vs. mtt validation

This patch adds a subroutine that handles message transport type validation by
first checking related patron contact information.

As an example, if there is no email address, we shouldn't let email be selected
as a messaging transport type.

The reason to isolate logic into a separate subroutine is that C4/Reserves.pm
_koha_notify_reserve() also uses the same logic and we should not duplicate it.
C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in
Bug 18595.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland
Comment 124 Lari Taskula 2021-04-12 19:37:40 UTC
Created attachment 119479 [details] [review]
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference

Koha::Patron::Message refers to "messages" table that is used for circulation
messages. MessagePreference now refers to a messaging preference that is used
for enqueued notifications.

To test:
1. prove t/db_dependent/Koha/Patron/MessagePreference/Attributes.t
2. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Preferences.t
3. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Types.t
4. prove t/db_dependent/Koha/Patron/MessagePreference/Transports.t
5. prove t/db_dependent/Koha/Patron/MessagePreferences.t

Sponsored-by: The National Library of Finland
Comment 125 Lari Taskula 2021-04-12 19:37:46 UTC
Created attachment 119480 [details] [review]
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor

To test:
1. prove t/db_dependent/Koha/Patron.t

Sponsored-by: The National Library of Finland
Comment 126 Michal Denar 2021-04-12 19:50:56 UTC
Created attachment 119481 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 127 Michal Denar 2021-04-12 19:51:03 UTC
Created attachment 119482 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 128 Michal Denar 2021-04-12 19:51:09 UTC
Created attachment 119483 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 129 Michal Denar 2021-04-12 19:51:15 UTC
Created attachment 119484 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 130 Michal Denar 2021-04-12 19:51:21 UTC
Created attachment 119485 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 131 Michal Denar 2021-04-12 19:51:28 UTC
Created attachment 119486 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 132 Michal Denar 2021-04-12 19:51:34 UTC
Created attachment 119487 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 133 Michal Denar 2021-04-12 19:51:40 UTC
Created attachment 119488 [details] [review]
Bug 17499: (follow-up) Validate phone and itiva transport types

Message transport types 'phone' and 'itiva' were not validated unlike
'email' and 'sms'. This patch adds equivalent checks to the missing mtts.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 134 Michal Denar 2021-04-12 19:51:46 UTC
Created attachment 119489 [details] [review]
Bug 17499: (follow-up) Contact information vs. mtt validation

This patch adds a subroutine that handles message transport type validation by
first checking related patron contact information.

As an example, if there is no email address, we shouldn't let email be selected
as a messaging transport type.

The reason to isolate logic into a separate subroutine is that C4/Reserves.pm
_koha_notify_reserve() also uses the same logic and we should not duplicate it.
C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in
Bug 18595.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 135 Michal Denar 2021-04-12 19:51:53 UTC
Created attachment 119490 [details] [review]
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference

Koha::Patron::Message refers to "messages" table that is used for circulation
messages. MessagePreference now refers to a messaging preference that is used
for enqueued notifications.

To test:
1. prove t/db_dependent/Koha/Patron/MessagePreference/Attributes.t
2. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Preferences.t
3. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Types.t
4. prove t/db_dependent/Koha/Patron/MessagePreference/Transports.t
5. prove t/db_dependent/Koha/Patron/MessagePreferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 136 Michal Denar 2021-04-12 19:52:00 UTC
Created attachment 119491 [details] [review]
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor

To test:
1. prove t/db_dependent/Koha/Patron.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 137 Marcel de Rooy 2021-07-09 07:55:43 UTC
Applying: Bug 17499: (follow-up) Add $patron->messaging_preferences accessor
error: sha1 information is lacking or useless (Koha/Patron.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 17499: (follow-up) Add $patron->messaging_preferences accessor
Comment 138 Lari Taskula 2022-10-06 12:02:14 UTC
Created attachment 141408 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 139 Lari Taskula 2022-10-06 12:02:21 UTC
Created attachment 141409 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 140 Lari Taskula 2022-10-06 12:02:29 UTC
Created attachment 141410 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 141 Lari Taskula 2022-10-06 12:02:40 UTC
Created attachment 141411 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 142 Lari Taskula 2022-10-06 12:02:46 UTC
Created attachment 141412 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 143 Lari Taskula 2022-10-06 12:02:53 UTC
Created attachment 141413 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 144 Lari Taskula 2022-10-06 12:02:59 UTC
Created attachment 141414 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 145 Lari Taskula 2022-10-06 12:03:07 UTC
Created attachment 141415 [details] [review]
Bug 17499: (follow-up) Validate phone and itiva transport types

Message transport types 'phone' and 'itiva' were not validated unlike
'email' and 'sms'. This patch adds equivalent checks to the missing mtts.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 146 Lari Taskula 2022-10-06 12:03:13 UTC
Created attachment 141416 [details] [review]
Bug 17499: (follow-up) Contact information vs. mtt validation

This patch adds a subroutine that handles message transport type validation by
first checking related patron contact information.

As an example, if there is no email address, we shouldn't let email be selected
as a messaging transport type.

The reason to isolate logic into a separate subroutine is that C4/Reserves.pm
_koha_notify_reserve() also uses the same logic and we should not duplicate it.
C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in
Bug 18595.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 147 Lari Taskula 2022-10-06 12:03:20 UTC
Created attachment 141417 [details] [review]
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference

Koha::Patron::Message refers to "messages" table that is used for circulation
messages. MessagePreference now refers to a messaging preference that is used
for enqueued notifications.

To test:
1. prove t/db_dependent/Koha/Patron/MessagePreference/Attributes.t
2. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Preferences.t
3. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Types.t
4. prove t/db_dependent/Koha/Patron/MessagePreference/Transports.t
5. prove t/db_dependent/Koha/Patron/MessagePreferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 148 Lari Taskula 2022-10-06 12:03:28 UTC
Created attachment 141418 [details] [review]
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor

To test:
1. prove t/db_dependent/Koha/Patron.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 149 Lari Taskula 2022-10-06 12:58:33 UTC
It seems Koha::MessageAttribute(s) classes were introduced before this Bug was able to move forward. As current patch set adds equivalent classes named Koha::Patron::Message::Attribute(s), they have to be removed. Also, Koha::Patron::Message::Transport(s) and Koha::Patron::Message::Transport::Type(s) should be renamed to match naming convention of Koha::MessageAttribute(s).

I'll start fixing these.
Comment 150 Martin Renvoize 2022-10-06 14:53:52 UTC
I actually prefer your original approach if I'm honest.. it doesn't look to me like MessageAttribute has been much yet.. perhaps we could go the other way and keep the work here and update core to match?

tl;dr I like Koha::Patron::Message::Attribute(s) better than Koha::MessageAttribute(s).

I'll ask Tomas to weight in.
Comment 151 Emmi Takkinen 2022-11-24 11:30:57 UTC
(In reply to Martin Renvoize from comment #150)
> I actually prefer your original approach if I'm honest.. it doesn't look to
> me like MessageAttribute has been much yet.. perhaps we could go the other
> way and keep the work here and update core to match?
> 
> tl;dr I like Koha::Patron::Message::Attribute(s) better than
> Koha::MessageAttribute(s).
> 
> I'll ask Tomas to weight in.

Has there been any decision on this?
Comment 152 Julian Maurice 2022-12-22 08:55:35 UTC
Any update ?

Koha::MessageAttribute(s) is not used anywhere (except in a test file: t/db_dependent/Illrequests.t). It should be easy to replace it with Koha::Patron::Message::Attribute(s).
Comment 153 Lari Taskula 2023-02-11 14:02:59 UTC
Created attachment 146529 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 154 Lari Taskula 2023-02-11 14:03:03 UTC
Created attachment 146530 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 155 Lari Taskula 2023-02-11 14:03:07 UTC
Created attachment 146531 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 156 Lari Taskula 2023-02-11 14:03:12 UTC
Created attachment 146532 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 157 Lari Taskula 2023-02-11 14:03:16 UTC
Created attachment 146533 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 158 Lari Taskula 2023-02-11 14:03:19 UTC
Created attachment 146534 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 159 Lari Taskula 2023-02-11 14:03:23 UTC
Created attachment 146535 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 160 Lari Taskula 2023-02-11 14:03:27 UTC
Created attachment 146536 [details] [review]
Bug 17499: (follow-up) Validate phone and itiva transport types

Message transport types 'phone' and 'itiva' were not validated unlike
'email' and 'sms'. This patch adds equivalent checks to the missing mtts.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 161 Lari Taskula 2023-02-11 14:03:31 UTC
Created attachment 146537 [details] [review]
Bug 17499: (follow-up) Contact information vs. mtt validation

This patch adds a subroutine that handles message transport type validation by
first checking related patron contact information.

As an example, if there is no email address, we shouldn't let email be selected
as a messaging transport type.

The reason to isolate logic into a separate subroutine is that C4/Reserves.pm
_koha_notify_reserve() also uses the same logic and we should not duplicate it.
C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in
Bug 18595.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 162 Lari Taskula 2023-02-11 14:03:35 UTC
Created attachment 146538 [details] [review]
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference

Koha::Patron::Message refers to "messages" table that is used for circulation
messages. MessagePreference now refers to a messaging preference that is used
for enqueued notifications.

To test:
1. prove t/db_dependent/Koha/Patron/MessagePreference/Attributes.t
2. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Preferences.t
3. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Types.t
4. prove t/db_dependent/Koha/Patron/MessagePreference/Transports.t
5. prove t/db_dependent/Koha/Patron/MessagePreferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 163 Lari Taskula 2023-02-11 14:03:39 UTC
Created attachment 146539 [details] [review]
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor

To test:
1. prove t/db_dependent/Koha/Patron.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 164 Lari Taskula 2023-02-11 14:03:43 UTC
Created attachment 146540 [details] [review]
Bug 17499: Replace Koha::MessageAttribute(s) with Koha::Patron::MessagePreference::Attribute(s)

To test:
1. prove t/db_dependent/Illrequests.t
2. grep -rn 'Koha::MessageAttribute', observe no results in Koha code
Comment 165 Lari Taskula 2023-02-11 14:05:19 UTC
(In reply to Julian Maurice from comment #152)
> Any update ?
> 
> Koha::MessageAttribute(s) is not used anywhere (except in a test file:
> t/db_dependent/Illrequests.t). It should be easy to replace it with
> Koha::Patron::Message::Attribute(s).

Attached a patch that makes the replacement
Comment 166 David Nind 2023-04-05 20:44:23 UTC
Patch no longer applies:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17499: Add Koha-objects for messaging preferences
Applying: Bug 17499: Define koha_object(s)_class where needed
Applying: Bug 17499: Replace test subroutines with build_object
Applying: Bug 17499: (QA follow-up) Fix Licence Statements
Applying: Bug 17499: (QA follow-up) Fix POD errors
Applying: Bug 17499: (follow-up) More explicit exceptions
Applying: Bug 17499: (follow-up) Fix warning
Applying: Bug 17499: (follow-up) Validate phone and itiva transport types
Applying: Bug 17499: (follow-up) Contact information vs. mtt validation
Applying: Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference
Applying: Bug 17499: (follow-up) Add $patron->messaging_preferences accessor
error: sha1 information is lacking or useless (Koha/Patron.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 17499: (follow-up) Add $patron->messaging_preferences accessor
Comment 167 Julian Maurice 2023-06-01 10:04:33 UTC
Created attachment 151900 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 168 Julian Maurice 2023-06-01 10:04:38 UTC
Created attachment 151901 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 169 Julian Maurice 2023-06-01 10:04:42 UTC
Created attachment 151902 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 170 Julian Maurice 2023-06-01 10:04:47 UTC
Created attachment 151903 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 171 Julian Maurice 2023-06-01 10:04:53 UTC
Created attachment 151904 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 172 Julian Maurice 2023-06-01 10:04:57 UTC
Created attachment 151905 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 173 Julian Maurice 2023-06-01 10:05:02 UTC
Created attachment 151906 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 174 Julian Maurice 2023-06-01 10:05:06 UTC
Created attachment 151907 [details] [review]
Bug 17499: (follow-up) Validate phone and itiva transport types

Message transport types 'phone' and 'itiva' were not validated unlike
'email' and 'sms'. This patch adds equivalent checks to the missing mtts.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 175 Julian Maurice 2023-06-01 10:05:11 UTC
Created attachment 151908 [details] [review]
Bug 17499: (follow-up) Contact information vs. mtt validation

This patch adds a subroutine that handles message transport type validation by
first checking related patron contact information.

As an example, if there is no email address, we shouldn't let email be selected
as a messaging transport type.

The reason to isolate logic into a separate subroutine is that C4/Reserves.pm
_koha_notify_reserve() also uses the same logic and we should not duplicate it.
C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in
Bug 18595.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 176 Julian Maurice 2023-06-01 10:05:16 UTC
Created attachment 151909 [details] [review]
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference

Koha::Patron::Message refers to "messages" table that is used for circulation
messages. MessagePreference now refers to a messaging preference that is used
for enqueued notifications.

To test:
1. prove t/db_dependent/Koha/Patron/MessagePreference/Attributes.t
2. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Preferences.t
3. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Types.t
4. prove t/db_dependent/Koha/Patron/MessagePreference/Transports.t
5. prove t/db_dependent/Koha/Patron/MessagePreferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 177 Julian Maurice 2023-06-01 10:05:20 UTC
Created attachment 151910 [details] [review]
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor

To test:
1. prove t/db_dependent/Koha/Patron.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 178 Julian Maurice 2023-06-01 10:05:24 UTC
Created attachment 151911 [details] [review]
Bug 17499: Replace Koha::MessageAttribute(s) with Koha::Patron::MessagePreference::Attribute(s)

To test:
1. prove t/db_dependent/Illrequests.t
2. grep -rn 'Koha::MessageAttribute', observe no results in Koha code
Comment 179 Julian Maurice 2023-06-01 10:05:58 UTC
Patches rebased on master
Comment 180 Sam Lau 2023-06-01 16:03:45 UTC
Created attachment 151937 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 181 Sam Lau 2023-06-01 16:03:48 UTC
Created attachment 151938 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 182 Sam Lau 2023-06-01 16:03:52 UTC
Created attachment 151939 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 183 Sam Lau 2023-06-01 16:03:56 UTC
Created attachment 151940 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 184 Sam Lau 2023-06-01 16:04:00 UTC
Created attachment 151941 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 185 Sam Lau 2023-06-01 16:04:03 UTC
Created attachment 151942 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 186 Sam Lau 2023-06-01 16:04:07 UTC
Created attachment 151943 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 187 Sam Lau 2023-06-01 16:04:11 UTC
Created attachment 151944 [details] [review]
Bug 17499: (follow-up) Validate phone and itiva transport types

Message transport types 'phone' and 'itiva' were not validated unlike
'email' and 'sms'. This patch adds equivalent checks to the missing mtts.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 188 Sam Lau 2023-06-01 16:04:14 UTC
Created attachment 151945 [details] [review]
Bug 17499: (follow-up) Contact information vs. mtt validation

This patch adds a subroutine that handles message transport type validation by
first checking related patron contact information.

As an example, if there is no email address, we shouldn't let email be selected
as a messaging transport type.

The reason to isolate logic into a separate subroutine is that C4/Reserves.pm
_koha_notify_reserve() also uses the same logic and we should not duplicate it.
C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in
Bug 18595.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 189 Sam Lau 2023-06-01 16:04:18 UTC
Created attachment 151946 [details] [review]
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference

Koha::Patron::Message refers to "messages" table that is used for circulation
messages. MessagePreference now refers to a messaging preference that is used
for enqueued notifications.

To test:
1. prove t/db_dependent/Koha/Patron/MessagePreference/Attributes.t
2. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Preferences.t
3. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Types.t
4. prove t/db_dependent/Koha/Patron/MessagePreference/Transports.t
5. prove t/db_dependent/Koha/Patron/MessagePreferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 190 Sam Lau 2023-06-01 16:04:22 UTC
Created attachment 151947 [details] [review]
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor

To test:
1. prove t/db_dependent/Koha/Patron.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 191 Sam Lau 2023-06-01 16:04:25 UTC
Created attachment 151948 [details] [review]
Bug 17499: Replace Koha::MessageAttribute(s) with Koha::Patron::MessagePreference::Attribute(s)

To test:
1. prove t/db_dependent/Illrequests.t
2. grep -rn 'Koha::MessageAttribute', observe no results in Koha code

Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 192 Marcel de Rooy 2023-09-08 07:52:29 UTC
QA: Looking here
Comment 193 Marcel de Rooy 2023-09-08 08:02:01 UTC
Created attachment 155365 [details] [review]
Bug 17499: Add Koha-objects for messaging preferences

This patch adds Koha-objects for messaging preferences.

Adds simple validation for messaging preferences.

The validation includes
- throw exception if both borrowernumber or categorycode is given for a new pref
- throw exception if patron for the given borrowernumber is not found
- throw exception if category for the given categorycode is not found
- throw exception if days in advance cannot be configured but is given
- throw exception if days in advance configuration is invalid (value between 0-30)
- throw exception if digest is not available but attempted to set on
- throw exception if digest must be enabled but attempted to set off
- throw exception on duplicate messaging preference

Adds a method for getting available messaging options.

Adds a method for setting default messaging preferenes.
  $patron->set_default_messaging_preferences (where $patron is a Koha::Patron)
  ...or...
  Koha::Patron::Message::Preference->new_from_default({
    borrowernumber => 123,
    categorycode => "ABC",
    message_attribute_id => 1,
  });

Since messaging preference is a feature that has multiple related database tables,
usage via Koha-objects is sometimes frustrating. This patch adds easy access to
message transport types via
  $preference->message_transport_types                              (for getting)
  $preference->set({ message_transport_types => ['email', 'sms'] }) (for setting)
  (also supports other calling conventions, see documentation for more)

Adds optional parameter message_name for Koha::Patron::Message::Preferences->find
and ->search. Simplifies the Koha-object usage by allowing developer to skip joins
and / or querying the message name via attribute_id from message_attributes table.

Includes test coverage for basic usage.

To test:
1. prove t/db_dependent/Koha/Patron/Message/*

Following Bug 17499, check also Bug 18595 that replaces C4::Members::Messaging
with these new Koha-objects.

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 194 Marcel de Rooy 2023-09-08 08:02:05 UTC
Created attachment 155366 [details] [review]
Bug 17499: Define koha_object(s)_class where needed

This patch adds koha_object(s)_class for needed modules
to fix issues with tests.

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 195 Marcel de Rooy 2023-09-08 08:02:09 UTC
Created attachment 155367 [details] [review]
Bug 17499: Replace test subroutines with build_object

This patch replaces subroutines used to build test
objects with call to $builder->build_object.

To test prove that tests are still running with:
prove t/db_dependent/Koha/Patron/Message/*

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 196 Marcel de Rooy 2023-09-08 08:02:13 UTC
Created attachment 155368 [details] [review]
Bug 17499: (QA follow-up) Fix Licence Statements

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 197 Marcel de Rooy 2023-09-08 08:02:17 UTC
Created attachment 155369 [details] [review]
Bug 17499: (QA follow-up) Fix POD errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 198 Marcel de Rooy 2023-09-08 08:02:21 UTC
Created attachment 155370 [details] [review]
Bug 17499: (follow-up) More explicit exceptions

Adds and throws more detailed exceptions.

This is useful for APIs and generating translatable errors in GUI.

In short, replaces Koha::Exceptions::BadParameter with:

Koha::Exceptions::Patron::NotFound
Koha::Exceptions::Patron::Category
Koha::Exceptions::Patron::Category::NotFound
Koha::Exceptions::Patron::Message::Preference
Koha::Exceptions::Patron::Message::Preference::AttributeNotFound
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceOutOfRange
Koha::Exceptions::Patron::Message::Preference::DaysInAdvanceNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestNotAvailable
Koha::Exceptions::Patron::Message::Preference::DigestRequired
Koha::Exceptions::Patron::Message::Preference::EmailAddressRequired
Koha::Exceptions::Patron::Message::Preference::NoTransportType
Koha::Exceptions::Patron::Message::Preference::SMSNumberRequired

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 199 Marcel de Rooy 2023-09-08 08:02:25 UTC
Created attachment 155371 [details] [review]
Bug 17499: (follow-up) Fix warning

Fix warning

DBIx::Class::Storage::DBI::select_single(): Query returned more than one row.
SQL that returns multiple rows is DEPRECATED for ->find and ->single.

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 200 Marcel de Rooy 2023-09-08 08:02:29 UTC
Created attachment 155372 [details] [review]
Bug 17499: (follow-up) Validate phone and itiva transport types

Message transport types 'phone' and 'itiva' were not validated unlike
'email' and 'sms'. This patch adds equivalent checks to the missing mtts.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 201 Marcel de Rooy 2023-09-08 08:02:33 UTC
Created attachment 155373 [details] [review]
Bug 17499: (follow-up) Contact information vs. mtt validation

This patch adds a subroutine that handles message transport type validation by
first checking related patron contact information.

As an example, if there is no email address, we shouldn't let email be selected
as a messaging transport type.

The reason to isolate logic into a separate subroutine is that C4/Reserves.pm
_koha_notify_reserve() also uses the same logic and we should not duplicate it.
C4::Reserves::_koha_notify_reserve() will be adjusted to use this new sub in
Bug 18595.

To test:
1. prove t/db_dependent/Koha/Patron/Message/Preferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 202 Marcel de Rooy 2023-09-08 08:02:37 UTC
Created attachment 155374 [details] [review]
Bug 17499: (follow-up) Rename to Koha::Patron::MessagePreference

Koha::Patron::Message refers to "messages" table that is used for circulation
messages. MessagePreference now refers to a messaging preference that is used
for enqueued notifications.

To test:
1. prove t/db_dependent/Koha/Patron/MessagePreference/Attributes.t
2. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Preferences.t
3. prove t/db_dependent/Koha/Patron/MessagePreference/Transport/Types.t
4. prove t/db_dependent/Koha/Patron/MessagePreference/Transports.t
5. prove t/db_dependent/Koha/Patron/MessagePreferences.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 203 Marcel de Rooy 2023-09-08 08:02:41 UTC
Created attachment 155375 [details] [review]
Bug 17499: (follow-up) Add $patron->messaging_preferences accessor

To test:
1. prove t/db_dependent/Koha/Patron.t

Sponsored-by: The National Library of Finland

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 204 Marcel de Rooy 2023-09-08 08:02:45 UTC
Created attachment 155376 [details] [review]
Bug 17499: Replace Koha::MessageAttribute(s) with Koha::Patron::MessagePreference::Attribute(s)

To test:
1. prove t/db_dependent/Illrequests.t
2. grep -rn 'Koha::MessageAttribute', observe no results in Koha code

Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 205 Marcel de Rooy 2023-09-08 08:06:40 UTC
Overall looks very good to me. Great job. And with so much patience :)

A few minor remarks (for the record, no blockers)
+=head3 new_from_default
+
+NOTE: This subroutine initializes and STORES the object (in order to set
+message transport types for the preference), so no need to call ->store when
+preferences are initialized via this method.

=> This might be a bit confusing. Do not store immediately? No blocker (waited too long, etc)

+=head3 find_with_message_name
+
+Koha::Patron::Message::Preferences->find_with_message_name({
+    borrowernumber => 123,
+    message_name => 'Hold_Filled',
+});

This should be a search instead of a find imo. No blocker.
Note that there is a search_with.. too ?

+=head2 Internal methods
Recently someone failed my patches for something like this. I wont do that here :)

QA tools mentions a few untidy lines. Please ignore them now in view of time passed.

The big question is however: Who sets the next step and applies this stuff in favor of C4 Messaging routines?
Comment 206 Marcel de Rooy 2023-09-08 08:07:47 UTC
(In reply to Marcel de Rooy from comment #205)
> The big question is however: Who sets the next step and applies this stuff
> in favor of C4 Messaging routines?

Note that 18595 does not apply.
Comment 207 Tomás Cohen Arazi 2023-09-12 13:16:35 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 208 Fridolin Somers 2023-09-14 18:28:44 UTC
Enhancement not pushed to 23.05.x