Skip to content

Make training an optional field#2379

Merged
tompollard merged 3 commits intodevfrom
cof/modifies-requirements-credentialed-access
Apr 15, 2025
Merged

Make training an optional field#2379
tompollard merged 3 commits intodevfrom
cof/modifies-requirements-credentialed-access

Conversation

@Chrystinne
Copy link
Collaborator

This PR allows the waiving of training submissions for credentialed access type. This modification has been implemented to support the release of the Bridge2AI project.

The PR modifies the handling of required trainings for credentialed access policy in the AccessMetadataForm.

The empty_field option doesn't work for the current AccessMetadataForm as it triggers the error "" is not a valid value (see image below):

Issue_PR

To fix that issue, we needed to add a custom CustomModelMultipleChoiceField to allow 'No training required' as a valid option to be selected by the authors when no training is required.

All the following tests passed:

  1. Creation and publication of a new project P1 with credentialed access type with no training required, and testing the access with different types of users:

    • User 1 with credentialing = False and training = False
    • User 2 with credentialing = True and training = True
    • User 3 with credentialing = True and training = False
    • User 4 with credentialing = False and training = True
  2. Creation and publication of a new project P2 with credentialed access type with CITI training required, and testing the access with different types of users:

    • User 1 with credentialing = False and training = False
    • User 2 with credentialing = True and training = True
    • User 3 with credentialing = True and training = False
    • User 4 with credentialing = False and training = True
  3. Creation and publication of a new version of project P1 keeping the same information as the previous version, and testing the access with different types of users:

    • User 2 with credentialing = True and training = True
    • User 3 with credentialing = False and training = False
  4. Creation and publication of a new version of project P2 keeping the same information as the previous version, and testing the access with different types of users:

    • User 2 with credentialing = True and training = True
    • User 3 with credentialing = False and training = False
  5. Creation and publication of a new version of project P1 changing the information of the previous version (i.e., required training information), and testing the access with different types of users:

    • User 2 with credentialing = True and training = True
    • User 3 with credentialing = False and training = False
  6. Creation and publication of a new version of project P2 changing the information of the previous version (i.e., required training information), and testing the access with different types of users:

    • User 2 with credentialing = True and training = True
    • User 3 with credentialing = False and training = False

@Chrystinne Chrystinne force-pushed the cof/modifies-requirements-credentialed-access branch from 45c5a27 to 8484ba3 Compare April 9, 2025 01:17
@tompollard tompollard changed the title Modify the required trainings field for credentialed access policy Make training an optional field Apr 9, 2025
@Chrystinne Chrystinne added the bridge2ai_voice Requested by the NIH Bridge2AI Voice project label Apr 9, 2025
)

if self.access_policy not in {AccessPolicy.CREDENTIALED, AccessPolicy.CONTRIBUTOR_REVIEW}:
if self.access_policy in {AccessPolicy.OPEN, AccessPolicy.RESTRICTED}:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here could be clearer. It's unclear how the current set of statements are organised. My preference would be a set of if else statements, working through each access policy, e.g.

if open: do x
elif restricted: do y
etc

…ld when no training has been selected for the project.
@Chrystinne
Copy link
Collaborator Author

@tompollard, I've modified the code to preselect the "No training required" option when the field is empty or was previously set to no training.

Now it appears like this when editing a project that does not require training:

Screen Shot 2025-04-09 at 3 46 42 PM

@tompollard
Copy link
Member

Looks good to me @Chrystinne. One minor thing is that the published project includes an empty "Training required:" section.

Screenshot 2025-04-11 at 4 46 17 PM

This section should either:

  1. Not be displayed if there are no training requirements, or
  2. It should say something like "None required" or "No training required".

@Chrystinne
Copy link
Collaborator Author

@tompollard Thanks, Tom. I've updated this section to display "No training required" if there are no training requirements:

Screen Shot 2025-04-14 at 4 26 02 PM

@tompollard tompollard self-requested a review April 14, 2025 21:53
Copy link
Member

@tompollard tompollard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Chrystinne, looks good to me!

@tompollard tompollard added this pull request to the merge queue Apr 15, 2025
Merged via the queue into dev with commit fb08026 Apr 15, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bridge2ai_voice Requested by the NIH Bridge2AI Voice project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants