IDEMSInternational / open-app-builder

PLH App Frontend
GNU General Public License v3.0
5 stars 24 forks source link

fix: set_item actions nested inside item loop #2309

Closed jfmcquade closed 2 months ago

jfmcquade commented 2 months ago

PR Checklist

Description

Fixes an issue where set_item and set_items actions would not function as expected when the row was deeply nested within an item loop, e.g. within a display_group component inside an item loop.

This was due to the setItemContext meta only being applied to actions within the top-level action_list inside the items loop.

Git Issues

Issue identified in this mattermost thread.

Screenshots/Videos

See new debug_data_items_actions component. The buttons on all actions now function as expected – on master, the buttons in the second items loop, nested inside display_groups, do not trigger the action correctly and an error is thrown.

Screenshot 2024-05-01 at 15 35 38

https://github.com/IDEMSInternational/parenting-app-ui/assets/64838927/a86caf98-8ad1-40dc-9d09-35f4dafb7c96

jfmcquade commented 2 months ago

I also noticed that it would probably be possible to reduce the diff by simply adapting the original code in the same way as you've done with the recursive function, namely keeping the original templateRows.map(r=>... function and simply adding the recursive call

 if (r.rows) {
      r.rows = r.rows.map((childRow) => this.setItemMeta(childRow, itemData));
    }

But I'm not strongly opposed to having two functions to handle the setItemMeta process so I'll leave you to decide if you think worth a quick refactor or not.

Ah, good point. Refactoring along these lines actually makes the logic simpler than this, because setItemMeta() takes an array of rows. See latest diff for all commits.