Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/DT-1803-grant-access-to-dataset #2993

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions dataworkspace/dataworkspace/apps/datasets/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,23 @@ def save_model(self, request, obj, form, change):
form.cleaned_data.get("authorized_users", get_user_model().objects.none())
)

if form.cleaned_data["request_approvers"]:
request_approvers_emails = list(form.cleaned_data["request_approvers"])

existing_data_catalogue_editors_emails = (
[item.email for item in form.cleaned_data["data_catalogue_editors"]]
if form.cleaned_data["data_catalogue_editors"]
else []
)
combined_emails = set().union(
request_approvers_emails, existing_data_catalogue_editors_emails
)

valid_request_approver_users = get_user_model().objects.filter(
email__in=combined_emails
)
form.cleaned_data["data_catalogue_editors"] = valid_request_approver_users

added_users = (
form.cleaned_data["data_catalogue_editors"].difference(
obj.data_catalogue_editors.all()
Expand Down
4 changes: 4 additions & 0 deletions dataworkspace/dataworkspace/apps/datasets/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,10 @@ def remove_authorised_editor(request, pk, user_id):
user = get_user_model().objects.get(id=user_id)

dataset.data_catalogue_editors.remove(user)
if dataset.request_approvers and user.email in dataset.request_approvers:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking until the SSO issue is resolved

dataset.request_approvers.remove(user.email)
dataset.save()

log_event(
request.user,
EventLog.TYPE_DATA_CATALOGUE_EDITOR_REMOVED,
Expand Down
84 changes: 83 additions & 1 deletion dataworkspace/dataworkspace/tests/datasets/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
from dataworkspace.apps.eventlog.models import EventLog
from dataworkspace.apps.your_files.models import UploadedTable
from dataworkspace.tests import factories
from dataworkspace.tests.common import get_http_sso_data, MatchUnorderedMembers
from dataworkspace.tests.common import BaseTestCase, get_http_sso_data, MatchUnorderedMembers
from dataworkspace.tests.factories import (
VisualisationCatalogueItemFactory,
UserFactory,
Expand Down Expand Up @@ -4879,3 +4879,85 @@ def test_master_dataset_detail_page_shows_pipeline_failures(client, metadata_db)
len([x for x in response.context["master_datasets_info"] if x.pipeline_last_run_succeeded])
== 1
)


class TestRemoveAuthorisedEditor(BaseTestCase):
@mock.patch("dataworkspace.apps.datasets.views.log_event")
def test_user_is_removed_from_data_catalogue_editors_when_request_approvers_missing(
self, mock_log_event
):
ds = factories.DataSetFactory(published=True)
remaining_user = UserFactory()

ds.data_catalogue_editors.set([self.user.id, remaining_user.id])

self._authenticated_get(
reverse(
"datasets:remove_authorised_editor",
args=(
ds.id,
self.user.id,
),
)
)

mock_log_event.assert_called()
updated_ds = DataSet.objects.filter(pk=ds.id).first()

assert updated_ds.data_catalogue_editors.count() == 1
assert updated_ds.data_catalogue_editors.filter(pk=self.user.id).exists() is False

@mock.patch("dataworkspace.apps.datasets.views.log_event")
def test_user_is_removed_from_data_catalogue_editors_only_when_not_present_in_request_approvers(
self, mock_log_event
):
remaining_user = UserFactory()
ds = factories.DataSetFactory(published=True, request_approvers=[remaining_user.email])

ds.data_catalogue_editors.set([self.user.id, remaining_user.id])

self._authenticated_get(
reverse(
"datasets:remove_authorised_editor",
args=(
ds.id,
self.user.id,
),
)
)

updated_ds = DataSet.objects.filter(pk=ds.id).first()

assert updated_ds.data_catalogue_editors.count() == 1
assert updated_ds.data_catalogue_editors.filter(pk=self.user.id).exists() is False

assert updated_ds.request_approvers == [remaining_user.email]

@mock.patch("dataworkspace.apps.datasets.views.log_event")
def test_user_is_removed_from_data_catalogue_editors_and_request_approvers_when_present(
self, mock_log_event
):
remaining_user = UserFactory()
ds = factories.DataSetFactory(
published=True, request_approvers=[self.user.email, remaining_user.email]
)

ds.data_catalogue_editors.set([self.user.id, remaining_user.id])

self._authenticated_get(
reverse(
"datasets:remove_authorised_editor",
args=(
ds.id,
self.user.id,
),
)
)

mock_log_event.assert_called()
updated_ds = DataSet.objects.filter(pk=ds.id).first()

assert updated_ds.data_catalogue_editors.count() == 1
assert updated_ds.data_catalogue_editors.filter(pk=self.user.id).exists() is False

assert updated_ds.request_approvers == [remaining_user.email]
142 changes: 142 additions & 0 deletions dataworkspace/dataworkspace/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import inspect
import sys
import mock
import factory

from botocore.exceptions import ClientError

Expand Down Expand Up @@ -2552,6 +2553,37 @@ def test_source_link_upload(self, mock_client):


class TestDatasetAdmin(BaseAdminTestCase):
def _valid_dataset(self, dataset, user):
return {
"published": dataset.published,
"name": dataset.name,
"slug": dataset.slug,
"short_description": "test short description",
"description": LONG_DATASET_DESCRIPTION,
"information_asset_owner": str(user.id),
"information_asset_manager": str(user.id),
"enquiries_contact": str(user.id),
"type": 2,
"user_access_type": UserAccessType.OPEN,
"sourcelink_set-TOTAL_FORMS": "0",
"sourcelink_set-INITIAL_FORMS": "0",
"sourcelink_set-MIN_NUM_FORMS": "0",
"sourcelink_set-MAX_NUM_FORMS": "1000",
"sourceview_set-TOTAL_FORMS": "0",
"sourceview_set-INITIAL_FORMS": "0",
"sourceview_set-MIN_NUM_FORMS": "0",
"sourceview_set-MAX_NUM_FORMS": "1000",
"customdatasetquery_set-TOTAL_FORMS": "0",
"customdatasetquery_set-INITIAL_FORMS": "0",
"customdatasetquery_set-MIN_NUM_FORMS": "0",
"customdatasetquery_set-MAX_NUM_FORMS": "1000",
"_continue": "Save and continue editing",
"charts-TOTAL_FORMS": "0",
"charts-INITIAL_FORMS": "0",
"charts-MIN_NUM_FORMS": "0",
"charts-MAX_NUM_FORMS": "1000",
}

def test_update_dataset_with_description_not_long_enough(self):
dataset = factories.DataSetFactory.create()
response = self._authenticated_post(
Expand Down Expand Up @@ -2911,6 +2943,116 @@ def test_delete_local_source_link(self, mock_client):
Bucket=settings.AWS_UPLOADS_BUCKET, Key="s3://sourcelink/a-file.txt"
)

@mock.patch("dataworkspace.apps.core.boto3_client.boto3.client")
def test_request_approvers_with_unknown_email_address_not_added_to_data_catalogue_editors(
self, mock_client
):
dataset = factories.DataSetFactory()
user = factories.UserFactory()
data = self._valid_dataset(dataset, user)
data["request_approvers"] = ["not_real"]

self._authenticated_post(
reverse("admin:datasets_datacutdataset_change", args=(dataset.id,)),
data,
)

reloaded_dataset = DataSet.objects.filter(pk=dataset.id).first()
assert reloaded_dataset.data_catalogue_editors.count() == 0

@mock.patch("dataworkspace.apps.core.boto3_client.boto3.client")
def test_request_approvers_with_new_email_address_added_to_data_catalogue_editors(
self, mock_client
):
dataset = factories.DataSetFactory()
user = factories.UserFactory()
data = self._valid_dataset(dataset, user)
# data['data_catalogue_editors'] = [user.id]

self._authenticated_post(
reverse("admin:datasets_datacutdataset_change", args=(dataset.id,)),
data,
)

data["request_approvers"] = [user.email]

self._authenticated_post(
reverse("admin:datasets_datacutdataset_change", args=(dataset.id,)),
data,
)

reloaded_dataset = DataSet.objects.filter(pk=dataset.id).first()
assert reloaded_dataset.data_catalogue_editors.count() == 1

@mock.patch("dataworkspace.apps.core.boto3_client.boto3.client")
def test_request_approvers_with_existing_email_address_in_data_catalogue_editors_not_added_again_to_data_catalogue_editors( # pylint: disable=line-too-long
self, mock_client
):
dataset = factories.DataSetFactory()
user = factories.UserFactory()
data = self._valid_dataset(dataset, user)
data["data_catalogue_editors"] = [user.id]

self._authenticated_post(
reverse("admin:datasets_datacutdataset_change", args=(dataset.id,)),
data,
)

data["request_approvers"] = [user.email]

self._authenticated_post(
reverse("admin:datasets_datacutdataset_change", args=(dataset.id,)),
data,
)

reloaded_dataset = DataSet.objects.filter(pk=dataset.id).first()
assert reloaded_dataset.data_catalogue_editors.count() == 1

@mock.patch("dataworkspace.apps.core.boto3_client.boto3.client")
def test_request_approvers_and_data_catalogue_editors_containing_same_email_only_added_once_to_data_catalogue_editors(
self, mock_client
):
dataset = factories.DataSetFactory()
user = factories.UserFactory()
data = self._valid_dataset(dataset, user)
data["data_catalogue_editors"] = [user.id]
data["request_approvers"] = [user.email]

self._authenticated_post(
reverse("admin:datasets_datacutdataset_change", args=(dataset.id,)),
data,
)

reloaded_dataset = DataSet.objects.filter(pk=dataset.id).first()
assert reloaded_dataset.data_catalogue_editors.count() == 1

@mock.patch("dataworkspace.apps.core.boto3_client.boto3.client")
def test_request_approvers_with_new_known_email_addresses_added_to_existing_data_catalogue_editors(
self, mock_client
):
dataset = factories.DataSetFactory()
user = factories.UserFactory()
matching_request_approvers = ["new_user1@example.com", "new_user2@example.com"]
factories.UserFactory.create_batch(
len(matching_request_approvers), email=factory.Iterator(matching_request_approvers)
)
data = self._valid_dataset(dataset, user)
data["data_catalogue_editors"] = [user.id]
self._authenticated_post(
reverse("admin:datasets_datacutdataset_change", args=(dataset.id,)),
data,
)

data["request_approvers"] = matching_request_approvers + ["not_real"]

self._authenticated_post(
reverse("admin:datasets_datacutdataset_change", args=(dataset.id,)),
data,
)

reloaded_dataset = DataSet.objects.filter(pk=dataset.id).first()
assert reloaded_dataset.data_catalogue_editors.count() == 3


class TestDatasetAdminPytest:
def test_sql_queries_must_be_reviewed_before_publishing(self, staff_client, user):
Expand Down