feat: integrate update redirect with self onboarding

[jkf276]

added the update-redirect step to the self/plg onboarding

Please ensure your pull request adheres to the following guidelines:

• make sure to link the related issues in this description. Or if there's no issue created, make sure you
  describe here the problem you're solving.
• when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

If the PR is changing the API specification:

• make sure you add a "Not implemented yet" note the endpoint description, if the implementation is not ready
  yet. Ideally, return a 501 status code with a message explaining the feature is not implemented yet.
• make sure you add at least one example of the request and response.

If the PR is changing the API implementation or an entity exposed through the API:

• make sure you update the API specification and the examples to reflect the changes.

If the PR is introducing a new audit type:

• make sure you update the API specification with the type, schema of the audit result and an example

Related Issues

Thanks for contributing!

[github-actions]

This PR will trigger a minor release when merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Kanishkavijay39]

hi @jkf276 ,
there are some review suggestions where we can have look.

Important

Duplicate authoring/delivery type validation — plg-onboarding.js:432-440

queueIdentifyRedirectsAudit already validates authoring/delivery types internally (utils.js:1174-1186) and returns { ok: false } if they don't match. The outer validForRedirects check duplicates this logic, so the two can drift out of sync.

The outer check does serve a purpose: distinguishing an intentional "skip" (site isn't CS) from an actual failure (site IS CS but the queue failed). If that's the intent, a comment explaining the distinction would help clarify why this is intentional duplication.

Missing happy-path test — plg-onboarding.test.js:1435

The two new tests only cover the ok: false branches. There's no test asserting that queueIdentifyRedirectsAuditStub is actually called with the correct arguments for a valid CS site (the success path). The existing tests implicitly skip redirects (default authoringType: null, deliveryType: null), so a CS-site happy-path test is missing.

steps object not updated — plg-onboarding.js:449

All other steps set a flag on steps (e.g., steps.configUpdated = true, steps.auditsEnabled = true) for auditability. The redirects step doesn't set steps.redirectsQueued. This makes it harder to diagnose what happened during onboarding from the stored state. (need to update spacecat shared as well there is map which needs to be updated)

Minor

[SiteModel.DELIVERY_TYPES.AEM_CS].includes(deliveryType) — creating a single-element array just to call .includes() is idiomatic but unnecessary; deliveryType === SiteModel.DELIVERY_TYPES.AEM_CS reads cleaner.

Verdict
Request changes. The core logic is sound and failure handling is correct (non-blocking warn). Fix the missing happy-path test and add steps.redirectsQueued tracking to match the existing pattern. The duplicate validation issue is lower priority but worth at least a clarifying comment.

[Kanishkavijay39]

Issues

Important

• src/support/utils.js:1229 — validateSiteForRedirects has no direct unit tests.
  It's a new exported function with 3 distinct code paths (invalid type, missing programId, missing environmentId, valid)
  and it's only exercised indirectly through queueDeliveryConfigWriter tests. Since it now drives behavior in two separate
  callers, it deserves its own describe('validateSiteForRedirects') block. E.g., the CS_CW authoring type path and the
  AEM_CS delivery type path aren't independently verified.

Minor

• src/support/utils.js:1357-1363 — getDeliveryConfig is called twice.
  queueDeliveryConfigWriter calls site.getDeliveryConfig?.() || {} to extract programId/environmentId, then
  validateSiteForRedirects calls it again internally. Consider either passing the already-extracted config into
  validateSiteForRedirects, or removing the pre-extraction from queueDeliveryConfigWriter and letting
  validateSiteForRedirects own it fully.

// current (double call):
const deliveryConfig = site.getDeliveryConfig?.() || {};
const { programId, environmentId } = deliveryConfig;
const { validForRedirects, skipMessage } = validateSiteForRedirects(site); // calls again internally

• src/controllers/plg/plg-onboarding.js:430 — minutes: 2000 (~33 hours) seems unusually high.
  Other callers don't pass minutes at all (uses default). If this is intentional for PLG onboarding's redirect update
  window, a comment explaining why would help.
• src/support/utils.js:1222 — } // end queueIdentifyRedirectsAudit() closing comment.
  Not a project convention — functions are generally not annotated this way. Minor noise.
• test/controllers/plg/plg-onboarding.test.js:1526 — Test assertion on literal undefined in warn message.
  calledWithMatch(/Failed to queue delivery config writer for site .*undefined/) works but is brittle — it's asserting on
  the string representation of undefined. A cleaner signal would be checking only that warn was called and
  deliveryConfigQueued was false, without tying to the "undefined" string. Low priority.

---

Verdict

Approve with minor concerns. The logic is sound — onboarding correctly delegates to queueDeliveryConfigWriter which
handles the CS/CW eligibility check internally and continues CDN detection regardless. The test coverage covers the three
key scenarios (success, ok:false with error, ok:false without). The main gap is a direct test suite for
validateSiteForRedirects as a standalone exported function.

[solaris007]

🎉 This PR is included in version 1.413.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀