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

Attempting to save a product without a required field should identify the field that failed #114

Closed
spencern opened this issue Sep 29, 2017 · 6 comments

Comments

@spencern
Copy link
Contributor

When trying to save a product that doesn't have a title, I receive a toast alert letting me know that the title is required, but the field does not get highlighted, nor does the panel open to the product edit panel when I try to save a product without a title.

The UI should identify the field that failed and open the action view panel to the correct section as it does when attempting to save an invalid field in a variant.

image

Server error is caused as well. We should attempt to catch this invalid product before sending it to the server.

Exception while invoking method 'revisions/publish' Error: Product Title is required
    at getErrorObject (packages/aldeed_collection2-core.js:480:15)
    at [object Object].doValidate (packages/aldeed_collection2-core.js:462:13)
    at [object Object].Mongo.Collection.(anonymous function) (packages/aldeed_collection2-core.js:214:25)
    at [object Object].Mongo.Collection.(anonymous function) [as update] (packages/dispatch_run-as-user.js:325:19)
    at [object Object].revisionsPublish (imports/plugins/core/revisions/server/methods.js:119:32)
    at packages/check.js:128:16
    at [object Object].EVp.withValue (packages/meteor.js:1134:15)
    at Object.exports.Match._failIfArgumentsAreNotAllChecked (packages/check.js:127:41)
    at maybeAuditArgumentChecks (packages/ddp-server/livedata_server.js:1765:18)
    at packages/ddp-server/livedata_server.js:719:19
    at [object Object].EVp.withValue (packages/meteor.js:1134:15)
    at packages/ddp-server/livedata_server.js:717:46
    at [object Object].EVp.withValue (packages/meteor.js:1134:15)
    at packages/ddp-server/livedata_server.js:715:46
    at [object Object]._.extend.protocol_handlers.method (packages/ddp-server/livedata_server.js:689:23)
    at packages/ddp-server/livedata_server.js:559:43
Sanitized and reported to the client as: Product Title is required [400]
@aaronjudd
Copy link

And open and focus on the first error input...

@brent-hoover brent-hoover changed the title Attempting to save a product without a title should identify the field that failed Attempting to save a product without a required field should identify the field that failed Oct 11, 2017
@brent-hoover
Copy link
Collaborator

brent-hoover commented Oct 11, 2017

So we've written about ten issues for this same problem now and can't seem to get this solved. So I think the requirements here are:

If a new product fail validation from Simple Schema it should:

  1. Stop the attempt to publish so that we don't get a server error
  2. Open the appropriate panel for the field that failed validation
  3. Highlight the field(s) that are/is failing validation
  4. Focus on the first failing field
  5. Show a validation error message e.g. "Title is Required"

Things it should not do

  1. Do a specific validation for a field in code. (e.g. if (!contains("label")) Validation should come from Simple Schema so that it's extensible.

@rymorgan
Copy link
Contributor

@zenweasel That UX feels right. Thanks for detailing that.

@spencern
Copy link
Contributor Author

I'd like to add that under no (normal) circumstance should saving a Product cause the server to throw an error because of invalid schema.

Exception while invoking method 'revisions/publish' Error: Product Title is required
    at getErrorObject (packages/aldeed_collection2-core.js:480:15)
    at [object Object].doValidate (packages/aldeed_collection2-core.js:462:13)
    at [object Object].Mongo.Collection.(anonymous function) (packages/aldeed_collection2-core.js:214:25)
    at [object Object].Mongo.Collection.(anonymous function) [as update] (packages/dispatch_run-as-user.js:325:19)
    at [object Object].revisionsPublish (imports/plugins/core/revisions/server/methods.js:119:32)
    at packages/check.js:128:16
    at [object Object].EVp.withValue (packages/meteor.js:1134:15)
    at Object.exports.Match._failIfArgumentsAreNotAllChecked (packages/check.js:127:41)
    at maybeAuditArgumentChecks (packages/ddp-server/livedata_server.js:1765:18)
    at packages/ddp-server/livedata_server.js:719:19
    at [object Object].EVp.withValue (packages/meteor.js:1134:15)
    at packages/ddp-server/livedata_server.js:717:46
    at [object Object].EVp.withValue (packages/meteor.js:1134:15)
    at packages/ddp-server/livedata_server.js:715:46
    at [object Object]._.extend.protocol_handlers.method (packages/ddp-server/livedata_server.js:689:23)
    at packages/ddp-server/livedata_server.js:559:43
Sanitized and reported to the client as: Product Title is required [400]

I'm still seeing this error when attempting to publish a product without a title. We should be validating on the client before sending to the server and not relying on the server to respond with an error before informing the user that the fields are invalid.

@brent-hoover
Copy link
Collaborator

Right, I think I covered that in point one, but it stands to be said again. This is webdev 101, validate on the client before validating on the server. If we are still getting server errors, this ticket is not complete.

@aldeed aldeed transferred this issue from reactioncommerce/reaction Nov 27, 2019
@aldeed
Copy link
Contributor

aldeed commented May 1, 2020

Referenced UI is gone

@aldeed aldeed closed this as completed May 1, 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

No branches or pull requests

5 participants