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

Created on 21 June 2024, 3 months ago
Updated 21 September 2024, 7 days ago

Problem/Motivation

When enabled "Output this field as a custom link" checkbox , add counter field token in link path , then counter token broken in href on FE.

Steps to reproduce

  • Add/edit the view
  • Add field: View result counter
  • Add field: Custom text, Add value in text field
  • After scroll down, you 'll find opt: rewrite results > click on it
  • Then you 'll find checkbox: "Output this field as a custom link" & enable it
  • You 'll see opt link path , add here counter field token for eg: /test/{{ counter }}
  • Click on apply btn & save the view

Issue is replicable Drupal 10 version as well
Note: I didn't find any issue w.r.t to counter that fails this mentioned scenario, if anyone finds then can close ir as duplicate by attaching the link of existing issue.

Expected behaviour

Instead of this broken counter field token "/test/%7B%7B%20counter%20%7D%7D" in href tag counter field's token value should render(eg: /test/1)

Proposed resolution

When checked "Output this field as a custom link" checkbox & add the counter field token in to link path, then counter token should work instead of broken in href

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Needs work

Version

11.0 πŸ”₯

Component
ViewsΒ  β†’

Last updated about 5 hours ago

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @pooja_sharma
  • Merge request !8486Resolve #3456341 "View result counter" β†’ (Open) created by pooja_sharma
  • Pipeline finished with Failed
    3 months ago
    Total: 588s
    #205148
  • Pipeline finished with Failed
    3 months ago
    Total: 1434s
    #205163
  • Pipeline finished with Failed
    3 months ago
    Total: 180s
    #205187
  • Pipeline finished with Failed
    3 months ago
    Total: 708s
    #205190
  • Pipeline finished with Failed
    3 months ago
    Total: 593s
    #205526
  • Pipeline finished with Failed
    3 months ago
    Total: 588s
    #205628
  • Pipeline finished with Success
    3 months ago
    Total: 512s
    #205637
  • Resolved the issue & enhancing the solution with comprehensive test coverage.

    Please review, moved NR

  • Status changed to Needs review 3 months ago
  • Status changed to Needs work 3 months ago
  • πŸ‡³πŸ‡±Netherlands Lendude Amsterdam

    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.

  • Pipeline finished with Canceled
    3 months ago
    Total: 285s
    #211644
  • Pipeline finished with Failed
    3 months ago
    #211648
  • Pipeline finished with Success
    3 months ago
    Total: 542s
    #211653
  • Pipeline finished with Success
    3 months ago
    Total: 516s
    #211657
  • 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.

  • Status changed to Needs review 3 months ago
  • Pipeline finished with Success
    3 months ago
    Total: 605s
    #213003
  • Status changed to RTBC 3 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    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.

  • Pipeline finished with Success
    3 months ago
    Total: 461s
    #223786
  • Pipeline finished with Success
    3 months ago
    Total: 464s
    #223792
  • Addressed the feedback , Rebased the MR with latest code & pipeline pass successfully.

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

  • Status changed to Needs work 2 months ago
  • 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.

  • Pipeline finished with Success
    2 months ago
    Total: 506s
    #229650
  • Rebased the MR with latest code.

    PLease review, moving NR

  • Status changed to Needs review 2 months ago
  • Status changed to RTBC 2 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Feedback from @lendude appears to be addressed.

  • Pipeline finished with Success
    2 months ago
    Total: 671s
    #238221
  • Only Rebased the MR with latest code, keeping it to the prev RTBC status

  • First commit to issue fork.
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    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

  • Pipeline finished with Success
    about 2 months ago
    Total: 781s
    #248684
  • Pipeline finished with Success
    about 1 month ago
    Total: 524s
    #264962
  • Only Rebased the MR with latest code, keeping it to the prev RTBC status.

  • Status changed to Needs work 8 days ago
  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    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.

  • Assigned to pooja_sharma
Production build 0.71.5 2024