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

fix(website): don't display data as "restricted" when the last restricted data use terms expired #2914

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

fengelniederhammer
Copy link
Contributor

@fengelniederhammer fengelniederhammer commented Oct 1, 2024

preview URL: http://fixdatausetermsdisplay.loculus.org

Bug

Website seq details showed restricted even after expiration of restriction.

See https://loculus.slack.com/archives/C05G172HL6L/p1727769592602569

Summary

Root cause was misunderstanding in website code of what the dataUseTermsHistory contains.

The website code assumed there would be a new entry when something becomes open, but in fact the history is just a log of changes.

The backend only returns the history - which does not include "OPEN" as a last entry when the restricted period expired. LAPIS has the correct values - let's simply rely on that.

This assumes the following as correct:

  • The backend just stores the history (not the state) of data use terms.
  • getting the history from the backend for a sequence entry that has expired restricted use terms will still only show "restricted" entries

We now simply take the use terms from LAPIS, that means the calculation is done in a single place for search and display.

Screenshot

PR Checklist

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

@fengelniederhammer fengelniederhammer changed the title fix(website): don't display data as "restricted" when the last restricted data user terms expired fix(website): don't display data as "restricted" when the last restricted data use terms expired Oct 1, 2024
@corneliusroemer corneliusroemer added preview Triggers a deployment to argocd website Tasks related to the web application bug Something isn't working labels Oct 1, 2024
…cted data use terms expired

The backend only returns the history - which does not include "OPEN" as a last entry when the restricted period expired.
LAPIS has the correct values - let's simply rely on that.
@corneliusroemer corneliusroemer added the format_me Triggers github_actions to format website code on PR label Oct 1, 2024
@corneliusroemer
Copy link
Contributor

Testing this here:

@corneliusroemer
Copy link
Contributor

Ok test works! 🎉
Brave Browser 2024-10-01 13 46 37

I'll add the line to check data use terms are string so as not to produce wrong data.

@fengelniederhammer fengelniederhammer merged commit 132d82c into main Oct 1, 2024
20 checks passed
@fengelniederhammer fengelniederhammer deleted the fixDataUseTermsDisplay branch October 1, 2024 11:48
}

const isRestricted = currentDataUseTerms.type === 'RESTRICTED';
const dataUseTerms = tableData.find((entry) => entry.name === DATA_USE_TERMS_FIELD);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const dataUseTerms = tableData.find((entry) => entry.name === DATA_USE_TERMS_FIELD);
const dataUseTerms = tableData.find((entry) => entry.name === DATA_USE_TERMS_FIELD);
// Calculation below is based on the assumption that dataUseTerms is a string
// Hence assert to get loud error instead of quiet false data
if (typeof dataUseTerms?.value !== 'string') {
throw new Error('Code assumes dataUseTerms are of type string. If it isn't, then need to change the calculation of isRestricted.');
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry - already merged. If you still think it's necessary, you can open a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working format_me Triggers github_actions to format website code on PR preview Triggers a deployment to argocd website Tasks related to the web application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants