Merge pull request #620 from thoth-pub/feature/324_implement_update_l…

…ocations

Feature/324 implement update locations

CHANGELOG.md

@@ -5,6 +5,8 @@ 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).
 
 ## [Unreleased]
+### Fixed
+  - [324](https://github.com/thoth-pub/thoth/issues/324) - Make Locations editable, including the ability to change the Canonical Location for a Publication
 
 ## [[0.12.10]](https://github.com/thoth-pub/thoth/releases/tag/v0.12.10) - 2024-10-01
 ### Added

thoth-api/src/graphql/model.rs

@@ -1984,13 +1984,6 @@ impl MutationRoot {
                 )?)?;
         }
 
-        if data.canonical != location.canonical {
-            // Each publication must have exactly one canonical location.
-            // Updating an existing location would always violate this,
-            // as it should always result in either zero or two canonical locations.
-            return Err(ThothError::CanonicalLocationError.into());
-        }
-
         if data.canonical {
             data.canonical_record_complete(&context.db)?;
         }

thoth-api/src/model/location/crud.rs

@@ -2,11 +2,11 @@ use super::{
     Location, LocationField, LocationHistory, LocationOrderBy, LocationPlatform, NewLocation,
     NewLocationHistory, PatchLocation,
 };
+use crate::db_insert;
 use crate::graphql::utils::Direction;
 use crate::model::{Crud, DbInsert, HistoryEntry};
 use crate::schema::{location, location_history};
-use crate::{crud_methods, db_insert};
-use diesel::{ExpressionMethods, QueryDsl, RunQueryDsl};
+use diesel::{Connection, ExpressionMethods, QueryDsl, RunQueryDsl};
 use thoth_errors::{ThothError, ThothResult};
 use uuid::Uuid;
 
@@ -126,7 +126,73 @@ impl Crud for Location {
         crate::model::publication::Publication::from_id(db, &self.publication_id)?.publisher_id(db)
     }
 
-    crud_methods!(location::table, location::dsl::location);
+    // `crud_methods!` cannot be used for update(), because we need to execute multiple statements
+    // in the same transaction for changing a non-canonical location to canonical.
+    // These functions recreate the `crud_methods!` logic.
+    fn from_id(db: &crate::db::PgPool, entity_id: &Uuid) -> ThothResult<Self> {
+        let mut connection = db.get()?;
+        location::table
+            .find(entity_id)
+            .get_result::<Self>(&mut connection)
+            .map_err(Into::into)
+    }
+
+    fn create(db: &crate::db::PgPool, data: &NewLocation) -> ThothResult<Self> {
+        db.get()?.transaction(|connection| {
+            diesel::insert_into(location::table)
+                .values(data)
+                .get_result::<Self>(connection)
+                .map_err(Into::into)
+        })
+    }
+
+    fn update(
+        &self,
+        db: &crate::db::PgPool,
+        data: &PatchLocation,
+        account_id: &Uuid,
+    ) -> ThothResult<Self> {
+        let mut connection = db.get()?;
+        connection
+            .transaction(|connection| {
+                if data.canonical == self.canonical {
+                    // No change in canonical status, just update the current location
+                    diesel::update(location::table.find(&self.location_id))
+                        .set(data)
+                        .get_result::<Self>(connection)
+                        .map_err(Into::into)
+                } else if self.canonical && !data.canonical {
+                    // Trying to change canonical location to non-canonical results in error.
+                    Err(ThothError::CanonicalLocationError)
+                } else {
+                    // Update the existing canonical location to non-canonical
+                    let mut old_canonical_location =
+                        PatchLocation::from(self.get_canonical_location(db)?);
+                    old_canonical_location.canonical = false;
+                    diesel::update(location::table.find(old_canonical_location.location_id))
+                        .set(old_canonical_location)
+                        .execute(connection)?;
+                    diesel::update(location::table.find(&self.location_id))
+                        .set(data)
+                        .get_result::<Self>(connection)
+                        .map_err(Into::into)
+                }
+            })
+            .and_then(|location| {
+                self.new_history_entry(account_id)
+                    .insert(&mut connection)
+                    .map(|_| location)
+            })
+    }
+
+    fn delete(self, db: &crate::db::PgPool) -> ThothResult<Self> {
+        db.get()?.transaction(|connection| {
+            diesel::delete(location::table.find(self.location_id))
+                .execute(connection)
+                .map(|_| self)
+                .map_err(Into::into)
+        })
+    }
 }
 
 impl HistoryEntry for Location {
@@ -181,6 +247,17 @@ impl NewLocation {
     }
 }
 
+impl Location {
+    pub fn get_canonical_location(&self, db: &crate::db::PgPool) -> ThothResult<Location> {
+        let mut connection = db.get()?;
+        location::table
+            .filter(location::publication_id.eq(self.publication_id))
+            .filter(location::canonical.eq(true))
+            .first::<Self>(&mut connection)
+            .map_err(Into::into)
+    }
+}
+
 impl PatchLocation {
     pub fn canonical_record_complete(&self, db: &crate::db::PgPool) -> ThothResult<()> {
         location_canonical_record_complete(

thoth-api/src/model/location/mod.rs

@@ -242,6 +242,42 @@ impl Default for LocationOrderBy {
     }
 }
 
+impl From<Location> for PatchLocation {
+    fn from(location: Location) -> Self {
+        PatchLocation {
+            location_id: location.location_id,
+            publication_id: location.publication_id,
+            landing_page: location.landing_page,
+            full_text_url: location.full_text_url,
+            location_platform: location.location_platform,
+            canonical: location.canonical,
+        }
+    }
+}
+
+#[test]
+fn test_location_to_patch_location() {
+    let location = Location {
+        location_id: Uuid::parse_str("00000000-0000-0000-AAAA-000000000001").unwrap(),
+        publication_id: Uuid::parse_str("00000000-0000-0000-AAAA-000000000002").unwrap(),
+        landing_page: Some("https://www.book.com/pb_landing".to_string()),
+        full_text_url: Some("https://example.com/full_text.pdf".to_string()),
+        location_platform: LocationPlatform::PublisherWebsite,
+        created_at: Default::default(),
+        updated_at: Default::default(),
+        canonical: true,
+    };
+
+    let patch_location = PatchLocation::from(location.clone());
+
+    assert_eq!(patch_location.location_id, location.location_id);
+    assert_eq!(patch_location.publication_id, location.publication_id);
+    assert_eq!(patch_location.landing_page, location.landing_page);
+    assert_eq!(patch_location.full_text_url, location.full_text_url);
+    assert_eq!(patch_location.location_platform, location.location_platform);
+    assert_eq!(patch_location.canonical, location.canonical);
+}
+
 #[test]
 fn test_locationplatform_default() {
     let locationplatform: LocationPlatform = Default::default();

thoth-api/src/model/mod.rs

@@ -400,24 +400,18 @@ macro_rules! crud_methods {
             use diesel::{QueryDsl, RunQueryDsl};
 
             let mut connection = db.get()?;
-            match $entity_dsl
+            $entity_dsl
                 .find(entity_id)
                 .get_result::<Self>(&mut connection)
-            {
-                Ok(t) => Ok(t),
-                Err(e) => Err(ThothError::from(e)),
-            }
+                .map_err(Into::into)
         }
 
         fn create(db: &$crate::db::PgPool, data: &Self::NewEntity) -> ThothResult<Self> {
             let mut connection = db.get()?;
-            match diesel::insert_into($table_dsl)
+            diesel::insert_into($table_dsl)
                 .values(data)
                 .get_result::<Self>(&mut connection)
-            {
-                Ok(t) => Ok(t),
-                Err(e) => Err(ThothError::from(e)),
-            }
+                .map_err(Into::into)
         }
 
         /// Makes a database transaction that first updates the entity and then creates a new
@@ -483,13 +477,10 @@ macro_rules! db_insert {
         fn insert(&self, connection: &mut diesel::PgConnection) -> ThothResult<Self::MainEntity> {
             use diesel::RunQueryDsl;
 
-            match diesel::insert_into($table_dsl)
+            diesel::insert_into($table_dsl)
                 .values(self)
                 .get_result(connection)
-            {
-                Ok(t) => Ok(t),
-                Err(e) => Err(ThothError::from(e)),
-            }
+                .map_err(Into::into)
         }
     };
 }

thoth-api/src/model/work_relation/crud.rs

@@ -118,13 +118,10 @@ impl Crud for WorkRelation {
     // This function recreates the `crud_methods!` from_id() logic.
     fn from_id(db: &crate::db::PgPool, entity_id: &Uuid) -> ThothResult<Self> {
         let mut connection = db.get()?;
-        match work_relation::table
+        work_relation::table
             .find(entity_id)
             .get_result::<Self>(&mut connection)
-        {
-            Ok(t) => Ok(t),
-            Err(e) => Err(ThothError::from(e)),
-        }
+            .map_err(Into::into)
     }
 
     fn create(db: &crate::db::PgPool, data: &NewWorkRelation) -> ThothResult<Self> {