Bug 17499 - Koha objects for messaging preferences
Summary: Koha objects for messaging preferences
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lari Taskula
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 17505 18595
  Show dependency treegraph
 
Reported: 2016-10-26 11:53 UTC by Lari Taskula
Modified: 2017-12-01 20:28 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17499: Add Koha-objects for messaging preferences (20.89 KB, patch)
2017-05-12 13:25 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add useful Koha::Exceptions (1.52 KB, patch)
2017-05-12 13:25 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add basic validation for messaging preference (5.48 KB, patch)
2017-05-12 13:26 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Throw an exception on duplicate messaging preference (4.03 KB, patch)
2017-05-12 13:26 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add a method for getting messaging options (4.50 KB, patch)
2017-05-12 13:26 UTC, Lari Taskula
Details | 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 | 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 | Splinter Review
Bug 17499: Simplify searching by message_name (4.62 KB, patch)
2017-05-12 13:27 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (35.08 KB, patch)
2017-05-29 09:56 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add useful Koha::Exceptions (1.52 KB, patch)
2017-05-29 09:56 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add basic validation for messaging preference (8.36 KB, patch)
2017-05-29 09:56 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Throw an exception on duplicate messaging preference (5.80 KB, patch)
2017-05-29 09:56 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add a method for getting messaging options (4.63 KB, patch)
2017-05-29 09:57 UTC, Lari Taskula
Details | 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 | 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 | Splinter Review
Bug 17499: Simplify searching by message_name (5.63 KB, patch)
2017-05-29 09:58 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (35.21 KB, patch)
2017-05-29 12:26 UTC, Marc Véron
Details | Splinter Review
Bug 17499: Add useful Koha::Exceptions (1.65 KB, patch)
2017-05-29 12:27 UTC, Marc Véron
Details | Splinter Review
Bug 17499: Add basic validation for messaging preference (8.49 KB, patch)
2017-05-29 12:28 UTC, Marc Véron
Details | 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 | 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 | 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 | 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 | Splinter Review
Bug 17499: Simplify searching by message_name (5.83 KB, patch)
2017-05-29 12:34 UTC, Marc Véron
Details | Splinter Review
Bug 17499: (follow-up) Bugfix for validation (7.96 KB, patch)
2017-05-29 12:34 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: (follow-up) Bugfix for validation (8.12 KB, patch)
2017-05-29 12:39 UTC, Marc Véron
Details | Splinter Review
Bug 17499: (follow-up) Bugfix for validation (8.90 KB, patch)
2017-05-29 12:40 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: (follow-up) Bugfix for validation (9.17 KB, patch)
2017-05-29 13:03 UTC, Marc Véron
Details | Splinter Review
Bug 17499: (follow-up) Bugfix for validation, 2 (6.51 KB, patch)
2017-05-31 14:57 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: (follow-up) Bugfix for validation, 2 (11.21 KB, patch)
2017-06-01 11:56 UTC, Lari Taskula
Details | 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 | Splinter Review
[SIGNED-OFF] Bug 17499: Add useful Koha::Exceptions (1.72 KB, patch)
2017-06-06 11:05 UTC, Josef Moravec
Details | 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 | 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 | 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 | 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 | 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 | Splinter Review
[SIGNED-OFF] Bug 17499: Simplify searching by message_name (5.89 KB, patch)
2017-06-06 11:06 UTC, Josef Moravec
Details | Splinter Review
[SIGNED-OFF] Bug 17499: (follow-up) Bugfix for validation (9.24 KB, patch)
2017-06-06 11:06 UTC, Josef Moravec
Details | 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 | 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 | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (84.39 KB, patch)
2017-09-01 06:41 UTC, Marcel de Rooy
Details | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (80.72 KB, patch)
2017-12-01 14:29 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (80.73 KB, patch)
2017-12-01 14:31 UTC, Lari Taskula
Details | Splinter Review
Bug 17499: Add Koha-objects for messaging preferences (80.79 KB, patch)
2017-12-01 20:27 UTC, Dominic Pichette
Details | 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