Open
Conversation
No ref Adds back a (Shade-driven) empty state for Members to emulate the existing one in Ember.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA new 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/posts/src/views/members/components/members-empty-state.tsx`:
- Around line 11-18: The code treats memberships as enabled when settings are
unresolved because membersSignupAccess becomes undefined; update the
membershipsEnabled calculation to only evaluate after settings are loaded (e.g.,
check settingsData or settingsData.settings exists) — for example in the
component that calls useBrowseSettings, change the membershipsEnabled derivation
(currently using membersSignupAccess and membershipsEnabled) to first gate on
settingsData (or a loading flag) and only set membershipsEnabled =
getSettingValue<string>(settingsData.settings, 'members_signup_access') !==
'none' when settingsData is present so the CTA is not shown while settings are
still loading.
- Around line 27-39: The POST to `${apiRoot}/members/` in this component is
missing the framework's required fetch settings (e.g., credentials and
cache/version headers); update the call to include credentials: 'include' and
add headers like 'app-pragma': 'no-cache' and the app/version header matching
the pattern in import-members-modal.tsx, or better yet, extract this logic into
a new mutation hook (e.g., useCreateMembers) mirroring existing hooks such as
useAddTier/useAddNewsletter so API auth/headers are centralized and reusable;
modify the code that calls getGhostPaths() and currentUser to use that hook or
the corrected fetch options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5bcc814-af29-4a4b-b70b-6a98c1d556e3
📒 Files selected for processing (3)
apps/posts/src/views/members/components/members-empty-state.tsxapps/posts/src/views/members/members.tsxapps/shade/src/components/ui/empty-indicator.tsx
Comment on lines
+11
to
+18
| const {data: settingsData} = useBrowseSettings({}); | ||
| const {data: currentUser} = useCurrentUser(); | ||
| const navigate = useNavigate(); | ||
| const [isAdding, setIsAdding] = useState(false); | ||
|
|
||
| const {assetRoot} = getGhostPaths(); | ||
| const membersSignupAccess = getSettingValue<string>(settingsData?.settings ?? null, 'members_signup_access'); | ||
| const membershipsEnabled = membersSignupAccess !== 'none'; |
Contributor
There was a problem hiding this comment.
Don’t treat unresolved settings as memberships enabled.
While useBrowseSettings is still loading, membersSignupAccess is undefined, so undefined !== 'none' renders the enabled CTA even on sites where memberships are disabled. Gate the branch until settings are loaded.
Proposed fix
-import {Button, EmptyIndicator} from '@tryghost/shade/components';
+import {Button, EmptyIndicator, LoadingIndicator} from '@tryghost/shade/components';
@@
- const {data: settingsData} = useBrowseSettings({});
+ const {data: settingsData, isLoading: isSettingsLoading} = useBrowseSettings({});
@@
const {assetRoot} = getGhostPaths();
const membersSignupAccess = getSettingValue<string>(settingsData?.settings ?? null, 'members_signup_access');
const membershipsEnabled = membersSignupAccess !== 'none';
+
+ if (isSettingsLoading || !settingsData?.settings) {
+ return (
+ <div className="flex h-full items-center justify-center px-4">
+ <LoadingIndicator size="lg" />
+ </div>
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const {data: settingsData} = useBrowseSettings({}); | |
| const {data: currentUser} = useCurrentUser(); | |
| const navigate = useNavigate(); | |
| const [isAdding, setIsAdding] = useState(false); | |
| const {assetRoot} = getGhostPaths(); | |
| const membersSignupAccess = getSettingValue<string>(settingsData?.settings ?? null, 'members_signup_access'); | |
| const membershipsEnabled = membersSignupAccess !== 'none'; | |
| const {data: settingsData, isLoading: isSettingsLoading} = useBrowseSettings({}); | |
| const {data: currentUser} = useCurrentUser(); | |
| const navigate = useNavigate(); | |
| const [isAdding, setIsAdding] = useState(false); | |
| const {assetRoot} = getGhostPaths(); | |
| const membersSignupAccess = getSettingValue<string>(settingsData?.settings ?? null, 'members_signup_access'); | |
| const membershipsEnabled = membersSignupAccess !== 'none'; | |
| if (isSettingsLoading || !settingsData?.settings) { | |
| return ( | |
| <div className="flex h-full items-center justify-center px-4"> | |
| <LoadingIndicator size="lg" /> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/posts/src/views/members/components/members-empty-state.tsx` around lines
11 - 18, The code treats memberships as enabled when settings are unresolved
because membersSignupAccess becomes undefined; update the membershipsEnabled
calculation to only evaluate after settings are loaded (e.g., check settingsData
or settingsData.settings exists) — for example in the component that calls
useBrowseSettings, change the membershipsEnabled derivation (currently using
membersSignupAccess and membershipsEnabled) to first gate on settingsData (or a
loading flag) and only set membershipsEnabled =
getSettingValue<string>(settingsData.settings, 'members_signup_access') !==
'none' when settingsData is present so the CTA is not shown while settings are
still loading.
Comment on lines
+27
to
+39
| const {apiRoot} = getGhostPaths(); | ||
| const response = await fetch(`${apiRoot}/members/`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json' | ||
| }, | ||
| body: JSON.stringify({ | ||
| members: [{ | ||
| email: currentUser.email, | ||
| name: currentUser.name | ||
| }] | ||
| }) | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect existing member-create/Admin API request conventions without modifying the repo.
set -euo pipefail
echo "Existing member API helpers or mutations:"
rg -n -C3 --type=ts --type=tsx '\b(use(Add|Create|Edit|Update)Member|addMember|createMember|createMembers|membersApi)\b' || true
echo
echo "Existing direct calls to the members endpoint:"
rg -n -C4 --type=ts --type=tsx 'fetch\s*\([^)]*/members/|apiRoot[^;\n]*/members/' || true
echo
echo "Admin API version/header conventions:"
rg -n -C3 --type=ts --type=tsx 'Accept-Version|Content-Type.*application/json|apiRoot' || trueRepository: TryGhost/Ghost
Length of output: 287
🏁 Script executed:
#!/bin/bash
# Search for member-related API helpers and mutations with corrected syntax
echo "=== Member API helpers/mutations ==="
rg -n -C2 '(useAddMember|useCreateMember|useEditMember|useUpdateMember|addMember|createMember|createMembers|membersApi)' --max-count=20 || true
echo
echo "=== Existing fetch calls to members endpoint ==="
rg -n -C3 '(fetch.*members|members.*fetch)' --max-count=20 || true
echo
echo "=== Admin API client usage patterns ==="
rg -n -C3 '(GhostAdminAPI|adminAPI|apiClient|useAdminAPI|useGhost)' apps/posts/src --max-count=30 || true
echo
echo "=== Looking at the specific file ==="
head -50 apps/posts/src/views/members/components/members-empty-state.tsx || trueRepository: TryGhost/Ghost
Length of output: 50373
🏁 Script executed:
#!/bin/bash
# Look at the specific file mentioned in the review
echo "=== File: members-empty-state.tsx ==="
cat -n apps/posts/src/views/members/components/members-empty-state.tsx
echo
echo "=== Search for Admin API/fetch patterns in apps/posts ==="
rg -n 'fetch|apiRoot|useGhost|adminAPI' apps/posts/src --max-count=30 -A 2
echo
echo "=== GhostAdminApiAdapter definition ==="
find . -name '*adapter*' -o -name '*Adapter*' | head -20 | xargs grep -l 'GhostAdminApiAdapter' 2>/dev/null || echo "No direct hits"
rg -n 'class GhostAdminApiAdapter|export.*GhostAdminApiAdapter' --max-count=10Repository: TryGhost/Ghost
Length of output: 25068
🏁 Script executed:
#!/bin/bash
# Search for member API patterns in the framework
echo "=== Member-related API code in admin-x-framework ==="
find . -path "*/admin-x-framework*" -name "*.ts" -o -name "*.tsx" | xargs rg -l "member" 2>/dev/null | head -20
echo
echo "=== Check what the framework provides for API calls ==="
rg -n 'fetch|apiRoot' packages/@tryghost/admin-x-framework --max-count=30 -A 2 2>/dev/null || echo "Searching in framework..."
echo
echo "=== Look for API client patterns in admin-x-framework ==="
rg -n 'class.*Api|function.*Api|export.*api' packages/@tryghost/admin-x-framework/api --max-count=20 2>/dev/null || true
echo
echo "=== Check existing Admin API calls for headers/versioning ==="
rg -n 'Accept-Version|X-Ghost|authorization' packages/@tryghost/admin-x-framework --max-count=20 || trueRepository: TryGhost/Ghost
Length of output: 1643
🏁 Script executed:
#!/bin/bash
# Search for useBrowseMembersInfinite and other member hooks
echo "=== Member browse hooks ==="
rg -n 'useBrowseMembersInfinite|useMember|useEditMember|useCreateMember' --max-count=20
echo
echo "=== Check the hook implementation ==="
rg -n 'export.*useBrowseMembersInfinite' -A 10 --max-count=5
echo
echo "=== Compare: settings API usage in members-empty-state ==="
grep -n 'useBrowseSettings\|useCurrentUser' apps/posts/src/views/members/components/members-empty-state.tsx
echo
echo "=== Check framework package.json to understand dependencies ==="
cat package.json | grep -A 5 '@tryghost/admin-x-framework' || echo "Check if it's in workspace"
echo
echo "=== Look for member mutation/add operations in the posts app ==="
rg -n 'members' apps/posts/src --type=ts -g '*.ts' --max-count=20 -B 2 | grep -E '(hook|function|const.*=.*\(|mutation|add|create)' || trueRepository: TryGhost/Ghost
Length of output: 12417
🏁 Script executed:
#!/bin/bash
# Look at the full members.ts API file to see all exports
echo "=== Full members.ts API file ==="
cat apps/admin-x-framework/src/api/members.ts | head -200
echo
echo "=== Check for any create/add/mutation functions ==="
rg -n 'export.*create|export.*add|export.*mutation' apps/admin-x-framework/src/api/members.ts
echo
echo "=== Check how other mutations are done in the framework ==="
rg -n 'useMutation|createMutation' apps/admin-x-framework/src/api --max-count=20Repository: TryGhost/Ghost
Length of output: 16926
🏁 Script executed:
#!/bin/bash
# Look at createMutation implementation
echo "=== createMutation implementation ==="
cat apps/admin-x-framework/src/utils/api/hooks.ts | head -300
echo
echo "=== Check for authentication/header handling ==="
rg -n 'Authorization|Accept-Version|headers' apps/admin-x-framework/src/utils/api --max-count=30 -A 2
echo
echo "=== Check if bare fetch in import-members-modal includes auth ==="
grep -A 20 'fetch.*members/upload' apps/posts/src/views/members/components/bulk-action-modals/import-members-modal.tsxRepository: TryGhost/Ghost
Length of output: 14149
🏁 Script executed:
#!/bin/bash
# Check if members endpoint requires auth
echo "=== Check Admin API documentation/tests for members endpoint auth ==="
rg -n 'POST.*members' ghost/core/tests/api --max-count=10 -B 2 -A 2 | head -50
echo
echo "=== Verify if there are any tests for creating members ==="
rg -n 'create.*member\|add.*member' ghost/core/tests/api/members --max-count=15Repository: TryGhost/Ghost
Length of output: 334
Use the framework's API abstraction or match its authentication headers.
This bare fetch call is missing critical headers that the framework normally provides. Specifically, it lacks credentials: 'include' (required for authentication), 'app-pragma': 'no-cache', and potential version headers. See import-members-modal.tsx for the correct bare-fetch pattern, or better yet, create a useCreateMembers mutation hook in the framework (similar to useAddTier, useAddNewsletter, etc.) to keep API concerns centralized.
Reference: Current bare fetch
const response = await fetch(`${apiRoot}/members/`, {
method: 'POST',
headers: {
'Content-Type': 'application/json'
},
body: JSON.stringify({...})
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/posts/src/views/members/components/members-empty-state.tsx` around lines
27 - 39, The POST to `${apiRoot}/members/` in this component is missing the
framework's required fetch settings (e.g., credentials and cache/version
headers); update the call to include credentials: 'include' and add headers like
'app-pragma': 'no-cache' and the app/version header matching the pattern in
import-members-modal.tsx, or better yet, extract this logic into a new mutation
hook (e.g., useCreateMembers) mirroring existing hooks such as
useAddTier/useAddNewsletter so API auth/headers are centralized and reusable;
modify the code that calls getGhostPaths() and currentUser to use that hook or
the corrected fetch options.
Contributor
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 24662282217 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No ref
Adds back a (Shade-driven) empty state for Members to emulate the existing one in Ember.
.