@mglaman you can pluralize the grantPermission config action - ie. there is a config action called grantPermissions that takes a list. See \Drupal\KernelTests\Core\Recipe\PermissionsPerBundleTest::testGrantPermissionsOnOneBundleThenAll for an example.
Nice test
This is going to require a lot of work and it might be easier to deal with specific issues on a higher level. For example, the recipe action could support a list of permissions instead of saving the role config entity 30 times, that's always going to be slower.
the grantPermissions() does exactly this.
While reviewing the code I realised we could ditch the Drupal\Core\StringTranslation\YamlTagTranslator class completely and just use the TranslatableMarkup constructor. Which kind of brings us inline with PHP attributes which feels nice. It also means that the Yaml::decodeTags() addition is even more flexible. I did toy with making the tag !\Drupal\Core\StringTranslation\TranslatableMarkup but it is really ugly and PHPStorm's yaml parser doesn't really like it either - plus we'd need to introduce an interface so you couldn't just construct any class here.
@jose reyero I hope it is okay but I pushed up the changes so people can see what I'm on about - if you don't like feel free to revert.
FWIW the concept of a fallback value seems go to me.
I think we also should support the fallback value for env too. The differences between the env and the config implementations are interesting. Env with return an empty string when the env var does not exist but config will throw an exception. I wonder what the right the thing to do here is...
I found a way around this in the contrib project but I still think this would be good to do given we're adding the route and we have the info.
It'd be good to have some test coverage somewhere I think you might be able to add a test processor in \Drupal\Tests\Core\Routing\UrlGeneratorTest to do this.
Editing a node with 4000 revisions... before the GET on node/N/edit takes 976ms afterwards 226ms... we also have autosave enabled... before the posts to autosave take 730ms and afterwards they take 50ms... YES nearly the entire time on an autosave is spent doing param conversion to get the latest revision ID... lolz...
In my opinion this is RTBC and given the code is @fabianx's and only the test is mine I think I'm in a good place to RTBC this.
Also given this affects param conversion and is used by workspaces I get the feeling this is actually a critical - we can also see the query in the performance tests.
Given we can't do the optimisation in \Drupal\Core\Entity\ContentEntityStorageBase::getLatestRevisionId() because of workspaces. We can just do the better approach in \Drupal\Core\Entity\Query\Sql\Query::prepare()
Updated the issue summary to reflect this.
Fixed the issue summary.
Updated issue credit.
alexpott → changed the visibility of the branch 2950869-entity-queries-querying to hidden.
alexpott → changed the visibility of the branch 10.1.x to hidden.
alexpott → changed the visibility of the branch 2950869-optimizing-revision-queries to hidden.
I'm hiding the old 10.x and earlier branches as they are not ever going to land and focussing on the 11.x branch.
Note that 📌 Integration with views bulk operations so actions can be selected against multiple jobs in the UI Active provides bulk functionality that can do this via the UI...
There's an interface we should be using to checking whether this is possible from a queue backend POV - see \Drupal\advancedqueue\Plugin\AdvancedQueue\Backend\SupportsDeletingJobsInterface - we could also consider adding another interface \Drupal\advancedqueue\Plugin\AdvancedQueue\Backend\SupportsClearingInterface and add a clear method to \Drupal\advancedqueue\Plugin\AdvancedQueue\Backend\Database that does the truncate.
I don't think this should be configuration - I think this ideally should be state. This feels a lot like maintenance mode and I would make the same arguments as to why. Just like you don't want to do infrastructure deployments to suspend a queue you don't want to think about Drupal configuration deployments either.
I'm not sure this is necessary - if you really want to do that you could use the post process event and check the job state there. We don't have an onSuccess either.
I think we need more justification with a real example use-case that couldn't be solved with an event listener.
No difference - they will just ignore it.
Note that the current approach does not require any core changes. We could drop a service if 📌 Add route name to outbound path processing Active lands but it's not going to be make a big impact. On an admin/content page this change results in 300 less queries against the database taking place.
Note the tests will not pass on 11.2.x Drupal core until 🐛 On Drupal core 11.2.x this module can cause issues with module installation Active is fixed.
alexpott → created an issue.
alexpott → created an issue.
alexpott → created an issue.
Yves thanks for everything. I have so many memories - you are an amazing whistler, deep thinker of both code and life and are missed.
I’m writing this in a car outside a boulangerie in a little town outside Paris… you me and Swentel need to have another meet up :)
alexpott → created an issue.
Committed 80fd4ba and pushed to 11.x. Thanks!
We also need to open an issue to make an 11.3.x version and up of default content that uses all the core stuff.
alexpott → created an issue.
Green is good.
alexpott → created an issue.
alexpott → created an issue.
I think this is more of task than a full on feature request.
Green is good.
@jsacksick pointed out that self:: was not right when paired with a protected const. Fixed this post commit. Great catch @jsacksick!
alexpott → created an issue.
I've tested this MR and added the filter to the default view provided by the module so new sites will benefit. Existing sites can add it themselves as updating existing views is harder and there are situations that are hard to account for.
Test evidence
alexpott → made their first commit to this issue’s fork.
This looks great - I checked for any other direct usages of the table in the module and there were none.
For me this is fantastic idea which we should definitely do. But given the disruption to patches I'd only do this in a major release so minor updates to sites that have patches applied don't break just because we've removed tests.
alexpott → made their first commit to this issue’s fork.
Posted an MR against 10.5.x (that will also work for 10.6.x) as this is definitely worth back porting.
+1 to #10 - plus we need to not set a default value in \Drupal\file\Plugin\Field\FieldFormatter\FileVideoFormatter::defaultSettings()
alexpott → created an issue.
We could deprecate the \Drupal\Core\Config\StorableConfigBase::$schemaWrapper property and add it as a third argument to \Drupal\Core\Config\StorableConfigBase::castValue(). But let's do that in a follow-up so we can backport this to 10.5.x and 11.2.x
@godotislate you can easily see this with Umami...
Before
[notice] Synchronized configuration: update views.view.archive. [1.04 sec, 53.45 MB]
[notice] Synchronized configuration: update views.view.articles_aside. [1.05 sec, 54.3 MB]
[notice] Synchronized configuration: update views.view.block_content. [1.07 sec, 55.4 MB]
[notice] Synchronized configuration: update views.view.content. [1.1 sec, 56.36 MB]
[notice] Synchronized configuration: update views.view.content_recent. [1.11 sec, 57.29 MB]
[notice] Synchronized configuration: update views.view.featured_articles. [1.12 sec, 58.13 MB]
[notice] Synchronized configuration: update views.view.files. [1.16 sec, 60.13 MB]
[notice] Synchronized configuration: update views.view.frontpage. [1.18 sec, 60.92 MB]
[notice] Synchronized configuration: update views.view.glossary. [1.2 sec, 62.38 MB]
[notice] Synchronized configuration: update views.view.media. [1.24 sec, 62.76 MB]
[notice] Synchronized configuration: update views.view.media_library. [1.29 sec, 64.87 MB]
[notice] Synchronized configuration: update views.view.moderated_content. [1.32 sec, 65.27 MB]
[notice] Synchronized configuration: update views.view.promoted_items. [1.34 sec, 66.98 MB]
[notice] Synchronized configuration: update views.view.recipe_collections. [1.36 sec, 66.65 MB]
[notice] Synchronized configuration: update views.view.recipes. [1.37 sec, 67.49 MB]
[notice] Synchronized configuration: update views.view.related_recipes. [1.38 sec, 68.44 MB]
[notice] Synchronized configuration: update views.view.taxonomy_term. [1.4 sec, 67.96 MB]
[notice] Synchronized configuration: update views.view.user_admin_people. [1.43 sec, 69.54 MB]
[notice] Synchronized configuration: update views.view.watchdog. [1.46 sec, 71.66 MB]
[notice] Synchronized configuration: update views.view.who_s_new. [1.47 sec, 71.16 MB]
[notice] Synchronized configuration: update views.view.who_s_online. [1.48 sec, 71.87 MB]
After
[notice] Synchronized configuration: update views.view.archive. [1.16 sec, 53.45 MB]
[notice] Synchronized configuration: update views.view.articles_aside. [1.17 sec, 54.3 MB]
[notice] Synchronized configuration: update views.view.block_content. [1.19 sec, 54.55 MB]
[notice] Synchronized configuration: update views.view.content. [1.22 sec, 53.74 MB]
[notice] Synchronized configuration: update views.view.content_recent. [1.23 sec, 54.67 MB]
[notice] Synchronized configuration: update views.view.featured_articles. [1.24 sec, 55.51 MB]
[notice] Synchronized configuration: update views.view.files. [1.28 sec, 56.5 MB]
[notice] Synchronized configuration: update views.view.frontpage. [1.3 sec, 55.66 MB]
[notice] Synchronized configuration: update views.view.glossary. [1.32 sec, 57.12 MB]
[notice] Synchronized configuration: update views.view.media. [1.35 sec, 56.18 MB]
[notice] Synchronized configuration: update views.view.media_library. [1.41 sec, 55.13 MB]
[notice] Synchronized configuration: update views.view.moderated_content. [1.45 sec, 54.43 MB]
[notice] Synchronized configuration: update views.view.promoted_items. [1.47 sec, 56.14 MB]
[notice] Synchronized configuration: update views.view.recipe_collections. [1.48 sec, 54.95 MB]
[notice] Synchronized configuration: update views.view.recipes. [1.51 sec, 55.79 MB]
[notice] Synchronized configuration: update views.view.related_recipes. [1.52 sec, 56.74 MB]
[notice] Synchronized configuration: update views.view.taxonomy_term. [1.54 sec, 55.03 MB]
[notice] Synchronized configuration: update views.view.user_admin_people. [1.58 sec, 54.87 MB]
[notice] Synchronized configuration: update views.view.watchdog. [1.6 sec, 56.75 MB]
[notice] Synchronized configuration: update views.view.who_s_new. [1.61 sec, 55.3 MB]
[notice] Synchronized configuration: update views.view.who_s_online. [1.62 sec, 56.01 MB]
Here's a script to change something meaningless on each of umami's views....
foreach (\Drupal\views\Entity\View::loadMultiple() as $view) {
$view->set('description', $view->get('description') . '1')->save();
}
While this fix will be most pronounced on config import - it will also benefit every config save where we don't set the trusted flag to true. So that's config forms, some post updates, hooks - lots of places.
🐛 Saving untrusted config results in memory leak Active fixes the config casting part of this :)
Finally we get around to fixing what @fabianx pointed out in #2294569-10: Determine cause of high memory consumption →
With this change we see a massive difference
[notice] Synchronized configuration: update views.view.archive. [6.76 sec, 135.48 MB]
[notice] Synchronized configuration: update views.view.association_entity_reference. [6.77 sec, 136.38 MB]
[notice] Synchronized configuration: update views.view.associations_multiply. [6.79 sec, 137.76 MB]
[notice] Synchronized configuration: update views.view.authmap. [6.8 sec, 138.55 MB]
[notice] Synchronized configuration: update views.view.changelogs. [6.8 sec, 138.95 MB]
[notice] Synchronized configuration: update views.view.channel_errors. [6.82 sec, 140.29 MB]
[notice] Synchronized configuration: update views.view.channels. [6.83 sec, 141.26 MB]
[notice] Synchronized configuration: update views.view.code_pools_collections. [6.84 sec, 141.95 MB]
[notice] Synchronized configuration: update views.view.content. [6.86 sec, 135.06 MB]
[notice] Synchronized configuration: update views.view.current_user_organizations. [6.88 sec, 136 MB]
[notice] Synchronized configuration: update views.view.custom_forms. [6.9 sec, 138.02 MB]
[notice] Synchronized configuration: update views.view.customer. [6.95 sec, 138.05 MB]
[notice] Synchronized configuration: update views.view.customer_emails. [7.32 sec, 155.2 MB]
[notice] Synchronized configuration: update views.view.easy_email_content. [7.33 sec, 156.01 MB]
[notice] Synchronized configuration: update views.view.email_evs_to_customer_report. [7.34 sec, 157.57 MB]
[notice] Synchronized configuration: update views.view.event. [7.35 sec, 158.29 MB]
[notice] Synchronized configuration: update views.view.event_winners. [7.38 sec, 153.9 MB]
[notice] Synchronized configuration: update views.view.events_list. [7.39 sec, 154.64 MB]
[notice] Synchronized configuration: update views.view.events_summary. [7.4 sec, 155.43 MB]
[notice] Synchronized configuration: update views.view.evs_file_upload_type. [7.4 sec, 155.73 MB]
[notice] Synchronized configuration: update views.view.evs_to_customer_submissions. [7.43 sec, 154.71 MB]
[notice] Synchronized configuration: update views.view.ffh_approval_queue. [7.47 sec, 154.67 MB]
[notice] Synchronized configuration: update views.view.files. [7.49 sec, 156.25 MB]
[notice] Synchronized configuration: update views.view.frontpage. [7.51 sec, 153.5 MB]
[notice] Synchronized configuration: update views.view.glossary. [7.52 sec, 154.24 MB]
[notice] Synchronized configuration: update views.view.manage_affiliates_banners. [7.53 sec, 154.92 MB]
[notice] Synchronized configuration: update views.view.manage_channels. [7.54 sec, 155.62 MB]
[notice] Synchronized configuration: update views.view.manage_offer_banners. [7.56 sec, 153.71 MB]
[notice] Synchronized configuration: update views.view.manage_offer_groups. [7.58 sec, 155.09 MB]
[notice] Synchronized configuration: update views.view.manage_offers. [7.64 sec, 156.49 MB]
[notice] Synchronized configuration: update views.view.manage_organization_channel_mappings. [7.65 sec, 157.2 MB]
[notice] Synchronized configuration: update views.view.manage_organizations. [7.68 sec, 160.52 MB]
[notice] Synchronized configuration: update views.view.manage_poe_uploads. [7.69 sec, 161.13 MB]
[notice] Synchronized configuration: update views.view.manage_pricing_promotion_announcements. [7.7 sec, 161.81 MB]
[notice] Synchronized configuration: update views.view.manage_regions. [7.72 sec, 153.73 MB]
[notice] Synchronized configuration: update views.view.manage_sbs_banners. [7.73 sec, 154.38 MB]
[notice] Synchronized configuration: update views.view.manage_sub_offers. [7.77 sec, 155.81 MB]
[notice] Synchronized configuration: update views.view.manage_telus_employees. [7.78 sec, 156.94 MB]
[notice] Synchronized configuration: update views.view.manage_webstore_urls. [7.81 sec, 154.35 MB]
[notice] Synchronized configuration: update views.view.message. [7.82 sec, 155.16 MB]
[notice] Synchronized configuration: update views.view.offer_sub_offers. [7.85 sec, 153.86 MB]
[notice] Synchronized configuration: update views.view.pricing_announcement. [7.86 sec, 154.78 MB]
[notice] Synchronized configuration: update views.view.redirect. [7.87 sec, 155.66 MB]
[notice] Synchronized configuration: update views.view.system_notifications. [7.91 sec, 155.07 MB]
[notice] Synchronized configuration: update views.view.taxonomy_term. [7.92 sec, 155.56 MB]
[notice] Synchronized configuration: update views.view.telus_channels. [7.92 sec, 156.22 MB]
[notice] Synchronized configuration: update views.view.telus_store. [7.95 sec, 154.59 MB]
[notice] Synchronized configuration: update views.view.telus_user_entity_reference. [7.98 sec, 154.55 MB]
[notice] Synchronized configuration: update views.view.unconfigured_accounts_report. [7.99 sec, 155.32 MB]
[notice] Synchronized configuration: update views.view.user_admin_people. [8.06 sec, 156.58 MB]
[notice] Synchronized configuration: update views.view.view_all_epp_organizations. [8.09 sec, 153.86 MB]
[notice] Synchronized configuration: update views.view.view_epp_organizations. [8.11 sec, 155.9 MB]
[notice] Synchronized configuration: update views.view.track_announcement_acknowledgements. [8.14 sec, 154.3 MB]
[notice] Synchronized configuration: update views.view.telus_request. [8.21 sec, 158.04 MB]
[notice] Synchronized configuration: update views.view.splash_page_sbs_orgs. [8.22 sec, 158.43 MB]
[notice] Synchronized configuration: update views.view.sbs_submissions_table. [8.48 sec, 175.01 MB]
[notice] Synchronized configuration: update views.view.sbs_submissions_data_lake_report. [8.54 sec, 180.27 MB]
[notice] Synchronized configuration: update views.view.sbs_data_lake_report. [8.58 sec, 184.39 MB]
[notice] Synchronized configuration: update views.view.referral_results_dashboard. [8.67 sec, 160.15 MB]
[notice] Synchronized configuration: update views.view.rebate_form_submissions. [8.73 sec, 156.97 MB]
[notice] Synchronized configuration: update views.view.poe_audit. [8.76 sec, 160.05 MB]
[notice] Synchronized configuration: update views.view.perks_campaign. [8.84 sec, 158.06 MB]
[notice] Synchronized configuration: update views.view.migration_requests. [8.87 sec, 161.27 MB]
[notice] Synchronized configuration: update views.view.master_event_report. [8.98 sec, 161.34 MB]
[notice] Synchronized configuration: update views.view.manage_channel_mappings. [8.99 sec, 162.51 MB]
[notice] Synchronized configuration: update views.view.lead_nurturing_for_dealers. [9.03 sec, 154.6 MB]
[notice] Synchronized configuration: update views.view.hot_leads_reporting. [9.07 sec, 155.03 MB]
[notice] Synchronized configuration: update views.view.evs_error_submissions. [9.11 sec, 155.97 MB]
[notice] Synchronized configuration: update views.view.evs_customer_events. [9.16 sec, 155.95 MB]
[notice] Synchronized configuration: update views.view.event_submission_report. [9.26 sec, 160.18 MB]
[notice] Synchronized configuration: update views.view.event_form_submissions. [9.32 sec, 156.51 MB]
[notice] Synchronized configuration: update views.view.epp_data_lake_report. [9.42 sec, 159.7 MB]
[notice] Synchronized configuration: update views.view.dealer_traffic_reporting. [9.44 sec, 160.82 MB]
[notice] Synchronized configuration: update views.view.customer_sbs_vip_referral_report. [9.55 sec, 160.18 MB]
[notice] Synchronized configuration: update views.view.customer_sbs_referral_report. [9.69 sec, 162.86 MB]
[notice] Synchronized configuration: update views.view.customer_iq_submissions. [9.73 sec, 166.37 MB]
[notice] Synchronized configuration: update views.view.customer_event_tracking. [9.74 sec, 167.45 MB]
[notice] Synchronized configuration: update views.view.customer_consumer_vip_referral_report. [9.82 sec, 158.14 MB]
[notice] Synchronized configuration: update views.view.customers_multiply. [9.88 sec, 156.37 MB]
[notice] Synchronized configuration: update views.view.customers_affiliates. [10.06 sec, 165.89 MB]
[notice] Synchronized configuration: update views.view.customers. [10.35 sec, 177.81 MB]
[notice] Synchronized configuration: update views.view.bib_request_queue. [10.45 sec, 159.49 MB]
[notice] Synchronized configuration: update views.view.all_customer_codes. [10.62 sec, 166.13 MB]
[notice] Synchronized configuration: update views.view.affiliates_data_lake_report. [10.67 sec, 170.31 MB]
[notice] Synchronized configuration: update views.view.admin_form_submissions. [10.73 sec, 155.47 MB]
alexpott → created an issue.
This is a bug and it'd help config validation if it was backported.
Here's the patch I'm using on a site to fix this.
alexpott → created an issue.
alexpott → created an issue.
Looking at the code in the module it feels like it would need some adaptation - for example the way it works will only result in the desired behaviour on english sites because the form op is in english. I think all the URL and from button text matching is very fragile. In order to bring this into the module what would be create is to have some steps to reproduce the issue from the standard profile in this issue summary and then we can consider the best UX and DX to implement the feature.
I've left some comments about concerns I have on the MR>
alexpott → created an issue.
Another thing to consider here is that @catch has pointed out there is a lot of complexity in \Drupal\node\Cache\NodeAccessGrantsCacheContext() that probably belongs in the API
@berdir thanks for the reviews.
Discussed with @phenaproxima - we agreed to removed the dependency on serialization as suggested by @berdir. @phenaproxima and I still think there is value in allowing fields to have some say in how they are exported via the setSetting() capability. Re 11.3.x vs previous releases - I think hardcoding a string is fine in this situation - we can create an enum from the string when we use it and error if it is wrong. Also @phenaproxima has a test showing that these settings do not end up in base field override configuration so I don't think we need to worry about configuration schema. @berdir maybe you have another suggestion that we could leverage for a field to give the exporter extra information. We're trying to avoid the exporter making too many assumptions about fields and give some control to the fields.
Added some more comments.
Backported to 11.2.x as a test-only change.
Committed and pushed ec4fdb42579 to 11.x and 248f84d59ab to 11.2.x. Thanks!
And we need an update function to at least save all blocks where the condition plugin is used...
If we add this validation then we also need to add the dependency :)
We need to add something like:
/**
* {@inheritdoc}
*/
public function calculateDependencies() {
if (!$this->configuration['theme']) {
return [];
}
return ['theme' => [$this->configuration['theme']]];
}
to \Drupal\system\Plugin\Condition\CurrentThemeCondition
This looks nearly ready - I think we can use API rather than exceptions to skip unrouted routes in the child link checking...
Committed and pushed 5771e076240 to 11.x and c8387fc4990 to 11.2.x. Thanks!
The post update should look something like:
/**
* Remove empty email addresses from update.settings configuration.
*/
function update_post_update_fix_update_emails(): void {
$config = \Drupal::configFactory()->getEditable('update.settings');
$emails = $config->get('notification.emails');
$filtered_emails = array_filter($emails);
if ($emails !== $filtered_emails) {
$config->set('notification.emails', $filtered_emails)->save();
}
}
There's no need to do the array_filter() stuff at runtime - we should ensure we have valid data in the configuration all the time. We need a post-update function to filter the config and then we can remove some of the changes here. With the change to multiLineStringToArray we're enusre the form does not put blank strings into the array.