Skip to content

Commit

Permalink
fix(eros): enable objectgroup in multiple runs & fix permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
wiwski committed Jun 14, 2023
1 parent 5d59cee commit cdea1be
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 21 deletions.
5 changes: 2 additions & 3 deletions lab/api_views/objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,17 @@
permission_classes,
)
from rest_framework.exceptions import NotFound
from rest_framework.permissions import IsAdminUser
from rest_framework.response import Response

from lab.objects.c2rmf import ErosHTTPError, fetch_partial_objectgroup_from_eros

from .permissions import IsLabAdminUser

logger = logging.getLogger(__name__)


@api_view(["POST"])
@authentication_classes([authentication.SessionAuthentication])
@permission_classes([IsLabAdminUser])
@permission_classes([IsAdminUser])
def get_eros_object(request):
"""Fetch object from Eros."""
c2rmf_id = request.data["query"]
Expand Down
15 changes: 13 additions & 2 deletions lab/objects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,20 @@ def __init__(self, *args, **kwargs: dict[str, Any]):
self.fields["label"].disabled = True
self.fields["label"].required = False

def _post_clean(self):
# Skip _post_clean if we already have an existing instance
if not self.instance.id:
super()._post_clean()

def clean(self) -> dict[str, Any]:
data = super().clean()
# First, check if objectgroup with this c2rmf id exists.
instance = ObjectGroup.objects.filter(
c2rmf_id=self.cleaned_data["c2rmf_id"]
).first()
if instance:
self.instance = instance
return self.cleaned_data
# If it does not exist, then fetch data from Eros
try:
eros_data = fetch_partial_objectgroup_from_eros(
self.cleaned_data["c2rmf_id"]
Expand All @@ -114,7 +126,6 @@ def clean(self) -> dict[str, Any]:
{"c2rmf_id": _("This ID was not found in Eros.")}
)
data = {
**data,
**eros_data,
"object_count": 1,
}
Expand Down
18 changes: 18 additions & 0 deletions lab/objects/tests/forms/test_object_group_import_c2rmf_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import pytest
from django.forms import ValidationError

from lab.tests import factories

from ...c2rmf import ErosHTTPError
from ...forms import ObjectGroupImportC2RMFForm

Expand All @@ -15,6 +17,7 @@ def test_form_conf():
assert not form.fields["label"].required


@pytest.mark.django_db
@mock.patch("lab.objects.forms.fetch_partial_objectgroup_from_eros")
def test_clean_fetch_data(mock_fetch: mock.MagicMock):
mock_fetch.return_value = {
Expand All @@ -30,6 +33,7 @@ def test_clean_fetch_data(mock_fetch: mock.MagicMock):
assert cleaned_data["object_count"] == 1


@pytest.mark.django_db
@mock.patch("lab.objects.forms.fetch_partial_objectgroup_from_eros")
def test_clean_raises_if_fetch_none(mock_fetch: mock.MagicMock):
mock_fetch.return_value = None
Expand All @@ -39,10 +43,24 @@ def test_clean_raises_if_fetch_none(mock_fetch: mock.MagicMock):
form.clean()


@pytest.mark.django_db
@mock.patch("lab.objects.forms.fetch_partial_objectgroup_from_eros")
def test_clean_raises_if_fetch_fails(mock_fetch: mock.MagicMock):
mock_fetch.side_effect = ErosHTTPError()
form = ObjectGroupImportC2RMFForm()
form.cleaned_data = {"c2rmf_id": "C2RMF00000"}
with pytest.raises(ValidationError):
form.clean()


@pytest.mark.django_db
def test_clean_set_form_if_obj_exists():
c2rmf_id = "C2RMF00000"
objectgroup = factories.ObjectGroupFactory(c2rmf_id=c2rmf_id)

form = ObjectGroupImportC2RMFForm()
form.cleaned_data = {"c2rmf_id": c2rmf_id}
form.clean()

assert form.instance
assert form.instance.id == objectgroup.id
70 changes: 60 additions & 10 deletions lab/objects/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@


class TestObjectImportC2RMFView(TestCase):
def test_form_valid(self):
run = factories.RunFactory()
def setUp(self):
self.run = run = factories.RunFactory()
self.view = ObjectImportC2RMFView()
self.url = reverse("admin:lab_objectgroup_c2rmfimport") + f"?run={run.id}"

view = ObjectImportC2RMFView()
def test_form_valid(self):
data = {
"c2rmf_id": "abc",
"label": "Rodondendron",
"object_count": 1,
IS_POPUP_VAR: 1,
}
view.request = RequestFactory().post(
reverse("admin:lab_objectgroup_c2rmfimport") + f"?run={run.id}", data
)
self.view.request = RequestFactory().post(self.url, data)
with mock.patch(
"lab.objects.forms.fetch_partial_objectgroup_from_eros"
) as mock_fn:
Expand All @@ -35,9 +35,9 @@ def test_form_valid(self):
"label": "Rodondendron",
}
form = forms.ObjectGroupImportC2RMFForm(data=data)
response = view.form_valid(form)
response = self.view.form_valid(form)

assert view.object.id
assert self.view.object.id
assert models.ObjectGroup.objects.get(c2rmf_id="abc", label="Rodondendron")
assert isinstance(response, TemplateResponse)
assert "popup_response_data" in response.context_data
Expand All @@ -46,8 +46,58 @@ def test_form_valid(self):
assert popup_response_data["obj"]
assert json.dumps(popup_response_data["objectgroup_run_run_ids"]) == json.dumps(
list(
view.object.runs.through.objects.filter(
objectgroup=view.object.id
self.view.object.runs.through.objects.filter(
objectgroup=self.view.object.id
).values_list("id", "run_id")
)
)

def test_form_valid_calls_invalid_if_run_relation_exists(self):
existing_obj = factories.ObjectGroupFactory(c2rmf_id="F22299900")
existing_obj.runs.add(self.run)

data = {
"c2rmf_id": existing_obj.c2rmf_id,
"label": "Rodondendron",
"object_count": 1,
IS_POPUP_VAR: 1,
}
self.view.request = RequestFactory().post(self.url, data)

with mock.patch(
"lab.objects.forms.fetch_partial_objectgroup_from_eros"
) as mock_fn:
mock_fn.return_value = {
"c2rmf_id": existing_obj.c2rmf_id,
"label": "Rodondendron",
}
form = forms.ObjectGroupImportC2RMFForm(data=data)

with mock.patch.object(self.view, "form_invalid") as mock_method:
self.view.form_valid(form)

mock_method.assert_called()
assert "c2rmf_id" in form.errors

def test_form_valid_do_not_save_commit_if_objectgroup_exists(self):
existing_obj = factories.ObjectGroupFactory(c2rmf_id="F2229990")
data = {
"c2rmf_id": existing_obj.c2rmf_id,
"label": "Rodondendron",
"object_count": 1,
IS_POPUP_VAR: 1,
}
self.view.request = RequestFactory().post(self.url, data)
with mock.patch(
"lab.objects.forms.fetch_partial_objectgroup_from_eros"
) as mock_fn:
mock_fn.return_value = {
"c2rmf_id": existing_obj.c2rmf_id,
"label": "Rodondendron",
}
form = forms.ObjectGroupImportC2RMFForm(data=data)
form.instance = existing_obj
with mock.patch.object(form, "save") as save_fn_mock:
save_fn_mock.return_value = existing_obj
self.view.form_valid(form)
save_fn_mock.assert_called_once_with(commit=False)
8 changes: 7 additions & 1 deletion lab/objects/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,15 @@ class ObjectImportC2RMFView(StaffUserRequiredMixin, CreateView):
def form_valid(self, form: forms.ObjectGroupImportC2RMFForm):
if IS_POPUP_VAR in self.request.POST:
# pylint: disable=attribute-defined-outside-init
self.object = form.save()
self.object = form.save(commit=not form.instance.id)
run_id = self.request.GET.get("run")
if run_id:
# At last, check if this object is not already in the run
if self.object.runs.filter(id=run_id).exists():
form.add_error(
"c2rmf_id", _("This object has already been added to the run.")
)
return self.form_invalid(form)
self.object.runs.add(run_id)
popup_response_data = json.dumps(
{
Expand Down
4 changes: 3 additions & 1 deletion lab/static/js/widgets/import-from-input.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,11 @@
if (response.status === 404) {
displayErrorMessage(
importFromInputEl,
window.gettext("We could not retrieve this object.")
window.gettext("No results found.")
);
return;
} else if (!response.ok) {
throw new Error(`${response.status}: ${await response.text()}`);
}
const data = await response.json();
for (const key in data) {
Expand Down
5 changes: 4 additions & 1 deletion locale/fr/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ msgid ""
msgstr ""
"Project-Id-Version: 0.1\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2023-06-14 11:29+0200\n"
"POT-Creation-Date: 2023-06-14 16:59+0200\n"
"PO-Revision-Date: 2021-09-09 19:04+0200\n"
"Language: \n"
"MIME-Version: 1.0\n"
Expand Down Expand Up @@ -545,6 +545,9 @@ msgstr "Objet(s) / Échantillon(s)"
msgid "Inventory"
msgstr "Inventaire"

msgid "This object has already been added to the run."
msgstr "Cet object a déjà été ajouté à ce run."

msgid "EROS import"
msgstr "Import EROS"

Expand Down
6 changes: 3 additions & 3 deletions locale/fr/LC_MESSAGES/djangojs.po
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ msgid ""
msgstr ""
"Project-Id-Version: 0.1\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2023-06-14 11:31+0200\n"
"POT-Creation-Date: 2023-06-14 17:02+0200\n"
"PO-Revision-Date: 2021-12-20 11:13+0100\n"
"Language: \n"
"MIME-Version: 1.0\n"
Expand Down Expand Up @@ -70,8 +70,8 @@ msgstr "Caractères restant: %s"
msgid "You must enter a valid value for this field."
msgstr "Entrez une valeur valide pour ce champs."

msgid "We could not retrieve this object."
msgstr "Nous n'avons pas pu trouver cet objet."
msgid "No results found."
msgstr "Aucun résultat trouvé."

msgid ""
"An error occured while retrieving the object. Please contact AGLAE team if "
Expand Down

0 comments on commit cdea1be

Please sign in to comment.