From 50a6c443ab5030f013d42b0a3c82d96e4e283e09 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Thu, 20 May 2021 11:25:38 +0100 Subject: [PATCH 01/19] Add diesel-derive-newtype crate, use to define Doi/Orcid wrapper types, add parsing --- Cargo.lock | 24 +++++++ thoth-api/Cargo.toml | 1 + thoth-api/src/contributor/model.rs | 102 ++++++++++++++++++++++++++++ thoth-api/src/errors.rs | 2 + thoth-api/src/lib.rs | 3 + thoth-api/src/work/model.rs | 104 +++++++++++++++++++++++++++++ 6 files changed, 236 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index e5d04b78..da87aed9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1125,6 +1125,18 @@ dependencies = [ "syn 1.0.33", ] +[[package]] +name = "diesel-derive-newtype" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e844e8e6f65dcf27aa0b97d4234f974d93dfbf56816033d71b5e0c7eb701709f" +dependencies = [ + "diesel", + "proc-macro2 0.4.30", + "quote 0.6.13", + "syn 0.14.9", +] + [[package]] name = "diesel_derives" version = "1.4.1" @@ -3468,6 +3480,17 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "343f3f510c2915908f155e94f17220b19ccfacf2a64a2a5d8004f2c3e311e7fd" +[[package]] +name = "syn" +version = "0.14.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "261ae9ecaa397c42b960649561949d69311f08eeaea86a65696e6e46517cf741" +dependencies = [ + "proc-macro2 0.4.30", + "quote 0.6.13", + "unicode-xid 0.1.0", +] + [[package]] name = "syn" version = "0.15.44" @@ -3621,6 +3644,7 @@ dependencies = [ "chrono", "diesel", "diesel-derive-enum", + "diesel-derive-newtype", "diesel_migrations", "dotenv", "failure", diff --git a/thoth-api/Cargo.toml b/thoth-api/Cargo.toml index 5c306ad3..727d9a8d 100644 --- a/thoth-api/Cargo.toml +++ b/thoth-api/Cargo.toml @@ -21,6 +21,7 @@ argon2rs = "0.2.5" chrono = { version = "0.4", features = ["serde"] } diesel = { version = "1.4.0", features = ["postgres", "uuidv07", "chrono", "r2d2", "64-column-tables", "serde_json"], optional = true } diesel-derive-enum = { version = "1.1.0", features = ["postgres"], optional = true } +diesel-derive-newtype = "0.1" diesel_migrations = { version = "1.4.0", features = ["postgres"], optional = true } dotenv = "0.9.0" failure = "0.1.6" diff --git a/thoth-api/src/contributor/model.rs b/thoth-api/src/contributor/model.rs index 4b46e20a..e82cd2ec 100644 --- a/thoth-api/src/contributor/model.rs +++ b/thoth-api/src/contributor/model.rs @@ -3,10 +3,12 @@ use chrono::Utc; use serde::Deserialize; use serde::Serialize; use std::fmt; +use std::str::FromStr; use strum::Display; use strum::EnumString; use uuid::Uuid; +use crate::errors::{ThothError, ThothResult}; use crate::graphql::utils::Direction; #[cfg(feature = "backend")] use crate::schema::contributor; @@ -34,6 +36,15 @@ pub enum ContributorField { UpdatedAt, } +#[cfg_attr( + feature = "backend", + derive(DieselNewType, juniper::GraphQLScalarValue) +)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct Orcid(String); + +const ORCID_DOMAIN: &str = "https://orcid.org/"; + #[cfg_attr(feature = "backend", derive(Queryable))] #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] @@ -138,6 +149,50 @@ impl fmt::Display for Contributor { } } +impl fmt::Display for Orcid { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.0.replace(ORCID_DOMAIN, "")) + } +} + +impl FromStr for Orcid { + type Err = ThothError; + + fn from_str(input: &str) -> ThothResult { + use lazy_static::lazy_static; + use regex::Regex; + lazy_static! { + static ref RE: Regex = Regex::new( + // ^ = beginning of string + // (?:) = non-capturing group + // i = case-insensitive flag + // $ = end of string + // Matches strings of format "[[http[s]://]orcid.org/]0000-000X-XXXX-XXXX" + // and captures the 16-digit identifier segment + // Corresponds to database constraints although regex syntax differs slightly + r#"^(?i:(?:https?://)?orcid\.org/)?(0000-000(?:1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$)"#).unwrap(); + } + if let Some(matches) = RE.captures(input) { + // The 0th capture always corresponds to the entire match + if let Some(identifier) = matches.get(1) { + let standardised = format!("{}{}", ORCID_DOMAIN, identifier.as_str()); + let orcid: Orcid = Orcid { 0: standardised }; + Ok(orcid) + } else { + Err(ThothError::IdentifierParseError( + input.to_string(), + "ORCID".to_string(), + )) + } + } else { + Err(ThothError::IdentifierParseError( + input.to_string(), + "ORCID".to_string(), + )) + } + } +} + #[test] fn test_contributorfield_default() { let contfield: ContributorField = Default::default(); @@ -187,3 +242,50 @@ fn test_contributorfield_fromstr() { assert!(ContributorField::from_str("Biography").is_err()); assert!(ContributorField::from_str("Institution").is_err()); } + +#[test] +fn test_orcid_display() { + let orcid = Orcid { + 0: "https://orcid.org/0000-0002-1234-5678".to_string(), + }; + assert_eq!(format!("{}", orcid), "0000-0002-1234-5678"); +} + +#[test] +fn test_orcid_fromstr() { + let standardised = Orcid { + 0: "https://orcid.org/0000-0002-1234-5678".to_string(), + }; + assert_eq!( + Orcid::from_str("https://orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("http://orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("HTTPS://ORCID.ORG/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("Https://ORCiD.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert!(Orcid::from_str("htts://orcid.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("https://0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("https://test.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("http://test.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("test.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("//orcid.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("https://orcid-org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("0000-0002-1234-5678https://orcid.org/").is_err()); +} diff --git a/thoth-api/src/errors.rs b/thoth-api/src/errors.rs index 48f032e7..2ee9d152 100644 --- a/thoth-api/src/errors.rs +++ b/thoth-api/src/errors.rs @@ -29,6 +29,8 @@ pub enum ThothError { EntityNotFound, #[fail(display = "Issue's Work and Series cannot have different Imprints.")] IssueImprintsError, + #[fail(display = "{} is not a validly formatted {}", _0, _1)] + IdentifierParseError(String, String), } impl juniper::IntoFieldError for ThothError { diff --git a/thoth-api/src/lib.rs b/thoth-api/src/lib.rs index d023e602..d66c083a 100644 --- a/thoth-api/src/lib.rs +++ b/thoth-api/src/lib.rs @@ -6,6 +6,9 @@ extern crate diesel; extern crate diesel_derive_enum; #[cfg(feature = "backend")] #[macro_use] +extern crate diesel_derive_newtype; +#[cfg(feature = "backend")] +#[macro_use] extern crate diesel_migrations; extern crate dotenv; #[macro_use] diff --git a/thoth-api/src/work/model.rs b/thoth-api/src/work/model.rs index 5be49c0b..88a1b62b 100644 --- a/thoth-api/src/work/model.rs +++ b/thoth-api/src/work/model.rs @@ -2,11 +2,14 @@ use chrono::naive::NaiveDate; use chrono::DateTime; use chrono::Utc; use serde::{Deserialize, Serialize}; +use std::fmt; +use std::str::FromStr; use strum::Display; use strum::EnumString; use uuid::Uuid; use crate::contribution::model::Contribution; +use crate::errors::{ThothError, ThothResult}; use crate::funding::model::FundingExtended as Funding; use crate::graphql::utils::Direction; use crate::imprint::model::ImprintExtended as Imprint; @@ -114,6 +117,15 @@ pub enum WorkField { UpdatedAt, } +#[cfg_attr( + feature = "backend", + derive(DieselNewType, juniper::GraphQLScalarValue) +)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct Doi(String); + +const DOI_DOMAIN: &str = "https://doi.org/"; + #[cfg_attr(feature = "backend", derive(Queryable))] #[derive(Serialize, Deserialize)] pub struct Work { @@ -381,6 +393,51 @@ impl Default for WorkExtended { } } +impl fmt::Display for Doi { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.0.replace(DOI_DOMAIN, "")) + } +} + +impl FromStr for Doi { + type Err = ThothError; + + fn from_str(input: &str) -> ThothResult { + use lazy_static::lazy_static; + use regex::Regex; + lazy_static! { + static ref RE: Regex = Regex::new( + // ^ = beginning of string + // (?:) = non-capturing group + // i = case-insensitive flag + // $ = end of string + // Matches strings of format "[[http[s]://]doi.org/]10.XXX/XXX" + // and captures the identifier segment starting with the "10." directory indicator + // Corresponds to database constraints although regex syntax differs slightly + // (e.g. `;()/` do not need to be escaped here) + r#"^(?i:(?:https?://)?doi\.org/)?(10\.\d{4,9}/[-._;()/:a-zA-Z0-9]+$)"#).unwrap(); + } + if let Some(matches) = RE.captures(input) { + // The 0th capture always corresponds to the entire match + if let Some(identifier) = matches.get(1) { + let standardised = format!("{}{}", DOI_DOMAIN, identifier.as_str()); + let doi: Doi = Doi { 0: standardised }; + Ok(doi) + } else { + Err(ThothError::IdentifierParseError( + input.to_string(), + "DOI".to_string(), + )) + } + } else { + Err(ThothError::IdentifierParseError( + input.to_string(), + "DOI".to_string(), + )) + } + } +} + #[test] fn test_worktype_default() { let worktype: WorkType = Default::default(); @@ -652,3 +709,50 @@ fn test_workfield_fromstr() { assert!(WorkField::from_str("Contributors").is_err()); assert!(WorkField::from_str("Publisher").is_err()); } + +#[test] +fn test_doi_display() { + let doi = Doi { + 0: "https://doi.org/10.12345/Test-Suffix.01".to_string(), + }; + assert_eq!(format!("{}", doi), "10.12345/Test-Suffix.01"); +} + +#[test] +fn test_doi_fromstr() { + let standardised = Doi { + 0: "https://doi.org/10.12345/Test-Suffix.01".to_string(), + }; + assert_eq!( + Doi::from_str("https://doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("http://doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("HTTPS://DOI.ORG/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("Https://DOI.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert!(Doi::from_str("htts://doi.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("https://10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("https://test.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("http://test.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("test.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("//doi.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("https://doi-org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("10.https://doi.org/12345/Test-Suffix.01").is_err()); +} From 0bdcbf44a4c0d5b839c2f0fac593a5e8b6875527 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Fri, 21 May 2021 10:02:51 +0100 Subject: [PATCH 02/19] POC: convert String to Orcid on UpdateContributor and only update if parsing succeeds --- thoth-app/src/component/contributor.rs | 56 ++++++++++++------- .../update_contributor_mutation.rs | 4 +- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index 7b73972c..597025e8 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -1,4 +1,4 @@ -use thoth_api::contributor::model::Contributor; +use thoth_api::contributor::model::{Contributor, Orcid}; use uuid::Uuid; use yew::html; use yew::prelude::*; @@ -184,23 +184,41 @@ impl Component for ContributorComponent { } } Msg::UpdateContributor => { - let body = UpdateContributorRequestBody { - variables: UpdateVariables { - contributor_id: self.contributor.contributor_id, - first_name: self.contributor.first_name.clone(), - last_name: self.contributor.last_name.clone(), - full_name: self.contributor.full_name.clone(), - orcid: self.contributor.orcid.clone(), - website: self.contributor.website.clone(), - }, - ..Default::default() - }; - let request = UpdateContributorRequest { body }; - self.push_contributor = Fetch::new(request); - self.link - .send_future(self.push_contributor.fetch(Msg::SetContributorPushState)); - self.link - .send_message(Msg::SetContributorPushState(FetchAction::Fetching)); + // Check ORCID is correctly formatted before proceeding with save. + // If no ORCID was provided, no check is required. + let mut parsed_orcid = None; + let mut ok_to_save = true; + if self.contributor.orcid.is_some() { + match self.contributor.orcid.as_ref().unwrap().parse::() { + Ok(result) => parsed_orcid = Some(result), + Err(err) => { + ok_to_save = false; + self.notification_bus.send(Request::NotificationBusMsg(( + err.to_string(), + NotificationStatus::Danger, + ))) + } + } + } + if ok_to_save { + let body = UpdateContributorRequestBody { + variables: UpdateVariables { + contributor_id: self.contributor.contributor_id, + first_name: self.contributor.first_name.clone(), + last_name: self.contributor.last_name.clone(), + full_name: self.contributor.full_name.clone(), + orcid: parsed_orcid, + website: self.contributor.website.clone(), + }, + ..Default::default() + }; + let request = UpdateContributorRequest { body }; + self.push_contributor = Fetch::new(request); + self.link + .send_future(self.push_contributor.fetch(Msg::SetContributorPushState)); + self.link + .send_message(Msg::SetContributorPushState(FetchAction::Fetching)); + } false } Msg::SetContributorDeleteState(fetch_state) => { @@ -363,7 +381,7 @@ impl Component for ContributorComponent { oninput=self.link.callback(|e: InputData| Msg::ChangeFullName(e.value)) required = true /> - { - // Check ORCID is correctly formatted before proceeding with save. - // If no ORCID was provided, no check is required. - let mut parsed_orcid = None; - let mut ok_to_save = true; - if self.contributor.orcid.is_some() { - match self.contributor.orcid.as_ref().unwrap().parse::() { - Ok(result) => parsed_orcid = Some(result), - Err(err) => { - ok_to_save = false; - self.notification_bus.send(Request::NotificationBusMsg(( - err.to_string(), - NotificationStatus::Danger, - ))) - } - } - } - if ok_to_save { - let body = UpdateContributorRequestBody { - variables: UpdateVariables { - contributor_id: self.contributor.contributor_id, - first_name: self.contributor.first_name.clone(), - last_name: self.contributor.last_name.clone(), - full_name: self.contributor.full_name.clone(), - orcid: parsed_orcid, - website: self.contributor.website.clone(), - }, - ..Default::default() - }; - let request = UpdateContributorRequest { body }; - self.push_contributor = Fetch::new(request); - self.link - .send_future(self.push_contributor.fetch(Msg::SetContributorPushState)); - self.link - .send_message(Msg::SetContributorPushState(FetchAction::Fetching)); - } + let body = UpdateContributorRequestBody { + variables: UpdateVariables { + contributor_id: self.contributor.contributor_id, + first_name: self.contributor.first_name.clone(), + last_name: self.contributor.last_name.clone(), + full_name: self.contributor.full_name.clone(), + orcid: self.contributor.orcid.clone(), + website: self.contributor.website.clone(), + }, + ..Default::default() + }; + let request = UpdateContributorRequest { body }; + self.push_contributor = Fetch::new(request); + self.link + .send_future(self.push_contributor.fetch(Msg::SetContributorPushState)); + self.link + .send_message(Msg::SetContributorPushState(FetchAction::Fetching)); false } Msg::SetContributorDeleteState(fetch_state) => { @@ -381,7 +363,7 @@ impl Component for ContributorComponent { oninput=self.link.callback(|e: InputData| Msg::ChangeFullName(e.value)) required = true /> - { - let orcid = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), + // Check ORCID is correctly formatted before proceeding with save. + // If no ORCID was provided, no check is required. + let mut parsed_orcid = None; + let mut ok_to_save = true; + if !value.trim().is_empty() { + match value.parse::() { + Ok(result) => parsed_orcid = Some(result), + Err(err) => { + ok_to_save = false; + self.notification_bus.send(Request::NotificationBusMsg(( + err.to_string(), + NotificationStatus::Danger, + ))) + } + } }; - self.contributor.orcid.neq_assign(orcid) + if ok_to_save { + self.contributor.orcid.neq_assign(parsed_orcid) + } else { + false + } } Msg::ChangeWebsite(value) => { let website = match value.trim().is_empty() { @@ -274,9 +290,9 @@ impl Component for NewContributorComponent { onblur=self.link.callback(|_| Msg::ToggleDuplicateTooltip(false)) required=true /> - c.to_owned(), None => Default::default(), }; + // Initialise user-entered ORCID variable to match ORCID in database + self.orcid = self + .contributor + .orcid + .clone() + .unwrap_or_default() + .to_string(); true } FetchState::Failed(_, _err) => false, @@ -160,6 +171,13 @@ impl Component for ContributorComponent { FetchState::Fetching(_) => false, FetchState::Fetched(body) => match &body.data.update_contributor { Some(c) => { + // Save was successful: update user-entered ORCID variable to match ORCID in database + self.orcid = self + .contributor + .orcid + .clone() + .unwrap_or_default() + .to_string(); self.notification_bus.send(Request::NotificationBusMsg(( format!("Saved {}", c.full_name), NotificationStatus::Success, @@ -269,24 +287,23 @@ impl Component for ContributorComponent { .full_name .neq_assign(full_name.trim().to_owned()), Msg::ChangeOrcid(value) => { - // Check ORCID is correctly formatted before proceeding with save. - // If no ORCID was provided, no check is required. - let mut parsed_orcid = None; - let mut ok_to_save = true; - if !value.trim().is_empty() { - match value.parse::() { - Ok(result) => parsed_orcid = Some(result), - Err(err) => { - ok_to_save = false; - self.notification_bus.send(Request::NotificationBusMsg(( + if self.orcid.neq_assign(value.trim().to_owned()) { + // Check ORCID is correctly formatted before updating structure. + // If no ORCID was provided, no check is required. + if self.orcid.trim().is_empty() { + self.contributor.orcid.neq_assign(None); + } else { + match self.orcid.parse::() { + Ok(result) => { + let _ = self.contributor.orcid.neq_assign(Some(result)); + } + Err(err) => self.notification_bus.send(Request::NotificationBusMsg(( err.to_string(), NotificationStatus::Danger, - ))) - } + ))), + }; } - }; - if ok_to_save { - self.contributor.orcid.neq_assign(parsed_orcid) + true } else { false } @@ -381,7 +398,7 @@ impl Component for ContributorComponent { /> Date: Tue, 25 May 2021 13:37:42 +0100 Subject: [PATCH 06/19] Replace parse error notifications with tooltip (invalid ORCIDs reset to DB version on save) --- thoth-api/src/errors.rs | 5 ++++- thoth-app/src/component/contributor.rs | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/thoth-api/src/errors.rs b/thoth-api/src/errors.rs index 2ee9d152..32c82de4 100644 --- a/thoth-api/src/errors.rs +++ b/thoth-api/src/errors.rs @@ -29,7 +29,10 @@ pub enum ThothError { EntityNotFound, #[fail(display = "Issue's Work and Series cannot have different Imprints.")] IssueImprintsError, - #[fail(display = "{} is not a validly formatted {}", _0, _1)] + #[fail( + display = "{} is not a validly formatted {} and will not be saved", + _0, _1 + )] IdentifierParseError(String, String), } diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index 5b224584..8d893b43 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -21,6 +21,7 @@ use crate::agent::notification_bus::NotificationStatus; use crate::agent::notification_bus::Request; use crate::component::delete_dialogue::ConfirmDeleteComponent; use crate::component::utils::FormTextInput; +use crate::component::utils::FormTextInputTooltip; use crate::component::utils::FormUrlInput; use crate::component::utils::Loader; use crate::models::contributor::contributor_activity_query::ContributorActivityResponseData; @@ -48,6 +49,7 @@ pub struct ContributorComponent { contributor: Contributor, // Track the user-entered ORCID string, which may not be validly formatted orcid: String, + orcid_warning: String, fetch_contributor: FetchContributor, push_contributor: PushUpdateContributor, delete_contributor: PushDeleteContributor, @@ -97,6 +99,7 @@ impl Component for ContributorComponent { let notification_bus = NotificationBus::dispatcher(); let contributor: Contributor = Default::default(); let orcid = Default::default(); + let orcid_warning = Default::default(); let router = RouteAgentDispatcher::new(); let mut _contributor_activity_checker = ContributorActivityChecker::bridge(link.callback(Msg::GetContributorActivity)); @@ -110,6 +113,7 @@ impl Component for ContributorComponent { ContributorComponent { contributor, orcid, + orcid_warning, fetch_contributor, push_contributor, delete_contributor, @@ -178,6 +182,7 @@ impl Component for ContributorComponent { .clone() .unwrap_or_default() .to_string(); + self.orcid_warning.clear(); self.notification_bus.send(Request::NotificationBusMsg(( format!("Saved {}", c.full_name), NotificationStatus::Success, @@ -292,15 +297,16 @@ impl Component for ContributorComponent { // If no ORCID was provided, no check is required. if self.orcid.trim().is_empty() { self.contributor.orcid.neq_assign(None); + self.orcid_warning.clear(); } else { match self.orcid.parse::() { Ok(result) => { - let _ = self.contributor.orcid.neq_assign(Some(result)); + self.contributor.orcid.neq_assign(Some(result)); + self.orcid_warning.clear(); + } + Err(err) => { + self.orcid_warning = err.to_string(); } - Err(err) => self.notification_bus.send(Request::NotificationBusMsg(( - err.to_string(), - NotificationStatus::Danger, - ))), }; } true @@ -396,9 +402,10 @@ impl Component for ContributorComponent { oninput=self.link.callback(|e: InputData| Msg::ChangeFullName(e.value)) required = true /> - Date: Thu, 27 May 2021 13:49:49 +0100 Subject: [PATCH 07/19] Implement static button by duplicating and extending FormTextTooltip util --- thoth-app/src/component/contributor.rs | 5 +- thoth-app/src/component/utils.rs | 68 ++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index 8d893b43..92ac14e8 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -21,7 +21,7 @@ use crate::agent::notification_bus::NotificationStatus; use crate::agent::notification_bus::Request; use crate::component::delete_dialogue::ConfirmDeleteComponent; use crate::component::utils::FormTextInput; -use crate::component::utils::FormTextInputTooltip; +use crate::component::utils::FormTextInputTooltipStatic; use crate::component::utils::FormUrlInput; use crate::component::utils::Loader; use crate::models::contributor::contributor_activity_query::ContributorActivityResponseData; @@ -402,8 +402,9 @@ impl Component for ContributorComponent { oninput=self.link.callback(|e: InputData| Msg::ChangeFullName(e.value)) required = true /> - ; pub type FormTextarea = Pure; pub type FormTextInputTooltip = Pure; +pub type FormTextInputTooltipStatic = Pure; pub type FormTextInput = Pure; pub type FormUrlInput = Pure; pub type FormDateInput = Pure; @@ -95,6 +96,22 @@ pub struct PureTextInputTooltip { pub required: bool, } +#[derive(Clone, PartialEq, Properties)] +pub struct PureTextInputTooltipStatic { + pub label: String, + pub value: String, + pub tooltip: String, + pub statictext: String, + #[prop_or_default] + pub oninput: Callback, + #[prop_or_default] + pub onfocus: Callback, + #[prop_or_default] + pub onblur: Callback, + #[prop_or(false)] + pub required: bool, +} + #[derive(Clone, PartialEq, Properties)] pub struct PureTextInput { pub label: String, @@ -376,6 +393,57 @@ impl PureComponent for PureTextInputTooltip { } } +impl PureComponent for PureTextInputTooltipStatic { + fn render(&self) -> VNode { + // Only display tooltip if its value is set. + // Yew release 0.18.0 will introduce optional attributes - + // at this point we can make `data-tooltip` optional + // and collapse down the duplicated `html!` declaration. + if self.tooltip.is_empty() { + html! { +
+ +
+ + +
+
+ } + } else { + html! { +
+ +
+ + +
+
+ } + } + } +} + impl PureComponent for PureTextInput { fn render(&self) -> VNode { html! { From 058cd6803b2c153ca0942a9b39e26cac82813661 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Thu, 27 May 2021 15:21:15 +0100 Subject: [PATCH 08/19] Implement new structs, parsing, error tooltip and static button in all Contributor/Work/Funder contexts --- thoth-api/src/contributor/model.rs | 6 +- thoth-api/src/funder/model.rs | 7 ++- thoth-api/src/graphql/model.rs | 4 +- thoth-api/src/work/model.rs | 24 ++++++-- thoth-app/src/component/contributor.rs | 4 +- thoth-app/src/component/funder.rs | 59 ++++++++++++++++--- thoth-app/src/component/new_contributor.rs | 50 +++++++++------- thoth-app/src/component/new_funder.rs | 44 +++++++++++--- thoth-app/src/component/new_work.rs | 42 ++++++++++--- thoth-app/src/component/work.rs | 50 +++++++++++++--- .../create_contributor_mutation.rs | 2 +- .../update_contributor_mutation.rs | 2 +- .../models/funder/create_funder_mutation.rs | 5 +- thoth-app/src/models/funder/mod.rs | 6 +- .../models/funder/update_funder_mutation.rs | 5 +- thoth-app/src/models/publication/mod.rs | 7 ++- .../src/models/work/create_work_mutation.rs | 5 +- thoth-app/src/models/work/mod.rs | 12 +++- .../src/models/work/update_work_mutation.rs | 5 +- 19 files changed, 256 insertions(+), 83 deletions(-) diff --git a/thoth-api/src/contributor/model.rs b/thoth-api/src/contributor/model.rs index 25d016ff..f3fd281d 100644 --- a/thoth-api/src/contributor/model.rs +++ b/thoth-api/src/contributor/model.rs @@ -43,7 +43,7 @@ pub enum ContributorField { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Orcid(String); -const ORCID_DOMAIN: &str = "https://orcid.org/"; +pub const ORCID_DOMAIN: &str = "https://orcid.org/"; #[cfg_attr(feature = "backend", derive(Queryable))] #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -68,7 +68,7 @@ pub struct NewContributor { pub first_name: Option, pub last_name: String, pub full_name: String, - pub orcid: Option, + pub orcid: Option, pub website: Option, } @@ -83,7 +83,7 @@ pub struct PatchContributor { pub first_name: Option, pub last_name: String, pub full_name: String, - pub orcid: Option, + pub orcid: Option, pub website: Option, } diff --git a/thoth-api/src/funder/model.rs b/thoth-api/src/funder/model.rs index 43b9f88a..c23e2119 100644 --- a/thoth-api/src/funder/model.rs +++ b/thoth-api/src/funder/model.rs @@ -12,6 +12,7 @@ use crate::graphql::utils::Direction; use crate::schema::funder; #[cfg(feature = "backend")] use crate::schema::funder_history; +use crate::work::model::Doi; #[cfg_attr( feature = "backend", @@ -37,7 +38,7 @@ pub enum FunderField { pub struct Funder { pub funder_id: Uuid, pub funder_name: String, - pub funder_doi: Option, + pub funder_doi: Option, pub created_at: DateTime, pub updated_at: DateTime, } @@ -49,7 +50,7 @@ pub struct Funder { )] pub struct NewFunder { pub funder_name: String, - pub funder_doi: Option, + pub funder_doi: Option, } #[cfg_attr( @@ -61,7 +62,7 @@ pub struct NewFunder { pub struct PatchFunder { pub funder_id: Uuid, pub funder_name: String, - pub funder_doi: Option, + pub funder_doi: Option, } #[cfg_attr(feature = "backend", derive(Queryable))] diff --git a/thoth-api/src/graphql/model.rs b/thoth-api/src/graphql/model.rs index 075fbb37..dd81fbee 100644 --- a/thoth-api/src/graphql/model.rs +++ b/thoth-api/src/graphql/model.rs @@ -1369,7 +1369,7 @@ impl Work { #[graphql( description = "Digital Object Identifier of the work as full URL. It must use the HTTPS scheme and the doi.org domain (e.g. https://doi.org/10.11647/obp.0001)" )] - pub fn doi(&self) -> Option<&String> { + pub fn doi(&self) -> Option<&Doi> { self.doi.as_ref() } @@ -2299,7 +2299,7 @@ impl Funder { &self.funder_name } - pub fn funder_doi(&self) -> Option<&String> { + pub fn funder_doi(&self) -> Option<&Doi> { self.funder_doi.as_ref() } diff --git a/thoth-api/src/work/model.rs b/thoth-api/src/work/model.rs index 88a1b62b..c9b455c1 100644 --- a/thoth-api/src/work/model.rs +++ b/thoth-api/src/work/model.rs @@ -124,7 +124,7 @@ pub enum WorkField { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Doi(String); -const DOI_DOMAIN: &str = "https://doi.org/"; +pub const DOI_DOMAIN: &str = "https://doi.org/"; #[cfg_attr(feature = "backend", derive(Queryable))] #[derive(Serialize, Deserialize)] @@ -138,7 +138,7 @@ pub struct Work { pub reference: Option, pub edition: i32, pub imprint_id: Uuid, - pub doi: Option, + pub doi: Option, pub publication_date: Option, pub place: Option, pub width: Option, @@ -175,7 +175,7 @@ pub struct WorkExtended { pub subtitle: Option, pub reference: Option, pub edition: i32, - pub doi: Option, + pub doi: Option, pub publication_date: Option, pub place: Option, pub width: Option, @@ -221,7 +221,7 @@ pub struct NewWork { pub reference: Option, pub edition: i32, pub imprint_id: Uuid, - pub doi: Option, + pub doi: Option, pub publication_date: Option, pub place: Option, pub width: Option, @@ -261,7 +261,7 @@ pub struct PatchWork { pub reference: Option, pub edition: i32, pub imprint_id: Uuid, - pub doi: Option, + pub doi: Option, pub publication_date: Option, pub place: Option, pub width: Option, @@ -393,6 +393,14 @@ impl Default for WorkExtended { } } +impl Default for Doi { + fn default() -> Doi { + Doi { + 0: Default::default(), + } + } +} + impl fmt::Display for Doi { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", &self.0.replace(DOI_DOMAIN, "")) @@ -710,6 +718,12 @@ fn test_workfield_fromstr() { assert!(WorkField::from_str("Publisher").is_err()); } +#[test] +fn test_doi_default() { + let doi: Doi = Default::default(); + assert_eq!(doi, Doi { 0: "".to_string() }); +} + #[test] fn test_doi_display() { let doi = Doi { diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index 92ac14e8..3be3088c 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -1,4 +1,4 @@ -use thoth_api::contributor::model::{Contributor, Orcid}; +use thoth_api::contributor::model::{Contributor, Orcid, ORCID_DOMAIN}; use uuid::Uuid; use yew::html; use yew::prelude::*; @@ -404,7 +404,7 @@ impl Component for ContributorComponent { /> c.to_owned(), None => Default::default(), }; + // Initialise user-entered DOI variable to match DOI in database + self.funder_doi = self + .funder + .funder_doi + .clone() + .unwrap_or_default() + .to_string(); true } FetchState::Failed(_, _err) => false, @@ -157,6 +172,14 @@ impl Component for FunderComponent { FetchState::Fetching(_) => false, FetchState::Fetched(body) => match &body.data.update_funder { Some(f) => { + // Save was successful: update user-entered DOI variable to match DOI in database + self.funder_doi = self + .funder + .funder_doi + .clone() + .unwrap_or_default() + .to_string(); + self.funder_doi_warning.clear(); self.notification_bus.send(Request::NotificationBusMsg(( format!("Saved {}", f.funder_name), NotificationStatus::Success, @@ -250,11 +273,27 @@ impl Component for FunderComponent { .funder_name .neq_assign(funder_name.trim().to_owned()), Msg::ChangeFunderDoi(value) => { - let funder_doi = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.funder.funder_doi.neq_assign(funder_doi) + if self.funder_doi.neq_assign(value.trim().to_owned()) { + // Check DOI is correctly formatted before updating structure. + // If no DOI was provided, no check is required. + if self.funder_doi.trim().is_empty() { + self.funder.funder_doi.neq_assign(None); + self.funder_doi_warning.clear(); + } else { + match self.funder_doi.parse::() { + Ok(result) => { + self.funder.funder_doi.neq_assign(Some(result)); + self.funder_doi_warning.clear(); + } + Err(err) => { + self.funder_doi_warning = err.to_string(); + } + }; + } + true + } else { + false + } } Msg::ChangeRoute(r) => { let route = Route::from(r); @@ -327,9 +366,11 @@ impl Component for FunderComponent { oninput=self.link.callback(|e: InputData| Msg::ChangeFunderName(e.value)) required=true /> - diff --git a/thoth-app/src/component/new_contributor.rs b/thoth-app/src/component/new_contributor.rs index 707fa5d2..99a707cf 100644 --- a/thoth-app/src/component/new_contributor.rs +++ b/thoth-app/src/component/new_contributor.rs @@ -1,4 +1,4 @@ -use thoth_api::contributor::model::{Contributor, Orcid}; +use thoth_api::contributor::model::{Contributor, Orcid, ORCID_DOMAIN}; use yew::html; use yew::prelude::*; use yew::ComponentLink; @@ -17,6 +17,7 @@ use crate::agent::notification_bus::NotificationStatus; use crate::agent::notification_bus::Request; use crate::component::utils::FormTextInput; use crate::component::utils::FormTextInputTooltip; +use crate::component::utils::FormTextInputTooltipStatic; use crate::component::utils::FormUrlInput; use crate::models::contributor::contributors_query::ContributorsRequest; use crate::models::contributor::contributors_query::ContributorsRequestBody; @@ -37,6 +38,9 @@ const MIN_FULLNAME_LEN: usize = 2; pub struct NewContributorComponent { contributor: Contributor, + // Track the user-entered ORCID string, which may not be validly formatted + orcid: String, + orcid_warning: String, push_contributor: PushCreateContributor, link: ComponentLink, router: RouteAgentDispatcher<()>, @@ -69,12 +73,16 @@ impl Component for NewContributorComponent { let router = RouteAgentDispatcher::new(); let notification_bus = NotificationBus::dispatcher(); let contributor: Contributor = Default::default(); + let orcid = Default::default(); + let orcid_warning = Default::default(); let show_duplicate_tooltip = false; let fetch_contributors = Default::default(); let contributors = Default::default(); NewContributorComponent { contributor, + orcid, + orcid_warning, push_contributor, link, router, @@ -201,24 +209,24 @@ impl Component for NewContributorComponent { } } Msg::ChangeOrcid(value) => { - // Check ORCID is correctly formatted before proceeding with save. - // If no ORCID was provided, no check is required. - let mut parsed_orcid = None; - let mut ok_to_save = true; - if !value.trim().is_empty() { - match value.parse::() { - Ok(result) => parsed_orcid = Some(result), - Err(err) => { - ok_to_save = false; - self.notification_bus.send(Request::NotificationBusMsg(( - err.to_string(), - NotificationStatus::Danger, - ))) - } + if self.orcid.neq_assign(value.trim().to_owned()) { + // Check ORCID is correctly formatted before updating structure. + // If no ORCID was provided, no check is required. + if self.orcid.trim().is_empty() { + self.contributor.orcid.neq_assign(None); + self.orcid_warning.clear(); + } else { + match self.orcid.parse::() { + Ok(result) => { + self.contributor.orcid.neq_assign(Some(result)); + self.orcid_warning.clear(); + } + Err(err) => { + self.orcid_warning = err.to_string(); + } + }; } - }; - if ok_to_save { - self.contributor.orcid.neq_assign(parsed_orcid) + true } else { false } @@ -290,9 +298,11 @@ impl Component for NewContributorComponent { onblur=self.link.callback(|_| Msg::ToggleDuplicateTooltip(false)) required=true /> - , router: RouteAgentDispatcher<()>, @@ -50,10 +54,14 @@ impl Component for NewFunderComponent { let push_funder = Default::default(); let notification_bus = NotificationBus::dispatcher(); let funder: Funder = Default::default(); + let funder_doi = Default::default(); + let funder_doi_warning = Default::default(); let router = RouteAgentDispatcher::new(); NewFunderComponent { funder, + funder_doi, + funder_doi_warning, push_funder, link, router, @@ -117,11 +125,27 @@ impl Component for NewFunderComponent { .funder_name .neq_assign(funder_name.trim().to_owned()), Msg::ChangeFunderDoi(value) => { - let funder_doi = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.funder.funder_doi.neq_assign(funder_doi) + if self.funder_doi.neq_assign(value.trim().to_owned()) { + // Check DOI is correctly formatted before updating structure. + // If no DOI was provided, no check is required. + if self.funder_doi.trim().is_empty() { + self.funder.funder_doi.neq_assign(None); + self.funder_doi_warning.clear(); + } else { + match self.funder_doi.parse::() { + Ok(result) => { + self.funder.funder_doi.neq_assign(Some(result)); + self.funder_doi_warning.clear(); + } + Err(err) => { + self.funder_doi_warning = err.to_string(); + } + }; + } + true + } else { + false + } } Msg::ChangeRoute(r) => { let route = Route::from(r); @@ -158,9 +182,11 @@ impl Component for NewFunderComponent { oninput=self.link.callback(|e: InputData| Msg::ChangeFunderName(e.value)) required=true /> - diff --git a/thoth-app/src/component/new_work.rs b/thoth-app/src/component/new_work.rs index d5e6a78d..19d1923f 100644 --- a/thoth-app/src/component/new_work.rs +++ b/thoth-app/src/component/new_work.rs @@ -4,6 +4,7 @@ use thoth_api::imprint::model::ImprintExtended as Imprint; use thoth_api::work::model::WorkExtended as Work; use thoth_api::work::model::WorkStatus; use thoth_api::work::model::WorkType; +use thoth_api::work::model::{Doi, DOI_DOMAIN}; use uuid::Uuid; use yew::html; use yew::prelude::*; @@ -25,6 +26,7 @@ use crate::component::utils::FormDateInput; use crate::component::utils::FormImprintSelect; use crate::component::utils::FormNumberInput; use crate::component::utils::FormTextInput; +use crate::component::utils::FormTextInputTooltipStatic; use crate::component::utils::FormTextarea; use crate::component::utils::FormUrlInput; use crate::component::utils::FormWorkStatusSelect; @@ -51,6 +53,10 @@ use crate::string::SAVE_BUTTON; pub struct NewWorkComponent { work: Work, + // Track the user-entered DOI string, which may not be validly formatted + doi: String, + doi_warning: String, + // Track imprint stored in database, as distinct from imprint selected in dropdown imprint_id: Uuid, push_work: PushCreateWork, data: WorkFormData, @@ -124,6 +130,8 @@ impl Component for NewWorkComponent { let router = RouteAgentDispatcher::new(); let notification_bus = NotificationBus::dispatcher(); let work: Work = Default::default(); + let doi = Default::default(); + let doi_warning = Default::default(); let imprint_id: Uuid = Default::default(); let data: WorkFormData = Default::default(); let fetch_imprints: FetchImprints = Default::default(); @@ -136,6 +144,8 @@ impl Component for NewWorkComponent { NewWorkComponent { work, + doi, + doi_warning, imprint_id, push_work, data, @@ -326,11 +336,27 @@ impl Component for NewWorkComponent { self.work.edition.neq_assign(edition) } Msg::ChangeDoi(value) => { - let doi = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.doi.neq_assign(doi) + if self.doi.neq_assign(value.trim().to_owned()) { + // Check DOI is correctly formatted before updating structure. + // If no DOI was provided, no check is required. + if self.doi.trim().is_empty() { + self.work.doi.neq_assign(None); + self.doi_warning.clear(); + } else { + match self.doi.parse::() { + Ok(result) => { + self.work.doi.neq_assign(Some(result)); + self.doi_warning.clear(); + } + Err(err) => { + self.doi_warning = err.to_string(); + } + }; + } + true + } else { + false + } } Msg::ChangeDate(date) => self.work.publication_date.neq_assign(Some(date)), Msg::ChangePlace(value) => { @@ -612,9 +638,11 @@ impl Component for NewWorkComponent {
- w.to_owned(), None => Default::default(), }; + // Initialise user-entered DOI variable to match DOI in database + self.doi = self.work.doi.clone().unwrap_or_default().to_string(); self.imprint_id = self.work.imprint.imprint_id; self.data.imprints = body.data.imprints.to_owned(); self.data.work_types = body.data.work_types.enum_values.to_owned(); @@ -224,11 +235,14 @@ impl Component for WorkComponent { FetchState::Fetching(_) => false, FetchState::Fetched(body) => match &body.data.update_work { Some(w) => { + // Save was successful: update user-entered DOI variable to match DOI in database + self.doi = self.work.doi.clone().unwrap_or_default().to_string(); + self.doi_warning.clear(); + self.imprint_id = self.work.imprint.imprint_id; self.notification_bus.send(Request::NotificationBusMsg(( format!("Saved {}", w.title), NotificationStatus::Success, ))); - self.imprint_id = self.work.imprint.imprint_id; true } None => { @@ -389,11 +403,27 @@ impl Component for WorkComponent { self.work.edition.neq_assign(edition) } Msg::ChangeDoi(value) => { - let doi = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.doi.neq_assign(doi) + if self.doi.neq_assign(value.trim().to_owned()) { + // Check DOI is correctly formatted before updating structure. + // If no DOI was provided, no check is required. + if self.doi.trim().is_empty() { + self.work.doi.neq_assign(None); + self.doi_warning.clear(); + } else { + match self.doi.parse::() { + Ok(result) => { + self.work.doi.neq_assign(Some(result)); + self.doi_warning.clear(); + } + Err(err) => { + self.doi_warning = err.to_string(); + } + }; + } + true + } else { + false + } } Msg::ChangeDate(date) => self.work.publication_date.neq_assign(Some(date)), Msg::ChangePlace(value) => { @@ -703,9 +733,11 @@ impl Component for WorkComponent {
- , + pub funder_doi: Option, } #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] diff --git a/thoth-app/src/models/funder/mod.rs b/thoth-app/src/models/funder/mod.rs index 042dc5f2..58bc00ef 100644 --- a/thoth-app/src/models/funder/mod.rs +++ b/thoth-app/src/models/funder/mod.rs @@ -24,7 +24,11 @@ impl EditRoute for Funder { impl MetadataTable for Funder { fn as_table_row(&self, callback: Callback) -> Html { - let funder_doi = self.funder_doi.clone().unwrap_or_else(|| "".to_string()); + let funder_doi = self + .funder_doi + .as_ref() + .map(|s| s.to_string()) + .unwrap_or_else(|| "".to_string()); html! { Html { - let doi = self.doi.clone().unwrap_or_else(|| "".to_string()); + let doi = self + .doi + .as_ref() + .map(|s| s.to_string()) + .unwrap_or_else(|| "".to_string()); let cover_url = self .cover_url .clone() diff --git a/thoth-app/src/models/work/update_work_mutation.rs b/thoth-app/src/models/work/update_work_mutation.rs index 359448d9..8619af47 100644 --- a/thoth-app/src/models/work/update_work_mutation.rs +++ b/thoth-app/src/models/work/update_work_mutation.rs @@ -1,5 +1,6 @@ use serde::Deserialize; use serde::Serialize; +use thoth_api::work::model::Doi; use thoth_api::work::model::WorkStatus; use thoth_api::work::model::WorkType; use uuid::Uuid; @@ -15,7 +16,7 @@ const UPDATE_WORK_MUTATION: &str = " $reference: String, $edition: Int!, $imprintId: Uuid!, - $doi: String, + $doi: Doi, $publicationDate: NaiveDate, $place: String, $width: Int, @@ -99,7 +100,7 @@ pub struct Variables { pub subtitle: Option, pub reference: Option, pub edition: i32, - pub doi: Option, + pub doi: Option, pub publication_date: Option, pub place: Option, pub width: Option, From bfbac5cf996940ade0373b540f862d0b1cee0120 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Wed, 2 Jun 2021 10:56:55 +0100 Subject: [PATCH 09/19] Use diesel-derive-newtype to create Timestamp wrapper type allowing derive of defaults --- thoth-api/src/account/model.rs | 15 ++-- thoth-api/src/contribution/model.rs | 30 ++----- thoth-api/src/contributor/model.rs | 28 ++----- thoth-api/src/funder/model.rs | 25 ++---- thoth-api/src/funding/crud.rs | 17 ---- thoth-api/src/funding/model.rs | 11 ++- thoth-api/src/graphql/model.rs | 107 ++++++++++++------------ thoth-api/src/imprint/crud.rs | 13 --- thoth-api/src/imprint/model.rs | 29 ++----- thoth-api/src/issue/crud.rs | 13 --- thoth-api/src/issue/model.rs | 11 ++- thoth-api/src/language/model.rs | 13 ++- thoth-api/src/model/mod.rs | 42 ++++++++++ thoth-api/src/price/model.rs | 24 ++---- thoth-api/src/publication/crud.rs | 14 ---- thoth-api/src/publication/model.rs | 47 ++--------- thoth-api/src/publisher/model.rs | 26 ++---- thoth-api/src/series/crud.rs | 16 ---- thoth-api/src/series/model.rs | 32 ++----- thoth-api/src/subject/model.rs | 15 ++-- thoth-api/src/work/crud.rs | 40 --------- thoth-api/src/work/model.rs | 85 ++++++++++--------- thoth-app/src/models/contributor/mod.rs | 2 +- thoth-app/src/models/funder/mod.rs | 2 +- thoth-app/src/models/imprint/mod.rs | 2 +- thoth-app/src/models/mod.rs | 2 +- thoth-app/src/models/publication/mod.rs | 2 +- thoth-app/src/models/publisher/mod.rs | 2 +- thoth-app/src/models/series/mod.rs | 2 +- thoth-app/src/models/work/mod.rs | 2 +- 30 files changed, 228 insertions(+), 441 deletions(-) diff --git a/thoth-api/src/account/model.rs b/thoth-api/src/account/model.rs index 95ed0dcc..72ee10c6 100644 --- a/thoth-api/src/account/model.rs +++ b/thoth-api/src/account/model.rs @@ -1,10 +1,9 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::errors::ThothError; use crate::errors::ThothResult; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::account; #[cfg(feature = "backend")] @@ -22,8 +21,8 @@ pub struct Account { pub is_superuser: bool, pub is_bot: bool, pub is_active: bool, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, pub token: Option, } @@ -54,8 +53,8 @@ pub struct PublisherAccount { pub account_id: Uuid, pub publisher_id: Uuid, pub is_admin: bool, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[cfg_attr(feature = "backend", derive(Insertable))] @@ -99,8 +98,8 @@ pub struct AccountDetails { pub surname: String, pub email: String, pub token: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, pub resource_access: AccountAccess, } diff --git a/thoth-api/src/contribution/model.rs b/thoth-api/src/contribution/model.rs index 0b2bc877..46a35522 100644 --- a/thoth-api/src/contribution/model.rs +++ b/thoth-api/src/contribution/model.rs @@ -1,10 +1,9 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use strum::Display; use strum::EnumString; use uuid::Uuid; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::contribution; #[cfg(feature = "backend")] @@ -54,7 +53,7 @@ pub enum ContributionField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct Contribution { pub contribution_id: Uuid, @@ -64,8 +63,8 @@ pub struct Contribution { pub main_contribution: bool, pub biography: Option, pub institution: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, pub first_name: Option, pub last_name: String, pub full_name: String, @@ -113,7 +112,7 @@ pub struct ContributionHistory { pub contribution_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr( @@ -133,25 +132,6 @@ impl Default for ContributionType { } } -impl Default for Contribution { - fn default() -> Contribution { - Contribution { - contribution_id: Default::default(), - work_id: Default::default(), - contributor_id: Default::default(), - contribution_type: Default::default(), - main_contribution: Default::default(), - biography: None, - institution: None, - created_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - first_name: None, - last_name: Default::default(), - full_name: Default::default(), - } - } -} - #[test] fn test_contributiontype_default() { let contributiontype: ContributionType = Default::default(); diff --git a/thoth-api/src/contributor/model.rs b/thoth-api/src/contributor/model.rs index f3fd281d..f2026a55 100644 --- a/thoth-api/src/contributor/model.rs +++ b/thoth-api/src/contributor/model.rs @@ -1,5 +1,3 @@ -use chrono::DateTime; -use chrono::Utc; use serde::Deserialize; use serde::Serialize; use std::fmt; @@ -10,6 +8,7 @@ use uuid::Uuid; use crate::errors::{ThothError, ThothResult}; use crate::graphql::utils::Direction; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::contributor; #[cfg(feature = "backend")] @@ -46,7 +45,7 @@ pub struct Orcid(String); pub const ORCID_DOMAIN: &str = "https://orcid.org/"; #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct Contributor { pub contributor_id: Uuid, @@ -55,8 +54,8 @@ pub struct Contributor { pub full_name: String, pub orcid: Option, pub website: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[cfg_attr( @@ -93,7 +92,7 @@ pub struct ContributorHistory { pub contributor_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr( @@ -112,7 +111,7 @@ pub struct NewContributorHistory { derive(juniper::GraphQLInputObject), graphql(description = "Field and order to use when sorting contributors list") )] -#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] pub struct ContributorOrderBy { pub field: ContributorField, pub direction: Direction, @@ -124,21 +123,6 @@ impl Default for ContributorField { } } -impl Default for Contributor { - fn default() -> Contributor { - Contributor { - contributor_id: Default::default(), - first_name: None, - last_name: "".to_string(), - full_name: "".to_string(), - orcid: None, - website: None, - created_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - } - } -} - impl fmt::Display for Contributor { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { if let Some(orcid) = &self.orcid { diff --git a/thoth-api/src/funder/model.rs b/thoth-api/src/funder/model.rs index c23e2119..8dd7177e 100644 --- a/thoth-api/src/funder/model.rs +++ b/thoth-api/src/funder/model.rs @@ -1,5 +1,3 @@ -use chrono::DateTime; -use chrono::Utc; use serde::Deserialize; use serde::Serialize; use std::fmt; @@ -8,6 +6,7 @@ use strum::EnumString; use uuid::Uuid; use crate::graphql::utils::Direction; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::funder; #[cfg(feature = "backend")] @@ -33,14 +32,14 @@ pub enum FunderField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct Funder { pub funder_id: Uuid, pub funder_name: String, pub funder_doi: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[cfg_attr( @@ -71,7 +70,7 @@ pub struct FunderHistory { pub funder_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr(feature = "backend", derive(Insertable), table_name = "funder_history")] @@ -86,7 +85,7 @@ pub struct NewFunderHistory { derive(juniper::GraphQLInputObject), graphql(description = "Field and order to use when sorting funders list") )] -#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] pub struct FunderOrderBy { pub field: FunderField, pub direction: Direction, @@ -98,18 +97,6 @@ impl Default for FunderField { } } -impl Default for Funder { - fn default() -> Funder { - Funder { - funder_id: Default::default(), - funder_name: "".to_string(), - funder_doi: None, - created_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - } - } -} - impl fmt::Display for Funder { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { if let Some(doi) = &self.funder_doi { diff --git a/thoth-api/src/funding/crud.rs b/thoth-api/src/funding/crud.rs index e2158e34..d9f42c28 100644 --- a/thoth-api/src/funding/crud.rs +++ b/thoth-api/src/funding/crud.rs @@ -159,23 +159,6 @@ impl DbInsert for NewFundingHistory { mod tests { use super::*; - impl Default for Funding { - fn default() -> Self { - Funding { - funding_id: Default::default(), - work_id: Default::default(), - funder_id: Default::default(), - program: Default::default(), - project_name: Default::default(), - project_shortname: Default::default(), - grant_number: Default::default(), - jurisdiction: Default::default(), - created_at: chrono::Utc::now(), - updated_at: chrono::Utc::now(), - } - } - } - #[test] fn test_funding_pk() { let funding: Funding = Default::default(); diff --git a/thoth-api/src/funding/model.rs b/thoth-api/src/funding/model.rs index 8acab346..31567ff6 100644 --- a/thoth-api/src/funding/model.rs +++ b/thoth-api/src/funding/model.rs @@ -1,9 +1,8 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::funder::model::Funder; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::funding; #[cfg(feature = "backend")] @@ -28,7 +27,7 @@ pub enum FundingField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Serialize, Deserialize)] +#[derive(Default, Serialize, Deserialize)] pub struct Funding { pub funding_id: Uuid, pub work_id: Uuid, @@ -38,8 +37,8 @@ pub struct Funding { pub project_shortname: Option, pub grant_number: Option, pub jurisdiction: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] @@ -94,7 +93,7 @@ pub struct FundingHistory { pub funding_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr( diff --git a/thoth-api/src/graphql/model.rs b/thoth-api/src/graphql/model.rs index dd81fbee..29c3de14 100644 --- a/thoth-api/src/graphql/model.rs +++ b/thoth-api/src/graphql/model.rs @@ -1,6 +1,4 @@ use chrono::naive::NaiveDate; -use chrono::DateTime; -use chrono::Utc; use diesel::prelude::*; use juniper::FieldResult; use juniper::RootNode; @@ -20,6 +18,7 @@ use crate::imprint::model::*; use crate::issue::model::*; use crate::language::model::*; use crate::model::Crud; +use crate::model::Timestamp; use crate::price::model::*; use crate::publication::model::*; use crate::publisher::model::*; @@ -1457,12 +1456,12 @@ impl Work { self.cover_caption.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn imprint(&self, context: &Context) -> FieldResult { @@ -1740,12 +1739,12 @@ impl Publication { self.publication_url.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } #[graphql( @@ -1811,12 +1810,12 @@ impl Publisher { self.publisher_url.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } #[graphql( @@ -1877,12 +1876,12 @@ impl Imprint { self.imprint_url.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn publisher(&self, context: &Context) -> FieldResult { @@ -1968,12 +1967,12 @@ impl Contributor { self.website.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } #[graphql( @@ -2047,12 +2046,12 @@ impl Contribution { self.institution.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn first_name(&self) -> Option<&String> { @@ -2102,12 +2101,12 @@ impl Series { self.series_url.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn imprint(&self, context: &Context) -> FieldResult { @@ -2171,12 +2170,12 @@ impl Issue { &self.issue_ordinal } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn series(&self, context: &Context) -> FieldResult { @@ -2210,12 +2209,12 @@ impl Language { self.main_language } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn work(&self, context: &Context) -> FieldResult { @@ -2241,12 +2240,12 @@ impl Price { self.unit_price } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn publication(&self, context: &Context) -> FieldResult { @@ -2276,12 +2275,12 @@ impl Subject { &self.subject_ordinal } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn work(&self, context: &Context) -> FieldResult { @@ -2303,12 +2302,12 @@ impl Funder { self.funder_doi.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } #[graphql( @@ -2384,12 +2383,12 @@ impl Funding { self.jurisdiction.as_ref() } - pub fn created_at(&self) -> DateTime { - self.created_at + pub fn created_at(&self) -> Timestamp { + self.created_at.clone() } - pub fn updated_at(&self) -> DateTime { - self.updated_at + pub fn updated_at(&self) -> Timestamp { + self.updated_at.clone() } pub fn work(&self, context: &Context) -> FieldResult { diff --git a/thoth-api/src/imprint/crud.rs b/thoth-api/src/imprint/crud.rs index acf05bb4..f487ea18 100644 --- a/thoth-api/src/imprint/crud.rs +++ b/thoth-api/src/imprint/crud.rs @@ -145,19 +145,6 @@ impl DbInsert for NewImprintHistory { mod tests { use super::*; - impl Default for Imprint { - fn default() -> Self { - Imprint { - imprint_id: Default::default(), - publisher_id: Default::default(), - imprint_name: Default::default(), - imprint_url: Default::default(), - created_at: chrono::Utc::now(), - updated_at: chrono::Utc::now(), - } - } - } - #[test] fn test_imprint_pk() { let imprint: Imprint = Default::default(); diff --git a/thoth-api/src/imprint/model.rs b/thoth-api/src/imprint/model.rs index b00c9c29..e51ee871 100644 --- a/thoth-api/src/imprint/model.rs +++ b/thoth-api/src/imprint/model.rs @@ -1,5 +1,3 @@ -use chrono::DateTime; -use chrono::Utc; use serde::Deserialize; use serde::Serialize; use strum::Display; @@ -7,6 +5,7 @@ use strum::EnumString; use uuid::Uuid; use crate::graphql::utils::Direction; +use crate::model::Timestamp; use crate::publisher::model::Publisher; #[cfg(feature = "backend")] use crate::schema::imprint; @@ -32,23 +31,23 @@ pub enum ImprintField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Serialize, Deserialize)] +#[derive(Default, Serialize, Deserialize)] pub struct Imprint { pub imprint_id: Uuid, pub publisher_id: Uuid, pub imprint_name: String, pub imprint_url: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct ImprintExtended { pub imprint_id: Uuid, pub imprint_name: String, pub imprint_url: Option, - pub updated_at: DateTime, + pub updated_at: Timestamp, pub publisher: Publisher, } @@ -82,7 +81,7 @@ pub struct ImprintHistory { pub imprint_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr( @@ -101,7 +100,7 @@ pub struct NewImprintHistory { derive(juniper::GraphQLInputObject), graphql(description = "Field and order to use when sorting imprints list") )] -#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] pub struct ImprintOrderBy { pub field: ImprintField, pub direction: Direction, @@ -113,18 +112,6 @@ impl Default for ImprintField { } } -impl Default for ImprintExtended { - fn default() -> ImprintExtended { - ImprintExtended { - imprint_id: Default::default(), - imprint_name: "".to_string(), - imprint_url: None, - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - publisher: Default::default(), - } - } -} - #[test] fn test_imprintfield_default() { let impfield: ImprintField = Default::default(); diff --git a/thoth-api/src/issue/crud.rs b/thoth-api/src/issue/crud.rs index e1049e13..11409cca 100644 --- a/thoth-api/src/issue/crud.rs +++ b/thoth-api/src/issue/crud.rs @@ -137,19 +137,6 @@ impl DbInsert for NewIssueHistory { mod tests { use super::*; - impl Default for Issue { - fn default() -> Self { - Issue { - issue_id: Default::default(), - series_id: Default::default(), - work_id: Default::default(), - issue_ordinal: Default::default(), - created_at: chrono::Utc::now(), - updated_at: chrono::Utc::now(), - } - } - } - #[test] fn test_issue_pk() { let issue: Issue = Default::default(); diff --git a/thoth-api/src/issue/model.rs b/thoth-api/src/issue/model.rs index 02505a9f..5d4ed672 100644 --- a/thoth-api/src/issue/model.rs +++ b/thoth-api/src/issue/model.rs @@ -1,8 +1,7 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use uuid::Uuid; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::issue; #[cfg(feature = "backend")] @@ -24,14 +23,14 @@ pub enum IssueField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Serialize, Deserialize)] +#[derive(Default, Serialize, Deserialize)] pub struct Issue { pub issue_id: Uuid, pub series_id: Uuid, pub work_id: Uuid, pub issue_ordinal: i32, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -74,7 +73,7 @@ pub struct IssueHistory { pub issue_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr(feature = "backend", derive(Insertable), table_name = "issue_history")] diff --git a/thoth-api/src/language/model.rs b/thoth-api/src/language/model.rs index 6e802e57..074329cb 100644 --- a/thoth-api/src/language/model.rs +++ b/thoth-api/src/language/model.rs @@ -1,10 +1,9 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use strum::Display; use strum::EnumString; use uuid::Uuid; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::language; #[cfg(feature = "backend")] @@ -47,8 +46,8 @@ pub struct Language { pub language_code: LanguageCode, pub language_relation: LanguageRelation, pub main_language: bool, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[cfg_attr( @@ -578,7 +577,7 @@ pub struct LanguageHistory { pub language_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr( @@ -612,8 +611,8 @@ impl Default for Language { language_code: Default::default(), language_relation: Default::default(), main_language: true, - created_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), + created_at: Default::default(), + updated_at: Default::default(), } } } diff --git a/thoth-api/src/model/mod.rs b/thoth-api/src/model/mod.rs index 7568c64a..352b2aeb 100644 --- a/thoth-api/src/model/mod.rs +++ b/thoth-api/src/model/mod.rs @@ -1,6 +1,48 @@ +use chrono::{DateTime, TimeZone, Utc}; +use serde::{Deserialize, Serialize}; +use std::fmt; + #[cfg(feature = "backend")] use crate::errors::ThothResult; +#[cfg_attr( + feature = "backend", + derive(DieselNewType, juniper::GraphQLScalarValue) +)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct Timestamp(DateTime); + +impl Default for Timestamp { + fn default() -> Timestamp { + Timestamp { + 0: TimeZone::timestamp(&Utc, 0, 0), + } + } +} + +impl fmt::Display for Timestamp { + fn fmt(&self, f: &mut std::fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.0.format("%F %T")) + } +} + +#[test] +fn test_timestamp_default() { + let stamp: Timestamp = Default::default(); + assert_eq!( + stamp, + Timestamp { + 0: TimeZone::timestamp(&Utc, 0, 0) + } + ); +} + +#[test] +fn test_timestamp_display() { + let stamp: Timestamp = Default::default(); + assert_eq!(format!("{}", stamp), "1970-01-01 00:00:00"); +} + #[cfg(feature = "backend")] #[allow(clippy::too_many_arguments)] /// Common functionality to perform basic CRUD actions on Thoth entities diff --git a/thoth-api/src/price/model.rs b/thoth-api/src/price/model.rs index 3788e94d..b082230d 100644 --- a/thoth-api/src/price/model.rs +++ b/thoth-api/src/price/model.rs @@ -1,10 +1,9 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use strum::Display; use strum::EnumString; use uuid::Uuid; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::price; #[cfg(feature = "backend")] @@ -25,15 +24,15 @@ pub enum PriceField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct Price { pub price_id: Uuid, pub publication_id: Uuid, pub currency_code: CurrencyCode, pub unit_price: f64, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[cfg_attr( @@ -376,7 +375,7 @@ pub struct PriceHistory { pub price_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr(feature = "backend", derive(Insertable), table_name = "price_history")] @@ -386,19 +385,6 @@ pub struct NewPriceHistory { pub data: serde_json::Value, } -impl Default for Price { - fn default() -> Price { - Price { - price_id: Default::default(), - publication_id: Default::default(), - currency_code: Default::default(), - unit_price: 0.00, - created_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - } - } -} - impl Default for CurrencyCode { fn default() -> CurrencyCode { CurrencyCode::Gbp diff --git a/thoth-api/src/publication/crud.rs b/thoth-api/src/publication/crud.rs index 561e3331..e67f5038 100644 --- a/thoth-api/src/publication/crud.rs +++ b/thoth-api/src/publication/crud.rs @@ -187,20 +187,6 @@ impl DbInsert for NewPublicationHistory { mod tests { use super::*; - impl Default for Publication { - fn default() -> Self { - Publication { - publication_id: Default::default(), - publication_type: Default::default(), - work_id: Default::default(), - isbn: Default::default(), - publication_url: Default::default(), - created_at: chrono::Utc::now(), - updated_at: chrono::Utc::now(), - } - } - } - #[test] fn test_publication_pk() { let publication: Publication = Default::default(); diff --git a/thoth-api/src/publication/model.rs b/thoth-api/src/publication/model.rs index 33c29259..7dd91cd2 100644 --- a/thoth-api/src/publication/model.rs +++ b/thoth-api/src/publication/model.rs @@ -1,11 +1,10 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use strum::Display; use strum::EnumString; use uuid::Uuid; use crate::graphql::utils::Direction; +use crate::model::Timestamp; use crate::price::model::Price; #[cfg(feature = "backend")] use crate::schema::publication; @@ -60,18 +59,18 @@ pub enum PublicationField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Serialize, Deserialize)] +#[derive(Default, Serialize, Deserialize)] pub struct Publication { pub publication_id: Uuid, pub publication_type: PublicationType, pub work_id: Uuid, pub isbn: Option, pub publication_url: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct DetailedPublication { pub publication_id: Uuid, @@ -79,11 +78,11 @@ pub struct DetailedPublication { pub work_id: Uuid, pub isbn: Option, pub publication_url: Option, - pub updated_at: DateTime, + pub updated_at: Timestamp, pub work: Work, } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct PublicationExtended { pub publication_id: Uuid, @@ -145,7 +144,7 @@ pub struct PublicationHistory { pub publication_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr( @@ -164,7 +163,7 @@ pub struct NewPublicationHistory { derive(juniper::GraphQLInputObject), graphql(description = "Field and order to use when sorting publications list") )] -#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] pub struct PublicationOrderBy { pub field: PublicationField, pub direction: Direction, @@ -182,34 +181,6 @@ impl Default for PublicationField { } } -impl Default for DetailedPublication { - fn default() -> DetailedPublication { - DetailedPublication { - publication_id: Default::default(), - publication_type: Default::default(), - work_id: Default::default(), - isbn: None, - publication_url: None, - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - work: Default::default(), - } - } -} - -impl Default for PublicationExtended { - fn default() -> PublicationExtended { - PublicationExtended { - publication_id: Default::default(), - publication_type: PublicationType::Paperback, - work_id: Default::default(), - isbn: None, - publication_url: None, - prices: Default::default(), - work: Default::default(), - } - } -} - #[test] fn test_publicationtype_default() { let pubtype: PublicationType = Default::default(); diff --git a/thoth-api/src/publisher/model.rs b/thoth-api/src/publisher/model.rs index cf9035df..4360f4ae 100644 --- a/thoth-api/src/publisher/model.rs +++ b/thoth-api/src/publisher/model.rs @@ -1,5 +1,3 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use std::fmt; use strum::Display; @@ -7,6 +5,7 @@ use strum::EnumString; use uuid::Uuid; use crate::graphql::utils::Direction; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::publisher; #[cfg(feature = "backend")] @@ -33,15 +32,15 @@ pub enum PublisherField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct Publisher { pub publisher_id: Uuid, pub publisher_name: String, pub publisher_shortname: Option, pub publisher_url: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[cfg_attr( @@ -74,7 +73,7 @@ pub struct PublisherHistory { pub publisher_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr( @@ -93,7 +92,7 @@ pub struct NewPublisherHistory { derive(juniper::GraphQLInputObject), graphql(description = "Field and order to use when sorting publishers list") )] -#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] pub struct PublisherOrderBy { pub field: PublisherField, pub direction: Direction, @@ -105,19 +104,6 @@ impl Default for PublisherField { } } -impl Default for Publisher { - fn default() -> Publisher { - Publisher { - publisher_id: Default::default(), - publisher_name: "".to_string(), - publisher_shortname: None, - publisher_url: None, - created_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - } - } -} - impl fmt::Display for Publisher { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.publisher_name) diff --git a/thoth-api/src/series/crud.rs b/thoth-api/src/series/crud.rs index e8e950df..5b20642c 100644 --- a/thoth-api/src/series/crud.rs +++ b/thoth-api/src/series/crud.rs @@ -190,22 +190,6 @@ impl DbInsert for NewSeriesHistory { mod tests { use super::*; - impl Default for Series { - fn default() -> Self { - Series { - series_id: Default::default(), - series_type: Default::default(), - series_name: Default::default(), - issn_print: Default::default(), - issn_digital: Default::default(), - series_url: Default::default(), - imprint_id: Default::default(), - created_at: chrono::Utc::now(), - updated_at: chrono::Utc::now(), - } - } - } - #[test] fn test_series_pk() { let series: Series = Default::default(); diff --git a/thoth-api/src/series/model.rs b/thoth-api/src/series/model.rs index fcea1b12..0cde557e 100644 --- a/thoth-api/src/series/model.rs +++ b/thoth-api/src/series/model.rs @@ -1,5 +1,3 @@ -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use std::fmt; use strum::Display; @@ -8,6 +6,7 @@ use uuid::Uuid; use crate::graphql::utils::Direction; use crate::imprint::model::ImprintExtended as Imprint; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::series; #[cfg(feature = "backend")] @@ -48,7 +47,7 @@ pub enum SeriesField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Serialize, Deserialize)] +#[derive(Default, Serialize, Deserialize)] pub struct Series { pub series_id: Uuid, pub series_type: SeriesType, @@ -57,11 +56,11 @@ pub struct Series { pub issn_digital: String, pub series_url: Option, pub imprint_id: Uuid, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct SeriesExtended { pub series_id: Uuid, @@ -70,7 +69,7 @@ pub struct SeriesExtended { pub issn_print: String, pub issn_digital: String, pub series_url: Option, - pub updated_at: DateTime, + pub updated_at: Timestamp, pub imprint: Imprint, } @@ -110,7 +109,7 @@ pub struct SeriesHistory { pub series_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr(feature = "backend", derive(Insertable), table_name = "series_history")] @@ -125,7 +124,7 @@ pub struct NewSeriesHistory { derive(juniper::GraphQLInputObject), graphql(description = "Field and order to use when sorting seriess list") )] -#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] pub struct SeriesOrderBy { pub field: SeriesField, pub direction: Direction, @@ -143,21 +142,6 @@ impl Default for SeriesField { } } -impl Default for SeriesExtended { - fn default() -> SeriesExtended { - SeriesExtended { - series_id: Default::default(), - series_type: Default::default(), - series_name: "".to_string(), - issn_print: "".to_string(), - issn_digital: "".to_string(), - series_url: None, - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - imprint: Default::default(), - } - } -} - impl fmt::Display for SeriesExtended { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!( diff --git a/thoth-api/src/subject/model.rs b/thoth-api/src/subject/model.rs index d965d592..5ba9f6ec 100644 --- a/thoth-api/src/subject/model.rs +++ b/thoth-api/src/subject/model.rs @@ -1,5 +1,3 @@ -use chrono::DateTime; -use chrono::Utc; use phf::phf_map; use phf::Map; use serde::{Deserialize, Serialize}; @@ -9,6 +7,7 @@ use uuid::Uuid; use crate::errors::ThothError; use crate::errors::ThothResult; +use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::subject; #[cfg(feature = "backend")] @@ -54,8 +53,8 @@ pub struct Subject { pub subject_type: SubjectType, pub subject_code: String, pub subject_ordinal: i32, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[cfg_attr( @@ -90,7 +89,7 @@ pub struct SubjectHistory { pub subject_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr( @@ -135,10 +134,10 @@ impl Default for Subject { subject_id: Default::default(), work_id: Default::default(), subject_type: Default::default(), - subject_code: "".to_string(), + subject_code: Default::default(), subject_ordinal: 1, - created_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), + created_at: Default::default(), + updated_at: Default::default(), } } } diff --git a/thoth-api/src/work/crud.rs b/thoth-api/src/work/crud.rs index 213f66ff..bf27536f 100644 --- a/thoth-api/src/work/crud.rs +++ b/thoth-api/src/work/crud.rs @@ -347,46 +347,6 @@ impl DbInsert for NewWorkHistory { mod tests { use super::*; - impl Default for Work { - fn default() -> Self { - Work { - work_id: Default::default(), - work_type: Default::default(), - work_status: Default::default(), - full_title: Default::default(), - title: Default::default(), - subtitle: Default::default(), - reference: Default::default(), - edition: Default::default(), - imprint_id: Default::default(), - doi: Default::default(), - publication_date: Default::default(), - place: Default::default(), - width: Default::default(), - height: Default::default(), - page_count: Default::default(), - page_breakdown: Default::default(), - image_count: Default::default(), - table_count: Default::default(), - audio_count: Default::default(), - video_count: Default::default(), - license: Default::default(), - copyright_holder: Default::default(), - landing_page: Default::default(), - lccn: Default::default(), - oclc: Default::default(), - short_abstract: Default::default(), - long_abstract: Default::default(), - general_note: Default::default(), - toc: Default::default(), - cover_url: Default::default(), - cover_caption: Default::default(), - created_at: chrono::Utc::now(), - updated_at: chrono::Utc::now(), - } - } - } - #[test] fn test_work_pk() { let work: Work = Default::default(); diff --git a/thoth-api/src/work/model.rs b/thoth-api/src/work/model.rs index c9b455c1..68981a49 100644 --- a/thoth-api/src/work/model.rs +++ b/thoth-api/src/work/model.rs @@ -1,6 +1,4 @@ use chrono::naive::NaiveDate; -use chrono::DateTime; -use chrono::Utc; use serde::{Deserialize, Serialize}; use std::fmt; use std::str::FromStr; @@ -15,6 +13,7 @@ use crate::graphql::utils::Direction; use crate::imprint::model::ImprintExtended as Imprint; use crate::issue::model::IssueExtended as Issue; use crate::language::model::Language; +use crate::model::Timestamp; use crate::publication::model::PublicationExtended as Publication; #[cfg(feature = "backend")] use crate::schema::work; @@ -127,7 +126,7 @@ pub struct Doi(String); pub const DOI_DOMAIN: &str = "https://doi.org/"; #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Serialize, Deserialize)] +#[derive(Default, Serialize, Deserialize)] pub struct Work { pub work_id: Uuid, pub work_type: WorkType, @@ -160,8 +159,8 @@ pub struct Work { pub toc: Option, pub cover_url: Option, pub cover_caption: Option, - pub created_at: DateTime, - pub updated_at: DateTime, + pub created_at: Timestamp, + pub updated_at: Timestamp, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -197,7 +196,7 @@ pub struct WorkExtended { pub toc: Option, pub cover_url: Option, pub cover_caption: Option, - pub updated_at: DateTime, + pub updated_at: Timestamp, pub contributions: Option>, pub publications: Option>, pub languages: Option>, @@ -291,7 +290,7 @@ pub struct WorkHistory { pub work_id: Uuid, pub account_id: Uuid, pub data: serde_json::Value, - pub timestamp: DateTime, + pub timestamp: Timestamp, } #[cfg_attr(feature = "backend", derive(Insertable), table_name = "work_history")] @@ -306,7 +305,7 @@ pub struct NewWorkHistory { derive(juniper::GraphQLInputObject), graphql(description = "Field and order to use when sorting works list") )] -#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] pub struct WorkOrderBy { pub field: WorkField, pub direction: Direction, @@ -352,42 +351,42 @@ impl Default for WorkExtended { fn default() -> WorkExtended { WorkExtended { work_id: Default::default(), - work_type: WorkType::Monograph, - work_status: WorkStatus::Inactive, - full_title: "".to_string(), - title: "".to_string(), - subtitle: None, - reference: None, + work_type: Default::default(), + work_status: Default::default(), + full_title: Default::default(), + title: Default::default(), + subtitle: Default::default(), + reference: Default::default(), edition: 1, - doi: None, - publication_date: None, - place: None, - width: None, - height: None, - page_count: None, - page_breakdown: None, - image_count: None, - table_count: None, - audio_count: None, - video_count: None, - license: None, - copyright_holder: "".to_string(), - landing_page: None, - lccn: None, - oclc: None, - short_abstract: None, - long_abstract: None, - general_note: None, - toc: None, - cover_url: None, - cover_caption: None, - updated_at: chrono::TimeZone::timestamp(&Utc, 0, 0), - contributions: None, - publications: None, - languages: None, - fundings: None, - subjects: None, - issues: None, + doi: Default::default(), + publication_date: Default::default(), + place: Default::default(), + width: Default::default(), + height: Default::default(), + page_count: Default::default(), + page_breakdown: Default::default(), + image_count: Default::default(), + table_count: Default::default(), + audio_count: Default::default(), + video_count: Default::default(), + license: Default::default(), + copyright_holder: Default::default(), + landing_page: Default::default(), + lccn: Default::default(), + oclc: Default::default(), + short_abstract: Default::default(), + long_abstract: Default::default(), + general_note: Default::default(), + toc: Default::default(), + cover_url: Default::default(), + cover_caption: Default::default(), + updated_at: Default::default(), + contributions: Default::default(), + publications: Default::default(), + languages: Default::default(), + fundings: Default::default(), + subjects: Default::default(), + issues: Default::default(), imprint: Default::default(), } } diff --git a/thoth-app/src/models/contributor/mod.rs b/thoth-app/src/models/contributor/mod.rs index e01f558b..cf5eb32e 100644 --- a/thoth-app/src/models/contributor/mod.rs +++ b/thoth-app/src/models/contributor/mod.rs @@ -37,7 +37,7 @@ impl MetadataTable for Contributor { {&self.contributor_id} {&self.full_name} {orcid} - {&self.updated_at.format("%F %T")} + {&self.updated_at} } } diff --git a/thoth-app/src/models/funder/mod.rs b/thoth-app/src/models/funder/mod.rs index 58bc00ef..ffb6b87b 100644 --- a/thoth-app/src/models/funder/mod.rs +++ b/thoth-app/src/models/funder/mod.rs @@ -37,7 +37,7 @@ impl MetadataTable for Funder { {&self.funder_id} {&self.funder_name} {funder_doi} - {&self.updated_at.format("%F %T")} + {&self.updated_at} } } diff --git a/thoth-app/src/models/imprint/mod.rs b/thoth-app/src/models/imprint/mod.rs index 73c64ed5..4a15b6d5 100644 --- a/thoth-app/src/models/imprint/mod.rs +++ b/thoth-app/src/models/imprint/mod.rs @@ -32,7 +32,7 @@ impl MetadataTable for Imprint { {&self.imprint_name} {&self.publisher.publisher_name} {imprint_url} - {&self.updated_at.format("%F %T")} + {&self.updated_at} } } diff --git a/thoth-app/src/models/mod.rs b/thoth-app/src/models/mod.rs index bd275eec..92100794 100644 --- a/thoth-app/src/models/mod.rs +++ b/thoth-app/src/models/mod.rs @@ -21,7 +21,7 @@ macro_rules! graphql_query_builder { pub type $fetch = Fetch<$request, $response_body>; pub type $fetch_action = FetchAction<$response_body>; - #[derive(Default, Debug, Clone)] + #[derive(Debug, Clone, Default)] pub struct $request { pub body: $request_body, } diff --git a/thoth-app/src/models/publication/mod.rs b/thoth-app/src/models/publication/mod.rs index 36d83b08..fb6a4c3c 100644 --- a/thoth-app/src/models/publication/mod.rs +++ b/thoth-app/src/models/publication/mod.rs @@ -49,7 +49,7 @@ impl MetadataTable for Publication { {&self.publication_type} {isbn} {publication_url} - {&self.updated_at.format("%F %T")} + {&self.updated_at} } } diff --git a/thoth-app/src/models/publisher/mod.rs b/thoth-app/src/models/publisher/mod.rs index decdd5bb..5edb94d5 100644 --- a/thoth-app/src/models/publisher/mod.rs +++ b/thoth-app/src/models/publisher/mod.rs @@ -37,7 +37,7 @@ impl MetadataTable for Publisher { {&self.publisher_name} {publisher_shortname} {publisher_url} - {&self.updated_at.format("%F %T")} + {&self.updated_at} } } diff --git a/thoth-app/src/models/series/mod.rs b/thoth-app/src/models/series/mod.rs index 86c2321b..885585d2 100644 --- a/thoth-app/src/models/series/mod.rs +++ b/thoth-app/src/models/series/mod.rs @@ -49,7 +49,7 @@ impl MetadataTable for Series { {&self.series_type} {&self.issn_print} {&self.issn_digital} - {&self.updated_at.format("%F %T")} + {&self.updated_at} } } diff --git a/thoth-app/src/models/work/mod.rs b/thoth-app/src/models/work/mod.rs index f0f1dcb1..22cf350f 100644 --- a/thoth-app/src/models/work/mod.rs +++ b/thoth-app/src/models/work/mod.rs @@ -89,7 +89,7 @@ impl MetadataTable for Work { {doi} {&self.publisher()} - {&self.updated_at.format("%F %T")} + {&self.updated_at} } } From dcfd9687fac4c5ae258230023c14b09a9eee2d2b Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Wed, 2 Jun 2021 11:25:12 +0100 Subject: [PATCH 10/19] Move Doi and Orcid out into model/mod.rs (no changes) for shareability across objects --- thoth-api/src/contributor/model.rs | 117 +------- thoth-api/src/funder/model.rs | 2 +- thoth-api/src/graphql/model.rs | 2 + thoth-api/src/model/mod.rs | 256 +++++++++++++++++- thoth-api/src/work/model.rs | 119 +------- thoth-app/src/component/contributor.rs | 3 +- thoth-app/src/component/funder.rs | 2 +- thoth-app/src/component/new_contributor.rs | 3 +- thoth-app/src/component/new_funder.rs | 2 +- thoth-app/src/component/new_work.rs | 2 +- thoth-app/src/component/work.rs | 2 +- .../create_contributor_mutation.rs | 3 +- .../update_contributor_mutation.rs | 3 +- .../models/funder/create_funder_mutation.rs | 2 +- .../models/funder/update_funder_mutation.rs | 2 +- .../src/models/work/create_work_mutation.rs | 2 +- .../src/models/work/update_work_mutation.rs | 2 +- 17 files changed, 263 insertions(+), 261 deletions(-) diff --git a/thoth-api/src/contributor/model.rs b/thoth-api/src/contributor/model.rs index f2026a55..1a8c9e60 100644 --- a/thoth-api/src/contributor/model.rs +++ b/thoth-api/src/contributor/model.rs @@ -1,13 +1,12 @@ use serde::Deserialize; use serde::Serialize; use std::fmt; -use std::str::FromStr; use strum::Display; use strum::EnumString; use uuid::Uuid; -use crate::errors::{ThothError, ThothResult}; use crate::graphql::utils::Direction; +use crate::model::Orcid; use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::contributor; @@ -35,15 +34,6 @@ pub enum ContributorField { UpdatedAt, } -#[cfg_attr( - feature = "backend", - derive(DieselNewType, juniper::GraphQLScalarValue) -)] -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct Orcid(String); - -pub const ORCID_DOMAIN: &str = "https://orcid.org/"; - #[cfg_attr(feature = "backend", derive(Queryable))] #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] @@ -133,58 +123,6 @@ impl fmt::Display for Contributor { } } -impl Default for Orcid { - fn default() -> Orcid { - Orcid { - 0: Default::default(), - } - } -} - -impl fmt::Display for Orcid { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", &self.0.replace(ORCID_DOMAIN, "")) - } -} - -impl FromStr for Orcid { - type Err = ThothError; - - fn from_str(input: &str) -> ThothResult { - use lazy_static::lazy_static; - use regex::Regex; - lazy_static! { - static ref RE: Regex = Regex::new( - // ^ = beginning of string - // (?:) = non-capturing group - // i = case-insensitive flag - // $ = end of string - // Matches strings of format "[[http[s]://]orcid.org/]0000-000X-XXXX-XXXX" - // and captures the 16-digit identifier segment - // Corresponds to database constraints although regex syntax differs slightly - r#"^(?i:(?:https?://)?orcid\.org/)?(0000-000(?:1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$)"#).unwrap(); - } - if let Some(matches) = RE.captures(input) { - // The 0th capture always corresponds to the entire match - if let Some(identifier) = matches.get(1) { - let standardised = format!("{}{}", ORCID_DOMAIN, identifier.as_str()); - let orcid: Orcid = Orcid { 0: standardised }; - Ok(orcid) - } else { - Err(ThothError::IdentifierParseError( - input.to_string(), - "ORCID".to_string(), - )) - } - } else { - Err(ThothError::IdentifierParseError( - input.to_string(), - "ORCID".to_string(), - )) - } - } -} - #[test] fn test_contributorfield_default() { let contfield: ContributorField = Default::default(); @@ -234,56 +172,3 @@ fn test_contributorfield_fromstr() { assert!(ContributorField::from_str("Biography").is_err()); assert!(ContributorField::from_str("Institution").is_err()); } - -#[test] -fn test_orcid_default() { - let orcid: Orcid = Default::default(); - assert_eq!(orcid, Orcid { 0: "".to_string() }); -} - -#[test] -fn test_orcid_display() { - let orcid = Orcid { - 0: "https://orcid.org/0000-0002-1234-5678".to_string(), - }; - assert_eq!(format!("{}", orcid), "0000-0002-1234-5678"); -} - -#[test] -fn test_orcid_fromstr() { - let standardised = Orcid { - 0: "https://orcid.org/0000-0002-1234-5678".to_string(), - }; - assert_eq!( - Orcid::from_str("https://orcid.org/0000-0002-1234-5678").unwrap(), - standardised - ); - assert_eq!( - Orcid::from_str("http://orcid.org/0000-0002-1234-5678").unwrap(), - standardised - ); - assert_eq!( - Orcid::from_str("orcid.org/0000-0002-1234-5678").unwrap(), - standardised - ); - assert_eq!( - Orcid::from_str("0000-0002-1234-5678").unwrap(), - standardised - ); - assert_eq!( - Orcid::from_str("HTTPS://ORCID.ORG/0000-0002-1234-5678").unwrap(), - standardised - ); - assert_eq!( - Orcid::from_str("Https://ORCiD.org/0000-0002-1234-5678").unwrap(), - standardised - ); - assert!(Orcid::from_str("htts://orcid.org/0000-0002-1234-5678").is_err()); - assert!(Orcid::from_str("https://0000-0002-1234-5678").is_err()); - assert!(Orcid::from_str("https://test.org/0000-0002-1234-5678").is_err()); - assert!(Orcid::from_str("http://test.org/0000-0002-1234-5678").is_err()); - assert!(Orcid::from_str("test.org/0000-0002-1234-5678").is_err()); - assert!(Orcid::from_str("//orcid.org/0000-0002-1234-5678").is_err()); - assert!(Orcid::from_str("https://orcid-org/0000-0002-1234-5678").is_err()); - assert!(Orcid::from_str("0000-0002-1234-5678https://orcid.org/").is_err()); -} diff --git a/thoth-api/src/funder/model.rs b/thoth-api/src/funder/model.rs index 8dd7177e..d96e7725 100644 --- a/thoth-api/src/funder/model.rs +++ b/thoth-api/src/funder/model.rs @@ -6,12 +6,12 @@ use strum::EnumString; use uuid::Uuid; use crate::graphql::utils::Direction; +use crate::model::Doi; use crate::model::Timestamp; #[cfg(feature = "backend")] use crate::schema::funder; #[cfg(feature = "backend")] use crate::schema::funder_history; -use crate::work::model::Doi; #[cfg_attr( feature = "backend", diff --git a/thoth-api/src/graphql/model.rs b/thoth-api/src/graphql/model.rs index 29c3de14..d4048502 100644 --- a/thoth-api/src/graphql/model.rs +++ b/thoth-api/src/graphql/model.rs @@ -18,6 +18,8 @@ use crate::imprint::model::*; use crate::issue::model::*; use crate::language::model::*; use crate::model::Crud; +use crate::model::Doi; +use crate::model::Orcid; use crate::model::Timestamp; use crate::price::model::*; use crate::publication::model::*; diff --git a/thoth-api/src/model/mod.rs b/thoth-api/src/model/mod.rs index 352b2aeb..96e66683 100644 --- a/thoth-api/src/model/mod.rs +++ b/thoth-api/src/model/mod.rs @@ -1,9 +1,26 @@ use chrono::{DateTime, TimeZone, Utc}; use serde::{Deserialize, Serialize}; use std::fmt; +use std::str::FromStr; -#[cfg(feature = "backend")] -use crate::errors::ThothResult; +use crate::errors::{ThothError, ThothResult}; + +pub const DOI_DOMAIN: &str = "https://doi.org/"; +pub const ORCID_DOMAIN: &str = "https://orcid.org/"; + +#[cfg_attr( + feature = "backend", + derive(DieselNewType, juniper::GraphQLScalarValue) +)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct Doi(String); + +#[cfg_attr( + feature = "backend", + derive(DieselNewType, juniper::GraphQLScalarValue) +)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct Orcid(String); #[cfg_attr( feature = "backend", @@ -12,6 +29,22 @@ use crate::errors::ThothResult; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Timestamp(DateTime); +impl Default for Doi { + fn default() -> Doi { + Doi { + 0: Default::default(), + } + } +} + +impl Default for Orcid { + fn default() -> Orcid { + Orcid { + 0: Default::default(), + } + } +} + impl Default for Timestamp { fn default() -> Timestamp { Timestamp { @@ -20,27 +53,99 @@ impl Default for Timestamp { } } +impl fmt::Display for Doi { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.0.replace(DOI_DOMAIN, "")) + } +} + +impl fmt::Display for Orcid { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.0.replace(ORCID_DOMAIN, "")) + } +} + impl fmt::Display for Timestamp { fn fmt(&self, f: &mut std::fmt::Formatter) -> fmt::Result { write!(f, "{}", &self.0.format("%F %T")) } } -#[test] -fn test_timestamp_default() { - let stamp: Timestamp = Default::default(); - assert_eq!( - stamp, - Timestamp { - 0: TimeZone::timestamp(&Utc, 0, 0) +impl FromStr for Doi { + type Err = ThothError; + + fn from_str(input: &str) -> ThothResult { + use lazy_static::lazy_static; + use regex::Regex; + lazy_static! { + static ref RE: Regex = Regex::new( + // ^ = beginning of string + // (?:) = non-capturing group + // i = case-insensitive flag + // $ = end of string + // Matches strings of format "[[http[s]://]doi.org/]10.XXX/XXX" + // and captures the identifier segment starting with the "10." directory indicator + // Corresponds to database constraints although regex syntax differs slightly + // (e.g. `;()/` do not need to be escaped here) + r#"^(?i:(?:https?://)?doi\.org/)?(10\.\d{4,9}/[-._;()/:a-zA-Z0-9]+$)"#).unwrap(); } - ); + if let Some(matches) = RE.captures(input) { + // The 0th capture always corresponds to the entire match + if let Some(identifier) = matches.get(1) { + let standardised = format!("{}{}", DOI_DOMAIN, identifier.as_str()); + let doi: Doi = Doi { 0: standardised }; + Ok(doi) + } else { + Err(ThothError::IdentifierParseError( + input.to_string(), + "DOI".to_string(), + )) + } + } else { + Err(ThothError::IdentifierParseError( + input.to_string(), + "DOI".to_string(), + )) + } + } } -#[test] -fn test_timestamp_display() { - let stamp: Timestamp = Default::default(); - assert_eq!(format!("{}", stamp), "1970-01-01 00:00:00"); +impl FromStr for Orcid { + type Err = ThothError; + + fn from_str(input: &str) -> ThothResult { + use lazy_static::lazy_static; + use regex::Regex; + lazy_static! { + static ref RE: Regex = Regex::new( + // ^ = beginning of string + // (?:) = non-capturing group + // i = case-insensitive flag + // $ = end of string + // Matches strings of format "[[http[s]://]orcid.org/]0000-000X-XXXX-XXXX" + // and captures the 16-digit identifier segment + // Corresponds to database constraints although regex syntax differs slightly + r#"^(?i:(?:https?://)?orcid\.org/)?(0000-000(?:1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$)"#).unwrap(); + } + if let Some(matches) = RE.captures(input) { + // The 0th capture always corresponds to the entire match + if let Some(identifier) = matches.get(1) { + let standardised = format!("{}{}", ORCID_DOMAIN, identifier.as_str()); + let orcid: Orcid = Orcid { 0: standardised }; + Ok(orcid) + } else { + Err(ThothError::IdentifierParseError( + input.to_string(), + "ORCID".to_string(), + )) + } + } else { + Err(ThothError::IdentifierParseError( + input.to_string(), + "ORCID".to_string(), + )) + } + } } #[cfg(feature = "backend")] @@ -257,3 +362,126 @@ macro_rules! db_insert { } }; } + +#[test] +fn test_doi_default() { + let doi: Doi = Default::default(); + assert_eq!(doi, Doi { 0: "".to_string() }); +} + +#[test] +fn test_orcid_default() { + let orcid: Orcid = Default::default(); + assert_eq!(orcid, Orcid { 0: "".to_string() }); +} + +#[test] +fn test_timestamp_default() { + let stamp: Timestamp = Default::default(); + assert_eq!( + stamp, + Timestamp { + 0: TimeZone::timestamp(&Utc, 0, 0) + } + ); +} + +#[test] +fn test_doi_display() { + let doi = Doi { + 0: "https://doi.org/10.12345/Test-Suffix.01".to_string(), + }; + assert_eq!(format!("{}", doi), "10.12345/Test-Suffix.01"); +} + +#[test] +fn test_orcid_display() { + let orcid = Orcid { + 0: "https://orcid.org/0000-0002-1234-5678".to_string(), + }; + assert_eq!(format!("{}", orcid), "0000-0002-1234-5678"); +} + +#[test] +fn test_timestamp_display() { + let stamp: Timestamp = Default::default(); + assert_eq!(format!("{}", stamp), "1970-01-01 00:00:00"); +} + +#[test] +fn test_doi_fromstr() { + let standardised = Doi { + 0: "https://doi.org/10.12345/Test-Suffix.01".to_string(), + }; + assert_eq!( + Doi::from_str("https://doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("http://doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("HTTPS://DOI.ORG/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("Https://DOI.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert!(Doi::from_str("htts://doi.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("https://10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("https://test.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("http://test.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("test.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("//doi.org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("https://doi-org/10.12345/Test-Suffix.01").is_err()); + assert!(Doi::from_str("10.https://doi.org/12345/Test-Suffix.01").is_err()); +} + +#[test] +fn test_orcid_fromstr() { + let standardised = Orcid { + 0: "https://orcid.org/0000-0002-1234-5678".to_string(), + }; + assert_eq!( + Orcid::from_str("https://orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("http://orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("HTTPS://ORCID.ORG/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("Https://ORCiD.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert!(Orcid::from_str("htts://orcid.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("https://0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("https://test.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("http://test.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("test.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("//orcid.org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("https://orcid-org/0000-0002-1234-5678").is_err()); + assert!(Orcid::from_str("0000-0002-1234-5678https://orcid.org/").is_err()); +} diff --git a/thoth-api/src/work/model.rs b/thoth-api/src/work/model.rs index 68981a49..3a7fd081 100644 --- a/thoth-api/src/work/model.rs +++ b/thoth-api/src/work/model.rs @@ -1,18 +1,16 @@ use chrono::naive::NaiveDate; use serde::{Deserialize, Serialize}; -use std::fmt; -use std::str::FromStr; use strum::Display; use strum::EnumString; use uuid::Uuid; use crate::contribution::model::Contribution; -use crate::errors::{ThothError, ThothResult}; use crate::funding::model::FundingExtended as Funding; use crate::graphql::utils::Direction; use crate::imprint::model::ImprintExtended as Imprint; use crate::issue::model::IssueExtended as Issue; use crate::language::model::Language; +use crate::model::Doi; use crate::model::Timestamp; use crate::publication::model::PublicationExtended as Publication; #[cfg(feature = "backend")] @@ -116,15 +114,6 @@ pub enum WorkField { UpdatedAt, } -#[cfg_attr( - feature = "backend", - derive(DieselNewType, juniper::GraphQLScalarValue) -)] -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct Doi(String); - -pub const DOI_DOMAIN: &str = "https://doi.org/"; - #[cfg_attr(feature = "backend", derive(Queryable))] #[derive(Default, Serialize, Deserialize)] pub struct Work { @@ -392,59 +381,6 @@ impl Default for WorkExtended { } } -impl Default for Doi { - fn default() -> Doi { - Doi { - 0: Default::default(), - } - } -} - -impl fmt::Display for Doi { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", &self.0.replace(DOI_DOMAIN, "")) - } -} - -impl FromStr for Doi { - type Err = ThothError; - - fn from_str(input: &str) -> ThothResult { - use lazy_static::lazy_static; - use regex::Regex; - lazy_static! { - static ref RE: Regex = Regex::new( - // ^ = beginning of string - // (?:) = non-capturing group - // i = case-insensitive flag - // $ = end of string - // Matches strings of format "[[http[s]://]doi.org/]10.XXX/XXX" - // and captures the identifier segment starting with the "10." directory indicator - // Corresponds to database constraints although regex syntax differs slightly - // (e.g. `;()/` do not need to be escaped here) - r#"^(?i:(?:https?://)?doi\.org/)?(10\.\d{4,9}/[-._;()/:a-zA-Z0-9]+$)"#).unwrap(); - } - if let Some(matches) = RE.captures(input) { - // The 0th capture always corresponds to the entire match - if let Some(identifier) = matches.get(1) { - let standardised = format!("{}{}", DOI_DOMAIN, identifier.as_str()); - let doi: Doi = Doi { 0: standardised }; - Ok(doi) - } else { - Err(ThothError::IdentifierParseError( - input.to_string(), - "DOI".to_string(), - )) - } - } else { - Err(ThothError::IdentifierParseError( - input.to_string(), - "DOI".to_string(), - )) - } - } -} - #[test] fn test_worktype_default() { let worktype: WorkType = Default::default(); @@ -716,56 +652,3 @@ fn test_workfield_fromstr() { assert!(WorkField::from_str("Contributors").is_err()); assert!(WorkField::from_str("Publisher").is_err()); } - -#[test] -fn test_doi_default() { - let doi: Doi = Default::default(); - assert_eq!(doi, Doi { 0: "".to_string() }); -} - -#[test] -fn test_doi_display() { - let doi = Doi { - 0: "https://doi.org/10.12345/Test-Suffix.01".to_string(), - }; - assert_eq!(format!("{}", doi), "10.12345/Test-Suffix.01"); -} - -#[test] -fn test_doi_fromstr() { - let standardised = Doi { - 0: "https://doi.org/10.12345/Test-Suffix.01".to_string(), - }; - assert_eq!( - Doi::from_str("https://doi.org/10.12345/Test-Suffix.01").unwrap(), - standardised - ); - assert_eq!( - Doi::from_str("http://doi.org/10.12345/Test-Suffix.01").unwrap(), - standardised - ); - assert_eq!( - Doi::from_str("doi.org/10.12345/Test-Suffix.01").unwrap(), - standardised - ); - assert_eq!( - Doi::from_str("10.12345/Test-Suffix.01").unwrap(), - standardised - ); - assert_eq!( - Doi::from_str("HTTPS://DOI.ORG/10.12345/Test-Suffix.01").unwrap(), - standardised - ); - assert_eq!( - Doi::from_str("Https://DOI.org/10.12345/Test-Suffix.01").unwrap(), - standardised - ); - assert!(Doi::from_str("htts://doi.org/10.12345/Test-Suffix.01").is_err()); - assert!(Doi::from_str("https://10.12345/Test-Suffix.01").is_err()); - assert!(Doi::from_str("https://test.org/10.12345/Test-Suffix.01").is_err()); - assert!(Doi::from_str("http://test.org/10.12345/Test-Suffix.01").is_err()); - assert!(Doi::from_str("test.org/10.12345/Test-Suffix.01").is_err()); - assert!(Doi::from_str("//doi.org/10.12345/Test-Suffix.01").is_err()); - assert!(Doi::from_str("https://doi-org/10.12345/Test-Suffix.01").is_err()); - assert!(Doi::from_str("10.https://doi.org/12345/Test-Suffix.01").is_err()); -} diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index 3be3088c..325c08d4 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -1,4 +1,5 @@ -use thoth_api::contributor::model::{Contributor, Orcid, ORCID_DOMAIN}; +use thoth_api::contributor::model::Contributor; +use thoth_api::model::{Orcid, ORCID_DOMAIN}; use uuid::Uuid; use yew::html; use yew::prelude::*; diff --git a/thoth-app/src/component/funder.rs b/thoth-app/src/component/funder.rs index 7d5cb198..2e0ec018 100644 --- a/thoth-app/src/component/funder.rs +++ b/thoth-app/src/component/funder.rs @@ -1,5 +1,5 @@ use thoth_api::funder::model::Funder; -use thoth_api::work::model::{Doi, DOI_DOMAIN}; +use thoth_api::model::{Doi, DOI_DOMAIN}; use uuid::Uuid; use yew::html; use yew::prelude::*; diff --git a/thoth-app/src/component/new_contributor.rs b/thoth-app/src/component/new_contributor.rs index 99a707cf..b6862577 100644 --- a/thoth-app/src/component/new_contributor.rs +++ b/thoth-app/src/component/new_contributor.rs @@ -1,4 +1,5 @@ -use thoth_api::contributor::model::{Contributor, Orcid, ORCID_DOMAIN}; +use thoth_api::contributor::model::Contributor; +use thoth_api::model::{Orcid, ORCID_DOMAIN}; use yew::html; use yew::prelude::*; use yew::ComponentLink; diff --git a/thoth-app/src/component/new_funder.rs b/thoth-app/src/component/new_funder.rs index 8a1d083b..2b640977 100644 --- a/thoth-app/src/component/new_funder.rs +++ b/thoth-app/src/component/new_funder.rs @@ -1,5 +1,5 @@ use thoth_api::funder::model::Funder; -use thoth_api::work::model::{Doi, DOI_DOMAIN}; +use thoth_api::model::{Doi, DOI_DOMAIN}; use yew::html; use yew::prelude::*; use yew::ComponentLink; diff --git a/thoth-app/src/component/new_work.rs b/thoth-app/src/component/new_work.rs index 19d1923f..41614967 100644 --- a/thoth-app/src/component/new_work.rs +++ b/thoth-app/src/component/new_work.rs @@ -1,10 +1,10 @@ use std::str::FromStr; use thoth_api::account::model::AccountDetails; use thoth_api::imprint::model::ImprintExtended as Imprint; +use thoth_api::model::{Doi, DOI_DOMAIN}; use thoth_api::work::model::WorkExtended as Work; use thoth_api::work::model::WorkStatus; use thoth_api::work::model::WorkType; -use thoth_api::work::model::{Doi, DOI_DOMAIN}; use uuid::Uuid; use yew::html; use yew::prelude::*; diff --git a/thoth-app/src/component/work.rs b/thoth-app/src/component/work.rs index 43d976bf..09aeb79e 100644 --- a/thoth-app/src/component/work.rs +++ b/thoth-app/src/component/work.rs @@ -5,12 +5,12 @@ use thoth_api::funding::model::FundingExtended as Funding; use thoth_api::imprint::model::ImprintExtended as Imprint; use thoth_api::issue::model::IssueExtended as Issue; use thoth_api::language::model::Language; +use thoth_api::model::{Doi, DOI_DOMAIN}; use thoth_api::publication::model::PublicationExtended as Publication; use thoth_api::subject::model::Subject; use thoth_api::work::model::WorkExtended as Work; use thoth_api::work::model::WorkStatus; use thoth_api::work::model::WorkType; -use thoth_api::work::model::{Doi, DOI_DOMAIN}; use uuid::Uuid; use yew::html; use yew::prelude::*; diff --git a/thoth-app/src/models/contributor/create_contributor_mutation.rs b/thoth-app/src/models/contributor/create_contributor_mutation.rs index 9ff7c437..26250a61 100644 --- a/thoth-app/src/models/contributor/create_contributor_mutation.rs +++ b/thoth-app/src/models/contributor/create_contributor_mutation.rs @@ -1,6 +1,7 @@ use serde::Deserialize; use serde::Serialize; -use thoth_api::contributor::model::{Contributor, Orcid}; +use thoth_api::contributor::model::Contributor; +use thoth_api::model::Orcid; const CREATE_CONTRIBUTOR_MUTATION: &str = " mutation CreateContributor( diff --git a/thoth-app/src/models/contributor/update_contributor_mutation.rs b/thoth-app/src/models/contributor/update_contributor_mutation.rs index f74c46fb..b349c27e 100644 --- a/thoth-app/src/models/contributor/update_contributor_mutation.rs +++ b/thoth-app/src/models/contributor/update_contributor_mutation.rs @@ -1,6 +1,7 @@ use serde::Deserialize; use serde::Serialize; -use thoth_api::contributor::model::{Contributor, Orcid}; +use thoth_api::contributor::model::Contributor; +use thoth_api::model::Orcid; use uuid::Uuid; const UPDATE_CONTRIBUTOR_MUTATION: &str = " diff --git a/thoth-app/src/models/funder/create_funder_mutation.rs b/thoth-app/src/models/funder/create_funder_mutation.rs index b2dc841e..3ada3924 100644 --- a/thoth-app/src/models/funder/create_funder_mutation.rs +++ b/thoth-app/src/models/funder/create_funder_mutation.rs @@ -1,7 +1,7 @@ use serde::Deserialize; use serde::Serialize; use thoth_api::funder::model::Funder; -use thoth_api::work::model::Doi; +use thoth_api::model::Doi; const CREATE_FUNDER_MUTATION: &str = " mutation CreateFunder( diff --git a/thoth-app/src/models/funder/update_funder_mutation.rs b/thoth-app/src/models/funder/update_funder_mutation.rs index 4a42b868..39cdf024 100644 --- a/thoth-app/src/models/funder/update_funder_mutation.rs +++ b/thoth-app/src/models/funder/update_funder_mutation.rs @@ -1,7 +1,7 @@ use serde::Deserialize; use serde::Serialize; use thoth_api::funder::model::Funder; -use thoth_api::work::model::Doi; +use thoth_api::model::Doi; use uuid::Uuid; const UPDATE_FUNDER_MUTATION: &str = " diff --git a/thoth-app/src/models/work/create_work_mutation.rs b/thoth-app/src/models/work/create_work_mutation.rs index 733e04f6..25da1d1a 100644 --- a/thoth-app/src/models/work/create_work_mutation.rs +++ b/thoth-app/src/models/work/create_work_mutation.rs @@ -1,6 +1,6 @@ use serde::Deserialize; use serde::Serialize; -use thoth_api::work::model::Doi; +use thoth_api::model::Doi; use thoth_api::work::model::WorkStatus; use thoth_api::work::model::WorkType; use uuid::Uuid; diff --git a/thoth-app/src/models/work/update_work_mutation.rs b/thoth-app/src/models/work/update_work_mutation.rs index 8619af47..52b7a71f 100644 --- a/thoth-app/src/models/work/update_work_mutation.rs +++ b/thoth-app/src/models/work/update_work_mutation.rs @@ -1,6 +1,6 @@ use serde::Deserialize; use serde::Serialize; -use thoth_api::work::model::Doi; +use thoth_api::model::Doi; use thoth_api::work::model::WorkStatus; use thoth_api::work::model::WorkType; use uuid::Uuid; From b06ced2620679f4277aca239d2ddc0aecdec1793 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Wed, 2 Jun 2021 14:13:16 +0100 Subject: [PATCH 11/19] Fix bug where saving on an invalid value overwrote stored version if a valid value had previously been entered --- thoth-app/src/component/contributor.rs | 29 +++++++++++---------- thoth-app/src/component/funder.rs | 30 ++++++++++++---------- thoth-app/src/component/new_contributor.rs | 29 +++++++++++---------- thoth-app/src/component/new_funder.rs | 30 ++++++++++++---------- thoth-app/src/component/new_work.rs | 29 +++++++++++---------- thoth-app/src/component/work.rs | 29 +++++++++++---------- 6 files changed, 92 insertions(+), 84 deletions(-) diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index 325c08d4..2858605d 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -208,6 +208,14 @@ impl Component for ContributorComponent { } } Msg::UpdateContributor => { + // Only update the ORCID value with the current user-entered string + // if it is validly formatted - otherwise keep the database version. + // If no ORCID was provided, no format check is required. + if self.orcid.is_empty() { + self.contributor.orcid.neq_assign(None); + } else if let Ok(result) = self.orcid.parse::() { + self.contributor.orcid.neq_assign(Some(result)); + } let body = UpdateContributorRequestBody { variables: UpdateVariables { contributor_id: self.contributor.contributor_id, @@ -294,21 +302,14 @@ impl Component for ContributorComponent { .neq_assign(full_name.trim().to_owned()), Msg::ChangeOrcid(value) => { if self.orcid.neq_assign(value.trim().to_owned()) { - // Check ORCID is correctly formatted before updating structure. - // If no ORCID was provided, no check is required. - if self.orcid.trim().is_empty() { - self.contributor.orcid.neq_assign(None); - self.orcid_warning.clear(); + // If ORCID is not correctly formatted, display a warning. + // If no ORCID was provided, no format check is required. + // Don't update self.contributor.orcid yet, as user may later + // overwrite a new valid value with an invalid one. + if !self.orcid.is_empty() && self.orcid.parse::().is_err() { + self.orcid_warning = self.orcid.parse::().unwrap_err().to_string(); } else { - match self.orcid.parse::() { - Ok(result) => { - self.contributor.orcid.neq_assign(Some(result)); - self.orcid_warning.clear(); - } - Err(err) => { - self.orcid_warning = err.to_string(); - } - }; + self.orcid_warning.clear(); } true } else { diff --git a/thoth-app/src/component/funder.rs b/thoth-app/src/component/funder.rs index 2e0ec018..9c42ba59 100644 --- a/thoth-app/src/component/funder.rs +++ b/thoth-app/src/component/funder.rs @@ -204,6 +204,14 @@ impl Component for FunderComponent { } } Msg::UpdateFunder => { + // Only update the DOI value with the current user-entered string + // if it is validly formatted - otherwise keep the database version. + // If no DOI was provided, no format check is required. + if self.funder_doi.is_empty() { + self.funder.funder_doi.neq_assign(None); + } else if let Ok(result) = self.funder_doi.parse::() { + self.funder.funder_doi.neq_assign(Some(result)); + } let body = UpdateFunderRequestBody { variables: UpdateVariables { funder_id: self.funder.funder_id, @@ -274,21 +282,15 @@ impl Component for FunderComponent { .neq_assign(funder_name.trim().to_owned()), Msg::ChangeFunderDoi(value) => { if self.funder_doi.neq_assign(value.trim().to_owned()) { - // Check DOI is correctly formatted before updating structure. - // If no DOI was provided, no check is required. - if self.funder_doi.trim().is_empty() { - self.funder.funder_doi.neq_assign(None); - self.funder_doi_warning.clear(); + // If DOI is not correctly formatted, display a warning. + // If no DOI was provided, no format check is required. + // Don't update self.funder.funder_doi yet, as user may later + // overwrite a new valid value with an invalid one. + if !self.funder_doi.is_empty() && self.funder_doi.parse::().is_err() { + self.funder_doi_warning = + self.funder_doi.parse::().unwrap_err().to_string(); } else { - match self.funder_doi.parse::() { - Ok(result) => { - self.funder.funder_doi.neq_assign(Some(result)); - self.funder_doi_warning.clear(); - } - Err(err) => { - self.funder_doi_warning = err.to_string(); - } - }; + self.funder_doi_warning.clear(); } true } else { diff --git a/thoth-app/src/component/new_contributor.rs b/thoth-app/src/component/new_contributor.rs index b6862577..d14f58fe 100644 --- a/thoth-app/src/component/new_contributor.rs +++ b/thoth-app/src/component/new_contributor.rs @@ -130,6 +130,14 @@ impl Component for NewContributorComponent { } } Msg::CreateContributor => { + // Only update the ORCID value with the current user-entered string + // if it is validly formatted - otherwise keep the default. + // If no ORCID was provided, no format check is required. + if self.orcid.is_empty() { + self.contributor.orcid.neq_assign(None); + } else if let Ok(result) = self.orcid.parse::() { + self.contributor.orcid.neq_assign(Some(result)); + } let body = CreateContributorRequestBody { variables: Variables { first_name: self.contributor.first_name.clone(), @@ -211,21 +219,14 @@ impl Component for NewContributorComponent { } Msg::ChangeOrcid(value) => { if self.orcid.neq_assign(value.trim().to_owned()) { - // Check ORCID is correctly formatted before updating structure. - // If no ORCID was provided, no check is required. - if self.orcid.trim().is_empty() { - self.contributor.orcid.neq_assign(None); - self.orcid_warning.clear(); + // If ORCID is not correctly formatted, display a warning. + // If no ORCID was provided, no format check is required. + // Don't update self.contributor.orcid yet, as user may later + // overwrite a new valid value with an invalid one. + if !self.orcid.is_empty() && self.orcid.parse::().is_err() { + self.orcid_warning = self.orcid.parse::().unwrap_err().to_string(); } else { - match self.orcid.parse::() { - Ok(result) => { - self.contributor.orcid.neq_assign(Some(result)); - self.orcid_warning.clear(); - } - Err(err) => { - self.orcid_warning = err.to_string(); - } - }; + self.orcid_warning.clear(); } true } else { diff --git a/thoth-app/src/component/new_funder.rs b/thoth-app/src/component/new_funder.rs index 2b640977..f50e8358 100644 --- a/thoth-app/src/component/new_funder.rs +++ b/thoth-app/src/component/new_funder.rs @@ -105,6 +105,14 @@ impl Component for NewFunderComponent { } } Msg::CreateFunder => { + // Only update the DOI value with the current user-entered string + // if it is validly formatted - otherwise keep the default. + // If no DOI was provided, no format check is required. + if self.funder_doi.is_empty() { + self.funder.funder_doi.neq_assign(None); + } else if let Ok(result) = self.funder_doi.parse::() { + self.funder.funder_doi.neq_assign(Some(result)); + } let body = CreateFunderRequestBody { variables: Variables { funder_name: self.funder.funder_name.clone(), @@ -126,21 +134,15 @@ impl Component for NewFunderComponent { .neq_assign(funder_name.trim().to_owned()), Msg::ChangeFunderDoi(value) => { if self.funder_doi.neq_assign(value.trim().to_owned()) { - // Check DOI is correctly formatted before updating structure. - // If no DOI was provided, no check is required. - if self.funder_doi.trim().is_empty() { - self.funder.funder_doi.neq_assign(None); - self.funder_doi_warning.clear(); + // If DOI is not correctly formatted, display a warning. + // If no DOI was provided, no format check is required. + // Don't update self.funder.funder_doi yet, as user may later + // overwrite a new valid value with an invalid one. + if !self.funder_doi.is_empty() && self.funder_doi.parse::().is_err() { + self.funder_doi_warning = + self.funder_doi.parse::().unwrap_err().to_string(); } else { - match self.funder_doi.parse::() { - Ok(result) => { - self.funder.funder_doi.neq_assign(Some(result)); - self.funder_doi_warning.clear(); - } - Err(err) => { - self.funder_doi_warning = err.to_string(); - } - }; + self.funder_doi_warning.clear(); } true } else { diff --git a/thoth-app/src/component/new_work.rs b/thoth-app/src/component/new_work.rs index 41614967..ddfd5e75 100644 --- a/thoth-app/src/component/new_work.rs +++ b/thoth-app/src/component/new_work.rs @@ -258,6 +258,14 @@ impl Component for NewWorkComponent { } } Msg::CreateWork => { + // Only update the DOI value with the current user-entered string + // if it is validly formatted - otherwise keep the default. + // If no DOI was provided, no format check is required. + if self.doi.is_empty() { + self.work.doi.neq_assign(None); + } else if let Ok(result) = self.doi.parse::() { + self.work.doi.neq_assign(Some(result)); + } let body = CreateWorkRequestBody { variables: Variables { work_type: self.work.work_type.clone(), @@ -337,21 +345,14 @@ impl Component for NewWorkComponent { } Msg::ChangeDoi(value) => { if self.doi.neq_assign(value.trim().to_owned()) { - // Check DOI is correctly formatted before updating structure. - // If no DOI was provided, no check is required. - if self.doi.trim().is_empty() { - self.work.doi.neq_assign(None); - self.doi_warning.clear(); + // If DOI is not correctly formatted, display a warning. + // If no DOI was provided, no format check is required. + // Don't update self.work.doi yet, as user may later + // overwrite a new valid value with an invalid one. + if !self.doi.is_empty() && self.doi.parse::().is_err() { + self.doi_warning = self.doi.parse::().unwrap_err().to_string(); } else { - match self.doi.parse::() { - Ok(result) => { - self.work.doi.neq_assign(Some(result)); - self.doi_warning.clear(); - } - Err(err) => { - self.doi_warning = err.to_string(); - } - }; + self.doi_warning.clear(); } true } else { diff --git a/thoth-app/src/component/work.rs b/thoth-app/src/component/work.rs index 09aeb79e..9517bbc5 100644 --- a/thoth-app/src/component/work.rs +++ b/thoth-app/src/component/work.rs @@ -263,6 +263,14 @@ impl Component for WorkComponent { } } Msg::UpdateWork => { + // Only update the DOI value with the current user-entered string + // if it is validly formatted - otherwise keep the database version. + // If no DOI was provided, no format check is required. + if self.doi.is_empty() { + self.work.doi.neq_assign(None); + } else if let Ok(result) = self.doi.parse::() { + self.work.doi.neq_assign(Some(result)); + } let body = UpdateWorkRequestBody { variables: UpdateVariables { work_id: self.work.work_id, @@ -404,21 +412,14 @@ impl Component for WorkComponent { } Msg::ChangeDoi(value) => { if self.doi.neq_assign(value.trim().to_owned()) { - // Check DOI is correctly formatted before updating structure. - // If no DOI was provided, no check is required. - if self.doi.trim().is_empty() { - self.work.doi.neq_assign(None); - self.doi_warning.clear(); + // If DOI is not correctly formatted, display a warning. + // If no DOI was provided, no format check is required. + // Don't update self.work.doi yet, as user may later + // overwrite a new valid value with an invalid one. + if !self.doi.is_empty() && self.doi.parse::().is_err() { + self.doi_warning = self.doi.parse::().unwrap_err().to_string(); } else { - match self.doi.parse::() { - Ok(result) => { - self.work.doi.neq_assign(Some(result)); - self.doi_warning.clear(); - } - Err(err) => { - self.doi_warning = err.to_string(); - } - }; + self.doi_warning.clear(); } true } else { From f1b22102e01a3a6a169579dfcf39e419a4458741 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Wed, 2 Jun 2021 16:41:01 +0100 Subject: [PATCH 12/19] Add migrations to improve database DOI/ORCID checks so that they match Doi/Orcid parsing --- thoth-api/migrations/0.3.7/down.sql | 16 ++++++++++++++++ thoth-api/migrations/0.3.7/up.sql | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 thoth-api/migrations/0.3.7/down.sql create mode 100644 thoth-api/migrations/0.3.7/up.sql diff --git a/thoth-api/migrations/0.3.7/down.sql b/thoth-api/migrations/0.3.7/down.sql new file mode 100644 index 00000000..035922c9 --- /dev/null +++ b/thoth-api/migrations/0.3.7/down.sql @@ -0,0 +1,16 @@ +-- Reinstate earlier versions of ORCID and DOI validation + +ALTER TABLE contributor + DROP CONSTRAINT contributor_orcid_check, + ADD CONSTRAINT contributor_orcid_check + CHECK (orcid ~* '0000-000(1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]'); + +ALTER TABLE work + DROP CONSTRAINT work_doi_check, + ADD CONSTRAINT work_doi_check + CHECK (doi ~* 'https:\/\/doi.org\/10.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$'); + +ALTER TABLE funder + DROP CONSTRAINT funder_funder_doi_check, + ADD CONSTRAINT funder_funder_doi_check + CHECK (funder_doi ~* 'https:\/\/doi.org\/10.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$'); diff --git a/thoth-api/migrations/0.3.7/up.sql b/thoth-api/migrations/0.3.7/up.sql new file mode 100644 index 00000000..6e094816 --- /dev/null +++ b/thoth-api/migrations/0.3.7/up.sql @@ -0,0 +1,21 @@ +-- Improve validation of ORCID identifiers (include protocol/resource name) +-- Should be kept in line with Orcid::FromStr, although regex syntax differs slightly + +ALTER TABLE contributor + DROP CONSTRAINT contributor_orcid_check, + ADD CONSTRAINT contributor_orcid_check + CHECK (orcid ~* '^https:\/\/orcid\.org\/0000-000(1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$'); + +-- Improve validation of DOI identifiers (add line start marker, escape periods) +-- Should be kept in line with Orcid::FromStr, although regex syntax differs slightly +-- (e.g. `;()/` need to be escaped here but not in Orcid::FromStr) + +ALTER TABLE work + DROP CONSTRAINT work_doi_check, + ADD CONSTRAINT work_doi_check + CHECK (doi ~* '^https:\/\/doi\.org\/10\.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$'); + +ALTER TABLE funder + DROP CONSTRAINT funder_funder_doi_check, + ADD CONSTRAINT funder_funder_doi_check + CHECK (funder_doi ~* '^https:\/\/doi\.org\/10\.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$'); From cddb72be54c5f9ddad70887500c7225bb1b79319 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Thu, 3 Jun 2021 12:12:44 +0100 Subject: [PATCH 13/19] Merge duplicated tooltip/static button util into FormTextInputExtended --- thoth-api-server/src/onix.rs | 3 +- thoth-app/src/component/contributor.rs | 4 +- thoth-app/src/component/funder.rs | 4 +- thoth-app/src/component/new_contributor.rs | 7 +- thoth-app/src/component/new_funder.rs | 4 +- thoth-app/src/component/new_work.rs | 4 +- thoth-app/src/component/utils.rs | 94 ++++++---------------- thoth-app/src/component/work.rs | 4 +- 8 files changed, 40 insertions(+), 84 deletions(-) diff --git a/thoth-api-server/src/onix.rs b/thoth-api-server/src/onix.rs index 2c5f6d8b..1cdcead4 100644 --- a/thoth-api-server/src/onix.rs +++ b/thoth-api-server/src/onix.rs @@ -3,6 +3,7 @@ use std::io::Write; use chrono::prelude::*; use thoth_api::errors::{ThothError, ThothResult}; +use thoth_api::model::DOI_DOMAIN; use thoth_client::work::work_query::ContributionType; use thoth_client::work::work_query::LanguageRelation; use thoth_client::work::work_query::PublicationType; @@ -239,7 +240,7 @@ fn handle_event(w: &mut EventWriter, work: &mut WorkQueryWork) -> R }) .ok(); write_element_block("IDValue", None, None, w, |w| { - let sanitised_doi = doi.replace("https://doi.org/", ""); + let sanitised_doi = doi.replace(DOI_DOMAIN, ""); let event: XmlEvent = XmlEvent::Characters(&sanitised_doi); w.write(event).ok(); }) diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index 2858605d..5e4ff805 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -22,7 +22,7 @@ use crate::agent::notification_bus::NotificationStatus; use crate::agent::notification_bus::Request; use crate::component::delete_dialogue::ConfirmDeleteComponent; use crate::component::utils::FormTextInput; -use crate::component::utils::FormTextInputTooltipStatic; +use crate::component::utils::FormTextInputExtended; use crate::component::utils::FormUrlInput; use crate::component::utils::Loader; use crate::models::contributor::contributor_activity_query::ContributorActivityResponseData; @@ -404,7 +404,7 @@ impl Component for ContributorComponent { oninput=self.link.callback(|e: InputData| Msg::ChangeFullName(e.value)) required = true /> - - - - -
- ; pub type FormTextarea = Pure; -pub type FormTextInputTooltip = Pure; -pub type FormTextInputTooltipStatic = Pure; +pub type FormTextInputExtended = Pure; pub type FormTextInput = Pure; pub type FormUrlInput = Pure; pub type FormDateInput = Pure; @@ -81,26 +80,15 @@ pub struct PureTextarea { pub required: bool, } +// Variant of PureTextInput which supports tooltips, +// prepended static buttons, or both together. #[derive(Clone, PartialEq, Properties)] -pub struct PureTextInputTooltip { +pub struct PureTextInputExtended { pub label: String, pub value: String, - pub tooltip: String, - #[prop_or_default] - pub oninput: Callback, - #[prop_or_default] - pub onfocus: Callback, #[prop_or_default] - pub onblur: Callback, - #[prop_or(false)] - pub required: bool, -} - -#[derive(Clone, PartialEq, Properties)] -pub struct PureTextInputTooltipStatic { - pub label: String, - pub value: String, pub tooltip: String, + #[prop_or_default] pub statictext: String, #[prop_or_default] pub oninput: Callback, @@ -344,56 +332,7 @@ impl PureComponent for PureTextarea { } } -impl PureComponent for PureTextInputTooltip { - fn render(&self) -> VNode { - // Only display tooltip if its value is set. - // Yew release 0.18.0 will introduce optional attributes - - // at this point we can make `data-tooltip` optional - // and collapse down the duplicated `html!` declaration. - if self.tooltip.is_empty() { - html! { -
- -
- -
-
- } - } else { - html! { -
- -
- -
-
- } - } - } -} - -impl PureComponent for PureTextInputTooltipStatic { +impl PureComponent for PureTextInputExtended { fn render(&self) -> VNode { // Only display tooltip if its value is set. // Yew release 0.18.0 will introduce optional attributes - @@ -404,7 +343,16 @@ impl PureComponent for PureTextInputTooltipStatic {
- + { + // Only display static button if a static text value was provided. + if self.statictext.is_empty() { + html! {} + } else { + html! { + + } + } + } - + { + if self.statictext.is_empty() { + html! {} + } else { + html! { + + } + } + }
- Date: Wed, 16 Jun 2021 15:00:48 +0100 Subject: [PATCH 14/19] Re-version new migrations as latest merge from develop has put us up to v0.4.0 --- thoth-api/migrations/{0.3.7 => 0.4.1}/down.sql | 0 thoth-api/migrations/{0.3.7 => 0.4.1}/up.sql | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename thoth-api/migrations/{0.3.7 => 0.4.1}/down.sql (100%) rename thoth-api/migrations/{0.3.7 => 0.4.1}/up.sql (100%) diff --git a/thoth-api/migrations/0.3.7/down.sql b/thoth-api/migrations/0.4.1/down.sql similarity index 100% rename from thoth-api/migrations/0.3.7/down.sql rename to thoth-api/migrations/0.4.1/down.sql diff --git a/thoth-api/migrations/0.3.7/up.sql b/thoth-api/migrations/0.4.1/up.sql similarity index 100% rename from thoth-api/migrations/0.3.7/up.sql rename to thoth-api/migrations/0.4.1/up.sql From 4be055711da199e1447c47dc78532d6226861fdd Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Thu, 17 Jun 2021 11:19:30 +0100 Subject: [PATCH 15/19] Simplify PureTextInputExtended using Yew optional attributes (newly available following update to v0.18.0) --- thoth-app/src/component/utils.rs | 88 +++++++++++--------------------- 1 file changed, 30 insertions(+), 58 deletions(-) diff --git a/thoth-app/src/component/utils.rs b/thoth-app/src/component/utils.rs index b75a53a2..a1a8aa67 100644 --- a/thoth-app/src/component/utils.rs +++ b/thoth-app/src/component/utils.rs @@ -335,67 +335,39 @@ impl PureComponent for PureTextarea { impl PureComponent for PureTextInputExtended { fn render(&self) -> VNode { // Only display tooltip if its value is set. - // Yew release 0.18.0 will introduce optional attributes - - // at this point we can make `data-tooltip` optional - // and collapse down the duplicated `html!` declaration. - if self.tooltip.is_empty() { - html! { -
- -
- { - // Only display static button if a static text value was provided. - if self.statictext.is_empty() { - html! {} - } else { - html! { - - } - } - } - -
-
- } - } else { - html! { -
- -
- { - if self.statictext.is_empty() { - html! {} - } else { - html! { - - } + let optional_tooltip = match self.tooltip.is_empty() { + true => None, + false => Some(self.tooltip.clone()), + }; + html! { +
+ +
+ { + // Only display static button if a static text value was provided. + if self.statictext.is_empty() { + html! {} + } else { + html! { + } } - -
+ } +
- } +
} } } From 9f7d54177c94de5516d96ead68cb7f00b4a99119 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Thu, 17 Jun 2021 11:58:30 +0100 Subject: [PATCH 16/19] Make database doi/orcid constraints case-sensitive to prevent manual entry of uppercased protocol/resource name letters (the only letter orcid identifiers can contain is uppercase X, and doi identifier constraints already explicitly specify both a-z and A-Z as acceptable) --- thoth-api/migrations/0.4.1/up.sql | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/thoth-api/migrations/0.4.1/up.sql b/thoth-api/migrations/0.4.1/up.sql index 6e094816..2eb361b0 100644 --- a/thoth-api/migrations/0.4.1/up.sql +++ b/thoth-api/migrations/0.4.1/up.sql @@ -1,21 +1,21 @@ --- Improve validation of ORCID identifiers (include protocol/resource name) +-- Improve validation of ORCID identifiers (include protocol/resource name, make case-sensitive) -- Should be kept in line with Orcid::FromStr, although regex syntax differs slightly ALTER TABLE contributor DROP CONSTRAINT contributor_orcid_check, ADD CONSTRAINT contributor_orcid_check - CHECK (orcid ~* '^https:\/\/orcid\.org\/0000-000(1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$'); + CHECK (orcid ~ '^https:\/\/orcid\.org\/0000-000(1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$'); --- Improve validation of DOI identifiers (add line start marker, escape periods) +-- Improve validation of DOI identifiers (add line start marker, escape periods, make case-sensitive) -- Should be kept in line with Orcid::FromStr, although regex syntax differs slightly -- (e.g. `;()/` need to be escaped here but not in Orcid::FromStr) ALTER TABLE work DROP CONSTRAINT work_doi_check, ADD CONSTRAINT work_doi_check - CHECK (doi ~* '^https:\/\/doi\.org\/10\.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$'); + CHECK (doi ~ '^https:\/\/doi\.org\/10\.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$'); ALTER TABLE funder DROP CONSTRAINT funder_funder_doi_check, ADD CONSTRAINT funder_funder_doi_check - CHECK (funder_doi ~* '^https:\/\/doi\.org\/10\.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$'); + CHECK (funder_doi ~ '^https:\/\/doi\.org\/10\.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$'); From 69531de1eb923cfbd047c014d17c5707c7b82e8e Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Fri, 18 Jun 2021 17:18:20 +0100 Subject: [PATCH 17/19] Review markups: allow additional protocol/resource variants, improve formatting/errors --- thoth-api/src/errors.rs | 11 +- thoth-api/src/graphql/model.rs | 2 +- thoth-api/src/model/mod.rs | 140 +++++++++++++-------- thoth-api/src/work/crud.rs | 6 +- thoth-app/src/component/contributor.rs | 10 +- thoth-app/src/component/funder.rs | 11 +- thoth-app/src/component/new_contributor.rs | 10 +- thoth-app/src/component/new_funder.rs | 11 +- thoth-app/src/component/new_work.rs | 10 +- thoth-app/src/component/work.rs | 10 +- 10 files changed, 133 insertions(+), 88 deletions(-) diff --git a/thoth-api/src/errors.rs b/thoth-api/src/errors.rs index 2a27cd81..5b4325d1 100644 --- a/thoth-api/src/errors.rs +++ b/thoth-api/src/errors.rs @@ -32,10 +32,15 @@ pub enum ThothError { #[fail(display = "Could not generate {}: {}", _0, _1)] IncompleteMetadataRecord(String, String), #[fail( - display = "{} is not a validly formatted {} and will not be saved", - _0, _1 + display = "{} is not a validly formatted ORCID and will not be saved", + _0 )] - IdentifierParseError(String, String), + OrcidParseError(String), + #[fail( + display = "{} is not a validly formatted DOI and will not be saved", + _0 + )] + DoiParseError(String), } impl juniper::IntoFieldError for ThothError { diff --git a/thoth-api/src/graphql/model.rs b/thoth-api/src/graphql/model.rs index da4c289b..d527ad3b 100644 --- a/thoth-api/src/graphql/model.rs +++ b/thoth-api/src/graphql/model.rs @@ -151,7 +151,7 @@ impl QueryRoot { } #[graphql(description = "Query a single work using its DOI")] - fn work_by_doi(context: &Context, doi: String) -> FieldResult { + fn work_by_doi(context: &Context, doi: Doi) -> FieldResult { Work::from_doi(&context.db, doi).map_err(|e| e.into()) } diff --git a/thoth-api/src/model/mod.rs b/thoth-api/src/model/mod.rs index 8f5ae6ff..3cb31997 100644 --- a/thoth-api/src/model/mod.rs +++ b/thoth-api/src/model/mod.rs @@ -12,46 +12,47 @@ pub const ORCID_DOMAIN: &str = "https://orcid.org/"; #[cfg_attr( feature = "backend", - derive(DieselNewType, juniper::GraphQLScalarValue) + derive(DieselNewType, juniper::GraphQLScalarValue), + graphql( + description = r#"Digital Object Identifier. Expressed as `^https:\/\/doi\.org\/10\.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$`"# + ), )] #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Doi(String); #[cfg_attr( feature = "backend", - derive(DieselNewType, juniper::GraphQLScalarValue) + derive(DieselNewType, juniper::GraphQLScalarValue), + graphql( + description = r#"ORCID (Open Researcher and Contributor ID) identifier. Expressed as `^https:\/\/orcid\.org\/0000-000(1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$`"# + ) )] #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Orcid(String); #[cfg_attr( feature = "backend", - derive(DieselNewType, juniper::GraphQLScalarValue) + derive(DieselNewType, juniper::GraphQLScalarValue), + graphql(description = "RFC 3339 combined date and time in UTC time zone") )] #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Timestamp(DateTime); impl Default for Doi { fn default() -> Doi { - Doi { - 0: Default::default(), - } + Doi(Default::default()) } } impl Default for Orcid { fn default() -> Orcid { - Orcid { - 0: Default::default(), - } + Orcid(Default::default()) } } impl Default for Timestamp { fn default() -> Timestamp { - Timestamp { - 0: TimeZone::timestamp(&Utc, 0, 0), - } + Timestamp(TimeZone::timestamp(&Utc, 0, 0)) } } @@ -80,34 +81,28 @@ impl FromStr for Doi { use lazy_static::lazy_static; use regex::Regex; lazy_static! { - static ref RE: Regex = Regex::new( + static ref RE: Regex = Regex::new( // ^ = beginning of string // (?:) = non-capturing group // i = case-insensitive flag // $ = end of string - // Matches strings of format "[[http[s]://]doi.org/]10.XXX/XXX" + // Matches strings of format "[[http[s]://][www.][dx.]doi.org/]10.XXX/XXX" // and captures the identifier segment starting with the "10." directory indicator // Corresponds to database constraints although regex syntax differs slightly // (e.g. `;()/` do not need to be escaped here) - r#"^(?i:(?:https?://)?doi\.org/)?(10\.\d{4,9}/[-._;()/:a-zA-Z0-9]+$)"#).unwrap(); + r#"^(?i:(?:https?://)?(?:www\.)?(?:dx\.)?doi\.org/)?(10\.\d{4,9}/[-._;()/:a-zA-Z0-9]+$)"#).unwrap(); } if let Some(matches) = RE.captures(input) { // The 0th capture always corresponds to the entire match if let Some(identifier) = matches.get(1) { let standardised = format!("{}{}", DOI_DOMAIN, identifier.as_str()); - let doi: Doi = Doi { 0: standardised }; + let doi: Doi = Doi(standardised); Ok(doi) } else { - Err(ThothError::IdentifierParseError( - input.to_string(), - "DOI".to_string(), - )) + Err(ThothError::DoiParseError(input.to_string())) } } else { - Err(ThothError::IdentifierParseError( - input.to_string(), - "DOI".to_string(), - )) + Err(ThothError::DoiParseError(input.to_string())) } } } @@ -124,32 +119,32 @@ impl FromStr for Orcid { // (?:) = non-capturing group // i = case-insensitive flag // $ = end of string - // Matches strings of format "[[http[s]://]orcid.org/]0000-000X-XXXX-XXXX" + // Matches strings of format "[[http[s]://][www.]orcid.org/]0000-000X-XXXX-XXXX" // and captures the 16-digit identifier segment // Corresponds to database constraints although regex syntax differs slightly - r#"^(?i:(?:https?://)?orcid\.org/)?(0000-000(?:1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$)"#).unwrap(); + r#"^(?i:(?:https?://)?(?:www\.)?orcid\.org/)?(0000-000(?:1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$)"#).unwrap(); } if let Some(matches) = RE.captures(input) { // The 0th capture always corresponds to the entire match if let Some(identifier) = matches.get(1) { let standardised = format!("{}{}", ORCID_DOMAIN, identifier.as_str()); - let orcid: Orcid = Orcid { 0: standardised }; + let orcid: Orcid = Orcid(standardised); Ok(orcid) } else { - Err(ThothError::IdentifierParseError( - input.to_string(), - "ORCID".to_string(), - )) + Err(ThothError::OrcidParseError(input.to_string())) } } else { - Err(ThothError::IdentifierParseError( - input.to_string(), - "ORCID".to_string(), - )) + Err(ThothError::OrcidParseError(input.to_string())) } } } +impl Doi { + pub fn to_lowercase_string(&self) -> String { + self.0.to_lowercase() + } +} + #[cfg(feature = "backend")] #[allow(clippy::too_many_arguments)] /// Common functionality to perform basic CRUD actions on Thoth entities @@ -371,39 +366,30 @@ macro_rules! db_insert { #[test] fn test_doi_default() { let doi: Doi = Default::default(); - assert_eq!(doi, Doi { 0: "".to_string() }); + assert_eq!(doi, Doi("".to_string())); } #[test] fn test_orcid_default() { let orcid: Orcid = Default::default(); - assert_eq!(orcid, Orcid { 0: "".to_string() }); + assert_eq!(orcid, Orcid("".to_string())); } #[test] fn test_timestamp_default() { let stamp: Timestamp = Default::default(); - assert_eq!( - stamp, - Timestamp { - 0: TimeZone::timestamp(&Utc, 0, 0) - } - ); + assert_eq!(stamp, Timestamp(TimeZone::timestamp(&Utc, 0, 0))); } #[test] fn test_doi_display() { - let doi = Doi { - 0: "https://doi.org/10.12345/Test-Suffix.01".to_string(), - }; + let doi = Doi("https://doi.org/10.12345/Test-Suffix.01".to_string()); assert_eq!(format!("{}", doi), "10.12345/Test-Suffix.01"); } #[test] fn test_orcid_display() { - let orcid = Orcid { - 0: "https://orcid.org/0000-0002-1234-5678".to_string(), - }; + let orcid = Orcid("https://orcid.org/0000-0002-1234-5678".to_string()); assert_eq!(format!("{}", orcid), "0000-0002-1234-5678"); } @@ -415,9 +401,7 @@ fn test_timestamp_display() { #[test] fn test_doi_fromstr() { - let standardised = Doi { - 0: "https://doi.org/10.12345/Test-Suffix.01".to_string(), - }; + let standardised = Doi("https://doi.org/10.12345/Test-Suffix.01".to_string()); assert_eq!( Doi::from_str("https://doi.org/10.12345/Test-Suffix.01").unwrap(), standardised @@ -442,6 +426,42 @@ fn test_doi_fromstr() { Doi::from_str("Https://DOI.org/10.12345/Test-Suffix.01").unwrap(), standardised ); + assert_eq!( + Doi::from_str("https://www.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("http://www.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("www.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("https://dx.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("http://dx.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("dx.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("https://www.dx.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("http://www.dx.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); + assert_eq!( + Doi::from_str("www.dx.doi.org/10.12345/Test-Suffix.01").unwrap(), + standardised + ); assert!(Doi::from_str("htts://doi.org/10.12345/Test-Suffix.01").is_err()); assert!(Doi::from_str("https://10.12345/Test-Suffix.01").is_err()); assert!(Doi::from_str("https://test.org/10.12345/Test-Suffix.01").is_err()); @@ -454,9 +474,7 @@ fn test_doi_fromstr() { #[test] fn test_orcid_fromstr() { - let standardised = Orcid { - 0: "https://orcid.org/0000-0002-1234-5678".to_string(), - }; + let standardised = Orcid("https://orcid.org/0000-0002-1234-5678".to_string()); assert_eq!( Orcid::from_str("https://orcid.org/0000-0002-1234-5678").unwrap(), standardised @@ -481,6 +499,18 @@ fn test_orcid_fromstr() { Orcid::from_str("Https://ORCiD.org/0000-0002-1234-5678").unwrap(), standardised ); + assert_eq!( + Orcid::from_str("https://www.orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("http://www.orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); + assert_eq!( + Orcid::from_str("www.orcid.org/0000-0002-1234-5678").unwrap(), + standardised + ); assert!(Orcid::from_str("htts://orcid.org/0000-0002-1234-5678").is_err()); assert!(Orcid::from_str("https://0000-0002-1234-5678").is_err()); assert!(Orcid::from_str("https://test.org/0000-0002-1234-5678").is_err()); diff --git a/thoth-api/src/work/crud.rs b/thoth-api/src/work/crud.rs index b27bbf23..f2906e8c 100644 --- a/thoth-api/src/work/crud.rs +++ b/thoth-api/src/work/crud.rs @@ -4,7 +4,7 @@ use super::model::{ }; use crate::errors::{ThothError, ThothResult}; use crate::graphql::utils::Direction; -use crate::model::{Crud, DbInsert, HistoryEntry}; +use crate::model::{Crud, DbInsert, Doi, HistoryEntry}; use crate::schema::{work, work_history}; use crate::{crud_methods, db_insert}; use diesel::{ @@ -13,14 +13,14 @@ use diesel::{ use uuid::Uuid; impl Work { - pub fn from_doi(db: &crate::db::PgPool, doi: String) -> ThothResult { + pub fn from_doi(db: &crate::db::PgPool, doi: Doi) -> ThothResult { use diesel::sql_types::Nullable; use diesel::sql_types::Text; let connection = db.get().unwrap(); // Allow case-insensitive searching (DOIs in database may have mixed casing) sql_function!(fn lower(x: Nullable) -> Nullable); crate::schema::work::dsl::work - .filter(lower(crate::schema::work::dsl::doi).eq(doi.to_lowercase())) + .filter(lower(crate::schema::work::dsl::doi).eq(doi.to_lowercase_string())) .get_result::(&connection) .map_err(|e| e.into()) } diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index e540b270..fd18a8fa 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -306,10 +306,12 @@ impl Component for ContributorComponent { // If no ORCID was provided, no format check is required. // Don't update self.contributor.orcid yet, as user may later // overwrite a new valid value with an invalid one. - if !self.orcid.is_empty() && self.orcid.parse::().is_err() { - self.orcid_warning = self.orcid.parse::().unwrap_err().to_string(); - } else { - self.orcid_warning.clear(); + self.orcid_warning.clear(); + if !self.orcid.is_empty() { + match self.orcid.parse::() { + Err(e) => self.orcid_warning = e.to_string(), + Ok(value) => self.orcid = value.to_string(), + } } true } else { diff --git a/thoth-app/src/component/funder.rs b/thoth-app/src/component/funder.rs index 4337e4d0..22ff2eed 100644 --- a/thoth-app/src/component/funder.rs +++ b/thoth-app/src/component/funder.rs @@ -286,11 +286,12 @@ impl Component for FunderComponent { // If no DOI was provided, no format check is required. // Don't update self.funder.funder_doi yet, as user may later // overwrite a new valid value with an invalid one. - if !self.funder_doi.is_empty() && self.funder_doi.parse::().is_err() { - self.funder_doi_warning = - self.funder_doi.parse::().unwrap_err().to_string(); - } else { - self.funder_doi_warning.clear(); + self.funder_doi_warning.clear(); + if !self.funder_doi.is_empty() { + match self.funder_doi.parse::() { + Err(e) => self.funder_doi_warning = e.to_string(), + Ok(value) => self.funder_doi = value.to_string(), + } } true } else { diff --git a/thoth-app/src/component/new_contributor.rs b/thoth-app/src/component/new_contributor.rs index d021a6a6..99ea127f 100644 --- a/thoth-app/src/component/new_contributor.rs +++ b/thoth-app/src/component/new_contributor.rs @@ -222,10 +222,12 @@ impl Component for NewContributorComponent { // If no ORCID was provided, no format check is required. // Don't update self.contributor.orcid yet, as user may later // overwrite a new valid value with an invalid one. - if !self.orcid.is_empty() && self.orcid.parse::().is_err() { - self.orcid_warning = self.orcid.parse::().unwrap_err().to_string(); - } else { - self.orcid_warning.clear(); + self.orcid_warning.clear(); + if !self.orcid.is_empty() { + match self.orcid.parse::() { + Err(e) => self.orcid_warning = e.to_string(), + Ok(value) => self.orcid = value.to_string(), + } } true } else { diff --git a/thoth-app/src/component/new_funder.rs b/thoth-app/src/component/new_funder.rs index 46dc2d48..4e50064d 100644 --- a/thoth-app/src/component/new_funder.rs +++ b/thoth-app/src/component/new_funder.rs @@ -138,11 +138,12 @@ impl Component for NewFunderComponent { // If no DOI was provided, no format check is required. // Don't update self.funder.funder_doi yet, as user may later // overwrite a new valid value with an invalid one. - if !self.funder_doi.is_empty() && self.funder_doi.parse::().is_err() { - self.funder_doi_warning = - self.funder_doi.parse::().unwrap_err().to_string(); - } else { - self.funder_doi_warning.clear(); + self.funder_doi_warning.clear(); + if !self.funder_doi.is_empty() { + match self.funder_doi.parse::() { + Err(e) => self.funder_doi_warning = e.to_string(), + Ok(value) => self.funder_doi = value.to_string(), + } } true } else { diff --git a/thoth-app/src/component/new_work.rs b/thoth-app/src/component/new_work.rs index e5890990..4f2da4ad 100644 --- a/thoth-app/src/component/new_work.rs +++ b/thoth-app/src/component/new_work.rs @@ -349,10 +349,12 @@ impl Component for NewWorkComponent { // If no DOI was provided, no format check is required. // Don't update self.work.doi yet, as user may later // overwrite a new valid value with an invalid one. - if !self.doi.is_empty() && self.doi.parse::().is_err() { - self.doi_warning = self.doi.parse::().unwrap_err().to_string(); - } else { - self.doi_warning.clear(); + self.doi_warning.clear(); + if !self.doi.is_empty() { + match self.doi.parse::() { + Err(e) => self.doi_warning = e.to_string(), + Ok(value) => self.doi = value.to_string(), + } } true } else { diff --git a/thoth-app/src/component/work.rs b/thoth-app/src/component/work.rs index f0613c8e..b41d5b0e 100644 --- a/thoth-app/src/component/work.rs +++ b/thoth-app/src/component/work.rs @@ -416,10 +416,12 @@ impl Component for WorkComponent { // If no DOI was provided, no format check is required. // Don't update self.work.doi yet, as user may later // overwrite a new valid value with an invalid one. - if !self.doi.is_empty() && self.doi.parse::().is_err() { - self.doi_warning = self.doi.parse::().unwrap_err().to_string(); - } else { - self.doi_warning.clear(); + self.doi_warning.clear(); + if !self.doi.is_empty() { + match self.doi.parse::() { + Err(e) => self.doi_warning = e.to_string(), + Ok(value) => self.doi = value.to_string(), + } } true } else { From 514d60d401535d258a1c287f0e3ac15c7cbd5e71 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Mon, 21 Jun 2021 11:11:01 +0100 Subject: [PATCH 18/19] Move Doi/Orcid empty check to parsing, and ignore empty error in interface --- thoth-api/src/errors.rs | 4 ++++ thoth-api/src/model/mod.rs | 10 +++++++--- thoth-app/src/component/contributor.rs | 14 +++++++++----- thoth-app/src/component/funder.rs | 14 +++++++++----- thoth-app/src/component/new_contributor.rs | 14 +++++++++----- thoth-app/src/component/new_funder.rs | 14 +++++++++----- thoth-app/src/component/new_work.rs | 14 +++++++++----- thoth-app/src/component/work.rs | 14 +++++++++----- 8 files changed, 65 insertions(+), 33 deletions(-) diff --git a/thoth-api/src/errors.rs b/thoth-api/src/errors.rs index 5b4325d1..9a893ec3 100644 --- a/thoth-api/src/errors.rs +++ b/thoth-api/src/errors.rs @@ -41,6 +41,10 @@ pub enum ThothError { _0 )] DoiParseError(String), + #[fail(display = "Cannot parse ORCID: no value provided")] + OrcidEmptyError, + #[fail(display = "Cannot parse DOI: no value provided")] + DoiEmptyError, } impl juniper::IntoFieldError for ThothError { diff --git a/thoth-api/src/model/mod.rs b/thoth-api/src/model/mod.rs index 3cb31997..9a485578 100644 --- a/thoth-api/src/model/mod.rs +++ b/thoth-api/src/model/mod.rs @@ -15,7 +15,7 @@ pub const ORCID_DOMAIN: &str = "https://orcid.org/"; derive(DieselNewType, juniper::GraphQLScalarValue), graphql( description = r#"Digital Object Identifier. Expressed as `^https:\/\/doi\.org\/10\.\d{4,9}\/[-._\;\(\)\/:a-zA-Z0-9]+$`"# - ), + ) )] #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Doi(String); @@ -92,7 +92,9 @@ impl FromStr for Doi { // (e.g. `;()/` do not need to be escaped here) r#"^(?i:(?:https?://)?(?:www\.)?(?:dx\.)?doi\.org/)?(10\.\d{4,9}/[-._;()/:a-zA-Z0-9]+$)"#).unwrap(); } - if let Some(matches) = RE.captures(input) { + if input.is_empty() { + Err(ThothError::DoiEmptyError) + } else if let Some(matches) = RE.captures(input) { // The 0th capture always corresponds to the entire match if let Some(identifier) = matches.get(1) { let standardised = format!("{}{}", DOI_DOMAIN, identifier.as_str()); @@ -124,7 +126,9 @@ impl FromStr for Orcid { // Corresponds to database constraints although regex syntax differs slightly r#"^(?i:(?:https?://)?(?:www\.)?orcid\.org/)?(0000-000(?:1-[5-9]|2-[0-9]|3-[0-4])\d{3}-\d{3}[\dX]$)"#).unwrap(); } - if let Some(matches) = RE.captures(input) { + if input.is_empty() { + Err(ThothError::OrcidEmptyError) + } else if let Some(matches) = RE.captures(input) { // The 0th capture always corresponds to the entire match if let Some(identifier) = matches.get(1) { let standardised = format!("{}{}", ORCID_DOMAIN, identifier.as_str()); diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index fd18a8fa..ca8ee848 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -1,4 +1,5 @@ use thoth_api::contributor::model::Contributor; +use thoth_api::errors::ThothError; use thoth_api::model::{Orcid, ORCID_DOMAIN}; use uuid::Uuid; use yew::html; @@ -303,15 +304,18 @@ impl Component for ContributorComponent { Msg::ChangeOrcid(value) => { if self.orcid.neq_assign(value.trim().to_owned()) { // If ORCID is not correctly formatted, display a warning. - // If no ORCID was provided, no format check is required. // Don't update self.contributor.orcid yet, as user may later // overwrite a new valid value with an invalid one. self.orcid_warning.clear(); - if !self.orcid.is_empty() { - match self.orcid.parse::() { - Err(e) => self.orcid_warning = e.to_string(), - Ok(value) => self.orcid = value.to_string(), + match self.orcid.parse::() { + Err(e) => { + match e { + // If no ORCID was provided, no warning is required. + ThothError::OrcidEmptyError => {} + _ => self.orcid_warning = e.to_string(), + } } + Ok(value) => self.orcid = value.to_string(), } true } else { diff --git a/thoth-app/src/component/funder.rs b/thoth-app/src/component/funder.rs index 22ff2eed..6e826a3a 100644 --- a/thoth-app/src/component/funder.rs +++ b/thoth-app/src/component/funder.rs @@ -1,3 +1,4 @@ +use thoth_api::errors::ThothError; use thoth_api::funder::model::Funder; use thoth_api::model::{Doi, DOI_DOMAIN}; use uuid::Uuid; @@ -283,15 +284,18 @@ impl Component for FunderComponent { Msg::ChangeFunderDoi(value) => { if self.funder_doi.neq_assign(value.trim().to_owned()) { // If DOI is not correctly formatted, display a warning. - // If no DOI was provided, no format check is required. // Don't update self.funder.funder_doi yet, as user may later // overwrite a new valid value with an invalid one. self.funder_doi_warning.clear(); - if !self.funder_doi.is_empty() { - match self.funder_doi.parse::() { - Err(e) => self.funder_doi_warning = e.to_string(), - Ok(value) => self.funder_doi = value.to_string(), + match self.funder_doi.parse::() { + Err(e) => { + match e { + // If no DOI was provided, no warning is required. + ThothError::DoiEmptyError => {} + _ => self.funder_doi_warning = e.to_string(), + } } + Ok(value) => self.funder_doi = value.to_string(), } true } else { diff --git a/thoth-app/src/component/new_contributor.rs b/thoth-app/src/component/new_contributor.rs index 99ea127f..f976d686 100644 --- a/thoth-app/src/component/new_contributor.rs +++ b/thoth-app/src/component/new_contributor.rs @@ -1,4 +1,5 @@ use thoth_api::contributor::model::Contributor; +use thoth_api::errors::ThothError; use thoth_api::model::{Orcid, ORCID_DOMAIN}; use yew::html; use yew::prelude::*; @@ -219,15 +220,18 @@ impl Component for NewContributorComponent { Msg::ChangeOrcid(value) => { if self.orcid.neq_assign(value.trim().to_owned()) { // If ORCID is not correctly formatted, display a warning. - // If no ORCID was provided, no format check is required. // Don't update self.contributor.orcid yet, as user may later // overwrite a new valid value with an invalid one. self.orcid_warning.clear(); - if !self.orcid.is_empty() { - match self.orcid.parse::() { - Err(e) => self.orcid_warning = e.to_string(), - Ok(value) => self.orcid = value.to_string(), + match self.orcid.parse::() { + Err(e) => { + match e { + // If no ORCID was provided, no warning is required. + ThothError::OrcidEmptyError => {} + _ => self.orcid_warning = e.to_string(), + } } + Ok(value) => self.orcid = value.to_string(), } true } else { diff --git a/thoth-app/src/component/new_funder.rs b/thoth-app/src/component/new_funder.rs index 4e50064d..0ed8d7e7 100644 --- a/thoth-app/src/component/new_funder.rs +++ b/thoth-app/src/component/new_funder.rs @@ -1,3 +1,4 @@ +use thoth_api::errors::ThothError; use thoth_api::funder::model::Funder; use thoth_api::model::{Doi, DOI_DOMAIN}; use yew::html; @@ -135,15 +136,18 @@ impl Component for NewFunderComponent { Msg::ChangeFunderDoi(value) => { if self.funder_doi.neq_assign(value.trim().to_owned()) { // If DOI is not correctly formatted, display a warning. - // If no DOI was provided, no format check is required. // Don't update self.funder.funder_doi yet, as user may later // overwrite a new valid value with an invalid one. self.funder_doi_warning.clear(); - if !self.funder_doi.is_empty() { - match self.funder_doi.parse::() { - Err(e) => self.funder_doi_warning = e.to_string(), - Ok(value) => self.funder_doi = value.to_string(), + match self.funder_doi.parse::() { + Err(e) => { + match e { + // If no DOI was provided, no warning is required. + ThothError::DoiEmptyError => {} + _ => self.funder_doi_warning = e.to_string(), + } } + Ok(value) => self.funder_doi = value.to_string(), } true } else { diff --git a/thoth-app/src/component/new_work.rs b/thoth-app/src/component/new_work.rs index 4f2da4ad..ea303818 100644 --- a/thoth-app/src/component/new_work.rs +++ b/thoth-app/src/component/new_work.rs @@ -1,5 +1,6 @@ use std::str::FromStr; use thoth_api::account::model::AccountDetails; +use thoth_api::errors::ThothError; use thoth_api::imprint::model::ImprintExtended as Imprint; use thoth_api::model::{Doi, DOI_DOMAIN}; use thoth_api::work::model::WorkExtended as Work; @@ -346,15 +347,18 @@ impl Component for NewWorkComponent { Msg::ChangeDoi(value) => { if self.doi.neq_assign(value.trim().to_owned()) { // If DOI is not correctly formatted, display a warning. - // If no DOI was provided, no format check is required. // Don't update self.work.doi yet, as user may later // overwrite a new valid value with an invalid one. self.doi_warning.clear(); - if !self.doi.is_empty() { - match self.doi.parse::() { - Err(e) => self.doi_warning = e.to_string(), - Ok(value) => self.doi = value.to_string(), + match self.doi.parse::() { + Err(e) => { + match e { + // If no DOI was provided, no warning is required. + ThothError::DoiEmptyError => {} + _ => self.doi_warning = e.to_string(), + } } + Ok(value) => self.doi = value.to_string(), } true } else { diff --git a/thoth-app/src/component/work.rs b/thoth-app/src/component/work.rs index b41d5b0e..59a453e0 100644 --- a/thoth-app/src/component/work.rs +++ b/thoth-app/src/component/work.rs @@ -1,6 +1,7 @@ use std::str::FromStr; use thoth_api::account::model::AccountDetails; use thoth_api::contribution::model::Contribution; +use thoth_api::errors::ThothError; use thoth_api::funding::model::FundingExtended as Funding; use thoth_api::imprint::model::ImprintExtended as Imprint; use thoth_api::issue::model::IssueExtended as Issue; @@ -413,15 +414,18 @@ impl Component for WorkComponent { Msg::ChangeDoi(value) => { if self.doi.neq_assign(value.trim().to_owned()) { // If DOI is not correctly formatted, display a warning. - // If no DOI was provided, no format check is required. // Don't update self.work.doi yet, as user may later // overwrite a new valid value with an invalid one. self.doi_warning.clear(); - if !self.doi.is_empty() { - match self.doi.parse::() { - Err(e) => self.doi_warning = e.to_string(), - Ok(value) => self.doi = value.to_string(), + match self.doi.parse::() { + Err(e) => { + match e { + // If no DOI was provided, no warning is required. + ThothError::DoiEmptyError => {} + _ => self.doi_warning = e.to_string(), + } } + Ok(value) => self.doi = value.to_string(), } true } else { From b6641b0aa3c3e5266eaf66764df15cf784481c55 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Mon, 21 Jun 2021 15:14:21 +0100 Subject: [PATCH 19/19] Fix build error due to app/api dependencies following merge --- thoth-api/src/model/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/thoth-api/src/model/mod.rs b/thoth-api/src/model/mod.rs index e9a7179f..b97c8473 100644 --- a/thoth-api/src/model/mod.rs +++ b/thoth-api/src/model/mod.rs @@ -2,7 +2,6 @@ use chrono::{DateTime, TimeZone, Utc}; use serde::{Deserialize, Serialize}; use std::fmt; use std::str::FromStr; -#[cfg(feature = "backend")] use thoth_errors::{ThothError, ThothResult}; #[cfg(feature = "backend")] use uuid::Uuid;