diff options
author | Paladox none <thomasmulhall410@yahoo.com> | 2019-02-08 17:14:15 +0000 |
---|---|---|
committer | Paladox none <thomasmulhall410@yahoo.com> | 2019-03-05 08:50:13 +0000 |
commit | 0ee62135f2f91a500fc15953b14b99e74d95abb4 (patch) | |
tree | e50c6415fceb5ffeaf92dbe0b76150558d49091c | |
parent | 002c7931e2265a54c3890348f9f6ee16d7f67765 (diff) |
Fix avatars not showing correctly
Sometimes avatars do not load on the change metadata or the dashboard.
I found this is because the this._account check sometimes returns null
which then cause it to hit the this.hidden = true. Because there was
no setting hidden back to false, gr-avatar stayed permanently hidden
even if "account" became defined.
Bug: Issue 9851
Change-Id: I2627d7c830fc8825790f15359024496ff9e0e6be
-rw-r--r-- | polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.js | 21 | ||||
-rw-r--r-- | polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.html | 70 |
2 files changed, 78 insertions, 13 deletions
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.js b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.js index f32e940b3f..bf563823ac 100644 --- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.js +++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.js @@ -41,25 +41,30 @@ attached() { Promise.all([ - this.$.restAPI.getConfig(), + this._getConfig(), Gerrit.awaitPluginsLoaded(), ]).then(([cfg]) => { this._hasAvatars = !!(cfg && cfg.plugin && cfg.plugin.has_avatars); - if (this._hasAvatars && this.account) { - // src needs to be set if avatar becomes visible - this._updateAvatarURL(); - } else { - this.hidden = true; - } + + this._updateAvatarURL(); }); }, + _getConfig() { + return this.$.restAPI.getConfig(); + }, + _accountChanged(account) { this._updateAvatarURL(); }, _updateAvatarURL() { - if (this.hidden || !this._hasAvatars) { return; } + if (!this._hasAvatars || !this.account) { + this.hidden = true; + return; + } + this.hidden = false; + const url = this._buildAvatarURL(this.account); if (url) { this.style.backgroundImage = 'url("' + url + '")'; diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.html b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.html index f137c7fa01..5ce17c05b9 100644 --- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.html +++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.html @@ -35,14 +35,17 @@ limitations under the License. <script> suite('gr-avatar tests', () => { let element; + let sandbox; setup(() => { - stub('gr-rest-api-interface', { - getConfig() { return Promise.resolve({plugin: {has_avatars: true}}); }, - }); + sandbox = sinon.sandbox.create(); element = fixture('basic'); }); + teardown(() => { + sandbox.restore(); + }); + test('methods', () => { assert.equal(element._buildAvatarURL( { @@ -94,22 +97,32 @@ limitations under the License. ], }), '/accounts/123/avatar?s=16'); + assert.equal(element._buildAvatarURL(undefined), ''); }); test('dom for existing account', () => { assert.isFalse(element.hasAttribute('hidden')); + + sandbox.stub(element, '_getConfig', () => { + return Promise.resolve({plugin: {has_avatars: true}}); + }); + element.imageSize = 64; element.account = { _account_id: 123, }; + assert.strictEqual(element.style.backgroundImage, ''); + // Emulate plugins loaded. Gerrit._setPluginsPending([]); - return Promise.all([ + + Promise.all([ element.$.restAPI.getConfig(), Gerrit.awaitPluginsLoaded(), ]).then(() => { assert.isFalse(element.hasAttribute('hidden')); + assert.isTrue( element.style.backgroundImage.includes('/accounts/123/avatar?s=64')); }); @@ -117,10 +130,57 @@ limitations under the License. test('dom for non available account', () => { assert.isFalse(element.hasAttribute('hidden')); - element.account = null; + + sandbox.stub(element, '_getConfig', () => { + return Promise.resolve({plugin: {has_avatars: true}}); + }); + + // Emulate plugins loaded. + Gerrit._setPluginsPending([]); + + return Promise.all([ + element.$.restAPI.getConfig(), + Gerrit.awaitPluginsLoaded(), + ]).then(() => { + assert.isTrue(element.hasAttribute('hidden')); + + assert.strictEqual(element.style.backgroundImage, ''); + }); + }); + + test('avatar config not set and account not set', () => { + assert.isFalse(element.hasAttribute('hidden')); + + sandbox.stub(element, '_getConfig', () => { + return Promise.resolve({}); + }); + + // Emulate plugins loaded. + Gerrit._setPluginsPending([]); + + return Promise.all([ + element.$.restAPI.getConfig(), + Gerrit.awaitPluginsLoaded(), + ]).then(() => { + assert.isTrue(element.hasAttribute('hidden')); + }); + }); + + test('avatar config not set and account set', () => { assert.isFalse(element.hasAttribute('hidden')); + + sandbox.stub(element, '_getConfig', () => { + return Promise.resolve({}); + }); + + element.imageSize = 64; + element.account = { + _account_id: 123, + }; + // Emulate plugins loaded. Gerrit._setPluginsPending([]); + return Promise.all([ element.$.restAPI.getConfig(), Gerrit.awaitPluginsLoaded(), |