Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -183,24 +183,28 @@ 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)) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment by @solaris007
Important: This flips existing config rows where a site is in enabled.sites and the org is in disabled.orgs from disabled to enabled at deploy time. Run a query against prod to enumerate affected rows before merging.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have prod access @solaris007 What is the best way to do this query?

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);
}

isHandlerEnabledForOrg(type, org) {
Expand Down Expand Up @@ -243,20 +247,23 @@ class Configuration {
}

if (enabled) {
if (handler.enabledByDefault) {
handler.disabled[entityKey] = handler.disabled[entityKey]
/* c8 ignore next */
.filter((id) => id !== entityId) || [];
} else {
// Always remove from disabled first (handles re-enabling a previously disabled entry).
handler.disabled[entityKey] = (handler.disabled[entityKey] || [])
.filter((id) => id !== entityId);
if (!handler.enabledByDefault) {
Comment thread
tkotthakota-adobe marked this conversation as resolved.
Outdated
// For non-default handlers, explicitly add to enabled list so isHandlerEnabledForSite
// can find the site in enabled.sites and return true even when org is in disabled.orgs.
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,26 @@ 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;
});

Comment thread
tkotthakota-adobe marked this conversation as resolved.
it('returns true when site is in enabled.sites even if its org is in disabled.orgs', () => {
instance.addHandler('site-override-handler', {
enabledByDefault: true,
Comment thread
tkotthakota-adobe marked this conversation as resolved.
enabled: { sites: [site.getId()], orgs: [] },
disabled: { sites: [], orgs: [site.getOrganizationId()] },
});

expect(instance.isHandlerEnabledForSite('site-override-handler', site)).to.be.true;
});

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());
Expand Down
Loading