Call the functions defined by $firebaseArray.$add({...}).then when promise is fulfilled or rejected

[christiaanwesterbeek]

Currently in $firebaseArray(ref).$add({...}).then(onFulfilled, onRejected);, the functions onFulfilled and onRejected are not called.

It's practical for me to happen so that I can get a reference to the item that was added by the user adding it. Currently I use $watch on the firebaseArray now, but I needed to build something special to filter out other events that where triggered by other users actions on the same array.

My code can become simpler if those oFulfilled and onRejected functions are called.

@katowulf Can you give me some pointers on where to implement this in firebase-util?

Here's a fiddle that compare push and $add: http://jsfiddle.net/devotis/869sgr01/2/

[katowulf]

Don't use $watch. Use $extend and $$added as demonstrated here. I'm not sure what this question has to do with firebase-util as it appears to be a question about filtering with AngularFire. If you're having specific issues, please create a repro that demonstrates the behavior.

[christiaanwesterbeek]

Thanks for the demonstration @katowulf . After posting my question I updated it with a fiddle that demonstrates the behaviour.

You'll find that the function defined by then after $add is not called.

[katowulf]

A promise accepts two functions, the first is the success, the second is the error. Add an error function and you'll see it's being triggered with undefined. If you investigate why, I'll be happy to patch the project.

[phanikoneru]

@maxrevilo Is this issue with firebase util or angularfire. i sm also running into similar issue as mentioned in fiddle and using angularfire 1.1.2 and firebase util 0.2.4

[maxrevilo]

The problem is with Firebase Util. I made a PR that kato accepted, and now we have to wait until the next release for bower.

But in the mean time you can hotfix your local copy of Firebase Util. In the file firebase-util/dist/firebase-util.js look for the line 4015 and change it to:

      fn.apply(context, this.hasErrors()? this.getErrors() : [null]);

And import firebase-util.js instead of firebase-util.min.js

[phanikoneru]

Thank You @maxrevilo . It worked!! Any idea when will be next release?

[jaufgang]

@maxrevilo's pull request was merged on July 21. Version 2.5 was released on bower on August 20, but for some reason did not include the fix from this pull request. Hence, this issue is still outstanding for bower users. (I just encountered the bug today, and had to hunt around to find that it had been fixed 3 months ago.) It would be very helpful if a new version could be released containing this fix. Thanks.

[maxrevilo]

The bug got reintroduced. I will make another PR, but I want to make an unit test to make sure that the bug never gets reintroduced again.

The bug is in src/common/libs/queue.js should I make a unit test for handler(fn, context) in firebase-util/test/common/libs/queue.js? Or should I go for an integration test at a higher level?

[katowulf]

Unit tests at the lowest level seem ideal. Also, to be fixed, the PR had this:

 fn.apply(context, this.hasErrors()? [this.getErrors() : null]);

Which should actually be this:

 fn.apply(context, this.hasErrors()? this.getErrors() : [null]);

PRs welcome.

[maxrevilo]

Yes your are right, should be the former. Seems pretty easy I'll try to make te PR as soon as I can.

We can also consider to change getErrors from

return this.errors.slice(0);

to:

return this.errors.length? this.errors.slice(0): [null];

And that way the logic is encapsulated in the function. But I'm not sure if returning [null] on getErrors is an acceptable solution.

[katowulf]

The results from getErrors() are currently being passed directly into Firebase API wrappers. Those must return either null or an error value. Thus, to remain as close to the API as possible, we should keep returning null. It is annoying that the function.apply() calls are so prevalent and muck the simplicity.