There are a lot of out of scope/not needed changes in MR https://git.drupalcode.org/project/ai/-/merge_requests/395, more seems like rebase issue etc.
MR: https://git.drupalcode.org/project/ai/-/merge_requests/248 has only one file change.
I would suggest to create a fresh new branch and cherry-pick changes from from 395 and 248.
Listing out the commits which needs to be picked:
https://git.drupalcode.org/project/ai/-/merge_requests/248/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
https://git.drupalcode.org/project/ai/-/merge_requests/395/diffs?commit_...
Above commits could have overlapping changes as well.
prashant.c → created an issue.
Did not work for me.
1. I have 2 vocabularies Tags and Test
2. Tags has no terms
3. Test has 1 term
4. It is not listing any of these, in addition to that "Use source vocabulary" also disappeared after switching to the MR branch.
It needs work.
Left some review comments, moving to NW. Kindly address those and move back to NR.
On the page /admin/config/ai/settings getting the following error:
Fatal error: Class Drupal\ai\OperationType\DocumentToText\DocumentToTextInput contains 3 abstract methods and must therefore be declared abstract or implement the remaining methods (Drupal\ai\OperationType\InputInterface::getDebugData, Drupal\ai\OperationType\InputInterface::setDebugData, Drupal\ai\OperationType\InputInterface::setDebugDataValue) in /var/www/html/modules/contrib/ai/src/OperationType/DocumentToText/DocumentToTextInput.php on line 11
Thanks for the great work done @joevagyok on this but there is no activity from last 6 months changing the assignee to unassigned.
@marcus
I have done the following:
- Rebased the branch, resolved conflicts
- Cherry picked commits from https://git.drupalcode.org/project/ai/-/merge_requests/574/commits, hope that is okay to prevent conflicts
prashant.c → made their first commit to this issue’s fork.
@ishani
I tried it on 1.1.x of ai and without even entering anything in these fields i got some response data. In addition to this also got the validation message but the form still got submitted it seems.
prashant.c → created an issue.
With the help of an AI code assistant attempted the solution, which does the following:
- Created a base plugin class (OperationTypePluginBase) that implements the OperationTypeInterface
- Created a custom discovery class (OperationTypeDiscovery) that:
- Discovers interfaces in the OperationType directory
- Creates corresponding plugin class names
- Maintains the existing interface structure
- Works with the new attribute discovery system
- Updates the AiProviderPluginManager to use the discovery system
Possible benefits of this approach:
- Maintains backward compatibility - existing code using interfaces continues to work
- Works with the new attribute discovery system
- Doesn't require moving or restructuring existing interfaces
- This could be replaced/rewritten when a proper plugin system for operation types is in place going forward
- Now discover operation types from interfaces
Needs a thorough review of the code.
Need phpcs fixes to let the pipelines pass and it should be good to go.
I tried multiple tweaks, but nothing worked. I think the operation types are not getting discovered because they are not plugins.
Can we go with the approach of 1.0.x:where it is working fine
https://git.drupalcode.org/project/ai/-/blob/1.0.x/src/AiProviderPluginM...
Raised the MR against 1.1.xby fixing the logic to handle the Ajax loading of models per provider. Changes need to be reviewed.
prashant.c → made their first commit to this issue’s fork.
Different regions on the same page make sense, but what if both blocks are placed on different paths?
In case the response block is not on the same page, there are errors throw,n and the user/admin does not know whether it is working or not
prashant.c → created an issue.
prashant.c → created an issue.
prashant.c → created an issue.
Thank you, @dcam, maybe I misunderstood the issue, so now hyphens are allowed whereas underscores are not.
Yes, I was just going through the files in core/lib/Drupal/Core/Config/Checkpoint. You are correct, the underlying API does not provide a way to change the label. Then, in that case, you may close this issue.
Thanks
prashant.c → created an issue.
prashant.c → created an issue.
prashant.c → created an issue.
Will try to add test coverage, if hyphen(-) is used in the machine name, it will get replaced by underscore(_).
For this we only need to replace -with _in the replace_patternand replacekeys of the machine_nameelement.
This will allow lowercase letters, numbers, and underscores, and the "replace" key will replace other special characters with _.
Please review.
prashant.c → made their first commit to this issue’s fork.
Thank you @jrockowitz. This is extremely helpful to have these listed at one place.
Not sure, but this https://www.drupal.org/project/ai/issues/3519196 🐛 Handle errors for API explorers when a provider not configured Active might be related.
The search module does not yet have any functional JavaScript tests. So, this is the only file for it. However, the search module has another test type, functional, kernel, and unit, where this won't fit AFAIK.
The issue mentioned in #7 🐛 Text formats "Embed Media" filter throws error Needs work was related to validation error itself. Added validation to check if the Medie embed filter is enabled.
No status message appears in the functional test after "Save configuration", which has the text The text format ckeditor5 has been updated.However, this status message is being displayed correctly on manual testing.
Based on this, I removed the code
$assert_session->pageTextContains('The text format ckeditor5 has been updated');
If there is some alternative to this, please suggest.
Thanks
Attempted to write the test, needs to be reviewed.
prashant.c → created an issue.
This issue is from the core search module itself. I have provided a fix for Drupal 11 in this MR: https://git.drupalcode.org/project/drupal/-/merge_requests/11911. An attached patch → could be applied to your Drupal 11 core to have it fixed until it gets merged to core.
Added functional test coverage https://git.drupalcode.org/project/drupal/-/merge_requests/11911/diffs#4...
Ah, sorry, my bad, I somehow misunderstood The proposal was to only show the module's machine name if it is materially different., thanks for clarifying.
Works well otherwise.
Thanks
@nicxvan
I am on branch 3250376-uninstall-page-showswith the latest code pulled, and I am still seeing the module names on the "Required by" section like the following ones:
Required by: Text, Block Content, File, Text Editor (editor), CKEditor 5 (ckeditor5), Comment, Datetime, Field UI, Node, History, Image, Link, Custom Menu Links (menu_link_content), Menu UI, Options, Shortcut, Taxonomy
Required by: Custom Menu Links (menu_link_content), Menu UI, Shortcut
A review of the current code is needed before moving forward with this. After that, we can go ahead with writing tests for this.
- Type "modal" worked fine.
- Fixed issues for type "off_canvas"
- Handled for the "off_canvas_top"
To quickly test, return the following code in a block or controller:
Open link in a "modal" window:
$url = Url::fromRoute('entity.node.canonical', ['node' => 1], ['absolute' => TRUE]);
$link = Link::fromTextAndUrl($this->t('Read more'), $url);
$link->openInRenderer('modal');
return [
'read_more' => $link->toRenderable(),
];
Open link in an "off-canvas" window:
$link->openInRenderer('off_canvas');
Open link in an "off_canvas_top" window:
$link->openInRenderer('off_canvas', ['position' => 'top']);
Have just created an MR from the patch submitted in #18 → . Going to test these methods locally first.
Thanks @sdhruvi5142 for testing the changes, code review also needed for this, changing to NR.
prashant.c → changed the visibility of the branch 2944554-allow-easily-creating to hidden.
prashant.c → made their first commit to this issue’s fork.
Was trying to find occurences of
$url->setOption('query', \Drupal::destination()->getAsArray())
and $url->setOption('query', ['destination' => 'other/path'), could only find one, pushed that change.
Tests were added, so I am removing the tag for now.
prashant.c → changed the visibility of the branch 10.4.x to hidden.
I just did a keyword search in the codebase, and a few files in which this could be used are:
- https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/block...
- https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
- https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
- https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/conte...
There could be many more like these.
- Created a branch for 11.x, MR 3520385-route-not-found-11.x
- Added changes in the
core/modules/search/src/Form/SearchPageFormBase.phpinstead of thecore/modules/search/src/Form/SearchPageAddForm.php - Used the https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Routing%2...
Needs a review, and then the changes could be backported.
Great find. Was able to replicate easily on 11.x as well and got the following error, while creating any type of search page (node, user and help):
The website encountered an unexpected error. Try again later.
Symfony\Component\Routing\Exception\RouteNotFoundException: Route "search.view_content" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 214 of core/lib/Drupal/Core/Routing/RouteProvider.php).
Drupal\Core\Routing\UrlGenerator->getRoute() (Line: 290)
Drupal\Core\Routing\UrlGenerator->generateFromRoute() (Line: 105)
Drupal\Core\Render\MetadataBubblingUrlGenerator->generateFromRoute() (Line: 773)
Drupal\Core\Url->toString() (Line: 176)
Drupal\Core\Utility\LinkGenerator->generate() (Line: 104)
Drupal\Core\Render\Element\Link::preRenderLink()
call_user_func_array() (Line: 107)
Drupal\Core\Render\Renderer->doTrustedCallback() (Line: 848)
Drupal\Core\Render\Renderer->doCallback() (Line: 393)
Drupal\Core\Render\Renderer->doRender() (Line: 203)
Drupal\Core\Render\Renderer->render() (Line: 491)
Drupal\Core\Template\TwigExtension->escapeFilter() (Line: 184)
__TwigTemplate_cb3fcbd5c3a204683387ed7f057a01d2->doDisplay() (Line: 388)
Twig\Template->yield() (Line: 344)
Twig\Template->display() (Line: 359)
Twig\Template->render() (Line: 51)
Twig\TemplateWrapper->render() (Line: 34)
twig_render_template() (Line: 369)
Drupal\Core\Theme\ThemeManager->render() (Line: 452)
Drupal\Core\Render\Renderer->doRender() (Line: 465)
Drupal\Core\Render\Renderer->doRender() (Line: 465)
Drupal\Core\Render\Renderer->doRender() (Line: 203)
Drupal\Core\Render\Renderer->render() (Line: 242)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 599)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 235)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare() (Line: 131)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse() (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray() (Line: 246)
Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}() (Line: 206)
Symfony\Component\EventDispatcher\EventDispatcher->callListeners() (Line: 56)
Symfony\Component\EventDispatcher\EventDispatcher->dispatch() (Line: 188)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 715)
Drupal\Core\DrupalKernel->handle() (Line: 19)prashant.c → made their first commit to this issue’s fork.
- Rebased the merge request and resolved minor PHP CodeSniffer issues.
- The current permission name, "View own account details", is too generic for its actual purpose.
- This permission specifically controls access to the roles section on the user account edit page.
- Even without this permission, users can still view and edit their own account details.
- Ideally, if we retain the current permission name, it should only apply to the "/user/[uid]" page, and not to "/user/[uid]/edit".
- In my opinion, for better clarity and purpose alignment, a more appropriate permission name would be something like "View user roles" or "View own roles".
prashant.c → made their first commit to this issue’s fork.
- Rebased the branch with 11.x
- Reviewed the changes, was able to search the module by machine name as well as the human-readable name
- Attaching the GIF
- Keeping in NW as Accessibility review and change records are pending
prashant.c → made their first commit to this issue’s fork.
I have struggled with this a lot while uninstalling a module from the UI, it confuses when the name of the module is totally different from its machine name; you mostly go back and double-check it.
Having this added to the core would be very helpful to the site builders.
prashant.c → created an issue.
Was looking for the generic methods for the same. I have raised an MR with some additional checks, like the path should be internal only. Once this is reviewed, we can go ahead with writing tests also.
Current changes need to be reviewed
prashant.c → made their first commit to this issue’s fork.
I came across this comment https://www.drupal.org/project/drupal/issues/3028868#comment-13812371 → , which highlights that the Drupal image module does not support SVGs for a variety of reasons. Given this, it is unclear whether the current issue is still relevant.
Consequently, it appears that utilizing the SVG Image module might be a more suitable alternative, as it offers a widget and formatter for handling SVG fields.
@oakulm It would be great if you can remove the draft mode of the MR, however the change looks fine.
Kindly address the feedback, and it should be good to go.
prashant.c → changed the visibility of the branch 8.x-2.x to hidden.
prashant.c → changed the visibility of the branch 3049683-redirect-to-wrong to active.
Made some changes that need to be reviewed. Can't use setIgnoreDestination() as the redirect is not being done using setRedirect()
prashant.c → changed the visibility of the branch 3049683-redirect-to-wrong to hidden.
Changing the status to NW to address the feedback comments and a suggestion by #14 🐛 Redirect to wrong URL when "destination" query parameter is set Needs work
I am not sure whether it makes a difference because this text will be displayed in the UI /admin/config/ai/ai_image_alt_text in the field "Alt text generation prompt" and there it would be readable anyways.
By removing \r it may cause issues on Windows, if someone could check on windows that would be helpful to decide whether to go ahead with this or not.
- Overall changes looks good
- Need to fix the warning
PHP Deprecated: Using ${var} in strings is deprecated, use {$var} instead in /var/www/html/modules/contrib/ai_image_alt_text/modules/ai_image_bulk_alt_text/src/Form/BulkAltTextGenerateForm.php on line 304 - Added some comments to the PR
- Unassigning from @codebymikey as there is no activity since September.
- Keeping in the NR
This is, in general, like whenever we give a prompt that returns a code snippet also along with a description, for example: "Give me a sample PHP code".
Whenever the prompt returns a code snippet, it should not be displayed as a single string, and in addition to the formatting of the response should go off.
You may fix the PHPCS issues in this one https://www.drupal.org/project/ai_search_block/issues/3515395 📌 Fix coding standards violations in AI Talk With Node module Active
prashant.c → created an issue.
@haritha_C
Please check under the "Failed jobs" on this URL: https://git.drupalcode.org/project/ai_search_block/-/merge_requests/11/p...
prashant.c → created an issue.
@scott_euser Along with this, it would be great if the same could be added somewhere in the README files.
It seems like these are the same: https://www.drupal.org/project/ai/issues/3511926 🐛 Validate selected vocabulary term as a valid language in AI Assistant - Translate Active . We can close either one.
Yes, you are correct this should also be put under the package
package: AI (Experimental)
, currently under AI despite having the lifecycle: experimental
@sirclickalot I tried user tokens with Advanced mode, those are working properly. It could be a possibility that your token itself is returning empty/null values. However, I have checked it on 1.1.xversion.
Still issues with special characters, please refer to the attachment:
Moving to NW.
prashant.c → created an issue.
This is an extremely useful feature to have. It is working nicely; however, one minor observation is that sometimes when there is text below the selected text, then the positioning of the button "AI assistant" is a bit off, however working properly.
I have posted some comments on the MR, it would be good if these could be addressed.
Tried replicating on the 1.1.x but seems to be already fixed there.