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

Add test case for getActiveParticipationInvitation with multiple part… #483

Conversation

yuanchen233
Copy link
Collaborator

…icipation

@yuanchen233 yuanchen233 marked this pull request as ready for review September 24, 2024 22:30
Whathecode
Whathecode previously approved these changes Sep 26, 2024
Copy link
Member

@Whathecode Whathecode left a comment

Choose a reason for hiding this comment

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

To keep the test focused, I added a createTwoDevicesAndRolesProtocol() test helper, which is likely to also be useful in future test cases.

I added additional assertions to verify both accounts, as well as the devices they were assigned.

Lastly, I fixed to code styles to follow the non-idiomatic style of this project. ;) I think the static checker only checks for it in main sources, or it failed here. Either way: I'll gladly polish minor stuff like that in any PR you send along. :)

@Whathecode Whathecode force-pushed the 482-why-am-i-getting-all-invitations-incl-the-ones-not-assigned-to-me branch from dafaf76 to e122e42 Compare September 26, 2024 18:25
@Whathecode Whathecode self-requested a review September 26, 2024 18:26
Copy link
Member

@Whathecode Whathecode left a comment

Choose a reason for hiding this comment

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

I added an exception for test sources for detekt flagging destructuring operations > 3 fields.

@Whathecode Whathecode merged commit 9b46a19 into develop Sep 26, 2024
3 checks passed
@Whathecode Whathecode deleted the 482-why-am-i-getting-all-invitations-incl-the-ones-not-assigned-to-me branch September 26, 2024 18:35
@yuanchen233
Copy link
Collaborator Author

Thank you for the explanation and commentnt.

To keep the test focused, I added a createTwoDevicesAndRolesProtocol() test helper, which is likely to also be useful in future test cases.

I thought about re-using createComplexProtocol and expose Roles, but this information is not used by other test cases. This multi role protocol should also be relevant when adding more InputDataType.

I'll be adopting to the decision making logic (and maybe code style here)

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.

Why am I getting all invitations, incl. the ones not assigned to me?
2 participants