Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom Isbn struct and use it to auto-hyphenate and warn of parsing errors as user types (#125) #262

Merged
merged 2 commits into from
Jul 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions thoth-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
3 changes: 2 additions & 1 deletion thoth-api/src/graphql/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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()
}

Expand Down
77 changes: 77 additions & 0 deletions thoth-api/src/model/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use chrono::{DateTime, TimeZone, Utc};
use isbn2::Isbn13;
use serde::{Deserialize, Serialize};
use std::fmt;
use std::str::FromStr;
Expand All @@ -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),
Expand All @@ -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())
Expand All @@ -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, ""))
Expand Down Expand Up @@ -108,6 +131,24 @@ impl FromStr for Doi {
}
}

impl FromStr for Isbn {
type Err = ThothError;

fn from_str(input: &str) -> ThothResult<Isbn> {
if input.is_empty() {
Err(ThothError::IsbnEmptyError)
} else {
match input.parse::<Isbn13>() {
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;

Expand Down Expand Up @@ -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();
Expand All @@ -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());
Expand Down Expand Up @@ -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());
Expand Down
9 changes: 5 additions & 4 deletions thoth-api/src/publication/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -65,7 +66,7 @@ pub struct Publication {
pub publication_id: Uuid,
pub publication_type: PublicationType,
pub work_id: Uuid,
pub isbn: Option<String>,
pub isbn: Option<Isbn>,
pub publication_url: Option<String>,
pub created_at: Timestamp,
pub updated_at: Timestamp,
Expand All @@ -77,7 +78,7 @@ pub struct PublicationWithRelations {
pub publication_id: Uuid,
pub publication_type: PublicationType,
pub work_id: Uuid,
pub isbn: Option<String>,
pub isbn: Option<Isbn>,
pub publication_url: Option<String>,
pub updated_at: Timestamp,
pub prices: Option<Vec<Price>>,
Expand All @@ -92,7 +93,7 @@ pub struct PublicationWithRelations {
pub struct NewPublication {
pub publication_type: PublicationType,
pub work_id: Uuid,
pub isbn: Option<String>,
pub isbn: Option<Isbn>,
pub publication_url: Option<String>,
}

Expand All @@ -106,7 +107,7 @@ pub struct PatchPublication {
pub publication_id: Uuid,
pub publication_type: PublicationType,
pub work_id: Uuid,
pub isbn: Option<String>,
pub isbn: Option<Isbn>,
pub publication_url: Option<String>,
}

Expand Down
5 changes: 3 additions & 2 deletions thoth-app/src/component/publication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -236,7 +237,7 @@ impl Component for PublicationComponent {
<div class="field">
<label class="label">{ "ISBN" }</label>
<div class="control is-expanded">
{&self.publication.isbn.clone().unwrap_or_else(|| "".to_string())}
{&self.publication.isbn.as_ref().map(|s| s.to_string()).unwrap_or_else(|| "".to_string())}
</div>
</div>

Expand Down
Loading