Created from #1775842: [meta] Convert all variables to state and/or config systems.

The following variable/file pairs need to be converted:

allow_authorize_operations authorize.php
allow_authorize_operations update.module
authorize_filetransfer_connection_settings_ . $backend authorize.inc
authorize_filetransfer_default authorize.inc

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Issue tags: +Configuration system

adding tag.

8thom’s picture

Assigned: Unassigned » 8thom
FileSize
3.46 KB

Variable references converted

8thom’s picture

Status: Active » Needs review
aspilicious’s picture

Status: Needs review » Needs work
+++ b/core/includes/authorize.incundefined
@@ -40,7 +40,7 @@ function authorize_filetransfer_form($form, &$form_state) {
-  elseif ($authorize_filetransfer_default = variable_get('authorize_filetransfer_default', NULL));

authorize_filetransfer_default isn't listed in the config file, is there a way to list it anyway, for discoveribility reasons.

+++ b/core/modules/system/config/system.authorize.ymlundefined
@@ -0,0 +1 @@
+allow_authorize_operations: TRUE

All config items are stored as strings. In most cases we choose '1' as true.

alexpott’s picture

Yep we should list authorize_filetransfer_default in the yaml file if possible... and we can make the config names simple now they are in the system.authorise namespace.

  • authorize_filetransfer_default can become filetransfer_default
  • allow_authorize_operations can become allow_operations

Plus we need an update function to migrate the variables to config during a D7 to D8 upgrade see system_update_8009() as an example.

8thom’s picture

Status: Needs work » Needs review
FileSize
4.73 KB
  1. Added authorize_filetransfer_default to the config
  2. Updated default of allow_authorize_operations to 1
  3. Changed config names to filetransfer_default & allow_operations
  4. Added a new update function system_update_9000 to migrate variables during a D8 upgrade

Hope the numbering is ok for the update hook, does this get reassigned before the final commit?

aspilicious’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/system.installundefined
@@ -2179,3 +2179,15 @@ function system_update_8029() {
+function system_update_9000() {

You're making a D7 to D8 upgrade function so the name of the upgrade function must be 8030 (if it isn't increased in the meantime)

8thom’s picture

Status: Needs work » Needs review
FileSize
4.63 KB

Thanks aspilicious.

Updated system_update function to system_update_8030

Status: Needs review » Needs work

The last submitted patch, authorize_variables_to_config-1804346-8.patch, failed testing.

8thom’s picture

Status: Needs work » Needs review
FileSize
4.78 KB

fixed patch

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me. I hope I didn't missed something.

webchick’s picture

Status: Reviewed & tested by the community » Fixed
+++ b/core/includes/authorize.inc
--- /dev/null
+++ b/core/modules/system/config/system.authorize.yml

+++ b/core/modules/system/config/system.authorize.yml
@@ -0,0 +1,2 @@
+filetransfer_default:

Huh, interesting. Didn't know you could do that as an equivalent of NULL.

Committed and pushed to 8.x. Thanks! So great to see these conversion issues getting picked off, especially by new core contributors! :)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

8thom’s picture

Status: Closed (fixed) » Fixed

Automatically closed -- issue fixed for 2 weeks with no activity.