feat(dashboards): add reorder and starred endpoints#119784
feat(dashboards): add reorder and starred endpoints#119784adrianviquez wants to merge 4 commits into
Conversation
github-actions
Bot
added
the
Scope: Backend
github-actions
Bot
added
the
Scope: Frontend
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
DominikB2014
left a comment
DominikB2014
left a comment
There was a problem hiding this comment.
lgtm!
This looks pretty similar to the explore saved queries starring, i'm starting to wonder if we should make this more generic, but i'd just follow the rule of 3 here, if there's 3 places that have a similar starring patter, then we try to share logic.
I'm also going to tag @narsaynorath on this, as he worked on dashboard ordering before so maybe there's some edge cases i can't see here
| if features.has("organizations:dashboards-starred", organization, actor=request.user): | ||
| if is_favorited: | ||
| DashboardFavoriteUser.objects.insert_favorite_dashboard( | ||
| organization=organization, | ||
| user_id=request.user.id, | ||
| dashboard=dashboard, | ||
| ) | ||
| else: | ||
| DashboardFavoriteUser.objects.unfavorite_dashboard( | ||
| organization=organization, | ||
| user_id=request.user.id, | ||
| dashboard=dashboard, | ||
| ) | ||
| return Response(status=204) |
There was a problem hiding this comment.
nit: I feel like the variable name is_favourited is confusing, to me it's suggesting the current state of the dashboard, but in reality it represents if we should favourite the dashboard?
There was a problem hiding this comment.
Fair point, and I agree. Will change to should_favorite and have the backend read both from isFavorited and shouldFavorite, should_favorite = request.data.get("shouldFavorite", request.data.get("isFavorited")), so things don't break
|
@DominikB2014 yeah fair point. I opted for this logic to be separate atm to try it out first and not over abstract, and since we had appetite to do it before but ended up reverting those changes. Plus the way we represent queries and dashboards are a bit different in the backend, so I suspect there might be some work there as well to make the endpoints for both queries + dashboards. These are expected to behave the same way in principle, so I think it makes sense to do these changes in the future if the behavior stays. |
3 participants
This PR is one of many to bring the "starred dashboards" sidebar to parity with Explore's "starred queries" sidebar.
This PR adds the starred and starred order endpoints to the dashboard set of endpoints. These are going to be pinged by the frontend to get a customers' favorite/starred dashboards, as well as the option to change their order. Added tests + a test factory as I saw a similar method to create favorite dashboards used in 2 different tests.
Additionally, I saw that the backend doesn't currently add a position when it creates a dashboard, and to change it I would have had to make a change in the deprecated favorited_by setter, so I instead opted for the
insert_favorite_dashboard(and remove) logic from the DashboardFavoriteUser manager, feature flagged atm.Closes BROWSE-612