Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Archive Item Titles #3345

Merged
merged 31 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9222965
Copy display set naming changes, adapted for archive items
chrisvanrun May 6, 2024
b286939
Add test for copy to reader study
chrisvanrun May 6, 2024
a60547d
Add test that checks if title changes to not fire new jobs
chrisvanrun May 6, 2024
af12a72
Add test for archive item title integrity
chrisvanrun May 6, 2024
c086560
Fix archive item create view test
chrisvanrun May 7, 2024
a1a9200
Abstract the updating and creating of CIVSets
chrisvanrun May 7, 2024
d69b43e
Merge remote-tracking branch 'origin' into archive-item-names
chrisvanrun May 7, 2024
507da52
Revert to sorting by Order
chrisvanrun May 10, 2024
74c9776
Refactor Update/Create forms display set and archive items
chrisvanrun May 10, 2024
42b7435
Pull out some logic from the unique title mixins
chrisvanrun May 13, 2024
882b6b1
Replace transaction db marked tests by local atomic contextmanagers
chrisvanrun May 13, 2024
6ec009e
Add test for form mixin
chrisvanrun May 13, 2024
570672a
Remove is_editable logic from archive items
chrisvanrun May 13, 2024
6cbe251
Change and refactor the civ_set remove confirmations
chrisvanrun May 13, 2024
b888ea2
Ensure order and title are always first in CIVSet forms renders
chrisvanrun May 13, 2024
88b08a3
Remove full object print in breadcrumb
chrisvanrun May 13, 2024
26fbfb3
Fix archive item test
chrisvanrun May 13, 2024
24fbe4b
Also add warning about forever deletion to list view
chrisvanrun May 13, 2024
1a87ca8
Be more explicit about field ordering
chrisvanrun May 14, 2024
651fbf2
Refactor update and create mixins
chrisvanrun May 14, 2024
f31e436
Remove overly eager Meta additions
chrisvanrun May 14, 2024
4ac8520
Push civ data processing into the form
chrisvanrun May 14, 2024
0e146cf
Fix unique title mixins name and test
chrisvanrun May 14, 2024
2b011fa
Cleanup old unused function
chrisvanrun May 24, 2024
90ba723
Rename CIVSetContainerMixin
chrisvanrun May 24, 2024
24b6c1f
Remove useless function
chrisvanrun May 24, 2024
e981e06
Remove useless function
chrisvanrun May 24, 2024
9c3c737
Remove extra baseclass of MultipleCIVForm
chrisvanrun May 24, 2024
3419766
Remove 'View' suffix to better match current code style
chrisvanrun May 28, 2024
e5faecc
Remove transactional contexts for when testing display set title mani…
chrisvanrun May 28, 2024
d05dc88
Remove transactional contexts for when testing archive item title man…
chrisvanrun May 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions app/grandchallenge/archives/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@
ArchivePermissionRequest,
)
from grandchallenge.cases.forms import UploadRawImagesForm
from grandchallenge.components.forms import (
CIVSetCreateFormMixin,
CIVSetUpdateFormMixin,
MultipleCIVForm,
)
from grandchallenge.components.models import ComponentInterface, InterfaceKind
from grandchallenge.core.forms import (
PermissionRequestUpdateForm,
SaveFormInitMixin,
UniqueTitleCreateFormMixin,
UniqueTitleUpdateFormMixin,
WorkstationUserFilterMixin,
)
from grandchallenge.core.guardian import get_objects_for_user
Expand Down Expand Up @@ -225,3 +232,37 @@ def save(self, *args, **kwargs):
{"interface_pk": self.cleaned_data["interface"].pk}
)
return super().save(*args, **kwargs)


class ArchiveItemFormMixin:
class Meta:
non_interface_fields = ("title",)

@property
def model(self):
return self.base_obj.civ_set_model

def unique_title_query(self, *args, **kwargs):
return (
super()
.unique_title_query(*args, **kwargs)
.filter(archive=self.base_obj)
)


class ArchiveItemCreateForm(
ArchiveItemFormMixin,
UniqueTitleCreateFormMixin,
CIVSetCreateFormMixin,
MultipleCIVForm,
):
pass


class ArchiveItemUpdateForm(
ArchiveItemFormMixin,
UniqueTitleUpdateFormMixin,
CIVSetUpdateFormMixin,
MultipleCIVForm,
):
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Generated by Django 4.2.11 on 2024-05-06 13:29

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("archives", "0017_archive_item_delete_permission_granting"),
]

operations = [
migrations.AddField(
model_name="archiveitem",
name="title",
field=models.CharField(blank=True, default="", max_length=255),
),
migrations.AddConstraint(
model_name="archiveitem",
constraint=models.UniqueConstraint(
condition=models.Q(("title", ""), _negated=True),
fields=("title", "archive"),
name="unique_archive_item_title",
),
),
]
35 changes: 18 additions & 17 deletions app/grandchallenge/archives/models.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import os

from actstream.models import Follow
from django.conf import settings
from django.contrib.auth.models import Group
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.db.models import Q
from django_extensions.db.models import TitleSlugDescriptionModel
from guardian.models import GroupObjectPermissionBase, UserObjectPermissionBase
from guardian.shortcuts import assign_perm, remove_perm
Expand All @@ -14,6 +13,7 @@
from grandchallenge.anatomy.models import BodyStructure
from grandchallenge.components.models import (
CIVForObjectMixin,
CIVSetStringRepresentationMixin,
ComponentInterfaceValue,
ValuesForInterfacesMixin,
)
Expand Down Expand Up @@ -266,6 +266,9 @@ def civ_sets_related_manager(self):
def civ_set_model(self):
return ArchiveItem

def create_civ_set(self, data):
return self.civ_set_model.objects.create(archive=self, **data)

@property
def create_civ_set_url(self):
return reverse("archives:item-create", kwargs={"slug": self.slug})
Expand All @@ -283,22 +286,16 @@ class ArchiveGroupObjectPermission(GroupObjectPermissionBase):
content_object = models.ForeignKey(Archive, on_delete=models.CASCADE)


class ArchiveItem(CIVForObjectMixin, UUIDModel):
class ArchiveItem(
CIVSetStringRepresentationMixin, CIVForObjectMixin, UUIDModel
):
archive = models.ForeignKey(
Archive, related_name="items", on_delete=models.PROTECT
)
values = models.ManyToManyField(
ComponentInterfaceValue, blank=True, related_name="archive_items"
)

def __str__(self):
values = []
for value in self.values.all():
if value.image:
values.append(value.image.name)
if value.file:
values.append(os.path.basename(value.file.file.name))
return ", ".join(values)
title = models.CharField(max_length=255, default="", blank=True)
chrisvanrun marked this conversation as resolved.
Show resolved Hide resolved

def save(self, *args, **kwargs):
adding = self._state.adding
Expand All @@ -308,6 +305,15 @@ def save(self, *args, **kwargs):
if adding:
self.assign_permissions()

class Meta:
constraints = [
models.UniqueConstraint(
fields=["title", "archive"],
name="unique_archive_item_title",
condition=~Q(title=""),
)
]

def assign_permissions(self):
# Archive editors, uploaders and users can view this archive item
assign_perm(
Expand Down Expand Up @@ -339,11 +345,6 @@ def assign_permissions(self):
def base_object(self):
return self.archive

@property
def is_editable(self):
# always editable
return True

@property
def update_url(self):
return reverse(
Expand Down
1 change: 1 addition & 0 deletions app/grandchallenge/archives/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class Meta:
model = ArchiveItem
fields = (
"pk",
"title",
"archive",
"values",
"hanging_protocol",
Expand Down
8 changes: 4 additions & 4 deletions app/grandchallenge/archives/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
ArchiveCreate,
ArchiveDetail,
ArchiveEditorsUpdate,
ArchiveItemBulkDeleteView,
ArchiveItemBulkDelete,
ArchiveItemCreateView,
ArchiveItemDeleteView,
ArchiveItemDelete,
ArchiveItemInterfaceCreate,
ArchiveItemsList,
ArchiveItemsToReaderStudyUpdate,
Expand Down Expand Up @@ -66,7 +66,7 @@
),
path(
"<slug>/items/delete/",
ArchiveItemBulkDeleteView.as_view(),
ArchiveItemBulkDelete.as_view(),
name="items-bulk-delete",
),
path(
Expand All @@ -76,7 +76,7 @@
),
path(
"<slug>/items/<uuid:pk>/delete/",
ArchiveItemDeleteView.as_view(),
ArchiveItemDelete.as_view(),
name="item-delete",
),
path(
Expand Down
55 changes: 27 additions & 28 deletions app/grandchallenge/archives/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
from grandchallenge.archives.forms import (
AddCasesForm,
ArchiveForm,
ArchiveItemCreateForm,
ArchiveItemsToReaderStudyForm,
ArchiveItemUpdateForm,
ArchivePermissionRequestUpdateForm,
UploadersForm,
UsersForm,
Expand All @@ -49,8 +51,8 @@
from grandchallenge.cases.models import Image, RawImageUploadSession
from grandchallenge.components.forms import MultipleCIVForm, NewFileUploadForm
from grandchallenge.components.views import (
CIVSetBulkDeleteView,
CIVSetDeleteView,
CIVSetBulkDelete,
CIVSetDelete,
CIVSetFormMixin,
CivSetListView,
InterfacesCreateBaseView,
Expand Down Expand Up @@ -364,8 +366,11 @@ def get_context_data(self, **kwargs):
return context


class ArchiveItemUpdate(CIVSetFormMixin, MultipleCIVProcessingBaseView):
form_class = MultipleCIVForm
class ArchiveItemUpdate(
CIVSetFormMixin,
MultipleCIVProcessingBaseView,
):
form_class = ArchiveItemUpdateForm
permission_required = (
f"{ArchiveItem._meta.app_label}.change_{ArchiveItem._meta.model_name}"
)
Expand Down Expand Up @@ -410,15 +415,6 @@ def new_interface_url(self):
kwargs={"slug": self.base_object.slug, "pk": self.object.pk},
)

def process_data_for_object(self, data):
"""Updates the archive item"""
instance = self.object
for ci_slug, new_value in data.items():
instance.create_civ(
ci_slug=ci_slug, new_value=new_value, user=self.request.user
)
return instance


class ArchiveItemsList(CivSetListView):
model = ArchiveItem
Expand All @@ -427,10 +423,19 @@ class ArchiveItemsList(CivSetListView):
)
columns = [
Column(title=""),
Column(title="ArchiveItem ID", sort_field="pk"),
Column(title="ID", sort_field="pk"),
Column(
title="Title",
sort_field="title",
optional_condition=lambda obj: bool(obj.title),
),
*CivSetListView.columns,
]
default_sort_column = 1
default_sort_column = 2
search_fields = [
"title",
*CivSetListView.search_fields,
]

@cached_property
def base_object(self):
Expand Down Expand Up @@ -479,7 +484,10 @@ def form_valid(self, form):

for item in items:
values = item.values.all()
ds = DisplaySet.objects.create(reader_study=reader_study)
ds = DisplaySet.objects.create(
reader_study=reader_study,
title=item.title,
)
ds.values.set(values)

self.success_url = reader_study.get_absolute_url()
Expand Down Expand Up @@ -554,7 +562,7 @@ class ArchiveItemCreateView(
CIVSetFormMixin,
MultipleCIVProcessingBaseView,
):
form_class = MultipleCIVForm
form_class = ArchiveItemCreateForm
permission_required = (
f"{Archive._meta.app_label}.change_{Archive._meta.model_name}"
)
Expand Down Expand Up @@ -590,15 +598,6 @@ def new_interface_url(self):
kwargs={"slug": self.base_object.slug},
)

def process_data_for_object(self, data):
"""Creates an archive item"""
instance = ArchiveItem.objects.create(archive=self.base_object)
for slug in data:
instance.create_civ(
ci_slug=slug, new_value=data[slug], user=self.request.user
)
return instance

def get_success_url(self):
return self.return_url

Expand Down Expand Up @@ -641,14 +640,14 @@ def get_htmx_url(self):
)


class ArchiveItemDeleteView(CIVSetDeleteView):
class ArchiveItemDelete(CIVSetDelete):
model = ArchiveItem
permission_required = (
f"{Archive._meta.app_label}.delete_{ArchiveItem._meta.model_name}"
)


class ArchiveItemBulkDeleteView(CIVSetBulkDeleteView):
class ArchiveItemBulkDelete(CIVSetBulkDelete):
model = ArchiveItem

@property
Expand Down
42 changes: 42 additions & 0 deletions app/grandchallenge/components/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
ModelForm,
ModelMultipleChoiceField,
)
from django.utils.functional import empty

from grandchallenge.algorithms.models import AlgorithmImage
from grandchallenge.components.form_fields import InterfaceFormField
Expand Down Expand Up @@ -106,6 +107,9 @@ class MultipleCIVForm(Form):
SelectUploadWidget,
}

class Meta:
non_interface_fields = tuple()

def __init__(self, *args, instance, base_obj, user, **kwargs):
super().__init__(*args, **kwargs)
self.instance = instance
Expand Down Expand Up @@ -249,6 +253,44 @@ def _get_default_field(self, *, interface, current_value):
user=self.user,
).field

def process_object_data(self):
for key, value in self.cleaned_data.items():
if key not in self.Meta.non_interface_fields:
self.instance.create_civ(
ci_slug=key,
new_value=value,
user=self.user,
)


class CIVSetCreateFormMixin:
instance = None

def process_object_data(self):
non_civ_data = {
k: v
for k, v in self.cleaned_data.items()
if k in self.Meta.non_interface_fields
}
self.instance = self.base_obj.create_civ_set(data=non_civ_data)
super().process_object_data()


class CIVSetUpdateFormMixin:
def process_object_data(self):
instance = self.instance

save = False
for key in self.Meta.non_interface_fields:
value = self.cleaned_data.get(key, empty)
if value is not empty and value != getattr(instance, key):
setattr(instance, key, value)
save = True
if save:
instance.save()

super().process_object_data()


class SingleCIVForm(Form):
_possible_widgets = {
Expand Down
Loading