Skip to content

Add optional parameter for get /groups ?include_metrics #39

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.

Sign up for GitHub

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

Merged
Andrei-Dolgolev merged 2 commits into from
Aug 17, 2022
Merged

Add optional parameter for get /groups ?include_metrics #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bd6bee
Add optional parameter for get /groups ?include_metrics
Aug 17, 2022
4fce507
Black formating.
Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions brood/actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,7 @@ def create_group(


def get_groups_for_user(
session: Session, user_id: uuid.UUID
session: Session, user_id: uuid.UUID, include_metrics: bool = True
) -> List[data.GroupUserResponse]:
"""
Get list of groups current user belongs to.
Expand All @@ -1180,19 +1180,27 @@ def get_groups_for_user(
.all()
)

groups_response = [
data.GroupUserResponse(
group_id=group.id,
user_id=group.user_id,
user_type=group.user_type,
autogenerated=group.autogenerated,
group_name=group.name,
parent=group.parent,
num_users=get_num_users(session, group),
num_seats=get_num_seats(session, group),
groups_response = []

for group in groups:
num_users = None
num_seats = None
if include_metrics:
num_users = get_num_users(session, group)
num_seats = get_num_seats(session, group)

groups_response.append(
data.GroupUserResponse(
Copy link
Copy Markdown
Member

@zomglings zomglings Aug 17, 2022

Choose a reason for hiding this comment

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

@Andrei-Dolgolev : Do you have to change Pydantic model to make num_users and num_seats Optional?

Copy link
Copy Markdown
Contributor Author

@Andrei-Dolgolev Andrei-Dolgolev Aug 17, 2022

Choose a reason for hiding this comment

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

Already Optional

group_id=group.id,
user_id=group.user_id,
user_type=group.user_type,
autogenerated=group.autogenerated,
group_name=group.name,
parent=group.parent,
num_users=num_users,
num_seats=num_seats,
)
)
for group in groups
]

return groups_response

Expand Down
5 changes: 4 additions & 1 deletion brood/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,13 +810,16 @@ async def get_group_children_handler(
async def get_groups_handler(
current_user: models.User = Depends(get_current_user),
db_session=Depends(yield_db_session_from_env),
include_metrics: bool = Query(True),
) -> data.GroupUserListResponse:
"""
Get list of groups user belongs to.
"""
groups_list: Optional[List[data.GroupUserResponse]] = []
try:
groups_list = actions.get_groups_for_user(db_session, user_id=current_user.id)
groups_list = actions.get_groups_for_user(
db_session, user_id=current_user.id, include_metrics=include_metrics
)
except Exception as e:
logger.error(f"Error getting list of groups for user: {str(e)}")
raise HTTPException(status_code=500)
Expand Down