feat: add temporary replaceHandlerEnabledDisabled method for cleanup.

[tathagat2241]

TEMPORARY: This method was created to support a temporary API endpoint for cleaning up enabled and disabled lists by removing unnecessary site IDs.

Unlike updateHandlerProperties which merges arrays, this method completely replaces the specified arrays. This method will be removed once the cleanup task is completed.

Changes

• Add replaceHandlerEnabledDisabled method to Configuration model
• Method replaces enabled/disabled arrays instead of merging them
• Marked with c8 ignore comments to skip unit test coverage (temporary code)

Related

• Related to temporary API in spacecat-api-service repo
  Spacecat-api-service PR: feat: add temporary API to replace enabled/disabled lists spacecat-api-service#1789

Please ensure your pull request adheres to the following guidelines:

• make sure to link the related issues in this description
• when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues

https://jira.corp.adobe.com/browse/SITES-40312

Thanks for contributing!

[github-actions]

This PR will trigger a minor release when merged.

[solaris007]

Hey @tathagat2241 - thanks for putting this up. I understand the need for a replace-vs-merge operation for the cleanup work. A few things came up during review that I'd like to see addressed before merging.

Must Fix

1. data parameter crashes on null/undefined
The method doesn't validate its data parameter. Calling replaceHandlerEnabledDisabled('type', null) throws an uncontrolled TypeError. Every comparable method in this file guards against this - updateConfiguration checks isNonEmptyObject(data), updateQueues does the same. Quick fix:

if (!isNonEmptyObject(data)) {
  throw new Error('Data cannot be empty');
}

2. Null-propagation crash when data.enabled is null
null !== undefined is true, so { enabled: null } passes the guard and then data.enabled.sites throws a TypeError. This is a realistic scenario - JSON payloads routinely include explicit null. Fix by using loose equality:

if (data.enabled != null) { // catches both null and undefined

Same for data.disabled.

3. Silent data loss on non-array values
If a caller passes data.enabled.sites = "some-site-id" (string instead of array), the ternary silently replaces the entire sites list with []. That's a destructive operation on the global config singleton with no indication anything went wrong. The rest of this file throws on invalid input - this should too, rather than silently defaulting to empty.

Should Fix

4. Tests - please remove the c8 ignore
I get that this is meant to be temporary, but it's shipping in spacecat-shared which is consumed by multiple services. The existing c8 ignore usage in this file covers single edge-case lines - blanket-ignoring an entire method is a different category. Three bugs were found in this review that basic tests would have caught immediately. The method is straightforward to test - shouldn't take long.

5. Handler dependency checks are bypassed
The existing enableHandlerForSite/enableHandlerForOrg both validate handler dependencies before enabling. This method skips that entirely. Worth at least documenting in JSDoc that the caller is responsible for ensuring dependencies are satisfied.

6. TypeScript declarations
The method isn't declared in index.d.ts. If it ships, the types should be updated too.

Accountability for Removal

This is the big one. The PR description and code comments both say "temporary" and "will be removed once the cleanup task is completed." That's great - but there's no mechanism to hold anyone (including future-us) accountable for actually removing it.

I'd like to see:

• A follow-up Jira ticket linked to SITES-40312, something like "Remove replaceHandlerEnabledDisabled temporary method" with a target date. When does the cleanup run? A week from now? A month? Let's put a date on it.
• A @deprecated JSDoc tag on the method referencing that ticket, so anyone who stumbles across it later knows the story.
• A log.warn on invocation so that any usage after the cleanup is done shows up in logs and triggers a "hey, why is this still being called?" conversation.

We've all seen "temporary" code that's still around years later. Let's not let this be one of those. A concrete removal ticket with a date makes it easy to track and close the loop.

Alternative Worth Considering

Since this is a one-time cleanup - have you considered just doing it as a script against S3 directly? The config is versioned JSON at config/spacecat/global-config.json, so you can fetch it, modify the arrays, write it back, and S3 versioning gives you rollback for free. Zero changes to spacecat-shared, zero semver bumps.

If you do need it as an API method, another option is adding a { strategy: 'replace' } option to the existing updateHandlerProperties rather than a parallel method. Keeps the API surface smaller and reuses existing validation.

Either way - happy to re-review once the fixes are in. The core logic is sound, it just needs the guardrails.

[solaris007]

Requesting changes - see detailed review in the comment above. The must-fix items (null/undefined crash, null-propagation, silent data loss) are real bugs, and the accountability piece (follow-up removal ticket with a date) is important for keeping "temporary" honest.

[tathagat2241]

@solaris007, I have addressed all of your comments and have created the new commit, Please re-review. Also I have created a ticket to remove this endpoint (SITES-40878) but have not added the date when this will be completed as this is dependent on another task which needs to be completed before we can implement this which is removing Enable Handler check before running the audits "isHandlerEnabled" and that PR is in review. Once it is merged and verified that everything is working as expected then I will implement this in a 2PR fashion with @rpapani.