Duplicate rows for some entities with multiple entity references

Created on 4 June 2024, about 1 year ago

Problem/Motivation

When a single entity on a calendar has multiple entity references generating a row, the single entity can sometimes be displayed multiple times. Calendar checks for this, but the check only works if the two instances of the single entity are rendered sequentially. If another entity is rendered in between, the check fails. This is an issue with recurring dates and was called noticed in the smart_date queue at #3177760-10: Support Calendar module .

Steps to reproduce

  1. Install calendar, smart_date, smart_date_recur.
  2. Patch calendar per #3177761-6: Support for Smart Date, other contrib modules and smart_date per #3177760-13: Support Calendar module .
  3. Set up a content type with a smart_date recurring field.
  4. Create two nodes of that type. The smart_date field of one must recur over at least two dates. The date of the other node must be between the two occurrences of the first node.
  5. Set up a calendar such that it displays all dates of both nodes at once. The occurrences of the first node will appear twice.

Proposed resolution

Alter the check for repeated entities so that it remembers all entities processed, not just the last.

Remaining tasks

none

User interface changes

n/a

API changes

n/a

Data model changes

n/a

🐛 Bug report
Status

Active

Version

1.0

Component

Code

Created by

🇺🇸United States apkwilson

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

Merge Requests

Comments & Activities

  • Issue created by @apkwilson
  • 🇺🇸United States apkwilson

    apkwilson changed the visibility of the branch 3452532-duplicate-rows-for to hidden.

  • 🇺🇸United States apkwilson

    apkwilson changed the visibility of the branch 3452532-duplicate-rows-for to active.

  • 🇺🇸United States apkwilson

    apkwilson changed the visibility of the branch 3452532-duplicate-rows-for to hidden.

  • Merge request !29Skip rendering any entity already rendered. → (Merged) created by apkwilson
  • Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    Waiting for branch to pass
  • Status changed to RTBC about 1 month ago
  • 🇺🇸United States daniel korte Brooklyn, NY

    Works for me and is simple enough. Thanks!

  • First commit to issue fork.
  • 🇨🇦Canada joelpittet Vancouver

    Probably overkill but I want to get more tests in this module so getting the ball rolling here. Thanks for this, fix btw @apkwilson, it fixes a problem for me with date_recur I was seeing as well, so I will commit it really soon!

  • 🇨🇦Canada joelpittet Vancouver
  • 🇨🇦Canada joelpittet Vancouver

    joelpittet changed the visibility of the branch 8.x-1.x to hidden.

  • 🇨🇦Canada joelpittet Vancouver

    Moved the duplicate-check logic into a new instance-based isEntityAlreadyRendered($id) method. This change was motivated by two goals:

    1. Making the code much easier to test.
    2. Simplifying the render() method by removing internal static logic.

    Test highlights

    1. First call with an ID → false
    2. Second call with the same ID → true
    3. New ID resets correctly → false, then true
    4. Calling the first ID again later → true, confirming we now track all previously rendered IDs—not just the last one, which was the regression we fixed.

    Real‑world verification
    I manually reverted to the old static-based logic inside isEntityAlreadyRendered() and confirmed the last assertion failed. I also tested with a real date_recur scenario—this solution behaves identically to the static version but is hopefully easier to maintain and test.

    I manually reverted to the old static-based logic inside isEntityAlreadyRendered() and confirmed the last assertion failed. I also tested with a real date_recur scenario—this solution behaves identically to the static version but is cleaner and easier to maintain and test.

  • 🇨🇦Canada joelpittet Vancouver

    I've committed this to the dev branch for hopefully a quick release, thanks again @apkwilson and @daniel korte for bring this issue up and finding a solution!

Production build 0.71.5 2024