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

#7176 add testing for null host DV on DS Create #7180

Closed
wants to merge 2 commits into from

Conversation

sekmiller
Copy link
Contributor

What this PR does / why we need it:
For recent button updates we moved some of the render logic to the DatasetPage bean. This logic assumes that the dataset has an owner (host dataverse). if on create the user decides to null out the host dataverse this logic breaks and the page fails to render. This PR updates the render logic to guard against a null host dataverse.

Which issue(s) this PR closes:

Closes #7176 Create Dataset Validation fails etc.

Special notes for your reviewer:

Suggestions on how to test this:

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?:
No

Additional documentation:
None

@sekmiller sekmiller added this to the Dataverse 5 milestone Aug 11, 2020
@coveralls
Copy link

coveralls commented Aug 11, 2020

Coverage Status

Coverage decreased (-0.002%) to 19.559% when pulling f1a4a42 on 7176-create-ds-validation into 90c675d on develop.

Copy link
Contributor

@scolapasta scolapasta left a comment

Choose a reason for hiding this comment

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

I get the goal of this PR. I just don't love null checks for things that should never be null (it can hide real issues). I wonder if a better solution would be to disconnect the ui field from the dataset directly, and to set the owner from the field on save (additionally there can be validation to make sure a host dataverse is selected). @sekmiller what do you think? (one concern is that we'll never clean this up later)

@sekmiller
Copy link
Contributor Author

OK. I'll try to handle it that way.

@sekmiller
Copy link
Contributor Author

Abandoning in favor of another approach

@sekmiller sekmiller closed this Aug 12, 2020
@sekmiller sekmiller deleted the 7176-create-ds-validation branch August 12, 2020 19:57
@sekmiller sekmiller removed this from the Dataverse 5 milestone Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Dataset: Form validation not working on save, throws stack trace, gray screen
3 participants