From 30918bdf49b2e83527f89ceda97170a7baca25da Mon Sep 17 00:00:00 2001 From: Etienne Delclaux <150020787+edelclaux@users.noreply.github.com> Date: Mon, 14 Oct 2024 15:35:17 +0200 Subject: [PATCH] [GN_META] Enable acquisition framework deletion (#3224) * feat: add delete af button + update route to handle multiple authorization case * feat: add test for af deletion --------- Co-authored-by: Jacques Fize <4259846+jacquesfize@users.noreply.github.com> --- .../core/gn_meta/models/aframework.py | 21 ++-- backend/geonature/core/gn_meta/routes.py | 12 ++- backend/geonature/tests/fixtures.py | 30 ++++-- backend/geonature/tests/test_gn_meta.py | 96 ++++++++++++------- .../metadataModule/af/af-card.component.html | 18 +++- .../metadataModule/af/af-card.component.scss | 17 ++++ .../af/button-delete-af.component.html | 23 +++++ .../af/button-delete-af.component.scss | 0 .../af/button-delete-af.component.ts | 63 ++++++++++++ .../metadataModule/metadata.component.html | 5 + .../src/app/metadataModule/metadata.module.ts | 2 + 11 files changed, 226 insertions(+), 61 deletions(-) create mode 100644 frontend/src/app/metadataModule/af/button-delete-af.component.html create mode 100644 frontend/src/app/metadataModule/af/button-delete-af.component.scss create mode 100644 frontend/src/app/metadataModule/af/button-delete-af.component.ts diff --git a/backend/geonature/core/gn_meta/models/aframework.py b/backend/geonature/core/gn_meta/models/aframework.py index e959ac4f9e..73672f660c 100644 --- a/backend/geonature/core/gn_meta/models/aframework.py +++ b/backend/geonature/core/gn_meta/models/aframework.py @@ -121,14 +121,21 @@ def user_actors(self): def organism_actors(self): return [actor.organism for actor in self.cor_af_actor if actor.organism] - def is_deletable(self): - return not ( - db.session.scalar( - exists() - .select_from() - .where(TDatasets.id_acquisition_framework == self.id_acquisition_framework) - .select() + def has_datasets(self): + return db.session.scalar( + exists(TDatasets) + .where(TDatasets.id_acquisition_framework == self.id_acquisition_framework) + .select() + ) + + def has_child_acquisition_framework(self): + return db.session.scalar( + exists(TAcquisitionFramework) + .where( + TAcquisitionFramework.acquisition_framework_parent_id + == self.id_acquisition_framework ) + .select() ) def has_instance_permission(self, scope, _through_ds=True): diff --git a/backend/geonature/core/gn_meta/routes.py b/backend/geonature/core/gn_meta/routes.py index e405ece17f..5563613b8e 100644 --- a/backend/geonature/core/gn_meta/routes.py +++ b/backend/geonature/core/gn_meta/routes.py @@ -1,5 +1,5 @@ """ - Routes for gn_meta + Routes for gn_meta """ import datetime as dt @@ -772,11 +772,17 @@ def delete_acquisition_framework(scope, af_id): raise Forbidden( f"User {g.current_user} cannot delete acquisition framework {af.id_acquisition_framework}" ) - if not af.is_deletable(): + if af.has_datasets(): raise Conflict( - "La suppression du cadre d’acquisition n'est pas possible " + "La suppression du cadre d’acquisition est impossible " "car celui-ci contient des jeux de données." ) + + if af.has_child_acquisition_framework(): + raise Conflict( + "La suppression du cadre d’acquisition est impossible " + "car celui-ci est le parent d'autre(s) cadre(s) d'acquisition." + ) db.session.delete(af) db.session.commit() diff --git a/backend/geonature/tests/fixtures.py b/backend/geonature/tests/fixtures.py index 23b0e9a5da..bda91a4930 100644 --- a/backend/geonature/tests/fixtures.py +++ b/backend/geonature/tests/fixtures.py @@ -387,12 +387,16 @@ def acquisition_frameworks(users): ) ).scalar_one() - def create_af(name, creator): + def create_af(name, creator, is_parent, parent_af=None): with db.session.begin_nested(): af = TAcquisitionFramework( acquisition_framework_name=name, acquisition_framework_desc=name, creator=creator, + is_parent=is_parent, + acquisition_framework_parent_id=( + parent_af.id_acquisition_framework if parent_af else None + ), ) db.session.add(af) if creator and creator.organisme: @@ -400,20 +404,26 @@ def create_af(name, creator): organism=creator.organisme, nomenclature_actor_role=principal_actor_role ) af.cor_af_actor.append(actor) + db.session.flush() return af afs = { - name: create_af(name=name, creator=creator) - for name, creator in [ - ("own_af", users["user"]), - ("associate_af", users["associate_user"]), - ("stranger_af", users["stranger_user"]), - ("orphan_af", None), - ("af_1", None), - ("af_2", None), - ("af_3", None), + name: create_af(name=name, creator=creator, is_parent=is_parent, parent_af=None) + for name, creator, is_parent in [ + ("own_af", users["user"], False), + ("associate_af", users["associate_user"], False), + ("stranger_af", users["stranger_user"], False), + ("orphan_af", None, False), + ("af_1", None, False), + ("af_2", None, False), + ("af_3", None, False), + ("parent_af", users["user"], True), + ("parent_wo_children_af", users["user"], True), + ("delete_parent_wo_children_af", users["user"], True), + ("delete_af", users["user"], False), ] } + afs["child_af"] = create_af("child_af", users["user"], False, afs["parent_af"]) return afs diff --git a/backend/geonature/tests/test_gn_meta.py b/backend/geonature/tests/test_gn_meta.py index 64c29bbeff..d75a4a94be 100644 --- a/backend/geonature/tests/test_gn_meta.py +++ b/backend/geonature/tests/test_gn_meta.py @@ -159,11 +159,17 @@ def test_acquisition_frameworks_permissions(self, app, acquisition_frameworks, d ) ta = TAcquisitionFramework sc = db.session.scalars + assert set(sc(ta.filter_by_scope(0, query=qs)).unique().all()) == set([]) assert set(sc(ta.filter_by_scope(1, query=qs)).unique().all()) == set( [ acquisition_frameworks["own_af"], acquisition_frameworks["orphan_af"], # through DS + acquisition_frameworks["parent_af"], + acquisition_frameworks["child_af"], + acquisition_frameworks["parent_wo_children_af"], + acquisition_frameworks["delete_parent_wo_children_af"], + acquisition_frameworks["delete_af"], ] ) assert set(sc(ta.filter_by_scope(2, query=qs)).unique().all()) == set( @@ -171,17 +177,40 @@ def test_acquisition_frameworks_permissions(self, app, acquisition_frameworks, d acquisition_frameworks["own_af"], acquisition_frameworks["associate_af"], acquisition_frameworks["orphan_af"], # through DS + acquisition_frameworks["parent_af"], + acquisition_frameworks["child_af"], + acquisition_frameworks["parent_wo_children_af"], + acquisition_frameworks["delete_parent_wo_children_af"], + acquisition_frameworks["delete_af"], ] ) assert set(sc(ta.filter_by_scope(3, query=qs)).unique().all()) == set( acquisition_frameworks.values() ) - def test_acquisition_framework_is_deletable(self, app, acquisition_frameworks, datasets): - assert acquisition_frameworks["own_af"].is_deletable() == True - assert ( - acquisition_frameworks["orphan_af"].is_deletable() == False - ) # DS are attached to this AF + @pytest.mark.parametrize( + "af,has_datasets", + [ + ("own_af", False), + ("orphan_af", True), + ], + ) + def test_acquisition_framework_has_datasets( + self, app, acquisition_frameworks, datasets, af, has_datasets + ): + assert acquisition_frameworks[af].has_datasets() == has_datasets + + @pytest.mark.parametrize( + "af,has_child_af", + [ + ("parent_af", True), + ("parent_wo_children_af", False), + ], + ) + def test_acquisition_framework_has_child_acquisition_framework( + self, app, acquisition_frameworks, datasets, af, has_child_af + ): + assert acquisition_frameworks[af].has_child_acquisition_framework() == has_child_af def test_create_acquisition_framework(self, users): set_logged_user(self.client, users["user"]) @@ -204,37 +233,32 @@ def test_create_acquisition_framework_forbidden(self, users): assert response.status_code == Forbidden.code - def test_delete_acquisition_framework(self, app, users, acquisition_frameworks, datasets): - af_id = acquisition_frameworks["orphan_af"].id_acquisition_framework - - response = self.client.delete(url_for("gn_meta.delete_acquisition_framework", af_id=af_id)) - assert response.status_code == Unauthorized.code - - set_logged_user(self.client, users["noright_user"]) - - # The user has no rights on METADATA module - response = self.client.delete(url_for("gn_meta.delete_acquisition_framework", af_id=af_id)) - assert response.status_code == Forbidden.code - assert "METADATA" in response.json["description"] - - set_logged_user(self.client, users["self_user"]) - - # The user has right on METADATA module, but not on this specific AF - response = self.client.delete(url_for("gn_meta.delete_acquisition_framework", af_id=af_id)) - assert response.status_code == Forbidden.code - assert "METADATA" not in response.json["description"] - - set_logged_user(self.client, users["admin_user"]) - - # The AF can not be deleted due to attached DS - response = self.client.delete(url_for("gn_meta.delete_acquisition_framework", af_id=af_id)) - assert response.status_code == Conflict.code - - set_logged_user(self.client, users["user"]) - af_id = acquisition_frameworks["own_af"].id_acquisition_framework + @pytest.mark.parametrize( + "user,dataset,status_code", + [ + (None, "orphan_af", Unauthorized.code), + ("noright_user", "orphan_af", Forbidden.code), + ("self_user", "orphan_af", Forbidden.code), + ("admin_user", "orphan_af", Conflict.code), + ("admin_user", "parent_af", Conflict.code), + ("user", "own_af", 204), + ("user", "delete_parent_wo_children_af", 204), + ("user", "delete_af", 204), + ], + ) + def test_delete_acquisition_framework( + self, app, users, acquisition_frameworks, datasets, user, dataset, status_code + ): + if user: + set_logged_user(self.client, users[user]) - response = self.client.delete(url_for("gn_meta.delete_acquisition_framework", af_id=af_id)) - assert response.status_code == 204 + response = self.client.delete( + url_for( + "gn_meta.delete_acquisition_framework", + af_id=acquisition_frameworks[dataset].id_acquisition_framework, + ) + ) + assert response.status_code == status_code def test_update_acquisition_framework(self, users, acquisition_frameworks): new_name = "thenewname" @@ -1092,7 +1116,7 @@ def test_get_user_af(self, users, acquisition_frameworks): assert isinstance(afquery, Select) assert isinstance(afuser, list) - assert len(afuser) == 1 + assert len(afuser) == 6 assert isinstance(afdefault, list) assert len(afdefault) >= 1 diff --git a/frontend/src/app/metadataModule/af/af-card.component.html b/frontend/src/app/metadataModule/af/af-card.component.html index e79836423a..8b6aff2f33 100644 --- a/frontend/src/app/metadataModule/af/af-card.component.html +++ b/frontend/src/app/metadataModule/af/af-card.component.html @@ -15,10 +15,14 @@
-
-
Cadre d'acquisition
-

- {{ af?.acquisition_framework_name }} +
+
+
Cadre d'acquisition
+

+ {{ af?.acquisition_framework_name }} +

+
+
-

+ +
diff --git a/frontend/src/app/metadataModule/af/af-card.component.scss b/frontend/src/app/metadataModule/af/af-card.component.scss index 16d96119c1..5329e6594c 100644 --- a/frontend/src/app/metadataModule/af/af-card.component.scss +++ b/frontend/src/app/metadataModule/af/af-card.component.scss @@ -1,3 +1,20 @@ +.af-card-header { + display: flex; + flex-flow: row nowrap; + justify-content: space-between; + + &__left { + display: flex; + flex-flow: column nowrap; + justify-content: flex-start; + } + &__right { + display: flex; + flex-flow: row nowrap; + justify-content: flex-end; + } +} + .btn-secondary, .btn-primary, .btn-success, diff --git a/frontend/src/app/metadataModule/af/button-delete-af.component.html b/frontend/src/app/metadataModule/af/button-delete-af.component.html new file mode 100644 index 0000000000..2cf898969f --- /dev/null +++ b/frontend/src/app/metadataModule/af/button-delete-af.component.html @@ -0,0 +1,23 @@ + + + + diff --git a/frontend/src/app/metadataModule/af/button-delete-af.component.scss b/frontend/src/app/metadataModule/af/button-delete-af.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frontend/src/app/metadataModule/af/button-delete-af.component.ts b/frontend/src/app/metadataModule/af/button-delete-af.component.ts new file mode 100644 index 0000000000..1aab7fb507 --- /dev/null +++ b/frontend/src/app/metadataModule/af/button-delete-af.component.ts @@ -0,0 +1,63 @@ +import { Component, Input } from '@angular/core'; +import { DataFormService } from '@geonature_common/form/data-form.service'; +import { MetadataService } from '../services/metadata.service'; +import { ConfirmationDialog } from '@geonature_common/others/modal-confirmation/confirmation.dialog'; +import { MatDialog } from '@angular/material/dialog'; +import { Router } from '@angular/router'; + +enum ButtonType { + Toolbar = 'Toolbar', + Floating = 'Floating', +} + +const METADATA_URL = '/metadata'; +@Component({ + selector: 'gn-button-delete-af', + templateUrl: './button-delete-af.component.html', + styleUrls: ['./button-delete-af.component.scss'], +}) +export class ButtonDeleteAfComponent { + readonly ButtonType = ButtonType; + + @Input() + acquisitionFramework: any; + + @Input() + redirectionUrl: string = METADATA_URL; + + @Input() + buttonType: ButtonType = ButtonType.Toolbar; + + constructor( + private _dfs: DataFormService, + private _mds: MetadataService, + private _dialog: MatDialog, + private _router: Router + ) {} + + deleteAcquisitionFramework() { + const dialogRef = this._dialog.open(ConfirmationDialog, { + width: 'auto', + position: { top: '5%' }, + data: { + message: "Voulez-vous supprimer ce cadre d'acquisition ?", + yesColor: 'primary', + noColor: 'warn', + }, + }); + dialogRef.afterClosed().subscribe((result) => { + if (result) { + this._dfs.deleteAf(this.acquisitionFramework.id_acquisition_framework).subscribe((res) => { + this._mds.getMetadata(); + if (this.redirectionUrl) { + this._router.navigate([this.redirectionUrl]); + } + }); + } + }); + } + + get disabled() { + return !this.acquisitionFramework.cruved.D; + } +} diff --git a/frontend/src/app/metadataModule/metadata.component.html b/frontend/src/app/metadataModule/metadata.component.html index 08cd71ee64..4c56104446 100644 --- a/frontend/src/app/metadataModule/metadata.component.html +++ b/frontend/src/app/metadataModule/metadata.component.html @@ -164,6 +164,11 @@

Catalogue des jeux de données

> create +