Account created on 19 May 2010, over 15 years ago
#

Merge Requests

More

Recent comments

🇬🇷Greece vensires

Sorry to get in the middle here but I think we have to reevaluate the issue and please - anyone who knows otherwise - correct me.

The original issue summary states the following:

What I really like about the memcache Drupal module is that the site keeps working, even though the memcached server is down. This means that memcached is not a single point of failure within the architecture. When it's down, cache is unavailable and the site will not perform as good. But it will be still available.

From what I understand, that's only partially true.
If we check memcache's Installation chapter, we will see what we know as the most common configuration for websites using memcache. In this case, the website should be still operational even if memcache is down.

There are two other chapters though in that documentation where when memcache is down, the website should be down too: Cache Container on bootstrap (with cache tags on database) and Cache Container on bootstrap (pure memcache). It's the advanced configuration where the containers cache is also loaded into memcache. If memcache is down, website can't build the container cache and fails.

Same should happen for Redis too, I believe. If in our configuration, we omit $settings['bootstrap_container_definition'](lines 67-90), the website should still work even if redis is down.

That's not currently happening though, at least in 8.x-1.x. An error "RedisException: Connection refused in Redis->connect() (line 32 of .../redis/src/Client/PhpRedis.php). But we may catch this error so that an exception is not thrown. So maybe, just maybe, this is the exact thing we have to fix in this issue and nothing more.

🇬🇷Greece vensires

Thank you for the quick fix @ivnish

Do we have another issue for the list? This one maybe: 🐛 Multiple-value checkboxes do not render existing values Active ?
So that we continue the discussion there over that topic.

🇬🇷Greece vensires

I just tried this in a completely new Drupal 11 installation with nothing else installed but the Standard profile and this module.
I set the "Tags" field of the "Article" content type to use "Select or other" formatter. The rendered widget was this one; only the Other field:

After installing the "cweagans/composer-patches" package and my patch from above , I get the following widget:

In case you need it, I attach my configuration too.

🇬🇷Greece vensires

I think we need more feedback from other users…
The experience I have from my site is not the same you show in your gif.
I will try to reproduce in a simplytest environment next week and come back here.

🇬🇷Greece vensires

There is a small case this change was intended to fix 🐛 Multiple-value checkboxes do not render existing values Active though I doubt since it was altered in a completely different issue.
In any case, you might want to also check that issue too... just in case!

Thank you!

🇬🇷Greece vensires

The MR fixes the issue.
Attaching patch for use in composer-patches directive until this is fixed.

🇬🇷Greece vensires

vensires created an issue.

🇬🇷Greece vensires

Uploading patch from your MR @gromani14; just to link from composer.json patches directive until this is committed.
Marking as RTBC in the meantime.

In my case, there was no recorded error of this at any level: no apache log, no php log, no message log... Nothing!
The reason I searched this issue queue is that I discovered that when I set the entity to unpublished, the form submission succeeded. When I set it as published once again though, it failed with a HTTP Status 500. Since, in my website, this module was the only one affecting the published/unpublished flow (no other hooks/events existed) it was an easy fix... after I had eliminated the theme, Redis, Solr, custom code 😰

🇬🇷Greece vensires

I set this as RTBC since it really fixes the mentioned issue.

🇬🇷Greece vensires

Just some thoughts in order to make sure we have taken everything API-related into account:

  1. The entity API operation to check for labels is "view label". This was previously returning AccessResut::allowed() and using MR!13617 we add our own logic in there.
  2. For view label to not fallback to view the entity class handler must set viewLabelOperation to TRUE; something already done by the core user module.
  3. The view label permission is used at least by JSON:API module, the EntityAutocomplete element API and the entity_reference_label field formatter.

Though I agree with the rest of the code introduced into the MR, I don't really like the following code:

// Allow for the special anonymous user.
// Normally this user profile can't be seen, but it is exposed in JSON:API
// by default.
if ($entity->isAnonymous()) {
  return AccessResult::allowed();
}

The main issue of this module is that we don't have the username access as a configurable option. By transferring this issue to the anonymous users only, I think we are actually hiding the issue in a subset of affected roles. I would prefer to not have this code and instead only check that the current role has the required permissions.

As for the rest of the code:

// Require both 'access user profiles' AND 'view usernames' permissions.
$access_result = AccessResult::allowedIfHasPermission($account, 'access user profiles')
  ->andIf(AccessResult::allowedIfHasPermission($account, 'view usernames'));

Can you explain why we need an AND? Wouldn't OR be more suitable? That way we could even just check if ($account->id() == $entity->id()) {...} and immediately after check for either of the following permissions: administer users, access user profiles or view usernames.

As a last step, just to make sure that nothing breaks in current installations - that's the reason I turn this into Needs work - we could make sure to have a post_update hook to assign the new view usernames permission to all roles.

What do you think?

🇬🇷Greece vensires

Updating the issue summary with the public documentation shared by the bank.

🇬🇷Greece vensires

@emberhood, I have added your fix and credited you in the 2.0.x version.
Have you checked whether we need something extra in https://git.drupalcode.org/project/commerce_eurobank_redirect/-/blob/2.0... too?

🇬🇷Greece vensires

vensires made their first commit to this issue’s fork.

🇬🇷Greece vensires

Cleaning up the issue queue.

🇬🇷Greece vensires

Cleaning the issue queue.

🇬🇷Greece vensires

I have updated the bank via email. Waiting for a response.

🇬🇷Greece vensires

@platinum1, have you maybe reviewed this? If it's ok, you could set it as RTBC :)

🇬🇷Greece vensires

@pappis could you be kind enough of updating the MR with your fix? :)

🇬🇷Greece vensires

The original db_like() has been replaced by \Database::getConnection()->escapeLike() since marked as deprecated from Drupal 10. In case it exists, it should also get fixed by @davidwhthomas in his module.

Regarding the module, I think it was at last time someone took the decision on his own :-) I would really suggest having an extra active maintainer though, otherwise we would have to keep updating this MR.

For now, I take it up on me and close this!
I really appreciate though all of you who helped with this all these years; I have in my memory a lot of websites and applications where I had need of this functionality and you all were invisibly there to support my need :-)

🇬🇷Greece vensires

Added the missing "more" in the comparison between Varchar and Char.

🇬🇷Greece vensires

vensires created an issue.

🇬🇷Greece vensires

Taking into account the libraries depicted in https://h5p.org/node/99838, it seems that most don't support <div> but <iframe>:

While developing custom H5P libraries for a project of ours, I also used iFrame for two reasons:
a) I wanted my solution to coexist with other H5P content in a Quiz content type I had built
b) I liked the fact that my JS/CSS code wouldn't interfere with other code of the main page
More or less, the same reasons also described in that h5p.org node you mentioned in the issue summary.

If we are to hardcode the change from the iframe to div, I think a major release should exist. I personally find it better though to instead add a setting in the formatter in order to let the user decide whether iframe or div will be used.

🇬🇷Greece vensires

@catch, the only reason I discussed about the iframe -> div here is just because I thought you had tested your fix with DIV - I'm not fully aware of what hooks/events Opigno implements, that's why. It seems this isn't the case though.

As a result, since the problems described in the issue summary are partially fixed (Explan. The resource ... was blocked due to a MIME type mismatch), I totally agree with you on merging MR!38 and opening a follow-up issue which will be more specific for the Missing H5PIntegration settings. error!

Setting as RTBC!

🇬🇷Greece vensires

Unfortunately, it still doesn't work with Drupal 10.5 and latest H5P version (alpha6 or 2.x). All assets seems to get correctly loaded - so that's a good sign!

I still have the following errors though:
- I get an error"Uncaught ReferenceError: Drupal is not defined" which is easily solved by adding core/drupal as a dependency of the h5p.integration library.
- Immediately after the fix above, I get the following error: Uncaught TypeError: can't access property "H5PIntegration", settings.h5p is undefined for which the fix isn't straightforward.

From what I guess, it would work if the embed type of the H5P is "div" but

  1. Most libraries are "iframe" only
  2. The formatter still has iframe hardcoded
  3. Changing from iframe to div would require a major version release, locking websites like mine in H5p alpha3, where the issue will still be not fixed.
🇬🇷Greece vensires

@catch thank you for spending time on this! Really appreciate it!

H5P is a standalone module; it may usually be used along with Opigno but it definitely isn't a requirement (as in my case), so it's important that it works fine without it.
The only extra thing to have in mind is that the decided solution should still work with aggregation either enabled or disabled.

🇬🇷Greece vensires

@scott_euser I'm sure that no one ignored you :) But for a module of many downloads, it's normal that in some cases we expect two or three people to say that it's working for them in order to then set it as RTBC. For example, @randalv in #60 described a scenario where it's not working. It might be something not-related to this issue specifically but, in any case, the next one coming over should take it into account and decide whether it's ok to set it as RTBC or not - if he has the experience or the guts to do so.

I also did not get a clear answer on my question in #45 🐛 Non-translatable fields can only be changed when updating the current revision. Needs work ; but I think we can live without an answer.

So, let's keep the good news: It's RTBC! :)

🇬🇷Greece vensires

I add every possible issue I could find as related, just to make sure there is concentrated effort. Some already have a MR or patch, so we need to pick what's being fixed and needed in each case - or which are false positives (maybe #3200009?).

In my normal Drupal, non-Opigno, v10.5 installation, I personally resolved my issue only when I rolled back my H5P module version to v2.0.0-alpha3. I tried alpha6 and 2.x but none of the job did the job. I get some deprecation errors like Deprecated: Optional parameter $fileDir declared before required parameter $defaultLanguage is implicitly treated as a required parameter in /var/www/html/vendor/h5p/h5p-editor/h5peditor.class.php on line 376 but at least it works.

I tried comparing the changes between 2.0.0-alpha3 and 2.x but I can't say I found anything so specific other than the preprocess:false and minified: true specifications (check screenshot). But I think this should affect the jQuery library only; not the assets found in the sites/default/files folder...

🇬🇷Greece vensires

MR is now properly rebased on top of 5.x.
@ivnish or other, may we set it as RTBC? From me it's a yes of course.

🇬🇷Greece vensires

I am wondering if a form/structure like the following would be a good fit + how much performance cost would it cause...
So the URL stays as it currently is but on top of that, we check from the matching sources/aliases the query arguments.

🇬🇷Greece vensires

The truth is that as long as Drupal people (in general) decided that a specific side or stance should be taken and this decision was forced on the main Drupal website using a specific banner, it's easily derived that it is related to the Drupal community too.

🇬🇷Greece vensires

Adding related issue just for reference.
According to the issuer, patch from #3473891-11: WSOD: Call to undefined method ...::getEntity() solves the issue if applied to field_formatter_class module.

🇬🇷Greece vensires

Resolved conflict and rerolled.
The conflict was related to the main module not supporting Drupal version 9 anymore. So it should normally be RTBC.

🇬🇷Greece vensires

Change links to match the 11.x version to avoid error messages displayed on older pages ( https://api.drupal.org/api/drupal/core%21modules%21datetime%21src%21Plug... )

🇬🇷Greece vensires

Adding patch working with v8.x-4.x-beta6 versions due to changelog https://www.drupal.org/node/3458551 .

🇬🇷Greece vensires

Thank you both for your feedback. I am sure I wasn't clear enough but my question was actually focused on this issue's context.

💬 Support to use the datetime-local element Active was focused on the DateTime Element type as used in a render array. On the other hand, this exact issue was created in order to address . An example code came from Webform's created/completed/changed columns as displayed in https://git.drupalcode.org/project/webform/-/blob/6.3.x/src/WebformSubmissionListBuilder.php?ref_type=heads#L887.

🇬🇷Greece vensires

If what @vortexcentrum says is true, I would like to find the exact commit this was fixed in. I couldn't find any recent changes to the same files the patches here proposed.
Maybe the fix happening in Drupal v11.2 is related to one of the following issues but I might be completely off: 📌 [11.x] php 8.3 follow up for enum in Datetime range formatter Active , 🐛 Date based template_preprocess BC code is incorrect Active , 💬 Support to use the datetime-local element Active

Finding the exact commit that fixed the issue might also help generate a future-compatible patch for still supported earlier versions.

🇬🇷Greece vensires

I also agree we could use the submodule ckeditor5_plugin_pack_font from the "CKEditor 5 Plugin Pack" module for this functionality.

🇬🇷Greece vensires

I am trying to identify what should be done and in which module...
I scanned pathauto's code and it seems the only thing actually needed is the following code:

\Drupal::service('pathauto.generator')->updateEntityAlias($entity, 'update');

So, we could directly invoke this call with a proper if pathauto module exists then condition. The question already rose in #2872697-23: Stop saving an entity when it gets added to or removed from a group by @berdir but it's not yet clear what should be done exactly since this might affect other custom or contrib modules we're not currently aware of.

I currently vote for a submodule for the pathauto implementation. It could event get installed by default as part of an upgrade if the module is enabled in the installation. This submodule could also work as a best-practice example for any other module to solve the same problem in its own context.

🇬🇷Greece vensires

Hm... Back to Needs work then...

🇬🇷Greece vensires

@axioteo I checked your code and seems to be correct. We already only support Drupal versions greater than 10.2 though, so we don't actually need the compatibility layer. I have done the required change in MR. Uploading patch too.

Please validate it's working as expected and I will gladly generate a new release immediately afterwards.

🇬🇷Greece vensires

For a website of ours, I use this fulltext facet filter to search for keywords. The client asked that whether user searches for "foo bar" or "bar foo" the returned result should be the same. I tried playing around tampering the query using proper Search API Events but instead fell back on changing the original patch from this issue's MR (attached) so that the 'LIKE' operator uses multiple conditions instead using AND.

I'm sharing this here too in case we would like a different approach for this or propose a different operator... Maybe...

if ($operator === 'LIKE') {
  $or_condition_group = $this->query->createConditionGroup('AND');
  $tokens = preg_split('/\s+/', trim($this->fulltext->getSearch()), -1, PREG_SPLIT_NO_EMPTY);
  foreach ($tokens as $token) {
    $or_condition_group->addCondition($this->facet->getFieldIdentifier(), $token);
   }
   $this->query->addConditionGroup($or_condition_group);
}
🇬🇷Greece vensires

Thank you @danrod! I see you haven't added the following line of code in the MR though. Is it intentional?

\Drupal::service('page_cache_kill_switch')->trigger();
🇬🇷Greece vensires

Since you provided the patch, I set it as Needs Review for someone else to approve this and set it as RTBC.
Can you also update the MR with the changes?

🇬🇷Greece vensires

Based on the previous two comments, I would take the chance to set it as RTBC but... Has anyone else experienced the I had to click on "Save configuration" twice for some reason issue described previously by @danrod?

Production build 0.71.5 2024