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

8832 toa validation error on ds create #8834

Merged
merged 11 commits into from
Jul 25, 2022

Conversation

sekmiller
Copy link
Contributor

@sekmiller sekmiller commented Jul 7, 2022

What this PR does / why we need it: Fixes the Terms of Access validator to notice when there are no restricted files in the Dataset version

Which issue(s) this PR closes:

Closes #8832 Problem with license selection and Terms of Access/Enable access request button
Closes #8847

Special notes for your reviewer:

Suggestions on how to test this: Make sure you can update the terms of use/license in a dataset that doesn't have restricted files. also verify that adding/editing templates Terms still works.
Also there is a minor ui change - see 8847 where terms of access won't be shown on the terms display tab if there are no restricted files.

Does this PR introduce a user interface change? If mockups are available, please link/include them here: no

Is there a release notes update needed for this change?: none

Additional documentation:none

@scolapasta scolapasta self-assigned this Jul 11, 2022
@coveralls
Copy link

coveralls commented Jul 19, 2022

Coverage Status

Coverage remained the same at 19.756% when pulling 8074358 on 8832-TOA-validation-error-on-ds-create into ba0183d on develop.

try{
termsofAccessHidden = document.getElementById("datasetForm:tabView:termsofAccessHiddenLT").value;
fileAccessRequestHidden = document.getElementById("datasetForm:tabView:fileAccessRequestHiddenLT").value;
}
catch (error){
alert("notdatasetform");
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems like it should not be here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, thanks

@@ -422,6 +422,14 @@ public Boolean isHasRestrictedFiles(){
}
}

public boolean getHasRestrictedFiles(){
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think you need this - being a boolean it should call the isHasRastrictedFiles method. But see my other note for if we even need the ui:hidden field.

@scolapasta scolapasta removed their assignment Jul 21, 2022
@kcondon kcondon self-assigned this Jul 22, 2022
@kcondon
Copy link
Contributor

kcondon commented Jul 25, 2022

It's now working with the minor exception of the string showing 0 restricted files once there are terms of access but no restricted files. The terms of access in this case have not been rendered by the file count remains.

From the user who reported it:
lmaylein commented 4 days ago
Would be nice if it could not come to the case that it says:
There are 0 restricted files in this dataset.
Terms of Access for Restricted Files: ...

@kcondon kcondon merged commit 75d2900 into develop Jul 25, 2022
@kcondon kcondon deleted the 8832-TOA-validation-error-on-ds-create branch July 25, 2022 23:38
@pdurbin pdurbin added this to the 5.12 milestone Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants