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

[Feature]: Add force flag in commands #3

Open
txsoura opened this issue Sep 2, 2022 · 3 comments
Open

[Feature]: Add force flag in commands #3

txsoura opened this issue Sep 2, 2022 · 3 comments
Assignees
Labels
feature New feature or request good first issue Good for newcomers

Comments

@txsoura
Copy link
Member

txsoura commented Sep 2, 2022

🚀 Feature Proposal

Commands that affect saved data, should be ignored in the production environment if it has no force flag.

The run/revert migration commands and database wipe command, should not be easily run in production, these commands could be found at the above paths of the project:

Motivation

To prevent production data lost.

Example

If NODE_ENV=production, the above command should throw an exception with an explanation to the developer that he needs to use --force flag to run this command in production:

node artisan migration:run 

If NODE_ENV=production, the above command should work as expected:

node artisan migration:run --force
@txsoura txsoura added the feature New feature or request label Sep 2, 2022
@RobsonTrasel
Copy link

RobsonTrasel commented Dec 15, 2022

Guys, there is a library called "argparse" with which you can implement the --force flag, for example by putting it in a conditional.
Example code:

const argparse = require('argparse');

const parser = new argparse.ArgumentParser({
addHelp: true,
description: 'My command line tool'
});

parser.addArgument(['-f', '--force'], {
help: 'Force the operation to be performed',
action: 'storeTrue'
});

const args = parser.parseArgs();

if (args.force) {
// Perform the operation with force
} else {
// Perform the operation without force
}

As soon as I'm available, I'll fork and contribute to this issue.

@jlenon7
Copy link
Member

jlenon7 commented Dec 15, 2022

Hey @RobsonTrasel nice to have you here. Actually, we are using the commander npm library to handle arguments and options for the Artisan commands.

How do I define an Artisan option?

You can use the addFlags method in Artisan commands classes to call all the commander instance methods. Let's use the DbWipe command class as an example:

/**
 * Set additional flags in the commander instance.
 * This method is executed when registering your command.
 *
 * @param {import('commander').Command} commander
 * @return {import('commander').Command}
 */
addFlags(commander) {
  return commander.option(
    '-c, --connection <connection>',
    'Set the the database connection.',
    'default',
  )
}

This class already got the addFlags method implemented to add the -c, --connection option. The first argument of the commander option method is the signature of your options (-c, --connection <connection>), the second argument is the description of your option (Set the the database connection.) and the third argument is the default value of the connection option if it is not defined (default).

How do I get the values of my options?

Let's add the force option in the addFlags method of DbWipe first:

/**
 * Set additional flags in the commander instance.
 * This method is executed when registering your command.
 *
 * @param {import('commander').Command} commander
 * @return {import('commander').Command}
 */
addFlags(commander) {
  return commander
    .option('-f, --force', 'Force the command to run in production.', false)
    .option('-c, --connection <connection>', 'Set the the database connection.', 'default')
}

Now in the handle method we need to verify if the force option is false and if the NODE_ENV environment variable is equals to production. If all the statements are true, we need to throw a customized exception (You can create this exception copying some that is inside src/Exceptions folder):

/**
 * Execute the console command.
 *
 * @param {any} options
 * @return {Promise<void>}
 */
async handle(options) {
  this.title(`WIPING DATABASE\n`, 'bold', 'green')

  if (Env('NODE_ENV') === 'production' && !options.force) {
    // You can create whatever the name you want for this exception.
    throw new ProductionProtectionException()
  }

  const DB = await Database.connection(options.connection).connect()

  const dbName = await DB.getCurrentDatabase()

  await DB.revertMigrations()
  await DB.dropTable(Config.get('database.migrations'))

  await DB.close()

  this.success(`Database ({yellow} "${dbName}") successfully wiped.`)
}

@RobsonTrasel
Copy link

Good, I'll study more about this package, I liked the project so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers
Projects
Status: Backlog
Development

No branches or pull requests

4 participants