Merge pull request #152 from openmsupply/138-recipients-in-config-field

Parse recipient_ids & lists into their own fields

backend/graphql/notification_config/src/mutations/mod.rs

@@ -29,7 +29,8 @@ pub fn map_error(error: ModifyNotificationConfigError) -> Result<ModifyNotificat
         }
         ModifyNotificationConfigError::ModifiedRecordNotFound => InternalError(formatted_error),
         ModifyNotificationConfigError::DatabaseError(_) => InternalError(formatted_error),
-        ModifyNotificationConfigError::GenericError(s) => InternalError(s),
+        ModifyNotificationConfigError::InternalError(s) => InternalError(s),
+        ModifyNotificationConfigError::BadUserInput(s) => BadUserInput(s),
     };
 
     Err(graphql_error.extend())

backend/graphql/notification_config/src/mutations/update.rs

@@ -15,6 +15,9 @@ pub struct UpdateNotificationConfigInput {
     pub configuration_data: Option<String>,
     pub parameters: Option<String>,
     pub status: Option<ConfigStatus>,
+    pub recipient_ids: Option<Vec<String>>,
+    pub recipient_list_ids: Option<Vec<String>>,
+    pub sql_recipient_list_ids: Option<Vec<String>>,
 }
 
 pub fn update_notification_config(
@@ -49,6 +52,9 @@ impl From<UpdateNotificationConfigInput> for UpdateNotificationConfig {
             configuration_data,
             status,
             parameters,
+            recipient_ids,
+            recipient_list_ids,
+            sql_recipient_list_ids,
         }: UpdateNotificationConfigInput,
     ) -> Self {
         UpdateNotificationConfig {
@@ -57,6 +63,9 @@ impl From<UpdateNotificationConfigInput> for UpdateNotificationConfig {
             configuration_data,
             status: status.map(ConfigStatus::to_domain),
             parameters,
+            recipient_ids,
+            recipient_list_ids,
+            sql_recipient_list_ids,
         }
     }
 }

backend/graphql/types/src/types/notification_config.rs

@@ -1,9 +1,9 @@
 use super::{dataloader::DataLoader, LogNode};
 use async_graphql::{Context, Enum, Object, SimpleObject, Union};
 use graphql_core::{loader::AuditLogLoader, simple_generic_errors::NodeError, ContextExt};
-use repository::{NotificationConfig, NotificationConfigKind, NotificationConfigStatus};
+use repository::{NotificationConfigKind, NotificationConfigStatus};
 use serde::Serialize;
-use service::ListResult;
+use service::{notification_config::query::NotificationConfig, ListResult};
 use util::usize_to_u32;
 
 #[derive(Union)]
@@ -44,6 +44,18 @@ impl NotificationConfigNode {
         &self.row().parameters
     }
 
+    pub async fn recipient_ids(&self) -> &[String] {
+        &self.row().recipient_ids
+    }
+
+    pub async fn recipient_list_ids(&self) -> &[String] {
+        &self.row().recipient_list_ids
+    }
+
+    pub async fn sql_recipient_list_ids(&self) -> &[String] {
+        &self.row().sql_recipient_list_ids
+    }
+
     pub async fn audit_logs(
         &self,
         ctx: &Context<'_>,

backend/repository/migrations/2023-09-25-031335_recipient-notification-config/down.sql

@@ -0,0 +1 @@
+-- This file should undo anything in `up.sql`

backend/repository/migrations/2023-09-25-031335_recipient-notification-config/up.sql

@@ -0,0 +1,4 @@
+-- Your SQL goes here
+ALTER TABLE notification_config ADD COLUMN recipient_ids TEXT NOT NULL default '[]';
+ALTER TABLE notification_config ADD COLUMN recipient_list_ids TEXT NOT NULL default '[]';
+ALTER TABLE notification_config ADD COLUMN sql_recipient_list_ids TEXT NOT NULL default '[]';

backend/repository/src/db_diesel/notification_config.rs

@@ -7,13 +7,12 @@ use super::{
 use crate::{
     diesel_macros::{apply_equal_filter, apply_sort_no_case, apply_string_filter},
     repository_error::RepositoryError,
-    EqualFilter, NotificationConfigKind, NotificationConfigRow, Pagination, Sort, StringFilter, NotificationConfigStatus, 
+    EqualFilter, NotificationConfigKind, NotificationConfigRow, NotificationConfigStatus,
+    Pagination, Sort, StringFilter,
 };
 
 use diesel::{dsl::IntoBoxed, prelude::*};
 
-pub type NotificationConfig = NotificationConfigRow;
-
 #[derive(Clone, Default, Debug, PartialEq)]
 pub struct NotificationConfigFilter {
     pub id: Option<EqualFilter<String>>,
@@ -48,14 +47,14 @@ impl<'a> NotificationConfigRepository<'a> {
     pub fn query_by_filter(
         &self,
         filter: NotificationConfigFilter,
-    ) -> Result<Vec<NotificationConfig>, RepositoryError> {
+    ) -> Result<Vec<NotificationConfigRow>, RepositoryError> {
         self.query(Pagination::new(), Some(filter), None)
     }
 
     pub fn query_one(
         &self,
         filter: NotificationConfigFilter,
-    ) -> Result<Option<NotificationConfig>, RepositoryError> {
+    ) -> Result<Option<NotificationConfigRow>, RepositoryError> {
         Ok(self.query_by_filter(filter)?.pop())
     }
 
@@ -64,7 +63,7 @@ impl<'a> NotificationConfigRepository<'a> {
         pagination: Pagination,
         filter: Option<NotificationConfigFilter>,
         sort: Option<NotificationConfigSort>,
-    ) -> Result<Vec<NotificationConfig>, RepositoryError> {
+    ) -> Result<Vec<NotificationConfigRow>, RepositoryError> {
         let mut query = create_filtered_query(filter);
 
         if let Some(sort) = sort {
@@ -81,7 +80,7 @@ impl<'a> NotificationConfigRepository<'a> {
             .offset(pagination.offset as i64)
             .limit(pagination.limit as i64);
 
-        let result = final_query.load::<NotificationConfig>(&self.connection.connection)?;
+        let result = final_query.load::<NotificationConfigRow>(&self.connection.connection)?;
         Ok(result)
     }
 }

backend/repository/src/db_diesel/notification_config_row.rs

@@ -14,6 +14,9 @@ table! {
         configuration_data -> Text,
         status -> crate::db_diesel::notification_config_row::NotificationConfigStatusMapping,
         parameters -> Text,
+        recipient_ids -> Text,
+        recipient_list_ids -> Text,
+        sql_recipient_list_ids -> Text,
         last_run_datetime -> Nullable<Timestamp>,
         next_due_datetime -> Nullable<Timestamp>,
     }
@@ -82,7 +85,10 @@ pub struct NotificationConfigRow {
     // it would appear the diesel JSON types are only available if the postgres feature is enabled...
     pub configuration_data: String,
     pub status: NotificationConfigStatus,
-    pub parameters: String,
+    pub parameters: String,             // JSON object {key: "value"}
+    pub recipient_ids: String,          // JSON array of strings (ids)
+    pub recipient_list_ids: String,     // JSON array of strings (ids)
+    pub sql_recipient_list_ids: String, // JSON array of strings (ids)
     pub last_run_datetime: Option<NaiveDateTime>,
     pub next_due_datetime: Option<NaiveDateTime>,
 }
@@ -145,4 +151,21 @@ impl<'a> NotificationConfigRowRepository<'a> {
             .load::<NotificationConfigRow>(&self.connection.connection)?;
         Ok(result)
     }
+
+    pub fn set_last_run_by_id(
+        &self,
+        id: &str,
+        last_run: NaiveDateTime,
+        next_due: Option<NaiveDateTime>,
+    ) -> Result<(), RepositoryError> {
+        let query = diesel::update(notification_config_dsl::notification_config)
+            .filter(notification_config_dsl::id.eq(id))
+            .set((
+                notification_config_dsl::next_due_datetime.eq(next_due),
+                notification_config_dsl::last_run_datetime.eq(last_run),
+            ));
+
+        query.execute(&self.connection.connection)?;
+        Ok(())
+    }
 }

backend/scheduled/src/process.rs

@@ -1,10 +1,8 @@
 use chrono::{DateTime, NaiveDateTime, Utc};
-use repository::{
-    NotificationConfigKind, NotificationConfigRow, NotificationConfigRowRepository,
-    NotificationType,
-};
+use repository::{NotificationConfigKind, NotificationConfigRowRepository, NotificationType};
 use service::{
     notification::enqueue::{create_notification_events, NotificationContext, NotificationTarget},
+    notification_config::query::NotificationConfig,
     service_provider::ServiceContext,
 };
 
@@ -65,7 +63,7 @@ enum ProcessingResult {
 
 fn try_process_scheduled_notifications(
     ctx: &ServiceContext,
-    scheduled_notification: NotificationConfigRow,
+    scheduled_notification: NotificationConfig,
     now: NaiveDateTime,
 ) -> Result<ProcessingResult, NotificationError> {
     // Load the notification config
@@ -80,11 +78,11 @@ fn try_process_scheduled_notifications(
     // Update the last_checked time and next_check time
     // We do this before checking if the notification is due so that if the notification is skipped, we still set a good next check time
     NotificationConfigRowRepository::new(&ctx.connection)
-        .update_one(&NotificationConfigRow {
-            last_run_datetime: Some(now),
-            next_due_datetime: Some(next_due_datetime.naive_utc()),
-            ..scheduled_notification.clone()
-        })
+        .set_last_run_by_id(
+            &scheduled_notification.id,
+            now,
+            Some(next_due_datetime.naive_utc()),
+        )
         .map_err(|e| NotificationError::InternalError(format!("{:?}", e)))?;
 
     // Should notification run ?

backend/service/src/notification_config/create.rs

@@ -1,13 +1,14 @@
 use chrono::Utc;
 use repository::{
-    LogType, NotificationConfig, NotificationConfigKind, NotificationConfigRow,
-    NotificationConfigRowRepository, StorageConnection, NotificationConfigStatus, 
+    LogType, NotificationConfigKind, NotificationConfigRow, NotificationConfigRowRepository,
+    NotificationConfigStatus, StorageConnection,
 };
 
 use crate::{audit_log::audit_log_entry, service_provider::ServiceContext};
 
 use super::{
-    query::get_notification_config, validate::check_notification_config_does_not_exist,
+    query::{get_notification_config, NotificationConfig},
+    validate::check_notification_config_does_not_exist,
     ModifyNotificationConfigError,
 };
 
@@ -65,6 +66,9 @@ pub fn generate(
         configuration_data: "{}".to_string(),
         status: NotificationConfigStatus::Disabled,
         parameters: "{}".to_string(),
+        recipient_ids: "[]".to_string(),
+        recipient_list_ids: "[]".to_string(),
+        sql_recipient_list_ids: "[]".to_string(),
         last_run_datetime: None,
         next_due_datetime: None,
     })

backend/service/src/notification_config/mod.rs

@@ -1,7 +1,7 @@
 use chrono::NaiveDateTime;
 use repository::{
-    NotificationConfig, NotificationConfigFilter, NotificationConfigKind, NotificationConfigSort,
-    PaginationOption, RepositoryError,
+    NotificationConfigFilter, NotificationConfigKind, NotificationConfigSort, PaginationOption,
+    RepositoryError,
 };
 
 use crate::{service_provider::ServiceContext, ListError, ListResult, SingleRecordError};
@@ -10,8 +10,7 @@ use self::{
     create::{create_notification_config, CreateNotificationConfig},
     delete::{delete_notification_config, DeleteNotificationConfigError},
     query::{
-        get_notification_config, get_notification_configs,
-        get_notification_configs_by_kind_and_next_check_date,
+        find_all_due_by_kind, get_notification_config, get_notification_configs, NotificationConfig,
     },
     update::{update_notification_config, UpdateNotificationConfig},
 };
@@ -31,7 +30,7 @@ pub trait NotificationConfigServiceTrait: Sync + Send {
         kind: NotificationConfigKind,
         datetime: NaiveDateTime,
     ) -> Result<Vec<NotificationConfig>, ListError> {
-        get_notification_configs_by_kind_and_next_check_date(ctx, kind, datetime)
+        find_all_due_by_kind(ctx, kind, datetime)
     }
 
     fn get_notification_configs(
@@ -86,7 +85,8 @@ pub enum ModifyNotificationConfigError {
     ModifiedRecordNotFound,
     DatabaseError(RepositoryError),
     NotificationConfigDoesNotExist,
-    GenericError(String),
+    InternalError(String),
+    BadUserInput(String),
 }
 
 // PartialEq is only needed for tests

backend/service/src/notification_config/query.rs

@@ -1,7 +1,8 @@
 use chrono::NaiveDateTime;
 use repository::{
     EqualFilter, NotificationConfigFilter, NotificationConfigKind, NotificationConfigRepository,
-    NotificationConfigRowRepository, NotificationConfigSort, PaginationOption,
+    NotificationConfigRow, NotificationConfigRowRepository, NotificationConfigSort,
+    NotificationConfigStatus, PaginationOption,
 };
 use util::i64_to_u32;
 
@@ -10,11 +11,57 @@ use crate::{
     SingleRecordError,
 };
 
-use super::NotificationConfig;
-
 pub const MAX_LIMIT: u32 = 1000;
 pub const MIN_LIMIT: u32 = 1;
 
+#[derive(Clone, Debug, PartialEq)]
+pub struct NotificationConfig {
+    pub id: String,
+    pub title: String,
+    pub kind: NotificationConfigKind,
+    pub configuration_data: String,
+    pub status: NotificationConfigStatus,
+    pub parameters: String,
+    pub recipient_ids: Vec<String>,
+    pub recipient_list_ids: Vec<String>,
+    pub sql_recipient_list_ids: Vec<String>,
+    pub last_run_datetime: Option<NaiveDateTime>,
+    pub next_due_datetime: Option<NaiveDateTime>,
+}
+
+impl From<NotificationConfigRow> for NotificationConfig {
+    fn from(
+        NotificationConfigRow {
+            id,
+            title,
+            kind,
+            configuration_data,
+            status,
+            parameters,
+            recipient_ids,
+            recipient_list_ids,
+            sql_recipient_list_ids,
+            last_run_datetime,
+            next_due_datetime,
+        }: NotificationConfigRow,
+    ) -> Self {
+        NotificationConfig {
+            id,
+            title,
+            kind,
+            configuration_data,
+            status,
+            parameters,
+            recipient_ids: serde_json::from_str(&recipient_ids).unwrap_or_default(),
+            recipient_list_ids: serde_json::from_str(&recipient_list_ids).unwrap_or_default(),
+            sql_recipient_list_ids: serde_json::from_str(&sql_recipient_list_ids)
+                .unwrap_or_default(),
+            last_run_datetime,
+            next_due_datetime,
+        }
+    }
+}
+
 pub fn get_notification_configs(
     ctx: &ServiceContext,
     pagination: Option<PaginationOption>,
@@ -24,8 +71,10 @@ pub fn get_notification_configs(
     let pagination = get_default_pagination(pagination, MAX_LIMIT, MIN_LIMIT)?;
     let repository = NotificationConfigRepository::new(&ctx.connection);
 
+    let rows = repository.query(pagination, filter.clone(), sort)?;
+
     Ok(ListResult {
-        rows: repository.query(pagination, filter.clone(), sort)?,
+        rows: rows.into_iter().map(|row| row.into()).collect(),
         count: i64_to_u32(repository.count(filter)?),
     })
 }
@@ -40,19 +89,19 @@ pub fn get_notification_config(
         .query_by_filter(NotificationConfigFilter::new().id(EqualFilter::equal_to(&id)))?;
 
     if let Some(record) = result.pop() {
-        Ok(record)
+        Ok(record.into())
     } else {
         Err(SingleRecordError::NotFound(id))
     }
 }
 
-pub fn get_notification_configs_by_kind_and_next_check_date(
+pub fn find_all_due_by_kind(
     ctx: &ServiceContext,
     kind: NotificationConfigKind,
     datetime: NaiveDateTime,
 ) -> Result<Vec<NotificationConfig>, ListError> {
     let repository = NotificationConfigRowRepository::new(&ctx.connection);
     let result = repository.find_all_due_by_kind(kind, datetime)?;
 
-    Ok(result)
+    Ok(result.into_iter().map(|row| row.into()).collect())
 }