From 2bc86a58d338d21a48f7f6cf454d2103bda8fb0c Mon Sep 17 00:00:00 2001
From: Yohanna Lisnichuk <yohanitalisnichuk@gmail.com>
Date: Fri, 3 Nov 2023 14:44:24 -0300
Subject: [PATCH 1/6] feat: Improve exporter URLs

---
 data_registry/templates/includes/files.html |  4 ++--
 exporter/urls.py                            |  2 +-
 exporter/views.py                           | 23 ++++++++++++---------
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/data_registry/templates/includes/files.html b/data_registry/templates/includes/files.html
index 8859771b..dbd26438 100644
--- a/data_registry/templates/includes/files.html
+++ b/data_registry/templates/includes/files.html
@@ -4,13 +4,13 @@
 <ul>
 {% if files.full %}
     <li>
-        <a href="{% url 'download-export' %}?spider={{ collection.source_id }}&job_id={{ job.id }}&full=true&suffix={{ suffix }}" rel="nofollow" download>{% translate "All time" %}</a>
+        <a href="{% url 'download' collection.id %}?name=full.{{ suffix }}" rel="nofollow" download>{% translate "All time" %}</a>
         <span class="text-muted small">{{ files.full|humanfilesize }}</span>
     </li>
 {% endif %}
 {% for file in files.by_year|dictsortreversed:"year" %}
     <li>
-        <a href="{% url 'download-export' %}?spider={{ collection.source_id }}&job_id={{ job.id }}&year={{ file.year }}&suffix={{ suffix }}" rel="nofollow" download>{{ file.year }}</a>
+        <a href="{% url 'download' collection.id %}?name={{ file.year }}.{{ suffix }}" rel="nofollow" download>{{ file.year }}</a>
         <span class="text-muted small">{{ file.size|humanfilesize }}</span>
     </li>
 {% empty %}
diff --git a/exporter/urls.py b/exporter/urls.py
index 96e067fd..39213bf2 100644
--- a/exporter/urls.py
+++ b/exporter/urls.py
@@ -3,5 +3,5 @@
 from exporter import views
 
 urlpatterns = [
-    path("api/download_export", views.download_export, name="download-export"),
+    path("publication/<int:id>/download", views.download_export, name="download"),
 ]
diff --git a/exporter/views.py b/exporter/views.py
index 4595a9b1..16e20799 100644
--- a/exporter/views.py
+++ b/exporter/views.py
@@ -1,25 +1,28 @@
 from django.http import FileResponse
 from django.http.response import HttpResponseBadRequest, HttpResponseNotFound
+from django.shortcuts import get_object_or_404
 
+from data_registry.models import Collection
 from exporter.util import Export, TaskStatus
 
 
-def download_export(request):
+def download_export(request, id):
     """
     Returns an exported file as a FileResponse object.
     """
-    job_id = int(request.GET.get("job_id") or 0)  # guard against path traversal
-    full = request.GET.get("full")
-    year = int(request.GET.get("year") or 0)  # guard against path traversal
-    suffix = request.GET.get("suffix")
-    spider = request.GET.get("spider")
+    name = request.GET.get("name")
 
     # Guard against path traversal.
-    if suffix not in ("jsonl.gz", "csv.tar.gz", "xlsx"):
+    if not name.endswith(("jsonl.gz", "csv.tar.gz", "xlsx")):
         return HttpResponseBadRequest("Suffix not recognized")
 
-    stem = "full" if full else year
-    export = Export(job_id, basename=f"{stem}.{suffix}")
+    collection = get_object_or_404(Collection, id=id, public=True)
+
+    active_job = collection.job.filter(active=True).first()
+    if not active_job:
+        return HttpResponseNotFound("No active job was found for this collection")
+
+    export = Export(active_job.id, basename=f"{name}")
 
     if export.status != TaskStatus.COMPLETED:
         return HttpResponseNotFound("File not found")
@@ -27,7 +30,7 @@ def download_export(request):
     return FileResponse(
         export.path.open("rb"),
         as_attachment=True,
-        filename=f"{spider}_{stem}.{suffix}",
+        filename=f"{collection.source_id}_{name}",
         # Set Content-Encoding to skip GZipMiddleware. (ContentEncodingMiddleware removes the empty header.)
         # https://docs.djangoproject.com/en/4.2/ref/middleware/#module-django.middleware.gzip
         headers={"Content-Encoding": ""},

From 462d9b31f3a256447ad7a12db6038cb998b7ad7f Mon Sep 17 00:00:00 2001
From: Yohanna Lisnichuk <yohanitalisnichuk@gmail.com>
Date: Fri, 3 Nov 2023 14:52:32 -0300
Subject: [PATCH 2/6] fix tests

---
 exporter/views.py                       |   5 ++-
 tests/data_registry/test_views.py       |   2 +-
 tests/exporter/test_views.py            |  44 +++++++++++++++---------
 tests/fixtures/{1 => 2}/2000.csv.tar.gz | Bin
 tests/fixtures/{1 => 2}/2000.jsonl.gz   | Bin
 tests/fixtures/{1 => 2}/2000.xlsx       | Bin
 6 files changed, 30 insertions(+), 21 deletions(-)
 rename tests/fixtures/{1 => 2}/2000.csv.tar.gz (100%)
 rename tests/fixtures/{1 => 2}/2000.jsonl.gz (100%)
 rename tests/fixtures/{1 => 2}/2000.xlsx (100%)

diff --git a/exporter/views.py b/exporter/views.py
index 16e20799..d8b8d12c 100644
--- a/exporter/views.py
+++ b/exporter/views.py
@@ -2,7 +2,7 @@
 from django.http.response import HttpResponseBadRequest, HttpResponseNotFound
 from django.shortcuts import get_object_or_404
 
-from data_registry.models import Collection
+from data_registry.util import collection_queryset
 from exporter.util import Export, TaskStatus
 
 
@@ -16,14 +16,13 @@ def download_export(request, id):
     if not name.endswith(("jsonl.gz", "csv.tar.gz", "xlsx")):
         return HttpResponseBadRequest("Suffix not recognized")
 
-    collection = get_object_or_404(Collection, id=id, public=True)
+    collection = get_object_or_404(collection_queryset(request), id=id)
 
     active_job = collection.job.filter(active=True).first()
     if not active_job:
         return HttpResponseNotFound("No active job was found for this collection")
 
     export = Export(active_job.id, basename=f"{name}")
-
     if export.status != TaskStatus.COMPLETED:
         return HttpResponseNotFound("File not found")
 
diff --git a/tests/data_registry/test_views.py b/tests/data_registry/test_views.py
index 2709d41a..d156b809 100644
--- a/tests/data_registry/test_views.py
+++ b/tests/data_registry/test_views.py
@@ -21,7 +21,7 @@ def setUpTestData(cls):
     @patch("exporter.util.Export.get_files")
     def test_detail(self, get_files):
         get_files.return_value = {"jsonl": {"by_year": [{"year": 2022, "size": 1}]}}
-        url = f"/api/download_export?spider=paraguay_dncp_records&job_id={self.job.id}&year=2022&suffix=jsonl.gz"
+        url = f"/publication/{self.collection.id}/download?name=2022.jsonl.gz"
 
         with self.assertNumQueries(2):
             response = Client().get(f"/en/publication/{self.collection.id}")
diff --git a/tests/exporter/test_views.py b/tests/exporter/test_views.py
index d37e26e0..3266f983 100644
--- a/tests/exporter/test_views.py
+++ b/tests/exporter/test_views.py
@@ -3,36 +3,47 @@
 
 from django.test import Client, TestCase, override_settings
 
+from data_registry.models import Collection
+
 
 @override_settings(EXPORTER_DIR=os.path.join("tests", "fixtures"))
 class ViewsTests(TestCase):
+    @classmethod
+    def setUp(cls):
+        cls.collection = Collection.objects.create(
+            id=2,
+            title="Dirección Nacional de Contrataciones Públicas (DNCP)",
+            source_id="abc",
+            public=True,
+        )
+        cls.job = cls.collection.job.create(
+            active=True,
+        )
+
     def test_download_export_invalid_suffix(self):
         with self.assertNumQueries(0):
-            response = Client().get("/api/download_export?suffix=invalid")
+            response = Client().get("/publication/2/download?name=invalid")
 
         self.assertEqual(response.status_code, 400)
         self.assertEqual(response.content, b"Suffix not recognized")
 
     def test_download_export_empty_parameter(self):
-        for parameter in ("job_id", "year"):
-            with self.subTest(parameter=parameter):
-                with self.assertNumQueries(0):
-                    response = Client().get(f"/api/download_export?suffix=jsonl.gz&{parameter}=")
+        with self.assertNumQueries(0):
+            response = Client().get("/publication/2/download?name=")
 
-                self.assertEqual(response.status_code, 404)
-                self.assertEqual(response.content, b"File not found")
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.content, b"Suffix not recognized")
 
     def test_download_export_waiting(self):
-        with self.assertNumQueries(0):
-            response = Client().get("/api/download_export?suffix=jsonl.gz&year=2000&job_id=0")
+        with self.assertNumQueries(1):
+            response = Client().get("/publication/1/download?name=2000.jsonl.gz")
 
         self.assertEqual(response.status_code, 404)
-        self.assertEqual(response.content, b"File not found")
 
     @patch("exporter.util.Export.lockfile", new_callable=PropertyMock)
     def test_download_export_running(self, exists):
-        with self.assertNumQueries(0):
-            response = Client().get("/api/download_export?suffix=jsonl.gz&year=2000&job_id=1")
+        with self.assertNumQueries(2):
+            response = Client().get("/publication/2/download?name=2000.jsonl.gz")
 
         self.assertEqual(response.status_code, 404)
         self.assertEqual(response.content, b"File not found")
@@ -44,12 +55,11 @@ def test_download_export_completed(self):
             ("xlsx", "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"),
         ):
             with self.subTest(suffix=suffix):
-                with self.assertNumQueries(0):
+                with self.assertNumQueries(2):
                     response = Client().get(
-                        f"/api/download_export?suffix={suffix}&year=2000&job_id=1&spider=abc",
+                        f"/publication/2/download?name=2000.{suffix}",
                         HTTP_ACCEPT_ENCODING="gzip",
                     )
-
                 self.assertEqual(response.status_code, 200)
                 response.headers.pop("Content-Length")
                 self.assertDictEqual(
@@ -60,10 +70,10 @@ def test_download_export_completed(self):
                         "Content-Type": content_type,
                         "Cross-Origin-Opener-Policy": "same-origin",
                         "Referrer-Policy": "same-origin",
-                        "Vary": "Accept-Language",
+                        "Vary": "Accept-Language, Cookie",
                         "X-Content-Type-Options": "nosniff",
                         "X-Frame-Options": "DENY",
                     },
                 )
-                with open(os.path.join("tests", "fixtures", "1", f"2000.{suffix}"), "rb") as f:
+                with open(os.path.join("tests", "fixtures", "2", f"2000.{suffix}"), "rb") as f:
                     self.assertEqual(b"".join(response.streaming_content), f.read())
diff --git a/tests/fixtures/1/2000.csv.tar.gz b/tests/fixtures/2/2000.csv.tar.gz
similarity index 100%
rename from tests/fixtures/1/2000.csv.tar.gz
rename to tests/fixtures/2/2000.csv.tar.gz
diff --git a/tests/fixtures/1/2000.jsonl.gz b/tests/fixtures/2/2000.jsonl.gz
similarity index 100%
rename from tests/fixtures/1/2000.jsonl.gz
rename to tests/fixtures/2/2000.jsonl.gz
diff --git a/tests/fixtures/1/2000.xlsx b/tests/fixtures/2/2000.xlsx
similarity index 100%
rename from tests/fixtures/1/2000.xlsx
rename to tests/fixtures/2/2000.xlsx

From 78a1836404c1a1f2f72af11640a7c3690367d02b Mon Sep 17 00:00:00 2001
From: Yohanna Lisnichuk <yohanitalisnichuk@gmail.com>
Date: Thu, 9 Nov 2023 13:59:55 -0300
Subject: [PATCH 3/6] Apply suggestions from code review

Co-authored-by: James McKinney <26463+jpmckinney@users.noreply.github.com>
---
 exporter/views.py            | 4 ++--
 tests/exporter/test_views.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/exporter/views.py b/exporter/views.py
index d8b8d12c..45496e83 100644
--- a/exporter/views.py
+++ b/exporter/views.py
@@ -20,9 +20,9 @@ def download_export(request, id):
 
     active_job = collection.job.filter(active=True).first()
     if not active_job:
-        return HttpResponseNotFound("No active job was found for this collection")
+        return HttpResponseNotFound("This OCDS dataset is not available for download")
 
-    export = Export(active_job.id, basename=f"{name}")
+    export = Export(active_job.id, basename=name)
     if export.status != TaskStatus.COMPLETED:
         return HttpResponseNotFound("File not found")
 
diff --git a/tests/exporter/test_views.py b/tests/exporter/test_views.py
index 3266f983..5d8af146 100644
--- a/tests/exporter/test_views.py
+++ b/tests/exporter/test_views.py
@@ -25,14 +25,14 @@ def test_download_export_invalid_suffix(self):
             response = Client().get("/publication/2/download?name=invalid")
 
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(response.content, b"Suffix not recognized")
+        self.assertEqual(response.content, b"The name query string parameter is invalid")
 
     def test_download_export_empty_parameter(self):
         with self.assertNumQueries(0):
             response = Client().get("/publication/2/download?name=")
 
         self.assertEqual(response.status_code, 400)
-        self.assertEqual(response.content, b"Suffix not recognized")
+        self.assertEqual(response.content, b"The name query string parameter is invalid")
 
     def test_download_export_waiting(self):
         with self.assertNumQueries(1):

From 128dfc550559d5c507628dd58191eb4f27827a7f Mon Sep 17 00:00:00 2001
From: Yohanna Lisnichuk <yohanitalisnichuk@gmail.com>
Date: Thu, 9 Nov 2023 14:01:40 -0300
Subject: [PATCH 4/6] fix: apply suggestions from code review

---
 exporter/views.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/exporter/views.py b/exporter/views.py
index 45496e83..b40a8448 100644
--- a/exporter/views.py
+++ b/exporter/views.py
@@ -1,3 +1,5 @@
+import re
+
 from django.http import FileResponse
 from django.http.response import HttpResponseBadRequest, HttpResponseNotFound
 from django.shortcuts import get_object_or_404
@@ -5,6 +7,8 @@
 from data_registry.util import collection_queryset
 from exporter.util import Export, TaskStatus
 
+EXPORT_PATTERN = re.compile(r"\A(full|\d{4})\.(jsonl\.gz|csv\.tar\.gz|xlsx)\Z")
+
 
 def download_export(request, id):
     """
@@ -13,8 +17,8 @@ def download_export(request, id):
     name = request.GET.get("name")
 
     # Guard against path traversal.
-    if not name.endswith(("jsonl.gz", "csv.tar.gz", "xlsx")):
-        return HttpResponseBadRequest("Suffix not recognized")
+    if not EXPORT_PATTERN.match(name):
+        return HttpResponseBadRequest("The name query string parameter is invalid")
 
     collection = get_object_or_404(collection_queryset(request), id=id)
 

From af64b798e75b0287ac64cd755b4a2a1404d7736a Mon Sep 17 00:00:00 2001
From: Yohanna Lisnichuk <yohanitalisnichuk@gmail.com>
Date: Thu, 9 Nov 2023 16:14:26 -0300
Subject: [PATCH 5/6] feat: merge exporter and data_registry apps

---
 core/settings.py                          |  3 +-
 core/urls.py                              |  1 -
 {exporter => data_registry}/middleware.py |  0
 data_registry/urls.py                     |  1 +
 data_registry/views.py                    | 36 ++++++++++++++++++--
 exporter/apps.py                          |  5 ---
 exporter/urls.py                          |  7 ----
 exporter/views.py                         | 40 -----------------------
 tests/data_registry/test_views.py         |  2 +-
 tests/exporter/test_views.py              | 33 ++++++++++++++-----
 10 files changed, 62 insertions(+), 66 deletions(-)
 rename {exporter => data_registry}/middleware.py (100%)
 delete mode 100644 exporter/apps.py
 delete mode 100644 exporter/urls.py
 delete mode 100644 exporter/views.py

diff --git a/core/settings.py b/core/settings.py
index d639b0eb..6a6f7f17 100644
--- a/core/settings.py
+++ b/core/settings.py
@@ -54,13 +54,12 @@
     "django.contrib.humanize",
     "data_registry",
     "markdownx",
-    "exporter",
 ]
 
 MIDDLEWARE = [
     "django.middleware.security.SecurityMiddleware",
     # Add before GZipMiddleware to modify its response.
-    "exporter.middleware.ContentEncodingMiddleware",
+    "data_registry.middleware.ContentEncodingMiddleware",
     # This site is not affected by BREACH.
     # https://docs.djangoproject.com/en/4.2/ref/middleware/#django.middleware.gzip.GZipMiddleware
     "django.middleware.gzip.GZipMiddleware",
diff --git a/core/urls.py b/core/urls.py
index 73a61136..8ba19bde 100644
--- a/core/urls.py
+++ b/core/urls.py
@@ -4,7 +4,6 @@
 
 urlpatterns = [
     path("", include("data_registry.urls"), name="data-registry"),
-    path("", include("exporter.urls"), name="exporter"),
     path("admin/", admin.site.urls),
     path("markdownx/", include("markdownx.urls")),
 ]
diff --git a/exporter/middleware.py b/data_registry/middleware.py
similarity index 100%
rename from exporter/middleware.py
rename to data_registry/middleware.py
diff --git a/data_registry/urls.py b/data_registry/urls.py
index 7e9b21c1..2d6ac3de 100644
--- a/data_registry/urls.py
+++ b/data_registry/urls.py
@@ -6,6 +6,7 @@
     path("", views.index, name="index"),
     path("search/", views.search, name="search"),
     path("publication/<int:id>", views.detail, name="detail"),
+    path("publication/<int:id>/download", views.download_export, name="download"),
     # https://code.djangoproject.com/ticket/26556
     path("i18n/setlang/", i18n.set_language, name="set-language"),
     # Uncomment after re-integrating Spoonbill.
diff --git a/data_registry/views.py b/data_registry/views.py
index b5bbe736..2d56a6d8 100644
--- a/data_registry/views.py
+++ b/data_registry/views.py
@@ -1,4 +1,5 @@
 import logging
+import re
 import string
 from collections import defaultdict
 from datetime import date, datetime, timedelta
@@ -11,20 +12,21 @@
 from django.contrib.postgres.aggregates import ArrayAgg
 from django.db.models import Count, OuterRef, Q, Subquery
 from django.db.models.functions import Substr
-from django.http.response import HttpResponse, JsonResponse
+from django.http.response import FileResponse, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound, JsonResponse
 from django.shortcuts import get_object_or_404, redirect, render
 from django.utils.translation import get_language, get_language_from_request
 from django.utils.translation import gettext as _
 
 from data_registry.models import Collection, Job
 from data_registry.util import collection_queryset
-from exporter.util import Export
+from exporter.util import Export, TaskStatus
 
 logger = logging.getLogger(__name__)
 
 alphabets = defaultdict(lambda: string.ascii_uppercase)
 # https://en.wikipedia.org/wiki/Cyrillic_script_in_Unicode#Basic_Cyrillic_alphabet
 alphabets["ru"] = "АБВГДЕЁЖЗИЙКЛМНОПРСТУФХЦЧШЩЪЫЬЭЮЯ"
+EXPORT_PATTERN = re.compile(r"\A(full|\d{4})\.(jsonl\.gz|csv\.tar\.gz|xlsx)\Z")
 
 
 def index(request):
@@ -174,6 +176,36 @@ def spiders(request):
     return JsonResponse(json.get("spiders"), safe=False)
 
 
+def download_export(request, id):
+    """
+    Returns an exported file as a FileResponse object.
+    """
+    name = request.GET.get("name")
+
+    # Guard against path traversal.
+    if not EXPORT_PATTERN.match(name):
+        return HttpResponseBadRequest("The name query string parameter is invalid")
+
+    collection = get_object_or_404(collection_queryset(request), id=id)
+
+    active_job = collection.job.filter(active=True).first()
+    if not active_job:
+        return HttpResponseNotFound("This OCDS dataset is not available for download")
+
+    export = Export(active_job.id, basename=name)
+    if export.status != TaskStatus.COMPLETED:
+        return HttpResponseNotFound("File not found")
+
+    return FileResponse(
+        export.path.open("rb"),
+        as_attachment=True,
+        filename=f"{collection.source_id}_{name}",
+        # Set Content-Encoding to skip GZipMiddleware. (ContentEncodingMiddleware removes the empty header.)
+        # https://docs.djangoproject.com/en/4.2/ref/middleware/#module-django.middleware.gzip
+        headers={"Content-Encoding": ""},
+    )
+
+
 def excel_data(request, job_id, job_range=None):
     job = Job.objects.get(id=job_id)
     export = Export(job_id)
diff --git a/exporter/apps.py b/exporter/apps.py
deleted file mode 100644
index e8fed610..00000000
--- a/exporter/apps.py
+++ /dev/null
@@ -1,5 +0,0 @@
-from django.apps import AppConfig
-
-
-class ExporterConfig(AppConfig):
-    name = "exporter"
diff --git a/exporter/urls.py b/exporter/urls.py
deleted file mode 100644
index 39213bf2..00000000
--- a/exporter/urls.py
+++ /dev/null
@@ -1,7 +0,0 @@
-from django.urls import path
-
-from exporter import views
-
-urlpatterns = [
-    path("publication/<int:id>/download", views.download_export, name="download"),
-]
diff --git a/exporter/views.py b/exporter/views.py
deleted file mode 100644
index b40a8448..00000000
--- a/exporter/views.py
+++ /dev/null
@@ -1,40 +0,0 @@
-import re
-
-from django.http import FileResponse
-from django.http.response import HttpResponseBadRequest, HttpResponseNotFound
-from django.shortcuts import get_object_or_404
-
-from data_registry.util import collection_queryset
-from exporter.util import Export, TaskStatus
-
-EXPORT_PATTERN = re.compile(r"\A(full|\d{4})\.(jsonl\.gz|csv\.tar\.gz|xlsx)\Z")
-
-
-def download_export(request, id):
-    """
-    Returns an exported file as a FileResponse object.
-    """
-    name = request.GET.get("name")
-
-    # Guard against path traversal.
-    if not EXPORT_PATTERN.match(name):
-        return HttpResponseBadRequest("The name query string parameter is invalid")
-
-    collection = get_object_or_404(collection_queryset(request), id=id)
-
-    active_job = collection.job.filter(active=True).first()
-    if not active_job:
-        return HttpResponseNotFound("This OCDS dataset is not available for download")
-
-    export = Export(active_job.id, basename=name)
-    if export.status != TaskStatus.COMPLETED:
-        return HttpResponseNotFound("File not found")
-
-    return FileResponse(
-        export.path.open("rb"),
-        as_attachment=True,
-        filename=f"{collection.source_id}_{name}",
-        # Set Content-Encoding to skip GZipMiddleware. (ContentEncodingMiddleware removes the empty header.)
-        # https://docs.djangoproject.com/en/4.2/ref/middleware/#module-django.middleware.gzip
-        headers={"Content-Encoding": ""},
-    )
diff --git a/tests/data_registry/test_views.py b/tests/data_registry/test_views.py
index d156b809..6112406a 100644
--- a/tests/data_registry/test_views.py
+++ b/tests/data_registry/test_views.py
@@ -21,7 +21,7 @@ def setUpTestData(cls):
     @patch("exporter.util.Export.get_files")
     def test_detail(self, get_files):
         get_files.return_value = {"jsonl": {"by_year": [{"year": 2022, "size": 1}]}}
-        url = f"/publication/{self.collection.id}/download?name=2022.jsonl.gz"
+        url = f"/en/publication/{self.collection.id}/download?name=2022.jsonl.gz"
 
         with self.assertNumQueries(2):
             response = Client().get(f"/en/publication/{self.collection.id}")
diff --git a/tests/exporter/test_views.py b/tests/exporter/test_views.py
index 5d8af146..4b3bd086 100644
--- a/tests/exporter/test_views.py
+++ b/tests/exporter/test_views.py
@@ -18,32 +18,49 @@ def setUp(cls):
         )
         cls.job = cls.collection.job.create(
             active=True,
+            id=2,
+        )
+        cls.collection_no_job = Collection.objects.create(
+            id=3,
+            title="Test",
+            source_id="abc",
+            public=True,
         )
+        cls.collection_no_job.job.create(
+            active=True,
+            id=4,
+        )
+
+    def test_collection_not_found(self):
+        with self.assertNumQueries(1):
+            response = Client().get("/en/publication/10/download?name=2000.jsonl.gz")
+
+        self.assertEqual(response.status_code, 404)
 
     def test_download_export_invalid_suffix(self):
         with self.assertNumQueries(0):
-            response = Client().get("/publication/2/download?name=invalid")
+            response = Client().get(f"/en/publication/{self.collection.id}/download?name=invalid")
 
         self.assertEqual(response.status_code, 400)
         self.assertEqual(response.content, b"The name query string parameter is invalid")
 
     def test_download_export_empty_parameter(self):
         with self.assertNumQueries(0):
-            response = Client().get("/publication/2/download?name=")
+            response = Client().get(f"/en/publication/{self.collection.id}/download?name=")
 
         self.assertEqual(response.status_code, 400)
         self.assertEqual(response.content, b"The name query string parameter is invalid")
 
     def test_download_export_waiting(self):
-        with self.assertNumQueries(1):
-            response = Client().get("/publication/1/download?name=2000.jsonl.gz")
-
+        with self.assertNumQueries(2):
+            response = Client().get(f"/en/publication/{self.collection_no_job.id}/download?name=2000.jsonl.gz")
         self.assertEqual(response.status_code, 404)
+        self.assertEqual(response.content, b"File not found")
 
     @patch("exporter.util.Export.lockfile", new_callable=PropertyMock)
     def test_download_export_running(self, exists):
         with self.assertNumQueries(2):
-            response = Client().get("/publication/2/download?name=2000.jsonl.gz")
+            response = Client().get(f"/en/publication/{self.collection.id}/download?name=2000.jsonl.gz")
 
         self.assertEqual(response.status_code, 404)
         self.assertEqual(response.content, b"File not found")
@@ -57,7 +74,7 @@ def test_download_export_completed(self):
             with self.subTest(suffix=suffix):
                 with self.assertNumQueries(2):
                     response = Client().get(
-                        f"/publication/2/download?name=2000.{suffix}",
+                        f"/en/publication/{self.collection.id}/download?name=2000.{suffix}",
                         HTTP_ACCEPT_ENCODING="gzip",
                     )
                 self.assertEqual(response.status_code, 200)
@@ -70,7 +87,7 @@ def test_download_export_completed(self):
                         "Content-Type": content_type,
                         "Cross-Origin-Opener-Policy": "same-origin",
                         "Referrer-Policy": "same-origin",
-                        "Vary": "Accept-Language, Cookie",
+                        "Vary": "Cookie",
                         "X-Content-Type-Options": "nosniff",
                         "X-Frame-Options": "DENY",
                     },

From 005d56809643b060b2e07461343ef4b569f5306a Mon Sep 17 00:00:00 2001
From: Yohanna Lisnichuk <yohanitalisnichuk@gmail.com>
Date: Fri, 10 Nov 2023 10:05:00 -0300
Subject: [PATCH 6/6] Apply suggestions from code review

Co-authored-by: James McKinney <26463+jpmckinney@users.noreply.github.com>
---
 data_registry/views.py | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/data_registry/views.py b/data_registry/views.py
index 2d56a6d8..b991ca7d 100644
--- a/data_registry/views.py
+++ b/data_registry/views.py
@@ -177,9 +177,6 @@ def spiders(request):
 
 
 def download_export(request, id):
-    """
-    Returns an exported file as a FileResponse object.
-    """
     name = request.GET.get("name")
 
     # Guard against path traversal.
@@ -187,10 +184,7 @@ def download_export(request, id):
         return HttpResponseBadRequest("The name query string parameter is invalid")
 
     collection = get_object_or_404(collection_queryset(request), id=id)
-
-    active_job = collection.job.filter(active=True).first()
-    if not active_job:
-        return HttpResponseNotFound("This OCDS dataset is not available for download")
+    active_job = get_object_or_404(collection.job, active=True)
 
     export = Export(active_job.id, basename=name)
     if export.status != TaskStatus.COMPLETED: