🇬🇧United Kingdom @scott_euser

Thanks for getting all these in and for the handy module :)

Here's an example in case someone ever wants to take this on https://www.drupal.org/project/geofield/issues/2969120 →

Icons are at the top level, e.g. via /admin/structure/types/manage/page/fields/add-field whereas 'Heading' is nested within 'Plain text' and doesn't need an icon.

(side note, I think he just meant Geofield as an example of how to add an icon)

I'll work on this once ✨ Enable gitlab-ci from DA and fix related issues Active is in. Thanks!

scott_euser → created an issue.

scott_euser → created an issue.

scott_euser → created an issue.

scott_euser → created an issue.

This is ready to go (though needs 🐛 Fix deprecations, Drupal 12 compatibility Active for the pipeline to be green)

'Oversight' is not entirely fair :)

In ✨ Continuation Add Views EntityReference filter to be available for all entity reference fields Active :

• #110 - This change was suggested by @catch
• #206/#207 - Breaking change flagged/explained a bit more because numeric allows more than entity reference, and valid things potentially

While those messages are just summaries of slack, we had a lot of discussion with @catch about it it and getting Entity Reference filter in without being used was agred to be the only realistic way to not have a breaking change for existing sites, so we had agreed to deal with the opt-in separately in follow-ups. The main candidate for follow-up was suggested to use 🐛 Views handler loading should respect configuration Active so that existing sites using numeric would respect the configuration and not automatically get swapped to entity_reference, but that turns out to be a big can of worms and ended up getting reverted.

For others stumbling across this issue, for now more sites are using https://www.drupal.org/project/views_core_entity_reference → to opt-in to it, or you can opt-in easily yourself as described on that module homepage.

Happy to help you if you need a hand with any of this, can also find me on Drupal Slack (scott_euser)

scott_euser → created an issue.

scott_euser → created an issue.

scott_euser → created an issue. See original summary → .

Absolutely not a problem, glad you managed

Going to hide the managed form display just to avoid future confusion (not delete of course in case ever revisiting UX).

scott_euser → changed the visibility of the branch 3257767-preselect-doesnt-work to hidden.

Thanks for all the work on this! Fixed it up a bit + added new test coverage that fails before this code and passes after.

I suppose that it's possible, though if we would do this, it should be optional in case the vector storage is paid by storage and the user has no purpose for it. E.g. It's also very likely the results would display the full entity in a view mode (eg card or teaser) rather than that of the chunk.

It may also be more difficult if the chunk does not have valid html due to chunking.

(setting back to active since no actual code here that needs work)

Thank you!

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

Could it be related to only 1 argument being supported and maybe you need 🐛 Multiple contextual filters / arguments are not supported when a value is not provided for them Needs work ?

Sure sounds good, if its okay I will wait until you have merged 📌 Gitlab CI template update Active since this will have some conflicts to resolve + can then double check that tests subsequently stay green.

Great thanks for reviewing, updated the info yml file, good to go then :)

Stylelint & Eslint are still disabled like the status quo, so maybe reasonable those stay in follow-ups.

For now started 📌 Gitlab CI template update Active as I think its first step either way, whether you decide its a hard blocker to do callable or not

scott_euser → created an issue.

Okay that works & tests pass locally on supported Drupal BUT I attempted to update gitlab-ci.yml because its not supported by Drupal 9 (and therefore also had to update PHP version because Drupal 10.3 (lowest supported version) does not support PHP 7.4). The error in current Gitlab CI setup is "You have requested a non-existent service "callable_resolver"" because it does not exist until Drupal 10.2.

So I reverted the change and left _gin_user_form_submit in.

I think there are two routes forward:

1. Postpone then and create a separate issue to update Gitlab CI template to latest from the DA applying your eslint/stylelint
2. Resolve any issues that reveals in further follow-ups (or potentially within Gitlab CI template issue if small)
3. Re-apply the reverted change and set back to Needs Review here

Or I could create two follow-up issues to avoid this getting blocked for a longer time:

1. Update gitlab CI template
2. Convert _gin_user_form_submit to call GinSettings directly

Yeah we have a couple issues related to the lesser used (but default) autocomplete widget that I'd like to get sorted. Is it a blocker for your drupal.org infrastructure updates @drumm?

(I'll try to have a review asap to list them here at least)

Ooh nice TIL, will update to use that then :)

That's what I meant by #25, callback is static so removes function but then loses ability to use $this so ends up statically loading itself - matter of preference which you think is the lesser of two evils...

Sure @jurgenhaas I have moved the contents of it over, I kept as public function, not public static function so $this can be used, figured that's better than a static callback and removing altogether, but fine to change to that if you prefer of course.

Thanks

Okay gave this a play! Seems like good improvement!

I'm not sure I am using it heavily enough so maybe needs someone who is e.g. using AI Chatbot with RAG for example to test this out.

When I used with custom rag implementation had to do a bit to get it to log as well due to the conditions in the View, but I think no change there, that's expected. When I used with AI API Explorer, I found a couple things:

1. I received 2 entries in the log for 1 request in 'Chat Generation Explorer'
2. If the request and response are small, the 'Summary' actually expanded on the response instead of summarizing it (openai 4o-mini). I wonder if there needs to be a threshold (strlen?) maybe where no summarize task is done?
3. LLM response in view log for me was markdown, so hard to read in that layout, but maybe supporting that can be a follow-up request and not part of this
4. (nitpick and not necessary) Not a big thing, but it seems like Queue + Process buttons are sub-tasks 'Processed logs' which could possibly be Local Action Links → instead to avoid any disconnect between the list & what needs to be done to populate the list

Beyond that I got fatal error because AssistantStreamIterator::getIterator() extends final method StreamedChatMessageIterator::getIterator() but I assume thats unrelated so I just manually changed to continue testing, but flagging in case.

But 1.1 here is requiring 1.1 of AI module which is where the interface change is right? So if you composer update to 1.1 here you'll get AI 1.1, and have no issue. Or if you lock to 1.0 and composer update you'll stay at AI 1.0 without the breaking change. So as long as you manage dependencies via composer you should never have any issues right?

Yeah does seem like duplicate.

Yep 10.1 so well before any currently supposed versions https://www.drupal.org/project/drupal/issues/3303067#comment-14888060 📌 Compress aggregate URL query strings Fixed

It does seem to be in Drupal 10 docs too https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Util...

This check let's the access continue if the user has either of these permissions:

• administer salesforce mapped objects
• administer salesforce

scott_euser → created an issue.

Only I wonder if we should do this without changing the method in case someone is extending... e.g. loop through raw embeddings to add

Okay this is now ready, let me know if it works for you

Just marking as 'Active' since there is no work in progress code

Okay got a chance to hit this issue myself and I I like the suggestion in #4. Since status quo is always random, nobody could yet have a dedicated link to a specific footnote, so I don't think we need to make this opt-in/opt-out. Added comments to the MR to make it clear.

scott_euser → changed the visibility of the branch 3450120-make-footnote-link-consistent to hidden.

Thank you very much! Schema validates well now for AI Content Suggestions sub-module!

(Just moving my notes over from Slack, Thank you very much for helping with this!)

Thanks! I don't think that solves it though, when I do config inspect, it still goes

ai_content_suggestions.prompts No schema

and when I run the test it still fatals on the checker:

Drupal\Core\Config\Schema\SchemaIncompleteException: No schema for ai_content_suggestions.prompts in Drupal\Core\Config\Development\ConfigSchemaChecker->onConfigSave() (line 91 of core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php).

scott_euser → created an issue.

While AVIF is better for compression, I am finding it hard to come to a conclusion as to whether AVIF is a good recommendation vs WEBP purely from an environmental impact of power consumption perspective. I.e., the power needeed to compress/decompress WEBP seems to be compared to PNG, but all I can find is random blog posts without any quantitative analysis that keep saying AVIF compress/decompress is more expensive, outweighing the savings from reduced data transfer/bandwidth. I really find it hard to find anything conclusive though - has anyone else looked into it from that perspective?

Thanks!

I guess the problem here is that it looks like a title and so user's ignore it. How can we update the design to have this be clear that its a link?

scott_euser → changed the visibility of the branch 8.x-3.x to hidden.

Confirmed yeah, we have been using this patch successfully for some time and it resolves the error when mapping to a date field. Thank you!

That's not the hard part though, the hard part is the upgrade path. Would have to batch bulk convert, update all existing code, twig functions, test coverage, etc, probably require and configure Config Ignore to maintain the status quo.

Yeah it's quite hard to do. I think you can use default content module perhaps (there may be other similar that convert content to config).

Okay thanks, will keep this open until someone can contribute a fix. Would expect drupal js behaviours not to get triggered until page is fully loaded so sounds unexpected to me yeah.

Yeah I don't think they can without some complex fragile code that hunts through all paragraphs, layout builder, XB to see where it's used, or alternatively an computationally expensive pre render of entire content item.

Would suggest you use the JS option which handles new elements appearing on the page e.g. via lazy builder/big pipe. That should render them.

I suppose on the twig side of things, given its just a textarea, the provider of that AI Prompt can decide to render it as twig. Optionally we could add helpers to help them render as twig, like part of prompt manager service.

Its a bit dangerous as evaluating as twig needs to be properly sandboxed I think... unless its maybe an option in the tableselect like 'Use the twig template for this type' (if the AI Prompt Type has opted into that), and if so, the twig template provided by the given module is used + the user can copy that twig template into their theme or own module to override.

Thanks for the contribution! The most common use case (I believe) is global settings for things that appear on all pages, so having published checkbox everywhere isn't ideal as it complicates the user interface when it normally does not matter (but of course that default for translation is unpublished is proper bug).

So we have a precedent for optionally showing/hiding things in the UI like this:

So I think on top of your current MR we should ideally:

1. Maintain the status quo of published being hidden - ie, update hook setting hide_published true
2. Let people opt-in to managing published status - ie, new hide_published in SiteSettingsConfigForm
3. Default to published for new translations if managing published status is disabled - I believe in SiteSettingEntity field status can be with ->setDefaultValue(TRUE)

Thank you!

I didn't review the code, but I tested this out with TMGMT and I can use both AI Translate and TMGMT functionality on the translate tab successfully.

Re #7 - that's a different functionality. TMGMT is for more advanced/complex translation workflows with shopping cart/review process. AI Translate should be compatible with TMGMT.

Ah thanks for pointing that one out! Just tested, works fine, no merge conflict.

Default: both TMGMT & AI Translate show
Untick new box: Only TMGMT interface is shown

Aha already exists 🐛 Warning: Undefined array key "#entity" in Drupal\ai_translate\Form\AiTranslateForm Active

scott_euser → created an issue.

MR ready, update hook maintains status quo. Changing settings rebuilds router so change takes effect immediately.

scott_euser → created an issue.

scott_euser → changed the visibility of the branch 2.0.x to hidden.

scott_euser → changed the visibility of the branch 3512340-translations-are-unpublished to hidden.

Thanks for the contribution however pipeline seems all green according to https://git.drupalcode.org/project/site_settings/-/pipelines

Oh absolutely contributions very welcome to help sort. I am only saying valid workaround. For now I did this in default core install with this module enable and blocks load fine:

1. Added 2 Blocks to the Admin > Content View at /admin/structure/views/view/content/edit
2. Created a Views Reference field in Article and left as default autocomplete
3. Attempted to create an Article and select a View, and I correctly get the two blocks

So it seems default use case works fine. Needs more info to reproduce in a clean install, error details, console errors, etc as appropriate.

Cheers thank you! Added a post update hook to clear the cache for the service args change just in case per https://www.drupal.org/project/drupal/issues/2743313 →

Merging, thanks!

Thanks for your comment

The pipeline issues are part of the main branch and unrelated to this: https://git.drupalcode.org/project/cloudflare/-/pipelines - this does not introduce new issues, so that should be resolved elsewhere.

The fact that update hooks update config is expected, you can read more on that here: https://www.drupal.org/node/2535454 →

If the issue itself is resolved for you (ie, no more flooded logs) after using the merge request, perhaps you could mark it as 'Reviewed & tested by the community' to help progress the issue.

Thanks!

The update now seems fine, my View doesn't get broken any more, thanks! Some comments:

1. I assume I shouldn't get this double raw + processed logs
2. We need some sort of help text here explaining what these things are... what does 'processed mean'? I go to it, its empty. I guessed maybe I need to mark something as processed (like reviewed)? but I open a log and I don't see such a button, nor in dropbutton, so I hit a dead end there
3. Is tabs the right interface? The typical interface for this would be Local Tasks I believe, like attached. In which case 'Delete' could also be a local task like it is for Drupal Core database logs rather than the more scary danger button?

Screenshot of what I see after upgrade:

Screenshot of local tasks in database log from core:

scott_euser → created an issue.

scott_euser → created an issue.

What I mean is on the entity you added the field to, go to manage form display, and change to the select widget instead. Then both the view and display fields are selects rather than the view field being an autocomplete.

Thanks! I opened 💬 Offering to maintain Footnotes Needs review and have reminders set for the 2 and 4 week marks to move it over to the projectownership queue and then add the follow-up comment per documentation.

scott_euser → created an issue.

Thanks for the MR, looks good - merging! Side note: if you do see something that could be improved in the class causing you to need to override, happy to consider as well.

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

• Okay I refactored to add GinSettings::userOverrideAccess() and updated the code to make use of that in GinSettings::get()
• GinSettings::get() continues to be used in gin_page_attachments_alter()
• I added an alter hook in there
• I added test coverage to ensure the alter hook works

Thanks for providing more detail.

Already completed

There should be fine-grained controls over which of the theme settings are allowed to be changed by the user and which aren't. Difference to what we have today: there is only one switch that either allows users to override theme settings or not, i.e. all or nothing.
This part should be implemented in Gin.
Following my comment #6 this needs some attention as there could always be a user setting around which isn't allowed to be made any longer, so then the global theme setting has to be applied.

Yes that's the case already with the MR + has test coverage here which I tried to explain in #13.

Here is the test coverage proving this case:
https://git.drupalcode.org/project/gin/-/merge_requests/575/diffs#586603... where:

Here is the code that ensures that only enabled settings will use the user override:
https://git.drupalcode.org/project/gin/-/merge_requests/575/diffs#411dd7...

Already completed

For Gin to allow this fine-grained control, it needs extra settings where the admin can decide which settings is accessible by users. This is done without permissions because Gin should work even without the gin_permission module being installed. But those settings are only available, if the gin_permission module is not available, otherwise that module takes over.

Yes that is the case, per screenshot of the UI:

To do here

Getting the current values for each setting and getting the setting if a setting field is available to the current user or not, both should be implemented in a service, and all code in Gin needs to call that service to get the values and whether the user can alter the setting.

Okay understood will refactor the existing code in this MR to cover this.

To do in Gin Permissions

The second step is then to make this permission based in the gin_permission module. Here is where the service comes in, that I mentioned above: in this central place, every time Gin needs to know if the user has access to override a setting, that service calls an alter hook with which the gin_permission module can override the default Gin decision with its own permission-based algorithm.

Okay created follow-up here: 📌 Create permissions per option that user can override Postponed

scott_euser → created an issue.

New release made, thanks!