diff --git a/packages/spacecat-shared-data-access/src/models/configuration/configuration.model.js b/packages/spacecat-shared-data-access/src/models/configuration/configuration.model.js index f8f3ac1d6..8c9f13dbd 100644 --- a/packages/spacecat-shared-data-access/src/models/configuration/configuration.model.js +++ b/packages/spacecat-shared-data-access/src/models/configuration/configuration.model.js @@ -174,6 +174,21 @@ class Configuration { .map((job) => job.type); } + /** + * Returns whether a handler is enabled for a given site. + * + * Precedence (highest to lowest): + * 1. disabled.sites includes siteId → false (site explicitly disabled) + * 2. enabled.sites includes siteId → true (site explicitly enabled; overrides org-level) + * 3. disabled.orgs includes orgId → false (org disabled, no site-level override present) + * 4. enabledByDefault → true + * 5. enabled.orgs includes orgId → true + * 6. otherwise → false + * + * Note: disabled.sites and enabled.sites are assumed disjoint; disabled.sites wins if violated + * (see #updatedHandler which keeps these lists disjoint on every write). + * isHandlerEnabledForOrg uses a different (simpler) precedence — see its JSDoc. + */ isHandlerEnabledForSite(type, site) { const handler = this.getHandlers()?.[type]; if (!handler) { @@ -183,26 +198,45 @@ class Configuration { const siteId = site.getId(); const orgId = site.getOrganizationId(); - if (handler.disabled) { - const sites = handler.disabled.sites || []; - const orgs = handler.disabled.orgs || []; - if (sites.includes(siteId) || orgs.includes(orgId)) { - return false; + const disabledSites = handler.disabled?.sites || []; + const enabledSites = handler.enabled?.sites || []; + const disabledOrgs = handler.disabled?.orgs || []; + const enabledOrgs = handler.enabled?.orgs || []; + + // Site-level takes priority over org-level: an explicit site enable/disable + // overrides the org-level setting, allowing a single site to be upgraded to + // a paid profile even if the org is in disabled.orgs. + if (disabledSites.includes(siteId)) { + return false; + } + if (enabledSites.includes(siteId)) { + if (disabledOrgs.includes(orgId)) { + this.log.debug('[isHandlerEnabledForSite] site-override: site in enabled.sites overrides org in disabled.orgs', { type, siteId, orgId }); } + return true; } + if (disabledOrgs.includes(orgId)) { + return false; + } if (handler.enabledByDefault) { return true; } - if (handler.enabled) { - const sites = handler.enabled.sites || []; - const orgs = handler.enabled.orgs || []; - return sites.includes(siteId) || orgs.includes(orgId); - } - - return false; + return enabledOrgs.includes(orgId); } + /** + * Returns whether a handler is enabled for a given org. + * + * Precedence (highest to lowest): + * 1. disabled.orgs includes orgId → false + * 2. enabledByDefault → true + * 3. enabled.orgs includes orgId → true + * 4. otherwise → false + * + * Note: unlike isHandlerEnabledForSite, there is no site-level override here. + * An org in disabled.orgs is always disabled regardless of site-level entries. + */ isHandlerEnabledForOrg(type, org) { const handler = this.getHandlers()?.[type]; if (!handler) { @@ -243,20 +277,21 @@ class Configuration { } if (enabled) { - if (handler.enabledByDefault) { - handler.disabled[entityKey] = handler.disabled[entityKey] - /* c8 ignore next */ - .filter((id) => id !== entityId) || []; - } else { - handler.enabled[entityKey] = Array - .from(new Set([...(handler.enabled[entityKey] || []), entityId])); - } + // Always remove from disabled first (handles re-enabling a previously disabled entry). + handler.disabled[entityKey] = (handler.disabled[entityKey] || []) + .filter((id) => id !== entityId); + // Always add to enabled so isHandlerEnabledForSite can find the site in enabled.sites + // and return true even when the org is in disabled.orgs (site-level override). + handler.enabled[entityKey] = Array + .from(new Set([...(handler.enabled[entityKey] || []), entityId])); } else if (handler.enabledByDefault) { handler.disabled[entityKey] = Array .from(new Set([...(handler.disabled[entityKey] || []), entityId])); + handler.enabled[entityKey] = (handler.enabled[entityKey] || []) + .filter((id) => id !== entityId); } else { - /* c8 ignore next */ - handler.enabled[entityKey] = handler.enabled[entityKey].filter((id) => id !== entityId) || []; + handler.enabled[entityKey] = (handler.enabled[entityKey] || []) + .filter((id) => id !== entityId); } handlers[type] = handler; diff --git a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.model.test.js b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.model.test.js index 969725a9f..74442563d 100755 --- a/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.model.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/configuration/configuration.model.test.js @@ -235,6 +235,62 @@ describe('ConfigurationModel', () => { expect(isEnabled).to.be.true; }); + it('returns false for a site whose org is in disabled.orgs and the site is not explicitly listed', () => { + instance.addHandler('org-disabled-handler', { + enabledByDefault: true, + enabled: { sites: [], orgs: [] }, + disabled: { sites: [], orgs: [site.getOrganizationId()] }, + }); + + expect(instance.isHandlerEnabledForSite('org-disabled-handler', site)).to.be.false; + }); + + it('returns true when site is in enabled.sites even if its org is in disabled.orgs', () => { + instance.addHandler('site-override-handler', { + enabledByDefault: true, + enabled: { sites: [site.getId()], orgs: [] }, + disabled: { sites: [], orgs: [site.getOrganizationId()] }, + }); + + expect(instance.isHandlerEnabledForSite('site-override-handler', site)).to.be.true; + }); + + it('returns true for non-default handler when site is in enabled.sites and org is in disabled.orgs', () => { + instance.addHandler('paid-handler', { + enabledByDefault: false, + enabled: { sites: [site.getId()], orgs: [] }, + disabled: { sites: [], orgs: [site.getOrganizationId()] }, + }); + + expect(instance.isHandlerEnabledForSite('paid-handler', site)).to.be.true; + }); + + it('removes site from disabled.sites when re-enabling for a non-default handler', () => { + instance.addHandler('cleanup-handler', { + enabledByDefault: false, + enabled: { sites: [], orgs: [] }, + disabled: { sites: [site.getId()], orgs: [] }, + }); + + instance.updateHandlerSites('cleanup-handler', site.getId(), true); + + expect(instance.getHandler('cleanup-handler').disabled.sites).to.not.include(site.getId()); + expect(instance.getHandler('cleanup-handler').enabled.sites).to.include(site.getId()); + }); + + it('removes site from enabled.sites when disabling a default handler', () => { + instance.addHandler('default-cleanup-handler', { + enabledByDefault: true, + enabled: { sites: [site.getId()], orgs: [] }, + disabled: { sites: [], orgs: [] }, + }); + + instance.updateHandlerSites('default-cleanup-handler', site.getId(), false); + + expect(instance.getHandler('default-cleanup-handler').enabled.sites).to.not.include(site.getId()); + expect(instance.getHandler('default-cleanup-handler').disabled.sites).to.include(site.getId()); + }); + it('updates handler orgs for a handler disabled by default with enabled', () => { instance.updateHandlerOrgs('lhs-mobile', org.getId(), true); expect(instance.getHandler('lhs-mobile').enabled.orgs).to.include(org.getId());