Not sure this is working. I set a field value to be =SUM(1,3)
and the CSV file had "=SUM(1,3)"
for that column.
But, most importantly, this patch will only work when the output is a CSV file. There are other options with this module, such as XLSX, and this patch will actually alter the data in such a way that it will no longer be a valid XLSX file.
If I am following along with this change, am I correct that this
public function getFieldDefinitionSettings(): array {
return [
'target_type' => 'media',
'handler' => 'default:media',
'handler_settings' => [
'target_bundles' => [
'image',
],
],
];
}
needs to be changed to
public function getStorageDefinitionSettings(): array {
return [
'target_type' => 'media',
];
}
public function getFieldDefinitionSettings(): array {
return [
'handler' => 'default:media',
'handler_settings' => [
'target_bundles' => [
'image',
],
],
];
}
Am I understanding this change correctly?
I am also noticing a memory issue introduced by admin_toolbar/admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks::getDerivativeDefinitions()
I end up with over 400 items in $links. If I hack the code to exclude some entity types, and get the total count of $links down to around 300, all seems to work well. It doesn't appear to matter which entity types I exclude.
Strangely, we are most likely to trigger the issue one some node bundles' Manage Field page, but the issue doesn't trigger on other entity types' bundles.
Sorry, this is probably not very useful information.
Disabling the module, or returning an empty array from ::getDerivativeDefinitions()
does seem to not cause the issue to happen.
The Facets patch at 🐛 Facets breaks all AJAX views that uses pagers even without facets Active (#25) fixes the issue for us. The fix is not yet in an official release version.
Interestingly, the patch at #44 of this issue ALSO fixes the issue.
This fixed multiple issues for use. Our theme CSS is at the end of (as it should be) and then a module's library CSS (select2) ends up also added to the end of when the library is included with an AJAX dialog. With our theme CSS no longer at the end of we would need to review all CSS selectors to potentially add more specificity.
This patch works well for us (10.2.6).
JonMcL → changed the visibility of the branch 3347343-10-2-x--do-not-merge to hidden.
JonMcL → changed the visibility of the branch 3347343-10-2-x--do-not-merge to active.
Sorry in advance for the "noise" but can someone give a a summary of where things stand with this REGRESSION? I feel like it is not getting the proper attention from core maintainers that it should? Or am I wrong in that impression.
I see two MRs. One is marked for Drupal 10 and the other, I assume, is for Drupal 11? I suppose work has stopped completely on 9.5.x even though this regression was introduced in 9.5.9?
Also, I think there is still problem with menu items that reference a view that has a contextual filter. I recall others discussion that, but I don't know if it was in this issue or another.
I am curious if anyone has tried to tackle a token handler that might work like this:
$message->addContext('salutation', 'Greetings ' . $name);
And then in the message template could be a token like: [message:contexts:salutation]
Setting back to "Needs review"
Potentially major problem with the else statement in this block of code:
// Special treatment for Core's user entity.
if ($entity_type_id == 'user') {
$widget = &$form['account']['name'];
}
else {
$widget = &$form[$label]['widget'][0]['value'];
}
Not all entity form displays are going to have a label field in the form. Assigning a non-existent form element to $widget by reference actually created the element with a value of null;
Problem is that ContentEntityForm::copyFormValuesToEntity
then gets called and because the label widget was added to the form (with a value of null), ::copyFormValuesToEntity then sets the label to be null.
This is probably a problem only when auto_entitylabel is disabled for a particular entity type and when the label widget is not present in the form currently being submitted.
Very frustrating to find out that using the "theme" category on a module's library, with a group value that is higher than CSS_AGGREGATE_THEME (100), does not allow the module's CSS to come after the theme's CSS in the DOM. Impossible to do simple CSS refinements when the module wants to do them.
I am glad I am not alone in this :)
For what it may be worth, here is my implementation of hook_css_alter that re-applies the group property from the library data:
function modulename_css_alter(&$css, \Drupal\Core\Asset\AttachedAssetsInterface $assets) {
foreach($assets->getLibraries() as $name) {
[$module, $id] = explode('/', $name);
if ($module && $id) {
$library = \Drupal::service('library.discovery')->getLibraryByName($module, $id);
foreach($library['css'] ?? [] as $definition) {
if (isset($definition['group']) && $path = $definition['data']) {
if (isset($css[$path])) {
$css[$path]['group'] = $definition['group'];
}
}
}
}
}
}
It is probably not very efficient, but should only run when needed and then the alterations are cached.
I was thinking of simplifying my question:
Where does the IEF element set the new values, taken from $form_state, into the form elements?
If I can find the code, I can maybe determine what else we need to do to our multistep framework to make it compatible with the IEF element.
JonMcL → created an issue.
Sorry for the delay. I haven't had a chance to do any testing in a long while, so hopefully everything internal is still working well enough for D10. I know it's difficult to upgrade without module's having new core version requirements, so I wanted to get this out there.
I know it should probably be another issue, but if this issue ever manages to make it into core, it would be great if one small addition could be added:
class: $originalButton.attr('class'),
disabled: $originalButton.attr('disabled'),
allowing the dialog buttons to inherit the disabled attribute from the original button.
Previous behavior was to prepend the CSS:
$('head').prepend(response.data);
I think the new code is appending to which can cause order of precedence issues. A module might have CSS component assets (weight of 0) which are added into after our theme's CSS_THEME assets (weight of 200).
This might not be an issue for most situation, but I just noticed it with some quick D10 testing in advance of an upgrade. Maybe it was just a coincidence that we were able to have our theme CSS override the module's component CSS?
Any known workarounds for this (potential) issue?
This MR also allows for:
- Use TNS string to specify connection
- Add init commands to be executed before each connection
@bircher My mistake and please accept my apologies if my question seemed like a complaint.
I searched for the this patch's config property and I thought I saw it in the 3.0 module's schema file, and then no references to the property in the code. However, I was incorrect and the enable_export_filtering property is not used or needed.
The 3.0 version of the module is working great, for both import and export, in simple mode. Thank you!!
Never mind. I definitely don't know what I am doing. :)
reflection/validator won't get installed by composer if the git.drupalcode.org drupal/form_alter_service repo comes before https://packages.drupal.org/8 in the repositories list. The git.drupalcode.org repos need to come after.
Either I did something wrong with the issue fork, or there is a problem. After installing the patched version from the issue fork, I am getting:
PHP Fatal error: Uncaught Error: Class "Reflection\Validator\Annotation\ReflectionValidatorAnnotationReader" not found in /var/www/html/web/modules/contrib/form_alter_service/src/FormAlterCompilerPass.php:95
I went ahead and created an issue fork and merge request. I don't know if I will ever get this new process done right, but hopefully it is?
Adding this to my composer.json repositories section worked for me:
(it must come before https://packages.drupal.org/8 in the repositories section)
{
"type": "package",
"package": {
"name": "drupal/form_alter_service",
"version": "3.x-dev",
"type": "drupal-module",
"source": {
"url": "https://git.drupalcode.org/issue/form_alter_service-3297260.git",
"type": "git",
"reference": "3297260-automated-drupal-10"
}
}
}
Then run composer update drupal/form_alter_service
Yes please. Without this fix, this module causes a fatal error with the entity_ui module because that module attempts to get this module's label in the EntityTabListBuilder class.
Completely correct to add provider to tabs created by a derivative plugin.
Patch does not apply to the 8.x-3.0 branch. The enable_export_filtering setting is now part of the module's schema, but it is set to false by default. I also can't find anywhere it is used and no way to change it on the settings form.
Any chance some of the maintainers could chime in? Maybe the setting was added in anticipation of a new release?
I don't think this notice has anything to do with the message logged in trace on doTrustedCallback.
I think the notice is because \Drupal\Core\Entity\Element\EntityAutocomplete::processEntityAutocomplete
is expecting to get a form object from $form_state with this snippet from line 180:
// Put entity into settings.
$form_object = $form_state->getFormObject();
Right below it is a check to see if $form_object has an expected value, and if not, it proceeds on.
I don't know if this is a correct solution, but this seems to get us past the notice:
protected static function setAutocompleteRouteParameters(array &$element): array {
$complete_form = [];
$form_state = new FormState();
$form_state->addBuildInfo('callback_object', NULL);
$element = EntityAutocomplete::processEntityAutocomplete($element, $form_state, $complete_form);
$element['#autocomplete_route_name'] = 'select2.entity_autocomplete';
return $element;
}
Patch is attached.
@amateescu: Thank you for your work on this.
Unfortunately it no longer applies to 9.5.x or 10.x. Any chance you have rerolled this patch recently?
I think @Berdir is correctly pointing out that there is unlikely a good/proper way to have this module enabled along with either the context or menu_position modules. All 3 modules are attempting to take over the menu.active_trail.
The decorator approach does allow both context and this module to be enabled successfully, and this module's functionality does seem to work correctly in my limited tests. However, it appears that the context's ContextMenuActiveTrail class will no longer get called unless "Drupal Core Behavior" is chosen. This is probably to be expected.
However, I think I might accept this for our use cases. If this module works well enough for us, we might not need to use the context module's "Menu" reaction. Maybe a warning on the status page (requirements) that this module is disabling the active trail functionality within the context module?
Configuring a menu's Menu Trail Source to use Drupal Core Behavior will also again allow the context module to set the active menu trail (since it takes over core).
JonMcL → created an issue. See original summary → .
I hope you all don't mind me creating merge request !5. It is for Drupal 10 compatibility and config_filter ^2.4 (necessary to Drupal 10 compatibility). !5 was made off of !4.
Patch in #8 works with with the 8.x-1.3 branch, even though this issue is tagged with the 8.x-2.x-dev branch.
I think creating an issue fork would help make upgrading to D10 with this patch a little easier. (composer can fetch modules from issue forks on drupalcode). I was going to create it, but then I realized I might be attributed as the contributor.
Sorry, my mistake. I thought I would create an issue fork so that the patched code can be pulled down by composer.
I didn't realize you already have Drupal ^9 || ^10 in the current dev branch. I can pull from that.
So please ignore the unnecessary issue fork I created.
Obviously an extremely simple MR :) I have no idea yet if there are incompatibilities between context_breadcrumb 2.0.1 and context 5.0.0-rc1
To be able to upgrade both modules, I added the the following changes to my composer.json:
"repositories": [
{
"type": "composer",
"url": "https://packages.drupal.org/8",
"exclude": [
"drupal/context_breadcrumb"
]
},
{
"type": "package",
"package": {
"name": "drupal/context_breadcrumb",
"version": "2.0.1@dev",
"type": "drupal-module",
"source": {
"url": "https://git.drupalcode.org/issue/context_breadcrumb-3369056.git",
"type": "git",
"reference": "2.0.x"
}
}
}
],
Then I ran:
composer require 'drupal/context:^5.0@RC' 'drupal/context_breadcrumb:^2.0@beta'
Per my update in #55, I think something like this is needed in EntityReference::validateExtraOptionsForm
. This will be to handle entities with no bundles and possibly for selecting no bundles on the subform.
// If no checkboxes were checked for 'target_bundles', store NULL ("all
// bundles are referenceable") rather than empty array ("no bundle is
// referenceable" - typically happens when all referenceable bundles have
// been deleted or when an entity type has no bundles).
if ($subform_options['target_bundles'] === []) {
$subform_options['target_bundles'] = NULL;
}
// Store the sub handler options in sub_handler_settings.
$form_state->setValue(['options', 'sub_handler_settings'], $subform_options);
@DieterHolvoet: I just happened to bump up against a similar situation. In my case it's a custom entity type that has no bundles.
I think the issue is actually in DefaultSelection, which I do not think is being touched by this patch. I suspect that only users of this patch would be trying to use DefaultSelection with a bundle-less entity, so maybe that is why it hasn't been an issue before? I'm not entirely sure.
In DefaultSelection::buildEntityQuery, there is this snippet:
// If 'target_bundles' is NULL, all bundles are referenceable, no further
// conditions are needed.
if (is_array($configuration['target_bundles'])) {
// If 'target_bundles' is an empty array, no bundle is referenceable,
// force the query to never return anything and bail out early.
if ($configuration['target_bundles'] === []) {
$query->condition($entity_type->getKey('id'), NULL, '=');
return $query;
}
else {
$query->condition($entity_type->getKey('bundle'), $configuration['target_bundles'], 'IN');
}
}
And in DefaultSelection::buildConfigurationForm, there is this condition:
if ($entity_type->hasKey('bundle')) {
// ..code removed for comment..
}
else {
$form['target_bundles'] = [
'#type' => 'value',
'#value' => [],
];
}
So I think that '#value' => []
needs to become '#value' => null
, but I am not entirely sure that is the best solution or what other impact that change would have elsewhere. Alternatively, EntityReference::buildExtraOptionsForm could be modified to set target_bundles to null if the entity type does have have bundles.
As a workaround, I ended up writing a custom EntityReferenceSelection plugin that extends DefaultSelection and changes target_bundles to null, when the entity has no bundles, inside ::buildEntityQuery. A little messy, but gets the job done (for now).
No problem, other Jon :)
I didn't do much testing, primarily because flysystem_s3 need updates to work with the flysystem 3.0.x branch and my primary use case is S3. Please open another issue if you find problems. I'd be happy to investigate.
Working well for us. We are using this to set a specific route parameter based on external information:
/** @var \Drupal\Core\Url $url */
$url = &$local_actions['local_action.id']['#link']['url'];
$url->setRouteParameter('node', $node->id());
A word to the wise: don't be like me and remember that you may likely need to have a method of invalidating the cache for this rendered item.
Yes, please!
Works well for us to chain through two entity reference fields.
Adding my use case for this ..
We are in active development of a new Drupal application. Migration source is an Oracle database (non Drupal). We do not have any migrations created by Drupal itself. All of our migrations are being created like foo_migrate/migrations/*.yml. These migration configs are very rapidly changing and will go through many rounds of refinement before they can be considered ready for config/install or config/sync directories.
Our issue is that all our testing and staging environments are in Kubernetes clusters and ssh access is not readily available. So no easy drush execution.
It would be great to have a UI that can run theses migrations. I fully understand it is not a simple problem to solve. The current lists are based on displaying migrate_plus.migration configs while migrations that exist inside of a migrations/ folder are actually plugins. So maybe a separate UI that only lists plugins might be needed.
So if I'm reading things correctly, this cannot be fixed in D7 until it is fixed in D8? (since it is a core patch and there is the backport policy)
So where does D8 patch at #85 stand? Is this still an issue in D8?
It would be great if this can be put into D7 for a future release. Some of us forget to re-apply core patches after we install security upgrades to core :)
Any suggestions for getting to to move forward with D7 or do we have to wait for the D8 fix no matter what?