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

Relationship updates at CSV file import don't check confidence #6518

Closed
Archidoit opened this issue Mar 29, 2024 · 3 comments
Closed

Relationship updates at CSV file import don't check confidence #6518

Archidoit opened this issue Mar 29, 2024 · 3 comments
Assignees
Labels
bug use for describing something not working as expected solved use to identify issue that has been solved (must be linked to the solving PR)

Comments

@Archidoit
Copy link
Member

Description

Relationship updates at CSV file import don't check confidence

Reproducible Steps

  • Log as userA with confidence = 100
  • Create entities/relationships that are in a csv file, with confidence = 100
  • Log as a userB with confidence = 50
  • Import a file in Data > Import
  • Launch an import of the file via a CSV mapper
  • The entities/relationships are updated with the userB name in 'author'

Expected behavior

The entities/relationsihps shouldn't be update because their confidences are higher than the one of userB

@Archidoit Archidoit added bug use for describing something not working as expected needs triage use to identify issue needing triage from Filigran Product team labels Mar 29, 2024
@SamuelHassine SamuelHassine added this to the Release 6.0.9 milestone Mar 30, 2024
@jborozco jborozco removed the needs triage use to identify issue needing triage from Filigran Product team label Apr 2, 2024
@nino-filigran nino-filigran added needs more info Intel needed about the use case and removed needs more info Intel needed about the use case labels Apr 2, 2024
@SamuelHassine SamuelHassine modified the milestones: Release 6.0.9, Release 6.0.10 Apr 3, 2024
@lndrtrbn lndrtrbn self-assigned this Apr 5, 2024
@lndrtrbn
Copy link
Member

lndrtrbn commented Apr 5, 2024

@Archidoit don t you mean 'creator' instead of 'author' ? That's what I can reproduce locally in any case. If it is the field author you are effectively talking too then I'll need some help to reproduce it

EDIT:

  • yes we are talking of the creator field

@lndrtrbn
Copy link
Member

Ok so the root cause was found and I fixed it in the draft PR above.
Nonetheless I would like confirmation from @nino-filigran and @richard-julien about it.

Recap of the actual behavior: when upserting entities, we add the user upserting in the array of creators no matter the confidence of the user. Why? In the function upsertElement of the file middleware.js there is an algorithm to determine which fields should be upserted or not and in this algorithm we found this line (l2520):

// Ids and creators consolidation is always granted
const isStructuralUpsert = attributeKey === xOpenctiStixIds.name || attributeKey === creators.name;

This boolean isStructuralUpsert is then used to bypass other checks like the confidence.

// Upsert will be done if upsert is well-defined but also in full synchro mode or if the current value is empty
if (isStructuralUpsert || canBeUpsert || isFullSync || isCurrentlyEmpty) {
  inputs.push(...buildAttributeUpdate(isFullSync, attribute, element[attributeKey], inputData));
}

I'm not sure to understand the purpose of this boolean but if it exists, there should have a reason. But at the end, I think that if there is only the array of creators that has been changed with the upsert, and that user confidence is too low, then we should not taking it into account.

Maybe I'm not right, can you confirm?

@lndrtrbn lndrtrbn added the needs more info Intel needed about the use case label Apr 10, 2024
@richard-julien
Copy link
Member

richard-julien commented Apr 10, 2024

Creators must always be upserted independently of the confidence level.
This is intended an so not a bug

@SamuelHassine SamuelHassine added solved use to identify issue that has been solved (must be linked to the solving PR) and removed needs more info Intel needed about the use case labels Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug use for describing something not working as expected solved use to identify issue that has been solved (must be linked to the solving PR)
Projects
None yet
6 participants