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

Validate microschema lists, Node update error handling #202

Merged

Conversation

literalplus
Copy link

Ellipsis

This PR addresses some of the validation issues @xVinci6 talked about on Gitter last week. Specifically, it stops DataService from swallowing errors on product updates. Further, it adds active validation to microschema lists. So if there is a validation error in a microschema list, the invalid node can no longer be sent now, stopping the Bad Request errors from happening in the first place.

Detailed issue description

  1. Node update errors are swallowed in DataService (no return in catch -> mapped to resolved promise with undefined as parameter)
  2. For the "Save & Publish" button, it seems like it should be possible to publish an existing draft even if the currently entered data is invalid. (caption changes to "Publish") However, this also enables the button if there is no draft to publish. When clicking it, actually, the current version is saved before publishing. Because of (1), the result of the save call is ignored and the changes are lost. The success handler gets undefined as argument and fails badly, which is also why the buttons stay hidden.
  3. For lists of microschemas, the validity is not validated. Hence, forms are still seen as valid, and the user is able to submit invalid data to get a backend error message.

Solution

  1. Rethrow the error, so that the resulting promise is correctly rejected, with the error.
  2. Disable the "Publish" button if there is no saved draft and the current form is invalid. Also, don't try to save invalid forms when publishing.
  3. Validate lists of microschemas in the formIsValid() method.

Testing

There were no existing Integration Tests for this, so I did not add any new ones. I have verified that given issues are resolved locally.

If necessary for the workflow, I can split issues 1-3 into separate PRs and create issues.

Also, don't try to save when the form is invalid and 'Publish' is
pressed, and don't allow 'Save & Publish' if the form is invalid.
Swallowing errors in promises means that consumers will receive
undefined as argument to their success handler (instead of the
error in their error handler)
@philippguertler
Copy link
Contributor

Thanks again for your effort.

If necessary for the workflow, I can split issues 1-3 into separate PRs and create issues.
This is not necessary. For small changes like these, PRs like this are perfectly fine. You described which problem the PR solves, which is enough.

The code changes look good to me. The change will be available in the next version of Mesh.

@philippguertler philippguertler merged commit 9ebc52a into gentics:beta Apr 15, 2019
@Jotschi
Copy link
Contributor

Jotschi commented May 7, 2019

Released with 0.31.5

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.

3 participants