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

153/maintenance mode #174

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .lando.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ config:
php: 7.4
webroot: backdrop
database: mariadb
# Although Drush isn't needed/used, the version that Lando includes by default
# has a bug that causes all build steps to fail. We therefore need to update
# Drush to the latest version just to make sure that build steps run properly.
Comment on lines -7 to -9
Copy link

Choose a reason for hiding this comment

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

I'd like to keep this comment for now, since it explains why we need to specify the Drush version. It's because Lando defaults to 1.4.0 (https://github.com/lando/backdrop/blob/505b65ebf113887a4d341f252a543c5d8c4ca83d/recipes/backdrop/builder.js#L48) but that version doesn't include this necessary change: backdrop-contrib/backdrop-drush-extension#232 (need a new release for that).

backdrush: 1.x-1.x
backdrush: '1.x-1.x'

services:
appserver:
build:
Expand Down
115 changes: 115 additions & 0 deletions commands/state.bee.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
<?php
/**
* @file
* Command(s) for getting and setting backdrop states.
Copy link

Choose a reason for hiding this comment

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

Suggested change
* Command(s) for getting and setting backdrop states.
* Command(s) for getting and setting Backdrop states.

*/

/**
* Implements hook_bee_command().
*/
function state_bee_command() {
return [
Copy link

Choose a reason for hiding this comment

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

Backdrop hasn't yet changed to using the short array syntax, and neither has Bee. So for consistency with the rest of the code we should use the longer syntax.

If we really want to change to the short syntax, we need a new issue to address this throughout all of Bee.

'state-get' => [
'description' => bt('Get the value of a Backdrop state.'),
'callback' => 'state_get_bee_callback',
'arguments' => [
'state' => bt('The name of the state to get.'),
Copy link

Choose a reason for hiding this comment

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

I don't know much about states, but is it worth making this optional so you can run bee state-get to see a list of all states and their values...? (similar to bee config-get)

Looks like state_initialize() would do it.

Copy link

Choose a reason for hiding this comment

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

Thought about this some more, and this can be a followup issue if there's interest. So happy to leave this as-is for now.

],
'aliases' => ['sg, sget'],
Copy link

Choose a reason for hiding this comment

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

Suggested change
'aliases' => ['sg, sget'],
'aliases' => ['sg', 'sget'],

'bootstrap' => BEE_BOOTSTRAP_FULL,
'examples' => [
'bee state-get maintanance_mode' => bt('Get the value of the Backdrop state named "maintanance_mode".'),
Copy link

Choose a reason for hiding this comment

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

Let's match the wording of the config-get example:

'bee config-get system.core site_name' => bt("Get the value of the 'site_name' config option."),

Suggested change
'bee state-get maintanance_mode' => bt('Get the value of the Backdrop state named "maintanance_mode".'),
'bee state-get maintanance_mode' => bt("Get the value of the 'maintenance_mode' state."),

],
],
'state-set' => [
'description' => bt('Set the value of a Backdrop state.'),
'callback' => 'state_set_bee_callback',
'arguments' => [
'state' => bt('The name of the state to set.'),
'value' => bt('The value to set the state to.'),
],
'aliases' => ['ss, sset'],
Copy link

Choose a reason for hiding this comment

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

Suggested change
'aliases' => ['ss, sset'],
'aliases' => ['ss', 'sset'],

'bootstrap' => BEE_BOOTSTRAP_FULL,
'examples' => [
'bee state-set maintanance_mode 1' => bt('Set the value of the Backdrop state named "maintanance_mode" to 1.'),
Copy link

Choose a reason for hiding this comment

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

Let's explain what this actually does (similar to

'bee config-set image.style.thumbnail effects.0.data.width 200' => bt('Change the width of the Thumbnail image style.'),
).

Suggested change
'bee state-set maintanance_mode 1' => bt('Set the value of the Backdrop state named "maintanance_mode" to 1.'),
'bee state-set maintanance_mode 1' => bt('Enable maintenance mode for the site.'),

],
],
'maintenance-mode' => [
'description' => bt('Set the value of maintenance_mode for Backdrop.'),
'callback' => 'maintenance_mode_bee_callback',
'arguments' => [
'value' => bt('The value to set maintenance_mode to.'),
Copy link

Choose a reason for hiding this comment

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

Maybe make this optional so you can get the value as well...? E.g. bee maintenance-mode If so, also update the description and examples appropriately.

],
'aliases' => ['mm',],
Copy link

Choose a reason for hiding this comment

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

Suggested change
'aliases' => ['mm',],
'aliases' => ['mm'],

'bootstrap' => BEE_BOOTSTRAP_FULL,
'examples' => [
'bee maintanance-mode 1' => bt('Set the value of the Backdrop maintenance_mode to 1.'),
Copy link

Choose a reason for hiding this comment

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

Just to be different from the above example...

Suggested change
'bee maintanance-mode 1' => bt('Set the value of the Backdrop maintenance_mode to 1.'),
'bee maintanance-mode 0' => bt('Disable maintenance mode for the site.'),

],
],
];
}

/**
* Command callback: Get the value of a Backdrop state.
*/
function state_get_bee_callback($arguments, $options) {
try {
$value = state_get($arguments['state']);
$msg = bt('The value of the Backdrop state named "@state" is "@value".', [
'@state' => $arguments['state'],
'@value' => $value,
Comment on lines +58 to +60
Copy link

Choose a reason for hiding this comment

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

Suggested change
$msg = bt('The value of the Backdrop state named "@state" is "@value".', [
'@state' => $arguments['state'],
'@value' => $value,
$msg = bt("The value of the '!state' state is: !value", [
'!state' => $arguments['state'],
'!value' => $value,

Copy link

Choose a reason for hiding this comment

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

Actually I'm now thinking this should be displayed via the return value, rather than a message... See https://github.com/backdrop-contrib/bee/blob/1.x-1.x/API.md#command_bee_callback for details and bee config-get for an example (though I don't mind outputting something like the text above as opposed to just the value itself).

]);
bee_message($msg, 'status');
}
catch(ParseError $e) {
// This is more readable than the default error we would get from PHP.
$err_msg = bt('!msg in: !state', array(
'!msg' => $e->getMessage(),
'!state' => $arguments['state'],
));
bee_message($err_msg, 'error');
}
}

/**
* Command callback: Set the value of a Backdrop state.
*/
function state_set_bee_callback($arguments, $options) {
try {
state_set($arguments['state'], (int) $arguments['value']);
Copy link

Choose a reason for hiding this comment

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

Why is the value being cast to an integer here? That doesn't seem right or necessary... https://docs.backdropcms.org/api/backdrop/core%21includes%21bootstrap.inc/function/state_set/1

$msg = bt('The value of the Backdrop state named "@state" is "@value".', [
'@state' => $arguments['state'],
'@value' => $arguments['value'],
Comment on lines +80 to +82
Copy link

Choose a reason for hiding this comment

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

Suggested change
$msg = bt('The value of the Backdrop state named "@state" is "@value".', [
'@state' => $arguments['state'],
'@value' => $arguments['value'],
$msg = bt("The '!state' state was set to: !value", [
'!state' => $arguments['state'],
'!value' => $arguments['value'],

]);
bee_message($msg, 'success');
}
catch(ParseError $e) {
// This is more readable than the default error we would get from PHP.
$err_msg = bt('!msg in: !state', array(
'!msg' => $e->getMessage(),
'!state' => $arguments['state'],
));
bee_message($err_msg, 'error');
}
}

/**
* Command callback: Set the value of a maintenance_mode for Backdrop.
Copy link

Choose a reason for hiding this comment

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

Suggested change
* Command callback: Set the value of a maintenance_mode for Backdrop.
* Command callback: Set the value of maintenance_mode for Backdrop.

*/
function maintenance_mode_bee_callback($arguments, $options) {
try {
state_set('maintenance_mode', (int) $arguments['value']);
$msg = bt('The value of the Backdrop maintenance_mode is "@value".', [
'@value' => $arguments['value'],
Comment on lines +102 to +103
Copy link

Choose a reason for hiding this comment

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

Suggested change
$msg = bt('The value of the Backdrop maintenance_mode is "@value".', [
'@value' => $arguments['value'],
$msg = bt("The 'maintenance_mode' state was set to: !value", [
'!value' => $arguments['value'],

]);
bee_message($msg, 'success');
}
catch(ParseError $e) {
// This is more readable than the default error we would get from PHP.
$err_msg = bt('!msg in: !value', array(
'!msg' => $e->getMessage(),
'!value' => $arguments['value'],
));
bee_message($err_msg, 'error');
}
}
43 changes: 43 additions & 0 deletions tests/backdrop/StateCommandsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/**
* @file
* PHPUnit tests for Bee State commands.
*/

use PHPUnit\Framework\TestCase;

class StateCommandsTest extends TestCase {

/**
* Make sure that the state-get command works.
*/
public function test_state_get_command_works() {
$output_all = shell_exec('bee state-get maintenance_mode');
$this->assertStringContainsString('0', $output_all);
$this->assertStringContainsString('maintenance_mode', $output_all);
Comment on lines +15 to +17
Copy link

Choose a reason for hiding this comment

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

Suggested change
$output_all = shell_exec('bee state-get maintenance_mode');
$this->assertStringContainsString('0', $output_all);
$this->assertStringContainsString('maintenance_mode', $output_all);
$output = shell_exec('bee state-get maintenance_mode');
$this->assertStringContainsString("The value of the 'maintenance_mode' state is: 0", $output);

}

/**
* Make sure that the state-set command works.
*/
public function test_state_set_command_works() {
$output = shell_exec('bee state-get maintenance_mode');
$this->assertStringContainsString('maintenance_mode', $output);
$this->assertStringContainsString('0', $output);

$output = shell_exec('bee state-set maintenance_mode 1');
$this->assertStringContainsString('1', $output);
Comment on lines +24 to +29
Copy link

Choose a reason for hiding this comment

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

Suggested change
$output = shell_exec('bee state-get maintenance_mode');
$this->assertStringContainsString('maintenance_mode', $output);
$this->assertStringContainsString('0', $output);
$output = shell_exec('bee state-set maintenance_mode 1');
$this->assertStringContainsString('1', $output);
// Make sure maintenance_mode is '0'.
$output = shell_exec('bee state-get maintenance_mode');
$this->assertStringContainsString("The value of the 'maintenance_mode' state is: 0", $output);
// Set maintenance_mode to '1'.
$output = shell_exec('bee state-set maintenance_mode 1');
$this->assertStringContainsString("The 'maintenance_mode' state was set to: 1", $output);
// Make sure maintenance_mode is '1'.
$output = shell_exec('bee state-get maintenance_mode');
$this->assertStringContainsString("The value of the 'maintenance_mode' state is: 1", $output);

}

/**
Copy link

Choose a reason for hiding this comment

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

Suggested change
/**
/**

* Make sure that the maintenance-mode command works.
*/
public function test_maintenance_mode_command_works() {
$output = shell_exec('bee maintenance_mode 1');
$this->assertStringContainsString('maintenance_mode', $output);
$this->assertStringContainsString('1', $output);
Comment on lines +37 to +38
Copy link

Choose a reason for hiding this comment

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

Suggested change
$this->assertStringContainsString('maintenance_mode', $output);
$this->assertStringContainsString('1', $output);
$this->assertStringContainsString("The 'maintenance_mode' state was set to: 1", $output);


$output = shell_exec('bee state-set maintenance_mode 0');
$this->assertStringContainsString('0', $output);
Copy link

Choose a reason for hiding this comment

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

Suggested change
$this->assertStringContainsString('0', $output);
$this->assertStringContainsString("The 'maintenance_mode' state was set to: 0", $output);

}
}
1 change: 1 addition & 0 deletions tests/phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<file>backdrop/BeeCoreTest.php</file>
<file>backdrop/CacheCommandsTest.php</file>
<file>backdrop/ConfigCommandsTest.php</file>
<file>backdrop/StateCommandsTest.php</file>
Copy link

Choose a reason for hiding this comment

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

The rest of the tests are listed alphabetically. Can we move this one down appropriately?

<file>backdrop/CronCommandsTest.php</file>
<file>backdrop/DBCommandsTest.php</file>
<file>backdrop/DBLogCommandsTest.php</file>
Expand Down