Return translated term name on views "Content: Has taxonomy term ID (with depth)"

While we do have test coverage, Lendude mentioned that one additional test is required.

See MR 6625

Pretty sure to fix the tests this yml file needs to be updated in a vanilla install of D11 or D10.2

core/modules/taxonomy/tests/modules/taxonomy_test_views/test_views/views.view.taxonomy_translated_term_name_test.yml

- Tests are green now.
There was one deprecated config change in view. Sorted that out.

- Updated issue summary and moving it to Needs Review
- Removed Needs test tag since it is not required anymore.

Working on the test coverage for the second plugin.

- Added the test case for the another ViewsArgument plugin.
- Tests are passing on local as well as CI
- Unusual failure form nightwatch tests, triggered the job again.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

@mohit_rocks , thanks for the work on the tests! Marking this as ready.

Hiding all the patch files since this issue is using an MR.

Added some review comments to the MR. This is a nice - we need to take care of classes that extend the core classes and I think there is a slightly better way of doing things.

Addressed the PR feedback.
I think https://git.drupalcode.org/project/drupal/-/merge_requests/6625#note_276326 would need some verification while doing review.
I'm somewhat confused about that.

Other point seems clear.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

ok, it's clean now.
RTBC

I've made a mistake here. DeprecatedServicePropertyTrait doesn't support entity storages. I'll fix the MR.

@alexpott do we need a change record for this one?

@mohit_aghera - yes we should have something small and simple... and the @see should point to it and not this issue. Great catch.

Can you add one, fix the MR and then set this back to RTBC?

Here's an example MR doing the same thing... https://www.drupal.org/node/3397213 →

Change record created. Back to RTBC

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Resolved the coding standard issue at last Pr due to which the pipeline was failing and setting back the status to RTBC from Needs works

Please review and suggest if any other changes are required.

@adwivedi008, can you please be sure that we need to fix the issues?

Reason is comment in #100is saying that patch is no longer applied to the core.
In this case, you need to merge latest 11.x to the branch.
I tried that and it seemed to be working fine.

@adwivedi008 your fixes are not fixes - core's coding standards are checked on every commit to an MR - the pipeline was green after #99 but your changes broke the pipeline. The "drupal" coding standard as detailed by coder is not the the core coding standard. Core is working towards it but we should never make out of scope changes to satisfy it and we should adhere to the coding standard set in core/phpcs.xml.dist as the "drupal" coding standard from coder actually needs fixing. I have reverted your changes.

Updated branch with latest 11.x head since test case failures were related to 11.x HEAD only.
Keeping it to RTBC, will check if there are any failures.

Committed and pushed 18c45f5f23 to 11.x and 462d3ac638 to 10.3.x. Thanks!

It'd be good to finish up 🐛 Fix label token replacement for views entity reference arguments Fixed and put a more generic fix in place.

This caused the gitlab performance test jobs to fail over the weekend, with:

Remaining self deprecation notices (2)
  1x: The "Drupal\KernelTests\KernelTestBase::expectDeprecationMessage()" method is considered internal use expectDeprecation() instead. It may change without further notice. You should not extend it from "Drupal\Tests\taxonomy\Kernel\Views\ViewsArgumentDeprecationTest".
    1x in DrupalListener::endTest from Drupal\Tests\Listeners
  1x: The "Drupal\KernelTests\KernelTestBase::expectDeprecationMessageMatches()" method is considered internal use expectDeprecation() instead. It may change without further notice. You should not extend it from "Drupal\Tests\taxonomy\Kernel\Views\ViewsArgumentDeprecationTest".
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

Since ViewsArgumentDeprecationTest appears to be mainly (only?) testing the constructor deprecation, I don't think that was necessary to add in the first place and we could just remove the test, but I didn't review to see if there was a better reason for adding it.

@catch
I added that initially and my intention was to validate two things.

1. Validate the deprecation message being displayed
2. I should be able use and invoke the older plugins with different constructor argument
   

   $plugin = \Drupal::service('plugin.manager.views.argument')->createInstance('taxonomy_views_argument_test', []);
       $this->assertInstanceOf(TaxonomyViewsArgumentTest::class, $plugin);

Happy to remove the test case entirely or just expect deprecation only.
Let me know your thoughts.

@catch @mohit_aghera this test only caused that because it unnecessarily used the trait. Remove the use and this will pass fine as we literally have hundreds of other deprecation tests doing the same thing.

I think leaving a test in in this case is okay because of the slightly more complicated dance of changing a parameter instead of just adding or removing a parameter.

Given this is just removing usage of a trait setting back to rtbc.

Makes sense to me - I can re-commit this a bit later but also think it'd be fine for @alexpott to commit the updated MR here since it's a tiny change.

Committed and pushed 1bcc7f34e6 to 11.x and 46564ee733 to 10.3.x. Thanks!

Second time lucky.

Automatically closed - issue fixed for 2 weeks with no activity.

This is still an issue. Since upgrading to 10.3 now instead of displaying the untranslated term title, no term title is displayed. It is shown in the english version of the page. Please re-open.

@Matt B thanks for reporting this regression. We can't revert this anymore and reopening a long running issue like this will lead to mistakes. I've opened 🐛 [regression] "Content: Has taxonomy term ID (with depth)": instead of displaying the untranslated term title, no term title is displayed Active to address this issue, can you provide a clear description of what broke and some steps to reproduce your issue there? Thanks!