getoutreach / epf

A framework for keeping your Ember.js apps in sync.
http://epf.io
MIT License
369 stars 33 forks source link

session.flush not issuing PUT #61

Open belluzj opened 11 years ago

belluzj commented 11 years ago

Problem The call to session.flush() does not persists changes to the backend. The promise resolves immediately, and no request is sent.

I don't know if I do something wrong or EPF does not work.

What I do I create a child session in the router using this code:

    var childSession = this.session.newSession();
    controller.set('newComp', childSession.add(App.Company.create({
      financialPeriodDuration: 12,
      /* fields */
    })));
    controller.set('newFY', childSession.add(App.FinancialYear.create({
      company: controller.get('newComp')
    })));

I also tried with the other syntax childSession.create('company', { .... I also tried by not creating a child session (replacing this.session.newSession() with this.session).

Then, after setting properties on these objects through a form, I add an account to the company using this code, from another controller

      var session = this.get('controllers.companiesAdd.newComp.session');
      var comp = this.get('controllers.companiesAdd.newComp');
      var account = session.create('account', {
        company: comp,
        /* fields */
      });
     /* EDIT: I thought this part would not be useful, but maybe it is: */
     if (!comp.get('mainBankAccount')) {
        comp.set('mainBankAccount', account);
      }
     /* END of edit */

Finally I flush, using this code, from the 'companiesAdd' controller:

      this.get('newComp.session').flush().then(function (val) {
        console.log(val);
      });

the value returned in the promise is: [isEnumerable: true, nextObject: function, firstObject: undefined, lastObject: undefined, contains: function…]. It has a length: 0 attribute. It should be a list of updated models, right?

Other info I tried looking at EPF internals in the debugger, and in the childSession flush method, I could see my three objects newComp, newFY and account in the dirtyObjects collection. But I could not track them inside the bowels of EPF.

I though that if EPF does something, it goes in Operation.perform() (am I right?) so I placed a breakpoint in here but it was never reached.

I just migrated my models from Ember Data, so there may be some configuration problem. Its works for reading records from the server, though.

Thanks in advance for your help. EPF seems very promising, and I would be very pleased to be able to use it.

ghempton commented 11 years ago

This should work. Can you post your model definitions?

belluzj commented 11 years ago

I have first a file in which I create all model classes like so:

App.Account = Ep.Model.extend();
App.Company = Ep.Model.extend();
App.FinancialYear = Ep.Model.extend();

Then I reopen each one to define attributes and relationships, following the example at https://github.com/rayb/epf_skeleton/blob/master/app/assets/javascripts/models/foo.coffee

var accountExtendsObject = {
  company: Ep.belongsTo(App.Company, { inverse: 'accounts' }),

  number: Ep.attr('string'),
  name: Ep.attr('string'),

  // Stats
  operationCount: Ep.attr('number'),
  totalDebit: Ep.attr('number'),
  totalCredit: Ep.attr('number'),
  balance: Ep.attr('number'),

  hasPrefix: function (prefix) {
    return this.get('number') && (new RegExp('^' + prefix)).test(this.get('number'));
  },
  fullName: function () {
    return this.get('number') + ' - ' + this.get('name');
  }.property('number', 'name'),
};

var prefixes = {...};
for (var key in prefixes) {
  /* Add a bunch of computed properties to accountExtendsObject */
}

App.Account.reopen(accountExtendsObject);
App.Company.reopen({
  mainBankAccount: Ep.belongsTo(App.Account),
  accounts: Ep.hasMany(App.Account, { inverse: 'company' }),
  financialYears: Ep.hasMany(App.FinancialYear, { inverse: 'company' }),
  lastClosedFinancialYear: Ep.belongsTo(App.FinancialYear),

  name: Ep.attr('string'),
  logoUrl: Ep.attr('string'),
  reportsFrequency: Ep.attr('number'), // number of weeks
  financialPeriodDuration: Ep.attr('number'), // number of months
  vatMode: Ep.attr('number'),
  vatPeriod: Ep.attr('number'),
  firstDay: Ep.attr('date'),
});
App.FinancialYear.reopen({
  company: Ep.belongsTo(App.Company, {inverse: 'financialYears'}),

  lastDay: Ep.attr('date'),
});
belluzj commented 11 years ago

Looking at my models, and knowing that EPF is 'smart' about relationships, it may encounter some kind of problem because of the relationships between Companies and Accounts: the Company I send has many Accounts, each account belongs to this Company, and the Company "belongs to" (it should be "has one") a main bank Account.

I edited my first comment to add two lines of code that set the main bank account.

Maybe this is some kind of chicken and egg problem that blocks EPF from persisting records? I don't think my models are that complex, though, and IMHO such a case could be handled by sending the company, then the accounts, then updating the company to set its main bank account (as ids become available, persist them). But I don't know if it is possible for you to detect such loops and treat them accordingly. Let me know if I should modify my models.

belluzj commented 11 years ago

Update: EPF does send some PUT requests when I comment out the following lines:

App.Company.reopen({
  //mainBankAccount: Ep.belongsTo(App.Account),
  accounts: Ep.hasMany(App.Account, { inverse: 'company' }),
  financialYears: Ep.hasMany(App.FinancialYear, { inverse: 'company' }),
  //lastClosedFinancialYear: Ep.belongsTo(App.FinancialYear),

But this is still a problem, first because I need these two relationships, and then because the issued PUT's are not what I expected. Indeed, EPF persists the company first, gets an id back from the server, and then persists financialYear and the account, but its supplies them with a null company_id.

ghempton commented 11 years ago

Ah so if you are creating a 1-1 relationship you should set the "owner" of the relationship (this determines which side persists it). To do this you could so something like the following:

App.Adapter.map(App.Company,
        {mainBankAccount: { owner: false }}
);
belluzj commented 11 years ago

I still have the original problem. Focusing on the relationships between Account and Company, I tried:

App.Company.reopen({
  //mainBankAccount: Ep.belongsTo(App.Account); // sends PUT
  accounts: Ep.hasMany(App.Account),
});
App.Account.reopent({
  company: Ep.belongsTo(App.Company, {inverse: 'accounts'});
});

I uncommented the problematic line, and added your adapter mapping, but it didn't help. Anyway, I do not think it says what I want to say.

App.Company.reopen({
  mainBankAccount: Ep.belongsTo(App.Account); // does not PUT
  accounts: Ep.hasMany(App.Account),
});
App.Account.reopent({
  company: Ep.belongsTo(App.Company, {inverse: 'accounts'});
});

Here is an example of what I would like to send to the backend:

  1. company1 = {name: ...} -> get back the id (1)
  2. account1 = {company_id: 1, name: "Main account", ...} account2 = {company_id: 1, name: "other account", ...} -> get back ids (1 and 2)
  3. update company company1 = {id: 1, main_bank_account_id: 1, ...}

It means that company should hold the id of its main account, and that every account should hold the id of its company (regardless of whether it is the main account).

Is there a way to express that?

Note: I also tried what is below, without success. It seems closer to what I want to say, even if it is dumb to have two times the same company id in Account.

App.Company.reopen({
  mainBankAccount: Ep.belongsTo(App.Account, {inverse: 'company2'}); // does not PUT
  accounts: Ep.hasMany(App.Account, {inverse: 'company'}),
});
App.Account.reopent({
  company: Ep.belongsTo(App.Company, {inverse: 'accounts'});
  company2: Ep.belongsTo(App.Company, {inverse: 'mainBankAccount'});
});
Ep.RestAdapter.map(App.Account, {
  company2: { owner: false }
});
belluzj commented 11 years ago

I corrected some of the aforementioned problems (my mistakes), but the main issue is still here.

To make myself clear, I forked the epf_example repo, and reproduced my bug on it. I added an "emergency phone number" to the contact model. See my commit: https://github.com/belluzj/epf_example/commit/f8d53cb669aa4c5cfcfeaaccbfd4e745ce37f506

Please clone and run the application. Try to add one contact, add numbers, mark one as the emergency number, press create and watch the network pane of the debugger: no POST request.

belluzj commented 11 years ago

I found a workaround. I would still find it cool to have that feature built in EPF, in some later version. And for now it would great that a relationship being defined did not break the whole thing, even if it is not handled correctly.

  //mainBankAccount: Ep.belongsTo(App.Account),
  mainBankAccountId: Ep.attr('number'),
  mainBankAccount: function (key, model) {
    if (arguments.length === 2) {
      if (!model.get('id')) {
        // FIXME hack to store main bank account id when the account gets an id
        this.set('pendingMainBankAccount', model);
      } else {
        this.set('mainBankAccountId', model.get('id'));
      }
      return model;
    } else {
      var id = this.get('mainBankAccountId');
      if (Ember.typeOf(id) === 'number') {
        return this.session.find('account', id);
      }
    }
  }.property('mainBankAccountId'),

  pendingMainBankAccount: null,
  flushOnID: function () {
    var id = this.get('pendingMainBankAccount.id');
    if (id) {
      this.set('mainBankAccountId', id);
      this.get('session').flush();
      this.set('pendingMainBankAccount', null);
    }
  }.observes('pendingMainBankAccount.id'),