Skip to content

Commit

Permalink
Merge pull request #614 from thoth-pub/feature/548_contributor_deletion
Browse files Browse the repository at this point in the history
Prevent deletion of contributors/institutions used by other publishers (#548)
  • Loading branch information
rhigman authored Sep 3, 2024
2 parents ee65db8 + ecbfea3 commit 180720f
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 15 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
- [548](https://github.com/thoth-pub/thoth/issues/548) - Prevent users from deleting contributors/institutions which are linked to works by other publishers

## [[0.12.8]](https://github.com/thoth-pub/thoth/releases/tag/v0.12.8) - 2024-09-03
### Fixed
Expand Down
20 changes: 12 additions & 8 deletions thoth-api/src/graphql/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1983,10 +1983,12 @@ impl MutationRoot {

fn delete_contributor(context: &Context, contributor_id: Uuid) -> FieldResult<Contributor> {
context.token.jwt.as_ref().ok_or(ThothError::Unauthorised)?;
Contributor::from_id(&context.db, &contributor_id)
.unwrap()
.delete(&context.db)
.map_err(|e| e.into())
let contributor = Contributor::from_id(&context.db, &contributor_id).unwrap();
for linked_publisher_id in contributor.linked_publisher_ids(&context.db)? {
context.account_access.can_edit(linked_publisher_id)?;
}

contributor.delete(&context.db).map_err(|e| e.into())
}

fn delete_contribution(context: &Context, contribution_id: Uuid) -> FieldResult<Contribution> {
Expand Down Expand Up @@ -2041,10 +2043,12 @@ impl MutationRoot {

fn delete_institution(context: &Context, institution_id: Uuid) -> FieldResult<Institution> {
context.token.jwt.as_ref().ok_or(ThothError::Unauthorised)?;
Institution::from_id(&context.db, &institution_id)
.unwrap()
.delete(&context.db)
.map_err(|e| e.into())
let institution = Institution::from_id(&context.db, &institution_id).unwrap();
for linked_publisher_id in institution.linked_publisher_ids(&context.db)? {
context.account_access.can_edit(linked_publisher_id)?;
}

institution.delete(&context.db).map_err(|e| e.into())
}

fn delete_funding(context: &Context, funding_id: Uuid) -> FieldResult<Funding> {
Expand Down
24 changes: 24 additions & 0 deletions thoth-api/src/model/contributor/crud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,30 @@ impl DbInsert for NewContributorHistory {
db_insert!(contributor_history::table);
}

impl Contributor {
pub fn linked_publisher_ids(&self, db: &crate::db::PgPool) -> ThothResult<Vec<Uuid>> {
contributor_linked_publisher_ids(self.contributor_id, db)
}
}

fn contributor_linked_publisher_ids(
contributor_id: Uuid,
db: &crate::db::PgPool,
) -> ThothResult<Vec<Uuid>> {
let mut connection = db.get().unwrap();
crate::schema::publisher::table
.inner_join(
crate::schema::imprint::table.inner_join(
crate::schema::work::table.inner_join(crate::schema::contribution::table),
),
)
.select(crate::schema::publisher::publisher_id)
.filter(crate::schema::contribution::contributor_id.eq(contributor_id))
.distinct()
.load::<Uuid>(&mut connection)
.map_err(Into::into)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
35 changes: 35 additions & 0 deletions thoth-api/src/model/institution/crud.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,41 @@ impl DbInsert for NewInstitutionHistory {
db_insert!(institution_history::table);
}

impl Institution {
pub fn linked_publisher_ids(&self, db: &crate::db::PgPool) -> ThothResult<Vec<Uuid>> {
institution_linked_publisher_ids(self.institution_id, db)
}
}

fn institution_linked_publisher_ids(
institution_id: Uuid,
db: &crate::db::PgPool,
) -> ThothResult<Vec<Uuid>> {
let mut connection = db.get().unwrap();
let publishers_via_affiliation = crate::schema::publisher::table
.inner_join(crate::schema::imprint::table.inner_join(
crate::schema::work::table.inner_join(
crate::schema::contribution::table.inner_join(crate::schema::affiliation::table),
),
))
.select(crate::schema::publisher::publisher_id)
.filter(crate::schema::affiliation::institution_id.eq(institution_id))
.distinct()
.load::<Uuid>(&mut connection)
.map_err(|_| ThothError::InternalError("Unable to load records".into()))?;
let publishers_via_funding = crate::schema::publisher::table
.inner_join(
crate::schema::imprint::table
.inner_join(crate::schema::work::table.inner_join(crate::schema::funding::table)),
)
.select(crate::schema::publisher::publisher_id)
.filter(crate::schema::funding::institution_id.eq(institution_id))
.distinct()
.load::<Uuid>(&mut connection)
.map_err(|_| ThothError::InternalError("Unable to load records".into()))?;
Ok([publishers_via_affiliation, publishers_via_funding].concat())
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
8 changes: 6 additions & 2 deletions thoth-app/src/component/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,9 @@ fn switch_admin(
}
AdminRoute::NewImprint => html! {<NewImprintComponent { current_user } />},
AdminRoute::Institutions => html! {<InstitutionsComponent { current_user } />},
AdminRoute::Institution { id } => html! {<InstitutionComponent institution_id={ *id } />},
AdminRoute::Institution { id } => {
html! {<InstitutionComponent institution_id={ *id } { current_user } />}
}
AdminRoute::NewInstitution => html! {<NewInstitutionComponent/>},
AdminRoute::Publications => html! {<PublicationsComponent { current_user } />},
AdminRoute::Publication { id } => {
Expand All @@ -205,7 +207,9 @@ fn switch_admin(
}
}
AdminRoute::Contributors => html! {<ContributorsComponent { current_user } />},
AdminRoute::Contributor { id } => html! {<ContributorComponent contributor_id={ *id } />},
AdminRoute::Contributor { id } => {
html! {<ContributorComponent contributor_id={ *id } { current_user } />}
}
AdminRoute::NewContributor => html! {<NewContributorComponent/>},
AdminRoute::Serieses => html! {<SeriesesComponent { current_user } />},
AdminRoute::NewSeries => html! {<NewSeriesComponent { current_user } />},
Expand Down
20 changes: 19 additions & 1 deletion thoth-app/src/component/contributor.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use thoth_api::account::model::AccountDetails;
use thoth_api::model::contribution::ContributionWithWork;
use thoth_api::model::contributor::Contributor;
use thoth_api::model::{Orcid, ORCID_DOMAIN};
Expand Down Expand Up @@ -81,6 +82,7 @@ pub enum Msg {
#[derive(PartialEq, Eq, Properties)]
pub struct Props {
pub contributor_id: Uuid,
pub current_user: AccountDetails,
}

impl Component for ContributorComponent {
Expand Down Expand Up @@ -327,6 +329,21 @@ impl Component for ContributorComponent {
event.prevent_default();
Msg::UpdateContributor
});
let mut delete_callback = Some(ctx.link().callback(|_| Msg::DeleteContributor));
let mut delete_deactivated = false;
// If user doesn't have permission to delete this contributor (i.e. because it's connected to a work
// from a publisher they're not associated with), deactivate the delete button and unset its callback
if let Some(publishers) = ctx.props().current_user.resource_access.restricted_to() {
for contribution in &self.contributor_activity {
if !publishers
.contains(&contribution.work.imprint.publisher.publisher_id.to_string())
{
delete_callback = None;
delete_deactivated = true;
break;
}
}
}
html! {
<>
<nav class="level">
Expand All @@ -338,8 +355,9 @@ impl Component for ContributorComponent {
<div class="level-right">
<p class="level-item">
<ConfirmDeleteComponent
onclick={ ctx.link().callback(|_| Msg::DeleteContributor) }
onclick={ delete_callback }
object_name={ self.contributor.full_name.clone() }
deactivated={ delete_deactivated }
/>
</p>
</div>
Expand Down
12 changes: 9 additions & 3 deletions thoth-app/src/component/delete_dialogue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ pub struct ConfirmDeleteComponent {

#[derive(PartialEq, Properties)]
pub struct Props {
pub onclick: Callback<MouseEvent>,
pub onclick: Option<Callback<MouseEvent>>,
pub object_name: String,
#[prop_or_default]
pub deactivated: bool,
}

pub enum Msg {
Expand Down Expand Up @@ -45,7 +47,11 @@ impl Component for ConfirmDeleteComponent {
});
html! {
<>
<button class="button is-danger" onclick={ open_modal }>
<button
class="button is-danger"
onclick={ open_modal }
disabled={ ctx.props().deactivated }
>
{ DELETE_BUTTON }
</button>
<div class={ self.confirm_delete_status() }>
Expand All @@ -69,7 +75,7 @@ impl Component for ConfirmDeleteComponent {
<footer class="modal-card-foot">
<button
class="button is-success"
onclick={ &ctx.props().onclick }
onclick={ ctx.props().onclick.clone() }
>
{ DELETE_BUTTON }
</button>
Expand Down
19 changes: 18 additions & 1 deletion thoth-app/src/component/institution.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::str::FromStr;
use thoth_api::account::model::AccountDetails;
use thoth_api::model::institution::CountryCode;
use thoth_api::model::institution::Institution;
use thoth_api::model::work::WorkWithRelations;
Expand Down Expand Up @@ -97,6 +98,7 @@ pub enum Msg {
#[derive(PartialEq, Eq, Properties)]
pub struct Props {
pub institution_id: Uuid,
pub current_user: AccountDetails,
}

impl Component for InstitutionComponent {
Expand Down Expand Up @@ -414,6 +416,20 @@ impl Component for InstitutionComponent {
event.prevent_default();
Msg::UpdateInstitution
});
let mut delete_callback = Some(ctx.link().callback(|_| Msg::DeleteInstitution));
let mut delete_deactivated = false;
// If user doesn't have permission to delete this institution (i.e. because it's connected to a work
// from a publisher they're not associated with), deactivate the delete button and unset its callback
if let Some(publishers) = ctx.props().current_user.resource_access.restricted_to() {
for work in [self.affiliated_works.clone(), self.funded_works.clone()].concat()
{
if !publishers.contains(&work.imprint.publisher.publisher_id.to_string()) {
delete_callback = None;
delete_deactivated = true;
break;
}
}
}
html! {
<>
<nav class="level">
Expand All @@ -425,8 +441,9 @@ impl Component for InstitutionComponent {
<div class="level-right">
<p class="level-item">
<ConfirmDeleteComponent
onclick={ ctx.link().callback(|_| Msg::DeleteInstitution) }
onclick={ delete_callback }
object_name={ self.institution.institution_name.clone() }
deactivated={ delete_deactivated }
/>
</p>
</div>
Expand Down

0 comments on commit 180720f

Please sign in to comment.