From 12c7a513fca155d3576d5f961748d463f333e974 Mon Sep 17 00:00:00 2001 From: Dan Wells Date: Fri, 19 May 2017 15:08:55 -0400 Subject: [PATCH] LP#1496522 Restructure balance notification action trigger This commit builds on the work that Blake had done, with two main goals. First, it pushes some of the logic down the stack to reduce churn. Second, it renames and relabels a number of things to better match precedent. For the first part, one particular concern with the existing code is that it unavoidably generates 'invalid' events. With the balance_owed filter applied, this problem was reduced but not eliminated. The solution in this commit may be overkill, but it might also serve as a model for similar cases. In brief, this code extends the new view to do the necessary setting lookup and subsequent filtering in the DB layer, thereby avoiding any dead ends, and in fact eliminating the need for event validation at all. For the second, the following strings were changed, mostly to match existing precedent, but occasionally just for clarity: Trigger name: - was: patron_has_bills - now: money.usr_exceeds_balance_threshold View name: - was: money.usr_summary_per_org_unit (still exists for future reuse) - now: money.usr_exceeds_balance_threshold (builds on original generic view) Setting name: - was: patron.notify_bills_when_exceeds - now: circ.notify_when_balance_exceeds Setting label: - was: Notify Patron bill when exceeds - now: Notify Patron When Balance Exceeds Event definition name: - was: Patron recurring 1 month billing notice - now: Monthly Patron Balance Notice Additional notes: - I did not understand the supplied tests as written, as they appeared to merely create everything, then see if those things were created. I left one test which checks if the base view exists and at least functions. We probably really want a live test for this if possible. - The template in the seed file was all indented within the multi-line quote, which seemed likely to cause problems. This indent is removed. - The new view requires (I believe) PG 9.3+. It references a previous column in a later join, similar to a lateral join. Version 9.3 is the baseline since 2.11, so this should be fine for all supported EG versions. Signed-off-by: Dan Wells --- .../examples/action_trigger_filters.json.example | 7 +- Open-ILS/examples/fm_IDL.xml | 5 +- .../lib/OpenILS/Application/Trigger/Validator.pm | 18 ----- Open-ILS/src/sql/Pg/500.view.cross-schema.sql | 10 ++- Open-ILS/src/sql/Pg/950.data.seed-values.sql | 43 ++++++----- ...ction_trigger_for_periodic_billing_statement.pg | 88 +--------------------- ...tion_trigger_for_periodic_billing_statement.sql | 38 +++++----- ...tion_trigger_for_periodic_billing_statement.txt | 15 ++-- 8 files changed, 59 insertions(+), 165 deletions(-) diff --git a/Open-ILS/examples/action_trigger_filters.json.example b/Open-ILS/examples/action_trigger_filters.json.example index dd7bb7f9ed..262ad5f462 100644 --- a/Open-ILS/examples/action_trigger_filters.json.example +++ b/Open-ILS/examples/action_trigger_filters.json.example @@ -41,10 +41,7 @@ "deleted":"f" } }, - "patron_has_bills" : { - "context_org" : "billing_ou", - "filter": { - "balance_owed": {">": "0"} - } + "money.usr_exceeds_balance_threshold" : { + "context_org" : "billing_ou" } } diff --git a/Open-ILS/examples/fm_IDL.xml b/Open-ILS/examples/fm_IDL.xml index 6cc2535195..1e465f8203 100644 --- a/Open-ILS/examples/fm_IDL.xml +++ b/Open-ILS/examples/fm_IDL.xml @@ -6585,7 +6585,7 @@ SELECT usr, - + @@ -6593,10 +6593,13 @@ SELECT usr, + + + diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Trigger/Validator.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Trigger/Validator.pm index 2d63a38192..db909a51b1 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Trigger/Validator.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Trigger/Validator.pm @@ -187,23 +187,5 @@ sub PatronNotInCollections { return @$existing ? 0 : 1; } -# core type "muspou" -sub PatronExceedsBills { - my ($self, $env) = @_; - my $user = $env->{target}->usr; - my $org = $env->{target}->billing_ou; - my $balance_owed = $env->{target}->balance_owed; - - # beware environment fleshing - $user = $user->id if ref $user; - $org = $org->id if ref $org; - - my $e = new_editor(); - my $threshold = $U->ou_ancestor_setting_value($org, 'patron.notify_bills_when_exceeds', $e ); - $threshold = 0 unless $threshold; - - return $balance_owed > $threshold; -} - 1; diff --git a/Open-ILS/src/sql/Pg/500.view.cross-schema.sql b/Open-ILS/src/sql/Pg/500.view.cross-schema.sql index 19ea7d7a81..a85dadad6b 100644 --- a/Open-ILS/src/sql/Pg/500.view.cross-schema.sql +++ b/Open-ILS/src/sql/Pg/500.view.cross-schema.sql @@ -49,6 +49,7 @@ CREATE OR REPLACE VIEW money.open_usr_circulation_summary AS WHERE xact_type = 'circulation' AND xact_finish IS NULL GROUP BY usr; +-- Create the root view CREATE OR REPLACE VIEW money.usr_summary_per_org_unit AS WITH located_xact AS ( SELECT id, circ_lib AS billing_ou FROM action.circulation @@ -65,9 +66,12 @@ CREATE OR REPLACE VIEW money.usr_summary_per_org_unit AS JOIN located_xact ON located_xact.id = mmbts.id GROUP BY mmbts.usr, billing_ou; -ALTER TABLE -money.usr_summary_per_org_unit - OWNER TO evergreen; +-- Create the filtered view +CREATE OR REPLACE VIEW money.usr_exceeds_balance_threshold AS + SELECT m.*, s.id AS setting_id, s.org_unit AS setting_ou, s.value AS setting_value + FROM money.usr_summary_per_org_unit m + LEFT JOIN actor.org_unit_ancestor_setting('circ.notify_when_balance_exceeds', billing_ou) AS s ON TRUE + WHERE balance_owed > COALESCE(s.value::float, 0); -- Not a view, but it's cross-schema.. diff --git a/Open-ILS/src/sql/Pg/950.data.seed-values.sql b/Open-ILS/src/sql/Pg/950.data.seed-values.sql index e465072236..f1bcd54e41 100644 --- a/Open-ILS/src/sql/Pg/950.data.seed-values.sql +++ b/Open-ILS/src/sql/Pg/950.data.seed-values.sql @@ -3361,6 +3361,15 @@ INSERT into config.org_unit_setting_type 'coust', 'description'), 'link', 'ccs') +,( 'circ.notify_when_balance_exceeds', 'finance', + oils_i18n_gettext('circ.notify_when_balance_exceeds', + 'Notify Patron When Balance Exceeds', + 'coust', 'label'), + oils_i18n_gettext('circ.notify_when_balance_exceeds', + 'Notify the patron when their balance owed exceeds this setting. Default is 0.', + 'coust', 'description'), + 'currency', null) + ,( 'circ.obscure_dob', 'sec', oils_i18n_gettext('circ.obscure_dob', 'Obscure the Date of Birth field', @@ -4094,15 +4103,6 @@ INSERT into config.org_unit_setting_type 'coust', 'description'), 'bool', null) -,( 'patron.notify_bills_when_exceeds', 'finance', - oils_i18n_gettext('patron.notify_bills_when_exceeds', - 'Notify Patron bill when exceeds', - 'coust', 'label'), - oils_i18n_gettext('patron.notify_bills_when_exceeds', - 'Notify the patron when their balance owed exceeds this setting. Default is 0. Action trigger validator "PatronExceedsBills" required', - 'coust', 'description'), - 'currency', null) - ,( 'print.custom_js_file', 'circ', oils_i18n_gettext('print.custom_js_file', 'Printing: Custom Javascript File', @@ -11462,25 +11462,24 @@ INSERT INTO action_trigger.environment ( -- Periodic email to patrons with balances over a certain amount INSERT INTO action_trigger.hook (key,core_type,description,passive) -VALUES('patron_has_bills','muspou','Patron has bills. Usually used in conjunction with reactor SendEmail and library setting "Notify Patron bill when exceeds".','t'); - -INSERT INTO action_trigger.validator (module,description) VALUES('PatronExceedsBills','Event is valid if the library setting "Notify Patron bill when exceeds" setting threshold is exceeded'); +VALUES('money.usr_exceeds_balance_threshold','muebt','Used in conjunction with reactor SendEmail and library setting "Notify Patron When Balance Exceeds".','t'); +-- Create the action trigger event definition INSERT INTO action_trigger.event_definition (active, owner, name, hook, validator, reactor, delay_field, repeat_delay, template) - VALUES ('f', '1', 'Patron recurring 1 month billing notice', 'patron_has_bills', 'PatronExceedsBills', 'SendEmail', 'last_payment_ts', '1 month', + VALUES ('f', '1', 'Monthly Patron Balance Notice', 'money.usr_exceeds_balance_threshold', 'NOOP_True', 'SendEmail', 'last_payment_ts', '1 month', $$ - [%- USE date -%] - [%- user = target.usr -%] - To: [%- params.recipient_email || user.email %] - From: [%- user.home_ou.name %] <[% helpers.get_org_setting(user.home_ou, 'org.bounced_emails') || lib.email || params.sender_email || default_sender %]> - Subject: Library bills +[%- USE date -%] +[%- user = target.usr -%] +To: [%- params.recipient_email || user.email %] +From: [%- user.home_ou.name %] <[% helpers.get_org_setting(user.home_ou, 'org.bounced_emails') || lib.email || params.sender_email || default_sender %]> +Subject: Library bills - [%# You can use the library setting "Notify Patron bill when exceeds" to squelch this notification when the balance owed doesnt exceed your configuration %] - Dear [% user.first_given_name %] [% user.family_name %], +[%# You can use the library setting "Notify Patron When Balance Exceeds" to squelch this notification when the balance owed doesnt exceed your configuration %] +Dear [% user.first_given_name %] [% user.family_name %], - You have an outstanding balance at the library: +You have an outstanding balance at the library: - [%- target.balance_owed -%] +[%- target.balance_owed -%] $$ ); diff --git a/Open-ILS/src/sql/Pg/t/lp1496522_action_trigger_for_periodic_billing_statement.pg b/Open-ILS/src/sql/Pg/t/lp1496522_action_trigger_for_periodic_billing_statement.pg index e14859c033..905d1dfc8d 100644 --- a/Open-ILS/src/sql/Pg/t/lp1496522_action_trigger_for_periodic_billing_statement.pg +++ b/Open-ILS/src/sql/Pg/t/lp1496522_action_trigger_for_periodic_billing_statement.pg @@ -1,99 +1,13 @@ BEGIN; -SELECT plan(4); +SELECT plan(1); --- ******************************************* --- Create hook -INSERT INTO action_trigger.hook (key,core_type,description,passive) -SELECT 'patron_has_bills','muspou','Patron has bills. Usually used in conjunction with reactor SendEmail and library setting "Notify Patron bill when exceeds".','t' -WHERE NOT EXISTS ( - SELECT key - FROM action_trigger.hook - WHERE key = 'patron_has_bills' - ); - --- Create validator -INSERT INTO action_trigger.validator (module,description) -SELECT 'PatronExceedsBills','Event is valid if the library setting "Notify Patron bill when exceeds" setting threshold is exceeded' -WHERE NOT EXISTS ( - SELECT module - FROM action_trigger.validator - WHERE module = 'PatronExceedsBills' - ); - --- Create library setting -INSERT into config.org_unit_setting_type (name, grp, datatype, label, description) -SELECT - 'patron.notify_bills_when_exceeds', - 'finance', 'currency', - oils_i18n_gettext( - 'patron.notify_bills_when_exceeds', - 'Notify Patron bill when exceeds', - 'coust', - 'label' - ), - oils_i18n_gettext( - 'patron.notify_bills_when_exceeds', - 'Notify the patron when their balance owed exceeds this setting. Default is 0. Action trigger validator "PatronExceedsBills" required', - 'coust', - 'description' - ) -WHERE NOT EXISTS ( - SELECT name - FROM config.org_unit_setting_type - WHERE name = 'patron.notify_bills_when_exceeds' - ); - - - --- Create view -CREATE OR REPLACE VIEW money.usr_summary_per_org_unit AS - SELECT materialized_billable_xact_summary.usr, - money.grocery.billing_location AS billing_ou, - sum(materialized_billable_xact_summary.total_paid) AS total_paid, - sum(materialized_billable_xact_summary.total_owed) AS total_owed, - sum(materialized_billable_xact_summary.balance_owed) AS balance_owed, - -- This needs to be non-null for patron bill notification - COALESCE(MAX(materialized_billable_xact_summary.last_payment_ts),'0001-01-01'::TIMESTAMP) AS last_payment_ts - FROM money.materialized_billable_xact_summary, money.grocery - WHERE money.grocery.id = money.materialized_billable_xact_summary.id - GROUP BY materialized_billable_xact_summary.usr, money.grocery.billing_location; - -ALTER TABLE money.usr_summary_per_org_unit - OWNER TO evergreen; - - - - --- ******************************************* - - --- Test it --- ******************************************* - - --- Test to make sure that the hook exists - - -SELECT isnt_empty( - 'SELECT * FROM action_trigger.hook WHERE key = $$patron_has_bills$$', - 'action_trigger.hook patron_has_bills exists' ); - -SELECT isnt_empty( - 'SELECT * FROM action_trigger.validator WHERE module = $$PatronExceedsBills$$', - 'action_trigger.validator PatronExceedsBills exists' ); - -SELECT isnt_empty( - 'SELECT * FROM config.org_unit_setting_type WHERE name = $$patron.notify_bills_when_exceeds$$', - 'Library setting patron.notify_bills_when_exceeds exists' ); - SELECT isnt_empty( 'SELECT total_owed,balance_owed,total_paid,billing_ou,usr,last_payment_ts FROM money.usr_summary_per_org_unit limit 100', 'View money.usr_summary_per_org_unit exists' ); - -- Finish the tests and clean up. SELECT * FROM finish(); ROLLBACK; diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.action_trigger_for_periodic_billing_statement.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.action_trigger_for_periodic_billing_statement.sql index 9340bd3fb3..d89577b009 100644 --- a/Open-ILS/src/sql/Pg/upgrade/XXXX.action_trigger_for_periodic_billing_statement.sql +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.action_trigger_for_periodic_billing_statement.sql @@ -2,31 +2,27 @@ BEGIN; -- Create a new hook INSERT INTO action_trigger.hook (key,core_type,description,passive) -VALUES('patron_has_bills','muspou','Patron has bills. Usually used in conjunction with reactor SendEmail and library setting "Notify Patron bill when exceeds".','t'); - --- Create a new validator -INSERT INTO action_trigger.validator (module,description) VALUES('PatronExceedsBills','Event is valid if the library setting "Notify Patron bill when exceeds" setting threshold is exceeded'); +VALUES('money.usr_exceeds_balance_threshold','muebt','Used in conjunction with reactor SendEmail and library setting "Notify Patron When Balance Exceeds".','t'); -- Create the library setting INSERT into config.org_unit_setting_type (name, grp, datatype, label, description) VALUES ( - 'patron.notify_bills_when_exceeds', + 'circ.notify_when_balance_exceeds', 'finance', 'currency', oils_i18n_gettext( - 'patron.notify_bills_when_exceeds', - 'Notify Patron bill when exceeds', + 'circ.notify_when_balance_exceeds', + 'Notify Patron When Balance Exceeds', 'coust', 'label' ), oils_i18n_gettext( - 'patron.notify_bills_when_exceeds', - 'Notify the patron when their balance owed exceeds this setting. Default is 0. Action trigger -validator "PatronExceedsBills" required', + 'circ.notify_when_balance_exceeds', + 'Notify the patron when their balance owed exceeds this setting. Default is 0.', 'coust', 'description' ) ); --- Create the view +-- Create the root view CREATE OR REPLACE VIEW money.usr_summary_per_org_unit AS WITH located_xact AS ( SELECT id, circ_lib AS billing_ou FROM action.circulation @@ -43,29 +39,31 @@ CREATE OR REPLACE VIEW money.usr_summary_per_org_unit AS JOIN located_xact ON located_xact.id = mmbts.id GROUP BY mmbts.usr, billing_ou; -ALTER TABLE -money.usr_summary_per_org_unit - OWNER TO evergreen; - +-- Create the filtered view +CREATE OR REPLACE VIEW money.usr_exceeds_balance_threshold AS + SELECT m.*, s.id AS setting_id, s.org_unit AS setting_ou, s.value AS setting_value + FROM money.usr_summary_per_org_unit m + LEFT JOIN actor.org_unit_ancestor_setting('circ.notify_when_balance_exceeds', billing_ou) AS s ON TRUE + WHERE balance_owed > COALESCE(s.value::float, 0); -- Create the action trigger event definition INSERT INTO action_trigger.event_definition (active, owner, name, hook, validator, reactor, delay_field, repeat_delay, template) - VALUES ('f', '1', 'Patron recurring 1 month billing notice', 'patron_has_bills', 'PatronExceedsBills', 'SendEmail', 'last_payment_ts', '1 month', - $$ + VALUES ('f', '1', 'Monthly Patron Balance Notice', 'money.usr_exceeds_balance_threshold', 'NOOP_True', 'SendEmail', 'last_payment_ts', '1 month', +$$ [%- USE date -%] [%- user = target.usr -%] To: [%- params.recipient_email || user.email %] From: [%- user.home_ou.name %] <[% helpers.get_org_setting(user.home_ou, 'org.bounced_emails') || lib.email || params.sender_email || default_sender %]> Subject: Library bills -[%# You can use the library setting "Notify Patron bill when exceeds" to squelch this notification when the balance owed doesnt exceed your configuration %] +[%# You can use the library setting "Notify Patron When Balance Exceeds" to squelch this notification when the balance owed doesnt exceed your configuration %] Dear [% user.first_given_name %] [% user.family_name %], You have an outstanding balance at the library: [%- target.balance_owed -%] - - $$ + +$$ ); -- Add the environment var for usr fleshing diff --git a/docs/RELEASE_NOTES_NEXT/LP1496522_action_trigger_for_periodic_billing_statement.txt b/docs/RELEASE_NOTES_NEXT/LP1496522_action_trigger_for_periodic_billing_statement.txt index d3559753f6..3ab842497d 100644 --- a/docs/RELEASE_NOTES_NEXT/LP1496522_action_trigger_for_periodic_billing_statement.txt +++ b/docs/RELEASE_NOTES_NEXT/LP1496522_action_trigger_for_periodic_billing_statement.txt @@ -1,16 +1,13 @@ -Patron Periodic Billing Statement Action Trigger +Patron Periodic Balance Statement Action Trigger ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -You can setup an action trigger that will notify patrons about their oustanding bills. -A new library setting "patron.notify_bills_when_exceeds" can be setup to only notify -patrons who exceed a certain amount. By default, patrons will be notified if they exceed -0. +You can setup an action trigger that will notify patrons about their oustanding +balance. A new library setting can be setup to only notify patrons who exceed a +certain amount. By default, patrons will be notified if they exceed $0.00. * New Library Setting - ** Notify Patron bill when exceeds + ** Notify Patron When Balance Exceeds (circ.notify_when_balance_exceeds) * New Action Trigger Hook - ** patron_has_bills - * New Action Trigger Validator - ** PatronExceedsBills + ** money.usr_exceeds_balance_threshold -- 2.11.0