Skip to content

Add UUID initialization in model save method #26

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.

Sign up for GitHub

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

Closed
FaboBorgesLima wants to merge 1 commit into from
Closed

Add UUID initialization in model save method #26

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/Model/Concerns/HasUuids.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,27 @@ public function getIncrementing()

return $this->incrementing;
}

/**
* Initialize the model with unique identifiers.
*
* @param array $columns
* @return void
*/
protected function initialize(array $columns): void
{
foreach ($columns as $column) {
if (in_array($column, $this->uniqueIds()) && ! $this->{$column}) {
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

initialize() is currently called with $this->uniqueIds(), but inside the loop it re-checks in_array($column, $this->uniqueIds()) on every iteration. That condition is redundant (and recomputes uniqueIds() repeatedly); simplifying it would make the intent clearer and avoid unnecessary work.

Suggested change
if (in_array($column, $this->uniqueIds()) && ! $this->{$column}) {
if (! $this->{$column}) {

Copilot uses AI. Check for mistakes.
$this->{$column} = $this->newUniqueId();
}
}
}
Comment on lines +73 to +80
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

initialize() is a very generic method name in a codebase where models already have an initializeTraits() mechanism and a convention of initialize{TraitName} initializers. Renaming this helper to something more specific (e.g., initializeUniqueIds / initializeUuids) would reduce the risk of name collisions in consuming models/traits and make the call site clearer.

Copilot uses AI. Check for mistakes.

public function save(array $options = []): bool
{
$this->initialize($this->uniqueIds());
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Overriding save() to always (re)initialize unique IDs can unintentionally mutate existing records on update. If a model already exists and a unique-id column isn't loaded (or is intentionally set to a falsy value), this will generate a new UUID and mark the attribute dirty, causing an UPDATE that changes a supposedly stable identifier. Consider only initializing unique IDs for new models (e.g., when ! $this->exists) or hooking into the insert/creating path instead of every save.

Suggested change
$this->initialize($this->uniqueIds());
if (! $this->exists) {
$this->initialize($this->uniqueIds());
}

Copilot uses AI. Check for mistakes.

return parent::save($options);
Comment on lines +82 to +86
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

This PR introduces new behavior (UUID assignment during save()), but current tests only assert the format returned by newUniqueId(). Adding a test that saving a new model using HasUuids auto-populates the configured unique-id columns would help prevent regressions, especially around create vs update behavior.

Copilot uses AI. Check for mistakes.
}
}

Loading