JackAdams / meteor-transactions

App level transactions for Meteor + Mongo
http://transactions.taonova.com/
MIT License
113 stars 16 forks source link

Overridden tx.checkPermission fn is given undefined 'doc' parameter during commit #32

Closed rjsmith closed 9 years ago

rjsmith commented 9 years ago

Found in 0.7.1

When the tx.checkPermission fn is called from the _meteorTransactionsProcess Meteor method (via the tx.checkTransactionFields internal method for an 'insert' action, the expected 'doc' parameter is undefined.

The following test spec is failing at the expect(doc).toBeDefined(); line.

Note that the first checkPermission call, when the insert action is added to the transaction stack works fine. But when the checkPermission is invoked again during the commit, it fails. I think this is a bug.

'use strict';

describe('Use custom checkPermission check', function () {

  var transaction_id, insertedFooDoc, transactionDoc, fakeId;

  beforeEach(function () {

    // Fake userId to get through tx userId checks
    fakeId = 'or6YSgs6nT8Bs5o6p';

    // Because `tx.requireUser = true` (by default)
    spyOn(Meteor,'userId').and.returnValue(fakeId);

    tx.checkPermission =  function(action, collection, doc, modifier) {
      console.log('doc'+JSON.stringify(doc));
      if (action === 'insert') {
        expect(doc).toBeDefined();
      }
      return true;
    }

  });

  fit('calls custom checkPermission', function () {
    tx.start('transaction check');
    var newId = fooCollection.insert(
      {foo: "After insert"}, {tx: true});
    tx.commit();
  });

  afterEach(function () {
    fooCollection.remove({});
    tx.Transactions.remove({});
  });

})
JackAdams commented 9 years ago

Thanks for bringing this up. I'll work on a fix and get a new release out asap.

JackAdams commented 9 years ago

All tests passing now -- including the new one. I just learnt something new: fit and fdescribe !!

Thank you so much for submitting a bug report with a test. That's just ... words fail me, but it made me very happy.