From aebb454db2940108bfb76c56b33f95fd9072877d Mon Sep 17 00:00:00 2001 From: "Yaroslav O. Halchenko" Date: Wed, 14 May 2025 15:10:13 -0400 Subject: [PATCH 1/9] BF: avoid clobbering of dict(..., **rec) type and always add a record for the author for new dandiset Closes #2376 @asmacdo reported that Embergoed dandisets have a difference of not having their author automatically added to the metadata. Identified via visual inspection this piece of code where there contributors list would be populated with the entry provided with the funder. This code seems to be used only in create_dandiset so I do not see much of side-effects --- dandiapi/api/services/version/metadata.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dandiapi/api/services/version/metadata.py b/dandiapi/api/services/version/metadata.py index bd82bcc40..1a52154cd 100644 --- a/dandiapi/api/services/version/metadata.py +++ b/dandiapi/api/services/version/metadata.py @@ -22,7 +22,14 @@ def _normalize_version_metadata( version_metadata = { 'schemaKey': 'Dandiset', 'schemaVersion': settings.DANDI_SCHEMA_VERSION, - 'contributor': [ + 'contributor': [], + **version_metadata, + } + # if function did not receive already a record for that name + # we create one: + if not any(r.get('name') == name for r in version_metadata['contributor']): + version_metadata['contributor'].insert( + 0, { 'name': name, 'email': email, @@ -31,9 +38,7 @@ def _normalize_version_metadata( 'affiliation': [], 'includeInCitation': True, }, - ], - **version_metadata, - } + ) # Run the version_metadata through the pydantic model to automatically include any boilerplate # like the access or repository fields return PydanticDandiset.model_construct(**version_metadata).model_dump( From de1218f761b4dafd96d7542498b3ef7129766e63 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Fri, 23 May 2025 11:54:35 -0500 Subject: [PATCH 2/9] Make sure both contributors (creator and added contrib) are in metadata --- dandiapi/api/tests/test_dandiset.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 6f1c3cfb1..2d91a4c00 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -547,7 +547,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): 'schemaKey': 'Person', 'affiliation': [], 'includeInCitation': True, - } + }, ], } @@ -571,7 +571,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): 'created': TIMESTAMP_RE, 'modified': TIMESTAMP_RE, }, - 'contact_person': 'Jane Doe', + 'contact_person': 'Doe, John', 'embargo_status': 'OPEN', 'star_count': 0, 'is_starred': False, @@ -603,13 +603,24 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): 'version': 'draft', 'url': url, 'dateCreated': UTC_ISO_TIMESTAMP_RE, - 'citation': (f'Jane Doe ({year}) {name} (Version draft) [Data set]. DANDI Archive. {url}'), + 'citation': ( + f'Doe, John; Jane Doe ({year}) {name} (Version draft) [Data set]. ' + f'DANDI Archive. {url}' + ), '@context': f'https://raw.githubusercontent.com/dandi/schema/master/releases/{settings.DANDI_SCHEMA_VERSION}/context.json', 'schemaVersion': settings.DANDI_SCHEMA_VERSION, 'schemaKey': 'Dandiset', 'access': [{'schemaKey': 'AccessRequirements', 'status': 'dandi:OpenAccess'}], 'repository': settings.DANDI_WEB_APP_URL, 'contributor': [ + { + 'affiliation': [], + 'email': 'admin@example.com', + 'includeInCitation': True, + 'name': 'Doe, John', + 'roleName': ['dcite:ContactPerson'], + 'schemaKey': 'Person', + }, { 'name': 'Jane Doe', 'email': 'jane.doe@kitware.com', @@ -617,7 +628,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): 'schemaKey': 'Person', 'affiliation': [], 'includeInCitation': True, - } + }, ], 'assetsSummary': { 'schemaKey': 'AssetsSummary', From 683fec4df579ccb6f053e5eaea7d99c74bb7aa8a Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Fri, 23 May 2025 11:59:00 -0500 Subject: [PATCH 3/9] fixup: name format --- dandiapi/api/tests/test_dandiset.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 2d91a4c00..5d0d93476 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -541,7 +541,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): # This contributor is different from the admin_user 'contributor': [ { - 'name': 'Jane Doe', + 'name': 'Doe, Jane', 'email': 'jane.doe@kitware.com', 'roleName': ['dcite:ContactPerson'], 'schemaKey': 'Person', @@ -603,10 +603,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): 'version': 'draft', 'url': url, 'dateCreated': UTC_ISO_TIMESTAMP_RE, - 'citation': ( - f'Doe, John; Jane Doe ({year}) {name} (Version draft) [Data set]. ' - f'DANDI Archive. {url}' - ), + 'citation': (f'Doe, John; Doe, Jane ({year}) {name} (Version draft) [Data set]. DANDI Archive. {url}'), '@context': f'https://raw.githubusercontent.com/dandi/schema/master/releases/{settings.DANDI_SCHEMA_VERSION}/context.json', 'schemaVersion': settings.DANDI_SCHEMA_VERSION, 'schemaKey': 'Dandiset', @@ -622,7 +619,7 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): 'schemaKey': 'Person', }, { - 'name': 'Jane Doe', + 'name': 'Doe, Jane', 'email': 'jane.doe@kitware.com', 'roleName': ['dcite:ContactPerson'], 'schemaKey': 'Person', From b86f0c222700f9a31554a69d06544d66becf74b5 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Fri, 23 May 2025 12:02:24 -0500 Subject: [PATCH 4/9] fixup: linter --- dandiapi/api/tests/test_dandiset.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dandiapi/api/tests/test_dandiset.py b/dandiapi/api/tests/test_dandiset.py index 5d0d93476..3dfd8b290 100644 --- a/dandiapi/api/tests/test_dandiset.py +++ b/dandiapi/api/tests/test_dandiset.py @@ -603,7 +603,10 @@ def test_dandiset_rest_create_with_contributor(api_client, admin_user): 'version': 'draft', 'url': url, 'dateCreated': UTC_ISO_TIMESTAMP_RE, - 'citation': (f'Doe, John; Doe, Jane ({year}) {name} (Version draft) [Data set]. DANDI Archive. {url}'), + 'citation': ( + f'Doe, John; Doe, Jane ({year}) {name} ' + f'(Version draft) [Data set]. DANDI Archive. {url}' + ), '@context': f'https://raw.githubusercontent.com/dandi/schema/master/releases/{settings.DANDI_SCHEMA_VERSION}/context.json', 'schemaVersion': settings.DANDI_SCHEMA_VERSION, 'schemaKey': 'Dandiset', From da7b1bf31e9a29d9eb9b17dad370f85d5f7a990b Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Tue, 27 May 2025 15:40:48 -0500 Subject: [PATCH 5/9] commit to bump the tests From 745110ab41354b64629026a8cea2bc330bd888b8 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 28 May 2025 13:56:20 -0500 Subject: [PATCH 6/9] Allow creation of superuser with a name If the superuser is used to create any dandisets (happens in tests) then the superuser is automatically added as a contributor If the superuser does not have a name, the dandiset version will be invalid. --- .../management/commands/createsuperuser.py | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/dandiapi/api/management/commands/createsuperuser.py b/dandiapi/api/management/commands/createsuperuser.py index e21975112..792224520 100644 --- a/dandiapi/api/management/commands/createsuperuser.py +++ b/dandiapi/api/management/commands/createsuperuser.py @@ -1,5 +1,6 @@ from __future__ import annotations +import os from typing import TYPE_CHECKING from composed_configuration._allauth_support.management.commands import createsuperuser @@ -11,8 +12,13 @@ from composed_configuration._allauth_support.createsuperuser import EmailAsUsernameProxyUser -def create_usermetadata(instance: EmailAsUsernameProxyUser, *args, **kwargs): - UserMetadata.objects.create(user=instance, status=UserMetadata.Status.APPROVED) +def create_usermetadata(instance: EmailAsUsernameProxyUser, created: bool, **kwargs): + if created and not hasattr(instance, '_usermetadata_created'): + UserMetadata.objects.get_or_create( + user=instance, + defaults={'status': UserMetadata.Status.APPROVED} + ) + instance._usermetadata_created = True class Command(createsuperuser.Command): @@ -25,6 +31,24 @@ def handle(self, *args, **kwargs) -> str | None: # Save the return value of the parent class function so we can return it later return_value = super().handle(*args, **kwargs) + # Set first_name and last_name from environment variables if provided + first_name = os.environ.get('DJANGO_SUPERUSER_FIRST_NAME') + last_name = os.environ.get('DJANGO_SUPERUSER_LAST_NAME') + + if first_name or last_name: + # Find the user that was just created + email = kwargs.get('email') or os.environ.get('DJANGO_SUPERUSER_EMAIL') + if email: + try: + user = createsuperuser.user_model.objects.get(email=email) + if first_name: + user.first_name = first_name + if last_name: + user.last_name = last_name + user.save() + except createsuperuser.user_model.DoesNotExist: + pass + # Disconnect the signal handler. This isn't strictly necessary, but this avoids any # unexpected behavior if, for example, someone extends this command and doesn't # realize there's a signal handler attached dynamically. From 6c473e06088d2b446bf16ed0d9881103fdae7733 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 28 May 2025 14:04:42 -0500 Subject: [PATCH 7/9] Add Requires_CLI_PR to test against a specific PR --- .github/workflows/cli-integration.yml | 39 ++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cli-integration.yml b/.github/workflows/cli-integration.yml index 57fb3b7c8..5ac41b627 100644 --- a/.github/workflows/cli-integration.yml +++ b/.github/workflows/cli-integration.yml @@ -78,9 +78,42 @@ jobs: if: matrix.dandi-version == 'release' run: pip install "dandi[test]" - - name: Install dev dandi - if: matrix.dandi-version == 'master' - run: pip install "dandi[test] @ git+https://github.com/dandi/dandi-cli" + - name: Extract CLI PR reference from description + if: matrix.dandi-version == 'master' && github.event_name == 'pull_request' + id: cli-pr + run: | + if echo "${{ github.event.pull_request.body }}" | grep -i "Requires_CLI_PR:"; then + CLI_PR_URL=$(echo "${{ github.event.pull_request.body }}" | grep -i "Requires_CLI_PR:" | sed 's/.*Requires_CLI_PR:[[:space:]]*//' | head -1) + echo "CLI PR URL found: $CLI_PR_URL" + # Extract PR number from URL (e.g., https://github.com/dandi/dandi-cli/pull/123) + CLI_PR_NUM=$(echo "$CLI_PR_URL" | grep -o '[0-9]\+$') + echo "cli_pr_num=$CLI_PR_NUM" >> $GITHUB_OUTPUT + else + echo "No CLI PR reference found in description" + echo "cli_pr_num=" >> $GITHUB_OUTPUT + fi + + - name: Get CLI PR branch name + if: matrix.dandi-version == 'master' && steps.cli-pr.outputs.cli_pr_num != '' + id: cli-branch + run: | + CLI_PR_NUM="${{ steps.cli-pr.outputs.cli_pr_num }}" + echo "Getting branch name for CLI PR #$CLI_PR_NUM" + BRANCH_NAME=$(curl -s "https://api.github.com/repos/dandi/dandi-cli/pulls/$CLI_PR_NUM" | jq -r '.head.ref') + echo "Branch name: $BRANCH_NAME" + echo "branch_name=$BRANCH_NAME" >> $GITHUB_OUTPUT + + - name: Install dev dandi from specific PR branch + if: matrix.dandi-version == 'master' && steps.cli-branch.outputs.branch_name != '' && steps.cli-branch.outputs.branch_name != 'null' + run: | + echo "Installing dandi from PR branch: ${{ steps.cli-branch.outputs.branch_name }}" + pip install "dandi[test] @ git+https://github.com/dandi/dandi-cli@${{ steps.cli-branch.outputs.branch_name }}" + + - name: Install dev dandi from master + if: matrix.dandi-version == 'master' && (steps.cli-branch.outputs.branch_name == '' || steps.cli-branch.outputs.branch_name == 'null') + run: | + echo "Installing dandi from master branch" + pip install "dandi[test] @ git+https://github.com/dandi/dandi-cli" - name: Run dandi-api tests in dandi-cli run: | From d21e1e251492c2dbc70f9bebdea53f04d271cd14 Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 28 May 2025 14:15:24 -0500 Subject: [PATCH 8/9] fixup: parse CLI PR --- .github/workflows/cli-integration.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cli-integration.yml b/.github/workflows/cli-integration.yml index 5ac41b627..cd07791f4 100644 --- a/.github/workflows/cli-integration.yml +++ b/.github/workflows/cli-integration.yml @@ -83,10 +83,11 @@ jobs: id: cli-pr run: | if echo "${{ github.event.pull_request.body }}" | grep -i "Requires_CLI_PR:"; then - CLI_PR_URL=$(echo "${{ github.event.pull_request.body }}" | grep -i "Requires_CLI_PR:" | sed 's/.*Requires_CLI_PR:[[:space:]]*//' | head -1) + CLI_PR_URL=$(echo "${{ github.event.pull_request.body }}" | grep -i "Requires_CLI_PR:" | sed 's/.*Requires_CLI_PR:[[:space:]]*//' | head -1 | tr -d '\r\n') echo "CLI PR URL found: $CLI_PR_URL" # Extract PR number from URL (e.g., https://github.com/dandi/dandi-cli/pull/123) - CLI_PR_NUM=$(echo "$CLI_PR_URL" | grep -o '[0-9]\+$') + CLI_PR_NUM=$(echo "$CLI_PR_URL" | grep -o '[0-9]\+' | tail -1) + echo "CLI PR number: $CLI_PR_NUM" echo "cli_pr_num=$CLI_PR_NUM" >> $GITHUB_OUTPUT else echo "No CLI PR reference found in description" From d52b9275e86f005083585f17a9e8b40a69f839aa Mon Sep 17 00:00:00 2001 From: Austin Macdonald Date: Wed, 28 May 2025 16:19:25 -0500 Subject: [PATCH 9/9] fixup precommit --- dandiapi/api/management/commands/createsuperuser.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/dandiapi/api/management/commands/createsuperuser.py b/dandiapi/api/management/commands/createsuperuser.py index 792224520..94dc37fcc 100644 --- a/dandiapi/api/management/commands/createsuperuser.py +++ b/dandiapi/api/management/commands/createsuperuser.py @@ -12,13 +12,12 @@ from composed_configuration._allauth_support.createsuperuser import EmailAsUsernameProxyUser -def create_usermetadata(instance: EmailAsUsernameProxyUser, created: bool, **kwargs): +def create_usermetadata(instance: EmailAsUsernameProxyUser, created: bool, **kwargs): # noqa: FBT001 if created and not hasattr(instance, '_usermetadata_created'): UserMetadata.objects.get_or_create( - user=instance, - defaults={'status': UserMetadata.Status.APPROVED} + user=instance, defaults={'status': UserMetadata.Status.APPROVED} ) - instance._usermetadata_created = True + instance._usermetadata_created = True # noqa: SLF001 class Command(createsuperuser.Command): @@ -34,7 +33,7 @@ def handle(self, *args, **kwargs) -> str | None: # Set first_name and last_name from environment variables if provided first_name = os.environ.get('DJANGO_SUPERUSER_FIRST_NAME') last_name = os.environ.get('DJANGO_SUPERUSER_LAST_NAME') - + if first_name or last_name: # Find the user that was just created email = kwargs.get('email') or os.environ.get('DJANGO_SUPERUSER_EMAIL')