Merge pull request #3316 from uktrade/fix/dt-2303-add-table-bugs

Fix/DT-2303: add table bugs

dataworkspace/dataworkspace/apps/datasets/add_table/forms.py

@@ -141,7 +141,7 @@ class AddTableDataTypesForm(GOVUKDesignSystemForm):
         The ID will be an increasing integer, e.g. 1, 2, 3.",
         choices=[("True", "Yes"), ("False", "No")],
         widget=ConditionalSupportTypeRadioWidget(heading="h2", label_size="m", small=False),
-        required=True,
+        required=False,
     )
 
     def __init__(self, *args, **kwargs):

dataworkspace/dataworkspace/apps/datasets/add_table/views.py

@@ -9,8 +9,8 @@
 from django.urls import reverse
 from django.views.generic import DetailView, FormView, TemplateView
 from django.http import HttpResponseRedirect, HttpResponseServerError
+from django.shortcuts import redirect
 from requests import HTTPError
-
 from dataworkspace.apps.core.boto3_client import get_s3_client
 from dataworkspace.apps.core.constants import SCHEMA_POSTGRES_DATA_TYPE_MAP, PostgresDataTypes
 from dataworkspace.apps.core.models import Database
@@ -43,6 +43,12 @@
 class AddTableView(DetailView):
     template_name = "datasets/add_table/about_this_service.html"
 
+    def dispatch(self, request, *args, **kwargs):
+        source = self.get_object()
+        if not source.user_can_add_table(self.request.user):
+            return redirect(reverse("datasets:dataset_detail", args={self.kwargs["pk"]}))
+        return super().dispatch(request, *args, **kwargs)
+
     def get_object(self, queryset=None):
         return find_dataset(self.kwargs["pk"], self.request.user)
 
@@ -407,7 +413,7 @@ def get_context_data(self, **kwargs):
             {
                 "task_name": self.task_name,
                 "next_step": f"{reverse(self.next_step_url_name, args=(self.kwargs['pk'],))}?{urlencode(query_params)}",
-                "failure_url": f"{reverse('datasets:add_table:create-table-failed', args=(self.kwargs['pk'],))}?{urlencode(query_params)}",
+                "failure_url": f"{reverse('datasets:add_table:create-table-failed', args=(self.kwargs['pk'],))}?{urlencode(query_params)}",  # pylint: disable=line-too-long
             }
         )
         return context
@@ -504,6 +510,7 @@ class AddTableSuccessView(BaseAddTableTemplateView):
 
     template_name = "datasets/add_table/confirmation.html"
     step = 5
+    default_download_limit = 5000
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
@@ -515,6 +522,8 @@ def get_context_data(self, **kwargs):
             database=database,
             name=self._get_query_parameters()["table_name"],
             table=self._get_query_parameters()["table_name"],
+            data_grid_download_enabled=True,
+            data_grid_download_limit=self.default_download_limit,
         )
         context["backlink"] = reverse("datasets:dataset_detail", args={self.kwargs["pk"]})
         context["edit_link"] = reverse("datasets:edit_dataset", args={self.kwargs["pk"]})

dataworkspace/dataworkspace/apps/datasets/models.py

@@ -463,6 +463,12 @@ def user_has_access(self, user):
             + tuple(self.data_catalogue_editors.values_list("id", flat=True))
         )
 
+    def user_can_add_table(self, user):
+        return user.id in (
+            self.information_asset_owner_id,
+            self.information_asset_manager_id,
+        ) + tuple(self.data_catalogue_editors.values_list("id", flat=True))
+
     def user_has_bookmarked(self, user):
         return self.datasetbookmark_set.filter(user=user).exists()
 

dataworkspace/dataworkspace/templates/datasets/add_table/data_types.html

@@ -11,6 +11,7 @@
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
+    {% include 'design_system/error_summary.html' with form=form %}
     <h1 class="govuk-heading-xl">Data Types</h1>
     <p class="govuk-body">Data types affect the efficiency of queries. Selecting the correct data type means quicker queries and cheaper data.</p>
     <p class="govuk-body">Using a data type that cannot describe your data may cause your table build to fail. For
@@ -40,7 +41,7 @@ <h2 class="govuk-heading-l">Choose data types for {{table_name }}</h2>
     <p class="govuk-body">Data workspace has estimated an appropriate data type for your columns. You must check the
       data type for each column in the table below and change it if necessary.</p>
   </div>
-  {% include 'design_system/error_summary.html' with form=form %}
+
   <form method='post' novalidate>
     {% csrf_token %}
     {{ form.table_name.as_hidden }}

dataworkspace/dataworkspace/templates/datasets/add_table/upload_csv.html

@@ -15,7 +15,7 @@
         <h1 class="govuk-heading-xl">Upload CSV</h1>
         <h2 class="govuk-heading-l">Before you upload your CSV</h2>
             <p class="govuk-body">Check your CSV against each of the below points. This can help you avoid common issues when the table is being built.</p>
-            <h2 class="govuk-heading-m">Check your files format and name</h2>
+            <h2 class="govuk-heading-m">Check your file's format and name</h2>
             <p class="govuk-body">Check that:</p>
             <ul class="govuk-list govuk-list--bullet">
                 <li>the file has more than one row</li>

dataworkspace/dataworkspace/tests/datasets/add_table/test_views.py

@@ -12,12 +12,54 @@
 from dataworkspace.tests.common import get_http_sso_data
 
 
+@pytest.mark.django_db
+class TestAddTablePagePermissions(TestCase):
+    def test_user_without_permissons_get_redirected(self):
+        user = factories.UserFactory.create(is_superuser=False)
+        client = Client(**get_http_sso_data(user))
+        dataset = factories.MasterDataSetFactory.create()
+
+        response = client.get(
+            reverse("datasets:add_table:add-table", kwargs={"pk": dataset.id}),
+        )
+        assert response.status_code == 302
+        assert response["Location"] == f"/datasets/{dataset.id}"
+
+    def test_iam_user_can_access_page(self):
+        user = factories.UserFactory.create(is_superuser=False)
+        client = Client(**get_http_sso_data(user))
+        dataset = factories.MasterDataSetFactory.create(information_asset_manager=user)
+        response = client.get(
+            reverse("datasets:add_table:add-table", kwargs={"pk": dataset.id}),
+        )
+        assert response.status_code == 200
+
+    def test_iao_user_can_access_page(self):
+        user = factories.UserFactory.create(is_superuser=False)
+        client = Client(**get_http_sso_data(user))
+        dataset = factories.MasterDataSetFactory.create(information_asset_owner=user)
+        response = client.get(
+            reverse("datasets:add_table:add-table", kwargs={"pk": dataset.id}),
+        )
+        assert response.status_code == 200
+
+    def test_editor_user_can_access_page(self):
+        user = factories.UserFactory.create(is_superuser=False)
+        client = Client(**get_http_sso_data(user))
+        dataset = factories.MasterDataSetFactory.create()
+        dataset.data_catalogue_editors.set([user])
+        response = client.get(
+            reverse("datasets:add_table:add-table", kwargs={"pk": dataset.id}),
+        )
+        assert response.status_code == 200
+
+
 @pytest.mark.django_db
 class TestAddTablePage(TestCase):
     def setUp(self):
         self.user = factories.UserFactory.create(is_superuser=False)
         self.client = Client(**get_http_sso_data(self.user))
-        self.dataset = factories.MasterDataSetFactory.create()
+        self.dataset = factories.MasterDataSetFactory.create(information_asset_owner=self.user)
 
     def test_about_service_page(self):
         response = self.client.get(
@@ -777,7 +819,7 @@ def test_confirmation_page(self):
     def test_event_log_has_been_added(self, mock_log_event):
         response = self.client.get(
             reverse("datasets:add_table:add-table-success", kwargs={"pk": self.dataset.id})
-            + f"?filename=allowed_chars-.csv&schema={self.source.schema}&table_name={self.table_name}&"
+            + f"?descriptive_name={self.descriptive_name}&filename=allowed_chars-.csv&schema={self.source.schema}&table_name={self.table_name}&"
             f"execution_date=2021-01-01+01%3A01%3A01",
         )
         mock_log_event.assert_called_with(self.user, event_type=57, related_object=self.dataset)