Dpat 1749 #1212
Dpat 1749 #1212
Conversation
…to call it Changes view to use form_valid rather than delete to handle deletion, as this was a change introduced in django 4.0. See docs/release notes for full info.
- For admins, list deleted datasources - Fix bugs in templates so that admin view displays datasource type
michaeljcollinsuk
force-pushed
the
DPAT-1749
branch
from
8431f15 to
7c01998
Compare
| {% if request.user.is_superuser and deleted_datasources %} | ||
| <h3 class="govuk-heading-m">Deleted data sources</h3> | ||
| {{ datasource_list(deleted_datasources, datasource_type|default(""), request.user) }} | ||
| {% endif %} |
There was a problem hiding this comment.
good idea to show this on admin page on a separated group
| # TODO when soft delete is added, this should be updated to use the user that | ||
| # has deleted the parent S3bucket to ensure we store the user that has sent the | ||
| # task in the case of cascading deletes | ||
| tasks.S3BucketRevokeUserAccess(self, self.current_user).create_task() | ||
| revoked_by = revoked_by or self.current_user |
There was a problem hiding this comment.
same as above about revoked_by 😊 , happy to discuss it
There was a problem hiding this comment.
TBH it was just so that I update my test to check that the task is called with the correct user... was difficult to test this by assigning it directly to the instance as current_user. Also felt it added a bit more flexibility, but maybe not necessary. And updating the code just for the test also isnt ideal.... i'll take another look and simplify
There was a problem hiding this comment.
I see, hmm, passing user info by this way (I meant as curent_user) is something I feel always is not very clean solution , but haven't got better approach 😊
📝 Summary
This PR resolves ministryofjustice/analytical-platform#1749
This PR replaces S3Bucket deletion with a soft-delete action. This means that the database object is retained, but marked as deleted. This will allow us to capture details of who has deleted the bucket and when. Will also allow possibility of restoring S3 buckets in future.
In the immediate term, it means that work on ministryofjustice/analytical-platform#1887 can begin.
When the S3bucket is soft deleted:
As part of the changes, I also fixed some bugs within the template for all datasources - such as displaying the type of the bucket (warehouse or app).
🔍 What should the reviewer concentrate on?
🧑💻 How should the reviewer test these changes?
📚 Documentation status