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

Delete empty objects before validate #123

Closed
sakulstra opened this issue Nov 1, 2016 · 9 comments
Closed

Delete empty objects before validate #123

sakulstra opened this issue Nov 1, 2016 · 9 comments
Assignees
Labels
Type: Feature New features and feature requests

Comments

@sakulstra
Copy link

Hello there,
thanks for this nice library ;)

I'm currently facing a bug with conditional form inputs:

courseType: {
    type: String,
    allowedValues: ['songCourse', 'learningPath'],
  },
  songCourse: {
    type: SongCourse,
    optional: true,
    custom() {
      const field = this.field('courseType');
      const value = 'songCourse';
      if (field && field.isSet) {
        if (field.value === value) {
          // inserts
          if (!this.operator) {
            if (!this.isSet || this.value === null || this.value === '') return 'required';
          }
          // updates
          if (this.operator && this.isSet) {
            if ((this.operator === '$set' && this.value === null) || this.value === '') return 'required';
            if (this.operator === '$unset') return 'required';
            if (this.operator === '$rename') return 'required';
          }
        }
      }
    },
  },
  learningPath: {
    type: LearningPath,
    optional: true,
    custom() {
      const field = this.field('courseType');
      const value = 'learningPath';
      if (field && field.isSet) {
        if (field.value === value) {
          // inserts
          if (!this.operator) {
            if (!this.isSet || this.value === null || this.value === '') return 'required';
          }
          // updates
          if (this.operator && this.isSet) {
            if ((this.operator === '$set' && this.value === null) || this.value === '') return 'required';
            if (this.operator === '$unset') return 'required';
            if (this.operator === '$rename') return 'required';
          }
        }
      }
    },
  },

So i basically select a courseType and then display a nested form in regards to the selection using the DisplayIf https://github.com/vazco/uniforms#example-displayif approach.

Problem

The Problem I now face is, that after selecting e.g. 'learningPath' an empty 'learningPath' object will be generated inside the model. When now changing the selection to 'songCourse' - filling out the form and trying to submit the form I will get an error inside the learningPath.xxx fields as simpleschema tries to validate the empty 'learningPath' object, but is of course missing all the props inside it.

Workaround

I currently use a kind of ugly workaround where I basically delete the empty object out of the model inside onValidate and than rerun the validation.

Proposed solution

Eventually one could iterate over the model and remove empty objects before validation?

Regards,
Lukas

@serkandurusoy
Copy link
Contributor

@radekmie the way I see it, either or better yet both of following solutions would help a lot

a) expose an onBeforeValidate option that takes in the model and returns a new model
b) remove keys whose values are empty {} objects from the model

@radekmie radekmie added the Type: Feature New features and feature requests label Nov 1, 2016
@radekmie radekmie self-assigned this Nov 1, 2016
@radekmie
Copy link
Contributor

radekmie commented Nov 1, 2016

@serkandurusoy:

a) It surely won't be enough very soon: onBeforeSubmit is my first guess.
b) No - no altering model on our side. We have special fields and schemas for it.


Validation is one thing, another one is the form submit flow.

Right now, both SimpleSchemaBridge and SimpleSchema2Bridge are cleaning your model, so an autoValue with an emptiness check would be OK for the validation, but not for submission (you can clean it by yourself or your collection/method/anything can do it, but it have to be done somewhere).

As I was reviewing ModifierForm example yesterday, I've encountered quite a similar issue - how the hell can I alter the submitted model, but not the validated one?

Solution? I think, that BaseForm#getModel is our target. My idea is to give it a mode argument. It will be either submit or validate. We'll also need one another for the context (so we could pass a different model to the fields) and probably one more for... Just plain getModel() call.

This will simplify your suggested solution, @serkandurusoy - we could have a new prop (I can't think any possible name right now) for a hook of this method.

Example implementation in BaseForm:

getModel (mode) {
    let model = ...; // As currently implemented

    if (this.props.X) {
        model = this.props.X(model, mode);
    }

    return model;
}

What do you think?

@serkandurusoy
Copy link
Contributor

@radekmie onValidate actually can be used in place of onBeforeSubmit, right?

My first inclination was to actually alter the modifier form example, too! But it proved to be too verbose for something so seemingly trivial.

and yes a preexisting prop - I can't think of a name either :/ - as you suggested can definitely help mitigate this issue.

@radekmie
Copy link
Contributor

radekmie commented Nov 2, 2016

@serkandurusoy no, because onValidate is before onSubmit and it have separate model.

As soon as we come with a prop name, I'll implement it.

@radekmie
Copy link
Contributor

radekmie commented Nov 2, 2016

How about modelTransform? Proposed documentation:

// Model transform.
//   Function transforming one model into another. It's used in few
//   situations (modes) described below. 
modelTransform={(model, mode) => {
    // This model will be passed to the fields.
    if (mode === 'form') {/* ... */}

    // This model will be submitted.
    if (mode === 'submit') {/* ... */}

    // This model will be validated.
    if (mode === 'validate') {/* ... */}

    // Returning undefined is the same as returning unaltered model.
}}

The default (null) mode is internal. With this, we can solve this issue like this:

<AutoForm
    // Other props...
    modelTransform={(model, mode) => {
        if (mode === 'submit' || mode === 'validate') {
            return removeEmptyObjects(model);
        }
    }}
/>

@serkandurusoy
Copy link
Contributor

Ah, I think this is a little too verbose for small problems, but it also
looks quite flexible and powerful.

On Wed, Nov 2, 2016 at 7:06 PM, Radosław Miernik notifications@github.com
wrote:

How about modelTransform? Proposed documentation:

// Model transform.// Function transforming one model into another. It's used in few// situations (modes) described below.
modelTransform={(model, mode) => {
// This model will be passed to the fields.
if (mode === 'form') {/* ... */}

// This model will be submitted.
if (mode === 'submit') {/* ... */}

// This model will be validated.
if (mode === 'validate') {/* ... */}

// Returning undefined is the same as returning unaltered model.

}}

The default (null) mode is internal. With this, we can solve this issue
like this:

<AutoForm
// Other props...
modelTransform={(model, mode) => {
if (mode === 'submit' || mode === 'validate') {
return removeEmptyObjects(model);
}
}}/>


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#123 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEbz3PRKCWOV8oHwa2HaxSLg-aOeSTPtks5q6LT-gaJpZM4KmITI
.

@sakulstra
Copy link
Author

Hello,
any progress on this :)?

I think the modelTransform approach is pretty nice.
I currently face a problem where conditional required validation fails due to some complex dependencies, where again I would have to unset fields to pass validation.

With the approach posted above this would be easily solvable.

Best regards and thanks for your efforts!

@radekmie
Copy link
Contributor

I was quite busy recently (but documentation is complete!). It's on my roadmap - it will be ready later this week.

@radekmie
Copy link
Contributor

Check it out in 1.7.0-beta.1. Documentation.

@Monteth Monteth added this to Closed in Open Source (migrated) Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

No branches or pull requests

3 participants