Thanks @beloglazov91 , code changes look good.
Thanks Manuel! Well done!
pavel.bulat β made their first commit to this issueβs fork.
The patch had an issue:
Error: Call to undefined method Drupal\Core\Field\BaseFieldDefinition::id() in limited_field_widgets_field_widget_third_party_settings_form() when you try to open the `Manage Form Display` tab with fields that were created on a code level.
For example: a Commerce Order has an Unlimited Order Items field. The field definition for the Order Items doesn't have id().
Steps to reproduce
1) Install Drupal Commerce and Commerce Order
2) Create a new order type
3) Open Manage Form Display tab
In the last commit, I added a check if ($field_definition instanceof FieldConfigInterface)
. It fixed the issue, but I am not sure that it was the right way to fix it, so feel free to review it.
pavel.bulat β made their first commit to this issueβs fork.
I have realized that the issue is related to the https://www.drupal.org/project/limited_field_widgets/issues/3201950 β¨ Implement custom validation constraint Needs review since the issue was introduced by patch #5 in the https://www.drupal.org/project/limited_field_widgets/issues/3201950 β¨ Implement custom validation constraint Needs review
pavel.bulat β created an issue.
Hi @goz, @benjifisher.
Again, I think we should agree on the order of parameters. (See Comment #2.)
Maybe let's use "$metrics" array as a parameter instead?
$expected_metrics = [
'stylesheet_count' => 2,
'script_count' => 1,
'stylesheet_bytes' => 40150,
'script_bytes' => 7250,
'query_count' => 0,
'cache_get_count' => 1,
'cache_set_count' => 0,
'cache_delete_count' => 0,
'cache_tag_checksum_count' => 0,
'cache_tag_is_valid_count' => 1,
'cache_tag_invalidation_count' => 0,
];
$this->assertMetrics($performance_data, $expected_metrics);
I assume the amount of metrics will constantly grow, so using them as parameters won't be optimal. Also, I think it is more readable since you can immediately understand which number to change in case of failed test.
Just summarising the options that we have:
- parameters
cons:
- the amount of them may grow
- harder to read numbers if name of each parameter is not highlighted by IDE on the test level)
- you need to pass all previous parameters to theassertMetrics
to be able to check the last one if needed. When a new important metric is introduced, it will be hard to re-order parameters, since a lot of tests will be already using theassertMetrics
, so, in this case, we always would need to add all previous parameters.
pros:
- each parameter has a type
- could be highlighted by IDE - array
cons:
- looks like it also will be copy pasted from other tests, since it requires a bit more effort to be created. Potentially we can add an example of the array in the description for theassertMetrics
helper)
- doesn't have types
pros:
- more readable on the test level, so easy to maintain
- doesn't have any problems when new metrics are added
I will be happy to hear your thoughts. Thanks in advance.
pavel.bulat β made their first commit to this issueβs fork.
pavel.bulat β created an issue.
Kristen Pol β credited pavel.bulat β .
Thanks for the update @farhadhf , I have tested it and it works as expected.
Thank you all for working on it! I have merged the path.
Thanks for the patch. It works for me.
Hi @sboden , I have tested it with
"name": "drupal/config_split",
"version": "2.0.0-rc4",
"name": "drupal/config_filter",
"version": "2.4.0",
"name": "drupal/config_ignore",
"version": "2.4.0",
and I can confirm that all ignored configs will be exported during `drush cex`. I think it should work this way. The most important is that ignored configs won't be imported during `drush cim`. I have tested it and ignored configs weren't imported in my case.