From c9f772f497f0aecd430157c1b63f958f59fde0c4 Mon Sep 17 00:00:00 2001 From: Alf Eaton <75253002+aeaton-overleaf@users.noreply.github.com> Date: Mon, 29 Mar 2021 12:07:55 +0100 Subject: [PATCH] Ensure that Features.hasFeature returns a boolean (#3798) GitOrigin-RevId: d6f286544f42db4d101ba06897044ada0bcd14d8 --- .../web/app/src/infrastructure/Features.js | 63 +++++++++++++------ .../unit/src/infrastructure/FeaturesTests.js | 46 ++++++-------- 2 files changed, 65 insertions(+), 44 deletions(-) diff --git a/services/web/app/src/infrastructure/Features.js b/services/web/app/src/infrastructure/Features.js index 80550e1b57..2e235f5fbc 100644 --- a/services/web/app/src/infrastructure/Features.js +++ b/services/web/app/src/infrastructure/Features.js @@ -14,45 +14,72 @@ const trackChangesModuleAvailable = fs.existsSync( `${__dirname}/../../../modules/track-changes` ) +/** + * @typedef {Object} Settings + * @property {Object | undefined} apis + * @property {Object | undefined} apis.linkedUrlProxy + * @property {string | undefined} apis.linkedUrlProxy.url + * @property {Object | undefined} apis.references + * @property {string | undefined} apis.references.url + * @property {boolean | undefined} enableGithubSync + * @property {boolean | undefined} enableGitBridge + * @property {boolean | undefined} enableHomepage + * @property {boolean | undefined} enableSaml + * @property {boolean | undefined} ldap + * @property {boolean | undefined} oauth + * @property {Object | undefined} overleaf + * @property {Object | undefined} overleaf.oauth + * @property {boolean | undefined} saml + */ + const Features = { + /** + * @returns {boolean} + */ externalAuthenticationSystemUsed() { return ( - !!Settings.ldap || - !!Settings.saml || - !!_.get(Settings, ['overleaf', 'oauth']) + Boolean(Settings.ldap) || + Boolean(Settings.saml) || + Boolean(_.get(Settings, ['overleaf', 'oauth'])) ) }, + /** + * Whether a feature is enabled in the appliation's configuration + * + * @param {string} feature + * @returns {boolean} + */ hasFeature(feature) { switch (feature) { case 'homepage': - return Settings.enableHomepage + return Boolean(Settings.enableHomepage) case 'registration': - return !Features.externalAuthenticationSystemUsed() || Settings.overleaf + return ( + !Features.externalAuthenticationSystemUsed() || + Boolean(Settings.overleaf) + ) case 'github-sync': - return Settings.enableGithubSync + return Boolean(Settings.enableGithubSync) case 'git-bridge': - return Settings.enableGitBridge + return Boolean(Settings.enableGitBridge) case 'custom-togglers': - return !!Settings.overleaf + return Boolean(Settings.overleaf) case 'oauth': - return !!Settings.oauth + return Boolean(Settings.oauth) case 'templates-server-pro': - return Settings.overleaf == null + return !Settings.overleaf case 'affiliations': case 'analytics': - // Checking both properties is needed for the time being to allow - // enabling the feature in web-api and disabling in Server Pro - // see https://github.com/overleaf/web-internal/pull/2127 - return Settings.apis && Settings.apis.v1 && !!Settings.apis.v1.url + return Boolean(_.get(Settings, ['apis', 'v1', 'url'])) case 'overleaf-integration': - return !!Settings.overleaf + return Boolean(Settings.overleaf) case 'references': - return !!_.get(Settings, ['apis', 'references', 'url']) + return Boolean(_.get(Settings, ['apis', 'references', 'url'])) case 'saml': - return Settings.enableSaml + return Boolean(Settings.enableSaml) case 'link-url': - return !!_.get(Settings, ['apis', 'linkedUrlProxy', 'url']) + return Boolean(_.get(Settings, ['apis', 'linkedUrlProxy', 'url'])) case 'public-registration': return publicRegistrationModuleAvailable case 'support': diff --git a/services/web/test/unit/src/infrastructure/FeaturesTests.js b/services/web/test/unit/src/infrastructure/FeaturesTests.js index bc86811a5c..d1a0c48213 100644 --- a/services/web/test/unit/src/infrastructure/FeaturesTests.js +++ b/services/web/test/unit/src/infrastructure/FeaturesTests.js @@ -53,19 +53,17 @@ describe('Features', function() { expect(this.Features.hasFeature('templates-server-pro')).to.be.true }) it('should return false', function() { + expect(this.Features.hasFeature('affiliations')).to.be.false + expect(this.Features.hasFeature('analytics')).to.be.false expect(this.Features.hasFeature('custom-togglers')).to.be.false + expect(this.Features.hasFeature('git-bridge')).to.be.false + expect(this.Features.hasFeature('github-sync')).to.be.false + expect(this.Features.hasFeature('homepage')).to.be.false + expect(this.Features.hasFeature('link-url')).to.be.false expect(this.Features.hasFeature('oauth')).to.be.false expect(this.Features.hasFeature('overleaf-integration')).to.be.false expect(this.Features.hasFeature('references')).to.be.false - expect(this.Features.hasFeature('link-url')).to.be.false - }) - it('should return undefined', function() { - expect(this.Features.hasFeature('affiliations')).to.be.undefined - expect(this.Features.hasFeature('analytics')).to.be.undefined - expect(this.Features.hasFeature('github-sync')).to.be.undefined - expect(this.Features.hasFeature('git-bridge')).to.be.undefined - expect(this.Features.hasFeature('homepage')).to.be.undefined - expect(this.Features.hasFeature('saml')).to.be.undefined + expect(this.Features.hasFeature('saml')).to.be.false }) }) describe('with settings', function() { @@ -80,18 +78,16 @@ describe('Features', function() { expect(this.Features.hasFeature('registration')).to.be.true }) it('should return false', function() { + expect(this.Features.hasFeature('affiliations')).to.be.false + expect(this.Features.hasFeature('analytics')).to.be.false + expect(this.Features.hasFeature('git-bridge')).to.be.false + expect(this.Features.hasFeature('github-sync')).to.be.false + expect(this.Features.hasFeature('homepage')).to.be.false + expect(this.Features.hasFeature('link-url')).to.be.false expect(this.Features.hasFeature('oauth')).to.be.false expect(this.Features.hasFeature('references')).to.be.false + expect(this.Features.hasFeature('saml')).to.be.false expect(this.Features.hasFeature('templates-server-pro')).to.be.false - expect(this.Features.hasFeature('link-url')).to.be.false - }) - it('should return undefined', function() { - expect(this.Features.hasFeature('affiliations')).to.be.undefined - expect(this.Features.hasFeature('analytics')).to.be.undefined - expect(this.Features.hasFeature('github-sync')).to.be.undefined - expect(this.Features.hasFeature('git-bridge')).to.be.undefined - expect(this.Features.hasFeature('homepage')).to.be.undefined - expect(this.Features.hasFeature('saml')).to.be.undefined }) describe('with APIs', function() { beforeEach(function() { @@ -111,21 +107,19 @@ describe('Features', function() { expect(this.Features.hasFeature('affiliations')).to.be.true expect(this.Features.hasFeature('analytics')).to.be.true expect(this.Features.hasFeature('custom-togglers')).to.be.true - expect(this.Features.hasFeature('link-url')).to.equal(true) + expect(this.Features.hasFeature('link-url')).to.be.true expect(this.Features.hasFeature('overleaf-integration')).to.be.true expect(this.Features.hasFeature('references')).to.be.true expect(this.Features.hasFeature('registration')).to.be.true }) it('should return false', function() { + expect(this.Features.hasFeature('git-bridge')).to.be.false + expect(this.Features.hasFeature('github-sync')).to.be.false + expect(this.Features.hasFeature('homepage')).to.be.false expect(this.Features.hasFeature('oauth')).to.be.false + expect(this.Features.hasFeature('saml')).to.be.false expect(this.Features.hasFeature('templates-server-pro')).to.be.false }) - it('should return undefined', function() { - expect(this.Features.hasFeature('github-sync')).to.be.undefined - expect(this.Features.hasFeature('git-bridge')).to.be.undefined - expect(this.Features.hasFeature('homepage')).to.be.undefined - expect(this.Features.hasFeature('saml')).to.be.undefined - }) describe('with all other settings flags', function() { beforeEach(function() { this.settings.enableHomepage = true @@ -141,7 +135,7 @@ describe('Features', function() { expect(this.Features.hasFeature('github-sync')).to.be.true expect(this.Features.hasFeature('git-bridge')).to.be.true expect(this.Features.hasFeature('homepage')).to.be.true - expect(this.Features.hasFeature('link-url')).to.equal(true) + expect(this.Features.hasFeature('link-url')).to.be.true expect(this.Features.hasFeature('oauth')).to.be.true expect(this.Features.hasFeature('overleaf-integration')).to.be .true