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

Teams redesign #1528

Merged
merged 60 commits into from
Mar 21, 2023
Merged

Teams redesign #1528

merged 60 commits into from
Mar 21, 2023

Conversation

iskhakov
Copy link
Contributor

@iskhakov iskhakov commented Mar 13, 2023

What this PR does

  • api returns all the resources available to the user by default
  • substitutes team switcher with multi-select team filter
  • allow referencing between integrations - escalations chains - [schedules, outgoing webhooks] across teams
Screen.Recording.2023-03-16.at.21.37.41.mov

Which issue(s) this PR fixes

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated

@iskhakov iskhakov requested review from a team March 13, 2023 08:40
@iskhakov iskhakov marked this pull request as draft March 13, 2023 08:40
@iskhakov
Copy link
Contributor Author

there are two bugs in search/filter right now

  • filtering by team by w/o team doesn't work
  • filtering in maintenances doesn't work

@iskhakov
Copy link
Contributor Author

iskhakov commented Mar 20, 2023

@vadimkerr thank you for the thorough review

  • When loading a page with the team filter pre-selected, it first loads all resources from org and only then re-renders the page with relevant results, which makes all the pages look a bit glitchy. Is this expected?

Maxim is catching up on that

  • Does the user page show all users from org? I'm not sure which users are shown and there's no info on that. Probably treating users as all other resources would make sense? For example, it could be useful to filter users by team and see their notification policies IMO.
    It is now showing all the users, you're thinking in the right direction, it is going to be the next goal

  • Will the old frontend version work well with the new backend?
    Old versions of backend will show all the resources user has access to, without filtering abiity

Thank you for poitning the bug with General team, it is fixed

@iskhakov
Copy link
Contributor Author

@joeyorlando thank you for all the ideas you proposed, I implemented most of them already. The rest will go in the separate PR

try:
obj = organization.custom_buttons.get(public_primary_key=pk)
obj = organization.custom_buttons.filter(*self.available_teams_lookup_args).get(public_primary_key=pk)
Copy link
Member

Choose a reason for hiding this comment

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

This is used to show selected object in escalation step if this object belongs to other team (case when escalation chain was moved to another team). I don't think that we should filter by available teams here 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we will display this kind of schedules as "🔒private outgoing webhook"

Copy link
Member

Choose a reason for hiding this comment

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

got it, good idea 👍

@@ -219,6 +227,7 @@ def get_object_from_organization(self):
organization = self.request.auth.organization
queryset = organization.oncall_schedules.filter(
public_primary_key=pk,
*self.available_teams_lookup_args,
Copy link
Member

Choose a reason for hiding this comment

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

Same case as with custom buttons. This method is used to show selected object in escalation step if this object belongs to other team (case when escalation chain was moved to another team). I don't think that we should filter by available teams here 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we will display this kind of schedules as "🔒private schedule"

@@ -708,20 +708,19 @@ class AlertRules extends React.Component<AlertRulesProps, AlertRulesState> {
{channelFilterIds.length > 1 && <Text keyboard>ELSE</Text>}
<Text>route to escalation chain:</Text>
<WithPermissionControlTooltip userAction={UserActions.IntegrationsWrite}>
<div onClick={(e) => e.stopPropagation()}>
Copy link
Member

Choose a reason for hiding this comment

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

Why the removal of stopPropagation ? There's no comment indicating why we're doing that but I assume it's fix for a different scenario.

serializer.is_valid(raise_exception=True)

users = [(user["instance"], user["important"]) for user in serializer.validated_data["users"]]
schedules = [
(schedule["instance"], schedule["important"]) for schedule in serializer.validated_data["schedules"]
]

print(serializer.validated_data["team"])
Copy link
Contributor

Choose a reason for hiding this comment

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

print statement can be removed

Copy link
Contributor

Choose a reason for hiding this comment

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

Note we introduced an OutgoingWebhooks2 page for the new webhooks backend (that will eventually replace the original one), I guess we will need a similar update there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, good point

@iskhakov iskhakov added the pr:no public docs Added to a PR that does not require public documentation updates label Mar 21, 2023
@iskhakov
Copy link
Contributor Author

public docs will go in the separate PR

@iskhakov iskhakov merged commit d3c6621 into dev Mar 21, 2023
@iskhakov iskhakov deleted the iskhakov/teams-refactoring branch March 21, 2023 16:57
brojd pushed a commit that referenced this pull request Sep 18, 2024
# What this PR does

* api returns all the resources available to the user by default 
* substitutes `team switcher` with `multi-select team filter`
* allow referencing between integrations - escalations chains -
[schedules, outgoing webhooks] across teams



https://user-images.githubusercontent.com/2262529/225634581-2d2e8af2-15ce-4c01-a90e-8267d98f5a23.mov



## Which issue(s) this PR fixes

## Checklist

- [ ] Tests updated
- [ ] Documentation added
- [ ] `CHANGELOG.md` updated

---------

Co-authored-by: Maxim <maxim.mordasov@grafana.com>
Co-authored-by: Joey Orlando <joey.orlando@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants