-
Notifications
You must be signed in to change notification settings - Fork 29
feat(teams): add team icon, recommended status and favorites features #204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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 21 minutes and 25 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 (2)
WalkthroughThis pull request introduces a team favorites feature allowing users to mark teams as favorites and browse recommended teams. It includes a database migration to store user-team favorite relationships, backend API endpoints for favorite operations, frontend components for displaying and managing favorites, UI enhancements for team icons and recommendation status, and localization strings for the new feature. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Summary
Test plan
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.