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

cascading delete #2041

Merged
merged 65 commits into from
Nov 19, 2023
Merged

cascading delete #2041

merged 65 commits into from
Nov 19, 2023

Conversation

sleidig
Copy link
Member

@sleidig sleidig commented Oct 18, 2023

closes: #220

  • unit tests for all scenarios
  • recursive delete logic for related entities
  • undo function for all affected entities
  • schema config for entity-ref as aggregate / composite
  • mark entities as a whole as "no personal data"
  • ❓ manual review in some cases (and/or warn user in confirmation dialog that related entities will also be affected)
  • ❓ discuss whether code of anonymize and delete should be split to reduce complexity
  • progress indicators while processing action + undo

cascading-delete

@github-actions
Copy link

Deployed to https://pr-2041.aam-digital.net/

@sleidig sleidig changed the title [WIP] cascading delete cascading delete Oct 20, 2023
@sleidig sleidig requested a review from TheSlimvReal October 20, 2023 16:18
Copy link
Collaborator

@TheSlimvReal TheSlimvReal left a comment

Choose a reason for hiding this comment

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

Really nice job. I like how little places needed to be modified and also how little core code is necessary in general. Also a very extensive test suite makes pretty clear what happens.
My comments are mostly around making thins a bit more precise in terms of naming. Happy to discuss this a bit more.

src/app/core/entity/entity-config.service.ts Show resolved Hide resolved
src/app/core/entity/entity-remove.service.spec.ts Outdated Show resolved Hide resolved
src/app/core/entity/entity-remove.service.spec.ts Outdated Show resolved Hide resolved
src/app/core/entity/entity-remove.service.spec.ts Outdated Show resolved Hide resolved
src/app/core/entity/entity-remove.service.spec.ts Outdated Show resolved Hide resolved
src/app/core/entity/entity-remove.service.ts Outdated Show resolved Hide resolved
src/app/core/entity/entity-remove.service.ts Outdated Show resolved Hide resolved
src/app/core/entity/entity-remove.service.ts Outdated Show resolved Hide resolved
src/app/core/entity/model/entity.ts Outdated Show resolved Hide resolved
src/app/core/entity/schema/entity-schema-field.ts Outdated Show resolved Hide resolved
@sleidig sleidig marked this pull request as ready for review November 14, 2023 18:18
Copy link
Collaborator

@TheSlimvReal TheSlimvReal left a comment

Choose a reason for hiding this comment

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

Good UI tweaks. Works very well.

I was wondering whether we should also write to the URL if inactive records are currently shown in the entity list. This makes it a little bit easier to navigate around.

@@ -0,0 +1,14 @@
<ng-container
*ngIf="!(displayAsAnonymized || isPartiallyAnonymized); else anonymized"
Copy link
Collaborator

Choose a reason for hiding this comment

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

A bit annoying, that both of these properties are necessary. Every component that is also used in templates will have to implement both which is a bit against the idea that they can be used everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, agreed. I wanted to reuse date-component's template for how month-component is displaying. Unfortunately isPartiallyAnonymized doubling as an @Input just gets overwritten ...

sleidig and others added 15 commits November 15, 2023 18:09
Co-authored-by: snyk-bot <snyk-bot@snyk.io>
Co-authored-by: Simon <simon@aam-digital.com>
Co-authored-by: snyk-bot <snyk-bot@snyk.io>
Co-authored-by: snyk-bot <snyk-bot@snyk.io>
Snyk has created this PR to upgrade flag-icons from 6.11.1 to 6.11.2.

See this package in npm:
https://www.npmjs.com/package/flag-icons

See this project in Snyk:
https://app.snyk.io/org/aam-digital-ci/project/a4b5c50c-28f4-4e30-af2d-840962b9beaf?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <snyk-bot@snyk.io>
to enable their use in popup forms like NoteDetails, which are always in edit mode

---------
This functionality has been developed for the project “codo”.
codo is developed under the projects “Landungsbrücken – Patenschaften in Hamburg stärken” and “openTransfer Patenschaften”. It is funded through the program “Menschen stärken Menschen” by the German Federal Ministry of Family Affairs, Senior Citizens, Women and Youth.
More information at https://github.com/codo-mentoring

“Landungsbrücken – Patenschaften in Hamburg stärken” is a project of BürgerStiftung Hamburg in cooperation with the Mentor.Ring Hamburg. With a mix of networking opportunities, capacity building and financial support the project strengthens Hamburg’s scene of mentoring projects since its founding in 2016.

The “Stiftung Bürgermut” foundation since 2007 supports the digital and real exchange of experiences and connections of active citizens. Within the federal program “Menschen stärken Menschen” the foundation as part of its program “openTransfer Patenschaften” offers support services for connecting, spreading and upskilling mentoring organisations across Germany.

Diese Funktion wurde entwickelt für das Projekt codo.
codo wird entwickelt im Rahmen der Projekte Landungsbrücken – Patenschaften in Hamburg stärken und openTransfer Patenschaften. Er ist gefördert durch das Bundesprogramm Menschen stärken Menschen des Bundesministeriums für Familie, Senioren, Frauen und Jugend.
Mehr Informationen unter https://github.com/codo-mentoring

“Landungsbrücken – Patenschaften in Hamburg stärken” ist ein Projekt der BürgerStiftung Hamburg in Kooperation mit dem Mentor.Ring Hamburg. Mit einer Mischung aus Vernetzungsangeboten, Qualifizierungsmaßnahmen und finanzieller Förderung stärkt das Projekt die Hamburger Szene der Patenschaftsprojekte seit der Gründung im Jahr 2016.

Die Stiftung Bürgermut fördert seit 2007 den digitalen und realen Erfahrungsaustausch und die Vernetzung von engagierten Bürger:innen. Innerhalb des Bundesprogramms „Menschen stärken Menschen” bietet die Stiftung im Rahmen ihres Programms openTransfer Patenschaften Unterstützungsleistungen zur Vernetzung, Verbreitung und Qualifizierung von Patenschafts- und Mentoringorganisationen bundesweit.

Co-authored-by: codo-mentoring <117934638+codo-mentoring@users.noreply.github.com>
closes #1872

---------

Co-authored-by: Sebastian Leidig <sebastian@aam-digital.com>
closes #2081

---------

Co-authored-by: Sebastian <sebastian@aam-digital.com>
@sleidig sleidig merged commit 12b5855 into master Nov 19, 2023
7 of 8 checks passed
@sleidig sleidig deleted the anon-3 branch November 19, 2023 22:04
@aam-digital-ci
Copy link
Collaborator

🎉 This PR is included in version 3.26.0-master.21 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@aam-digital-ci aam-digital-ci added the released on @master managed by CI (semantic-release) label Nov 19, 2023
@aam-digital-ci
Copy link
Collaborator

🎉 This PR is included in version 3.26.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@aam-digital-ci aam-digital-ci added the released managed by CI (semantic-release) label Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @master managed by CI (semantic-release) released managed by CI (semantic-release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cascading delete (remove related entities / references when deleting an entity)
6 participants