Skip to content

Commit

Permalink
FIX: more resilient lookup in plugin-api (#6961)
Browse files Browse the repository at this point in the history
Ember3 is more picky about having a container being destroyed and it's easier to cause exceptions, especially in tests.

This fix has been initially created for an exception occuring in tests when running discourse-code-review and discourse-polls tests at the same time. `getCurrentUser` method body was failing as the container was destroyed.

Original  stacktrace:

```
"Error: Assertion Failed: expected container not to be destroyed
    at new EmberError (ember:2929:31)
    at assert (ember:1793:23)
    at Container.lookup (ember:17736:64)
    at PluginApi.getCurrentUser (discourse/lib/plugin-api:56:31)
    at allowUser (javascripts/discourse/initializers/init-code-review:38:29)
    at eval (javascripts/discourse/initializers/init-code-review:78:11)
    at eval (select-kit/mixins/plugin-api:86:19)
    at Array.forEach (<anonymous>)
    at eval (select-kit/mixins/plugin-api:85:44)
    at Array.forEach (<anonymous>)"
```
  • Loading branch information
jjaffeux committed Jan 29, 2019
1 parent fb7f21f commit 0d0303e
Showing 1 changed file with 18 additions and 5 deletions.
23 changes: 18 additions & 5 deletions app/assets/javascripts/discourse/lib/plugin-api.js.es6
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,19 @@ class PluginApi {
* If the user is not logged in, it will be `null`.
**/
getCurrentUser() {
return this.container.lookup("current-user:main");
return this._lookupContainer("current-user:main");
}

_lookupContainer(path) {
if (
!this.container ||
this.container.isDestroying ||
this.container.isDestroyed
) {
return;
}

return this.container.lookup(path);
}

_resolveClass(resolverName, opts) {
Expand Down Expand Up @@ -222,7 +234,7 @@ class PluginApi {
* ```
**/
addPosterIcon(cb) {
const site = this.container.lookup("site:main");
const site = this._lookupContainer("site:main");
const loc = site && site.mobileView ? "before" : "after";

decorateWidget(`poster-name:${loc}`, dec => {
Expand Down Expand Up @@ -424,8 +436,8 @@ class PluginApi {
```
**/
onAppEvent(name, fn) {
let appEvents = this.container.lookup("app-events:main");
appEvents.on(name, fn);
const appEvents = this._lookupContainer("app-events:main");
appEvents && appEvents.on(name, fn);
}

/**
Expand Down Expand Up @@ -562,7 +574,8 @@ class PluginApi {
* will issue a request to `/mice.json`
**/
addStorePluralization(thing, plural) {
this.container.lookup("service:store").addPluralization(thing, plural);
const store = this._lookupContainer("service:store");
store && store.addPluralization(thing, plural);
}

/**
Expand Down

0 comments on commit 0d0303e

Please sign in to comment.