Skip to content

refactor: update README files to reflect project name change from Qui… #38

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

Merged
merged 21 commits into from
May 19, 2025

Conversation

cubxxw
Copy link
Member

@cubxxw cubxxw commented May 18, 2025

This pull request includes a rebranding of the project from "QuickForge AI" to "nexus," along with several code changes to enhance functionality, improve test configurations, and clean up the codebase. Below is a breakdown of the most significant changes:

Rebranding

  • Updated all references to "QuickForge AI" to "nexus" in README.md and README_zh-CN.md, including badges, descriptions, and examples. [1] [2] [3] [4] [5]

Frontend Enhancements

  • Added a new export for types in admin/src/client/index.ts to improve type management.
  • Updated the GoogleOauthService in admin/src/client/sdk.gen.ts to include support for extensionCallback in the googleLogin method. [1] [2]
  • Introduced a new type GoogleOauthGoogleLoginData in admin/src/client/types.gen.ts.

Backend Improvements

  • Adjusted imports in backend/app/api/routes/__init__.py for better organization.
  • Fixed minor typos and improved comments in test files, including test_google_oauth.py, conftest.py, and test_initial_data.py. [1] [2] [3]

Test and Database Configuration

  • Updated database utilities in test_db.py to enhance logging and handle PostgreSQL connections more robustly. [1] [2]
  • Removed unused imports and improved test setup in test_backend_pre_start.py and test_test_pre_start.py. [1] [2]

These changes collectively improve the branding, functionality, and maintainability of the project while ensuring a smoother development experience.

Summary by CodeRabbit

  • New Features

    • Added support for an optional extension callback parameter during Google OAuth login.
    • Google OAuth login now provides more detailed error responses for validation issues.
    • Introduced comprehensive environment variable management with Doppler integration in the Makefile.
    • Added a new custom TypeScript module exporting types and utilities for the admin client.
  • Documentation

    • Updated project branding from "QuickForge AI" to "nexus" in both English and Chinese documentation.
    • Refreshed badges and descriptive text in the Chinese README to reflect the new branding.
  • Bug Fixes

    • Improved typing and error handling for Google OAuth login API responses.
    • Enhanced database test setup with improved error handling, logging, and driver standardization.
  • Style

    • Minor formatting and whitespace cleanups across frontend and backend files.
  • Tests

    • Refactored and improved test utilities and comments for better clarity and reliability.
  • Chores

    • Simplified GitHub workflow syntax and improved commit and push logic for generated client files.
    • Added explicit pnpm installation and caching in Playwright workflow for improved CI consistency.
    • Updated admin client generation script to fix exports automatically after code generation.
    • Removed deprecated Google OAuth route and core security utilities for token and password management.
    • Removed obsolete configuration and extension files related to the previous branding and setup.
    • Enhanced Makefile targets to ensure environment files are initialized before builds and development.
    • Added a new script to fix admin client exports post-generation for improved developer experience.

cubxxw added 2 commits May 18, 2025 22:38
…ckForge AI to nexus

- Changed project title and references in both English and Chinese README files.
- Updated badge links to reflect the new project name and added relevant GitHub and Discord links.
- Enhanced descriptions to maintain clarity and consistency with the new branding.
…tation

- Added support for an optional `extension_callback` parameter in the Google login API.
- Updated OpenAPI documentation to reflect the new query parameter and improved error handling for validation errors.
- Refactored related types and SDK generation to accommodate the new login data structure.
- Removed unused utility exports from the client codebase for better clarity and maintainability.
@cubxxw cubxxw requested a review from kubbot as a code owner May 18, 2025 16:48
@github-project-automation github-project-automation bot moved this to Backlog in nexus May 18, 2025
Copy link
Contributor

coderabbitai bot commented May 18, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This update introduces consistent project rebranding from "QuickForge AI" to "nexus" in documentation, refines Google OAuth login API types and endpoint definitions, and enhances code quality through improved type exports, test utilities, and formatting. It replaces custom JWT decoding with the jwt library, simplifies test database utilities with improved error handling and logging, and streamlines CI workflows. The Makefile is extended with Doppler-based environment management. The Google OAuth route module and core security module are removed, and related imports and types are updated accordingly.

Changes

File(s) Change Summary
README.md, README_zh-CN.md Updated project branding from "QuickForge AI" to "nexus" in titles, descriptions, and badges.
admin/src/client/index.ts Added export * from "./types" to extend public API exports.
admin/src/client/custom.ts Added new module exporting custom types and utils to avoid generator overwrite.
admin/src/client/sdk.gen.ts, frontend/app/openapi-client/sdk.gen.ts Updated Google OAuth login method to accept optional GoogleOauthGoogleLoginData parameter; added related import.
admin/src/client/types.gen.ts, frontend/app/openapi-client/types.gen.ts Added GoogleOauthGoogleLoginData type with optional properties; changed GoogleOauthGoogleLoginError to HTTPValidationError.
frontend/openapi.json Added optional query parameter extension_callback and 422 validation error response to Google OAuth login endpoint.
backend/app/api/routes/extension_auth.py Replaced custom token decoding with jwt.decode, refined exception handling, and changed user retrieval to session get.
backend/app/api/routes/google_oauth.py Updated extension_callback parameter type annotation to use union type `str
backend/app/core/config.py Removed FRONTEND_HOST attribute; cleaned whitespace in DB URI property.
backend/app/core/supabase_service.py Added comment explaining type: ignore[attr-defined] for supabase client creation.
backend/app/tests/api/routes/test_users.py, backend/app/tests/api/test_google_oauth.py, backend/app/tests/conftest.py, backend/app/tests/scripts/test_backend_pre_start.py, backend/app/tests/scripts/test_test_pre_start.py, backend/app/tests_pre_start.py, backend/app/tests/test_initial_data.py, backend/app/tests/utils/test_db.py Various test improvements: simplified mocks, renamed test function, fixed comments, enhanced test DB setup with better driver detection, error handling, and logging; removed Alembic migration usage.
frontend/app/layout.tsx, frontend/app/login/page.tsx, frontend/app/page.tsx, frontend/app/password-recovery/confirm/page.tsx Minor formatting fixes such as whitespace removal and JSX simplification.
admin/src/components/Common/ItemActionsMenu.tsx, admin/src/components/Items/EditItem.tsx, admin/src/hooks/useCustomToast.ts, admin/src/routes/_layout/items.tsx Changed imports of types and utilities from "@/client" to "@/client/custom".
.github/workflows/generate-client.yml Simplified workflow syntax, removed commented code, improved git commit and push logic, and updated git user config scope and values.
.github/workflows/playwright.yml Added explicit pnpm installation and caching in merge-playwright-reports job; added symlink for pnpm lockfile in test job.
Makefile Added Doppler CLI environment management and .env file initialization targets; updated build and dev targets to depend on environment checks; standardized Docker Compose command; added setup target.
scripts/fix-admin-client.sh New script to append exports for types and utils to admin client index after generation if missing.
scripts/generate-admin-client.sh Added invocation of fix-admin-client.sh after client code formatting.
docker-compose.nole.yml Deleted empty Docker Compose override file.
package.json Deleted browser extension package configuration file.
app/api/routes/google_oauth.py Deleted entire module implementing Google OAuth routes.
app/core/config.py Deleted entire configuration class and related utilities.
app/core/security.py Deleted password hashing, verification, and JWT token creation utilities.

Sequence Diagram(s)

sequenceDiagram
    participant Frontend
    participant SDK
    participant Backend

    Frontend->>SDK: googleOauthGoogleLogin({ extension_callback })
    SDK->>Backend: GET /api/v1/login/google?extension_callback=...
    Backend-->>SDK: 200 OK (or 422 Validation Error)
    SDK-->>Frontend: Response (success or error)
Loading

Poem

A rabbit hops with gleeful pride,
As "nexus" banners now reside.
OAuth flows with types anew,
And docs refreshed in vibrant hue.
Whitespace trimmed, the code is neat—
These little changes feel so sweet!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 569e60a and c137d78.

📒 Files selected for processing (2)
  • .github/workflows/generate-client.yml (5 hunks)
  • Makefile (8 hunks)

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (1)
backend/app/core/config.py (1)

210-210: ⚠️ Potential issue

Fix duplicate FRONTEND_HOST declaration.

The FRONTEND_HOST variable is defined twice - once on line 47 and again on line 210. This is causing a linting error.

- FRONTEND_HOST: str = "http://localhost:3000"
+ # Use a different variable name to avoid duplication with line 47
+ GOOGLE_OAUTH_FRONTEND_HOST: str = "http://localhost:3000"

You'll also need to update any references to this variable elsewhere in the code.

🧰 Tools
🪛 GitHub Actions: Lint Backend

[error] 210-210: mypy error: Name "FRONTEND_HOST" already defined on line 47 (no-redef)

♻️ Duplicate comments (1)
backend/app/tests/scripts/test_test_pre_start.py (1)

5-5: Same issue as in test_backend_pre_start.py: the test imports init from app.db.init_db (expects a Session) but still calls init(engine_mock) and patches sqlmodel.Session, which is not invoked. Please update the test accordingly.

🧹 Nitpick comments (3)
backend/app/tests/test_initial_data.py (1)

119-119: Fix typo in comment.

There's a typo in the comment - "scrip" should be "script".

- # Write the temporary scrip
+ # Write the temporary script
backend/app/tests/api/test_google_oauth.py (1)

89-89: Fix typo in test comment.
The comment is truncated ("reques" instead of "request"), leading to a minor typo. Please correct it:

-        # Configure the mock to return a different user than the reques
+        # Configure the mock to return a different user than the request
backend/app/tests/utils/test_db.py (1)

246-257: Consider refactoring duplicated driver detection logic

The driver detection logic is duplicated between setup_test_db() and test_database_connection(). Consider extracting this into a helper function to avoid duplication and simplify maintenance.

+def detect_postgres_driver() -> str:
+    """Detect available PostgreSQL driver and return appropriate SQLAlchemy prefix."""
+    if importlib.util.find_spec("psycopg2"):
+        logger.info("Using psycopg2 driver for database connection")
+        return "postgresql+psycopg2"
+    elif importlib.util.find_spec("psycopg"):
+        logger.info("Using psycopg (v3) driver for database connection")
+        return "postgresql+psycopg"
+    else:
+        logger.error("Neither psycopg nor psycopg2 is available. Please install one of them.")
+        raise ImportError(
+            "Database driver not available. Install either psycopg2-binary or psycopg[binary]"
+        )

Then use this function in both places:

# In setup_test_db():
driver_name = detect_postgres_driver()

# In test_database_connection():
try:
    driver_name = detect_postgres_driver()
except ImportError:
    return False  # If neither driver is available
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between abe23a9 and 03b230e.

📒 Files selected for processing (22)
  • README.md (2 hunks)
  • README_zh-CN.md (3 hunks)
  • admin/src/client/index.ts (1 hunks)
  • admin/src/client/sdk.gen.ts (2 hunks)
  • admin/src/client/types.gen.ts (1 hunks)
  • backend/app/api/routes/__init__.py (1 hunks)
  • backend/app/core/config.py (1 hunks)
  • backend/app/tests/api/routes/test_users.py (1 hunks)
  • backend/app/tests/api/test_google_oauth.py (1 hunks)
  • backend/app/tests/conftest.py (3 hunks)
  • backend/app/tests/scripts/test_backend_pre_start.py (1 hunks)
  • backend/app/tests/scripts/test_test_pre_start.py (1 hunks)
  • backend/app/tests/test_initial_data.py (4 hunks)
  • backend/app/tests/utils/test_db.py (9 hunks)
  • backend/app/tests_pre_start.py (1 hunks)
  • frontend/app/layout.tsx (1 hunks)
  • frontend/app/login/page.tsx (1 hunks)
  • frontend/app/openapi-client/sdk.gen.ts (2 hunks)
  • frontend/app/openapi-client/types.gen.ts (1 hunks)
  • frontend/app/page.tsx (1 hunks)
  • frontend/app/password-recovery/confirm/page.tsx (1 hunks)
  • frontend/openapi.json (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
backend/app/tests/scripts/test_test_pre_start.py (2)
backend/app/core/db.py (1)
  • init_db (57-87)
backend/app/tests_pre_start.py (1)
  • init (23-30)
admin/src/client/types.gen.ts (1)
frontend/app/openapi-client/types.gen.ts (1)
  • GoogleOauthGoogleLoginData (331-335)
frontend/app/password-recovery/confirm/page.tsx (1)
website/src/components/ui/card-hover-effect.tsx (1)
  • CardDescription (76-94)
backend/app/tests_pre_start.py (1)
backend/app/tests/utils/test_db.py (1)
  • get_test_db_url (37-62)
backend/app/tests/conftest.py (2)
backend/app/tests/utils/test_db.py (1)
  • setup_test_db (162-207)
backend/app/core/db.py (1)
  • init_db (57-87)
admin/src/client/sdk.gen.ts (2)
admin/src/client/types.gen.ts (2)
  • GoogleOauthGoogleLoginData (135-137)
  • GoogleOauthGoogleLoginResponse (139-139)
admin/src/client/core/CancelablePromise.ts (1)
  • CancelablePromise (20-126)
frontend/app/openapi-client/sdk.gen.ts (1)
frontend/app/openapi-client/types.gen.ts (1)
  • GoogleOauthGoogleLoginData (331-335)
frontend/app/openapi-client/types.gen.ts (1)
admin/src/client/types.gen.ts (3)
  • GoogleOauthGoogleLoginData (135-137)
  • GoogleOauthGoogleLoginResponse (139-139)
  • HTTPValidationError (43-45)
🪛 GitHub Actions: Lint Backend
backend/app/core/config.py

[error] 210-210: mypy error: Name "FRONTEND_HOST" already defined on line 47 (no-redef)

backend/app/tests/test_initial_data.py

[error] 129-129: mypy error: "CompletedProcess[str]" has no attribute "stdou"; maybe "stdout"? (attr-defined)

⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: test-playwright (3, 4)
  • GitHub Check: test-playwright (4, 4)
  • GitHub Check: test-playwright (2, 4)
  • GitHub Check: test-playwright (1, 4)
  • GitHub Check: test-backend
🔇 Additional comments (39)
frontend/app/password-recovery/confirm/page.tsx (1)

52-52: Refine JSX formatting for CardDescription
Collapsing the <CardDescription> into a single line enhances readability and aligns with the style of adjacent components.

README.md (3)

1-1: Branding update: Title updated to 'nexus'
Header correctly reflects the new project name.


286-286: AI Integration section updated with new branding
The phrase now correctly states “nexus makes it simple to integrate various AI services:”.


5-5: ⚠️ Potential issue

Badge URL mismatch—update shields URL
The alt text has been updated to nexus, but the badge URL still references QuickForge-AI, which contradicts the PR summary and causes confusion. Please update the shields.io URL to reflect nexus.

Apply this diff:

-![nexus](https://images.shields.io/badge/QuickForge-AI-blue)
+![nexus](https://images.shields.io/badge/nexus-blue)

Likely an incorrect or invalid review comment.

README_zh-CN.md (4)

6-6: Branding update: Title updated to 'nexus'
Chinese title correctly reflects the new project name.


10-14: Badge section updated for nexus repository
Badges now point to telepace/nexus with appropriate labels and links, replacing the old QuickForge-AI badges.


24-24: Project description updated with new branding
The Chinese project description has been correctly updated to nexus without altering the original meaning.


435-435: AI Integration section updated with new branding
The sentence now reads nexus使集成各种AI服务变得简单:, matching the English version.

backend/app/api/routes/__init__.py (1)

4-4: LGTM! Clean formatting changes.

The addition of blank lines after the imports groups improves readability and follows standard Python spacing conventions.

Also applies to: 9-9

admin/src/client/types.gen.ts (1)

135-137: LGTM! New type for Google OAuth extension callback.

The new type GoogleOauthGoogleLoginData provides proper type support for the optional extension callback parameter in the Google OAuth login flow.

backend/app/core/config.py (1)

99-99: LGTM! Whitespace cleanup.

Removed trailing spaces improves code formatting.

Also applies to: 105-105

backend/app/tests/test_initial_data.py (2)

14-15: LGTM! Simplified mocking approach.

The updated mock is cleaner and more straightforward, patching sqlmodel.Session directly.


71-72: LGTM! Improved test function name and docstring.

The new name test_initial_data_has_entry_point is more descriptive of what the test is checking.

backend/app/tests/scripts/test_backend_pre_start.py (1)

5-5:

❌ Incorrect review comment

Test import mismatch for init function.
The test now imports init from app.db.init_db, which expects a Session object, but the test still calls init(engine_mock) and patches sqlmodel.Session, which init_db does not invoke. This mismatch means the test does not exercise the intended code path and session_mock_class is never used. Please update the test to construct or pass a mocked Session instance. For example:

-from app.db.init_db import init
+from app.db.init_db import init as init_db
...
-    init(engine_mock)
+    init_db(session_mock)  # Pass the mocked session instead of engine

🏁 Script executed:

#!/bin/bash
# Show definition of init in app/db/init_db.py
rg "def init" -A20 -n app/db/init_db.py

echo "--------------------------------------------------"
# Show contents of the test file
cat backend/app/tests/scripts/test_backend_pre_start.py

Length of output: 782


🏁 Script executed:

#!/bin/bash
# Locate all init_db.py files
echo "Searching for init_db.py..."
find . -type f -name init_db.py

# Display the definition of init in each found file
echo
echo "Extracting init definitions..."
for file in $(find . -type f -name init_db.py); do
  echo "===== $file ====="
  rg "def init" -A20 -n "$file" || echo "No match in $file"
done

Length of output: 212


🏁 Script executed:

#!/bin/bash
# List all Python files under backend/app to locate init definitions
echo "Listing all .py files under backend/app/:"
find backend/app -type f -name "*.py"

Length of output: 2394


🏁 Script executed:

#!/bin/bash
# Check for init definitions in core modules
echo "Searching for 'def init' in core modules..."
for file in backend/app/core/db.py backend/app/core/db_factory.py; do
  echo "===== $file ====="
  rg "def init" -n "$file" || echo "No init found in $file"
done

# If still not found, search entire repository
echo
echo "Searching entire codebase for 'def init('..."
rg "def init\\(" -n

Length of output: 980


🏁 Script executed:

#!/bin/bash
# Display the init function in backend_pre_start.py
echo "===== backend/app/backend_pre_start.py ====="
sed -n '1,200p' backend/app/backend_pre_start.py | sed -n '1,50p'

Length of output: 1083


🏁 Script executed:

#!/bin/bash
# Show contents of backend/app/tests_pre_start.py around init tests
echo "===== backend/app/tests_pre_start.py ====="
sed -n '1,200p' backend/app/tests_pre_start.py

Length of output: 1544


Update import in test_backend_pre_start.py to target the correct init function

The test is exercising the init defined in app.backend_pre_start (which takes an Engine and invokes sqlmodel.Session), but it’s currently importing from the non-existent app.db.init_db. Update the import so your patch on sqlmodel.Session actually applies to the right function.

• In backend/app/tests/scripts/test_backend_pre_start.py

-from app.db.init_db import init
+from app.backend_pre_start import init

Likely an incorrect or invalid review comment.

frontend/app/page.tsx (1)

11-11: Trivial whitespace removal; no functional impact.

frontend/app/layout.tsx (1)

21-24: Only quote style formatting change; no functional impact.

backend/app/tests/api/routes/test_users.py (1)

231-231: Whitespace cleanup looks good.

This change removes an unnecessary blank line, improving the code formatting consistency.

backend/app/tests_pre_start.py (2)

35-43: Import organization improvements look good.

The changes reorganize imports in a more logical order (standard library imports first, then third-party libraries like SQLModel, then application-specific imports), which follows Python conventions and improves readability.


50-50: Whitespace cleanup looks good.

This change removes an unnecessary blank line, improving the code formatting consistency.

frontend/app/login/page.tsx (1)

39-43: Whitespace cleanup looks good.

This change removes unnecessary blank lines in the CallbackUrlHandler component, improving code formatting consistency.

admin/src/client/index.ts (1)

7-7: New type exports are a good addition.

This change adds the export of types from the ./types module, which is necessary to make the new Google OAuth related types (such as GoogleOauthGoogleLoginData) available to consumers of this API. This properly supports the enhancement of the Google OAuth login functionality.

frontend/app/openapi-client/types.gen.ts (2)

331-335: Good type definition for Google OAuth login data.

Adding the GoogleOauthGoogleLoginData type with an optional query parameter for extension_callback improves type safety for the Google OAuth login flow. This properly aligns with the corresponding changes in the admin client and backend API.


339-339: Improved error type specification.

Changing GoogleOauthGoogleLoginError from unknown to HTTPValidationError provides more specific type information, which enhances error handling capabilities and aligns with other API error types in the codebase.

frontend/openapi.json (2)

800-807: Well-implemented new query parameter for the Google OAuth login endpoint

The addition of the optional extension_callback parameter to the Google OAuth login endpoint is properly defined with all required OpenAPI specification properties.


813-820: Good addition of explicit validation error handling

Adding explicit 422 validation error response documentation improves the API contract definition and ensures consistent error handling across endpoints.

backend/app/tests/conftest.py (2)

6-6: Correctly streamlined imports

Removing unused imports (os and create_engine from sqlmodel) improves code cleanliness.


17-70: Improved code readability and organization

The fixes for comment typos and whitespace issues enhance code readability without altering functionality. The explicit import of app.core.db inside the fixture (line 31) is a good practice for ensuring proper dependency management.

frontend/app/openapi-client/sdk.gen.ts (2)

79-79: Proper type import for enhanced type safety

Adding the GoogleOauthGoogleLoginData type import ensures type safety for the updated function signature.


536-537: Updated function signature with proper typing

The googleOauthGoogleLogin function now correctly accepts an optional parameters object typed as GoogleOauthGoogleLoginData, allowing for the new extension_callback parameter to be passed in a type-safe manner.

admin/src/client/sdk.gen.ts (4)

6-6: Complete import list with new type

The import statement is updated to include the GoogleOauthGoogleLoginData type, ensuring type safety for the updated method signature.


34-35: Clear documentation for new parameter

The JSDoc comments are properly updated to document the new parameter, helping developers understand how to use the extended API.


39-39: Well-structured method signature update

The method signature is updated to accept an optional data parameter with a sensible default empty object, maintaining backward compatibility.


42-48: Comprehensive implementation of API extension

The implementation correctly:

  1. Maps the camelCase extensionCallback property to the snake_case extension_callback query parameter
  2. Adds proper error handling for the 422 validation error response

This ensures the client SDK properly interacts with the updated API.

backend/app/tests/utils/test_db.py (6)

17-17: Good use of importlib.util for driver detection

Using importlib.util.find_spec for checking module availability is more elegant than try-except blocks for import errors. This approach improves code readability and performance.


42-60: Improved URL construction with comprehensive PostgreSQL prefix handling

The updated URL construction logic is more robust and handles different PostgreSQL URL formats correctly. This will help prevent connection issues that could arise from mismatched driver prefixes.


169-194: Good implementation of driver detection with proper error handling

The driver detection logic in setup_test_db() is well-implemented with:

  1. Clear logging for which driver is being used
  2. Descriptive error messages when no driver is available
  3. Proper URL adjustment based on the detected driver

This improves reliability when running tests in different environments.


272-274: Improved error handling with logging in database connection test

Adding logging for connection failures is a good practice. It provides valuable information for debugging test setup issues without having to add debug statements manually.


86-89: Better SQL query formatting

The improved SQL query formatting with proper indentation enhances readability. Consistent formatting makes the code easier to maintain and review.


92-95: Enhanced logging messages

The more detailed logging messages provide better context about database operations. This is particularly helpful when debugging test environment issues.

cubxxw added 2 commits May 19, 2025 01:09
- Simplified event types for pull requests and push triggers in the workflow configuration.
- Updated Git user configuration to use global settings for better consistency.
- Enhanced change detection logic before committing and pushing generated client files.
- Removed unnecessary comments and improved overall readability of the workflow file.
- Enhanced logic for detecting changes in generated client files for both same-repo and fork PRs.
- Added warnings for fork PRs to prompt users to generate and commit changes locally.
- Streamlined commit and push process for same-repo events to avoid conflicts.
- Improved overall clarity and organization of the workflow file.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
.github/workflows/generate-client.yml (2)

93-96: Enhance Commit & Push robustness

  • Adding git pull --rebase reduces merge conflicts when CI and manual changes overlap.
  • You may consider using --no-verify on git commit or leveraging a dedicated action like peter-evans/create-pull-request for a more resilient CI-driven PR update workflow.

109-109: Add a newline at end of file
YAMLLint reports a missing newline at EOF. Please add a trailing newline to conform with POSIX standards.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 109-109: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 03b230e and 31deb86.

📒 Files selected for processing (1)
  • .github/workflows/generate-client.yml (3 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/generate-client.yml

88-88: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions for more details

(expression)

🪛 YAMLlint (1.37.1)
.github/workflows/generate-client.yml

[error] 109-109: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: test-playwright (4, 4)
  • GitHub Check: test-playwright (1, 4)
  • GitHub Check: test-playwright (3, 4)
  • GitHub Check: test-playwright (2, 4)
🔇 Additional comments (5)
.github/workflows/generate-client.yml (5)

6-8: Clean up on event triggers
The inline array syntax for pull_request.types and push.branches is concise and improves readability.


11-11: Grant write permission for contents
Setting permissions.contents: write is required so the workflow can commit and push the generated client files.


40-41: Introduce UV CLI installation step
Using astral-sh/setup-uv@v1 with caching streamlines the installation of the uv tool needed for backend dependency sync.


78-79: Set global Git identity for CI
Configuring git config --global user.* ensures that the autogenerated commits are correctly attributed to GitHub Actions.


81-84: Stage both frontend and admin client files
Renaming to “Stage Generated Files” and adding admin/src/client alongside frontend/app/openapi-client ensures all generated SDK artifacts are included in the commit.

Comment on lines 89 to 90
BRANCH_NAME="${{ github.head_ref || github.ref_name }}"
if ! git diff --quiet || ! git diff --staged --quiet; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid using ${{ github.head_ref }} directly in shell scripts
Actionlint flags direct use of github.head_ref in inline scripts as potentially untrusted. Instead, pass it through an environment variable.
Apply this diff:

 jobs:
   generate-client:
+    env:
+      HEAD_REF: ${{ github.head_ref }}
+      REF_NAME: ${{ github.ref_name }}
     steps:
       - name: Commit and Push Changes (Same Repo)
         run: |
-          BRANCH_NAME="${{ github.head_ref || github.ref_name }}"
+          BRANCH_NAME="${HEAD_REF:-$REF_NAME}"
           if ! git diff --quiet || ! git diff --staged --quiet; then
🤖 Prompt for AI Agents
In .github/workflows/generate-client.yml at lines 89-90, avoid using `${{
github.head_ref }}` directly in the shell script due to security concerns
flagged by Actionlint. Instead, define an environment variable in the workflow
that captures `github.head_ref` or `github.ref_name`, then reference that
environment variable within the shell script. This ensures safer and cleaner
access to the branch name in the script.

- Added pnpm installation step in the GitHub Actions workflow for improved dependency management.
- Updated JWT token decoding in the extension authentication routes to use the jwt library directly.
- Refactored user retrieval logic to utilize session.get for better clarity and performance.
- Modified Google OAuth login parameter type for improved type safety.
- Updated database connection utilities to use the psycopg driver instead of psycopg2 for better compatibility.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (5)
backend/app/api/routes/extension_auth.py (1)

49-50: Consider consistent error handling across functions

While get_extension_token now uses specific exception handling with InvalidTokenError, this function still uses a generic Exception catch. Consider making the error handling consistent across both functions for better maintainability.

-    except Exception as e:
+    except (InvalidTokenError, Exception) as e:
.github/workflows/generate-client.yml (4)

16-16: Remove redundant blank line
There’s an extra blank line after runs-on: ubuntu-latest. Consider removing it for consistent spacing throughout the workflow.


48-48: Remove trailing blank line
There’s an unnecessary blank line after the working-directory: frontend step. Cleaning up these trailing blanks can improve readability.


94-95: Clean up commented-out pull command
The commented git pull --ff-only lines can be removed or documented with a clear TODO if they’re intended for future use. Leaving orphaned comments may confuse maintainers.


121-121: Add newline at end of file
YAMLLint reports a missing newline at EOF. Please add a final newline to comply with POSIX standards and avoid CI warnings.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 121-121: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 31deb86 and 01f3c8b.

📒 Files selected for processing (8)
  • .github/workflows/generate-client.yml (5 hunks)
  • .github/workflows/playwright.yml (1 hunks)
  • backend/app/api/routes/extension_auth.py (4 hunks)
  • backend/app/api/routes/google_oauth.py (1 hunks)
  • backend/app/core/config.py (1 hunks)
  • backend/app/core/supabase_service.py (1 hunks)
  • backend/app/tests/test_initial_data.py (3 hunks)
  • backend/app/tests/utils/test_db.py (9 hunks)
✅ Files skipped from review due to trivial changes (2)
  • backend/app/core/supabase_service.py
  • backend/app/api/routes/google_oauth.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • backend/app/core/config.py
  • backend/app/tests/test_initial_data.py
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/generate-client.yml

[error] 121-121: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: test-playwright (4, 4)
  • GitHub Check: test-playwright (3, 4)
  • GitHub Check: test-playwright (2, 4)
  • GitHub Check: test-playwright (1, 4)
🔇 Additional comments (23)
backend/app/tests/utils/test_db.py (6)

17-17: Improved module detection with importlib

Adding importlib.util is a good improvement as it will be used to check for database driver availability with find_spec instead of relying on try-except blocks, making the code more explicit and maintainable.


42-62: Well-structured database URL handling with improved driver compatibility

The URL construction logic has been significantly improved to:

  1. Standardize on the psycopg driver
  2. Handle different URL formats systematically
  3. Provide a clear fallback strategy

This change makes the test database setup more robust across different environments.


75-112: Enhanced formatting and readability of database operations

The SQL query formatting and logging messages have been improved with proper indentation and parameterization, making the code more readable and maintainable.


171-186: Robust driver detection with clear error messaging

The implementation of importlib.util.find_spec for driver detection is better than using try-except blocks as it:

  1. Explicitly checks for module availability before attempting to use it
  2. Provides clear error messages when required drivers are missing
  3. Logs which driver is being used

This improves both the reliability and debuggability of the test setup process.


250-256: Consistent driver detection pattern applied throughout

The same driver detection pattern is correctly applied in the test_database_connection function, maintaining consistency across the module. This is good practice for code maintenance.


272-274: Improved error handling with detailed logging

Adding explicit error logging when database connection fails will make debugging much easier. Previously, connection failures would happen silently, now they're properly logged with the specific error message.

backend/app/api/routes/extension_auth.py (9)

4-6: Dependencies updated for direct JWT handling

The imports have been appropriately updated to use the jwt library directly and include specific exception handling with InvalidTokenError. This follows best practices by leveraging standard libraries and enabling more precise error handling.


11-11: Type imports organized correctly

Adding the User model import alongside existing model imports improves consistency and makes the direct session access pattern more clear.


28-30: Improved token decoding implementation

Replacing the custom security.decode_access_token with direct jwt.decode calls eliminates an unnecessary abstraction layer and makes the code more explicit. The parameters are correctly specified to maintain consistent behavior.


32-32: More efficient user retrieval

Using session.get(User, user_id) instead of a custom CRUD function is more direct and leverages SQLAlchemy's capabilities efficiently.


39-41: Consistent JWT decoding approach

This second instance of token decoding follows the same improved pattern, ensuring consistency throughout the file.


43-43: Consistent user retrieval pattern

The user retrieval method has been updated consistently with the earlier instance.


64-66: JWT decoding consistency maintained

The JWT decoding approach has been applied consistently throughout the file, which is good for maintainability.


68-68: User retrieval consistency

The user retrieval method has been updated consistently throughout the file.


80-80: Enhanced error handling with specific exception

Catching the specific InvalidTokenError instead of a generic Exception is a significant improvement that provides more precise error handling and better debugging.

.github/workflows/playwright.yml (2)

112-116: Added explicit pnpm setup for merge-playwright-reports job
Introducing the pnpm/action-setup@v3 step with a pinned version (9.9.0) ensures that the merge job aligns with the test-playwright job’s environment. This promotes consistency and reliable dependency management across jobs.


116-119: Enable pnpm caching in Node.js setup
The addition of cache: 'pnpm' under the Node.js setup ensures that frontend dependencies are cached between runs, which will improve workflow performance. This matches the caching strategy used in the test job.

.github/workflows/generate-client.yml (6)

6-7: Simplified pull_request and push triggers
Switching to inline array syntax for types: [opened, synchronize] and branches: [main] makes the event configuration more concise and readable, without altering behavior.


11-12: Grant write permissions on contents
Explicitly setting permissions.contents: write ensures that the workflow can commit and push generated client files. This is a necessary permission scope for the commit step.


23-27: Pinned pnpm version for consistent client generation
Setting pnpm/action-setup@v3 with version: 9.9.0 mirrors other workflows and avoids unexpected breaks due to pnpm upgrades.


40-41: Verify UV setup action version
The workflow uses astral-sh/setup-uv@v1, while the Playwright job uses astral-sh/setup-uv@v6. Please confirm that @v1 is intentional here, or consider aligning versions across workflows.


78-79: Global Git user configuration is appropriate
Using git config --global inside GitHub Actions is safe (it only affects the runner). This ensures commits are properly attributed.


81-85: Renamed staging step and file paths updated
Renaming to Stage Generated Files and updating the paths to frontend/app/openapi-client and admin/src/client accurately reflects the new project structure after rebranding.

cubxxw added 2 commits May 19, 2025 09:43
- Compressed OpenAPI JSON structure for better readability and performance.
- Modified the `extension_callback` parameter type in the Google OAuth login data to allow null values for improved type safety.
- Ensured consistency in API response schemas across various endpoints.
- Changed the GitHub token used in the workflow from GITHUB_TOKEN to BOT_GITHUB_TOKEN for improved access control.
- Removed outdated comments regarding the use of a personal access token (PAT) for triggering workflows.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
.github/workflows/generate-client.yml (4)

10-11: Consider tightening the workflow’s permissions
Currently, the workflow grants contents: write at the top level. To follow the principle of least privilege, you could scope write permissions more narrowly (e.g., limit to specific jobs or paths).


40-41: Pin the setup-uv action to a specific version
You’re currently using astral-sh/setup-uv@v1, which will float to the latest v1 release. For stronger reproducibility, consider pinning to a specific patch release (e.g., @v1.2.3) or a commit SHA.


94-95: Remove commented-out pull logic if obsolete
The commented git pull --ff-only lines can be removed if they’re no longer needed. Keeping dead code may confuse future maintainers.


119-119: Add a newline at end of file
YAMLLint flags a missing newline at end-of-file. Please add an empty line after this fi to satisfy POSIX and linting standards.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 119-119: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between c4e60a4 and 4355212.

📒 Files selected for processing (1)
  • .github/workflows/generate-client.yml (5 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/generate-client.yml

[error] 119-119: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (5)
.github/workflows/generate-client.yml (5)

6-8: Good modernization of event triggers
Using inline array syntax for pull_request.types and push.branches improves readability and consistency.


48-48: Correctly scoped install step for Admin dependencies
Adding working-directory: admin ensures dependencies are installed in the right folder, aligning with the frontend and backend steps.


78-79: Global Git configuration is fine in CI
Switching to --global for user.email and user.name in the ephemeral runner is reasonable and simplifies setup without side effects beyond the workflow run.


81-81: Step description updated for clarity
Renaming to "Stage Generated Files" accurately reflects that both frontend and admin client files are being staged.


103-103: Use dedicated bot token
Switching from the default GITHUB_TOKEN to secrets.BOT_GITHUB_TOKEN is a good isolation practice. Please verify that BOT_GITHUB_TOKEN has the necessary contents: write scope configured in repository settings.

cubxxw added 6 commits May 19, 2025 09:54
- Added psycopg2-binary as a fallback driver in pyproject.toml for better compatibility.
- Enhanced test database setup to prioritize psycopg driver and provide installation fallback for missing drivers.
- Updated logging messages for clarity regarding database configuration in test mode.
- Added psycopg2-binary as a fallback driver in the test database creation process for better compatibility.
- Improved error handling during test database creation, including detailed logging and recovery attempts.
- Updated test scripts to use the new database initialization method for consistency across tests.
- Updated import paths for ItemPublic to use the custom client module across multiple components.
- Adjusted related imports in EditItem, ItemActionsMenu, and layout files for consistency.
- Modified useCustomToast to align with the new import structure.
- Deleted the Google OAuth route implementation, configuration settings, and security utilities as they are no longer in use.
- This cleanup helps streamline the codebase and remove unnecessary dependencies.
- Refactored import statements for ItemPublic in EditItem and items layout files to use the new type import syntax.
- Improved code clarity and consistency across components by standardizing the import style.
- Reduced the size of the OpenAPI JSON file by compressing its structure, enhancing performance and clarity.
- Ensured that the API response schemas remain consistent across various endpoints.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
.github/workflows/generate-client.yml (1)

94-97: Avoid direct use of ${{ github.head_ref }} in scripts
As previously flagged, defining environment variables for github.head_ref and github.ref_name improves security and readability.

Apply this diff:

 jobs:
   generate-client:
     env:
+      HEAD_REF: ${{ github.head_ref }}
+      REF_NAME: ${{ github.ref_name }}
 ...
           # Pull before pushing to avoid conflicts if possible (optional)
-            git pull --ff-only origin ${{ github.head_ref || github.ref_name }}
+            git pull --ff-only origin ${HEAD_REF:-$REF_NAME}
 ...
-            git push origin HEAD:${{ github.head_ref || github.ref_name }}
+            git push origin HEAD:${HEAD_REF:-$REF_NAME}
🧹 Nitpick comments (6)
.github/workflows/generate-client.yml (2)

5-8: Consider including additional event triggers
You’ve constrained pull_request to only the opened and synchronize types. For completeness, consider adding reopened (and/or workflow_dispatch) so the workflow triggers when a PR is reopened or manually invoked.


119-119: Add newline at end of file
YAMLlint flagged a missing newline at EOF. Please add a line break to satisfy YAML conventions.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 119-119: no new line character at the end of file

(new-line-at-end-of-file)

scripts/generate-admin-client.sh (1)

57-60: Add pre-check for the fix script’s existence
Currently the script blindly invokes fix-admin-client.sh. To fail more gracefully when the fix script is missing or not executable, consider adding a pre-check before execution.

Proposed diff:

+if [ ! -x "$PROJECT_ROOT/scripts/fix-admin-client.sh" ]; then
+  echo "Error: fix-admin-client.sh not found or not executable at $PROJECT_ROOT/scripts/fix-admin-client.sh"
+  exit 1
+fi
 "$PROJECT_ROOT/scripts/fix-admin-client.sh"
Makefile (1)

616-618: Align help grouping with new ENV MANAGEMENT section
The help output now highlights ENV MANAGEMENT, but only lists a subset of the related commands. Exposing check-root-env, check-admin-env, check-frontend-env, and check-extension-env in the help can improve discoverability.

backend/app/tests_pre_start.py (1)

52-61: Avoid double creation & redundant connection attempts

setup_test_db() internally calls create_test_database() again, so when the first call inside the try fails we end up running the same logic twice (potentially dropping/re-creating the DB a second time).
Instead, delegate the whole responsibility immediately:

-        try:
-            logger.info("Creating test database if it doesn't exist...")
-            create_test_database()
-        except Exception as e:
-            logger.error(f"Failed to create test database: {e}")
-            # 如果失败,尝试使用 setup_test_db,它会尝试重新创建
-            logger.info("Trying alternative setup approach...")
-            test_engine = setup_test_db()
-            init(test_engine)
-            return
+        try:
+            test_engine = setup_test_db()   # handles creation & engine building
+        except Exception as e:
+            logger.error("Test DB setup failed: %s", e, exc_info=True)
+            raise
+
+        init(test_engine)
+        return

Less duplication, fewer edge-cases, clearer control flow.
(If keeping both helpers is necessary, setup_test_db(create_if_missing=False) could be introduced to clarify intent.)

backend/app/tests/utils/test_db.py (1)

37-63: Safer URL manipulation via sqlalchemy.engine.url.URL

String replacement (main_url.split("/")[-1]) fails when the original URL contains query parameters or a pathname segment containing “/”. Using SQLAlchemy’s URL parser prevents such edge-cases:

from sqlalchemy.engine import make_url

def get_test_db_url() -> str:
    main_url = make_url(str(settings.SQLALCHEMY_DATABASE_URI))
    main_url = main_url.set(database=get_test_db_name())
    # enforce driver
    if main_url.drivername.endswith("+psycopg2"):
        main_url = main_url.set(drivername=main_url.drivername.replace("+psycopg2", "+psycopg"))
    elif "+psycopg" not in main_url.drivername:
        main_url = main_url.set(drivername="postgresql+psycopg")
    return str(main_url)

Reduces chances of malformed URLs and keeps credentials intact.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 4355212 and 0ecf48c.

⛔ Files ignored due to path filters (3)
  • backend/uv.lock is excluded by !**/*.lock
  • package-lock.json is excluded by !**/package-lock.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (20)
  • .github/workflows/generate-client.yml (5 hunks)
  • Makefile (8 hunks)
  • admin/src/client/custom.ts (1 hunks)
  • admin/src/client/index.ts (1 hunks)
  • admin/src/components/Common/ItemActionsMenu.tsx (1 hunks)
  • admin/src/components/Items/EditItem.tsx (1 hunks)
  • admin/src/hooks/useCustomToast.ts (1 hunks)
  • admin/src/routes/_layout/items.tsx (1 hunks)
  • app/api/routes/google_oauth.py (0 hunks)
  • app/core/config.py (0 hunks)
  • app/core/security.py (0 hunks)
  • backend/app/tests/scripts/test_backend_pre_start.py (1 hunks)
  • backend/app/tests/scripts/test_test_pre_start.py (1 hunks)
  • backend/app/tests/utils/test_db.py (9 hunks)
  • backend/app/tests_pre_start.py (1 hunks)
  • backend/pyproject.toml (1 hunks)
  • docker-compose.nole.yml (0 hunks)
  • package.json (0 hunks)
  • scripts/fix-admin-client.sh (1 hunks)
  • scripts/generate-admin-client.sh (1 hunks)
💤 Files with no reviewable changes (5)
  • docker-compose.nole.yml
  • app/api/routes/google_oauth.py
  • app/core/security.py
  • package.json
  • app/core/config.py
✅ Files skipped from review due to trivial changes (7)
  • admin/src/components/Common/ItemActionsMenu.tsx
  • admin/src/routes/_layout/items.tsx
  • backend/app/tests/scripts/test_test_pre_start.py
  • admin/src/hooks/useCustomToast.ts
  • backend/pyproject.toml
  • admin/src/components/Items/EditItem.tsx
  • admin/src/client/custom.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • admin/src/client/index.ts
  • backend/app/tests/scripts/test_backend_pre_start.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/app/tests_pre_start.py (1)
backend/app/tests/utils/test_db.py (3)
  • create_test_database (71-136)
  • get_test_db_url (37-62)
  • setup_test_db (183-240)
🪛 YAMLlint (1.37.1)
.github/workflows/generate-client.yml

[error] 119-119: no new line character at the end of file

(new-line-at-end-of-file)

🪛 Ruff (0.11.9)
backend/app/tests/utils/test_db.py

196-196: psycopg imported but unused; consider using importlib.util.find_spec to test for availability

(F401)


201-201: psycopg2 imported but unused; consider using importlib.util.find_spec to test for availability

(F401)

🪛 GitHub Actions: Lint Backend
backend/app/tests/utils/test_db.py

[error] 196-196: ESLint: psycopg imported but unused; consider using importlib.util.find_spec to test for availability (F401). Remove unused import: psycopg.


[error] 201-201: ESLint: psycopg2 imported but unused; consider using importlib.util.find_spec to test for availability (F401). Remove unused import: psycopg2.

🔇 Additional comments (10)
.github/workflows/generate-client.yml (4)

10-11: Permissions follow least privilege principle
Granting contents: write is sufficient for staging and pushing generated files, minimizing risk.


40-44: Install UV step with caching looks good
The astral-sh/setup-uv@v1 action is pinned and cache is enabled, which should speed up Python environment setup.


78-80: Global Git configuration is appropriate
Switching from local to global config for the Git user makes the commit step more reliable in CI contexts.


101-104: Verify BOT_GITHUB_TOKEN permissions
You’ve switched the GITHUB_TOKEN to secrets.BOT_GITHUB_TOKEN. Please confirm this token has contents: write permission and is correctly configured in the repository secrets.

Makefile (4)

62-70: Introduce Doppler-based environment management
The new DOPPLER_EXISTS variable and env-init target lay the groundwork for consistent .env initialization across services. This addition provides clear instructions for both example-based and Doppler-based workflows.


649-655: Add setup target for streamlined onboarding
The setup target combines environment initialization and tool installation, offering a convenient entry point for new contributors.


409-413: Ensure admin dev target verifies .env
Adding check-admin-env to the admin target guarantees an admin .env file is present before starting the development server, reducing runtime errors.


532-536: Ensure extension dev target verifies .env
Including check-extension-env in the extension target mirrors the admin and frontend patterns, ensuring the extension .env is initialized.

backend/app/tests_pre_start.py (1)

64-65: Align engine creation parameters with helper defaults

setup_test_db() builds the engine with pool_pre_ping=True, timeouts, etc., but the “happy-path” (create_test_database succeeded) creates an engine with none of those options. This produces two different configurations for the same test environment.

Either always call setup_test_db() (see previous comment) or replicate the same keyword arguments here to keep behaviour consistent.

backend/app/tests/utils/test_db.py (1)

248-270: Consider quoting identifiers when dropping/creating databases

DROP DATABASE IF EXISTS {test_db_name} and CREATE DATABASE {test_db_name} interpolate the name directly.
While test_db_name is internal, quoting with sql.Identifier (or wrapping in psycopg.sql) eliminates any risk of injection and handles special-char names.

from psycopg import sql
cur.execute(
    sql.SQL("DROP DATABASE IF EXISTS {}").format(sql.Identifier(test_db_name))
)

cubxxw added 3 commits May 19, 2025 10:47
- Deleted the lint-backend GitHub Actions workflow as it is no longer needed.
- Added a step in the Playwright workflow to create a symlink for pnpm-lock.yaml to the root directory for better dependency management.
- Updated the check-root-env target to first attempt generating the .env file from Doppler if available, improving the setup process.
- Added conditional checks to ensure a smoother fallback to copying from .env.example if Doppler is not configured or available.
…cript

- Introduced tolerant boolean parsing using distutils.util.strtobool for improved environment variable handling.
- Updated test mode detection logic to handle various input formats and ensure robustness against invalid values.
- Refactored the admin client fix script to check and append missing export statements for better modularity.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
Makefile (2)

200-202: Ensure modern Docker Compose availability.
The dev target now leverages docker compose up (CLI plugin) instead of the deprecated docker-compose. Confirm that your CI/dev environments have the v2 plugin installed; otherwise, adding a fallback check could improve DX.


621-623: Enhance help listing for env commands.
The regex only captures env, check-doppler, and doppler targets; it omits check-root-env, env-doppler, doppler-login-check, etc. Consider broadening the pattern to surface these commands in make help.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 546d775 and 466642e.

📒 Files selected for processing (3)
  • .github/workflows/playwright.yml (2 hunks)
  • .github/workflows/use-make-test.yml (2 hunks)
  • Makefile (8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/playwright.yml
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: test-playwright (4, 4)
  • GitHub Check: test-playwright (3, 4)
  • GitHub Check: test-playwright (2, 4)
  • GitHub Check: test-playwright (1, 4)
🔇 Additional comments (6)
.github/workflows/use-make-test.yml (2)

1-1: Workflow name updated appropriately.
The display name has been changed to “Use Make All Test,” which accurately conveys the step being executed.


27-28: Verify Makefile availability in the backend directory.
With working-directory: backend, the runner will expect a Makefile inside backend/ defining the all and test targets. Please confirm that such a Makefile exists there; otherwise, this step will fail.

Makefile (4)

62-62: Add Doppler CLI presence check.
Introducing DOPPLER_EXISTS ensures subsequent env management targets can gracefully handle a missing Doppler CLI. Nice enhancement for robustness.


64-70: Structure environment management targets.
The new ENV MANAGEMENT header and env-init phony target clearly document and enforce .env initialization across all components. This aligns well with the project’s need for consistent environment setup.


195-195: Verify omission of test from the all target.
The # test comment suggests tests are intentionally excluded from make all. Please confirm this matches CI expectations—if you want tests run by default, consider including the test target here.


654-660: Add setup target for streamlined onboarding.
Introducing the setup phony target to run env-init and install-tools provides a one-step initialization for new contributors—great for improving developer experience.

cubxxw added 2 commits May 19, 2025 10:58
…make commands

- Added a step to install pnpm using the pnpm/action-setup@v3 action for better package management.
- Modified the make command execution to simplify the workflow by directly running 'make all' without specifying the working directory.
…ectories

- Added a step to run pnpm install for the frontend, admin, and extension directories, improving package management.
- This change enhances the workflow by ensuring all necessary dependencies are installed before executing make commands.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
.github/workflows/use-make-test.yml (3)

27-34: Review pnpm setup step and dependency installation

The pnpm/action-setup action’s run_install input is intended as a boolean flag to automatically run pnpm install after setup. Passing a multiline string of install commands may not be interpreted correctly. Consider splitting this into:

  1. A dedicated setup step for pnpm (with run_install: false).
  2. Separate steps to install dependencies in each directory.

Example refactor:

-      - name: Install pnpm
-        uses: pnpm/action-setup@v3
-        with:
-          run_install: |
-            - pnpm install --dir frontend
-            - pnpm install --dir admin
-            - pnpm install --dir extension
+      - name: Setup pnpm
+        uses: pnpm/action-setup@v3
+        with:
+          version: "7"          # or your chosen pnpm version
+          run_install: false    # skip auto install
+
+      - name: Install frontend dependencies
+        run: pnpm install --dir frontend
+
+      - name: Install admin dependencies
+        run: pnpm install --dir admin
+
+      - name: Install extension dependencies
+        run: pnpm install --dir extension

This clarifies intent and ensures each install command runs as expected.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 34-34: trailing spaces

(trailing-spaces)


34-34: Remove trailing spaces
Line 34 contains trailing whitespace which triggers a YAML lint error. Please remove the extra spaces.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 34-34: trailing spaces

(trailing-spaces)


36-36: Add newline at end of file
The file currently lacks a terminating newline, causing a lint warning. Please ensure there’s a blank line at EOF.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 36-36: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 466642e and 1666a96.

📒 Files selected for processing (3)
  • .github/workflows/use-make-test.yml (2 hunks)
  • backend/app/tests_pre_start.py (2 hunks)
  • scripts/fix-admin-client.sh (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • scripts/fix-admin-client.sh
  • backend/app/tests_pre_start.py
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/use-make-test.yml

[error] 34-34: trailing spaces

(trailing-spaces)


[error] 36-36: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: test-playwright (3, 4)
  • GitHub Check: test-playwright (4, 4)
  • GitHub Check: test-playwright (1, 4)
  • GitHub Check: test-playwright (2, 4)

…ment and rebase strategy

- Modified the generate-client workflow to use rebase instead of pull for better conflict resolution.
- Added a step in the test-backend workflow to install backend dependencies before running migrations and tests.
- Updated the test-docker-compose workflow to include a health check for the admin service.
- Enhanced the use-make-test workflow to install dependencies for frontend, admin, and extension directories separately, ensuring all necessary packages are available before executing make commands.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
.github/workflows/generate-client.yml (1)

94-95: Avoid direct use of GitHub context in scripts
Directly interpolating ${{ github.head_ref || github.ref_name }} can be flagged by Actionlint as untrusted. As per prior reviews, define environment variables (e.g., HEAD_REF, REF_NAME) at the job level and reference those instead.

🧹 Nitpick comments (4)
.github/workflows/test-backend.yml (1)

46-47: Specify Python dependency installation in UV environment
To ensure pip install -e . runs inside the UV-managed virtual environment, consider invoking it via uv run pip install -e . instead of uv pip install -e ..

- run: docker compose --project-name $STACK_NAME run --rm backend uv pip install -e .
+ run: docker compose --project-name $STACK_NAME run --rm backend uv run pip install -e .
.github/workflows/generate-client.yml (1)

40-41: Unify UV GitHub Action version
This step uses astral-sh/setup-uv@v1, whereas other workflows (e.g., test-backend.yml) use @v6. For consistency and improved caching, consider aligning the version across workflows.

.github/workflows/use-make-test.yml (2)

28-41: Remove trailing whitespace
Several lines (e.g., 28, 33, 37, 41) contain trailing spaces, which can trigger YAMLlint warnings. Please remove these to comply with lint rules.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 28-28: trailing spaces

(trailing-spaces)


[error] 33-33: trailing spaces

(trailing-spaces)


[error] 37-37: trailing spaces

(trailing-spaces)


[error] 41-41: trailing spaces

(trailing-spaces)


43-43: Ensure newline at end of file
Add a newline at the end of this file to satisfy the new-line-at-end-of-file rule in YAMLlint.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 43-43: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 1666a96 and 569e60a.

📒 Files selected for processing (4)
  • .github/workflows/generate-client.yml (6 hunks)
  • .github/workflows/test-backend.yml (1 hunks)
  • .github/workflows/test-docker-compose.yml (1 hunks)
  • .github/workflows/use-make-test.yml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/test-docker-compose.yml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/use-make-test.yml

[error] 28-28: trailing spaces

(trailing-spaces)


[error] 33-33: trailing spaces

(trailing-spaces)


[error] 37-37: trailing spaces

(trailing-spaces)


[error] 41-41: trailing spaces

(trailing-spaces)


[error] 43-43: no new line character at the end of file

(new-line-at-end-of-file)

.github/workflows/generate-client.yml

[error] 119-119: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test-playwright (3, 4)
🔇 Additional comments (13)
.github/workflows/generate-client.yml (8)

6-8: Adopt inline YAML list syntax for event triggers
Using [opened, synchronize] for pull_request.types and [main] for push.branches improves readability and aligns with GitHub Actions best practices.


11-11: Grant explicit write permission for contents
Adding contents: write under permissions ensures the workflow can commit and push the generated files.


27-27: Clean up pnpm setup step
Specifying version: 9.9.0 and removing commented‐out options makes the step concise and predictable.


48-48: Verify working-directory for frontend install
Specifying working-directory: frontend ensures dependencies install in the correct context. This matches the intended project structure.


52-52: Verify working-directory for admin install
Specifying working-directory: admin ensures dependencies install in the correct context.


78-79: Set Git user globally in runner
Using git config --global is fine in an ephemeral runner and ensures user identity persists across subsequent git commands.


81-84: Stage generated output paths
The step correctly stages the updated client files in both frontend/app/openapi-client and admin/src/client.


103-103: Validate BOT_GITHUB_TOKEN permissions
Switching to secrets.BOT_GITHUB_TOKEN grants elevated scopes. Please confirm that this token has the necessary write permissions and follows your rotation policy.

.github/workflows/use-make-test.yml (5)

1-11: Define workflow triggers with inline syntax
The push and pull_request events use clear inline list notation for branches and event types, ensuring precise CI invocation.


12-15: Configure lint-backend job
Naming the job lint-backend and running on ubuntu-latest aligns with your other CI workflows and provides a consistent environment.


16-27: Set up environment tools
Steps for checking out code, setting up Python 3.10, installing uv, and pinning pnpm to version 8 are well-structured and consistent with other workflows.


29-41: Optimize dependency installation via hashFiles
Installing dependencies only when the respective package.json changes (for frontend, admin, extension) is an excellent optimization to reduce CI runtime.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 33-33: trailing spaces

(trailing-spaces)


[error] 37-37: trailing spaces

(trailing-spaces)


[error] 41-41: trailing spaces

(trailing-spaces)


42-43: Run aggregated build/test via Makefile
Using make all centralizes build and test commands. Ensure the Makefile covers all necessary targets (lint, test, build).

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 43-43: no new line character at the end of file

(new-line-at-end-of-file)

cubxxw added 2 commits May 19, 2025 11:41
- Changed the commit command to include the `-a` flag, ensuring all changes are staged before committing.
- This adjustment improves the workflow by preventing potential issues with untracked changes during the client generation process.
- Adjusted the order of targets in the 'all' command to prioritize backend, frontend, and admin builds before formatting and linting.
- This change enhances the build process by ensuring that core components are built first, followed by checks and generation tasks.
@cubxxw cubxxw merged commit 8503701 into main May 19, 2025
8 of 13 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in nexus May 19, 2025
@cubxxw cubxxw deleted the feat/deployment-project branch May 19, 2025 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant