Remove system.authorize config

Rediscovered this via #2591827: Add YAML linting to core coding standards checks where we detected that system.authorize.yml is invalid.

There is also now a migration that writes config to this file, but the config is never used at runtime.

++ to remove and check 2 usages in contrib (both used to exclude it) so removal without deprecation looks straight

http://grep.xnddx.ru/search?text=system.authorize

+++ b/core/modules/system/system.post_update.php
@@ -189,3 +189,10 @@ function system_post_update_remove_key_value_expire_all_index() {
+function system_post_update_delete_authorize_settings() {
+  \Drupal::configFactory()->getEditable('system.authorize')->delete();

I bet it needs upgrade test

Filed CR https://www.drupal.org/node/3206320 and here's a test

Fix c/p typo

Thanks for adding the test and CR, the test looks good to me.

The only thing I'm not sure about is whether we can remove the migration directly or if we have to deprecate it first somehow? Marking RTBC to get core committer feedback on this.

We should change this comment
Remove obsolete system.authorize configuration.

to
Removes obsolete system.authorize configuration.

Because this same standard is used in other comments. E.g.

Tests system_post_update_delete_authorize_settings().

Just to be consistent with the same way on this file

I am not sure that is always the case in update hook docblocks, which are displayed to the user during update.php - we are a bit inconsistent:

core/modules/system/system.post_update.php: * Remove backwards-compatibility leftovers from entity type definitions.
core/modules/system/system.post_update.php: * Remove obsolete system.rss configuration.
core/modules/serialization/serialization.post_update.php: * Remove obsolete serialization.settings configuration.
core/modules/hal/hal.post_update.php: * Remove obsolete hal.settings configuration key.
core/modules/workspaces/workspaces.post_update.php: * Removes the workspace association entity and field schema data.
core/modules/action/action.post_update.php: * Removes action settings.
core/modules/rest/rest.post_update.php: * Remove obsolete rest.settings configuration.

Makes sense.

Ok, it was just it from my side.

Moving to RTBC

Makes sense

On the same patch, we're including another document using a different way

/**
 * Tests the upgrade path for removal the system.authorize configuration.
 *

Should we use the same standard to be consistent? E.g.:

/**
 * Test the upgrade path for removal the system.authorize configuration.
 *

@RenatoG no, only update function doc blocks are without-s other functions and classes has common rule

Ok makes sense.
So +1 to it

Please do not ask the testbot to try again until #3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest is fixed.

Hit by #3208791: [random test failure] Random fail in LayoutBuilderDisableInteractionsTest, back to RTBC.

+++ b/core/modules/system/config/schema/system.schema.yml
@@ -53,14 +53,6 @@ system.maintenance:
   type: config_object
   label: 'Cron settings'
diff --git a/core/modules/system/migrations/d7_system_authorize.yml b/core/modules/system/migrations/d7_system_authorize.yml

diff --git a/core/modules/system/migrations/d7_system_authorize.yml b/core/modules/system/migrations/d7_system_authorize.yml
deleted file mode 100644

deleted file mode 100644
index d3d83881a2..0000000000

index d3d83881a2..0000000000
--- a/core/modules/system/migrations/d7_system_authorize.yml

--- a/core/modules/system/migrations/d7_system_authorize.yml
+++ /dev/null

+++ /dev/null
+++ /dev/null
@@ -1,15 +0,0 @@

@@ -1,15 +0,0 @@
-id: d7_system_authorize
-label: Drupal 7 file transfer authorize configuration
-migration_tags:
-  - Drupal 7
-  - Configuration
-source:
-  plugin: variable
-  variables:
-    - authorize_filetransfer_default
-  source_module: system
-process:

Can we just remove this or do we somehow need to deprecate it? Not sure what the implications are for the various migration runners if this file disappears.

I asked that in #16, I don't know the answer either!

#3039240: Create a way to declare a plugin as deprecated IS suggests we do need to deprecate it, but we can't yet because of that issue

Let's follow #3022401: Remove useless config action.settings.recursion_limit and keep the migration but make it do nothing, so we don't have to postpone on plugin deprecations.

Good idea, thanks! changed to suggestion from #32

btw in #3039240: Create a way to declare a plugin as deprecated deprecated migrations are allowed to be skipped so better to improve deprecation when it lands for both (actions and migrations)

Do we need a test similar to MigrateActionConfigsTest to prove that the updated migration doesn't write any config?

@longwave Thank you!

Reverted execution of migration and extra assert added

Great work! RTBC if bot agrees.

+++ b/core/modules/system/tests/src/Kernel/Migrate/d7/MigrateSystemConfigurationTest.php
@@ -166,6 +164,7 @@ public function testConfigurationMigration() {
+    $this->assertTrue(\Drupal::config('system.authorize')->isNew(), 'Migration d7_system_authorize creating no config.');

I thought that assertions weren't to have messages, #3131946: [policy] Remove PHPUnit assertion messages when possible, and standardize remaining messages . This could be a comment.

Can we just remove this or do we somehow need to deprecate it? Not sure what the implications are for the various migration runners if this file disappears.

In my experience, there are none and in this case the migration can be deleted. If the migration id does not exist it is simply ignored. For example, drush (migrate run) typing 'drush mim 'migrationwithtypo' returns nothing, which can be annoying. But this also allows migration lookup to be very flexible in the list of migration. See 3143486#7. But to be sure let's get another opinion from a migrate maintainer.

Fixed the comment in the test.

I'm a lapsed Migrate maintainer, but from my cursory reading of https://git.drupalcode.org/project/migrate_tools/-/blob/8.x-5.x/src/Comm..., it appears to gracefully handle missing migration IDs. I haven't personally tested that, though,

The current patch just converts the migration to do nothing, so the issue of whether or not deleted migration plugins cause any issues is not a problem. The test comment was fixed and the test improved, so back to RTBC,