From cea4975a0b4959aeb0836d6f9b5ecb245c879a73 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Mon, 14 Sep 2020 10:48:25 +0000 Subject: [PATCH 01/37] Set up basic fitting experiment form --- weblab/fitting/forms.py | 22 ++++- weblab/fitting/tests/test_forms.py | 83 +++++++++++++++++++ weblab/fitting/tests/test_views.py | 29 +++++++ weblab/fitting/urls.py | 6 ++ weblab/fitting/views.py | 11 ++- weblab/repocache/models.py | 12 +++ weblab/repocache/tests/test_models.py | 17 +++- .../fitting/fittingresult_create_form.html | 9 ++ 8 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 weblab/fitting/tests/test_forms.py create mode 100644 weblab/templates/fitting/fittingresult_create_form.html diff --git a/weblab/fitting/forms.py b/weblab/fitting/forms.py index e2c8f7b72..6c827ef1c 100644 --- a/weblab/fitting/forms.py +++ b/weblab/fitting/forms.py @@ -1,7 +1,10 @@ +from django import forms +from django.core.exceptions import ValidationError + from entities.forms import EntityForm, EntityVersionForm from entities.models import ProtocolEntity -from .models import FittingSpec +from .models import FittingSpec, FittingResult class FittingSpecForm(EntityForm): @@ -24,3 +27,20 @@ class FittingSpecVersionForm(EntityVersionForm): This works almost the same as other entities, except we can't re-run experiments. """ rerun_expts = None + + +class FittingResultCreateForm(forms.ModelForm): + class Meta: + model = FittingResult + fields = ('model', 'model_version', 'protocol', 'protocol_version', + 'fittingspec', 'fittingspec_version', 'dataset') + + def clean(self): + if self.cleaned_data['model_version'].model != self.cleaned_data['model']: + raise ValidationError({'model_version': 'Model version must belong to model'}) + + if self.cleaned_data['protocol_version'].protocol != self.cleaned_data['protocol']: + raise ValidationError({'protocol_version': 'Protocol version must belong to protocol'}) + + if self.cleaned_data['fittingspec_version'].fittingspec != self.cleaned_data['fittingspec']: + raise ValidationError({'fittingspec_version': 'Fitting spec version must belong to fitting spec'}) diff --git a/weblab/fitting/tests/test_forms.py b/weblab/fitting/tests/test_forms.py new file mode 100644 index 000000000..8768d8037 --- /dev/null +++ b/weblab/fitting/tests/test_forms.py @@ -0,0 +1,83 @@ +import pytest + +from core import recipes +from fitting.forms import FittingResultCreateForm + + +@pytest.mark.django_db +class TestFittingResultCreateForm: + def test_fields(self): + form = FittingResultCreateForm() + assert 'model' in form.fields + assert 'model_version' in form.fields + assert 'protocol' in form.fields + assert 'protocol_version' in form.fields + assert 'fittingspec' in form.fields + assert 'fittingspec_version' in form.fields + assert 'dataset' in form.fields + + def test_valid_form( + self, model_with_version, protocol_with_version, + fittingspec_with_version, public_dataset + ): + form = FittingResultCreateForm({ + 'model': model_with_version.pk, + 'model_version': model_with_version.repocache.latest_version.pk, + 'protocol': protocol_with_version.pk, + 'protocol_version': protocol_with_version.repocache.latest_version.pk, + 'fittingspec': fittingspec_with_version.pk, + 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'dataset': public_dataset.pk, + }) + assert form.is_valid() + + def test_model_version_must_belong_to_model( + self, model_with_version, protocol_with_version, + fittingspec_with_version, public_dataset + ): + invalid_version = recipes.cached_model_version.make() + form = FittingResultCreateForm({ + 'model': model_with_version.pk, + 'model_version': invalid_version.pk, + 'protocol': protocol_with_version.pk, + 'protocol_version': protocol_with_version.repocache.latest_version.pk, + 'fittingspec': fittingspec_with_version.pk, + 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'dataset': public_dataset.pk, + }) + assert not form.is_valid() + assert 'model_version' in form.errors + + def test_protocol_version_must_belong_to_protocol( + self, model_with_version, protocol_with_version, + fittingspec_with_version, public_dataset + ): + invalid_version = recipes.cached_protocol_version.make() + form = FittingResultCreateForm({ + 'model': model_with_version.pk, + 'model_version': model_with_version.repocache.latest_version.pk, + 'protocol': protocol_with_version.pk, + 'protocol_version': invalid_version.pk, + 'fittingspec': fittingspec_with_version.pk, + 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'dataset': public_dataset.pk, + }) + assert not form.is_valid() + assert 'protocol_version' in form.errors + + def test_fittingspec_version_must_belong_to_fittingspec( + self, model_with_version, protocol_with_version, + fittingspec_with_version, public_dataset + ): + invalid_version = recipes.cached_fittingspec_version.make() + form = FittingResultCreateForm({ + 'model': model_with_version.pk, + 'model_version': model_with_version.repocache.latest_version.pk, + 'protocol': protocol_with_version.pk, + 'protocol_version': protocol_with_version.repocache.latest_version.pk, + 'fittingspec': fittingspec_with_version.pk, + 'fittingspec_version': invalid_version.pk, + 'dataset': public_dataset.pk, + }) + assert not form.is_valid() + assert 'fittingspec_version' in form.errors diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index 1f51814f9..e62640faa 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -5,6 +5,8 @@ from pathlib import Path import pytest +from django.contrib.auth.models import Permission +from django.contrib.contenttypes.models import ContentType from django.core.urlresolvers import reverse from django.utils.dateparse import parse_datetime from pytest_django.asserts import assertContains, assertTemplateUsed @@ -14,6 +16,17 @@ from repocache.populate import populate_entity_cache +@pytest.fixture +def fits_user(logged_in_user): + content_type = ContentType.objects.get_for_model(FittingResult) + permission = Permission.objects.get( + codename='run_fits', + content_type=content_type, + ) + logged_in_user.user_permissions.add(permission) + return logged_in_user + + @pytest.fixture def archive_file_path(): return str(Path(__file__).absolute().parent.joinpath('./test.omex')) @@ -424,3 +437,19 @@ def test_empty_fittingresult_list(self, client, fittingresult_version): assert response.status_code == 200 data = json.loads(response.content.decode()) assert len(data['getEntityInfos']['entities']) == 0 + + +@pytest.mark.django_db +class TestCreateFittingResultView: + def test_requires_login(self, client): + response = client.get('/fitting/results/new') + assert response.status_code == 302 + + def test_requires_permission(self, client, logged_in_user): + response = client.get('/fitting/results/new') + assert response.status_code == 302 + + def test_basic_page(self, client, fits_user): + response = client.get('/fitting/results/new') + assert response.status_code == 200 + assert 'form' in response.context diff --git a/weblab/fitting/urls.py b/weblab/fitting/urls.py index 2ff214761..244f40b9a 100644 --- a/weblab/fitting/urls.py +++ b/weblab/fitting/urls.py @@ -66,6 +66,12 @@ views.FittingResultComparisonJsonView.as_view(), name='compare_json', ), + + url( + r'^new$', + views.FittingResultCreateView.as_view(), + name='new' + ), ] urlpatterns = [ diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 4dd8178d4..1caf00d0b 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -19,13 +19,13 @@ from django.views import View from django.views.generic import TemplateView from django.views.generic.detail import DetailView, SingleObjectMixin -from django.views.generic.edit import CreateView +from django.views.generic.edit import CreateView, FormView from core.visibility import VisibilityMixin from datasets import views as dataset_views from entities.views import EntityNewVersionView, EntityTypeMixin -from .forms import FittingSpecForm, FittingSpecVersionForm +from .forms import FittingResultCreateForm, FittingSpecForm, FittingSpecVersionForm from .models import FittingResult, FittingResultVersion @@ -197,3 +197,10 @@ def get(self, request, *args, **kwargs): } return JsonResponse(response) + + +class FittingResultCreateView(LoginRequiredMixin, PermissionRequiredMixin, FormView): + permission_required = 'fitting.run_fits' + form_class = FittingResultCreateForm + + template_name = 'fitting/fittingresult_create_form.html' diff --git a/weblab/repocache/models.py b/weblab/repocache/models.py index 6d2ba638b..39bbe2811 100644 --- a/weblab/repocache/models.py +++ b/weblab/repocache/models.py @@ -216,6 +216,10 @@ class CachedModelVersion(CachedEntityVersion): """Cache for a single version / commit in a CellML model's repository.""" entity = models.ForeignKey(CachedModel, on_delete=models.CASCADE, related_name='versions') + @property + def model(self): + return self.entity.entity + class CachedModelTag(CachedEntityTag): """Cache for a tag in a CellML model's repository.""" @@ -235,6 +239,10 @@ class CachedProtocolVersion(CachedEntityVersion): """Cache for a single version / commit in a protocol's repository.""" entity = models.ForeignKey(CachedProtocol, on_delete=models.CASCADE, related_name='versions') + @property + def protocol(self): + return self.entity.entity + class CachedProtocolTag(CachedEntityTag): """Cache for a tag in a protocol's repository.""" @@ -254,6 +262,10 @@ class CachedFittingSpecVersion(CachedEntityVersion): """Cache for a single version / commit in a fitting specifications's repository.""" entity = models.ForeignKey(CachedFittingSpec, on_delete=models.CASCADE, related_name='versions') + @property + def fittingspec(self): + return self.entity.entity + class CachedFittingSpecTag(CachedEntityTag): """Cache for a tag in a fitting specifications's repository.""" diff --git a/weblab/repocache/tests/test_models.py b/weblab/repocache/tests/test_models.py index 436391063..d2ae01540 100644 --- a/weblab/repocache/tests/test_models.py +++ b/weblab/repocache/tests/test_models.py @@ -3,7 +3,7 @@ from core import recipes from repocache.exceptions import RepoCacheMiss -from repocache.models import CachedModel, CachedProtocol +from repocache.models import CachedFittingSpec, CachedModel, CachedProtocol from repocache.populate import populate_entity_cache @@ -12,6 +12,7 @@ class TestEntityCacheModels: @pytest.mark.parametrize("recipe,manager", [ (recipes.cached_model, CachedModel.objects), (recipes.cached_protocol, CachedProtocol.objects), + (recipes.cached_fittingspec, CachedFittingSpec.objects), ]) def test_cachedentity_is_deleted_when_entity_is_deleted(self, recipe, manager): cached = recipe.make() @@ -21,6 +22,7 @@ def test_cachedentity_is_deleted_when_entity_is_deleted(self, recipe, manager): @pytest.mark.parametrize("recipe", [ (recipes.cached_model_version), (recipes.cached_protocol_version), + (recipes.cached_fittingspec_version), ]) def test_related_names_for_versions(self, recipe): version = recipe.make() @@ -29,6 +31,7 @@ def test_related_names_for_versions(self, recipe): @pytest.mark.parametrize("recipe", [ (recipes.cached_model_tag), (recipes.cached_protocol_tag), + (recipes.cached_fittingspec_tag), ]) def test_related_names_for_tags(self, recipe): tag = recipe.make() @@ -37,6 +40,7 @@ def test_related_names_for_tags(self, recipe): @pytest.mark.parametrize("recipe", [ (recipes.cached_model_version), (recipes.cached_protocol_version), + (recipes.cached_fittingspec_version), ]) def test_uniqueness_of_entity_and_version_sha(self, recipe): version = recipe.make() @@ -46,6 +50,7 @@ def test_uniqueness_of_entity_and_version_sha(self, recipe): @pytest.mark.parametrize("recipe", [ (recipes.cached_model_tag), (recipes.cached_protocol_tag), + (recipes.cached_fittingspec_tag), ]) def test_uniqueness_of_entity_and_tag(self, recipe): version = recipe.make() @@ -55,12 +60,22 @@ def test_uniqueness_of_entity_and_tag(self, recipe): @pytest.mark.parametrize("recipe", [ (recipes.cached_model), (recipes.cached_protocol), + (recipes.cached_fittingspec), ]) def test_uniqueness_of_entity(self, recipe): cached = recipe.make() with pytest.raises(IntegrityError): recipe.make(entity=cached.entity) + @pytest.mark.parametrize("recipe,property_name", [ + (recipes.cached_model_version, 'model'), + (recipes.cached_protocol_version, 'protocol'), + (recipes.cached_fittingspec_version, 'fittingspec'), + ]) + def test_entity_property(self, recipe, property_name): + cached = recipe.make() + assert getattr(cached, property_name) == cached.entity.entity + @pytest.mark.django_db class TestEntityCacheModelsVisibility: diff --git a/weblab/templates/fitting/fittingresult_create_form.html b/weblab/templates/fitting/fittingresult_create_form.html new file mode 100644 index 000000000..9f8313953 --- /dev/null +++ b/weblab/templates/fitting/fittingresult_create_form.html @@ -0,0 +1,9 @@ +{% extends "base.html" %} + +{% block title %}Submit fitting experiment - {% endblock title %} + +{% block content %} + +{{ form }} + +{% endblock content %} From b45e0b527510a7bb0322bbab1c84e59e46277ba1 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Mon, 14 Sep 2020 12:01:12 +0000 Subject: [PATCH 02/37] Fitting result form: validate object combinations --- weblab/fitting/forms.py | 8 ++++- weblab/fitting/tests/test_forms.py | 56 ++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/weblab/fitting/forms.py b/weblab/fitting/forms.py index 6c827ef1c..aa0b734d7 100644 --- a/weblab/fitting/forms.py +++ b/weblab/fitting/forms.py @@ -4,7 +4,7 @@ from entities.forms import EntityForm, EntityVersionForm from entities.models import ProtocolEntity -from .models import FittingSpec, FittingResult +from .models import FittingResult, FittingSpec class FittingSpecForm(EntityForm): @@ -44,3 +44,9 @@ def clean(self): if self.cleaned_data['fittingspec_version'].fittingspec != self.cleaned_data['fittingspec']: raise ValidationError({'fittingspec_version': 'Fitting spec version must belong to fitting spec'}) + + if self.cleaned_data['dataset'].protocol != self.cleaned_data['protocol']: + raise ValidationError({'protocol': 'Protocol and dataset must match'}) + + if self.cleaned_data['fittingspec'].protocol != self.cleaned_data['protocol']: + raise ValidationError({'protocol': 'Protocol and fitting spec must match'}) diff --git a/weblab/fitting/tests/test_forms.py b/weblab/fitting/tests/test_forms.py index 8768d8037..f661e4345 100644 --- a/weblab/fitting/tests/test_forms.py +++ b/weblab/fitting/tests/test_forms.py @@ -16,10 +16,22 @@ def test_fields(self): assert 'fittingspec_version' in form.fields assert 'dataset' in form.fields + def _link_to_protocol(self, protocol, *objects): + """ + Link given objects to protocol (fitting specs or datasets) + @param protocol - protocol to link to + @param objects - list of objects to link to the protocol + """ + for obj in objects: + obj.protocol = protocol + obj.save() + def test_valid_form( self, model_with_version, protocol_with_version, fittingspec_with_version, public_dataset ): + self._link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + form = FittingResultCreateForm({ 'model': model_with_version.pk, 'model_version': model_with_version.repocache.latest_version.pk, @@ -35,6 +47,8 @@ def test_model_version_must_belong_to_model( self, model_with_version, protocol_with_version, fittingspec_with_version, public_dataset ): + self._link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + invalid_version = recipes.cached_model_version.make() form = FittingResultCreateForm({ 'model': model_with_version.pk, @@ -52,6 +66,8 @@ def test_protocol_version_must_belong_to_protocol( self, model_with_version, protocol_with_version, fittingspec_with_version, public_dataset ): + self._link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + invalid_version = recipes.cached_protocol_version.make() form = FittingResultCreateForm({ 'model': model_with_version.pk, @@ -69,6 +85,9 @@ def test_fittingspec_version_must_belong_to_fittingspec( self, model_with_version, protocol_with_version, fittingspec_with_version, public_dataset ): + + self._link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + invalid_version = recipes.cached_fittingspec_version.make() form = FittingResultCreateForm({ 'model': model_with_version.pk, @@ -81,3 +100,40 @@ def test_fittingspec_version_must_belong_to_fittingspec( }) assert not form.is_valid() assert 'fittingspec_version' in form.errors + + def test_protocol_and_fittingspec_must_be_linked( + self, model_with_version, protocol_with_version, + fittingspec_with_version, public_dataset + ): + self._link_to_protocol(protocol_with_version, public_dataset) + + form = FittingResultCreateForm({ + 'model': model_with_version.pk, + 'model_version': model_with_version.repocache.latest_version.pk, + 'protocol': protocol_with_version.pk, + 'protocol_version': protocol_with_version.repocache.latest_version.pk, + 'fittingspec': fittingspec_with_version.pk, + 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'dataset': public_dataset.pk, + }) + assert not form.is_valid() + assert 'protocol' in form.errors + + def test_protocol_and_dataset_must_be_linked( + self, model_with_version, protocol_with_version, + fittingspec_with_version, public_dataset + ): + + self._link_to_protocol(protocol_with_version, fittingspec_with_version) + + form = FittingResultCreateForm({ + 'model': model_with_version.pk, + 'model_version': model_with_version.repocache.latest_version.pk, + 'protocol': protocol_with_version.pk, + 'protocol_version': protocol_with_version.repocache.latest_version.pk, + 'fittingspec': fittingspec_with_version.pk, + 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'dataset': public_dataset.pk, + }) + assert not form.is_valid() + assert 'protocol' in form.errors From 3dbffc536a44dc13640f23b62b80b2b920b11b68 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Mon, 14 Sep 2020 14:19:13 +0000 Subject: [PATCH 03/37] Accept preselected entity in Fitting Result form --- weblab/fitting/tests/test_views.py | 20 ++++++++++++++++++++ weblab/fitting/views.py | 19 ++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index e62640faa..d57b62ce0 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -453,3 +453,23 @@ def test_basic_page(self, client, fits_user): response = client.get('/fitting/results/new') assert response.status_code == 200 assert 'form' in response.context + + def test_with_preselected_model(self, client, fits_user, public_model): + response = client.get('/fitting/results/new', {'model': public_model.pk}) + assert response.status_code == 200 + assert response.context['form'].initial['model'] == public_model + + def test_with_preselected_protocol(self, client, fits_user, public_protocol): + response = client.get('/fitting/results/new', {'protocol': public_protocol.pk}) + assert response.status_code == 200 + assert response.context['form'].initial['protocol'] == public_protocol + + def test_with_preselected_fittingspec(self, client, fits_user, public_fittingspec): + response = client.get('/fitting/results/new', {'fittingspec': public_fittingspec.pk}) + assert response.status_code == 200 + assert response.context['form'].initial['fittingspec'] == public_fittingspec + + def test_with_preselected_dataset(self, client, fits_user, public_dataset): + response = client.get('/fitting/results/new', {'dataset': public_dataset.pk}) + assert response.status_code == 200 + assert response.context['form'].initial['dataset'] == public_dataset diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 1caf00d0b..52d9262bd 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -14,6 +14,7 @@ from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin from django.http import JsonResponse +from django.shortcuts import get_object_or_404 from django.urls import reverse from django.utils.text import get_valid_filename from django.views import View @@ -23,10 +24,12 @@ from core.visibility import VisibilityMixin from datasets import views as dataset_views +from datasets.models import Dataset +from entities.models import ModelEntity, ProtocolEntity from entities.views import EntityNewVersionView, EntityTypeMixin from .forms import FittingResultCreateForm, FittingSpecForm, FittingSpecVersionForm -from .models import FittingResult, FittingResultVersion +from .models import FittingResult, FittingResultVersion, FittingSpec class FittingSpecCreateView( @@ -204,3 +207,17 @@ class FittingResultCreateView(LoginRequiredMixin, PermissionRequiredMixin, FormV form_class = FittingResultCreateForm template_name = 'fitting/fittingresult_create_form.html' + + def get_initial(self): + initial = super().get_initial() + if 'model' in self.request.GET: + initial['model'] = get_object_or_404(ModelEntity, pk=self.request.GET['model']) + elif 'protocol' in self.request.GET: + initial['protocol'] = get_object_or_404(ProtocolEntity, pk=self.request.GET['protocol']) + elif 'fittingspec' in self.request.GET: + initial['fittingspec'] = get_object_or_404(FittingSpec, pk=self.request.GET['fittingspec']) + elif 'dataset' in self.request.GET: + initial['dataset'] = get_object_or_404(Dataset, pk=self.request.GET['dataset']) + + + return initial From ebc17c7e9cded415f2bc6cfe22387454f07d2fa2 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 15 Sep 2020 08:54:11 +0000 Subject: [PATCH 04/37] Fitting result form submit to back end --- weblab/conftest.py | 11 ++++++++++ weblab/fitting/tests/test_forms.py | 34 +++++++++++------------------- weblab/fitting/tests/test_views.py | 32 ++++++++++++++++++++++++++++ weblab/fitting/views.py | 28 +++++++++++++++++++++++- 4 files changed, 82 insertions(+), 23 deletions(-) diff --git a/weblab/conftest.py b/weblab/conftest.py index 0cff5a104..5216c07c4 100644 --- a/weblab/conftest.py +++ b/weblab/conftest.py @@ -99,6 +99,17 @@ def add_permission(user, perm, model=Entity): def login(client, user): client.login(username=user.email, password='password') + @staticmethod + def link_to_protocol(protocol, *objects): + """ + Link given objects to protocol (fitting specs or datasets) + @param protocol - protocol to link to + @param objects - list of objects to link to the protocol + """ + for obj in objects: + obj.protocol = protocol + obj.save() + @pytest.fixture def helpers(): diff --git a/weblab/fitting/tests/test_forms.py b/weblab/fitting/tests/test_forms.py index f661e4345..7fd7adda7 100644 --- a/weblab/fitting/tests/test_forms.py +++ b/weblab/fitting/tests/test_forms.py @@ -16,21 +16,11 @@ def test_fields(self): assert 'fittingspec_version' in form.fields assert 'dataset' in form.fields - def _link_to_protocol(self, protocol, *objects): - """ - Link given objects to protocol (fitting specs or datasets) - @param protocol - protocol to link to - @param objects - list of objects to link to the protocol - """ - for obj in objects: - obj.protocol = protocol - obj.save() - def test_valid_form( self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset + fittingspec_with_version, public_dataset, helpers ): - self._link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + helpers.link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) form = FittingResultCreateForm({ 'model': model_with_version.pk, @@ -45,9 +35,9 @@ def test_valid_form( def test_model_version_must_belong_to_model( self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset + fittingspec_with_version, public_dataset, helpers ): - self._link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + helpers.link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) invalid_version = recipes.cached_model_version.make() form = FittingResultCreateForm({ @@ -64,9 +54,9 @@ def test_model_version_must_belong_to_model( def test_protocol_version_must_belong_to_protocol( self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset + fittingspec_with_version, public_dataset, helpers ): - self._link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + helpers.link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) invalid_version = recipes.cached_protocol_version.make() form = FittingResultCreateForm({ @@ -83,10 +73,10 @@ def test_protocol_version_must_belong_to_protocol( def test_fittingspec_version_must_belong_to_fittingspec( self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset + fittingspec_with_version, public_dataset, helpers ): - self._link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + helpers.link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) invalid_version = recipes.cached_fittingspec_version.make() form = FittingResultCreateForm({ @@ -103,9 +93,9 @@ def test_fittingspec_version_must_belong_to_fittingspec( def test_protocol_and_fittingspec_must_be_linked( self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset + fittingspec_with_version, public_dataset, helpers ): - self._link_to_protocol(protocol_with_version, public_dataset) + helpers.link_to_protocol(protocol_with_version, public_dataset) form = FittingResultCreateForm({ 'model': model_with_version.pk, @@ -121,10 +111,10 @@ def test_protocol_and_fittingspec_must_be_linked( def test_protocol_and_dataset_must_be_linked( self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset + fittingspec_with_version, public_dataset, helpers ): - self._link_to_protocol(protocol_with_version, fittingspec_with_version) + helpers.link_to_protocol(protocol_with_version, fittingspec_with_version) form = FittingResultCreateForm({ 'model': model_with_version.pk, diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index d57b62ce0..4ee928fa6 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -3,6 +3,7 @@ import zipfile from io import BytesIO from pathlib import Path +from unittest.mock import patch import pytest from django.contrib.auth.models import Permission @@ -473,3 +474,34 @@ def test_with_preselected_dataset(self, client, fits_user, public_dataset): response = client.get('/fitting/results/new', {'dataset': public_dataset.pk}) assert response.status_code == 200 assert response.context['form'].initial['dataset'] == public_dataset + + @patch('fitting.views.submit_fitting') + def test_submits_to_backend(self, mock_submit, client, fits_user, public_model, public_protocol, + public_fittingspec, public_dataset, helpers): + model_version = public_model.repocache.latest_version + protocol_version = public_protocol.repocache.latest_version + fittingspec_version = public_fittingspec.repocache.latest_version + helpers.link_to_protocol(public_protocol, public_fittingspec, public_dataset) + + runnable = recipes.fittingresult_version.make() + mock_submit.return_value = (runnable, False) + + response = client.post('/fitting/results/new', { + 'model': public_model.pk, + 'model_version': model_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': protocol_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': fittingspec_version.pk, + 'dataset': public_dataset.pk, + }) + + assert response.status_code == 302 + mock_submit.assert_called_with( + public_model, model_version.sha, + public_protocol, protocol_version.sha, + public_fittingspec, fittingspec_version.sha, + public_dataset, fits_user, False + ) + + assert response.url == '/fitting/results/%d/versions/%d' % (runnable.fittingresult.pk, runnable.pk) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 52d9262bd..eb9ed5776 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -30,6 +30,7 @@ from .forms import FittingResultCreateForm, FittingSpecForm, FittingSpecVersionForm from .models import FittingResult, FittingResultVersion, FittingSpec +from .processing import submit_fitting class FittingSpecCreateView( @@ -219,5 +220,30 @@ def get_initial(self): elif 'dataset' in self.request.GET: initial['dataset'] = get_object_or_404(Dataset, pk=self.request.GET['dataset']) - return initial + + def post(self, request, *args, **kwargs): + form = self.get_form() + if form.is_valid(): + return self.form_valid(form) + else: + return self.form_invalid(form) + + def form_valid(self, form): + self.runnable, is_new = submit_fitting( + form.cleaned_data['model'], + form.cleaned_data['model_version'].sha, + form.cleaned_data['protocol'], + form.cleaned_data['protocol_version'].sha, + form.cleaned_data['fittingspec'], + form.cleaned_data['fittingspec_version'].sha, + form.cleaned_data['dataset'], + self.request.user, + False, + ) + + return super().form_valid(form) + + def get_success_url(self): + if hasattr(self, 'runnable'): + return reverse('fitting:result:version', args=[self.runnable.fittingresult.pk, self.runnable.pk]) From 9b5b15b344c7a408c7983560c76b361b5c679b13 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 15 Sep 2020 09:06:05 +0000 Subject: [PATCH 05/37] Change signature of submit_fitting Accept CachedVersion-derived objects rather than SHAs for versions, and remove entity arguments since these can be taken from the versions. --- weblab/fitting/processing.py | 24 ++++++------- weblab/fitting/tests/test_processing.py | 48 +++++++++++-------------- weblab/fitting/tests/test_views.py | 6 ++-- weblab/fitting/views.py | 9 ++--- 4 files changed, 39 insertions(+), 48 deletions(-) diff --git a/weblab/fitting/processing.py b/weblab/fitting/processing.py index d504ddb7c..1256ae012 100644 --- a/weblab/fitting/processing.py +++ b/weblab/fitting/processing.py @@ -14,9 +14,9 @@ def submit_fitting( - model, model_version, - protocol, protocol_version, - fittingspec, fittingspec_version, + model_version, + protocol_version, + fittingspec_version, dataset, user, rerun_ok, ): """Submit a Celery task to run a fitting experiment @@ -26,13 +26,13 @@ def submit_fitting( @return the FittingResultVersion for the run """ fittingresult, _ = FittingResult.objects.get_or_create( - model=model, - protocol=protocol, - fittingspec=fittingspec, + model=model_version.model, + protocol=protocol_version.protocol, + fittingspec=fittingspec_version.fittingspec, dataset=dataset, - model_version=model.repocache.get_version(model_version), - protocol_version=protocol.repocache.get_version(protocol_version), - fittingspec_version=fittingspec.repocache.get_version(fittingspec_version), + model_version=model_version, + protocol_version=protocol_version, + fittingspec_version=fittingspec_version, defaults={ 'author': user, } @@ -59,15 +59,15 @@ def submit_fitting( model_url = reverse( 'entities:entity_archive', - args=['model', model.pk, model_version] + args=['model', model_version.model.pk, model_version.sha] ) protocol_url = reverse( 'entities:entity_archive', - args=['protocol', protocol.pk, protocol_version] + args=['protocol', protocol_version.protocol.pk, protocol_version.sha] ) fittingspec_url = reverse( 'fitting:entity_archive', - args=['spec', fittingspec.pk, fittingspec_version] + args=['spec', fittingspec_version.fittingspec.pk, fittingspec_version.sha] ) dataset_url = reverse( 'datasets:archive', diff --git a/weblab/fitting/tests/test_processing.py b/weblab/fitting/tests/test_processing.py index b4b6f7928..d562428e5 100644 --- a/weblab/fitting/tests/test_processing.py +++ b/weblab/fitting/tests/test_processing.py @@ -41,19 +41,16 @@ def test_creates_new_fittingresult_and_side_effects( protocol = protocol_with_version fittingspec = fittingspec_with_version dataset = public_dataset - model_version = model.repo.latest_commit.sha - protocol_version = protocol.repo.latest_commit.sha - fittingspec_version = fittingspec.repo.latest_commit.sha + model_version = model.repocache.latest_version + protocol_version = protocol.repocache.latest_version + fittingspec_version = fittingspec.repocache.latest_version assert FittingResult.objects.count() == 0 assert RunningExperiment.objects.count() == 0 version, is_new = submit_fitting( - model, model_version, - protocol, protocol_version, - fittingspec, fittingspec_version, dataset, user, @@ -68,20 +65,20 @@ def test_creates_new_fittingresult_and_side_effects( assert version.fittingresult.fittingspec == fittingspec assert version.fittingresult.dataset == dataset assert version.author == user - assert version.fittingresult.model_version.sha == model_version - assert version.fittingresult.protocol_version.sha == protocol_version - assert version.fittingresult.fittingspec_version.sha == fittingspec_version + assert version.fittingresult.model_version == model_version + assert version.fittingresult.protocol_version == protocol_version + assert version.fittingresult.fittingspec_version == fittingspec_version assert version.fittingresult.author == user assert version.status == FittingResultVersion.STATUS_QUEUED # Check it did submit to the webservice - model_url = '/entities/models/%d/versions/%s/archive' % (model.pk, model_version) + model_url = '/entities/models/%d/versions/%s/archive' % (model.pk, model_version.sha) protocol_url = ( '/entities/protocols/%d/versions/%s/archive' % - (protocol.pk, protocol_version)) + (protocol.pk, protocol_version.sha)) fittingspec_url = ( '/fitting/specs/%d/versions/%s/archive' % - (fittingspec.pk, fittingspec_version)) + (fittingspec.pk, fittingspec_version.sha)) dataset_url = '/datasets/%d/archive' % (dataset.pk) assert mock_post.call_count == 1 @@ -137,12 +134,9 @@ def test_uses_existing_fittingresult( ) version, is_new = submit_fitting( - fittingresult.model, - fittingresult.model_version.sha, - fittingresult.protocol, - fittingresult.protocol_version.sha, - fittingresult.fittingspec, - fittingresult.fittingspec_version.sha, + fittingresult.model_version, + fittingresult.protocol_version, + fittingresult.fittingspec_version, fittingresult.dataset, user, False, @@ -166,9 +160,9 @@ def test_raises_exception_on_webservice_error( mock_post.side_effect = generate_response('something %s') with pytest.raises(ProcessingException): submit_fitting( - model, model_version.sha, - protocol, protocol_version.sha, - fittingspec, fittingspec_version.sha, + model_version, + protocol_version, + fittingspec_version, dataset, user, False @@ -201,9 +195,9 @@ def test_records_submission_error( mock_post.side_effect = generate_response('%s an error occurred') version, is_new = submit_fitting( - model, model_version.sha, - protocol, protocol_version.sha, - fittingspec, fittingspec_version.sha, + model_version, + protocol_version, + fittingspec_version, dataset, user, False @@ -229,9 +223,9 @@ def test_records_inapplicable_result( mock_post.side_effect = generate_response('%s inapplicable') version, is_new = submit_fitting( - model, model_version.sha, - protocol, protocol_version.sha, - fittingspec, fittingspec_version.sha, + model_version, + protocol_version, + fittingspec_version, dataset, user, False diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index 4ee928fa6..b5e367706 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -498,9 +498,9 @@ def test_submits_to_backend(self, mock_submit, client, fits_user, public_model, assert response.status_code == 302 mock_submit.assert_called_with( - public_model, model_version.sha, - public_protocol, protocol_version.sha, - public_fittingspec, fittingspec_version.sha, + model_version, + protocol_version, + fittingspec_version, public_dataset, fits_user, False ) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index eb9ed5776..f5083e739 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -231,12 +231,9 @@ def post(self, request, *args, **kwargs): def form_valid(self, form): self.runnable, is_new = submit_fitting( - form.cleaned_data['model'], - form.cleaned_data['model_version'].sha, - form.cleaned_data['protocol'], - form.cleaned_data['protocol_version'].sha, - form.cleaned_data['fittingspec'], - form.cleaned_data['fittingspec_version'].sha, + form.cleaned_data['model_version'], + form.cleaned_data['protocol_version'], + form.cleaned_data['fittingspec_version'], form.cleaned_data['dataset'], self.request.user, False, From 492738060d877ec592ec44193c2a13ddc4d43fcf Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 15 Sep 2020 10:39:33 +0000 Subject: [PATCH 06/37] Change submit_experiment call signature Take CachedVersion derived objects rather than SHAs for the versions, and stop passing entity objects - take them from the version objects. --- weblab/experiments/processing.py | 16 +++++++------- weblab/experiments/tests/test_processing.py | 23 ++++++++++----------- weblab/experiments/views.py | 16 +++++++------- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/weblab/experiments/processing.py b/weblab/experiments/processing.py index 3bf362941..8f560edd1 100644 --- a/weblab/experiments/processing.py +++ b/weblab/experiments/processing.py @@ -100,7 +100,7 @@ def submit_runnable(runnable, body, user): runnable.save() -def submit_experiment(model, model_version, protocol, protocol_version, user, rerun_ok): +def submit_experiment(model_version, protocol_version, user, rerun_ok): """Submit a Celery task to run an experiment. @param rerun_ok if False and an ExperimentVersion already exists, will just return that. @@ -108,10 +108,10 @@ def submit_experiment(model, model_version, protocol, protocol_version, user, re @return the ExperimentVersion for the run """ experiment, _ = Experiment.objects.get_or_create( - model=model, - protocol=protocol, - model_version=model.repocache.get_version(model_version), - protocol_version=protocol.repocache.get_version(protocol_version), + model=model_version.model, + protocol=protocol_version.protocol, + model_version=model_version, + protocol_version=protocol_version, defaults={ 'author': user, } @@ -138,17 +138,17 @@ def submit_experiment(model, model_version, protocol, protocol_version, user, re model_url = reverse( 'entities:entity_archive', - args=['model', model.pk, model_version] + args=['model', model_version.model.pk, model_version.sha] ) protocol_url = reverse( 'entities:entity_archive', - args=['protocol', protocol.pk, protocol_version] + args=['protocol', protocol_version.protocol.pk, protocol_version.sha] ) body = { 'model': urljoin(settings.CALLBACK_BASE_URL, model_url), 'protocol': urljoin(settings.CALLBACK_BASE_URL, protocol_url), } - if protocol.is_fitting_spec: + if protocol_version.protocol.is_fitting_spec: body['dataset'] = body['fittingSpec'] = body['protocol'] submit_runnable(version, body, user) diff --git a/weblab/experiments/tests/test_processing.py b/weblab/experiments/tests/test_processing.py index 2820a9df6..193088b58 100644 --- a/weblab/experiments/tests/test_processing.py +++ b/weblab/experiments/tests/test_processing.py @@ -39,13 +39,13 @@ def test_creates_new_experiment_and_side_effects( user, model_with_version, protocol_with_version): model = model_with_version protocol = protocol_with_version - model_version = model.repo.latest_commit.sha - protocol_version = protocol.repo.latest_commit.sha + model_version = model.repocache.latest_version + protocol_version = protocol.repocache.latest_version assert Experiment.objects.count() == 0 assert RunningExperiment.objects.count() == 0 - version, is_new = submit_experiment(model, model_version, protocol, protocol_version, user, False) + version, is_new = submit_experiment(model_version, protocol_version, user, False) assert is_new # Check properties of the new experiment & version @@ -53,16 +53,16 @@ def test_creates_new_experiment_and_side_effects( assert version.experiment.model == model assert version.experiment.protocol == protocol assert version.author == user - assert version.experiment.model_version.sha == model_version - assert version.experiment.protocol_version.sha == protocol_version + assert version.experiment.model_version == model_version + assert version.experiment.protocol_version == protocol_version assert version.experiment.author == user assert version.status == ExperimentVersion.STATUS_QUEUED # Check it did submit to the webservice - model_url = '/entities/models/%d/versions/%s/archive' % (model.pk, model_version) + model_url = '/entities/models/%d/versions/%s/archive' % (model.pk, model_version.sha) protocol_url = ( '/entities/protocols/%d/versions/%s/archive' % - (protocol.pk, protocol_version)) + (protocol.pk, protocol_version.sha)) assert mock_post.call_count == 1 assert mock_post.call_args[0][0] == settings.CHASTE_URL @@ -106,8 +106,7 @@ def test_uses_existing_experiment(self, mock_post, protocol=protocol, protocol_version=protocol_version) - version, is_new = submit_experiment(model, model_version.sha, - protocol, protocol_version.sha, user, False) + version, is_new = submit_experiment(model_version, protocol_version, user, False) assert is_new assert version.experiment == experiment @@ -121,7 +120,7 @@ def test_raises_exception_on_webservice_error(self, mock_post, mock_post.side_effect = generate_response('something %s') with pytest.raises(ProcessingException): - submit_experiment(model, model_version.sha, protocol, protocol_version.sha, user, False) + submit_experiment(model_version, protocol_version, user, False) # There should be no running experiment assert RunningExperiment.objects.count() == 0 @@ -144,7 +143,7 @@ def test_records_submission_error(self, mock_post, mock_post.side_effect = generate_response('%s an error occurred') - version, is_new = submit_experiment(model, model_version.sha, protocol, protocol_version.sha, user, False) + version, is_new = submit_experiment(model_version, protocol_version, user, False) assert is_new assert version.status == ExperimentVersion.STATUS_FAILED @@ -160,7 +159,7 @@ def test_records_inapplicable_result(self, mock_post, mock_post.side_effect = generate_response('%s inapplicable') - version, is_new = submit_experiment(model, model_version.sha, protocol, protocol_version.sha, user, False) + version, is_new = submit_experiment(model_version, protocol_version, user, False) assert is_new assert version.status == ExperimentVersion.STATUS_INAPPLICABLE diff --git a/weblab/experiments/views.py b/weblab/experiments/views.py index 0514d8784..bd4fa0dae 100644 --- a/weblab/experiments/views.py +++ b/weblab/experiments/views.py @@ -314,22 +314,24 @@ def post(self, request, *args, **kwargs): exp = exp_ver.experiment model = exp.model protocol = exp.protocol - model_version = exp.model_version.sha - protocol_version = exp.protocol_version.sha + model_sha = exp.model_version.sha + protocol_sha = exp.protocol_version.sha else: model = get_object_or_404(ModelEntity, pk=request.POST['model']) protocol = get_object_or_404(ProtocolEntity, pk=request.POST['protocol']) - model_version = request.POST['model_version'] - protocol_version = request.POST['protocol_version'] + model_sha = request.POST['model_version'] + protocol_sha = request.POST['protocol_version'] - version, is_new = submit_experiment(model, model_version, protocol, protocol_version, + model_version = model.repocache.get_version(model_sha) + protocol_version = protocol.repocache.get_version(protocol_sha) + version, is_new = submit_experiment(model_version, protocol_version, request.user, 'rerun' in request.POST or 'planned' in request.POST) queued = version.status == ExperimentVersion.STATUS_QUEUED if is_new and version.status != ExperimentVersion.STATUS_FAILED: # Remove from planned experiments PlannedExperiment.objects.filter( - model=model, model_version=model_version, - protocol=protocol, protocol_version=protocol_version + model=model, model_version=model_sha, + protocol=protocol, protocol_version=protocol_sha ).delete() version_url = reverse('experiments:version', From 1e6dc4daa4ee79cbd0b328609b8086514e885958 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 15 Sep 2020 10:40:19 +0000 Subject: [PATCH 07/37] Add messages to fitting experiment submission --- weblab/fitting/views.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index f5083e739..9906296c6 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -239,6 +239,16 @@ def form_valid(self, form): False, ) + queued = self.runnable.status == FittingResultVersion.STATUS_QUEUED + + if is_new: + if queued: + messages.info(self.request, "Fitting experiment submitted to the queue.") + else: + messages.error(self.request, "Fitting experiment could not be run: " + self.runnable.return_text) + else: + messages.info(self.request, "Fitting experiment was already run.") + return super().form_valid(form) def get_success_url(self): From 1fa7533322a61366b9d903d7623e3862d372b9f7 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 15 Sep 2020 11:26:00 +0000 Subject: [PATCH 08/37] Allow reruns of fitting experiments --- weblab/fitting/tests/test_views.py | 2 +- weblab/fitting/views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index b5e367706..c4fdca085 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -501,7 +501,7 @@ def test_submits_to_backend(self, mock_submit, client, fits_user, public_model, model_version, protocol_version, fittingspec_version, - public_dataset, fits_user, False + public_dataset, fits_user, True ) assert response.url == '/fitting/results/%d/versions/%d' % (runnable.fittingresult.pk, runnable.pk) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 9906296c6..ed430c258 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -236,7 +236,7 @@ def form_valid(self, form): form.cleaned_data['fittingspec_version'], form.cleaned_data['dataset'], self.request.user, - False, + True, ) queued = self.runnable.status == FittingResultVersion.STATUS_QUEUED From 15c3fa93f09674a218577f804f1756fc75b1a36c Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 15 Sep 2020 11:50:02 +0000 Subject: [PATCH 09/37] Add basic javascript for fitting result form Initial functionality - deselect / disable versions when no entity selected Also a little bit of styling for that page, and a submit button --- weblab/static/js/fitting.js | 38 +++++++++++++++++++ weblab/static/js/main.js | 5 +++ .../fitting/fittingresult_create_form.html | 11 +++++- 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 weblab/static/js/fitting.js diff --git a/weblab/static/js/fitting.js b/weblab/static/js/fitting.js new file mode 100644 index 000000000..92605e7c1 --- /dev/null +++ b/weblab/static/js/fitting.js @@ -0,0 +1,38 @@ + +function init() { + var $model = $("#id_model"); + var $modelVersion = $("#id_model_version"); + var $protocol = $("#id_protocol"); + var $protocolVersion = $("#id_protocol_version"); + var $fittingSpec = $("#id_fittingspec"); + var $fittingSpecVersion = $("#id_fittingspec_version"); + var $dataset = $("#id_dataset"); + + function updateDropdowns() { + var modelId = parseInt($model.val(), 10); + var protocolId = parseInt($protocol.val(), 10); + var fittingSpecId = parseInt($fittingSpec.val(), 10); + var datasetId = parseInt($dataset.val(), 10); + + console.log(modelId, protocolId, fittingSpecId, datasetId); + + // Clear selected version if no entity selected + if (isNaN(modelId)) $modelVersion.val(''); + if (isNaN(protocolId)) $protocolVersion.val(''); + if (isNaN(fittingSpecId)) $fittingSpecVersion.val(''); + + // Disable version dropdowns if entity not selected + $modelVersion.prop('disabled', isNaN(modelId)); + $protocolVersion.prop('disabled', isNaN(protocolId)); + $fittingSpecVersion.prop('disabled', isNaN(fittingSpecId)); + } + + $("form select").change(updateDropdowns); + + updateDropdowns(); +} + + +module.exports = { + init: init, +}; diff --git a/weblab/static/js/main.js b/weblab/static/js/main.js index 96af8a9a8..a1b860c0e 100644 --- a/weblab/static/js/main.js +++ b/weblab/static/js/main.js @@ -8,6 +8,7 @@ require('./db.js'); var entity = require('./entity.js'); var experiment = require('./experiment.js'); var notifications = require('./lib/notifications.js'); +var fitting = require('./fitting.js'); require('./compare.js'); require('./entity_version_list.js'); require('./experiment_tasks.js'); @@ -168,6 +169,10 @@ function initPage () addText: 'add another', deleteText: 'remove', }); + + if ($('#fitting-result-submit').length > 0) { + fitting.init(); + } } document.addEventListener("DOMContentLoaded", initPage, false); diff --git a/weblab/templates/fitting/fittingresult_create_form.html b/weblab/templates/fitting/fittingresult_create_form.html index 9f8313953..8bbd1f8f9 100644 --- a/weblab/templates/fitting/fittingresult_create_form.html +++ b/weblab/templates/fitting/fittingresult_create_form.html @@ -2,8 +2,17 @@ {% block title %}Submit fitting experiment - {% endblock title %} +{% block body_id %}fitting-result-submit{% endblock body_id %} + {% block content %} -{{ form }} +
+ {% csrf_token %} + {{ form.as_p }} + +

+ +

+
{% endblock content %} From 219c29b10c56170d94b71c2992f2c61b143be150 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Thu, 17 Sep 2020 11:45:15 +0000 Subject: [PATCH 10/37] Add json endpoint for filtering fitting form --- weblab/fitting/tests/test_views.py | 172 +++++++++++++++++++++++++++++ weblab/fitting/urls.py | 7 ++ weblab/fitting/views.py | 74 +++++++++++++ 3 files changed, 253 insertions(+) diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index c4fdca085..4caac77e6 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -505,3 +505,175 @@ def test_submits_to_backend(self, mock_submit, client, fits_user, public_model, ) assert response.url == '/fitting/results/%d/versions/%d' % (runnable.fittingresult.pk, runnable.pk) + + +@pytest.mark.django_db +class TestFittingResultFilterJsonView: + def test_requires_login(self, client): + response = client.get('/fitting/results/new/filter') + assert response.status_code == 302 + + def test_requires_permission(self, client, logged_in_user): + response = client.get('/fitting/results/new/filter') + assert response.status_code == 302 + + def test_all_models_and_versions(self, client, fits_user, helpers): + model1 = recipes.model.make() + model2 = recipes.model.make() + m1v1 = helpers.cached_version(model1) + m2v1 = helpers.cached_version(model2) + + response = client.get('/fitting/results/new/filter', {}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + assert 'fittingResultOptions' in data + options = data['fittingResultOptions'] + assert set(options['models']) == {model1.id, model2.id} + assert set(options['model_versions']) == {m1v1.id, m2v1.id} + + def test_all_protocols_and_versions(self, client, fits_user, helpers): + protocol1 = recipes.protocol.make() + protocol2 = recipes.protocol.make() + p1v1 = helpers.cached_version(protocol1) + p2v1 = helpers.cached_version(protocol2) + + response = client.get('/fitting/results/new/filter', {}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert set(options['protocols']) == {protocol1.id, protocol2.id} + assert set(options['protocol_versions']) == {p1v1.id, p2v1.id} + + def test_all_fittingspecs_and_versions(self, client, fits_user, helpers): + fittingspec1 = recipes.fittingspec.make() + fittingspec2 = recipes.fittingspec.make() + f1v1 = helpers.cached_version(fittingspec1) + f2v1 = helpers.cached_version(fittingspec2) + + response = client.get('/fitting/results/new/filter', {}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert set(options['fittingspecs']) == {fittingspec1.id, fittingspec2.id} + assert set(options['fittingspec_versions']) == {f1v1.id, f2v1.id} + + def test_all_datasets(self, client, fits_user): + dataset1 = recipes.dataset.make() + dataset2 = recipes.dataset.make() + + response = client.get('/fitting/results/new/filter', {}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert set(options['datasets']) == {dataset1.id, dataset2.id} + + def test_model_and_versions_restricted_when_model_selected(self, client, fits_user, helpers): + model1 = recipes.model.make() + model2 = recipes.model.make() + m1v1 = helpers.cached_version(model1) + m2v1 = helpers.cached_version(model2) # noqa: F841 + + response = client.get('/fitting/results/new/filter', {'model': model1.id}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert options['models'] == [model1.id] + assert options['model_versions'] == [m1v1.id] + + def test_protocol_and_versions_restricted_when_protocol_selected(self, client, fits_user, helpers): + protocol1 = recipes.protocol.make() + protocol2 = recipes.protocol.make() + p1v1 = helpers.cached_version(protocol1) + p2v1 = helpers.cached_version(protocol2) # noqa: F841 + + response = client.get('/fitting/results/new/filter', {'protocol': protocol1.id}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert options['protocols'] == [protocol1.id] + assert options['protocol_versions'] == [p1v1.id] + + def test_fittingspec_and_versions_restricted_when_fittingspec_selected(self, client, fits_user, helpers): + fittingspec1 = recipes.fittingspec.make() + fittingspec2 = recipes.fittingspec.make() + f1v1 = helpers.cached_version(fittingspec1) + f2v1 = helpers.cached_version(fittingspec2) # noqa: F841 + + response = client.get('/fitting/results/new/filter', {'fittingspec': fittingspec1.id}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert options['fittingspecs'] == [fittingspec1.id] + assert options['fittingspec_versions'] == [f1v1.id] + + def test_dataset_restricted_when_selected(self, client, fits_user, helpers): + dataset1 = recipes.dataset.make() + dataset2 = recipes.dataset.make() # noqa: F841 + + response = client.get('/fitting/results/new/filter', {'dataset': dataset1.id}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + assert 'fittingResultOptions' in data + options = data['fittingResultOptions'] + assert options['datasets'] == [dataset1.id] + + def test_dataset_and_fittingspec_restricted_when_protocol_selected(self, client, fits_user, helpers): + protocol = recipes.protocol.make() + fittingspec1 = recipes.fittingspec.make(protocol=protocol) + fittingspec2 = recipes.fittingspec.make() + f1v1 = helpers.cached_version(fittingspec1) + f2v1 = helpers.cached_version(fittingspec2) # noqa: F841 + dataset1 = recipes.dataset.make(protocol=protocol) + dataset2 = recipes.dataset.make() # noqa: F841 + + response = client.get('/fitting/results/new/filter', {'protocol': protocol.id}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert options['datasets'] == [dataset1.id] + assert options['fittingspecs'] == [fittingspec1.id] + assert options['fittingspec_versions'] == [f1v1.id] + + def test_protocol_and_fittingspec_restricted_when_dataset_selected(self, client, fits_user, helpers): + protocol1 = recipes.protocol.make() + protocol2 = recipes.protocol.make() + p1v1 = helpers.cached_version(protocol1) + p2v1 = helpers.cached_version(protocol2) # noqa: F841 + + dataset = recipes.dataset.make(protocol=protocol1) + + response = client.get('/fitting/results/new/filter', {'dataset': dataset.id}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert options['protocols'] == [protocol1.id] + assert options['protocol_versions'] == [p1v1.id] + + def test_protocol_and_dataset_restricted_when_fittingspec_selected(self, client, fits_user, helpers): + protocol1 = recipes.protocol.make() + protocol2 = recipes.protocol.make() + p1v1 = helpers.cached_version(protocol1) + p2v1 = helpers.cached_version(protocol2) # noqa: F841 + dataset1 = recipes.dataset.make(protocol=protocol1) + dataset2 = recipes.dataset.make() # noqa: F841 + + fittingspec = recipes.fittingspec.make(protocol=protocol1) + + response = client.get('/fitting/results/new/filter', {'fittingspec': fittingspec.id}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + options = data['fittingResultOptions'] + assert options['protocols'] == [protocol1.id] + assert options['protocol_versions'] == [p1v1.id] + assert options['datasets'] == [dataset1.id] diff --git a/weblab/fitting/urls.py b/weblab/fitting/urls.py index 244f40b9a..35cc67cf0 100644 --- a/weblab/fitting/urls.py +++ b/weblab/fitting/urls.py @@ -72,6 +72,13 @@ views.FittingResultCreateView.as_view(), name='new' ), + + url( + r'^new/filter$', + views.FittingResultFilterJsonView.as_view(), + name='filter_json', + ), + ] urlpatterns = [ diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index ed430c258..14b96436a 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -13,6 +13,7 @@ from braces.views import UserFormKwargsMixin from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin +from django.core.exceptions import ObjectDoesNotExist from django.http import JsonResponse from django.shortcuts import get_object_or_404 from django.urls import reverse @@ -27,6 +28,7 @@ from datasets.models import Dataset from entities.models import ModelEntity, ProtocolEntity from entities.views import EntityNewVersionView, EntityTypeMixin +from repocache.models import CachedFittingSpecVersion, CachedModelVersion, CachedProtocolVersion from .forms import FittingResultCreateForm, FittingSpecForm, FittingSpecVersionForm from .models import FittingResult, FittingResultVersion, FittingSpec @@ -254,3 +256,75 @@ def form_valid(self, form): def get_success_url(self): if hasattr(self, 'runnable'): return reverse('fitting:result:version', args=[self.runnable.fittingresult.pk, self.runnable.pk]) + + +class FittingResultFilterJsonView(LoginRequiredMixin, PermissionRequiredMixin, View): + permission_required = 'fitting.run_fits' + + def get(self, request, *args, **kwargs): + options = {} + + models = ModelEntity.objects.all() + model_versions = CachedModelVersion.objects.all() + protocols = ProtocolEntity.objects.all() + protocol_versions = CachedProtocolVersion.objects.all() + fittingspecs = FittingSpec.objects.all() + fittingspec_versions = CachedFittingSpecVersion.objects.all() + datasets = Dataset.objects.all() + + def _get_int_param(fieldname, _model): + try: + pk = int(request.GET.get(fieldname, '')) + return _model.objects.get(pk=pk) + except ValueError: + pass + except ObjectDoesNotExist: + pass + + model = _get_int_param('model', ModelEntity) + protocol = _get_int_param('protocol', ProtocolEntity) + fittingspec = _get_int_param('fittingspec', FittingSpec) + dataset = _get_int_param('dataset', Dataset) + + if not protocol: + if fittingspec: + protocol = fittingspec.protocol + elif dataset: + protocol = dataset.protocol + + if model: + models = [model] + model_versions = model_versions.filter(entity__entity=model) + + if fittingspec: + fittingspecs = [fittingspec] + fittingspec_versions = fittingspec_versions.filter(entity__entity=fittingspec.id) + + if dataset: + datasets = [dataset] + + if protocol: + protocols = [protocol] + protocol_versions = protocol_versions.filter(entity__entity=protocol.id) + + if not fittingspec: + fittingspecs = fittingspecs.filter(protocol=protocol.id) + fittingspec_versions = fittingspec_versions.filter(entity__entity__in=fittingspecs) + + if not dataset: + datasets = datasets.filter(protocol=protocol.id) + + def _get_ids(qs): + return [item.id for item in qs] + + options['models'] = _get_ids(models) + options['model_versions'] = _get_ids(model_versions) + options['protocols'] = _get_ids(protocols) + options['protocol_versions'] = _get_ids(protocol_versions) + options['fittingspecs'] = _get_ids(fittingspecs) + options['fittingspec_versions'] = _get_ids(fittingspec_versions) + options['datasets'] = _get_ids(datasets) + + return JsonResponse({ + 'fittingResultOptions': options + }) From 9fbd961987153508c0161d452ac0db719e527c9a Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Fri, 18 Sep 2020 09:23:11 +0000 Subject: [PATCH 11/37] Apply restrictions to fitting form dropdowns --- weblab/static/js/fitting.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/weblab/static/js/fitting.js b/weblab/static/js/fitting.js index 92605e7c1..902d7c54a 100644 --- a/weblab/static/js/fitting.js +++ b/weblab/static/js/fitting.js @@ -25,6 +25,38 @@ function init() { $modelVersion.prop('disabled', isNaN(modelId)); $protocolVersion.prop('disabled', isNaN(protocolId)); $fittingSpecVersion.prop('disabled', isNaN(fittingSpecId)); + + function restrictIds(idList, $dropdown) { + $dropdown.find("option").each(function(i, opt) { + if (opt.value.length > 0) { + $(opt).toggle( + idList.includes(parseInt(opt.value, 10)) + ); + } + }); + } + + $.getJSON("/fitting/results/new/filter", + { + 'model': modelId, + 'protocol': protocolId, + 'fittingspec': fittingSpecId, + 'dataset': datasetId, + }, + function(json) { + if (json.fittingResultOptions) { + var results = json.fittingResultOptions; + + restrictIds(results.models, $model); + restrictIds(results.model_versions, $modelVersion); + restrictIds(results.protocols, $protocol); + restrictIds(results.protocol_versions, $protocolVersion); + restrictIds(results.fittingspecs, $fittingSpec); + restrictIds(results.fittingspec_versions, $fittingSpecVersion); + restrictIds(results.datasets, $dataset); + } + } + ); } $("form select").change(updateDropdowns); From 43f2f022acf3f75a3bb3c131cefe78d5162b1494 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Mon, 21 Sep 2020 10:18:22 +0000 Subject: [PATCH 12/37] Enforce visibility on fitting form --- weblab/conftest.py | 48 +++++- weblab/core/recipes.py | 18 +-- weblab/fitting/forms.py | 37 ++++- weblab/fitting/tests/test_forms.py | 213 +++++++++++++++++++------- weblab/fitting/tests/test_views.py | 2 +- weblab/fitting/views.py | 2 +- weblab/repocache/models.py | 24 +++ weblab/repocache/tests/test_models.py | 68 +++++++- 8 files changed, 329 insertions(+), 83 deletions(-) diff --git a/weblab/conftest.py b/weblab/conftest.py index 5216c07c4..86317e5a3 100644 --- a/weblab/conftest.py +++ b/weblab/conftest.py @@ -13,6 +13,7 @@ from core import recipes from datasets.models import Dataset from entities.models import Entity +from fitting.models import FittingResult from repocache.populate import populate_entity_cache @@ -167,28 +168,28 @@ def fittingspec_with_version(): @pytest.fixture def public_model(helpers): - model = recipes.model.make() + model = recipes.model.make(name='public model') helpers.add_version(model, visibility='public') return model @pytest.fixture def public_protocol(helpers): - protocol = recipes.protocol.make() + protocol = recipes.protocol.make(name='public protocol') helpers.add_version(protocol, visibility='public') return protocol @pytest.fixture def public_fittingspec(helpers): - fittingspec = recipes.fittingspec.make() + fittingspec = recipes.fittingspec.make(name='public fitting spec') helpers.add_version(fittingspec, visibility='public') return fittingspec @pytest.fixture -def public_dataset(helpers): - dataset = recipes.dataset.make(visibility='public') +def public_dataset(): + dataset = recipes.dataset.make(visibility='public', name='public dataset') return dataset @@ -206,6 +207,32 @@ def moderated_protocol(helpers): return protocol +@pytest.fixture +def private_model(helpers): + model = recipes.model.make(name='private model') + helpers.add_version(model, visibility='private') + return model + + +@pytest.fixture +def private_protocol(helpers): + protocol = recipes.protocol.make(name='private protocol') + helpers.add_version(protocol, visibility='private') + return protocol + + +@pytest.fixture +def private_fittingspec(helpers): + fittingspec = recipes.fittingspec.make(name='private fittingspec') + helpers.add_version(fittingspec, visibility='private') + return fittingspec + + +@pytest.fixture +def private_dataset(): + return recipes.dataset.make(visibility='private') + + @pytest.fixture def queued_experiment(model_with_version, protocol_with_version): version = recipes.experiment_version.make( @@ -334,6 +361,17 @@ def moderator(user, helpers): return user +@pytest.fixture +def fits_user(logged_in_user): + content_type = ContentType.objects.get_for_model(FittingResult) + permission = Permission.objects.get( + codename='run_fits', + content_type=content_type, + ) + logged_in_user.user_permissions.add(permission) + return logged_in_user + + @pytest.fixture def dataset_creator(user, helpers): helpers.add_permission(user, 'create_dataset', Dataset) diff --git a/weblab/core/recipes.py b/weblab/core/recipes.py index 1b289b485..4d16c227d 100644 --- a/weblab/core/recipes.py +++ b/weblab/core/recipes.py @@ -22,17 +22,17 @@ analysis_task = Recipe('AnalysisTask', entity=foreign_key(protocol)) -cached_model = Recipe('CachedModel') -cached_model_version = Recipe('CachedModelVersion') -cached_model_tag = Recipe('CachedModelTag') +cached_model = Recipe('CachedModel', entity=foreign_key(model)) +cached_model_version = Recipe('CachedModelVersion', entity=foreign_key(cached_model)) +cached_model_tag = Recipe('CachedModelTag', entity=foreign_key(cached_model)) -cached_protocol = Recipe('CachedProtocol') -cached_protocol_version = Recipe('CachedProtocolVersion') -cached_protocol_tag = Recipe('CachedProtocolTag') +cached_protocol = Recipe('CachedProtocol', entity=foreign_key(protocol)) +cached_protocol_version = Recipe('CachedProtocolVersion', entity=foreign_key(cached_protocol)) +cached_protocol_tag = Recipe('CachedProtocolTag', entity=foreign_key(cached_protocol)) -cached_fittingspec = Recipe('CachedFittingSpec') -cached_fittingspec_version = Recipe('CachedFittingSpecVersion') -cached_fittingspec_tag = Recipe('CachedFittingSpecTag') +cached_fittingspec = Recipe('CachedFittingSpec', entity=foreign_key(fittingspec)) +cached_fittingspec_version = Recipe('CachedFittingSpecVersion', entity=foreign_key(cached_fittingspec)) +cached_fittingspec_tag = Recipe('CachedFittingSpecTag', entity=foreign_key(cached_fittingspec)) experiment = Recipe( 'Experiment', diff --git a/weblab/fitting/forms.py b/weblab/fitting/forms.py index aa0b734d7..47760d55d 100644 --- a/weblab/fitting/forms.py +++ b/weblab/fitting/forms.py @@ -1,8 +1,11 @@ +from braces.forms import UserKwargModelFormMixin from django import forms from django.core.exceptions import ValidationError +from datasets.models import Dataset from entities.forms import EntityForm, EntityVersionForm -from entities.models import ProtocolEntity +from entities.models import ModelEntity, ProtocolEntity +from repocache.models import CachedFittingSpecVersion, CachedModelVersion, CachedProtocolVersion from .models import FittingResult, FittingSpec @@ -29,24 +32,44 @@ class FittingSpecVersionForm(EntityVersionForm): rerun_expts = None -class FittingResultCreateForm(forms.ModelForm): +class FittingResultCreateForm(UserKwargModelFormMixin, forms.ModelForm): class Meta: model = FittingResult fields = ('model', 'model_version', 'protocol', 'protocol_version', 'fittingspec', 'fittingspec_version', 'dataset') + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # Ensure only visible entities and versions are available to user + self.fields['model'].queryset = ModelEntity.objects.visible_to_user(self.user) + self.fields['protocol'].queryset = ProtocolEntity.objects.visible_to_user(self.user) + self.fields['fittingspec'].queryset = FittingSpec.objects.visible_to_user(self.user) + self.fields['dataset'].queryset = Dataset.objects.visible_to_user(self.user) + + self.fields['model_version'].queryset = CachedModelVersion.objects.visible_to_user(self.user) + self.fields['protocol_version'].queryset = CachedProtocolVersion.objects.visible_to_user(self.user) + self.fields['fittingspec_version'].queryset = CachedFittingSpecVersion.objects.visible_to_user(self.user) + def clean(self): - if self.cleaned_data['model_version'].model != self.cleaned_data['model']: + # Check ownership of versions by entities + model_version = self.cleaned_data.get('model_version') + if model_version and model_version.model != self.cleaned_data.get('model'): raise ValidationError({'model_version': 'Model version must belong to model'}) - if self.cleaned_data['protocol_version'].protocol != self.cleaned_data['protocol']: + protocol_version = self.cleaned_data.get('protocol_version') + if protocol_version and protocol_version.protocol != self.cleaned_data['protocol']: raise ValidationError({'protocol_version': 'Protocol version must belong to protocol'}) - if self.cleaned_data['fittingspec_version'].fittingspec != self.cleaned_data['fittingspec']: + fittingspec_version = self.cleaned_data.get('fittingspec_version') + if fittingspec_version and fittingspec_version.fittingspec != self.cleaned_data['fittingspec']: raise ValidationError({'fittingspec_version': 'Fitting spec version must belong to fitting spec'}) - if self.cleaned_data['dataset'].protocol != self.cleaned_data['protocol']: + # Check linkage between protocols, datasets and fitting specs + protocol = self.cleaned_data.get('protocol') + dataset = self.cleaned_data.get('dataset') + if dataset and dataset.protocol != protocol: raise ValidationError({'protocol': 'Protocol and dataset must match'}) - if self.cleaned_data['fittingspec'].protocol != self.cleaned_data['protocol']: + fittingspec = self.cleaned_data.get('fittingspec') + if fittingspec and fittingspec.protocol != protocol: raise ValidationError({'protocol': 'Protocol and fitting spec must match'}) diff --git a/weblab/fitting/tests/test_forms.py b/weblab/fitting/tests/test_forms.py index 7fd7adda7..aec1b8b57 100644 --- a/weblab/fitting/tests/test_forms.py +++ b/weblab/fitting/tests/test_forms.py @@ -6,8 +6,8 @@ @pytest.mark.django_db class TestFittingResultCreateForm: - def test_fields(self): - form = FittingResultCreateForm() + def test_fields(self, fits_user): + form = FittingResultCreateForm(user=fits_user) assert 'model' in form.fields assert 'model_version' in form.fields assert 'protocol' in form.fields @@ -17,113 +17,208 @@ def test_fields(self): assert 'dataset' in form.fields def test_valid_form( - self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset, helpers + self, public_model, public_protocol, + public_fittingspec, public_dataset, fits_user, helpers ): - helpers.link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + helpers.link_to_protocol(public_protocol, public_dataset, public_fittingspec) form = FittingResultCreateForm({ - 'model': model_with_version.pk, - 'model_version': model_with_version.repocache.latest_version.pk, - 'protocol': protocol_with_version.pk, - 'protocol_version': protocol_with_version.repocache.latest_version.pk, - 'fittingspec': fittingspec_with_version.pk, - 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'model': public_model.pk, + 'model_version': public_model.repocache.latest_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': public_protocol.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': public_fittingspec.repocache.latest_version.pk, 'dataset': public_dataset.pk, - }) + }, user=fits_user) assert form.is_valid() + def test_model_version_must_be_visible( + self, private_model, public_protocol, + public_fittingspec, public_dataset, fits_user, helpers + ): + helpers.link_to_protocol(public_protocol, public_dataset, public_fittingspec) + + form = FittingResultCreateForm({ + 'model': private_model.pk, + 'model_version': private_model.repocache.latest_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': public_protocol.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': public_fittingspec.repocache.latest_version.pk, + 'dataset': public_dataset.pk, + }, user=fits_user) + assert not form.is_valid() + assert 'model_version' in form.errors + assert form.errors['model_version'][0].startswith("Select a valid choice.") + + def test_protocol_version_must_be_visible( + self, public_model, private_protocol, + public_fittingspec, public_dataset, fits_user, helpers + ): + helpers.link_to_protocol(private_protocol, public_dataset, public_fittingspec) + + form = FittingResultCreateForm({ + 'model': public_model.pk, + 'model_version': public_model.repocache.latest_version.pk, + 'protocol': private_protocol.pk, + 'protocol_version': private_protocol.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': public_fittingspec.repocache.latest_version.pk, + 'dataset': public_dataset.pk, + }, user=fits_user) + assert not form.is_valid() + assert 'protocol_version' in form.errors + assert form.errors['protocol_version'][0].startswith("Select a valid choice.") + + def test_fittingspec_version_must_be_visible( + self, public_model, public_protocol, + private_fittingspec, public_dataset, fits_user, helpers + ): + helpers.link_to_protocol(public_protocol, public_dataset, private_fittingspec) + + form = FittingResultCreateForm({ + 'model': public_model.pk, + 'model_version': public_model.repocache.latest_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': public_protocol.repocache.latest_version.pk, + 'fittingspec': private_fittingspec.pk, + 'fittingspec_version': private_fittingspec.repocache.latest_version.pk, + 'dataset': public_dataset.pk, + }, user=fits_user) + assert not form.is_valid() + assert 'fittingspec_version' in form.errors + assert form.errors['fittingspec_version'][0].startswith("Select a valid choice.") + + def test_dataset_must_be_visible( + self, public_model, public_protocol, public_fittingspec, private_dataset, fits_user, helpers + ): + helpers.link_to_protocol(public_protocol, private_dataset, public_fittingspec) + + form = FittingResultCreateForm({ + 'model': public_model.pk, + 'model_version': public_model.repocache.latest_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': public_protocol.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': public_fittingspec.repocache.latest_version.pk, + 'dataset': private_dataset.pk, + }, user=fits_user) + assert not form.is_valid() + assert 'dataset' in form.errors + assert form.errors['dataset'][0].startswith("Select a valid choice.") + + def test_only_shows_visible_models(self, private_model, public_model, fits_user): + form = FittingResultCreateForm(user=fits_user) + assert form.fields['model'].valid_value(public_model.pk) + assert not form.fields['model'].valid_value(private_model.pk) + + def test_only_shows_visible_protocols(self, private_protocol, public_protocol, fits_user): + form = FittingResultCreateForm(user=fits_user) + assert form.fields['protocol'].valid_value(public_protocol.pk) + assert not form.fields['protocol'].valid_value(private_protocol.pk) + + def test_only_shows_visible_fittingspecs(self, private_fittingspec, public_fittingspec, fits_user): + form = FittingResultCreateForm(user=fits_user) + assert form.fields['fittingspec'].valid_value(public_fittingspec.pk) + assert not form.fields['fittingspec'].valid_value(private_fittingspec.pk) + + def test_only_shows_visible_datasets(self, private_dataset, public_dataset, fits_user): + form = FittingResultCreateForm(user=fits_user) + assert form.fields['dataset'].valid_value(public_dataset.pk) + assert not form.fields['dataset'].valid_value(private_dataset.pk) + def test_model_version_must_belong_to_model( - self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset, helpers + self, public_model, public_protocol, + public_fittingspec, public_dataset, fits_user, helpers ): - helpers.link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + helpers.link_to_protocol(public_protocol, public_dataset, public_fittingspec) invalid_version = recipes.cached_model_version.make() form = FittingResultCreateForm({ - 'model': model_with_version.pk, + 'model': public_model.pk, 'model_version': invalid_version.pk, - 'protocol': protocol_with_version.pk, - 'protocol_version': protocol_with_version.repocache.latest_version.pk, - 'fittingspec': fittingspec_with_version.pk, - 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': public_protocol.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': public_fittingspec.repocache.latest_version.pk, 'dataset': public_dataset.pk, - }) + }, user=fits_user) assert not form.is_valid() assert 'model_version' in form.errors def test_protocol_version_must_belong_to_protocol( - self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset, helpers + self, public_model, public_protocol, + public_fittingspec, public_dataset, fits_user, helpers ): - helpers.link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + helpers.link_to_protocol(public_protocol, public_dataset, public_fittingspec) invalid_version = recipes.cached_protocol_version.make() form = FittingResultCreateForm({ - 'model': model_with_version.pk, - 'model_version': model_with_version.repocache.latest_version.pk, - 'protocol': protocol_with_version.pk, + 'model': public_model.pk, + 'model_version': public_model.repocache.latest_version.pk, + 'protocol': public_protocol.pk, 'protocol_version': invalid_version.pk, - 'fittingspec': fittingspec_with_version.pk, - 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': public_fittingspec.repocache.latest_version.pk, 'dataset': public_dataset.pk, - }) + }, user=fits_user) assert not form.is_valid() assert 'protocol_version' in form.errors def test_fittingspec_version_must_belong_to_fittingspec( - self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset, helpers + self, public_model, public_protocol, + public_fittingspec, public_dataset, fits_user, helpers ): - helpers.link_to_protocol(protocol_with_version, public_dataset, fittingspec_with_version) + helpers.link_to_protocol(public_protocol, public_dataset, public_fittingspec) invalid_version = recipes.cached_fittingspec_version.make() form = FittingResultCreateForm({ - 'model': model_with_version.pk, - 'model_version': model_with_version.repocache.latest_version.pk, - 'protocol': protocol_with_version.pk, - 'protocol_version': protocol_with_version.repocache.latest_version.pk, - 'fittingspec': fittingspec_with_version.pk, + 'model': public_model.pk, + 'model_version': public_model.repocache.latest_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': public_protocol.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, 'fittingspec_version': invalid_version.pk, 'dataset': public_dataset.pk, - }) + }, user=fits_user) assert not form.is_valid() assert 'fittingspec_version' in form.errors def test_protocol_and_fittingspec_must_be_linked( - self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset, helpers + self, public_model, public_protocol, + public_fittingspec, public_dataset, fits_user, helpers ): - helpers.link_to_protocol(protocol_with_version, public_dataset) + helpers.link_to_protocol(public_protocol, public_dataset) form = FittingResultCreateForm({ - 'model': model_with_version.pk, - 'model_version': model_with_version.repocache.latest_version.pk, - 'protocol': protocol_with_version.pk, - 'protocol_version': protocol_with_version.repocache.latest_version.pk, - 'fittingspec': fittingspec_with_version.pk, - 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'model': public_model.pk, + 'model_version': public_model.repocache.latest_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': public_protocol.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': public_fittingspec.repocache.latest_version.pk, 'dataset': public_dataset.pk, - }) + }, user=fits_user) assert not form.is_valid() assert 'protocol' in form.errors def test_protocol_and_dataset_must_be_linked( - self, model_with_version, protocol_with_version, - fittingspec_with_version, public_dataset, helpers + self, public_model, public_protocol, + public_fittingspec, public_dataset, fits_user, helpers ): - helpers.link_to_protocol(protocol_with_version, fittingspec_with_version) + helpers.link_to_protocol(public_protocol, public_fittingspec) form = FittingResultCreateForm({ - 'model': model_with_version.pk, - 'model_version': model_with_version.repocache.latest_version.pk, - 'protocol': protocol_with_version.pk, - 'protocol_version': protocol_with_version.repocache.latest_version.pk, - 'fittingspec': fittingspec_with_version.pk, - 'fittingspec_version': fittingspec_with_version.repocache.latest_version.pk, + 'model': public_model.pk, + 'model_version': public_model.repocache.latest_version.pk, + 'protocol': public_protocol.pk, + 'protocol_version': public_protocol.repocache.latest_version.pk, + 'fittingspec': public_fittingspec.pk, + 'fittingspec_version': public_fittingspec.repocache.latest_version.pk, 'dataset': public_dataset.pk, - }) + }, user=fits_user) assert not form.is_valid() assert 'protocol' in form.errors diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index 4caac77e6..aa324704d 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -575,7 +575,7 @@ def test_model_and_versions_restricted_when_model_selected(self, client, fits_us model1 = recipes.model.make() model2 = recipes.model.make() m1v1 = helpers.cached_version(model1) - m2v1 = helpers.cached_version(model2) # noqa: F841 + m2v1 = helpers.cached_version(model2) # noqa: F841 response = client.get('/fitting/results/new/filter', {'model': model1.id}) assert response.status_code == 200 diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 14b96436a..9a0c001a1 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -205,7 +205,7 @@ def get(self, request, *args, **kwargs): return JsonResponse(response) -class FittingResultCreateView(LoginRequiredMixin, PermissionRequiredMixin, FormView): +class FittingResultCreateView(LoginRequiredMixin, PermissionRequiredMixin, UserFormKwargsMixin, FormView): permission_required = 'fitting.run_fits' form_class = FittingResultCreateForm diff --git a/weblab/repocache/models.py b/weblab/repocache/models.py index 39bbe2811..d94d67e67 100644 --- a/weblab/repocache/models.py +++ b/weblab/repocache/models.py @@ -1,5 +1,6 @@ from django.core.exceptions import ObjectDoesNotExist from django.db import models +from guardian.shortcuts import get_objects_for_user from core.models import VisibilityModelMixin from core.visibility import Visibility @@ -202,6 +203,23 @@ def _set_class_links(entity_cache_type, version_cache_type, tag_cache_type): tag_cache_type.CachedVersionClass = version_cache_type +class CachedEntityVersionManager(models.Manager): + def visible_to_user(self, user): + non_private = self.filter(visibility__in=['public', 'moderated']) + + if user.is_authenticated: + shared_pks = get_objects_for_user( + user, 'entities.edit_entity', with_superuser=False + ).values_list('pk', flat=True) + shared = self.filter(entity__entity__pk__in=shared_pks) + owned = self.filter(entity__entity__author=user) + else: + shared = self.none() + owned = self.none() + + return non_private | owned | shared + + #################################################################################################### # # Concrete cache classes go here @@ -216,6 +234,8 @@ class CachedModelVersion(CachedEntityVersion): """Cache for a single version / commit in a CellML model's repository.""" entity = models.ForeignKey(CachedModel, on_delete=models.CASCADE, related_name='versions') + objects = CachedEntityVersionManager() + @property def model(self): return self.entity.entity @@ -239,6 +259,8 @@ class CachedProtocolVersion(CachedEntityVersion): """Cache for a single version / commit in a protocol's repository.""" entity = models.ForeignKey(CachedProtocol, on_delete=models.CASCADE, related_name='versions') + objects = CachedEntityVersionManager() + @property def protocol(self): return self.entity.entity @@ -262,6 +284,8 @@ class CachedFittingSpecVersion(CachedEntityVersion): """Cache for a single version / commit in a fitting specifications's repository.""" entity = models.ForeignKey(CachedFittingSpec, on_delete=models.CASCADE, related_name='versions') + objects = CachedEntityVersionManager() + @property def fittingspec(self): return self.entity.entity diff --git a/weblab/repocache/tests/test_models.py b/weblab/repocache/tests/test_models.py index d2ae01540..f9a635227 100644 --- a/weblab/repocache/tests/test_models.py +++ b/weblab/repocache/tests/test_models.py @@ -3,7 +3,12 @@ from core import recipes from repocache.exceptions import RepoCacheMiss -from repocache.models import CachedFittingSpec, CachedModel, CachedProtocol +from repocache.models import ( + CachedFittingSpec, + CachedModel, + CachedModelVersion, + CachedProtocol, +) from repocache.populate import populate_entity_cache @@ -82,6 +87,7 @@ class TestEntityCacheModelsVisibility: @pytest.mark.parametrize("recipe", [ (recipes.cached_model_version), (recipes.cached_protocol_version), + (recipes.cached_fittingspec_version), ]) def test_entity_visibility(self, recipe): version = recipe.make(visibility='public') @@ -90,6 +96,7 @@ def test_entity_visibility(self, recipe): @pytest.mark.parametrize("recipe", [ (recipes.cached_model), (recipes.cached_protocol), + (recipes.cached_fittingspec), ]) def test_entity_visibility_is_private_if_no_versions(self, recipe): cached = recipe.make() @@ -98,6 +105,7 @@ def test_entity_visibility_is_private_if_no_versions(self, recipe): @pytest.mark.parametrize("recipe", [ (recipes.cached_model_version), (recipes.cached_protocol_version), + (recipes.cached_fittingspec_version), ]) def test_get_version(self, recipe): version = recipe.make() @@ -106,6 +114,7 @@ def test_get_version(self, recipe): @pytest.mark.parametrize("recipe", [ (recipes.cached_model_version), (recipes.cached_protocol_version), + (recipes.cached_fittingspec_version), ]) def test_set_entity_version_visibility(self, recipe): version = recipe.make() @@ -152,3 +161,60 @@ def test_nice_version(self, model_with_version): model_with_version.repo.tag('v1') populate_entity_cache(model_with_version) assert version.nice_version() == 'v1' + + +@pytest.mark.django_db +class TestCachedEntityVersionVisibleToUser: + def test_visibility_and_sharing(self, user, anon_user, other_user, admin_user, helpers): + # Own entities -> always visible + own_models = recipes.model.make(author=user, _quantity=3) + own_moderated_version = helpers.add_fake_version(own_models[0], 'moderated') + own_public_version = helpers.add_fake_version(own_models[1], 'public') + own_private_version = helpers.add_fake_version(own_models[2], 'private') + + # Other entity type shouldn't show up + own_protocol = recipes.protocol.make(author=user) + helpers.add_fake_version(own_protocol, 'moderated') + + # Non-shared public/moderated entities -> visible + other_public_models = recipes.model.make(author=other_user, _quantity=2) + other_moderated_version = helpers.add_fake_version(other_public_models[0], 'moderated') + other_public_version = helpers.add_fake_version(other_public_models[1], 'public') + + # Non-shared private entities -> not visible + other_private_model = recipes.model.make(author=other_user) + other_private_version = helpers.add_fake_version(other_private_model, 'private') # noqa: F841 + + # Shared public or private entities -> visible + other_shared_model = recipes.model.make(author=other_user) + other_shared_version = helpers.add_fake_version(other_shared_model, 'private') + other_shared_model.add_collaborator(user) + + other_shared_protocol = recipes.protocol.make(author=other_user) + helpers.add_fake_version(other_shared_protocol, 'private') + other_shared_protocol.add_collaborator(user) + + # User can see own versions, plus public, plus those explicitly shared + visible_to_self = CachedModelVersion.objects.visible_to_user(user).all() + assert visible_to_self.count() == 6 + assert set(visible_to_self) == { + own_moderated_version, own_public_version, own_private_version, + other_moderated_version, other_public_version, + other_shared_version + } + +# # Anonymous users only see public things + visible_to_anon = CachedModelVersion.objects.visible_to_user(anon_user).all() + assert visible_to_anon.count() == 4 + assert set(visible_to_anon) == { + own_moderated_version, own_public_version, + other_moderated_version, other_public_version, + } + + # Admins don't get special visibility rights, so only see public entities + visible_to_admin = CachedModelVersion.objects.visible_to_user(admin_user).all() + assert visible_to_admin.count() == 4 + assert set(visible_to_admin) == { + own_moderated_version, own_public_version, + other_moderated_version, other_public_version, + } From 7760b20449fac7668e1733fa64aa30f0b844ed74 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Mon, 21 Sep 2020 16:18:22 +0000 Subject: [PATCH 13/37] Enforce visibility on fitting filter json view --- weblab/fitting/tests/test_views.py | 118 ++++++++++++++++++++++------- weblab/fitting/views.py | 17 +++-- 2 files changed, 100 insertions(+), 35 deletions(-) diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index aa324704d..daff75a21 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -520,8 +520,8 @@ def test_requires_permission(self, client, logged_in_user): def test_all_models_and_versions(self, client, fits_user, helpers): model1 = recipes.model.make() model2 = recipes.model.make() - m1v1 = helpers.cached_version(model1) - m2v1 = helpers.cached_version(model2) + m1v1 = helpers.cached_version(model1, visibility='public') + m2v1 = helpers.cached_version(model2, visibility='public') response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -532,11 +532,26 @@ def test_all_models_and_versions(self, client, fits_user, helpers): assert set(options['models']) == {model1.id, model2.id} assert set(options['model_versions']) == {m1v1.id, m2v1.id} + def test_model_and_version_must_be_visible_to_user(self, client, fits_user, helpers): + model1 = recipes.model.make() + model2 = recipes.model.make() + m1v1 = helpers.cached_version(model1, visibility='public') + m2v1 = helpers.cached_version(model2, visibility='private') # noqa: F841 + + response = client.get('/fitting/results/new/filter', {}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + assert 'fittingResultOptions' in data + options = data['fittingResultOptions'] + assert set(options['models']) == {model1.id} + assert set(options['model_versions']) == {m1v1.id} + def test_all_protocols_and_versions(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1) - p2v1 = helpers.cached_version(protocol2) + p1v1 = helpers.cached_version(protocol1, visibility='public') + p2v1 = helpers.cached_version(protocol2, visibility='public') response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -546,11 +561,26 @@ def test_all_protocols_and_versions(self, client, fits_user, helpers): assert set(options['protocols']) == {protocol1.id, protocol2.id} assert set(options['protocol_versions']) == {p1v1.id, p2v1.id} + def test_protocol_and_version_must_be_visible_to_user(self, client, fits_user, helpers): + protocol1 = recipes.protocol.make() + protocol2 = recipes.protocol.make() + p1v1 = helpers.cached_version(protocol1, visibility='public') + p2v1 = helpers.cached_version(protocol2, visibility='private') # noqa: F841 + + response = client.get('/fitting/results/new/filter', {}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + assert 'fittingResultOptions' in data + options = data['fittingResultOptions'] + assert set(options['protocols']) == {protocol1.id} + assert set(options['protocol_versions']) == {p1v1.id} + def test_all_fittingspecs_and_versions(self, client, fits_user, helpers): fittingspec1 = recipes.fittingspec.make() fittingspec2 = recipes.fittingspec.make() - f1v1 = helpers.cached_version(fittingspec1) - f2v1 = helpers.cached_version(fittingspec2) + f1v1 = helpers.cached_version(fittingspec1, visibility='public') + f2v1 = helpers.cached_version(fittingspec2, visibility='public') response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -560,9 +590,24 @@ def test_all_fittingspecs_and_versions(self, client, fits_user, helpers): assert set(options['fittingspecs']) == {fittingspec1.id, fittingspec2.id} assert set(options['fittingspec_versions']) == {f1v1.id, f2v1.id} + def test_fittingspec_and_version_must_be_visible_to_user(self, client, fits_user, helpers): + fittingspec1 = recipes.fittingspec.make() + fittingspec2 = recipes.fittingspec.make() + p1v1 = helpers.cached_version(fittingspec1, visibility='public') + p2v1 = helpers.cached_version(fittingspec2, visibility='private') # noqa: F841 + + response = client.get('/fitting/results/new/filter', {}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + assert 'fittingResultOptions' in data + options = data['fittingResultOptions'] + assert set(options['fittingspecs']) == {fittingspec1.id} + assert set(options['fittingspec_versions']) == {p1v1.id} + def test_all_datasets(self, client, fits_user): - dataset1 = recipes.dataset.make() - dataset2 = recipes.dataset.make() + dataset1 = recipes.dataset.make(visibility='public') + dataset2 = recipes.dataset.make(visibility='public') response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -571,11 +616,23 @@ def test_all_datasets(self, client, fits_user): options = data['fittingResultOptions'] assert set(options['datasets']) == {dataset1.id, dataset2.id} + def test_dataset_must_be_visible_to_user(self, client, fits_user, helpers): + dataset1 = recipes.dataset.make(visibility='public') + dataset2 = recipes.dataset.make(visibility='private') # noqa: F841 + + response = client.get('/fitting/results/new/filter', {}) + assert response.status_code == 200 + + data = json.loads(response.content.decode()) + assert 'fittingResultOptions' in data + options = data['fittingResultOptions'] + assert set(options['datasets']) == {dataset1.id} + def test_model_and_versions_restricted_when_model_selected(self, client, fits_user, helpers): model1 = recipes.model.make() model2 = recipes.model.make() - m1v1 = helpers.cached_version(model1) - m2v1 = helpers.cached_version(model2) # noqa: F841 + m1v1 = helpers.cached_version(model1, visibility='public') + m2v1 = helpers.cached_version(model2, visibility='public') # noqa: F841 response = client.get('/fitting/results/new/filter', {'model': model1.id}) assert response.status_code == 200 @@ -588,8 +645,8 @@ def test_model_and_versions_restricted_when_model_selected(self, client, fits_us def test_protocol_and_versions_restricted_when_protocol_selected(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1) - p2v1 = helpers.cached_version(protocol2) # noqa: F841 + p1v1 = helpers.cached_version(protocol1, visibility='public') + p2v1 = helpers.cached_version(protocol2, visibility='public') # noqa: F841 response = client.get('/fitting/results/new/filter', {'protocol': protocol1.id}) assert response.status_code == 200 @@ -602,8 +659,8 @@ def test_protocol_and_versions_restricted_when_protocol_selected(self, client, f def test_fittingspec_and_versions_restricted_when_fittingspec_selected(self, client, fits_user, helpers): fittingspec1 = recipes.fittingspec.make() fittingspec2 = recipes.fittingspec.make() - f1v1 = helpers.cached_version(fittingspec1) - f2v1 = helpers.cached_version(fittingspec2) # noqa: F841 + f1v1 = helpers.cached_version(fittingspec1, visibility='public') + f2v1 = helpers.cached_version(fittingspec2, visibility='public') # noqa: F841 response = client.get('/fitting/results/new/filter', {'fittingspec': fittingspec1.id}) assert response.status_code == 200 @@ -614,8 +671,8 @@ def test_fittingspec_and_versions_restricted_when_fittingspec_selected(self, cli assert options['fittingspec_versions'] == [f1v1.id] def test_dataset_restricted_when_selected(self, client, fits_user, helpers): - dataset1 = recipes.dataset.make() - dataset2 = recipes.dataset.make() # noqa: F841 + dataset1 = recipes.dataset.make(visibility='public') + dataset2 = recipes.dataset.make(visibility='public') # noqa: F841 response = client.get('/fitting/results/new/filter', {'dataset': dataset1.id}) assert response.status_code == 200 @@ -627,12 +684,13 @@ def test_dataset_restricted_when_selected(self, client, fits_user, helpers): def test_dataset_and_fittingspec_restricted_when_protocol_selected(self, client, fits_user, helpers): protocol = recipes.protocol.make() + helpers.cached_version(protocol, visibility='public') fittingspec1 = recipes.fittingspec.make(protocol=protocol) fittingspec2 = recipes.fittingspec.make() - f1v1 = helpers.cached_version(fittingspec1) - f2v1 = helpers.cached_version(fittingspec2) # noqa: F841 - dataset1 = recipes.dataset.make(protocol=protocol) - dataset2 = recipes.dataset.make() # noqa: F841 + f1v1 = helpers.cached_version(fittingspec1, visibility='public') + f2v1 = helpers.cached_version(fittingspec2, visibility='public') # noqa: F841 + dataset1 = recipes.dataset.make(protocol=protocol, visibility='public') + dataset2 = recipes.dataset.make(visibility='public') # noqa: F841 response = client.get('/fitting/results/new/filter', {'protocol': protocol.id}) assert response.status_code == 200 @@ -646,10 +704,14 @@ def test_dataset_and_fittingspec_restricted_when_protocol_selected(self, client, def test_protocol_and_fittingspec_restricted_when_dataset_selected(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1) - p2v1 = helpers.cached_version(protocol2) # noqa: F841 + p1v1 = helpers.cached_version(protocol1, visibility='public') + p2v1 = helpers.cached_version(protocol2, visibility='public') # noqa: F841 + fittingspec1 = recipes.fittingspec.make(protocol=protocol1) + fittingspec2 = recipes.fittingspec.make() + f1v1 = helpers.cached_version(fittingspec1, visibility='public') + f2v1 = helpers.cached_version(fittingspec2, visibility='public') # noqa: F841 - dataset = recipes.dataset.make(protocol=protocol1) + dataset = recipes.dataset.make(protocol=protocol1, visibility='public') response = client.get('/fitting/results/new/filter', {'dataset': dataset.id}) assert response.status_code == 200 @@ -658,14 +720,16 @@ def test_protocol_and_fittingspec_restricted_when_dataset_selected(self, client, options = data['fittingResultOptions'] assert options['protocols'] == [protocol1.id] assert options['protocol_versions'] == [p1v1.id] + assert options['fittingspecs'] == [fittingspec1.id] + assert options['fittingspec_versions'] == [f1v1.id] def test_protocol_and_dataset_restricted_when_fittingspec_selected(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1) - p2v1 = helpers.cached_version(protocol2) # noqa: F841 - dataset1 = recipes.dataset.make(protocol=protocol1) - dataset2 = recipes.dataset.make() # noqa: F841 + p1v1 = helpers.cached_version(protocol1, visibility='public') + p2v1 = helpers.cached_version(protocol2, visibility='public') # noqa: F841 + dataset1 = recipes.dataset.make(protocol=protocol1, visibility='public') + dataset2 = recipes.dataset.make(visibility='public') # noqa: F841 fittingspec = recipes.fittingspec.make(protocol=protocol1) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 9a0c001a1..053515283 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -264,13 +264,13 @@ class FittingResultFilterJsonView(LoginRequiredMixin, PermissionRequiredMixin, V def get(self, request, *args, **kwargs): options = {} - models = ModelEntity.objects.all() - model_versions = CachedModelVersion.objects.all() - protocols = ProtocolEntity.objects.all() - protocol_versions = CachedProtocolVersion.objects.all() - fittingspecs = FittingSpec.objects.all() - fittingspec_versions = CachedFittingSpecVersion.objects.all() - datasets = Dataset.objects.all() + models = ModelEntity.objects.visible_to_user(request.user) + model_versions = CachedModelVersion.objects.visible_to_user(request.user) + protocols = ProtocolEntity.objects.visible_to_user(request.user) + protocol_versions = CachedProtocolVersion.objects.visible_to_user(request.user) + fittingspecs = FittingSpec.objects.visible_to_user(request.user) + fittingspec_versions = CachedFittingSpecVersion.objects.visible_to_user(request.user) + datasets = Dataset.objects.visible_to_user(request.user) def _get_int_param(fieldname, _model): try: @@ -309,7 +309,8 @@ def _get_int_param(fieldname, _model): if not fittingspec: fittingspecs = fittingspecs.filter(protocol=protocol.id) - fittingspec_versions = fittingspec_versions.filter(entity__entity__in=fittingspecs) + fsids = [fs.pk for fs in fittingspecs.filter(protocol=protocol.id)] + fittingspec_versions = fittingspec_versions.filter(entity__entity__in=fsids) if not dataset: datasets = datasets.filter(protocol=protocol.id) From 079be8a5889a3f2a66d1fd031a3928c576434d2f Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 22 Sep 2020 09:43:46 +0000 Subject: [PATCH 14/37] Disable pre-selected entity fields --- weblab/fitting/forms.py | 8 ++++++++ weblab/fitting/tests/test_forms.py | 25 ++++++++++++++++++++++++- weblab/static/js/fitting.js | 7 +++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/weblab/fitting/forms.py b/weblab/fitting/forms.py index 47760d55d..bdf0d739e 100644 --- a/weblab/fitting/forms.py +++ b/weblab/fitting/forms.py @@ -40,6 +40,14 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + + # Disable fields with preselected values + if not self.is_bound: + self.fields['model'].disabled = bool(self.initial.get('model')) + self.fields['protocol'].disabled = bool(self.initial.get('protocol')) + self.fields['fittingspec'].disabled = bool(self.initial.get('fittingspec')) + self.fields['dataset'].disabled = bool(self.initial.get('dataset')) + # Ensure only visible entities and versions are available to user self.fields['model'].queryset = ModelEntity.objects.visible_to_user(self.user) self.fields['protocol'].queryset = ProtocolEntity.objects.visible_to_user(self.user) diff --git a/weblab/fitting/tests/test_forms.py b/weblab/fitting/tests/test_forms.py index aec1b8b57..839ae466d 100644 --- a/weblab/fitting/tests/test_forms.py +++ b/weblab/fitting/tests/test_forms.py @@ -6,7 +6,7 @@ @pytest.mark.django_db class TestFittingResultCreateForm: - def test_fields(self, fits_user): + def test_fields_exist(self, fits_user): form = FittingResultCreateForm(user=fits_user) assert 'model' in form.fields assert 'model_version' in form.fields @@ -16,6 +16,29 @@ def test_fields(self, fits_user): assert 'fittingspec_version' in form.fields assert 'dataset' in form.fields + def test_fields_enabled_by_default(self, fits_user): + form = FittingResultCreateForm(user=fits_user) + assert not form.fields['model'].disabled + assert not form.fields['protocol'].disabled + assert not form.fields['fittingspec'].disabled + assert not form.fields['dataset'].disabled + + def test_disables_preselected_model(self, public_model, fits_user): + form = FittingResultCreateForm(initial={'model': public_model.pk}, user=fits_user) + assert form.fields['model'].disabled + + def test_disables_preselected_protocol(self, public_protocol, fits_user): + form = FittingResultCreateForm(initial={'protocol': public_protocol.pk}, user=fits_user) + assert form.fields['protocol'].disabled + + def test_disables_preselected_fittingspec(self, public_fittingspec, fits_user): + form = FittingResultCreateForm(initial={'fittingspec': public_fittingspec.pk}, user=fits_user) + assert form.fields['fittingspec'].disabled + + def test_disables_preselected_dataset(self, public_dataset, fits_user): + form = FittingResultCreateForm(initial={'dataset': public_dataset.pk}, user=fits_user) + assert form.fields['dataset'].disabled + def test_valid_form( self, public_model, public_protocol, public_fittingspec, public_dataset, fits_user, helpers diff --git a/weblab/static/js/fitting.js b/weblab/static/js/fitting.js index 902d7c54a..71313b649 100644 --- a/weblab/static/js/fitting.js +++ b/weblab/static/js/fitting.js @@ -61,6 +61,13 @@ function init() { $("form select").change(updateDropdowns); + // Disabled form fields will not be submitted - re-enable before the form is posted + $('form').submit(function() { + $(':disabled').each(function() { + $(this).removeAttr('disabled'); + }) + }); + updateDropdowns(); } From 0fa841757b32311c19fa113646b300d77c05749c Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 22 Sep 2020 10:15:57 +0000 Subject: [PATCH 15/37] Display "Fit" buttons on entity pages --- weblab/templates/entities/entity_version.html | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/weblab/templates/entities/entity_version.html b/weblab/templates/entities/entity_version.html index 003b19ac5..9eff0bddb 100644 --- a/weblab/templates/entities/entity_version.html +++ b/weblab/templates/entities/entity_version.html @@ -63,10 +63,14 @@

{% endif %}
  • Other versions
  • - From 23ff8b4f76af99ac9d9aa26b3719c6b207097be3 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Tue, 22 Sep 2020 10:18:39 +0000 Subject: [PATCH 16/37] Add fit button to dataset page --- weblab/templates/datasets/dataset_detail.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/weblab/templates/datasets/dataset_detail.html b/weblab/templates/datasets/dataset_detail.html index 09aa4ee7c..2743eb442 100644 --- a/weblab/templates/datasets/dataset_detail.html +++ b/weblab/templates/datasets/dataset_detail.html @@ -38,6 +38,9 @@ +

    {{ dataset.description }} From 8e6976732f9e99b6060e6459faa27bed5af01f01 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Thu, 24 Sep 2020 06:56:38 +0000 Subject: [PATCH 17/37] Rename cached_version to add_cached_version verb makes it clearer what's happening --- weblab/conftest.py | 7 ++-- weblab/fitting/tests/test_models.py | 24 ++++++------- weblab/fitting/tests/test_views.py | 54 ++++++++++++++--------------- 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/weblab/conftest.py b/weblab/conftest.py index 86317e5a3..ea4c78246 100644 --- a/weblab/conftest.py +++ b/weblab/conftest.py @@ -44,8 +44,11 @@ def add_version(entity, return commit @staticmethod - def cached_version(entity, **kwargs): - """Add a single commit/version to an entity and return the relevant repocache entry""" + def add_cached_version(entity, **kwargs): + """ + Add a single commit/version to an entity along with a repocache entry. + @return the relevant repocache entry + """ assert kwargs.get('cache', True), "Cache must be true for cached version" version = Helpers.add_version(entity, **kwargs) return entity.repocache.get_version(version.sha) diff --git a/weblab/fitting/tests/test_models.py b/weblab/fitting/tests/test_models.py index 3835b3914..b04c4ee09 100644 --- a/weblab/fitting/tests/test_models.py +++ b/weblab/fitting/tests/test_models.py @@ -156,12 +156,12 @@ def test_visibility(self, helpers): ds2 = recipes.dataset.make(visibility='public') fittingspec = recipes.fittingspec.make() - mv1 = helpers.cached_version(model, visibility='private') - mv2 = helpers.cached_version(model, visibility='public') - pv1 = helpers.cached_version(protocol, visibility='private') - pv2 = helpers.cached_version(protocol, visibility='public') - fv1 = helpers.cached_version(fittingspec, visibility='private') - fv2 = helpers.cached_version(fittingspec, visibility='public') + mv1 = helpers.add_cached_version(model, visibility='private') + mv2 = helpers.add_cached_version(model, visibility='public') + pv1 = helpers.add_cached_version(protocol, visibility='private') + pv2 = helpers.add_cached_version(protocol, visibility='public') + fv1 = helpers.add_cached_version(fittingspec, visibility='private') + fv2 = helpers.add_cached_version(fittingspec, visibility='public') # all public assert recipes.fittingresult.make( @@ -224,9 +224,9 @@ def test_viewers(self, helpers, user): # (https://github.com/ModellingWebLab/WebLab/issues/247) # so test with a public dataset for now dataset = recipes.dataset.make(visibility='public') - mv = helpers.cached_version(model, visibility='private') - pv = helpers.cached_version(protocol, visibility='private') - fv = helpers.cached_version(fittingspec, visibility='private') + mv = helpers.add_cached_version(model, visibility='private') + pv = helpers.add_cached_version(protocol, visibility='private') + fv = helpers.add_cached_version(fittingspec, visibility='private') fr = recipes.fittingresult.make( model=model, model_version=mv, @@ -254,9 +254,9 @@ def test_viewers_of_public_fittingresult(self, helpers, user): protocol = recipes.protocol.make() fittingspec = recipes.fittingspec.make() dataset = recipes.dataset.make(visibility='public') - mv = helpers.cached_version(model, visibility='public') - pv = helpers.cached_version(protocol, visibility='public') - fv = helpers.cached_version(fittingspec, visibility='public') + mv = helpers.add_cached_version(model, visibility='public') + pv = helpers.add_cached_version(protocol, visibility='public') + fv = helpers.add_cached_version(fittingspec, visibility='public') fr = recipes.fittingresult.make( model=model, model_version=mv, diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index daff75a21..e4f47ac71 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -520,8 +520,8 @@ def test_requires_permission(self, client, logged_in_user): def test_all_models_and_versions(self, client, fits_user, helpers): model1 = recipes.model.make() model2 = recipes.model.make() - m1v1 = helpers.cached_version(model1, visibility='public') - m2v1 = helpers.cached_version(model2, visibility='public') + m1v1 = helpers.add_cached_version(model1, visibility='public') + m2v1 = helpers.add_cached_version(model2, visibility='public') response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -535,8 +535,8 @@ def test_all_models_and_versions(self, client, fits_user, helpers): def test_model_and_version_must_be_visible_to_user(self, client, fits_user, helpers): model1 = recipes.model.make() model2 = recipes.model.make() - m1v1 = helpers.cached_version(model1, visibility='public') - m2v1 = helpers.cached_version(model2, visibility='private') # noqa: F841 + m1v1 = helpers.add_cached_version(model1, visibility='public') + m2v1 = helpers.add_cached_version(model2, visibility='private') # noqa: F841 response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -550,8 +550,8 @@ def test_model_and_version_must_be_visible_to_user(self, client, fits_user, help def test_all_protocols_and_versions(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1, visibility='public') - p2v1 = helpers.cached_version(protocol2, visibility='public') + p1v1 = helpers.add_cached_version(protocol1, visibility='public') + p2v1 = helpers.add_cached_version(protocol2, visibility='public') response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -564,8 +564,8 @@ def test_all_protocols_and_versions(self, client, fits_user, helpers): def test_protocol_and_version_must_be_visible_to_user(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1, visibility='public') - p2v1 = helpers.cached_version(protocol2, visibility='private') # noqa: F841 + p1v1 = helpers.add_cached_version(protocol1, visibility='public') + p2v1 = helpers.add_cached_version(protocol2, visibility='private') # noqa: F841 response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -579,8 +579,8 @@ def test_protocol_and_version_must_be_visible_to_user(self, client, fits_user, h def test_all_fittingspecs_and_versions(self, client, fits_user, helpers): fittingspec1 = recipes.fittingspec.make() fittingspec2 = recipes.fittingspec.make() - f1v1 = helpers.cached_version(fittingspec1, visibility='public') - f2v1 = helpers.cached_version(fittingspec2, visibility='public') + f1v1 = helpers.add_cached_version(fittingspec1, visibility='public') + f2v1 = helpers.add_cached_version(fittingspec2, visibility='public') response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -593,8 +593,8 @@ def test_all_fittingspecs_and_versions(self, client, fits_user, helpers): def test_fittingspec_and_version_must_be_visible_to_user(self, client, fits_user, helpers): fittingspec1 = recipes.fittingspec.make() fittingspec2 = recipes.fittingspec.make() - p1v1 = helpers.cached_version(fittingspec1, visibility='public') - p2v1 = helpers.cached_version(fittingspec2, visibility='private') # noqa: F841 + p1v1 = helpers.add_cached_version(fittingspec1, visibility='public') + p2v1 = helpers.add_cached_version(fittingspec2, visibility='private') # noqa: F841 response = client.get('/fitting/results/new/filter', {}) assert response.status_code == 200 @@ -631,8 +631,8 @@ def test_dataset_must_be_visible_to_user(self, client, fits_user, helpers): def test_model_and_versions_restricted_when_model_selected(self, client, fits_user, helpers): model1 = recipes.model.make() model2 = recipes.model.make() - m1v1 = helpers.cached_version(model1, visibility='public') - m2v1 = helpers.cached_version(model2, visibility='public') # noqa: F841 + m1v1 = helpers.add_cached_version(model1, visibility='public') + m2v1 = helpers.add_cached_version(model2, visibility='public') # noqa: F841 response = client.get('/fitting/results/new/filter', {'model': model1.id}) assert response.status_code == 200 @@ -645,8 +645,8 @@ def test_model_and_versions_restricted_when_model_selected(self, client, fits_us def test_protocol_and_versions_restricted_when_protocol_selected(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1, visibility='public') - p2v1 = helpers.cached_version(protocol2, visibility='public') # noqa: F841 + p1v1 = helpers.add_cached_version(protocol1, visibility='public') + p2v1 = helpers.add_cached_version(protocol2, visibility='public') # noqa: F841 response = client.get('/fitting/results/new/filter', {'protocol': protocol1.id}) assert response.status_code == 200 @@ -659,8 +659,8 @@ def test_protocol_and_versions_restricted_when_protocol_selected(self, client, f def test_fittingspec_and_versions_restricted_when_fittingspec_selected(self, client, fits_user, helpers): fittingspec1 = recipes.fittingspec.make() fittingspec2 = recipes.fittingspec.make() - f1v1 = helpers.cached_version(fittingspec1, visibility='public') - f2v1 = helpers.cached_version(fittingspec2, visibility='public') # noqa: F841 + f1v1 = helpers.add_cached_version(fittingspec1, visibility='public') + f2v1 = helpers.add_cached_version(fittingspec2, visibility='public') # noqa: F841 response = client.get('/fitting/results/new/filter', {'fittingspec': fittingspec1.id}) assert response.status_code == 200 @@ -684,11 +684,11 @@ def test_dataset_restricted_when_selected(self, client, fits_user, helpers): def test_dataset_and_fittingspec_restricted_when_protocol_selected(self, client, fits_user, helpers): protocol = recipes.protocol.make() - helpers.cached_version(protocol, visibility='public') + helpers.add_cached_version(protocol, visibility='public') fittingspec1 = recipes.fittingspec.make(protocol=protocol) fittingspec2 = recipes.fittingspec.make() - f1v1 = helpers.cached_version(fittingspec1, visibility='public') - f2v1 = helpers.cached_version(fittingspec2, visibility='public') # noqa: F841 + f1v1 = helpers.add_cached_version(fittingspec1, visibility='public') + f2v1 = helpers.add_cached_version(fittingspec2, visibility='public') # noqa: F841 dataset1 = recipes.dataset.make(protocol=protocol, visibility='public') dataset2 = recipes.dataset.make(visibility='public') # noqa: F841 @@ -704,12 +704,12 @@ def test_dataset_and_fittingspec_restricted_when_protocol_selected(self, client, def test_protocol_and_fittingspec_restricted_when_dataset_selected(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1, visibility='public') - p2v1 = helpers.cached_version(protocol2, visibility='public') # noqa: F841 + p1v1 = helpers.add_cached_version(protocol1, visibility='public') + p2v1 = helpers.add_cached_version(protocol2, visibility='public') # noqa: F841 fittingspec1 = recipes.fittingspec.make(protocol=protocol1) fittingspec2 = recipes.fittingspec.make() - f1v1 = helpers.cached_version(fittingspec1, visibility='public') - f2v1 = helpers.cached_version(fittingspec2, visibility='public') # noqa: F841 + f1v1 = helpers.add_cached_version(fittingspec1, visibility='public') + f2v1 = helpers.add_cached_version(fittingspec2, visibility='public') # noqa: F841 dataset = recipes.dataset.make(protocol=protocol1, visibility='public') @@ -726,8 +726,8 @@ def test_protocol_and_fittingspec_restricted_when_dataset_selected(self, client, def test_protocol_and_dataset_restricted_when_fittingspec_selected(self, client, fits_user, helpers): protocol1 = recipes.protocol.make() protocol2 = recipes.protocol.make() - p1v1 = helpers.cached_version(protocol1, visibility='public') - p2v1 = helpers.cached_version(protocol2, visibility='public') # noqa: F841 + p1v1 = helpers.add_cached_version(protocol1, visibility='public') + p2v1 = helpers.add_cached_version(protocol2, visibility='public') # noqa: F841 dataset1 = recipes.dataset.make(protocol=protocol1, visibility='public') dataset2 = recipes.dataset.make(visibility='public') # noqa: F841 From 31ffb66a1d4cd5d07957b3da4aac4ee42406f9c8 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Thu, 24 Sep 2020 07:55:04 +0000 Subject: [PATCH 18/37] Add more comments --- weblab/conftest.py | 3 ++- weblab/fitting/forms.py | 3 +++ weblab/fitting/views.py | 22 ++++++++++++++++++++++ weblab/repocache/models.py | 7 +++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/weblab/conftest.py b/weblab/conftest.py index ea4c78246..50324a259 100644 --- a/weblab/conftest.py +++ b/weblab/conftest.py @@ -233,7 +233,7 @@ def private_fittingspec(helpers): @pytest.fixture def private_dataset(): - return recipes.dataset.make(visibility='private') + return recipes.dataset.make(name='private dataset', visibility='private') @pytest.fixture @@ -366,6 +366,7 @@ def moderator(user, helpers): @pytest.fixture def fits_user(logged_in_user): + """User with permission to run fittings""" content_type = ContentType.objects.get_for_model(FittingResult) permission = Permission.objects.get( codename='run_fits', diff --git a/weblab/fitting/forms.py b/weblab/fitting/forms.py index bdf0d739e..0df71c7c7 100644 --- a/weblab/fitting/forms.py +++ b/weblab/fitting/forms.py @@ -33,6 +33,7 @@ class FittingSpecVersionForm(EntityVersionForm): class FittingResultCreateForm(UserKwargModelFormMixin, forms.ModelForm): + """Used for creating and running a new fitting result""" class Meta: model = FittingResult fields = ('model', 'model_version', 'protocol', 'protocol_version', @@ -42,6 +43,8 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # Disable fields with preselected values + # But only when there is no data bound - otherwise we will lose the + # posted values of these fields from the submitted form. if not self.is_bound: self.fields['model'].disabled = bool(self.initial.get('model')) self.fields['protocol'].disabled = bool(self.initial.get('protocol')) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 053515283..8f388e303 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -65,6 +65,7 @@ class FittingResultVersionListView(VisibilityMixin, DetailView): class FittingResultVersionView(VisibilityMixin, DetailView): + """Show a version of a fitting result""" model = FittingResultVersion context_object_name = 'version' @@ -206,6 +207,10 @@ def get(self, request, *args, **kwargs): class FittingResultCreateView(LoginRequiredMixin, PermissionRequiredMixin, UserFormKwargsMixin, FormView): + """ + Create and submit a fitting result from models, protocols, fitting specs and datasets + and (where relevant) their versions. + """ permission_required = 'fitting.run_fits' form_class = FittingResultCreateForm @@ -259,6 +264,16 @@ def get_success_url(self): class FittingResultFilterJsonView(LoginRequiredMixin, PermissionRequiredMixin, View): + """ + JSON view of valid fitting result input values based on those already selected + + For example, if a model id is specified (as a GET param), only versions of that model + (which are visible to the user) will be included in the results. Otherwise all visible + models and versions will be in the results (which are simply a list of database + IDs of the relevant objects) + + Connections between protocols, fitting specs and datasets are also enforced. + """ permission_required = 'fitting.run_fits' def get(self, request, *args, **kwargs): @@ -292,29 +307,36 @@ def _get_int_param(fieldname, _model): elif dataset: protocol = dataset.protocol + # Restrict to specified model and its versions if model: models = [model] model_versions = model_versions.filter(entity__entity=model) + # Restrict to specified fitting spec and its versions if fittingspec: fittingspecs = [fittingspec] fittingspec_versions = fittingspec_versions.filter(entity__entity=fittingspec.id) + # Restrict to specified dataset if dataset: datasets = [dataset] + # Restrict to specified protocol and its versions if protocol: protocols = [protocol] protocol_versions = protocol_versions.filter(entity__entity=protocol.id) + # If no fitting spec was chosen yet, restrict to those linked to this protocol if not fittingspec: fittingspecs = fittingspecs.filter(protocol=protocol.id) fsids = [fs.pk for fs in fittingspecs.filter(protocol=protocol.id)] fittingspec_versions = fittingspec_versions.filter(entity__entity__in=fsids) + # If no dataset was chosen yet, restrict to those linked to this protocol if not dataset: datasets = datasets.filter(protocol=protocol.id) + # These might be either querysets or ID lists def _get_ids(qs): return [item.id for item in qs] diff --git a/weblab/repocache/models.py b/weblab/repocache/models.py index d94d67e67..80475e449 100644 --- a/weblab/repocache/models.py +++ b/weblab/repocache/models.py @@ -205,6 +205,13 @@ def _set_class_links(entity_cache_type, version_cache_type, tag_cache_type): class CachedEntityVersionManager(models.Manager): def visible_to_user(self, user): + """Query over all cached entity versions that the given user can view. + + This includes those versions of entities of the relevant type for which either: + - the user is the author of the related entity + - the entity version is non-private + - or the entity is explicitly shared with the user + """ non_private = self.filter(visibility__in=['public', 'moderated']) if user.is_authenticated: From 951ab2cb46d313630bd504de86ba34edf5306f47 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Thu, 24 Sep 2020 14:19:29 +0000 Subject: [PATCH 19/37] Check visibility on fitting result form page load --- weblab/fitting/tests/test_views.py | 16 ++++++++++++++++ weblab/fitting/views.py | 16 ++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index e4f47ac71..df0c0ac17 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -475,6 +475,22 @@ def test_with_preselected_dataset(self, client, fits_user, public_dataset): assert response.status_code == 200 assert response.context['form'].initial['dataset'] == public_dataset + def test_with_non_visible_model(self, client, fits_user, private_model): + response = client.get('/fitting/results/new', {'model': private_model.pk}) + assert response.status_code == 404 + + def test_with_non_visible_protocol(self, client, fits_user, private_protocol): + response = client.get('/fitting/results/new', {'protocol': private_protocol.pk}) + assert response.status_code == 404 + + def test_with_non_visible_fittingspec(self, client, fits_user, private_fittingspec): + response = client.get('/fitting/results/new', {'fittingspec': private_fittingspec.pk}) + assert response.status_code == 404 + + def test_with_non_visible_dataset(self, client, fits_user, private_dataset): + response = client.get('/fitting/results/new', {'dataset': private_dataset.pk}) + assert response.status_code == 404 + @patch('fitting.views.submit_fitting') def test_submits_to_backend(self, mock_submit, client, fits_user, public_model, public_protocol, public_fittingspec, public_dataset, helpers): diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 8f388e303..daf5a7608 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -219,13 +219,21 @@ class FittingResultCreateView(LoginRequiredMixin, PermissionRequiredMixin, UserF def get_initial(self): initial = super().get_initial() if 'model' in self.request.GET: - initial['model'] = get_object_or_404(ModelEntity, pk=self.request.GET['model']) + initial['model'] = get_object_or_404( + ModelEntity.objects.visible_to_user(self.request.user), + pk=self.request.GET['model']) elif 'protocol' in self.request.GET: - initial['protocol'] = get_object_or_404(ProtocolEntity, pk=self.request.GET['protocol']) + initial['protocol'] = get_object_or_404( + ProtocolEntity.objects.visible_to_user(self.request.user), + pk=self.request.GET['protocol']) elif 'fittingspec' in self.request.GET: - initial['fittingspec'] = get_object_or_404(FittingSpec, pk=self.request.GET['fittingspec']) + initial['fittingspec'] = get_object_or_404( + FittingSpec.objects.visible_to_user(self.request.user), + pk=self.request.GET['fittingspec']) elif 'dataset' in self.request.GET: - initial['dataset'] = get_object_or_404(Dataset, pk=self.request.GET['dataset']) + initial['dataset'] = get_object_or_404( + Dataset.objects.visible_to_user(self.request.user), + pk=self.request.GET['dataset']) return initial From 90bf33299e6d6187c32ce0c05febd059190d7021 Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Thu, 24 Sep 2020 15:09:37 +0100 Subject: [PATCH 20/37] Support fitting experiments in access token checks And implement this for datasets too --- weblab/datasets/views.py | 11 +++++++++++ weblab/entities/views.py | 15 +++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/weblab/datasets/views.py b/weblab/datasets/views.py index 1be6ee147..4681bc6f5 100644 --- a/weblab/datasets/views.py +++ b/weblab/datasets/views.py @@ -195,6 +195,17 @@ class DatasetArchiveView(VisibilityMixin, SingleObjectMixin, View): """ model = Dataset + def check_access_token(self, token): + """ + Override to allow token based access to dataset archive downloads - + must match a (fitting) `RunningExperiment` set up against the dataset. + """ + from experiments.models import RunningExperiment + return RunningExperiment.objects.filter( + id=token, + runnable__fittingresultversion__fittingresult__dataset=self.get_object().id, + ).exists() + def get_archive_name(self, dataset): return dataset.archive_name diff --git a/weblab/entities/views.py b/weblab/entities/views.py index 8cf72e589..271df38db 100644 --- a/weblab/entities/views.py +++ b/weblab/entities/views.py @@ -659,16 +659,19 @@ class EntityArchiveView(SingleObjectMixin, EntityVersionMixin, View): def check_access_token(self, token): """ Override to allow token based access to entity archive downloads - - must match a `RunningExperiment` or `AnalysisTask` object set up against the entity + must match a `RunningExperiment` or `AnalysisTask` object set up against the entity. + + We support both simulation and fitting experiments. """ from entities.models import AnalysisTask from experiments.models import RunningExperiment - entity_field = 'runnable__experimentversion__experiment__%s' % self.kwargs['entity_type'] self_id = self._get_object().id - return (RunningExperiment.objects.filter( - id=token, - **{entity_field: self_id} - ).exists() or AnalysisTask.objects.filter(id=token, entity=self_id).exists()) + if AnalysisTask.objects.filter(id=token, entity=self_id).exists(): + return True + query_tpl = 'runnable__{subclass}version__{subclass}__{entity_type}' + q_experiment = Q(**{query_tpl.format(subclass='experiment', entity_type=self.kwargs['entity_type']): self_id}) + q_fit = Q(**{query_tpl.format(subclass='fittingresult', entity_type=self.kwargs['entity_type']): self_id}) + return RunningExperiment.objects.filter(Q(id=token) & (q_experiment | q_fit)).exists() def get(self, request, *args, **kwargs): entity = self._get_object() From 1dbd14cd42ca89468e5b05f05ffaf9f64fc4b37f Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Thu, 24 Sep 2020 14:40:26 +0000 Subject: [PATCH 21/37] Remove unnecessary code --- weblab/fitting/views.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index daf5a7608..5c22f4adb 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -237,13 +237,6 @@ def get_initial(self): return initial - def post(self, request, *args, **kwargs): - form = self.get_form() - if form.is_valid(): - return self.form_valid(form) - else: - return self.form_invalid(form) - def form_valid(self, form): self.runnable, is_new = submit_fitting( form.cleaned_data['model_version'], @@ -267,8 +260,7 @@ def form_valid(self, form): return super().form_valid(form) def get_success_url(self): - if hasattr(self, 'runnable'): - return reverse('fitting:result:version', args=[self.runnable.fittingresult.pk, self.runnable.pk]) + return reverse('fitting:result:version', args=[self.runnable.fittingresult.pk, self.runnable.pk]) class FittingResultFilterJsonView(LoginRequiredMixin, PermissionRequiredMixin, View): From c07b2b0a56da443dbe4458963871983973a327fa Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Fri, 25 Sep 2020 07:04:14 +0000 Subject: [PATCH 22/37] Query fixes on fitting json view --- weblab/fitting/views.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 5c22f4adb..29439518b 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -279,14 +279,15 @@ class FittingResultFilterJsonView(LoginRequiredMixin, PermissionRequiredMixin, V def get(self, request, *args, **kwargs): options = {} - models = ModelEntity.objects.visible_to_user(request.user) model_versions = CachedModelVersion.objects.visible_to_user(request.user) - protocols = ProtocolEntity.objects.visible_to_user(request.user) + models = ModelEntity.objects.filter(cachedmodel__versions__in=model_versions) protocol_versions = CachedProtocolVersion.objects.visible_to_user(request.user) - fittingspecs = FittingSpec.objects.visible_to_user(request.user) + protocols = ProtocolEntity.objects.filter(cachedprotocol__versions__in=protocol_versions) fittingspec_versions = CachedFittingSpecVersion.objects.visible_to_user(request.user) + fittingspecs = FittingSpec.objects.filter(cachedfittingspec__versions__in=fittingspec_versions) datasets = Dataset.objects.visible_to_user(request.user) + def _get_int_param(fieldname, _model): try: pk = int(request.GET.get(fieldname, '')) @@ -315,7 +316,7 @@ def _get_int_param(fieldname, _model): # Restrict to specified fitting spec and its versions if fittingspec: fittingspecs = [fittingspec] - fittingspec_versions = fittingspec_versions.filter(entity__entity=fittingspec.id) + fittingspec_versions = fittingspec_versions.filter(entity__entity=fittingspec) # Restrict to specified dataset if dataset: @@ -324,7 +325,7 @@ def _get_int_param(fieldname, _model): # Restrict to specified protocol and its versions if protocol: protocols = [protocol] - protocol_versions = protocol_versions.filter(entity__entity=protocol.id) + protocol_versions = protocol_versions.filter(entity__entity=protocol) # If no fitting spec was chosen yet, restrict to those linked to this protocol if not fittingspec: From dfc8c1474979e6ba3304a9c9267e300f5150f19d Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Fri, 25 Sep 2020 07:11:12 +0000 Subject: [PATCH 23/37] Use nice (tag-based) labels for version dropdowns --- weblab/fitting/forms.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/weblab/fitting/forms.py b/weblab/fitting/forms.py index 0df71c7c7..872a3b1fd 100644 --- a/weblab/fitting/forms.py +++ b/weblab/fitting/forms.py @@ -32,8 +32,17 @@ class FittingSpecVersionForm(EntityVersionForm): rerun_expts = None +class VersionChoiceField(forms.ModelChoiceField): + def label_from_instance(self, obj): + return obj.nice_version() + + class FittingResultCreateForm(UserKwargModelFormMixin, forms.ModelForm): """Used for creating and running a new fitting result""" + model_version = VersionChoiceField(queryset=CachedModelVersion.objects.all()) + protocol_version = VersionChoiceField(queryset=CachedProtocolVersion.objects.all()) + fittingspec_version = VersionChoiceField(queryset=CachedFittingSpecVersion.objects.all()) + class Meta: model = FittingResult fields = ('model', 'model_version', 'protocol', 'protocol_version', From aad1cb1b0382f12000288bd813e7f92ddda449ea Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Fri, 25 Sep 2020 10:25:18 +0000 Subject: [PATCH 24/37] Allow version to be specified on fitting form load --- weblab/fitting/tests/test_views.py | 36 +++++++++++++++++++++++++ weblab/fitting/views.py | 43 +++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 9 deletions(-) diff --git a/weblab/fitting/tests/test_views.py b/weblab/fitting/tests/test_views.py index df0c0ac17..26b452cd5 100644 --- a/weblab/fitting/tests/test_views.py +++ b/weblab/fitting/tests/test_views.py @@ -475,18 +475,54 @@ def test_with_preselected_dataset(self, client, fits_user, public_dataset): assert response.status_code == 200 assert response.context['form'].initial['dataset'] == public_dataset + def test_with_preselected_model_version(self, client, fits_user, public_model): + version = public_model.repocache.latest_version + response = client.get('/fitting/results/new', {'model_version': version.pk}) + assert response.status_code == 200 + assert response.context['form'].initial['model'] == public_model + assert response.context['form'].initial['model_version'] == version + + def test_with_preselected_protocol_version(self, client, fits_user, public_protocol): + version = public_protocol.repocache.latest_version + response = client.get('/fitting/results/new', {'protocol_version': version.pk}) + assert response.status_code == 200 + assert response.context['form'].initial['protocol'] == public_protocol + assert response.context['form'].initial['protocol_version'] == version + + def test_with_preselected_fittingspec_version(self, client, fits_user, public_fittingspec): + version = public_fittingspec.repocache.latest_version + response = client.get('/fitting/results/new', {'fittingspec_version': version.pk}) + assert response.status_code == 200 + assert response.context['form'].initial['fittingspec'] == public_fittingspec + assert response.context['form'].initial['fittingspec_version'] == version + def test_with_non_visible_model(self, client, fits_user, private_model): response = client.get('/fitting/results/new', {'model': private_model.pk}) assert response.status_code == 404 + def test_with_non_visible_model_version(self, client, fits_user, private_model): + version = private_model.repocache.latest_version + response = client.get('/fitting/results/new', {'model_version': version.pk}) + assert response.status_code == 404 + def test_with_non_visible_protocol(self, client, fits_user, private_protocol): response = client.get('/fitting/results/new', {'protocol': private_protocol.pk}) assert response.status_code == 404 + def test_with_non_visible_protocol_version(self, client, fits_user, private_protocol): + version = private_protocol.repocache.latest_version + response = client.get('/fitting/results/new', {'protocol_version': version.pk}) + assert response.status_code == 404 + def test_with_non_visible_fittingspec(self, client, fits_user, private_fittingspec): response = client.get('/fitting/results/new', {'fittingspec': private_fittingspec.pk}) assert response.status_code == 404 + def test_with_non_visible_fittingspec_version(self, client, fits_user, private_fittingspec): + version = private_fittingspec.repocache.latest_version + response = client.get('/fitting/results/new', {'fittingspec_version': version.pk}) + assert response.status_code == 404 + def test_with_non_visible_dataset(self, client, fits_user, private_dataset): response = client.get('/fitting/results/new', {'dataset': private_dataset.pk}) assert response.status_code == 404 diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 29439518b..efa2929a4 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -218,22 +218,48 @@ class FittingResultCreateView(LoginRequiredMixin, PermissionRequiredMixin, UserF def get_initial(self): initial = super().get_initial() - if 'model' in self.request.GET: + model_id = self.request.GET.get('model') + model_version_id = self.request.GET.get('model_version') + protocol_id = self.request.GET.get('protocol') + protocol_version_id = self.request.GET.get('protocol_version') + fittingspec_id = self.request.GET.get('fittingspec') + fittingspec_version_id = self.request.GET.get('fittingspec_version') + dataset_id = self.request.GET.get('dataset') + + if model_version_id: + initial['model_version'] = get_object_or_404( + CachedModelVersion.objects.visible_to_user(self.request.user), + pk=model_version_id) + initial['model'] = initial['model_version'].model + elif model_id: initial['model'] = get_object_or_404( ModelEntity.objects.visible_to_user(self.request.user), - pk=self.request.GET['model']) - elif 'protocol' in self.request.GET: + pk=model_id) + + elif protocol_version_id: + initial['protocol_version'] = get_object_or_404( + CachedProtocolVersion.objects.visible_to_user(self.request.user), + pk=protocol_version_id) + initial['protocol'] = initial['protocol_version'].protocol + elif protocol_id: initial['protocol'] = get_object_or_404( ProtocolEntity.objects.visible_to_user(self.request.user), - pk=self.request.GET['protocol']) - elif 'fittingspec' in self.request.GET: + pk=protocol_id) + + elif fittingspec_version_id: + initial['fittingspec_version'] = get_object_or_404( + CachedFittingSpecVersion.objects.visible_to_user(self.request.user), + pk=fittingspec_version_id) + initial['fittingspec'] = initial['fittingspec_version'].fittingspec + elif fittingspec_id: initial['fittingspec'] = get_object_or_404( FittingSpec.objects.visible_to_user(self.request.user), - pk=self.request.GET['fittingspec']) - elif 'dataset' in self.request.GET: + pk=fittingspec_id) + + elif dataset_id: initial['dataset'] = get_object_or_404( Dataset.objects.visible_to_user(self.request.user), - pk=self.request.GET['dataset']) + pk=dataset_id) return initial @@ -287,7 +313,6 @@ def get(self, request, *args, **kwargs): fittingspecs = FittingSpec.objects.filter(cachedfittingspec__versions__in=fittingspec_versions) datasets = Dataset.objects.visible_to_user(request.user) - def _get_int_param(fieldname, _model): try: pk = int(request.GET.get(fieldname, '')) From 68742835e8b531c11cb7122a0b96317acad2d6e4 Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Fri, 25 Sep 2020 12:35:15 +0000 Subject: [PATCH 25/37] Fitting form: allow specifying multiple entity types --- weblab/fitting/views.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index efa2929a4..636b8e52b 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -236,7 +236,7 @@ def get_initial(self): ModelEntity.objects.visible_to_user(self.request.user), pk=model_id) - elif protocol_version_id: + if protocol_version_id: initial['protocol_version'] = get_object_or_404( CachedProtocolVersion.objects.visible_to_user(self.request.user), pk=protocol_version_id) @@ -246,7 +246,7 @@ def get_initial(self): ProtocolEntity.objects.visible_to_user(self.request.user), pk=protocol_id) - elif fittingspec_version_id: + if fittingspec_version_id: initial['fittingspec_version'] = get_object_or_404( CachedFittingSpecVersion.objects.visible_to_user(self.request.user), pk=fittingspec_version_id) @@ -256,7 +256,7 @@ def get_initial(self): FittingSpec.objects.visible_to_user(self.request.user), pk=fittingspec_id) - elif dataset_id: + if dataset_id: initial['dataset'] = get_object_or_404( Dataset.objects.visible_to_user(self.request.user), pk=dataset_id) From c6f46dde2332ef77b4acd125234c450b68660e4f Mon Sep 17 00:00:00 2001 From: Helen Sherwood-Taylor Date: Fri, 25 Sep 2020 12:36:27 +0000 Subject: [PATCH 26/37] Pass entity version id to fitting form --- weblab/templates/entities/entity_version.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/weblab/templates/entities/entity_version.html b/weblab/templates/entities/entity_version.html index 9eff0bddb..c110ea4cb 100644 --- a/weblab/templates/entities/entity_version.html +++ b/weblab/templates/entities/entity_version.html @@ -64,11 +64,11 @@

  • Other versions
  • {% if entity.entity_type == 'model' %} - Fit + Fit {% elif entity.entity_type == 'protocol' %} - Fit + Fit {% elif entity.entity_type == 'fittingspec' %} - Fit + Fit {% endif %}