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

Add setting ALLOW_PATH_DELETION_TOPOLOGY #3128

Merged
merged 6 commits into from
Jun 7, 2022

Conversation

LePetitTim
Copy link
Contributor

No description provided.

geotrek/core/models.py Outdated Show resolved Hide resolved
@cypress
Copy link

cypress bot commented Jun 2, 2022



Test summary

22 0 0 0


Run details

Project Geotrek-admin
Status Passed
Commit 9084cfb
Started Jun 7, 2022 8:35 AM
Ended Jun 7, 2022 8:39 AM
Duration 03:48 💡
OS Linux Ubuntu - 20.04
Browser Electron 94

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #3128 (9084cfb) into master (65cb892) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3128   +/-   ##
=======================================
  Coverage   97.60%   97.60%           
=======================================
  Files         272      272           
  Lines       18818    18822    +4     
=======================================
+ Hits        18368    18372    +4     
  Misses        450      450           
Impacted Files Coverage Δ
geotrek/core/models.py 96.32% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65cb892...9084cfb. Read the comment docs.

<div class="alert alert-block">
{% if topologies_by_model|length and not allowed_path_deletion_topology %}
<h4 class="alert-heading">Warning!</h4>
{% blocktrans %}You can't remove <strong>{{ object }}</strong>, some topologies are linked with this path. MModify these topologies before deleting this path.{% endblocktrans %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un M de trop à "Modifiy"

@camillemonchicourt
Copy link
Member

camillemonchicourt commented Jun 2, 2022

En lien avec #2515

3.3.1 Suppression d’un tronçon

La suppression d’un tronçon parcouru par une ou plusieurs topologies linéaires alerte l’utilisateur sur les risques encourus, mais lui laisse tout de même la possibilité de casser les données.

Suite à l’étude, il faut soit :

  • interdire catégoriquement la suppression d’un tronçon parcouru par un linéaire. Le risque qu’une personne ne faisant pas attention ou n’étant pas au fait des conséquences de cette action existera toujours. Pas de trigger impacté, il suffit de mettre un ON DELETE PROTECT sur le path_id d’un core_pathaggregation
  • alerter l’utilisateur qu’en cas de suppression, les topologies linéaires seront aussi supprimées. C’est la méthode dure, mais elle a le mérite de maintenir l’intégrité des données. Il faudra personnaliser le message d’erreur SQL car si la modification a lieu coté QGIS, il ne sera pas possible de CASCADER la suppression des linéaires (Coté Django, il sera possible de tout supprimer).

Finalement un paramètre ALLOW_PATH_DELETION_TOPOLOGY a été ajouté et c'est seulement le message affiché quand on veut supprimer un tronçon qui a des topologies associées qui varie :

  • Si ce paramètre est à True, alors on a le fonctionnement actuel où les topologies liées au tronçon sont listées mais l'utilisateur peut quand même confirmer la suppression du tronçon
  • Si ce paramètre est à False, alors les topologies liées au tronçon sont listées et l'utilisateur ne peut pas supprimer le tronçon tant que celui-ci a des topologies qui lui sont associées

Mais pour garantir la cohérence de la BDD si on la modifie directement en SQL ou dans un outil d'administration externe, un trigger a été ajouté, réalisant le même contrôle : https://github.com/GeotrekCE/Geotrek-admin/pull/3129/files

@LePetitTim LePetitTim force-pushed the add_settings_confirmation_deletion_path branch from 3b5e68f to 005d470 Compare June 3, 2022 14:00
@LePetitTim LePetitTim requested a review from submarcos June 7, 2022 07:44
@LePetitTim LePetitTim merged commit cdb55e8 into master Jun 7, 2022
@LePetitTim LePetitTim deleted the add_settings_confirmation_deletion_path branch June 7, 2022 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants