Merge Requests

More

Recent comments

🇮🇳India prashant.c Dharamshala

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.

🇮🇳India prashant.c Dharamshala

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

🇮🇳India prashant.c Dharamshala

@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

🇮🇳India prashant.c Dharamshala

A review of the current code is needed before moving forward with this. After that, we can go ahead with writing tests for this.

🇮🇳India prashant.c Dharamshala
  1. Type "modal" worked fine.
  2. Fixed issues for type "off_canvas"
  3. 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']);
🇮🇳India prashant.c Dharamshala

Have just created an MR from the patch submitted in #18 . Going to test these methods locally first.

🇮🇳India prashant.c Dharamshala

Thanks @sdhruvi5142 for testing the changes, code review also needed for this, changing to NR.

🇮🇳India prashant.c Dharamshala

prashant.c changed the visibility of the branch 2944554-allow-easily-creating to hidden.

🇮🇳India prashant.c Dharamshala

prashant.c made their first commit to this issue’s fork.

🇮🇳India prashant.c Dharamshala

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.

🇮🇳India prashant.c Dharamshala

prashant.c changed the visibility of the branch 10.4.x to hidden.

🇮🇳India prashant.c Dharamshala

Needs a review, and then the changes could be backported.

🇮🇳India prashant.c Dharamshala

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)
🇮🇳India prashant.c Dharamshala

prashant.c made their first commit to this issue’s fork.

🇮🇳India prashant.c Dharamshala
  • 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".
🇮🇳India prashant.c Dharamshala
  • 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

🇮🇳India prashant.c Dharamshala

prashant.c made their first commit to this issue’s fork.

🇮🇳India prashant.c Dharamshala

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.

🇮🇳India prashant.c Dharamshala

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

🇮🇳India prashant.c Dharamshala

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.

🇮🇳India prashant.c Dharamshala

@oakulm It would be great if you can remove the draft mode of the MR, however the change looks fine.

🇮🇳India prashant.c Dharamshala

Kindly address the feedback, and it should be good to go.

🇮🇳India prashant.c Dharamshala

prashant.c changed the visibility of the branch 8.x-2.x to hidden.

🇮🇳India prashant.c Dharamshala

prashant.c changed the visibility of the branch 3049683-redirect-to-wrong to active.

🇮🇳India prashant.c Dharamshala

Made some changes that need to be reviewed. Can't use setIgnoreDestination() as the redirect is not being done using setRedirect()

🇮🇳India prashant.c Dharamshala

prashant.c changed the visibility of the branch 3049683-redirect-to-wrong to hidden.

🇮🇳India prashant.c Dharamshala

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

🇮🇳India prashant.c Dharamshala

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.

🇮🇳India prashant.c Dharamshala
  1. Overall changes looks good
  2. 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
  3. Added some comments to the PR
  4. Unassigning from @codebymikey as there is no activity since September.
  5. Keeping in the NR
🇮🇳India prashant.c Dharamshala

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.

🇮🇳India prashant.c Dharamshala

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

🇮🇳India prashant.c Dharamshala

@scott_euser Along with this, it would be great if the same could be added somewhere in the README files.

🇮🇳India prashant.c Dharamshala

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.

🇮🇳India prashant.c Dharamshala

Yes, you are correct this should also be put under the package

package: AI (Experimental)

, currently under AI despite having the lifecycle: experimental

🇮🇳India prashant.c Dharamshala

@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.

🇮🇳India prashant.c Dharamshala

Still issues with special characters, please refer to the attachment:

Moving to NW.

🇮🇳India prashant.c Dharamshala

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.

🇮🇳India prashant.c Dharamshala

Tried replicating on the 1.1.x but seems to be already fixed there.

🇮🇳India prashant.c Dharamshala

Tested with 1.1.xalso, the issue exists, and the changes suggested by @rajab natshah fix the issue. Looks good to be merged.

🇮🇳India prashant.c Dharamshala

Since there hasn't been any activity on this for the past 5 days and we are seeking for input from the maintainers, I am unassigning @anjaliprasannan for now.

Feel free to reassign it to yourself once the necessary inputs are available.

Also, we might want to consider leaving the prompt field empty, as the help text suggests: "This prompt will be prepended before the user prompt. This field may be left empty too."

🇮🇳India prashant.c Dharamshala

Added a couple of comments, changing the status to NW. Feel free to change in case you feel these changes are not so important.

🇮🇳India prashant.c Dharamshala

Not exactly sure about the steps to replicate, but I also got this error when all the modules ai, ai_agents, and ai_provider_openai were not on 1.1.x.

I am on 11.x of Drupal core.

🇮🇳India prashant.c Dharamshala

Seems it got duplicated in favor of https://www.drupal.org/project/ai_agents/issues/3518174 🐛 Unable to create new Agent Active . Closing this one

🇮🇳India prashant.c Dharamshala

It seems to be an error with <?PHP label ?> property being NULL. We can assign an empty string if the value is null.

ai_agents/src/Form/AiAgentForm.php

Existing code:

  $form['#title'] = $this->t('AI agent: %label', [
      '%label' => $this->entity->label(),
    ]);

Updated code:

     $form['#title'] = $this->t('AI agent: %label', [
      '%label' => $this->entity->label() ?? $this->t('New AI agent'),
    ]);

This should fix the issue.

🇮🇳India prashant.c Dharamshala

@avpaderno

Is it possible for you to add https://www.drupal.org/u/nehajyoti back to QED42 as her current organization? It seems there was a miscommunication regarding this earlier.

Thank you

🇮🇳India prashant.c Dharamshala

Codewise we can use Dependency injection in the defined Action plugins, that is the only thing I found and then you may move it to RTBC.

🇮🇳India prashant.c Dharamshala

@bigbabert

A few observations while testing:

  • Bulk translated "unpublished" node, it created translations for all the defined languages, can we somehow have these languages configured so that it bulk translates to selected languages only
  • The translated nodes are getting published which is correct as per the configuration but the main node is still unpublished by selecting which I triggered the bulk translation, Should we consider publishing that one also?
🇮🇳India prashant.c Dharamshala

I am thinking of the perspective, if we have multiple nodes to be translated, we can bulk translate and may be provide actions, something like "Translate & publish", "Translate & keep current state".

🇮🇳India prashant.c Dharamshala

Thanks for the fix @vivek panicker I faced the same issue and applying the changes fixed it for me on Drupal 11.x.

However getting $url always empty https://git.drupalcode.org/project/ai_seo/-/merge_requests/3/diffs#529cdd96345bf7dcbbce6cbd38ccb30984c74ed4_232_229, maybe we could create another issue to address the URL not generating part.

🇮🇳India prashant.c Dharamshala

We can close this issue in favor of #3504027 Add a Generate Alt text VBO plugin to generate alt text for selected medias Active , where it is already being addressed.

🇮🇳India prashant.c Dharamshala

It would also be good to get some reviews from other community members. Changing status to NR.

🇮🇳India prashant.c Dharamshala

It would also be good to get some reviews from other community members. Changing status to NR.

🇮🇳India prashant.c Dharamshala

Somehow not able to see this new VBO action as an administrator:

  1. Downloaded and installed VBO module
  2. On the 3504027-add-a-vbo-pluginbranch
  3. Visited Media listing page admin/content/media
  4. Able to see other bulk operation actions like "Delete media", "Publish media" etc but "Generate Alt text" not listed

Not sure if i am missing something here.

🇮🇳India prashant.c Dharamshala

IMO, the integration with VBO could be added as a submodule since we already have the submodule ai_image_bulk_alt_text for this purpose.
Including the VBO action integration within the main module would introduce a dependency on the contributed VBO module, requiring users to install it even if they do not always need it while using the default features of ai_image_bulk_alt_text

🇮🇳India prashant.c Dharamshala

Tagging for steps to reproduce as well.

🇮🇳India prashant.c Dharamshala

I agree with #7 🐛 Automators: "Edit when changed" ignored in "Advanced Mode (Token)" Active .

Since multiple tokens might be used, checking each token for changes would be challenging. Some tokens, like date/time tokens, will always have a different value, making validation even harder.

Instead, we could refine the checkbox label and description to better communicate its function. Additionally, we can include a note explaining the implications of enabling this option. By default, the checkbox should remain disabled, allowing site builders to enable it based on their specific needs.

I suggest something like:

Checkbox label: "Auto-Update on Change"

Description for checkbox: "By default, the initial value or manually set value remains unchanged, even if the base text field is updated. Check this option to automatically update the value whenever the base text field changes.
Note: Enabling this may trigger multiple calls to the LLM, potentially increasing API usage and costs."

Thankyou

🇮🇳India prashant.c Dharamshala

I stumbled upon this issue and learned about the moderation process in AI calls. I also tried it in the explorer and read more about it in modules/contrib/ai/docs/developers/call_moderation.md.

While reviewing the code in the MR to understand the moderation workflow better, I noticed that since skip_moderation is a Boolean field, we can simplify this code

  if (!empty($configuration['skip_moderation']) && is_numeric($configuration['skip_moderation'])) {
      $this->skipModeration = (bool) $configuration['skip_moderation'];
    }
    else {
      $this->skipModeration = FALSE;
    }

to:

$this->skipModeration = !empty($configuration['skip_moderation']);

Thank you.

🇮🇳India prashant.c Dharamshala

I am trying to replicate, but it seems to be working fine:

  • Tried on Vanilla Drupal 11 instance
  • Database server with search index already configured
  • Configured the Milvus DB provider, which was connected successfully
  • Added a new server with "AI server" and in that "Milvus DB" was available
  • I have caching enabled, and while doing the above steps, I did not clear cache

Steps to reproduce: "Go back to search API index and expect to see VDB provider, but have to clear cache to see it." Are we expecting some new option in the Search index, in addition to the Search API server?

Am I missing something to reproduce this?

Thanks

Production build 0.71.5 2024