Now that this update has broken all of our builds and I'm finding this issue for the first time, I feel like the initial "example" that people are referencing was misleading. The example of case-sensitive order was:
use Drupal\Core\Controller\ControllerBase;
use Drupal\car\CarInterface;
use Drupal\door\DoorInterface;
use Drupal\door_handle\HandleInterface;
But this is another example of what case-sensitive order would be:
use Drupal\Core\Controller\ControllerBase;
use Drupal\DoorHandle\Something
use Drupal\car\CarInterface;
use Drupal\door\DoorInterface;
use Drupal\door_handle\HandleInterface;
because "D" is < "c". This is actually more difficult for me personally to parse and read. I prefer what PHPStorm is doing. But I guess this decision has already been made and we are stuck with it because the new version was already released. But it's causing a major amount of work for us to update all of our projects to pass our linting/coder validation.
We accept Richard as a maintainer of the PURL module and I've updated his permissions as requested.
I think I see what happened. Our scripts run `drush updb` followed by `drush config-import`. The `updb` created the `view.site_settings` but then the `config-import` removed it (because it hadn't been exported yet)
Going to see if I can force some update hooks to re-run. But closing this since I don't think it's an issue in the site_settings module itself. It's more like a normal Drupal issue of complicated interactions between update hooks and config import.
It appears the issue is that the `site_settings` View did not get created. I ran update hooks but didn't see anything get triggered.
To help try to reproduce, in D10.1 I had 2 different site settings:
* `copyright` which is a single entity with a single text field
* `social_links` which is a MULTIPLE value entity with fields for `link` and `icon name`
mpotter → created an issue.
OK, sorry for all the confusion above as I tried to debug this, but I found the cause and solution.
The problem is that there is already an index on `acquia_dam_integration_link_tracking` with a name of `integration_link_id`. And you can't add this as a Primary key when this index already exists. The error message "Incorrect index name" is very misleading.
In the `acquia_dam_post_update_link_tracking_primary_key` hook, when you detect that there isn't any primary key, before trying to add the primary key you first need to check if there is an index named `integration_link_id` already, and drop that index first, and THEN create the primary index.
So the correct code for that update hook would be:
function acquia_dam_post_update_link_tracking_primary_key(&$sandbox) {
$schema = Database::getConnection()->schema();
if (!$schema->indexExists('acquia_dam_integration_link_tracking', 'PRIMARY')) {
if ($schema->indexExists('acquia_dam_integration_link_tracking', 'integration_link_id')) {
$schema->dropIndex('acquia_dam_integration_link_tracking', 'integration_link_id');
}
$schema->addPrimaryKey('acquia_dam_integration_link_tracking', ['integration_link_id']);
}
}
Of course, now it's a mess because that update hook already exists and potentially could have already run on an existing site. So I'm not sure simply changing this hook will work for all users. However, it worked for me when upgrading from 1.0.9 to 1.0.11
So I'm adding a patch here just so I can apply it to my local (and anyone else upgrading from versions prior to 1.0.10 where this problem first occured). But the actual fix for your next release might need to be different.
I think I see part of the problem here:
In 1.0.10 this post_update hook was added: `acquia_dam_post_update_link_tracking_primary_key` and is mistakenly set the primary key as `entity_uuid` which isn't unique.
Then in 1.0.11, the above update hook code was fixed to change the key to `integration_link_id` and a NEW post_update hook was added: `acquia_dam_post_update_link_tracking_change_primary_key`
But these post_update hooks run in alphabetical order. So when updating from 1.0.9 directly to 1.0.11, BOTH hooks run, with the new update hook added in 1.0.11 running FIRST and then the old "fixed" hook from 1.0.10 tries to run and gives the error above.
Still not clear why it throws an error and why the status page still indicates there is no primary key on this table.
OK, this is very odd. When I inspect the `acquia_dam_integration_link_tracking` table in our live site running 1.0.9 I already see "integration_link_id" marked as PRI for the key. And yet Drupal status page is still complaining that this table doesn't have a primary key. So now I'm really confused.
But here is the exact error when upgrading from 1.0.9 to 1.0.11 (different then I mentioned above)
> [notice] Update started: acquia_dam_post_update_link_tracking_primary_key
> [error] SQLSTATE[42000]: Syntax error or access violation: 1280 Incorrect index name 'integration_link_id': ALTER TABLE "acquia_dam_integration_link_tracking" ADD PRIMARY KEY ("integration_link_id"); Array
> (
> )
FYI, if I try updating directly from 1.0.9 to 1.0.11 the error above still occurs when running update hooks, except the error message is slightly different with "entity_uuid" replaced with "integration_link_id"
Here is a roll of this patch for D10.1.x that removes the `core/phpstan-baseline.neon` change that has already been applied to core.
Adding the minimal config recommended.
Put permission file in wrong place. Try again.
Changes resolved by #3256664
Looks like it can't be tested until this is merged because of composer, so going to merge this into dev and then run tests.
New patch to fix composer dependency in the test module.
Not sure why this got assigned. Issues should only be assigned to people doing the work. These changes will be rolled into issue #3196224 and I'll add credit for kdebisschop
kdebisschop: see the notes about an alternative way to handle the deprecated drupal_get_path, but I think it covers what you have here.
Going to hijack this issue to consolidate all of the D10 compatibility changes.
I approached the deprecated drupal_get_path is a somewhat different way than others. Instead of adding a new service argument to the plugin base class, which potentially breaks compatibility with other plugins that people might have created, I added the moduleExtensionList to the ConfigActions service itself. Plugins then use the new getModulePath function of the ConfigActionService instead of calling the Drupal moduleExtensionList directly.
This saved a lot of mocking and refactoring of tests and I think preserves proper dependency injection.
Uploading the full patch for the test bot.
Wondering if this is the cause of the problem on our site. We only have a few users, but on two occasions, when a non-admin user has had their password expire, they were not able to change it and kept getting login errors, eventually causing them to get added to the flood control table. We don't have email allowed on this server so they are not able to send a password reset link.
So would like some feedback on whether the patch presented above is the correct approach? If so, then I can try to reproduce the issue and see if this fixes it.
My worry about just setting the fields to hidden is that someone could hack the form to change these values when they don't have the permission. In which case it basically makes that permission invalid.
Yes, the rc4 release seems to fix this issue when I remove the variable from my custom theme. Thanks!
Hrm, I can see where this would be problematic given that upgrade_status is using the @deprecated flag to determine problems, and if the site is already in D10 those things have already been removed. So it has nothing to test against.
So probably no way to do this. I guess we really just need to stress to sites to scan modules on their D9 site first and not just edit their custom module info.yml files to D10 thinking it can be scanned later.
That patch seems reasonable to pull the Alt text from the mapped field from the DAM. In our case we didn't have any Alt Text field in the DAM itself and just created a preprocess function to use the Filename (without extension) as the Alt text for now.
Correct, the Alt Text is definitely not set. We are setting that in a preprocess from the image name. So still get that added when you can. But without the alt text it is still rendering a responsive image that works.
Oh, hmm, I think it's actually working already! I had missed a step and had not set up the display view modes for the DAM-Image media entity itself. In that display mode, for the "Asset Reference" in the Format option, instead of using "Embed Core" I see an option for "Acquia DAM Responsive Image". When I select that and point it to the correct image style, now it all works.
Thanks for the quick answer. Look forward to seeing it and will be happy to test it. It's the remaining issue preventing us from using the DAM on our web site.
OK, I've updated the patch to work with any number of items with the IN operator. I think this is ready for review now.
Closing this issue as it appears to have been a problem with our front-end theming of the Load More button and the paginator markup missing some classes.
Found the bug problem with taxonomy terms. The existing code had a bug when adding the "AND" clause to the filter query. I think I can also see how to chain multiple terms using the AND operator, so will post another update for that soon.
Updated patch to get the value rather than key of array.
However, while this patch works for Content Type, I'm not able to get it working for Taxonomy fields. I have a `field_topic` and with this patch it properly passes a filter of `field_topic:123` and in Algolia I have a node with a value of 123 for field_topic but the result isn't returned. I just get no results at all. So something is still not quite right with taxonomy filters.
Added a patch that will at least support the "IN" operator when only one option is selected. This allows a basic Content Type filter to work when you just need a single content type since Drupal Views doesn't have an option for '=' operator on that field.
It appears that the SearchApiAlgoliaBackend::extractConditions function does not support `IN` queries. Would be good to document this as it prevents many common Views filters such as Content Type from working.
I've been confused on this as well, and I think it would still be useful in the project description to note that D9/10 has added some webp support to core and to better describe the additional benefits of this module.
In my case I simply added a "convert to webp" effect to my image styles in Core without using this module and it seems to work fine, so not sure what I'm missing (I don't need to support old browsers without webp support)
Honestly when I tried the latest version of the webp module I was running into weird issues with some png images not getting a webp version, whereas just using the core convert effect worked fine.
Looks like the gin_lb template was taken from claro, so maybe Gin or gin_lb is just missing the preprocess for this?
Looks like in the working rc2, it loads the template: core/themes/stable9/templates/content-edit/file-managed-file.html.twig but in the broken rc3 it loads the template: modules/contrib/gin_lb/templates/content-edit/file-managed-file--gin-lb.html.twig. So something wrong in the new template?
Also, this is on Drupal core 9.5.5
Inspecting the markup in rc3 I don't even see the file upload input element. Digging more into a comparison of the markup to see what has changed.
I think there are some additional colors that need tweaking for dark mode. In addition to the toolbar background, the toolbar text color needs to be set, along with the background of the select drop-down fields (like heading)
Oh, nope, doh, I was testing CKEditor4 and not CKEditor5. I can still reproduce this (in D9.5.x)
* Install and enable latest versions of `gin` and `gin_lb`
* Set Admin theme to Gin, and enable "Use Admin theme for editing content"
* In Gin Settings, set Appearance to Dark
* Enable CKEditor5
* Go into the Text formats and Editors config, select Basic HTML, set the editor to CKEditor5
* Create a content type that uses Layout Builder
* Create a page of that content type and go to Layout tab
* Add a Basic Block to the page. The sidebar config flyout will be in dark mode, but CKEditor5 will have a white background.
Hmm, just tried it in a fresh site and also cannot reproduce. Must be something specific to this other project theme that is bleeding into layout builder and gin. Although not sure how it would prevent the gin variables from being set correctly.
In any case, closing due to not reproducing.
Patch in #20 works great in D9.5.x. This is super useful and much better than the default behavior.
Also wondering what the current workaround for this is. The original patch in #24 was to provide a Json Normalizer for SectionData so the default_content module could export landing page nodes to be deployed across sites.
In the past, landing pages could be designed using Panels and Page Manager and exported to config and deployed across site environments. With Layout Builder, landing pages are Node content and the main way our clients deployed this content was via the default_content module. Now with 8.7 this has all broken and clients no longer have a way to deploy configured landing pages between environments (such as a search results page with facet blocks and other blocks placed on the page).
Is there any other way to restore a Json Normalizer that default_content can use to export a Node with Layout SectionData as JSON without exposing it to the world via Json:api?
Here is a tweak to the patch. This doesn't feel right at all, but it got me past the errors in default_content when importing the section json. default_content import is never calling anything in the SectionDataNormalizer so not sure if that's an issue with default_content or with the normalizer. Given that Sam mentioned he tested the previous patch with JSON get/patch it makes me think either default_content has a problem with json importing typed-data or that there is another core issue buried somewhere.
In any case, posting this patch so a client can try it and to get more comments.
Not sure if this is working for REST, but it's not working for default_content. This patch allows me to export a node with layout_builder and I see the proper json data in the export. However when I try to import the node json, I get:
In SqlContentEntityStorage.php line 783:
Value assigned to "section" is not a valid section
In SectionData.php line 31:
Value assigned to "section" is not a valid section
When debugging this, it's trying to assign an array to the "section" property rather than a Section object class. Doesn't seem like the denormalizer is being called in LayoutSectionItemNormalizer::denormalize. But maybe it hasn't gotten to that point yet since the debug stack shows it is trying to get the property value in order to determine if it's empty first.
Seems maybe related to this: https://www.drupal.org/project/jsonapi/issues/2955615 → in JSONApi, but not fixed in core itself.
Edited: Looks like this is what @samuel.mortenson was saying in #17, so sorry for the dup.
Here is a reroll of #14 against the latest 8.6
Adding this as a stable blocker as sites need a way to export landing page content for deployment to other environments. This was possible with Lightning Landing Pages with Panelizer and is needed for nodes using Layout Builder with modules such as Default Content. Default Content exports are empty without this serializing patch.
smiletrl: rather than pasting a huge amount of text like that, it would probably be better to upload it as an actual file.
bleen: I can confirm both of the issues you reported, so agree this needs more work.
One problem with this patch is that the removeItemSubmit public function added by this patch is incompatible with the same function name used in the entity_browser module (entity_browser/src/Plugin/Field/FieldWidget/EntityReferenceBrowserWidget.php)
I suggest using a different name here so it doesn't break existing sites.
Or, if entity_browser needs to update it's argument list, let me know and I'll submit that as an issue for them, but wasn't sure they would care about a conflict with a patch that hasn't been committed yet.