Conversation
…ure we are logged in
WalkthroughReplaces manual multi-part URL assembly with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Preview as PreviewComponent
participant Window as window/location
participant App as AppState.getSite
participant User as UserState.isLoggedIn
participant ShopR as ShopRegionalCostsState
participant ShopS as ShopSettingsState
rect rgba(220,235,255,0.18)
note left of Preview: build base URL (new)
Preview->>Window: read location.origin
Window-->>Preview: origin (scheme + host[:port])
Preview->>Preview: compose final URL (origin + path)
end
rect rgba(200,230,200,0.12)
note over ShopR,ShopS: auth-gated initialization (new)
App-->>ShopR: site$
User-->>ShopR: isLoggedIn$
ShopR->>ShopR: combineLatest(site$, isLoggedIn$)\nfilter(site && isLoggedIn)\nmap(site)\ninitialize...
App-->>ShopS: site$
User-->>ShopS: isLoggedIn$
ShopS->>ShopS: combineLatest(site$, isLoggedIn$)\nfilter(site && isLoggedIn)\nmap(site)\ninitialize...
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
editor/src/app/shop/settings/shop-settings.state.ts (2)
59-68: Null guard prevents initialization errors.The early return on lines 59-61 correctly guards against accessing properties on
undefinedornullcurrentSiteSettings. This prevents runtime errors during state initialization before shop settings are loaded.Consider using optional chaining for additional robustness to handle cases where
group_configorweightUnitmight be missing:if (!currentSiteSettings) { return; } - return ( - currentSiteSettings - .find((group) => { - return group.slug === 'group_config'; - }) - .settings.find((setting) => setting.slug === 'weightUnit').value || '' - ); + return ( + currentSiteSettings + .find((group) => group.slug === 'group_config') + ?.settings.find((setting) => setting.slug === 'weightUnit')?.value || '' + );
73-82: Null guard prevents initialization errors.The early return correctly guards against accessing properties on
undefinedornullcurrentSiteSettings, consistent with thegetCurrentWeightUnitselector.Consider using optional chaining for additional robustness, similar to
getCurrentWeightUnit:if (!currentSiteSettings) { return; } - return ( - currentSiteSettings - .find((group) => { - return group.slug === 'group_config'; - }) - .settings.find((setting) => setting.slug === 'currency').value || '' - ); + return ( + currentSiteSettings + .find((group) => group.slug === 'group_config') + ?.settings.find((setting) => setting.slug === 'currency')?.value || '' + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
editor/src/app/preview/preview.component.ts(3 hunks)editor/src/app/shop/regional-costs/shop-regional-costs.state.ts(2 hunks)editor/src/app/shop/settings/shop-settings.state.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
editor/src/app/shop/regional-costs/shop-regional-costs.state.ts (1)
editor/src/app/user/user.state.ts (1)
isLoggedIn(49-51)
editor/src/app/shop/settings/shop-settings.state.ts (1)
editor/src/app/user/user.state.ts (1)
isLoggedIn(49-51)
🔇 Additional comments (5)
editor/src/app/shop/regional-costs/shop-regional-costs.state.ts (2)
38-38: LGTM!The
combineLatestimport is correctly added and used inngxsOnInitto gate initialization on both site and authentication state.
80-99: Approve authentication-gated initialization and logout cleanupThe
ngxsOnInitlogic waits for a valid site and authenticated user, and theShopStatelogout handler resets all shop sections—including regional costs—on logout.editor/src/app/preview/preview.component.ts (1)
27-54: LGTM!Formatting improvements to the component decorator. No behavioral changes.
editor/src/app/shop/settings/shop-settings.state.ts (2)
34-35: LGTM!The imports for
combineLatestandUserStateare correctly added to support authentication-gated initialization.
93-112: Correct implementation of authentication-gated initialization.The refactored
ngxsOnInitproperly waits for both a valid site and user authentication before initializing shop settings. This implementation is consistent with the pattern used inshop-regional-costs.state.ts, ensuring uniform behavior across shop state modules.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
editor/src/app/sites/media/entry-gallery-image-editor.component.ts (1)
187-189: Good defensive check; consider adding user feedback.The early return guard prevents runtime errors when
galleryFileis not found (e.g., invalid image order in URL, out-of-date media cache). This addresses the undefined error issues mentioned in the PR objectives. However, the user remains on the page without feedback. Consider navigating back or displaying an error message to improve the user experience.Apply this diff to navigate back when the file is not found:
if (!this.galleryFile) { + this._location.back(); return; }Alternatively, display an error message or log the issue for debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
editor/src/app/sites/media/entry-gallery-image-editor.component.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
editor/src/app/sites/media/entry-gallery-image-editor.component.ts (1)
editor/src/app/shared/animations.ts (1)
Animations(9-29)
Summary by CodeRabbit
Bug Fixes
Refactor
Style