jagi / meteor-astronomy

Model layer for Meteor
https://atmospherejs.com/jagi/astronomy
MIT License
604 stars 67 forks source link

Document not updating... #357

Closed joetidee closed 8 years ago

joetidee commented 8 years ago

(v1.2.10) I have a form that sets the content of a model and submits it to a Meteor method. The first time the form is submitted, the document saves as expected. If the title in the form, for example, gets changed and the model re-submitted, the document does not get updated wiht the new title.

The Meteor call to the method is as follows:

Meteor.call('/audits/template/save', self.at, function(err, res) {
                if (err) {
                    self.at.catchValidationException(err);
                    if( self.at.hasValidationErrors() ){
                        $(".validation-error-form").html("Technical fault - this stage could not be saved.").show();
                    }
                }
                else{
                    console.log(res);
                    self.at = res; // This sets the model to what was saved.
                }
            });

As you can see the result of the method call is printed to the console. When the model is saved after the first time, the title shows as having been changed and also the _isNew attribute is set to false, which would suggest that what came back from the method was an updated model and therefore the save() function on the server should have updated the document in the database. As is also observed, I am setting the model (on the client) which the result that was returned for the method call - is this the right thing to do?

Below is the method call for reference:

'/audits/template/save': function(at) {

        var now = new Date();
        var userId = Meteor.userId();

        if( at._isNew === true ) {
            var seq = getNextSequence("AuditTemplateRef");
            at.ref = seq;
            at.owners = [userId];
            at.created_date = now;
            at.created_user_id = userId;
        }
        at.updated_date = now;
        at.updated_user_id = userId;
        if ( at.validate(["ref","owners","status","title","desc","instructions","category","created_date","created_user_id","updated_date","updated_user_id"]) ) {
            at.save(["ref","owners","status","title","desc","instructions","category","created_date","created_user_id","updated_date","updated_user_id"]);
            return at;
        }

        // Send errors back to the client.
        at.throwValidationException();
    }
lukejagodzinski commented 8 years ago

You description of the problem is not precise. So is document in the database being updated or not? If you want to reload a document on the client with new version after method call you should do doc.reload(). The code you posted does not tell me anything about your problem. Show exactly where you have a problem.

joetidee commented 8 years ago

The document is being inserted into the database, but when saving again, the document does not update in the database.

In the documentation, the reload() method suggests that it reverts the model back to a previous version. Perhaps this can be clarified.

joetidee commented 8 years ago

To clarify, here is the code to submit the model to the Meteor method:

onSubmit(e) {
        var self = this;
        e.preventDefault();

        // Reset form validation.
        $(".validation-error-form").hide();
        $(".form-group").removeClass("validation-error");
        $(".validation-error-msg").remove();

        // Form validation...
        self.at.title = this.title.value;
        self.at.status = [{
            id: 1,
            name: 'In Progress'
        }];
        self.at.desc = this.desc.value;
        self.at.instructions = this.instructions.value;
        self.at.category = [{
            id: this.category[this.category.selectedIndex].value,
            name: this.category[this.category.selectedIndex].text
        }];
        var val = self.at.validate(["status","title","desc","instructions","category"]);
        if(val){
            Meteor.call('/audits/template/save', self.at, function(err, res) {
                if (err) {
                    self.at.catchValidationException(err);
                    if( self.at.hasValidationErrors() ){
                        $(".validation-error-form").html("Technical fault - this stage could not be saved.").show();
                    }
                }
                else{
                    self.at = res;
                    console.log(res);
                }
            });
        }
        else{
            self.showFormErrors(self.at);
        }
    }

... and here is the Metero method call:

'/audits/template/save': function(at) {

        var now = new Date();
        var userId = Meteor.userId();

        if( at._isNew === true ) {
            var seq = getNextSequence("AuditTemplateRef");
            at.ref = seq;
            at.owners = [userId];
            at.created_date = now;
            at.created_user_id = userId;
        }
        at.updated_date = now;
        at.updated_user_id = userId;
        if ( at.validate(["ref","owners","status","title","desc","instructions","category","created_date","created_user_id","updated_date","updated_user_id"]) ) {
            at.save(["ref","owners","status","title","desc","instructions","category","created_date","created_user_id","updated_date","updated_user_id"]);
            return at;
        }

        // Send errors back to the client.
        at.throwValidationException();
    }
lukejagodzinski commented 8 years ago

The code examples are too complex. When dealing with some problem you should try create the minimal reproduction. Just create simple class and try inserting and later updating it. It should work. You are just doing something wrong.

joetidee commented 8 years ago

ok. Can I ask though, when the Meteor method has run the save() command, how does the client-side model get re-populated with the updated data for the inserted document?

In the example above, when the Metero method is run and the document saved...

if ( at.validate(["ref","owners","status","title","desc","instructions","category","created_date","created_user_id","updated_date","updated_user_id"]) ) {
            at.save(["ref","owners","status","title","desc","instructions","category","created_date","created_user_id","updated_date","updated_user_id"]);
            return at;
        }

...I am sending the server-side model back to the client and setting the client side model with it...

self.at = res;

The documentation does not explain how to re-populate the client-side model, so is this the correct way to do it?

lukejagodzinski commented 8 years ago

The best way is to do it that way:

doc.save(function() {
  doc.reload();
});

However, if you're returning a document from the Meteor method then you have to know that it's not so as assigning a new document to the old one because of broken references. In such situation you can do something like:

doc.save(function(err, res) {
  doc.set(res.get());
});
joetidee commented 8 years ago

I have taken your advice, althought it's not clear where this code should be run. I my case I have tried:

Meteor.call('/audits/template/save', doc, function(err, res) {
                if (err) {
                    // do something...
                }
                else{
                    doc.set(res.get());
                }
            });

where the Meteor.method is:

'/audits/template/save': function(doc) {

        return doc.save(function(err, res) {
            if(err){
                return err;
            }
            return doc;
        });

        // Send errors back to the client.
        doc.throwValidationException();
    }

but i get the error:

Error: res.get is not a function

lukejagodzinski commented 8 years ago

You have to read more about Meteor and how it works before staring working with it. Sorry but I can't help every Meteor developer with problems not related with Astronomy even if I want to. I'm very busy releasing Astronomy 2.0. But this time I will make an exception.

First of all, you don't understand how Meteor method works. The callback function receives two arguments:

Meteor.call('methodName', function(err, res) {});

The res argument is what you return from the Meteor method. And what are you returning in /audits/template/save? Nothing. So you can't expect res to be anything but undefined. And you should learn a lot about debugging your code. When you see the Error: res.get is not a function error then you should check what res.get is and what res is by just printing it to the console.

However, as I told you you don't have to return anything from Meteor method to reload a document to its new state. doc.reload() is enough.

joetidee commented 8 years ago

I do completely understand how Meteor works. I understand that the callback receives two arguments - errand res. I understand that what is returned from the doc.save()function (if successfull) is the _id of the newly inserted document and that calling res.get() could not possibly work. I am only followng your example above where you said use:

doc.save(function(err, res) {
  doc.set(res.get());
});

How can res.get() possibly be correct if res is always the _id of the inserted document?

joetidee commented 8 years ago

Ok, I have made some progress whereby the document in the database does update EVENTUALLY, but it takes two form submissions to work. Below is a print out of the model objects immediately before the save function is called in my Meteor method:

(First form submission - saves to database.)

{ _modifiers: {},
    _errors:
    { _values: {},
      _size: 0,
      _sizeDep: { _dependentsById: {} },
      _entriesDep: { _dependentsById: {} },
      _keysDep: { _dependentsById: {} },
      _valuesDep: { _dependentsById: {} },
      _keysDeps: {},
      _valuesDeps: {}
},
_id: null,
ref: 31,
title: 'test',
desc: '',
instructions: '',
status: [ { id: 1, name: 'In Progress' } ],
owners: [ 'xPftXg6Ru42onGadH' ],
category: [ { id: '1', name: '-' } ],
last_audit_template_history_id: null,
created_date: Tue Apr 26 2016 00:15:45 GMT+0000 (UTC),
created_user_id: 'xPftXg6Ru42onGadH',
updated_date: Tue Apr 26 2016 00:15:45 GMT+0000 (UTC),
updated_user_id: 'xPftXg6Ru42onGadH',
discon_date: null,
discon_user_id: null,
_isNew: true }

(Second form submission with title changed - does NOT update the document in the database.)

{ _modifiers:
    { '$set':
        { _id: 'C4TP3Wq5pzgNNCeqM',
          title: 'test',
          desc: '',
          instructions: '',
          status: [Object],
          owners: [Object],
          category: [Object],
          last_audit_template_history_id: null,
          updated_date: Tue Apr 26 2016 00:15:45 GMT+0000 (UTC),
          updated_user_id: 'xPftXg6Ru42onGadH',
          discon_date: null,
          discon_user_id: null } },
_errors:
    { _values: {},
      _size: 0,
      _sizeDep: { _dependentsById: {} },
      _entriesDep: { _dependentsById: {} },
      _keysDep: { _dependentsById: {} },
      _valuesDep: { _dependentsById: {} },
      _keysDeps: {},
      _valuesDeps: {} },
_id: 'C4TP3Wq5pzgNNCeqM',
ref: 31,
title: 'test2',
desc: '',
instructions: '',
status: [ { id: 1, name: 'In Progress' } ],
owners: [ 'xPftXg6Ru42onGadH' ],
category: [ { id: '1', name: '-' } ],
last_audit_template_history_id: null,
created_date: Tue Apr 26 2016 00:15:45 GMT+0000 (UTC),
created_user_id: 'xPftXg6Ru42onGadH',
updated_date: Tue Apr 26 2016 00:15:58 GMT+0000 (UTC),
updated_user_id: 'xPftXg6Ru42onGadH',
discon_date: null,
discon_user_id: null,
_isNew: false }

(Third form submission - DOES update the document in the database.)

{ _modifiers:
    { '$set':
        { _id: 'C4TP3Wq5pzgNNCeqM',
          title: 'test2',
          desc: '',
          instructions: '',
          status: [Object],
          owners: [Object],
          category: [Object],
          last_audit_template_history_id: null,
          updated_date: Tue Apr 26 2016 00:15:58 GMT+0000 (UTC),
          updated_user_id: 'xPftXg6Ru42onGadH',
          discon_date: null,
          discon_user_id: null } },
_errors:
    { _values: {},
      _size: 0,
      _sizeDep: { _dependentsById: {} },
      _entriesDep: { _dependentsById: {} },
      _keysDep: { _dependentsById: {} },
      _valuesDep: { _dependentsById: {} },
      _keysDeps: {},
      _valuesDeps: {} },
_id: 'C4TP3Wq5pzgNNCeqM',
ref: 31,
title: 'test2',
desc: '',
instructions: '',
status: [ { id: 1, name: 'In Progress' } ],
owners: [ 'xPftXg6Ru42onGadH' ],
category: [ { id: '1', name: '-' } ],
last_audit_template_history_id: null,
created_date: Tue Apr 26 2016 00:15:45 GMT+0000 (UTC),
created_user_id: 'xPftXg6Ru42onGadH',
updated_date: Tue Apr 26 2016 00:16:51 GMT+0000 (UTC),
updated_user_id: 'xPftXg6Ru42onGadH',
discon_date: null,
discon_user_id: null,
_isNew: false }

It does not make sense why the document does not save on the second form submission! (?)

lukejagodzinski commented 8 years ago

First of all in many issues you post you are using two different Astronomy version. Please, decide to use one version. Is it 1.0 or 2.0? I see that in this issue you are using 1.0 so I will talk about this.

If you want to return document you have to do:

Meteor.methods({
  '/audits/template/save': function(doc) {
    if (doc.validate()) {
      doc.save();
      return doc;
    }

    doc.throwValidationException();
  }
});

If you want to follow my examples then you should take a look at this repository https://github.com/jagi/meteor-astronomy-examples

joetidee commented 8 years ago

The start of this issue states which version I am using - there was a mix-up with the syntax used for defining the model, but this has been sorted, so eveything should be in version 1.2.10. As per one of your previous suggestions, I am using the following syntax for returning the document back to the client-side model and this seems to be working ok, so we can draw a line under that now:

Meteor.methods({
  '/audits/template/save': function(doc) {
    if (doc.validate()) {
      doc.save();
      doc.set(doc.get());
      return doc;
    }

    doc.throwValidationException();
  }
});

But, as you can see, the printouts above of the model that is being passed to the save() function is showing that the title is different, but the save function is not saving it the first time it is submitted. I have checked for error values, but these come out as null. So the mystery still remains as to why the document is not being updated.

lukejagodzinski commented 8 years ago

You've modified my example and later you're surprised that it does not work. There is a difference between method call and method definition.

// Method definition.
Meteor.methods({
  '/audits/template/save': function(doc) {
    if (doc.validate()) {
      doc.save();
      return doc;
    }

    doc.throwValidationException();
  }
});
// Method call.
Meteor.call('/audits/template/save', doc, function(err, doc) {
  doc.set(doc.get());
});

And maybe it will solve the problem. I don't know. You are doing a lot of mistakes and I don't see all your code so it's hard to predict where error is and I don't have time to help every developer.

joetidee commented 8 years ago

What I was pointing out was that the code that you supplied does not work. If I use your code, the second submission creates an entirely new document in the database. The code I supplied at least ensures that the document eventually gets saved. Not sure if you are missing the point of my comment which shows the state of the model just before it is saved - how can a save() fucntion accept the correct values and not save it to the database?

lukejagodzinski commented 8 years ago

So if my code does not work for you, then it means that you have bug in some other place. Have you ever tried learning from my example repository https://github.com/jagi/meteor-astronomy-examples ?

joetidee commented 8 years ago

Maybe, or there is a bug i your code. Just tell me, how can the output, that I have posted above, be passed to the save() function and not update the document? Isn't it the case that Astronomy will see that there is an _id present and that _isNew == false and save the document?

joetidee commented 8 years ago

ok, found the problem. I wasn't using set() to set the fields on the model. Instead I was using the dot nataion, e.g. doc.title = "some title".

lukejagodzinski commented 8 years ago

@joetidee I told you that's a bug in your code. If I'm sure of something in 100% then I'm sure :). The 1.0 syntax requires usage of the set() method, however 2.0 is not. So keep it in my when you will switch to new version.

joetidee commented 8 years ago

yup, got it thanks :)

Sent £5 via Paypal for you help - lunch on me!

lukejagodzinski commented 8 years ago

Ok thanks :)