Skip to content
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

Update the make:publication command #786

Merged
merged 36 commits into from
Dec 29, 2022

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Dec 28, 2022

Refactors the make:publication command to fix bugs and provide a consistent console UI layer.

Fixes:

  • Don't ask users to fill in meta fields
  • Fix bug causing booleans to not be fillable (presumably as writing true to STDIN is the same as writing 'true')
  • Indent empty collection warnings
  • Normalize output styles for messages to enter array and text input
  • Add message when skipping a field?
  • Test that fields that have a required rule cannot be skipped
  • Unset skipped values when generating front matter?
  • Check if we should use $field->getValidationRules() instead of $field->type->rules() as it seems we might be missing checking custom rules
  • When using php hyde make:publication foo and there are no publications at all, the current message "Error: Unable to locate any publication types. Did you create any?" might not be entirely relevant.
  • Fix integers are serialized as strings
  • Make sure that the proper type is selected when asked

@codecov
Copy link

codecov bot commented Dec 28, 2022

Codecov Report

Merging #786 (b8588ab) into publications-feature (f9f30f7) will decrease coverage by 0.11%.
The diff coverage is n/a.

@@                    Coverage Diff                     @@
##             publications-feature     #786      +/-   ##
==========================================================
- Coverage                  100.00%   99.88%   -0.12%     
+ Complexity                   2796     1406    -1390     
==========================================================
  Files                         324      162     -162     
  Lines                        7028     3541    -3487     
==========================================================
- Hits                         7028     3537    -3491     
- Misses                          0        4       +4     
Impacted Files Coverage Δ
...rk/src/Console/Commands/MakePublicationCommand.php 95.95% <0.00%> (-4.05%) ⬇️
...rc/Framework/Actions/CreatesNewPublicationPage.php 100.00% <0.00%> (ø)
...rc/Console/Commands/Helpers/InputStreamHandler.php
...rk/src/Console/Commands/MakePublicationCommand.php
...rc/Framework/Actions/CreatesNewPublicationPage.php
.../src/Console/Commands/RebuildStaticSiteCommand.php
...rk/Features/Publications/PublicationFieldTypes.php
...ework/src/Framework/Services/ValidationService.php
...kages/framework/src/Foundation/RouteCollection.php
...amework/src/Console/Concerns/ValidatingCommand.php
... and 154 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@caendesilva caendesilva changed the title Publications feature refactors Refactor the MakePublicationCommand class Dec 28, 2022
@caendesilva caendesilva changed the title Refactor the MakePublicationCommand class Refactor the make:publication command Dec 28, 2022
@caendesilva caendesilva changed the title Refactor the make:publication command Update the make:publication command Dec 28, 2022
@caendesilva
Copy link
Member Author

caendesilva commented Dec 28, 2022

Fix bug causing booleans to not be fillable (presumably as writing true to STDIN is the same as writing 'true')

According to the docs (https://laravel.com/docs/9.x/validation#rule-boolean)

"The field under validation must be able to be cast as a boolean. Accepted input are true, false, 1, 0, "1", and "0"."

Just extracting bools to a helper method that casts to bool changes the generated YAML from name: '1' to name: true f4a5887

Not sure how to allow strings without losing custom validation rules and without too much added code

Edit: Think I figured it out b3303aa but we might want to extract that to a custom validation rule that we can swap out the standard one for

Resources: https://github.com/illuminate/validation/blob/3f63f1046f67377a64779baaa86d7f1997b5f748/Concerns/ValidatesAttributes.php#L448-L453
https://laravel.com/docs/9.x/validation#rule-boolean

@caendesilva caendesilva force-pushed the publications-feature-refactors branch from 20e5e0d to fe7d71f Compare December 28, 2022 20:42
@caendesilva caendesilva force-pushed the publications-feature-refactors branch from 5c68b60 to 1d712b3 Compare December 29, 2022 16:33
@caendesilva caendesilva marked this pull request as ready for review December 29, 2022 19:40
@caendesilva caendesilva merged commit 2caf1f4 into publications-feature Dec 29, 2022
@caendesilva caendesilva deleted the publications-feature-refactors branch December 29, 2022 19:40
@caendesilva
Copy link
Member Author

Some things left to do, moving todos to #685

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants