🇮🇳India @shalini_jha

Based on the current change, all FAQ items are generated at once when clicking on the “Automator FAQ” field.

The updated comment looks good, and I have also fixed a minor typo. +RTBC

Thank you for the review. yes, you are correct—form-level validation was required. I have added the validation to the form, verified that it is working as expected, and moved this back to Needs Review.

I have setup the Base Field Override UI module and content authoring for testing this.

1 . Manage fields for moderation state configuration

2. Mange form display for FWA configuration.

3. The moderation state is pre-populated with the correct value based on the prompt.

Working as expected + RTBC

I have set up the FAQ Field module and configured it using Field Widget Actions, selecting “Automator FAQ Field” as the widget. While testing this setup, I have a few questions regarding its behaviour.

I set the allowed number of values to unlimited for this field. Based on my prompt, the first FAQ item works correctly and the question and answer fields are pre-populated as expected.

However, when I add an additional FAQ item and trigger the Automator FAQ Field again, it does not populate the newly added item. Ideally, this should work for all added items ?

Based on point #9, I also set up the Chart module and tested this locally. I observed the same behaviour: the chart field is pre-populated when the node is saved, rather than when clicking the Automator Chart button.

I have reviewed the code and tested the functionality locally using the FWA and json_field modules. The following steps were performed to verify the behaviour:

Testing Steps

1) Added a JSON (text) field to the Article content type.

2) Configured the field to Enable AI Automator using the “LLM JSON Field” type.

3) In Manage form display, selected “Automator JSON” as the field widget.

4) Saved the configuration.

5) Navigated to the Add Article form and Verified that the JSON field is automatically pre-populated with the generated JSON data based on the prompt.

This is working as expected. RTBC+1

I was able to successfully replicate this issue. I have updated the form so that the Field dropdown is always enabled and includes an empty “Select field” option when a bundle is selected. Previously, when a bundle had no available fields, the field dropdown was disabled and the empty option was not shown, which allowed the form to be submitted without a valid field value.

Since this Field is a required field, keeping it enabled with an empty option ensures that form submission is properly blocked when no fields are available for the selected bundle. This prevents the form from being submitted in an invalid state and avoids the earlier error from occurring.

Moving this Needs review, Please review and let us know.

Moving this to Needs Review due to pipeline failures. Requesting guidance on the fix.

Re based and resolved conflicts, and regenerated the build JS files.

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

Rebased and fixed conflicts, but there are some pipeline failures.

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

I also tried to replicate this issue, but I was not able to reproduce it as mentioned in #3 . By default, when the AI Deep Chat bot block is placed, the Placement setting is set to “Toolbar.”

If this placement setting is not changed, the chatbot will continue to appear in the toolbar by default. Once the placement is updated to Bottom Left or Bottom Right, the chatbot becomes visible in the selected position accordingly.
Please review if this is the scenario in your case.

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

Rebased the MR, fixed conflict, and addressed the feedback. Moving this back to NR.

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

The feedback appears to be addressed, so the pipeline was re-run. Since the failure seems to be random.
so moving this back to review.

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

Addressed the feedback. Based on the changes, the tests were re-run and are working as expected, so moving this back to NR.

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

I’ve addressed the feedback related to @trigger_error. Moving this back to Needs Review. Please review and let me know if any further updates are required.

Thank you @quietone for your guidance.

I’ve addressed the feedback on the MR, and the test that was failing on the setTextMode method is now passing. Moving this back to review.

While reviewing the ImageAndAudioToVideoGenerator API Explorer plugin, I noticed a text correction was needed, so I have added it in the MR.
Thanks

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

Rebased the MR and resolved the conflicts, Checking for other feedback and pipeline failure issue

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

Addressed the feedback , moving this back to NR, kindly review.

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

Resolved the merge conflicts, pipeline is pass so moving this back to NR. Kindly review

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

I checked the pipeline failure and tested it locally — it appears to be a random failure. It has now passed, so I am moving the ticket back to NR.

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

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

Moving this back to NR.

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

Re based and fixed conflict. Moving back to previous status.

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

Review MR 19 for this issue.

shalini_jha → changed the visibility of the branch 3129570-permanent-token-fix to hidden.

Hi,

I’ve raised a PR to update the Facebook permanent token whenever required.
The update is triggered in these cases:

When the default config or a domain-specific config override is changed.

When editing a config, if the "Override permanent token" checkbox is checked.

For edits, this new "Override permanent token" field must be checked if you want the permanent token to be regenerated. After the token is refreshed, the field is automatically set back to false so that the token is not regenerated on every save.

Please review this and let me know if anything needs to update : https://git.drupalcode.org/project/socialfeed/-/merge_requests/20

shalini_jha → changed the visibility of the branch 3129570-pemenant-page-token to hidden.

I have added a MR for Instagram block to support the config overrides same as Facebook and twitter block using ImmutableConfig.
Moving this for review.

Thanks

I am also facing the same issue , the Permanent token is not refreshing after changing the configuration more then one time.
Means if permanent token is generated and saved in config so based on the current condition it will not refresh based on the latest page feed value.
Hence moving this in active status.

shalini_jha → created an issue.

Addressed the mentioned feedback on the MR. Also tested the failing test — it appears to be a random failure, so I re-ran the pipeline. All pipelines are now green. Moving this back to NR. Kindly review.

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

I have gone through the feedback on the MR and addressed the requested changes.I also re-ran the tests locally, and they are passing.
Moving this back to Needs Review for further feedback.kindly review.

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

I have checked the issue and applied fixes based on the MR feedback. I updated the code to cast to bool directly, as this handles both true and false cases. There is no need for a ternary operator in this situation. I have reverified the test, and it is passing as expected.

Additionally, I encountered linting failures for the existing code. I have addressed these issues, and the code now passes the linting checks. I am moving this back to Needs Review. Kindly review.

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

I have rebased this MR and resolved the merge conflicts. During the rebase, I noticed that some new behavior was introduced, so to ensure everything continues to work as expected, I re-ran the range test locally.

While debugging, I found that the test was failing in the third scenario where the datetime value falls within the defined range. Due to the newly introduced behavior Here ✨ Datetime fields should utilise #required_error Needs review , the form state now includes additional errors such as datetime_required_error and datetime_no_required_error.

To address this, I updated the third scenario’s assertion to only check that range_datetime_element is not present in the form state errors, as this is the specific error we are handling in this case in testRangeValidate() test method. With this change, the test now passes as expected.

Moving this back to Needs Review.Kindly review.

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

Thank you for the feedback , yes {@inheritDoc} is there so no need to add the deprecation here , i have checked other deprecated method also for same with {@inheritDoc}. i have updated here also.

Thank you for your feedback!
I’ve addressed all the points as suggested, and rerun the pipeline — it's now passing.

I’ve also updated the issue summary to reflect the changes made based on the MR feedback.
Please have a look and let me know if anything further needs to be adjusted.

Thank you for the review and feedback. i am working on it.

I’ve deprecated the getReplacementID() method after confirming it isn’t used in core or contrib, and I followed a similar approach to other deprecated methods. Moving this NR , Could you please review and let me know if anything needs to be updated or improved?

Thank you for the guidance!

Rebased and fixed conflicts also regenerated the based line for fixing the pipeline failure. Moving this back to NR.Kindly review

Thanks for the feedback!
Based on your suggestion, I haven’t modified the doGetUploadLocation() method. Instead, I added a trailing slash check directly in the same function where the destination path is being built for both FileItem and ImageItem. This way, we avoid changing the method’s return value and reduce the risk of breaking other usages.

Could you please review and let me know if this approach works, or if further changes are needed?
I have also update the IS based on my current approach for more clarity. pipeline is green so moving this for your review.

Kindly review, Thanks

Re based and fixed conflicts. pipeline is green so moving back to NR.

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

I reviewed the pipeline failure and found that it was caused by the cspell issue and same checked for MR feedback ✨ Missing documentation for loadModelsForm() Active , and now the pipeline failure is fixed after following the feedback. Moving this NR for further feedback.

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

Hi, I have re-run the previously failed test and it has now passed. It appears to have been a random failure :)

I have followed the same steps outlined in the IS for the 'AI DeepChat Chatbot' and was able to replicate the same error when no AI Assistant is configured. I believe we may also need to review and update this block accordingly?

Addressed all the feedback , moving this NR, Kindly review.

Rebased without conflicts , so moving this previous status.

Re based the MR, moving this for NR. Kindly review.

I have addressed all the feedback, including updating the comment for the single-word condition to improve clarity. Please let me know if we need to assert the single word in the test for further clarification.

Moving this to NR , Kindly review.

AS #6 is not applying, Fixed conflict.

Rebased and fixed conflicts.Moving this back to NR.

Added the clearCaches() method to PerformanceTestTrait, allowing other performance tests, such as OpenTelemetryFrontPagePerformanceTest and OpenTelemetryNodePagePerformanceTest, to use it. After this change, the clearCaches() method was removed from these performance test classes.

Added follow up ticket for this : https://www.drupal.org/project/drupal/issues/3504566 📌 Move clearCaches() Method to PerformanceTestTrait for Reusability Across All Performance Tests Active

shalini_jha → created an issue.

@smustgrave Thank you so much for your detailed explanation!

The clearCaches() method needs to be removed from the TestTrait. Yes, I found several performance test classes that could benefit from this change, so you are absolutely right—we should create a follow-up ticket to update all performance test methods accordingly.

For now, I have added the clearCaches() method within the same test, following the pattern used in other performance tests.Pipeline is also green , so moving this back to review.

Re base this MR and fixed the conflict .

I have checked the MR, and the changes made in #11 were removed from the MR after the latest rebase. I can no longer find the comment-related changes in the MR. moving this back to NR.

@borisson_ Thank you for your review & feedback. i am looking into it.

I Have checked this MR and found the pipeline failure for the enabled rule. I have updated the test class accordingly, following the same approach as in the other files. After making these changes locally it is also passing, and the pipeline is also green. I am now moving this MR for review.

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

@quietone Thank you so much for the clear explanation! I have gone through the documentation, and it makes sense. I truly appreciate your guidance :)

I Have checked this existing MR , and based on the IS i have run the phpcs command and got the failure what pipeline is also showing for ClassComment. i have updated all the mentioned class comment with referencing the existing class comment , and again rerun the phpcs locally and it is passing same as pipeline is also passing .

So moving this for NR, please review and let me know if any of class comment needs to update.

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

@quietone You are correct. I have also checked the usage of ImageStyle::getReplacementID, and I could not find any.

I have tried to address the added feedback, but I have two points to discuss:

Single Quotes: If we use single quotes, I have reviewed the entire MR and found additional places where updates are needed. Should we update all occurrences for consistency?
Full Stop in Keys: I have reviewed the MR and noticed the same behaviour for adding a full stop in keys.

Please review and let me know. Thanks

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

I am a bit unsure about the last feedback. Could you please clarify if we need to remove this ?
moving this for NR to get the feedback.

Re based this MR with fixed conflicts in phpcs.xml.dist.

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

I am moving this for review. Please take a look and let me know if it makes sense, or if any additional tests need to be added?