[MNOE-146] Display teams admin panel #583
Conversation
|
|
||
| protected | ||
|
|
||
| def fetch_all_teams(organization_id) |
There was a problem hiding this comment.
what is the difference between fetch_all_teams and fetch_teams ? keep fetch_teams only.
| @@ -19,7 +19,7 @@ def timestamp | |||
| end | |||
|
|
|||
| def parent_organization | |||
| @parent_organization ||= current_user.organizations.to_a.find { |o| o.id.to_s == params[:organization_id].to_s } | |||
| @parent_organization ||= MnoEnterprise::Organization.includes(:orga_relations).find(params[:organization_id]).first | |||
There was a problem hiding this comment.
replace by find_one(params[:organization_id], :orga_relations)
| #================================================================== | ||
| # GET /mnoe/jpi/v1/admin/organizations/1/teams | ||
| def index | ||
| authorize! :read, parent_organization |
There was a problem hiding this comment.
Do we need authorize there? It's in the admin endpoint
| def fetch_teams(organization_id) | ||
| MnoEnterprise::Team | ||
| .apply_query_params(params) | ||
| .includes(RELATIONSHIPS) |
There was a problem hiding this comment.
Should we use select here as part of the perf improvements?
| @@ -0,0 +1,20 @@ | |||
| @all_apps ||= MnoEnterprise::App.all.to_a | |||
There was a problem hiding this comment.
Do we really to load all apps for each team here?
Couldn't we load and include team -> app_instances -> app
Especially if it's just to display the logo
| @@ -19,7 +19,7 @@ def timestamp | |||
| end | |||
|
|
|||
| def parent_organization | |||
| @parent_organization ||= current_user.organizations.to_a.find { |o| o.id.to_s == params[:organization_id].to_s } | |||
| @parent_organization ||= MnoEnterprise::Organization.find_one(params[:organization_id], :orga_relations) | |||
There was a problem hiding this comment.
I think this helper was not used before and is just a bad copy paste from the regular jpi controller.
How about removing it and inline the code in the controller that uses it?
- Extract TeamsController to Concern - Do not load all apps - Make a smart select - Remove unused parent_organization method
x4d3
force-pushed
the
MNOE-146-display-teams-admin-panel
branch
from
ae9675e to
baec6de
Compare
| @@ -9,6 +9,7 @@ module MnoEnterprise::Concerns::Models::AppInstance | |||
| included do | |||
| property :created_at, type: :time | |||
| property :updated_at, type: :time | |||
| property :app_id, type: :string | |||
There was a problem hiding this comment.
I thought we were getting rid of this in #560
No description provided.