Skip to content

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Aug 26, 2025

This PR adds comprehensive UUID support for parent_id columns in filament-tree, addressing the limitation where the package only worked with integer parent IDs.

Problem

Previously, the package assumed integer parent_id values with -1 as the default for root nodes. This prevented users from using UUIDs in their tree schemas, as the package would:

  1. Create integer columns in migrations via treeColumns()
  2. Use numeric comparisons (<= 0) that don't work with UUIDs
  3. Hard-code -1 as the default parent value
  4. Fail to handle null parent values for UUID-based root nodes

Solution

The implementation adds UUID support while maintaining full backward compatibility:

Migration Support

Extended the treeColumns() macro to accept a parent column type parameter:

// Integer parent_id (existing behavior)
$table->treeColumns();

// UUID parent_id (new)
$table->treeColumns('string', 'uuid');

// String parent_id (new)  
$table->treeColumns('string', 'string');

Configuration Flexibility

Users can now configure the default parent ID to work with UUIDs:

// For UUID trees
'default_parent_id' => null,

// For integer trees (existing)
'default_parent_id' => -1,

Smart Logic

Updated comparison and tree-building logic to handle both numeric and non-numeric parent keys:

  • Root node detection works with both -1 (integers) and null (UUIDs)
  • Tree building logic distinguishes between numeric and string comparisons
  • Model methods like isRoot() and scopeIsRoot() handle both cases with proper SQL generation

Example Usage

// Model with UUID support
class Category extends Model
{
    use HasUuids, ModelTree;
    
    protected $fillable = ['parent_id', 'title', 'order'];
    protected $casts = ['parent_id' => 'string'];
}

// Data structure
[
    ['id' => 'uuid-1', 'parent_id' => null, 'title' => 'Root'],
    ['id' => 'uuid-2', 'parent_id' => 'uuid-1', 'title' => 'Child'],
]

Code Quality Improvements

During implementation review, several logic issues were identified and fixed:

  • Fixed SQL query generation for UUID root detection (WHERE parent_id IS NULL vs WHERE parent_id = NULL)
  • Standardized to strict type comparisons throughout the codebase
  • Added input validation for migration parameters
  • Enhanced documentation with clear configuration requirements

All existing integer-based implementations continue to work without any changes required.

Fixes #83.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copy link
Author

Copilot AI commented Aug 26, 2025

@lam0819 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

@Copilot Copilot AI changed the title [WIP] [Question]: UUID support for parent_id? Add UUID support for parent_id columns in filament-tree Aug 26, 2025
Copilot finished work on behalf of lam0819 August 26, 2025 14:14
@Copilot Copilot AI requested a review from lam0819 August 26, 2025 14:14
@lam0819
Copy link
Contributor

lam0819 commented Aug 27, 2025

@copilot review first before i review

Copy link
Author

Copilot AI commented Aug 27, 2025

@copilot review first before i review

I've completed my review of the UUID support implementation and identified several logic issues that I've now fixed:

Issues Found & Fixed:

  1. SQL Query Logic Error: scopeIsRoot() was using WHERE parent_id = NULL which never matches records. Fixed to use WHERE parent_id IS NULL for UUID keys.

  2. Inconsistent Comparisons: Mixed strict (===) and loose (==) comparisons. Standardized to strict comparisons throughout.

  3. Array Key Issues: selectArray() method tried to use null as array key for UUID scenarios. Added guard logic to only include root option for integer keys.

  4. Input Validation: Added validation for treeColumns() parent type parameter to prevent invalid values.

  5. Logic Cleanup: Removed unreachable conditions in tree building logic and improved null handling.

  6. Documentation: Enhanced README with clearer configuration requirements and troubleshooting guidance.

The implementation now properly handles both integer and UUID parent keys with correct SQL generation and type-safe comparisons. All backward compatibility is maintained for existing integer-based trees.

Commit: bd153ef

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/pestphp/pest-plugin/zipball/e79b26c65bc11c41093b10150c1341cc5cdbea83
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/kV9kKF /usr/bin/composer install --no-interaction (http block)
  • https://api.github.com/repos/phpstan/extension-installer/zipball/85e90b3942d06b2326fba0403ec24fe912372936
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/kV9kKF /usr/bin/composer install --no-interaction (http block)
  • https://api.github.com/repos/phpstan/phpstan/zipball/41600c8379eb5aee63e9413fe9e97273e25d57e4
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/kV9kKF /usr/bin/composer install --no-interaction (http block)
  • https://api.github.com/repos/symfony/polyfill-ctype/zipball/a3cc8b044a6ea513310cbd48ef7333b384945638
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/kV9kKF /usr/bin/composer install --no-interaction (http block)
  • https://api.github.com/repos/symfony/polyfill-iconv/zipball/5f3b930437ae03ae5dff61269024d8ea1b3774aa
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/kV9kKF /usr/bin/composer install --no-interaction (http block)
  • https://api.github.com/repos/symfony/polyfill-mbstring/zipball/6d857f4d76bd4b343eac26d6b539585d2bc56493
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/kV9kKF /usr/bin/composer install --no-interaction (http block)
  • https://api.github.com/repos/symfony/polyfill-php80/zipball/0cc9dd0f17f61d8131e7df6b84bd344899fe2608
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/kV9kKF /usr/bin/composer install --no-interaction (http block)
  • https://api.github.com/repos/voku/portable-ascii/zipball/b1d923f88091c6bf09699efcd7c8a1b1bfd7351d
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/kV9kKF /usr/bin/composer install --no-interaction (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot finished work on behalf of lam0819 August 27, 2025 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Question]: UUID support for parent_id?
2 participants