Account created on 8 December 2020, over 4 years ago
  • Engineer Drupal - Backend at QED42 
#

Merge Requests

More

Recent comments

🇮🇳India annmarysruthy

Fixed remaining violations and phpstan issue. Kindly review

🇮🇳India annmarysruthy

Fixed remaining violations. Kindly review

🇮🇳India annmarysruthy

annmarysruthy made their first commit to this issue’s fork.

🇮🇳India annmarysruthy

@quietone Should we replace all existences of function_exists() with is_callable() ?

🇮🇳India annmarysruthy

Replicated the issue. The below changes in core/modules/comment/src/Plugin/views/sort/StatisticsLastUpdated.php could fix the issue:

<?php

namespace Drupal\comment\Plugin\views\sort;

use Drupal\views\Attribute\ViewsSort;
use Drupal\views\Plugin\views\sort\Date;

/**
 * Sort handler for the newer of last comment / entity updated.
 *
 * @ingroup views_sort_handlers
 */
#[ViewsSort("comment_ces_last_updated")]
class StatisticsLastUpdated extends Date {

  /**
   * The node table.
   */
  // phpcs:ignore Drupal.NamingConventions.ValidVariableName.LowerCamelName, Drupal.Commenting.VariableComment.Missing
  protected ?string $node_table;

  /**
   * The field alias.
   */
  // phpcs:ignore Drupal.NamingConventions.ValidVariableName.LowerCamelName, Drupal.Commenting.VariableComment.Missing
  protected ?string $field_alias;

  /**
   * {@inheritdoc}
   */
  public function query() {
    $this->ensureMyTable();
    $this->node_table = $this->query->ensureTable('node_field_data', $this->relationship);
    $this->query->addField($this->node_table, 'changed');
    $this->field_alias = $this->query->addOrderBy(NULL, "GREATEST(" . $this->node_table . ".changed, " . $this->tableAlias . ".last_comment_timestamp)", $this->options['order'], $this->tableAlias . '_' . $this->field);
  }

}

In the above solution, issue is fixed with fewer changes than the proposed solution in the issue: making $field_alias nullable and using the proper table (node_field_data instead of node).

🇮🇳India annmarysruthy

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies .

🇮🇳India annmarysruthy

Deprecated the block and removed it from theme and profile. still getting a few test failures in merge request !11835.

🇮🇳India annmarysruthy

@acbramley Can I skip deprecating the block and directly remove it? But what if the block is currently being used in sites.

🇮🇳India annmarysruthy

annmarysruthy changed the visibility of the branch 3518990-deprecate-syndicateblock to hidden.

🇮🇳India annmarysruthy

Hi @prashant.c, I was unable to replicate the issue. tried with openai(different models) and gemini.

🇮🇳India annmarysruthy

Raised an MR #2 for improving the readability of code snippets in the Chat Generation Explorer.

🇮🇳India annmarysruthy

Changed validation error message to "This value is not a valid extension name." Kindly review MR.

🇮🇳India annmarysruthy

Reviewed the MR. Changes look good to me. Also ignoring the sniff in files using testN() seems to be fine.

🇮🇳India annmarysruthy

I've raised an MR!11764 implementing the suggested solution for this issue. The PR refactors the HookCollectorPass::checkForProceduralOnlyHooks() method to make it more reusable by other components.

Kindly review.

🇮🇳India annmarysruthy

Since this is being used in core and contrib modules, Can I create a CR and add a deprecation notice for Drupal.behaviors.fillUserInfoFromBrowser.

🇮🇳India annmarysruthy

Given the potential GDPR implications and the increasing security expectations around personal data handling, Drupal.behaviors.fillUserInfoFromBrowser could be removed from form.js. I think this can be safely removed in a minor release as it doesn’t break core form functionality.

We could:

  1. Remove the fillUserInfoFromBrowser behavior entirely from form.js bypassing deprecations
  2. Split remaining behaviors into separate files with their own libraries
  3. Update dependencies in existing modules to reference the new libraries
  4. Add a change record documenting both the removal and restructuring
🇮🇳India annmarysruthy

Raised MR!11722. Updated getDefinitionsByType($type) to ensure type is always treated as an array by casting it with (array) to prevent WSOD.

Kindly review

🇮🇳India annmarysruthy

Was able to replicate the issue with steps mentioned in description.

The issue occurs because the source and target parameters are explicitly set to NULL in the ContentTranslationRouteSubscriber.php file while defining the translation add route. This results in incorrect route processing, leading to duplicate "Add" links in the breadcrumb and an unexpected error when clicked.

if ($entity_type->hasLinkTemplate('drupal:content-translation-add')) {
  $route = new Route(
    $entity_type->getLinkTemplate('drupal:content-translation-add'),
    [
      '_controller' => '\Drupal\content_translation\Controller\ContentTranslationController::add',
      'source' => NULL,
      'target' => NULL,
      '_title' => 'Add',
      'entity_type_id' => $entity_type_id,
    ],

Removing the source and target parameters from the route definition allows Drupal to handle them dynamically based on context, preventing duplicate breadcrumb links:

if ($entity_type->hasLinkTemplate('drupal:content-translation-add')) {
  $route = new Route(
    $entity_type->getLinkTemplate('drupal:content-translation-add'),
    [
      '_controller' => '\Drupal\content_translation\Controller\ContentTranslationController::add',
      '_title' => 'Add',
      'entity_type_id' => $entity_type_id,
    ],

This change removes the duplicate "Add" breadcrumb links.The breadcrumb structure will now be:
Home > Page Title > Translations.
this ensures breadcrumb consistency with how Drupal structures breadcrumbs in other areas, such as when adding a content type (Home > Administration > Structure > Content types).

Would love to hear thoughts from others on this approach

🇮🇳India annmarysruthy

Added a check for empty vocabulary as per review comment. Also validation will occur only when translate is enabled in CK Editor.

Thanks for the input @mrdalesmith. Kindly review changes.

🇮🇳India annmarysruthy

Added validation in CkEditor AI tools - Translate settings. When a vocabulary is selected for translate settings, validation checks if the terms in selected vocabulary are languages supported in drupal. Otherwise an error message with the invalid terms is displayed.

Kindly review and test.

🇮🇳India annmarysruthy

Resolved all comments and resolved conflict in MR. Kindly review @borisson_.

🇮🇳India annmarysruthy

Reviewed and tested the MR. In configuration -> AI -> AI translate settings, an accordion 'Translation default behaviour' is now present. A checkbox 'Mark new translation as unpublished' is present

Tested Scenarios:

1. If checkbox is enabled and no content workflow applies to a content type, the translated node will be unpublished irrespective of state of source entity.
2. If checkbox is disabled and no content workflow applies to a content type, the status of translated node will be the same as the status of source entity.
3. If a content moderation workflow applies to a content type, then status of translated node will be same as default state in workflow.

Moving to RTBC.

🇮🇳India annmarysruthy

On applying the current MR, the automated field values update on each node save if 'Edit when changed' checkbox is enabled. This is not a recommended practice. Also the checkbox is labelled 'Edit when changed', the user is not warned that content of automated fields changes everytime on node save.

Proposed Resolutions:

  1. Retrieve tokens used in prompt and check if token values used have changed - This is a tough one
  2. On selecting 'Advanced Mode(Token)' ,Hide 'Edit when changed' checkbox or Rename the label and description of checkbox, so that user will know that on enabling the checkbox, the automated field value changes on every edit and this adds to load'
🇮🇳India annmarysruthy

@borisson_ made change as per one comment and for the other one, added reply in MR. Kindly check. Thanks for the review

🇮🇳India annmarysruthy

Tested the changes and Changes look good. Moving to RTBC.

🇮🇳India annmarysruthy

Reviewed MR !490. Tested the functionality and disable option is working fine.

I have a concern about re-sort option. Once we disable a field in AI Automator Run Order, On clicking the re-sort the disabled field is enabled and sorted. Shouldn't we only consider enabled fields while re-sorting?

Example scenario: If there is a large number of fields in 'AI Automator Run Order' , user disables one or more fields and needs to re-sort the enabled fields.

🇮🇳India annmarysruthy

Reviewed the MR. Issue is fixed. Thanks @anjaliprasannan

🇮🇳India annmarysruthy

@jbuttler Icould not replicate the issue. I followed these steps while trying to replicate the issue:

  1. Configure the AI Assistant to use RAG access check
  2. Index content type 'article' (5 items)
  3. Login as a non-admin user
  4. Try interacting with chatbot

Kindly let me know if there is any step I missed

🇮🇳India annmarysruthy

In the block configuration, a checkbox 'Display title' is present. However, for the chatbot block, this option is not relevant. The title should only be displayed inside the chatbot modal, which is already happening regardless of whether 'Display title' is enabled or disabled.

To improve clarity, should we hide the 'Display title' option in the block configuration form specifically for the chatbot block? This would prevent confusion while ensuring the chatbot title is displayed correctly within the modal.

Inviting suggestions on this approach.

🇮🇳India annmarysruthy

Tried replicating issue in local ddev drupal 11 setup.

  1. cache is already enabled in site.
  2. Installed Milvus VDB Provider module
  3. Tried creating a new server and No VDB provider found
  4. Configured Milvus VDB Provider module and connected successfully
  5. On creating a new server, Milvus DB is now available without clearing cache
🇮🇳India annmarysruthy

@smustgrave if a custom form with password_confirm has title, then that title would be displayed. Inorder to avoid duplicates, here we are checking if $element['#title'] is empty. if $element['#title'] is empty, default title 'password' would be displayed. Else the title from custom form will be used.
Kindly let me know If there is a concern here

🇮🇳India annmarysruthy

@mrdalesmith Thank you for reporting this issue! I plan to rename the AI Chatbot module to AI DeepChat Bot. Below are the steps I intend to follow:

  1. Copy the existing ai_chatbot module and rename the copied folder to ai_deepchat_bot.
  2. Rename all filenames inside ai_deepchat_bot, e.g., ai_chatbot.info.yml → ai_deepchat_bot.info.yml. Update the module name to 'AI DeepChat Bot' in the .info.yml file.
  3. Update namespaces, hooks, and function names to reflect the new module name.
  4. Deprecate the old ai_chatbot module and update relevant documentation.
  5. Verify that the module continues to function as expected after the renaming.

Could you confirm if this approach aligns with your expectations? Let me know if there are any additional considerations to address before proceeding. Thanks!

🇮🇳India annmarysruthy

While trying to rebase the MR!446 accidently created a new MR. Closed new MR !484.

🇮🇳India annmarysruthy

Tested the MR !446. On applying the changes, :

  1. The user can translate the content in which user has translate permission. Also if user has 'Translate any entity' permission, the 'Translate using @ai' button appears. Otherwise, N/A will appear under AI translations
🇮🇳India annmarysruthy

Reviewed the MR !460. The error message could not be replicated after the changes in MR. AI Assistant is also working fine.

🇮🇳India annmarysruthy

Raised MR #3. Kindly review.

If #title is provided in the form element, pass1 does not get an extra title. If #title is not provided, it defaults to "Password" as before.
Creating a custom form with a password_confirm form element with #title will only display the provided title, not the default one.

🇮🇳India annmarysruthy

annmarysruthy made their first commit to this issue’s fork.

🇮🇳India annmarysruthy

The comment above the actions section originally described a configuration related to user roles and permissions.

Original code:

# Configuration actions may be defined. The structure here should be
# entity_type.ID.action. Below the user role entity type with an ID of
# editor is having the permissions added. The permissions key will be
# mapped to the \Drupal\user\Entity\Role::grantPermission() method.
actions:
  user.role.editor:
    createIfNotExists:
      label: 'Editor'
    grantPermissions:
      - 'delete any article content'
      - 'edit any article content'

However, in a later commit, the actual configuration changed to modifying text.settings, but the comment remained unchanged:

actions:
  text.settings:
    simpleConfigUpdate:
      default_summary_length: 700

Now, the comment is misleading because it's still describing user roles and permissions, while the actual code is updating text summary length settings.

🇮🇳India annmarysruthy

Removed autocomplete in MR !499. Kindly review. however even if we enter text in searchbox, relevant results are not displayed. This is already mentioned in https://www.drupal.org/project/drupal_cms/issues/3501069 🐛 Search broken without search_api_autocomplete Active

🇮🇳India annmarysruthy

annmarysruthy made their first commit to this issue’s fork.

Production build 0.71.5 2024