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

Fix sync matching for multi team setup #34

Merged
merged 4 commits into from Dec 6, 2022

Conversation

scopsy
Copy link
Contributor

@scopsy scopsy commented Dec 4, 2022

Summary

When filtering sync, add a linearTeamId as part of the comparison to allow same user to be part of multiple linear teams. Internally we would love to sync issues from a Roadmap team to another repository at https://github.com/novuhq/roadmap, currently issues are not synced in this setup (we already sync issues for our main repo novuhq/novu

Reproduce

  • Create 2 linear teams
  • Assign a user to be part of both teams
  • Try to sync issues from both teams
  • Only issues created from the first team will be synced

Fixes a problem when same users part of multiple linear teams would always choose first sync
@scopsy scopsy changed the title Fix sync matching for multi team Fix sync matching for multi team setup Dec 4, 2022
@scopsy
Copy link
Contributor Author

scopsy commented Dec 4, 2022

Not sure if there are other parts of the code than need this to be added to address this. But for our particular cases it seems to work well.

Copy link
Collaborator

@tedspare tedspare left a comment

Choose a reason for hiding this comment

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

Linear comment events seem to not include their teamId - what happens in that case?

Otherwise, LGTM

@PeerRich
Copy link
Member

PeerRich commented Dec 5, 2022

is this safe to merge?

@scopsy
Copy link
Contributor Author

scopsy commented Dec 6, 2022

@tedspare Let me check how it works for comments on linear 🙏

@scopsy
Copy link
Contributor Author

scopsy commented Dec 6, 2022

@tedspare good catch, i've refactored the sync find to take in consideration cases for where teamId is not present. Because all further downstream calls only rely on the syncedIssue found on prisma the flow should work as usual, since the user is the relevant entity and doesn't matter much from what team he comes

Copy link
Collaborator

@tedspare tedspare left a comment

Choose a reason for hiding this comment

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

LGTM

@tedspare
Copy link
Collaborator

tedspare commented Dec 6, 2022

@scopsy would you be open to resolving the merge conflicts, by pulling from the parent repo's main branch? Apologies if I merged the previous PR #35 in the wrong order.

@scopsy
Copy link
Contributor Author

scopsy commented Dec 6, 2022

@tedspare done 🎉

@tedspare tedspare merged commit 9d72173 into calcom:main Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants