Add TODOs for upgrading to Django 6

[danlamanna]

Summary by CodeRabbit

• Documentation
  • Added upgrade notes identifying areas requiring review for future Django 6 compatibility.
  • Documented current validation behavior and migration considerations for accession and ZIP upload workflows.
  • Clarified that existing behavior remains unchanged in this release.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added Django 6 migration TODO comments to accession creation, extraction, constraint validation, and related tests. Runtime behavior remains unchanged.

Changes

Django 6 migration TODOs

Layer / File(s)	Summary
Accession validation and migration notes isic/core/management/commands/add_sample_data.py, isic/ingest/models/accession.py, isic/ingest/models/zip_upload.py, isic/ingest/services/accession/__init__.py, isic/ingest/tests/test_accession.py	The changes document Django 6 compatibility work, the current validate_constraints=False workaround, and related test locations. No runtime behavior changes.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: brianhelba

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the added Django 6 upgrade TODO comments, which are the main changes in the pull request.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch add-todos

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

isic/ingest/models/accession.py (1)

362-366: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Make the migration TODO explicit about the validation it restores.

validate_constraints=False skips Model.validate_constraints(), which validates every constraint in Meta.constraints, not only these two ExclusionConstraints. Update the TODO to remove this flag from all affected accession call sites after the Django 6 upgrade, and add a regression test for the constraints. Django 6.0 documents this parameter behavior, and PR #19535 fixes the ForeignKey attname failure. (docs.djangoproject.com)

Proposed comment update

-            # TODO(django 6): these two conditions reference foreign keys by attname, which
-            # django can't resolve during python-side constraint validation. that makes
-            # full_clean(validate_constraints=True) raise FieldError on every accession, so
-            # callers pass validate_constraints=False. fixed by
-            # https://github.com/django/django/pull/19535, released in django 6.
+            # TODO(django 6): remove validate_constraints=False from all accession
+            # full_clean() call sites after upgrading. Django 5.2 raises FieldError
+            # while validating these two ExclusionConstraints because they use
+            # foreign-key attnames. The flag also skips validation of every
+            # Meta.constraints entry. Fixed by
+            # https://github.com/django/django/pull/19535.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@isic/ingest/models/accession.py` around lines 362 - 366, Update the TODO near
the accession foreign-key constraint conditions to explicitly state that, after
upgrading to Django 6, all affected accession call sites must remove
validate_constraints=False so Model.validate_constraints() runs for every
Meta.constraints entry, and add a regression test covering the constraints.

Source: MCP tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@isic/ingest/models/accession.py`:
- Around line 362-366: Update the TODO near the accession foreign-key constraint
conditions to explicitly state that, after upgrading to Django 6, all affected
accession call sites must remove validate_constraints=False so
Model.validate_constraints() runs for every Meta.constraints entry, and add a
regression test covering the constraints.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 79ef8771-f0a3-4e53-81b6-f870baa31098

📥 Commits

Reviewing files that changed from the base of the PR and between bc11677 and bd32c51.

📒 Files selected for processing (5)

• isic/core/management/commands/add_sample_data.py
• isic/ingest/models/accession.py
• isic/ingest/models/zip_upload.py
• isic/ingest/services/accession/__init__.py
• isic/ingest/tests/test_accession.py