-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(profile): show Teams memberships on account details #57439
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Cristian Scheid <[email protected]>
Signed-off-by: Cristian Scheid <[email protected]>
come-nc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s a bit sad that we have to turn Circle instances into Team instances, I wonder why Team is not an ITeam interface implemented by Circle 🤷
But I guess it’s too late to change that without breaking API.
A few remarks but looks good 👍
| $team->getDisplayName(), | ||
| $this->urlGenerator->linkToRouteAbsolute('contacts.contacts.directcircle', ['singleId' => $team->getSingleId()]), | ||
| ); | ||
| }, $this->circlesManager->getCircles()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* WARNING: This method is not using Cached Memberships meaning that the request can be heavy and should
* only be used if probeCircles() does not fit your need.
*
* Always prefer probeCircles();
According to circles code it may be better to call probeCircles here?
ping @ArtificialOwl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out! Just to let you know I've tested locally using probeCircles() instead of getCircles() and it works perfectly.
Following the docblock recommendation, since this fits our use case well, I think probeCircles() is the appropriate way of doing it.
I'll wait for @ArtificialOwl's confirmation before updating the implementation.
Co-authored-by: Côme Chilliet <[email protected]> Signed-off-by: Cristian Scheid <[email protected]>
Signed-off-by: Cristian Scheid <[email protected]>
Summary
Adds Teams (circles) memberships to the personal account details page, providing users with a complete overview of all their group memberships. This enhances the current scenario where only group memberships from connected user backends are displayed, excluding Teams memberships.
Before
After
Checklist
3. to review, feature component)stable32)