server icon indicating copy to clipboard operation
server copied to clipboard
verified publisher icon bitwarden

Add Collections to public/groups API endpoint

Open Manolachi opened this issue 3 years ago • 2 comments

Type of change

  • [x] Bug fix
  • [ ] New feature development
  • [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • [ ] Build/deploy pipeline (DevOps)
  • [ ] Other

Objective

Amend the group objects returned when calling the /public/groups API endpoint to also include information about their associated collections. Resolves https://github.com/bitwarden/server/issues/852. Implementation is based on the discussion from https://github.com/bitwarden/server/pull/1455.

Asana task: https://app.asana.com/0/1200885895476554/1201809623023340/f

Code changes

  • IGroupRepository.cs: Added a new method which would return all the groups, each one paired with the list of it's associated collections.
  • Infrastructure.Dapper/Repositories/GroupRepository.cs: Added the Dapper implementation for the new method.
  • Infrastructure.EntityFramework/Repositories/GroupRepository.cs: Added the EF implementation for the new method.
  • GroupsController.cs: Replace the call to repository from the /public/groups API endpoint with the newly added method and use both tuple items returned when building the response.
  • Group_ReadWithCollectionsByOrganizationId.sql: Added a new stored procedure for the Dapper implementation.
  • 2017-08-19_00_InitialSetup.sql: Added the new stored procedure for migration purposes.
  • Sql.sqlproj: Included the newly added stored procedure.

Testing requirements

Verify that when sending a GET request to the public/groups API endpoint, the "collections" field of each returned group contains a list with it's associated collections.

Before you submit

  • [x] I have checked for formatting errors (dotnet tool run dotnet-format --check) (required)
  • [x] If making database changes - I have also updated Entity Framework queries and/or migrations
  • [ ] I have added unit tests where it makes sense to do so (encouraged but not required)
  • [ ] This change requires a documentation update (notify the documentation team)
  • [ ] This change has particular deployment requirements (notify the DevOps team)

Manolachi avatar Feb 11 '22 08:02 Manolachi

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Feb 11 '22 08:02 CLAassistant

@Manolachi Thank you for your contribution. I've added this to our internal Community PR board for review

djsmith85 avatar Feb 11 '22 22:02 djsmith85