-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(dashboards): Implements syncing prebuilt dashboards to backend #101968
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
feat(dashboards): Implements syncing prebuilt dashboards to backend #101968
Conversation
…rebuilt-dashboard-sync-backend
|
This PR has a migration; here is the generated SQL for for --
-- Add field prebuilt_id to dashboard
--
ALTER TABLE "sentry_dashboard" ADD COLUMN "prebuilt_id" integer DEFAULT NULL NULL CHECK ("prebuilt_id" >= 0);
--
-- Alter field created_by_id on dashboard
--
ALTER TABLE "sentry_dashboard" ALTER COLUMN "created_by_id" DROP NOT NULL;
--
-- Create constraint sentry_dashboard_organization_prebuilt_id_uniq on model dashboard
--
CREATE UNIQUE INDEX CONCURRENTLY "sentry_dashboard_organization_prebuilt_id_uniq" ON "sentry_dashboard" ("organization_id", "prebuilt_id") WHERE "prebuilt_id" IS NOT NULL;
--
-- Create constraint sentry_dashboard_prebuilt_null_created_by on model dashboard
--
ALTER TABLE "sentry_dashboard" ADD CONSTRAINT "sentry_dashboard_prebuilt_null_created_by" CHECK (("prebuilt_id" IS NULL OR "created_by_id" IS NULL)) NOT VALID;
ALTER TABLE "sentry_dashboard" VALIDATE CONSTRAINT "sentry_dashboard_prebuilt_null_created_by"; |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #101968 +/- ##
===========================================
+ Coverage 73.25% 80.96% +7.71%
===========================================
Files 8726 8729 +3
Lines 388171 388444 +273
Branches 24628 24627 -1
===========================================
+ Hits 284347 314519 +30172
+ Misses 103464 73565 -29899
Partials 360 360 |
…rebuilt-dashboard-sync-backend
|
Theres an awkward thing where we already have an existing concept of prebuilt dashboards ie the |
| for id in dashboards.values_list("created_by_id", flat=True).filter( | ||
| created_by_id__isnull=False | ||
| ) | ||
| if id is not None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're excluding none in the conditions, I don't think this filter condition will be hit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I had to add this here because mypy would still interpret id as int | None despite the filter condition.
I think this should be fine? Alternatively should we just add a type ignore? Let me know what you think!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok. Sacrifices must be made to the mypy gods 🔥
| assert final_count == initial_count | ||
| assert final_count == len(PREBUILT_DASHBOARDS) | ||
|
|
||
| def test_endpoint_deletes_old_prebuilt_dashboards_not_in_list(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when a user attempts to update a pre-built dashboard, or delete one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll be locking down these dashboards so they can't be edited or deleted by users. Will be adding this in a follow up PR!
…rebuilt-dashboard-sync-backend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Lazily creates prebuilt dashboards on fetch if they do not already exist in the database. Stored prebuilt dashboards are mostly empty except for a
prebuilt_idthat will be used in the frontend to determine the type of content to display in the ui.Model changes:
prebuilt_idas a new column in the dashboards model + a constraintcreated_by_idto be nullable, since prebuilt dashboards are not created by a userhttps://linear.app/getsentry/document/how-will-we-store-static-dashboards-0d85ebe8c8ae