feat(issue-views): Update GET endpoint to validate view's projects #84338
Conversation
github-actions
bot
added
the
Scope: Backend
MichaelSun48
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #84338 +/- ##
==========================================
+ Coverage 87.63% 87.65% +0.02%
==========================================
Files 9584 9597 +13
Lines 542887 543709 +822
Branches 21271 21271
==========================================
+ Hits 475745 476613 +868
+ Misses 66790 66744 -46
Partials 352 352 |
| for view in query: | ||
| if view.is_all_projects or view.projects.count() > 1 or view.projects.count() == 0: | ||
| view.is_all_projects = False | ||
| view.projects.set([pick_default_project(organization, request.user)]) |
There was a problem hiding this comment.
In the worst case this will make a db query for each view - can we make sure this only gets evaluated once?
There was a problem hiding this comment.
Good catch, just updated the logic. Should be much more efficient now!
| for view in query: | ||
| if view.is_all_projects or view.projects.count() > 1 or view.projects.count() == 0: | ||
| views_to_updates = [] | ||
| default_project = pick_default_project(organization, request.user) |
There was a problem hiding this comment.
Technically this query might not be necessary if all the views have a single project, so we can make this even better if we avoid it in that case
There was a problem hiding this comment.
I think this is handled by Django's lazy loading of query sets - so if default project is never accessed in the for loop after this, the DB query isn't actually made.
| if self.has_global_views is False: | ||
| is_all_projects = False | ||
|
|
||
| projects = list(obj.projects.values_list("id", flat=True)) |
There was a problem hiding this comment.
Does this make a query for each view? This would be slow for many views and would be better if we could prefetch them in the initial query since we know we'll need it anyway
There was a problem hiding this comment.
Added! (organization_group_search_views, line 75)
This PR updates the GET
organization/.../group-search-viewsendpoint to validate a views projecst before being sent. The logic is:If your org doesn't have the multi project stream feature (
organizations:global-views) any views that have an invalid projects parameter ("All Projects", "My Projects", or len(projects) > 1) will be set to the first project you have, alphabetically.No validation is necessary for organizations that do have the multi project stream feature.