Meteor-Community-Packages / meteor-autoform

AutoForm is a Meteor package that adds UI components and helpers to easily create basic forms with automatic insert and update events, and automatic reactive validation.
MIT License
1.44k stars 328 forks source link

[5.0] Insert form carries over doc from same form previously rendered with type=update #751

Closed fungilation closed 9 years ago

fungilation commented 9 years ago

From the README:

Can I reuse the same quickForm or autoForm for both inserts and updates?

Yes. Your code that flips between states should do the following in this order:

Change the type attribute's value to "insert" or "update" as appropriate, probably by updating a reactive variable. Change the doc attribute's value to the correct document for an update or to null (or a document containing default values) for an insert, probably by updating a reactive variable. Call AutoForm.resetForm(formId). This will clear any existing validation errors for the form.

Before 5.0, I've been setting doc=null in my {{#autoform}} block when form is type=insert. Fields are correctly empty/reset. After updating to 5.0, form fields get populated with doc that was last rendered when the same form was type=update. I've worked around this with supplying a doc object with default empty values instead of doc=null. But the bug still is setting doc=null on insert form isn't behaving as expected or documented.

delgermurun commented 9 years ago

https://github.com/delgermurun/autoform-error/tree/error2 (on error2 branch) here is reproduction repo.

aldeed commented 9 years ago

OK, probably a side effect of the fixes to hot code reload state. I will investigate.

aldeed commented 9 years ago

Strange but the values don't seem to be coming from code reload cache. Will have to look into it more tomorrow unless anyone else can figure it out and submit a PR.

lucascurti commented 9 years ago

Having the same problem here. Any workaround until this is fixed?

lucascurti commented 9 years ago

In case someone needs a workaround, aldeed:template-extension can be added and then:

Template.autoForm.hooks({
    rendered: function () {
        if (this.data.type == 'insert') {
            AutoForm.resetForm(this.data.id);

            //Below is only for selectize elements that for some reason 
            //doesn't reset with the above restForm command.
            var $select = $('select').selectize();
            for (var i = 0; i < $select.length; i++) {
                $select[i].selectize.clear();
            }
        }
    }
});
lucascurti commented 9 years ago

@aldeed, Is there anything I can do to help you troubleshoot this one? Turns out that I have some datetimepicker elements that won't clear with the above workaround and doing it manually is a bit of a pain. So, I thought it maybe better if I spend that time troubleshooting the main issue. The thing is that I'm kind of a meteor's newbie and not sure where I should start looking. If you think I can help in any way and you can point me on the right direction, I will be more than happy to help.

lucascurti commented 9 years ago

Another bug that most likely is related to this one. Taking the @delgermurun repo sample:

  1. Add a Book
  2. Go to the form to add another book but this time leave the Name field empty to trigger the validation error "Name is required"
  3. Don't do anything and just to back to the Books list.
  4. Click on Edit for the book added on step 1.
  5. Validation error "Name is required" is there when it shouldn't be.
lucascurti commented 9 years ago

I updated to AutoForm 5.0.3 and now, when the insert form carries the values from the update form, I'm getting the following error on the console: Uncaught Error: No form with ID FormAsistenciaTaller is currently rendered. If this call originated from within a template event or helper, you should call without passing a formId to avoid seeing this errorAutoForm.viewForForm @ autoform-api.js:900AutoForm.templateInstanceForForm @ autoform-api.js:883(anonymous function) @ afFieldInput.js:72

aldeed commented 9 years ago

@lucatros that error is reported in #789

lucascurti commented 9 years ago

Ok. I think it's worth mentioning that I'm getting this error only when the values on the insert form are carried from the update form.

bshamblen commented 9 years ago

@aldeed I think I found the source of the old data that's being used to populate the form in this use case, but I'm afraid fixing it is beyond my depth of knowledge of how all the libraries work together. I've included the relevant code from the existing source. Hopefully this will help come up with a solution.

meteor-autoform/components/autoForm/autoForm.js In the else condition below null is passed into the sourceDoc function. My assumption is that this should delete the old version of the doc, since a null doc value was passed into the form.

var mDoc;
if (doc && !_.isEmpty(doc)) {
  var hookCtx = {formId: formId};
  // Pass doc through docToForm hooks
  _.each(Hooks.getHooks(formId, 'docToForm'), function autoFormEachDocToForm(hook) {
    doc = hook.call(hookCtx, doc, data._resolvedSchema);
    if (!doc) {
      throw new Error('Oops! Did you forget to return the modified document from your docToForm hook for the ' + formId + ' form?');
    }
  });
  // Create a "flat doc" that can be used to easily get values for corresponding
  // form fields.
  mDoc = new MongoObject(doc);
  AutoForm.reactiveFormData.sourceDoc(formId, mDoc);
} else {
  AutoForm.reactiveFormData.sourceDoc(formId, null); 
}

meteor-autoform/autoform-formdata.js The sourceDoc parameter below is null, which should probably clear the last document that was cached, but it just returns the last version.

FormData.prototype.sourceDoc = function (formId, sourceDoc) {
  var self = this;
  self.initForm(formId);

  if (sourceDoc) {
    //setter
    self.forms[formId].sourceDoc = sourceDoc;
    self.forms[formId].deps.sourceDoc.changed();
  } else {
     //getter
    self.forms[formId].deps.sourceDoc.depend();
    return self.forms[formId].sourceDoc;
  }
};

meteor-autoform/components/afFieldInput/afFieldInput.js Since the sourceDoc function below returns the old version of the doc, the old values are used to populate the form.

var mDoc = AutoForm.reactiveFormData.sourceDoc(formId); 
fpaboim commented 9 years ago

Same problem here.. +1 on getting this fixed

jaumeMR commented 9 years ago

Same problem here! I found an easy workaround: assign doc to a helper that returns an object with some default field value in insert form. Returning an empty object or null does'n work, you need to return some field. Example: jade:

    +autoForm collection="Movements" id="manageMovement" type="insert" doc=empty

coffee:

    Template.movementsManage.helpers
        empty: ->
            state: 'waiting'

EDIT: this is basically the same as proposed in the first comment by @fungilation. I did not see that in his comment also posed a workaround.

fungilation commented 9 years ago

That's what I said initially ;)

I've worked around this with supplying a doc object with default empty values instead of doc=null

jaumeMR commented 9 years ago

Ouch!! I read too fast and did not see the end of your comment. Sorry!

fungilation commented 9 years ago

Haha no problem. Good you elaborated with some sample code On Fri, May 22, 2015 at 9:39 PM jaumeMR notifications@github.com wrote:

Ouch!! I read too fast and did not see the end of your comment. Sorry!

— Reply to this email directly or view it on GitHub https://github.com/aldeed/meteor-autoform/issues/751#issuecomment-104840957 .

aldeed commented 9 years ago

This issue should be fixed as of last release (https://github.com/aldeed/meteor-autoform/commit/868ca08f46e4d7fe4059622dabc3bc8444fa7e58). Can anyone confirm?

fungilation commented 9 years ago

Yup, confirm fixed. Thanks

rizkysyazuli commented 9 years ago

a bit OOT, my insert/update form is a bootstrap modal dialog. calling AutoForm.resetForm(formID) on the template's onRendered() doesn't work since the modal is already there from the start. and using the modal's built in event shown.bs.modal somehow not working as well.

any other workaround or hooks i can use to fix this?