From 6a09fe5fb66540894e8af437ab3be7b497b084fc Mon Sep 17 00:00:00 2001 From: x4d3 Date: Wed, 8 Nov 2017 16:53:53 +0000 Subject: [PATCH] Refactor extracting stub_orga_relation --- .../jpi/v1/app_reviews_controller.rb | 4 - .../jpi/v1/marketplace_controller.rb | 2 +- .../jpi/v1/organizations_controller.rb | 2 +- .../controllers/jpi/v1/teams_controller.rb | 23 +++-- api/lib/mno_enterprise/csv_importer.rb | 6 +- .../v1/admin/organizations_controller_spec.rb | 7 +- .../jpi/v1/app_answers_controller_spec.rb | 2 +- .../jpi/v1/app_comments_controller_spec.rb | 2 +- .../jpi/v1/app_feedbacks_controller_spec.rb | 2 +- .../jpi/v1/app_instances_controller_spec.rb | 2 +- .../v1/app_instances_sync_controller_spec.rb | 2 +- .../jpi/v1/app_questions_controller_spec.rb | 2 +- .../jpi/v1/app_reviews_controller_spec.rb | 2 +- .../jpi/v1/audit_events_controller_spec.rb | 2 +- .../jpi/v1/impac/widgets_controller_spec.rb | 2 +- .../jpi/v1/marketplace_controller_spec.rb | 2 +- .../jpi/v1/organizations_controller_spec.rb | 96 +++++++++++-------- .../jpi/v1/team_controller_spec.rb | 21 ++-- core/app/models/mno_enterprise/user.rb | 3 +- .../mno_enterprise/concerns/models/ability.rb | 32 ++++--- .../mno_enterprise_api_test_helper.rb | 5 + 21 files changed, 124 insertions(+), 97 deletions(-) diff --git a/api/app/controllers/mno_enterprise/jpi/v1/app_reviews_controller.rb b/api/app/controllers/mno_enterprise/jpi/v1/app_reviews_controller.rb index 496d17880..94a085ac6 100644 --- a/api/app/controllers/mno_enterprise/jpi/v1/app_reviews_controller.rb +++ b/api/app/controllers/mno_enterprise/jpi/v1/app_reviews_controller.rb @@ -52,10 +52,6 @@ def current_app @app ||= MnoEnterprise::App.find_one(params[:id]) end - def orga_relation - @orga_relation ||= MnoEnterprise::OrgaRelation.where(organization_id: organization_id, user_id: current_user.id).first - end - def ensure_app_exists render_not_found('App') unless current_app.present? end diff --git a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/marketplace_controller.rb b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/marketplace_controller.rb index 8c8853343..c571a82d0 100644 --- a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/marketplace_controller.rb +++ b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/marketplace_controller.rb @@ -59,7 +59,7 @@ def app_relation(org_id = nil) rel end - # Return the organization_id passed as query parameters if the current_user has access to it + # Return the organization_id passed as query parameters if the current_user has access to it def app_instance_organization_id return params[:organization_id] if current_user && params[:organization_id].presence && orga_relation end diff --git a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/organizations_controller.rb b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/organizations_controller.rb index d4ae09cfc..5aea7fd57 100644 --- a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/organizations_controller.rb +++ b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/organizations_controller.rb @@ -18,7 +18,7 @@ module MnoEnterprise::Concerns::Controllers::Jpi::V1::OrganizationsController #================================================================== # GET /mnoe/jpi/v1/organizations def index - @organizations ||= current_user.organizations + @organizations ||= MnoEnterprise::Organization.where('users.id': current_user.id) end # GET /mnoe/jpi/v1/organizations/1 diff --git a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/teams_controller.rb b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/teams_controller.rb index 309595570..eade6958a 100644 --- a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/teams_controller.rb +++ b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/teams_controller.rb @@ -17,26 +17,31 @@ module MnoEnterprise::Concerns::Controllers::Jpi::V1::TeamsController def index authorize! :read, orga_relation @teams = MnoEnterprise::Team.includes(:organization, :app_instances, :users).where('organization.id': parent_organization_id) + load_parent_organization(parent_organization_id) end # GET /mnoe/jpi/v1/teams/:id def show + authorize! :read, orga_relation @team = MnoEnterprise::Team.find_one(params[:id], :organization, :app_instances, :users) authorize! :read, current_user.orga_relation(@team.organization) + load_parent_organization(@team.organization.id) end # POST /mnoe/jpi/v1/organizations/:organization_id/teams def create authorize! :manage_teams, orga_relation - @team = MnoEnterprise::Team.create(create_params) - MnoEnterprise::EventLogger.info('team_add', current_user.id, 'Team created', @team) if @team + @team = MnoEnterprise::Team.create!(create_params) + MnoEnterprise::EventLogger.info('team_add', current_user.id, 'Team created', @team) + load_parent_organization(parent_organization_id) render 'show' end # PUT /mnoe/jpi/v1/teams/:id def update @team = MnoEnterprise::Team.find_one!(params[:id], :organization) - authorize! :manage_teams, current_user.orga_relation(@team.organization) + organization = @team.organization + authorize! :manage_teams, current_user.orga_relation(organization) @team.update_attributes!(update_params) # # Update permissions if params[:team] && params[:team][:app_instances] @@ -44,6 +49,7 @@ def update MnoEnterprise::EventLogger.info('team_apps_update', current_user.id, 'Team apps updated', @team, {apps: list.map{|l| l['name']}}) end + load_parent_organization(organization.id) render 'show' end @@ -68,14 +74,19 @@ def destroy private + def load_parent_organization(id) + @parent_organization = MnoEnterprise::Organization.find_one(id, :orga_relations) + end + # Update the members of a team # Reduce duplication between add and remove def update_members(action) @team = MnoEnterprise::Team.find_one!(params[:id], :organization) - authorize! :manage_teams, current_user.orga_relation(@team.organization) - + organization = @team.organization + authorize! :manage_teams, current_user.orga_relation(organization) if params[:team] && params[:team][:users] id_list = params[:team][:users].map { |h| h[:id].to_i }.compact + # TODO: use a Custom method to update user_ids user_ids = case action when :add_user @team.user_ids | id_list @@ -87,7 +98,7 @@ def update_members(action) {action: action.to_s, user_ids: user_ids}) end @team = @team.load_required(:organization, :users, :app_instances) - @parent_organization = MnoEnterprise::Organization.find_one(@team.organization.id, :orga_relations) + load_parent_organization(organization.id) render 'show' end diff --git a/api/lib/mno_enterprise/csv_importer.rb b/api/lib/mno_enterprise/csv_importer.rb index 056c190ef..1a41166cd 100644 --- a/api/lib/mno_enterprise/csv_importer.rb +++ b/api/lib/mno_enterprise/csv_importer.rb @@ -46,7 +46,7 @@ def self.process(file_path) if event_type == :added address = MnoEnterprise::Address.new( - city: row['city'], + city: row['city'], country_code: row['country'], street: [row['address1'], row['address2']].reject(&:blank?).join(' '), state_code: row['state_province'], @@ -72,10 +72,10 @@ def self.process(file_path) user.phone = row['phone'] user.save - orga_relation = MnoEnterprise::OrgaRelation.where(user_id: user.id, organization_id: organization.id).first + orga_relation = MnoEnterprise::OrgaRelation.where('user.id': user.id, 'organization.id': organization.id).first # Add User as Super Admin to Organization if he is not already in it unless orga_relation - MnoEnterprise::OrgaRelation.create(user_id: user.id, organization_id: organization.id, role: 'Super Admin') + MnoEnterprise::OrgaRelation.create(user: user, organization: organization, role: 'Super Admin') end end report diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/admin/organizations_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/admin/organizations_controller_spec.rb index 2870426db..57ca1d15c 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/admin/organizations_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/admin/organizations_controller_spec.rb @@ -213,10 +213,9 @@ module MnoEnterprise stub_api_v2(:post, '/users', user2), stub_api_v2(:patch, "/users/#{user1.id}", user1), - stub_api_v2(:get, '/orga_relations', [], [], { filter: { user_id: user1.id, organization_id: organization1.id }, page: { number: 1, size: 1 } }), - stub_api_v2(:get, '/orga_relations', [orga_relation1], [], { filter: { user_id: user2.id, organization_id: organization2.id }, page: { number: 1, size: 1 } }), - - stub_api_v2(:post, '/orga_relations', orga_relation2) + stub_orga_relation(user1, organization1, nil), + stub_orga_relation(user2, organization2, orga_relation2), + stub_api_v2(:post, '/orga_relations', orga_relation1) ] } diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/app_answers_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/app_answers_controller_spec.rb index da0622103..43612852d 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/app_answers_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/app_answers_controller_spec.rb @@ -51,7 +51,7 @@ module MnoEnterprise let(:params) { {organization_id: organization.id, description: 'A Review', foo: 'bar', question_id: 'qid'} } before do - stub_api_v2(:get, '/orga_relations', [orga_relation], [], {filter: {organization_id: organization.id, user_id: user.id}, page: {number: 1, size: 1}}) + stub_orga_relation(user, organization, orga_relation) stub_api_v2(:post, '/answers', answer1) end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/app_comments_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/app_comments_controller_spec.rb index 80774a7dd..de5b77ab5 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/app_comments_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/app_comments_controller_spec.rb @@ -50,7 +50,7 @@ module MnoEnterprise let(:params) { {organization_id: organization.id, description: 'A Review', foo: 'bar', feedback_id: 'fid'} } before do - stub_api_v2(:get, '/orga_relations', [orga_relation], [], {filter: {organization_id: organization.id, user_id: user.id}, page: {number: 1, size: 1}}) + stub_orga_relation(user, organization, orga_relation) stub_api_v2(:post, '/comments', comment1) end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/app_feedbacks_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/app_feedbacks_controller_spec.rb index 36138c700..1d1087861 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/app_feedbacks_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/app_feedbacks_controller_spec.rb @@ -56,7 +56,7 @@ module MnoEnterprise let(:params) { {organization_id: organization.id, description: 'A Review', rating: 5} } let(:feedback) { build(:feedback, id: feedback_id, comments: []) } before do - stub_api_v2(:get, '/orga_relations', [orga_relation], [], {filter: {organization_id: organization.id, user_id: user.id}, page: {number: 1, size: 1}}) + stub_orga_relation(user, organization, orga_relation) stub_api_v2(:post, '/feedbacks', feedback) end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/app_instances_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/app_instances_controller_spec.rb index 7bccfede7..7cc1a8001 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/app_instances_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/app_instances_controller_spec.rb @@ -18,7 +18,7 @@ module MnoEnterprise let(:orga_relation) { build(:orga_relation) } let!(:current_user_stub) { stub_user(user) } - before { stub_api_v2(:get, '/orga_relations', [orga_relation], [], { filter: { 'user.id': user.id, 'organization.id': organization.id }, page: { number: 1, size: 1 } }) } + before { stub_orga_relation(user, organization, orga_relation) } describe 'GET #index' do let(:app_instance) { build(:app_instance, status: 'running', under_free_trial: false) } diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/app_instances_sync_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/app_instances_sync_controller_spec.rb index 9399f1b22..1c6cb624c 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/app_instances_sync_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/app_instances_sync_controller_spec.rb @@ -27,7 +27,7 @@ module MnoEnterprise let(:orga_relation) { build(:orga_relation) } let!(:organization) { build(:organization) } before do - stub_api_v2(:get, '/orga_relations', [orga_relation], [], { filter: { 'user.id': user.id, 'organization.uid': organization.uid }, page: { number: 1, size: 1 } }) + stub_orga_relation(user, organization, orga_relation) stub_api_v2(:get, '/organizations', [organization], [], { filter: { uid: organization.uid } }) end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/app_questions_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/app_questions_controller_spec.rb index 31db2d5ab..7732610d3 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/app_questions_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/app_questions_controller_spec.rb @@ -56,7 +56,7 @@ module MnoEnterprise let(:params) { {organization_id: organization.id, description: 'A Review', foo: 'bar'} } let(:question) { build(:question, id: question_id, answers: []) } before do - stub_api_v2(:get, '/orga_relations', [orga_relation], [], {filter: {organization_id: organization.id, user_id: user.id}, page: {number: 1, size: 1}}) + stub_orga_relation(user, organization, orga_relation) stub_api_v2(:post, '/questions', question) end subject { post :create, id: app.id, app_question: params } diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/app_reviews_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/app_reviews_controller_spec.rb index 829249fb3..35230109c 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/app_reviews_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/app_reviews_controller_spec.rb @@ -52,7 +52,7 @@ module MnoEnterprise describe 'POST #create', focus: true do let(:params) { {organization_id: organization.id, description: 'A Review', rating: 5} } before do - stub_api_v2(:get, '/orga_relations', [orga_relation], [], {filter: {organization_id: organization.id, user_id: user.id}, page: {number: 1, size: 1}}) + stub_orga_relation(user, organization, orga_relation) stub_api_v2(:post, '/reviews', review) end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/audit_events_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/audit_events_controller_spec.rb index 86aaf9b43..7ceec0ca6 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/audit_events_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/audit_events_controller_spec.rb @@ -24,7 +24,7 @@ module MnoEnterprise let!(:current_user_stub) { stub_user(user) } before { stub_api_v2(:get, "/organizations/#{organization.id}", organization) } before do - stub_api_v2(:get, '/orga_relations', [orga_relation], [], { filter: { 'user.id': user.id, 'organization.id': organization.id }, page: { number: 1, size: 1 } }) + stub_orga_relation(user, organization, orga_relation) stub_api_v2(:get, '/audit_events', [audit_event], [], { filter: { organization_id: organization.id } }) sign_in user end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/impac/widgets_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/impac/widgets_controller_spec.rb index e2cf74537..8eb5e7fab 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/impac/widgets_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/impac/widgets_controller_spec.rb @@ -18,7 +18,7 @@ module MnoEnterprise let!(:current_user_stub) { stub_user(user) } before { sign_in user } - before { stub_api_v2(:get, '/orga_relations', [orga_relation], [], { filter: { 'user.id': user.id, 'organization.uid': organization.uid }, page: { number: 1, size: 1 } }) } + before { stub_orga_relation(user, organization, orga_relation, 'uid') } describe 'GET index' do diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/marketplace_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/marketplace_controller_spec.rb index ca40820cf..4e78a5588 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/marketplace_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/marketplace_controller_spec.rb @@ -133,7 +133,7 @@ def hash_for_apps(apps) before do sign_in(user) stub_api_v2(:get, "/organizations", [organization], [], { fields: { organizations: 'id' }, filter: { id: organization.id, 'users.id' => user.id }, page: one_page }) - stub_api_v2(:get, '/orga_relations', [orga_relation], [], { filter: { 'user.id': user.id, 'organization.id': organization.id }, page: one_page }) + stub_orga_relation(user, organization, orga_relation) stub_api_v2(:get, '/apps', [app], DEPENDENCIES, { _metadata: { organization_id: organization.id }, filter: { active: true } }) stub_api_v2(:get, '/apps', [app], [], { diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/organizations_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/organizations_controller_spec.rb index 5e45169fd..e75591f68 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/organizations_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/organizations_controller_spec.rb @@ -21,13 +21,7 @@ module MnoEnterprise # Stub organization + associations let(:metadata) { {} } let!(:organization) { build(:organization, metadata: metadata, orga_invites: [], users: [], orga_relations: [], credit_card: credit_card, invoices: []) } - let(:role) { 'Admin' } - let!(:user) { - u = build(:user, organizations: [organization], orga_relations: [orga_relation], dashboards: []) - orga_relation.user_id = u.id - u - } - let!(:orga_relation) { build(:orga_relation, organization_id: organization.id, role: role) } + let!(:user) { build(:user) } let!(:organization_stub) { stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations credit_card invoices)) } # Stub user and user call @@ -45,7 +39,7 @@ module MnoEnterprise #=============================================== shared_examples 'an organization management action' do context 'when Organization management is disabled' do - before { Settings.merge!(dashboard: {organization_management: {enabled: false}}) } + before { Settings.merge!(dashboard: { organization_management: { enabled: false } }) } after { Settings.reload! } it { is_expected.to have_http_status(:forbidden) } @@ -55,7 +49,10 @@ module MnoEnterprise describe 'GET #index' do subject { get :index } it_behaves_like 'jpi v1 protected action' - + before do + stub_api_v2(:get, '/organizations', [organization], [], { filter: { 'users.id': user.id } }) + stub_orga_relation(user, organization, build(:orga_relation)) + end context 'success' do before { subject } @@ -71,8 +68,10 @@ module MnoEnterprise it_behaves_like 'jpi v1 protected action' + before { stub_orga_relation(user, organization, build(:orga_relation)) } + context 'success' do - before { subject} + before { subject } it 'returns a complete description of the organization' do expect(response).to be_success @@ -82,19 +81,20 @@ module MnoEnterprise context 'contains invoices' do subject { get :show, id: organization.id } - + let(:money) { Money.new(0, 'AUD') } let(:role) { 'Super Admin' } let(:member_role) { role } let(:member) { build(:user, id: user.id, email: user.email) } - - before { + + before do allow_any_instance_of(MnoEnterprise::Organization).to receive(:current_billing).and_return(money) allow_any_instance_of(MnoEnterprise::Organization).to receive(:current_credit).and_return(money) organization.invoices << invoice - stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations credit_card invoices)) - } - + stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations credit_card invoices)) + end + + it 'renders the list of invoices' do subject expect(JSON.parse(response.body)['invoices']).to eq(partial_hash_for_invoices(organization)) @@ -103,12 +103,14 @@ module MnoEnterprise end describe 'POST #create' do - let(:params) { {'name' => organization.name} } + let(:params) { { 'name' => organization.name } } + let(:orga_relation) { build(:orga_relation) } subject { post :create, organization: params } - before { stub_api_v2(:post, '/organizations', organization) } - before { stub_api_v2(:post, '/orga_relations', orga_relation) } - # reloading organization - before { stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations)) } + before do + stub_api_v2(:post, '/organizations', organization) + stub_api_v2(:post, '/orga_relations', orga_relation) + stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations)) + end it_behaves_like 'jpi v1 protected action' it_behaves_like 'an organization management action' @@ -128,8 +130,12 @@ module MnoEnterprise describe 'PUT #update' do - let(:params) { {'name' => organization.name + 'a', 'soa_enabled' => !organization.soa_enabled} } - before { stub_api_v2(:patch, "/organizations/#{organization.id}", updated_organization) } + let(:params) { { 'name' => organization.name + 'a', 'soa_enabled' => !organization.soa_enabled } } + before do + stub_api_v2(:patch, "/organizations/#{organization.id}", updated_organization) + stub_orga_relation(user, organization, build(:orga_relation)) + end + let!(:updated_organization) { build(:organization, name: params['name'], id: organization.id, soa_enabled: params['soa_enabled']) } subject { put :update, id: organization.id, organization: params } @@ -196,7 +202,7 @@ module MnoEnterprise end describe 'when payment restrictions are set' do - let(:metadata) { {payment_restriction: [:visa]} } + let(:metadata) { { payment_restriction: [:visa] } } let(:visa) { '4111111111111111' } let(:mastercard) { '5105105105105100' } @@ -227,7 +233,7 @@ module MnoEnterprise it 'returns an error' do subject expect(response).to have_http_status(:bad_request) - expect(JSON.parse(response.body)).to eq({'number' => ['Payment is limited to Visa Card Holders']}) + expect(JSON.parse(response.body)).to eq({ 'number' => ['Payment is limited to Visa Card Holders'] }) end end end @@ -243,7 +249,7 @@ module MnoEnterprise before { stub_api_v2(:patch, "/orga_invites/#{organization.id}", orga_invite) } let(:team) { build(:team, organization: organization) } - let(:params) { [{email: 'newmember@maestrano.com', role: 'Power User', team_id: team.id}] } + let(:params) { [{ email: 'newmember@maestrano.com', role: 'Power User', team_id: team.id }] } subject { put :invite_members, id: organization.id, invites: params } it_behaves_like 'jpi v1 authorizable action' @@ -270,23 +276,26 @@ module MnoEnterprise it 'returns a partial representation of the entity' do subject - expect(JSON.parse(response.body)).to eq({'members' => partial_hash_for_members(organization)}) + expect(JSON.parse(response.body)).to eq({ 'members' => partial_hash_for_members(organization) }) end end end describe 'PUT #update_member' do - + let(:role) { 'Admin' } + let!(:orga_relation) { build(:orga_relation, organization_id: organization.id, role: role) } let(:email) { 'somemember@maestrano.com' } let(:member_role) { 'Member' } let(:member) { build(:user) } let(:email) { member.email } - let(:new_member_role) { 'Power User' } - let(:params) { {email: email, role: new_member_role} } + let(:params) { { email: email, role: new_member_role } } - # reloading organization - before { stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations)) } + before do + stub_orga_relation(user, organization, build(:orga_relation)) + # reloading organization + stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations)) + end subject { put :update_member, id: organization.id, member: params } it_behaves_like 'jpi v1 authorizable action' @@ -294,14 +303,17 @@ module MnoEnterprise context 'with user' do let(:member_orga_relation) { build(:orga_relation, user_id: member.id, organization_id: organization.id, role: member_role) } - let(:organization_stub) { + let(:organization_stub) do organization.users << member organization.orga_relations << member_orga_relation stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations credit_card invoices)) - } - before { stub_api_v2(:get, "/orga_relations", [member_orga_relation], [], {filter: {organization_id: organization.id, user_id: member.id}, page:{ number: 1, size: 1}}) } - before { stub_api_v2(:post, "/orga_relations/#{member_orga_relation.id}", orga_relation) } - before { stub_api_v2(:patch, "/orga_relations/#{member_orga_relation.id}") } + end + before do + stub_orga_relation(user, organization, orga_relation) + stub_api_v2(:get, "/orga_relations", [member_orga_relation], [], { filter: { organization_id: organization.id, user_id: member.id }, page: one_page }) + stub_api_v2(:post, "/orga_relations/#{member_orga_relation.id}", orga_relation) + stub_api_v2(:patch, "/orga_relations/#{member_orga_relation.id}") + end # Happy path it 'updates the member role' do @@ -381,7 +393,7 @@ module MnoEnterprise it 'renders a the user list' do subject - expect(JSON.parse(response.body)).to eq({'members' => partial_hash_for_members(organization)}) + expect(JSON.parse(response.body)).to eq({ 'members' => partial_hash_for_members(organization) }) end end @@ -399,7 +411,7 @@ module MnoEnterprise before { stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations)) } - let(:params) { {email: 'somemember@maestrano.com'} } + let(:params) { { email: 'somemember@maestrano.com' } } subject { put :remove_member, id: organization.id, member: params } it_behaves_like 'jpi v1 authorizable action' @@ -407,7 +419,7 @@ module MnoEnterprise context 'with user' do before { stub_api_v2(:delete, "/orga_relations/#{member_orga_relation.id}") } - let(:params) { {email: member.email} } + let(:params) { { email: member.email } } it 'removes the member' do subject end @@ -420,7 +432,7 @@ module MnoEnterprise stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(users orga_invites orga_relations credit_card invoices)) } - before { stub_api_v2(:get, "/orga_invites/#{orga_invite.id}/decline")} + before { stub_api_v2(:get, "/orga_invites/#{orga_invite.id}/decline") } it 'removes the member' do subject end @@ -428,7 +440,7 @@ module MnoEnterprise it 'renders a the user list' do subject - expect(JSON.parse(response.body)).to eq({'members' => partial_hash_for_members(organization)}) + expect(JSON.parse(response.body)).to eq({ 'members' => partial_hash_for_members(organization) }) end end end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/team_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/team_controller_spec.rb index 079280a0c..3561a36d3 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/team_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/team_controller_spec.rb @@ -58,18 +58,21 @@ def hash_for_team(team) # Specs #=============================================== describe 'PUT #add_users' do + subject { put :add_users, id: team.id, team: {users: [{id: user.id}]} } + before do + stub_user(user) + stub_orga_relation + stub_api_v2(:get, "/apps", [app]) + stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(orga_relations)) + stub_api_v2(:get, "/teams/#{team.id}", team, %i(organization)) + stub_api_v2(:patch, "/teams/#{team.id}") + # team reload + stub_api_v2(:get, "/teams/#{team.id}", team, %i(organization users app_instances)) + stub_audit_events + end - let!(:current_user_stub) { stub_user(user) } - before { stub_api_v2(:get, "/apps", [app]) } - before { stub_api_v2(:get, "/organizations/#{organization.id}", organization, %i(orga_relations)) } - before { stub_api_v2(:get, "/teams/#{team.id}", team, %i(organization)) } - before { stub_api_v2(:patch, "/teams/#{team.id}") } - before { stub_audit_events } - subject { put :add_users, id: team.id, team: {users: [{id: user.id}]} } - # team reload - before { stub_api_v2(:get, "/teams/#{team.id}", team, %i(organization users app_instances)) } context 'success' do before { subject } diff --git a/core/app/models/mno_enterprise/user.rb b/core/app/models/mno_enterprise/user.rb index 797b1ebf3..4a0f8e2a3 100644 --- a/core/app/models/mno_enterprise/user.rb +++ b/core/app/models/mno_enterprise/user.rb @@ -101,8 +101,7 @@ def expire_user_cache def refresh_user_cache self.expire_view_cache - reloaded = self.load_required_dependencies - Rails.cache.write(['user', reloaded.to_key], reloaded) + Rails.cache.write(['user', self.to_key], self) end def load_required_dependencies diff --git a/core/lib/mno_enterprise/concerns/models/ability.rb b/core/lib/mno_enterprise/concerns/models/ability.rb index 9b30cee46..4479f963a 100644 --- a/core/lib/mno_enterprise/concerns/models/ability.rb +++ b/core/lib/mno_enterprise/concerns/models/ability.rb @@ -25,14 +25,14 @@ def initialize(user) #=================================================== # Organization #=================================================== - can :create, MnoEnterprise::Organization + can :create, MnoEnterprise::OrgaRelation - can :read, MnoEnterprise::Organization do |organization| - !!user.role(organization) + can :read, MnoEnterprise::OrgaRelation do |orga_relation| + !!orga_relation end - can [:update, :destroy, :manage_billing], MnoEnterprise::Organization do |organization| - user.role(organization) == 'Super Admin' + can [:update, :destroy, :manage_billing], MnoEnterprise::OrgaRelation do |orga_relation| + orga_relation&.role == 'Super Admin' end @@ -62,12 +62,14 @@ def initialize(user) # AppInstance #=================================================== can :access, MnoEnterprise::AppInstance do |app_instance| - role = user.role_from_id(app_instance.owner_id) - !!role && ( - ['Super Admin', 'Admin'].include?(role) || - user.teams.empty? || - user.teams.map(&:app_instances).compact.flatten.map(&:id).include?(app_instance.id) - ) + orga_relation = MnoEnterprise::OrgaRelation.where('user.id': user.id, 'organization.id': app_instance.owner_id).first + role = orga_relation&.role + if role && ['Super Admin', 'Admin'].include?(role) + true + else + teams = MnoEnterprise::Team.where('users.id': user.id).includes(:app_instances).with_params(fields: { app_instances: 'id' }) + teams.empty? || teams.map(&:app_instances).compact.flatten.map(&:id).include?(app_instance.id) + end end #=================================================== @@ -111,15 +113,15 @@ def initialize(user) def impac_abilities(user) can :manage_impac, MnoEnterprise::Dashboard do |dhb| dhb.organizations.any? && dhb.organizations.all? do |org| - !!user.role(org) && ['Super Admin', 'Admin'].include?(user.role(org)) + role = orga_relation(user.id, org.id)&.role + role && ['Super Admin', 'Admin'].include?(role) end end can :manage_dashboard, MnoEnterprise::Dashboard do |dashboard| - if dashboard.owner_type == "Organization" + if dashboard.owner_type == 'Organization' # The current user is a member of the organization that owns the dashboard that has the kpi attached to - owner = MnoEnterprise::Organization.find(dashboard.owner_id) - owner && !!user.role(owner) + !!orga_relation(user, dashboard.owner_id) elsif dashboard.owner_type == "User" # The current user is the owner of the dashboard that has the kpi attached to dashboard.owner_id == user.id diff --git a/core/lib/mno_enterprise/testing_support/mno_enterprise_api_test_helper.rb b/core/lib/mno_enterprise/testing_support/mno_enterprise_api_test_helper.rb index b6e8e18f6..c35ec2818 100644 --- a/core/lib/mno_enterprise/testing_support/mno_enterprise_api_test_helper.rb +++ b/core/lib/mno_enterprise/testing_support/mno_enterprise_api_test_helper.rb @@ -58,6 +58,11 @@ def stub_user(user, included = [], params = {}) stub_api_v2(:get, "/users/#{user.id}", user, included, params) end + def stub_orga_relation(user, organization, orga_relation, id_field = 'id') + filter = { 'user.id': user.id, "organization.#{id_field}": organization.public_send(id_field) } + stub_api_v2(:get, '/orga_relations', [orga_relation].compact, [], { filter: filter, page: one_page }) + end + def stub_deletion_requests(user, deletion_requests) filter = { 'created_at.gt': MnoEnterprise::DeletionRequest::EXPIRATION_TIME.minutes.ago,