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

Specify commands that will not be available on http calls #89

Merged
merged 1 commit into from Apr 27, 2023

Conversation

systemsolutionweb
Copy link
Contributor

@systemsolutionweb systemsolutionweb commented Mar 31, 2023

There are some commands that should not be able to be executed from the context of web application interfaces, like InstallCommand, NetworkCommand

This PR gives us the option to register commands that will exclusively be used from the console.

Also on laravel documentation there is an example https://laravel.com/docs/10.x/packages#commands (runningInConsole)

Example:

$package
    ->name('your-package-name')
    ->hasConfigFile()
    // console context command
    ->hasConsoleCommand(ConsoleExclusiveCommand::class)
    // console/web context command
    ->hasCommand(GlobalCommand::class)
    // InstallCommand must be console context command
    ->hasInstallCommand(function(InstallCommand $command) {
        $command
            ->publishMigrations();
    })

This was broken here: #23

@freekmurze
Copy link
Member

Thanks!

I'd don't see any difference in handling compared to our current method: https://github.com/spatie/laravel-package-tools/pull/89/files#diff-019f7c923ae71364e0c7e058efe9cd6c43a4955d8c5b6461b2a69e3f04979bcfR101-R108

@systemsolutionweb
Copy link
Contributor Author

@freekmurze hi, thanks for answer

I'd don't see any difference in handling compared to our current method

in that PR it was added that all the commands can be executed from the context of http calls,
with this PR we can decide if the commands should be accessible from http calls or are exclusively from the context of console calls, or divide them

Example:
Currently the InstallCommand command is registered so that it is accessible from http calls, which is unnecessary and should only be accessible from the console context

$installCommand = new InstallCommand($this);
$callable($installCommand);
$this->commands[] = $installCommand;

Here is the conditional

if ($this->app->runningInConsole()) {

I will modify the PR for better readability

@systemsolutionweb systemsolutionweb changed the title Specify commands that will not be available on web interfaces Specify commands that will not be available on http calls Apr 3, 2023
src/Package.php Outdated
@@ -163,9 +165,9 @@ public function hasMigrations(...$migrationFileNames): static
return $this;
}

public function hasCommand(string $commandClassName): static
public function hasCommand(string $commandClassName, bool $justConsole = false): static
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this flag being used somewhere? Seems like the dedicated hasConsoleCommand already covers this functionality. Could you remove the flag?

@freekmurze
Copy link
Member

Seems like the tests are failing, could you have a look

@freekmurze freekmurze merged commit efab184 into spatie:main Apr 27, 2023
21 checks passed
@freekmurze
Copy link
Member

Thanks!

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.

None yet

2 participants