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

add several new database columns + emit two new Django signals #1522

Merged
merged 30 commits into from
Apr 14, 2023

Conversation

joeyorlando
Copy link
Contributor

@joeyorlando joeyorlando commented Mar 10, 2023

What this PR does

  • add new columns gcom_org_contract_type, gcom_org_irm_sku_subscription_start_date, and gcom_org_oldest_admin_with_billing_privileges_user_id to user_management_organization table + is_restricted column to alerts_alertgroup table
  • emit two new Django signals
    • org_sync_signal at the end of the engine/apps/user_management/sync.py::sync_organization method
    • alert_group_created_signal when a new Alert Group is created

Checklist

  • Tests updated (N/A)
  • Documentation added (N/A)
  • CHANGELOG.md updated

@joeyorlando joeyorlando added pr:no public docs Added to a PR that does not require public documentation updates pr:no changelog labels Mar 10, 2023
@joeyorlando joeyorlando force-pushed the jorlando/sync-cloud-org-license-type branch from b83b393 to 1573288 Compare March 10, 2023 10:28
@joeyorlando joeyorlando changed the title send django signal at end of sync_organization method add gcom_contract_type column + send django signal at end of sync_organization method Mar 10, 2023
@joeyorlando joeyorlando marked this pull request as ready for review March 10, 2023 10:31
@joeyorlando joeyorlando requested a review from a team March 10, 2023 10:31
@joeyorlando joeyorlando force-pushed the jorlando/sync-cloud-org-license-type branch from 3063c1d to 588b4fc Compare March 21, 2023 11:17
@joeyorlando joeyorlando changed the title add gcom_contract_type column + send django signal at end of sync_organization method add 3 new database columns + emit two new Django signals Mar 21, 2023
@joeyorlando joeyorlando self-assigned this Mar 21, 2023
@joeyorlando joeyorlando marked this pull request as draft March 21, 2023 11:55
@joeyorlando joeyorlando force-pushed the jorlando/sync-cloud-org-license-type branch 2 times, most recently from 5d0d2d2 to e554c6b Compare March 28, 2023 13:35
@joeyorlando joeyorlando changed the title add 3 new database columns + emit two new Django signals add several new database columns + emit two new Django signals Mar 28, 2023
Copy link
Contributor

@iskhakov iskhakov left a comment

Choose a reason for hiding this comment

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

nice done! Left a few minor comments

@joeyorlando joeyorlando force-pushed the jorlando/sync-cloud-org-license-type branch from 5525e81 to 57f3549 Compare March 29, 2023 11:34
Comment on lines -139 to -148
with patch.object(GrafanaAPIClient, "is_rbac_enabled_for_organization", return_value=False):
with patch.object(GrafanaAPIClient, "get_users", return_value=api_users_response):
with patch.object(GrafanaAPIClient, "get_teams", return_value=(api_teams_response, None)):
with patch.object(GrafanaAPIClient, "get_team_members", return_value=(api_members_response, None)):
with patch.object(
GrafanaAPIClient, "check_token", return_value=(None, api_check_token_call_status)
):
with patch.object(
GrafanaAPIClient, "get_grafana_plugin_settings", return_value=({"enabled": True}, None)
):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mostly just refactored this for readability sake.. having 8 nested context managers is really difficult to deal with...

@joeyorlando joeyorlando force-pushed the jorlando/sync-cloud-org-license-type branch from d077df6 to 63c9e01 Compare March 31, 2023 12:58
CHANGELOG.md Outdated Show resolved Hide resolved

class Meta:
model = Alert
fields = [
"id",
"raw_request_data",
]

def get_raw_request_data(self, obj):
# TODO:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we want to show something more useful than just an empty object in this case?

- add new columns
  - `gcom_org_contract_type` and `gcom_org_has_irm_sku` on
    `user_management_organization` table
  - `is_restricted` on `alerts_alertgroup` table
  - `alert_groups_created_counter_current_month` and
    `alert_groups_created_this_month` on `alerts_alertreceivechannel`
    table
- emit two new Django signals
  - `org_sync_signal` at the end of the organization sync task
  - `alert_group_created_signal` when a new Alert Group is created
@joeyorlando joeyorlando force-pushed the jorlando/sync-cloud-org-license-type branch from 219f2a3 to 3cc9fcf Compare April 6, 2023 09:58
@joeyorlando joeyorlando marked this pull request as ready for review April 6, 2023 10:00
}

.tag {
font-size: 12px;
Copy link
Member

Choose a reason for hiding this comment

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

Initially the plan was to add these tags to show up on Integration page as well but now that was removed from scope - for now I'll let them here as we might iterate again and possibly add the limit tag display in here.

@joeyorlando joeyorlando merged commit 3b274f4 into dev Apr 14, 2023
@joeyorlando joeyorlando deleted the jorlando/sync-cloud-org-license-type branch April 14, 2023 07:15

Choose a reason for hiding this comment

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

It might be required to change slack, telegram and MSTeams renderers as well to restrict alert groups there. Worth to check if not escalation alert group also means not delivering it to the chatops integration.
Check distribute_alert task.

@@ -14,6 +14,12 @@
]
)

alert_group_created_signal = django.dispatch.Signal(

Choose a reason for hiding this comment

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

Please, add comment that subscriber to that signal must be super-fast, since it's process of creating alert group

from .user import FastUserSerializer

logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)


class AlertGroupFieldsCacheSerializerMixin:
class AlertGroupFieldsCacheSerializerMixin(AlertsFieldCacheBusterMixin):
CACHE_KEY_FORMAT_TEMPLATE = "{field_name}_alert_group_{object_id}"

Choose a reason for hiding this comment

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

👍🏻

brojd pushed a commit that referenced this pull request Sep 18, 2024
# What this PR does

- add new columns `gcom_org_contract_type`,
`gcom_org_irm_sku_subscription_start_date`, and
`gcom_org_oldest_admin_with_billing_privileges_user_id` to
`user_management_organization` table + `is_restricted` column to
`alerts_alertgroup` table
- emit two new Django signals
- `org_sync_signal` at the end of the
`engine/apps/user_management/sync.py::sync_organization` method
  - `alert_group_created_signal` when a new Alert Group is created

## Checklist

- [ ] Tests updated (N/A)
- [ ] Documentation added (N/A)
- [x] `CHANGELOG.md` updated

---------

Co-authored-by: Rares Mardare <rares.mardare@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.

6 participants