View result counter as token not work when use in Output this field as a custom link

Resolved the issue & enhancing the solution with comprehensive test coverage.

Please review, moved NR

Nice, good to see coverage added.

The Counter plugin uses UncacheableFieldHandlerTrait but this seems to break that, since the existing tests now require cache rebuilds. That feels like a regression we need to look at.

Thanks for reviewing the MR.

I 'm kind of aware about this regression should not happen, as existing test needs cache rebuild for test pass for specific scenario. However tried to remove UncacheableFieldHandlerTrait for debugging purpose but then it lead to break test for advanced cache one. which is also not appropriate fix.

Used renderText() to fix the issue, it does not lead cache regression in views/tests/src/Kernel/Plugin/RowRenderCacheTest.php

Please review, Moving to NR.

Rebased seems fine

Nice kernel test, would think it could be folded into FieldCounterTest but see that one doesn't have a viewsData() so standalone is probably fine.

Ran the test-only feature

1) Drupal\Tests\views\Kernel\Handler\FieldCounterTokenTest::testFieldCounterToken
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<a href="/counter/2">Counter 2</a>'
+'<a href="/counter/%7B%7B%20counter%20%7D%7D">Counter 2</a>'
/builds/issue/drupal-3456341/core/modules/views/tests/src/Kernel/Handler/FieldCounterTokenTest.php:69
FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

Which shows the coverage +1 there

Believe this one may be good.

Addressed the feedback , Rebased the MR with latest code & pipeline pass successfully.

keeping it in RTBC as there is only minor change done.

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.

Rebased the MR with latest code.

PLease review, moving NR

Feedback from @lendude appears to be addressed.

Only Rebased the MR with latest code, keeping it to the prev RTBC status

I modified some comment to be easier to understand and fixed grammar. I also changed $desired_output to the common $expected_output.

Leaving at RTBC

Only Rebased the MR with latest code, keeping it to the prev RTBC status.

There's two unanswered questions on the MR.

I think we need to do the root cause analysis as to why this change now requires an extra cache clear in the test.

Thanks @pooja_sharma for working on this!

@larowlan and I discussed this in person at DrupalCon Singapore and agreed that the best approach is to leave the cache invalidation for now, but to have a followup issue to see why it's needed and maybe fix it.

The specific format in the MR currently needs a little work. It should be an inline comment containing a @todo comment and a link to a new followup issue. Here are a couple references that should be helpful:

• @todo comment formatting instructions → .
• Contributor task steps for filing a followup issue →

Hope those help. It was great to meet you in person!