Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow disabling data use terms #3468

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

feat: Allow disabling data use terms #3468

wants to merge 13 commits into from

Conversation

fhennig
Copy link
Contributor

@fhennig fhennig commented Dec 18, 2024

resolves #3127

preview URL: https://disable-dut.loculus.org

Summary

  • Adds a new config value dataUseTermsEnabled (Potentially BREAKING the way it works now, maybe should just default to true in the Chart? - https://loculus.slack.com/archives/C05G172HL6L/p1738166112981629)
  • DUT stuff gone on the submit dialog
  • DUT note gone on the API docs page
  • TODO: On the 'View' page for my groups sequences, hide the 'Edit data use terms' button (for bulk editing)
  • TODO(maybe): on the individual sequence page, disable the button for editing data use terms?

Testing

  • Frontend: Added a test that submission is possible when DUT is disabled
  • TODO Backend: Test that the submit endpoint can be called without the DUT type now (if disabled) and test that an error is raised if it is omitted but is required.
  • TODO Backend: Add a test that the DUT related things are not returned by get-released-data

Screenshot

The submission dialog without the DUT elements:
image

The API docs don't mention the DUT anmore:
image

PR Checklist

  • All necessary documentation has been adapted.
  • The implemented feature is covered by an appropriate test.

@fhennig
Copy link
Contributor Author

fhennig commented Dec 18, 2024

As was discussed earlier, here are the code places marked where I think changes will be introduced to implement this feature.

@corneliusroemer you wanted to have a look first. Putting this on hold till then.

cc @chaoran-chen

@theosanderson
Copy link
Member

Would the backend not need any changes?

@fhennig
Copy link
Contributor Author

fhennig commented Dec 18, 2024

That wasn't in the ticket I think. But maybe that was a mistake? I remember that we talked about that too actually. I'd be curious to look into how to do that

@theosanderson
Copy link
Member

My guess (maybe wrong) of what would happen if you made the changes annotated would be:

  • backend would still expect data use terms on submission so would give an error
  • backend would still emit data use terms in get-released-data so SILO would get confused at the unexpected field

@chaoran-chen
Copy link
Member

We need to do some changes so that (1) the backend accepts submissions that don't provide a data use term and (2) don't expose it in get-released-data. But if it make things easier, it could write "open" into the database.

@fhennig
Copy link
Contributor Author

fhennig commented Dec 18, 2024

Ah, makes sense

@fhennig
Copy link
Contributor Author

fhennig commented Dec 18, 2024

that should cover it.

@fhennig
Copy link
Contributor Author

fhennig commented Jan 29, 2025

I tried quite some time to get the "Submit" button to move to the end of the page, but I couldn't do it without making changes to the BaseLayout and so I'll just leave it for now. If you think it's important then I'll spend some more time on it, but maybe we can also leave it.

image

@theosanderson
Copy link
Member

(I actually don't like huge gaps between the submit button and everything else!)

@fhennig
Copy link
Contributor Author

fhennig commented Jan 29, 2025

perfect! 😄

@fhennig fhennig added the preview Triggers a deployment to argocd label Jan 29, 2025
@fhennig fhennig self-assigned this Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Triggers a deployment to argocd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable data use terms
3 participants