Skip to content

Conversation

@iamigo
Copy link
Contributor

@iamigo iamigo commented Dec 21, 2016

When an aspect’s tags change, send “delete” and “add” realtime events (instead of “update”)
so that perspectives which filter by aspect tag will get the right sample events.

… (instead of “update”)

so that perspectives which filter by aspect tag will get the right sample events.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 83.814% when pulling 19634e5 on aspect-tag-change-realtime-events into 3d106f4 on master.

@iamigo iamigo requested a review from annyhe December 21, 2016 21:57
@iamigo iamigo added the bug label Dec 21, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 83.814% when pulling e893969 on aspect-tag-change-realtime-events into 3d106f4 on master.

}))
.then((s) => {
expect(s.dataValues.aspect.tags).to.have.members(['T3']);
done();
Copy link
Contributor

Choose a reason for hiding this comment

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

@iamigo Wouldn't we want the test to assert
expect(aspect.tags).to.deep.equal(['T3'])?

});
})
.then(() => resolve(inst))
.catch(reject);
Copy link
Contributor

Choose a reason for hiding this comment

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

@iamigo do we have a test that asserts on unpublishing an aspect, it loses all of its samples?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 83.814% when pulling 9c92d8e on aspect-tag-change-realtime-events into 3d106f4 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 83.814% when pulling 33dec0f on aspect-tag-change-realtime-events into 3d106f4 on master.

@annyhe annyhe merged commit 4a2f2a6 into master Dec 22, 2016
@annyhe annyhe deleted the aspect-tag-change-realtime-events branch December 22, 2016 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants