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

[backend] Improve objects resolutions for container (#4153) #4159

Merged
merged 3 commits into from
Aug 25, 2023

Conversation

richard-julien
Copy link
Member

No description provided.

@richard-julien richard-julien linked an issue Aug 24, 2023 that may be closed by this pull request
@richard-julien richard-julien added the filigran team use to identify PR from the Filigran team label Aug 24, 2023
@SouadHadjiat
Copy link
Member

SouadHadjiat commented Aug 24, 2023

When I create an ObservedData from a domain name, it has the name of the domain name, then I add a note, there is no error anymore but the ObservedData name is now "empty".

Capture d'écran 2023-08-24 213228

===> update
I began to debug objects method which returns an empty list, and inside relations returns only one relationship with the domain name, and elements returns the note only. I guess the method should return both ?
=> it returns only one element because first argument is 1 in resolveName. So we only query 1 relationship and 1 element, but both are not the same.

const data = args.all ? await paginateAllThings(context, user, types, R.assoc('filters', filters, args))
: await listThings(context, user, types, R.assoc('filters', filters, args));
const queryFilters = { ...args, filters, connectionFormat: false };
const elementsPromise = args.all ? listAllThings(context, user, types, queryFilters) : listThings(context, user, types, queryFilters);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we retrieve elements from another query rather than retrieve them from the relations ?
We can have const toIds = relations.map((rel) => rel.toId) and resolve toIds, no ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a general problem of the current model. In order to support case of pagination/sorting/filtering we use the denormalized id of elements stored in rel_. Unfortunalty we miss the fact that storing also the direction was needed and so when we list through that part we have sometime to much elements.

@RomuDeuxfois
Copy link
Member

When I create an ObservedData from a domain name, it has the name of the domain name, then I add a note, there is no error anymore but the ObservedData name is now "empty".

Capture d'écran 2023-08-24 213228

===> update I began to debug objects method which returns an empty list, and inside relations returns only one relationship with the domain name, and elements returns the note only. I guess the method should return both ? => it returns only one element because first argument is 1 in resolveName. So we only query 1 relationship and 1 element, but both are not the same.

I test this case and it's fixed now

@RomuDeuxfois
Copy link
Member

Tested 🆗

@RomuDeuxfois RomuDeuxfois merged commit 816413c into master Aug 25, 2023
6 checks passed
@RomuDeuxfois RomuDeuxfois deleted the issue/JRI_4153 branch August 25, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filigran team use to identify PR from the Filigran team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating a note on observed data crashes the website
3 participants