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

Does async belongsTo support related model assignment? #1542

Closed
toranb opened this issue Dec 4, 2013 · 24 comments
Closed

Does async belongsTo support related model assignment? #1542

toranb opened this issue Dec 4, 2013 · 24 comments

Comments

@toranb
Copy link
Contributor

toranb commented Dec 4, 2013

I have a simple hasMany / belongsTo relationship that's async

App.Appointment = DS.Model.extend({
  details: attr('string'),
  customer: belongsTo('customer', { async: true})
});

App.Customer = DS.Model.extend({
  name: attr('string'),
  appointments: hasMany()
});

When I'm creating a new appointment object I need to fetch a customer first and set it like so

this.store.find('customer', 1).then(function(customer) {
  var appointment = { details: 'foo', customer: customer.get('id') };
  this.store.createRecord('appointment', appointment).save();
});

When I do this, I can log the ember appointment object and it looks solid

Object {details: "asd", customer: 1}

But when I pause in the "createRecord" method itself, I see this "customer" is no longer an integer but instead a promise. And when the adapter tries to serialize this in the "serializeBelongsTo" method it fails to capture this (as it's a promise and not a concrete object or integer value)

Does belongsTo support this type of behavior when it's async? If yes, what should be done / what can I do to help? I have an adapter that I use with the latest master build and I've found this is an issue (but I assume it's also a core issue to even the RESTAdapter / Serializer that ships with ember-data core)

For reference, here are a few integration tests that show this behavior

https://github.com/toranb/ember-data-django-rest-adapter/pull/63/files

@jherdman
Copy link

jherdman commented Dec 4, 2013

Possibly related #1535?

@toranb
Copy link
Contributor Author

toranb commented Dec 5, 2013

@jherdman ah yes! And my biggest issue (handling this myself) has been the issue you raised in the discussion

"It seems like store.serialize() is finishing before polymorphic type stuff is done — it's async after all!"

As this "works" but the serlializer won't wait for the async work to finish

Ember.RSVP.resolve(belongsTo).then(function (rec) {
    //finish the serialize work ... but wait ... it's async :(
});

Did you get any feedback yet about how to solve this issue?

@jherdman
Copy link

jherdman commented Dec 5, 2013

@toranb a little! We're hammering out some details, specifically if we're willing to make serialize() return a promise.

@toranb
Copy link
Contributor Author

toranb commented Jan 1, 2014

I'm waiting for the #1535 to be pulled into master but here are 2 tests I used to drive out this behavior in my stand alone adapter

asyncTest('async belongsTo relationship will POST serialized data', function() {
    var store,
    assertion;

    store = App.__container__.lookup('store:main');
    Ember.run(App, function(){
        store.serializerFor('customer').pushSinglePayload(store, 'customer', {"id": 10, "name": "toranb", "appointments": []});
    });

    visit("/").then(function() {
        return store.find('customer', 10);
    }).then(function(customer){
        var appointment;
        appointment = store.createRecord('appointment', {
            start: 'today',
            details: 'more',
            customer: customer
        });
        return appointment.save().then(
                assertion,
                assertion
            ); // check the assertion regardless of success or failure of appointment.save()
    });

    assertion = function() {
        equal(ajaxHash.data, '{"start":"today","details":"more","customer":"10"}');
        start(); // end the asyncTest
    };
});

asyncTest('async belongsTo relationship will PUT serialized data', function() {
    var store,
    assertion;

    store = App.__container__.lookup('store:main');
    Ember.run(App, function(){
        store.serializerFor('customer').pushSinglePayload(store, 'customer', {"id": 10, "name": "toranb", "appointments": [2]});
        store.serializerFor('appointment').pushSinglePayload(store, 'appointment', {"id": 2, "start": "x", "details": "y", "customer": 10});
    });

    visit("/").then(function() {
        return store.find('appointment', 2);
    }).then(function(appointment){
        appointment.set('start', 'updated');
        return appointment.save().then(
                assertion,
                assertion
            ); // check the assertion regardless of success or failure of appointment.save()
    });

    assertion = function() {
        equal(ajaxHash.data, '{"start":"updated","details":"y","customer":"10"}');
        start(); // end the asyncTest
    };
});

The implementation is hacky at best but this got the tests passing for the above (totally subject to change as you know the core of this project best)

    serializeBelongsTo: function(record, json, relationship) {
        var self = this;
        var key = relationship.key;
        var belongsTo = record.get(key);
        var finalizer = function () { return json; };

        if (Ember.isNone(belongsTo)) {
          json[key] = belongsTo;
        } else {
            return Ember.RSVP.resolve(belongsTo).then(function(record) {
                json[key] = record.get('id');
            }).then(finalizer);
        }

        if (relationship.options.polymorphic) {
          return Ember.RSVP.resolve(belongsTo).then(function(record) {
                   self.serializePolymorphicType(record, json, relationship);
                 }).then(finalizer);
        }
    }

@toranb
Copy link
Contributor Author

toranb commented Jan 1, 2014

also here are the models I used in the test code above (could be put into a single PR once I get to work on this)

App.Customer = DS.Model.extend({
    name: DS.attr('string'),
    appointments: DS.hasMany('appointment', { async: true})
});

App.Appointment = DS.Model.extend({
    start: DS.attr('string'),
    details: DS.attr('string'),
    customer: DS.belongsTo('customer', { async: true}),
});

@toranb
Copy link
Contributor Author

toranb commented Jan 17, 2014

I'm seeing this issue pop up more and more -anyone on the core team aware of it? (this just came up on discuss yesterday but I've been using a fork of ember-data for a month+ now)

http://discuss.emberjs.com/t/how-should-async-belongsto-relationships-be-serialized/3858

@stefanpenner
Copy link
Member

Sorry I havent been been to busy to dig into this one yet.

@toranb
Copy link
Contributor Author

toranb commented Jan 17, 2014

No worries -I'm tight for time myself this month, I just wanted to see if anyone else had noticed this was or wasn't an issue in ember-data 1.0 beta 5

I'm currently waiting on the related issue to be pulled in as it changes the serializer used here (returns a promise now as you've seen). When that PR is pulled in I think this change should be fairly simple (just resolve the belongsTo or hasMany essentially)

@aBuder
Copy link

aBuder commented Jan 29, 2014

Hi friends is the bug with the belongsTo relationship solved? I have the same issue.

@kingpin2k
Copy link
Contributor

@aBuder, no, they are still hashing out implementation in #1535

@toranb
Copy link
Contributor Author

toranb commented Jan 29, 2014

Agreed - I'm using a custom fork if you really need to ship and can't wait for the issue @kingpin2k mentioned. Ping me at my gmail and I'll pass you a link -I shipped a commercial app using it last month and so far it's been bug free for async hassMany/belongsTo (non-polymorphic) relationships

@bmac
Copy link
Member

bmac commented Apr 23, 2014

I just ran into this problem too. :( Looks like there are about 4 or 5 open issues for this bug.

@ashrafhasson
Copy link

I believe I have hit this same problem using ember-data.1.0.0-beta.8. Having a hasMany/belongsTo relationship, when I attempt to save the model object that has the belongsTo definition, the PUT request shows a null value for the belongsTo relationship key. Is this related to the issue above?

@bmac
Copy link
Member

bmac commented Jul 18, 2014

@ashrafhasson that sounds exactly like this issue. One workaround is to get the async property before saving.

model.set('asyncRelationship', record);
model.get('asyncRelationship').then(function() {
  model.save();
});

@johanneswuerbach
Copy link
Contributor

Sadly this doesn't work anymore after ssot in beta10. Is there another workaround to set / unset and async belongsTo?

//cc @igorT

Update: the new workaround:

model.set('asyncRelationship.id', null);
model.save()

@sly7-7
Copy link
Contributor

sly7-7 commented Oct 16, 2014

I think this is fixed in canary now. cc/ @igorT

@sly7-7
Copy link
Contributor

sly7-7 commented Nov 1, 2014

@johanneswuerbach Is there any chance you could test with current canary ?

@johanneswuerbach
Copy link
Contributor

@sly7-7 yes, unassigning works now for me. 👍

@sly7-7
Copy link
Contributor

sly7-7 commented Nov 1, 2014

@johanneswuerbach Great, thank you.
@igorT this could be closed

@igorT igorT closed this as completed Nov 12, 2014
@shaunc
Copy link

shaunc commented Dec 19, 2014

Hmm... is it possible its not fixed in the case that the relationship is polymorphic as well as async?

@sly7-7
Copy link
Contributor

sly7-7 commented Dec 20, 2014

@shaunc Would it be possible for you to open a new issue demonstrating the bug please ?
Currently, a test like

test("set polymorphic async belongsTo", function() {
  expect(2);
  Contact = DS.model.extend({
    posts: DS.hasMany('post')
  });
  Email = Contact.extend();
  Post = DS.Model.extend({
    contact: DS.belongsTo('contact', { async: true, polymorphic: true })
  });

  Ember.run(function () {
    email = env.store.createRecord('email');
    post = env.store.createRecord('post', {
      contact: email
    });
  });

  post.get('contact').then(async(function(contact){
    equal(contact, email, 'The polymorphic belongsTo is set up correctly');
    equal(get(email, 'posts.length'), 1, "The inverse has many is set up correctly on the email side.");
  }));
});

is passing. Maybe it does not reflect your use case though.

@shaunc
Copy link

shaunc commented Dec 20, 2014

I'll try to get one up -- the problem is that, in RESTSerializer. serializePolymorphicType,

  json[key + "Type"] = Ember.String.camelize(belongsTo.constructor.typeKey);

will fail when "belongsTo" is a PromiseObject. Using:

belongsTo.content.constructor.typeKey

in this case works. (I'm trying to run your tests now, but is getting sort of late at night here. :))

@sly7-7
Copy link
Contributor

sly7-7 commented Dec 20, 2014

@shaunc Ok, so your issue is a well known one, see #2571

@shaunc
Copy link

shaunc commented Dec 20, 2014

thanks!

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