♻️ refactor tracex views

tracex_project/tracex/views.py

@@ -7,8 +7,10 @@
 DownloadXesView -- View for the download of XES file(s).
 """
 import os
+import traceback
 import zipfile
 from tempfile import NamedTemporaryFile
+from typing import List
 
 from django.views import View
 from django.http import HttpResponse, FileResponse
@@ -20,33 +22,33 @@
 
 class TracexLandingPage(TemplateView):
     """View for the landing page of the tracex app."""
+
     template_name = "landing_page.html"
 
-    def get(self, _request, *_args, **kwargs):
+    def get(self, _request, *args, **kwargs):
         """Handle GET requests by initializing a form for API key entry and adding it to the context."""
-        form = ApiKeyForm()
         context = self.get_context_data(**kwargs)
-        context['form'] = form
+        context['form'] = ApiKeyForm()
 
         return self.render_to_response(context)
 
     def post(self, request):
-        """Handles POST requests by processing a submitted form containing an API key.
-        If valid, saves the API key to the environment and redirects to the landing page;
-        otherwise, renders the form with errors."""
+        """Handle POST requests by processing a submitted form containing an API key."""
         form = ApiKeyForm(request.POST)
         if form.is_valid():
             api_key = form.cleaned_data['api_key']
-            os.environ['OPENAI_API_KEY'] = api_key  # This sets it for the current process only
+            os.environ['OPENAI_API_KEY'] = api_key
+
             return redirect('landing_page')
+        else:
+            context = self.get_context_data()
+            context['form'] = form
 
-        return render(request, self.template_name, {'form': form})
+            return render(request, self.template_name, context)
 
     def get_context_data(self, **kwargs):
-        """Retrieves and returns the base context data enhanced with the presence check for the API key.
-        Indicates if a prompt for the API key is needed based on its absence."""
+        """Return context data and adds a flag indicating whether an API key is set."""
         context = super().get_context_data(**kwargs)
-        self.request.session.flush()
         api_key = os.getenv('OPENAI_API_KEY')
         context['prompt_for_key'] = not bool(api_key)
 
@@ -59,16 +61,23 @@ class ResetApiKey(RedirectView):
     url = reverse_lazy("landing_page")
 
     def get(self, request, *args, **kwargs):
-        """Handles GET requests by deleting the API key from the environment and redirecting to the landing page."""
-        del os.environ['OPENAI_API_KEY']
+        """Handle GET requests by deleting the API key from the environment and redirecting to the landing page."""
+        try:
+            del os.environ['OPENAI_API_KEY']
+        except KeyError as e:
+            return render(
+                self.request,
+                'error_page.html',
+                {'type': type(e).__name__,'error_traceback': traceback.format_exc()}
+            )
 
         return super().get(request, *args, **kwargs)
 
 
 class DownloadXesView(View):
-    """View for the download of on or more XES files."""
+    """View for the downloading of XES file(s)."""
 
-    def post(self, request, *_args, **_kwargs):
+    def post(self, request, *args, **kwargs):
         """
         Process a POST request to download specified trace types as XES files.
 
@@ -79,7 +88,7 @@ def post(self, request, *_args, **_kwargs):
             return HttpResponse("No file type specified.", status=400)
 
         files_to_download = self.collect_files(request, trace_types)
-        if files_to_download is None:  # Check for None explicitly to handle error scenario
+        if files_to_download is None:
             return HttpResponse("One or more files could not be found.", status=404)
 
         if len(files_to_download) == 1:
@@ -93,7 +102,7 @@ def get_trace_types(request):
 
         return request.POST.getlist("trace_type[]")
 
-    def collect_files(self, request, trace_types):
+    def collect_files(self, request, trace_types: List[str]):
         """Collects file for the specified trace types to download, checking for their existence."""
         files_to_download = []
         for trace_type in trace_types:
@@ -102,7 +111,7 @@ def collect_files(self, request, trace_types):
                 if os.path.exists(file_path):
                     files_to_download.append(file_path)
                 else:
-                    return None  # Return None if any file path is invalid
+                    return None
 
         return files_to_download
 
@@ -113,30 +122,25 @@ def process_trace_type(request, trace_type):
     # pylint: disable=consider-using-with
     @staticmethod
     def single_file_response(file_path):
-        """Prepare a file if there is only a single XES file."""
-        file = open(file_path, "rb")
-        response = FileResponse(file, as_attachment=True)
-        response[
-            "Content-Disposition"
-        ] = f'attachment; filename="{os.path.basename(file_path)}"'
+        """Prepare a single XES file for download."""
+        file_name = os.path.basename(file_path)
+        response = FileResponse(open(file_path, "rb"), as_attachment=True)
+        response["Content-Disposition"] = f'attachment; filename="{file_name}"'
 
         return response
 
     @staticmethod
     def zip_files_response(files_to_download):
-        """Prepare a ZIP file if there are multiple XES files to download."""
-        temp_zip = NamedTemporaryFile(mode="w+b", suffix=".zip", delete=False)
-        zipf = zipfile.ZipFile(temp_zip, "w", zipfile.ZIP_DEFLATED)
-        for file_path in files_to_download:
-            zipf.write(file_path, arcname=os.path.basename(file_path))
-        zipf.close()
-        temp_zip_path = temp_zip.name
-        temp_zip.close()
-
-        file = open(temp_zip_path, "rb")
-        response = FileResponse(file, as_attachment=True)
-        response[
-            "Content-Disposition"
-        ] = 'attachment; filename="downloaded_xes_files.zip"'
+        """Prepare a ZIP file for multiple files to download."""
+        with NamedTemporaryFile(mode="w+b", suffix=".zip", delete=False) as temp_zip:
+            with zipfile.ZipFile(temp_zip, "w", zipfile.ZIP_DEFLATED) as zipf:
+                for file_path in files_to_download:
+                    file_name = os.path.basename(file_path)
+                    zipf.write(file_path, arcname=file_name)
+
+            response = FileResponse(open(temp_zip.name, "rb"), as_attachment=True)
+            response["Content-Disposition"] = 'attachment; filename="downloaded_files.zip"'
+
+        os.remove(temp_zip.name)
 
         return response