twigphp / Twig

Twig, the flexible, fast, and secure template language for PHP
https://twig.symfony.com/
BSD 3-Clause "New" or "Revised" License
8.14k stars 1.25k forks source link

loop.last missing when applying filter #3297

Closed mbarbey closed 2 months ago

mbarbey commented 4 years ago

Hi,

It seem that the property loop.last disappear from the properties of the loop object when we apply a filter on the loop.

Here is the code I use :

{% for payment in payments | filter(p => p.enabled) %}
    {{ ('global.payment_type.' ~ payment.name) | trans }}
    {% if not loop.last %}
        <br/>
    {% endif %}
{% endfor %}

This code crash because the property loop.last doesn't exists. But if you remove the filter at the beginning of the loop, the property loop.last is back again.

The same problem occur if we filter the list before and loop on it :

{% set filteredPayments = payments | filter(p => p.enabled) %}
{% for payment in filteredPayments %}
    {{ ('global.payment_type.' ~ payment.name) | trans }}
    {% if not loop.last %}
        <br/>
    {% endif %}
{% endfor %}

Again, crash due to the missing loop.last.

I am on Symfony 4.4.5 with twig/twig 3.0.3.

Is it a restriction like the old for...if ? If I remember well, the usage of filter should have fixed it.

I can still count the number of items in the filtered list first and check with the loop.index0, but it's the exact reason for the existence of loop.last.

Any idea why it disappear with filtered list ?

stof commented 4 years ago

This looks weird to me. It should not be disappearing in that case.

stof commented 4 years ago

I tried your case in https://twigfiddle.com/v8d8ol and it does not trigger an error. Are you sure that this loop is the one breaking ?

mbarbey commented 4 years ago

If I dump some data :

{% for payment in operation.paymentMethodsConfig | filter(p => p.enabled) %} 
    {{ dump(loop) }}
{% endfor %}
{{ dump(operation.paymentMethodsConfig) }}

Here is the results :

Loop object

array:4 [▼
  "parent" => array:16 [▶]
  "index0" => 0
  "index" => 1
  "first" => true
]

Collection of payment methods

Doctrine\ORM\PersistentCollection {#1238 ▼
  -snapshot: array:5 [ …5]
  -owner: App\Entity\Operation {#1034 ▶}
  -association: array:15 [ …15]
  -em: Doctrine\ORM\EntityManager {#290 …11}
  -backRefFieldName: "operation"
  -typeClass: Doctrine\ORM\Mapping\ClassMetadata {#1190 …}
  -isDirty: false
  #collection: Doctrine\Common\Collections\ArrayCollection {#1240 ▼
    -elements: array:5 [▼
      0 => App\Entity\OperationPaymentConfig {#3139 ▼
        -id: 407
        -operation: App\Entity\Operation {#1034 ▶}
        -name: "postfinance_epayment"
        -amountFix: null
        -amountPercent: null
        -enabled: true
        -hidden: false
      }
      1 => App\Entity\OperationPaymentConfig {#3141 ▶}
      2 => App\Entity\OperationPaymentConfig {#3137 ▶}
      3 => App\Entity\OperationPaymentConfig {#3135 ▶}
      4 => App\Entity\OperationPaymentConfig {#3131 ▶}
    ]
  }
  #initialized: true
}

I also tried with hardcoded data like in yout twigfiddle and it works.

Here is the working sample :

{% set payments = [{name: 'first', enabled: true}, {name: 'second', enabled: false}] %}
{% for payment in payments | filter(p => p.enabled) %}
    {{ dump(loop) }}
{% endfor %}

Which return :

array:8 [▼
  "parent" => array:17 [▶]
  "index0" => 0
  "index" => 1
  "first" => true
  "revindex0" => 0
  "revindex" => 1
  "length" => 1
  "last" => true
]

I can't find why it works with hardcoded array but not collection.

stof commented 4 years ago

OK, I think this is related to the usage of CallbackFilterIterator to implement the filter filter when being passed an iterator. That forbids using loop.last because it is not countable.

@fabpot we might need a way to force the conversion to array.

Dr-Ash commented 2 years ago

Hello,

Top solve this issue i used in my case: {% for be in form.bookingEquipments.children|filter(be => be.id.vars.data == true) %} Children is an array, so it works as expected.

In the case of OP, i think operation.paymentMethodsConfig.collection may work

djpretzel commented 1 year ago

I just hit this bug today - I would have thought it would be annoying more folks... which makes me think I'm using Twig wrong or something, since this seems like a common use case?

stof commented 1 year ago

@djpretzel In my experience, most use cases for loop.last can be transformed to rely on loop.first instead (for instance, the example in the issue description can be reworked to add a <br> before each non-first item instead of adding it after each non-last item). And loop.first is always available (as it does not rely on future iterations but on past ones)

djpretzel commented 1 year ago

@stof Absolutely, but that's not working either - for every element, loop.first is coming back as true, loop.last is coming back as true, and loop.index is coming back as 1... so both first AND last are useless.

I'm in a nested for loop; the parent loop on track/tracks is not filtered, but the nested loop is:

{%- set album_tracks_artists = view.album_tracks_artists|filter(album_tracks_artist => album_tracks_artist.album_track_id == track.id) -%}
{%- for album_tracks_artist in album_tracks_artists %}
    {%- set artists = view.artists|filter(artist => artist.id == album_tracks_artist.artist_id) -%}
    {%- for artist in artists %}
        <span style="color:yellow;">{{ loop.index }} - {{ loop.first }} - {{ loop.last }}</span>
        {%- if loop.index > 1 %}, {% endif -%}
        <a href="https://ocremix.org/artist/{{ artist.id }}/{{ artist.urlname }}">{{ artist.name }}</a>
    {% endfor -%}
{% endfor -%}

image

stof commented 1 year ago

there is no usage of loop at all in your code snippet.

djpretzel commented 1 year ago

@stof updated, with code and sample output - all three numbers are "1", for both artists...

stof commented 1 year ago

@djpretzel Are those different album_tracks_artist or different artist for the same album_tracks_artist ? If the various values come from different iteration of the outer loop with only one element each in the inner loop, it is totally expected that you are always the first iteration of the inner loop.

As your code snippet never renders the word By, I guess that it is rendered before the outer loop, so it could be the outer loop having multiple iterations.

djpretzel commented 1 year ago

@stof isn't "loop" supposed to refer to the loop you are active in, i.e. the nested loop, based on context? Why would it matter, if so? I was under the impression that the scope of "loop" was the lowest-level enclosing for statement?

stof commented 1 year ago

@djpretzel yes indeed. and if artists has a single item each time, you will indeed always be on the first iteration of the loop (the inner one, which is the active one). My guess is that your Proticity and zykO artists are belong to different album_tracks_artist for the iteration of the outer loop.

djpretzel commented 1 year ago

@stof Yep, that was it... and using the index is working now, even if last isn't. Thanks!

fabpot commented 2 months ago

Closing as loop.last will always be available in Twig 4.0. See #4134