{"action":"created","issue":{"url":"https://api.github.com/repos/pkp/pkp-lib/issues/2163","repository_url":"https://api.github.com/repos/pkp/pkp-lib","labels_url":"https://api.github.com/repos/pkp/pkp-lib/issues/2163/labels{/name}","comments_url":"https://api.github.com/repos/pkp/pkp-lib/issues/2163/comments","events_url":"https://api.github.com/repos/pkp/pkp-lib/issues/2163/events","html_url":"https://github.com/pkp/pkp-lib/issues/2163","id":199590590,"number":2163,"title":"Routing events between handlers in the JS framework","user":{"login":"NateWr","id":2306629,"avatar_url":"https://avatars2.githubusercontent.com/u/2306629?v=4","gravatar_id":"","url":"https://api.github.com/users/NateWr","html_url":"https://github.com/NateWr","followers_url":"https://api.github.com/users/NateWr/followers","following_url":"https://api.github.com/users/NateWr/following{/other_user}","gists_url":"https://api.github.com/users/NateWr/gists{/gist_id}","starred_url":"https://api.github.com/users/NateWr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NateWr/subscriptions","organizations_url":"https://api.github.com/users/NateWr/orgs","repos_url":"https://api.github.com/users/NateWr/repos","events_url":"https://api.github.com/users/NateWr/events{/privacy}","received_events_url":"https://api.github.com/users/NateWr/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"NateWr","id":2306629,"avatar_url":"https://avatars2.githubusercontent.com/u/2306629?v=4","gravatar_id":"","url":"https://api.github.com/users/NateWr","html_url":"https://github.com/NateWr","followers_url":"https://api.github.com/users/NateWr/followers","following_url":"https://api.github.com/users/NateWr/following{/other_user}","gists_url":"https://api.github.com/users/NateWr/gists{/gist_id}","starred_url":"https://api.github.com/users/NateWr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NateWr/subscriptions","organizations_url":"https://api.github.com/users/NateWr/orgs","repos_url":"https://api.github.com/users/NateWr/repos","events_url":"https://api.github.com/users/NateWr/events{/privacy}","received_events_url":"https://api.github.com/users/NateWr/received_events","type":"User","site_admin":false},"assignees":[{"login":"NateWr","id":2306629,"avatar_url":"https://avatars2.githubusercontent.com/u/2306629?v=4","gravatar_id":"","url":"https://api.github.com/users/NateWr","html_url":"https://github.com/NateWr","followers_url":"https://api.github.com/users/NateWr/followers","following_url":"https://api.github.com/users/NateWr/following{/other_user}","gists_url":"https://api.github.com/users/NateWr/gists{/gist_id}","starred_url":"https://api.github.com/users/NateWr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NateWr/subscriptions","organizations_url":"https://api.github.com/users/NateWr/orgs","repos_url":"https://api.github.com/users/NateWr/repos","events_url":"https://api.github.com/users/NateWr/events{/privacy}","received_events_url":"https://api.github.com/users/NateWr/received_events","type":"User","site_admin":false}],"milestone":null,"comments":60,"created_at":"2017-01-09T16:03:51Z","updated_at":"2017-07-26T22:30:45Z","closed_at":null,"body":"This issue is a placeholder to track some work/experimentation to improve the way that our JS handlers can communicate between themselves. The suggested approach is to have a kind of global event router which is based on the [observer pattern](https://en.wikipedia.org/wiki/Observer_pattern) common in several JS frameworks.\r\n\r\nI'd like to use this to document some of the lessons learned about how events in the JS Handlers (eg - UI \"widgets) work.\r\n\r\n## Overview\r\nAs a general rule, events that are executed on a Handler do not bubble up like normal JS events. They're captured in an effort to ensure that Handlers remain encapsulated.\r\n\r\nHowever, in some cases we have needed Handlers to talk to each other. This is done by white-listing certain events to be \"published\", using the `publishEvent`, `trigger` and `triggerPublicEvent_` methods. These published events are triggered on the Handler's parent element in the DOM, from which they can bubble up.\r\n\r\nThis event is captured by a parent DOM element which has a handler attached, typically a complex Handler like a grid or the `SiteHandler`.\r\n\r\nBecause modals exist outside the normal DOM hierarchy, bubbling doesn't really get the job done. To accomodate this, there's a `eventBridge` option which can be passed to Handlers. This is used by modals to redirect events to the `LinkAction` which opened the modal.\r\n\r\nThe `notifyUser` event also gets it's own routing. Several handlers deliberately bubble the event up to the SiteHandler, which then makes a request to the server to decide what notifications to display.\r\n\r\nSo we have two ways for Handlers which are separate in the DOM (ie - not nested) to communicate with each other: an event that bubbles to `SiteHandler` and is then re-routed by `SiteHandler`, or an event bridge that reaches out to another element with a jQuery selector.\r\n\r\n### The maintenance problem\r\n\r\nThis architecture has worked to keep the Handlers highly re-usable. But it means that in cases where we do need Handlers to interact, including in cases where bubbling may not be sufficient, we have written a lot of code to reach out from one Handler to another.\r\n\r\nThe result has been that the connections between coupled Handlers tend to be ad-hoc, written into whatever method of the Handler happens to be doing that job right then. As a result, it's hard to have a clear sense of the dependencies between Handlers where they do exist, and to guard against breaking these when refactoring.\r\n\r\n### The (first) proposed solution\r\n\r\nI looked into addressing this with the [observer pattern](https://en.wikipedia.org/wiki/Observer_pattern), something I was familiar with from the [Backbone.js](http://backbonejs.org/) framework. It would also give us an opportunity to start piecemeal refactoring towards a more common approach to JS-heavy applications.\r\n\r\n## The blocking problem with implementing the observer pattern\r\n\r\nThe observer pattern can’t be implemented because we lack a central registry of handlers. A handler which updates its content will remove large chunks of the DOM without tracking which handlers were caught up in the destruction. We will have references to listeners that are no longer present in the DOM (the [lapsed listener problem](https://en.wikipedia.org/wiki/Lapsed_listener_problem)).\r\n\r\nThe usual way to resolve this is with a central registry of handlers which deregister their events when removed (the [dispose pattern](https://en.wikipedia.org/wiki/Dispose_pattern)). But because a handler doesn’t know what handlers are caught up in it’s refresh operations, it can’t trigger deregistration on embedded handlers (eg - a `LinkActionHandler` that's part of a `GridHandler`).\r\n\r\n### A new proposal\r\n\r\nWithout a significant refactor, we can’t have truly decoupled handlers. Instead, I’d suggest using a modified observer pattern in which the relationships between events and listeners are stored explicitly in the observer’s code. So handlers would not subscribe to events on the observer. Instead, the observer would have explicit instructions for passing specific events to specific handlers.\r\nThe typical observer pattern looks like this:\r\n\r\n```\r\nSender.init() {\r\n\tObserver.trigger(‘changeEvent’);\r\n}\r\nListener.init() {\r\n\tObserver.listenTo(‘changeEvent’, Listener.respondToChangeEvent);\r\n}\r\n```\r\n\r\nThe Observer can be completely ignorant of the events that are passing through it. The modified observer pattern I’m proposing would nest lookup logic in the Observer.\r\n\r\n```\r\nSender.init() {\r\n\tObserver.trigger(‘changeEvent’, eventData);\r\n}\r\nListener.init() {\r\n\tthis.bind(‘changeEvent’, Listener.respondToChangeEvent);\r\n}\r\nObserver.trigger(event, eventData) {\r\n\tif (event === ‘changeEvent’) {\r\n\t\t$(‘.listener’).trigger(‘changeEvent’, eventData);\r\n\t}\r\n}\r\n```\r\n\r\nThe Observer would not store references to handlers. Instead, it would look up any specific handlers by their associated DOM elements, performing a fresh lookup each time an event was triggered. \r\nThis would provide decoupling of a sort, though the coupling would still have to be specified in a central Observer. It’s only marginally better than the current practice of triggering an event from one handler on another handler’s DOM element directly.\r\n\r\n### But is it really an improvement?\r\n\r\nRelying on DOM lookups is not something I feel good about, to be honest. All this pattern really does is restructure the hacky DOM-based lookups away from the Handler into a central location ([example](https://github.com/NateWr/pkp-lib/commit/4ca7186a9b89272b00fd3cd33d5fcea945c407d6#diff-bb2511829d5d9a825dd2e6e3f4d82fddR34)). I think it will help (a little) with the maintenance issue by exposing these relationships more clearly. But the same could be done in theory with the existing `eventBridge` technique.\r\n\r\nI don't see a way around these DOM lookup approaches without a pretty significant refactor of our Handlers. I've been wracking my brain but I haven't come up with anything.\r\n\r\nOne very simple refactor that I have made which I think is genuinely useful is that, on the PHP side, the `JSONMessage` object now supports multiple events. So you can register more than one event to return in a JSON response. Example: when publishing an issue you can return the `dataChanged` event that the grid is programmed to respond to, as well as a `issuePublished` event that can be broadcast separately.\r\n\r\nThe global event router, combined with multiple events in a `JSONMessage`, could be a good pattern to adopt more broadly (triggering notifications, tasks, etc from the PHP side). But I'm not sure it gets us very far down the road I originally had in mind of an event-driven UI with stronger decoupling of Handlers.\r\n\r\n## Examples\r\n\r\npkp-lib: https://github.com/NateWr/pkp-lib/commits/i2163_event_router\r\nojs: https://github.com/NateWr/ojs/tree/i2163_event_router"},"comment":{"url":"https://api.github.com/repos/pkp/pkp-lib/issues/comments/318201843","html_url":"https://github.com/pkp/pkp-lib/issues/2163#issuecomment-318201843","issue_url":"https://api.github.com/repos/pkp/pkp-lib/issues/2163","id":318201843,"user":{"login":"asmecher","id":200411,"avatar_url":"https://avatars0.githubusercontent.com/u/200411?v=4","gravatar_id":"","url":"https://api.github.com/users/asmecher","html_url":"https://github.com/asmecher","followers_url":"https://api.github.com/users/asmecher/followers","following_url":"https://api.github.com/users/asmecher/following{/other_user}","gists_url":"https://api.github.com/users/asmecher/gists{/gist_id}","starred_url":"https://api.github.com/users/asmecher/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/asmecher/subscriptions","organizations_url":"https://api.github.com/users/asmecher/orgs","repos_url":"https://api.github.com/users/asmecher/repos","events_url":"https://api.github.com/users/asmecher/events{/privacy}","received_events_url":"https://api.github.com/users/asmecher/received_events","type":"User","site_admin":false},"created_at":"2017-07-26T22:30:45Z","updated_at":"2017-07-26T22:30:45Z","body":"All right, it's merged! @kaschioudi, there are some warnings I'm seeing:\r\n\r\n````\r\nPHP Notice:  Undefined index: collation in /home/asmecher/git/ojs/lib/pkp/lib/vendor/illuminate/database/Illuminate/Database/Connectors/MySqlConnector.php on line 22\r\n[26-Jul-2017 22:31:22 UTC] PHP Stack trace:\r\n[26-Jul-2017 22:31:22 UTC] PHP   1. {main}() /home/asmecher/git/ojs/index.php:0\r\n[26-Jul-2017 22:31:22 UTC] PHP   2. PKPApplication->execute() /home/asmecher/git/ojs/index.php:68\r\n[26-Jul-2017 22:31:22 UTC] PHP   3. Dispatcher->dispatch() /home/asmecher/git/ojs/lib/pkp/classes/core/PKPApplication.inc.php:239\r\n[26-Jul-2017 22:31:22 UTC] PHP   4. APIRouter->route() /home/asmecher/git/ojs/lib/pkp/classes/core/Dispatcher.inc.php:134\r\n[26-Jul-2017 22:31:22 UTC] PHP   5. Slim\\App->run() /home/asmecher/git/ojs/lib/pkp/classes/core/APIRouter.inc.php:110\r\n[26-Jul-2017 22:31:22 UTC] PHP   6. Slim\\App->process() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/App.php:295\r\n[26-Jul-2017 22:31:22 UTC] PHP   7. Slim\\App->callMiddlewareStack() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/App.php:370\r\n[26-Jul-2017 22:31:22 UTC] PHP   8. Slim\\App->Slim\\{closure}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:122\r\n[26-Jul-2017 22:31:22 UTC] PHP   9. call_user_func:{/home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73\r\n[26-Jul-2017 22:31:22 UTC] PHP  10. Slim\\DeferredCallable->__invoke() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73\r\n[26-Jul-2017 22:31:22 UTC] PHP  11. call_user_func_array:{/home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43\r\n[26-Jul-2017 22:31:22 UTC] PHP  12. APIHandler->{closure:/home/asmecher/git/ojs/lib/pkp/classes/handler/APIHandler.inc.php:68-87}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43\r\n[26-Jul-2017 22:31:22 UTC] PHP  13. Slim\\App->Slim\\{closure}() /home/asmecher/git/ojs/lib/pkp/classes/handler/APIHandler.inc.php:86\r\n[26-Jul-2017 22:31:22 UTC] PHP  14. call_user_func:{/home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73\r\n[26-Jul-2017 22:31:22 UTC] PHP  15. Slim\\DeferredCallable->__invoke() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73\r\n[26-Jul-2017 22:31:22 UTC] PHP  16. call_user_func_array:{/home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43\r\n[26-Jul-2017 22:31:22 UTC] PHP  17. APIHandler->{closure:/home/asmecher/git/ojs/lib/pkp/classes/handler/APIHandler.inc.php:51-65}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43\r\n[26-Jul-2017 22:31:22 UTC] PHP  18. Slim\\App->Slim\\{closure}() /home/asmecher/git/ojs/lib/pkp/classes/handler/APIHandler.inc.php:64\r\n[26-Jul-2017 22:31:22 UTC] PHP  19. call_user_func:{/home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73\r\n[26-Jul-2017 22:31:22 UTC] PHP  20. Slim\\DeferredCallable->__invoke() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:73\r\n[26-Jul-2017 22:31:22 UTC] PHP  21. call_user_func_array:{/home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43\r\n[26-Jul-2017 22:31:22 UTC] PHP  22. ApiAuthorizationMiddleware->__invoke() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/DeferredCallable.php:43\r\n[26-Jul-2017 22:31:22 UTC] PHP  23. Slim\\App->__invoke() /home/asmecher/git/ojs/lib/pkp/classes/security/authorization/internal/ApiAuthorizationMiddleware.inc.php:74\r\n[26-Jul-2017 22:31:22 UTC] PHP  24. Slim\\Route->run() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/App.php:476\r\n[26-Jul-2017 22:31:22 UTC] PHP  25. Slim\\Route->callMiddlewareStack() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/Route.php:316\r\n[26-Jul-2017 22:31:22 UTC] PHP  26. Slim\\Route->__invoke() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/MiddlewareAwareTrait.php:122\r\n[26-Jul-2017 22:31:22 UTC] PHP  27. Slim\\Handlers\\Strategies\\RequestResponse->__invoke() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/Route.php:344\r\n[26-Jul-2017 22:31:22 UTC] PHP  28. call_user_func:{/home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php:41}() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php:41\r\n[26-Jul-2017 22:31:22 UTC] PHP  29. PKPBackendSubmissionsHandler->getSubmissions() /home/asmecher/git/ojs/lib/pkp/lib/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php:41\r\n[26-Jul-2017 22:31:22 UTC] PHP  30. PKP\\Services\\PKPSubmissionService->getSubmissionList() /home/asmecher/git/ojs/lib/pkp/api/v1/_submissions/PKPBackendSubmissionsHandler.inc.php:137\r\n[26-Jul-2017 22:31:22 UTC] PHP  31. PKP\\Services\\QueryBuilders\\PKPSubmissionListQueryBuilder->get() /home/asmecher/git/ojs/lib/pkp/classes/services/PKPSubmissionService.inc.php:71\r\n[26-Jul-2017 22:31:22 UTC] PHP  32. Illuminate\\Database\\Capsule\\Manager::table() /home/asmecher/git/ojs/lib/pkp/classes/services/queryBuilders/PKPSubmissionListQueryBuilder.inc.php:134\r\n[26-Jul-2017 22:31:22 UTC] PHP  33. Illuminate\\Database\\Capsule\\Manager::connection() /home/asmecher/git/ojs/lib/pkp/lib/vendor/illuminate/database/Illuminate/Database/Capsule/Manager.php:110\r\n[26-Jul-2017 22:31:22 UTC] PHP  34. Illuminate\\Database\\Capsule\\Manager->getConnection() /home/asmecher/git/ojs/lib/pkp/lib/vendor/illuminate/database/Illuminate/Database/Capsule/Manager.php:98\r\n[26-Jul-2017 22:31:22 UTC] PHP  35. Illuminate\\Database\\DatabaseManager->connection() /home/asmecher/git/ojs/lib/pkp/lib/vendor/illuminate/database/Illuminate/Database/Capsule/Manager.php:132\r\n[26-Jul-2017 22:31:22 UTC] PHP  36. Illuminate\\Database\\DatabaseManager->makeConnection() /home/asmecher/git/ojs/lib/pkp/lib/vendor/illuminate/database/Illuminate/Database/DatabaseManager.php:63\r\n[26-Jul-2017 22:31:22 UTC] PHP  37. Illuminate\\Database\\Connectors\\ConnectionFactory->make() /home/asmecher/git/ojs/lib/pkp/lib/vendor/illuminate/database/Illuminate/Database/DatabaseManager.php:127\r\n[26-Jul-2017 22:31:22 UTC] PHP  38. Illuminate\\Database\\Connectors\\ConnectionFactory->createSingleConnection() /home/asmecher/git/ojs/lib/pkp/lib/vendor/illuminate/database/Illuminate/Database/Connectors/ConnectionFactory.php:47\r\n[26-Jul-2017 22:31:22 UTC] PHP  39. Illuminate\\Database\\Connectors\\MySqlConnector->connect() /home/asmecher/git/ojs/lib/pkp/lib/vendor/illuminate/database/Illuminate/Database/Connectors/ConnectionFactory.php:59\r\n````"}}
{"actor":{"display_login":"asmecher"}}