diff --git a/Cargo.lock b/Cargo.lock index 7bae1fa9..a635ca49 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -853,6 +853,15 @@ dependencies = [ "bitflags", ] +[[package]] +name = "codegen" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34c59e8a9b988977ec3bd61ab380cf1167048817ecd3d6999fac03657f85a609" +dependencies = [ + "indexmap", +] + [[package]] name = "combine" version = "3.8.1" @@ -2040,6 +2049,17 @@ dependencies = [ "winapi 0.3.8", ] +[[package]] +name = "isbn2" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32ec02acd9e12bee98c320d8ff9fe1c8882174067ac7932866f7ad7bf5309cd8" +dependencies = [ + "arrayvec 0.5.2", + "codegen", + "roxmltree", +] + [[package]] name = "itertools" version = "0.9.0" @@ -3291,6 +3311,15 @@ dependencies = [ "winapi 0.3.8", ] +[[package]] +name = "roxmltree" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbf7d7b1ea646d380d0e8153158063a6da7efe30ddbf3184042848e3f8a6f671" +dependencies = [ + "xmlparser", +] + [[package]] name = "rust-argon2" version = "0.8.3" @@ -3934,6 +3963,7 @@ dependencies = [ "diesel_migrations", "dotenv", "futures 0.3.5", + "isbn2", "jsonwebtoken", "juniper", "lazy_static 1.4.0", @@ -4841,6 +4871,12 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "541b12c998c5b56aa2b4e6f18f03664eef9a4fd0a246a55594efae6cc2d964b5" +[[package]] +name = "xmlparser" +version = "0.13.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "114ba2b24d2167ef6d67d7d04c8cc86522b87f490025f39f0303b7db5bf5e3d8" + [[package]] name = "yaml-rust" version = "0.4.4" diff --git a/thoth-api/Cargo.toml b/thoth-api/Cargo.toml index 42e2106d..5477a8dc 100644 --- a/thoth-api/Cargo.toml +++ b/thoth-api/Cargo.toml @@ -19,6 +19,7 @@ backend = ["diesel", "diesel-derive-enum", "diesel_migrations", "futures", "acti thoth-errors = { version = "0.4.1", path = "../thoth-errors" } actix-web = { version = "3.3.2", optional = true } argon2rs = "0.2.5" +isbn2 = "0.4.0" 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 } diff --git a/thoth-api/src/graphql/model.rs b/thoth-api/src/graphql/model.rs index e8df777d..51e846e8 100644 --- a/thoth-api/src/graphql/model.rs +++ b/thoth-api/src/graphql/model.rs @@ -16,6 +16,7 @@ use crate::issue::model::*; use crate::language::model::*; use crate::model::Crud; use crate::model::Doi; +use crate::model::Isbn; use crate::model::Orcid; use crate::model::Timestamp; use crate::price::model::*; @@ -1805,7 +1806,7 @@ impl Publication { self.work_id } - pub fn isbn(&self) -> Option<&String> { + pub fn isbn(&self) -> Option<&Isbn> { self.isbn.as_ref() } diff --git a/thoth-api/src/model/mod.rs b/thoth-api/src/model/mod.rs index b97c8473..0c0ad028 100644 --- a/thoth-api/src/model/mod.rs +++ b/thoth-api/src/model/mod.rs @@ -1,4 +1,5 @@ use chrono::{DateTime, TimeZone, Utc}; +use isbn2::Isbn13; use serde::{Deserialize, Serialize}; use std::fmt; use std::str::FromStr; @@ -19,6 +20,16 @@ pub const ORCID_DOMAIN: &str = "https://orcid.org/"; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Doi(String); +#[cfg_attr( + feature = "backend", + derive(DieselNewType, juniper::GraphQLScalarValue), + graphql( + description = "13-digit International Standard Book Number, with its parts separated by hyphens" + ) +)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct Isbn(String); + #[cfg_attr( feature = "backend", derive(DieselNewType, juniper::GraphQLScalarValue), @@ -43,6 +54,12 @@ impl Default for Doi { } } +impl Default for Isbn { + fn default() -> Isbn { + Isbn(Default::default()) + } +} + impl Default for Orcid { fn default() -> Orcid { Orcid(Default::default()) @@ -61,6 +78,12 @@ impl fmt::Display for Doi { } } +impl fmt::Display for Isbn { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.0) + } +} + impl fmt::Display for Orcid { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", &self.0.replace(ORCID_DOMAIN, "")) @@ -108,6 +131,24 @@ impl FromStr for Doi { } } +impl FromStr for Isbn { + type Err = ThothError; + + fn from_str(input: &str) -> ThothResult { + if input.is_empty() { + Err(ThothError::IsbnEmptyError) + } else { + match input.parse::() { + Ok(parsed) => match parsed.hyphenate() { + Ok(hyphenated) => Ok(Isbn(hyphenated.to_string())), + Err(_) => Err(ThothError::IsbnParseError(input.to_string())), + }, + Err(_) => Err(ThothError::IsbnParseError(input.to_string())), + } + } + } +} + impl FromStr for Orcid { type Err = ThothError; @@ -372,6 +413,12 @@ fn test_doi_default() { assert_eq!(doi, Doi("".to_string())); } +#[test] +fn test_isbn_default() { + let isbn: Isbn = Default::default(); + assert_eq!(isbn, Isbn("".to_string())); +} + #[test] fn test_orcid_default() { let orcid: Orcid = Default::default(); @@ -390,6 +437,12 @@ fn test_doi_display() { assert_eq!(format!("{}", doi), "10.12345/Test-Suffix.01"); } +#[test] +fn test_isbn_display() { + let isbn = Isbn("978-3-16-148410-0".to_string()); + assert_eq!(format!("{}", isbn), "978-3-16-148410-0"); +} + #[test] fn test_orcid_display() { let orcid = Orcid("https://orcid.org/0000-0002-1234-5678".to_string()); @@ -475,6 +528,30 @@ fn test_doi_fromstr() { assert!(Doi::from_str("10.https://doi.org/12345/Test-Suffix.01").is_err()); } +#[test] +fn test_isbn_fromstr() { + // Note the `isbn2` crate contains tests of valid/invalid ISBN values - + // this focuses on testing that a valid ISBN in any format is standardised + let standardised = Isbn("978-3-16-148410-0".to_string()); + assert_eq!(Isbn::from_str("978-3-16-148410-0").unwrap(), standardised); + assert_eq!(Isbn::from_str("9783161484100").unwrap(), standardised); + assert_eq!(Isbn::from_str("978 3 16 148410 0").unwrap(), standardised); + assert_eq!(Isbn::from_str("978 3 16-148410-0").unwrap(), standardised); + assert_eq!(Isbn::from_str("9-7-831614-8-4-100").unwrap(), standardised); + assert_eq!( + Isbn::from_str(" 97831 614 84 100 ").unwrap(), + standardised + ); + assert_eq!( + Isbn::from_str("---97--831614----8-4100--").unwrap(), + standardised + ); + assert!(Isbn::from_str("978-3-16-148410-1").is_err()); + assert!(Isbn::from_str("1234567890123").is_err()); + assert!(Isbn::from_str("0-684-84328-5").is_err()); + assert!(Isbn::from_str("abcdef").is_err()); +} + #[test] fn test_orcid_fromstr() { let standardised = Orcid("https://orcid.org/0000-0002-1234-5678".to_string()); diff --git a/thoth-api/src/publication/model.rs b/thoth-api/src/publication/model.rs index 4b359082..1f9013d6 100644 --- a/thoth-api/src/publication/model.rs +++ b/thoth-api/src/publication/model.rs @@ -4,6 +4,7 @@ use strum::EnumString; use uuid::Uuid; use crate::graphql::utils::Direction; +use crate::model::Isbn; use crate::model::Timestamp; use crate::price::model::Price; #[cfg(feature = "backend")] @@ -65,7 +66,7 @@ pub struct Publication { pub publication_id: Uuid, pub publication_type: PublicationType, pub work_id: Uuid, - pub isbn: Option, + pub isbn: Option, pub publication_url: Option, pub created_at: Timestamp, pub updated_at: Timestamp, @@ -77,7 +78,7 @@ pub struct PublicationWithRelations { pub publication_id: Uuid, pub publication_type: PublicationType, pub work_id: Uuid, - pub isbn: Option, + pub isbn: Option, pub publication_url: Option, pub updated_at: Timestamp, pub prices: Option>, @@ -92,7 +93,7 @@ pub struct PublicationWithRelations { pub struct NewPublication { pub publication_type: PublicationType, pub work_id: Uuid, - pub isbn: Option, + pub isbn: Option, pub publication_url: Option, } @@ -106,7 +107,7 @@ pub struct PatchPublication { pub publication_id: Uuid, pub publication_type: PublicationType, pub work_id: Uuid, - pub isbn: Option, + pub isbn: Option, pub publication_url: Option, } diff --git a/thoth-app/src/component/publication.rs b/thoth-app/src/component/publication.rs index deeba261..7c5649c4 100644 --- a/thoth-app/src/component/publication.rs +++ b/thoth-app/src/component/publication.rs @@ -145,7 +145,8 @@ impl Component for PublicationComponent { format!( "Deleted {}", &p.isbn - .clone() + .as_ref() + .map(|s| s.to_string()) .unwrap_or_else(|| p.publication_id.to_string()) ), NotificationStatus::Success, @@ -236,7 +237,7 @@ impl Component for PublicationComponent {
- {&self.publication.isbn.clone().unwrap_or_else(|| "".to_string())} + {&self.publication.isbn.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string())}
diff --git a/thoth-app/src/component/publications_form.rs b/thoth-app/src/component/publications_form.rs index 407c9fa1..b468d28e 100644 --- a/thoth-app/src/component/publications_form.rs +++ b/thoth-app/src/component/publications_form.rs @@ -1,6 +1,8 @@ use std::str::FromStr; +use thoth_api::model::Isbn; use thoth_api::publication::model::Publication; use thoth_api::publication::model::PublicationType; +use thoth_errors::ThothError; use uuid::Uuid; use yew::html; use yew::prelude::*; @@ -19,7 +21,7 @@ use crate::agent::notification_bus::NotificationDispatcher; use crate::agent::notification_bus::NotificationStatus; use crate::agent::notification_bus::Request; use crate::component::utils::FormPublicationTypeSelect; -use crate::component::utils::FormTextInput; +use crate::component::utils::FormTextInputExtended; use crate::component::utils::FormUrlInput; use crate::models::publication::create_publication_mutation::CreatePublicationRequest; use crate::models::publication::create_publication_mutation::CreatePublicationRequestBody; @@ -45,6 +47,9 @@ pub struct PublicationsFormComponent { props: Props, data: PublicationsFormData, new_publication: Publication, + // Track the user-entered ISBN string, which may not be validly formatted + isbn: String, + isbn_warning: String, show_add_form: bool, fetch_publication_types: FetchPublicationTypes, push_publication: PushCreatePublication, @@ -89,6 +94,8 @@ impl Component for PublicationsFormComponent { let data: PublicationsFormData = Default::default(); let show_add_form = false; let new_publication: Publication = Default::default(); + let isbn = Default::default(); + let isbn_warning = Default::default(); let push_publication = Default::default(); let delete_publication = Default::default(); let notification_bus = NotificationBus::dispatcher(); @@ -100,6 +107,8 @@ impl Component for PublicationsFormComponent { props, data, new_publication, + isbn, + isbn_warning, show_add_form, fetch_publication_types: Default::default(), push_publication, @@ -114,6 +123,12 @@ impl Component for PublicationsFormComponent { match msg { Msg::ToggleAddFormDisplay(value) => { self.show_add_form = value; + // Ensure ISBN variables are cleared on re-opening form, + // otherwise a previously-entered valid ISBN value may be + // saved although an invalid value was subsequently entered + self.new_publication.isbn = Default::default(); + self.isbn = Default::default(); + self.isbn_warning = Default::default(); true } Msg::SetPublicationTypesFetchState(fetch_state) => { @@ -170,6 +185,14 @@ impl Component for PublicationsFormComponent { } } Msg::CreatePublication => { + // Only update the ISBN value with the current user-entered string + // if it is validly formatted - otherwise keep the default. + // If no ISBN was provided, no format check is required. + if self.isbn.is_empty() { + self.new_publication.isbn.neq_assign(None); + } else if let Ok(result) = self.isbn.parse::() { + self.new_publication.isbn.neq_assign(Some(result)); + } let body = CreatePublicationRequestBody { variables: Variables { work_id: self.props.work_id, @@ -241,11 +264,25 @@ impl Component for PublicationsFormComponent { self.new_publication.publication_type.neq_assign(val) } Msg::ChangeIsbn(value) => { - let isbn = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.new_publication.isbn.neq_assign(isbn) + if self.isbn.neq_assign(value.trim().to_owned()) { + // If ISBN is not correctly formatted, display a warning. + // Don't update self.new_publication.isbn yet, as user may later + // overwrite a new valid value with an invalid one. + self.isbn_warning.clear(); + match self.isbn.parse::() { + Err(e) => { + match e { + // If no ISBN was provided, no warning is required. + ThothError::IsbnEmptyError => {} + _ => self.isbn_warning = e.to_string(), + } + } + Ok(value) => self.isbn = value.to_string(), + } + true + } else { + false + } } Msg::ChangeUrl(value) => { let url = match value.trim().is_empty() { @@ -322,9 +359,10 @@ impl Component for PublicationsFormComponent { }) required = true /> -
- {&p.isbn.clone().unwrap_or_else(|| "".to_string())} + {&p.isbn.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string())}
diff --git a/thoth-app/src/models/publication/create_publication_mutation.rs b/thoth-app/src/models/publication/create_publication_mutation.rs index 7e286883..2b292d7b 100644 --- a/thoth-app/src/models/publication/create_publication_mutation.rs +++ b/thoth-app/src/models/publication/create_publication_mutation.rs @@ -1,5 +1,6 @@ use serde::Deserialize; use serde::Serialize; +use thoth_api::model::Isbn; use thoth_api::publication::model::Publication; use thoth_api::publication::model::PublicationType; use uuid::Uuid; @@ -8,7 +9,7 @@ const CREATE_PUBLICATION_MUTATION: &str = " mutation CreatePublication( $publicationType: PublicationType!, $workId: Uuid!, - $isbn: String, + $isbn: Isbn, $publicationUrl: String, ) { createPublication(data: { @@ -44,7 +45,7 @@ graphql_query_builder! { pub struct Variables { pub publication_type: PublicationType, pub work_id: Uuid, - pub isbn: Option, + pub isbn: Option, pub publication_url: Option, } diff --git a/thoth-app/src/models/publication/delete_publication_mutation.rs b/thoth-app/src/models/publication/delete_publication_mutation.rs index 61e200e1..82e49353 100644 --- a/thoth-app/src/models/publication/delete_publication_mutation.rs +++ b/thoth-app/src/models/publication/delete_publication_mutation.rs @@ -13,6 +13,7 @@ const DELETE_PUBLICATION_MUTATION: &str = " publicationId publicationType workId + isbn createdAt updatedAt } diff --git a/thoth-app/src/models/publication/mod.rs b/thoth-app/src/models/publication/mod.rs index a58626b4..e9f11a3c 100644 --- a/thoth-app/src/models/publication/mod.rs +++ b/thoth-app/src/models/publication/mod.rs @@ -38,7 +38,11 @@ impl EditRoute for PublicationWithRelations { impl MetadataTable for PublicationWithRelations { fn as_table_row(&self, callback: Callback) -> Html { - let isbn = &self.isbn.clone().unwrap_or_else(|| "".to_string()); + let isbn = self + .isbn + .as_ref() + .map(|s| s.to_string()) + .unwrap_or_else(|| "".to_string()); let doi = self .work .doi diff --git a/thoth-errors/src/lib.rs b/thoth-errors/src/lib.rs index 507bc030..45b1633f 100644 --- a/thoth-errors/src/lib.rs +++ b/thoth-errors/src/lib.rs @@ -42,10 +42,17 @@ pub enum ThothError { _0 )] DoiParseError(String), + #[fail( + display = "{} is not a validly formatted ISBN and will not be saved", + _0 + )] + IsbnParseError(String), #[fail(display = "Cannot parse ORCID: no value provided")] OrcidEmptyError, #[fail(display = "Cannot parse DOI: no value provided")] DoiEmptyError, + #[fail(display = "Cannot parse ISBN: no value provided")] + IsbnEmptyError, } #[cfg(not(target_arch = "wasm32"))]