From e70b5251a8ef3fe9d4c4fb09875b0de2b365be23 Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Thu, 28 Nov 2019 14:25:04 +0000 Subject: [PATCH 01/30] Reduce direct uses of `Entity.objects` --- weblab/entities/tests/test_views.py | 13 ++++--------- weblab/entities/views.py | 14 ++++++++++---- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/weblab/entities/tests/test_views.py b/weblab/entities/tests/test_views.py index d45a331da..492f75495 100644 --- a/weblab/entities/tests/test_views.py +++ b/weblab/entities/tests/test_views.py @@ -15,12 +15,7 @@ from guardian.shortcuts import assign_perm from core import recipes -from entities.models import ( - AnalysisTask, - Entity, - ModelEntity, - ProtocolEntity, -) +from entities.models import AnalysisTask, ModelEntity, ProtocolEntity from experiments.models import Experiment, PlannedExperiment from repocache.models import ProtocolInterface @@ -101,7 +96,7 @@ def test_owner_can_delete_entity( entity = recipe.make(author=logged_in_user) repo_path = entity.repo_abs_path helpers.add_version(entity) - assert Entity.objects.filter(pk=entity.pk).exists() + assert type(entity).objects.filter(pk=entity.pk).exists() assert repo_path.exists() response = client.post(url % entity.pk) @@ -109,7 +104,7 @@ def test_owner_can_delete_entity( assert response.status_code == 302 assert response.url == list_url - assert not Entity.objects.filter(pk=entity.pk).exists() + assert not type(entity).objects.filter(pk=entity.pk).exists() assert not repo_path.exists() @pytest.mark.usefixtures('logged_in_user') @@ -125,7 +120,7 @@ def test_non_owner_cannot_delete_entity( response = client.post(url % entity.pk) assert response.status_code == 403 - assert Entity.objects.filter(pk=entity.pk).exists() + assert type(entity).objects.filter(pk=entity.pk).exists() assert repo_path.exists() diff --git a/weblab/entities/views.py b/weblab/entities/views.py index 059c03081..7ce9d0033 100644 --- a/weblab/entities/views.py +++ b/weblab/entities/views.py @@ -68,6 +68,14 @@ def model(self): if et.entity_type == self.kwargs['entity_type'] ) + @property + def other_model(self): + return next( + et + for et in (ModelEntity, ProtocolEntity) + if et.entity_type != self.kwargs['entity_type'] + ) + def get_context_data(self, **kwargs): kwargs.update({ 'type': self.model.entity_type, @@ -294,7 +302,7 @@ def get_context_data(self, **kwargs): for version in self.kwargs['versions'].strip('/').split('/'): id, sha = version.split(':') try: - entity = Entity.objects.get(pk=id) + entity = self.model.objects.get(pk=id) if entity.is_version_visible_to_user(sha, self.request.user): valid_versions.append(version) except (RepoCacheMiss, Entity.DoesNotExist): @@ -1040,9 +1048,7 @@ def get_context_data(self, **kwargs): # ended up using a nested dict as nested lists caused django's unpacking in forloops to # mess things up slightly cached_name = 'cached' + entity.other_type - other_entities = Entity.objects.filter( - entity_type=entity.other_type - ).select_related( + other_entities = self.other_model.objects.select_related( cached_name ).prefetch_related( cached_name + '__versions', From c24b1d3f62826ccd1b998a155988526d2ddbf84d Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Thu, 28 Nov 2019 16:37:17 +0000 Subject: [PATCH 02/30] Add basic `FittingSpec` model in a new `fitting` app This also involved adding a new `create_fittingspec` permission and linking it up in a few unexpected places. Probably some more basic tests are still needed but it's a start. --- weblab/accounts/views.py | 1 + weblab/config/settings/base.py | 1 + weblab/core/recipes.py | 5 ++ .../migrations/0015_auto_20191128_1601.py | 24 ++++++++ weblab/entities/models.py | 3 + weblab/fitting/__init__.py | 0 weblab/fitting/admin.py | 6 ++ weblab/fitting/apps.py | 17 ++++++ weblab/fitting/migrations/0001_initial.py | 29 +++++++++ weblab/fitting/migrations/__init__.py | 0 weblab/fitting/models.py | 45 ++++++++++++++ weblab/fitting/tests/__init__.py | 0 weblab/fitting/tests/test_models.py | 60 +++++++++++++++++++ weblab/fitting/views.py | 3 + 14 files changed, 194 insertions(+) create mode 100644 weblab/entities/migrations/0015_auto_20191128_1601.py create mode 100644 weblab/fitting/__init__.py create mode 100644 weblab/fitting/admin.py create mode 100644 weblab/fitting/apps.py create mode 100644 weblab/fitting/migrations/0001_initial.py create mode 100644 weblab/fitting/migrations/__init__.py create mode 100644 weblab/fitting/models.py create mode 100644 weblab/fitting/tests/__init__.py create mode 100644 weblab/fitting/tests/test_models.py create mode 100644 weblab/fitting/views.py diff --git a/weblab/accounts/views.py b/weblab/accounts/views.py index ffd183eb1..b397f49c8 100644 --- a/weblab/accounts/views.py +++ b/weblab/accounts/views.py @@ -37,6 +37,7 @@ def get_success_url(self): def get_context_data(self, **kwargs): perms = { + 'entities.create_fittingspec', 'entities.create_protocol', 'entities.create_model', } diff --git a/weblab/config/settings/base.py b/weblab/config/settings/base.py index 6c98030a5..94cd9b89a 100644 --- a/weblab/config/settings/base.py +++ b/weblab/config/settings/base.py @@ -56,6 +56,7 @@ 'datasets', 'entities', 'experiments', + 'fitting', 'repocache', ] diff --git a/weblab/core/recipes.py b/weblab/core/recipes.py index e3cd827cd..729fe02a5 100644 --- a/weblab/core/recipes.py +++ b/weblab/core/recipes.py @@ -11,6 +11,11 @@ 'ProtocolEntity', entity_type='protocol', name=seq('myprotocol') ) +fittingspec = Recipe( + 'FittingSpec', + entity_type='fittingspec', name=seq('myspec'), + protocol=foreign_key(protocol), +) model_file = Recipe('EntityFile', entity=foreign_key(model)) protocol_file = Recipe('EntityFile', entity=foreign_key(protocol)) diff --git a/weblab/entities/migrations/0015_auto_20191128_1601.py b/weblab/entities/migrations/0015_auto_20191128_1601.py new file mode 100644 index 000000000..6f200f4cb --- /dev/null +++ b/weblab/entities/migrations/0015_auto_20191128_1601.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-11-28 16:01 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('entities', '0014_entity_is_fitting_spec'), + ] + + operations = [ + migrations.AlterModelOptions( + name='entity', + options={'ordering': ['name'], 'permissions': (('create_model', 'Can create models'), ('create_protocol', 'Can create protocols'), ('create_fittingspec', 'Can create fitting specifications'), ('edit_entity', 'Can edit entity'), ('moderator', 'Can promote public entity versions to moderated'))}, + ), + migrations.AlterField( + model_name='entity', + name='entity_type', + field=models.CharField(choices=[('model', 'model'), ('protocol', 'protocol'), ('fittingspec', 'fittingspec')], max_length=16), + ), + ] diff --git a/weblab/entities/models.py b/weblab/entities/models.py index 3ce310a76..8a1724d8e 100644 --- a/weblab/entities/models.py +++ b/weblab/entities/models.py @@ -25,9 +25,11 @@ class Entity(UserCreatedModelMixin, models.Model): ENTITY_TYPE_MODEL = 'model' ENTITY_TYPE_PROTOCOL = 'protocol' + ENTITY_TYPE_FITTINGSPEC = 'fittingspec' ENTITY_TYPE_CHOICES = ( (ENTITY_TYPE_MODEL, ENTITY_TYPE_MODEL), (ENTITY_TYPE_PROTOCOL, ENTITY_TYPE_PROTOCOL), + (ENTITY_TYPE_FITTINGSPEC, ENTITY_TYPE_FITTINGSPEC), ) entity_type = models.CharField( @@ -48,6 +50,7 @@ class Meta: permissions = ( ('create_model', 'Can create models'), ('create_protocol', 'Can create protocols'), + ('create_fittingspec', 'Can create fitting specifications'), # Edit entity is used as an object-level permission ('edit_entity', 'Can edit entity'), ('moderator', 'Can promote public entity versions to moderated'), diff --git a/weblab/fitting/__init__.py b/weblab/fitting/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/weblab/fitting/admin.py b/weblab/fitting/admin.py new file mode 100644 index 000000000..11d38a313 --- /dev/null +++ b/weblab/fitting/admin.py @@ -0,0 +1,6 @@ +from django.contrib import admin + +from .models import FittingSpec + + +admin.site.register(FittingSpec) diff --git a/weblab/fitting/apps.py b/weblab/fitting/apps.py new file mode 100644 index 000000000..690d60e47 --- /dev/null +++ b/weblab/fitting/apps.py @@ -0,0 +1,17 @@ +from django.apps import AppConfig +from django.db.models.signals import post_save, pre_delete + +from entities.signals import entity_created, entity_deleted + + +class FittingConfig(AppConfig): + name = 'fitting' + + def ready(self): + from .models import FittingSpec + + # Messages might come from the base Entity class or our new subclass + # depending on how the views are set up / where the action is invoked. + # This covers all bases for creation and deletion. + post_save.connect(entity_created, FittingSpec) + pre_delete.connect(entity_deleted, FittingSpec) diff --git a/weblab/fitting/migrations/0001_initial.py b/weblab/fitting/migrations/0001_initial.py new file mode 100644 index 000000000..ada5ea8a4 --- /dev/null +++ b/weblab/fitting/migrations/0001_initial.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-11-28 16:01 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('entities', '0015_auto_20191128_1601'), + ] + + operations = [ + migrations.CreateModel( + name='FittingSpec', + fields=[ + ('entity_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='entities.Entity')), + ('protocol', models.ForeignKey(help_text='the experimental scenario used to fit models', on_delete=django.db.models.deletion.CASCADE, related_name='fitting_specs', to='entities.ProtocolEntity')), + ], + options={ + 'verbose_name': 'fitting specification', + }, + bases=('entities.entity',), + ), + ] diff --git a/weblab/fitting/migrations/__init__.py b/weblab/fitting/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/weblab/fitting/models.py b/weblab/fitting/models.py new file mode 100644 index 000000000..998829d41 --- /dev/null +++ b/weblab/fitting/models.py @@ -0,0 +1,45 @@ +from django.db import models + +from entities.models import Entity, EntityManager, ProtocolEntity + + +class FittingSpec(Entity): + """ + Represents parameter fitting specifications. + These are versioned entities, backed by a git repository. + + It links to a ProtocolEntity (not a specific version thereof) representing + the experimental scenario which can be used to fit models. + + Running a fitting specification with (specific versions of) a ModelEntity, + ProtocolEntity and Dataset will result in a FittingResult being generated. + """ + entity_type = Entity.ENTITY_TYPE_FITTINGSPEC + other_type = Entity.ENTITY_TYPE_MODEL + is_fitting_spec = True + + protocol = models.ForeignKey( + ProtocolEntity, related_name='fitting_specs', + help_text='the experimental scenario used to fit models', + ) + + objects = EntityManager() + + class Meta: + verbose_name = 'fitting specification' + + # The 'edit_entity' object-level permission is only in the entities app, + # so we need to delegate via our parent link when accessing it. + + def is_editable_by(self, user): + return self.entity_ptr.is_editable_by(user) + + def add_collaborator(self, user): + return self.entity_ptr.add_collaborator(user) + + def remove_collaborator(self, user): + return self.entity_ptr.remove_collaborator(user) + + @property + def collaborators(self): + return self.entity_ptr.collaborators diff --git a/weblab/fitting/tests/__init__.py b/weblab/fitting/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/weblab/fitting/tests/test_models.py b/weblab/fitting/tests/test_models.py new file mode 100644 index 000000000..4746a23dc --- /dev/null +++ b/weblab/fitting/tests/test_models.py @@ -0,0 +1,60 @@ +import pytest +from django.db.utils import IntegrityError +from django.shortcuts import get_object_or_404 +from guardian.shortcuts import assign_perm + +from core import recipes + + +@pytest.mark.django_db +class TestNameUniqueness: + def test_user_cannot_have_same_named_fittingspec(self, user): + recipes.fittingspec.make(author=user, name='myspec') + + with pytest.raises(IntegrityError): + recipes.fittingspec.make(author=user, name='myspec') + + def test_user_can_have_same_named_fittingspec_and_other_entities(self, user): + recipes.fittingspec.make(author=user, name='myentity') + recipes.model.make(author=user, name='myentity') + recipes.protocol.make(author=user, name='myentity') + + def test_different_users_can_have_same_named_fittingspec(self, user, other_user): + recipes.fittingspec.make(author=user, name='myspec') + assert recipes.fittingspec.make(author=other_user, name='myspec') + + +@pytest.mark.django_db +def test_permissions(): + user, other_user = recipes.user.make(_quantity=2) + superuser = recipes.user.make(is_superuser=True) + fittingspec = recipes.fittingspec.make(author=user) + + assert fittingspec.viewers == {user} + + assert not fittingspec.is_editable_by(user) + assert fittingspec.is_editable_by(superuser) + assert not fittingspec.is_editable_by(other_user) + + assign_perm('entities.create_fittingspec', user) + user = get_object_or_404(user.__class__, pk=user.id) # Reset permission cache! + assert fittingspec.is_editable_by(user) + + assert fittingspec.is_deletable_by(user) + assert fittingspec.is_deletable_by(superuser) + assert not fittingspec.is_deletable_by(other_user) + + fittingspec.add_collaborator(other_user) + assert other_user in fittingspec.collaborators + assert fittingspec.viewers == {user, other_user} + assert not fittingspec.is_editable_by(other_user) + assert not fittingspec.is_deletable_by(other_user) + + assign_perm('entities.create_fittingspec', other_user) + other_user = get_object_or_404(user.__class__, pk=other_user.id) # Reset permission cache! + assert fittingspec.is_editable_by(other_user) + + fittingspec.remove_collaborator(other_user) + assert other_user not in fittingspec.collaborators + assert not fittingspec.is_editable_by(other_user) + assert not fittingspec.is_deletable_by(other_user) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py new file mode 100644 index 000000000..fd0e04495 --- /dev/null +++ b/weblab/fitting/views.py @@ -0,0 +1,3 @@ +# from django.shortcuts import render + +# Create your views here. From 1f2dd97c7a08741b4f0906c4227df269dc99582b Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Thu, 28 Nov 2019 16:39:38 +0000 Subject: [PATCH 03/30] Drive-by docstring fixing --- weblab/accounts/managers.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/weblab/accounts/managers.py b/weblab/accounts/managers.py index 7461236cc..ac59196f4 100644 --- a/weblab/accounts/managers.py +++ b/weblab/accounts/managers.py @@ -7,8 +7,7 @@ def admins(self): def create_user(self, email, full_name, institution='', password=None): """ - Creates and saves a superuser with the given email, date of - birth and password. + Creates and saves a user with the given details and password. """ user = self.model( email=self.normalize_email(email), @@ -22,8 +21,7 @@ def create_user(self, email, full_name, institution='', password=None): def create_superuser(self, email, full_name, institution, password): """ - Creates and saves a superuser with the given email, date of - birth and password. + Creates and saves a superuser with the given details and password. """ user = self.create_user( email, From 84aecb8e739e57ebcb18ad193b4894222d821754 Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Thu, 28 Nov 2019 17:05:26 +0000 Subject: [PATCH 04/30] Add repocache for fitting specs --- weblab/entities/signals.py | 3 ++- weblab/fitting/tests/test_models.py | 31 ++++++++++++++++++++++++++++- weblab/repocache/models.py | 22 ++++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/weblab/entities/signals.py b/weblab/entities/signals.py index 62f674d0c..0491ba4d1 100644 --- a/weblab/entities/signals.py +++ b/weblab/entities/signals.py @@ -11,4 +11,5 @@ def entity_deleted(sender, instance, **kwargs): """ Signal callback when an entity is about to be deleted. """ - instance.repo.delete() + if instance.repo_abs_path.exists(): + instance.repo.delete() diff --git a/weblab/fitting/tests/test_models.py b/weblab/fitting/tests/test_models.py index 4746a23dc..445276bac 100644 --- a/weblab/fitting/tests/test_models.py +++ b/weblab/fitting/tests/test_models.py @@ -4,12 +4,14 @@ from guardian.shortcuts import assign_perm from core import recipes +from repocache.models import CachedFittingSpec @pytest.mark.django_db class TestNameUniqueness: def test_user_cannot_have_same_named_fittingspec(self, user): - recipes.fittingspec.make(author=user, name='myspec') + spec = recipes.fittingspec.make(author=user, name='myspec') + assert str(spec) == 'myspec' with pytest.raises(IntegrityError): recipes.fittingspec.make(author=user, name='myspec') @@ -58,3 +60,30 @@ def test_permissions(): assert other_user not in fittingspec.collaborators assert not fittingspec.is_editable_by(other_user) assert not fittingspec.is_deletable_by(other_user) + + +@pytest.mark.django_db +class TestRepository: + def test_repo_path_create_and_delete(self, fake_repo_path): + spec = recipes.fittingspec.make() + path = '%s/%d/fittingspecs/%d' % (fake_repo_path, spec.author.pk, spec.pk) + + assert spec.repo._root == path + assert str(spec.repo_abs_path) == path + + def test_repo_is_deleted(self): + spec = recipes.fittingspec.make() + spec.repo.create() + assert spec.repo_abs_path.exists() + spec.delete() + assert not spec.repo_abs_path.exists() + + def test_get_repocache(self): + spec = recipes.fittingspec.make() + assert CachedFittingSpec.objects.count() == 0 + assert spec.repocache + assert CachedFittingSpec.objects.count() == 1 + assert spec.repocache + assert CachedFittingSpec.objects.count() == 1 + spec.delete() + assert CachedFittingSpec.objects.count() == 0 diff --git a/weblab/repocache/models.py b/weblab/repocache/models.py index 501539e02..e1f06b61a 100644 --- a/weblab/repocache/models.py +++ b/weblab/repocache/models.py @@ -4,6 +4,7 @@ from core.models import VisibilityModelMixin from core.visibility import Visibility from entities.models import ModelEntity, ProtocolEntity +from fitting.models import FittingSpec from .exceptions import RepoCacheMiss @@ -203,14 +204,35 @@ class CachedProtocolTag(CachedEntityTagMixin): _set_class_links(CachedProtocol, CachedProtocolVersion, CachedProtocolTag) +class CachedFittingSpec(CachedEntityMixin): + """Cache for a fitting specifications's repository.""" + entity = models.OneToOneField(FittingSpec, on_delete=models.CASCADE, related_name='cachedfittingspec') + + +class CachedFittingSpecVersion(CachedEntityVersionMixin): + """Cache for a single version / commit in a fitting specifications's repository.""" + entity = models.ForeignKey(CachedFittingSpec, on_delete=models.CASCADE, related_name='versions') + + +class CachedFittingSpecTag(CachedEntityTagMixin): + """Cache for a tag in a fitting specifications's repository.""" + entity = models.ForeignKey(CachedFittingSpec, related_name='tags') + version = models.ForeignKey(CachedFittingSpecVersion, on_delete=models.CASCADE, related_name='tags') + + +_set_class_links(CachedFittingSpec, CachedFittingSpecVersion, CachedFittingSpecTag) + + CACHE_TYPE_MAP = { 'model': CachedModel, 'protocol': CachedProtocol, + 'fittingspec': CachedFittingSpec, } CACHED_VERSION_TYPE_MAP = { 'model': CachedModelVersion, 'protocol': CachedProtocolVersion, + 'fittingspec': CachedFittingSpecVersion, } From 9f77284c3127a4e85ac10e0b00e5f724f7909b9d Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Fri, 29 Nov 2019 16:48:02 +0000 Subject: [PATCH 05/30] Illustrate a potential approach to fitting views --- weblab/config/urls.py | 1 + weblab/entities/views.py | 1 + weblab/fitting/forms.py | 13 +++++ weblab/fitting/urls.py | 32 ++++++++++++ weblab/fitting/views.py | 60 +++++++++++++++++++++- weblab/templates/entities/entity_list.html | 2 +- 6 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 weblab/fitting/forms.py create mode 100644 weblab/fitting/urls.py diff --git a/weblab/config/urls.py b/weblab/config/urls.py index 619b7ad1f..debb2b0f4 100644 --- a/weblab/config/urls.py +++ b/weblab/config/urls.py @@ -32,5 +32,6 @@ url(r'^entities/', include('entities.urls', namespace='entities')), url(r'^datasets/', include('datasets.urls', namespace='datasets')), url(r'^experiments/', include('experiments.urls', namespace='experiments')), + url(r'^fitting/', include('fitting.urls', namespace='fitting')), url(r'^admin/', admin.site.urls), ] diff --git a/weblab/entities/views.py b/weblab/entities/views.py index 7ce9d0033..1fe4c1021 100644 --- a/weblab/entities/views.py +++ b/weblab/entities/views.py @@ -80,6 +80,7 @@ def get_context_data(self, **kwargs): kwargs.update({ 'type': self.model.entity_type, 'other_type': self.model.other_type, + 'current_namespace': 'entities', }) return super().get_context_data(**kwargs) diff --git a/weblab/fitting/forms.py b/weblab/fitting/forms.py new file mode 100644 index 000000000..8e479d2df --- /dev/null +++ b/weblab/fitting/forms.py @@ -0,0 +1,13 @@ +from entities.forms import EntityForm + +from .models import FittingSpec + + +class FittingSpecForm(EntityForm): + """Used for creating an entirely new fitting specification.""" + class Meta: + model = FittingSpec + fields = ['name'] + + # TODO: Add protocol link, like for datasets (ensuring visibility respected) + # Perhaps sort available protocols so 'mine' first, then moderated, then others? diff --git a/weblab/fitting/urls.py b/weblab/fitting/urls.py new file mode 100644 index 000000000..8b0334a27 --- /dev/null +++ b/weblab/fitting/urls.py @@ -0,0 +1,32 @@ +from django.conf.urls import url + +from . import views + + +_ENTITY_TYPE = '(?Pfittingspec)s' + +urlpatterns = [ + url( + r'^%s/$' % _ENTITY_TYPE, + views.FittingSpecListView.as_view(), + name='list', + ), + + url( + r'^%s/new$' % _ENTITY_TYPE, + views.FittingSpecCreateView.as_view(), + name='new', + ), + + # url( + # r'^specs/(?P\d+)$', + # views.FittingSpecView.as_view(), + # name='detail', + # ), + + # url( + # r'^specs/(?P\d+)/delete$', + # views.FittingSpecDeleteView.as_view(), + # name='delete', + # ), +] diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index fd0e04495..67ceb3a67 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -1,3 +1,59 @@ -# from django.shortcuts import render +""" +Views for fitting specifications and results. -# Create your views here. +As far as possible these reuse code & templates from entities and experiments. + +At present I'm not sure what the best way to do this is. Many of the forms & views will +be the same, but some will differ, and sometimes quite a long way in. So we may need to +create separate versions of everything (but reuse code & templates where possible). Or +we may be able to extend the base classes to be able to delegate to elsewhere, for instance +by removing the hardcoded 'entities:' namespace from reverse() calls. +""" + +from braces.views import UserFormKwargsMixin +from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin +from django.urls import reverse +from django.views.generic.edit import CreateView +from django.views.generic.list import ListView + +from .forms import FittingSpecForm +from .models import FittingSpec + + +class FittingSpecMixin: + """ + Mixin for entity-style pages, setting the entity type to be a fitting specification. + """ + model = FittingSpec + other_model = None + + def get_context_data(self, **kwargs): + kwargs.update({ + 'type': self.model.entity_type, + 'current_namespace': 'fitting', + }) + return super().get_context_data(**kwargs) + + +class FittingSpecCreateView( + LoginRequiredMixin, PermissionRequiredMixin, FittingSpecMixin, + UserFormKwargsMixin, CreateView +): + """Create a new fitting specification, initially without any versions.""" + template_name = 'entities/entity_form.html' + permission_required = 'entities.create_fittingspec' + form_class = FittingSpecForm + + def get_success_url(self): + return reverse('fitting:newspecversion', + args=[self.object.pk]) + + +class FittingSpecListView(LoginRequiredMixin, FittingSpecMixin, ListView): + """ + List all user's fitting specifications. + """ + template_name = 'entities/entity_list.html' + + def get_queryset(self): + return self.model.objects.filter(author=self.request.user) diff --git a/weblab/templates/entities/entity_list.html b/weblab/templates/entities/entity_list.html index abecfea0d..95fed1a23 100644 --- a/weblab/templates/entities/entity_list.html +++ b/weblab/templates/entities/entity_list.html @@ -14,7 +14,7 @@

Your {{ type }}s

{% can_create_entity type as permission %} {% if permission %} - Create a new {{ type }} + Create a new {{ type }} {% else %} Your account doesn't have the authority to upload {{ type }}s; please contact us to request permission. {% endif %} From ac0e86b89d70d555c1186cb8854b181b789b6c44 Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Mon, 2 Dec 2019 16:57:34 +0000 Subject: [PATCH 06/30] Add `url_type` and `display_type` entity properties These let us write nicer templates that generalise to fitting specs. --- weblab/entities/models.py | 16 ++++++++++++++++ weblab/entities/views.py | 4 +++- weblab/fitting/models.py | 4 ++++ weblab/fitting/urls.py | 3 ++- weblab/fitting/views.py | 5 ++++- weblab/templates/entities/compare.html | 4 ++-- .../entities/entity_collaborators_form.html | 6 +++--- .../entities/entity_confirm_delete.html | 4 ++-- weblab/templates/entities/entity_list.html | 6 +++--- weblab/templates/entities/entity_version.html | 4 ++-- 10 files changed, 41 insertions(+), 15 deletions(-) diff --git a/weblab/entities/models.py b/weblab/entities/models.py index 8a1724d8e..b1ac36175 100644 --- a/weblab/entities/models.py +++ b/weblab/entities/models.py @@ -59,6 +59,22 @@ class Meta: def __str__(self): return self.name + @property + def display_type(self): + """Used to display the type of this entity in templates. + + Defaults to ``entity_type`` but may be changed by subclasses. + """ + return self.entity_type + + @property + def url_type(self): + """Used as a URL fragment to refer to this entity type. + + Defaults to ``entity_type`` but may be changed by subclasses. + """ + return self.entity_type + @property def repo(self): """This entity's git repository wrapper. diff --git a/weblab/entities/views.py b/weblab/entities/views.py index 1fe4c1021..d5bfa2262 100644 --- a/weblab/entities/views.py +++ b/weblab/entities/views.py @@ -78,8 +78,10 @@ def other_model(self): def get_context_data(self, **kwargs): kwargs.update({ - 'type': self.model.entity_type, + 'entity_type': self.model.entity_type, 'other_type': self.model.other_type, + 'type': self.model.display_type, + 'url_type': self.model.url_type, 'current_namespace': 'entities', }) return super().get_context_data(**kwargs) diff --git a/weblab/fitting/models.py b/weblab/fitting/models.py index 998829d41..71d57ae29 100644 --- a/weblab/fitting/models.py +++ b/weblab/fitting/models.py @@ -28,6 +28,10 @@ class FittingSpec(Entity): class Meta: verbose_name = 'fitting specification' + # We change the default display & URL form for this entity type, since the default looks bad! + display_type = Meta.verbose_name + url_type = 'spec' + # The 'edit_entity' object-level permission is only in the entities app, # so we need to delegate via our parent link when accessing it. diff --git a/weblab/fitting/urls.py b/weblab/fitting/urls.py index 8b0334a27..fa3ce2a0c 100644 --- a/weblab/fitting/urls.py +++ b/weblab/fitting/urls.py @@ -1,9 +1,10 @@ from django.conf.urls import url from . import views +from .models import FittingSpec -_ENTITY_TYPE = '(?Pfittingspec)s' +_ENTITY_TYPE = '(?P%s)s' % FittingSpec.url_type urlpatterns = [ url( diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 67ceb3a67..5ebda9131 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -29,7 +29,10 @@ class FittingSpecMixin: def get_context_data(self, **kwargs): kwargs.update({ - 'type': self.model.entity_type, + 'entity_type': self.model.entity_type, + 'other_type': self.model.other_type, + 'type': self.model.display_type, + 'url_type': self.model.url_type, 'current_namespace': 'fitting', }) return super().get_context_data(**kwargs) diff --git a/weblab/templates/entities/compare.html b/weblab/templates/entities/compare.html index 95ff569cc..8fcbf4282 100644 --- a/weblab/templates/entities/compare.html +++ b/weblab/templates/entities/compare.html @@ -11,8 +11,8 @@

Comparison of {{ type }}s

loading...
diff --git a/weblab/templates/entities/entity_collaborators_form.html b/weblab/templates/entities/entity_collaborators_form.html index 42366b23e..844e712bc 100644 --- a/weblab/templates/entities/entity_collaborators_form.html +++ b/weblab/templates/entities/entity_collaborators_form.html @@ -2,18 +2,18 @@ {% load entities %} {% load staticfiles %} -{% block title %}{{ entity.entity_type|capfirst }} collaborators - {% endblock title %} +{% block title %}{{ entity.display_type|capfirst }} collaborators - {% endblock title %} {% block content %} {% include "./includes/entity_header.html" %} -

Add or remove collaborators for this {{ entity.entity_type }}

+

Add or remove collaborators for this {{ entity.display_type }}

{% csrf_token %} {{ formset.management_form }} -

Collaborators can create new versions of the {{ entity.entity_type }}, add tags and change the {{ entity.entity_type }} visibility.

+

Collaborators can create new versions of the {{ entity.display_type }}, add tags and change the {{ entity.display_type }} visibility.

diff --git a/weblab/templates/entities/entity_confirm_delete.html b/weblab/templates/entities/entity_confirm_delete.html index 5a5388fe8..523832589 100644 --- a/weblab/templates/entities/entity_confirm_delete.html +++ b/weblab/templates/entities/entity_confirm_delete.html @@ -1,11 +1,11 @@ {% extends "base.html" %} -{% block title %}Delete {{ object.entity_type }} - {% endblock title %} +{% block title %}Delete {{ object.display_type }} - {% endblock title %} {% block content %} {% csrf_token %} -

Are you sure you want to delete all versions of {{ object.entity_type }} "{{ object }}"?

+

Are you sure you want to delete all versions of {{ object.display_type }} "{{ object }}"?

This operation cannot be undone.

diff --git a/weblab/templates/entities/entity_list.html b/weblab/templates/entities/entity_list.html index 95fed1a23..e6764d86c 100644 --- a/weblab/templates/entities/entity_list.html +++ b/weblab/templates/entities/entity_list.html @@ -9,12 +9,12 @@ protocolsexperiments -
+

Your {{ type }}s

- {% can_create_entity type as permission %} + {% can_create_entity entity_type as permission %} {% if permission %} - Create a new {{ type }} + Create a new {{ type }} {% else %} Your account doesn't have the authority to upload {{ type }}s; please contact us to request permission. {% endif %} diff --git a/weblab/templates/entities/entity_version.html b/weblab/templates/entities/entity_version.html index 2b70c0e34..d82aa4c1a 100644 --- a/weblab/templates/entities/entity_version.html +++ b/weblab/templates/entities/entity_version.html @@ -52,7 +52,7 @@

Run experiments: Run experiments: + title="Run experiments using this {{ type }}"/> {% endif %} @@ -60,7 +60,7 @@

From 5d43ecef530802c053d99ca6f2f22c4b7dedb224 Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Fri, 13 Dec 2019 16:55:30 +0000 Subject: [PATCH 20/30] Fix style checks --- weblab/entities/tests/test_templatetags.py | 4 +++- weblab/fitting/views.py | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/weblab/entities/tests/test_templatetags.py b/weblab/entities/tests/test_templatetags.py index c1f623343..9cde55543 100644 --- a/weblab/entities/tests/test_templatetags.py +++ b/weblab/entities/tests/test_templatetags.py @@ -88,7 +88,9 @@ def test_protocol_urls(protocol_with_version): assert entity_tags.url_entity_comparison_base(context, 'protocol') == '/entities/protocols/compare' assert entity_tags.url_entity_diff_base(context, 'protocol') == '/entities/protocols/diff' - assert (entity_tags.entity_comparison_json_url(context, ['%d:%s' % (protocol.pk, protocol_version.hexsha)], 'protocol') == + assert (entity_tags.entity_comparison_json_url(context, + ['%d:%s' % (protocol.pk, protocol_version.hexsha)], + 'protocol') == '/entities/protocols/compare/%d:%s/info' % (protocol.pk, protocol_version.hexsha)) diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 7a5f6f214..764a3d5f1 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -14,12 +14,10 @@ from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin from django.urls import reverse from django.views.generic.edit import CreateView -# from django.views.generic.list import ListView from entities.views import EntityTypeMixin from .forms import FittingSpecForm -# from .models import FittingSpec class FittingSpecCreateView( From ff8cf5f3ef3e238a5845255d0317c3fea247b19f Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Fri, 3 Jan 2020 10:27:35 +0000 Subject: [PATCH 21/30] Adapt to new base class names --- weblab/repocache/models.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/weblab/repocache/models.py b/weblab/repocache/models.py index bcbb1fc86..602c63c20 100644 --- a/weblab/repocache/models.py +++ b/weblab/repocache/models.py @@ -204,17 +204,17 @@ class CachedProtocolTag(CachedEntityTag): _set_class_links(CachedProtocol, CachedProtocolVersion, CachedProtocolTag) -class CachedFittingSpec(CachedEntityMixin): +class CachedFittingSpec(CachedEntity): """Cache for a fitting specifications's repository.""" entity = models.OneToOneField(FittingSpec, on_delete=models.CASCADE, related_name='cachedfittingspec') -class CachedFittingSpecVersion(CachedEntityVersionMixin): +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') -class CachedFittingSpecTag(CachedEntityTagMixin): +class CachedFittingSpecTag(CachedEntityTag): """Cache for a tag in a fitting specifications's repository.""" entity = models.ForeignKey(CachedFittingSpec, related_name='tags') version = models.ForeignKey(CachedFittingSpecVersion, on_delete=models.CASCADE, related_name='tags') From f9a9b415a9c9eb87286d5aa6b8de807eaca83fda Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Fri, 10 Jan 2020 16:58:32 +0000 Subject: [PATCH 22/30] Re-run expts is not an option for fitting specs --- weblab/entities/forms.py | 4 +++- weblab/fitting/forms.py | 10 +++++++++- weblab/fitting/urls.py | 2 +- weblab/fitting/views.py | 12 ++++++++++-- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/weblab/entities/forms.py b/weblab/entities/forms.py index af2f1c76c..2ef48547a 100644 --- a/weblab/entities/forms.py +++ b/weblab/entities/forms.py @@ -69,7 +69,9 @@ class EntityVersionForm(forms.Form): def __init__(self, *args, **kwargs): entity_type = kwargs.pop('entity_type') super().__init__(*args, **kwargs) - self.fields['rerun_expts'].label = self.fields['rerun_expts'].label % entity_type + rerun_field = self.fields.get('rerun_expts', None) + if rerun_field: + rerun_field.label = rerun_field.label % entity_type class EntityChangeVisibilityForm(UserKwargModelFormMixin, forms.Form): diff --git a/weblab/fitting/forms.py b/weblab/fitting/forms.py index 053893c60..92990713e 100644 --- a/weblab/fitting/forms.py +++ b/weblab/fitting/forms.py @@ -1,4 +1,4 @@ -from entities.forms import EntityForm +from entities.forms import EntityForm, EntityVersionForm from entities.models import ProtocolEntity from .models import FittingSpec @@ -16,3 +16,11 @@ def __init__(self, *args, **kwargs): self.fields['protocol'].queryset = ProtocolEntity.objects.visible_to_user(self.user) # TODO: Perhaps sort available protocols so 'mine' first, then moderated, then others? + + +class FittingSpecVersionForm(EntityVersionForm): + """Used for creating a new version of a fitting specification. + + This works almost the same as other entities, except we can't re-run experiments. + """ + rerun_expts = None diff --git a/weblab/fitting/urls.py b/weblab/fitting/urls.py index a4662b304..9f71f9263 100644 --- a/weblab/fitting/urls.py +++ b/weblab/fitting/urls.py @@ -44,7 +44,7 @@ url( r'^%s/(?P\d+)/versions/new$' % _ENTITY_TYPE, - entity_views.EntityNewVersionView.as_view(), + views.FittingSpecNewVersionView.as_view(), name='newversion', ), diff --git a/weblab/fitting/views.py b/weblab/fitting/views.py index 764a3d5f1..5ce650eac 100644 --- a/weblab/fitting/views.py +++ b/weblab/fitting/views.py @@ -15,9 +15,9 @@ from django.urls import reverse from django.views.generic.edit import CreateView -from entities.views import EntityTypeMixin +from entities.views import EntityNewVersionView, EntityTypeMixin -from .forms import FittingSpecForm +from .forms import FittingSpecForm, FittingSpecVersionForm class FittingSpecCreateView( @@ -32,3 +32,11 @@ class FittingSpecCreateView( def get_success_url(self): return reverse('fitting:newversion', args=[self.kwargs['entity_type'], self.object.pk]) + + +class FittingSpecNewVersionView(EntityNewVersionView): + """Create a new version of a fitting specification. + + This is almost identical to other entities, except that we can't re-run experiments. + """ + form_class = FittingSpecVersionForm From 584cf8cd01198c23d0c6080c62baf62c87f7743b Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Fri, 10 Jan 2020 16:59:07 +0000 Subject: [PATCH 23/30] Make more entity views work for fitting specs From my manual testing, it looks like everything we need is now implemented! Except for automated tests, of course... --- weblab/entities/views.py | 26 ++++++++++--------- weblab/fitting/urls.py | 6 +++++ weblab/static/js/compare.js | 4 ++- weblab/templates/entities/compare.html | 4 +-- weblab/templates/entities/entity_version.html | 10 ++++--- .../templates/entities/entity_versions.html | 2 +- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/weblab/entities/views.py b/weblab/entities/views.py index 11609a73d..87d661429 100644 --- a/weblab/entities/views.py +++ b/weblab/entities/views.py @@ -210,7 +210,7 @@ def _file_json(self, blob): 'created': commit.committed_at, 'url': reverse( ns + ':file_download', - args=[obj.entity_type, obj.id, commit.hexsha, blob.name] + args=[obj.url_type, obj.id, commit.hexsha, blob.name] ), } @@ -253,15 +253,15 @@ def get(self, request, *args, **kwargs): 'planned_experiments': planned_experiments, 'url': reverse( ns + ':version', - args=[obj.entity_type, obj.id, commit.hexsha] + args=[obj.url_type, obj.id, commit.hexsha] ), 'download_url': reverse( ns + ':entity_archive', - args=[obj.entity_type, obj.id, commit.hexsha] + args=[obj.url_type, obj.id, commit.hexsha] ), 'change_url': reverse( ns + ':change_visibility', - args=[obj.entity_type, obj.id, commit.hexsha] + args=[obj.url_type, obj.id, commit.hexsha] ), } }) @@ -321,7 +321,7 @@ def get_context_data(self, **kwargs): return super().get_context_data(**kwargs) -class EntityComparisonJsonView(View): +class EntityComparisonJsonView(EntityTypeMixin, View): """ Serve up JSON view of multiple entity versions for comparison """ @@ -343,7 +343,7 @@ def _file_json(self, entity, commit, blob): 'size': blob.size, 'url': reverse( ns + ':file_download', - args=[entity.entity_type, entity.id, commit.hexsha, blob.name] + args=[entity.url_type, entity.id, commit.hexsha, blob.name] ), } @@ -378,7 +378,7 @@ def get(self, request, *args, **kwargs): for version in self.kwargs['versions'].strip('/').split('/'): id, sha = version.split(':') try: - entity = Entity.objects.get(pk=id) + entity = self.model.objects.get(pk=id) if entity.is_version_visible_to_user(sha, request.user): json_entities.append( self._version_json(entity, entity.repo.get_commit(sha)) @@ -450,7 +450,8 @@ def get_success_url(self): entity = self._get_object() version = self.kwargs['sha'] ns = self.request.resolver_match.namespace - return reverse(ns + ':version', args=[entity.entity_type, entity.id, version]) + url_type = entity.entity_type.replace('fitting', '') # TODO: Horrible hack! + return reverse(ns + ':version', args=[url_type, entity.id, version]) class EntityDeleteView(UserPassesTestMixin, DeleteView): @@ -547,10 +548,11 @@ def post(self, request, *args, **kwargs): record_experiments_to_run(request.user, entity, commit) # Show the user the new version + ns = self.request.resolver_match.namespace return JsonResponse({ self.RESPONSE_OBJECT: { 'response': True, - 'url': reverse('entities:version', args=[entity.entity_type, entity.id, commit.hexsha]), + 'url': reverse(ns + ':version', args=[entity.url_type, entity.id, commit.hexsha]), } }) @@ -576,7 +578,7 @@ def get_initial(self): def get_form_kwargs(self): """Build the kwargs required to instantiate an EntityVersionForm.""" kwargs = super().get_form_kwargs() - kwargs['entity_type'] = self.object.entity_type + kwargs['entity_type'] = self.object.display_type return kwargs def get_context_data(self, **kwargs): @@ -668,7 +670,7 @@ def post(self, request, *args, **kwargs): # Show the user the new version ns = self.request.resolver_match.namespace return HttpResponseRedirect( - reverse(ns + ':version', args=[entity.entity_type, entity.id, commit.hexsha])) + reverse(ns + ':version', args=[entity.url_type, entity.id, commit.hexsha])) else: # Nothing changed, so inform the user and do nothing else. form = self.get_form() @@ -878,7 +880,7 @@ def get_success_url(self): """What page to show when the form was processed OK.""" entity = self.object ns = self.request.resolver_match.namespace - return reverse(ns + ':entity_collaborators', args=[entity.entity_type, entity.id]) + return reverse(ns + ':entity_collaborators', args=[entity.url_type, entity.id]) def get_context_data(self, **kwargs): if 'formset' not in kwargs: diff --git a/weblab/fitting/urls.py b/weblab/fitting/urls.py index 9f71f9263..2ceb658bc 100644 --- a/weblab/fitting/urls.py +++ b/weblab/fitting/urls.py @@ -48,6 +48,12 @@ name='newversion', ), + url( + r'^%s/(?P\d+)/versions/edit$' % _ENTITY_TYPE, + entity_views.EntityAlterFileView.as_view(), + name='alter_file', + ), + url( r'^%s/(?P\d+)/versions/%s(?:/%s)?$' % (_ENTITY_TYPE, _COMMIT, _FILEVIEW), entity_views.EntityVersionView.as_view(), diff --git a/weblab/static/js/compare.js b/weblab/static/js/compare.js index 3dd0ef6a6..ec69ab403 100644 --- a/weblab/static/js/compare.js +++ b/weblab/static/js/compare.js @@ -602,10 +602,12 @@ function parseUrl (event) basicurl = parts.slice(0, i+2).join('/') + '/'; entityType = 'experiment'; entityIds = parts.slice(i+2); - } else if ((parts[i] == 'models' || parts[i] == 'protocols') && parts[i+1] == 'compare') { + break; + } else if (parts[i+1] == 'compare') { basicurl = parts.slice(0, i+2).join('/') + '/'; entityType = parts[i].slice(0, parts[i].length-1); entityIds = parts.slice(i+2); + break; } } diff --git a/weblab/templates/entities/compare.html b/weblab/templates/entities/compare.html index 153db0e3f..8251a05b8 100644 --- a/weblab/templates/entities/compare.html +++ b/weblab/templates/entities/compare.html @@ -11,8 +11,8 @@

Comparison of {{ type }}s

loading...
diff --git a/weblab/templates/entities/entity_version.html b/weblab/templates/entities/entity_version.html index a8de7f0c0..1afbd9773 100644 --- a/weblab/templates/entities/entity_version.html +++ b/weblab/templates/entities/entity_version.html @@ -15,7 +15,7 @@
@@ -48,13 +48,13 @@

Visibility: {{ visibility }} help {% endif %} - {% if perms.experiments.create_experiment %} + {% if perms.experiments.create_experiment %}{% if type == 'model' or type == 'protocol' %} Run experiments: Run experiments: - {% endif %} + {% endif %}{% endif %}

@@ -62,9 +62,11 @@

{% can_create_version entity as permission %} {% if permission %} - delete this file + delete this file {% endif %}