-
-
Notifications
You must be signed in to change notification settings - Fork 219
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
[FIX] Refactoring powergrid:create command #1404
Merged
luanfreitasdev
merged 25 commits into
Power-Components:5.x
from
dansysanalyst:improve_create_command
Feb 10, 2024
Merged
[FIX] Refactoring powergrid:create command #1404
luanfreitasdev
merged 25 commits into
Power-Components:5.x
from
dansysanalyst:improve_create_command
Feb 10, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
merge com functions no passado
@luanfreitasdev please review it thoroughly! |
…into improve_create_command
Thank you @dansysanalyst. This looks amazing! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
⚡ PowerGrid - Pull Request
Motivation
Description
This Pull Request aims to organize the code structure in everything related to creating a new PowerGrid component, while also resolving the issue mentioned below.
Main Interface Changes
Laravel Prompt Windows Fallback
There is no support for Laravel Prompt in Windows (read more here). I have tried to adapt the prompts to be usable when it goes into "asking question" mode. We might have to put more work to ensure a good user experience in the future. I propose to monitor issues on this topic.
Check for Component Existence
Until now, this check was happening at the end of the wizard, after the user has answered all asked questions. With this PR, this check happens right after the user informed a component name, and it defaults to "no".
Dependencies Notification
I have moved the notification check into a separated command
powergrid:check-dependencies
.I rewrote the notification messages to bring more clarity.
In addition, I have also added a "confirmation" step to make sure the user doesn't miss this message. While this might be inconvenient, specially in a prompt fallback situation, we can easily remove this step in
src/Commands/CheckDependenciesCommand.php
.Installation Wizard
I have provided some new texts and removed tables such as passwords, tokens, migrations from the table listing.
In addition, I improved the parsing of different ways to receive component names for a nested folder structure, such like "tables.admins.users.ListTable" or "\Tables\Admins\Users\ListTable".
In this PR, I have also modified the order questions were asked. I think it makes sense to for the table name in the same step we ask for models (when it applies).
To follow the new nomenclature, the wizard asks to import "fields" and not columns anymore.
To finalize, instead of throwing exceptions, some prompts run on a loop.
When asked for models, if the given model doesn't exist the setup will return to the same question, avoiding having the user starting the wizard all over again.
Same for tables:
Before suggesting importing database table columns as fields, the wizard is now checking if there is a database connection, if the database has tables. If both fail, the setup will assume the given table and continue creating the component. When there are tables, it will suggest tables and validate that the given table exists.
While I think this scenario is less common, this PR allows the users to create components before running migrations.
Main Structural changes
.fillable
) at the end of the filename, so we can decide when to load each type.src/Support/PowerGridStub.php
).src/Enums/Datasource.php
. It may be a bit inflated with methods, but I think it is just simple to go this way.src/Support/PowerGridComponentMaker.php
)TO DO
❗
The Laravel Prompt tests are failing, I assume, due to a fallback to Symphony Question Helper. I need to dig deeper into testing prompts.❗
As mentioned above, we have to keep a look at Windows user's experience when creating a component.Related Issue(s):
#1394
Documentation
This PR requires Documentation update?