Skip to content

Added changed, removed and added events back to the observer #11

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.

Sign up for GitHub

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

Open
navelpluisje wants to merge 5 commits into
base: minimongo-cache
Choose a base branch
from
Open

Added changed, removed and added events back to the observer #11

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f870847
Added changed, removed and added events back to the observer
navelpluisje Oct 17, 2015
d31f48f
newFields now gets the result of the upsert
navelpluisje Oct 17, 2015
7f104cb
Added changed as a parameter for observe
navelpluisje Oct 17, 2015
d69ebf2
Had a mindfart while looking at the observe;-)
navelpluisje Oct 17, 2015
52ad01b
Renamed the wrong function
navelpluisje Oct 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,31 +185,57 @@ class DDPClient extends EventEmitter{
}

self.collections[name].upsert(item);

if (self._observers[name]) {
_.each(self._observers[name], function(observer) {
observer.added(id, item);
});
}
}

// remove document from collection
} else if (data.msg === "removed") {
if (self.maintainCollections && data.collection) {
var name = data.collection, id = data.id;
var name = data.collection,
id = data.id
oldFields = self.collections[name].get(id);

self.collections[name].remove({"_id": id});

if (self._observers[name]) {
_.each(self._observers[name], function(observer) {
observer.removed(id, oldFields);
});
}
}

// change document in collection
// change document in collection
} else if (data.msg === "changed") {
if (self.maintainCollections && data.collection) {
var name = data.collection, id = data.id;
var name = data.collection,
id = data.id,
oldFields = {},
newFields = {},
clearedFields = data.cleared || [];

var item = {
"_id": id
};

if (data.fields) {
oldFields = self.collections[name].get(id);
_.each(data.fields, function(value, key) {
item[key] = value;
})
}

self.collections[name].upsert(item);
newFields = self.collections[name].upsert(item);

if (self._observers[name]) {
_.each(self._observers[name], function(observer) {
observer.changed(id, oldFields, clearedFields, newFields);
Copy link

@sunny-g sunny-g Oct 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@navelpluisje pardon me if I'm mistaken, but by this point in the code, newFields is still an empty object right? If you look here in the master branch, we add every key-value pair to newFields and then pass it into observer.changed.

In this case however, I believe just passing in item in place of newFields would suffice as it is on line 191 here in your PR.

Copy link

@sunny-g sunny-g Oct 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I can't leave a line note on line 466, but the changed DDP message's observer function should be renamed back to changed from updated to be consistent with the DDP message type and the code you've written here.

Copy link
Author

@navelpluisje navelpluisje Oct 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sunny-g Thanx for mentioning about the newFields. I will submit a change in a minute. Like you mentioned it is possible to set item as the parameter, but if the submit to the cache fails this is not the right value. I'm setting the value of newFields with the result of the upsert. Just forgot to copy this part;-)

});
}
}

// subscriptions ready
Expand Down Expand Up @@ -420,10 +446,10 @@ class DDPClient extends EventEmitter{
/**
* Adds an observer to a collection and returns the observer.
* Observation can be stopped by calling the stop() method on the observer.
* Functions for added, updated and removed can be added to the observer
* Functions for added, removed and changed can be added to the observer
* afterward.
*/
observe(name, added, updated, removed) {
observe(name, added, changed, removed) {
var self = this;
var observer = {};
var id = self._getNextId();
Expand All @@ -437,7 +463,7 @@ class DDPClient extends EventEmitter{
Object.defineProperty(observer, "_id", { get: function() { return id; }});

observer.added = added || function(){};
observer.updated = updated || function(){};
observer.changed = changed || function(){};
observer.removed = removed || function(){};

observer.stop = function() {
Expand Down