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
merged 2 commits into from Apr 15, 2019

Conversation

Projects
None yet
3 participants
@xxyy
Copy link

commented Apr 12, 2019

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.

xxyy added some commits Apr 11, 2019

Validate microschema lists in editor pane
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.
Fix no error messages being shown on node save
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

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

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

This comment has been minimized.

Copy link
Member

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
You can’t perform that action at this time.