Settings no longer stored as third party settings

@JeroenT what's the status here? Is the MR final?

The changes look very good and important. Are you using this in production already?
It also adds very important tests, so let's please finalize this :)

I'll tag this for review. Any active maintainer who can also have a look? Otherwise, would you perhaps jump in as Co-Maintainer @JeroenT?

As of the general importance

Tests fail, update require-dev to newest context version.

Hey Mats, just tested the patch and found no major issues, only a couple of notes.

I've tried to test path on Gitpod env via Drupalpod.
And it seems Drupalpod do not install require-dev packages when you create env right from browser extension.

Automatic tests.
In order to run tests there I run manually `composer require drupal/context:^5.1` and similar for menu_block module.

1. minor issue is in composer deps.

+++ b/composer.json
@@ -19,6 +19,7 @@
+    "drupal/context": "^5.1"

There is no 5.1 version for context module but composer will install dev version instead:

composer show -a drupal/context
...
versions : * 5.x-dev, 5.0.0-rc1, 4.x-dev, 4.1.0, 4.0.0, 4.0.0-beta6, 4.0.0-beta5, 4.0.0-beta4, 4.0.0-beta3, 4.0.0-beta2, 4.0.0-beta1, 1.x-dev, 1.0.0-alpha1, 0.x-dev, dev-1.x, dev-0.x, * dev-5.x, dev-4.x

I'm not sure but we can update it to ^5.0 or even specify beta. BTW: I used context v4.1 for manuall testing, so this is really minor.

Manuall testing.
I used system menus with blocks + context module.
Created a couple of menu links and translated them, show them in blocks. No issues, works great, as expected.
Then I ensured that third party settings are in place within exported configs:

context.context.test_context.yml
-----
name: test_context
reactions:
  blocks:
    id: blocks
    blocks:
      00d9886c-364f-44c6-85f4-a4d86d2adb9e:
        id: 'system_menu_block:main'
        label: 'Main navigation'
        provider: system
        custom_id: system_menu_block_main
        ...
        third_party_settings:
          menu_multilingual:
            only_translated_labels: 1
            only_translated_content: false
====
block.block.olivero_main_menu.yml
----
third_party_settings:
  menu_multilingual:
    only_translated_labels: true
    only_translated_content: true
...
id: olivero_main_menu
plugin: 'system_menu_block:main'
settings:
  id: 'system_menu_block:main'
  ...
  provider: system
  level: 1
  depth: 2
  expand_all_items: true

Now, small code nitpeaks:)
PHPCS didn't provided any helpfull advices.
But

+++ b/tests/fixtures/update/menu-multilingual-block-settings.php
@@ -0,0 +1,58 @@
+ * Contains database additions to search-api-db-base.php.
+ *
+ * Can be used for setting up a base Search API sorts installation.

Seems like odd strings related to search api. Let's update them.

That's all!

Greak work @matsbla, @Jeroen!
Thanks @Anybody for pushing forward.

@matsbla please commit your changes to this issue's issue fork, or at least provide an interdiff with your changes!

Well, I updated fork and now @Grevil your turn with help :)
You could help us in the next ways:
- create interdiff and ensure that changes were done only in composer.json,
- or review and give feedback on patch/fork
- or do manual testing
- or run automated tests locally

@vlad.dancer, thanks!

On it! :)

We should think about either dropping the Drupal 8 compatibility entirely, or instead create a new 2.x branch where we drop the Drupal 8 compatibility and drop the Drupal 10 compatibility in the "old" branch. The reasoning behind this is, that "drupal/context" 4.1 is not Drupal 10 compatible, making this module not compatible either. Your decision @vlad.dancer and @matsbla!

I also found this issue here: 🐛 Enable strict config schema for tests Closed: duplicate , simply removing the variable will lead to the tests failing, which would be out of scope of this issue, so I created a seperate issue.

Wow, @Grevil. That's great feedback! Thank you.

Found another tiny issue, otherwise code looks great and tests run successful locally! Great work! :)

Let's discuss #38 🐛 Settings no longer stored as third party settings Fixed and then get this stuff committed soon! If we decide to use drupal/context:5.0-rc and drop the D8 compatibility, we should quickly do some further manual testing.

@vlad.dancer, glad I could help! :)

Great work @Grevil! Let's set this RTBC for maintainer review and decision regarding #38. I'd vote for a new 2.x semver branch.

Thank you folks! Just merged and commited.
I created 2.x branch, let's plan integration with drupal/context:5.0 now.

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