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

Background tasks trigger multiple multiple history events instead of one #6295

Closed
SamuelHassine opened this issue Mar 9, 2024 · 4 comments
Closed
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)
Milestone

Comments

@SamuelHassine
Copy link
Member

Description

  1. Update the author of a list of entities via a background task

Result in history:

image

The replace "nothing" is incorrect.

@SamuelHassine SamuelHassine added bug use for describing something not working as expected needs triage use to identify issue needing triage from Filigran Product team and removed needs triage use to identify issue needing triage from Filigran Product team labels Mar 9, 2024
@SamuelHassine SamuelHassine added this to the Release 6.0.6 milestone Mar 9, 2024
@SouadHadjiat SouadHadjiat self-assigned this Mar 11, 2024
@Jipegien Jipegien modified the milestones: Release 6.0.6, Release 6.0.7 Mar 12, 2024
@SouadHadjiat SouadHadjiat removed their assignment Mar 14, 2024
@Jipegien Jipegien modified the milestones: Release 6.0.7, Release 6.0.8 Mar 14, 2024
@marieflorescontact marieflorescontact self-assigned this Mar 14, 2024
@marieflorescontact
Copy link
Member

Author is a relation so it is deleted then recreated by taskmanager. That's why we have 2 events. What can we do to prevent this in your opinion @SamuelHassine ?
see: taskmanger.js > executeReplace()

Is it only message clarity problem? we can replace :
admin replaces nothing in Author by admin removes value in Author

@SamuelHassine
Copy link
Member Author

SamuelHassine commented Mar 18, 2024

No, I think this is an event problem, it should not generate a remove / create. Check with @Kedae please.

@marieflorescontact
Copy link
Member

any thoughts on that @richard-julien ?

@richard-julien
Copy link
Member

Looking the code maybe the problem is the manager is not using the new capabilities of the patch function.

The current code is like this

if (contextType === ACTION_TYPE_RELATION) {
   // 01 - Delete all relations of the element
   const rels = await listAllRelations(context, user, field, { fromId: element.id });
   for (let indexRel = 0; indexRel < rels.length; indexRel += 1) {
     const rel = rels[indexRel];
     await deleteElementById(context, user, rel.id, rel.entity_type);
   }
   // 02 - Create new ones
   for (let indexCreate = 0; indexCreate < values.length; indexCreate += 1) {
     const target = values[indexCreate];
     await createRelation(context, user, { fromId: element.id, toId: target, relationship_type: field });
   }
 }

And i suppose that the ACTION_TYPE_RELATION is in fact only related to ref relationships.

With latest platform improvements, the patch function shoud now support to do ref replace directly.

await patchAttribute(context, user, element.id, element.entity_type, patch);

So for me the first part of the code must be removed / migrate to also use the patchAttribute function.

@Jipegien Jipegien modified the milestones: Release 6.0.8, Release 6.0.9 Mar 20, 2024
marieflorescontact added a commit that referenced this issue Mar 26, 2024
Co-authored-by: Angelique <angelique.jard@filigran.io>
@SamuelHassine SamuelHassine added the solved use to identify issue that has been solved (must be linked to the solving PR) label Mar 26, 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
Development

No branches or pull requests

5 participants