Merge pull request #406 from i-dot-ai/bugfix/REDBOX-256-delete-file-a…

…nd-chunks

call core api delete from django delete

django_app/redbox_app/redbox_core/client.py

@@ -74,3 +74,10 @@ def get_file_status(self, file_id: UUID, user: User) -> SimpleNamespace:
         response.raise_for_status()
         response_data = response.json(object_hook=lambda d: SimpleNamespace(**d))
         return response_data
+
+    def delete_file(self, file_id: UUID, user: User) -> SimpleNamespace:
+        url = self.url / "file" / str(file_id)
+        response = requests.delete(url, headers={"Authorization": user.get_bearer_token()}, timeout=60)
+        response.raise_for_status()
+        response_data = response.json(object_hook=lambda d: SimpleNamespace(**d))
+        return response_data

django_app/redbox_app/redbox_core/views.py

@@ -24,6 +24,7 @@
 from yarl import URL
 
 logger = logging.getLogger(__name__)
+core_api = CoreApiClient(host=settings.CORE_API_HOST, port=settings.CORE_API_PORT)
 
 CHUNK_SIZE = 1024
 # move this somewhere
@@ -121,7 +122,6 @@ def upload_view(request):
 
 def ingest_file(uploaded_file: UploadedFile, user: User) -> list[str]:
     errors: list[str] = []
-    api = CoreApiClient(host=settings.CORE_API_HOST, port=settings.CORE_API_PORT)
     try:
         file = File.objects.create(
             processing_status=ProcessingStatusEnum.uploaded.value,
@@ -135,7 +135,7 @@ def ingest_file(uploaded_file: UploadedFile, user: User) -> list[str]:
         errors.append(e.args[0])
     else:
         try:
-            upload_file_response = api.upload_file(file.unique_name, user)
+            upload_file_response = core_api.upload_file(file.unique_name, user)
         except HTTPError as e:
             logger.error("Error uploading file object %s.", file, exc_info=e)
             file.delete()
@@ -149,14 +149,24 @@ def ingest_file(uploaded_file: UploadedFile, user: User) -> list[str]:
 @login_required
 def remove_doc_view(request, doc_id: uuid):
     file = File.objects.get(pk=doc_id)
+    errors: list[str] = []
+
     if request.method == "POST":
-        logger.info("Removing document: %s", request.POST["doc_id"])
-        file.delete()
-        return redirect("documents")
+        try:
+            core_api.delete_file(file.core_file_uuid, request.user)
+        except HTTPError as e:
+            logger.error("Error deleting file object %s.", file, exc_info=e)
+            errors.append("There was an error deleting this file")
+
+        else:
+            logger.info("Removing document: %s", request.POST["doc_id"])
+            file.delete()
+            return redirect("documents")
+
     return render(
         request,
         template_name="remove-doc.html",
-        context={"request": request, "doc_id": doc_id, "doc_name": file.name},
+        context={"request": request, "doc_id": doc_id, "doc_name": file.name, "errors": errors},
     )
 
 
@@ -203,7 +213,6 @@ def post_message(request: HttpRequest) -> HttpResponse:
         {"role": message.role, "text": message.text}
         for message in ChatMessage.objects.all().filter(chat_history=session)
     ]
-    core_api = CoreApiClient(host=settings.CORE_API_HOST, port=settings.CORE_API_PORT)
     response_data = core_api.rag_chat(message_history, request.user)
 
     llm_message = ChatMessage(chat_history=session, text=response_data.output_text, role=ChatRoleEnum.ai)
@@ -228,7 +237,6 @@ def file_status_api_view(request: HttpRequest) -> JsonResponse:
     except File.DoesNotExist as ex:
         logger.error("File object information not found in django - file does not exist %s.", file_id, exc_info=ex)
         return JsonResponse({"status": ProcessingStatusEnum.unknown.label})
-    core_api = CoreApiClient(host=settings.CORE_API_HOST, port=settings.CORE_API_PORT)
     try:
         core_file_status_response = core_api.get_file_status(file_id=file.core_file_uuid, user=request.user)
     except HTTPError as ex:

django_app/redbox_app/templates/macros/govuk-error-summary.html

@@ -0,0 +1,18 @@
+{% macro govukErrorSummary(title="There is a problem", error_list=None, heading_level=2) %}
+{% if error_list %}
+  <div class="govuk-error-summary" data-module="govuk-error-summary">
+    <div role="alert">
+      <h{{ heading_level }} class="govuk-error-summary__title">
+        {{ title }}
+      </h{{ heading_level }}>
+      <div class="govuk-error-summary__body">
+        <ul class="govuk-list govuk-error-summary__list">
+          {% for error in error_list %}
+            <li>{{error}}</li>
+          {% endfor %}
+        </ul>
+      </div>
+    </div>
+  </div>
+{% endif %}
+{% endmacro %}

django_app/redbox_app/templates/remove-doc.html

@@ -2,6 +2,7 @@
 
 {% extends "base.html" %}
 {% from "macros/govuk-button.html" import govukButton %}
+{% from "macros/govuk-error-summary.html" import govukErrorSummary %}
 
 {% block content %}
 
@@ -18,6 +19,8 @@
 
     <h1 class="govuk-heading-m">Are you sure you want to remove this document?</h1>
 
+    {{ govukErrorSummary(error_list=errors) }}
+
     <dl class="govuk-summary-list">
       <div class="govuk-summary-list__row">
         <dt class="govuk-summary-list__key">

django_app/tests/test_views.py

@@ -176,6 +176,49 @@ def test_upload_view_no_file(alice, client):
     assert "No document selected" in str(response.content)
 
 
+@pytest.mark.django_db
+def test_remove_doc_view(client: Client, alice: User, file_pdf_path: str, s3_client: Client, requests_mock: Mocker):
+    file_name = file_pdf_path.split("/")[-1]
+
+    client.force_login(alice)
+    # we begin by removing any file in minio that has this key
+    s3_client.delete_object(Bucket=settings.BUCKET_NAME, Key=file_name.replace(" ", "_"))
+
+    previous_count = count_s3_objects(s3_client)
+
+    mocked_response = {
+        "key": file_name,
+        "bucket": settings.BUCKET_NAME,
+        "uuid": str(uuid.uuid4()),
+    }
+    requests_mock.post(
+        f"http://{settings.CORE_API_HOST}:{settings.CORE_API_PORT}/file",
+        status_code=201,
+        json=mocked_response,
+    )
+
+    with open(file_pdf_path, "rb") as f:
+        # create file before testing deletion
+        client.post("/upload/", {"uploadDoc": f})
+        assert file_exists(s3_client, file_name)
+        assert count_s3_objects(s3_client) == previous_count + 1
+
+        new_file = File.objects.filter(user=alice).order_by("-created_at")[0]
+        requests_mock.delete(
+            f"http://{settings.CORE_API_HOST}:{settings.CORE_API_PORT}/file/{new_file.core_file_uuid}",
+            status_code=201,
+            json=mocked_response,
+        )
+
+        client.post(f"/remove-doc/{new_file.id}", {"doc_id": new_file.id})
+        assert not file_exists(s3_client, file_name)
+        assert count_s3_objects(s3_client) == previous_count
+        assert requests_mock.request_history[-1].method == "DELETE"
+
+        with pytest.raises(File.DoesNotExist):
+            File.objects.get(id=new_file.id)
+
+
 @pytest.mark.django_db
 def test_post_message_to_new_session(alice: User, client: Client, requests_mock: Mocker):
     # Given