🇯🇵Japan @ptmkenny

Hmmm... I don't see why this is causing phpunit to fail on 11.1. I'll have to come back to this later, but if anyone sees a solution, feel free to jump in!

ptmkenny → created an issue.

Hmm, for 11.2, the requirements check fails on the status report, as this text is still shown:

Some modules have database schema updates to install. You should run the database update script immediately.

ptmkenny → created an issue.

@berdir: Thanks! The CR is much more clear now about the benefits.

Comment indicating this should have no BC issues (and probably not do much of anything?): https://www.drupal.org/project/drupal/issues/3495943#comment-16107488 📌 Handle module preprocess functions Active

Actually, this can be done in a backwards-compatible way:

https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/src/Entity/...

@rajeshreeputra Thanks for the detailed explanation. I didn't know about the Slack channel and added some comments there. Since Slack isn't searchable from the web, I think it would be good to cut-and-paste any important conversation threads here.

ptmkenny → created an issue.

This needs to be fixed in Encrypt first.

ptmkenny → created an issue.

ptmkenny → created an issue.

Regarding the change record https://www.drupal.org/node/3498595 → , if a contrib module like field_encrypt already implemented hooks_converted, what are the backwards compatibility implications of doing the rename? It would be great if the CR said something about this.

ptmkenny → created an issue.

Patch for 11.2 for installing with composer.

Fixed phpcs and setting back to "needs review". We still need a change record though. If someone who wrote the code writes a draft change record, I'm happy to clean it up.

The deprecation message in this MR refers to "key:2.0.0" but the next dev version of key is 4.0.x. The version number for the next version of Key needs to be determined before deprecations can be made.

Glad you got it working! Thanks for writing back in.

Drupal 7 and 7.x modules are EOL and no longer supported.

Key for Drupal 8+ has this feature, so I'm closing this issue.

Drupal 7 and 7.x modules are EOL and no longer supported.

I'm closing this as there has been no follow-up in 5 months. Feel free to reopen if you continue to have issues.

The Supported Modules page is now part of the Key module docs, so I'm marking this as "Fixed."

If this change is made, Attribute support must also be changed.

Unit tests are no longer passing so setting to "Needs work."

I hadn't seen this Key Configuration Overrides feature before, and when I read the form, it wasn't clear to me what a key configuration override actually was.

So I checked the docs → and found:

The Drupal 8 version of Key provides the ability to override any configuration value with a key. This allows site administrators to store configuration values in a more secure method than in the database or in settings.php.

So these are "Configuration overrides enabled by the Key module" rather than "Overrides of configuration for the Key module."

As a result, although I didn't know the feature existed before (there should be a better way of letting people know it exists?), I don't think just moving it to the Key module page makes sense because this is about configuration of Drupal as a whole, not configuration of individual keys. But the current title (Key Configuration Overrides) implies it is about "Key configuration", in other words, the configuration of keys, and the tab title ("Key overrides") suggests that even more strongly.

ptmkenny → made their first commit to this issue’s fork.

Key needs a KeyProvider to access the key value. By default, the module includes configuration, file, and environment variable key providers, as described in the README.

Key has no access to the system keychain, but you could mirror the values (for example, as environment variables).

Updated IS to make it clear that a maintainer needs to make a decision about the correct behavior.

If support for Drupal 8/9 is dropped, I think we should not just "require Drupal 10" but actually take the opportunity to modernize the code and use some of the new features of Drupal 10.

For example, these two issues already exist:

✨ Modernize services: Add autowiring aliases, use autoconfigure, constructors, etc Needs review

📌 Add attributes to plugins in addition to annotations Postponed

Committing these two would require 10.2+.

Why is the list an ordered list? Are the issues dependent on one another? (I'm guessing no because some of the later issues have already been committed.)

Also, if this is a list of issues to address that's fine, but if this is for the release notes, mglaman has created an excellent release notes generator that can make this kind of list automatically.

The big question is should Key drop support for Drupal 8 and/or 9. Historically, Key (and Encrypt) has tried to maintain compatibility even pass Drupal core's support window.

As shown in the MR for 📌 Remove support for Drupal < 9.1 Needs review , simply dropping Drupal 8 + 9 support is not sufficient here; there is also code that handles a special case for D8 that needs to be removed. I think Drupal 8/9 support should be removed in a separate issue before this is committed, and this issue should be made dependent on that one.

ptmkenny → created an issue. See original summary → .

As per #8, I think this is ready to go, but if it gets committed, we should create a follow-up issue to actually use the attributes within Key and then drop the annotations when Key requires at least Drupal 10.2.

ptmkenny → made their first commit to this issue’s fork.

The patch no longer applied so I fixed it and converted it to an MR.

ptmkenny → made their first commit to this issue’s fork.

This is tagged "needs tests" and there are no tests in the MR, so it cannot be RTBC.

ptmkenny → created an issue.

I'm sorry for the long delay in reviewing this.

I like the way this works, and I confirmed:

• Views are renamed correctly to the new path when "Rename admin path" is toggled on.
• Views are renamed correctly to the admin path when "Rename admin path" is toggled off.
• Views are renamed correctly to the admin path when the module is uninstalled.

However, the following flow will break renaming:

1. Toggle on "Rename admin path" and set the value to "hello". Save configuration.
2. Change the "Replace admin path" from "hello" to "goodbye". Save configuration.

Problem: the views are still renamed "hello", not "goodbye".

This flow needs to be handled, and I think we need test coverage for this.

The test should check at least the following cases:

• Toggle on "rename admin path" and check that view path is renamed to new value.
• Toggle off "rename admin path" after it is on and check that view path is still "admin".
• Toggle on "rename admin path" and uninstall module; check that view path is still "admin".
• Toggle on "rename admin path" and then rename the renamed path (hello/goodbye); make sure that the view path is renamed to "goodbye". Then uninstall and ensure that it is back to "admin".

No follow-up so I will close this.

Thank you for this suggestion, but I feel like adding arbitrary renaming of any path is out-of-scope for this module. Such renaming should be done in a different module (whether custom code or another contrib module).

There is an open issue for allowing node paths to be renamed ( ✨ Rename Node paths Needs work ), which I would consider adding to this module, but I think that allowing any path to be renamed adds too much complexity and could result in lots of support requests in the event of misconfiguration.

If you create your own module that adds this feature, I'd be happy to link to it as an alternative on the project page.

ptmkenny → made their first commit to this issue’s fork.

Thanks for reporting this.

The BaseFieldDefinition::create($type) is this one in SettingsForm.php?

      foreach ($field_types as $name => $field_type) {
        // Special handling for preconfigured definitions.
        // @see \Drupal\Core\Field\FieldTypePluginManager::getUiDefinitions()
        $type = str_starts_with($name, 'field_ui:') ? $field_type['id'] : $name;
        $field_definition = BaseFieldDefinition::create($type);

We could probably follow the comment here and do something closer to what getUiDefinitions does in FieldTypePluginManager:

  /**
   * {@inheritdoc}
   */
  public function getUiDefinitions() {
    $definitions = $this->getDefinitions();

    // Filter out definitions that can not be configured in Field UI.
    $definitions = array_filter($definitions, function ($definition) {
      return empty($definition['no_ui']);
    });

    // Add preconfigured definitions.
    foreach ($definitions as $id => $definition) {
      if (is_subclass_of($definition['class'], '\Drupal\Core\Field\PreconfiguredFieldUiOptionsInterface')) {
        foreach ($this->getPreconfiguredOptions($definition['id']) as $key => $option) {
          $definitions["field_ui:$id:$key"] = array_intersect_key(
            $option,
            ['label' => 0, 'category' => 1, 'weight' => 1, 'description' => 0]
          ) + $definition;
        }
      }
    }

    return $definitions;
  }

Also, I have documented that this module is not compatible with Data Field on the project page.

I've updated the MR to check for ConfigEntityInterface instead of method_exists(), and I also checked the rest of the code for calls to getThirdPartySetting. I identified two more places where we need to check before calling the method.

Please test this new MR and confirm it works with computed_field.

ptmkenny → made their first commit to this issue’s fork.

According to the change record → , it seems ->original is deprecated, but it should still work (for the time being)-- unless I am misunderstanding something? So it is possible that this is a core bug that needs to be addressed in core rather than in field_encrypt.

Moving to "Major" because this only applies if you are using the dev version of this module with a dev version of core.

Drupal 7 is end of life and no further development will happen.

Drupal 7 is end of life and no further development will happen.

Drupal 7 is end of life and no further development will happen.

Drupal 7 is end of life and no further development will happen.

Drupal 7 is end of life and no further development will happen.

Drupal 7 is end of life and no further development will happen.

Duplicate, please ignore.

Thanks for reporting this. As a maintainer, I'm happy to review an MR that contains a fix for this and a test, but I don't have the time to work on the code myself.

I also didn't start maintaining this module until a few years after that code was committed, so I'm not able to explain why it is written the way it is.

Reducing priority to "normal" because the main issue was fixed in the Address module. This change will need to go into 4.x now, since that is the dev branch.

ptmkenny → created an issue.

Drupal 7 is EOL (end of life) so 7.x issues are now being closed.

Feature requests now need to target 4.x.

See the discussion here: https://www.drupal.org/project/field_encrypt/issues/2155339 🐛 Encrypted fields do not work with Views exposed filter(s) Active

I'm closing this as there was no follow-up in over six months. Feel free to re-open if you are still having the same issue.