Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent deletion of contributors/institutions used by other publishers (#548) #614

Merged
merged 11 commits into from
Sep 3, 2024
Merged
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
Loading