feat(frontend): add Groups with leaderboard, roles, and invite system#249
feat(frontend): add Groups with leaderboard, roles, and invite system#249
Conversation
Introduce group creation, membership management, and group-scoped leaderboards so teams can track and compare token usage together. - Schema: groups, group_members, group_invites tables with role hierarchy (owner/admin/member) - API: 8 route files covering CRUD, invite, join, leave, role change, and group leaderboard - Frontend: 6 pages (list, create, detail+leaderboard, settings, members, join invite) - Helpers: permission checks, slug generation, group leaderboard query (reuses global leaderboard pattern) - Migration: 0005_add_groups.sql with all FKs, indexes, and unique constraints
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Add cookie session fallback to getSessionFromHeader for web UI flows - CAST COUNT(*) AS integer in my=true member count subquery - Add abort signal guard on setData in GroupsClient to prevent race conditions - Remove 3 redundant indexes (slug, group_id, token) already covered by UNIQUE constraints - Combine submittedUserCount into groupStats query to eliminate extra DB round-trip - Deduplicate ActionLink by extending ActionButton via styled-components attrs - Remove manual getSession fallback in leaderboard route (handled by getSessionFromHeader)
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/frontend/src/lib/groups/getGroupLeaderboard.ts">
<violation number="1" location="packages/frontend/src/lib/groups/getGroupLeaderboard.ts:130">
P3: Cast the new `COUNT(DISTINCT ...)` aggregate to `integer` for consistent numeric typing across Postgres count queries.
(Based on your team's feedback about casting Postgres COUNT results to integer for consistency.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/frontend/src/lib/auth/session.ts">
<violation number="1" location="packages/frontend/src/lib/auth/session.ts:179">
P1: Security: invalid Authorization header silently falls back to cookie auth instead of failing. When a client sends `Authorization: Bearer <invalid_token>`, the DB lookup returns 0 results and the code falls through to `return getSession()`, potentially authenticating via cookies. An explicit but invalid auth header should return `null`, not silently try another auth mechanism. Add `return null` at the end of the `if (authHeader)` block to prevent the fallback when a header was provided but invalid.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| if (result.length === 0) { | ||
| return null; | ||
| if (result.length > 0) { |
There was a problem hiding this comment.
P1: Security: invalid Authorization header silently falls back to cookie auth instead of failing. When a client sends Authorization: Bearer <invalid_token>, the DB lookup returns 0 results and the code falls through to return getSession(), potentially authenticating via cookies. An explicit but invalid auth header should return null, not silently try another auth mechanism. Add return null at the end of the if (authHeader) block to prevent the fallback when a header was provided but invalid.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/auth/session.ts, line 179:
<comment>Security: invalid Authorization header silently falls back to cookie auth instead of failing. When a client sends `Authorization: Bearer <invalid_token>`, the DB lookup returns 0 results and the code falls through to `return getSession()`, potentially authenticating via cookies. An explicit but invalid auth header should return `null`, not silently try another auth mechanism. Add `return null` at the end of the `if (authHeader)` block to prevent the fallback when a header was provided but invalid.</comment>
<file context>
@@ -150,48 +150,44 @@ export async function validateApiToken(
-
- if (result.length === 0) {
- return null;
+ if (result.length > 0) {
+ const { user } = result[0];
+ return {
</file context>
| .select({ | ||
| totalTokens: sql<number>`SUM(${submissions.totalTokens})`, | ||
| totalCost: sql<number>`SUM(CAST(${submissions.totalCost} AS DECIMAL(12,4)))`, | ||
| submittedUserCount: sql<number>`COUNT(DISTINCT ${submissions.userId})`, |
There was a problem hiding this comment.
P3: Cast the new COUNT(DISTINCT ...) aggregate to integer for consistent numeric typing across Postgres count queries.
(Based on your team's feedback about casting Postgres COUNT results to integer for consistency.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/groups/getGroupLeaderboard.ts, line 130:
<comment>Cast the new `COUNT(DISTINCT ...)` aggregate to `integer` for consistent numeric typing across Postgres count queries.
(Based on your team's feedback about casting Postgres COUNT results to integer for consistency.) </comment>
<file context>
@@ -117,27 +117,17 @@ async function fetchGroupLeaderboardData(
.select({
totalTokens: sql<number>`SUM(${submissions.totalTokens})`,
totalCost: sql<number>`SUM(CAST(${submissions.totalCost} AS DECIMAL(12,4)))`,
+ submittedUserCount: sql<number>`COUNT(DISTINCT ${submissions.userId})`,
})
.from(submissions)
</file context>
| submittedUserCount: sql<number>`COUNT(DISTINCT ${submissions.userId})`, | |
| submittedUserCount: sql<number>`CAST(COUNT(DISTINCT ${submissions.userId}) AS integer)`, |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| useEffect(() => { | ||
| if (!hasEverFetched.current && period === "all" && effectiveSortBy === "tokens") { | ||
| hasEverFetched.current = true; | ||
| return; | ||
| } | ||
| hasEverFetched.current = true; | ||
| fetchLeaderboard(period, effectiveSortBy); | ||
| }, [period, effectiveSortBy, fetchLeaderboard]); |
There was a problem hiding this comment.
🟡 Race condition in GroupDetailClient due to missing abort controller on leaderboard fetches
The fetchLeaderboard function called from the useEffect at GroupDetailClient.tsx:449-456 has no abort controller, so rapid changes to period or effectiveSortBy can cause out-of-order responses to overwrite newer data with stale results. For example, if a user clicks "This Month" then quickly "This Week", the slower "This Month" response could arrive last and overwrite the correct "This Week" data. This is inconsistent with both GroupsClient.tsx:186-211 and the main LeaderboardClient.tsx:1086-1091, which both properly create an AbortController, pass its signal to fetch, and call abort() in the cleanup function.
Comparison with correct pattern in GroupsClient.tsx
In GroupsClient.tsx:186-211:
useEffect(() => {
const abortController = new AbortController();
// ...
fetch(url, { signal: abortController.signal })
// ...
return () => abortController.abort();
}, [tab]);But in GroupDetailClient.tsx:432-456, fetchLeaderboard doesn't accept a signal, and the effect has no cleanup:
const fetchLeaderboard = useCallback(
async (p: Period, sortBy: string) => {
// no signal parameter
const res = await fetch(...);
// ...
}, [group.slug]
);
useEffect(() => {
// ...
fetchLeaderboard(period, effectiveSortBy);
// no cleanup / no abort
}, [period, effectiveSortBy, fetchLeaderboard]);Prompt for agents
In packages/frontend/src/app/(main)/groups/[slug]/GroupDetailClient.tsx, the fetchLeaderboard function (lines 432-447) needs to accept an AbortSignal parameter and pass it to fetch. Then the useEffect (lines 449-456) needs to create an AbortController, pass its signal to fetchLeaderboard, and return a cleanup function that aborts.
1. Change fetchLeaderboard (line 433) signature to: async (p: Period, sortBy: string, signal?: AbortSignal)
2. Pass the signal to fetch on line 436: fetch(`/api/groups/${group.slug}/leaderboard?...`, { signal })
3. In the useEffect at line 449, create an AbortController before calling fetchLeaderboard:
const abortController = new AbortController();
fetchLeaderboard(period, effectiveSortBy, abortController.signal);
4. Return a cleanup: return () => abortController.abort();
5. In the finally block of fetchLeaderboard, guard the setIsLoading with an aborted check, similar to the pattern in GroupsClient.tsx:205-208.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Adds a complete Groups feature to the Tokscale frontend:
Database
groups,group_members,group_invites0005_add_groups.sqlwith indexes, FKs, cascades, unique constraintsAPI (15 endpoints)
/api/groups/api/groups/[slug]/api/groups/[slug]/members/api/groups/[slug]/members/[userId]/role/api/groups/[slug]/invite/api/groups/[slug]/leave/api/groups/[slug]/leaderboard/api/groups/join/[token]/api/users/[username]/groups/api/leaderboard/groupsFrontend (11 pages/components)
Summary by cubic
Add Groups with roles, invites, and leaderboards so teams can track and compare token usage together. Integrated into profiles and the main leaderboard; run migration 0005_add_groups.sql. Includes auth fallback and query cleanup.
New Features
Bug Fixes
Written for commit 8494ab2. Summary will update on new commits.