Skip to content

Commit

Permalink
Refactored update_canonical_location into update function in crud
Browse files Browse the repository at this point in the history
  • Loading branch information
@brendan-oconnell
brendan-oconnell committed Sep 25, 2024
1 parent 15553fb commit d0e9ea5
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 69 deletions.
36 changes: 3 additions & 33 deletions thoth-api/src/graphql/model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1991,39 +1991,9 @@ impl MutationRoot {
if data.canonical {
data.canonical_record_complete(&context.db)?;
}
// if changes to a location don't change its canonical or non-canonical status, perform a regular update.
if data.canonical == location.canonical {
location
.update(&context.db, &data, &account_id)
.map_err(|e| e.into())
// trying to change canonical location to non-canonical results in error.
} else if location.canonical && (data.canonical != location.canonical) {
Err(ThothError::CanonicalLocationError.into())
// if user changes a non-canonical location to canonical, perform two simultaneous updates:
// change the old canonical location to non-canonical, and change the old non-canonical location to canonical
} else {
let canonical_location = data.get_canonical_location(&context.db);

let final_canonical_location = match canonical_location {
Ok(location) => location,
Err(e) => {
return Err(ThothError::from(e).into());
}
};

let old_canonical_location = PatchLocation {
location_id: final_canonical_location.location_id,
publication_id: final_canonical_location.publication_id,
landing_page: final_canonical_location.landing_page.clone(),
full_text_url: final_canonical_location.full_text_url.clone(),
location_platform: final_canonical_location.location_platform.clone(),
canonical: false,
};

location
.update_canonical_location(&context.db, data, &old_canonical_location, old_canonical_location.location_id, &account_id)
.map_err(|e| e.into())
}
location
.update(&context.db, &data, &account_id)
.map_err(|e| e.into())
}

#[graphql(description = "Update an existing price with the specified values")]
Expand Down
144 changes: 108 additions & 36 deletions thoth-api/src/model/location/crud.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -126,7 +126,109 @@ 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 sometimes 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> {
// if changes to a location don't change its canonical or non-canonical status, perform a single update to that location.
if &data.canonical == &self.canonical {
let mut connection = db.get()?;
connection.transaction(|connection| {
match diesel::update(location::table.find(&self.location_id))
.set(data)
.get_result::<Self>(connection)
{
// On success, create a new history table entry.
Ok(l) => match self.new_history_entry(account_id).insert(connection) {
Ok(_) => Ok(l),
Err(e) => Err(e),
},
Err(e) => Err(ThothError::from(e)),
}
})
// trying to change canonical location to non-canonical results in error.
} else if self.canonical && (data.canonical != self.canonical) {
Err(ThothError::CanonicalLocationError.into())
// if user changes a non-canonical location to canonical, perform two simultaneous updates:
// change the old canonical location to non-canonical, and change the old non-canonical location to canonical
} else {
let canonical_location = self.get_canonical_location(db);
let final_canonical_location = match canonical_location {
Ok(location) => location,
Err(e) => {
return Err(ThothError::from(e).into());
}
};

let old_canonical_location = PatchLocation {
location_id: final_canonical_location.location_id,
publication_id: final_canonical_location.publication_id,
landing_page: final_canonical_location.landing_page.clone(),
full_text_url: final_canonical_location.full_text_url.clone(),
location_platform: final_canonical_location.location_platform.clone(),
canonical: false,
};
let mut connection = db.get()?;
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)?;
match diesel::update(location::table.find(&self.location_id))
// Update the data from the currently non-canonical location to canonical
.set(data)
.get_result::<Self>(connection)
{
// On success, create a new history table entry.
Ok(t) => match self.new_history_entry(account_id).insert(connection) {
Ok(_) => Ok(t),
Err(e) => Err(e),
},
Err(e) => Err(ThothError::from(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 {
Expand Down Expand Up @@ -182,38 +284,6 @@ impl NewLocation {
}

impl Location {
pub fn update_canonical_location(
&self,
db: &crate::db::PgPool,
data: PatchLocation,
old_canonical_location: &PatchLocation,
old_canonical_location_id: Uuid,
account_id: &Uuid,
) -> ThothResult<Self> {
use crate::diesel::Connection;
let mut connection = db.get()?;
connection.transaction(|connection| {
// Update the currently canonical location to non-canonical
diesel::update(location::table.find(old_canonical_location_id))
.set(old_canonical_location)
.execute(connection)?;
match diesel::update(location::table.find(&self.location_id))
// Update the data from the currently non-canonical location to canonical
.set(data)
.get_result::<Self>(connection)
{
// On success, create a new history table entry.
Ok(t) => match self.new_history_entry(account_id).insert(connection) {
Ok(_) => Ok(t),
Err(e) => Err(e),
},
Err(e) => Err(ThothError::from(e)),
}
})
}
}

impl PatchLocation {
pub fn get_canonical_location(
&self,
db: &crate::db::PgPool
Expand All @@ -226,7 +296,9 @@ impl PatchLocation {
.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(
self.publication_id,
Expand Down

0 comments on commit d0e9ea5

Please sign in to comment.