🇬🇧United Kingdom @scott_euser

Just had a quick look at the WIP and it seems its going down the key value store direction; ie, the option 1 from the issue summary (converted IS to ol from ul now to be clear) which I raised concerns about in #4. If you feel you really need a different storage than Media/File fields maybe its better to have load/save in a service so you can replace the service in your own way for your projects?

Updated issue summary per #2

Thanks very much!

I did a retest with:

1. Automatic enabled
2. Saved English and Spanish
3. Ran cron
4. Both PDFs updated as expected

Thanks! Manually tested and works well

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

Aha I see, thank you! Updated hook name, manually tested. Thanks

Thanks!

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

Works for me, Language no longer has an expand button, everything else that used to have an Expand button continues to have one. Thanks!

scott_euser → changed the visibility of the branch 3359619-ux-when-using to hidden.

New MR to 4.x (I assume that was the right thing to do too, sorry if not!)

Screenshot (issue described in #17 also no longer applies in 4.x it seems):

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

Thanks!

It's been flagged before that too many dependencies get loaded unnecessarily, maybe the better approach is:

• composer suggest to recommend additional dependencies
• hook requirements to prevent install until the required dependencies are in place

Hook install should be called on initial install for new installations (according to the docs at least) whereas hook updates are only called when a module is already installed and is then subsequently updated to a new version.

Yeah that sounds fine. At least for our sites we don't have custom plugins, so no BC for us. I can add it to next release notes in any case for the few other sites that are using the module that are not from us. Thanks!

Thanks! So this is 'Needs work' in that you want us to review & merge ✨ Add support for generating a separate PDF per domain Active first right? Or that you want to do more work on it first?

Thanks! So this is 'Needs work' in that you want us to review & merge the issue mentioned in #5 first?

So actually maybe it's better to just improve the plugin description and leave the title as is?

Very true and actually that's what it does, it boosts existing results but also potentially adds back in results that are otherwise excluded by the traditional search

Updated to fix merge conflict from latest 4.x.

Sure, I sent you a DM via Drupal Slack, but for now we have had to lock to 4.5.4 on the live site

This is great thank you! Nice solution

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

Thanks, makes sense; just checking with a colleague to see if any implications I am not thinking of.

I think the proposed resolutions are 'OR' right? If so I think any of options 2-4 are fine. I guess 2 & 3 would be least work for you but if you think others could benefit from 4, sounds great too!

On option 1, I am worried it will remove features. E.g. some things we have some clients using:

1. Translation of fields in/surrounding media, the UX let's them translate on the node page
2. Access control in members area auto-applying to attachments via Entity Usage
3. Ability for clients to e.g. maybe start with a quick page to PDF and use that for most, but then maybe more important annual report or something they decide to replace with an InDesign curated version

Thanks for your contributions in any case!

Will mark as needs work since its partially done (ie, homepage update)

Thanks!

Okay I have updated the homepage. I moved it to the end of the section instead as I believe the most common use of reading that text is for the average user who would be wanting to use the latest of all things rather than selectively upgrading Footnotes but not Drupal Core nor CKE (which was EOL June 2023 and must use a paid long term support plan).

I don't intend to do any updates to 3.1.x, so I have removed (or not added) that as well sorry. Unless someone comes forward to maintain that, I think it needs to stay that way - I don't want to give false hopes there.

Yes adding a 'conflict' section won't help because I can't retrospectively add it to any previous releases, so it'll just mean that composer suggests an older 4x version rather than latest 4x. I think instead we can add a hook requirements fail so if someone has CKE4 installed that it gives a warning.

Updated the issue summary to reflect the state of things now.

I suspect this will fix it, but will let tests run before merging.

Hi thanks for flagging - can you say your version of Drupal Core + version of PHP please? Asking since it has to do with constructor promotion and D11 + php 8.3 seems fine, so I suspect older version of core or older php causing issues. Should be easy to sort once I know those details. Thanks!

What's your suggestion for doc updates? At the moment the 3x to 4x upgrade section on the module homepage specifies cke5 a couple times.

I don't have the capacity to support cke4 myself (it's very different, has minimal test coverage, and tends to rely on a bunch of patches) so there isn't really an option to stay on cke4 as it stands unless you stay on older versions of Drupal Core.

Happy to make things more explicit + put more documentation elsewhere if you have ideas

Should it also be a conflict in the composer.json perhaps to prevent it altogether? Or too far?

Needs some test coverage updates to match the code change

Yep I agree with this - thanks for fixing it! Will just let the pipeline run and see if any corresponding test updates are needed

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

Is there a use-case where people would want to run cron in maintenance mode though? E.g. maybe doing maintenance mode pre-live as a sort of holding page. I think we should make this a checkbox in the configuration with:

- Update hook to make it disabled for existing sites
- Default config install to enable it for new sites

Thanks!

Thanks!

Thanks!

Solution:
The simple cron job, should update last run time even if it fails. The responsibility of making sure it doesn't fail belongs to the developer.

P.S. If this needs a separate issue, I can create that. It's a simple one, this.

Yes if you could create a separate issue please, as may need to also update test coverage for that (I haven't properly considered whether I agree with your logic yet, but happy to consider + get opinions of others in a new issue).

Merged into 📌 Automated Drupal 11 compatibility fixes for simple_cron Needs review and fixed

scott_euser → created an issue.

scott_euser → changed the visibility of the branch 3379107-simple-cron-should to active.

And thanks for raising in the first place, always helps make things better for the next person!

Sure no problem at all

Okay going to see if one of my colleagues can give this a test run in one of our heavier usages of the module next regular maintenance round (which unless there is a security release that affects us, is probably in the new year in fairness, in case someone else wants to review earlier).

Thanks for the work on this!

Had to fix a gitlab CI deprecation but updated the install process to also trigger the update now.

Question:
I am able to add the filters by adding 'field_name' but NOT the 'Related field_name..' as illustrated in the change record at https://www.drupal.org/node/3403710 → Is that expected? Maybe because this module successfully opted me in?

Yeah when I made those screenshots I had actually named the fields 'Related articles' that's all. At the moment in core, there is no option to choose between them other than programmatically (like this module does for you) until ✨ Configurable views filters to allow for different widgets Active .

Looks like this module attempts to remove the '_reference' but that didn't happen for me. I wasn't sure if this line was a factor:
if (
$filter['plugin_id'] !== 'entity_reference'

That condition ends in a continue; as in don't proceed if not entity_reference filter (along with the other criteria). From what I can see from your config, your code should pass that condition so I think it is more that the install hook should trigger the same update.

You said you resolved it manually, so that means you did not try the https://drupal.stackexchange.com/a/179811 - is that correct? I think yes.

Thanks, good spot! I know you just moved the comment over, but took the opportunity to explain it a bit better just for future me/us. I reinstated the return on no display ID in case someone is extending the field formatter for example. Doesn't hurt. Will merge shortly.

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

Would you be willing to entertain this feature? If so I would be happy to work on implementing it as an MR

scott_euser → created an issue.

Haven't looked at this in any detail, but to note there is https://www.drupal.org/project/sm → . I believe the idea is that the Queue system in Drupal can be leveraged and what is then used to processing queues is handled e.g. by that. In case there is any duplication of efforts

Not sure if you have gitlab comments enabled (I think off by default) so just adding that I replied to your reply. Thanks!

The str_contains closing bracket was in the wrong spot, but otherwise LGTM thanks!

Thanks I can see this being a really useful feature for some sites, thanks for your contributions. I added a couple comments.

That would be fine I guess, but in sites with lots of languages given its in some cases a page-as-you-go thing + a bit of an opinionated choice depending on the type of change made/configuration of the node type, I think it should be opt-in when configuring, e.g. at NodeTypeFormPdfPartialForm

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

Thank you!

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

Thanks for the details! Will get this onto the status page.

I think this is the core issue for it https://www.drupal.org/project/drupal/issues/2741877 🐛 Nested modals don't work: opening a modal from a modal closes the original Needs work and yeah i expect this is going to have to wait until core resolves it as we just use their modal.

You could check the ckeditor 5 embedded content module and see if they found a workaround for it in their v2. I remember they used to have a warning about that on their homepage but the warning is no longer there but from what I can tell at a glance it hasn't significantly changed it's code (ie, another example of modal from modal in cke5 context).

So probably nothing we can do here until it's sorted in Core. For modal view modes you could use an alternative text format that only has toolbar icons that are supported. That's what I do in the Footnotes module text format for example (also modal within modal is possible).

Could be one of two things.

1. Update hook not running on fresh install, maybe we need to call it from hook install, but for now you can run it manually https://drupal.stackexchange.com/a/179811
2. Your version of the patch differs from the update hook. There were so many versions of the patch over the years, contributions are needed to cover more versions (see module homepage) but for now if you check the source code, the test submodule has configuration for one 'as a Reference' version, if your version matches that then (1) should work.

Thanks

Nice, thanks for checking! Seems like worth merging this then + creating a follow-up in TMGMT core module for restarting/retrying failed machine translation jobs. Do you agree?

Thanks!

Thanks @berdir for the details. So it seems there are two issues then with the current MR correct?

1. Without categories does not work
2. With categories does work but sends too many events

Beyond that we also do not really use adwords so hopefully someone following this issue can also test that out.

With categories does work but sends too many events
For this bit, perhaps we can update this bit in the current MR then:

gtag('consent', 'update', data);

To conditionally only call it if its different from the original? E.g. when we loop through the categories here:

    for (let category in details) {
      if ('third_party_settings' in details[category]) {
        for (let prop in details[category].third_party_settings.eu_cookie_compliance_gtm.gtm_data) {

We could have some sort of flag that checks if there is a change from the original state, and have something like

if (hasChanges) {
  gtag('consent', 'update', data);
}

It has now been 15 days since the request for co-maintainership of https://www.drupal.org/project/simple_cron →

There is another issue open from https://www.drupal.org/project/simple_cron/issues/3453929 → but this was denied due to no security coverage.

Thanks for your consideration!

Postponed until SOLR issue is solved first

This is true for database search because of 📌 For AI Search combined with database search, results are reordered by new results are not added Active

But is not yet true for SOLR until 📌 Check that SOLR 'boost' of results finds results that are not found by traditional SOLR search Active

I think this probably is an issue actually. Currently using elevateIds but actually also need something like

q=(text:"my content query" OR id:(123 124))

Note: syntax could be wrong here...

Goal is to ensure that if 123 or 124 are not results in a SOLR query, but are in a Vector query, they end up as results in the combined query. The elevateIds is still correct as is and should be left in place.

Looking at the issue queue, I updated issue summary with what I think would be good to get sorted.

scott_euser → created an issue.

scott_euser → created an issue.

scott_euser → created an issue.

I think this is outdated now

Already in AiSearchIndexFieldsForm

Would be good to know if the issue still occurs since we merged 🐛 Broken Byte-Pair Encoding (BPE) Active

Yeah the processor is in search api, not views, so executing search API can use the boost. And maybe boost is the wrong word as (at least with database) you can get results that traditional search failed to find - so it combines as you say (haven't thoroughly tested solr, might need tweaking).

As long as you are executing search api in a chatbot with that processor enabled you'll get what you're asking already.

Screenshot of updated payment gateway exposed filter:

I couldn't see if there is automated test coverage for the existing Views Filter plugin for entity bundles. Ignoring that, do you want test coverage for this, if so, test sub-module within commerce_payment preferred? (since this relies on the Payment sub-module).

I'm happy it got in yeah!

scott_euser → created an issue.