diff --git a/isic/conftest.py b/isic/conftest.py index 30424e4c3..065ce95e6 100644 --- a/isic/conftest.py +++ b/isic/conftest.py @@ -15,7 +15,7 @@ get_elasticsearch_client, maybe_create_index, ) -from isic.core.tests.factories import CollectionFactory, DoiFactory, ImageFactory, IsicIdFactory +from isic.core.tests.factories import CollectionFactory, ImageFactory, IsicIdFactory from isic.ingest.tests.factories import ( AccessionFactory, AccessionReviewFactory, @@ -108,4 +108,3 @@ def s3ff_random_field_value(s3ff_field_value_factory): register(IsicIdFactory) register(ImageFactory) register(CollectionFactory) -register(DoiFactory) diff --git a/isic/core/admin.py b/isic/core/admin.py index ace10bafa..25c277702 100644 --- a/isic/core/admin.py +++ b/isic/core/admin.py @@ -215,7 +215,7 @@ class SupplementalFileInline(admin.TabularInline): @admin.register(Doi) class DoiAdmin(StaffReadonlyAdmin): list_select_related = ["collection"] - list_display = ["id", "url", "collection", "bundle", "num_supplemental_files"] + list_display = ["id", "external_url", "collection", "bundle", "num_supplemental_files"] inlines = [SupplementalFileInline] autocomplete_fields = ["creator"] diff --git a/isic/core/api/collection.py b/isic/core/api/collection.py index 9f60ac25b..d5716194e 100644 --- a/isic/core/api/collection.py +++ b/isic/core/api/collection.py @@ -45,9 +45,10 @@ def query_min_length(cls, v: str): class CollectionOut(ModelSchema): class Meta: model = Collection - fields = ["id", "name", "description", "public", "pinned", "locked", "doi"] + fields = ["id", "name", "description", "public", "pinned", "locked"] - doi_url: str | None = Field(alias="doi_url") + doi: str | None = Field(None, alias="doi.id") + doi_url: str | None = Field(None, alias="doi.external_url") @router.get( diff --git a/isic/core/api/doi.py b/isic/core/api/doi.py index a59d8e4ed..dedd2aee8 100644 --- a/isic/core/api/doi.py +++ b/isic/core/api/doi.py @@ -41,7 +41,9 @@ def parse_s3_file_field_values(cls, v): # noqa: N805 include_in_schema=False, ) def create_doi(request, payload: CreateDOIIn): - collection = get_object_or_404(Collection, id=payload.collection_id) + collection = get_object_or_404( + Collection.objects.select_related("doi"), pk=payload.collection_id + ) if not request.user.is_staff: return 403, {"error": "You do not have permission to create a DOI."} diff --git a/isic/core/migrations/0026_invert_doi_fk.py b/isic/core/migrations/0026_invert_doi_fk.py new file mode 100644 index 000000000..c373653ac --- /dev/null +++ b/isic/core/migrations/0026_invert_doi_fk.py @@ -0,0 +1,76 @@ +import django.core.validators +from django.db import migrations, models +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps +from django.db.models import OuterRef, Subquery +import django.db.models.deletion + +import isic.core.models.doi + + +def invert_doi_fk(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor): + Doi = apps.get_model("core", "Doi") + Collection = apps.get_model("core", "Collection") + + Doi.objects.update( + collection=Subquery(Collection.objects.filter(doi=OuterRef("pk")).values("pk")[:1]) + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0025_alter_collection_shares_alter_image_shares"), + ] + + operations = [ + migrations.AlterField( + model_name="collection", + name="doi", + field=models.OneToOneField( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="collection_reverse", + to="core.doi", + ), + ), + migrations.AddField( + model_name="doi", + name="collection", + field=models.OneToOneField( + default=None, + null=True, + on_delete=django.db.models.deletion.PROTECT, + to="core.collection", + ), + preserve_default=False, + ), + migrations.RunPython(invert_doi_fk, elidable=True), + migrations.RemoveField( + model_name="collection", + name="doi", + ), + migrations.AlterField( + model_name="doi", + name="collection", + field=models.OneToOneField( + on_delete=django.db.models.deletion.PROTECT, + to="core.collection", + ), + ), + migrations.AlterField( + model_name="doi", + name="id", + field=models.CharField( + default=isic.core.models.doi._generate_random_doi_id, # noqa: SLF001 + max_length=30, + primary_key=True, + serialize=False, + validators=[django.core.validators.RegexValidator("^\\d+\\.\\d+/\\d+$")], + ), + ), + migrations.RemoveField( + model_name="doi", + name="url", + ), + ] diff --git a/isic/core/models/collection.py b/isic/core/models/collection.py index f46fbaac8..6839969fa 100644 --- a/isic/core/models/collection.py +++ b/isic/core/models/collection.py @@ -10,7 +10,6 @@ from django.urls import reverse from django_extensions.db.models import TimeStampedModel -from .doi import Doi from .image import Image @@ -83,8 +82,6 @@ class Meta(TimeStampedModel.Meta): pinned = models.BooleanField(default=False) - doi = models.OneToOneField(Doi, on_delete=models.PROTECT, null=True, blank=True) - locked = models.BooleanField(default=False) objects = CollectionQuerySet.as_manager() @@ -100,15 +97,6 @@ def is_magic(self) -> bool: """Magic collections are collections pointed to by a cohort.""" return hasattr(self, "cohort") - @property - def has_doi(self) -> bool: - return self.doi is not None - - @property - def doi_url(self): - if self.doi: - return f"https://doi.org/{self.doi}" - @property def num_lesions(self): return ( diff --git a/isic/core/models/doi.py b/isic/core/models/doi.py index 09b0f4927..6cb431e3d 100644 --- a/isic/core/models/doi.py +++ b/isic/core/models/doi.py @@ -1,5 +1,7 @@ import json +import random +from django.conf import settings from django.contrib.auth.models import User from django.core.files.storage import storages from django.core.validators import RegexValidator @@ -7,6 +9,8 @@ from django.urls import reverse from django_extensions.db.models import TimeStampedModel +from .collection import Collection + def doi_upload_to(instance: "Doi", filename: str) -> str: return f"dois/{instance.id.replace('/', '-')}/{filename}" @@ -16,19 +20,26 @@ def doi_storage(): return storages["sponsored"] +def _generate_random_doi_id(): + # pad DOI with leading zeros so all DOIs are prefix/6 digits + return f"{settings.ISIC_DATACITE_DOI_PREFIX}/{random.randint(10_000, 999_999):06}" # noqa: S311 + + class Doi(TimeStampedModel): class Meta: verbose_name = "DOI" verbose_name_plural = "DOIs" id = models.CharField( - max_length=30, primary_key=True, validators=[RegexValidator(r"^\d+\.\d+/\d+$")] + max_length=30, + primary_key=True, + default=_generate_random_doi_id, + validators=[RegexValidator(r"^\d+\.\d+/\d+$")], ) slug = models.SlugField(max_length=150, unique=True) + collection = models.OneToOneField(Collection, on_delete=models.PROTECT) creator = models.ForeignKey(User, on_delete=models.RESTRICT) - url = models.CharField(max_length=200) - bundle = models.FileField(upload_to=doi_upload_to, storage=doi_storage, null=True, blank=True) bundle_size = models.PositiveBigIntegerField(null=True, blank=True) @@ -38,10 +49,14 @@ class Meta: citations = models.JSONField(default=dict, blank=True) schema_org_dataset = models.JSONField(default=dict, blank=True) - def __str__(self): + def __str__(self) -> str: return self.id - def get_absolute_url(self): + @property + def external_url(self) -> str: + return f"https://doi.org/{self.id}" + + def get_absolute_url(self) -> str: return reverse("core/doi-detail", kwargs={"slug": self.slug}) def get_schema_org_dataset_json(self): diff --git a/isic/core/services/collection/__init__.py b/isic/core/services/collection/__init__.py index 58e296099..9500f529f 100644 --- a/isic/core/services/collection/__init__.py +++ b/isic/core/services/collection/__init__.py @@ -58,7 +58,7 @@ def collection_delete(*, collection: Collection, ignore_lock: bool = False) -> N if collection.studies.exists(): raise ValidationError("Collections with derived studies cannot be deleted.") - if collection.has_doi: + if hasattr(collection, "doi"): raise ValidationError("Collections with DOIs cannot be deleted.") collection.delete() @@ -131,7 +131,7 @@ def collection_merge_magic_collections( if hasattr(src_collection, "cohort") and src_collection.cohort != dest_collection.cohort: logger.info("Abandoning cohort %s", src_collection.cohort.pk) - for field in ["public", "pinned", "doi", "locked"]: + for field in ["public", "pinned", "locked"]: dest_collection_value = getattr(dest_collection, field) collection_value = getattr(src_collection, field) if dest_collection_value != collection_value: diff --git a/isic/core/services/collection/doi.py b/isic/core/services/collection/doi.py index 2836a1592..74e68b9e8 100644 --- a/isic/core/services/collection/doi.py +++ b/isic/core/services/collection/doi.py @@ -1,6 +1,5 @@ import logging from pathlib import Path -import random from typing import TYPE_CHECKING, Any from urllib import parse @@ -19,7 +18,6 @@ from isic.core.services.collection import ( collection_get_creators_in_attribution_order, collection_lock, - collection_update, ) from isic.core.services.snapshot import snapshot_images from isic.core.tasks import ( @@ -101,17 +99,12 @@ def collection_build_draft_doi(*, doi_id: str) -> dict: } -def collection_generate_random_doi_id(): - # pad DOI with leading zeros so all DOIs are prefix/6 digits - return f"{settings.ISIC_DATACITE_DOI_PREFIX}/{random.randint(10_000, 999_999):06}" # noqa: S311 - - def collection_check_create_doi_allowed( *, user: User, collection: Collection, supplemental_files=None ) -> None: if not user.has_perm("core.create_doi", collection): raise ValidationError("You don't have permissions to do that.") - if collection.doi: + if hasattr(collection, "doi"): raise ValidationError("This collection already has a DOI.") if not collection.public: raise ValidationError("A collection must be public to issue a DOI.") @@ -175,21 +168,13 @@ def collection_create_doi(*, user: User, collection: Collection, supplemental_fi user=user, collection=collection, supplemental_files=supplemental_files ) - doi_id = collection_generate_random_doi_id() - draft_doi_dict = collection_build_draft_doi(doi_id=doi_id) - doi_dict = collection_build_doi(collection=collection, doi_id=doi_id) - with transaction.atomic(): # First, create the local DOI record to validate uniqueness within our known set - doi = Doi( - id=doi_id, slug=slugify(collection.name), creator=user, url=f"https://doi.org/{doi_id}" - ) + doi = Doi(slug=slugify(collection.name), collection=collection, creator=user) doi.full_clean() doi.save() - # Lock the collection, set the DOI on it collection_lock(collection=collection) - collection_update(collection=collection, doi=doi, ignore_lock=True) if supplemental_files: for supplemental_file in supplemental_files: @@ -200,17 +185,20 @@ def collection_create_doi(*, user: User, collection: Collection, supplemental_fi size=supplemental_file["blob"].size, ) + draft_doi_dict = collection_build_draft_doi(doi_id=doi.id) + doi_dict = collection_build_doi(collection=collection, doi_id=doi.id) + # Reserve the DOI using the draft mechanism. # If it fails, transaction will rollback, nothing in our database will change. _datacite_create_doi(draft_doi_dict) # Convert to a published DOI. If this fails, someone will have to come along later and # retry to publish it. (May want a django-admin action for this if it ever happens.) - _datacite_update_doi(doi_dict, doi_id) + _datacite_update_doi(doi_dict, doi.id) - create_doi_bundle_task.delay_on_commit(doi_id) - fetch_doi_citations_task.delay_on_commit(doi_id) - fetch_doi_schema_org_dataset_task.delay_on_commit(doi_id) + create_doi_bundle_task.delay_on_commit(doi.id) + fetch_doi_citations_task.delay_on_commit(doi.id) + fetch_doi_schema_org_dataset_task.delay_on_commit(doi.id) logger.info("User %d created DOI %s for collection %d", user.id, doi.id, collection.id) diff --git a/isic/core/static/core/js/alpine/accessionsStore.js b/isic/core/static/core/js/alpine/accessionsStore.js new file mode 100644 index 000000000..cf7289180 --- /dev/null +++ b/isic/core/static/core/js/alpine/accessionsStore.js @@ -0,0 +1,9 @@ +export default () => ({ + items: {}, + setReview(id, value) { + this.items[id] = value; + }, + addItem(id) { + this.items[id] = null; + }, +}); diff --git a/isic/core/static/core/js/alpine/data/collectionAttributionInformation.js b/isic/core/static/core/js/alpine/data/collectionAttributionInformation.js new file mode 100644 index 000000000..288b0af72 --- /dev/null +++ b/isic/core/static/core/js/alpine/data/collectionAttributionInformation.js @@ -0,0 +1,15 @@ +import axiosSession from '../../axios.js'; + +export default (collectionId) => ({ + attributions: [], + fetched: false, + loading: false, + async fetchMetaInformation() { + this.loading = true; + const resp = await axiosSession.get(`/api/v2/collections/${collectionId}/attribution/`); + + this.attributions = resp.data; + this.loading = false; + this.fetched = true; + } +}); diff --git a/isic/core/static/core/js/alpine/data/collectionDetail.js b/isic/core/static/core/js/alpine/data/collectionDetail.js new file mode 100644 index 000000000..48a5cbcda --- /dev/null +++ b/isic/core/static/core/js/alpine/data/collectionDetail.js @@ -0,0 +1,69 @@ +import 'jquery'; // Defines "jQuery" and "$" + +import axiosSession from '../../axios.js'; + +function formatUser(user) { + if(user.loading) { + return user.text; + } + + const $container = $( + "
" + + "
" + user.email + "
" + + "
" + user.first_name + " " + user.last_name + "
" + + "
" + ); + + return $container; +} + +export default (collectionId) => ({ + init() { + $("#user-selection").select2({ + ajax: { + url: '{% url "api:user_autocomplete" %}', + data: function (params) { + // remap the "term" parameter from select2 to "query" so it's + // consistent with other autocomplete endpoints. + return { + query: params.term + } + }, + processResults: function (data) { + return { + results: data + }; + }, + delay: 50, + }, + placeholder: 'Search for a user by email', + minimumInputLength: 3, + templateResult: formatUser, + templateSelection: function (user) { + return user.email || user.text; + } + }); + }, + modalOpen: false, + errorMessage: '', + async shareCollectionWithUsers() { + if ($('#user-selection').val().length === 0) { + this.errorMessage = 'Please select at least one user to share the collection with.'; + return; + } + + if (confirm('Are you sure you want to grant additional access to this collection?')) { + try { + const resp = await axiosSession.post(`/api/v2/collections/${collectionId}/share/`, { + user_ids: $('#user-selection').val().map(function (n) { + return parseInt(n) + }) + }); + } catch (error) { + this.errorMessage = error.response.data[0]; + return; + } + window.location.reload(); + } + }, +}); diff --git a/isic/core/static/core/js/alpine/data/collectionEditor.js b/isic/core/static/core/js/alpine/data/collectionEditor.js new file mode 100644 index 000000000..c42a1b1ef --- /dev/null +++ b/isic/core/static/core/js/alpine/data/collectionEditor.js @@ -0,0 +1,48 @@ +import axiosSession from '../../axios.js'; + +export default (collectionId) => ({ + init() { + const existingStorage = localStorage.getItem(this.storageKey); + if (existingStorage !== null) { + this.images = new Set(JSON.parse(existingStorage)); + } + }, + storageKey: `images_to_remove_collection_${collectionId}`, + images: new Set(), + _imagesToArray() { + return Array.from(this.images.values()) + }, + _persist() { + localStorage.setItem(this.storageKey, JSON.stringify(this._imagesToArray())); + }, + toggleImage(imageId) { + if (this.images.has(imageId)) { + this.images.delete(imageId); + } else { + this.images.add(imageId); + } + + this._persist(); + }, + removeImage(imageId) { // TODO: unused?? + this._persist(); + }, + async deleteImages() { + if (confirm(`Are you sure you want to remove ${this.images.size} images from this collection?`)) { + try { + const resp = await axiosSession.post(`/api/v2/collections/${collectionId}/remove-from-list/`, { + 'isic_ids': this._imagesToArray() + }); + } catch (error) { + alert('Something went wrong.'); + return; + } + this.resetImages(`/collections/${collectionId}/`); + } + }, + resetImages(url) { + this.images.clear(); + this._persist(); + window.location.reload(url); + } +}); diff --git a/isic/core/static/core/js/alpine/data/quickfind.js b/isic/core/static/core/js/alpine/data/quickfind.js new file mode 100644 index 000000000..991f6f70e --- /dev/null +++ b/isic/core/static/core/js/alpine/data/quickfind.js @@ -0,0 +1,25 @@ +const controller = new AbortController(); + +export default () => ({ + quickfindOpen: false, + findText: '', + results: {}, + openQuickfindModal() { + this.$nextTick(() => this.$refs.quickfind.focus()); + this.quickfindOpen = true; + }, + closeQuickfindModal() { + this.quickfindOpen = false; + }, + async performFind() { + if (this.findText.length < 3) { + this.results = {}; + return; + } + + const { data } = await axiosSession.get(`/api/v2/quickfind/?query=${this.findText}`, { + signal: controller.signal, + }); + this.results = data; + }, +}); diff --git a/isic/core/static/core/js/alpine/data/thumbnailGrid.js b/isic/core/static/core/js/alpine/data/thumbnailGrid.js new file mode 100644 index 000000000..eeb1905ae --- /dev/null +++ b/isic/core/static/core/js/alpine/data/thumbnailGrid.js @@ -0,0 +1,19 @@ +// allow between 4-8 columns +export default () => ({ + gridClassNames: { + 4: 'sm:grid-cols-4', + 5: 'sm:grid-cols-5', + 6: 'sm:grid-cols-6', + 7: 'sm:grid-cols-7', + 8: 'sm:grid-cols-8', + }, + numCols: parseInt(localStorage.getItem('numCols')) || 8, + increase() { + this.numCols = Math.min(8, this.numCols + 1); + localStorage.setItem('numCols', this.numCols); + }, + decrease() { + this.numCols = Math.max(4, this.numCols - 1); + localStorage.setItem('numCols', this.numCols); + } +}); diff --git a/isic/core/static/core/js/axios.js b/isic/core/static/core/js/axios.js new file mode 100644 index 000000000..bdaecb530 --- /dev/null +++ b/isic/core/static/core/js/axios.js @@ -0,0 +1,18 @@ +import axios from 'axios'; + +const axiosSession = axios.create({ + headers: { + 'Content-Type': 'application/json', + 'X-CSRFToken': '{{ csrf_token }}' + } +}); + +axiosSession.interceptors.request.use((config) => { + const token = window.cookieStore.get('csrftoken'); + if (token) { + config.headers['X-CSRFToken'] = token.value; + } + return config; +}); + +export default axiosSession; diff --git a/isic/core/static/core/js/base.js b/isic/core/static/core/js/base.js new file mode 100644 index 000000000..6cb28685d --- /dev/null +++ b/isic/core/static/core/js/base.js @@ -0,0 +1,19 @@ +import Alpine from 'alpinejs'; + +import accessions from './alpine/accessionsStore.js'; + +import collectionDetail from './alpine/data/collectionDetail.js'; +import collectionEditor from './alpine/data/collectionEditor.js'; +import thumbnailGrid from './alpine/data/thumbnailGrid.js'; +import quickfind from './alpine/data/quickfind.js'; + + +document.addEventListener('alpine:init', () => { + Alpine.store('accessions', accessions); + + Alpine.data('collectionDetail', collectionDetail); + Alpine.data('collectionEditor', collectionEditor); + Alpine.data('thumbnailGrid', thumbnailGrid); + Alpine.data('quickfind', quickfind); +}); +Alpine.start() diff --git a/isic/core/storages/static_files.py b/isic/core/storages/static_files.py new file mode 100644 index 000000000..b6955fa11 --- /dev/null +++ b/isic/core/storages/static_files.py @@ -0,0 +1,5 @@ +from whitenoise.storage import CompressedManifestStaticFilesStorage + + +class JsModuleCompressedManifestStaticFilesStorage(CompressedManifestStaticFilesStorage): + support_js_module_import_aggregation = True diff --git a/isic/core/templates/core/base.html b/isic/core/templates/core/base.html index 8f0a927d3..497e65976 100644 --- a/isic/core/templates/core/base.html +++ b/isic/core/templates/core/base.html @@ -17,9 +17,27 @@ {% if JS_SENTRY %} {% endif %} - - - + + + + + @@ -30,56 +48,6 @@ gtag('config', 'G-VBHRJSWF1T'); - - - - - {% endblock %} {% block content %} -
+
{{ collection.name }}
@@ -30,7 +27,7 @@
-
- - + +
{% endif %} -
+
{% for image in images %} @@ -169,56 +166,5 @@
{% include 'studies/partials/cursor_pagination.html' %} - - {% include 'ingest/partials/thumbnail_grid_js.html' %} - {% include 'core/partials/edit_collection_js.html' %} -
- - {% endblock %} diff --git a/isic/core/templates/core/image_browser.html b/isic/core/templates/core/image_browser.html index 176ba7b1c..b1ec16ad3 100644 --- a/isic/core/templates/core/image_browser.html +++ b/isic/core/templates/core/image_browser.html @@ -77,10 +77,10 @@ {% include 'studies/partials/cursor_pagination.html' %}
-
+
{% for image in images %} @@ -90,8 +90,6 @@
{% include 'studies/partials/cursor_pagination.html' %} - - {% include 'ingest/partials/thumbnail_grid_js.html' %}
{% include 'core/partials/image_browser_js.html' %} diff --git a/isic/core/templates/core/image_detail/images_tab.html b/isic/core/templates/core/image_detail/images_tab.html index 61e3f752e..fea48bb83 100644 --- a/isic/core/templates/core/image_detail/images_tab.html +++ b/isic/core/templates/core/image_detail/images_tab.html @@ -1,9 +1,9 @@
-
+
{% for image in images|slice:MAX_RELATED_SHOW_FIRST_N %} @@ -15,7 +15,5 @@
Showing first {{ MAX_RELATED_SHOW_FIRST_N }} images.
{% endif %}
- - {% include 'ingest/partials/thumbnail_grid_js.html' %}
diff --git a/isic/core/templates/core/partials/collection_share_modal.html b/isic/core/templates/core/partials/collection_share_modal.html index f86139322..c1f727ddc 100644 --- a/isic/core/templates/core/partials/collection_share_modal.html +++ b/isic/core/templates/core/partials/collection_share_modal.html @@ -12,51 +12,9 @@
-
- - diff --git a/isic/core/templates/core/partials/edit_collection_js.html b/isic/core/templates/core/partials/edit_collection_js.html deleted file mode 100644 index beb6f908d..000000000 --- a/isic/core/templates/core/partials/edit_collection_js.html +++ /dev/null @@ -1,55 +0,0 @@ - diff --git a/isic/core/tests/factories.py b/isic/core/tests/factories.py index 56da16050..7a385ac94 100644 --- a/isic/core/tests/factories.py +++ b/isic/core/tests/factories.py @@ -46,4 +46,5 @@ class Meta: model = Doi slug = factory.Faker("slug") + collection = factory.SubFactory(CollectionFactory) creator = factory.SubFactory(UserFactory) diff --git a/isic/core/tests/test_doi.py b/isic/core/tests/test_doi.py index e16a48b95..02ecb021a 100644 --- a/isic/core/tests/test_doi.py +++ b/isic/core/tests/test_doi.py @@ -12,6 +12,7 @@ collection_build_doi, collection_create_doi, ) +from isic.core.tests.factories import CollectionFactory, DoiFactory @pytest.fixture @@ -69,18 +70,20 @@ def test_collection_create_doi( @pytest.mark.django_db -def test_doi_form_requires_public_collection(private_collection, staff_user_request): - with pytest.raises(ValidationError): - collection_create_doi(user=staff_user_request.user, collection=private_collection) +def test_doi_form_requires_public_collection(staff_user_request): + collection = CollectionFactory.create(public=False) + + with pytest.raises(ValidationError, match="must be public"): + collection_create_doi(user=staff_user_request.user, collection=collection) @pytest.mark.django_db -def test_doi_form_requires_no_existing_doi(public_collection, staff_user_request): - public_collection.doi = Doi.objects.create(id="foo", creator=staff_user_request.user, url="foo") - public_collection.save() +def test_doi_form_requires_no_existing_doi(staff_user_request): + collection = CollectionFactory.create(public=True) + DoiFactory.create(collection=collection, creator=staff_user_request.user) - with pytest.raises(ValidationError): - collection_create_doi(user=staff_user_request.user, collection=public_collection) + with pytest.raises(ValidationError, match="already has a DOI"): + collection_create_doi(user=staff_user_request.user, collection=collection) @pytest.mark.django_db(transaction=True) diff --git a/isic/core/tests/test_sitemap.py b/isic/core/tests/test_sitemap.py index defb69bf7..0d4fe25d2 100644 --- a/isic/core/tests/test_sitemap.py +++ b/isic/core/tests/test_sitemap.py @@ -1,10 +1,13 @@ from django.urls import reverse import pytest +from isic.core.tests.factories import DoiFactory + @pytest.mark.django_db -def test_sitemap_dois(client, doi_factory): - doi = doi_factory() +def test_sitemap_dois(client): + doi = DoiFactory.create() + response = client.get(reverse("django.contrib.sitemaps.views.sitemap")) assert response.status_code == 200 assert doi.slug in response.content.decode("utf-8") diff --git a/isic/ingest/templates/ingest/cohort_detail.html b/isic/ingest/templates/ingest/cohort_detail.html index 2171b69f4..ae8192c85 100644 --- a/isic/ingest/templates/ingest/cohort_detail.html +++ b/isic/ingest/templates/ingest/cohort_detail.html @@ -19,10 +19,10 @@ {% include 'studies/partials/pagination.html' with page_obj=accessions %}
-
+
{% for accession in accessions %} @@ -32,7 +32,4 @@
{% include 'studies/partials/pagination.html' with page_obj=accessions %} - - {% include 'ingest/partials/thumbnail_grid_js.html' %} - {% endblock %} diff --git a/isic/ingest/templates/ingest/partials/thumbnail_grid_js.html b/isic/ingest/templates/ingest/partials/thumbnail_grid_js.html deleted file mode 100644 index cfc865f90..000000000 --- a/isic/ingest/templates/ingest/partials/thumbnail_grid_js.html +++ /dev/null @@ -1,23 +0,0 @@ - diff --git a/isic/ingest/templates/ingest/review_gallery.html b/isic/ingest/templates/ingest/review_gallery.html index e8db9f54e..7736a946b 100644 --- a/isic/ingest/templates/ingest/review_gallery.html +++ b/isic/ingest/templates/ingest/review_gallery.html @@ -19,14 +19,14 @@ {{ progress.num_reviewed|intcomma }} / {{ progress.num_reviewable|intcomma }} ({{ progress.percentage }}%) -
+
sorted by original filename
@@ -39,7 +39,5 @@ {% if page_obj %} {% include 'ingest/partials/review_footer.html' %} {% endif %} - - {% include 'ingest/partials/thumbnail_grid_js.html' %} {% endif %} {% endblock %} diff --git a/isic/ingest/templates/ingest/review_lesion_gallery.html b/isic/ingest/templates/ingest/review_lesion_gallery.html index fef067577..234e03eca 100644 --- a/isic/ingest/templates/ingest/review_lesion_gallery.html +++ b/isic/ingest/templates/ingest/review_lesion_gallery.html @@ -38,7 +38,5 @@ {% if page_obj %} {% include 'ingest/partials/review_footer.html' %} {% endif %} - - {% include 'ingest/partials/thumbnail_grid_js.html' %} {% endif %} {% endblock %} diff --git a/isic/ingest/tests/test_merge.py b/isic/ingest/tests/test_merge.py index 2ba157848..4f24aac7d 100644 --- a/isic/ingest/tests/test_merge.py +++ b/isic/ingest/tests/test_merge.py @@ -1,14 +1,13 @@ from django.core.exceptions import ValidationError from django.urls import reverse import pytest -from pytest_lazy_fixtures import lf from isic.core.models.base import CopyrightLicense from isic.core.models.collection import Collection -from isic.core.models.doi import Doi from isic.core.services.collection import collection_merge_magic_collections from isic.core.services.collection.image import collection_add_images -from isic.core.tests.factories import CollectionFactory +from isic.core.tests.factories import CollectionFactory, DoiFactory +from isic.factories import UserFactory from isic.ingest.models.cohort import Cohort from isic.ingest.services.cohort import cohort_merge from isic.ingest.services.contributor import contributor_merge @@ -154,22 +153,6 @@ def test_merge_cohorts_view(full_cohort, staff_client): assert r.url == reverse("cohort-detail", args=[cohort_a.pk]) -@pytest.fixture -def collection_with_doi(collection, user): - collection.doi = Doi.objects.create( - id="10.1000/xyz123", creator=user, url="https://doi.org/10.1000/xyz123" - ) - collection.save() - return collection - - -@pytest.fixture -def collection_with_shares(collection, user_factory): - user = user_factory() - collection.shares.add(user, through_defaults={"grantor": collection.creator}) - return collection - - @pytest.fixture def full_collection(collection_factory, image_factory, cohort_factory): def _full_collection(*, public: bool): @@ -200,17 +183,27 @@ def test_merge_collections(full_collection): @pytest.mark.django_db -@pytest.mark.parametrize( - ("unmergeable_collection", "error"), - [ - (lf("collection_with_doi"), "DOI"), - (lf("collection_with_shares"), "shares"), - ], -) -def test_merge_collections_unmergeable(collection, unmergeable_collection, error): - with pytest.raises(ValidationError, match=error): +def test_merge_collections_unmergeable_doi(): + src_collection = CollectionFactory.create() + DoiFactory.create(collection=src_collection) + dest_collection = CollectionFactory.create() + + with pytest.raises(ValidationError, match="DOI"): + collection_merge_magic_collections( + dest_collection=dest_collection, src_collection=src_collection + ) + + +@pytest.mark.django_db +def test_merge_collections_unmergeable_shares(): + src_collection = CollectionFactory.create() + user = UserFactory.create() + src_collection.shares.add(user, through_defaults={"grantor": src_collection.creator}) + dest_collection = CollectionFactory.create() + + with pytest.raises(ValidationError, match="shares"): collection_merge_magic_collections( - dest_collection=collection, src_collection=unmergeable_collection + dest_collection=dest_collection, src_collection=src_collection ) diff --git a/isic/settings/base.py b/isic/settings/base.py index 212568719..e05c6dcff 100644 --- a/isic/settings/base.py +++ b/isic/settings/base.py @@ -105,7 +105,7 @@ STORAGES = { # Inject the "default" storage in particular run configurations "staticfiles": { - "BACKEND": "whitenoise.storage.CompressedManifestStaticFilesStorage", + "BACKEND": "isic.core.storages.static_files.JsModuleCompressedManifestStaticFilesStorage", }, } diff --git a/isic/studies/templates/studies/study_detail.html b/isic/studies/templates/studies/study_detail.html index f1379e0da..b442f7671 100644 --- a/isic/studies/templates/studies/study_detail.html +++ b/isic/studies/templates/studies/study_detail.html @@ -83,11 +83,11 @@
{% include 'studies/partials/pagination.html' with page_obj=images %} -
+
{% for image in images %} @@ -96,7 +96,6 @@
{% include 'studies/partials/pagination.html' with page_obj=images %} - {% include 'ingest/partials/thumbnail_grid_js.html' %}
    {% for question in study.questions.all %}