Account created on 29 November 2011, over 12 years ago
#

Merge Requests

Recent comments

🇫🇮Finland hartsak

Ok, here's another try. I tried to combine your patch with mine. Hopefully I didn't mess up something there...

And by the way, how to use the latest version in tests? Now it seems only "media_entity_dreambroker 8.x-1.x-dev" is available in the "Test with" drop-down?

🇫🇮Finland hartsak

Oh, you were fast! Nice that you have taken into account the translations too, I completely forgot about them.
I can test your approach too and provide a new patch.

🇫🇮Finland hartsak

Here's a simple patch for starters. Trying to use the media entity label for the title and providing a default value "Video" (translatable).
Also, noticed some problems in tests (unrelated) and tried to fix them too.

🇫🇮Finland hartsak

Thank you very much for the reply @drunken monkey!

It's just as you said - one of the fields enabled in the highlight processor was indeed a "Search api attachments" field. I hadn't paid any attention to those settings as I wasn't the original creator of that particular search index. That might very well be the reason why the highlight filter caused some performance issues in my case. I did a double check of that (it's always good to conduct testing in a production environment...) by enabling the highlight filter again and looking at the transaction data in New Relic.

After I selected these in the "Exclude fields from excerpt" options the performance issues were gone:
* rendered html output (might have very well contained some attachments in some cases)
* search api attachments fields (important!)
* taxonomy reference fields (not completely sure was this needed too, but it might have helped)

And just for the record I have the following module versions enabled:
* search_api: 8.x-1.31
* search_api_solr: 4.3.1
* search_api_attachments: 9.0.2
* Drupal: 10.1.6
* SOLR: 8.5.2

So I guess this ticket is no longer needed, as the problem is solved in my case. Thanks a lot for help!

🇫🇮Finland hartsak

In case this helps a bit more, here are all the items from the same slow stack. I realized I had previously only added one of these as an example but I don't know how to get this information out of New Relic in a better way.

Could it be that I have too many (fulltext) fields where the highlight processor should run?

  1. Drupal\search_api\Query\Query::postExecute
  2. Drupal\search_api\Entity\Index::postprocessSearchResults
  3. Drupal\search_api\Plugin\search_api\processor\Highlight::postprocessSearchResults
  4. Drupal\search_api\Plugin\search_api\processor\Highlight::addExcerpts
  5. Drupal\search_api\Plugin\search_api\processor\Highlight::getFulltextFields
  6. Drupal\search_api\Utility\FieldsHelper::extractItemValues
🇫🇮Finland hartsak

Just adding some findings here after a quick debugging. Maybe these will help in solving the issue?
In my case, it looks like the extra setting for "title" has been saved as a boolean value where the code assumes it is a string.

For example from the config yml file of my "viewsreference" field:

enabled_settings:
    argument: argument
    title: true

In this example the argument works as expected, but the title doesn't.

Because of that, in the node edit form, the "Include view title" field doesn't appear. The setting for the title doesn't get through in line 154 of file
viewsreference/src/Plugin/Field/FieldWidget/ViewsReferenceTrait.php

foreach ($enabled_settings as $enabled_setting) {
  if (!empty($plugin_definitions[$enabled_setting])) {

In addition to that, the setting for title doesn't work when it is passed forward with the $enabled_settings in the file
viewsreference/src/Plugin/Field/FieldFormatter/ViewsReferenceFieldFormatter.php

$view->element['#viewsreference'] = [
  'data' => unserialize($item->getValue()['data'], ['allowed_classes' => FALSE]),
  'enabled_settings' => $enabled_settings,
  'parent_entity_type' => $parent_entity_type,
  'parent_entity_id' => $parent_entity_id,
  'parent_field_name' => $parent_field_name,
];

Not sure yet how this could be fixed, as simply manually fixing the config file and doing a config import doesn't seem to help. I have added a quick hotfix that helps in my case, but this only helps with the symptoms not with the cause. So if others have the same issue too, you can try the hotfix but a proper fix would be better :)

🇫🇮Finland hartsak

Nice! It seems that the errors are gone when I use the patch from #12.
I tried with openid_connect_windows_aad 2.0.0-beta6 and without the "quick fix" from #4.

Thanks for the fix!

🇫🇮Finland hartsak

Hello @kekkis!
I finally had time and an opportunity to test the issue fork in my D10 (10.1.5) project. Works as expected: D10 compatible, both quality and lang parameters work. The videos also work without the additional parameters.

Thank you for your efforts in maintaining this!

🇫🇮Finland hartsak

Yes, I think that the "Manual mappings" textarea from openid_connect_windows_aad module works better for the long hashes used in Azure groups. Having those two keys sounds like a good idea if it allows us to continue using the textarea in the AAD settings for role mappings.

Unfortunately my project is currently in a state where I can't really do any debugging until I get the D10 update to progress further. I think I can still use it for testing some patches etc. when they are available if it is of any help!

🇫🇮Finland hartsak

Sorry about mixing things up a bit, but I just noticed that the patch in this issue also fixes a lot of Drupal 10 compatibility issues. Because this project doesn't have the Automated D10 patch, I guess we could use this issue to come up with a simple D10 support at the same time?

I checked the patch from #5 with Upgrade Status module and only thing missing from it is the version number. So here is another patch that should add it as well. After that this could be used for Drupal 10 upgrade too?

> -core_version_requirement: ^8.8 || ^9
> +core_version_requirement: ^8.8 || ^9 || ^10
🇫🇮Finland hartsak

Thanks for the new release @webflo!

I also tried the latest openid_connect_windows_aad 2.0.0-beta6 (with openid_connect 3.0.0-alpha2) and the same PHP error still persists when logging in as an Azure user.
Warning: Array to string conversion
in
Drupal\openid_connect\OpenIDConnect->saveUserinfo()
here -> openid_connect/src/OpenIDConnect.php

If I use the quick fix from #4 I get a new error with the openid_connect_windows_aad 2.0.0-beta6:
TypeError: Cannot access offset of type string on string in function openid_connect_windows_aad_openid_connect_userinfo_save() (line 70

At this point it looks like I can only get the logging in part to work without errors if I use openid_connect_windows_aad 2.0.0-beta5 and the quick fix from #4.

Maybe there's still something wrong with the role mapping (or something else) at least in my end. I have set the role mapping in the general (experimental) OpenID connect settings and I tried with and without them in OpenID AAD settings. My project is still D9 (I'm in the process of updating it to D10).

🇫🇮Finland hartsak

Thanks @mrinalini9! The patch in #5 seems to work.

🇫🇮Finland hartsak

Tested the fix in a clean project and it works. Thanks @scott_euser for the fix!

🇫🇮Finland hartsak

The quick solution in #4 helped in my case, thanks @bbu23 very good instructions!

🇫🇮Finland hartsak

Thank you for the quick reply @WalkingDexter !

I'm probably going to re-think if I really need the message feature in the future. In the meantime, I will continue using the 1.x version and see if I'm able to patch it with the D10 patch from here:
https://www.drupal.org/project/content_translation_redirect/issues/3369274 📌 Automated Drupal 10 compatibility fixes Closed: won't fix

Great work with the module so far and have a nice weekend!

🇫🇮Finland hartsak

Ok, another try.
The first patch was a failure as it would not allow using DreamBroker videos without lang and quality parameters. That should be fixed now. In addition, I tried to fix deprecated/broken tests. So let's see how this attempt turns out (fingers crossed).

🇫🇮Finland hartsak

Here is a quick fix that allows using just those mentioned parameters - "lang" and "quality".
Obviously, this could be made much better, but like mentioned, this is a quick fix :)

🇫🇮Finland hartsak

Thanks for the fix! I just encountered this issue in my project and the fix in #24 solved my problem.

Production build 0.69.0 2024