Fix some more bugs in Document ⇆ Entity interface

wjt commented

2016-04-22 14:20:52 +00:00

I fixed some problems in #2769 but editing entities for a document was still kind of buggy: changes were not reliably saved. I tracked down the problem today: choosing an entity from the auto-complete list triggers a 'submit' event, which is ignored, and no 'change' event, which is what the documents panel listens for.

It looks like before [this commit in oxjs]changeset:7c7f970/oxjs this would have worked: FormElementGroup used to emit a 'change' event whenever a child element emitted 'submit'. Presumably that was changed for a reason, so I've fixed this by propagating 'submit' outwards and handling that.

I fixed some problems in #2769 but editing entities for a document was still kind of buggy: changes were not reliably saved. I tracked down the problem today: choosing an entity from the auto-complete list triggers a `'submit'` event, which is ignored, and no `'change'` event, which is what the documents panel listens for. It looks like before [this commit in oxjs]changeset:7c7f970/oxjs this would have worked: `FormElementGroup` used to emit a `'change'` event whenever a child element emitted `'submit'`. Presumably that was changed for a reason, so I've fixed this by propagating `'submit'` outwards and handling that.

wjt added the

frontend

label 2016-04-22 14:20:52 +00:00

wjt added this to the 14.04 milestone 2016-04-22 14:20:52 +00:00

0x2620 was assigned by wjt

2016-04-22 14:20:52 +00:00

wjt added the

normal

defect

labels 2016-04-22 14:20:52 +00:00

wjt commented

2016-04-22 14:27:32 +00:00

Author

https://gitlab.com/wjt/oxjs.git branch 2914-document-entities-ui, diff: https://gitlab.com/wjt/oxjs/compare/master...2914-document-entities-ui
https://gitlab.com/wjt/pandora.git branch 2914-document-entities-ui – the full diff is huge because of a couple of patches that just move code around, so it's better reviewed with git log -p -b :-)

* <https://gitlab.com/wjt/oxjs.git> branch `2914-document-entities-ui`, diff: <https://gitlab.com/wjt/oxjs/compare/master...2914-document-entities-ui> * <https://gitlab.com/wjt/pandora.git> branch `2914-document-entities-ui` – the full diff is huge because of a couple of patches that just move code around, so it's better reviewed with `git log -p -b` :-)

wjt commented

2016-04-22 16:46:54 +00:00

Author

Thinking about this a bit more, it would probably be better if 'change' was emitted – it's annoying to have to vary which signal you listen to, and you risk getting double-notified for other field types (eg a plain Input emits both 'change' and 'submit' when you type some text and then hit enter). So maybe a better fix would be: arrange for 'change' to be emitted when you select an entry from the autocomplete menu? Maybe it is not emitted as a side-effect of autocompleteReplace: true, and that is the real bug.

Thinking about this a bit more, it would probably be better if `'change'` was emitted – it's annoying to have to vary which signal you listen to, and you risk getting double-notified for other field types (eg a plain `Input` emits both `'change'` and `'submit'` when you type some text and then hit enter). So maybe a better fix would be: arrange for `'change'` to be emitted when you select an entry from the autocomplete menu? Maybe it is not emitted as a side-effect of `autocompleteReplace: true`, and that is the real bug.

wjt commented

2016-07-01 17:24:47 +00:00

Author

As a result of running these patches in the instance here, people have actually been using this bit of the UI, and so a new problem emerges…

Suppose an entity's name has a typo. A user who didn't fully understand the data model searched in Manage Documents for the typoed name, and then for each matching document (well, using the bulk-edit patch from #2934) changed the name of the entity from within Manage Documents. This had the effect of removing the tag because no entity by that name exists and the error is ignored.

So I think this means the change-handling flow needs to be:

Look up new entities' IDs
If any are incorrect, flag that field as containing an error, and halt
Otherwise, add new entities, and remove the removed entities.

As opposed to what happens here now, which is to add and remove in parallel.

As a result of running these patches in the instance here, people have actually been using this bit of the UI, and so a new problem emerges… Suppose an entity's name has a typo. A user who didn't fully understand the data model searched in Manage Documents for the typoed name, and then for each matching document (well, using the bulk-edit patch from #2934) changed the name of the entity from within Manage Documents. This had the effect of **removing the tag** because no entity by that name exists and the error is ignored. So I think this means the change-handling flow needs to be: * Look up new entities' IDs * If any are incorrect, flag that field as containing an error, and halt * Otherwise, add new entities, and remove the removed entities. As opposed to what happens here now, which is to add and remove in parallel.

Fix some more bugs in Document ⇆ Entity interface #2914