Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lists of RecordSets cannot be typed as Reference List #1130

Closed
vviers opened this issue Jul 29, 2024 · 7 comments
Closed

Lists of RecordSets cannot be typed as Reference List #1130

vviers opened this issue Jul 29, 2024 · 7 comments
Assignees
Labels
anct enhancement New feature or request

Comments

@vviers
Copy link
Collaborator

vviers commented Jul 29, 2024

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
Copy link
Collaborator Author

vviers commented Jul 29, 2024

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

@vviers vviers added enhancement New feature or request good first issue Good for newcomers labels Aug 6, 2024
@hexaltation hexaltation self-assigned this Sep 12, 2024
@hexaltation
Copy link
Collaborator

@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
Copy link
Collaborator Author

vviers commented Sep 12, 2024

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

@berhalak
Copy link
Contributor

berhalak commented Oct 1, 2024

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 1d2cf3d, 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
Copy link
Collaborator

hexaltation commented Oct 2, 2024

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.

@hexaltation hexaltation assigned berhalak and unassigned hexaltation Oct 2, 2024
@fflorent fflorent removed the good first issue Good for newcomers label Oct 10, 2024
@berhalak
Copy link
Contributor

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.

@hexaltation
Copy link
Collaborator

hexaltation commented Oct 24, 2024

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 :)

@github-project-automation github-project-automation bot moved this from In Progress to Done in French administration Board Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
anct enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants