gristlabs / grist-core

Grist is the evolution of spreadsheets.
https://www.getgrist.com/
Apache License 2.0
7.21k stars 319 forks source link

Lists of RecordSets cannot be typed as Reference List #1130

Closed vviers closed 1 week ago

vviers commented 3 months ago

Problem

Say in table Carts there's a column $Cart of type Reference List, pointing to a Foods table

In a third table (say, Customers), we want to list all Foods that are in a customer's cart.

Carts.lookupRecords(customer=$id).Cart returns a "list of lists" ([Foods[[1, 2, 3]], Foods[[1]]]) which is not handled properly by Grist when trying to type it as a Reference List

A fix is to return a flattened list using a list comprehension [el for val in Carts.lookupRecords(customer=$id).Cart for el in val]

An example document can be found here : https://docs.getgrist.com/bAjQhzJRgBJB/Issue-1130-Example/p/1

Proposed solution

Option 1 : Implement a FLATTEN_RECORDS_LIST function that transforms a list of lists of records into a list of records. We should offer a deduplicate option to this function.

Option 2 : Make Grist handle those list of lists of records as a valid input for the Reference List type

WDYT ?

vviers commented 3 months ago

Note that it's possible that such nested lists of records set are deeper than 2, both options should account for this

hexaltation commented 1 month ago

@vviers I'm studing the option 1. What do you expect to be passed as argument of FLATTEN_RECORD_LIST? Is FLATTEN_RECORD_LIST(Carts.lookupRecords(customer=$id).Cart) a good candidate?

vviers commented 1 month ago

Something like FLATTEN_RECORDS_LIST(Carts.lookupRecords(customer=$id).Cart, dedup=True) would make sense to me yes !

berhalak commented 1 month ago

Hi @hexaltation,

I look into this issue and I think the easiest and straightforward solution would be to follow option 2. To be honest, for me personally, it looks more like a bug than a behavior we want to keep. I had hope that it will be sorted out with release of 1d2cf3de49d636755da2c6c453c2232479cb05f1, but it wasn't in the scope.

Let me know if you want to work on it, if not I can take it on this or next week.

hexaltation commented 1 month ago

Hello @berhalak , I've done some work on the option 1 cause it was simple too me for a first exploration of this part of the code base. #1215 If you can quickly solve it with option 2 I'll let you do it. I will focus on close two others issues this week.

berhalak commented 1 week ago

Hi @fflorent @hexaltation.

The change that I proposed was merged today, please take a look https://github.com/gristlabs/grist-core/commit/cbbfa5137467ca523c4f7523fc4adc6714714bf3, and if it addresses your use case we can close this Issue.

hexaltation commented 1 week ago

Hi @fflorent @hexaltation.

The change that I proposed was merged today, please take a look cbbfa51, and if it addresses your use case we can close this Issue.

It seems to fix the issue as the Table.lookupRecords().OtherRecords formulas will now display a nice flattened list.

Thanks a lot :)