[PM-8895] Moving the groups controller business logic to a service #4209
Conversation
| /// view all groups in the organization</param> | ||
| /// <param name="orgId">Organization id</param> | ||
| /// <returns>List of GroupDetailsResponseModel</returns> | ||
| public async Task<IEnumerable<GroupDetailsResponseModel>> GetOrganizationGroupsDetails(ClaimsPrincipal user, Guid orgId) |
There was a problem hiding this comment.
Returning IEnumerable<GroupDetailsResponseModel>' instead of ListResponseModelthe Controller will return theListResponseModel`
This can be changed back if it's determined to be better.
| private readonly IOrganizationUserRepository _organizationUserRepository; | ||
| private readonly ICollectionRepository _collectionRepository; | ||
|
|
||
| public GroupsControllerService( |
There was a problem hiding this comment.
The name can be anything really. This was just what first came to mind. Up to suggestions if anyone has any.
There was a problem hiding this comment.
Hey @ttalty - I brought these changes back to the team to discuss and we want to take a step back to fully understand the why/problem you're solving. In its current state, we don't see what is gained with the changes in this PR. The mixing of API layer and Core layer concerns doesn't seem to align with our CQRS approach, overall. Can you help us dig a little deeper and expand on the end goal?
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-8995
📔 Objective
The GroupsController contained a lot of logic that is responsible for creating the objects for the endpoints. Moving to a service allows reusability of the code in the new reports being developed. The controller has repositories injected in. Exposing repos in a controller could open up issues with bypassing logic in services designed to filter. Also a potential issue if the repo gets exposed.
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes