Feature/324 implement update locations

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
   - [565](https://github.com/thoth-pub/thoth/issues/565) - Don't generate Crossref metadata output if no DOIs (work or chapter) are present
+  - [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.9]](https://github.com/thoth-pub/thoth/releases/tag/v0.12.9) - 2024-09-06
 ### 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,98 @@ 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()?;
+        match location::table
+            .find(entity_id)
+            .get_result::<Self>(&mut connection)
+        {
+            Ok(t) => Ok(t),
+            Err(e) => Err(ThothError::from(e)),
+        }
+    }
+
+    fn create(db: &crate::db::PgPool, data: &NewLocation) -> ThothResult<Self> {
+        let mut connection = db.get()?;
+
+        connection.transaction(|connection| {
+            diesel::insert_into(location::table)
+                .values(data)
+                .get_result::<Self>(connection)
+                .map_err(|e| e.into())
+        })
+    }
+
+    fn update(
+        &self,
+        db: &crate::db::PgPool,
+        data: &PatchLocation,
+        account_id: &Uuid,
+    ) -> ThothResult<Self> {
+        let mut connection = db.get()?;
+        let update_result: Result<Self, ThothError>;
+        // if changes to a location don't change its canonical or non-canonical status, only update that location.
+        if data.canonical == self.canonical {
+            update_result = connection.transaction(|connection| {
+                diesel::update(location::table.find(&self.location_id))
+                    .set(data)
+                    .get_result::<Self>(connection)
+                    .map_err(ThothError::from)
+            });
+        // trying to change canonical location to non-canonical results in error.
+        } else if self.canonical && (data.canonical != self.canonical) {
+            return Err(ThothError::CanonicalLocationError);
+        // if user changes a non-canonical location to canonical, perform two simultaneous updates:
+        // change the former canonical location to non-canonical, the former non-canonical location to canonical
+        } else {
+            let canonical_location = self.get_canonical_location(db).map_err(ThothError::from)?;
+
+            let old_canonical_location = PatchLocation {
+                location_id: canonical_location.location_id,
+                publication_id: canonical_location.publication_id,
+                landing_page: canonical_location.landing_page.clone(),
+                full_text_url: canonical_location.full_text_url.clone(),
+                location_platform: canonical_location.location_platform.clone(),
+                canonical: false,
+            };
+            update_result = connection.transaction(|connection| {
+                // Update the currently canonical location to non-canonical
+                diesel::update(location::table.find(&old_canonical_location.location_id.clone()))
+                    .set(old_canonical_location)
+                    .execute(connection)?;
+                diesel::update(location::table.find(&self.location_id))
+                    // Update the data from the currently non-canonical location to make it canonical,
+                    // along with any other changes from PatchLocation
+                    .set(data)
+                    .get_result::<Self>(connection)
+                    .map_err(ThothError::from)
+            });
+        }
+
+        match update_result {
+            Ok(l) => {
+                let mut connection = db.get()?;
+                match self.new_history_entry(account_id).insert(&mut connection) {
+                    Ok(_) => Ok(l),
+                    Err(e) => Err(e),
+                }
+            }
+            Err(e) => Err(e),
+        }
+    }
+
+    fn delete(self, db: &crate::db::PgPool) -> ThothResult<Self> {
+        let mut connection = db.get()?;
+        connection.transaction(|connection| {
+            match diesel::delete(location::table.find(self.location_id)).execute(connection) {
+                Ok(_) => Ok(self),
+                Err(e) => Err(ThothError::from(e)),
+            }
+        })
+    }
 }
 
 impl HistoryEntry for Location {
@@ -181,6 +272,18 @@ impl NewLocation {
     }
 }
 
+impl Location {
+    pub fn get_canonical_location(&self, db: &crate::db::PgPool) -> ThothResult<Location> {
+        let mut connection = db.get()?;
+        let canonical_location = crate::schema::location::table
+            .filter(crate::schema::location::publication_id.eq(self.publication_id))
+            .filter(crate::schema::location::canonical.eq(true))
+            .first::<Location>(&mut connection)
+            .expect("Error loading canonical location for publication");
+        Ok(canonical_location)
+    }
+}
+
 impl PatchLocation {
     pub fn canonical_record_complete(&self, db: &crate::db::PgPool) -> ThothResult<()> {
         location_canonical_record_complete(