🇬🇷Greece @idimopoulos

Let's focus in a single ticket. This is an active bug-ish thing. Closing this as a duplicate of https://www.drupal.org/project/drupal/issues/3050261 🐛 index.php randomly appears in friendly URLs Active and adding the relevant link for anyone that is interested in (google pointed me to this link first).

Note that according to https://www.drupal.org/project/drupal/issues/3050261#comment-13118805 🐛 index.php randomly appears in friendly URLs Active , there is a working workaround to use redirect module's forced clean URLs to automatically remove the prefix on request.

I am not adding anything to the argument, the patch has solved the problem for me too where I was setting the value of the product to 300 VAT included, and in the checkout, I was getting 300.01. Very bad experience for the client too. I am adding the patch that applies to 2.x.

NOTE: Some of the fragments of the patch in the BuyXGetY class were not there anymore. Code was removed. I discarded them. Take care whoever uses it. It might be that more is needed there.

The \Drupal\Tests\email_contact\Functional\EmailContactLinkFormatterTest::testFormatter should already suffice for the testing part.

idimopoulos → created an issue.

Thanks. Looks good.

idimopoulos → created an issue.

claudiu.cristea → credited idimopoulos → .

idimopoulos → changed the visibility of the branch 2381293-delete-action-10.2.x to hidden.

I think this is back to active since https://www.drupal.org/project/state_machine/issues/3058874 ✨ Allow same From/To state Fixed has been merged. Now that same from/to state transitions are allowed, they should adhere to proper check from the guard class.

This should include an update that goes through the roles and updates the permissions.

idimopoulos → made their first commit to this issue’s fork.

For reference, this is reproducible only if the user does not have the access to delete the entity removed. In web/modules/contrib/inline_entity_form/src/Plugin/Field/FieldWidget/InlineEntityFormComplex.php:914 there is a check already

    if (!empty($entity_id) && $this->getSetting('removed_reference') === self::REMOVED_OPTIONAL && $entity->access('delete')) {

and the delete checkbox is not added if the user does not have 'delete' access. This, checking either for the access or if the array key exists is a valid case.
I don't know if tests are needed as this is a warning and a simple missing check that does not affect the end result. The user does not have access, the entity is also not marked for deletion.

These are just failsafe's so I didn't provide tests. I don't think they are necessary.

idimopoulos → created an issue.

Fixed a php sniff.

I am making a slight change to check for the ID, not just the entity. The reason is that OG menu seems to break the layout builder which passes as context an entity without an ID.

idimopoulos → created an issue.

A but more precise with this:
I am defining 3 paragraphs. 1 for a single layout, 1 for the accordion, 1 for the accordion item.
Then I have in the entity a paragraphs revision for the paragraphs which is using layout builder.
Then I have the 1 column layout use the stable paragraphs.
And that is how it is indeed the problem is reproduced. The patch fixes the issue. +1 RTBC. Tested in vanilla Drupal

I prefer no to see any changes in a module configuration if it works like it did before

It is a fair explanation for me. I will leave it to the maintainer. I would still prefer the other way, but I don't have a reason to insist :) +1 RTBC

This is working as described but I would like to challenge the BC approach. In the MR, you are optionally saving the value in the config yaml file, in order to maintain BC. You are also providing a test that saving the settings will omit the value if it is the default (to test BC).
Why not simply provide an update path which update the config file to include the default value as a standard way of storing it? that way, you do not even need tests to verify the changes, and the projects adapting to it, can use tests that cover hook updates. I don't think that BC means you cannot change stuff. It only means that existing projects should work as they already do.
If there is a reason for that, then it is RTBC from me (apart from one typo nitpick).

We tried this in our project, fair request, good improvement.

idimopoulos → created an issue.

Good simple addition.

This turned out to not be really something that is worth intervening here. Core logs an exception by default and should not be part of this module to force avoid that.
Projects should either handle this explicitly or it should be handled in core. Closing.

idimopoulos → created an issue.

So, the MR does:
* Prevent saving and loading entities when their subject contains unsupported characters.
* Prevent attempting to load entities in the router parameter converter class, when the passed URI contains unsupported characters.

idimopoulos → created an issue.

@elber may I ask what you are trying to achieve? SPARQL is supposed to work with string IDs and more specifically, adhere to the RDF standard that the entity ID is a URI mainly.
Now, that does not mean that a resource URI cannot be a number, for example "<44>", however, it might be preferable to adapt the entity IDs to be string before they are passed in the method here. Further more, the code you provided, will not construct the <resource> resource URI (encapsulated with <>). Does this even work? I don't think it makes a valid RDF scheme to have data with simple numeric ID.
If you still need them to be numeric ID though, I think you should use some hook_entity_load in order to typecast the ID.

idimopoulos → created an issue.

idimopoulos → created an issue.

idimopoulos → made their first commit to this issue’s fork.

Straight fix.

Thanks for the work :)

idimopoulos → made their first commit to this issue’s fork.

Rerolled patch 66 for 10.1.x and 11.x.
However, I should note that this is a blind reroll. Did not check if there are any changes in core that might affect this.

idimopoulos → made their first commit to this issue’s fork.

idimopoulos → created an issue.

I have opened an MR in gitlab only so that anyone that needs it now and cannot wait for a merge here, can follow https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → in order to update. I used the patch from #7

+1RTBC here :)

Adding https://www.drupal.org/project/entity_legal/issues/3358135 🐛 Missing query access check WSOD Closed: duplicate as a related issue only because that issue is more specific to something that was already fixed in the current one without being in the scope.

Patch applies and works for us. Also, comments from #13 are addressed. Tests are green and applying.
+1 RTBC

Tested, works, +1 RTBC. The fix is pretty straightforward and a good improvement though I am not sure how much value does it since I doubt many are creating entities with string ID. Still, works for us.

As agreed, we will ignore 7.4. It will be removed. RTBC+1.

I added 1 more commit because there are cases for the inline editors that breaks the code.
This is for example when there are multiple text filter plugins, for example \Drupal\filter\Plugin\Filter\FilterAlign::process which adds the class using the dom element, thus adding it as a string.

That causes

        $variables['image']['#attributes']['class'][] = 'no-image-style';

to throw an exception. I am handling this case by splitting the class apart first.

Adding https://www.drupal.org/project/svg_image/issues/3321542 → as a related issue which was about the same thing mainly.

idimopoulos → made their first commit to this issue’s fork.

idimopoulos → made their first commit to this issue’s fork.

idimopoulos → created an issue.

idimopoulos → created an issue.

Straight reroll.

idimopoulos → made their first commit to this issue’s fork.

We should decide which patch to use. It is preferable though to go with Gitlab.
I added a small fix that covers a false negative due to a wrong type check.

idimopoulos → made their first commit to this issue’s fork.

Ugh... True... sorry, bad miss on my side. Added the message to the interface.

@webflo, the deprecation message has already been added. Please, check the Gitlab MR rather than the attached file. Hiding attached file.

Thanks for the work.