Skip to content

fix(admin-ui): Seed channel token before config fetch#4760

Open
g8rr5dg2p7-svg wants to merge 2 commits into
vendurehq:masterfrom
g8rr5dg2p7-svg:codex/vendure-external-auth-channel-token
Open

fix(admin-ui): Seed channel token before config fetch#4760
g8rr5dg2p7-svg wants to merge 2 commits into
vendurehq:masterfrom
g8rr5dg2p7-svg:codex/vendure-external-auth-channel-token

Conversation

@g8rr5dg2p7-svg
Copy link
Copy Markdown

@g8rr5dg2p7-svg g8rr5dg2p7-svg commented May 21, 2026

/claim #2363

Summary

  • seed the Admin UI active channel token from the current user before the server config request when it is missing
  • preserve the existing active-channel choice behavior by preferring the stored token, then the default channel, then the first channel
  • add focused ServerConfigService coverage for token seeding, existing-token behavior, and unauthenticated bootstrap fallback

Why

External authentication can leave the Admin UI authenticated but without an activeChannelToken in local storage. Since the app waits for ServerConfigService during bootstrap, setting the channel token there ensures the initial Admin API requests include the channel token instead of requiring a page reload.

Testing

  • npm run build in packages/common
  • PUPPETEER_EXECUTABLE_PATH="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" npm run test -- --include src/lib/core/src/data/server-config.spec.ts

Fixes #2363


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
vendure-storybook Ready Ready Preview, Comment May 22, 2026 1:46am

Request Review

@vendure-ci-automation-bot
Copy link
Copy Markdown
Contributor

vendure-ci-automation-bot Bot commented May 21, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2f97c410-1673-4d99-9264-b92c75d579d1

📥 Commits

Reviewing files that changed from the base of the PR and between c2b48fc and 247b29e.

📒 Files selected for processing (2)
  • packages/admin-ui/src/lib/core/src/data/server-config.spec.ts
  • packages/admin-ui/src/lib/core/src/data/server-config.ts

📝 Walkthrough

Walkthrough

ServerConfigService gains channel token initialization logic to resolve timing issues with external authentication strategies. The service now injects LocalStorageService and calls ensureActiveChannelToken() before fetching server config. This method checks if activeChannelToken exists in storage; if not, it fetches the current user, selects an appropriate channel (last-active or fallback to DEFAULT_CHANNEL_CODE), and stores the token. New tests verify the three scenarios: token initialization on first load, skipping user fetch when token exists, and handling authentication errors gracefully.

Possibly related PRs

  • vendurehq/vendure#4472: Both PRs address race conditions by pre-populating channel token into localStorage before triggering subsequent data fetching (server config in this PR vs. channel queries during Dashboard login).

Suggested reviewers

  • michaelbromley
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: seeding the channel token before the server config fetch.
Description check ✅ Passed The PR description covers the summary, rationale, and testing instructions. It includes the issue claim, clear explanation of the fix, and reasoning for the change.
Linked Issues check ✅ Passed The PR successfully addresses #2363 by seeding the activeChannelToken from the current user before the ServerConfigService fetch, ensuring initial API requests include the Vendure-Token header without requiring a page reload.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue: ServerConfigService implementation modifications and focused test coverage for the token-seeding behavior and fallback scenarios.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@g8rr5dg2p7-svg
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@g8rr5dg2p7-svg
Copy link
Copy Markdown
Author

recheck

@g8rr5dg2p7-svg
Copy link
Copy Markdown
Author

Quick review-ready status note for the bounty claim: this PR is still narrowly scoped to #2363, the CLA is signed, all visible checks are green, and CodeRabbit reported no actionable comments. The included coverage exercises the missing-token bootstrap path, preserves an existing stored token, and verifies unauthenticated config loading still falls back without blocking startup. Happy to adjust the approach if you would prefer the token seeding to live in a different Admin UI bootstrap layer.

@g8rr5dg2p7-svg
Copy link
Copy Markdown
Author

Friendly follow-up on the bounty claim: this PR is still green, CLA is signed, CodeRabbit reported no actionable comments, and the Vercel preview remains ready. Since the claim is still awaiting maintainer review, I wanted to surface it once more and ask whether you would prefer any narrower change, additional test case, or different placement for the channel-token seeding logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Not Authorized error with External Authentication Strategy

1 participant