Feature Improved Migration Handling

[StanBarrows]

No description provided.

[Copilot]

Pull request overview

This PR improves migration handling for the Laravel Event Logs package by adding support for publishing migrations and enhancing the schema management commands. The changes focus on making the migration workflow more flexible and user-friendly.

Key changes:

• Added migration publishing capability to the service provider
• Enhanced CreateSchemaCommand to automatically publish and run migrations
• Simplified DropSchemaCommand by replacing schema existence checks with table existence checks
• Updated documentation to reflect the new optional migration publishing workflow

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/LaravelEventLogsServiceProvider.php	Adds migration publishing configuration with the 'laravel-event-logs-migrations' tag
src/Commands/CreateSchemaCommand.php	Implements automatic migration publishing and execution with file existence checks and user feedback
src/Commands/DropSchemaCommand.php	Simplifies table existence checking by using hasTable() instead of database schema queries and adds success message
composer.lock	Updates Laravel framework and various Symfony dependencies to newer versions, including Symfony 8.x components
README.md	Documents the new optional migration publishing workflow and clarifies command behavior

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The error message states "Migration file not found. Please publish migrations first", but the check on line 37 is looking for the migration file in the package's own directory (__DIR__.'/../../database/migrations/...'). If this file doesn't exist, it's a package structure issue, not a user error. The user shouldn't be asked to publish migrations to fix this.

Consider either:

1. Removing this check entirely (the migration should always exist in the package)
2. Changing the error message to indicate this is an internal package error

Suggested change

	$this->error('Migration file not found. Please publish migrations first:');
	$this->line('php artisan vendor:publish --tag=laravel-event-logs-migrations');
	$this->error('Internal error: migration file missing from package. Please contact the package maintainer.');

[Copilot]

The migration path is hardcoded with a specific timestamp ('2025_08_09_115521'). This creates tight coupling and makes the command fragile. If the migration file is ever renamed or regenerated with a different timestamp, this command will break.

Consider using a more flexible approach, such as:

• Using a glob pattern to find the migration file: glob(__DIR__.'/../../database/migrations/*_create_event_logs_table.php')
• Storing the migration filename in a constant or configuration

[Copilot]

After publishing migrations, the command doesn't verify that the publish operation was successful. If Artisan::call('vendor:publish', ...) fails silently, the subsequent migration command on line 59 will fail with an unclear error.

Consider checking the result of the publish operation:

$exitCode = Artisan::call('vendor:publish', [
    '--tag' => 'laravel-event-logs-migrations',
    '--force' => true,
]);

if ($exitCode !== 0) {
    $this->error('Failed to publish migrations.');
    return self::FAILURE;
}

// Verify the file was actually published
if (! File::exists($publishedMigrationPath)) {
    $this->error('Migration file was not created after publishing.');
    return self::FAILURE;
}

Suggested change

	Artisan::call('vendor:publish', [
	'--tag' => 'laravel-event-logs-migrations',
	'--force' => true,
	]);
	$exitCode = Artisan::call('vendor:publish', [
	'--tag' => 'laravel-event-logs-migrations',
	'--force' => true,
	]);
	if ($exitCode !== 0) {
	$this->error('Failed to publish migrations.');
	return self::FAILURE;
	}
	// Verify the file was actually published
	if (! File::exists($publishedMigrationPath)) {
	$this->error('Migration file was not created after publishing.');
	return self::FAILURE;
	}

[Copilot]

The migration command doesn't check for success/failure. If the migration fails, the command will still report "Event logs table created successfully" and return SUCCESS, which is misleading.

Consider checking the result:

$exitCode = Artisan::call('migrate', [
    '--database' => $connection,
    '--path' => 'database/migrations/2025_08_09_115521_create_event_logs_table.php',
]);

if ($exitCode !== 0) {
    $this->error('Migration failed.');
    return self::FAILURE;
}

$this->info('Event logs table created successfully');

Suggested change

	Artisan::call('migrate', [
	'--database' => $connection,
	'--path' => 'database/migrations/2025_08_09_115521_create_event_logs_table.php',
	]);
	$exitCode = Artisan::call('migrate', [
	'--database' => $connection,
	'--path' => 'database/migrations/2025_08_09_115521_create_event_logs_table.php',
	]);
	if ($exitCode !== 0) {
	$this->error('Migration failed.');
	return self::FAILURE;
	}

[Copilot]

The command signature includes a --force option, but it's never used in the implementation. The drop operation executes without any confirmation, regardless of whether --force is provided.

Either:

1. Remove the unused --force option from the signature
2. Implement confirmation logic that can be bypassed with --force:

if (! $this->option('force') && ! $this->confirm('Are you sure you want to drop the event_logs table?')) {
    $this->info('Operation cancelled.');
    return self::SUCCESS;
}

Suggested change

	if (! $this->option('force') && ! $this->confirm('Are you sure you want to drop the event_logs table?')) {
	$this->info('Operation cancelled.');
	return self::SUCCESS;
	}