search

jasperweyne / helpless-kiwi

Manage members, create and manage activities, send mails and more
Apache License 2.0
11 stars 7 forks source link

feat: activity archive #325

Closed A-Daneel closed 1 year ago

A-Daneel commented 1 year ago

PR Draft to see if I can finally satisfy the database tests

A-Daneel commented 1 year ago

Question: With these changes we can delete an activity. It deletes the corresponding PriceOptions (which makes sense, and is desirable), and it deletes the Registrations.

Not sure if deleting the registration is the most ideal solution, it does keep in line with out "database should reflect what a user can see" philosophy. @jasperweyne you're usually pretty vocal about this, do you agree with my reasoning?

A-Daneel commented 1 year ago

Didn't realize that PR #321 was already merged. I'll update the annotations to attributes tonight.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

jasperweyne commented 1 year ago

I do agree with your reasoning: if an activity gets deleted, the corresponding data (in this case, registrations) should make sense. I do think that with this PR, deleting data gets too easy however; by allowing deletions with the current amount of safeguards (to be specific, simply an "are you sure" message), I feel that the functionality that this PR enables might be a bit risky.

Therefore, I'd like to see some added safeguards within this PR. My suggestion would be to only allow deletions for activities that haven't been visible yet. If the visibleAfter or the deadline has passed the default button should be to hide the activity. This could possibly be accompanied with instructions how to get to the point of activity deletion, but I feel that should be a multi-step process once an activity has been shown to regular users, especially for activities that already have passed.

A-Daneel commented 1 year ago

@jasperweyne took a quick stab at building this out. in broad strokes the functionality works as I intend them to.

I know the test coverage isn't high enough, but would appreciate some feedback. Hoping to finish this PR up before our next meeting.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

84.6% 84.6% Coverage
0.0% 0.0% Duplication