Is this an AI generated merge request? Please create separate issues with clear description of the problem, how to reproduce the bug, and how to fix, ideally with corresponding test coverage to ensure the fix remains in place. I can't accept something like this blindly, sorry
Looks like a valid test failure. I'm not sure if other implications here without digging deeper though so not confident about the direction of this at this time.
Looking at the code there it expects to find a 'body' tag. If you run your html through that what do you find as the markup instead? If we just prevent the error you'll end up with an empty translation.
Thanks! Gave this a test run and works well, getting merged. Will make a new release shortly with it.
We discussed something similar in embedding strategy change here 🐛 Flush index if Embedding Strategy is set Needs review . If we do this, we should make a big warning message show after saving if before & after change or I'm worried we will get head-scratching support requests only to realise people have done this not understanding the consequences which could increase the maintenance burden - perhaps the greater of two evils vs the dev just changing manually in configuration to work around this.
Was hoping to get a few more in, but had a pass at the issue queue and nothing close to ready, created a new release :)
Also, its already having D11 compatibility https://git.drupalcode.org/project/site_settings/-/blob/2.0.x/site_setti... which that patch seems to be re-adding. Maybe you are just on a really really old version? In which case if you're not upgrading would suggest you store your patch locally instead. Thanks!
Hi! Can you add some screenshots please so its easier to see what you are proposing as a UX? Then please change to Needs Review so it gets in my queue to look at. Thanks!
That MR (though closed) seems to have a ton of changes beyond the scope of the issue. But maybe you closed it for that reason. Willing to consider other MRs but ideally with upgrade test coverage (there is already some there as example to follow) as there are still a good number of sites on 1.0.x branch that haven't yet upgraded.
Thanks for raising!
Looks like this is from the original widget base. Would have to defer to Artem in case it has a purpose, but thinking maybe that the simplest step forward here is:
- Remove data-widget-settings from default base
- Add release notes to specify that plugins needing it should opt-in by extending ::actionButton() method and adding it
- Add example to the create.md
But will wait for Artem/others to hear if that will might cause some known breaking changes.
This seemed to sort the issue for us as well. Moving to NW because it needs an MR and I imagine some test coverage to demonstrate.
Priority-wise perhaps this is 'major' unless we consider disabling big pipe a valid alternative?
Thanks for this! We tested this out on a client's site and confirmed it solves the issue, and CK Editor 5 now loads again. This is quite critical since when the error occurs, the CK Editor instance is completely empty with no content, yet still loads the toolbar. A hard refresh shows the content is there before CKE loads, but once CKE loads it loads empty.
Best to focus on the AI module which has the OpenAI Provider. Response API is supported there.
See the module homepage.
Thanks for all the work on this in general. I would be keen to understand how to have a dialog be a modal form and submission of the modal form update the parent form. Looking at how Media Library does it in Core with an opener resolver is not very intuitive, so sort of hopefully that htmx make that type of thing easier to handle.
Okay made some progress:
- Became a maintainer of Field Widget Actions
- Landed ✨ Improve Field Widget Actions architecture with Form API in dialog Active (+ added some test coverage
- Working on ✨ Support select type widgets (select, tagify, etc) Active which this relies on
Steps (using the above #3563961 MR):
- Site builder adds FWA action to an entity reference field (working on supporting select, tagify, entity reference autocomplete, and entity reference autocomplete tags style)
- User clicks button:
- Modal window appears:
- Insert then fills in the parent node edit form <-- just this bit is almost there, but running out of time
What's an example setup for this? Is it via an automator? I think steps to reproduce in the issue summary and/or screenshot might make this more clear
There is more control via plugins extending FieldWidgetFormActionBase in terms of single/multiple filling in but I think its probably not that that you're referring to.
WIP but I'm running out of time today and might be a bit before I get back to it. Not ready for use yet (mostly the JS doesn't actually select things yet, but generally it makes the widget buttons now show up at least)
Okay moved out, added basic test coverage, made an alpha release https://www.drupal.org/project/ai_logging →
I did have to ddev restart for the file moves to be recognised but I think that's a problem people have noted in general for any of the moves.
Good to go for merge of deletion in 2x from my point of view.
Thanks!
I've got a 2.0.x dev branch running now https://git.drupalcode.org/project/ai_logging
Will give this a test and if its going okay will make an alpha release of it
I don't think this is worth you spending any more energy on, as you noted it can be overridden per plugin anyways, so not a big deal at all :) Sorry!
Yep sounds good to me, its not necessary to have that part. Thank you very much for the quick review & test! Good to go from my perspective. Thanks!
Maybe the fields drupal_long_id and drupal_entity_id can also be listed there too? It seems that this way the fields are on index, but they are not visible in admin UI, that is of course a good thing, as users don't really need to change them.
If you think it adds value, open to it. I am not sure the benefit at this stage, but sounds anyways like a separate issue, so we can focus on the language here maybe
If its hidden then it's not deletable though right? It should be deletable as language is not required for vector database as by nature it's language agnostic.
Re 'search_api_language' dk you just mean to change from the default Machine Name? The default from Search API is Language (machine name langcode). The alternative in the UI is Language (with Fallback) (machine name language_with_fallback). Is there a good reason to deviate from the default?
Thanks, will try to get back to retesting this asap... will also consider test coverage further
Thanks! I created an MR for ✨ Language metadata is not added to indexed items Active ready for testing/review
scott_euser → changed the visibility of the branch 1.2.x to hidden.
Because its not an interface I guess this would be a moduleExists check and if so, then a check if its an instanceof AutomatorBaseAction? Or should AutomatorBaseAction implement a new interface in AI Automators like AiAutomatorFieldWidgetActionInterface first over there and then implement it here?
Or could keep it simple and check if method_exists isAvailable() for now
Howabout if an action is enabled but then the automator is subsequently deleted. It seems in that case the FWA should have a config dependency on the Automator to prevent that case or we check that here as well and show some warning in the manage form display widget settings.
Okay this works now, added a screenshot to the issue summary + added in test coverage. Details in the issue summary note which MRs this contains (those should get in first - ie, schema fix + basic test coverage)
At least to have something that verifies the plugin system works, schema issues aren't introduced, etc
This naturally includes 🐛 Schema is invalid Active otherwise it results in schema errors failing tests.
Updating issue summary and title + downgrading to low and feature request per discussion in the MR
Need to figure out how to solve 📌 Passing a $options array to constraint constructors is deprecated, use named arguments instead Postponed in contrib to get tests to pass in gitlab CI (they pass locally only). That solution seems to work for core only.
Also watching ✨ Ignore phpunit deprecations already ignored by Core Active in Canvas where they haven't solve the same issue it seems.
It looks like its still throwing that deprecation (stumbling across this when I hit it myself). Moving back to needs work but will update if I manage to solve it elsewhere in contrib. Trying to see what's different from the Core fix: https://www.drupal.org/project/drupal/issues/3522497 📌 Passing a $options array to constraint constructors is deprecated, use named arguments instead Postponed
Also here, found via ✨ Improve Field Widget Actions architecture with Form API in dialog Active and so included there or test coverage doesn't pass, but its an issue on its own. Hopefully separating out this smaller bits makes it easier to keep the main feature request issue isolated to the feature better.
Separating this out from ✨ Improve Field Widget Actions architecture with Form API in dialog Active as its of course much simpler and unrelated to that functionality, but its included in there too otherwise test coverage fails.
Working for me, but very rough; will add test coverage into this and refactor/refine a bit. Fairly painful to get the form working and can see that Media Library in core (closest example I could find) had similar pain, with an 'opener' wrapper + messing with form state in the same way. Tried to follow that pattern in core as much as I could.
Not yet ready at all to review.
but if node access data could be indexed by the vector database that would further optimize the entity access check because irrelevant nodes would be filtered out in query phase.
Most vector databases have some sort of metadata options, but we never managed (or tried) to normalise the filtering layer so the support is quite patchy. I doubt it would be possible to properly normalise because A) the support e.g. in views filters is far more than a lot of vector databases allow and B) each vector database does it so differently.
What could be a potential follow-up is having vector databases have some support flag to say e.g. "Hey I support entity access checks" and have the bypass access checks in the back-end then occur if its already happening at the vector database level. Then a specific VDB provider could implement, but I suppose its only worth the effort building in that option if there is a VDB provider ready to do that.
Moving back to needs work for tiny comment + if this MR can be to 2.x branch of AI Search please and we'll cherry-pick it back. Thanks!
Thanks for raising, we have quite a similar one here: ✨ Language metadata is not added to indexed items Active
For those coming across this per the doc page of Claude, its Voyage used for embeddings which is https://www.drupal.org/project/ai_provider_voyage →
Can potentially be a follow-up but should we target 2.0.x since that's the default branch? Similarly recommendations for ai_translate standalone module, etc would be for 2.0.x I believe
Hi @zero2one
Can you add in this test coverage please? We are nervous to rely on the module without automated test coverage.
Thanks,
Scott
In case someone stumbles across this, we found this error which ended up being a result of an issue in Simple Oauth. Posting it as a related issue in case it helps future travellers to find it!
We hit this as well after upgrading and can confirm your MR 172 from #3 sorts it out - thank you! Updated the title to try to make this a bit more findable + will cross-reference from things that do turn up in search to push to here so others can find it easier.
Thanks!
Okay cool thanks, on my list to work on then. Thanks!
Hi Artem,
So the intention is not to support the inserting into the fields (ie you don't want the ajax commands part of my suggestion?). Or is that still okay? I'm not suggesting any BC, just suggesting to improve the flexibility.
Use case for essentially a subform within the modal is here: https://www.drupal.org/project/ai_auto_reference/issues/3545461 ✨ Improve UX to avoid data loss and support moderation Active - ie, more complex decision making than just choosing something. But allowing the plugin to handle how it wants to submit with ajax commands also allows extending modules to do far more.
The multivalue thing was someone else hijacking the thread, had already suggested he/she create as a seperate issue if they feel something missing.
Thanks,
Scott
Running out of time this morning but hopefully that will resolve the test coverage. Could use a check from one of us here again to get an RTBC if so. Thanks!
Have tried to have a bit of a pass on the issue queue (still more to get through) but it doesn't look ready for stable. There are quite a few feature requests which should likely go into https://www.drupal.org/project/viewsreference_extras → but there are also a number of bug fixes which I haven't been able to sufficiently check are legitimate or not.
For now added a couple more to the issue summary.
Will try to get out a new beta release soon in the meantime
scott_euser → changed the visibility of the branch 8.x-2.x to hidden.
Thank you! Gave this a manual test run, rebased into MR and pipeline all green now too. Merging. Thanks!
This was fixed in the main branch but granting credit, thank you!
scott_euser → made their first commit to this issue’s fork.
Thanks for the work on this! Test coverage is failing though it seems.
Thanks for the contribution! Could you please make it as a merge request and add in test coverage? Thanks!
Haven't checked this but from the sound of it it seems like a feature request (just trying to get a handle on the issue queue and prioritise bug fixes)
I think this is outdated with latest facets 3x as stable. Feel free to disagree
I believe that filter was an earlier patch from ✨ Continuation Add Views EntityReference filter to be available for all entity reference fields Active but since that is now merged probably this is outdated. Feel free to disagree
Oddly this was stuck in rtbc, moving to fixed since was committed
Hmmmm yeah, actually its perfectly fine now with the issue merged into Core. Instead of search_api_reference it just uses entity_reference plugin. Can use BEF etc to get checkboxes and all the niceties. So actually maybe this is closed outdated? Self-face palm for not rechecking first!
I raised this in Slack in #ai-contrib and would like to work on it. Would however like to have general buy-in before putting in the time & effort refactoring and improving things.
Thank you! The warning on next minor is unrelated so added to merge train.
Hmm will take a look (probably next week), thanks for checking!
Oh just to clarify I'm talking about auto-numbering only. If you manually number the citations with actual values those will show in the preview.
Best to consider eg one of the symbols listed on the Wikipedia page https://en.wikipedia.org/wiki/Note_(typography)
The number itself won't work though as the WYSIWYG is unaware of how many other WYSIWYGs are on the page. You'll have to pick a generic character or perhaps unicode symbol (it's controllable).
Okay thanks, I created this issue in Gin: #3560487: Gin Top Bar styling is fully gone in 11.3.x →
Yes I suppose it could affect other themes or modules, so perhaps a general announcement about it could be helpful
Very true! This should be marked as 'Fixed' not 'Postponed', sorry!
I noticed this is breaking the Gin Top bar in Gin 5.x when in the admin theme. Should gin then go extra-extra-specificity to override this? Or would this be a follow-up in Core?
Probably true, but previous AI core generated drupal/ai_search 1x. With the sub-module fully deleted from drupal/ai in 2x branch it shouldn't then still do that any more (and if it does it should now be drupal/ai-ai_search after the namespace change) BUT there are commits on the drupal/ai 2x dev branch that still contained ai_search. So yeah probably not an issue, its me being extra cautious given there is no changing it once a release is made and doesn't hurt as far as I can tell (and I think your answers support that it doesn't hurt?)
But we created a 1x dev release first, not knowing the metadata in composer lock for the submodule would reference this drupal/ai_search 1x branch, so this sort of works like a 'conflict' right? I'm leaning towards more protective because once it's released it's not possible to change the release. It doesn't hurt right?
It clearly wasn't happy with not having 2x dev release first. Here is commit https://git.drupalcode.org/project/ai_related_content/-/commit/5b67214ef... which I wanted in place prior to dev release