From dda5d74f1e44be1623b4b03d2797a2836a8f2778 Mon Sep 17 00:00:00 2001 From: Jared Galanis Date: Fri, 12 Jan 2024 10:35:19 -0500 Subject: [PATCH] fix: favicon called repeatedly (#1246) - also refactors use of app-static-config where needed --- app/components/found-manuscripts/index.js | 3 +- app/components/nav-bar/index.js | 4 +-- app/components/notice-banner/index.js | 8 +++-- app/components/workflow-basics/index.js | 3 +- app/components/workflow-grants/index.js | 3 +- app/controllers/grants/index.js | 4 +-- app/controllers/submissions/index.js | 4 +-- app/controllers/thanks.js | 6 ++-- app/routes/application.js | 27 ++-------------- app/routes/not-found-error.js | 2 +- app/services/app-static-config.js | 24 ++++++++++++++ tests/integration/components/nav-bar-test.js | 20 ++++++------ .../components/notice-banner-test.js | 18 +++++------ tests/integration/helpers/format-date-test.js | 1 - tests/unit/controllers/grants/index-test.js | 4 +-- .../controllers/submissions/index-test.js | 4 +-- tests/unit/controllers/thanks-test.js | 31 ------------------- tests/unit/routes/thanks-test.js | 12 ------- 18 files changed, 68 insertions(+), 110 deletions(-) delete mode 100644 tests/unit/controllers/thanks-test.js delete mode 100644 tests/unit/routes/thanks-test.js diff --git a/app/components/found-manuscripts/index.js b/app/components/found-manuscripts/index.js index fefbf132c..475376136 100644 --- a/app/components/found-manuscripts/index.js +++ b/app/components/found-manuscripts/index.js @@ -38,8 +38,7 @@ export default class FoundManuscriptsComponent extends Component { @task getAppConfig = function* () { - let config = yield this.appStaticConfig.getStaticConfig(); - this.contactUrl = config.branding.pages.contactUrl; + this.contactUrl = yield this.appStaticConfig.config?.branding?.pages?.contactUrl; }; @task diff --git a/app/components/nav-bar/index.js b/app/components/nav-bar/index.js index cdcd8f9e2..dfb91f275 100644 --- a/app/components/nav-bar/index.js +++ b/app/components/nav-bar/index.js @@ -46,8 +46,8 @@ export default class NavBar extends Component { @task _setupAppStaticConfig = function* () { - let config = yield this.appStaticConfig.getStaticConfig(); - if (config.branding.showPagesNavBar) { + let config = yield this.appStaticConfig.config; + if (config && config.branding.showPagesNavBar) { this.aboutUrl = config.branding.pages.aboutUrl; this.contactUrl = config.branding.pages.contactUrl; this.faqUrl = config.branding.pages.faqUrl; diff --git a/app/components/notice-banner/index.js b/app/components/notice-banner/index.js index 0ee09d811..a23c25736 100644 --- a/app/components/notice-banner/index.js +++ b/app/components/notice-banner/index.js @@ -21,8 +21,10 @@ export default class NoticeBanner extends Component { @task _setupAppStaticConfig = function* () { - let config = yield this.appStaticConfig.getStaticConfig(); - this.contactUrl = config.branding.pages.contactUrl; - this.instructionsUrl = config.branding.pages.instructionsUrl; + let config = yield this.appStaticConfig.config; + if (config) { + this.contactUrl = config.branding.pages.contactUrl; + this.instructionsUrl = config.branding.pages.instructionsUrl; + } }; } diff --git a/app/components/workflow-basics/index.js b/app/components/workflow-basics/index.js index ed52483a4..ad630f5c3 100644 --- a/app/components/workflow-basics/index.js +++ b/app/components/workflow-basics/index.js @@ -69,8 +69,7 @@ export default class WorkflowBasics extends Component { } async setupConfig() { - let config = await this.appStaticConfig.getStaticConfig(); - this.contactUrl = config.branding.pages.contactUrl; + this.contactUrl = this.appStaticConfig?.config?.branding?.pages?.contactUrl; } setPreparers() { diff --git a/app/components/workflow-grants/index.js b/app/components/workflow-grants/index.js index fdf5179f8..5d11c32e4 100644 --- a/app/components/workflow-grants/index.js +++ b/app/components/workflow-grants/index.js @@ -90,8 +90,7 @@ export default class WorkflowGrants extends Component { @task setup = function* () { - let config = yield this.appStaticConfig.getStaticConfig(); - this.contactUrl = config.branding.pages.contactUrl; + this.contactUrl = this.appStaticConfig?.config?.branding?.pages?.contactUrl; if (get(this, 'args.submission.submitter.id')) { yield this.updateGrants.perform(); diff --git a/app/controllers/grants/index.js b/app/controllers/grants/index.js index b977dddc6..a44a9b898 100644 --- a/app/controllers/grants/index.js +++ b/app/controllers/grants/index.js @@ -8,7 +8,7 @@ import { grantsIndexGrantQuery, grantsIndexSubmissionQuery } from '../../util/pa export default class GrantsIndexController extends Controller { @service currentUser; - @service('app-static-config') configurator; + @service('app-static-config') staticConfig; @service('emt-themes/bootstrap4') themeInstance; @service router; @service store; @@ -16,7 +16,7 @@ export default class GrantsIndexController extends Controller { constructor() { super(...arguments); - this.configurator.getStaticConfig().then((config) => this.set('faqUrl', config.branding.pages.faqUrl)); + this.faqUrl = this.staticConfig.config.branding.pages.faqUrl; } // TODO Reduce duplication in column definitions diff --git a/app/controllers/submissions/index.js b/app/controllers/submissions/index.js index 0849329cc..a408b3939 100644 --- a/app/controllers/submissions/index.js +++ b/app/controllers/submissions/index.js @@ -7,7 +7,7 @@ import { submissionsIndexQuery } from '../../util/paginated-query'; export default class SubmissionsIndex extends Controller { @service currentUser; - @service('app-static-config') configurator; + @service('app-static-config') staticConfig; @service('emt-themes/bootstrap4') themeInstance; @service router; @service store; @@ -38,7 +38,7 @@ export default class SubmissionsIndex extends Controller { constructor() { super(...arguments); - this.configurator.getStaticConfig().then((config) => this.set('faqUrl', config.branding.pages.faqUrl)); + this.faqUrl = this.staticConfig.config.branding.pages.faqUrl; } // Columns displayed depend on the user role diff --git a/app/controllers/thanks.js b/app/controllers/thanks.js index 363979814..d6ad49ddd 100644 --- a/app/controllers/thanks.js +++ b/app/controllers/thanks.js @@ -6,14 +6,14 @@ import { inject as service } from '@ember/service'; export default class ThanksController extends Controller { queryParams = ['submission']; - @service('app-static-config') - configurator; + @service('app-static-config') staticConfig; @tracked submission = null; @tracked faqUrl = null; constructor() { super(...arguments); - this.configurator.getStaticConfig().then((config) => this.set('faqUrl', config.branding.pages.faqUrl)); + + this.faqUrl = this.staticConfig.config.branding.pages.faqUrl; } } diff --git a/app/routes/application.js b/app/routes/application.js index 3d2a95ed9..7d31b2088 100644 --- a/app/routes/application.js +++ b/app/routes/application.js @@ -31,38 +31,17 @@ export default class ApplicationRoute extends CheckSessionRoute { } } - async model() { - const config = await this.staticConfig.getStaticConfig(); - - return { - staticConfig: config, - }; - } - /** * Add styling from static branding. TODO: Should this be moved to an initializer or something? */ - afterModel(model, transition) { + async afterModel(model, transition) { const loader = document.getElementById('initial-loader'); if (loader) { loader.style.display = 'none'; } - if (model.staticConfig) { - if (model.staticConfig.branding.stylesheet) { - const stylesheet = `${model.staticConfig.branding.stylesheet}`; - this.staticConfig.addCSS(stylesheet); - } else { - console.log('%cNo branding stylesheet was configured', 'color:red'); - } - if (model.staticConfig.branding.overrides) { - const overrides = `${model.staticConfig.branding.overrides}`; - this.staticConfig.addCSS(overrides); - } - if (model.staticConfig.branding.favicon) { - const favicon = `${model.staticConfig.branding.favicon}`; - this.staticConfig.addFavicon(favicon); - } + if (!this.staticConfig.config) { + await this.staticConfig.setupStaticConfig(); } } } diff --git a/app/routes/not-found-error.js b/app/routes/not-found-error.js index e82fab9be..afd29425e 100644 --- a/app/routes/not-found-error.js +++ b/app/routes/not-found-error.js @@ -7,7 +7,7 @@ export default class NotFoundErrorRoute extends CheckSessionRoute { model() { return RSVP.hash({ - config: this.staticConfig.getStaticConfig(), + config: this.staticConfig.config, }); } } diff --git a/app/services/app-static-config.js b/app/services/app-static-config.js index c72d25617..b56b6138c 100644 --- a/app/services/app-static-config.js +++ b/app/services/app-static-config.js @@ -15,6 +15,30 @@ export default class AppStaticConfigService extends Service { this.configUrl = ENV.APP.staticConfigUri; } + get config() { + return this._config; + } + + async setupStaticConfig() { + await this.getStaticConfig(); + if (this._config) { + if (this._config.branding.stylesheet) { + const stylesheet = `${this._config.branding.stylesheet}`; + this.addCSS(stylesheet); + } else { + console.log('%cNo branding stylesheet was configured', 'color:red'); + } + if (this._config.branding.overrides) { + const overrides = `${this._config.branding.overrides}`; + this.addCSS(overrides); + } + if (this._config.branding.favicon) { + const favicon = `${this._config.branding.favicon}`; + this.addFavicon(favicon); + } + } + } + /** * Get the static configuration for PASS -- from ENV vars * diff --git a/tests/integration/components/nav-bar-test.js b/tests/integration/components/nav-bar-test.js index e6f9dec06..6ead993ee 100644 --- a/tests/integration/components/nav-bar-test.js +++ b/tests/integration/components/nav-bar-test.js @@ -1,3 +1,4 @@ +/* eslint-disable ember/avoid-leaking-state-in-ember-objects */ /* eslint-disable ember/no-classic-classes */ import Service from '@ember/service'; import { setupRenderingTest } from 'ember-qunit'; @@ -10,17 +11,16 @@ module('Integration | Component | nav bar', (hooks) => { test('it renders', async function (assert) { const mockStaticConfig = Service.extend({ - getStaticConfig: () => - Promise.resolve({ - branding: { - stylesheet: '', - pages: { - aboutUrl: '', - contactUrl: '', - faqUrl: '', - }, + config: { + branding: { + stylesheet: '', + pages: { + aboutUrl: '', + contactUrl: '', + faqUrl: '', }, - }), + }, + }, addCss: () => {}, }); diff --git a/tests/integration/components/notice-banner-test.js b/tests/integration/components/notice-banner-test.js index 7e5c20f50..404101169 100644 --- a/tests/integration/components/notice-banner-test.js +++ b/tests/integration/components/notice-banner-test.js @@ -1,3 +1,4 @@ +/* eslint-disable ember/avoid-leaking-state-in-ember-objects */ /* eslint-disable ember/no-classic-classes */ import Service from '@ember/service'; import { setupRenderingTest } from 'ember-qunit'; @@ -10,16 +11,15 @@ module('Integration | Component | notice-banner', (hooks) => { test('it renders', async function (assert) { const mockStaticConfig = Service.extend({ - getStaticConfig: () => - Promise.resolve({ - branding: { - stylesheet: '', - pages: { - contactUrl: 'http://test-contact/', - instructionsUrl: 'http://test-instructions/', - }, + config: { + branding: { + stylesheet: '', + pages: { + contactUrl: 'http://test-contact/', + instructionsUrl: 'http://test-instructions/', }, - }), + }, + }, addCss: () => {}, }); diff --git a/tests/integration/helpers/format-date-test.js b/tests/integration/helpers/format-date-test.js index 8d1ac376f..50d3ecf1d 100644 --- a/tests/integration/helpers/format-date-test.js +++ b/tests/integration/helpers/format-date-test.js @@ -6,7 +6,6 @@ import { module, test } from 'qunit'; module('Integration | Helper | format date', (hooks) => { setupRenderingTest(hooks); - // Replace this with your real tests. test('it renders', async function (assert) { // eslint-disable-line this.set('inputValue', 'August 19, 1975 23:15:30'); diff --git a/tests/unit/controllers/grants/index-test.js b/tests/unit/controllers/grants/index-test.js index 010602158..0af1bb363 100644 --- a/tests/unit/controllers/grants/index-test.js +++ b/tests/unit/controllers/grants/index-test.js @@ -5,8 +5,8 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; class MockConfigService extends Service { - getStaticConfig() { - return Promise.resolve({ branding: { stylesheet: '', pages: { faqUrl: '' } } }); + get config() { + return { branding: { stylesheet: '', pages: { faqUrl: '' } } }; } addCss() {} } diff --git a/tests/unit/controllers/submissions/index-test.js b/tests/unit/controllers/submissions/index-test.js index 99df4093e..91adac303 100644 --- a/tests/unit/controllers/submissions/index-test.js +++ b/tests/unit/controllers/submissions/index-test.js @@ -3,8 +3,8 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; class MockConfigService extends Service { - getStaticConfig() { - return Promise.resolve({ branding: { stylesheet: '', pages: { faqUrl: '' } } }); + get config() { + return { branding: { stylesheet: '', pages: { faqUrl: '' } } }; } addCss() {} } diff --git a/tests/unit/controllers/thanks-test.js b/tests/unit/controllers/thanks-test.js deleted file mode 100644 index c74e9d717..000000000 --- a/tests/unit/controllers/thanks-test.js +++ /dev/null @@ -1,31 +0,0 @@ -/* eslint-disable ember/no-classic-classes */ -import Service from '@ember/service'; -import { module, test } from 'qunit'; -import { setupTest } from 'ember-qunit'; - -module('Unit | Controller | thanks', (hooks) => { - setupTest(hooks); - - hooks.beforeEach(function () { - const mockStaticConfig = Service.extend({ - getStaticConfig: () => - Promise.resolve({ - branding: { - stylesheet: '', - pages: { - faqUrl: '', - }, - }, - }), - addCss: () => {}, - }); - - this.owner.register('service:app-static-config', mockStaticConfig); - }); - - // Replace this with your real tests. - test('it exists', function (assert) { - let controller = this.owner.lookup('controller:thanks'); - assert.ok(controller); - }); -}); diff --git a/tests/unit/routes/thanks-test.js b/tests/unit/routes/thanks-test.js deleted file mode 100644 index 1c94a41c6..000000000 --- a/tests/unit/routes/thanks-test.js +++ /dev/null @@ -1,12 +0,0 @@ -import { module, test } from 'qunit'; -import { setupTest } from 'ember-qunit'; - -module('Unit | Route | thanks', (hooks) => { - setupTest(hooks); - - // Replace this with your real tests. - test('it exists', function (assert) { - let route = this.owner.lookup('route:thanks'); - assert.ok(route); - }); -});