-
Notifications
You must be signed in to change notification settings - Fork 1
[55] additional admins migrations #57
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.
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
base: main
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 17669561026Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for generating additional admin migrations when setting up Laravel projects with Telescope and Nova admin panels. It handles different authentication types (none vs Clerk) and creates appropriate migration files for additional admin users.
- Adds logic to track and create admin migrations based on authentication type
- Implements separate handling for users table vs admins table depending on auth configuration
- Updates test fixtures to validate the new admin migration functionality
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/Commands/InitCommand.php | Adds core logic for publishing additional admin migrations with auth type handling |
resources/views/users_add_additional_admin.blade.php | Template for adding admin users to users table |
resources/views/admins_create_or_add_additional_admin.blade.php | Template for creating/adding to admins table |
tests/InitCommandTest.php | Updates tests to validate new admin migration behavior |
tests/fixtures/InitCommandTest/*.php | Test fixture files for various admin migration scenarios |
Comments suppressed due to low confidence (1)
resources/views/admins_create_or_add_additional_admin.blade.php:1
- The down() method attempts to delete a specific admin record after dropping the entire admins table. This will fail because the table no longer exists. The deletion should happen before dropping the table, or the table drop should be conditional on whether this migration created it.
use Illuminate\Database\Migrations\Migration;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/Commands/InitCommand.php
Outdated
$defaultPassword = substr(md5(uniqid()), 0, 8); | ||
|
||
$email = $this->ask("Please enter a {$title}'s admin email", "admin@{$kebabName}.com"); | ||
$email = $this->ask("Please enter a {$title}'s admin email", "{$key}_admin@{$kebabName}.com"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$email = $this->ask("Please enter a {$title}'s admin email", "{$key}_admin@{$kebabName}.com"); | |
$email = $this->ask("Please enter a {$title}'s admin email", "admin.{$key}@{$kebabName}.com"); |
src/Commands/InitCommand.php
Outdated
$email = $this->ask("Please enter a {$title}'s admin email", "{$key}_admin@{$kebabName}.com"); | ||
$password = $this->ask("Please enter a {$title}'s admin password", $defaultPassword); | ||
|
||
$this->publishAdditionalAdminMigration($title, $key, $email, $password); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we use createAdminUser
method instead of all else block? It looks it make the same but need to add more arguments to it
src/Commands/InitCommand.php
Outdated
'email' => $adminEmail, | ||
'password' => $adminPassword, | ||
'credentialName' => $credentialName, | ||
'isAdminsCreated' => $this->isAdminsTableCreated, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may we just check that the $adminCredentials
property is not empty instead of storing isAdminsTableCreated
field?
use Illuminate\Database\Schema\Blueprint; | ||
@endif | ||
|
||
class Add{{ ucfirst($credentialName) }}Admin extends Migration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class Add{{ ucfirst($credentialName) }}Admin extends Migration | |
class Add{{ ucfirst($serviceName) }}Admin extends Migration |
|
||
public function up(): void | ||
{ | ||
@if (!$isAdminsCreated) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@if (!$isAdminsCreated) | |
@if (!$needToCreateTable) |
public function up(): void | ||
{ | ||
@if (!$isAdminsCreated) | ||
Schema::create('admins', function (Blueprint $table) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that we need to publish already exists migration admins_create_table.blade.php
in the base command before publish this migration
…or into 55-admins-migrations
refs: #55
} | ||
|
||
protected function createAdminUser(string $kebabName): void | ||
protected function createAdminUser(string $kebabName, string $serviceName = '', string $adminPrefix = 'an'): array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
protected function createAdminUser(string $kebabName, string $serviceName = '', string $adminPrefix = 'an'): array | |
protected function createAdminUser(string $kebabName, string $serviceKey = '', string $serviceName = ''): array |
protected function createAdminUser(string $kebabName): void | ||
protected function createAdminUser(string $kebabName, string $serviceName = '', string $adminPrefix = 'an'): array | ||
{ | ||
$adminEmail = $serviceName ? "admin.{$serviceName}@{$kebabName}.com" : "admin@{$kebabName}.com"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$adminEmail = $serviceName ? "admin.{$serviceName}@{$kebabName}.com" : "admin@{$kebabName}.com"; | |
$adminEmail = when($serviceName, "admin.{$serviceName}@{$kebabName}.com", "admin@{$kebabName}.com"); |
'email' => $this->ask('Please enter an admin email', "admin@{$kebabName}.com"), | ||
'password' => $this->ask('Please enter an admin password', $defaultPassword), | ||
$adminCredentials = [ | ||
'email' => $this->ask("Please enter {$adminPrefix} admin email", $adminEmail), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'email' => $this->ask("Please enter {$adminPrefix} admin email", $adminEmail), | |
'email' => $this->ask("Please enter admin email for {$serviceName}", $adminEmail), |
if (!empty($this->adminCredentials) && $this->confirm("Is {$title}'s admin the same as default one?", true)) { | ||
$email = $this->adminCredentials['email']; | ||
$password = $this->adminCredentials['password']; | ||
if (isset($this->adminCredentials['email']) && $this->confirm("Is {$title}'s admin the same as default one?", true)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (isset($this->adminCredentials['email']) && $this->confirm("Is {$title}'s admin the same as default one?", true)) { | |
if (!empty($this->adminCredentials) && $this->confirm("Is {$title}'s admin the same as default one?", true)) { |
$adminCredentials['email'] = $this->adminCredentials['email']; | ||
$adminCredentials['password'] = $this->adminCredentials['password']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$adminCredentials['email'] = $this->adminCredentials['email']; | |
$adminCredentials['password'] = $this->adminCredentials['password']; | |
$adminCredentials = $this->adminCredentials; |
|
||
$adminCredentials['migrationName'] = Str::studly($migrationName); | ||
} else { | ||
$viewName = $migrationName = 'admins_create_table'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure that such view is exists. Anyway, let's pusblish migration for the admins table creation outside of this method just after we decided that need to create user with the cleark auth
No description provided.