guardian / scribe-plugin-noting

9 stars 3 forks source link

Unnoting another note #55

Closed robinedman closed 9 years ago

robinedman commented 9 years ago

This is an old one.

unnoting-too-much-bug

Both notes get unnoted since they still share the same id. If we make any change to the content, e.g. by typing a letter then ensureNoteIntegrity will sort that out for us and unnoting either note will then work as expected.

I probably introduced this bug when I fixed another one (make three paragraphs, unnote some text in the middle one, then unnote one of the paragraphs and everything would be unnoted). Previously we didn't rely on ids (rather on adjacenty as defined by stillWithinNote/isWitinNote), but sometimes that turned out to be problematic, so fixing might not be trivial.

One idea could be to use the "id strategy" when the note encompasses more than a paragraph and the "adjacency strategy" if not. But it might make sense to have a think about this. Maybe there's a better way.

robinedman commented 9 years ago

A much simpler fix, if this is OK performance wise would be to run ensureNoteIntegrity at the start of unnote, or (probably better) at the end of unnotePartOfNote.

cutandpastey commented 9 years ago

Should be fixed by https://github.com/guardian/scribe-plugin-noting/pull/58