Backbone-relational icon indicating copy to clipboard operation
Backbone-relational copied to clipboard

Keep keySource value even if no matching model

Open philfreo opened this issue 12 years ago • 6 comments

It's a common pattern for me to use foo_id in JSON to/from server that maps to a Foo model. Throughout my code I've run into issues like this several times regarding the id vs. the object.

test("Keep keySource value around if model doesn't exist", function() {
    var Game = Backbone.RelationalModel.extend();
    var Ticket = Backbone.RelationalModel.extend({
        relations: [{
            type: Backbone.HasOne,
            key: 'game',
            keySource: 'game_id',
            relatedModel: Game,
            includeInJSON: 'id'
        }]
    });

    var game1 = new Game({ id: 1, name: 'Game 1' });

    var ticket1 = new Ticket({
        game_id: 1
    });
    var ticket2 = new Ticket({
        game_id: 2
    });

    equal(ticket1.get('game').get('name'), 'Game 1', 'ticket1 game exists (game previously instantiated)');

    // FAIL #1
    equal(ticket1.get('game_id'), 1, 'ticket1 game_id still exists (game previously instantiated)');

    var ticket1json = ticket1.toJSON();
    ok(!ticket1json.game, 'ticket1 json should not have game')
    equal(ticket1json.game_id, 1, 'ticket1 json should have game_id')

    ok(!ticket2.get('game'), 'ticket2 game does not exist (game never instantiated)');

    // FAIL #2
    equal(ticket2.get('game_id'), 2, 'ticket2 game_id still exists exist (game never instantiated)');

    var ticket2json = ticket2.toJSON();
    ok(!ticket2json.game, 'ticket2 json should not have game')

    // FAIL #3
    equal(ticket2json.game_id, 2, 'ticket2 json should have game_id')
});

I'd like this entire thing to pass, but Fail #2 and #3 are the most important because you basically lose data. If I do ticket1.save(), the server gets passed the game_id. But if I do ticket2.save(), the game_id goes missing even though we had it on the client side.

It makes sense that you shouldn't always have to have the full objects in the client in order to keep track of references.

Thoughts?

philfreo avatar Dec 07 '12 09:12 philfreo

If you need it, it is still possible to access the original key value by doing ticket2.getRelation('game_id').keyContents. I'm not sure keeping them around in the model itself would improve the situation; sometimes it's there, sometimes it's not?

I'm doubting about #3. On the one hand, it sounds logical to add the original value if, apparently, it hasn't gotten modified. However, this does effectively make it impossible to unset a value (actually post a null) if a relation has been specified originally by the server?

PaulUithol avatar Dec 14 '12 08:12 PaulUithol

I assume you mean getRelation('game').keyContents above.

So now I have to do this in many of my models so that we don't lose data when resaving a model that was initialized only with a relation's id.

        toJSON: function() {
            var data = Model.prototype.toJSON.apply(this, arguments);

            // one line like this PER relation
            if (!data.lead_id) data.lead_id = this.getRelation('lead').keyContents;
            if (!data.contact_id) data.contact_id = this.getRelation('contact').keyContents;

            return data;
        }

Yes, you should be able to post null for both game and game_id if you want (especially if you ticket2.set('game', null) or ticket2.set('game_id', null); -- however I still really think there's a bug / incorrect behavior in the current case. Basically, I think it makes sense to retain access to both the object, and the id, all the time, and for the two to be in sync.

philfreo avatar Dec 18 '12 21:12 philfreo

Closing in favor of #191

philfreo avatar Dec 19 '12 22:12 philfreo

While I still think #1, #2, and #3 should all pass (as discussed in #191), even just getting #1 and #2 to pass would be a big help.

philfreo avatar Mar 06 '13 22:03 philfreo

@PaulUithol the 3rd of the 3 previously failing assertions above pass now, thanks to dcaf29975a2b23f85124a060526b5b5f27f11b51 (thanks!).

Do you agree that the first 2 assertions should pass as well? It's super helpful to be able to rely on this.model.get('foo_id') places where you don't need the full object / it not necessarily being there. And it's much more logical especially when objects are constructed with the ids.

philfreo avatar Mar 11 '13 03:03 philfreo

I've been mulling these over, but I don't think I agree these should be part of a model's attributes:

  • It breaks the abstraction bb-rel offers; imo, keySource and keyDestionation should only have to do with server communications, and not be exposed in regular usage on the client.
  • The general contract in Backbone is that set, unset, clear and get all operate on the same data and affect it similarly; this would break that contract as well.

A possibility would be to implement this as a sort of read-only "accessor" on get, so the data won't live in attributes; but this is a bit of a foreign concept (to JavaScript at least), so I don't like this too much either.

What is your use case for actually wanting this? Why wouldn't a different accessor (on RelationalModel for instance, or the relation itself) suffice?

PaulUithol avatar Mar 11 '13 13:03 PaulUithol