From 9a3ca1c0ccf826040ebf57ea086827f1d65a6b62 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Mon, 27 Sep 2021 15:03:29 +0100 Subject: [PATCH 01/38] Expand KBART test to cover more code paths --- thoth-export-server/src/csv/kbart_oclc.rs | 166 +++++++++++++++++++--- 1 file changed, 146 insertions(+), 20 deletions(-) diff --git a/thoth-export-server/src/csv/kbart_oclc.rs b/thoth-export-server/src/csv/kbart_oclc.rs index a944d804..2b4adce0 100644 --- a/thoth-export-server/src/csv/kbart_oclc.rs +++ b/thoth-export-server/src/csv/kbart_oclc.rs @@ -175,7 +175,7 @@ mod tests { use super::*; use crate::record::DELIMITER_TAB; use csv::QuoteStyle; - use lazy_static::lazy_static; + use std::fmt; use std::str::FromStr; use thoth_api::model::Doi; use thoth_api::model::Isbn; @@ -187,8 +187,50 @@ mod tests { }; use uuid::Uuid; - lazy_static! { - static ref TEST_WORK: Work = Work { + struct TestResult { + headers: String, + title: String, + print_identifier: String, + online_identifier: String, + title_url: String, + first_author: String, + title_id: String, + publisher_name: String, + publication_type: String, + date_monograph_published_print: String, + date_monograph_published_online: String, + monograph_volume: String, + monograph_edition: String, + first_editor: String, + parent_publication_title_id: String, + } + + impl fmt::Display for TestResult { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + writeln!(f, + "{}{}\t{}\t{}\t\t\t\t\t\t\t{}\t{}\t{}\t\tfulltext\t\t{}\t{}\t{}\t{}\t{}\t{}\t{}\t{}\t\tF", + self.headers, + self.title, + self.print_identifier, + self.online_identifier, + self.title_url, + self.first_author, + self.title_id, + self.publisher_name, + self.publication_type, + self.date_monograph_published_print, + self.date_monograph_published_online, + self.monograph_volume, + self.monograph_edition, + self.first_editor, + self.parent_publication_title_id, + ) + } + } + + #[test] + fn test_kbart_oclc() { + let mut test_work: Work = Work { work_id: Uuid::from_str("00000000-0000-0000-AAAA-000000000001").unwrap(), work_status: WorkStatus::ACTIVE, full_title: "Book Title: Book Subtitle".to_string(), @@ -202,7 +244,7 @@ mod tests { copyright_holder: "Author 1; Author 2".to_string(), short_abstract: Some("Lorem ipsum dolor sit amet".to_string()), long_abstract: Some( - "Lorem ipsum dolor sit amet, consectetur adipiscing elit".to_string() + "Lorem ipsum dolor sit amet, consectetur adipiscing elit".to_string(), ), general_note: None, place: Some("León, Spain".to_string()), @@ -250,7 +292,7 @@ mod tests { issn_digital: "3333-4444".to_string(), series_url: None, }, - } + }, ], contributions: vec![ WorkContributions { @@ -264,7 +306,7 @@ mod tests { contribution_ordinal: 1, contributor: WorkContributionsContributor { orcid: Some( - Orcid::from_str("https://orcid.org/0000-0002-0000-0001").unwrap() + Orcid::from_str("https://orcid.org/0000-0002-0000-0001").unwrap(), ), }, }, @@ -279,14 +321,25 @@ mod tests { contribution_ordinal: 2, contributor: WorkContributionsContributor { orcid: None }, }, + WorkContributions { + contribution_type: ContributionType::EDITOR, + first_name: Some("Editor".to_string()), + last_name: "FirstEd".to_string(), + full_name: "Editor FirstEd".to_string(), + main_contribution: true, + biography: None, + institution: None, + contribution_ordinal: 3, + contributor: WorkContributionsContributor { orcid: None }, + }, ], languages: vec![], publications: vec![ WorkPublications { - publication_id: Uuid::from_str("00000000-0000-0000-BBBB-000000000002").unwrap(), - publication_type: PublicationType::PAPERBACK, - publication_url: Some("https://www.book.com/paperback".to_string()), - isbn: Some(Isbn::from_str("978-3-16-148410-0").unwrap()), + publication_id: Uuid::from_str("00000000-0000-0000-DDDD-000000000004").unwrap(), + publication_type: PublicationType::PDF, + publication_url: Some("https://www.book.com/pdf".to_string()), + isbn: Some(Isbn::from_str("978-1-56619-909-4").unwrap()), prices: vec![], }, WorkPublications { @@ -297,10 +350,10 @@ mod tests { prices: vec![], }, WorkPublications { - publication_id: Uuid::from_str("00000000-0000-0000-DDDD-000000000004").unwrap(), - publication_type: PublicationType::PDF, - publication_url: Some("https://www.book.com/pdf".to_string()), - isbn: Some(Isbn::from_str("978-1-56619-909-4").unwrap()), + publication_id: Uuid::from_str("00000000-0000-0000-BBBB-000000000002").unwrap(), + publication_type: PublicationType::PAPERBACK, + publication_url: Some("https://www.book.com/paperback".to_string()), + isbn: Some(Isbn::from_str("978-3-16-148410-0").unwrap()), prices: vec![], }, WorkPublications { @@ -321,15 +374,88 @@ mod tests { subjects: vec![], fundings: vec![], }; - } + let mut test_result = TestResult { + headers: "publication_title\tprint_identifier\tonline_identifier\tdate_first_issue_online\tnum_first_vol_online\tnum_first_issue_online\tdate_last_issue_online\tnum_last_vol_online\tnum_last_issue_online\ttitle_url\tfirst_author\ttitle_id\tembargo_info\tcoverage_depth\tnotes\tpublisher_name\tpublication_type\tdate_monograph_published_print\tdate_monograph_published_online\tmonograph_volume\tmonograph_edition\tfirst_editor\tparent_publication_title_id\tpreceding_publication_title_id\taccess_type\n".to_string(), + title: "Book Title: Separate Subtitle".to_string(), + print_identifier: "978-3-16-148410-0".to_string(), + online_identifier: "978-1-56619-909-4".to_string(), + title_url: "https://www.book.com".to_string(), + first_author: "First".to_string(), + title_id: "10.00001/BOOK.0001".to_string(), + publisher_name: "OA Editions".to_string(), + publication_type: "Monograph".to_string(), + date_monograph_published_print: "1999".to_string(), + date_monograph_published_online: "1999".to_string(), + monograph_volume: "20".to_string(), + monograph_edition: "1".to_string(), + first_editor: "".to_string(), + parent_publication_title_id: "8765-4321".to_string(), + }; + let to_test = + KbartOclc.generate(&[test_work.clone()], QuoteStyle::Necessary, DELIMITER_TAB); + assert_eq!(to_test, Ok(test_result.to_string())); - const TEST_RESULT: &str = "publication_title\tprint_identifier\tonline_identifier\tdate_first_issue_online\tnum_first_vol_online\tnum_first_issue_online\tdate_last_issue_online\tnum_last_vol_online\tnum_last_issue_online\ttitle_url\tfirst_author\ttitle_id\tembargo_info\tcoverage_depth\tnotes\tpublisher_name\tpublication_type\tdate_monograph_published_print\tdate_monograph_published_online\tmonograph_volume\tmonograph_edition\tfirst_editor\tparent_publication_title_id\tpreceding_publication_title_id\taccess_type\nBook Title: Separate Subtitle\t978-3-16-148410-0\t978-1-56619-909-4\t\t\t\t\t\t\thttps://www.book.com\tFirst\t10.00001/BOOK.0001\t\tfulltext\t\tOA Editions\tMonograph\t1999\t1999\t20\t1\t\t8765-4321\t\tF\n"; + // Remove subtitle: full title is used instead of title + subtitle + test_work.subtitle = None; + test_result.title = "Book Title: Book Subtitle".to_string(); + // Remove DOI: no title_id + test_work.doi = None; + test_result.title_id = "".to_string(); + // Remove paperback publication: date_monograph_published_print (for hardback) + // still appears, but no print_identifier (paperback ISBN) is present + test_work.publications.remove(2); + test_result.print_identifier = "".to_string(); + // Make first-numbered author a non-main contributor: + // second-numbered author appears as first_author instead + test_work.contributions[0].main_contribution = false; + test_result.first_author = "Second".to_string(); + // Make work a book set: publication_type becomes Serial + test_work.work_type = WorkType::BOOK_SET; + test_result.publication_type = "Serial".to_string(); + let to_test = + KbartOclc.generate(&[test_work.clone()], QuoteStyle::Necessary, DELIMITER_TAB); + assert_eq!(to_test, Ok(test_result.to_string())); - #[test] - fn test_kbart_oclc() { + // Remove hardback publication: no date_monograph_published_print + test_work.publications.remove(1); + test_result.date_monograph_published_print = "".to_string(); + // Remove PDF publication's ISBN: no online_identifier + test_work.publications[0].isbn = None; + test_result.online_identifier = "".to_string(); + // Make work an edited book: first_author becomes empty, + // first_editor lists first-numbered editor, + // publication_type reverts to Monograph + test_work.work_type = WorkType::EDITED_BOOK; + test_result.first_author = "".to_string(); + test_result.first_editor = "FirstEd".to_string(); + test_result.publication_type = "Monograph".to_string(); + let to_test = + KbartOclc.generate(&[test_work.clone()], QuoteStyle::Necessary, DELIMITER_TAB); + assert_eq!(to_test, Ok(test_result.to_string())); + + // Remove landing page: KBART fails to generate + test_work.landing_page = None; let to_test = - KbartOclc.generate(&[TEST_WORK.clone()], QuoteStyle::Necessary, DELIMITER_TAB); + KbartOclc.generate(&[test_work.clone()], QuoteStyle::Necessary, DELIMITER_TAB); + assert_eq!( + to_test, + Err(ThothError::IncompleteMetadataRecord( + "kbart::oclc".to_string(), + "Missing Landing Page".to_string(), + )) + ); - assert_eq!(to_test, Ok(TEST_RESULT.to_string())) + // Reinstate landing page but remove publication date: ditto + test_work.landing_page = Some("https://www.book.com".to_string()); + test_work.publication_date = None; + let to_test = + KbartOclc.generate(&[test_work.clone()], QuoteStyle::Necessary, DELIMITER_TAB); + assert_eq!( + to_test, + Err(ThothError::IncompleteMetadataRecord( + "kbart::oclc".to_string(), + "Missing Publication Date".to_string(), + )) + ); } } From 33472ce7a3bc43ec9e05033fa4e7bcdda7d15d91 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Fri, 1 Oct 2021 14:08:39 +0100 Subject: [PATCH 02/38] MUSE feedback (on JSTOR output): must include at least one BIC/BISAC code; don't include keywords --- .../src/xml/onix3_project_muse.rs | 85 ++++++++++++++----- 1 file changed, 62 insertions(+), 23 deletions(-) diff --git a/thoth-export-server/src/xml/onix3_project_muse.rs b/thoth-export-server/src/xml/onix3_project_muse.rs index b7b53fcf..41164068 100644 --- a/thoth-export-server/src/xml/onix3_project_muse.rs +++ b/thoth-export-server/src/xml/onix3_project_muse.rs @@ -58,15 +58,26 @@ impl XmlSpecification for Onix3ProjectMuse { impl XmlElementBlock for Work { fn xml_element(&self, w: &mut EventWriter) -> ThothResult<()> { - let work_id = format!("urn:uuid:{}", self.work_id.to_string()); - let (main_isbn, isbns) = get_publications_data(&self.publications); + // Project MUSE can only ingest works which have at least one BIC or BISAC subject code + if !self + .subjects + .iter() + .any(|s| s.subject_type.eq(&SubjectType::BISAC) || s.subject_type.eq(&SubjectType::BIC)) + { + Err(ThothError::IncompleteMetadataRecord( + "onix_3.0::project_muse".to_string(), + "No BIC or BISAC subject code".to_string(), + )) + } // We can only generate the document if there's a PDF - if let Some(pdf_url) = self + else if let Some(pdf_url) = self .publications .iter() .find(|p| p.publication_type.eq(&PublicationType::PDF)) .and_then(|p| p.publication_url.as_ref()) { + let work_id = format!("urn:uuid:{}", self.work_id.to_string()); + let (main_isbn, isbns) = get_publications_data(&self.publications); write_element_block("Product", w, |w| { write_element_block("RecordReference", w, |w| { w.write(XmlEvent::Characters(&work_id)) @@ -194,13 +205,19 @@ impl XmlElementBlock for Work { })?; } for subject in &self.subjects { - write_element_block("Subject", w, |w| { - XmlElement::::xml_element(&subject.subject_type, w)?; - write_element_block("SubjectCode", w, |w| { - w.write(XmlEvent::Characters(&subject.subject_code)) - .map_err(|e| e.into()) - }) - })?; + // Project MUSE can't process records containing keywords + if subject.subject_type != SubjectType::KEYWORD { + write_element_block("Subject", w, |w| { + XmlElement::::xml_element( + &subject.subject_type, + w, + )?; + write_element_block("SubjectCode", w, |w| { + w.write(XmlEvent::Characters(&subject.subject_code)) + .map_err(|e| e.into()) + }) + })?; + } } Ok(()) })?; @@ -425,11 +442,11 @@ impl XmlElement for SubjectType { match self { SubjectType::BIC => "12", SubjectType::BISAC => "10", - SubjectType::KEYWORD => "20", SubjectType::LCC => "04", SubjectType::THEMA => "93", SubjectType::CUSTOM => "B2", - SubjectType::Other(_) => unreachable!(), + // Keywords are not output for Project MUSE + SubjectType::KEYWORD | SubjectType::Other(_) => unreachable!(), } } } @@ -811,8 +828,6 @@ mod tests { assert!(output.contains(r#" JA85"#)); assert!(output.contains(r#" 93"#)); assert!(output.contains(r#" JWA"#)); - assert!(output.contains(r#" 20"#)); - assert!(output.contains(r#" keyword1"#)); assert!(output.contains(r#" B2"#)); assert!(output.contains(r#" custom1"#)); assert!(output.contains(r#" "#)); @@ -859,6 +874,8 @@ mod tests { assert!(output.contains(r#" https://www.book.com/pdf"#)); // Test that OAPEN-only blocks are not output in Project MUSE format + assert!(!output.contains(r#" 20"#)); + assert!(!output.contains(r#" keyword1"#)); assert!(!output.contains(r#" "#)); assert!(!output.contains(r#" 01"#)); assert!(!output.contains(r#" 06"#)); @@ -888,7 +905,7 @@ mod tests { test_work.place = None; test_work.publication_date = None; test_work.landing_page = None; - test_work.subjects.clear(); + test_work.subjects.drain(1..); let output = generate_test_output(&test_work); // No DOI supplied assert!(!output.contains(r#" 06"#)); @@ -930,18 +947,13 @@ mod tests { r#" Publisher's website: web shop"# )); assert!(!output.contains(r#" https://www.book.com"#)); - // No subjects supplied - assert!(!output.contains(r#" "#)); - assert!(!output.contains(r#" 12"#)); - assert!(!output.contains(r#" AAB"#)); + // All subjects removed except BIC assert!(!output.contains(r#" 10"#)); assert!(!output.contains(r#" AAA000000"#)); assert!(!output.contains(r#" 04"#)); assert!(!output.contains(r#" JA85"#)); assert!(!output.contains(r#" 93"#)); assert!(!output.contains(r#" JWA"#)); - assert!(!output.contains(r#" 20"#)); - assert!(!output.contains(r#" keyword1"#)); assert!(!output.contains(r#" B2"#)); assert!(!output.contains(r#" custom1"#)); @@ -970,8 +982,35 @@ mod tests { assert!(!output.contains(r#" 04"#)); assert!(!output.contains(r#" 1. Chapter 1"#)); - // Remove the only publication, which is the PDF - // Result: error (can't generate OAPEN ONIX without PDF URL) + // Remove the only remaining (BIC) subject + // Result: error (can't generate Project MUSE ONIX without either a BIC or BISAC subject) + test_work.subjects.clear(); + // Can't use helper function for this as it assumes Ok rather than Err + let mut buffer = Vec::new(); + let mut writer = xml::writer::EmitterConfig::new() + .perform_indent(true) + .create_writer(&mut buffer); + let wrapped_output = + XmlElementBlock::::xml_element(&test_work, &mut writer) + .map(|_| buffer) + .and_then(|onix| { + String::from_utf8(onix) + .map_err(|_| ThothError::InternalError("Could not parse XML".to_string())) + }); + assert!(wrapped_output.is_err()); + let output = wrapped_output.unwrap_err().to_string(); + assert_eq!( + output, + "Could not generate onix_3.0::project_muse: No BIC or BISAC subject code".to_string() + ); + + // Reinstate the BIC subject but remove the only publication, which is the PDF + // Result: error (can't generate Project MUSE ONIX without PDF URL) + test_work.subjects = vec![WorkSubjects { + subject_code: "AAB".to_string(), + subject_type: SubjectType::BIC, + subject_ordinal: 1, + }]; test_work.publications.clear(); // Can't use helper function for this as it assumes Ok rather than Err let mut buffer = Vec::new(); From ef77b553e5bc81fb4bed41dfb691db1408958c65 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Fri, 1 Oct 2021 14:09:30 +0100 Subject: [PATCH 03/38] MUSE feedback: minor changes in JSTOR output fine; carry back over to MUSE output for consistency/compatibility --- thoth-export-server/src/data.rs | 16 +- .../src/xml/onix3_project_muse.rs | 148 +++++++++--------- 2 files changed, 81 insertions(+), 83 deletions(-) diff --git a/thoth-export-server/src/data.rs b/thoth-export-server/src/data.rs index 0626b860..4033fda5 100644 --- a/thoth-export-server/src/data.rs +++ b/thoth-export-server/src/data.rs @@ -11,7 +11,10 @@ lazy_static! { id: "onix_3.0::project_muse", name: "Project MUSE ONIX 3.0", format: concat!(env!("THOTH_EXPORT_API"), "/formats/onix_3.0"), - accepted_by: vec![concat!(env!("THOTH_EXPORT_API"), "/platforms/project_muse"),], + accepted_by: vec![ + concat!(env!("THOTH_EXPORT_API"), "/platforms/project_muse"), + concat!(env!("THOTH_EXPORT_API"), "/platforms/jstor") + ], }, Specification { id: "onix_3.0::oapen", @@ -72,10 +75,13 @@ lazy_static! { Platform { id: "jstor", name: "JSTOR", - accepts: vec![concat!( - env!("THOTH_EXPORT_API"), - "/specifications/onix_3.0::jstor" - ),], + accepts: vec![ + concat!(env!("THOTH_EXPORT_API"), "/specifications/onix_3.0::jstor"), + concat!( + env!("THOTH_EXPORT_API"), + "/specifications/onix_3.0::project_muse" + ) + ], }, Platform { id: "oclc_kb", diff --git a/thoth-export-server/src/xml/onix3_project_muse.rs b/thoth-export-server/src/xml/onix3_project_muse.rs index 41164068..6816078d 100644 --- a/thoth-export-server/src/xml/onix3_project_muse.rs +++ b/thoth-export-server/src/xml/onix3_project_muse.rs @@ -221,44 +221,55 @@ impl XmlElementBlock for Work { } Ok(()) })?; - if self.long_abstract.is_some() || self.toc.is_some() { - write_element_block("CollateralDetail", w, |w| { - if let Some(labstract) = &self.long_abstract { - write_element_block("TextContent", w, |w| { - let mut lang_fmt: HashMap<&str, &str> = HashMap::new(); - lang_fmt.insert("language", "eng"); - // 03 Description ("30 Abstract" not implemented in OAPEN) - write_element_block("TextType", w, |w| { - w.write(XmlEvent::Characters("03")).map_err(|e| e.into()) - })?; - // 00 Unrestricted - write_element_block("ContentAudience", w, |w| { - w.write(XmlEvent::Characters("00")).map_err(|e| e.into()) - })?; - write_full_element_block("Text", None, Some(lang_fmt), w, |w| { - w.write(XmlEvent::Characters(labstract)) - .map_err(|e| e.into()) - }) + write_element_block("CollateralDetail", w, |w| { + if let Some(labstract) = &self.long_abstract { + write_element_block("TextContent", w, |w| { + let mut lang_fmt: HashMap<&str, &str> = HashMap::new(); + lang_fmt.insert("language", "eng"); + // 03 Description ("30 Abstract" not implemented in OAPEN) + write_element_block("TextType", w, |w| { + w.write(XmlEvent::Characters("03")).map_err(|e| e.into()) })?; - } - if let Some(toc) = &self.toc { - write_element_block("TextContent", w, |w| { - // 04 Table of contents - write_element_block("TextType", w, |w| { - w.write(XmlEvent::Characters("04")).map_err(|e| e.into()) - })?; - // 00 Unrestricted - write_element_block("ContentAudience", w, |w| { - w.write(XmlEvent::Characters("00")).map_err(|e| e.into()) - })?; - write_element_block("Text", w, |w| { - w.write(XmlEvent::Characters(toc)).map_err(|e| e.into()) - }) + // 00 Unrestricted + write_element_block("ContentAudience", w, |w| { + w.write(XmlEvent::Characters("00")).map_err(|e| e.into()) })?; - } - Ok(()) - })?; - } + write_full_element_block("Text", None, Some(lang_fmt), w, |w| { + w.write(XmlEvent::Characters(labstract)) + .map_err(|e| e.into()) + }) + })?; + } + if let Some(toc) = &self.toc { + write_element_block("TextContent", w, |w| { + // 04 Table of contents + write_element_block("TextType", w, |w| { + w.write(XmlEvent::Characters("04")).map_err(|e| e.into()) + })?; + // 00 Unrestricted + write_element_block("ContentAudience", w, |w| { + w.write(XmlEvent::Characters("00")).map_err(|e| e.into()) + })?; + write_element_block("Text", w, |w| { + w.write(XmlEvent::Characters(toc)).map_err(|e| e.into()) + }) + })?; + } + write_element_block("TextContent", w, |w| { + // 20 Open access statement + write_element_block("TextType", w, |w| { + w.write(XmlEvent::Characters("20")).map_err(|e| e.into()) + })?; + // 00 Unrestricted + write_element_block("ContentAudience", w, |w| { + w.write(XmlEvent::Characters("00")).map_err(|e| e.into()) + })?; + write_element_block("Text", w, |w| { + w.write(XmlEvent::Characters("Open Access")) + .map_err(|e| e.into()) + }) + }) + })?; write_element_block("PublishingDetail", w, |w| { write_element_block("Imprint", w, |w| { write_element_block("ImprintName", w, |w| { @@ -285,15 +296,15 @@ impl XmlElementBlock for Work { if let Some(date) = self.publication_date { write_element_block("PublishingDate", w, |w| { let mut date_fmt: HashMap<&str, &str> = HashMap::new(); - date_fmt.insert("dateformat", "01"); // 01 YYYYMM + date_fmt.insert("dateformat", "00"); // 00 YYYYMMDD write_element_block("PublishingDateRole", w, |w| { - // 19 Publication date of print counterpart - w.write(XmlEvent::Characters("19")).map_err(|e| e.into()) + // 01 Publication date + w.write(XmlEvent::Characters("01")).map_err(|e| e.into()) })?; - // dateformat="01" YYYYMM + // dateformat="00" YYYYMMDD write_full_element_block("Date", None, Some(date_fmt), w, |w| { - w.write(XmlEvent::Characters(&date.format("%Y%m").to_string())) + w.write(XmlEvent::Characters(&date.format("%Y%m%d").to_string())) .map_err(|e| e.into()) }) })?; @@ -372,9 +383,9 @@ impl XmlElementBlock for Work { write_element_block("ProductAvailability", w, |w| { w.write(XmlEvent::Characters("99")).map_err(|e| e.into()) })?; - // 04 Contact supplier + // 01 Free of charge write_element_block("UnpricedItemType", w, |w| { - w.write(XmlEvent::Characters("04")).map_err(|e| e.into()) + w.write(XmlEvent::Characters("01")).map_err(|e| e.into()) }) })?; } @@ -838,6 +849,9 @@ mod tests { assert!(output.contains(r#" "#)); assert!(output.contains(r#" 04"#)); assert!(output.contains(r#" 1. Chapter 1"#)); + assert!(output.contains(r#" "#)); + assert!(output.contains(r#" 20"#)); + assert!(output.contains(r#" Open Access"#)); assert!(output.contains(r#" "#)); assert!(output.contains(r#" "#)); assert!(output.contains(r#" OA Editions Imprint"#)); @@ -847,8 +861,8 @@ mod tests { assert!(output.contains(r#" León, Spain"#)); assert!(output.contains(r#" 04"#)); assert!(output.contains(r#" "#)); - assert!(output.contains(r#" 19"#)); - assert!(output.contains(r#" 199912"#)); + assert!(output.contains(r#" 01"#)); + assert!(output.contains(r#" 19991231"#)); assert!(output.contains(r#" "#)); assert!(output.contains(r#" 06"#)); assert!(output.contains(r#" "#)); @@ -866,7 +880,7 @@ mod tests { )); assert!(output.contains(r#" https://www.book.com"#)); assert!(output.contains(r#" 99"#)); - assert!(output.contains(r#" 04"#)); + assert!(output.contains(r#" 01"#)); assert!(output.contains(r#" 09"#)); assert!(output.contains(r#" OA Editions"#)); assert!(output.contains(r#" 29"#)); @@ -902,6 +916,7 @@ mod tests { test_work.subtitle = None; test_work.page_count = None; test_work.long_abstract = None; + test_work.toc = None; test_work.place = None; test_work.publication_date = None; test_work.landing_page = None; @@ -927,20 +942,22 @@ mod tests { assert!(!output.contains(r#" 00"#)); assert!(!output.contains(r#" 334"#)); assert!(!output.contains(r#" 03"#)); - // No long abstract supplied: CollateralDetail block only contains TOC + // No long abstract supplied + assert!(!output.contains(r#" 03"#)); + assert!(!output.contains(r#" Lorem ipsum dolor sit amet"#)); + // No TOC supplied + assert!(!output.contains(r#" 04"#)); + assert!(!output.contains(r#" 1. Chapter 1"#)); + // CollateralDetail block is still present as it always contains Open Access statement assert!(output.contains(r#" "#)); assert!(output.contains(r#" "#)); - assert!(output.contains(r#" 04"#)); assert!(output.contains(r#" 00"#)); - assert!(output.contains(r#" 1. Chapter 1"#)); - assert!(!output.contains(r#" 03"#)); - assert!(!output.contains(r#" Lorem ipsum dolor sit amet"#)); // No place supplied assert!(!output.contains(r#" León, Spain"#)); // No publication date supplied assert!(!output.contains(r#" "#)); - assert!(!output.contains(r#" 19"#)); - assert!(!output.contains(r#" 199912"#)); + assert!(!output.contains(r#" 01"#)); + assert!(!output.contains(r#" 19991231"#)); // No landing page supplied: only one SupplyDetail block, linking to PDF download assert!(!output.contains(r#" 01"#)); assert!(!output.contains( @@ -957,31 +974,6 @@ mod tests { assert!(!output.contains(r#" B2"#)); assert!(!output.contains(r#" custom1"#)); - // Replace long abstract but remove TOC - // Result: CollateralDetail block still present, but now only contains long abstract - test_work.long_abstract = Some("Lorem ipsum dolor sit amet".to_string()); - test_work.toc = None; - let output = generate_test_output(&test_work); - assert!(output.contains(r#" "#)); - assert!(output.contains(r#" "#)); - assert!(output.contains(r#" 03"#)); - assert!(output.contains(r#" 00"#)); - assert!(output.contains(r#" Lorem ipsum dolor sit amet"#)); - assert!(!output.contains(r#" 04"#)); - assert!(!output.contains(r#" 1. Chapter 1"#)); - - // Remove both TOC and long abstract - // Result: No CollateralDetail block present at all - test_work.long_abstract = None; - let output = generate_test_output(&test_work); - assert!(!output.contains(r#" "#)); - assert!(!output.contains(r#" "#)); - assert!(!output.contains(r#" 03"#)); - assert!(!output.contains(r#" 00"#)); - assert!(!output.contains(r#" Lorem ipsum dolor sit amet"#)); - assert!(!output.contains(r#" 04"#)); - assert!(!output.contains(r#" 1. Chapter 1"#)); - // Remove the only remaining (BIC) subject // Result: error (can't generate Project MUSE ONIX without either a BIC or BISAC subject) test_work.subjects.clear(); From 035f21c8fba7f3656cc682671f6fcebdd0102848 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Mon, 4 Oct 2021 14:22:25 +0100 Subject: [PATCH 04/38] Add tests for individual csv_thoth functions --- thoth-export-server/src/csv/csv_thoth.rs | 170 ++++++++++++++++++++++- 1 file changed, 169 insertions(+), 1 deletion(-) diff --git a/thoth-export-server/src/csv/csv_thoth.rs b/thoth-export-server/src/csv/csv_thoth.rs index d0340b10..238fe601 100644 --- a/thoth-export-server/src/csv/csv_thoth.rs +++ b/thoth-export-server/src/csv/csv_thoth.rs @@ -332,7 +332,8 @@ mod tests { use thoth_api::model::Orcid; use thoth_client::{ ContributionType, CurrencyCode, LanguageCode, LanguageRelation, PublicationType, - WorkContributionsContributor, WorkImprint, WorkImprintPublisher, WorkStatus, WorkType, + SeriesType, WorkContributionsContributor, WorkFundingsFunder, WorkImprint, + WorkImprintPublisher, WorkIssuesSeries, WorkStatus, WorkType, }; use uuid::Uuid; @@ -528,4 +529,171 @@ mod tests { assert_eq!(to_test, Ok(TEST_RESULT.to_string())) } + + #[test] + fn test_csv_thoth_cell() { + assert_eq!(CsvCell::::csv_cell(&vec![]), "".to_string()); + assert_eq!( + CsvCell::::csv_cell(&vec!["String1".to_string()]), + "[String1]".to_string() + ); + assert_eq!( + CsvCell::::csv_cell(&vec!["String1".to_string(), "String2".to_string()]), + "[String1,String2]".to_string() + ); + } + + #[test] + fn test_csv_thoth_publications() { + let mut publication = WorkPublications { + publication_id: Uuid::from_str("00000000-0000-0000-BBBB-000000000002").unwrap(), + publication_type: PublicationType::PAPERBACK, + publication_url: Some("https://www.book.com/paperback".to_string()), + isbn: Some(Isbn::from_str("978-3-16-148410-0").unwrap()), + prices: vec![WorkPublicationsPrices { + currency_code: CurrencyCode::EUR, + unit_price: 25.95, + }], + }; + assert_eq!(CsvCell::::csv_cell(&publication), + r#"("PAPERBACK", "978-3-16-148410-0", "https://www.book.com/paperback", [("EUR", "25.95")])"#.to_string()); + publication.publication_type = PublicationType::HARDBACK; + publication.isbn = None; + publication.publication_url = None; + publication.prices.clear(); + assert_eq!( + CsvCell::::csv_cell(&publication), + r#"("HARDBACK", "", "", )"#.to_string() + ); + } + + #[test] + fn test_csv_thoth_contributions() { + let mut contribution = WorkContributions { + contribution_type: ContributionType::AUTHOR, + first_name: Some("Author".to_string()), + last_name: "1".to_string(), + full_name: "Author 1".to_string(), + main_contribution: true, + biography: None, + institution: Some("University of Life".to_string()), + contribution_ordinal: 1, + contributor: WorkContributionsContributor { + orcid: Some(Orcid::from_str("https://orcid.org/0000-0002-0000-0001").unwrap()), + }, + }; + assert_eq!( + CsvCell::::csv_cell(&contribution), + r#"("AUTHOR", "Author", "1", "Author 1", "University of Life", "0000-0002-0000-0001")"# + .to_string() + ); + contribution.contribution_type = ContributionType::TRANSLATOR; + contribution.first_name = None; + contribution.institution = None; + contribution.contributor.orcid = None; + assert_eq!( + CsvCell::::csv_cell(&contribution), + r#"("TRANSLATOR", "", "1", "Author 1", "", "")"#.to_string() + ); + } + + #[test] + fn test_csv_thoth_prices() { + let mut price = WorkPublicationsPrices { + currency_code: CurrencyCode::GBP, + unit_price: 22.95, + }; + assert_eq!( + CsvCell::::csv_cell(&price), + r#"("GBP", "22.95")"#.to_string() + ); + price.currency_code = CurrencyCode::USD; + price.unit_price = 31.95; + assert_eq!( + CsvCell::::csv_cell(&price), + r#"("USD", "31.95")"#.to_string() + ); + } + + #[test] + fn test_csv_thoth_issues() { + let mut issue = WorkIssues { + issue_ordinal: 1, + series: WorkIssuesSeries { + series_type: SeriesType::JOURNAL, + series_name: "Name of series".to_string(), + issn_print: "1234-5678".to_string(), + issn_digital: "8765-4321".to_string(), + series_url: Some("https://www.series.com".to_string()), + }, + }; + assert_eq!(CsvCell::::csv_cell(&issue), + r#"("JOURNAL", "Name of series", "1234-5678", "8765-4321", "https://www.series.com", "1")"#.to_string()); + issue.issue_ordinal = 2; + issue.series.series_type = SeriesType::BOOK_SERIES; + issue.series.series_url = None; + assert_eq!( + CsvCell::::csv_cell(&issue), + r#"("BOOK_SERIES", "Name of series", "1234-5678", "8765-4321", "", "2")"#.to_string() + ); + } + + #[test] + fn test_csv_thoth_languages() { + let mut language = WorkLanguages { + language_code: LanguageCode::SPA, + language_relation: LanguageRelation::TRANSLATED_FROM, + main_language: true, + }; + assert_eq!( + CsvCell::::csv_cell(&language), + r#"("TRANSLATED_FROM", "SPA", "true")"#.to_string() + ); + language.language_code = LanguageCode::WEL; + language.language_relation = LanguageRelation::TRANSLATED_INTO; + language.main_language = false; + assert_eq!( + CsvCell::::csv_cell(&language), + r#"("TRANSLATED_INTO", "WEL", "false")"#.to_string() + ); + } + + #[test] + fn test_csv_thoth_subjects() { + let subject = WorkSubjects { + subject_code: "AAB".to_string(), + subject_type: SubjectType::BIC, + subject_ordinal: 2, + }; + assert_eq!( + CsvCell::::csv_cell(&subject), + r#""AAB""#.to_string() + ); + } + + #[test] + fn test_csv_thoth_fundings() { + let mut funding = WorkFundings { + program: Some("Name of program".to_string()), + project_name: Some("Name of project".to_string()), + project_shortname: None, + grant_number: Some("Number of grant".to_string()), + jurisdiction: Some("Funding jurisdiction".to_string()), + funder: WorkFundingsFunder { + funder_name: "Name of funder".to_string(), + funder_doi: Some(Doi::from_str("https://doi.org/10.00001/FUNDER.0001").unwrap()), + }, + }; + assert_eq!(CsvCell::::csv_cell(&funding), + r#"("Name of funder", "10.00001/FUNDER.0001", "Name of program", "Name of project", "Number of grant", "Funding jurisdiction")"#.to_string()); + funding.funder.funder_doi = None; + funding.program = None; + funding.project_name = None; + funding.grant_number = None; + funding.jurisdiction = None; + assert_eq!( + CsvCell::::csv_cell(&funding), + r#"("Name of funder", "", "", "", "", "")"#.to_string() + ); + } } From 3e051743238d2d0e46756166af1ee1ea8f8ac212 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Mon, 4 Oct 2021 14:58:48 +0100 Subject: [PATCH 05/38] Extend main csv_thoth test to cover all optional Work fields --- thoth-export-server/src/csv/csv_thoth.rs | 53 ++++++++++++++++++------ 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/thoth-export-server/src/csv/csv_thoth.rs b/thoth-export-server/src/csv/csv_thoth.rs index 238fe601..1eaed069 100644 --- a/thoth-export-server/src/csv/csv_thoth.rs +++ b/thoth-export-server/src/csv/csv_thoth.rs @@ -347,12 +347,12 @@ mod tests { work_type: WorkType::MONOGRAPH, edition: 1, doi: Some(Doi::from_str("https://doi.org/10.00001/BOOK.0001").unwrap()), - publication_date: None, + publication_date: Some(chrono::NaiveDate::from_ymd(1999, 12, 31)), license: Some("http://creativecommons.org/licenses/by/4.0/".to_string()), copyright_holder: "Author 1; Author 2".to_string(), short_abstract: Some("Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum vel libero eleifend, ultrices purus vitae, suscipit ligula. Aliquam ornare quam et nulla vestibulum, id euismod tellus malesuada. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.".to_string()), long_abstract: Some("Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum vel libero eleifend, ultrices purus vitae, suscipit ligula. Aliquam ornare quam et nulla vestibulum, id euismod tellus malesuada. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Nullam ornare bibendum ex nec dapibus. Proin porta risus elementum odio feugiat tempus. Etiam eu felis ac metus viverra ornare. In consectetur neque sed feugiat ornare. Mauris at purus fringilla orci tincidunt pulvinar sed a massa. Nullam vestibulum posuere augue, sit amet tincidunt nisl pulvinar ac.".to_string()), - general_note: None, + general_note: Some("This is a general note".to_string()), place: Some("León, Spain".to_string()), width_mm: Some(156.0), width_cm: Some(15.6), @@ -363,15 +363,15 @@ mod tests { page_count: Some(334), page_breakdown: Some("x+334".to_string()), image_count: Some(15), - table_count: None, - audio_count: None, - video_count: None, + table_count: Some(20), + audio_count: Some(25), + video_count: Some(30), landing_page: Some("https://www.book.com".to_string()), - toc: None, - lccn: None, - oclc: None, + toc: Some("1. Chapter 1".to_string()), + lccn: Some("123456789".to_string()), + oclc: Some("987654321".to_string()), cover_url: Some("https://www.book.com/cover".to_string()), - cover_caption: None, + cover_caption: Some("This is a cover caption".to_string()), imprint: WorkImprint { imprint_name: "OA Editions Imprint".to_string(), publisher: WorkImprintPublisher { @@ -379,7 +379,16 @@ mod tests { publisher_url: None, }, }, - issues: vec![], + issues: vec![WorkIssues { + issue_ordinal: 1, + series: WorkIssuesSeries { + series_type: SeriesType::JOURNAL, + series_name: "Name of series".to_string(), + issn_print: "1234-5678".to_string(), + issn_digital: "8765-4321".to_string(), + series_url: Some("https://www.series.com".to_string()), + }, + }], contributions: vec![ WorkContributions { contribution_type: ContributionType::AUTHOR, @@ -514,13 +523,33 @@ mod tests { subject_type: SubjectType::KEYWORD, subject_ordinal: 1, }, + WorkSubjects { + subject_code: "JA85".to_string(), + subject_type: SubjectType::LCC, + subject_ordinal: 1, + }, + WorkSubjects { + subject_code: "JWA".to_string(), + subject_type: SubjectType::THEMA, + subject_ordinal: 1, + }, ], - fundings: vec![], + fundings: vec![WorkFundings { + program: Some("Name of program".to_string()), + project_name: Some("Name of project".to_string()), + project_shortname: None, + grant_number: Some("Number of grant".to_string()), + jurisdiction: Some("Funding jurisdiction".to_string()), + funder: WorkFundingsFunder { + funder_name: "Name of funder".to_string(), + funder_doi: Some(Doi::from_str("https://doi.org/10.00001/FUNDER.0001").unwrap()), + }, + }], }; } const TEST_RESULT: &str = r#""publisher","imprint","work_type","work_status","title","subtitle","edition","doi","publication_date","publication_place","license","copyright_holder","landing_page","width (mm)","width (cm)","width (in)","height (mm)","height (cm)","height (in)","page_count","page_breakdown","image_count","table_count","audio_count","video_count","lccn","oclc","short_abstract","long_abstract","general_note","toc","cover_url","cover_caption","contributions [(type, first_name, last_name, full_name, institution, orcid)]","publications [(type, isbn, url, [(ISO_4217_currency, price)])]","series [(type, name, issn_print, issn_digital, url, issue)]","languages [(relation, ISO_639-3/B_language, is_main)]","BIC [code]","THEMA [code]","BISAC [code]","LCC [code]","custom_categories [category]","keywords [keyword]","funding [(funder, funder_doi, program, project, grant, jurisdiction)]" -"OA Editions","OA Editions Imprint","MONOGRAPH","ACTIVE","Book Title","Book Subtitle","1","10.00001/BOOK.0001","","León, Spain","http://creativecommons.org/licenses/by/4.0/","Author 1; Author 2","https://www.book.com","156.0","15.6","6.14","234.0","23.4","9.21","334","x+334","15","","","","","","Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum vel libero eleifend, ultrices purus vitae, suscipit ligula. Aliquam ornare quam et nulla vestibulum, id euismod tellus malesuada. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.","Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum vel libero eleifend, ultrices purus vitae, suscipit ligula. Aliquam ornare quam et nulla vestibulum, id euismod tellus malesuada. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Nullam ornare bibendum ex nec dapibus. Proin porta risus elementum odio feugiat tempus. Etiam eu felis ac metus viverra ornare. In consectetur neque sed feugiat ornare. Mauris at purus fringilla orci tincidunt pulvinar sed a massa. Nullam vestibulum posuere augue, sit amet tincidunt nisl pulvinar ac.","","","https://www.book.com/cover","","[(""AUTHOR"", ""Author"", ""1"", ""Author 1"", """", ""0000-0002-0000-0001""),(""AUTHOR"", ""Author"", ""2"", ""Author 2"", """", """")]","[(""PAPERBACK"", ""978-3-16-148410-0"", ""https://www.book.com/paperback"", [(""EUR"", ""25.95""),(""GBP"", ""22.95""),(""USD"", ""31.95"")]),(""HARDBACK"", ""978-1-4028-9462-6"", ""https://www.book.com/hardback"", [(""EUR"", ""36.95""),(""GBP"", ""32.95""),(""USD"", ""40.95"")]),(""PDF"", ""978-1-56619-909-4"", ""https://www.book.com/pdf"", ),(""HTML"", """", ""https://www.book.com/html"", ),(""XML"", ""978-92-95055-02-5"", ""https://www.book.com/xml"", )]","","[(""ORIGINAL"", ""SPA"", ""true"")]","[""AAA"",""AAB""]","","[""AAA000000"",""AAA000001""]","","[""Category1""]","[""keyword1"",""keyword2""]","" +"OA Editions","OA Editions Imprint","MONOGRAPH","ACTIVE","Book Title","Book Subtitle","1","10.00001/BOOK.0001","1999-12-31","León, Spain","http://creativecommons.org/licenses/by/4.0/","Author 1; Author 2","https://www.book.com","156.0","15.6","6.14","234.0","23.4","9.21","334","x+334","15","20","25","30","123456789","987654321","Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum vel libero eleifend, ultrices purus vitae, suscipit ligula. Aliquam ornare quam et nulla vestibulum, id euismod tellus malesuada. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.","Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum vel libero eleifend, ultrices purus vitae, suscipit ligula. Aliquam ornare quam et nulla vestibulum, id euismod tellus malesuada. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Nullam ornare bibendum ex nec dapibus. Proin porta risus elementum odio feugiat tempus. Etiam eu felis ac metus viverra ornare. In consectetur neque sed feugiat ornare. Mauris at purus fringilla orci tincidunt pulvinar sed a massa. Nullam vestibulum posuere augue, sit amet tincidunt nisl pulvinar ac.","This is a general note","1. Chapter 1","https://www.book.com/cover","This is a cover caption","[(""AUTHOR"", ""Author"", ""1"", ""Author 1"", """", ""0000-0002-0000-0001""),(""AUTHOR"", ""Author"", ""2"", ""Author 2"", """", """")]","[(""PAPERBACK"", ""978-3-16-148410-0"", ""https://www.book.com/paperback"", [(""EUR"", ""25.95""),(""GBP"", ""22.95""),(""USD"", ""31.95"")]),(""HARDBACK"", ""978-1-4028-9462-6"", ""https://www.book.com/hardback"", [(""EUR"", ""36.95""),(""GBP"", ""32.95""),(""USD"", ""40.95"")]),(""PDF"", ""978-1-56619-909-4"", ""https://www.book.com/pdf"", ),(""HTML"", """", ""https://www.book.com/html"", ),(""XML"", ""978-92-95055-02-5"", ""https://www.book.com/xml"", )]","[(""JOURNAL"", ""Name of series"", ""1234-5678"", ""8765-4321"", ""https://www.series.com"", ""1"")]","[(""ORIGINAL"", ""SPA"", ""true"")]","[""AAA"",""AAB""]","[""JWA""]","[""AAA000000"",""AAA000001""]","[""JA85""]","[""Category1""]","[""keyword1"",""keyword2""]","[(""Name of funder"", ""10.00001/FUNDER.0001"", ""Name of program"", ""Name of project"", ""Number of grant"", ""Funding jurisdiction"")]" "#; #[test] From f022833083434d140b13740abdbf0bed3ae5c75e Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Tue, 5 Oct 2021 13:51:03 +0100 Subject: [PATCH 06/38] If publication date is set and then cleared, send Null instead of empty string --- thoth-app/src/component/new_work.rs | 8 +++++++- thoth-app/src/component/work.rs | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/thoth-app/src/component/new_work.rs b/thoth-app/src/component/new_work.rs index 2ec5eba0..1f992fbb 100644 --- a/thoth-app/src/component/new_work.rs +++ b/thoth-app/src/component/new_work.rs @@ -397,7 +397,13 @@ impl Component for NewWorkComponent { false } } - Msg::ChangeDate(date) => self.work.publication_date.neq_assign(Some(date)), + Msg::ChangeDate(value) => { + let date = match value.trim().is_empty() { + true => None, + false => Some(value.trim().to_owned()), + }; + self.work.publication_date.neq_assign(date) + } Msg::ChangePlace(value) => { let place = match value.trim().is_empty() { true => None, diff --git a/thoth-app/src/component/work.rs b/thoth-app/src/component/work.rs index 993aa56f..1ef49932 100644 --- a/thoth-app/src/component/work.rs +++ b/thoth-app/src/component/work.rs @@ -448,7 +448,13 @@ impl Component for WorkComponent { false } } - Msg::ChangeDate(date) => self.work.publication_date.neq_assign(Some(date)), + Msg::ChangeDate(value) => { + let date = match value.trim().is_empty() { + true => None, + false => Some(value.trim().to_owned()), + }; + self.work.publication_date.neq_assign(date) + } Msg::ChangePlace(value) => { let place = match value.trim().is_empty() { true => None, From 0783fbac703b037832ecd0e6a171f8fdb6592e1d Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Tue, 5 Oct 2021 14:37:11 +0100 Subject: [PATCH 07/38] Add issue templates --- .github/ISSUE_TEMPLATE/bug_report.md | 32 +++++++++++++++++++++++ .github/ISSUE_TEMPLATE/feature_request.md | 20 ++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000..d1ba2bc0 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,32 @@ +--- +name: Bug report +about: Create a report to help us improve +title: '' +labels: bug +assignees: '' + +--- + +**Describe the bug** +A clear and concise description of what the bug is. + +**To Reproduce** +Steps to reproduce the behavior: +1. Go to '...' +2. Click on '....' +3. Scroll down to '....' +4. See error + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**Screenshots** +If applicable, add screenshots to help explain your problem. + +**Desktop (please complete the following information):** + - OS: [e.g. iOS] + - Browser [e.g. chrome, safari] + - Version [e.g. 22] + +**Additional context** +Add any other context about the problem here. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 00000000..9d06944f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,20 @@ +--- +name: Feature request +about: Suggest an idea for Thoth +title: '' +labels: feature +assignees: '' + +--- + +**Is your feature request related to a problem? Please describe.** +A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] + +**Describe the solution you'd like** +A clear and concise description of what you want to happen. + +**Describe alternatives you've considered** +A clear and concise description of any alternative solutions or features you've considered. + +**Additional context** +Add any other context or screenshots about the feature request here. From 658107cc02e9815bf94920cdbf7b1fd50feb486b Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Tue, 5 Oct 2021 16:44:36 +0100 Subject: [PATCH 08/38] Factor out repeated Option creation logic into trait --- thoth-app/src/component/contributor.rs | 21 +-- thoth-app/src/component/imprint.rs | 8 +- thoth-app/src/component/mod.rs | 31 ++++ thoth-app/src/component/new_contributor.rs | 21 +-- thoth-app/src/component/new_imprint.rs | 8 +- thoth-app/src/component/new_publisher.rs | 26 ++- thoth-app/src/component/new_series.rs | 10 +- thoth-app/src/component/new_work.rs | 173 +++---------------- thoth-app/src/component/publications_form.rs | 13 +- thoth-app/src/component/publisher.rs | 26 ++- thoth-app/src/component/series.rs | 10 +- thoth-app/src/component/work.rs | 173 +++---------------- 12 files changed, 131 insertions(+), 389 deletions(-) diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index 90de1f8c..d707d2d8 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -48,6 +48,8 @@ use crate::route::AdminRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct ContributorComponent { contributor: Contributor, // Track the user-entered ORCID string, which may not be validly formatted @@ -287,13 +289,10 @@ impl Component for ContributorComponent { .send_message(Msg::SetContributorDeleteState(FetchAction::Fetching)); false } - Msg::ChangeFirstName(value) => { - let first_name = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.contributor.first_name.neq_assign(first_name) - } + Msg::ChangeFirstName(value) => self + .contributor + .first_name + .neq_assign(value.to_opt_string()), Msg::ChangeLastName(last_name) => self .contributor .last_name @@ -323,13 +322,7 @@ impl Component for ContributorComponent { false } } - Msg::ChangeWebsite(value) => { - let website = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.contributor.website.neq_assign(website) - } + Msg::ChangeWebsite(value) => self.contributor.website.neq_assign(value.to_opt_string()), Msg::ChangeRoute(r) => { let route = Route::from(r); self.router.send(RouteRequest::ChangeRoute(route)); diff --git a/thoth-app/src/component/imprint.rs b/thoth-app/src/component/imprint.rs index 7d9bae19..e415b7cd 100644 --- a/thoth-app/src/component/imprint.rs +++ b/thoth-app/src/component/imprint.rs @@ -47,6 +47,8 @@ use crate::route::AdminRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct ImprintComponent { imprint: ImprintWithPublisher, fetch_imprint: FetchImprint, @@ -304,11 +306,7 @@ impl Component for ImprintComponent { .imprint_name .neq_assign(imprint_name.trim().to_owned()), Msg::ChangeImprintUrl(value) => { - let imprint_url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.imprint.imprint_url.neq_assign(imprint_url) + self.imprint.imprint_url.neq_assign(value.to_opt_string()) } Msg::ChangeRoute(r) => { let route = Route::from(r); diff --git a/thoth-app/src/component/mod.rs b/thoth-app/src/component/mod.rs index fac6d5a5..353aea0b 100644 --- a/thoth-app/src/component/mod.rs +++ b/thoth-app/src/component/mod.rs @@ -349,6 +349,37 @@ macro_rules! pagination_component { }; } +pub trait ToOption { + fn to_opt_string(self) -> Option; + fn to_opt_float(self) -> Option; + fn to_opt_int(self) -> Option; +} + +impl ToOption for String { + fn to_opt_string(self) -> Option { + match self.trim().is_empty() { + true => None, + false => Some(self.trim().to_owned()), + } + } + + fn to_opt_float(self) -> Option { + let value = self.parse().unwrap_or(0.0); + match value == 0.0 { + true => None, + false => Some(value), + } + } + + fn to_opt_int(self) -> Option { + let value = self.parse().unwrap_or(0); + match value == 0 { + true => None, + false => Some(value), + } + } +} + pub mod admin; pub mod catalogue; pub mod contributions_form; diff --git a/thoth-app/src/component/new_contributor.rs b/thoth-app/src/component/new_contributor.rs index e9869161..f02f353e 100644 --- a/thoth-app/src/component/new_contributor.rs +++ b/thoth-app/src/component/new_contributor.rs @@ -34,6 +34,8 @@ use crate::models::EditRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + // Account for possibility of e.g. Chinese full names with only 2 characters. const MIN_FULLNAME_LEN: usize = 2; @@ -173,13 +175,10 @@ impl Component for NewContributorComponent { .send_message(Msg::SetContributorsFetchState(FetchAction::Fetching)); false } - Msg::ChangeFirstName(value) => { - let first_name = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.contributor.first_name.neq_assign(first_name) - } + Msg::ChangeFirstName(value) => self + .contributor + .first_name + .neq_assign(value.to_opt_string()), Msg::ChangeLastName(last_name) => self .contributor .last_name @@ -236,13 +235,7 @@ impl Component for NewContributorComponent { false } } - Msg::ChangeWebsite(value) => { - let website = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.contributor.website.neq_assign(website) - } + Msg::ChangeWebsite(value) => self.contributor.website.neq_assign(value.to_opt_string()), Msg::ChangeRoute(r) => { let route = Route::from(r); self.router.send(RouteRequest::ChangeRoute(route)); diff --git a/thoth-app/src/component/new_imprint.rs b/thoth-app/src/component/new_imprint.rs index 89c0d23e..77db27e2 100644 --- a/thoth-app/src/component/new_imprint.rs +++ b/thoth-app/src/component/new_imprint.rs @@ -35,6 +35,8 @@ use crate::models::EditRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct NewImprintComponent { imprint: Imprint, publisher_id: Uuid, @@ -178,11 +180,7 @@ impl Component for NewImprintComponent { .imprint_name .neq_assign(imprint_name.trim().to_owned()), Msg::ChangeImprintUrl(value) => { - let imprint_url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.imprint.imprint_url.neq_assign(imprint_url) + self.imprint.imprint_url.neq_assign(value.to_opt_string()) } Msg::ChangeRoute(r) => { let route = Route::from(r); diff --git a/thoth-app/src/component/new_publisher.rs b/thoth-app/src/component/new_publisher.rs index 46892b95..5082e8b1 100644 --- a/thoth-app/src/component/new_publisher.rs +++ b/thoth-app/src/component/new_publisher.rs @@ -26,6 +26,8 @@ use crate::models::EditRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct NewPublisherComponent { publisher: Publisher, push_publisher: PushCreatePublisher, @@ -116,22 +118,14 @@ impl Component for NewPublisherComponent { .publisher .publisher_name .neq_assign(publisher_name.trim().to_owned()), - Msg::ChangePublisherShortname(value) => { - let publisher_shortname = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.publisher - .publisher_shortname - .neq_assign(publisher_shortname) - } - Msg::ChangePublisherUrl(value) => { - let publisher_url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.publisher.publisher_url.neq_assign(publisher_url) - } + Msg::ChangePublisherShortname(value) => self + .publisher + .publisher_shortname + .neq_assign(value.to_opt_string()), + Msg::ChangePublisherUrl(value) => self + .publisher + .publisher_url + .neq_assign(value.to_opt_string()), Msg::ChangeRoute(r) => { let route = Route::from(r); self.router.send(RouteRequest::ChangeRoute(route)); diff --git a/thoth-app/src/component/new_series.rs b/thoth-app/src/component/new_series.rs index 2a6912a4..4b2acf15 100644 --- a/thoth-app/src/component/new_series.rs +++ b/thoth-app/src/component/new_series.rs @@ -41,6 +41,8 @@ use crate::models::EditRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct NewSeriesComponent { series: Series, push_series: PushCreateSeries, @@ -219,13 +221,7 @@ impl Component for NewSeriesComponent { .series .issn_digital .neq_assign(issn_digital.trim().to_owned()), - Msg::ChangeSeriesUrl(value) => { - let series_url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.series.series_url.neq_assign(series_url) - } + Msg::ChangeSeriesUrl(value) => self.series.series_url.neq_assign(value.to_opt_string()), Msg::ChangeRoute(r) => { let route = Route::from(r); self.router.send(RouteRequest::ChangeRoute(route)); diff --git a/thoth-app/src/component/new_work.rs b/thoth-app/src/component/new_work.rs index 1f992fbb..5c5e96cc 100644 --- a/thoth-app/src/component/new_work.rs +++ b/thoth-app/src/component/new_work.rs @@ -57,6 +57,8 @@ use crate::models::EditRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct NewWorkComponent { work: WorkWithRelations, // Track the user-entered DOI string, which may not be validly formatted @@ -351,11 +353,7 @@ impl Component for NewWorkComponent { } } Msg::ChangeSubtitle(value) => { - let subtitle = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - if self.work.subtitle.neq_assign(subtitle) { + if self.work.subtitle.neq_assign(value.to_opt_string()) { self.work.full_title = self.work.compile_fulltitle(); true } else { @@ -364,13 +362,7 @@ impl Component for NewWorkComponent { } Msg::ChangeWorkType(work_type) => self.work.work_type.neq_assign(work_type), Msg::ChangeWorkStatus(work_status) => self.work.work_status.neq_assign(work_status), - Msg::ChangeReference(value) => { - let reference = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.reference.neq_assign(reference) - } + Msg::ChangeReference(value) => self.work.reference.neq_assign(value.to_opt_string()), Msg::ChangeImprint(imprint_id) => self.imprint_id.neq_assign(imprint_id), Msg::ChangeEdition(edition) => { let edition: i32 = edition.parse().unwrap_or(1); @@ -397,160 +389,43 @@ impl Component for NewWorkComponent { false } } - Msg::ChangeDate(value) => { - let date = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.publication_date.neq_assign(date) - } - Msg::ChangePlace(value) => { - let place = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.place.neq_assign(place) - } - Msg::ChangeWidth(value) => { - let count: f64 = value.parse().unwrap_or(0.0); - let width = match count == 0.0 { - true => None, - false => Some(count), - }; - self.work.width.neq_assign(width) - } - Msg::ChangeHeight(value) => { - let count: f64 = value.parse().unwrap_or(0.0); - let height = match count == 0.0 { - true => None, - false => Some(count), - }; - self.work.height.neq_assign(height) - } + Msg::ChangeDate(value) => self.work.publication_date.neq_assign(value.to_opt_string()), + Msg::ChangePlace(value) => self.work.place.neq_assign(value.to_opt_string()), + Msg::ChangeWidth(value) => self.work.width.neq_assign(value.to_opt_float()), + Msg::ChangeHeight(value) => self.work.height.neq_assign(value.to_opt_float()), Msg::ChangeLengthUnit(length_unit) => { self.props.update_units_selection.emit(length_unit); false } - Msg::ChangePageCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let page_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.page_count.neq_assign(page_count) - } + Msg::ChangePageCount(value) => self.work.page_count.neq_assign(value.to_opt_int()), Msg::ChangePageBreakdown(value) => { - let breakdown = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.page_breakdown.neq_assign(breakdown) - } - Msg::ChangeImageCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let image_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.image_count.neq_assign(image_count) - } - Msg::ChangeTableCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let table_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.table_count.neq_assign(table_count) - } - Msg::ChangeAudioCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let audio_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.audio_count.neq_assign(audio_count) - } - Msg::ChangeVideoCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let video_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.video_count.neq_assign(video_count) - } - Msg::ChangeLicense(value) => { - let license = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.license.neq_assign(license) + self.work.page_breakdown.neq_assign(value.to_opt_string()) } + Msg::ChangeImageCount(value) => self.work.image_count.neq_assign(value.to_opt_int()), + Msg::ChangeTableCount(value) => self.work.table_count.neq_assign(value.to_opt_int()), + Msg::ChangeAudioCount(value) => self.work.audio_count.neq_assign(value.to_opt_int()), + Msg::ChangeVideoCount(value) => self.work.video_count.neq_assign(value.to_opt_int()), + Msg::ChangeLicense(value) => self.work.license.neq_assign(value.to_opt_string()), Msg::ChangeCopyright(copyright) => self .work .copyright_holder .neq_assign(copyright.trim().to_owned()), Msg::ChangeLandingPage(value) => { - let landing_page = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.landing_page.neq_assign(landing_page) - } - Msg::ChangeLccn(value) => { - let lccn = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.lccn.neq_assign(lccn) - } - Msg::ChangeOclc(value) => { - let oclc = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.oclc.neq_assign(oclc) + self.work.landing_page.neq_assign(value.to_opt_string()) } + Msg::ChangeLccn(value) => self.work.lccn.neq_assign(value.to_opt_string()), + Msg::ChangeOclc(value) => self.work.oclc.neq_assign(value.to_opt_string()), Msg::ChangeShortAbstract(value) => { - let short_abstract = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.short_abstract.neq_assign(short_abstract) + self.work.short_abstract.neq_assign(value.to_opt_string()) } Msg::ChangeLongAbstract(value) => { - let long_abstract = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.long_abstract.neq_assign(long_abstract) - } - Msg::ChangeNote(value) => { - let note = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.general_note.neq_assign(note) - } - Msg::ChangeToc(value) => { - let toc = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.toc.neq_assign(toc) - } - Msg::ChangeCoverUrl(value) => { - let cover_url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.cover_url.neq_assign(cover_url) + self.work.long_abstract.neq_assign(value.to_opt_string()) } + Msg::ChangeNote(value) => self.work.general_note.neq_assign(value.to_opt_string()), + Msg::ChangeToc(value) => self.work.toc.neq_assign(value.to_opt_string()), + Msg::ChangeCoverUrl(value) => self.work.cover_url.neq_assign(value.to_opt_string()), Msg::ChangeCoverCaption(value) => { - let cover_caption = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.cover_caption.neq_assign(cover_caption) + self.work.cover_caption.neq_assign(value.to_opt_string()) } Msg::ChangeRoute(r) => { let route = Route::from(r); diff --git a/thoth-app/src/component/publications_form.rs b/thoth-app/src/component/publications_form.rs index 27d7ebeb..c3864ed3 100644 --- a/thoth-app/src/component/publications_form.rs +++ b/thoth-app/src/component/publications_form.rs @@ -43,6 +43,8 @@ use crate::string::EMPTY_PUBLICATIONS; use crate::string::REMOVE_BUTTON; use crate::string::VIEW_BUTTON; +use super::ToOption; + pub struct PublicationsFormComponent { props: Props, data: PublicationsFormData, @@ -284,13 +286,10 @@ impl Component for PublicationsFormComponent { false } } - Msg::ChangeUrl(value) => { - let url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.new_publication.publication_url.neq_assign(url) - } + Msg::ChangeUrl(value) => self + .new_publication + .publication_url + .neq_assign(value.to_opt_string()), Msg::ChangeRoute(r) => { let route = Route::from(r); self.router.send(RouteRequest::ChangeRoute(route)); diff --git a/thoth-app/src/component/publisher.rs b/thoth-app/src/component/publisher.rs index aa5d2652..71cd9612 100644 --- a/thoth-app/src/component/publisher.rs +++ b/thoth-app/src/component/publisher.rs @@ -40,6 +40,8 @@ use crate::route::AdminRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct PublisherComponent { publisher: Publisher, fetch_publisher: FetchPublisher, @@ -239,22 +241,14 @@ impl Component for PublisherComponent { .publisher .publisher_name .neq_assign(publisher_name.trim().to_owned()), - Msg::ChangePublisherShortname(value) => { - let publisher_shortname = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.publisher - .publisher_shortname - .neq_assign(publisher_shortname) - } - Msg::ChangePublisherUrl(value) => { - let publisher_url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.publisher.publisher_url.neq_assign(publisher_url) - } + Msg::ChangePublisherShortname(value) => self + .publisher + .publisher_shortname + .neq_assign(value.to_opt_string()), + Msg::ChangePublisherUrl(value) => self + .publisher + .publisher_url + .neq_assign(value.to_opt_string()), Msg::ChangeRoute(r) => { let route = Route::from(r); self.router.send(RouteRequest::ChangeRoute(route)); diff --git a/thoth-app/src/component/series.rs b/thoth-app/src/component/series.rs index 55111b76..8a730f62 100644 --- a/thoth-app/src/component/series.rs +++ b/thoth-app/src/component/series.rs @@ -53,6 +53,8 @@ use crate::route::AdminRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct SeriesComponent { series: SeriesWithImprint, fetch_series: FetchSeries, @@ -336,13 +338,7 @@ impl Component for SeriesComponent { .series .issn_digital .neq_assign(issn_digital.trim().to_owned()), - Msg::ChangeSeriesUrl(value) => { - let series_url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.series.series_url.neq_assign(series_url) - } + Msg::ChangeSeriesUrl(value) => self.series.series_url.neq_assign(value.to_opt_string()), Msg::ChangeRoute(r) => { let route = Route::from(r); self.router.send(RouteRequest::ChangeRoute(route)); diff --git a/thoth-app/src/component/work.rs b/thoth-app/src/component/work.rs index 1ef49932..7b2cc343 100644 --- a/thoth-app/src/component/work.rs +++ b/thoth-app/src/component/work.rs @@ -70,6 +70,8 @@ use crate::route::AdminRoute; use crate::route::AppRoute; use crate::string::SAVE_BUTTON; +use super::ToOption; + pub struct WorkComponent { work: WorkWithRelations, // Track the user-entered DOI string, which may not be validly formatted @@ -388,11 +390,7 @@ impl Component for WorkComponent { } } Msg::ChangeSubtitle(value) => { - let subtitle = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - if self.work.subtitle.neq_assign(subtitle) { + if self.work.subtitle.neq_assign(value.to_opt_string()) { self.work.full_title = self.work.compile_fulltitle(); true } else { @@ -401,13 +399,7 @@ impl Component for WorkComponent { } Msg::ChangeWorkType(work_type) => self.work.work_type.neq_assign(work_type), Msg::ChangeWorkStatus(work_status) => self.work.work_status.neq_assign(work_status), - Msg::ChangeReference(value) => { - let reference = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.reference.neq_assign(reference) - } + Msg::ChangeReference(value) => self.work.reference.neq_assign(value.to_opt_string()), Msg::ChangeImprint(imprint_id) => { // we already have the full list of imprints if let Some(index) = self @@ -448,36 +440,10 @@ impl Component for WorkComponent { false } } - Msg::ChangeDate(value) => { - let date = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.publication_date.neq_assign(date) - } - Msg::ChangePlace(value) => { - let place = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.place.neq_assign(place) - } - Msg::ChangeWidth(value) => { - let count: f64 = value.parse().unwrap_or(0.0); - let width = match count == 0.0 { - true => None, - false => Some(count), - }; - self.work.width.neq_assign(width) - } - Msg::ChangeHeight(value) => { - let count: f64 = value.parse().unwrap_or(0.0); - let height = match count == 0.0 { - true => None, - false => Some(count), - }; - self.work.height.neq_assign(height) - } + Msg::ChangeDate(value) => self.work.publication_date.neq_assign(value.to_opt_string()), + Msg::ChangePlace(value) => self.work.place.neq_assign(value.to_opt_string()), + Msg::ChangeWidth(value) => self.work.width.neq_assign(value.to_opt_float()), + Msg::ChangeHeight(value) => self.work.height.neq_assign(value.to_opt_float()), Msg::ChangeLengthUnit(length_unit) => { self.props.update_units_selection.emit(length_unit); // Callback will prompt parent to update this component's props. @@ -485,126 +451,35 @@ impl Component for WorkComponent { // to also re-render here. false } - Msg::ChangePageCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let page_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.page_count.neq_assign(page_count) - } + Msg::ChangePageCount(value) => self.work.page_count.neq_assign(value.to_opt_int()), Msg::ChangePageBreakdown(value) => { - let breakdown = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.page_breakdown.neq_assign(breakdown) - } - Msg::ChangeImageCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let image_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.image_count.neq_assign(image_count) - } - Msg::ChangeTableCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let table_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.table_count.neq_assign(table_count) - } - Msg::ChangeAudioCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let audio_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.audio_count.neq_assign(audio_count) - } - Msg::ChangeVideoCount(value) => { - let count: i32 = value.parse().unwrap_or(0); - let video_count = match count == 0 { - true => None, - false => Some(count), - }; - self.work.video_count.neq_assign(video_count) - } - Msg::ChangeLicense(value) => { - let license = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.license.neq_assign(license) + self.work.page_breakdown.neq_assign(value.to_opt_string()) } + Msg::ChangeImageCount(value) => self.work.image_count.neq_assign(value.to_opt_int()), + Msg::ChangeTableCount(value) => self.work.table_count.neq_assign(value.to_opt_int()), + Msg::ChangeAudioCount(value) => self.work.audio_count.neq_assign(value.to_opt_int()), + Msg::ChangeVideoCount(value) => self.work.video_count.neq_assign(value.to_opt_int()), + Msg::ChangeLicense(value) => self.work.license.neq_assign(value.to_opt_string()), Msg::ChangeCopyright(copyright) => self .work .copyright_holder .neq_assign(copyright.trim().to_owned()), Msg::ChangeLandingPage(value) => { - let landing_page = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.landing_page.neq_assign(landing_page) - } - Msg::ChangeLccn(value) => { - let lccn = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.lccn.neq_assign(lccn) - } - Msg::ChangeOclc(value) => { - let oclc = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.oclc.neq_assign(oclc) + self.work.landing_page.neq_assign(value.to_opt_string()) } + Msg::ChangeLccn(value) => self.work.lccn.neq_assign(value.to_opt_string()), + Msg::ChangeOclc(value) => self.work.oclc.neq_assign(value.to_opt_string()), Msg::ChangeShortAbstract(value) => { - let short_abstract = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.short_abstract.neq_assign(short_abstract) + self.work.short_abstract.neq_assign(value.to_opt_string()) } Msg::ChangeLongAbstract(value) => { - let long_abstract = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.long_abstract.neq_assign(long_abstract) - } - Msg::ChangeNote(value) => { - let note = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.general_note.neq_assign(note) - } - Msg::ChangeToc(value) => { - let toc = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.toc.neq_assign(toc) - } - Msg::ChangeCoverUrl(value) => { - let cover_url = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.cover_url.neq_assign(cover_url) + self.work.long_abstract.neq_assign(value.to_opt_string()) } + Msg::ChangeNote(value) => self.work.general_note.neq_assign(value.to_opt_string()), + Msg::ChangeToc(value) => self.work.toc.neq_assign(value.to_opt_string()), + Msg::ChangeCoverUrl(value) => self.work.cover_url.neq_assign(value.to_opt_string()), Msg::ChangeCoverCaption(value) => { - let cover_caption = match value.trim().is_empty() { - true => None, - false => Some(value.trim().to_owned()), - }; - self.work.cover_caption.neq_assign(cover_caption) + self.work.cover_caption.neq_assign(value.to_opt_string()) } Msg::UpdateContributions(contributions) => { self.work.contributions.neq_assign(contributions) From 4897754e6d9e33b11217db4de423b1c8ae9e4f2a Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Tue, 5 Oct 2021 17:14:26 +0100 Subject: [PATCH 09/38] Use trait to replace Option creation logic which previously wasn't trimming strings before checking emptiness (bug) --- thoth-app/src/component/contributions_form.rs | 35 +++++------- thoth-app/src/component/fundings_form.rs | 54 +++++++------------ 2 files changed, 33 insertions(+), 56 deletions(-) diff --git a/thoth-app/src/component/contributions_form.rs b/thoth-app/src/component/contributions_form.rs index 0f45607c..57c7771e 100644 --- a/thoth-app/src/component/contributions_form.rs +++ b/thoth-app/src/component/contributions_form.rs @@ -45,6 +45,8 @@ use crate::string::NO; use crate::string::REMOVE_BUTTON; use crate::string::YES; +use super::ToOption; + pub struct ContributionsFormComponent { props: Props, data: ContributionsFormData, @@ -311,29 +313,20 @@ impl Component for ContributionsFormComponent { self.link.send_message(Msg::GetContributors); false } - Msg::ChangeFirstName(val) => { - let value = match val.is_empty() { - true => None, - false => Some(val), - }; - self.new_contribution.first_name.neq_assign(value) - } + Msg::ChangeFirstName(val) => self + .new_contribution + .first_name + .neq_assign(val.to_opt_string()), Msg::ChangeLastName(val) => self.new_contribution.last_name.neq_assign(val), Msg::ChangeFullName(val) => self.new_contribution.full_name.neq_assign(val), - Msg::ChangeInstitution(val) => { - let value = match val.is_empty() { - true => None, - false => Some(val), - }; - self.new_contribution.institution.neq_assign(value) - } - Msg::ChangeBiography(val) => { - let value = match val.is_empty() { - true => None, - false => Some(val), - }; - self.new_contribution.biography.neq_assign(value) - } + Msg::ChangeInstitution(val) => self + .new_contribution + .institution + .neq_assign(val.to_opt_string()), + Msg::ChangeBiography(val) => self + .new_contribution + .biography + .neq_assign(val.to_opt_string()), Msg::ChangeContributiontype(val) => { self.new_contribution.contribution_type.neq_assign(val) } diff --git a/thoth-app/src/component/fundings_form.rs b/thoth-app/src/component/fundings_form.rs index 453e16dd..42f056ab 100644 --- a/thoth-app/src/component/fundings_form.rs +++ b/thoth-app/src/component/fundings_form.rs @@ -35,6 +35,8 @@ use crate::string::CANCEL_BUTTON; use crate::string::EMPTY_FUNDINGS; use crate::string::REMOVE_BUTTON; +use super::ToOption; + pub struct FundingsFormComponent { props: Props, data: FundingsFormData, @@ -259,41 +261,23 @@ impl Component for FundingsFormComponent { self.link.send_message(Msg::GetFunders); false } - Msg::ChangeProgram(val) => { - let value = match val.is_empty() { - true => None, - false => Some(val), - }; - self.new_funding.program.neq_assign(value) - } - Msg::ChangeProjectName(val) => { - let value = match val.is_empty() { - true => None, - false => Some(val), - }; - self.new_funding.project_name.neq_assign(value) - } - Msg::ChangeProjectShortname(val) => { - let value = match val.is_empty() { - true => None, - false => Some(val), - }; - self.new_funding.project_shortname.neq_assign(value) - } - Msg::ChangeGrant(val) => { - let value = match val.is_empty() { - true => None, - false => Some(val), - }; - self.new_funding.grant_number.neq_assign(value) - } - Msg::ChangeJurisdiction(val) => { - let value = match val.is_empty() { - true => None, - false => Some(val), - }; - self.new_funding.jurisdiction.neq_assign(value) - } + Msg::ChangeProgram(val) => self.new_funding.program.neq_assign(val.to_opt_string()), + Msg::ChangeProjectName(val) => self + .new_funding + .project_name + .neq_assign(val.to_opt_string()), + Msg::ChangeProjectShortname(val) => self + .new_funding + .project_shortname + .neq_assign(val.to_opt_string()), + Msg::ChangeGrant(val) => self + .new_funding + .grant_number + .neq_assign(val.to_opt_string()), + Msg::ChangeJurisdiction(val) => self + .new_funding + .jurisdiction + .neq_assign(val.to_opt_string()), Msg::DoNothing => false, // callbacks need to return a message } } From 69580010ebaacd2d38ac2c15392b9db0a0ba7f20 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Wed, 6 Oct 2021 09:45:34 +0100 Subject: [PATCH 10/38] Ensure all non-optional strings are being trimmed before saving (contributions/subjects bug) --- thoth-app/src/component/contributions_form.rs | 10 ++++++++-- thoth-app/src/component/subjects_form.rs | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/thoth-app/src/component/contributions_form.rs b/thoth-app/src/component/contributions_form.rs index 57c7771e..e146edfc 100644 --- a/thoth-app/src/component/contributions_form.rs +++ b/thoth-app/src/component/contributions_form.rs @@ -317,8 +317,14 @@ impl Component for ContributionsFormComponent { .new_contribution .first_name .neq_assign(val.to_opt_string()), - Msg::ChangeLastName(val) => self.new_contribution.last_name.neq_assign(val), - Msg::ChangeFullName(val) => self.new_contribution.full_name.neq_assign(val), + Msg::ChangeLastName(val) => self + .new_contribution + .last_name + .neq_assign(val.trim().to_owned()), + Msg::ChangeFullName(val) => self + .new_contribution + .full_name + .neq_assign(val.trim().to_owned()), Msg::ChangeInstitution(val) => self .new_contribution .institution diff --git a/thoth-app/src/component/subjects_form.rs b/thoth-app/src/component/subjects_form.rs index 88bfac6c..97ea3454 100644 --- a/thoth-app/src/component/subjects_form.rs +++ b/thoth-app/src/component/subjects_form.rs @@ -226,7 +226,10 @@ impl Component for SubjectsFormComponent { false } Msg::ChangeSubjectType(val) => self.new_subject.subject_type.neq_assign(val), - Msg::ChangeCode(code) => self.new_subject.subject_code.neq_assign(code), + Msg::ChangeCode(code) => self + .new_subject + .subject_code + .neq_assign(code.trim().to_owned()), Msg::ChangeOrdinal(ordinal) => { let ordinal = ordinal.parse::().unwrap_or(0); self.new_subject.subject_ordinal.neq_assign(ordinal); From b25f72fa7c89d83c3ca5cfac6a46865e3ec758a7 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Wed, 6 Oct 2021 14:20:00 +0100 Subject: [PATCH 11/38] Add missing required field markers and bring Contribution Ordinal default into line with Issue/Subject --- thoth-api/src/model/contribution/mod.rs | 22 ++++++++++++++++++- thoth-app/src/component/contributions_form.rs | 3 +++ thoth-app/src/component/contributor.rs | 1 + thoth-app/src/component/issues_form.rs | 1 + thoth-app/src/component/subjects_form.rs | 2 ++ 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/thoth-api/src/model/contribution/mod.rs b/thoth-api/src/model/contribution/mod.rs index 42f823ee..805153d3 100644 --- a/thoth-api/src/model/contribution/mod.rs +++ b/thoth-api/src/model/contribution/mod.rs @@ -55,7 +55,7 @@ pub enum ContributionField { } #[cfg_attr(feature = "backend", derive(Queryable))] -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct Contribution { pub contribution_id: Uuid, @@ -143,6 +143,26 @@ 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: Default::default(), + institution: Default::default(), + created_at: Default::default(), + updated_at: Default::default(), + first_name: Default::default(), + last_name: Default::default(), + full_name: Default::default(), + contribution_ordinal: 1, + } + } +} + #[test] fn test_contributiontype_default() { let contributiontype: ContributionType = Default::default(); diff --git a/thoth-app/src/component/contributions_form.rs b/thoth-app/src/component/contributions_form.rs index e146edfc..855036a9 100644 --- a/thoth-app/src/component/contributions_form.rs +++ b/thoth-app/src/component/contributions_form.rs @@ -428,11 +428,13 @@ impl Component for ContributionsFormComponent { label="Contributor's Family Name" value=self.new_contribution.last_name.clone() oninput=self.link.callback(|e: InputData| Msg::ChangeLastName(e.value)) + required = true /> diff --git a/thoth-app/src/component/contributor.rs b/thoth-app/src/component/contributor.rs index d707d2d8..e838cedb 100644 --- a/thoth-app/src/component/contributor.rs +++ b/thoth-app/src/component/contributor.rs @@ -397,6 +397,7 @@ impl Component for ContributorComponent { label = "Family Name" value=self.contributor.last_name.clone() oninput=self.link.callback(|e: InputData| Msg::ChangeLastName(e.value)) + required = true /> diff --git a/thoth-app/src/component/subjects_form.rs b/thoth-app/src/component/subjects_form.rs index 97ea3454..3f8c59eb 100644 --- a/thoth-app/src/component/subjects_form.rs +++ b/thoth-app/src/component/subjects_form.rs @@ -302,11 +302,13 @@ impl Component for SubjectsFormComponent { label = "Subject Code" value=self.new_subject.subject_code.clone() oninput=self.link.callback(|e: InputData| Msg::ChangeCode(e.value)) + required = true /> From d786421b4acef3284f4e6a7aaeee8cc7a6376bc1 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Thu, 7 Oct 2021 11:56:30 +0100 Subject: [PATCH 12/38] Adjust subform save button logic so that field checks (e.g. required, numeric, URL) become active --- thoth-app/src/component/contributions_form.rs | 12 ++++-------- thoth-app/src/component/fundings_form.rs | 12 ++++-------- thoth-app/src/component/issues_form.rs | 12 ++++-------- thoth-app/src/component/languages_form.rs | 12 ++++-------- thoth-app/src/component/prices_form.rs | 12 ++++-------- thoth-app/src/component/publications_form.rs | 12 ++++-------- thoth-app/src/component/subjects_form.rs | 12 ++++-------- 7 files changed, 28 insertions(+), 56 deletions(-) diff --git a/thoth-app/src/component/contributions_form.rs b/thoth-app/src/component/contributions_form.rs index 855036a9..129b6745 100644 --- a/thoth-app/src/component/contributions_form.rs +++ b/thoth-app/src/component/contributions_form.rs @@ -88,7 +88,6 @@ pub enum Msg { ChangeContributiontype(ContributionType), ChangeMainContribution(bool), ChangeOrdinal(String), - DoNothing, } #[derive(Clone, Properties, PartialEq)] @@ -346,7 +345,6 @@ impl Component for ContributionsFormComponent { .neq_assign(ordinal); false // otherwise we re-render the component and reset the value } - Msg::DoNothing => false, // callbacks need to return a message } } @@ -414,9 +412,9 @@ impl Component for ContributionsFormComponent { > diff --git a/thoth-app/src/component/utils.rs b/thoth-app/src/component/utils.rs index 75c48f0a..6fe08f2d 100644 --- a/thoth-app/src/component/utils.rs +++ b/thoth-app/src/component/utils.rs @@ -92,6 +92,7 @@ pub struct PureTextarea { // Variant of PureTextInput which supports tooltips, // prepended static buttons, or both together. +// Also supports deactivating the input. #[derive(Clone, PartialEq, Properties)] pub struct PureTextInputExtended { pub label: String, @@ -108,6 +109,8 @@ pub struct PureTextInputExtended { pub onblur: Callback, #[prop_or(false)] pub required: bool, + #[prop_or(false)] + pub deactivated: bool, } #[derive(Clone, PartialEq, Properties)] @@ -178,6 +181,8 @@ pub struct PureNumberInput { pub struct PureWorkTypeSelect { pub label: String, pub data: Vec, + #[prop_or_default] + pub deactivate: Vec, pub value: WorkType, pub onchange: Callback, #[prop_or(false)] @@ -399,6 +404,7 @@ impl PureComponent for PureTextInputExtended { onfocus=self.onfocus.clone() onblur=self.onblur.clone() required={ self.required } + disabled={ self.deactivated } /> @@ -492,7 +498,7 @@ impl PureComponent for PureWorkTypeSelect {
@@ -761,8 +767,15 @@ impl PureComponent for PurePublisherSelect { } impl PureWorkTypeSelect { - fn render_worktype(&self, w: &WorkTypeValues) -> VNode { - if w.name == self.value { + fn render_worktype(&self, w: &WorkTypeValues, deactivate: &[WorkType]) -> VNode { + // It should not be possible for the selected option to require deactivation. + if deactivate.contains(&w.name) { + html! { + + } + } else if w.name == self.value { html! {
@@ -413,7 +429,8 @@ impl LocationsFormComponent {
{ REMOVE_BUTTON } diff --git a/thoth-errors/src/lib.rs b/thoth-errors/src/lib.rs index df161079..77337502 100644 --- a/thoth-errors/src/lib.rs +++ b/thoth-errors/src/lib.rs @@ -55,6 +55,8 @@ pub enum ThothError { IsbnEmptyError, #[fail(display = "Works of type Book Chapter cannot have ISBNs in their Publications.")] ChapterIsbnError, + #[fail(display = "Each Publication must have exactly one canonical Location.")] + CanonicalLocationError, } #[cfg(not(target_arch = "wasm32"))] From 708d396719f3d6c38106c9ad653d3e5a349cf7ac Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Thu, 21 Oct 2021 15:12:36 +0100 Subject: [PATCH 26/38] Make location landing page non-mandatory --- thoth-api/migrations/0.4.8/up.sql | 2 +- thoth-api/src/graphql/model.rs | 4 +-- thoth-api/src/model/location/mod.rs | 6 ++-- thoth-api/src/schema.rs | 2 +- thoth-app/src/component/locations_form.rs | 5 ++-- .../location/create_location_mutation.rs | 4 +-- .../location/delete_location_mutation.rs | 1 - thoth-client/assets/schema.json | 30 ++++++------------- thoth-export-server/src/csv/csv_thoth.rs | 14 ++++----- .../src/xml/onix21_ebsco_host.rs | 4 +-- thoth-export-server/src/xml/onix3_jstor.rs | 2 +- thoth-export-server/src/xml/onix3_oapen.rs | 2 +- .../src/xml/onix3_project_muse.rs | 2 +- 13 files changed, 32 insertions(+), 46 deletions(-) diff --git a/thoth-api/migrations/0.4.8/up.sql b/thoth-api/migrations/0.4.8/up.sql index 21421176..86010a21 100644 --- a/thoth-api/migrations/0.4.8/up.sql +++ b/thoth-api/migrations/0.4.8/up.sql @@ -8,7 +8,7 @@ CREATE TYPE location_platform AS ENUM ( CREATE TABLE location ( location_id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), publication_id UUID NOT NULL REFERENCES publication(publication_id) ON DELETE CASCADE, - landing_page TEXT NOT NULL CHECK (landing_page ~* '^[^:]*:\/\/(?:[^\/:]*:[^\/@]*@)?(?:[^\/:.]*\.)+([^:\/]+)'), + landing_page TEXT CHECK (landing_page ~* '^[^:]*:\/\/(?:[^\/:]*:[^\/@]*@)?(?:[^\/:.]*\.)+([^:\/]+)'), full_text_url TEXT CHECK (full_text_url ~* '^[^:]*:\/\/(?:[^\/:]*:[^\/@]*@)?(?:[^\/:.]*\.)+([^:\/]+)'), location_platform location_platform NOT NULL DEFAULT 'Other', canonical BOOLEAN NOT NULL DEFAULT False, diff --git a/thoth-api/src/graphql/model.rs b/thoth-api/src/graphql/model.rs index 728c91bb..6e7ce3c9 100644 --- a/thoth-api/src/graphql/model.rs +++ b/thoth-api/src/graphql/model.rs @@ -2520,8 +2520,8 @@ impl Location { self.publication_id } - pub fn landing_page(&self) -> &String { - &self.landing_page + pub fn landing_page(&self) -> Option<&String> { + self.landing_page.as_ref() } pub fn full_text_url(&self) -> Option<&String> { diff --git a/thoth-api/src/model/location/mod.rs b/thoth-api/src/model/location/mod.rs index 0dd9bdce..afeae0ca 100644 --- a/thoth-api/src/model/location/mod.rs +++ b/thoth-api/src/model/location/mod.rs @@ -49,7 +49,7 @@ pub enum LocationField { pub struct Location { pub location_id: Uuid, pub publication_id: Uuid, - pub landing_page: String, + pub landing_page: Option, pub full_text_url: Option, pub location_platform: LocationPlatform, pub canonical: bool, @@ -64,7 +64,7 @@ pub struct Location { )] pub struct NewLocation { pub publication_id: Uuid, - pub landing_page: String, + pub landing_page: Option, pub full_text_url: Option, pub location_platform: LocationPlatform, pub canonical: bool, @@ -79,7 +79,7 @@ pub struct NewLocation { pub struct PatchLocation { pub location_id: Uuid, pub publication_id: Uuid, - pub landing_page: String, + pub landing_page: Option, pub full_text_url: Option, pub location_platform: LocationPlatform, pub canonical: bool, diff --git a/thoth-api/src/schema.rs b/thoth-api/src/schema.rs index b506515e..07fc13b8 100644 --- a/thoth-api/src/schema.rs +++ b/thoth-api/src/schema.rs @@ -215,7 +215,7 @@ table! { location (location_id) { location_id -> Uuid, publication_id -> Uuid, - landing_page -> Text, + landing_page -> Nullable, full_text_url -> Nullable, location_platform -> Location_platform, canonical -> Bool, diff --git a/thoth-app/src/component/locations_form.rs b/thoth-app/src/component/locations_form.rs index e075fb1b..8f5bcce3 100644 --- a/thoth-app/src/component/locations_form.rs +++ b/thoth-app/src/component/locations_form.rs @@ -237,7 +237,7 @@ impl Component for LocationsFormComponent { .send_message(Msg::SetLocationDeleteState(FetchAction::Fetching)); false } - Msg::ChangeLandingPage(val) => self.new_location.landing_page.neq_assign(val), + Msg::ChangeLandingPage(val) => self.new_location.landing_page.neq_assign(val.to_opt_string()), Msg::ChangeFullTextUrl(val) => self .new_location .full_text_url @@ -297,7 +297,6 @@ impl Component for LocationsFormComponent { label="Landing Page" value=self.new_location.landing_page.clone() oninput=self.link.callback(|e: InputData| Msg::ChangeLandingPage(e.value)) - required = true />
- {&l.landing_page} + {&l.landing_page.clone().unwrap_or_else(|| "".to_string())}
diff --git a/thoth-app/src/models/location/create_location_mutation.rs b/thoth-app/src/models/location/create_location_mutation.rs index 3049b923..d51b1f5b 100644 --- a/thoth-app/src/models/location/create_location_mutation.rs +++ b/thoth-app/src/models/location/create_location_mutation.rs @@ -7,7 +7,7 @@ use uuid::Uuid; const CREATE_LOCATION_MUTATION: &str = " mutation CreateLocation( $publicationId: Uuid!, - $landingPage: String!, + $landingPage: String, $fullTextUrl: String, $locationPlatform: LocationPlatform!, $canonical: Boolean! @@ -46,7 +46,7 @@ graphql_query_builder! { #[serde(rename_all = "camelCase")] pub struct Variables { pub publication_id: Uuid, - pub landing_page: String, + pub landing_page: Option, pub full_text_url: Option, pub location_platform: LocationPlatform, pub canonical: bool, diff --git a/thoth-app/src/models/location/delete_location_mutation.rs b/thoth-app/src/models/location/delete_location_mutation.rs index 32f8de85..441191b3 100644 --- a/thoth-app/src/models/location/delete_location_mutation.rs +++ b/thoth-app/src/models/location/delete_location_mutation.rs @@ -12,7 +12,6 @@ const DELETE_LOCATION_MUTATION: &str = " ){ locationId publicationId - landingPage locationPlatform canonical createdAt diff --git a/thoth-client/assets/schema.json b/thoth-client/assets/schema.json index 97ccdced..4b8f705b 100644 --- a/thoth-client/assets/schema.json +++ b/thoth-client/assets/schema.json @@ -9964,13 +9964,9 @@ "description": null, "name": "landingPage", "type": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "SCALAR", - "name": "String", - "ofType": null - } + "kind": "SCALAR", + "name": "String", + "ofType": null } }, { @@ -10477,13 +10473,9 @@ "description": null, "name": "landingPage", "type": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "SCALAR", - "name": "String", - "ofType": null - } + "kind": "SCALAR", + "name": "String", + "ofType": null } }, { @@ -10693,13 +10685,9 @@ "isDeprecated": false, "name": "landingPage", "type": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "SCALAR", - "name": "String", - "ofType": null - } + "kind": "SCALAR", + "name": "String", + "ofType": null } }, { diff --git a/thoth-export-server/src/csv/csv_thoth.rs b/thoth-export-server/src/csv/csv_thoth.rs index 5f0b9f7e..4f514106 100644 --- a/thoth-export-server/src/csv/csv_thoth.rs +++ b/thoth-export-server/src/csv/csv_thoth.rs @@ -280,7 +280,7 @@ impl CsvCell for WorkPublicationsLocations { fn csv_cell(&self) -> String { format!( "(\"{}\", \"{}\", \"{:?}\", \"{}\")", - self.landing_page, + self.landing_page.clone().unwrap_or_else(|| "".to_string()), self.full_text_url.clone().unwrap_or_else(|| "".to_string()), self.location_platform, self.canonical, @@ -464,13 +464,13 @@ mod tests { ], locations: vec![ WorkPublicationsLocations { - landing_page: "https://www.book.com/paperback".to_string(), + landing_page: Some("https://www.book.com/paperback".to_string()), full_text_url: None, location_platform: LocationPlatform::OTHER, canonical: true, }, WorkPublicationsLocations { - landing_page: "https://www.jstor.com/paperback".to_string(), + landing_page: Some("https://www.jstor.com/paperback".to_string()), full_text_url: None, location_platform: LocationPlatform::JSTOR, canonical: false, @@ -503,7 +503,7 @@ mod tests { isbn: Some(Isbn::from_str("978-1-56619-909-4").unwrap()), prices: vec![], locations: vec![WorkPublicationsLocations { - landing_page: "https://www.book.com/pdf_landing".to_string(), + landing_page: Some("https://www.book.com/pdf_landing".to_string()), full_text_url: Some("https://www.book.com/pdf_fulltext".to_string()), location_platform: LocationPlatform::OTHER, canonical: true, @@ -515,7 +515,7 @@ mod tests { isbn: None, prices: vec![], locations: vec![WorkPublicationsLocations { - landing_page: "https://www.book.com/html_landing".to_string(), + landing_page: Some("https://www.book.com/html_landing".to_string()), full_text_url: Some("https://www.book.com/html_fulltext".to_string()), location_platform: LocationPlatform::OTHER, canonical: true, @@ -625,7 +625,7 @@ mod tests { unit_price: 25.95, }], locations: vec![WorkPublicationsLocations { - landing_page: "https://www.book.com/paperback".to_string(), + landing_page: Some("https://www.book.com/paperback".to_string()), full_text_url: None, location_platform: LocationPlatform::PROJECT_MUSE, canonical: true, @@ -694,7 +694,7 @@ mod tests { #[test] fn test_csv_thoth_locations() { let mut location = WorkPublicationsLocations { - landing_page: "https://www.book.com/pdf_landing".to_string(), + landing_page: Some("https://www.book.com/pdf_landing".to_string()), full_text_url: Some("https://www.book.com/pdf_fulltext".to_string()), location_platform: LocationPlatform::OTHER, canonical: true, diff --git a/thoth-export-server/src/xml/onix21_ebsco_host.rs b/thoth-export-server/src/xml/onix21_ebsco_host.rs index f1f05334..44c87c53 100644 --- a/thoth-export-server/src/xml/onix21_ebsco_host.rs +++ b/thoth-export-server/src/xml/onix21_ebsco_host.rs @@ -899,7 +899,7 @@ mod tests { isbn: Some(Isbn::from_str("978-3-16-148410-0").unwrap()), prices: vec![], locations: vec![WorkPublicationsLocations { - landing_page: "https://www.book.com/epub_landing".to_string(), + landing_page: Some("https://www.book.com/epub_landing".to_string()), full_text_url: Some("https://www.book.com/epub_fulltext".to_string()), location_platform: LocationPlatform::OTHER, canonical: true, @@ -924,7 +924,7 @@ mod tests { }, ], locations: vec![WorkPublicationsLocations { - landing_page: "https://www.book.com/pdf_landing".to_string(), + landing_page: Some("https://www.book.com/pdf_landing".to_string()), full_text_url: Some("https://www.book.com/pdf_fulltext".to_string()), location_platform: LocationPlatform::OTHER, canonical: true, diff --git a/thoth-export-server/src/xml/onix3_jstor.rs b/thoth-export-server/src/xml/onix3_jstor.rs index 93d8c4c8..7c8e5923 100644 --- a/thoth-export-server/src/xml/onix3_jstor.rs +++ b/thoth-export-server/src/xml/onix3_jstor.rs @@ -729,7 +729,7 @@ mod tests { isbn: Some(Isbn::from_str("978-3-16-148410-0").unwrap()), prices: vec![], locations: vec![WorkPublicationsLocations { - landing_page: "https://www.book.com/pdf_landing".to_string(), + landing_page: Some("https://www.book.com/pdf_landing".to_string()), full_text_url: Some("https://www.book.com/pdf_fulltext".to_string()), location_platform: LocationPlatform::OTHER, canonical: true, diff --git a/thoth-export-server/src/xml/onix3_oapen.rs b/thoth-export-server/src/xml/onix3_oapen.rs index 363e5995..a811c3e4 100644 --- a/thoth-export-server/src/xml/onix3_oapen.rs +++ b/thoth-export-server/src/xml/onix3_oapen.rs @@ -993,7 +993,7 @@ mod tests { isbn: Some(Isbn::from_str("978-3-16-148410-0").unwrap()), prices: vec![], locations: vec![WorkPublicationsLocations { - landing_page: "https://www.book.com/pdf_landing".to_string(), + landing_page: Some("https://www.book.com/pdf_landing".to_string()), full_text_url: Some("https://www.book.com/pdf_fulltext".to_string()), location_platform: LocationPlatform::OTHER, canonical: true, diff --git a/thoth-export-server/src/xml/onix3_project_muse.rs b/thoth-export-server/src/xml/onix3_project_muse.rs index afef1b4f..d2aecc1e 100644 --- a/thoth-export-server/src/xml/onix3_project_muse.rs +++ b/thoth-export-server/src/xml/onix3_project_muse.rs @@ -746,7 +746,7 @@ mod tests { isbn: Some(Isbn::from_str("978-3-16-148410-0").unwrap()), prices: vec![], locations: vec![WorkPublicationsLocations { - landing_page: "https://www.book.com/pdf_landing".to_string(), + landing_page: Some("https://www.book.com/pdf_landing".to_string()), full_text_url: Some("https://www.book.com/pdf_fulltext".to_string()), location_platform: LocationPlatform::OTHER, canonical: true, From 9f10a4eb7bee83b0fe5d984737651089b62220e7 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Thu, 21 Oct 2021 17:08:40 +0100 Subject: [PATCH 27/38] Fix new Clippy errors (if and else branches the same) under 1.56.0 --- thoth-api/src/account/model.rs | 11 +++++------ thoth-app/src/component/issues_form.rs | 9 ++++----- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/thoth-api/src/account/model.rs b/thoth-api/src/account/model.rs index bd0c3dc3..9c8cce41 100644 --- a/thoth-api/src/account/model.rs +++ b/thoth-api/src/account/model.rs @@ -137,12 +137,11 @@ impl DecodedToken { impl AccountAccess { pub fn can_edit(&self, publisher_id: Uuid) -> ThothResult<()> { - if self.is_superuser { - Ok(()) - } else if let Some(_found) = &self - .linked_publishers - .iter() - .position(|publisher| publisher.publisher_id == publisher_id) + if self.is_superuser + || self + .linked_publishers + .iter() + .any(|publisher| publisher.publisher_id == publisher_id) { Ok(()) } else { diff --git a/thoth-app/src/component/issues_form.rs b/thoth-app/src/component/issues_form.rs index fd1f40f3..98ae5412 100644 --- a/thoth-app/src/component/issues_form.rs +++ b/thoth-app/src/component/issues_form.rs @@ -323,16 +323,15 @@ impl Component for IssuesFormComponent { for self.data.serieses.iter().map(|s| { let series = s.clone(); // avoid listing series already present in issues list - if let Some(_index) = self.props.issues + if self.props.issues .as_ref() .unwrap() .iter() - .position(|ser| ser.series_id == series.series_id) + .any(|ser| ser.series_id == series.series_id) + // avoid listing series whose imprint doesn't match work + || series.imprint.imprint_id != self.props.imprint_id { html! {} - // avoid listing series whose imprint doesn't match work - } else if series.imprint.imprint_id != self.props.imprint_id { - html! {} } else { s.as_dropdown_item( self.link.callback(move |_| { From e8c2b0a580e2cbd9feeaaaf3f7e906fd0f7604b3 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Fri, 22 Oct 2021 09:25:29 +0100 Subject: [PATCH 28/38] Add database and API checks to make at least one URL mandatory (and improve down migration) --- thoth-api/migrations/0.4.8/down.sql | 24 +++++++--- thoth-api/migrations/0.4.8/up.sql | 4 +- thoth-api/src/graphql/model.rs | 8 +++- thoth-api/src/model/location/crud.rs | 53 +++++++++++++++++++++++ thoth-app/src/component/locations_form.rs | 5 ++- thoth-errors/src/lib.rs | 4 ++ 6 files changed, 89 insertions(+), 9 deletions(-) diff --git a/thoth-api/migrations/0.4.8/down.sql b/thoth-api/migrations/0.4.8/down.sql index af0920c7..bd2be90e 100644 --- a/thoth-api/migrations/0.4.8/down.sql +++ b/thoth-api/migrations/0.4.8/down.sql @@ -6,19 +6,31 @@ ALTER TABLE publication -- set the landing page of the canonical location (if any) as the main publication_url, -- then create duplicate publications to store all other location URLs (landing page/full text). UPDATE publication - SET publication_url = location.landing_page - FROM location - WHERE publication.publication_id = location.publication_id - AND location.canonical - AND location.full_text_url IS NULL; + SET publication_url = location.landing_page + FROM location + WHERE publication.publication_id = location.publication_id + AND location.canonical + AND location.landing_page IS NOT NULL; +UPDATE publication + SET publication_url = location.full_text_url + FROM location + WHERE publication.publication_id = location.publication_id + AND location.canonical + AND location.full_text_url IS NOT NULL + AND location.landing_page IS NULL; INSERT INTO publication(publication_type, work_id, publication_url) SELECT publication.publication_type, publication.work_id, location.landing_page FROM publication, location WHERE publication.publication_id = location.publication_id + AND location.landing_page IS NOT NULL AND NOT location.canonical; INSERT INTO publication(publication_type, work_id, publication_url) SELECT publication.publication_type, publication.work_id, location.full_text_url FROM publication, location WHERE publication.publication_id = location.publication_id - AND location.full_text_url IS NOT NULL; + AND location.full_text_url IS NOT NULL + AND ( + NOT location.canonical + OR (location.canonical AND location.landing_page IS NOT NULL) + ); DROP TABLE location_history; DROP TRIGGER set_updated_at ON location; diff --git a/thoth-api/migrations/0.4.8/up.sql b/thoth-api/migrations/0.4.8/up.sql index 86010a21..a8f8dcc7 100644 --- a/thoth-api/migrations/0.4.8/up.sql +++ b/thoth-api/migrations/0.4.8/up.sql @@ -13,7 +13,9 @@ CREATE TABLE location ( location_platform location_platform NOT NULL DEFAULT 'Other', canonical BOOLEAN NOT NULL DEFAULT False, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + -- Location must contain at least one of landing_page or full_text_url + CONSTRAINT location_url_check CHECK (landing_page IS NOT NULL OR full_text_url IS NOT NULL) ); SELECT diesel_manage_updated_at('location'); diff --git a/thoth-api/src/graphql/model.rs b/thoth-api/src/graphql/model.rs index 6e7ce3c9..5cfd35ce 100644 --- a/thoth-api/src/graphql/model.rs +++ b/thoth-api/src/graphql/model.rs @@ -1115,7 +1115,9 @@ impl MutationRoot { data.publication_id, )?)?; - if !data.canonical { + if data.canonical { + data.canonical_record_complete(&context.db)?; + } else { data.can_be_non_canonical(&context.db)?; } @@ -1359,6 +1361,10 @@ impl MutationRoot { return Err(ThothError::CanonicalLocationError.into()); } + if data.canonical { + data.canonical_record_complete(&context.db)?; + } + let account_id = context.token.jwt.as_ref().unwrap().account_id(&context.db); location .update(&context.db, &data, &account_id) diff --git a/thoth-api/src/model/location/crud.rs b/thoth-api/src/model/location/crud.rs index 639cf1e6..37b9fc10 100644 --- a/thoth-api/src/model/location/crud.rs +++ b/thoth-api/src/model/location/crud.rs @@ -180,6 +180,59 @@ impl NewLocation { Ok(()) } } + + pub fn canonical_record_complete(&self, db: &crate::db::PgPool) -> ThothResult<()> { + location_canonical_record_complete( + self.publication_id, + &self.landing_page, + &self.full_text_url, + db, + ) + } +} + +impl PatchLocation { + pub fn canonical_record_complete(&self, db: &crate::db::PgPool) -> ThothResult<()> { + location_canonical_record_complete( + self.publication_id, + &self.landing_page, + &self.full_text_url, + db, + ) + } +} + +fn location_canonical_record_complete( + publication_id: Uuid, + landing_page: &Option, + full_text_url: &Option, + db: &crate::db::PgPool, +) -> ThothResult<()> { + // If a canonical location has both the possible URLs, it is always complete. + if landing_page.is_some() && full_text_url.is_some() { + Ok(()) + } else { + use crate::model::publication::PublicationType; + use diesel::prelude::*; + + let connection = db.get().unwrap(); + let publication_type = crate::schema::publication::table + .select(crate::schema::publication::publication_type) + .filter(crate::schema::publication::publication_id.eq(publication_id)) + .first::(&connection) + .expect("Error loading publication type for location"); + // If a canonical location's publication is of a digital type, + // it must have both the possible URLs to count as complete. + if publication_type != PublicationType::Hardback + && publication_type != PublicationType::Paperback + { + Err(ThothError::LocationUrlError) + } else { + // For non-digital types, at least one URL must be present, + // but exceptions to this will be caught at the database level. + Ok(()) + } + } } #[cfg(test)] diff --git a/thoth-app/src/component/locations_form.rs b/thoth-app/src/component/locations_form.rs index 8f5bcce3..1dc784d1 100644 --- a/thoth-app/src/component/locations_form.rs +++ b/thoth-app/src/component/locations_form.rs @@ -237,7 +237,10 @@ impl Component for LocationsFormComponent { .send_message(Msg::SetLocationDeleteState(FetchAction::Fetching)); false } - Msg::ChangeLandingPage(val) => self.new_location.landing_page.neq_assign(val.to_opt_string()), + Msg::ChangeLandingPage(val) => self + .new_location + .landing_page + .neq_assign(val.to_opt_string()), Msg::ChangeFullTextUrl(val) => self .new_location .full_text_url diff --git a/thoth-errors/src/lib.rs b/thoth-errors/src/lib.rs index 77337502..0b422b5d 100644 --- a/thoth-errors/src/lib.rs +++ b/thoth-errors/src/lib.rs @@ -57,6 +57,10 @@ pub enum ThothError { ChapterIsbnError, #[fail(display = "Each Publication must have exactly one canonical Location.")] CanonicalLocationError, + #[fail( + display = "Canonical Locations for digital Publications must have both a Landing Page and a Full Text URL." + )] + LocationUrlError, } #[cfg(not(target_arch = "wasm32"))] From 95e7c522026cc700151ba373e2b0225463025845 Mon Sep 17 00:00:00 2001 From: rhigman <73792779+rhigman@users.noreply.github.com> Date: Fri, 22 Oct 2021 13:59:00 +0100 Subject: [PATCH 29/38] Fill out location platform list, minor improvements --- thoth-api/migrations/0.4.8/down.sql | 3 +- thoth-api/migrations/0.4.8/up.sql | 7 ++++ thoth-api/src/model/location/mod.rs | 59 +++++++++++++++++++++++++++++ thoth-app/src/component/utils.rs | 24 ++++-------- thoth-client/assets/schema.json | 42 ++++++++++++++++++++ 5 files changed, 117 insertions(+), 18 deletions(-) diff --git a/thoth-api/migrations/0.4.8/down.sql b/thoth-api/migrations/0.4.8/down.sql index bd2be90e..8b6ab3bf 100644 --- a/thoth-api/migrations/0.4.8/down.sql +++ b/thoth-api/migrations/0.4.8/down.sql @@ -3,8 +3,9 @@ ALTER TABLE publication ADD COLUMN publication_url TEXT CHECK (publication_url ~* '^[^:]*:\/\/(?:[^\/:]*:[^\/@]*@)?(?:[^\/:.]*\.)+([^:\/]+)'); -- Migrate location URLs back into publication table as far as possible before dropping location table: --- set the landing page of the canonical location (if any) as the main publication_url, +-- set the landing_page or full_text_url of the canonical location as the main publication_url, -- then create duplicate publications to store all other location URLs (landing page/full text). +-- Note this will create multiple identical publications if the same URL is re-used across location fields. UPDATE publication SET publication_url = location.landing_page FROM location diff --git a/thoth-api/migrations/0.4.8/up.sql b/thoth-api/migrations/0.4.8/up.sql index a8f8dcc7..9cbb0c11 100644 --- a/thoth-api/migrations/0.4.8/up.sql +++ b/thoth-api/migrations/0.4.8/up.sql @@ -1,7 +1,14 @@ CREATE TYPE location_platform AS ENUM ( 'Project MUSE', 'OAPEN', + 'DOAB', 'JSTOR', + 'EBSCO Host', + 'OCLC KB', + 'ProQuest KB', + 'ProQuest ExLibris', + 'EBSCO KB', + 'JISC KB', 'Other' ); diff --git a/thoth-api/src/model/location/mod.rs b/thoth-api/src/model/location/mod.rs index afeae0ca..6bd64cf7 100644 --- a/thoth-api/src/model/location/mod.rs +++ b/thoth-api/src/model/location/mod.rs @@ -20,9 +20,30 @@ pub enum LocationPlatform { #[cfg_attr(feature = "backend", db_rename = "OAPEN")] #[strum(serialize = "OAPEN")] Oapen, + #[cfg_attr(feature = "backend", db_rename = "DOAB")] + #[strum(serialize = "DOAB")] + Doab, #[cfg_attr(feature = "backend", db_rename = "JSTOR")] #[strum(serialize = "JSTOR")] Jstor, + #[cfg_attr(feature = "backend", db_rename = "EBSCO Host")] + #[strum(serialize = "EBSCO Host")] + EbscoHost, + #[cfg_attr(feature = "backend", db_rename = "OCLC KB")] + #[strum(serialize = "OCLC KB")] + OclcKb, + #[cfg_attr(feature = "backend", db_rename = "ProQuest KB")] + #[strum(serialize = "ProQuest KB")] + ProquestKb, + #[cfg_attr(feature = "backend", db_rename = "ProQuest ExLibris")] + #[strum(serialize = "ProQuest ExLibris")] + ProquestExlibris, + #[cfg_attr(feature = "backend", db_rename = "EBSCO KB")] + #[strum(serialize = "EBSCO KB")] + EbscoKb, + #[cfg_attr(feature = "backend", db_rename = "JISC KB")] + #[strum(serialize = "JISC KB")] + JiscKb, #[cfg_attr(feature = "backend", db_rename = "Other")] Other, } @@ -121,7 +142,17 @@ fn test_locationplatform_default() { fn test_locationplatform_display() { assert_eq!(format!("{}", LocationPlatform::ProjectMuse), "Project MUSE"); assert_eq!(format!("{}", LocationPlatform::Oapen), "OAPEN"); + assert_eq!(format!("{}", LocationPlatform::Doab), "DOAB"); assert_eq!(format!("{}", LocationPlatform::Jstor), "JSTOR"); + assert_eq!(format!("{}", LocationPlatform::EbscoHost), "EBSCO Host"); + assert_eq!(format!("{}", LocationPlatform::OclcKb), "OCLC KB"); + assert_eq!(format!("{}", LocationPlatform::ProquestKb), "ProQuest KB"); + assert_eq!( + format!("{}", LocationPlatform::ProquestExlibris), + "ProQuest ExLibris" + ); + assert_eq!(format!("{}", LocationPlatform::EbscoKb), "EBSCO KB"); + assert_eq!(format!("{}", LocationPlatform::JiscKb), "JISC KB"); assert_eq!(format!("{}", LocationPlatform::Other), "Other"); } @@ -136,10 +167,38 @@ fn test_locationplatform_fromstr() { LocationPlatform::from_str("OAPEN").unwrap(), LocationPlatform::Oapen ); + assert_eq!( + LocationPlatform::from_str("DOAB").unwrap(), + LocationPlatform::Doab + ); assert_eq!( LocationPlatform::from_str("JSTOR").unwrap(), LocationPlatform::Jstor ); + assert_eq!( + LocationPlatform::from_str("EBSCO Host").unwrap(), + LocationPlatform::EbscoHost + ); + assert_eq!( + LocationPlatform::from_str("OCLC KB").unwrap(), + LocationPlatform::OclcKb + ); + assert_eq!( + LocationPlatform::from_str("ProQuest KB").unwrap(), + LocationPlatform::ProquestKb + ); + assert_eq!( + LocationPlatform::from_str("ProQuest ExLibris").unwrap(), + LocationPlatform::ProquestExlibris + ); + assert_eq!( + LocationPlatform::from_str("EBSCO KB").unwrap(), + LocationPlatform::EbscoKb + ); + assert_eq!( + LocationPlatform::from_str("JISC KB").unwrap(), + LocationPlatform::JiscKb + ); assert_eq!( LocationPlatform::from_str("Other").unwrap(), LocationPlatform::Other diff --git a/thoth-app/src/component/utils.rs b/thoth-app/src/component/utils.rs index 6fe08f2d..4ba4a318 100644 --- a/thoth-app/src/component/utils.rs +++ b/thoth-app/src/component/utils.rs @@ -181,6 +181,7 @@ pub struct PureNumberInput { pub struct PureWorkTypeSelect { pub label: String, pub data: Vec, + // Subset of `data` list which should be deactivated, if any #[prop_or_default] pub deactivate: Vec, pub value: WorkType, @@ -768,23 +769,12 @@ impl PureComponent for PurePublisherSelect { impl PureWorkTypeSelect { fn render_worktype(&self, w: &WorkTypeValues, deactivate: &[WorkType]) -> VNode { - // It should not be possible for the selected option to require deactivation. - if deactivate.contains(&w.name) { - html! { - - } - } else if w.name == self.value { - html! { - - } - } else { - html! { - - } + let deactivated = deactivate.contains(&w.name); + let selected = w.name == self.value; + html! { + } } } diff --git a/thoth-client/assets/schema.json b/thoth-client/assets/schema.json index 4b8f705b..19092426 100644 --- a/thoth-client/assets/schema.json +++ b/thoth-client/assets/schema.json @@ -5406,12 +5406,54 @@ "isDeprecated": false, "name": "OAPEN" }, + { + "deprecationReason": null, + "description": null, + "isDeprecated": false, + "name": "DOAB" + }, { "deprecationReason": null, "description": null, "isDeprecated": false, "name": "JSTOR" }, + { + "deprecationReason": null, + "description": null, + "isDeprecated": false, + "name": "EBSCO_HOST" + }, + { + "deprecationReason": null, + "description": null, + "isDeprecated": false, + "name": "OCLC_KB" + }, + { + "deprecationReason": null, + "description": null, + "isDeprecated": false, + "name": "PROQUEST_KB" + }, + { + "deprecationReason": null, + "description": null, + "isDeprecated": false, + "name": "PROQUEST_EXLIBRIS" + }, + { + "deprecationReason": null, + "description": null, + "isDeprecated": false, + "name": "EBSCO_KB" + }, + { + "deprecationReason": null, + "description": null, + "isDeprecated": false, + "name": "JISC_KB" + }, { "deprecationReason": null, "description": null, From ef22301fe9a33414071958ea5cb7e0d7f68de662 Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Mon, 1 Nov 2021 15:05:57 +0000 Subject: [PATCH 30/38] Test that all specifications must be listed in formats --- thoth-export-server/src/data.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/thoth-export-server/src/data.rs b/thoth-export-server/src/data.rs index 35eaf3eb..99354437 100644 --- a/thoth-export-server/src/data.rs +++ b/thoth-export-server/src/data.rs @@ -262,6 +262,20 @@ mod tests { } } + #[test] + fn test_all_specifications_listed_in_formats() { + for s in ALL_SPECIFICATIONS.iter() { + let specification_id = s.id; + let format_id = format_id_from_url(s.format); + let format = find_format(format_id).unwrap(); + assert!(format.specifications.iter() + .find(|specification| specification_id_from_url(specification) == specification_id) + .cloned() + .ok_or(ThothError::EntityNotFound) + .is_ok()) + } + } + #[test] fn test_platform_specifications_in_all_specifications() { for p in ALL_PLATFORMS.iter() { From 3d759323c53fec155091831a5a2a164c49217f58 Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Mon, 1 Nov 2021 15:06:17 +0000 Subject: [PATCH 31/38] Add jstor specification to onix format --- thoth-export-server/src/data.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/thoth-export-server/src/data.rs b/thoth-export-server/src/data.rs index 99354437..3ef2893b 100644 --- a/thoth-export-server/src/data.rs +++ b/thoth-export-server/src/data.rs @@ -160,6 +160,7 @@ lazy_static! { "/specifications/onix_3.0::project_muse" ), concat!(env!("THOTH_EXPORT_API"), "/specifications/onix_3.0::oapen"), + concat!(env!("THOTH_EXPORT_API"), "/specifications/onix_3.0::jstor"), ], }, Format { From e737c0429deeebbb5138e0c4b14a5e4ba2ff7971 Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Mon, 1 Nov 2021 15:12:31 +0000 Subject: [PATCH 32/38] Break down statements --- thoth-export-server/src/data.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/thoth-export-server/src/data.rs b/thoth-export-server/src/data.rs index 3ef2893b..69be867c 100644 --- a/thoth-export-server/src/data.rs +++ b/thoth-export-server/src/data.rs @@ -269,7 +269,9 @@ mod tests { let specification_id = s.id; let format_id = format_id_from_url(s.format); let format = find_format(format_id).unwrap(); - assert!(format.specifications.iter() + assert!(format + .specifications + .iter() .find(|specification| specification_id_from_url(specification) == specification_id) .cloned() .ok_or(ThothError::EntityNotFound) From 6f9e605696f418a414d0f35e1c71bf4178f81a96 Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Sun, 28 Nov 2021 13:09:24 +0000 Subject: [PATCH 33/38] Add data specific migration for automatic ingest --- thoth-api/migrations/0.4.8/up.sql | 314 +++++++++++++++++++++++++++++- 1 file changed, 310 insertions(+), 4 deletions(-) diff --git a/thoth-api/migrations/0.4.8/up.sql b/thoth-api/migrations/0.4.8/up.sql index 9cbb0c11..8081eb81 100644 --- a/thoth-api/migrations/0.4.8/up.sql +++ b/thoth-api/migrations/0.4.8/up.sql @@ -42,11 +42,317 @@ CREATE TABLE location_history ( timestamp TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ); --- Create location entries for every existing publication_url (assume all are landing pages) --- If a publication has locations, exactly one of them must be canonical; --- this command will create at most one location per publication, so make them all canonical. +-------------------------------------------------------------------------------- +--- START - Data migration for live database. Delete this patch after migration +-------------------------------------------------------------------------------- + +-- Migrate punctum PDF publications (publisher ID 9c41b13c-cecc-4f6a-a151-be4682915ef5): + +-- Each work that has publications should have a canonical PDF publication under a URL beginning "https://cloud.punctumbooks.com/s/". Create a new canonical location for each of these, using the URL as the full_text_url and the work's landing page as the landing_page. + +INSERT INTO location(publication_id, landing_page, full_text_url, canonical) + SELECT publication_id, landing_page, publication_url, True + FROM publication + INNER JOIN work ON publication.work_id = work.work_id + INNER JOIN imprint ON work.imprint_id = imprint.imprint_id + WHERE imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND publication.publication_type = 'PDF' + AND publication.publication_url ILIKE 'https://cloud.punctumbooks.com/s/%'; + +-- Some works may have additional PDF publications under Project MUSE or JSTOR URLs. Create a new non-canonical location for each of these, using the URL as the landing_page, omitting the full_text_url, and linking to the canonical (punctum cloud) publication. + +INSERT INTO location(publication_id, landing_page, canonical, location_platform) + SELECT a.publication_id, b.publication_url, False, 'Project MUSE' + FROM publication a, publication b + INNER JOIN work ON b.work_id = work.work_id + INNER JOIN imprint ON work.imprint_id = imprint.imprint_id + WHERE imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND a.publication_type = 'PDF' + AND a.work_id = b.work_id + AND a.publication_url ILIKE 'https://cloud.punctumbooks.com/s/%' + AND b.publication_url ILIKE 'https://muse.jhu.edu/book/%'; + +INSERT INTO location(publication_id, landing_page, canonical, location_platform) + SELECT a.publication_id, b.publication_url, False, 'JSTOR' + FROM publication a, publication b + INNER JOIN work ON b.work_id = work.work_id + INNER JOIN imprint ON work.imprint_id = imprint.imprint_id + WHERE imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND a.publication_type = 'PDF' + AND a.work_id = b.work_id + AND a.publication_url ILIKE 'https://cloud.punctumbooks.com/s/%' + AND b.publication_url ILIKE 'https://www.jstor.org/stable/%'; + +-- Some works may have additional PDF publications under OAPEN URLs. Usually, one publication stores the OAPEN publication landing page, and another stores the OAPEN full text link. Create a new non-canonical location for each pair of OAPEN publications, using each URL as either the landing_page or the full_text_url as appropriate, and linking to the canonical (punctum cloud) publication. + +INSERT INTO location(publication_id, landing_page, full_text_url, canonical, location_platform) + SELECT a.publication_id, b.publication_url, c.publication_url, False, 'OAPEN' + FROM publication a, publication b, publication c + INNER JOIN work ON c.work_id = work.work_id + INNER JOIN imprint ON work.imprint_id = imprint.imprint_id + WHERE imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND a.publication_type = 'PDF' + AND a.work_id = b.work_id + AND a.work_id = c.work_id + AND a.publication_url ILIKE 'https://cloud.punctumbooks.com/s/%' + AND (b.publication_url ILIKE 'https://library.oapen.org/handle/%' OR b.publication_url ILIKE 'http://library.oapen.org/handle/%') + AND (c.publication_url ILIKE 'https://library.oapen.org/bitstream/%' OR c.publication_url ILIKE 'http://library.oapen.org/bitstream/%'); + +-- All MUSE, JSTOR and OAPEN PDF publications should now have had their URL data migrated to location objects. They should not contain any additional (ISBN/price, non-duplicated) data so should be safe to delete. +-- In a small number of cases, the OAPEN publications have been misclassified as 'Paperback' rather than 'PDF', so don't restrict the type when deleting. + +DELETE FROM publication USING work, imprint + WHERE publication.work_id = work.work_id + AND work.imprint_id = imprint.imprint_id + AND imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND (publication_url ILIKE 'https://muse.jhu.edu/book/%' OR publication_url ILIKE 'https://www.jstor.org/stable/%' OR publication_url ILIKE 'https://library.oapen.org/handle/%' OR publication_url ILIKE 'http://library.oapen.org/handle/%' OR publication_url ILIKE 'https://library.oapen.org/bitstream/%' OR publication_url ILIKE 'http://library.oapen.org/bitstream/%') + AND (isbn IS NULL OR EXISTS ( + SELECT * FROM publication b + WHERE publication.work_id = b.work_id + AND publication.isbn = b.isbn + AND b.publication_url ILIKE 'https://cloud.punctumbooks.com/s/%')) + AND NOT EXISTS (SELECT * FROM price WHERE publication.publication_id = price.publication_id); + +-- All canonical (punctum cloud) publications should now have had their URL data migrated to location objects. Their publication_url fields should therefore be safe to clear. + +UPDATE publication SET publication_url = NULL + FROM work, imprint + WHERE publication.work_id = work.work_id + AND work.imprint_id = imprint.imprint_id + AND imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND publication_type = 'PDF' + AND publication_url ILIKE 'https://cloud.punctumbooks.com/s/%' + AND EXISTS (SELECT * FROM location WHERE publication.publication_id = location.publication_id AND publication.publication_url = location.full_text_url); + +-- Migrate punctum paperback publications (publisher ID 9c41b13c-cecc-4f6a-a151-be4682915ef5): + +-- If a work only has one paperback publication, assume that it is the canonical one. Create a new canonical location for each of these, using the URL as the landing_page. + +INSERT INTO location(publication_id, landing_page, canonical) + SELECT publication_id, publication_url, True + FROM publication + INNER JOIN work ON publication.work_id = work.work_id + INNER JOIN imprint ON work.imprint_id = imprint.imprint_id + WHERE imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND publication_type = 'Paperback' + AND publication_url IS NOT NULL + AND NOT EXISTS + (SELECT * FROM publication b + WHERE publication.work_id = b.work_id + AND NOT publication.publication_id = b.publication_id + AND b.publication_type = 'Paperback'); + +-- Some works have multiple paperback publications. Inspection of the data shows that there are never more than two, they never have more than one distinct ISBN between them, and they never have more than one distinct set of prices between them (although they may have more than one distinct URL). +-- Assume that the main publication in these cases is the only one with prices, or else the only one with a URL, or else the one where the URL is a punctumbooks.com landing page (or, if all else is equal, the first one found). Create a canonical location for this publication. + +INSERT INTO location(publication_id, landing_page, canonical) + SELECT a.publication_id, a.publication_url, True + FROM publication a + LEFT JOIN price aprice ON a.publication_id = aprice.publication_id + INNER JOIN work ON a.work_id = work.work_id + INNER JOIN imprint ON work.imprint_id = imprint.imprint_id, + publication b + LEFT JOIN price bprice ON b.publication_id = bprice.publication_id + WHERE imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND a.publication_type = 'Paperback' + AND b.publication_type = 'Paperback' + AND a.work_id = b.work_id + AND NOT a.publication_id = b.publication_id + AND a.publication_url IS NOT NULL + AND ((aprice.publication_id IS NOT NULL AND bprice.publication_id IS NULL) + OR ((aprice.currency_code IS NOT DISTINCT FROM bprice.currency_code AND aprice.unit_price IS NOT DISTINCT FROM bprice.unit_price) + AND (b.publication_url IS NULL OR b.publication_url NOT ILIKE 'https://punctumbooks.com/titles/%'))); + +-- A single work (ID 98ce9caa-487e-4391-86c9-e5d8129be5b6) has one paperback publication with prices but no URL, and another with a URL but no prices, so it is not covered by the above. Make a canonical location for it manually, attached to the publication with prices, then remove the publication without prices. + +INSERT INTO location(publication_id, landing_page, canonical) + SELECT a.publication_id, b.publication_url, True + FROM publication a, publication b + WHERE a.work_id = '98ce9caa-487e-4391-86c9-e5d8129be5b6' + AND b.work_id = '98ce9caa-487e-4391-86c9-e5d8129be5b6' + AND a.publication_type = 'Paperback' + AND b.publication_type = 'Paperback' + AND NOT a.publication_id = b.publication_id + AND a.publication_url IS NULL + AND b.publication_url IS NOT NULL + AND EXISTS (SELECT * FROM price WHERE price.publication_id = a.publication_id) + AND NOT EXISTS (SELECT * FROM price WHERE price.publication_id = b.publication_id); + +DELETE FROM publication + WHERE work_id = '98ce9caa-487e-4391-86c9-e5d8129be5b6' + AND publication_type = 'Paperback' + AND NOT EXISTS (SELECT * FROM price WHERE price.publication_id = publication_id); + +-- Create non-canonical locations under the main publication for all the other URLs associated with this work. + +INSERT INTO location(publication_id, landing_page, canonical) + SELECT a.publication_id, b.publication_url, False + FROM publication a + INNER JOIN work ON a.work_id = work.work_id + INNER JOIN imprint ON work.imprint_id = imprint.imprint_id, + publication b + WHERE imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND a.publication_type = 'Paperback' + AND b.publication_type = 'Paperback' + AND a.work_id = b.work_id + AND NOT a.publication_id = b.publication_id + AND EXISTS (SELECT * FROM location WHERE a.publication_id = location.publication_id) + AND b.publication_url IS NOT NULL + AND b.publication_url IS DISTINCT FROM a.publication_url; + +-- For any case where the main publication lacks an ISBN, carry over the ISBN (if any) from the other publication. + +UPDATE publication + SET isbn = b.isbn + FROM publication b, work, imprint + WHERE publication.work_id = work.work_id + AND work.imprint_id = imprint.imprint_id + AND imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND publication.publication_type = 'Paperback' + AND b.publication_type = 'Paperback' + AND publication.work_id = b.work_id + AND NOT publication.publication_id = b.publication_id + AND EXISTS (SELECT * FROM location WHERE publication.publication_id = location.publication_id) + AND publication.isbn IS NULL; + +-- All price, ISBN and URL information in non-main publications should now either be duplicated on the main publication or stored in the location table. Delete these publications. + +DELETE FROM publication USING work, imprint, publication b + WHERE publication.work_id = work.work_id + AND work.imprint_id = imprint.imprint_id + AND imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND publication.publication_type = 'Paperback' + AND b.publication_type = 'Paperback' + AND publication.work_id = b.work_id + AND NOT publication.publication_id = b.publication_id + AND NOT EXISTS (SELECT * FROM location WHERE publication.publication_id = location.publication_id) + AND (publication.publication_url IS NULL OR EXISTS (SELECT * FROM location WHERE b.publication_id = location.publication_id AND publication.publication_url = location.landing_page)) + AND (publication.isbn IS NOT DISTINCT FROM b.isbn OR publication.isbn IS NULL) + AND NOT EXISTS (SELECT unit_price, currency_code FROM price WHERE price.publication_id = publication.publication_id EXCEPT SELECT unit_price, currency_code FROM price WHERE price.publication_id = b.publication_id); + +-- All remaining publication_urls should now be listed in the location table as the canonical URL for that publication. Remove them from the publications. + +UPDATE publication SET publication_url = NULL + FROM work, imprint + WHERE publication.work_id = work.work_id + AND work.imprint_id = imprint.imprint_id + AND imprint.publisher_id = '9c41b13c-cecc-4f6a-a151-be4682915ef5' + AND publication_type = 'Paperback' + AND publication_url IS NOT NULL + AND EXISTS (SELECT * FROM location WHERE publication.publication_id = location.publication_id AND publication.publication_url = location.landing_page); + +-- Migrate remaining duplicate publications: + +-- A single meson press work (ID 38872158-58b9-4ddf-a90e-f6001ac6c62d) accounts for all remaining duplicate publications. Inspection of the data shows two PDFs with differing URLs, identical ISBNs and no prices, and three paperbacks with differing URLs, identical ISBNs and two different prices (each in a different currency) between them. Handle these individually. + +-- PDFs: one has a meson.press URL, the other an OAPEN URL. Assume that the former is the main one. Create a canonical location for it, create a secondary location for the other one, and then delete the other one and remove the main one's publication_url. + +INSERT INTO location(publication_id, landing_page, full_text_url, canonical) + SELECT publication_id, landing_page, publication_url, True + FROM publication + INNER JOIN work ON publication.work_id = work.work_id + WHERE publication.work_id = '38872158-58b9-4ddf-a90e-f6001ac6c62d' + AND publication.publication_type = 'PDF' + AND publication.publication_url ILIKE 'https://meson.press/wp-content/uploads/%'; + +INSERT INTO location(publication_id, landing_page, canonical, location_platform) + SELECT a.publication_id, b.publication_url, False, 'OAPEN' + FROM publication a, publication b + WHERE a.work_id = '38872158-58b9-4ddf-a90e-f6001ac6c62d' + AND b.work_id = '38872158-58b9-4ddf-a90e-f6001ac6c62d' + AND a.publication_type = 'PDF' + AND b.publication_type = 'PDF' + AND a.publication_url ILIKE 'https://meson.press/wp-content/uploads/%' + AND b.publication_url ILIKE 'https://library.oapen.org/bitstream/%'; + +DELETE FROM publication + WHERE publication.work_id = '38872158-58b9-4ddf-a90e-f6001ac6c62d' + AND publication.publication_type = 'PDF' + AND publication.publication_url ILIKE 'https://library.oapen.org/bitstream/%' + AND (isbn IS NULL OR EXISTS ( + SELECT * FROM publication b + WHERE publication.work_id = b.work_id + AND publication.isbn = b.isbn + AND b.publication_url ILIKE 'https://meson.press/wp-content/uploads/%')) + AND NOT EXISTS (SELECT * FROM price WHERE publication.publication_id = price.publication_id); + +UPDATE publication SET publication_url = NULL + WHERE publication.work_id = '38872158-58b9-4ddf-a90e-f6001ac6c62d' + AND publication.publication_type = 'PDF' + AND publication.publication_url ILIKE 'https://meson.press/wp-content/uploads/%' + AND EXISTS (SELECT * FROM location WHERE publication.publication_id = location.publication_id AND publication.publication_url = location.full_text_url); + +-- Paperbacks: none of the URLs are meson.press, so assume that the first publication entered (which has ID 1382662a-ae40-47ae-98a0-34e03ae71366) is the main one. Create a canonical location for it. + +INSERT INTO location(publication_id, landing_page, canonical) + SELECT publication_id, publication_url, True + FROM publication + WHERE publication.publication_id = '1382662a-ae40-47ae-98a0-34e03ae71366'; + +-- Create non-canonical locations for the other publications, linked to the main one. + +INSERT INTO location(publication_id, landing_page, canonical) + SELECT '1382662a-ae40-47ae-98a0-34e03ae71366', publication_url, False + FROM publication + WHERE publication.work_id = '38872158-58b9-4ddf-a90e-f6001ac6c62d' + AND publication.publication_type = 'Paperback' + AND NOT publication.publication_id = '1382662a-ae40-47ae-98a0-34e03ae71366'; + +-- One of the prices linked to a non-main publication is not duplicated on the main publication. Move it to the main publication. + +UPDATE price SET publication_id = '1382662a-ae40-47ae-98a0-34e03ae71366' + WHERE publication_id = '49003581-5829-457a-b626-a5ab30df9a55'; + +-- The non-main paperback publications can now be deleted, and the main publication_url cleared. + +DELETE FROM publication + WHERE publication.work_id = '38872158-58b9-4ddf-a90e-f6001ac6c62d' + AND publication.publication_type = 'Paperback' + AND NOT publication.publication_id = '1382662a-ae40-47ae-98a0-34e03ae71366'; + +UPDATE publication SET publication_url = NULL WHERE publication_id = '1382662a-ae40-47ae-98a0-34e03ae71366'; + +-- Migrate all remaining publications: + +-- All remaining publications across all publishers should now be unique per work/publication type. Therefore, any URLs which they have can be converted to canonical locations. For hard copy types, convert the publication_url to the location landing_page. For soft copy types, convert the publication_url to the location full_text_url and use the work landing_page as the location landing_page. +-- Double-check that no location entry already exists for the publication, and no duplicate publication exists. + INSERT INTO location(publication_id, landing_page, canonical) - SELECT publication_id, publication_url, True FROM publication WHERE publication_url IS NOT NULL; + SELECT publication_id, publication_url, True + FROM publication + WHERE (publication.publication_type = 'Paperback' OR publication.publication_type = 'Hardback') + AND publication_url IS NOT NULL + AND NOT EXISTS (SELECT * FROM publication b + WHERE publication.work_id = b.work_id + AND NOT publication.publication_id = b.publication_id + AND publication.publication_type = b.publication_type) + AND NOT EXISTS (SELECT * FROM location WHERE publication.publication_id = location.publication_id AND publication.publication_url = location.landing_page); + +INSERT INTO location(publication_id, landing_page, full_text_url, canonical) + SELECT publication_id, landing_page, publication_url, True + FROM publication + INNER JOIN work ON publication.work_id = work.work_id + WHERE (publication.publication_type = 'PDF' OR publication.publication_type = 'Epub' OR publication.publication_type = 'XML' OR publication.publication_type = 'Mobi' OR publication.publication_type = 'HTML') + AND publication_url IS NOT NULL + AND NOT EXISTS (SELECT * FROM publication b + WHERE publication.work_id = b.work_id + AND NOT publication.publication_id = b.publication_id + AND publication.publication_type = b.publication_type) + AND NOT EXISTS (SELECT * FROM location WHERE publication.publication_id = location.publication_id AND publication.publication_url = location.landing_page); + +-- All these publications can now have their URLs cleared. + +UPDATE publication SET publication_url = NULL + FROM work + WHERE publication_url IS NOT NULL + AND NOT EXISTS (SELECT * FROM publication b + WHERE publication.work_id = b.work_id + AND NOT publication.publication_id = b.publication_id + AND publication.publication_type = b.publication_type) + AND EXISTS (SELECT * FROM location WHERE publication.publication_id = location.publication_id AND (publication.publication_url = location.landing_page OR publication.publication_url = location.full_text_url)); +----------------------------------------------------------------------------- +--- END - Data migration for live database. Delete this patch after migration +----------------------------------------------------------------------------- ALTER TABLE publication -- Only allow one publication of each type per work (existing data may breach this) From 36deb83d435fe24c4ae25f03400dbba8d8299cc0 Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Sun, 28 Nov 2021 13:33:57 +0000 Subject: [PATCH 34/38] Update Changelog --- CHANGELOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67e2dabd..57ec033d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,24 @@ All notable changes to thoth will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [[0.5.0]](https://github.com/thoth-pub/thoth/releases/tag/v0.5.0) - 2021-11-28 +### Added + - [#297](https://github.com/thoth-pub/thoth/issues/297) - Implement publication location + +### Changed + - Requirement to Number fields preventing user from entering numbers below 0 for Counts/below 1 for Editions and Ordinals, and sets Contribution Ordinal default to 1 instead of 0 + - [#299](https://github.com/thoth-pub/thoth/pull/299) - Update Project MUSE ONIX subject output logic + - Updated if and else branches to comply with [`rustc 1.56.0`](https://github.com/rust-lang/rust/releases/tag/1.56.0) + +### Fixed + - [#292](https://github.com/thoth-pub/thoth/issues/292) - Cannot unset pubiication date: error when trying to clear a previously set publication date + - [#295](https://github.com/thoth-pub/thoth/issues/295) - various subforms failing to trim strings before saving (including on mandatory fields which are checked for emptiness) + - Duplicated logic for handling optional field values, simplifying the code and reducing the likelihood of further bugs such as + - Minor issue where some required fields were not marked as "required" (so empty values would be sent to the API and raise an error) + - Issue with subforms where clicking save button bypassed field requirements (so instead of displaying a warning message such as "Please enter a number", invalid values would be sent to the API and raise an error) + - [#310](https://github.com/thoth-pub/thoth/issues/310) - Add jstor specification to formats + + ## [[0.4.7]](https://github.com/thoth-pub/thoth/releases/tag/v0.4.7) - 2021-10-04 ### Added - [#43](https://github.com/thoth-pub/thoth/issues/43), [#49](https://github.com/thoth-pub/thoth/issues/49) - Implement EBSCO Host's ONIX 2.1 specification From c95708ce46089dc49b712e21a48c77c63cdc7c02 Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Sun, 28 Nov 2021 13:37:15 +0000 Subject: [PATCH 35/38] Rename migration --- thoth-api/migrations/{0.4.8 => 0.5.0}/down.sql | 0 thoth-api/migrations/{0.4.8 => 0.5.0}/up.sql | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename thoth-api/migrations/{0.4.8 => 0.5.0}/down.sql (100%) rename thoth-api/migrations/{0.4.8 => 0.5.0}/up.sql (100%) diff --git a/thoth-api/migrations/0.4.8/down.sql b/thoth-api/migrations/0.5.0/down.sql similarity index 100% rename from thoth-api/migrations/0.4.8/down.sql rename to thoth-api/migrations/0.5.0/down.sql diff --git a/thoth-api/migrations/0.4.8/up.sql b/thoth-api/migrations/0.5.0/up.sql similarity index 100% rename from thoth-api/migrations/0.4.8/up.sql rename to thoth-api/migrations/0.5.0/up.sql From 39f8cf70c70ec6bf93b177f6eaa2ce42e80803f5 Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Sun, 28 Nov 2021 13:40:46 +0000 Subject: [PATCH 36/38] Bump v0.5.0 --- Cargo.lock | 16 ++++++++-------- Cargo.toml | 12 ++++++------ thoth-api-server/Cargo.toml | 6 +++--- thoth-api/Cargo.toml | 4 ++-- thoth-app-server/Cargo.toml | 2 +- thoth-app/Cargo.toml | 6 +++--- thoth-app/manifest.json | 2 +- thoth-client/Cargo.toml | 6 +++--- thoth-errors/Cargo.toml | 2 +- thoth-export-server/Cargo.toml | 8 ++++---- 10 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 96c13bca..9ddaeecb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3936,7 +3936,7 @@ dependencies = [ [[package]] name = "thoth" -version = "0.4.7" +version = "0.5.0" dependencies = [ "cargo-husky", "clap", @@ -3951,7 +3951,7 @@ dependencies = [ [[package]] name = "thoth-api" -version = "0.4.7" +version = "0.5.0" dependencies = [ "actix-web", "argon2rs", @@ -3980,7 +3980,7 @@ dependencies = [ [[package]] name = "thoth-api-server" -version = "0.4.7" +version = "0.5.0" dependencies = [ "actix-cors", "actix-identity", @@ -3995,7 +3995,7 @@ dependencies = [ [[package]] name = "thoth-app" -version = "0.4.7" +version = "0.5.0" dependencies = [ "anyhow", "chrono", @@ -4018,7 +4018,7 @@ dependencies = [ [[package]] name = "thoth-app-server" -version = "0.4.7" +version = "0.5.0" dependencies = [ "actix-cors", "actix-web", @@ -4027,7 +4027,7 @@ dependencies = [ [[package]] name = "thoth-client" -version = "0.4.7" +version = "0.5.0" dependencies = [ "chrono", "graphql_client", @@ -4041,7 +4041,7 @@ dependencies = [ [[package]] name = "thoth-errors" -version = "0.4.7" +version = "0.5.0" dependencies = [ "actix-web", "csv", @@ -4056,7 +4056,7 @@ dependencies = [ [[package]] name = "thoth-export-server" -version = "0.4.7" +version = "0.5.0" dependencies = [ "actix-cors", "actix-web", diff --git a/Cargo.toml b/Cargo.toml index cbe02a96..1ff6f735 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "thoth" -version = "0.4.7" +version = "0.5.0" authors = ["Javier Arias ", "Ross Higman "] edition = "2018" license = "Apache-2.0" @@ -16,11 +16,11 @@ maintenance = { status = "actively-developed" } members = ["thoth-api", "thoth-api-server", "thoth-app", "thoth-app-server", "thoth-client", "thoth-errors", "thoth-export-server"] [dependencies] -thoth-api = { version = "0.4.7", path = "thoth-api", features = ["backend"] } -thoth-api-server = { version = "0.4.7", path = "thoth-api-server" } -thoth-app-server = { version = "0.4.7", path = "thoth-app-server" } -thoth-errors = { version = "0.4.7", path = "thoth-errors" } -thoth-export-server = { version = "0.4.7", path = "thoth-export-server" } +thoth-api = { version = "0.5.0", path = "thoth-api", features = ["backend"] } +thoth-api-server = { version = "0.5.0", path = "thoth-api-server" } +thoth-app-server = { version = "0.5.0", path = "thoth-app-server" } +thoth-errors = { version = "0.5.0", path = "thoth-errors" } +thoth-export-server = { version = "0.5.0", path = "thoth-export-server" } clap = "2.33.3" dialoguer = "0.7.1" dotenv = "0.9.0" diff --git a/thoth-api-server/Cargo.toml b/thoth-api-server/Cargo.toml index 6b7506d3..2c1a70d3 100644 --- a/thoth-api-server/Cargo.toml +++ b/thoth-api-server/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "thoth-api-server" -version = "0.4.7" +version = "0.5.0" authors = ["Javier Arias ", "Ross Higman "] edition = "2018" license = "Apache-2.0" @@ -9,8 +9,8 @@ repository = "https://github.com/thoth-pub/thoth" readme = "README.md" [dependencies] -thoth-api = { version = "0.4.7", path = "../thoth-api", features = ["backend"] } -thoth-errors = { version = "0.4.7", path = "../thoth-errors" } +thoth-api = { version = "0.5.0", path = "../thoth-api", features = ["backend"] } +thoth-errors = { version = "0.5.0", path = "../thoth-errors" } actix-web = "3.3.2" actix-cors = "0.5.4" actix-identity = "0.3.1" diff --git a/thoth-api/Cargo.toml b/thoth-api/Cargo.toml index c3789716..01fd41ec 100644 --- a/thoth-api/Cargo.toml +++ b/thoth-api/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "thoth-api" -version = "0.4.7" +version = "0.5.0" authors = ["Javier Arias ", "Ross Higman "] edition = "2018" license = "Apache-2.0" @@ -16,7 +16,7 @@ maintenance = { status = "actively-developed" } backend = ["diesel", "diesel-derive-enum", "diesel_migrations", "futures", "actix-web"] [dependencies] -thoth-errors = { version = "0.4.7", path = "../thoth-errors" } +thoth-errors = { version = "0.5.0", path = "../thoth-errors" } actix-web = { version = "3.3.2", optional = true } argon2rs = "0.2.5" isbn2 = "0.4.0" diff --git a/thoth-app-server/Cargo.toml b/thoth-app-server/Cargo.toml index db3648e1..7432a7b6 100644 --- a/thoth-app-server/Cargo.toml +++ b/thoth-app-server/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "thoth-app-server" -version = "0.4.7" +version = "0.5.0" authors = ["Javier Arias ", "Ross Higman "] edition = "2018" license = "Apache-2.0" diff --git a/thoth-app/Cargo.toml b/thoth-app/Cargo.toml index 5527080b..4d944524 100644 --- a/thoth-app/Cargo.toml +++ b/thoth-app/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "thoth-app" -version = "0.4.7" +version = "0.5.0" authors = ["Javier Arias ", "Ross Higman "] edition = "2018" license = "Apache-2.0" @@ -33,5 +33,5 @@ serde = { version = "1.0.115", features = ["derive"] } serde_json = "1.0" url = "2.1.1" uuid = { version = "0.7", features = ["serde", "v4"] } -thoth-api = { version = "0.4.7", path = "../thoth-api" } -thoth-errors = { version = "0.4.7", path = "../thoth-errors" } +thoth-api = { version = "0.5.0", path = "../thoth-api" } +thoth-errors = { version = "0.5.0", path = "../thoth-errors" } diff --git a/thoth-app/manifest.json b/thoth-app/manifest.json index 83546eaa..3c55a5c1 100644 --- a/thoth-app/manifest.json +++ b/thoth-app/manifest.json @@ -9,7 +9,7 @@ "start_url": "/?homescreen=1", "background_color": "#ffffff", "theme_color": "#ffdd57", - "version": "0.4.7", + "version": "0.5.0", "icons": [ { "src": "\/android-icon-36x36.png", diff --git a/thoth-client/Cargo.toml b/thoth-client/Cargo.toml index 4d0cae25..21ed0598 100644 --- a/thoth-client/Cargo.toml +++ b/thoth-client/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "thoth-client" -version = "0.4.7" +version = "0.5.0" authors = ["Javier Arias ", "Ross Higman "] edition = "2018" license = "Apache-2.0" @@ -9,8 +9,8 @@ repository = "https://github.com/thoth-pub/thoth" readme = "README.md" [dependencies] -thoth-api = {version = "0.4.7", path = "../thoth-api" } -thoth-errors = {version = "0.4.7", path = "../thoth-errors" } +thoth-api = {version = "0.5.0", path = "../thoth-api" } +thoth-errors = {version = "0.5.0", path = "../thoth-errors" } graphql_client = "0.9.0" chrono = { version = "0.4", features = ["serde"] } reqwest = { version = "0.10", features = ["json"] } diff --git a/thoth-errors/Cargo.toml b/thoth-errors/Cargo.toml index 09009e80..51a1af78 100644 --- a/thoth-errors/Cargo.toml +++ b/thoth-errors/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "thoth-errors" -version = "0.4.7" +version = "0.5.0" authors = ["Javier Arias ", "Ross Higman "] edition = "2018" license = "Apache-2.0" diff --git a/thoth-export-server/Cargo.toml b/thoth-export-server/Cargo.toml index fcbc9e89..b76411db 100644 --- a/thoth-export-server/Cargo.toml +++ b/thoth-export-server/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "thoth-export-server" -version = "0.4.7" +version = "0.5.0" authors = ["Javier Arias ", "Ross Higman "] edition = "2018" license = "Apache-2.0" @@ -9,9 +9,9 @@ repository = "https://github.com/thoth-pub/thoth" readme = "README.md" [dependencies] -thoth-api = { version = "0.4.7", path = "../thoth-api" } -thoth-errors = { version = "0.4.7", path = "../thoth-errors" } -thoth-client = { version = "0.4.7", path = "../thoth-client" } +thoth-api = { version = "0.5.0", path = "../thoth-api" } +thoth-errors = { version = "0.5.0", path = "../thoth-errors" } +thoth-client = { version = "0.5.0", path = "../thoth-client" } actix-web = "3.3.2" actix-cors = "0.5.4" chrono = { version = "0.4", features = ["serde"] } From 93cd1997f5539ae1ffdc970c08831ed29806b03b Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Sun, 28 Nov 2021 13:49:59 +0000 Subject: [PATCH 37/38] Attempt to update docker image --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 836434e4..c1fdfee3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -ARG RUST_IMAGE=rust:1.51.0 +ARG RUST_IMAGE=rust:1.56.0 ARG MUSL_IMAGE=ekidd/rust-musl-builder:1.51.0 FROM ${RUST_IMAGE} as wasm From 42c94671a59104d4537cfe651971588a3f9445bd Mon Sep 17 00:00:00 2001 From: Javier Arias Date: Mon, 29 Nov 2021 09:44:20 +0000 Subject: [PATCH 38/38] Pin wasm-pack version in docker --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index c1fdfee3..45385f1f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,7 +16,7 @@ RUN npm install -g npm@6.14.8 RUN npm install -g n@6.7.0 RUN n 12.19.0 RUN npm install -g rollup@2.28.2 -RUN cargo install wasm-pack +RUN cargo install wasm-pack --version 0.9.1 # Get source COPY . .