{"action":"created","issue":{"url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/issues/190","labels_url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/issues/190/labels{/name}","comments_url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/issues/190/comments","events_url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/issues/190/events","html_url":"https://github.com/sfu-natlang/lensingwikipedia/issues/190","id":99764320,"number":190,"title":"Redoing frontend queries API","user":{"login":"theq629","id":1696740,"avatar_url":"https://avatars.githubusercontent.com/u/1696740?v=3","gravatar_id":"","url":"https://api.github.com/users/theq629","html_url":"https://github.com/theq629","followers_url":"https://api.github.com/users/theq629/followers","following_url":"https://api.github.com/users/theq629/following{/other_user}","gists_url":"https://api.github.com/users/theq629/gists{/gist_id}","starred_url":"https://api.github.com/users/theq629/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/theq629/subscriptions","organizations_url":"https://api.github.com/users/theq629/orgs","repos_url":"https://api.github.com/users/theq629/repos","events_url":"https://api.github.com/users/theq629/events{/privacy}","received_events_url":"https://api.github.com/users/theq629/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/labels/enhancement","name":"enhancement","color":"84b6eb"},{"url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/labels/frontend","name":"frontend","color":"fad8c7"},{"url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/labels/proposal","name":"proposal","color":"5319e7"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-08-08T03:08:36Z","updated_at":"2015-12-11T12:07:06Z","closed_at":null,"body":"I made the current frontend query system (queries.js) based on the original LensingWikipedia but before we had a clear picture of how many things should work. In retrospect I think it's much more complex than it needs to be. This is my recent thoughts on how to redo it. None of this relates to the protocol for sending queries to the backend, which I think is still fine.\r\n\r\n# Redundancy with selections system\r\n\r\nThe new selections system (#183) encapsulates most of the places where directly using constraints (``Queries.Constraint``) was needed. In general I'm finding the selections interface much more convenient to use. It directly represents the state of the view in ways the constraints don't (for example the map merges all selections of points into a single constraint), while still making it easy to synchronize the selection state with constraints.\r\n\r\nTherefore in conjunction with the specific changes below it is likely possible to simplify the constraints system in favour of using the selections system. Of course we do still want to be present the constraint list in the UI, and there may be some unusual uses of constraints to consider (eg I believe there is one in compare.js that can't be wrapped by selections).\r\n\r\n# Constraint watchers\r\n\r\nI think the choice to have watcher callbacks for individual constraints (adding and removing) was a bad idea. It leads to complex code and requires many separate callbacks instances. I think it would be better to have per-query watchers that are triggered when constraints are added or removed; that is, instead of doing `someConstraint.onChange(...)`, you would have `someQuery.on('change', ...)` and have the callback filter for the constraints it's interested in. Additionally, I think this helps with the state saving situation (below).\r\n\r\nWith use of the selections system there are relatively few uses for watchers on constraints anyway. It's basically only for the UI's list of constraints, and so that the selection-query synchronizing code knows when a constraint was deleted through the UI list.\r\n\r\n# Combining queries\r\n\r\nThe original query system was based around the possibility of combining queries in various ways to get new queries. This leads to some complexity in queries having to be able to synchronize themselves with other queries to work out which constraints they actually contain at any moment. However, the only combination we've actually needed to implement and use is set-minus, since most views just want to get the global query result and the result without the view's own constraints. If we can agree on what range of operations we might need in the future we could probably simplify things.\r\n\r\nAdditionally, if we use the selections interface more then some of the complexity in combined queries can likely be removed. Especially there may be no need to for watcher callbacks which can detect when a constraint in a combined query is added or removed.\r\n\r\n# Saving state\r\n\r\nIn #125 @vlad003 noted that the current watcher system was an issue for saving state. The change to constraint watchers would presumably make this easier. However, I think we could probably save state in terms of selections rather than constraints. On restoring a saved selection it would just trigger the same watcher callbacks that would be triggered if the user had made those selections, and the code that synchronizes the selection with constraints would run normally. In contrast if we save constraints directly then it's hard for the views to reconstruct their state, eg the map view would need to parse its single constraint to work out which points were selected. Since constraints are independent I don't think we need to worry about order of selections. The same selections interface could also be used for any UI things that need to be saved (eg the choice of facet in the storyline view).\r\n\r\n# Practicality\r\n\r\nAlthough this is a fairly large and complex part of the code to redo, it's not necessarily too much trouble. The selections interface already standardizes most of the interaction with constraints. Most views also use queries in a very standard way. Therefore it's fairly easy to work out which parts of the query API we actually need."},"comment":{"url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/issues/comments/163923299","html_url":"https://github.com/sfu-natlang/lensingwikipedia/issues/190#issuecomment-163923299","issue_url":"https://api.github.com/repos/sfu-natlang/lensingwikipedia/issues/190","id":163923299,"user":{"login":"theq629","id":1696740,"avatar_url":"https://avatars.githubusercontent.com/u/1696740?v=3","gravatar_id":"","url":"https://api.github.com/users/theq629","html_url":"https://github.com/theq629","followers_url":"https://api.github.com/users/theq629/followers","following_url":"https://api.github.com/users/theq629/following{/other_user}","gists_url":"https://api.github.com/users/theq629/gists{/gist_id}","starred_url":"https://api.github.com/users/theq629/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/theq629/subscriptions","organizations_url":"https://api.github.com/users/theq629/orgs","repos_url":"https://api.github.com/users/theq629/repos","events_url":"https://api.github.com/users/theq629/events{/privacy}","received_events_url":"https://api.github.com/users/theq629/received_events","type":"User","site_admin":false},"created_at":"2015-12-11T12:07:06Z","updated_at":"2015-12-11T12:07:06Z","body":"I just did the full code review and can't find any issues, so does the above plan sound ok?"}}