{"action":"created","issue":{"url":"https://api.github.com/repos/sharelatex/web-sharelatex/issues/518","repository_url":"https://api.github.com/repos/sharelatex/web-sharelatex","labels_url":"https://api.github.com/repos/sharelatex/web-sharelatex/issues/518/labels{/name}","comments_url":"https://api.github.com/repos/sharelatex/web-sharelatex/issues/518/comments","events_url":"https://api.github.com/repos/sharelatex/web-sharelatex/issues/518/events","html_url":"https://github.com/sharelatex/web-sharelatex/pull/518","id":232922385,"number":518,"title":"Aggregate adjacent inserts and deletes","user":{"login":"paulojreis","id":10046706,"avatar_url":"https://avatars1.githubusercontent.com/u/10046706?v=3","gravatar_id":"","url":"https://api.github.com/users/paulojreis","html_url":"https://github.com/paulojreis","followers_url":"https://api.github.com/users/paulojreis/followers","following_url":"https://api.github.com/users/paulojreis/following{/other_user}","gists_url":"https://api.github.com/users/paulojreis/gists{/gist_id}","starred_url":"https://api.github.com/users/paulojreis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/paulojreis/subscriptions","organizations_url":"https://api.github.com/users/paulojreis/orgs","repos_url":"https://api.github.com/users/paulojreis/repos","events_url":"https://api.github.com/users/paulojreis/events{/privacy}","received_events_url":"https://api.github.com/users/paulojreis/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-06-01T15:40:32Z","updated_at":"2017-06-01T16:35:33Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/sharelatex/web-sharelatex/pulls/518","html_url":"https://github.com/sharelatex/web-sharelatex/pull/518","diff_url":"https://github.com/sharelatex/web-sharelatex/pull/518.diff","patch_url":"https://github.com/sharelatex/web-sharelatex/pull/518.patch"},"body":"**Do not accept yet**; some details still need work.\r\n\r\nChanges:\r\n\r\n* The `updateEntries` cycle now includes logic to determine if changes should be aggregated (i.e. a deletion just next to an insertion, made by the same user);\r\n* A new directive to show aggregated change entries;\r\n* The logic to determine selected changes (for bulk actions) is different:\r\n    * `$scope.reviewPanel.selectedEntryIds` contains all selected changes—if an aggregate change is selected, then two ids will be present (the insertion + the deletion);\r\n    * `$scope.reviewPanel.nVisibleSelectedChanges` is the number of entries from a user PoV. An aggregate change counts as one (although it contains two changes). This is used to be shown in the UI and to determine bulk actions visibility."},"comment":{"url":"https://api.github.com/repos/sharelatex/web-sharelatex/issues/comments/305549348","html_url":"https://github.com/sharelatex/web-sharelatex/pull/518#issuecomment-305549348","issue_url":"https://api.github.com/repos/sharelatex/web-sharelatex/issues/518","id":305549348,"user":{"login":"jpallen","id":31305,"avatar_url":"https://avatars3.githubusercontent.com/u/31305?v=3","gravatar_id":"","url":"https://api.github.com/users/jpallen","html_url":"https://github.com/jpallen","followers_url":"https://api.github.com/users/jpallen/followers","following_url":"https://api.github.com/users/jpallen/following{/other_user}","gists_url":"https://api.github.com/users/jpallen/gists{/gist_id}","starred_url":"https://api.github.com/users/jpallen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jpallen/subscriptions","organizations_url":"https://api.github.com/users/jpallen/orgs","repos_url":"https://api.github.com/users/jpallen/repos","events_url":"https://api.github.com/users/jpallen/events{/privacy}","received_events_url":"https://api.github.com/users/jpallen/received_events","type":"User","site_admin":false},"created_at":"2017-06-01T16:35:33Z","updated_at":"2017-06-01T16:35:33Z","body":"Approach looks sensible and clean. A few points:\r\n\r\n1) 'Agg' is not a clear acronym. Can we change e.g. `agg-change` -> `aggregate-change`, `acceptAggChange` -> `acceptAggregateChange` to be more explicit.\r\n2) Instead of having additional `agg_op` and `agg_op_id` properties, can we instead treat both parts of the change symmetrically. I.e. An array of ops and op_ids that contain two elements.\r\n3) This might let us simplify all the code, and avoid the need for a separate `acceptAggChange` methods. If all entries contained an array of their ops and ids (with inserts and deletes only having one entry, and aggregate changes having two), then one code path could deal with them all. It's a little more refactoring, but I think would be cleaner in total, since we'd only need to understand one model/data structure.\r\n\r\nAll of this is just from a cursory look, so we should probably discuss in more detail before deciding anything."}}
{"actor":{"display_login":"jpallen"}}