checking for test failures.
Fixed the conflict and rebased MR, seems like some random test failure.
shalini_jha → made their first commit to this issue’s fork.
I have added test coverage for showing this issue, pipeline is green so moving this NR.
Kindly review.
Failing test :
There was 1 failure:
1) Drupal\Tests\workspaces\Functional\PathWorkspacesTest::testPathAliasesWithTranslation
Behat\Mink\Exception\ExpectationException: Current response status code is 404, but 200 expected.
/var/www/html/vendor/behat/mink/src/WebAssert.php:888
/var/www/html/vendor/behat/mink/src/WebAssert.php:145
/var/www/html/core/modules/workspaces/tests/src/Functional/PathWorkspacesTest.php:345
/var/www/html/core/modules/workspaces/tests/src/Functional/PathWorkspacesTest.php:331
I have replicated this issue, and the merge request fixes it. I will work for the test coverage.
Moving this for NR.
I have checked this MR and found some conflicts, which I have addressed. The pipeline was failing due to an unknown word. After reviewing the existing test method, I added it to the cspell:ignore list, and the pipeline passed successfully. After that, I revalidated the test coverage, and it is working as expected. Let me know if you need any further adjustments!
Kindly review.
Failing test :
There was 1 failure:
1) Drupal\Tests\layout_builder\Functional\LayoutBuilderTest::testExtraFields
Behat\Mink\Exception\ExpectationException: An element matching css ".block-extra-field-blocknodebundle-with-section-fieldlayout-builder-test-empty" appears on this page, but it should not.
/var/www/html/vendor/behat/mink/src/WebAssert.php:888
/var/www/html/vendor/behat/mink/src/WebAssert.php:492
/var/www/html/core/modules/layout_builder/tests/src/Functional/LayoutBuilderTest.php:505
moving this for NR.
Thanks
I have address the feedback, also verified with another method no type cast is added, also again re ran the test and its working as expected, pipeline is also green so moving this for NR.
Kindly review.
I was able to replicate the issue and after debugging, I found that the data was not updating on module installation due to caching. To resolve this, I invalidated the configuration cache tag in the hook_install(), and it is working as expected now. Moving forward for NR. Let me know if anything else needs to be updated regarding this.
Kindly review.
shalini_jha → changed the visibility of the branch 3479924-project-browser-devel to active.
shalini_jha → changed the visibility of the branch 3479924-project-browser-devel to hidden.
shalini_jha → made their first commit to this issue’s fork.
I Have addressed the feedback also checked the https://3v4l.org/UuKsj and re-ran the test and its is working as expected. pipeline is also green so moving this NR.
Kindly review.
shalini_jha → made their first commit to this issue’s fork.
@ankitv18, thanks for your review. However, I started with the Project Browser issue because I wanted to begin with a simple problem to understand the flow better."
@ankitv18, thanks for your review. However, I started with the Project Browser issue because I wanted to begin with a simple problem to understand the flow better."
Removed the legacy code from DrupalDotOrgJsonApi.php. and raised a MR for this .moving this NR. Kindly review.
shalini_jha → made their first commit to this issue’s fork.
I have updated the route URL for drupal-org-proxy to /project-browser/data, and have made the necessary updates across all relevant occurrences. I have also generated the build to reflect these changes and raised a MR for review. Could you please review the changes and let me know if I have missed anything that needs to be updated?
Fixed issue reported by bot but seems some phpstan failure.
shalini_jha → made their first commit to this issue’s fork.
I have consolidated both test methods into a single one and tested it. It is working as expected. Moving this to NR for review. Kindly review.
Failing test :
PHPUnit 10.5.38 by Sebastian Bergmann and contributors.
Runtime: PHP 8.3.12
Configuration: /var/www/html/core/phpunit.xml.dist
F 1 / 1 (100%)
Time: 00:04.851, Memory: 186.00 MB
There was 1 failure:
1) Drupal\Tests\Core\Controller\TitleResolverTest::testStaticTitleWithArguments
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
Drupal\Core\StringTranslation\TranslatableMarkup Object (
'string' => 'static title @test @test2'
'arguments' => Array (
- '@test' => 'override value'
+ '@test' => 'value'
'%test' => 'override value'
'@test2' => 'value2'
)
/var/www/html/core/tests/Drupal/Tests/Core/Controller/TitleResolverTest.php:153
I have checked the related issues.
I will look into the pipeline failure issues
I reviewed the merge request and found some conflicts, so I rebased and resolved them. The pipeline has also passed successfully, so I’m moving it to "Needs Review."
Thank you for your review and feedback; I'll incorporate this. I've updated the issue summary and added a draft mode
change request →
for review.
Please take a look and let me know if there’s anything further needed on my end.
shalini_jha → made their first commit to this issue’s fork.
Moving this for Review.
Hi,
I was able to replicate the issue and reviewed the existing fix in this MR. The solution appears correct, and functionality works as expected based on my testing. To validate this, I have tried to add test coverage in the FilterStringTest class, aiming to demonstrate both the issue and its resolution.
I’ve submitted a new MR against 11.x. Since there was an existing MR on 10, I’ve hidden that one to avoid any confusion. I wasn’t sure about the status of the existing MR, Apologies if you would have preferred updating the existing MR.
Please review when possible, and let me know if there’s anything further I should address.
Updated issue summary also.
Thank you!
shalini_jha → changed the visibility of the branch 3055152-views-stringfilter-doesnt to hidden.
shalini_jha → changed the visibility of the branch 3055152-views-stringfilter-doesnt-11x to active.
shalini_jha → changed the visibility of the branch 3055152-views-stringfilter-doesnt-11x to hidden.
I have able to replicate this issue and MR fixes the issue , so i will work on this for test coverage.
Addressed the feedback regarding grammatical issues. The pipeline has passed, so I'm moving this to 'Needs Review.Kindly review.
I've rebase the MR to address the pipeline failure,so pipeline is fixed now. and as noted in #6, the changes are now ready for review. Moving this to 'Needs Review.
shalini_jha → changed the visibility of the branch 3476439-term-list-performance-test to active.
I will check for pipeline failure.
I have fixed the pipeline failure issue. Also, the last point mentioned in #9 makes more sense, so I have updated the views_area label. Moving this to 'Needs Review'. Kindly review.
shalini_jha → made their first commit to this issue’s fork.
shalini_jha → made their first commit to this issue’s fork.
I have added test coverage for this issue and verified all filter options.
failing test :
1) Drupal\Tests\user\Functional\Views\DuplicateRoleFilterTest::testUserRolesFilter
Behat\Mink\Exception\ResponseTextException: The text "User with role two" was not found anywhere in the text of the current page.
/var/www/html/vendor/behat/mink/src/WebAssert.php:907
/var/www/html/vendor/behat/mink/src/WebAssert.php:293
/var/www/html/core/tests/Drupal/Tests/WebAssert.php:979
/var/www/html/core/modules/user/tests/src/Functional/Views/DuplicateRoleFilterTest.php:84
Now pipeline failure is also fixed so moving this to NR. Kindly review.
It seems the "Test-only" test is failing when it shouldn’t.
I have added the Test coverage for this functionality. That still needs some work and will verify properly, my work was in progress. I am assigning this to me for now.
Seems like some test failure , that needs to check, i am looking into it.
After rebasing merge request (MR), it seems that PHPStan is flagging issues in StringTranslationTrait.php .
shalini_jha → made their first commit to this issue’s fork.
Hi @Joachim,
I’ve reviewed the test failures caused by recent changes to ConfigSchemaChecker. It seems that while some failures relate to changes in the exception messages, others are due to how we handle cases where $key isn’t in the expected foo.bar:bar.biz format. Specifically, we’re seeing integer keys instead of the expected string format, which leads to $error_property being null. This causes the error:
TypeError: Drupal\Component\Utility\Html::escape(): Argument #1 ($text) must be of type string, null given
For example, in scenarios like:
array:4 [
0 => "[dependencies.theme.0] Theme 'bluemarine' is not installed."
1 => "[theme] Theme 'bluemarine' is not installed."
2 => "[region] This value should not be blank."
3 => "[region] This is not a valid region of the <em class=\"placeholder\">bluemarine</em> theme."
]
Here, $key does not split as expected, leading to null in $error_property. Currently, we haven’t implemented any handling for integer keys, which leads to the failures to these test method i have checked like : testInvalidConfiguration(), testConfigSchemaChecker(), and testBlockMigration() .
Could you please advise on how to handle integer keys? One approach could be to check if the $key is an integer and set $error_property as blank in such cases, or we could use the string representation of the key, like [dependencies.theme.0], as the property. Please let me know if you have a preferred direction for managing this scenario, or if I'm overlooking any potential cases. I am moving this for review to get your input on it. Thank you!
shalini_jha → made their first commit to this issue’s fork.
I have re running the pipeline and now it is fixed. so moving this for NR. Kindly review.
I have verified that the testStaticTitleWithDefaultArguments method checks the default behaviour of using static titles with preset arguments without testing dynamic overrides, which is why it does not fail. The issue is specifically addressed in testStaticTitleWithDefaultArgumentsOverridden, where dynamic URL parameters should replace default values but are not being applied correctly.
Please let me know if we should remove the method that tests the default functionality. Moving this to "Needs Review."
Thank you for your help, I have verified that the MediaSource attribute has already been added.
my bad missed to check this. I have addressed the feedback accordingly and am moving this to "Needs Review."
i have addressed the feedback. and updated the MR , it seems again pipeline fail with not related to this changes.
I have investigated this test case and observed that the warning appears only under specific conditions. Initially, when we add a date as an exposed filter and select the 'Between' operator, the warning does not appear upon saving the view. However, if we edit the date exposed filter, switch from 'single filter' to 'group filters,' and again select the 'Between' operator for the group filters, saving the view triggers the warning. When we access the view page and select the 'All' option, the warning arises because the single filter operator is still set to 'Between,' but the 'min' and 'max' values are not available for the 'All' option.
To demonstrate this behaviour, I modified the test case to show the warning. After applying the solution, the test case passes successfully.
pipeline still failing somehow.
I have looked into how to convert the annotation for MediaSource to an attribute. However, I noticed that there is currently no predefined attribute available for MediaSource, unlike some other components like Action, which do have attributes defined.
Could you please guide me on whether we need to create a new attribute for MediaSource, similar to Action, before changing the plugin from annotation to attribute? Your guidance on this would be greatly appreciated.
Moving this to 'Needs Review' to get your help. Sorry for the inconvenience.
I have cleaned up the branch. MR 4902 now contains all required updates and is ready for review. Additionally, I have updated the issue summary for clarity.
Moving this MR to Review. Kindly review the changes.
shalini_jha → changed the visibility of the branch 3343670-allow-to-merge to hidden.
shalini_jha → changed the visibility of the branch 3343670-allow-to-merge-ajaxcommands-10-1-x to hidden.
I have rebased this MR, and the pipeline has passed successfully. Moving it to 'Needs Review'.
Kindly review.
I've checked the issue and identified an item related to the processDefinition method's return type. I've added the necessary type hint and am now moving this to Needs Review, Please review.
shalini_jha → made their first commit to this issue’s fork.
I have added test coverage for this functionality and thoroughly tested it. The pipeline has also passed successfully. I am now moving this to the 'Needs Review' stage.
Please review.
I have added test coverage for the mergeCommands method, and the pipeline issues have also been resolved.
I am now moving this for review. Please take a look.
I have replicated the issue regarding the source field name length and applied the necessary fixes, which appear to resolve the issue. After reviewing the test cases, I identified several underlying problems, all of which have been addressed. Following these fixes, the functionality is now working correctly, and the source field name is properly truncated to a maximum of 32 characters. I have submitted a merge request for this change and am moving it to "Needs Review." Kindly review it at your convenience.
I have tested MR 8584, and after these changes, when the parameter is available in the URL, the getTitle method returns the value from the URL parameter instead of the static value from the routing. Additionally, I have updated the issue summary.
Moving this to NR, Kindly review.
shalini_jha → made their first commit to this issue’s fork.
I have updated the code based on PR feedback and fixed the pipeline for the failing test. moving this to NR.
Kindly review.