Skip to content

Commit 13aa22d

Browse files
committed
Store default_attribution on publish requests
This was due to an inexplicable bug where an attribution wasn't copied to one of the images. This solves the potential race condition where the default attribution is changed while publishing.
1 parent c357b85 commit 13aa22d

File tree

7 files changed

+57
-2
lines changed

7 files changed

+57
-2
lines changed

isic/core/health.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,25 @@ def check_iptc_metadata_consistency() -> HealthCheckResult:
196196
)
197197

198198

199+
def check_published_images_have_attribution() -> HealthCheckResult:
200+
published_without_attribution = Image.objects.filter(
201+
accession__attribution="",
202+
).count()
203+
204+
passed = published_without_attribution == 0
205+
message = (
206+
"All published images have attribution"
207+
if passed
208+
else f"{published_without_attribution} published images missing attribution"
209+
)
210+
211+
return HealthCheckResult(
212+
name="published_images_have_attribution",
213+
passed=passed,
214+
message=message,
215+
)
216+
217+
199218
def check_embeddings_only_for_public_images() -> HealthCheckResult:
200219
embeddings_for_private_images = ImageEmbedding.objects.filter(image__public=False).count()
201220

@@ -224,6 +243,7 @@ def check_embeddings_only_for_public_images() -> HealthCheckResult:
224243
("every_user_has_profile", check_every_user_has_profile),
225244
("collection_image_consistency", check_collection_image_consistency),
226245
("iptc_metadata_consistency", check_iptc_metadata_consistency),
246+
("published_images_have_attribution", check_published_images_have_attribution),
227247
("embeddings_only_for_public_images", check_embeddings_only_for_public_images),
228248
]
229249

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# Generated by Django 5.2.3 on 2026-02-23 17:38
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
dependencies = [
8+
("ingest", "0036_add_anatom_site_hierarchical_fields"),
9+
]
10+
11+
operations = [
12+
migrations.AddField(
13+
model_name="publishrequest",
14+
name="default_attribution",
15+
field=models.CharField(default="", max_length=200),
16+
),
17+
]

isic/ingest/models/publish_request.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class PublishRequest(models.Model):
2525
# the additional collections to which the images will be added, including the magic collection
2626
collections = models.ManyToManyField("core.Collection")
2727
public = models.BooleanField(default=False)
28+
default_attribution = models.CharField(max_length=200, default="")
2829

2930
def __str__(self) -> str:
3031
return f"PublishRequest {self.pk}"

isic/ingest/services/publish/__init__.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ def cohort_publish_initialize(
5959
publish_request = PublishRequest.objects.create(
6060
creator=publisher,
6161
public=public,
62+
default_attribution=cohort.default_attribution,
6263
)
6364
publish_request.accessions.set(cohort.accessions.publishable())
6465
publish_request.collections.set(collections)
@@ -83,6 +84,7 @@ def cohort_publish(*, publish_request: PublishRequest) -> None:
8384
public=publish_request.public,
8485
publisher_pk=publish_request.creator.pk,
8586
additional_collection_ids=additional_collection_ids,
87+
default_attribution=publish_request.default_attribution,
8688
)
8789

8890

@@ -92,13 +94,18 @@ def accession_publish(
9294
public: bool,
9395
publisher: User,
9496
additional_collection_ids: Iterable[int] | None = None,
97+
default_attribution: str = "",
9598
):
9699
additional_collection_ids = additional_collection_ids or []
97100

98101
# wrapping this inside of a transaction ensures that this function can be retried easily
99102
with lock_table_for_writes(IsicId):
100103
if accession.attribution == "":
101-
accession.attribution = accession.cohort.default_attribution
104+
if not default_attribution:
105+
raise ValueError(
106+
"default_attribution must be provided when accession has no attribution"
107+
)
108+
accession.attribution = default_attribution
102109
accession.save(update_fields=["attribution"])
103110

104111
image_create(

isic/ingest/tasks.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ def publish_accession_task(
186186
public: bool,
187187
publisher_pk: int,
188188
additional_collection_ids: list[int] | None = None,
189+
default_attribution: str = "",
189190
):
190191
accession = Accession.objects.select_related("cohort").get(pk=accession_pk)
191192
publisher = User.objects.get(pk=publisher_pk)
@@ -194,4 +195,5 @@ def publish_accession_task(
194195
public=public,
195196
publisher=publisher,
196197
additional_collection_ids=additional_collection_ids,
198+
default_attribution=default_attribution,
197199
)

isic/ingest/tests/test_accession.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ def test_accession_reprocess(user, cohort_factory, django_capture_on_commit_call
6363

6464
# refresh the accession to get the blob that was generated by the celery task
6565
accession.refresh_from_db()
66+
accession.attribution = "test attribution"
67+
accession.save(update_fields=["attribution"])
6668

6769
with django_capture_on_commit_callbacks(execute=True):
6870
accession_publish(accession=accession, public=True, publisher=user)

isic/ingest/tests/test_publish.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,14 @@ def test_publish_copies_default_attribution(
8686
publishable_cohort_for_attributions.save(update_fields=["default_attribution"])
8787

8888
with django_capture_on_commit_callbacks(execute=True):
89-
cohort_publish_initialize(
89+
publish_request = cohort_publish_initialize(
9090
cohort=publishable_cohort_for_attributions,
9191
publisher=user,
9292
public=True,
9393
)
9494

95+
assert publish_request.default_attribution == "default attribution"
96+
9597
published_images = Image.objects.filter(accession__cohort=publishable_cohort_for_attributions)
9698

9799
assert published_images.count() == 2
@@ -161,6 +163,8 @@ def test_unembargo_images(user, cohort_factory, django_capture_on_commit_callbac
161163
original_blob_size=blob_path.stat().st_size,
162164
)
163165
accession.refresh_from_db()
166+
accession.attribution = "test attribution"
167+
accession.save(update_fields=["attribution"])
164168

165169
with django_capture_on_commit_callbacks(execute=True):
166170
accession_publish(accession=accession, public=True, publisher=user)
@@ -198,6 +202,8 @@ def test_iptc_metadata_embedding(
198202
original_blob_size=blob_path.stat().st_size,
199203
)
200204
accession.refresh_from_db()
205+
accession.attribution = "test attribution"
206+
accession.save(update_fields=["attribution"])
201207

202208
with django_capture_on_commit_callbacks(execute=True):
203209
accession_publish(accession=accession, public=False, publisher=user)

0 commit comments

Comments
 (0)