feat(teams): add team icon, recommended status and favorites features #204
Conversation
- Add icon and is_recommended fields to Team model (CRD spec) - Create user_team_favorites table for user-team favorite relationships - Add favorites API endpoints (add/remove/list favorites) - Add showcase API endpoints (recommended teams, favorite teams) - Create icon-picker component with Lucide icons - Create TeamCard and TeamShowcase components for chat page - Update TeamEdit with icon picker and recommended switch - Add i18n translations for new features - Add database migration for user_team_favorites table
|
Warning Rate limit exceeded@qdaxb has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds team favorites: DB migration and ORM model, backend service and endpoints to add/remove/list favorites and recommended teams, schema/type updates for icons/recommendation, frontend APIs/components (IconPicker, TeamCard, TeamShowcase, TeamEdit) and i18n entries. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Frontend (TeamShowcase / TeamCard)
participant API as Backend API (teams adapter)
participant Svc as TeamFavoriteService
participant DB as Database (user_team_favorites, kinds)
UI->>API: GET /teams/showcase/recommended?limit=N
API->>Svc: fetch recommended teams + favorites for user
Svc->>DB: SELECT kinds WHERE kind='Team' AND is_active AND spec.isRecommended
Svc->>DB: SELECT team_ids FROM user_team_favorites WHERE user_id=?
DB-->>Svc: rows
Svc-->>API: team list (with is_favorited flags)
API-->>UI: 200 Team[]
UI->>API: POST /teams/{id}/favorite
API->>Svc: add_favorite(team_id, user_id)
Svc->>DB: SELECT kind by id (validate exists)
DB-->>Svc: kind row
Svc->>DB: INSERT INTO user_team_favorites (user_id, team_id) (if not exists)
DB-->>Svc: insert result
Svc-->>API: { message, is_favorited: true }
API-->>UI: 200 { message, is_favorited: true }
UI->>API: DELETE /teams/{id}/favorite
API->>Svc: remove_favorite(team_id, user_id)
Svc->>DB: DELETE FROM user_team_favorites WHERE user_id=? AND team_id=?
DB-->>Svc: delete result
Svc-->>API: { message, is_favorited: false }
API-->>UI: 200 { message, is_favorited: false }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (14)
backend/app/services/adapters/team_kinds.py (1)
467-474: update_with_user: consider normalizing cleared icons toNoneUpdating
team_crd.spec.iconandteam_crd.spec.isRecommendedfromupdate_datais correct. To keep the CRD cleaner and semantics clearer when clearing an icon, you might normalize falsy values toNoneinstead of storing empty strings:- if "icon" in update_data: - team_crd.spec.icon = update_data["icon"] + if "icon" in update_data: + # Normalize empty strings to None so CRD stays boolean/nullable, not empty text + team_crd.spec.icon = update_data["icon"] or NoneThis is optional and depends on how strictly you want to distinguish “no icon” vs. “empty string” in stored CRDs.
frontend/src/components/ui/icon-picker.tsx (1)
1-145: IconPicker implementation is solid; a few small polish opportunitiesOverall behavior (dynamic icon rendering, search filter, fallback initial, disabled state) looks correct and matches the i18n keys you added.
A couple of minor improvements you may consider:
Normalize cleared value: The reset button calls
onChange(''). If upstream treatsundefined/omitted as “no icon”, you might preferonChange('')→onChange('') || undefinedat the call site, or here:onClick={() => { onChange(''); setOpen(false); setSearch(''); }}so the local search state is also reset.
Bundle size / Lucide usage: Importing
* as LucideIconsand indexing by string is convenient but may force bundling the entire icon set. If bundle size becomes a concern later, you could switchCOMMON_ICONSto a map of explicitly imported icons instead of a dynamic lookup.Icon list validity: Since you defensively skip missing
LucideIcons[iconName], it would be good to double-check that all entries inCOMMON_ICONSexist in your installedlucide-reactversion so users don’t see silent gaps.Please verify the Lucide icon names against your installed version, for example by checking the
lucide-reactexports or running a quick type-check in your project.backend/alembic/versions/2a3b4c5d6e7f_add_user_team_favorites_table.py (1)
23-46: Migration is functionally sound; consider dropping redundant PK indexThe migration correctly creates
user_team_favoriteswith the expected columns and a unique(user_id, team_id)index, and the downgrade reverses changes cleanly.Two minor schema considerations:
- Most SQL backends already index the primary key, so
ix_user_team_favorites_idis typically redundant. It’s harmless but can be omitted in future migrations to reduce index clutter.created_atis nullable with no server default; this is fine given the ORM default, but if you expect every row to have a timestamp, you might prefernullable=Falsewith a DB-side default (e.g.,sa.func.now()).Neither point blocks this PR; they’re just schema hygiene tips for future migrations.
backend/app/models/user_team_favorite.py (1)
5-7: Add type hints to Column definitions.Per coding guidelines, type hints are required in all Python code. Consider adding type annotations to the class attributes.
+from typing import Optional from datetime import datetime -from sqlalchemy import Column, Integer, DateTime, Index +from sqlalchemy import Column, Integer, DateTime, Index +from sqlalchemy.orm import Mapped, mapped_column from app.db.base import BaseAlternatively, if using SQLAlchemy 2.0 style, you could use
Mappedannotations for better type inference.frontend/src/features/tasks/components/TeamShowcase.tsx (2)
57-91: Consider using functional state update to avoid stale closure.On line 78,
favoriteTeams.some(...)references thefavoriteTeamsstate directly within the callback. While the dependency array includesfavoriteTeams, using the functional update pattern would be more robust and avoid potential stale closure issues.} else { await teamApis.addTeamToFavorites(team.id); // Update recommended list setRecommendedTeams(prev => prev.map(t => (t.id === team.id ? { ...t, is_favorited: true } : t)) ); // Add to favorites if not already there - if (!favoriteTeams.some(t => t.id === team.id)) { - setFavoriteTeams(prev => [{ ...team, is_favorited: true }, ...prev].slice(0, 6)); - } + setFavoriteTeams(prev => { + if (prev.some(t => t.id === team.id)) return prev; + return [{ ...team, is_favorited: true }, ...prev].slice(0, 6); + }); }This also allows removing
favoriteTeamsfrom the dependency array.
82-84: Consider providing user feedback on errors.Currently, errors are only logged to the console. Users won't know if their favorite toggle failed. Consider adding a toast notification for failed operations.
frontend/src/features/tasks/components/TeamCard.tsx (2)
27-29: Add runtime validation for the icon name.If
team.iconcontains an invalid icon name,IconComponentwill beundefined(notnull), and the component will still try to render it, potentially causing issues.Consider adding explicit validation:
const IconComponent = team.icon - ? (LucideIcons as Record<string, React.ComponentType<{ className?: string }>>)[team.icon] + ? (LucideIcons as Record<string, React.ComponentType<{ className?: string }> | undefined>)[team.icon] ?? null : null;
81-99: Addaria-labelfor better accessibility.The button has a
titleattribute but lacks anaria-label. Screen readers benefit from explicit labeling on interactive elements.<button onClick={handleFavoriteClick} disabled={isTogglingFavorite} className={cn( 'flex-shrink-0 p-1.5 rounded-md transition-colors', 'hover:bg-primary/10', isTogglingFavorite && 'opacity-50 cursor-not-allowed' )} title={team.is_favorited ? 'Remove from favorites' : 'Add to favorites'} + aria-label={team.is_favorited ? 'Remove from favorites' : 'Add to favorites'} >backend/app/schemas/team.py (1)
37-38: Consider adding validation for the icon field.The
iconfield accepts any string, but it should be a valid Lucide icon name. Consider adding a validator to ensure icon names are valid or at least follow the expected format (PascalCase).Example validator:
from pydantic import field_validator import re @field_validator('icon') @classmethod def validate_icon(cls, v: str | None) -> str | None: if v is not None and not re.match(r'^[A-Z][a-zA-Z0-9]*$', v): raise ValueError('Icon must be a valid Lucide icon name (PascalCase)') return vbackend/app/services/team_favorite.py (3)
19-23: Use SQLAlchemy'sis_()for boolean comparison.As flagged by Ruff (E712), comparing booleans with
==can be problematic in SQLAlchemy. Use.is_(True)for explicit boolean comparison.team = db.query(Kind).filter( Kind.id == team_id, Kind.kind == "Team", - Kind.is_active == True + Kind.is_active.is_(True) ).first()
40-48: Consider handling potential database errors.The
db.add()anddb.commit()sequence could fail (e.g., due to a race condition causing a unique constraint violation). While the unique indexidx_user_team_favoriteshould prevent duplicates, a concurrent request could slip past the existence check.# Create favorite record favorite = UserTeamFavorite( user_id=user_id, team_id=team_id ) - db.add(favorite) - db.commit() + try: + db.add(favorite) + db.commit() + except IntegrityError: + db.rollback() + return {"message": "Team already in favorites", "is_favorited": True} return {"message": "Team added to favorites", "is_favorited": True}
72-78: Consider using.exists()for efficiency.For a simple existence check, using
exists()is more efficient than fetching the entire record.def is_team_favorited(self, db: Session, *, team_id: int, user_id: int) -> bool: """Check if a team is in user's favorites""" - favorite = db.query(UserTeamFavorite).filter( + return db.query( + db.query(UserTeamFavorite).filter( UserTeamFavorite.user_id == user_id, UserTeamFavorite.team_id == team_id - ).first() - return favorite is not None + ).exists() + ).scalar()backend/app/api/endpoints/adapter/teams.py (2)
170-173: Use SQLAlchemy'sis_()for boolean comparison.As flagged by Ruff (E712), use
.is_(True)instead of== Truefor SQLAlchemy boolean comparisons.teams = db.query(Kind).filter( Kind.kind == "Team", - Kind.is_active == True + Kind.is_active.is_(True) ).all()team = db.query(Kind).filter( Kind.id == favorite.team_id, Kind.kind == "Team", - Kind.is_active == True + Kind.is_active.is_(True) ).first()Also applies to: 206-210
190-216: N+1 query pattern in get_favorite_teams.For each favorite, this endpoint makes a separate database query to fetch the team. Consider using a single query with
INclause.def get_favorite_teams(...): """Get user's favorite teams""" from app.models.user_team_favorite import UserTeamFavorite # Get user's favorite team IDs favorites = db.query(UserTeamFavorite).filter( UserTeamFavorite.user_id == current_user.id ).order_by(UserTeamFavorite.created_at.desc()).limit(limit).all() + if not favorites: + return [] + + team_ids = [f.team_id for f in favorites] + teams = db.query(Kind).filter( + Kind.id.in_(team_ids), + Kind.kind == "Team", + Kind.is_active.is_(True) + ).all() + + # Create lookup map and preserve order + team_map = {t.id: t for t in teams} favorite_teams = [] - for favorite in favorites: - team = db.query(Kind).filter( - Kind.id == favorite.team_id, - Kind.kind == "Team", - Kind.is_active == True - ).first() - + for favorite in favorites: + team = team_map.get(favorite.team_id) if team: team_dict = team_kinds_service._convert_to_team_dict(team, db, team.user_id) team_dict["is_favorited"] = True favorite_teams.append(team_dict) return favorite_teams
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
backend/alembic/versions/2a3b4c5d6e7f_add_user_team_favorites_table.py(1 hunks)backend/app/api/endpoints/adapter/teams.py(3 hunks)backend/app/models/__init__.py(1 hunks)backend/app/models/user_team_favorite.py(1 hunks)backend/app/schemas/kind.py(1 hunks)backend/app/schemas/team.py(4 hunks)backend/app/services/adapters/team_kinds.py(4 hunks)backend/app/services/team_favorite.py(1 hunks)frontend/src/apis/team.ts(2 hunks)frontend/src/components/ui/icon-picker.tsx(1 hunks)frontend/src/features/settings/components/TeamEdit.tsx(6 hunks)frontend/src/features/tasks/components/ChatArea.tsx(2 hunks)frontend/src/features/tasks/components/TeamCard.tsx(1 hunks)frontend/src/features/tasks/components/TeamShowcase.tsx(1 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)frontend/src/types/api.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx}: Use TypeScript strict mode in all TypeScript/React code
Use functional components with hooks in React code instead of class components
Use Prettier formatter for TypeScript/React frontend code
Use ESLint with Next.js configuration for TypeScript/React frontend code
Use single quotes and no semicolons in TypeScript/React code
Useconstoverlet, never usevarin TypeScript/React code
Use onlyNEXT_PUBLIC_*environment variables for client-safe values in frontend
Files:
frontend/src/features/tasks/components/TeamShowcase.tsxfrontend/src/features/settings/components/TeamEdit.tsxfrontend/src/features/tasks/components/TeamCard.tsxfrontend/src/apis/team.tsfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/types/api.tsfrontend/src/components/ui/icon-picker.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use PEP 8 compliant code with Black formatter (line length: 88) and isort for imports in Python modules
Require type hints in all Python code
Use descriptive names for Python functions and variables
Write docstrings for all public Python functions and classes
Extract magic numbers to named constants in Python code
Keep Python functions to a maximum of 50 lines (preferred)
Files:
backend/app/schemas/kind.pybackend/app/models/__init__.pybackend/app/models/user_team_favorite.pybackend/app/services/team_favorite.pybackend/app/api/endpoints/adapter/teams.pybackend/app/schemas/team.pybackend/alembic/versions/2a3b4c5d6e7f_add_user_team_favorites_table.pybackend/app/services/adapters/team_kinds.py
backend/app/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
backend/app/**/*.py: Backend encrypts Git tokens before storing in database
Create new endpoint by adding route inapp/api/, schema inapp/schemas/, logic inapp/services/
Files:
backend/app/schemas/kind.pybackend/app/models/__init__.pybackend/app/models/user_team_favorite.pybackend/app/services/team_favorite.pybackend/app/api/endpoints/adapter/teams.pybackend/app/schemas/team.pybackend/app/services/adapters/team_kinds.py
backend/app/models/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add new SQLAlchemy model in
app/models/and generate migration with Alembic
Files:
backend/app/models/__init__.pybackend/app/models/user_team_favorite.py
frontend/src/types/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/types/**/*.{ts,tsx}: Store TypeScript type definitions insrc/types/directory
Define TypeScript types insrc/types/directory instead of inline
Files:
frontend/src/types/api.ts
frontend/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/components/**/*.{ts,tsx}: Use PascalCase for React component names and kebab-case for component file names
Use mobile-first responsive design approach in frontend components
Files:
frontend/src/components/ui/icon-picker.tsx
backend/alembic/versions/*.py
📄 CodeRabbit inference engine (AGENTS.md)
backend/alembic/versions/*.py: Always review auto-generated Alembic database migrations before applying them
Never edit already-applied Alembic migrations; create new migrations instead
Keep database migrations small and focused on specific changes
Files:
backend/alembic/versions/2a3b4c5d6e7f_add_user_team_favorites_table.py
🧠 Learnings (2)
📚 Learning: 2025-11-28T17:12:08.803Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T17:12:08.803Z
Learning: Applies to frontend/**/*.{ts,tsx} : Use functional components with hooks in React code instead of class components
Applied to files:
frontend/src/features/tasks/components/TeamShowcase.tsx
📚 Learning: 2025-11-28T17:12:08.803Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T17:12:08.803Z
Learning: Applies to backend/app/models/*.py : Add new SQLAlchemy model in `app/models/` and generate migration with Alembic
Applied to files:
backend/app/models/user_team_favorite.py
🧬 Code graph analysis (7)
backend/app/models/__init__.py (1)
backend/app/models/user_team_favorite.py (1)
UserTeamFavorite(10-22)
frontend/src/features/settings/components/TeamEdit.tsx (4)
frontend/src/apis/team.ts (1)
createTeam(66-68)frontend/src/features/settings/services/teams.ts (1)
createTeam(19-21)frontend/src/components/ui/icon-picker.tsx (1)
IconPicker(39-145)frontend/src/components/ui/switch.tsx (1)
Switch(33-33)
frontend/src/features/tasks/components/TeamCard.tsx (3)
backend/app/schemas/kind.py (1)
Team(189-195)frontend/src/types/api.ts (1)
Team(86-105)frontend/src/lib/utils.ts (1)
cn(8-10)
frontend/src/apis/team.ts (3)
frontend/src/apis/client.ts (1)
apiClient(105-105)backend/app/schemas/kind.py (1)
Team(189-195)frontend/src/types/api.ts (1)
Team(86-105)
frontend/src/features/tasks/components/ChatArea.tsx (1)
frontend/src/features/tasks/components/TeamShowcase.tsx (1)
TeamShowcase(19-151)
backend/app/services/team_favorite.py (2)
backend/app/models/user_team_favorite.py (1)
UserTeamFavorite(10-22)backend/app/models/kind.py (1)
Kind(14-30)
backend/app/api/endpoints/adapter/teams.py (5)
backend/app/core/security.py (1)
get_current_user(27-50)backend/app/services/team_favorite.py (3)
add_favorite(16-48)remove_favorite(50-63)get_user_favorite_team_ids(65-70)backend/app/models/kind.py (1)
Kind(14-30)backend/app/services/adapters/team_kinds.py (1)
_convert_to_team_dict(718-806)backend/app/models/user_team_favorite.py (1)
UserTeamFavorite(10-22)
🪛 Ruff (0.14.6)
backend/app/services/team_favorite.py
22-22: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
backend/app/api/endpoints/adapter/teams.py
135-135: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
136-136: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
149-149: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
150-150: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
163-163: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
164-164: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
172-172: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
193-193: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
194-194: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
209-209: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Executor
🔇 Additional comments (25)
backend/app/schemas/kind.py (1)
176-181: TeamSpec: icon & isRecommended fields look consistent with CRD usageThe new
iconandisRecommendedfields align with existing camelCase CRD conventions and with howTeamKindsServicereads/writes these values. No issues from a schema perspective.backend/app/services/adapters/team_kinds.py (2)
123-131: create_with_user: CRD spec mapping for icon & isRecommended is correctPersisting
iconandisRecommendedintoteam_json["spec"]usinggetattr(obj_in, 'icon', None)/getattr(obj_in, 'is_recommended', False)safely handles older payloads and keeps the snake_case ↔ camelCase mapping consistent.
790-806: _convert_to_team_dict: new icon & is_recommended fields match frontend Team typeExposing
iconfromteam_crd.spec.iconandis_recommendedfromteam_crd.spec.isRecommendedhere matches the frontendTeaminterface and keeps the internal CRD camelCase separate from API snake_case. Looks good.frontend/src/i18n/locales/en/common.json (1)
186-191: New teams i18n keys align with feature surfaceThe added
teams.*strings for recommended/favorites/icon selection are coherent and match how the UI components reference them (e.g.,teams.select_iconinIconPicker). No functional issues.frontend/src/i18n/locales/zh-CN/common.json (1)
187-192: Zh-CN teams keys correctly mirror the new English stringsThe Chinese translations for recommended apps, favorites, and icon selection match the English keys one-to-one and read naturally. Internationalization coverage looks complete for this feature.
backend/app/models/__init__.py (1)
17-25: Exporting UserTeamFavorite from models package is correctImporting
UserTeamFavoritehere and including it in__all__keeps the models package consistent and allows other code to import the new model viaapp.models. No issues.frontend/src/types/api.ts (1)
86-105: Team type extensions match backend and UI expectationsAdding
icon?: string,is_recommended?: boolean, andis_favorited?: booleantoTeamcleanly reflects the new backend fields and what the UI components need to render showcases and favorites. The naming matches the dict keys fromTeamKindsService, so the typing looks accurate.frontend/src/features/tasks/components/ChatArea.tsx (2)
12-12: LGTM!Import is correctly added for the new TeamShowcase component.
608-610: LGTM!The TeamShowcase integration is well-placed below the repository/branch controls in the pre-chat view. It correctly passes
handleTeamChangeto enable team selection, and the placement makes sense as users can browse recommended/favorite teams before starting a conversation.frontend/src/features/tasks/components/TeamShowcase.tsx (1)
19-150: LGTM - Well-structured component.The component follows React best practices:
- Uses functional component with hooks (per coding guidelines)
- Properly handles loading states
- Deduplicates concurrent toggle operations via
togglingFavoriteIdsSet- Responsive grid layout with appropriate breakpoints
- Proper i18n usage for labels
frontend/src/features/settings/components/TeamEdit.tsx (5)
71-72: LGTM!State variables for the new icon and recommendation features are properly initialized with sensible defaults.
206-207: LGTM!The state restoration and reset logic correctly handles both editing existing teams (restoring icon/is_recommended from
formTeam) and creating new teams (resetting to defaults).Also applies to: 215-216
406-407: LGTM!The save payloads correctly include the new
iconandis_recommendedfields for both update and create operations. Usingicon || undefinedensures empty strings are not sent unnecessarily.Also applies to: 415-416
490-499: LGTM!The IconPicker integration is well-implemented, placed adjacent to the name input for good UX. Props are correctly wired:
value={icon},onChange={setIcon}, andteamName={name}for the default avatar letter fallback.
502-511: LGTM!The "Set as Recommended" switch is cleanly implemented with proper i18n label and controlled state binding.
frontend/src/apis/team.ts (2)
15-16: LGTM!The
CreateTeamRequestinterface is properly extended with optionaliconandis_recommendedfields, matching the Team interface intypes/api.ts.
87-98: LGTM!The new API methods are well-implemented:
addTeamToFavoritesandremoveTeamFromFavoritesuse appropriate HTTP verbs (POST/DELETE)getRecommendedTeamsandgetFavoriteTeamsprovide sensible defaults for thelimitparameter- Return types are properly specified
- Follows the existing patterns in the file
backend/app/models/user_team_favorite.py (1)
15-16: I'll help you verify this review comment about foreign key constraints. Let me start by exploring the codebase to understand the actual table structure and naming conventions.
<function_calls>
#!/bin/bashFirst, let's find and examine the user_team_favorite.py file
find backend/app/models -name "*.py" -type f | head -20
</function_calls>
#!/bin/bashCheck the user_team_favorite.py file
cat -n backend/app/models/user_team_favorite.py 2>/dev/null || echo "File not found, searching for similar files..."
fd "user_team" backend/app/models/ -type f
</function_calls>
<function_calls>
#!/bin/bashList all model files to understand the structure
ls -la backend/app/models/ 2>/dev/null | head -30
</function_calls>
<function_calls>
#!/bin/bashSearch for User and Team model definitions to find actual table names
rg -n "class User\b|class Team\b" --type=py backend/app/models/ -A3
</function_calls>
<function_calls>
#!/bin/bashLook for any ForeignKey usage in existing models to see the pattern
rg -n "ForeignKey" --type=py backend/app/models/ -B2 -A2 | head -50
</function_calls>frontend/src/features/tasks/components/TeamCard.tsx (3)
7-11: LGTM!Imports are clean and well-organized. Using
* as LucideIconsfor dynamic icon lookup is a reasonable approach for this use case.
31-32: Handle empty team name edge case.If
team.nameis an empty string,firstLetterdefaults to'T', which is good defensive coding. However, the logicteam.name.trim().charAt(0)will return an empty string if the name is whitespace-only.
41-115: LGTM!The component structure is well-organized with clear separation of concerns: header with icon/name/favorite button, and an optional agent type badge. Event propagation is correctly stopped on the favorite button click.
backend/app/schemas/team.py (1)
50-51: LGTM!The schema updates correctly propagate the new fields through the appropriate models:
iconandis_recommendedinTeamUpdateare optional for partial updatesis_favoritedis only in response schemas (TeamInDB,TeamDetail), not in create/update schemas, which is correct since it's user-specific derived stateAlso applies to: 62-62, 79-81
backend/app/services/team_favorite.py (1)
65-70: LGTM!Efficient query that only selects the
team_idcolumn and returns a set for O(1) lookups. This is used appropriately in the showcase endpoints to batch-check favorited status.backend/app/api/endpoints/adapter/teams.py (2)
132-143: LGTM!The favorite endpoints correctly delegate to the service layer and use proper authentication. The Ruff B008 warnings about
Depends()in default arguments are false positives - this is the standard FastAPI pattern used consistently throughout this file.Also applies to: 146-157
160-187: Route ordering conflict: showcase routes may be unreachable.FastAPI matches routes in declaration order. The routes
/showcase/recommendedand/showcase/favoritesare defined after/{team_id}/...routes. When a request comes for/showcase/recommended, FastAPI may match it against/{team_id}/input-parametersor/{team_id}/favoritewithteam_id="showcase", causing a validation error or unexpected behavior.Move the showcase routes before any
/{team_id}routes:+@router.get("/showcase/recommended", response_model=List[Dict[str, Any]]) +def get_recommended_teams(...): + ... + +@router.get("/showcase/favorites", response_model=List[Dict[str, Any]]) +def get_favorite_teams(...): + ... + @router.get("/{team_id}", response_model=TeamDetail) def get_team(...): ...Likely an incorrect or invalid review comment.
| # Get all teams where isRecommended is true | ||
| teams = db.query(Kind).filter( | ||
| Kind.kind == "Team", | ||
| Kind.is_active == True | ||
| ).all() | ||
|
|
||
| recommended_teams = [] | ||
| favorite_team_ids = team_favorite_service.get_user_favorite_team_ids(db=db, user_id=current_user.id) | ||
|
|
||
| for team in teams: | ||
| team_crd = Team.model_validate(team.json) | ||
| if team_crd.spec.isRecommended: | ||
| team_dict = team_kinds_service._convert_to_team_dict(team, db, team.user_id) | ||
| team_dict["is_favorited"] = team.id in favorite_team_ids | ||
| recommended_teams.append(team_dict) | ||
| if len(recommended_teams) >= limit: | ||
| break | ||
|
|
There was a problem hiding this comment.
Inefficient query: fetches all teams and filters in Python.
This endpoint queries ALL active teams from the database and then filters for isRecommended in Python. This doesn't scale well as the number of teams grows.
Consider adding a database-level filter if possible, or document the expected scale. If isRecommended is stored in the JSON column, you may need to use JSON path queries depending on your database:
# For PostgreSQL with JSONB:
# Kind.json['spec']['isRecommended'].as_boolean() == True
# Or if you cannot filter at DB level, at least add a comment explaining why:
# Note: isRecommended is stored in JSON, filtering in Python is intentional🧰 Tools
🪛 Ruff (0.14.6)
172-172: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/teams.py around lines 169 to 186, the code
pulls all active Team kinds and filters isRecommended in Python which is
inefficient and unbounded; change the DB query to filter on the isRecommended
flag at the database level (and apply the limit there) — for Postgres JSONB use
the JSON path/boolean extraction (e.g.
Kind.json['spec']['isRecommended'].as_boolean() == True or equivalent SQLAlchemy
expression) and include .limit(limit) so only needed rows are fetched; if your
DB/drivers do not support JSON filtering, add a clear comment explaining
isRecommended lives inside the JSON and why in-Python filtering is intentional,
and at minimum apply .limit(limit) to the initial query to avoid loading the
entire table.
| id = Column(Integer, primary_key=True, index=True) | ||
| user_id = Column(Integer, nullable=False, index=True) # User who favorited the team | ||
| team_id = Column(Integer, nullable=False, index=True) # Team that was favorited | ||
| created_at = Column(DateTime, default=datetime.now) |
There was a problem hiding this comment.
Use datetime.utcnow or timezone-aware datetime instead of datetime.now.
Using datetime.now stores local server time, which can cause inconsistencies across different server timezones. Consider using datetime.utcnow for consistency with typical database practices.
- created_at = Column(DateTime, default=datetime.now)
+ created_at = Column(DateTime, default=datetime.utcnow)📝 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.
| created_at = Column(DateTime, default=datetime.now) | |
| created_at = Column(DateTime, default=datetime.utcnow) |
🤖 Prompt for AI Agents
In backend/app/models/user_team_favorite.py around line 17, the Column default
uses datetime.now which records local server time; replace it with a UTC or
timezone-aware default — e.g. use default=datetime.utcnow (imported from
datetime) or prefer a timezone-aware value like default=lambda:
datetime.now(timezone.utc) after importing timezone, or use SQLAlchemy's
func.now() to rely on DB-side UTC timestamps; update imports accordingly.
Resolved conflicts while preserving team icon and favorites showcase features: - teams.py: Added favorite and recommended teams API endpoints - __init__.py: Added UserTeamFavorite model import - team_kinds.py: Added icon and isRecommended fields support in team creation
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/app/api/endpoints/adapter/teams.py (1)
170-196: Recommended teams query: global visibility and full‑table scanA couple of points to tighten here:
- You currently load all active
Teamkinds and then filterisRecommendedin Python. This doesn’t scale well as the number of teams grows (same concern as the earlier review). Where your DB supports JSON filtering, it’s preferable to push both theisRecommendedpredicate and the limit down to the database; otherwise, at least consider a DB‑level limit and/or documenting the expected small scale.get_recommended_teamsreturns any team withisRecommended=Trueregardless of owner or sharing state. Because any caller who canupdate_teamcan setis_recommended=True, this effectively lets users broadcast their teams into a global showcase. Please double‑check that this cross‑user exposure is intentional; if not, you likely want to:
- Filter recommended teams to those the current user owns or has shared access to, or
- Restrict who is allowed to mark a team as recommended (e.g., admin‑only).
- Minor: you’re calling the “private” helper
team_kinds_service._convert_to_team_dictfrom the API layer. If this becomes more widely used, consider promoting a public wrapper method on the service.
🧹 Nitpick comments (2)
backend/app/models/__init__.py (1)
16-18: Add looks good; sort__all__to satisfy Ruff/isort.The new
UserTeamFavoriteimport and inclusion in__all__are correct and consistent with the model. To comply with the Ruff RUF022 hint and the isort/PEP 8 style, please sort the__all__entries alphabetically:-__all__ = ["User", "Kind", "Subtask", "SharedTeam", "SkillBinary", "UserTeamFavorite"] +__all__ = ["Kind", "SharedTeam", "SkillBinary", "Subtask", "User", "UserTeamFavorite"]backend/app/api/endpoints/adapter/teams.py (1)
199-234: Favorite teams endpoint is correct but does N+1 lookupsFunctionally this looks fine: you page through
UserTeamFavoritefor the current user, hydrate eachTeam, and markis_favorited=True. Given the hardlimitof 20, the per‑favoriteKindlookup is unlikely to be a real bottleneck, but you could collapse this into a single join query onKindif this endpoint ever needs to scale.Also, as with the recommended endpoint, you’re using
Kind.is_active == True; if you decide to align with Ruff’s preference later, considerfilter(Kind.is_active)or an explicit.is_(True)expression.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/app/api/endpoints/adapter/teams.py(3 hunks)backend/app/models/__init__.py(1 hunks)backend/app/schemas/kind.py(1 hunks)backend/app/schemas/team.py(4 hunks)backend/app/services/adapters/team_kinds.py(3 hunks)frontend/src/features/tasks/components/ChatArea.tsx(2 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/schemas/kind.py
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{py,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments MUST be written in English, including inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions
Files:
backend/app/api/endpoints/adapter/teams.pyfrontend/src/features/tasks/components/ChatArea.tsxbackend/app/services/adapters/team_kinds.pybackend/app/schemas/team.pybackend/app/models/__init__.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code must be PEP 8 compliant, formatted with Black (line length: 88), and use isort for imports
Python code must use type hints
Python functions should have descriptive names, include docstrings for public functions/classes, extract magic numbers to constants, and keep functions to a maximum of 50 lines preferred
Files:
backend/app/api/endpoints/adapter/teams.pybackend/app/services/adapters/team_kinds.pybackend/app/schemas/team.pybackend/app/models/__init__.py
**/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript/React code must use strict mode, functional components with hooks, Prettier formatter, ESLint (Next.js config), single quotes, and no semicolons
Files:
frontend/src/features/tasks/components/ChatArea.tsx
**/frontend/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
constoverlet, nevervarin TypeScript/JavaScript
Files:
frontend/src/features/tasks/components/ChatArea.tsx
**/frontend/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/frontend/**/*.{tsx,jsx}: Use Tailwind CSS utility classes with the color system CSS variables for consistent styling across the calm UI philosophy
Frontend mobile-first responsive design approach should be used with Tailwind CSS breakpoints (sm, md, lg, etc.)
Files:
frontend/src/features/tasks/components/ChatArea.tsx
**/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
React hooks from react-hook-form and zod validation must be used for form implementation in TypeScript components
Files:
frontend/src/features/tasks/components/ChatArea.tsx
🧬 Code graph analysis (2)
frontend/src/features/tasks/components/ChatArea.tsx (1)
frontend/src/features/tasks/components/TeamShowcase.tsx (1)
TeamShowcase(19-151)
backend/app/models/__init__.py (1)
backend/app/models/user_team_favorite.py (1)
UserTeamFavorite(10-22)
🪛 Ruff (0.14.6)
backend/app/api/endpoints/adapter/teams.py
149-149: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
150-150: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
161-161: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
162-162: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
173-173: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
174-174: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
180-180: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
202-202: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
203-203: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
224-224: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
backend/app/models/__init__.py
18-18: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (9)
frontend/src/i18n/locales/en/common.json (1)
186-191: ✓ i18n translations for team features look good.The six new translation keys are well-placed within the "teams" section and follow the existing snake_case naming convention. The strings are clear, concise, and align with the feature descriptions in the PR objectives.
Please verify that corresponding Chinese translations are added to
frontend/src/i18n/locales/zh-CN/common.jsonwith equivalent completeness to maintain cross-language consistency.frontend/src/i18n/locales/zh-CN/common.json (1)
187-192: English locale file path isen/common.json(noten-US/common.json); all translations verified and properly synced.The Chinese translations are well-crafted and follow established conventions in the file. All six new keys have been verified to exist with corresponding English translations in
frontend/src/i18n/locales/en/common.json, confirming proper i18n alignment across both available locales.frontend/src/features/tasks/components/ChatArea.tsx (2)
12-12: LGTM!The import is clean and follows the established pattern for local component imports.
621-623: LGTM!The TeamShowcase integration is clean and well-placed:
- Only renders on the new task screen (
!hasMessages), which is appropriate for team discovery- Uses the existing
handleTeamChangehandler, ensuring consistent behavior with the TeamSelector- Placement after repository controls provides a logical flow for users to discover and select teams
The callback type compatibility is safe—
handleTeamChangeacceptsTeam | null, so passingTeamobjects from TeamShowcase works correctly.backend/app/services/adapters/team_kinds.py (2)
137-145: CRD spec now carries icon and is_recommended consistentlyThe new
spec.iconandspec.isRecommendedassignments on create line up withTeamCreatefields and the CRD schema; usinggetattrkeeps this backward compatible. No issues here.
891-904: Team dict output correctly surfaces icon and is_recommendedExposing
iconandis_recommendedfromteam_crd.spechere keeps the API payloads aligned with the new schemas and frontend expectations (Lucide icon name + recommended flag). MappingisRecommended→is_recommendedalso stays consistent with the rest of the API.backend/app/schemas/team.py (1)
38-47: Schema additions for icon / recommended / favorited are coherentThe new fields on
TeamBase,TeamUpdate,TeamInDB, andTeamDetailuse sensible defaults and match how the service layer and endpoints consume/emiticon,is_recommended, andis_favorited. This keeps the data model consistent end‑to‑end.Also applies to: 55-64, 66-77, 82-97
backend/app/api/endpoints/adapter/teams.py (2)
7-8: New imports are minimal and purpose‑drivenAdding
List/Dict/Anyandteam_favorite_serviceis appropriate for the new response models and endpoints; import usage is clear and localized.Also applies to: 28-28
146-167: Favorite add/remove endpoints cleanly delegate to service layerThe
POST /{team_id}/favoriteandDELETE /{team_id}/favoriteendpoints are straightforward, REST‑ish, and properly reuseteam_favorite_servicewith the authenticated user context. As long asteam_favorite_serviceenforces access/ownership checks, this wiring looks good.
| # Handle icon update | ||
| if "icon" in update_data: | ||
| team_crd.spec.icon = update_data["icon"] | ||
|
|
||
| # Handle is_recommended update | ||
| if "is_recommended" in update_data: | ||
| team_crd.spec.isRecommended = update_data["is_recommended"] | ||
|
|
There was a problem hiding this comment.
Avoid persisting None into spec.isRecommended
TeamUpdate.is_recommended is Optional[bool], but Team.spec.isRecommended is effectively a boolean flag. With the current code, a client sending "is_recommended": null will write None into the spec, which can break assumptions that this is always a boolean.
Consider only updating when a concrete boolean is provided:
- # Handle is_recommended update
- if "is_recommended" in update_data:
- team_crd.spec.isRecommended = update_data["is_recommended"]
+ # Handle is_recommended update (ignore explicit nulls)
+ if "is_recommended" in update_data and update_data["is_recommended"] is not None:
+ team_crd.spec.isRecommended = update_data["is_recommended"]You can decide separately whether you want a way to “reset” this flag (e.g., treat None as False explicitly).
🤖 Prompt for AI Agents
In backend/app/services/adapters/team_kinds.py around lines 519-526, the code
unconditionally assigns update_data["is_recommended"] to
team_crd.spec.isRecommended which allows None to be persisted; change the logic
to only update team_crd.spec.isRecommended when update_data["is_recommended"] is
a concrete boolean (i.e., check "is_recommended" in update_data and
update_data["is_recommended"] is not None or
isinstance(update_data["is_recommended"], bool)) so None values are ignored (or,
if you prefer a reset behavior, explicitly set to False when the value is None).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/app/api/endpoints/adapter/teams.py (1)
179-205: Recommended teams query still loads all teams; filter/limit at DB level and refine boolean checkThis implementation still queries all active
Teamkinds and then:
- Parses each CRD to check
spec.isRecommendedin Python.- Applies the
limitonly after in‑memory filtering.On a large dataset this can become a scalability problem and negate the
limitentirely. It also repeats the previous review concern about doing theisRecommendedfiltering in the database when possible.
At minimum, you should:
- Apply
.limit(limit)at the database level so you don’t load the entire table.- If your database supports JSON/JSONB querying (e.g., PostgreSQL), move the
isRecommendedfilter into the SQLAlchemy query using a JSON path expression.- Replace
Kind.is_active == Truewith an explicit boolean expression to satisfy Ruff and make intent clearer, e.g.:- teams = db.query(Kind).filter(Kind.kind == "Team", Kind.is_active == True).all() + teams = ( + db.query(Kind) + .filter(Kind.kind == "Team", Kind.is_active.is_(True)) + .all() + )(And mirror the
is_(True)pattern wherever you filter onis_active.)If JSON‑level filtering is not supported in your deployment, it would help to add a brief comment explaining why Python‑side filtering is intentional here and that the expected table size is bounded.
🧹 Nitpick comments (6)
frontend/src/features/tasks/components/TeamCard.tsx (2)
87-87: Use i18n translations instead of hardcoded strings.The button title uses hardcoded English strings, but the codebase has translation keys available (
teams.remove_favoriteandteams.add_favorite). This will cause inconsistency for non-English users.Add the
useTranslationhook and use the translation keys:+'use client'; + +import React from 'react'; +import { Star } from 'lucide-react'; +import * as LucideIcons from 'lucide-react'; +import { cn } from '@/lib/utils'; +import type { Team } from '@/types/api'; +import { useTranslation } from '@/hooks/useTranslation'; ... export default function TeamCard({ team, onSelect, onToggleFavorite, isTogglingFavorite = false, }: TeamCardProps) { + const { t } = useTranslation('common'); // Get the icon component dynamically ... - title={team.is_favorited ? 'Remove from favorites' : 'Add to favorites'} + title={team.is_favorited ? t('teams.remove_favorite') : t('teams.add_favorite')}
27-31: Consider extracting the dynamic icon lookup to a shared utility.This icon resolution pattern is duplicated in
TeamList.tsx(lines 277-299). Extracting it to a reusable utility or component would improve maintainability.Example utility:
// utils/lucide-icon.ts import * as LucideIcons from 'lucide-react'; export function getLucideIcon(iconName?: string): React.ComponentType<{ className?: string }> | null { if (!iconName) return null; return (LucideIcons as unknown as Record<string, React.ComponentType<{ className?: string }>>)[iconName] ?? null; }frontend/src/features/settings/components/TeamList.tsx (1)
277-299: Consider simplifying the IIFE icon rendering pattern.The immediately-invoked function expression (IIFE) adds unnecessary complexity. This could be refactored to use a small helper component or inline conditional rendering for better readability.
Alternative approach using a local component:
// Define above the return statement const TeamIcon = ({ team }: { team: Team }) => { const IconComponent = team.icon ? (LucideIcons as unknown as Record<string, React.ComponentType<{ className?: string }>>)[team.icon] : null; const firstLetter = team.name.trim().charAt(0).toUpperCase() || 'T'; return ( <div className="flex-shrink-0 w-10 h-10 rounded-lg flex items-center justify-center bg-primary/10 text-primary"> {IconComponent ? ( <IconComponent className="w-5 h-5" /> ) : ( <span className="text-lg font-semibold">{firstLetter}</span> )} </div> ); }; // Then use: <TeamIcon team={team} />backend/app/services/team_favorite.py (1)
5-5: Remove unused import.
Listis imported but not used in this file.-from typing import Any, Dict, List, Set +from typing import Any, Dict, Setbackend/app/api/endpoints/adapter/teams.py (2)
155-176: Favorite add/remove endpoints are fine; consider typing responsesThe favorite/unfavorite endpoints are thin wrappers around
team_favorite_service, which is good. One improvement to consider is adding explicitresponse_models (e.g., a small Pydantic schema withmessageandis_favorited: bool) so the OpenAPI spec documents these responses and consumers can rely on a stable shape instead of a genericDict[str, Any].
208-243: Favorite showcase works but has an N+1 pattern and repeat boolean filterThe
/showcase/favoritesendpoint correctly:
- Orders favorites by
created_at descand enforces thelimitat theUserTeamFavoritelevel.- Filters out inactive or non‑Team kinds.
- Marks each returned team as
is_favorited = True.Two possible refinements (non‑blocking):
N+1 queries: For each favorite you run a separate
Kindquery. Withlimitcapped at 20 this is acceptable, but you could reduce chatter by fetching all needed teams in one query usingin_([f.team_id for f in favorites])and then preserving order in Python.Boolean column style: Same as in
get_recommended_teams, you can avoidKind.is_active == Trueand useKind.is_active.is_(True)(or whatever pattern you standardize on) to keep Ruff quiet and make the intent explicit.Neither change is critical now, but they will help if the number of favorites per user grows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/app/api/endpoints/adapter/teams.py(4 hunks)backend/app/models/user_team_favorite.py(1 hunks)backend/app/services/team_favorite.py(1 hunks)frontend/src/components/ui/icon-picker.tsx(1 hunks)frontend/src/features/settings/components/TeamList.tsx(5 hunks)frontend/src/features/settings/services/teams.ts(1 hunks)frontend/src/features/tasks/components/TeamCard.tsx(1 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/ui/icon-picker.tsx
- backend/app/models/user_team_favorite.py
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{py,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments MUST be written in English, including inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions
Files:
frontend/src/features/settings/services/teams.tsfrontend/src/features/settings/components/TeamList.tsxbackend/app/api/endpoints/adapter/teams.pyfrontend/src/features/tasks/components/TeamCard.tsxbackend/app/services/team_favorite.py
**/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript/React code must use strict mode, functional components with hooks, Prettier formatter, ESLint (Next.js config), single quotes, and no semicolons
Files:
frontend/src/features/settings/services/teams.tsfrontend/src/features/settings/components/TeamList.tsxfrontend/src/features/tasks/components/TeamCard.tsx
**/frontend/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
constoverlet, nevervarin TypeScript/JavaScript
Files:
frontend/src/features/settings/services/teams.tsfrontend/src/features/settings/components/TeamList.tsxfrontend/src/features/tasks/components/TeamCard.tsx
**/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
React hooks from react-hook-form and zod validation must be used for form implementation in TypeScript components
Files:
frontend/src/features/settings/services/teams.tsfrontend/src/features/settings/components/TeamList.tsxfrontend/src/features/tasks/components/TeamCard.tsx
**/frontend/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/frontend/**/*.{tsx,jsx}: Use Tailwind CSS utility classes with the color system CSS variables for consistent styling across the calm UI philosophy
Frontend mobile-first responsive design approach should be used with Tailwind CSS breakpoints (sm, md, lg, etc.)
Files:
frontend/src/features/settings/components/TeamList.tsxfrontend/src/features/tasks/components/TeamCard.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code must be PEP 8 compliant, formatted with Black (line length: 88), and use isort for imports
Python code must use type hints
Python functions should have descriptive names, include docstrings for public functions/classes, extract magic numbers to constants, and keep functions to a maximum of 50 lines preferred
Files:
backend/app/api/endpoints/adapter/teams.pybackend/app/services/team_favorite.py
🧬 Code graph analysis (4)
frontend/src/features/settings/services/teams.ts (1)
frontend/src/apis/team.ts (1)
teamApis(60-99)
backend/app/api/endpoints/adapter/teams.py (5)
backend/app/services/team_favorite.py (3)
get_user_favorite_team_ids(70-77)add_favorite(17-48)remove_favorite(50-68)backend/app/core/security.py (1)
get_current_user(28-50)backend/app/api/dependencies.py (1)
get_db(12-21)backend/app/models/kind.py (1)
Kind(25-42)backend/app/models/user_team_favorite.py (1)
UserTeamFavorite(12-25)
frontend/src/features/tasks/components/TeamCard.tsx (2)
frontend/src/types/api.ts (1)
Team(86-105)frontend/src/lib/utils.ts (1)
cn(8-10)
backend/app/services/team_favorite.py (2)
backend/app/models/kind.py (1)
Kind(25-42)backend/app/models/user_team_favorite.py (1)
UserTeamFavorite(12-25)
🪛 Ruff (0.14.6)
backend/app/api/endpoints/adapter/teams.py
158-158: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
159-159: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
170-170: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
171-171: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
182-182: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
183-183: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
189-189: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
211-211: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
212-212: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
233-233: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
backend/app/services/team_favorite.py
24-24: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e-tests
🔇 Additional comments (10)
frontend/src/features/settings/services/teams.ts (1)
43-56: LGTM!The toggle logic is correct and follows the existing service patterns. The function properly delegates to the appropriate API method based on the current favorite state.
frontend/src/i18n/locales/en/common.json (1)
186-192: LGTM!The new translation keys are well-named and consistent with the existing conventions. The English text is clear and grammatically correct.
frontend/src/features/tasks/components/TeamCard.tsx (1)
43-108: Well-structured component with proper event handling.The component correctly handles click propagation, provides accessible button states with
disabled, and uses consistent Tailwind utility classes with the design system CSS variables.frontend/src/features/settings/components/TeamList.tsx (2)
213-232: LGTM!The favorite toggle handler correctly prevents concurrent requests, updates local state optimistically based on API response, and handles errors with appropriate user feedback.
335-352: LGTM!The favorite button correctly displays filled/outline icons based on state, uses proper accessibility attributes (
title,disabled), and leverages i18n translations.backend/app/services/team_favorite.py (3)
22-26: Note on static analysis hint:Kind.is_active == Trueis correct here.The Ruff E712 warning is a false positive in this context. When building SQLAlchemy filter expressions,
== Trueis required to generate the correct SQL comparison. Using justKind.is_activewould pass the column object itself rather than creating a comparison expression.
50-68: LGTM!The
remove_favoritemethod is properly idempotent—returning a success response even when the favorite doesn't exist is the correct pattern for DELETE operations.
70-88: LGTM!The query methods are efficient:
get_user_favorite_team_idsfetches only the required column and returns a set for O(1) lookups, andis_team_favoritedusesfirst()for an existence check.frontend/src/i18n/locales/zh-CN/common.json (1)
187-193: Ensure key paths match English locale and frontend usageThe new keys and translations look good. Please just double‑check that the key paths (e.g.,
teams.recommended,teams.favorites,teams.select_icon) match both the English locale file and howt()is called in the UI (notteams.share.recommendedvsteams.recommended, etc.), so we don’t end up with missing translations at runtime.backend/app/api/endpoints/adapter/teams.py (1)
47-52: is_favorited annotation inlist_teamslooks correctUsing
get_user_favorite_team_idsand annotating each returned team dict with anis_favoritedboolean is a clean way to expose favorite state without changing the service API. This looks consistent and side‑effect‑free.
| if existing: | ||
| return {"message": "Team already in favorites", "is_favorited": True} | ||
|
|
||
| # Create favorite record | ||
| favorite = UserTeamFavorite(user_id=user_id, team_id=team_id) | ||
| db.add(favorite) | ||
| db.commit() | ||
|
|
||
| return {"message": "Team added to favorites", "is_favorited": True} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for UserTeamFavorite model and foreign key definitions
echo "=== Searching for UserTeamFavorite model ==="
fd -t py -i "*favorite*" | head -20
echo -e "\n=== Searching for ForeignKey references in Python files ==="
rg -n "ForeignKey|foreign_key" --type py -i "team_id|user_team_favorite" -A 2 -B 2
echo -e "\n=== Searching for model class definitions ==="
rg -n "class UserTeamFavorite\|class.*Favorite" --type py -A 10Repository: wecode-ai/Wegent
Length of output: 466
🏁 Script executed:
#!/bin/bash
# Search for the file under review
echo "=== Looking for team_favorite.py ==="
find . -name "team_favorite.py" -type f
echo -e "\n=== Looking for model files with 'favorite' in name ==="
find . -name "*favorite*.py" -type f
echo -e "\n=== Searching for UserTeamFavorite class definition ==="
rg -n "class UserTeamFavorite" --type python -A 15
echo -e "\n=== Searching for ForeignKey on team_id ==="
rg -n "team_id.*ForeignKey\|ForeignKey.*team_id" --type python -B 2 -A 2
echo -e "\n=== General search for ForeignKey usage ==="
rg -n "ForeignKey" --type python -B 1 -A 1 | head -50Repository: wecode-ai/Wegent
Length of output: 146
🏁 Script executed:
#!/bin/bash
# Read the file under review
echo "=== Reading backend/app/services/team_favorite.py ==="
cat -n backend/app/services/team_favorite.py
echo -e "\n=== List files in backend/app ==="
ls -la backend/app/
echo -e "\n=== Look for database model files ==="
find backend/app -name "*.py" -path "*/models/*" -o -name "*model*.py" | head -10Repository: wecode-ai/Wegent
Length of output: 4557
🏁 Script executed:
#!/bin/bash
# Read the UserTeamFavorite model
echo "=== Reading backend/app/models/user_team_favorite.py ==="
cat -n backend/app/models/user_team_favorite.py
echo -e "\n=== Look for migration or schema files ==="
find backend -name "*migration*" -o -name "*alembic*" -o -name "*schema*" | head -20
echo -e "\n=== Check db directory ==="
ls -la backend/app/db/Repository: wecode-ai/Wegent
Length of output: 1531
🏁 Script executed:
#!/bin/bash
# Read the migration file
echo "=== Reading Alembic migration file ==="
cat -n backend/alembic/versions/0c086b93f8b9_initial_migration.pyRepository: wecode-ai/Wegent
Length of output: 6472
🏁 Script executed:
#!/bin/bash
# List all migration files
echo "=== All migration files ==="
ls -la backend/alembic/versions/
echo -e "\n=== Search for user_team_favorites in all migration files ==="
grep -r "user_team_favorites" backend/alembic/
echo -e "\n=== Search for user_team_favorites in all Python files ==="
grep -r "user_team_favorites" backend/app/ --include="*.py" | grep -i "create\|table\|migration"Repository: wecode-ai/Wegent
Length of output: 2408
🏁 Script executed:
#!/bin/bash
# Read the migration file for user_team_favorites
cat -n backend/alembic/versions/2a3b4c5d6e7f_add_user_team_favorites_table.pyRepository: wecode-ai/Wegent
Length of output: 2060
No foreign key constraint exists on user_team_favorites.team_id—the review concern is valid.
The migration file (backend/alembic/versions/2a3b4c5d6e7f_add_user_team_favorites_table.py) creates the table with only indexes and a unique constraint on (user_id, team_id), but no ForeignKey constraint on team_id. This confirms the race condition scenario: a team could be deleted after the existence check at lines 22–26 but before the commit at line 46, resulting in an orphaned favorite record pointing to a non-existent team.
Adding a foreign key constraint to enforce referential integrity would prevent this edge case.
🤖 Prompt for AI Agents
In backend/app/services/team_favorite.py lines 40–48 and related migration, add
a DB-level foreign key on user_team_favorites.team_id referencing teams.id to
enforce referential integrity and prevent orphaned favorites: update the Alembic
migration (or create a new revision) to ALTER TABLE user_team_favorites ADD
CONSTRAINT fk_user_team_favorites_team_id FOREIGN KEY (team_id) REFERENCES
teams(id) (optionally WITH ON DELETE CASCADE); also update the SQLAlchemy model
to declare team_id = Column(Integer, ForeignKey("teams.id")) and, to handle
existing deployments, add a new migration that either backfills/removes orphan
rows before adding the constraint or uses the appropriate ALTER ... NOT VALID /
VALIDATE pattern; finally, in the service wrap the db.add/db.commit in a
try/except catching IntegrityError to rollback and return a clear error if the
referenced team was concurrently deleted.
Summary
Test plan
Summary by CodeRabbit
New Features
UI
Localization
✏️ Tip: You can customize this high-level summary in your review settings.