feat: add exception for models with conflicting url/urls columns #154
Conversation
- Add validation in HasUniqueUrls trait initialization - Check if model has 'url' or 'urls' database columns - Throw descriptive exception with guidance on fixing conflicts - Add tests (test #12 and #13) to verify exception behavior Fixes #67 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Vladislav Stoitsov <vlados@users.noreply.github.com>
WalkthroughAdds an initializer to the HasUniqueUrls trait that checks for conflicting 'url' or 'urls' database columns during model initialization and throws an exception if found; adds tests that create tables with those columns and assert the exception is thrown. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant M as Model (uses HasUniqueUrls)
participant T as HasUniqueUrls Trait
participant DB as Schema/DB
Dev->>M: Define model using HasUniqueUrls
activate M
M->>T: initializeHasUniqueUrls()
activate T
T->>T: checkForConflictingAttributes()
T->>DB: hasColumn('url') / hasColumn('urls')
alt Column exists
T-->>M: Throw exception (conflicting column)
else Schema unavailable or no column
T-->>M: Continue initialization
end
deactivate T
deactivate M
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
Pull Request Review: feat: add exception for models with conflicting url/urls columnsSummaryThis PR adds validation to detect conflicting ✅ Strengths
🔍 Code Quality Issues1. Exception Type (src/HasUniqueUrls.php:44)Issue: Using generic Current: throw new Exception(
"Model [{$modelClass}] has a conflicting column '{$attribute}'. " ...
);Recommendation: Create a custom exception or use throw new \InvalidArgumentException(
"Model [{$modelClass}] has a conflicting column '{$attribute}'. " ...
);2. Incomplete Comment (src/HasUniqueUrls.php:42)Issue: Comment mentions checking "fillable, guarded, or as a database column" but implementation only checks database columns. Current: // Check if attribute exists in fillable, guarded, or as a database column
if ($this->hasColumn($attribute)) {Recommendation: Either update the comment to match implementation or extend the check: // Check if attribute exists as a database column
if ($this->hasColumn($attribute)) {3. Silent Exception Swallowing (src/HasUniqueUrls.php:64)Issue: The catch block silently swallows all exceptions without logging or providing feedback. Current: } catch (\Exception $e) {
// If we can't check the schema (e.g., during testing without migrations), skip the check
}Concern: This could hide genuine errors (connection issues, permission problems, etc.) beyond just missing migrations. Recommendation: Be more specific about when to skip: } catch (\Exception $e) {
// Skip check if table doesn't exist (e.g., during testing)
// Log other errors for debugging
if (!str_contains($e->getMessage(), 'does not exist') &&
!str_contains($e->getMessage(), 'no such table')) {
// Consider logging: \Log::warning("Unable to check schema: " . $e->getMessage());
}
}🐛 Potential Bugs4. Timing Issue: Check happens after model instantiationCritical Issue: The
Test verification concern: Looking at tests 12 and 13 (lines 175-247), they test
Recommendation: Consider moving the check to a static boot method that runs once per model class: protected static function bootHasUniqueUrls(): void
{
static::creating(static function (Model $model): void {
// Run check only on first instance
if (!static::$conflictCheckRun ?? false) {
$model->checkForConflictingAttributes();
static::$conflictCheckRun = true;
}
if ($model->isAutoGenerateUrls()) {
// existing created logic
}
});
// ... rest of boot logic
}⚡ Performance Considerations5. Schema check on every model instantiationIssue: Impact:
Recommendation: Cache the schema check result:
private static array $checkedModels = [];
private function checkForConflictingAttributes(): void
{
$modelClass = get_class($this);
// Skip if already checked for this model class
if (isset(self::$checkedModels[$modelClass])) {
return;
}
// ... existing check logic ...
self::$checkedModels[$modelClass] = true;
}🔒 Security Considerations✅ No security concerns identified. The code doesn't handle user input or sensitive data. 🧪 Test CoverageStrengths:
Gaps:
Suggested additional tests: test('14. Check validation is skipped when table does not exist', function () {
$modelClass = new class () extends \Illuminate\Database\Eloquent\Model {
use \Vlados\LaravelUniqueUrls\HasUniqueUrls;
protected $table = 'non_existent_table';
// ... rest of setup
};
// Should not throw exception
expect(fn () => new $modelClass())->not->toThrow(Exception::class);
});
test('15. Check validation runs only once per model class', function () {
Schema::create('test_table', function (Blueprint $table) {
$table->increments('id');
$table->string('name');
$table->string('url');
});
// Count DB queries to verify check runs only once
// ... implementation
Schema::dropIfExists('test_table');
});📋 Recommendations SummaryHigh Priority:
Medium Priority: Low Priority: 🎯 ConclusionThis PR successfully addresses issue #67 and adds valuable validation. The implementation is functional but would benefit from performance optimization (caching) and better exception handling. The test coverage is good but could be enhanced with edge case testing. Recommendation: ✅ Approve with minor changes - The current implementation is acceptable, but I strongly suggest implementing the caching mechanism before merging to avoid performance issues in production. Great work on the clear error messaging and comprehensive testing! 🚀 |
|
@claude some checks are failing |
|
Claude finished @vlados's task —— View job Fixed failing tests ✅
Root CauseTests 12 and 13 were failing because the exception is being thrown during model instantiation (when the anonymous class is created), not during the Fix AppliedWrapped the entire model class creation inside the expect closure so the exception can be properly caught and tested. The tests now correctly validate that the exception is thrown when models have conflicting Changes committed: The CI checks should pass now! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/HasUniqueUrls.php (1)
53-68: Consider catching more specific exceptions.Catching the generic
\Exceptionon line 63 may silently ignore unexpected errors beyond schema access issues. Laravel typically throws specific exceptions for schema-related problems.Consider catching more specific exceptions:
private function hasColumn(string $column): bool { try { // Check if the table exists and has the column if ($this->getConnection()->getSchemaBuilder()->hasColumn($this->getTable(), $column)) { return true; } - } catch (\Exception $e) { - // If we can't check the schema (e.g., during testing without migrations), skip the check + } catch (\Illuminate\Database\QueryException | \PDOException $e) { + // If we can't check the schema (e.g., table doesn't exist), skip the check + // This allows models to be instantiated before migrations run } return false; }This ensures only database/schema-related exceptions are caught while allowing other unexpected errors to surface.
tests/HasUniqueUrlsTest.php (1)
212-247: Apply the same cleanup improvements as test 12.This test has an identical structure to test 12 and would benefit from the same cleanup improvements (dropping table before creation and using try-finally for guaranteed cleanup).
Additionally, consider extracting a helper function to reduce duplication between tests 12 and 13:
function createConflictingColumnTest(string $columnName): void { $tableName = "models_with_{$columnName}_column"; Schema::dropIfExists($tableName); Schema::create($tableName, function (Blueprint $table) use ($columnName) { $table->increments('id'); $table->string('name'); $table->string($columnName); }); try { $modelClass = new class ($tableName) extends \Illuminate\Database\Eloquent\Model { use \Vlados\LaravelUniqueUrls\HasUniqueUrls; protected $guarded = []; public $timestamps = false; public function __construct(string $table = null, array $attributes = []) { $this->table = $table ?? $this->table; parent::__construct($attributes); } public function urlHandler(): array { return [ 'controller' => \Vlados\LaravelUniqueUrls\Tests\TestUrlHandler::class, 'method' => 'view', 'arguments' => [], ]; } public function urlStrategy($language, $locale): string { return \Illuminate\Support\Str::slug($this->name); } }; expect(fn () => $modelClass::create(['name' => 'test', $columnName => "test-{$columnName}"])) ->toThrow(Exception::class, "has a conflicting column '{$columnName}'"); } finally { Schema::dropIfExists($tableName); } } test('12. Check if exception is thrown when model has conflicting url column', function () { createConflictingColumnTest('url'); }); test('13. Check if exception is thrown when model has conflicting urls column', function () { createConflictingColumnTest('urls'); });This eliminates duplication while maintaining test clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/HasUniqueUrls.php(1 hunks)tests/HasUniqueUrlsTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/HasUniqueUrlsTest.php (4)
src/HasUniqueUrls.php (2)
urlHandler(19-19)urlStrategy(105-108)tests/Models/TestModel.php (2)
urlHandler(36-43)urlStrategy(26-29)tests/Models/ChildModel.php (2)
urlHandler(35-42)urlStrategy(30-33)tests/TestUrlHandler.php (1)
TestUrlHandler(7-13)
🪛 GitHub Actions: run-tests
tests/HasUniqueUrlsTest.php
[error] 184-184: Test 12 failed: exception thrown when model has conflicting url column. HasUniqueUrls trait throws a conflicting column error.
[error] 221-221: Test 13 failed: exception thrown when model has conflicting urls column. HasUniqueUrls trait throws a conflicting column error.
src/HasUniqueUrls.php
[error] 44-48: Model [{$modelClass}] has a conflicting column '{$attribute}'. The HasUniqueUrls trait uses 'urls' as a relationship name and provides 'relative_url' and 'absolute_url' attributes. Please rename the '{$attribute}' column in your model to avoid conflicts.
🔇 Additional comments (3)
src/HasUniqueUrls.php (1)
21-29: LGTM! Proper use of Laravel's trait initialization pattern.The
initializeHasUniqueUrls()method follows Laravel's convention for trait initialization, ensuring conflict detection occurs automatically when a model instance is created.tests/HasUniqueUrlsTest.php (2)
3-3: LGTM! Required imports for table schema manipulation.The
BlueprintandSchemaimports are necessary for the new tests that create and drop tables with conflicting columns.Also applies to: 5-5
175-210: Cannot run tests in this sandbox (missingartisan/phpunit). Please execute the updated test locally to confirm it now passes.
| private function checkForConflictingAttributes(): void | ||
| { | ||
| $conflictingAttributes = ['url', 'urls']; | ||
| $modelClass = get_class($this); | ||
|
|
||
| foreach ($conflictingAttributes as $attribute) { | ||
| // Check if attribute exists in fillable, guarded, or as a database column | ||
| if ($this->hasColumn($attribute)) { | ||
| throw new Exception( | ||
| "Model [{$modelClass}] has a conflicting column '{$attribute}'. " . | ||
| "The HasUniqueUrls trait uses 'urls' as a relationship name and provides 'relative_url' and 'absolute_url' attributes. " . | ||
| "Please rename the '{$attribute}' column in your model to avoid conflicts." | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Update the misleading comment on line 42.
The comment states "Check if attribute exists in fillable, guarded, or as a database column" but the implementation only checks database columns via hasColumn(). This could confuse future maintainers.
Apply this diff to fix the comment:
- // Check if attribute exists in fillable, guarded, or as a database column
+ // Check if attribute exists as a database columnAlternatively, if you intended to also check fillable and guarded arrays, consider adding those checks. For example:
if ($this->hasColumn($attribute) ||
in_array($attribute, $this->getFillable()) ||
(! $this->isGuarded($attribute) && count($this->getGuarded()) > 0)) {However, checking only database columns is likely sufficient for this use case since that's where the actual conflict would occur.
🧰 Tools
🪛 GitHub Actions: run-tests
[error] 44-48: Model [{$modelClass}] has a conflicting column '{$attribute}'. The HasUniqueUrls trait uses 'urls' as a relationship name and provides 'relative_url' and 'absolute_url' attributes. Please rename the '{$attribute}' column in your model to avoid conflicts.
The exception is thrown during model initialization (when using the trait), not during the create() call. This fix wraps the entire model class creation inside the expect closure so the exception can be properly caught. Co-authored-by: Vladislav Stoitsov <vlados@users.noreply.github.com>
PR Review: Add exception for models with conflicting url/urls columnsSummaryThis PR adds validation to detect conflicting ✅ Strengths
🔍 Code Quality Issues1. Silent Exception Swallowing (src/HasUniqueUrls.php:64)} catch (\Exception $e) {
// If we can't check the schema (e.g., during testing without migrations), skip the check
}Issue: This silently catches ALL exceptions, which could hide legitimate errors (connection issues, permission problems, etc.). Recommendation: Be more specific about what exceptions to catch: } catch (\Doctrine\DBAL\Exception|\PDOException $e) {
// Schema inspection failed - skip validation
return false;
}2. Inconsistent Return Type Declaration (src/HasUniqueUrls.php:56)The Recommendation: Add return type for consistency: private function checkForConflictingAttributes(): void✅ This is already correct in the code - good job! 3. Missing Type Import (src/HasUniqueUrls.php:7)The code uses Current: 🐛 Potential Bugs1. Timing Issue in Tests (tests/HasUniqueUrlsTest.php:208 & 247)$modelClass::create(['name' => 'test', 'url' => 'test-url']);Issue: The Concern: The test might be passing for the wrong reason. The exception is thrown during the first instantiation (line 186/225), not during the create call. Test Fix: The test should be more explicit: expect(function () {
$model = new class () extends \Illuminate\Database\Eloquent\Model {
use \Vlados\LaravelUniqueUrls\HasUniqueUrls;
// ... rest of definition
};
})->toThrow(Exception::class, "has a conflicting column 'url'");2. Database State PollutionThe tests create and drop tables but run within the same test suite. If test 12 fails before Recommendation: Use try {
expect(function () {
// test code
})->toThrow(...);
} finally {
Schema::dropIfExists('models_with_url_column');
}⚡ Performance ConsiderationsSchema Inspection on Every Model InstantiationThe Impact:
Recommendations:
Example optimization: private static array $validatedModels = [];
public function initializeHasUniqueUrls(): void
{
$class = static::class;
if (!isset(self::$validatedModels[$class])) {
$this->checkForConflictingAttributes();
self::$validatedModels[$class] = true;
}
}🔒 Security ConcernsNo security issues identified. The code doesn't handle user input or expose sensitive data. 📋 Test CoverageCoverage: Good ✅
Suggested Additional Tests:
📝 Recommendations SummaryHigh Priority:
Medium Priority:
Low Priority:
Overall AssessmentStatus: ✅ Approved with suggestions This is a solid improvement that addresses a real pain point (issue #67). The code is well-structured and the error messages are helpful. The main concern is the performance impact of schema inspection on every model instantiation. I recommend implementing the caching solution before merging. Code Quality: 7/10 (good structure, needs optimization) Great work! 🎉 🤖 Review generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/HasUniqueUrlsTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/HasUniqueUrlsTest.php (2)
src/HasUniqueUrls.php (2)
urlHandler(19-19)urlStrategy(105-108)tests/Models/TestModel.php (2)
urlHandler(36-43)urlStrategy(26-29)
🔇 Additional comments (1)
tests/HasUniqueUrlsTest.php (1)
3-5: LGTM!The new imports for
BlueprintandSchemaare correctly added to support table creation and manipulation in the new tests.
| test('12. Check if exception is thrown when model has conflicting url column', function () { | ||
| // Create a table with 'url' column | ||
| Schema::create('models_with_url_column', function (Blueprint $table) { | ||
| $table->increments('id'); | ||
| $table->string('name'); | ||
| $table->string('url'); | ||
| }); | ||
|
|
||
| // Create a model class that uses HasUniqueUrls trait | ||
| // The exception should be thrown during model instantiation | ||
| expect(function () { | ||
| $modelClass = new class () extends \Illuminate\Database\Eloquent\Model { | ||
| use \Vlados\LaravelUniqueUrls\HasUniqueUrls; | ||
|
|
||
| protected $table = 'models_with_url_column'; | ||
| protected $guarded = []; | ||
| public $timestamps = false; | ||
|
|
||
| public function urlHandler(): array | ||
| { | ||
| return [ | ||
| 'controller' => \Vlados\LaravelUniqueUrls\Tests\TestUrlHandler::class, | ||
| 'method' => 'view', | ||
| 'arguments' => [], | ||
| ]; | ||
| } | ||
|
|
||
| public function urlStrategy($language, $locale): string | ||
| { | ||
| return \Illuminate\Support\Str::slug($this->name); | ||
| } | ||
| }; | ||
|
|
||
| $modelClass::create(['name' => 'test', 'url' => 'test-url']); | ||
| })->toThrow(Exception::class, "has a conflicting column 'url'"); | ||
|
|
||
| Schema::dropIfExists('models_with_url_column'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify exception timing and improve test reliability.
Several concerns with this test:
-
Exception timing mismatch: The comment on line 184 states the exception should be thrown during "model instantiation," but the test wraps
create()(line 208) which both instantiates and persists the model. According to the PR objectives, the check happens ininitializeHasUniqueUrls()during trait initialization (when the model is instantiated). The current test may pass, but it's not clear whether the exception occurs during instantiation or later during the create operation. -
Table cleanup risk: Line 211's
dropIfExistsexecutes outside theexpect()closure. If the assertion fails or an unexpected exception is thrown, the table won't be cleaned up, potentially affecting subsequent tests. -
Generic exception type: Using
Exception::classis broad. Consider verifying whether a more specific exception type is thrown (e.g.,RuntimeException,LogicException, or a custom exception class).
Consider this refactor to make the test more explicit and reliable:
test('12. Check if exception is thrown when model has conflicting url column', function () {
// Create a table with 'url' column
Schema::create('models_with_url_column', function (Blueprint $table) {
$table->increments('id');
$table->string('name');
$table->string('url');
});
- // Create a model class that uses HasUniqueUrls trait
- // The exception should be thrown during model instantiation
- expect(function () {
+ try {
+ // The exception should be thrown during model instantiation
$modelClass = new class () extends \Illuminate\Database\Eloquent\Model {
use \Vlados\LaravelUniqueUrls\HasUniqueUrls;
protected $table = 'models_with_url_column';
protected $guarded = [];
public $timestamps = false;
public function urlHandler(): array
{
return [
'controller' => \Vlados\LaravelUniqueUrls\Tests\TestUrlHandler::class,
'method' => 'view',
'arguments' => [],
];
}
public function urlStrategy($language, $locale): string
{
return \Illuminate\Support\Str::slug($this->name);
}
};
-
- $modelClass::create(['name' => 'test', 'url' => 'test-url']);
- })->toThrow(Exception::class, "has a conflicting column 'url'");
-
- Schema::dropIfExists('models_with_url_column');
+
+ // If we reach here, no exception was thrown during instantiation
+ // Try to trigger it via create() as fallback
+ $modelClass::create(['name' => 'test', 'url' => 'test-url']);
+
+ throw new \Exception('Expected exception was not thrown');
+ } catch (\Exception $e) {
+ expect($e->getMessage())->toContain("has a conflicting column 'url'");
+ } finally {
+ Schema::dropIfExists('models_with_url_column');
+ }
});Alternatively, verify the exact behavior by running:
Also verify the exception type being thrown:
🏁 Script executed:
#!/bin/bash
# Check the HasUniqueUrls trait implementation to confirm when the exception is thrown
ast-grep --pattern $'trait HasUniqueUrls {
$$$
initializeHasUniqueUrls($$$) {
$$$
}
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for the exception being thrown for conflicting columns
rg -nP --type=php -A3 -B3 "throw.*conflicting column"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Locate the HasUniqueUrls trait file
rg -lP '\btrait\s+HasUniqueUrls' -g '*.php'
# Inspect the first match if found
FILE=$(rg -lP '\btrait\s+HasUniqueUrls' -g '*.php' | head -n1)
echo "Trait file: $FILE"
if [ -n "$FILE" ]; then
echo "=== File Header ==="
sed -n '1,200p' "$FILE"
echo "=== initializeHasUniqueUrls ==="
rg -n "function initializeHasUniqueUrls" -C3 "$FILE"
echo "=== throw statements ==="
rg -n "throw new" -C3 "$FILE"
fiLength of output: 5092
Clarify exception timing and enforce cleanup
- Reflect that the conflicting‐column exception is thrown in initializeHasUniqueUrls() during model construction; you can trigger it by instantiating the anonymous model (
new class …) without calling create(). - Move Schema::dropIfExists('models_with_url_column') into a finally block (or use Pest’s afterEach) to guarantee teardown even if the assertion fails.
- (Optional) Replace the generic \Exception with a dedicated exception class for conflicting columns.
🤖 Prompt for AI Agents
In tests/HasUniqueUrlsTest.php around lines 175 to 212, the test currently
expects the conflicting-column exception to occur on create(), but the exception
is actually thrown during model construction in initializeHasUniqueUrls();
update the test to instantiate the anonymous model (new class ...) to trigger
the exception immediately instead of calling create(), and wrap the
Schema::dropIfExists('models_with_url_column') call in a finally block (or use
Pest’s afterEach) so the table is always dropped regardless of test outcome;
optionally replace the generic \Exception assertion with the specific
conflicting-column exception class if one exists.
| test('13. Check if exception is thrown when model has conflicting urls column', function () { | ||
| // Create a table with 'urls' column | ||
| Schema::create('models_with_urls_column', function (Blueprint $table) { | ||
| $table->increments('id'); | ||
| $table->string('name'); | ||
| $table->string('urls'); | ||
| }); | ||
|
|
||
| // Create a model class that uses HasUniqueUrls trait | ||
| // The exception should be thrown during model instantiation | ||
| expect(function () { | ||
| $modelClass = new class () extends \Illuminate\Database\Eloquent\Model { | ||
| use \Vlados\LaravelUniqueUrls\HasUniqueUrls; | ||
|
|
||
| protected $table = 'models_with_urls_column'; | ||
| protected $guarded = []; | ||
| public $timestamps = false; | ||
|
|
||
| public function urlHandler(): array | ||
| { | ||
| return [ | ||
| 'controller' => \Vlados\LaravelUniqueUrls\Tests\TestUrlHandler::class, | ||
| 'method' => 'view', | ||
| 'arguments' => [], | ||
| ]; | ||
| } | ||
|
|
||
| public function urlStrategy($language, $locale): string | ||
| { | ||
| return \Illuminate\Support\Str::slug($this->name); | ||
| } | ||
| }; | ||
|
|
||
| $modelClass::create(['name' => 'test', 'urls' => 'test-urls']); | ||
| })->toThrow(Exception::class, "has a conflicting column 'urls'"); | ||
|
|
||
| Schema::dropIfExists('models_with_urls_column'); | ||
| }); |
There was a problem hiding this comment.
Same concerns as test 12, plus code duplication.
This test has the same structural issues as test 12:
- Exception timing mismatch: Comment says instantiation, but test wraps
create(). - Table cleanup risk:
dropIfExistsoutside the exception-handling closure. - Generic exception type: Consider a more specific exception class.
- Code duplication: Tests 12 and 13 are nearly identical except for the column name ('url' vs 'urls') and table name.
Consider extracting a helper function or using Pest's dataset feature to reduce duplication:
test('12. Check if exception is thrown when model has conflicting url column', function () {
testConflictingColumn('url');
});
test('13. Check if exception is thrown when model has conflicting urls column', function () {
testConflictingColumn('urls');
});
function testConflictingColumn(string $columnName): void
{
$tableName = "models_with_{$columnName}_column";
Schema::create($tableName, function (Blueprint $table) use ($columnName) {
$table->increments('id');
$table->string('name');
$table->string($columnName);
});
try {
$modelClass = new class ($tableName) extends \Illuminate\Database\Eloquent\Model {
use \Vlados\LaravelUniqueUrls\HasUniqueUrls;
protected $guarded = [];
public $timestamps = false;
public function __construct(string $table = null, array $attributes = [])
{
if ($table) {
$this->table = $table;
}
parent::__construct($attributes);
}
public function urlHandler(): array
{
return [
'controller' => \Vlados\LaravelUniqueUrls\Tests\TestUrlHandler::class,
'method' => 'view',
'arguments' => [],
];
}
public function urlStrategy($language, $locale): string
{
return \Illuminate\Support\Str::slug($this->name);
}
};
$modelClass::create(['name' => 'test', $columnName => "test-{$columnName}"]);
throw new \Exception('Expected exception was not thrown');
} catch (\Exception $e) {
expect($e->getMessage())->toContain("has a conflicting column '{$columnName}'");
} finally {
Schema::dropIfExists($tableName);
}
}Or use Pest's dataset feature:
test('check if exception is thrown when model has conflicting column', function (string $column) {
// test implementation using $column
})->with(['url', 'urls']);🤖 Prompt for AI Agents
tests/HasUniqueUrlsTest.php around lines 214 to 251: the test duplicates test 12
and has three issues — it asserts an exception on create() while the comment
says instantiation, it leaves Schema::dropIfExists outside exception handling
risking leftover state, and it uses a generic Exception and duplicates code; fix
by asserting the exception at the actual failing point (either instantiate the
anonymous model to trigger the trait check or wrap the create() call inside the
expect(...)->toThrow), move table cleanup into a finally block (or use
try/catch/finally) so Schema::dropIfExists always runs, replace Exception::class
with the more specific exception class thrown by the trait, and remove
duplication by extracting a helper function or using Pest datasets to run the
same test for 'url' and 'urls'.
Summary
Fixes #67
🤖 Generated with Claude Code
Summary by CodeRabbit