Skip to content

fix: get the right page path even in edit mode#401

Merged
amol-anand merged 2 commits into
mainfrom
fix-scheduler-edit-mode
Jul 8, 2026
Merged

fix: get the right page path even in edit mode#401
amol-anand merged 2 commits into
mainfrom
fix-scheduler-edit-mode

Conversation

@amol-anand

Copy link
Copy Markdown
Collaborator

Issue: When using schedule sidekick plugin in edit mode, the path is incorrect.

Fix: We need to resolve the path using status api for that to get the right path to use for preview and publish.

Test URLs:

@amol-anand amol-anand requested a review from shsteimer July 8, 2026 18:12
@aem-code-sync

aem-code-sync Bot commented Jul 8, 2026

Copy link
Copy Markdown

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run all PSI checks
  • Re-run failed PSI checks
  • Re-sync branch
Commits

@aem-code-sync

aem-code-sync Bot commented Jul 8, 2026

Copy link
Copy Markdown
Page Scores Audits Google
📱 /tools/scheduler/schedule.html PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /tools/scheduler/schedule.html PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Code Review

Reviewed the fix for scheduler path resolution in Sidekick edit mode (tools/scheduler/schedule.js, tools/scheduler/utils.js, tools/scheduler/README.md, tests/tools/scheduler/utils.test.js).

Summary

  • isPageHost/parseSidekickParams cleanly separates "referrer is the page itself" (.aem.page/.aem.live/.aem.reviews) from "referrer is an authoring surface" (SharePoint, Google Docs, custom prod domains), and the new isProject flag correctly gates when the pathname can be trusted directly.
  • resolvePagePath in schedule.js falls back to the Admin API's status endpoint with editUrl — the same mechanism Sidekick itself uses — only when path is empty and a referrer is present. The gating condition (!path && !isProject && org && site && referrer) is correct, and since path can only be non-empty when isProject is true, the checks are consistent (the !path check is redundant but harmless).
  • Error handling follows the existing { path/ok, error, resp } envelope pattern used elsewhere in the file (ensurePreview, writeIntent), and the missing-context message is correctly overridden with the specific resolution error when available.
  • New tests (isPageHost, parseSidekickParams) cover the pure branching logic per this repo's testing philosophy (TESTING.md) — the DOM/fetch orchestration in resolvePagePath is reasonably left untested since it isn't a shared-library contract.
  • README update accurately documents the new fallback behavior and its rationale.

I did not find any BLOCKING or SHOULD FIX issues. One thing worth a manual sanity check before merge (not a code issue, just unverifiable from static review): confirm the Admin status API response field is actually webPath for your account/site, since that's the crux of the fix — the PR description's "Before/After" preview links suggest this was already tested manually via Sidekick edit mode, which would confirm it.

Note: I was unable to execute npm run lint / npm test in this sandbox (tool execution was restricted), so please confirm CI is green via gh pr checks.

Verdict

APPROVE

@amol-anand amol-anand requested a review from rofe July 8, 2026 18:28
@amol-anand amol-anand merged commit 8d7f4a1 into main Jul 8, 2026
6 checks passed
@amol-anand amol-anand deleted the fix-scheduler-edit-mode branch July 8, 2026 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants