Skip to content

Conversation

@taka-oyama
Copy link
Collaborator

@taka-oyama taka-oyama commented Mar 26, 2025

Summary by CodeRabbit

  • Chores

    • Upgraded core dependencies to newer major versions, ensuring enhanced compatibility and performance.
  • Refactor

    • Streamlined database connection, schema operations, and query handling to improve reliability and efficiency in data management.
    • Enhanced type safety in model relationships for a more robust and consistent experience.

@taka-oyama taka-oyama added enhancement New feature or request waiting for review labels Mar 26, 2025
@taka-oyama taka-oyama requested a review from a team March 26, 2025 06:52
@taka-oyama taka-oyama self-assigned this Mar 26, 2025
@coderabbitai
Copy link

coderabbitai bot commented Mar 26, 2025

Walkthrough

This pull request updates dependency versions and adjusts method signatures across several files. The composer.json file now requires Laravel 12 and Testbench 10. In the source files, methods in connection, builder, and grammar classes have been simplified and extended with additional parameters for schema handling. Test files have been modified accordingly, with updated assertions and streamlined Blueprint instantiations and calls for SQL generation. Overall, the changes align the codebase with newer library versions and improve consistency in method signatures and testing.

Changes

File(s) Change Summary
composer.json Updated dependency versions: Laravel Framework from ^11.31.0 to ^12.0 and Orchestra Testbench from ~9 to ~10.
src/Connection.php Simplified getDefaultQueryGrammar and getDefaultSchemaGrammar methods to directly return new grammar instances while removing explicit connection and table prefix setup.
src/Schema/Builder.php Modified getTables to accept an optional $schema parameter; updated createBlueprint to include the connection; simplified dropAllTables by removing extra parameters from toSql calls.
src/Schema/Grammar.php Updated method signatures for compileTables, compileIndexes, compileForeignKeys, and compileColumns to include a new $schema parameter; removed the $connection parameter from compileChange.
tests/* (ConnectionTest.php, Eloquent/ModelTest.php, Query/BuilderTest.php, Schema/BlueprintTest.php, Schema/BuilderTestLast.php) Adjusted tests to reflect source changes: removed table prefix assertions, added return type to the items method, updated Blueprint instantiation and toSql calls, removed redundant inline comments, and fixed minor comment typos.

Suggested reviewers

  • halnique
  • zeriyoshi

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e0a9618 and 25497fb.

📒 Files selected for processing (9)
  • composer.json (1 hunks)
  • src/Connection.php (2 hunks)
  • src/Schema/Builder.php (4 hunks)
  • src/Schema/Grammar.php (4 hunks)
  • tests/ConnectionTest.php (2 hunks)
  • tests/Eloquent/ModelTest.php (1 hunks)
  • tests/Query/BuilderTest.php (4 hunks)
  • tests/Schema/BlueprintTest.php (43 hunks)
  • tests/Schema/BuilderTestLast.php (1 hunks)
🧰 Additional context used
🧬 Code Definitions (5)
tests/ConnectionTest.php (1)
src/Schema/Grammar.php (1)
  • Grammar (33-1080)
tests/Schema/BlueprintTest.php (3)
tests/TestCase.php (3)
  • getDefaultConnection (102-108)
  • generateTableName (61-64)
  • generateUuid (69-72)
src/Schema/Blueprint.php (4)
  • Blueprint (30-491)
  • uuid (129-134)
  • string (119-123)
  • integer (49-52)
src/Schema/UuidColumnDefinition.php (1)
  • generateUuid (29-32)
src/Schema/Builder.php (3)
src/Connection.php (2)
  • select (269-272)
  • table (247-250)
src/Schema/Grammar.php (1)
  • compileTables (48-51)
src/Schema/Blueprint.php (1)
  • Blueprint (30-491)
tests/Query/BuilderTest.php (2)
src/Schema/Blueprint.php (5)
  • Blueprint (30-491)
  • uuid (129-134)
  • string (119-123)
  • tokenList (256-263)
  • fullText (296-309)
src/Connection.php (1)
  • table (247-250)
src/Schema/Grammar.php (2)
src/Connection.php (1)
  • table (247-250)
src/Schema/Blueprint.php (1)
  • Blueprint (30-491)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Run-PHPUnit
🔇 Additional comments (31)
tests/Schema/BuilderTestLast.php (1)

316-316: Minor text correction in comment

Fixed a small grammatical error in the comment, changing "before hand" to "beforehand" which is the correct spelling for this adverb.

composer.json (2)

21-21: Updated testbench dependency to match Laravel version

Correctly updated the Orchestra Testbench version to ~10 to match the Laravel 12 upgrade. This is necessary as testbench versions are typically aligned with Laravel major versions.


14-14:

Details

❓ Verification inconclusive

Major framework upgrade to Laravel 12

The project is being upgraded from Laravel 11.31.0 to Laravel 12.0, which is a major version change that may include breaking changes.


🏁 Script executed:

#!/bin/bash
# Description: Check for significant changes or breaking changes in Laravel 12

# Check Laravel 12 release notes or upgrade guide references
echo "Checking for Laravel 12 upgrade documentation..."
curl -s https://api.github.com/repos/laravel/laravel/releases | grep -A 5 "v12.0.0"

Length of output: 1825


Attention: Verify Compatibility with Laravel 12

The dependency in composer.json has been updated to "laravel/framework": "^12.0", marking an upgrade from Laravel 11.31.0 to Laravel 12.0. The release notes (as seen from v12.0.0 on GitHub) indicate several changes that may introduce breaking changes (e.g., removal of APP_TIMEZONE, updated static analysis enhancements, etc.).

Please ensure that:

  • All breaking changes detailed in the release notes are reviewed and addressed.
  • The application is thoroughly tested against the Laravel 12 Upgrade Guide.
tests/Eloquent/ModelTest.php (1)

127-127: Improved type safety with return type declaration

Added a return type declaration (BelongsToMany) to the items() method, which improves type safety and code readability. This change aligns with modern PHP best practices and Laravel 12's increased emphasis on type declarations.

tests/ConnectionTest.php (3)

23-23: Added import for Schema Grammar class

Added an import for the Colopl\Spanner\Schema\Grammar class, which is now used directly in the assertions below.


624-624: Updated assertion to check grammar instance type

Changed the test to verify that getQueryGrammar() returns the expected instance type rather than checking table prefix behavior. This approach is more focused on validating the core responsibility of the method and aligns with Laravel 12 changes.


631-631: Updated assertion to check schema grammar instance type

Changed the test to verify that getSchemaGrammar() returns the expected instance type rather than checking table prefix behavior. This is a more appropriate test for the method's core responsibility.

src/Connection.php (2)

205-205: Method refactored for simplicity while maintaining the same functionality.

The implementation has been simplified to directly create and return a QueryGrammar instance with the connection passed as a constructor parameter, rather than creating an instance and then setting connection and table prefix separately.


214-214: Method refactored for simplicity while maintaining the same functionality.

Similarly to the QueryGrammar implementation, this change streamlines the SchemaGrammar instantiation by passing the connection directly to the constructor rather than setting it afterwards.

tests/Query/BuilderTest.php (4)

1138-1139: Updated Blueprint instantiation to match new signature.

The code now uses the new pattern for Blueprint construction where connection is passed as the first parameter, and the build() method no longer requires explicit parameters as it uses the connection from the constructor. This follows Laravel 12's patterns.

Also applies to: 1146-1146


1162-1163: Updated Blueprint instantiation to match new signature.

Similar to the previous test, this search test has been updated to use the new Blueprint construction pattern and build method that doesn't require explicit parameters.

Also applies to: 1170-1170


1186-1187: Updated Blueprint instantiation to match new signature.

This test for ngram search functionality has been updated to use the new connection-first Blueprint constructor and simplified build method signature.

Also applies to: 1194-1194


1210-1211: Updated Blueprint instantiation to match new signature.

This test for substring search functionality follows the same pattern of updates, ensuring consistency with Laravel 12's approach to constructing Blueprint objects.

Also applies to: 1218-1218

tests/Schema/BlueprintTest.php (8)

20-20: Added Connection import to support updated Blueprint construction.

Added the Connection import to support the new Blueprint construction pattern where a connection instance is the first parameter.


33-38: Added helper method to standardize connection preparation.

This new method encapsulates the common pattern of retrieving the default connection and applying the default schema grammar, which improves code maintainability and reduces duplication throughout the test file.


44-44: Updated Blueprint instantiation and simplified method calls.

The Blueprint constructor now accepts a connection as its first parameter, and the toSql() method no longer requires explicit parameters as it uses the connection from the constructor. This change aligns with Laravel 12's approach to handling connections and grammars.

Also applies to: 70-70


103-103: Updated Blueprint instantiation and simplified method calls.

Similar to previous changes, this test has been updated to use the new Blueprint construction pattern and simplified method calls, consistent with Laravel 12's approach.

Also applies to: 109-109


447-474: Consistent pattern of Blueprint instantiation and method simplification.

These tests follow the same pattern of updating Blueprint instantiation to accept a connection as the first parameter and simplifying method calls by removing explicit parameters. The changes maintain functional equivalence while aligning with Laravel 12's approach.

Also applies to: 474-474, 529-542, 542-542, 596-606, 606-606


653-691: Consistent changes across sequence and change stream tests.

These tests for sequence and change stream operations have been updated with the same pattern of changes: connection-first Blueprint constructor and simplified method calls. The changes are consistent and maintain the same functionality while aligning with Laravel 12.

Also applies to: 691-691, 693-805, 805-805


817-918: Updated Blueprint instantiation across remaining tests.

The remaining tests in the file have been updated with the same pattern of changes, ensuring consistency across the entire test suite and alignment with Laravel 12's approach to handling connections and grammars.

Also applies to: 918-918, 958-968, 984-1005, 1005-1005


1009-1047: Index-related tests updated for consistency.

The tests for various index types (interleaving, storing, null-filtered) have been updated with the same pattern of changes as the rest of the file, completing the transition to Laravel 12's approach uniformly across all test cases.

Also applies to: 1047-1047

src/Schema/Grammar.php (5)

45-48: Method signature updated for schema support

The compileTables method has been updated to accept a schema parameter, which aligns with Laravel 12's architecture. This change improves flexibility for multi-schema environments while maintaining backward compatibility.


56-60: Parameter type and order changed for schema support

The compileIndexes method signature has been updated to include a nullable string schema parameter as the first argument. This follows Laravel 12's database interface patterns for consistent schema handling across the framework.


71-75: Parameter type and order changed for schema support

Similar to other methods, compileForeignKeys now accepts a schema parameter. This maintains consistency across the database schema manipulation methods.


86-90: Parameter type and order changed for schema support

The compileColumns method has been updated to match the same pattern as other schema-related methods, accepting a nullable schema parameter.


143-143: Method signature simplified by removing connection parameter

The compileChange method signature has been simplified by removing the $connection parameter, which is a common pattern in Laravel 12 where connection dependencies are managed differently.

src/Schema/Builder.php (5)

44-49: Method signature updated for schema support

The getTables method now accepts an optional schema parameter, which aligns with the changes in the Grammar class. This enhances the flexibility of the method to potentially work with different database schemas.


53-53: Updated method call to match new signature

The call to compileTables now explicitly passes null as the schema parameter to match the updated method signature in the Grammar class.


91-91: Blueprint instantiation updated to include connection

The Blueprint constructor now accepts a Connection object as its first parameter, which follows Laravel 12's pattern of making database objects connection-aware. This allows the blueprint to directly access connection properties and methods without requiring them to be passed separately.


139-139: Simplified method call by removing parameters

The call to toSql() has been simplified by removing the previously required parameters. This suggests that the Blueprint class now stores its own references to the connection and grammar, reducing parameter coupling.


157-157: Simplified method call by removing parameters

Similar to the previous change, this call to toSql() has been simplified to leverage the Blueprint's internal connection and grammar references rather than passing them explicitly.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 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.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate unit testing code.
    • @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.

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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @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

@kt81 kt81 left a comment

Choose a reason for hiding this comment

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

LGTM

@taka-oyama taka-oyama merged commit 9b0590b into master Apr 3, 2025
1 check passed
@taka-oyama taka-oyama deleted the feat/laravel-12 branch April 3, 2025 06:07
@coderabbitai coderabbitai bot mentioned this pull request Apr 28, 2025
@coderabbitai coderabbitai bot mentioned this pull request Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request waiting for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants