Skip to content

Commit

Permalink
Merge pull request #181 from openmsupply/180-add-reference_name-to-qu…
Browse files Browse the repository at this point in the history
…eries

Add reference name to queries
  • Loading branch information
jmbrunskill authored Oct 10, 2023
2 parents 0980c67 + 8e8dc16 commit 797652d
Show file tree
Hide file tree
Showing 27 changed files with 274 additions and 74 deletions.
15 changes: 13 additions & 2 deletions backend/graphql/notification_query/src/mutations/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use super::{map_error, ModifyNotificationQueryResponse};
pub struct CreateNotificationQueryInput {
pub id: String,
pub name: String,
pub reference_name: String,
}

pub fn create_notification_query(
Expand Down Expand Up @@ -41,7 +42,17 @@ pub fn create_notification_query(
}

impl From<CreateNotificationQueryInput> for CreateNotificationQuery {
fn from(CreateNotificationQueryInput { id, name }: CreateNotificationQueryInput) -> Self {
CreateNotificationQuery { id, name }
fn from(
CreateNotificationQueryInput {
id,
name,
reference_name,
}: CreateNotificationQueryInput,
) -> Self {
CreateNotificationQuery {
id,
name,
reference_name,
}
}
}
3 changes: 3 additions & 0 deletions backend/graphql/notification_query/src/mutations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ pub fn map_error(error: ModifyNotificationQueryError) -> Result<ModifyNotificati
ModifyNotificationQueryError::InternalError(s) => InternalError(s),
ModifyNotificationQueryError::InvalidNotificationQueryName => BadUserInput(formatted_error),
ModifyNotificationQueryError::BadUserInput(s) => BadUserInput(s),
ModifyNotificationQueryError::ReferenceNameAlreadyExists => {
BadUserInput("Reference name must be unique".to_string())
}
};

Err(graphql_error.extend())
Expand Down
3 changes: 3 additions & 0 deletions backend/graphql/notification_query/src/mutations/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use super::{map_error, ModifyNotificationQueryResponse};
pub struct UpdateNotificationQueryInput {
pub id: String,
pub name: Option<String>,
pub reference_name: Option<String>,
pub description: Option<String>,
pub query: Option<String>,
pub required_parameters: Option<Vec<String>>,
Expand Down Expand Up @@ -47,6 +48,7 @@ impl From<UpdateNotificationQueryInput> for UpdateNotificationQuery {
UpdateNotificationQueryInput {
id,
name,
reference_name,
description,
query,
required_parameters,
Expand All @@ -55,6 +57,7 @@ impl From<UpdateNotificationQueryInput> for UpdateNotificationQuery {
UpdateNotificationQuery {
id,
name,
reference_name,
description,
query,
required_parameters,
Expand Down
1 change: 1 addition & 0 deletions backend/graphql/notification_query/src/types/inputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ impl From<NotificationQueryFilterInput> for NotificationQueryFilter {
NotificationQueryFilter {
id: f.id.map(EqualFilter::from),
name: f.name.map(StringFilter::from),
reference_name: None,
search: f.search,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ impl NotificationQueryNode {
pub async fn name(&self) -> &str {
&self.row().name
}
pub async fn reference_name(&self) -> &str {
&self.row().reference_name
}
pub async fn description(&self) -> &str {
&self.row().description
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-- This file should undo anything in `up.sql`
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ALTER TABLE notification_query ADD COLUMN reference_name TEXT NOT NULL default id;
UPDATE notification_query SET reference_name = id;

CREATE UNIQUE INDEX ui_notification_query_reference_name ON notification_query(reference_name);
21 changes: 19 additions & 2 deletions backend/repository/src/db_diesel/notification_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub type NotificationQuery = NotificationQueryRow;
pub struct NotificationQueryFilter {
pub id: Option<EqualFilter<String>>,
pub name: Option<StringFilter>,
pub reference_name: Option<StringFilter>,
pub search: Option<String>,
}

Expand Down Expand Up @@ -100,17 +101,29 @@ fn create_filtered_query(filter: Option<NotificationQueryFilter>) -> BoxedQuery
let mut query = notification_query_dsl::notification_query.into_boxed();

if let Some(f) = filter {
let NotificationQueryFilter { id, name, search } = f;
let NotificationQueryFilter {
id,
name,
reference_name,
search,
} = f;

apply_equal_filter!(query, id, notification_query_dsl::id);
apply_string_filter!(query, name, notification_query_dsl::name);
apply_string_filter!(
query,
reference_name,
notification_query_dsl::reference_name
);

if let Some(search) = search {
let search_term = format!("%{}%", search);
query = query.filter(
notification_query_dsl::name
.like(search_term.clone())
.or(notification_query_dsl::description.like(search_term)),
.or(notification_query_dsl::description.like(search_term.clone()))
.or(notification_query_dsl::reference_name.like(search_term.clone()))
.or(notification_query_dsl::query.like(search_term.clone())),
);
}
}
Expand All @@ -131,6 +144,10 @@ impl NotificationQueryFilter {
self.name = Some(filter);
self
}
pub fn reference_name(mut self, filter: StringFilter) -> Self {
self.reference_name = Some(filter);
self
}

pub fn search(mut self, filter: String) -> Self {
self.search = Some(filter);
Expand Down
2 changes: 2 additions & 0 deletions backend/repository/src/db_diesel/notification_query_row.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ table! {
notification_query (id) {
id -> Text,
name -> Text,
reference_name -> Text,
description -> Text,
query -> Text,
required_parameters -> Text,
Expand All @@ -24,6 +25,7 @@ table! {
pub struct NotificationQueryRow {
pub id: String,
pub name: String,
pub reference_name: String,
pub description: String,
pub query: String,
pub required_parameters: String,
Expand Down
17 changes: 16 additions & 1 deletion backend/service/src/notification_query/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use super::{
validate::{
check_list_name_doesnt_contain_special_characters, check_list_name_is_appropriate_length,
check_notification_query_does_not_exist, check_notification_query_name_is_unique,
check_notification_query_reference_name_is_unique,
},
ModifyNotificationQueryError,
};
Expand All @@ -19,6 +20,7 @@ use repository::{
pub struct CreateNotificationQuery {
pub id: String,
pub name: String,
pub reference_name: String,
}

pub fn create_notification_query(
Expand Down Expand Up @@ -73,15 +75,28 @@ pub fn validate(
return Err(ModifyNotificationQueryError::NotificationQueryAlreadyExists);
}

if !check_notification_query_reference_name_is_unique(
&new_notification_query.id,
Some(new_notification_query.reference_name.clone()),
connection,
)? {
return Err(ModifyNotificationQueryError::ReferenceNameAlreadyExists);
}

Ok(())
}

pub fn generate(
CreateNotificationQuery { id, name }: CreateNotificationQuery,
CreateNotificationQuery {
id,
name,
reference_name,
}: CreateNotificationQuery,
) -> Result<NotificationQueryRow, ModifyNotificationQueryError> {
Ok(NotificationQueryRow {
id,
name: name.trim().to_string(),
reference_name: reference_name.trim().to_string(),
description: "".to_string(),
query: "".to_string(),
required_parameters: "[]".to_string(),
Expand Down
35 changes: 2 additions & 33 deletions backend/service/src/notification_query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,48 +69,17 @@ pub trait NotificationQueryServiceTrait: Sync + Send {
pub struct NotificationQueryService {}
impl NotificationQueryServiceTrait for NotificationQueryService {}

#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub enum ModifyNotificationQueryError {
NotificationQueryAlreadyExists,
ReferenceNameAlreadyExists,
ModifiedRecordNotFound,
DatabaseError(RepositoryError),
NotificationQueryDoesNotExist,
InvalidNotificationQueryName,
InternalError(String),
BadUserInput(String),
}

// PartialEq is only needed for tests
impl PartialEq for ModifyNotificationQueryError {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(
ModifyNotificationQueryError::NotificationQueryAlreadyExists,
ModifyNotificationQueryError::NotificationQueryAlreadyExists,
) => true,

(
ModifyNotificationQueryError::ModifiedRecordNotFound,
ModifyNotificationQueryError::ModifiedRecordNotFound,
) => true,
(
ModifyNotificationQueryError::DatabaseError(self_err),
ModifyNotificationQueryError::DatabaseError(other_err),
) => self_err == other_err,

(
ModifyNotificationQueryError::NotificationQueryDoesNotExist,
ModifyNotificationQueryError::NotificationQueryDoesNotExist,
) => true,
(
ModifyNotificationQueryError::InvalidNotificationQueryName,
ModifyNotificationQueryError::InvalidNotificationQueryName,
) => true,
_ => false,
}
}
}

impl From<RepositoryError> for ModifyNotificationQueryError {
fn from(err: RepositoryError) -> Self {
ModifyNotificationQueryError::DatabaseError(err)
Expand Down
3 changes: 3 additions & 0 deletions backend/service/src/notification_query/tests/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ mod notification_query_query_test {
id: id1.clone(),
name: name1.clone(),
description: description1.clone(),
reference_name: "reference_name1".to_string(),
..Default::default()
};
repo.insert_one(&notification_query).unwrap();
Expand All @@ -124,6 +125,7 @@ mod notification_query_query_test {
id: id2.clone(),
name: name2.clone(),
description: description2.clone(),
reference_name: "reference_name2".to_string(),
..Default::default()
};
repo.insert_one(&notification_query).unwrap();
Expand All @@ -135,6 +137,7 @@ mod notification_query_query_test {
id: id3.clone(),
name: name3.clone(),
description: description3.clone(),
reference_name: "reference_name3".to_string(),
..Default::default()
};
repo.insert_one(&notification_query).unwrap();
Expand Down
15 changes: 15 additions & 0 deletions backend/service/src/notification_query/tests/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ mod notification_query_update_tests {
let notification_query = NotificationQueryRow {
id: id1.clone(),
name: name1.clone(),
reference_name: "reference_name1".to_string(),
..Default::default()
};
repo.insert_one(&notification_query).unwrap();
Expand All @@ -44,6 +45,7 @@ mod notification_query_update_tests {
let notification_query = NotificationQueryRow {
id: id2.clone(),
name: name2.clone(),
reference_name: "reference_name2".to_string(),
..Default::default()
};
repo.insert_one(&notification_query).unwrap();
Expand Down Expand Up @@ -74,6 +76,19 @@ mod notification_query_update_tests {
Err(ModifyNotificationQueryError::NotificationQueryAlreadyExists)
);

// Trying to update to a reference_name that already exists should fail (even with added whitespace)
assert_eq!(
service.update_notification_query(
&context,
UpdateNotificationQuery {
id: id1.clone(),
reference_name: Some("reference_name2 ".to_string()),
..Default::default()
},
),
Err(ModifyNotificationQueryError::ReferenceNameAlreadyExists)
);

// Trying to update to a name with illegal characters should fail
assert_eq!(
service.update_notification_query(
Expand Down
14 changes: 14 additions & 0 deletions backend/service/src/notification_query/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use super::{
validate::{
check_list_name_doesnt_contain_special_characters, check_list_name_is_appropriate_length,
check_notification_query_exists, check_notification_query_name_is_unique,
check_notification_query_reference_name_is_unique,
},
ModifyNotificationQueryError,
};
Expand All @@ -17,6 +18,7 @@ use repository::{
pub struct UpdateNotificationQuery {
pub id: String,
pub name: Option<String>,
pub reference_name: Option<String>,
pub description: Option<String>,
pub query: Option<String>,
pub required_parameters: Option<Vec<String>>,
Expand Down Expand Up @@ -78,13 +80,22 @@ pub fn validate(
return Err(ModifyNotificationQueryError::NotificationQueryAlreadyExists);
}

if !check_notification_query_reference_name_is_unique(
&new_notification_query.id,
new_notification_query.reference_name.clone(),
connection,
)? {
return Err(ModifyNotificationQueryError::ReferenceNameAlreadyExists);
}

Ok(notification_query_row)
}

pub fn generate(
UpdateNotificationQuery {
id: _id, //ID is already used for look up so we can assume it's the same
name,
reference_name,
description,
query,
required_parameters,
Expand All @@ -95,6 +106,9 @@ pub fn generate(
if let Some(name) = name {
new_notification_query_row.name = name.trim().to_string();
}
if let Some(reference_name) = reference_name {
new_notification_query_row.reference_name = reference_name.trim().to_string();
}
if let Some(description) = description {
new_notification_query_row.description = description;
}
Expand Down
18 changes: 18 additions & 0 deletions backend/service/src/notification_query/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,24 @@ pub fn check_notification_query_name_is_unique(
}
}

pub fn check_notification_query_reference_name_is_unique(
id: &str,
reference_name: Option<String>,
connection: &StorageConnection,
) -> Result<bool, RepositoryError> {
let Some(reference_name) = reference_name else {
return Ok(true);
};

let notification_queries = NotificationQueryRepository::new(connection).query_by_filter(
NotificationQueryFilter::new()
.reference_name(StringFilter::equal_to(&reference_name.trim().to_string()))
.id(EqualFilter::not_equal_to(id)),
)?;

Ok(notification_queries.is_empty())
}

// TODO: Refactor as part of https://github.com/openmsupply/notify/issues/140
pub fn check_list_name_is_appropriate_length(name: &str) -> Result<bool, RepositoryError> {
Ok(name.trim().len() >= 3 && name.len() <= 70)
Expand Down
3 changes: 2 additions & 1 deletion frontend/packages/common/src/intl/locales/en/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -181,5 +181,6 @@
"warning.caps-lock": "Warning: Caps lock is on",
"label.subject-template": "Subject template",
"label.body-template": "Body template",
"label.parameters": "Parameters"
"label.parameters": "Parameters",
"label.reference-name": "Reference Name"
}
Loading

0 comments on commit 797652d

Please sign in to comment.