From 6e9dac499330005b53fb6ebbb9dca9520ad1a7db Mon Sep 17 00:00:00 2001 From: Lucas Parzianello Date: Tue, 7 Jul 2026 14:34:01 -0400 Subject: [PATCH 1/8] gh action to push gateway image to ghcr; updated gh action versions --- .github/workflows/gwy-code-quality.yaml | 55 +++++++++++++++++-- .github/workflows/gwy-promote-stable.yaml | 31 +++++++++++ .github/workflows/sdk-build-and-publish.yaml | 14 ++--- .github/workflows/sdk-code-quality.yaml | 16 +++--- .../workflows/sdk-cross-platform-tests.yaml | 4 +- gateway/compose.production.yaml | 20 ++----- 6 files changed, 103 insertions(+), 37 deletions(-) create mode 100644 .github/workflows/gwy-promote-stable.yaml diff --git a/.github/workflows/gwy-code-quality.yaml b/.github/workflows/gwy-code-quality.yaml index 67a711138..bf9f5c8b4 100644 --- a/.github/workflows/gwy-code-quality.yaml +++ b/.github/workflows/gwy-code-quality.yaml @@ -36,7 +36,7 @@ jobs: env: UV_LINK_MODE: copy steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 # this ubuntu-latest version ships node 18, which was causing issues # https://github.com/actions/setup-node#usage @@ -48,12 +48,12 @@ jobs: uses: actions-rust-lang/setup-rust-toolchain@v1 - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 # https://github.com/marketplace/actions/astral-sh-setup-uv - name: Cache prek hooks id: cache-prek - uses: actions/cache@v5 + uses: actions/cache@v6 # https://github.com/actions/cache/blob/main/examples.md#python---pip with: key: prek-gateway-${{ hashFiles('gateway/.pre-commit-config.yaml') }} @@ -89,7 +89,7 @@ jobs: # This is faster, saving some GH action minutes and dev time. platform: [ubuntu-latest] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 # - name: Install dependencies # run: sudo apt update && sudo apt install -y rsync @@ -107,3 +107,50 @@ jobs: - name: Run tests run: just test working-directory: ./gateway + + # Build and push Docker image to ghcr.io (only if checks pass, only on push, not PR) + gwy-build-and-push: + needs: [gwy-pre-commit, gwy-django-tests] + if: github.event_name == 'push' && github.ref_name == 'master' || github.event_name == 'push' && github.ref_name == 'main' || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + steps: + - name: Checkout repository + uses: actions/checkout@v7 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v4 + + - name: Log in to ghcr.io + uses: docker/login-action@v4 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Extract metadata + id: meta + uses: docker/metadata-action@v6 + with: + images: ghcr.io/spectrumx/sds-gateway + tags: | + type=sha,prefix=dev-,format=short + type=raw,value=dev,enable={{ is_default_branch }} + flavor: | + latest=false + + - name: Build and push + uses: docker/build-push-action@v7 + with: + context: gateway + file: gateway/compose/production/django/Dockerfile + push: true + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + build-args: | + GIT_COMMIT=${{ github.sha }} + BUILD_ENVIRONMENT=production + cache-from: type=gha + cache-to: type=gha,mode=max diff --git a/.github/workflows/gwy-promote-stable.yaml b/.github/workflows/gwy-promote-stable.yaml new file mode 100644 index 000000000..d560247ed --- /dev/null +++ b/.github/workflows/gwy-promote-stable.yaml @@ -0,0 +1,31 @@ +name: gateway-promote-stable + +on: + workflow_dispatch: + inputs: + commit_sha: + description: 'Short commit SHA to promote (e.g. a1b2c3d)' + required: true + type: string + +permissions: + packages: write + +jobs: + gwy-promote: + runs-on: ubuntu-latest + steps: + - name: Log in to ghcr.io + uses: docker/login-action@v4 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Promote dev- to stable + run: | + SHA="${{ inputs.commit_sha }}" + REGISTRY="ghcr.io/spectrumx/sds-gateway" + docker buildx imagetools create \ + --tag "${REGISTRY}:stable" \ + "${REGISTRY}:dev-${SHA}" diff --git a/.github/workflows/sdk-build-and-publish.yaml b/.github/workflows/sdk-build-and-publish.yaml index 82c748032..86143bc61 100644 --- a/.github/workflows/sdk-build-and-publish.yaml +++ b/.github/workflows/sdk-build-and-publish.yaml @@ -29,7 +29,7 @@ jobs: steps: # https://github.com/actions/checkout - name: Checkout code - uses: actions/checkout@v6 + uses: actions/checkout@v7 # https://github.com/actions/setup-python - name: Set up Python @@ -39,7 +39,7 @@ jobs: # https://github.com/astral-sh/setup-uv - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 with: # The version of uv to install version: latest # optional, default is latest @@ -100,7 +100,7 @@ jobs: - name: Store artifact # https://github.com/actions/upload-artifact?tab=readme-ov-file#inputs - uses: actions/upload-artifact@main + uses: actions/upload-artifact@v7 with: name: dist path: ./sdk/dist @@ -113,7 +113,7 @@ jobs: steps: - name: Download artifact for GH release # https://github.com/actions/download-artifact?tab=readme-ov-file#inputs - uses: actions/download-artifact@main + uses: actions/download-artifact@v8 with: name: dist path: ./sdk/dist @@ -121,7 +121,7 @@ jobs: - name: Automatic GitHub release # https://github.com/marketplace/actions/automatic-releases # generates a `@latest` release on github with automatic changelog - uses: "marvinpinto/action-automatic-releases@latest" + uses: "marvinpinto/action-automatic-releases@v1.2.1" with: repo_token: "${{ secrets.GITHUB_TOKEN }}" prerelease: false @@ -140,14 +140,14 @@ jobs: steps: - name: Download artifact for PyPI deploy # https://github.com/actions/download-artifact?tab=readme-ov-file#inputs - uses: actions/download-artifact@main + uses: actions/download-artifact@v8 with: name: dist path: ./sdk/dist # https://github.com/astral-sh/setup-uv - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 with: version: latest # optional, default is latest # used to increase the rate limit when retrieving versions and downloading uv. diff --git a/.github/workflows/sdk-code-quality.yaml b/.github/workflows/sdk-code-quality.yaml index 85fd317db..b68a3967d 100644 --- a/.github/workflows/sdk-code-quality.yaml +++ b/.github/workflows/sdk-code-quality.yaml @@ -35,7 +35,7 @@ jobs: env: UV_LINK_MODE: copy steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 # this ubuntu-latest version ships node 18, which was causing issues # https://github.com/actions/setup-node#usage @@ -44,12 +44,12 @@ jobs: node-version: 24 - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 # https://github.com/marketplace/actions/astral-sh-setup-uv - name: Cache prek hooks id: cache-prek - uses: actions/cache@v5 + uses: actions/cache@v6 # https://github.com/actions/cache/blob/main/examples.md#python---pip with: key: prek-${{ hashFiles('.pre-commit-config.yaml') }} @@ -73,10 +73,10 @@ jobs: UV_LINK_MODE: copy runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 - name: Install Python working-directory: ./sdk @@ -86,7 +86,7 @@ jobs: working-directory: ./sdk run: uv sync -p ${{ env.PYTHON_VERSION }} --frozen --dev - - uses: jakebailey/pyright-action@v2 + - uses: jakebailey/pyright-action@v3 # https://github.com/jakebailey/pyright-action#options with: pylance-version: latest-release @@ -106,7 +106,7 @@ jobs: platform: [ubuntu-latest] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install just on ubuntu if: matrix.platform == 'ubuntu-latest' @@ -115,7 +115,7 @@ jobs: npm install -g rust-just - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 # https://github.com/marketplace/actions/astral-sh-setup-uv - name: Run all tests diff --git a/.github/workflows/sdk-cross-platform-tests.yaml b/.github/workflows/sdk-cross-platform-tests.yaml index 190b6f6eb..aa093d795 100644 --- a/.github/workflows/sdk-cross-platform-tests.yaml +++ b/.github/workflows/sdk-cross-platform-tests.yaml @@ -15,10 +15,10 @@ jobs: platform: [ubuntu-latest, windows-latest, macos-latest] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 # https://github.com/marketplace/actions/astral-sh-setup-uv - name: Set up Python ${{ matrix.python-version }} diff --git a/gateway/compose.production.yaml b/gateway/compose.production.yaml index 97f7b3f33..9cdefe3a0 100644 --- a/gateway/compose.production.yaml +++ b/gateway/compose.production.yaml @@ -39,10 +39,7 @@ networks: services: sds-gateway-prod-app: - build: - context: . - dockerfile: ./compose/production/django/Dockerfile - image: sds-gateway-prod-app + image: ghcr.io/spectrumx/sds-gateway:${SDS_GATEWAY_TAG:-stable} container_name: sds-gateway-prod-app tty: true depends_on: @@ -337,10 +334,7 @@ services: # =================== # Celery services for background tasks celery-worker: - build: - context: . - dockerfile: ./compose/production/django/Dockerfile - image: sds-gateway-prod-app + image: ghcr.io/spectrumx/sds-gateway:${SDS_GATEWAY_TAG:-stable} container_name: sds-gateway-prod-celery-worker tty: true depends_on: @@ -398,10 +392,7 @@ services: celery-beat: # Celery Beat scheduler for periodic tasks - build: - context: . - dockerfile: ./compose/production/django/Dockerfile - image: sds-gateway-prod-app + image: ghcr.io/spectrumx/sds-gateway:${SDS_GATEWAY_TAG:-stable} container_name: sds-gateway-prod-celery-beat depends_on: sds-gateway-prod-app: @@ -458,10 +449,7 @@ services: celery-flower: # Celery monitoring and administration tool - build: - context: . - dockerfile: ./compose/production/django/Dockerfile - image: sds-gateway-prod-app + image: ghcr.io/spectrumx/sds-gateway:${SDS_GATEWAY_TAG:-stable} container_name: sds-gateway-prod-celery-flower tty: true depends_on: From 4ba2178ee769d3a97cc2ede921d2331bc5636f0d Mon Sep 17 00:00:00 2001 From: Lucas Parzianello Date: Tue, 7 Jul 2026 14:47:11 -0400 Subject: [PATCH 2/8] fix: add branch guard to workflow_dispatch in gwy-build-and-push job The gwy-build-and-push job's 'if' condition omitted the branch guard for workflow_dispatch events. Due to &&-over-|| precedence, any workflow_dispatch from any branch could push dev- images to ghcr.io. Wrap both push and workflow_dispatch clauses with the master/main branch check. --- .github/workflows/gwy-code-quality.yaml | 2 +- .github/workflows/gwy-promote-stable.yaml | 29 ++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/.github/workflows/gwy-code-quality.yaml b/.github/workflows/gwy-code-quality.yaml index bf9f5c8b4..3a4c3f667 100644 --- a/.github/workflows/gwy-code-quality.yaml +++ b/.github/workflows/gwy-code-quality.yaml @@ -111,7 +111,7 @@ jobs: # Build and push Docker image to ghcr.io (only if checks pass, only on push, not PR) gwy-build-and-push: needs: [gwy-pre-commit, gwy-django-tests] - if: github.event_name == 'push' && github.ref_name == 'master' || github.event_name == 'push' && github.ref_name == 'main' || github.event_name == 'workflow_dispatch' + if: (github.event_name == 'push' && (github.ref_name == 'master' || github.ref_name == 'main')) || (github.event_name == 'workflow_dispatch' && (github.ref_name == 'master' || github.ref_name == 'main')) runs-on: ubuntu-latest permissions: contents: read diff --git a/.github/workflows/gwy-promote-stable.yaml b/.github/workflows/gwy-promote-stable.yaml index d560247ed..2c97927c3 100644 --- a/.github/workflows/gwy-promote-stable.yaml +++ b/.github/workflows/gwy-promote-stable.yaml @@ -9,12 +9,38 @@ on: type: string permissions: + contents: read packages: write jobs: gwy-promote: runs-on: ubuntu-latest steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Verify commit is on default branch + env: + SHA: ${{ inputs.commit_sha }} + run: | + SHA_INPUT="${SHA}" + # Resolve the short SHA to a full SHA + if ! FULL_SHA=$(git rev-parse --verify "${SHA_INPUT}^{commit}" 2>/dev/null); then + echo "ERROR: Could not resolve commit '${SHA_INPUT}' in this repository." + echo "Ensure you are using a valid short SHA from the default branch (main)." + exit 1 + fi + echo "Resolved SHA: ${FULL_SHA}" + # Verify the commit is an ancestor of the default branch (main) + if ! git merge-base --is-ancestor "${FULL_SHA}" origin/main; then + echo "ERROR: Commit ${SHA_INPUT} (${FULL_SHA}) is not an ancestor of the default branch (main)." + echo "Only commits from the default branch can be promoted to stable." + exit 1 + fi + echo "✓ Commit ${SHA_INPUT} is a valid ancestor of main. Proceeding with promotion." + - name: Log in to ghcr.io uses: docker/login-action@v4 with: @@ -23,8 +49,9 @@ jobs: password: ${{ secrets.GITHUB_TOKEN }} - name: Promote dev- to stable + env: + SHA: ${{ inputs.commit_sha }} run: | - SHA="${{ inputs.commit_sha }}" REGISTRY="ghcr.io/spectrumx/sds-gateway" docker buildx imagetools create \ --tag "${REGISTRY}:stable" \ From 1dd6c12548b02391d669f3f4386701e661b40099 Mon Sep 17 00:00:00 2001 From: Lucas Parzianello Date: Tue, 7 Jul 2026 14:51:10 -0400 Subject: [PATCH 3/8] astral action tag fix --- .github/workflows/gwy-code-quality.yaml | 2 +- .github/workflows/sdk-build-and-publish.yaml | 4 ++-- .github/workflows/sdk-code-quality.yaml | 6 +++--- .github/workflows/sdk-cross-platform-tests.yaml | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/gwy-code-quality.yaml b/.github/workflows/gwy-code-quality.yaml index 3a4c3f667..c9cf64394 100644 --- a/.github/workflows/gwy-code-quality.yaml +++ b/.github/workflows/gwy-code-quality.yaml @@ -48,7 +48,7 @@ jobs: uses: actions-rust-lang/setup-rust-toolchain@v1 - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@d31148d669074a8d0a63714ba94f3201e7020bc3 # v8.3.0 # https://github.com/marketplace/actions/astral-sh-setup-uv - name: Cache prek hooks diff --git a/.github/workflows/sdk-build-and-publish.yaml b/.github/workflows/sdk-build-and-publish.yaml index 86143bc61..5f434f245 100644 --- a/.github/workflows/sdk-build-and-publish.yaml +++ b/.github/workflows/sdk-build-and-publish.yaml @@ -39,7 +39,7 @@ jobs: # https://github.com/astral-sh/setup-uv - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@d31148d669074a8d0a63714ba94f3201e7020bc3 # v8.3.0 with: # The version of uv to install version: latest # optional, default is latest @@ -147,7 +147,7 @@ jobs: # https://github.com/astral-sh/setup-uv - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@d31148d669074a8d0a63714ba94f3201e7020bc3 # v8.3.0 with: version: latest # optional, default is latest # used to increase the rate limit when retrieving versions and downloading uv. diff --git a/.github/workflows/sdk-code-quality.yaml b/.github/workflows/sdk-code-quality.yaml index b68a3967d..dd8f8ab93 100644 --- a/.github/workflows/sdk-code-quality.yaml +++ b/.github/workflows/sdk-code-quality.yaml @@ -44,7 +44,7 @@ jobs: node-version: 24 - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@d31148d669074a8d0a63714ba94f3201e7020bc3 # v8.3.0 # https://github.com/marketplace/actions/astral-sh-setup-uv - name: Cache prek hooks @@ -76,7 +76,7 @@ jobs: - uses: actions/checkout@v7 - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@d31148d669074a8d0a63714ba94f3201e7020bc3 # v8.3.0 - name: Install Python working-directory: ./sdk @@ -115,7 +115,7 @@ jobs: npm install -g rust-just - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@d31148d669074a8d0a63714ba94f3201e7020bc3 # v8.3.0 # https://github.com/marketplace/actions/astral-sh-setup-uv - name: Run all tests diff --git a/.github/workflows/sdk-cross-platform-tests.yaml b/.github/workflows/sdk-cross-platform-tests.yaml index aa093d795..e9fdf27db 100644 --- a/.github/workflows/sdk-cross-platform-tests.yaml +++ b/.github/workflows/sdk-cross-platform-tests.yaml @@ -18,7 +18,7 @@ jobs: - uses: actions/checkout@v7 - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@d31148d669074a8d0a63714ba94f3201e7020bc3 # v8.3.0 # https://github.com/marketplace/actions/astral-sh-setup-uv - name: Set up Python ${{ matrix.python-version }} From 5dae463c03c7236958ad96df4a634a6a35ec24ef Mon Sep 17 00:00:00 2001 From: Lucas Parzianello Date: Tue, 7 Jul 2026 15:50:12 -0400 Subject: [PATCH 4/8] gwy: normalize dataset authors format on save + add migration command --- .../commands/migrate_dataset_authors.py | 114 ++++++++++++++++++ gateway/sds_gateway/api_methods/models.py | 22 ++++ 2 files changed, 136 insertions(+) create mode 100644 gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py diff --git a/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py b/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py new file mode 100644 index 000000000..b988b63d5 --- /dev/null +++ b/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py @@ -0,0 +1,114 @@ +"""Convert dataset authors from old string format to new object format. + +Old format: ["Author One", "Author Two"] (list of strings) +New format: [{"name": "Author One", "orcid_id": ""}, ...] (list of dicts) + +This is a one-time data fix for datasets that were created or versioned +before the model-level auto-normalization was added to Dataset.save(). +""" + +from django.core.management.base import BaseCommand +from loguru import logger + +from sds_gateway.api_methods.models import Dataset + + +class Command(BaseCommand): + help = ( + "Convert dataset authors from old string format ['Name'] " + "to new object format [{'name': 'Name', 'orcid_id': ''}]" + ) + + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Only count affected datasets without modifying them", + ) + parser.add_argument( + "--uuid", + type=str, + help="Migrate only a specific dataset by UUID", + ) + + def handle(self, *args, **options): + dry_run = options["dry_run"] + specific_uuid = options.get("uuid") + + # Query datasets with non-empty authors + queryset = Dataset.objects.filter(authors__isnull=False).exclude(authors="") + if specific_uuid: + queryset = queryset.filter(uuid=specific_uuid) + + total = queryset.count() + updated = 0 + skipped = 0 + errors = 0 + + self.stdout.write(f"Scanning {total} dataset(s) with authors...") + + for dataset in queryset: + try: + # from_db already deserialized authors from JSON string to Python object + authors = dataset.authors + + # Skip if not a list + if not isinstance(authors, list): + skipped += 1 + continue + + # Skip empty lists + if not authors: + skipped += 1 + continue + + # Skip if already in new format (list of dicts with 'name' key) + if isinstance(authors[0], dict): + skipped += 1 + continue + + # Convert old format: list of strings → list of dicts + if isinstance(authors[0], str): + new_authors = [ + {"name": author, "orcid_id": ""} + for author in authors + if isinstance(author, str) + ] + + if not new_authors: + logger.warning( + f"Dataset {dataset.uuid}: no valid author strings found, " + "skipping", + ) + skipped += 1 + continue + + if dry_run: + logger.info( + f"[DRY-RUN] Would convert dataset {dataset.uuid}: " + f"{len(authors)} author(s) in old format" + ) + else: + dataset.authors = new_authors + dataset.save(update_fields=["authors"]) + logger.info( + f"Converted dataset {dataset.uuid}: " + f"{len(authors)} author(s) → new format" + ) + updated += 1 + else: + # Unexpected format + logger.warning( + f"Dataset {dataset.uuid}: unexpected author type " + f"{type(authors[0]).__name__}, skipping" + ) + skipped += 1 + + except Exception as e: # noqa: BLE001 # intentional: continue processing remaining datasets + logger.error(f"Error processing dataset {dataset.uuid}: {e}") + errors += 1 + + self.stdout.write( + f"Done: {updated} updated, {skipped} skipped, {errors} errors" + + (" (dry run)" if dry_run else "") + ) diff --git a/gateway/sds_gateway/api_methods/models.py b/gateway/sds_gateway/api_methods/models.py index 8c56b3c87..5bf57c82f 100644 --- a/gateway/sds_gateway/api_methods/models.py +++ b/gateway/sds_gateway/api_methods/models.py @@ -990,6 +990,28 @@ def save(self, *args, **kwargs) -> None: field_value = getattr(self, field) if field_value: if isinstance(field_value, list): + # --- Authors format normalization --- + # Prevent "authors still in old string format" warning by + # auto-converting old-style list-of-strings to list-of-dicts + # on save. Catches all write paths (form, versioning, API, + # direct ORM create) so old format cannot persist even when + # entering through code paths that bypass form validation + # (e.g. dataset versioning which copies raw field values + # via getattr + Dataset.objects.create). + # + # Old format: ["Author One", "Author Two"] + # New format: [{"name": "Author One", "orcid_id": ""}, ...] + if ( + field == "authors" + and field_value + and isinstance(field_value[0], str) + ): + field_value = [ + {"name": author, "orcid_id": ""} + for author in field_value + if isinstance(author, str) + ] + # --- End authors format normalization --- setattr(self, field, json.dumps(field_value)) elif isinstance(field_value, str): # If it's already a string, it might be JSON - try to parse it From 7fa54174192ecffe97bd0a6b1556725cf4e95248 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 7 Jul 2026 20:07:49 +0000 Subject: [PATCH 5/8] Fix migrate_dataset_authors to handle invalid authors JSON per row Iterate queryset.values() instead of model instances so Dataset.from_db() does not json.loads(authors) before per-row error handling. Parse authors manually and use queryset.update() for writes, matching migration 0017. Co-authored-by: Lucas Parzianello --- .../commands/migrate_dataset_authors.py | 36 +++++++--- .../test_migrate_dataset_authors_command.py | 66 +++++++++++++++++++ 2 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 gateway/sds_gateway/api_methods/tests/test_migrate_dataset_authors_command.py diff --git a/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py b/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py index b988b63d5..f520fd62e 100644 --- a/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py +++ b/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py @@ -7,6 +7,8 @@ before the model-level auto-normalization was added to Dataset.save(). """ +import json + from django.core.management.base import BaseCommand from loguru import logger @@ -47,10 +49,23 @@ def handle(self, *args, **options): self.stdout.write(f"Scanning {total} dataset(s) with authors...") - for dataset in queryset: + # Iterate raw DB values so Dataset.from_db() does not json.loads(authors) + # before we can handle invalid JSON per row (see migration 0017). + for row in queryset.values("uuid", "authors"): + dataset_uuid = row["uuid"] try: - # from_db already deserialized authors from JSON string to Python object - authors = dataset.authors + authors_raw = row["authors"] + if isinstance(authors_raw, str): + try: + authors = json.loads(authors_raw) + except (json.JSONDecodeError, TypeError): + logger.warning( + f"Dataset {dataset_uuid}: invalid authors JSON, skipping", + ) + skipped += 1 + continue + else: + authors = authors_raw # Skip if not a list if not isinstance(authors, list): @@ -77,7 +92,7 @@ def handle(self, *args, **options): if not new_authors: logger.warning( - f"Dataset {dataset.uuid}: no valid author strings found, " + f"Dataset {dataset_uuid}: no valid author strings found, " "skipping", ) skipped += 1 @@ -85,27 +100,28 @@ def handle(self, *args, **options): if dry_run: logger.info( - f"[DRY-RUN] Would convert dataset {dataset.uuid}: " + f"[DRY-RUN] Would convert dataset {dataset_uuid}: " f"{len(authors)} author(s) in old format" ) else: - dataset.authors = new_authors - dataset.save(update_fields=["authors"]) + Dataset.objects.filter(uuid=dataset_uuid).update( + authors=json.dumps(new_authors), + ) logger.info( - f"Converted dataset {dataset.uuid}: " + f"Converted dataset {dataset_uuid}: " f"{len(authors)} author(s) → new format" ) updated += 1 else: # Unexpected format logger.warning( - f"Dataset {dataset.uuid}: unexpected author type " + f"Dataset {dataset_uuid}: unexpected author type " f"{type(authors[0]).__name__}, skipping" ) skipped += 1 except Exception as e: # noqa: BLE001 # intentional: continue processing remaining datasets - logger.error(f"Error processing dataset {dataset.uuid}: {e}") + logger.error(f"Error processing dataset {dataset_uuid}: {e}") errors += 1 self.stdout.write( diff --git a/gateway/sds_gateway/api_methods/tests/test_migrate_dataset_authors_command.py b/gateway/sds_gateway/api_methods/tests/test_migrate_dataset_authors_command.py new file mode 100644 index 000000000..19597efa2 --- /dev/null +++ b/gateway/sds_gateway/api_methods/tests/test_migrate_dataset_authors_command.py @@ -0,0 +1,66 @@ +"""Tests for the migrate_dataset_authors management command.""" + +from __future__ import annotations + +import json + +import pytest +from django.core.management import call_command +from django.test.utils import captured_stdout + +from sds_gateway.api_methods.models import Dataset +from sds_gateway.api_methods.tests.factories import DatasetFactory + + +def _set_raw_authors(dataset: Dataset, authors_value: str) -> None: + Dataset.objects.filter(uuid=dataset.uuid).update(authors=authors_value) + + +@pytest.mark.django_db +def test_migrate_dataset_authors_continues_after_invalid_json() -> None: + valid_dataset = DatasetFactory(authors=["Alice", "Bob"]) + invalid_dataset = DatasetFactory(authors=["Legacy Author"]) + _set_raw_authors(invalid_dataset, "not valid json") + + with captured_stdout() as stdout: + call_command("migrate_dataset_authors") + + output = stdout.getvalue() + assert "1 updated" in output + assert "1 skipped" in output + assert "0 errors" in output + + valid_authors = json.loads( + Dataset.objects.filter(uuid=valid_dataset.uuid).values_list( + "authors", flat=True + ).get(), + ) + assert valid_authors == [ + {"name": "Alice", "orcid_id": ""}, + {"name": "Bob", "orcid_id": ""}, + ] + + invalid_authors = Dataset.objects.filter(uuid=invalid_dataset.uuid).values_list( + "authors", flat=True + ).get() + assert invalid_authors == "not valid json" + + +@pytest.mark.django_db +def test_migrate_dataset_authors_dry_run_does_not_modify_rows() -> None: + dataset = DatasetFactory(authors=["Alice"]) + original_authors = Dataset.objects.filter(uuid=dataset.uuid).values_list( + "authors", flat=True + ).get() + + with captured_stdout() as stdout: + call_command("migrate_dataset_authors", "--dry-run") + + output = stdout.getvalue() + assert "1 updated" in output + assert "(dry run)" in output + + current_authors = Dataset.objects.filter(uuid=dataset.uuid).values_list( + "authors", flat=True + ).get() + assert current_authors == original_authors From 34ffe3678b1fcff8e820cba9708a95dff9044fa6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 7 Jul 2026 20:08:23 +0000 Subject: [PATCH 6/8] Fix authors normalization for legacy JSON string values on save Dataset.save() only normalized authors when the field was already a Python list. Legacy values passed as valid JSON strings (e.g. from versioning or direct ORM writes) were validated but left unchanged, allowing the old list-of-strings format to persist and trigger production warnings. Extract _normalize_authors_list() and apply it in both the list and JSON string branches. Add regression tests for JSON-string write paths. Co-authored-by: Lucas Parzianello --- gateway/sds_gateway/api_methods/models.py | 29 ++++--- .../tests/test_dataset_authors_save.py | 82 +++++++++++++++++++ 2 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py diff --git a/gateway/sds_gateway/api_methods/models.py b/gateway/sds_gateway/api_methods/models.py index 5bf57c82f..42cd2a4c0 100644 --- a/gateway/sds_gateway/api_methods/models.py +++ b/gateway/sds_gateway/api_methods/models.py @@ -984,6 +984,17 @@ class Dataset(BaseModel): def __str__(self) -> str: return self.name + @staticmethod + def _normalize_authors_list(authors: list) -> list: + """Convert legacy list-of-strings authors to list-of-dicts on save.""" + if authors and isinstance(authors[0], str): + return [ + {"name": author, "orcid_id": ""} + for author in authors + if isinstance(author, str) + ] + return authors + def save(self, *args, **kwargs) -> None: # Serialize the list fields to a JSON string before saving for field in self.list_fields: @@ -1001,23 +1012,17 @@ def save(self, *args, **kwargs) -> None: # # Old format: ["Author One", "Author Two"] # New format: [{"name": "Author One", "orcid_id": ""}, ...] - if ( - field == "authors" - and field_value - and isinstance(field_value[0], str) - ): - field_value = [ - {"name": author, "orcid_id": ""} - for author in field_value - if isinstance(author, str) - ] + if field == "authors": + field_value = self._normalize_authors_list(field_value) # --- End authors format normalization --- setattr(self, field, json.dumps(field_value)) elif isinstance(field_value, str): # If it's already a string, it might be JSON - try to parse it try: - # Validate that it's valid JSON - json.loads(field_value) + parsed_value = json.loads(field_value) + if field == "authors" and isinstance(parsed_value, list): + normalized = self._normalize_authors_list(parsed_value) + setattr(self, field, json.dumps(normalized)) # If it's already valid JSON, leave it as is except (json.JSONDecodeError, TypeError): # If it's not valid JSON, raise an error diff --git a/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py b/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py new file mode 100644 index 000000000..3e7460d67 --- /dev/null +++ b/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py @@ -0,0 +1,82 @@ +"""Tests for Dataset.save() authors format normalization.""" + +import json +from unittest.mock import patch + +from django.contrib.auth import get_user_model +from django.test import TestCase + +from sds_gateway.api_methods.models import Dataset + +User = get_user_model() + + +class TestDatasetAuthorsSaveNormalization(TestCase): + """Test authors format normalization in Dataset.save().""" + + def setUp(self): + self.user = User.objects.create_user( + email="authors-save@example.com", + password="testpass123", # noqa: S106 + name="Authors Save Test", + is_approved=True, + ) + + def _create_dataset(self, **kwargs): + defaults = {"name": "authors-save-test", "owner": self.user} + defaults.update(kwargs) + return Dataset.objects.create(**defaults) + + def test_save_normalizes_python_list_of_strings(self): + """Legacy list-of-strings authors are converted on save.""" + dataset = self._create_dataset( + name="authors-list-normalize", + authors=["Author One", "Author Two"], + ) + + stored = json.loads(dataset.authors) + assert stored == [ + {"name": "Author One", "orcid_id": ""}, + {"name": "Author Two", "orcid_id": ""}, + ] + + def test_save_normalizes_json_string_of_strings(self): + """Legacy JSON string list-of-strings authors are converted on save.""" + dataset = self._create_dataset( + name="authors-json-normalize", + authors='["Author One", "Author Two"]', + ) + + stored = json.loads(dataset.authors) + assert stored == [ + {"name": "Author One", "orcid_id": ""}, + {"name": "Author Two", "orcid_id": ""}, + ] + + def test_save_leaves_new_format_json_string_unchanged_semantically(self): + """New-format authors JSON strings are stored as list-of-dicts.""" + authors_json = json.dumps( + [{"name": "Author One", "orcid_id": "0000-0001-2345-6789"}] + ) + dataset = self._create_dataset( + name="authors-new-json", + authors=authors_json, + ) + + stored = json.loads(dataset.authors) + assert stored == [ + {"name": "Author One", "orcid_id": "0000-0001-2345-6789"}, + ] + + def test_get_authors_display_does_not_warn_for_normalized_authors(self): + """Normalized authors should not trigger old-format warnings on read.""" + dataset = self._create_dataset( + name="authors-no-warning", + authors='["Author One"]', + ) + + with patch("sds_gateway.api_methods.models.log") as mock_log: + authors = dataset.get_authors_display() + + assert authors == [{"name": "Author One", "orcid_id": ""}] + mock_log.warning.assert_not_called() From 5abf55144bc49e8385d0744b99f524ab253855ab Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 7 Jul 2026 20:08:30 +0000 Subject: [PATCH 7/8] Fix Dataset.save dropping dict authors in mixed author lists When authors contained both legacy strings and new dict entries, save() only normalized when the first entry was a string and rebuilt the list from string entries only. Dict-shaped co-authors were silently dropped. Normalize any string entries while preserving existing dict entries, matching AuthorsManager.normalizeAuthorEntries behavior. Co-authored-by: Lucas Parzianello --- gateway/sds_gateway/api_methods/models.py | 5 ++-- .../tests/test_dataset_authors_save.py | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/gateway/sds_gateway/api_methods/models.py b/gateway/sds_gateway/api_methods/models.py index 42cd2a4c0..d82f84f5f 100644 --- a/gateway/sds_gateway/api_methods/models.py +++ b/gateway/sds_gateway/api_methods/models.py @@ -987,11 +987,12 @@ def __str__(self) -> str: @staticmethod def _normalize_authors_list(authors: list) -> list: """Convert legacy list-of-strings authors to list-of-dicts on save.""" - if authors and isinstance(authors[0], str): + if authors and any(isinstance(author, str) for author in authors): return [ {"name": author, "orcid_id": ""} - for author in authors if isinstance(author, str) + else author + for author in authors ] return authors diff --git a/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py b/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py index 3e7460d67..2bf1966d6 100644 --- a/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py +++ b/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py @@ -68,6 +68,34 @@ def test_save_leaves_new_format_json_string_unchanged_semantically(self): {"name": "Author One", "orcid_id": "0000-0001-2345-6789"}, ] + def test_save_preserves_dict_authors_when_mixed_with_strings_first(self): + """Mixed legacy strings and dict authors keep dict entries when string is first.""" + co_author = {"name": "Co Author", "orcid_id": "0000-0001-2345-6789"} + dataset = self._create_dataset( + name="mixed-authors-string-first", + authors=["Legacy Author", co_author], + ) + + stored = json.loads(dataset.authors) + assert stored == [ + {"name": "Legacy Author", "orcid_id": ""}, + co_author, + ] + + def test_save_preserves_dict_authors_when_mixed_with_strings_last(self): + """Mixed legacy strings and dict authors keep dict entries when string is last.""" + co_author = {"name": "Co Author", "orcid_id": "0000-0001-2345-6789"} + dataset = self._create_dataset( + name="mixed-authors-string-last", + authors=[co_author, "Legacy Author"], + ) + + stored = json.loads(dataset.authors) + assert stored == [ + co_author, + {"name": "Legacy Author", "orcid_id": ""}, + ] + def test_get_authors_display_does_not_warn_for_normalized_authors(self): """Normalized authors should not trigger old-format warnings on read.""" dataset = self._create_dataset( From 298105861a679f97578d785d5e58646656d2cdc0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 7 Jul 2026 21:09:27 +0000 Subject: [PATCH 8/8] Fix lint issues and harden dataset authors migration tests - Refactor migrate_dataset_authors command to satisfy ruff complexity rules - Move Dataset.save before _normalize_authors_list per Django style guide - Parse JSON-string authors in get_authors_display for in-memory instances - Update migrate command tests to use raw DB authors and avoid factory bug - Shorten test docstrings to satisfy line-length checks --- gateway/package.json | 12 +- .../commands/migrate_dataset_authors.py | 161 +++++++++--------- gateway/sds_gateway/api_methods/models.py | 42 +++-- .../tests/test_dataset_authors_save.py | 4 +- .../test_migrate_dataset_authors_command.py | 54 ++++-- 5 files changed, 158 insertions(+), 115 deletions(-) diff --git a/gateway/package.json b/gateway/package.json index fe30d0786..83f6f4a2f 100644 --- a/gateway/package.json +++ b/gateway/package.json @@ -42,10 +42,16 @@ "onFail": "download" } }, - "browserslist": ["last 2 versions"], + "browserslist": [ + "last 2 versions" + ], "babel": { - "presets": ["@babel/preset-env"], - "plugins": ["@babel/plugin-transform-runtime"] + "presets": [ + "@babel/preset-env" + ], + "plugins": [ + "@babel/plugin-transform-runtime" + ] }, "parser": "babel-eslint", "parserOptions": { diff --git a/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py b/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py index f520fd62e..8018e0c9f 100644 --- a/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py +++ b/gateway/sds_gateway/api_methods/management/commands/migrate_dataset_authors.py @@ -8,12 +8,15 @@ """ import json +from typing import Literal from django.core.management.base import BaseCommand from loguru import logger from sds_gateway.api_methods.models import Dataset +MigrationOutcome = Literal["updated", "skipped", "error"] + class Command(BaseCommand): help = ( @@ -37,94 +40,98 @@ def handle(self, *args, **options): dry_run = options["dry_run"] specific_uuid = options.get("uuid") - # Query datasets with non-empty authors queryset = Dataset.objects.filter(authors__isnull=False).exclude(authors="") if specific_uuid: queryset = queryset.filter(uuid=specific_uuid) total = queryset.count() - updated = 0 - skipped = 0 - errors = 0 + counts = {"updated": 0, "skipped": 0, "error": 0} self.stdout.write(f"Scanning {total} dataset(s) with authors...") # Iterate raw DB values so Dataset.from_db() does not json.loads(authors) # before we can handle invalid JSON per row (see migration 0017). for row in queryset.values("uuid", "authors"): - dataset_uuid = row["uuid"] - try: - authors_raw = row["authors"] - if isinstance(authors_raw, str): - try: - authors = json.loads(authors_raw) - except (json.JSONDecodeError, TypeError): - logger.warning( - f"Dataset {dataset_uuid}: invalid authors JSON, skipping", - ) - skipped += 1 - continue - else: - authors = authors_raw - - # Skip if not a list - if not isinstance(authors, list): - skipped += 1 - continue - - # Skip empty lists - if not authors: - skipped += 1 - continue - - # Skip if already in new format (list of dicts with 'name' key) - if isinstance(authors[0], dict): - skipped += 1 - continue - - # Convert old format: list of strings → list of dicts - if isinstance(authors[0], str): - new_authors = [ - {"name": author, "orcid_id": ""} - for author in authors - if isinstance(author, str) - ] - - if not new_authors: - logger.warning( - f"Dataset {dataset_uuid}: no valid author strings found, " - "skipping", - ) - skipped += 1 - continue - - if dry_run: - logger.info( - f"[DRY-RUN] Would convert dataset {dataset_uuid}: " - f"{len(authors)} author(s) in old format" - ) - else: - Dataset.objects.filter(uuid=dataset_uuid).update( - authors=json.dumps(new_authors), - ) - logger.info( - f"Converted dataset {dataset_uuid}: " - f"{len(authors)} author(s) → new format" - ) - updated += 1 - else: - # Unexpected format - logger.warning( - f"Dataset {dataset_uuid}: unexpected author type " - f"{type(authors[0]).__name__}, skipping" - ) - skipped += 1 - - except Exception as e: # noqa: BLE001 # intentional: continue processing remaining datasets - logger.error(f"Error processing dataset {dataset_uuid}: {e}") - errors += 1 + outcome = self._migrate_row(row, dry_run=dry_run) + counts[outcome] += 1 self.stdout.write( - f"Done: {updated} updated, {skipped} skipped, {errors} errors" - + (" (dry run)" if dry_run else "") + f"Done: {counts['updated']} updated, {counts['skipped']} skipped, " + f"{counts['error']} errors" + (" (dry run)" if dry_run else "") ) + + def _migrate_row(self, row: dict, *, dry_run: bool) -> MigrationOutcome: + dataset_uuid = row["uuid"] + try: + authors = self._parse_authors(row["authors"], dataset_uuid) + if authors is None: + return "skipped" + + new_authors = self._convert_legacy_authors(authors, dataset_uuid) + if new_authors is None: + return "skipped" + + if dry_run: + logger.info( + f"[DRY-RUN] Would convert dataset {dataset_uuid}: " + f"{len(authors)} author(s) in old format" + ) + else: + Dataset.objects.filter(uuid=dataset_uuid).update( + authors=json.dumps(new_authors), + ) + logger.info( + f"Converted dataset {dataset_uuid}: " + f"{len(authors)} author(s) → new format" + ) + except Exception as e: # noqa: BLE001 # intentional: continue processing remaining datasets + logger.error(f"Error processing dataset {dataset_uuid}: {e}") + return "error" + else: + return "updated" + + def _parse_authors(self, authors_raw, dataset_uuid: str) -> list | None: + if isinstance(authors_raw, str): + try: + authors = json.loads(authors_raw) + except (json.JSONDecodeError, TypeError): + logger.warning( + f"Dataset {dataset_uuid}: invalid authors JSON, skipping", + ) + return None + else: + authors = authors_raw + + if not isinstance(authors, list) or not authors: + return None + + return authors + + def _convert_legacy_authors( + self, + authors: list, + dataset_uuid: str, + ) -> list | None: + if isinstance(authors[0], dict): + return None + + if not isinstance(authors[0], str): + logger.warning( + f"Dataset {dataset_uuid}: unexpected author type " + f"{type(authors[0]).__name__}, skipping" + ) + return None + + new_authors = [ + {"name": author, "orcid_id": ""} + for author in authors + if isinstance(author, str) + ] + + if not new_authors: + logger.warning( + f"Dataset {dataset_uuid}: no valid author strings found, skipping", + ) + return None + + return new_authors diff --git a/gateway/sds_gateway/api_methods/models.py b/gateway/sds_gateway/api_methods/models.py index d82f84f5f..8547dfd8f 100644 --- a/gateway/sds_gateway/api_methods/models.py +++ b/gateway/sds_gateway/api_methods/models.py @@ -984,18 +984,6 @@ class Dataset(BaseModel): def __str__(self) -> str: return self.name - @staticmethod - def _normalize_authors_list(authors: list) -> list: - """Convert legacy list-of-strings authors to list-of-dicts on save.""" - if authors and any(isinstance(author, str) for author in authors): - return [ - {"name": author, "orcid_id": ""} - if isinstance(author, str) - else author - for author in authors - ] - return authors - def save(self, *args, **kwargs) -> None: # Serialize the list fields to a JSON string before saving for field in self.list_fields: @@ -1048,6 +1036,16 @@ def save(self, *args, **kwargs) -> None: super().save(*args, **kwargs) + @staticmethod + def _normalize_authors_list(authors: list) -> list: + """Convert legacy list-of-strings authors to list-of-dicts on save.""" + if authors and any(isinstance(author, str) for author in authors): + return [ + {"name": author, "orcid_id": ""} if isinstance(author, str) else author + for author in authors + ] + return authors + def soft_delete(self) -> None: """Soft delete this record after checking for blockers.""" from sds_gateway.api_methods.utils.asset_access_control import ( # noqa: PLC0415 @@ -1119,25 +1117,35 @@ def get_authors_display(self): if not self.authors: return [] + authors = self.authors + if isinstance(authors, str): + try: + authors = json.loads(authors) + except (json.JSONDecodeError, TypeError): + log.warning( + f"Dataset {self.uuid}: authors field is not valid JSON", + ) + return [] + # from_db should have already converted JSON string to list - if not isinstance(self.authors, list): + if not isinstance(authors, list): log.warning( f"Dataset {self.uuid}: authors field is not a list " - f"(type: {type(self.authors).__name__})", + f"(type: {type(authors).__name__})", ) return [] # Check if authors are in old string format and need conversion - if self.authors and isinstance(self.authors[0], str): + if authors and isinstance(authors[0], str): log.warning( f"Dataset {self.uuid}: authors still in old string format, " f"needs migration", ) # Convert old format for backward compatibility - return [{"name": author, "orcid_id": ""} for author in self.authors] + return [{"name": author, "orcid_id": ""} for author in authors] # Authors should already be in new object format - return self.authors + return authors class TemporaryZipFile(BaseModel): diff --git a/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py b/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py index 2bf1966d6..67844fbe5 100644 --- a/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py +++ b/gateway/sds_gateway/api_methods/tests/test_dataset_authors_save.py @@ -69,7 +69,7 @@ def test_save_leaves_new_format_json_string_unchanged_semantically(self): ] def test_save_preserves_dict_authors_when_mixed_with_strings_first(self): - """Mixed legacy strings and dict authors keep dict entries when string is first.""" + """Mixed string/dict authors keep dict entries when string is first.""" co_author = {"name": "Co Author", "orcid_id": "0000-0001-2345-6789"} dataset = self._create_dataset( name="mixed-authors-string-first", @@ -83,7 +83,7 @@ def test_save_preserves_dict_authors_when_mixed_with_strings_first(self): ] def test_save_preserves_dict_authors_when_mixed_with_strings_last(self): - """Mixed legacy strings and dict authors keep dict entries when string is last.""" + """Mixed string/dict authors keep dict entries when string is last.""" co_author = {"name": "Co Author", "orcid_id": "0000-0001-2345-6789"} dataset = self._create_dataset( name="mixed-authors-string-last", diff --git a/gateway/sds_gateway/api_methods/tests/test_migrate_dataset_authors_command.py b/gateway/sds_gateway/api_methods/tests/test_migrate_dataset_authors_command.py index 19597efa2..f88e1ae18 100644 --- a/gateway/sds_gateway/api_methods/tests/test_migrate_dataset_authors_command.py +++ b/gateway/sds_gateway/api_methods/tests/test_migrate_dataset_authors_command.py @@ -5,11 +5,25 @@ import json import pytest +from django.contrib.auth import get_user_model from django.core.management import call_command from django.test.utils import captured_stdout from sds_gateway.api_methods.models import Dataset -from sds_gateway.api_methods.tests.factories import DatasetFactory + +User = get_user_model() + + +def _create_dataset(**kwargs): + user = User.objects.create_user( + email=f"migrate-authors-{User.objects.count()}@example.com", + password="testpass123", # noqa: S106 + name="Migrate Authors Test", + is_approved=True, + ) + defaults = {"name": "migrate-authors-test", "owner": user} + defaults.update(kwargs) + return Dataset.objects.create(**defaults) def _set_raw_authors(dataset: Dataset, authors_value: str) -> None: @@ -18,8 +32,9 @@ def _set_raw_authors(dataset: Dataset, authors_value: str) -> None: @pytest.mark.django_db def test_migrate_dataset_authors_continues_after_invalid_json() -> None: - valid_dataset = DatasetFactory(authors=["Alice", "Bob"]) - invalid_dataset = DatasetFactory(authors=["Legacy Author"]) + valid_dataset = _create_dataset(authors=[]) + _set_raw_authors(valid_dataset, json.dumps(["Alice", "Bob"])) + invalid_dataset = _create_dataset(authors=[]) _set_raw_authors(invalid_dataset, "not valid json") with captured_stdout() as stdout: @@ -31,27 +46,32 @@ def test_migrate_dataset_authors_continues_after_invalid_json() -> None: assert "0 errors" in output valid_authors = json.loads( - Dataset.objects.filter(uuid=valid_dataset.uuid).values_list( - "authors", flat=True - ).get(), + Dataset.objects.filter(uuid=valid_dataset.uuid) + .values_list("authors", flat=True) + .get(), ) assert valid_authors == [ {"name": "Alice", "orcid_id": ""}, {"name": "Bob", "orcid_id": ""}, ] - invalid_authors = Dataset.objects.filter(uuid=invalid_dataset.uuid).values_list( - "authors", flat=True - ).get() + invalid_authors = ( + Dataset.objects.filter(uuid=invalid_dataset.uuid) + .values_list("authors", flat=True) + .get() + ) assert invalid_authors == "not valid json" @pytest.mark.django_db def test_migrate_dataset_authors_dry_run_does_not_modify_rows() -> None: - dataset = DatasetFactory(authors=["Alice"]) - original_authors = Dataset.objects.filter(uuid=dataset.uuid).values_list( - "authors", flat=True - ).get() + dataset = _create_dataset(authors=[]) + _set_raw_authors(dataset, json.dumps(["Alice"])) + original_authors = ( + Dataset.objects.filter(uuid=dataset.uuid) + .values_list("authors", flat=True) + .get() + ) with captured_stdout() as stdout: call_command("migrate_dataset_authors", "--dry-run") @@ -60,7 +80,9 @@ def test_migrate_dataset_authors_dry_run_does_not_modify_rows() -> None: assert "1 updated" in output assert "(dry run)" in output - current_authors = Dataset.objects.filter(uuid=dataset.uuid).values_list( - "authors", flat=True - ).get() + current_authors = ( + Dataset.objects.filter(uuid=dataset.uuid) + .values_list("authors", flat=True) + .get() + ) assert current_authors == original_authors