feat(client): User permission set correctly also if user is not present on the 1st page of pagination in app/engine settings #1205

pallabi2303 · 2025-05-22T09:17:23Z

Description

The logged-in user's permission is determined by a dedicated API call, ensuring correct access rights are set even if the user does not appear on the first page of the members list. This guarantees accurate UI controls regardless of pagination.

Changes Made

Added a dedicated API call to fetch the logged-in user's member record directly, using their user ID and a limit of 1.
Implemented the setUserDetails function to set the logged-in user's data and permission based on the result of this
dedicated API call, ensuring correct permission assignment even if the user is not present in the paginated members list.

How to Test

Log in as a user with elevated permissions (Author or Editor).
Navigate to the Members settings page for an app or engine.
Ensure the members list is long enough that the logged-in user does not appear on the first page
User can see that they can access member setting permission even if user doesn't appear on the first page of members list

… page of pagination

github-actions · 2025-05-22T09:17:32Z

@CodiumAI-Agent /describe

CodiumAI-Agent · 2025-05-22T09:18:04Z

Title

feat(client): User permission set correctly also if user is not present on the 1st page of pagination in app/engine settings

User description

Description

The logged-in user's permission is determined by a dedicated API call, ensuring correct access rights are set even if the user does not appear on the first page of the members list. This guarantees accurate UI controls regardless of pagination.

Changes Made

Added a dedicated API call to fetch the logged-in user's member record directly, using their user ID and a limit of 1.
Implemented the setUserDetails function to set the logged-in user's data and permission based on the result of this
dedicated API call, ensuring correct permission assignment even if the user is not present in the paginated members list.

How to Test

Log in as a user with elevated permissions (Author or Editor).
Navigate to the Members settings page for an app or engine.
Ensure the members list is long enough that the logged-in user does not appear on the first page
User can see that they can access member setting permission even if user doesn't appear on the first page of members list

PR Type

Bug fix, Enhancement

Description

Add dedicated API call for current user details
Refactor setUserDetails to use new userDetails data
Update effect to trigger on userDetails.status success
Simplify disabled logic removing adminMode checks

Changes walkthrough 📝

Relevant files

Enhancement

MembersTable.tsx `Dedicated user fetch and permission logic refactor` packages/client/src/components/settings/MembersTable.tsx Introduced `getUserDataApi` and `userDetails` hook Refactored `setUserDetails` to use `userDetails.data.members[0]` Updated `useEffect` to depend on `userDetails` instead of `getMembers` Simplified `disabled` conditions by removing `adminMode` checks	+99/-66

Need help?
Type /help how to ... in the comments thread for any questions about PR-Agent usage.
Check out the documentation for more information.

github-actions · 2025-05-22T09:18:18Z

@CodiumAI-Agent /review

github-actions · 2025-05-22T09:19:03Z

@CodiumAI-Agent /improve

CodiumAI-Agent · 2025-05-22T09:19:09Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Missing Data Check Accessing userDetails.data.members[0] without verifying that members array is non-empty could cause runtime errors if the API returns no members setUserData(userDetails.data.members[0]); if (adminMode) { Null-Safety Inconsistency Permission priority is accessed without optional chaining in some places, which can cause undefined property errors if permissionPriorityMapper returns undefined permissionPriorityMapper( userPermission, ) .priority > 1

CodiumAI-Agent · 2025-05-22T09:20:25Z

PR Code Suggestions ✨

Latest suggestions up to d90be3d

Category	Suggestion	Impact
Possible issue	Use username for filtering The engine‐user fetch should filter by the current user's name, not their numeric or UUID ID, to match the original behavior of filtering by `member.name`. Replace `configStore.store.user.id` with `configStore.store.user.name` for the search parameter. packages/client/src/components/settings/MembersTable.tsx [293-301] getUserDataApi = [ 'getEngineUsers', adminMode, id, - configStore.store.user.id, + configStore.store.user.name, // use username to filter permissionPriorityMapper(permissionFilter)?.permission, 0, // offset 1, // limit ]; Suggestion importance[1-10]: 9 __ Why: Replacing `configStore.store.user.id` with `configStore.store.user.name` fixes the filter to match by username as intended and prevents incorrect user lookup.	High
	Restore engine pagination The members API for engine types lost pagination and search functionality. Restore use of `debouncedSearch` and `rowsPerPage` to correctly page through engine users. packages/client/src/components/settings/MembersTable.tsx [303-310] getMembersApi = [ 'getEngineUsers', adminMode, id, - configStore.store.user.id, + debouncedSearch \|\| undefined, permissionPriorityMapper(permissionFilter)?.permission, - 0, // offset - 1, // limit + (page + 1) * rowsPerPage - rowsPerPage, // offset + rowsPerPage, // limit ]; Suggestion importance[1-10]: 8 __ Why: The current engine users call always returns only the first record (`limit = 1`) and ignores `debouncedSearch`, breaking pagination and search; restoring those parameters fixes functionality.	Medium
	Add guard for empty members Guard against empty member arrays to avoid runtime crashes when `members[0]` is undefined. Check that `userDetails.data.members` has at least one entry before indexing. packages/client/src/components/settings/MembersTable.tsx [327-344] const setUserDetails = () => { - setUserData(userDetails.data.members[0]); + const members = userDetails.data.members; + if (!members \|\| members.length === 0) return; + setUserData(members[0]); if (adminMode) { const adminPermissionPriority = 'Author'; setUserPermission( permissionPriorityMapper(adminPermissionPriority) ?.permission as SETTINGS_ROLE, ); } else { setUserPermission( permissionPriorityMapper( - userDetails.data.members[0].permission === 'OWNER' - ? 'Author' - : userDetails.data.members[0].permission, + members[0].permission === 'OWNER' ? 'Author' : members[0].permission, )?.permission as SETTINGS_ROLE, ); } }; Suggestion importance[1-10]: 7 __ Why: Adding a check for `userDetails.data.members` length prevents runtime errors when the members array is empty and ensures safe indexing of `members[0]`.	Medium

Previous suggestions

Suggestions up to commit ba4f454

Category	Suggestion	Impact
Possible issue	Prevent indexing empty members Guard against empty or missing `members` before accessing index 0 to prevent runtime errors. Return early or provide a fallback if no member is found. packages/client/src/components/settings/MembersTable.tsx [334] +if (!userDetails.data.members \|\| userDetails.data.members.length === 0) { + return; +} setUserData(userDetails.data.members[0]); Suggestion importance[1-10]: 7 __ Why: Guarding against empty `userDetails.data.members` prevents a runtime error when accessing index 0 if the array is empty.	Medium
Possible issue	Add default permission fallback Provide a safe default when `permissionPriorityMapper` returns undefined to avoid casting `undefined` as a role. Extract the raw permission first, then coalesce to a fallback like `'Read-Only'`. packages/client/src/components/settings/MembersTable.tsx [342-348] -setUserPermission( - permissionPriorityMapper( - userDetails.data.members[0].permission === 'OWNER' - ? 'Author' - : userDetails.data.members[0].permission, - )?.permission as SETTINGS_ROLE, -); +const rawPermission = userDetails.data.members[0].permission === 'OWNER' + ? 'Author' + : userDetails.data.members[0].permission; +const mapped = permissionPriorityMapper(rawPermission)?.permission ?? 'Read-Only'; +setUserPermission(mapped as SETTINGS_ROLE); Suggestion importance[1-10]: 6 __ Why: Providing a fallback prevents casting `undefined` from `permissionPriorityMapper` to `SETTINGS_ROLE`, avoiding potential type or runtime issues.	Low
General	Default API permission value Ensure the permission argument passed to the API is never undefined by falling back to a safe default. This prevents sending `null` or `undefined` to the backend. packages/client/src/components/settings/MembersTable.tsx [309] -permissionPriorityMapper(permissionFilter)?.permission, +permissionPriorityMapper(permissionFilter)?.permission ?? 'Read-Only', Suggestion importance[1-10]: 5 __ Why: Falling back when `permissionPriorityMapper(permissionFilter)?.permission` is `undefined` avoids sending an invalid permission to the backend, though the impact is minor.	Low

anurag91jain · 2025-05-23T18:51:50Z

packages/client/src/components/settings/MembersTable.tsx

-    //         return;
-    //     }
+        setUserDetails();
+    }, [userDetails.status, userDetails.data]);


Let's try to remove the dependencies and check it once. We don't need to call setUserDetails again and again on pagination as it wouldn't change

Yes , I have removed userDetails.data dependency as it wouldn't affect the behavior, so that setUserDetails wouldn't be called again and again.

AAfghahi · 2025-05-25T22:35:19Z

packages/client/src/components/settings/MembersTable.tsx

@@ -295,65 +295,69 @@ export const MembersTable = (props: MembersTableProps) => {
        setPage(0);
    }, [debouncedSearch]);

+    const getUserDataApi: Parameters<typeof useAPI>[0] =


This double ternary seems worse than just a if else, and it is harder to read.

Now I've changed the double ternary to simple if-else statements.

AAfghahi · 2025-05-28T20:35:27Z

packages/client/src/components/settings/MembersTable.tsx

              ]
            : type === 'APP'
            ? [
                  'getProjectUsers',
                  adminMode,
                  id,
-                  debouncedSearch ? debouncedSearch : undefined,


why did we get rid of this?

Fixed it @AAfghahi and consolidated 2 if conditions into 1.

johbaxter · 2025-05-30T16:56:55Z

May be a separate issue.

Im logged in as cc:

Should i be able to demote authors, or should i only be able to demote users of the same status as me along with myself:

I can push this, but yeah still think there's issues.

Lmk what you think @AAfghahi

pallabi2303 · 2025-06-03T13:32:31Z

Yes, the issue has been resolved. Editors no longer have the ability to demote Authors.

johbaxter · 2025-06-05T18:19:14Z

Looks good ty

feat(client): permission issue resolved when the user is not in first…

Loading
Loading status checks…

ba4f454

… page of pagination

pallabi2303 requested review from anurag91jain, AAfghahi and johbaxter May 22, 2025 09:17

pallabi2303 self-assigned this May 22, 2025

pallabi2303 requested a review from a team as a code owner May 22, 2025 09:17

pallabi2303 linked an issue May 22, 2025 that may be closed by this pull request

Permissions will not be set correctly if the user is not present on the 1st page of pagination in app/engine settings #1201

Closed

3 tasks

Removed unnecessary commented lines MembersTable.tsx

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

e442e2e

anurag91jain requested changes May 23, 2025

View reviewed changes

anurag91jain and others added 2 commits May 24, 2025 00:22

Merge branch 'dev' into MembersTable-setUserDetailsChange-paginationBug

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

e824d3c

Removed unnecessary dependenices

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

88c5bc3

AAfghahi reviewed May 25, 2025

View reviewed changes

pallabi2303 and others added 2 commits May 26, 2025 15:40

Changed the double ternary to simple if-else statements

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

2071d2f

Merge branch 'dev' into MembersTable-setUserDetailsChange-paginationBug

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

b1f7e79

AAfghahi reviewed May 28, 2025

View reviewed changes

anurag91jain added 2 commits May 29, 2025 14:25

Fixing build issue

Loading
Loading status checks…

e31cce6

Merge branch 'dev' into MembersTable-setUserDetailsChange-paginationBug

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

d90be3d

anurag91jain approved these changes May 29, 2025

View reviewed changes

neelneelneel force-pushed the dev branch from 4328fb0 to 6eebd69 Compare May 30, 2025 22:46

pallabi2303 added 2 commits June 3, 2025 11:27

Merge branch 'dev' into MembersTable-setUserDetailsChange-paginationBug

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

ca45538

Editor will not be able to demote Authors

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

b4973b0

Merge branch 'dev' into MembersTable-setUserDetailsChange-paginationBug

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

e34ced6

johbaxter merged commit 83a373a into dev Jun 5, 2025
3 checks passed

johbaxter deleted the MembersTable-setUserDetailsChange-paginationBug branch June 5, 2025 18:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(client): User permission set correctly also if user is not present on the 1st page of pagination in app/engine settings #1205

feat(client): User permission set correctly also if user is not present on the 1st page of pagination in app/engine settings #1205

pallabi2303 commented May 22, 2025

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

CodiumAI-Agent commented May 22, 2025

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

CodiumAI-Agent commented May 22, 2025

Uh oh!

CodiumAI-Agent commented May 22, 2025 •

edited

Loading

Uh oh!

anurag91jain May 23, 2025

Uh oh!

pallabi2303 May 24, 2025

Uh oh!

AAfghahi May 25, 2025

Uh oh!

pallabi2303 May 26, 2025

Uh oh!

AAfghahi May 28, 2025

Uh oh!

anurag91jain May 29, 2025

Uh oh!

johbaxter commented May 30, 2025 •

edited

Loading

Uh oh!

pallabi2303 commented Jun 3, 2025

Uh oh!

johbaxter commented Jun 5, 2025

Uh oh!

Uh oh!

feat(client): User permission set correctly also if user is not present on the 1st page of pagination in app/engine settings #1205

feat(client): User permission set correctly also if user is not present on the 1st page of pagination in app/engine settings #1205

Conversation

pallabi2303 commented May 22, 2025

Description

Changes Made

How to Test

Uh oh!

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

CodiumAI-Agent commented May 22, 2025

Title

User description

Description

Changes Made

How to Test

PR Type

Description

Changes walkthrough 📝

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

CodiumAI-Agent commented May 22, 2025

PR Reviewer Guide 🔍

Uh oh!

CodiumAI-Agent commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

Uh oh!

anurag91jain May 23, 2025

Choose a reason for hiding this comment

Uh oh!

pallabi2303 May 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AAfghahi May 25, 2025

Choose a reason for hiding this comment

Uh oh!

pallabi2303 May 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AAfghahi May 28, 2025

Choose a reason for hiding this comment

Uh oh!

anurag91jain May 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

johbaxter commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pallabi2303 commented Jun 3, 2025

Uh oh!

Uh oh!

johbaxter commented Jun 5, 2025

Uh oh!

Uh oh!

CodiumAI-Agent commented May 22, 2025 •

edited

Loading

johbaxter commented May 30, 2025 •

edited

Loading