cubism icon indicating copy to clipboard operation
cubism copied to clipboard

Fix bug on rule.remove

Open ghost opened this issue 13 years ago • 5 comments

context.rule creates a line with the datum {id: id}.

In the rule focus handler, this whole datum is set to an integer i, and the id is lost.

rule.remove will then throw an error because it tries to access d.id (with d being the datum) but the datum is just an integer now and has no 'id' property.

The fix consists of simply adding the integer i as an additional property of the datum instead of completely replacing it.

ghost avatar Dec 29 '12 20:12 ghost

Thanks for this pull request; apologies for the delayed response.

I think I understand what's going on and this sounds like the right fix. I don't fully understand the way that things with class line are used, I see there are also metric line objects which have .data(values) - do these need an ID as well? In your fork they are here.

Can you help me merge this by:

  1. [ ] signing our individual contributor license agreement
  2. [ ] making the change to the corresponding file in cubism's src folder?
  3. [ ] dig into the way metric line classed things are used and make sure this fix doesn't break those?

Thanks again!

RandomEtc avatar Mar 22 '13 17:03 RandomEtc

:ship:

jra3 avatar Apr 04 '13 17:04 jra3

Thanks for the boat of confidence, @jra3, but this pull request isn't quite ready. Can you help with items 2 or 3 on my checklist?

RandomEtc avatar Apr 04 '13 17:04 RandomEtc

I'll give it a shot. Ran across this pull request while investigating the bug myself.

jra3 avatar Apr 04 '13 17:04 jra3

Awesome - thanks!

You'll need a different pull request from your fork, just reference this one and we'll pick up the discussion there. We'll need you to fill out the contributor agreement too, it's just a quick Google Doc to make a couple of things explicit.

RandomEtc avatar Apr 04 '13 17:04 RandomEtc