diff --git a/api/app/views/mno_enterprise/jpi/v1/impac/alerts/_alert.json.jbuilder b/api/app/views/mno_enterprise/jpi/v1/impac/alerts/_alert.json.jbuilder index aebb775b2..594a801ba 100644 --- a/api/app/views/mno_enterprise/jpi/v1/impac/alerts/_alert.json.jbuilder +++ b/api/app/views/mno_enterprise/jpi/v1/impac/alerts/_alert.json.jbuilder @@ -1,7 +1,6 @@ json.ignore_nil! -json.extract! alert, :id, :title, :webhook, :service, :sent +json.extract! alert, :id, :title, :webhook, :service, :sent, :kpi_id json.metadata alert.settings -json.kpi_id alert.kpi_id json.recipients alert.recipients.map do |recipient| json.extract! recipient, :id, :email end diff --git a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/alerts_controller.rb b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/alerts_controller.rb index 26cfacbf8..74416692b 100644 --- a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/alerts_controller.rb +++ b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/alerts_controller.rb @@ -12,8 +12,7 @@ module MnoEnterprise::Concerns::Controllers::Jpi::V1::Impac::AlertsController # GET /jpi/v1/impac/alerts def index - u = current_user.load_required(:alerts) - @alerts = u.alerts + @alerts = MnoEnterprise::Alert.includes(:recipients).where('recipient.id' => current_user.id) end # POST /jpi/v1/impac/kpis/:kpi_id/alerts @@ -23,8 +22,9 @@ def create # TODO: Manage authorization #authorize! :manage_alert, kpi_alert - - @alert = MnoEnterprise::Alert.create!(alert_params.merge(recipient_ids: [current_user.id])) + @alert = MnoEnterprise::Alert.create!(alert_params) + @alert.update_recipients!([current_user.id]) + @alert = @alert.load_required(:recipients) render 'show' end diff --git a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/dashboards_controller.rb b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/dashboards_controller.rb index d7f6d361d..18a5880ac 100644 --- a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/dashboards_controller.rb +++ b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/dashboards_controller.rb @@ -10,7 +10,7 @@ module MnoEnterprise::Concerns::Controllers::Jpi::V1::Impac::DashboardsControlle respond_to :json end - DASHBOARD_DEPENDENCIES = [:widgets, :'widgets.kpis', :kpis, :'kpis.alerts'] + DASHBOARD_DEPENDENCIES = [:widgets, :'widgets.kpis', :kpis, :'kpis.alerts', :'kpis.alerts.recipients'] #================================================================== # Instance methods @@ -29,9 +29,6 @@ def show # POST /mnoe/jpi/v1/impac/dashboards # -> POST /api/mnoe/v1/users/1/dashboards def create - # TODO: dashboards.build breaks as dashboard.organization_ids returns nil, instead of an - # empty array. (see MnoEnterprise::Impac::Dashboard #organizations) - # @dashboard = dashboards.build(dashboard_create_params) # TODO: enable authorization # authorize! :manage_dashboard, @dashboard # if @dashboard.save diff --git a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/kpis_controller.rb b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/kpis_controller.rb index 149523fcc..fe2e7be32 100644 --- a/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/kpis_controller.rb +++ b/api/lib/mno_enterprise/concerns/controllers/jpi/v1/impac/kpis_controller.rb @@ -25,9 +25,9 @@ def index auth = { username: MnoEnterprise.tenant_id, password: MnoEnterprise.tenant_key } begin - response = MnoEnterprise::ImpacClient.send_get('/api/v2/kpis', attrs, basic_auth: auth) # TODO check there was no error, something like # return render json: { message: "Unable to retrieve kpis from Impac API | Error #{response.code}" } unless response.success? + response = MnoEnterprise::ImpacClient.send_get('/api/v2/kpis', attrs, basic_auth: auth) rescue => e return render json: { message: "Unable to retrieve kpis from Impac API | Error #{e}" } end @@ -39,11 +39,10 @@ def index kpi.merge( name: "#{kpi[:name]} #{watchable.capitalize unless kpi[:name].downcase.index(watchable)}".strip, watchables: [watchable], - target_placeholders: {watchable => kpi[:target_placeholders][watchable]}, + target_placeholders: { watchable => kpi[:target_placeholders][watchable] }, ) end - end - .flatten + end.flatten render json: { kpis: kpis } end @@ -59,16 +58,15 @@ def create return render_not_found('dashboard') if dashboard.blank? authorize! :manage_dashboard, dashboard end - # TODO: nest alert in as a param, with the current user as a recipient. @kpi = MnoEnterprise::Kpi.create!(kpi_create_params) # Creates a default alert for kpis created with targets defined. if kpi.targets.present? - MnoEnterprise::Alert.create({service: 'inapp', kpi_id: kpi.id, recipient_ids: [current_user.id]}) + MnoEnterprise::Alert.create_with_recipients!({ service: 'inapp', kpi_id: kpi.id }, [current_user.id]) # TODO: should widget KPIs create an email alert automatically? - MnoEnterprise::Alert.create({service: 'email', kpi_id: kpi.id, recipient_ids: [current_user.id]}) if widget.present? + MnoEnterprise::Alert.create_with_recipients!({ service: 'email', kpi_id: kpi.id }, [current_user.id]) if widget.present? # TODO: reload is adding the recipients to the kpi alerts (making another request). end - @kpi = kpi.load_required(:alerts) + @kpi = kpi.load_required(:alerts, :'alerts.recipients') render 'show' end @@ -85,17 +83,17 @@ def update # -- # Creates an in-app alert if target is set for the first time (in-app alerts should be activated by default) if kpi.targets.blank? && params[:targets].present? - MnoEnterprise::Alert.create!(service: 'inapp', kpi_id: kpi.id, recipient_ids: [current_user.id]) - # If targets have changed, reset all the alerts 'sent' status to false. + MnoEnterprise::Alert.create_with_recipients!({ service: 'inapp', kpi_id: kpi.id }, [current_user.id]) + # If targets have changed, reset all the alerts 'sent' status to false. elsif kpi.targets && params[:targets].present? && params[:targets] != kpi.targets - kpi.alerts.each { |alert| alert.update_attributes!(sent: false) } - # Removes all the alerts if the targets are removed (kpi has no targets set, - # and params contains no targets to be set) + kpi.alerts.each { |alert| alert.update_attributes(sent: false) } + # Removes all the alerts if the targets are removed (kpi has no targets set, + # and params contains no targets to be set) elsif params[:targets].blank? && kpi.targets.blank? kpi.alerts.each(&:destroy!) end kpi.update_attributes!(kpi_update_params) - @kpi = kpi.load_required(:dashboard, :alerts) + @kpi = kpi.load_required(:dashboard, :alerts, :'alerts.recipients') render 'show' end @@ -114,45 +112,44 @@ def destroy #================================================= private - def dashboard - @dashboard ||= MnoEnterprise::Dashboard.find_one(params.require(:dashboard_id)) - end + def dashboard + @dashboard ||= MnoEnterprise::Dashboard.find_one(params.require(:dashboard_id)) + end - def widget - widget_id = params.require(:kpi)[:widget_id] - @widget ||= (widget_id.present? && MnoEnterprise::Widget.find_one(widget_id.to_i)) - end + def widget + widget_id = params.require(:kpi)[:widget_id] + @widget ||= (widget_id.present? && MnoEnterprise::Widget.find_one(widget_id.to_i)) + end - def kpi - @kpi ||= MnoEnterprise::Kpi.find_one(params[:id], :dashboard, :widget, :alerts) - end + def kpi + @kpi ||= MnoEnterprise::Kpi.find_one(params[:id], :dashboard, :widget, :alerts, :'alerts.recipients') + end - def kpi_create_params - whitelist = [:widget_id, :endpoint, :source, :element_watched, {extra_watchables: []}] - create_params = extract_params(whitelist) - #either it is a widget kpi or a dashboard kpi - if create_params[:widget_id] - create_params - else - create_params.merge(dashboard_id: params[:dashboard_id]) - end + def kpi_create_params + whitelist = [:widget_id, :endpoint, :source, :element_watched, { extra_watchables: [] }] + create_params = extract_params(whitelist) + #either it is a widget kpi or a dashboard kpi + if create_params[:widget_id] + create_params + else + create_params.merge(dashboard_id: params[:dashboard_id]) end + end - def kpi_update_params - whitelist = [:name, :element_watched, {extra_watchables: []}] - extract_params(whitelist) - end + def kpi_update_params + whitelist = [:name, :element_watched, { extra_watchables: [] }] + extract_params(whitelist) + end - def extract_params(whitelist) - (p = params).require(:kpi).permit(*whitelist).tap do |whitelisted| - whitelisted[:settings] = p[:kpi][:metadata] || {} - # TODO: strong params for targets & extra_params attributes (keys will depend on the kpi). - whitelisted[:targets] = p[:kpi][:targets] unless p[:kpi][:targets].blank? - whitelisted[:extra_params] = p[:kpi][:extra_params] unless p[:kpi][:extra_params].blank? - end - .except(:metadata) - end + def extract_params(whitelist) + (p = params).require(:kpi).permit(*whitelist).tap do |whitelisted| + whitelisted[:settings] = p[:kpi][:metadata] || {} + # TODO: strong params for targets & extra_params attributes (keys will depend on the kpi). + whitelisted[:targets] = p[:kpi][:targets] unless p[:kpi][:targets].blank? + whitelisted[:extra_params] = p[:kpi][:extra_params] unless p[:kpi][:extra_params].blank? + end.except(:metadata) + end - alias :find_valid_kpi :kpi + alias :find_valid_kpi :kpi end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/impac/alerts_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/impac/alerts_controller_spec.rb index 1289e569b..56fc05e1f 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/impac/alerts_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/impac/alerts_controller_spec.rb @@ -12,40 +12,47 @@ module MnoEnterprise before { allow(ability).to receive(:can?).with(any_args).and_return(true) } # Stub user and user call - let!(:user) { build(:user, alerts: [alert]) } + let!(:user) { build(:user) } let!(:current_user_stub) { stub_user(user) } - before { sign_in user } let(:kpi) { build(:impac_kpi) } - let(:alert) { build(:impac_alert, kpi: kpi) } + let(:alert) { build(:impac_alert, kpi: kpi, recipients: [build(:user)]) } let(:alert_hash) { serialize_type(alert).except(:kpi) } describe 'GET #index' do - before { stub_api_v2(:get, "/users/#{user.id}", user, %i(alerts)) } + before { stub_api_v2(:get, "/alerts", alert, [:recipients], { filter: { 'recipient.id': user.id } }) } subject { get :index } - # TODO: Add and test authorization + it { is_expected.to be_success } end describe 'POST #create' do - before { stub_api_v2(:post, '/alerts', alert) } - before { stub_api_v2(:get, "/kpis/#{kpi.id}", kpi) } + before do + stub_api_v2(:post, '/alerts', alert) + stub_api_v2(:get, "/kpis/#{kpi.id}", kpi) + stub_api_v2(:patch, "/alerts/#{alert.id}/update_recipients", alert) + stub_api_v2(:get, "/alerts/#{alert.id}", alert, [:recipients]) + end + subject { post :create, kpi_id: kpi.id, alert: alert_hash } # TODO: Add authorization # it_behaves_like 'jpi v1 authorizable action' - it { subject; expect(response.code).to eq('200') } + it { is_expected.to be_success } end describe 'PUT #update' do - let(:update_alert_hash) { {title: 'test', webhook: 'test', sent: true, forbidden: 'test', } } - let(:updated_alert) { build(:impac_alert, kpi_id: kpi.id, title: 'test', webhook: 'test', sent: true) } + before do + stub_api_v2(:get, "/alerts/#{alert.id}", alert) + stub_api_v2(:patch, "/alerts/#{alert.id}", updated_alert) + stub_api_v2(:get, "/alerts/#{alert.id}", alert, [:recipients]) + end - before { stub_api_v2(:get, "/alerts/#{alert.id}", alert) } - before { stub_api_v2(:patch, "/alerts/#{alert.id}", updated_alert) } + let(:update_alert_hash) { { title: 'test', webhook: 'test', sent: true, forbidden: 'test', } } + let(:updated_alert) { build(:impac_alert, id: alert.id, kpi_id: kpi.id, title: 'test', webhook: 'test', sent: true) } subject { put :update, id: alert.id, alert: update_alert_hash } @@ -53,8 +60,7 @@ module MnoEnterprise # it_behaves_like "jpi v1 authorizable action" # TODO: Test that rendering is equal to update_alert_hash - - it { subject; expect(response.code).to eq('200') } + it { is_expected.to be_success } end describe 'DELETE #destroy' do @@ -65,8 +71,8 @@ module MnoEnterprise # TODO: Add and test authorization # it_behaves_like "jpi v1 authorizable action" - it { subject; expect(response.code).to eq('200') } - it { subject; expect(JSON.parse(response.body)).to eq({'deleted' => {'service' => alert.service}}) } + it { is_expected.to be_success } + it { subject; expect(JSON.parse(response.body)).to eq({ 'deleted' => { 'service' => alert.service } }) } end end end diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/impac/dashboards_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/impac/dashboards_controller_spec.rb index dad5ef8f1..f7b41e054 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/impac/dashboards_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/impac/dashboards_controller_spec.rb @@ -8,7 +8,7 @@ module MnoEnterprise before { request.env["HTTP_ACCEPT"] = 'application/json' } before { Rails.cache.clear } - let(:dashboard_dependencies) { [:widgets, :'widgets.kpis', :kpis, :'kpis.alerts'] } + let(:dashboard_dependencies) { [:widgets, :'widgets.kpis', :kpis, :'kpis.alerts', :'kpis.alerts.recipients'] } # Stub user and user call let(:org) { build(:organization, users: [], orga_relations: []) } @@ -104,7 +104,7 @@ def hash_for_kpi(kpi) describe 'GET #show' do before do - stub_api_v2(:get, "/dashboards/#{dashboard.id}", dashboard, [:widgets, :'widgets.kpis', :kpis, :'kpis.alerts'], { filter: { owner_id: user.id } }) + stub_api_v2(:get, "/dashboards/#{dashboard.id}", dashboard, dashboard_dependencies, {filter: {owner_id: user.id}}) end subject { get :show, id: dashboard.id } diff --git a/api/spec/controllers/mno_enterprise/jpi/v1/impac/kpis_controller_spec.rb b/api/spec/controllers/mno_enterprise/jpi/v1/impac/kpis_controller_spec.rb index 6e40a0e04..38436cc60 100644 --- a/api/spec/controllers/mno_enterprise/jpi/v1/impac/kpis_controller_spec.rb +++ b/api/spec/controllers/mno_enterprise/jpi/v1/impac/kpis_controller_spec.rb @@ -93,12 +93,16 @@ module MnoEnterprise context "a dashboard KPI" do subject { post :create, dashboard_id: dashboard.id, kpi: kpi_hash } let(:created_kpi) { build(:impac_kpi) } + let(:alert) { build(:impac_alert, kpi: created_kpi, recipients: [build(:user)]) } + before do stub_api_v2(:get, "/dashboards/#{dashboard.id}", dashboard) stub_api_v2(:post, "/kpis", created_kpi) - stub_api_v2(:post, "/alerts") + stub_api_v2(:post, "/alerts", alert) + stub_api_v2(:patch, "/alerts/#{alert.id}/update_recipients") + # kpi reload - stub_api_v2(:get, "/kpis/#{created_kpi.id}", [created_kpi], [:alerts]) + stub_api_v2(:get, "/kpis/#{created_kpi.id}", [created_kpi], [:alerts, :'alerts.recipients']) end it_behaves_like 'jpi v1 authorizable action' @@ -113,7 +117,7 @@ module MnoEnterprise let(:kpi_targets) { {evolution: [{max: "20"}]} } it "creates a kpi inapp alert" do - expect(MnoEnterprise::Alert).to receive(:create).once.with(hash_including(service: 'inapp')) + expect(MnoEnterprise::Alert).to receive(:create_with_recipients!).once.with(hash_including(service: 'inapp'), array_including(user.id)) subject expect(response).to have_http_status(:ok) end @@ -123,14 +127,17 @@ module MnoEnterprise context "a widget KPI" do let(:widget) { build(:impac_widget) } let(:created_kpi) { build(:impac_kpi) } + let(:alert) { build(:impac_alert, kpi: created_kpi, recipients: [build(:user)]) } + subject { post :create, dashboard_id: dashboard.id, kpi: kpi_hash.merge(widget_id: widget.id) } before do stub_api_v2(:get, "/widgets/#{widget.id}", widget) stub_api_v2(:post, "/kpis", created_kpi) - stub_api_v2(:post, "/alerts") + stub_api_v2(:post, "/alerts", alert) + stub_api_v2(:patch, "/alerts/#{alert.id}/update_recipients") # kpi reload - stub_api_v2(:get, "/kpis/#{created_kpi.id}", [created_kpi], [:alerts]) + stub_api_v2(:get, "/kpis/#{created_kpi.id}", [created_kpi], [:alerts, :'alerts.recipients']) end it_behaves_like 'jpi v1 authorizable action' @@ -147,8 +154,8 @@ module MnoEnterprise let(:kpi_targets) { { evolution: [{max: "20"}] } } it "creates kpi alerts" do - expect(MnoEnterprise::Alert).to receive(:create).once.with(hash_including(service: 'inapp')) - expect(MnoEnterprise::Alert).to receive(:create).once.with(hash_including(service: 'email')) + expect(MnoEnterprise::Alert).to receive(:create_with_recipients!).once.with(hash_including(service: 'inapp'), array_including(user.id)) + expect(MnoEnterprise::Alert).to receive(:create_with_recipients!).once.with(hash_including(service: 'email'), array_including(user.id)) subject expect(response).to have_http_status(:ok) @@ -168,9 +175,11 @@ module MnoEnterprise context 'target set for the first time' do let(:kpi_targets) { nil } let(:params) { {targets: {evolution: [{max: '20'}]}} } + let(:alert) { build(:impac_alert) } before do - stub_api_v2(:post, '/alerts') + stub_api_v2(:post, '/alerts', alert) + stub_api_v2(:patch, "/alerts/#{alert.id}/update_recipients", alert) end it 'creates an alert' do @@ -236,9 +245,9 @@ module MnoEnterprise before { kpi.alerts << alert updated_kpi.alerts << alert - stub_api_v2(:get, "/kpis/#{kpi.id}", kpi, %i(dashboard widget alerts)) + stub_api_v2(:get, "/kpis/#{kpi.id}", kpi, %i(dashboard widget alerts alerts.recipients)) # reload of the kpi - stub_api_v2(:get, "/kpis/#{kpi.id}", updated_kpi, %i(dashboard alerts)) + stub_api_v2(:get, "/kpis/#{kpi.id}", updated_kpi, %i(dashboard alerts alerts.recipients)) stub_api_v2(:patch, "/kpis/#{kpi.id}", updated_kpi) stub_api_v2(:patch, "/alerts/#{alert.id}", alert) } @@ -261,7 +270,7 @@ module MnoEnterprise subject { delete :destroy, id: kpi.id } before { - stub_api_v2(:get, "/kpis/#{kpi.id}", kpi, %i(dashboard widget alerts)) + stub_api_v2(:get, "/kpis/#{kpi.id}", kpi, %i(dashboard widget alerts alerts.recipients)) stub_api_v2(:delete, "/kpis/#{kpi.id}") } diff --git a/core/app/models/mno_enterprise/alert.rb b/core/app/models/mno_enterprise/alert.rb index ff640480b..2643a2127 100644 --- a/core/app/models/mno_enterprise/alert.rb +++ b/core/app/models/mno_enterprise/alert.rb @@ -2,5 +2,22 @@ module MnoEnterprise class Alert < BaseResource property :created_at, type: :time property :updated_at, type: :time + property :kpi_id + has_one :kpi + + custom_endpoint :update_recipients, on: :member, request_method: :patch + + def self.create_with_recipients!(attributes, recipient_ids) + alert = create(attributes) + alert.update_recipients!(recipient_ids) + alert + end + + def update_recipients!(ids) + input = {data: {attributes: {set: ids}}} + result = update_recipients(input) + process_custom_result(result) + end + end end diff --git a/core/lib/mno_enterprise/testing_support/factories/impac/alerts.rb b/core/lib/mno_enterprise/testing_support/factories/impac/alerts.rb index a9ba4fdfb..e0977fb60 100644 --- a/core/lib/mno_enterprise/testing_support/factories/impac/alerts.rb +++ b/core/lib/mno_enterprise/testing_support/factories/impac/alerts.rb @@ -7,9 +7,9 @@ kpi_id '1' service 'inapp' title 'Test Alert' - recipients [{id: 1, email: 'test@maestrano.com'}] webhook 'webhook' sent false + recipients [] settings {} end end 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 03c9f576f..7f9eede8d 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 @@ -148,14 +148,15 @@ def from_apiv2(entity, included) # Generate first and second level inclusions. E.g. includes='app_instances.app' # The second level entities get added to the included_entities hash - included_entities.values.each { |obj, inclusions| serialize_data(obj, inclusions, included_entities) } + # only taking the first level of inclusions + included_entities.values.each { |obj, inclusions| serialize_data(obj, inclusions[0,1], included_entities) } { data: data, meta: { record_count: entity_count(entity) }, - included: included_entities.values.map { |obj, inclusions| serialize_data(obj, inclusions, included_entities) } + included: included_entities.values.map { |obj, inclusions| serialize_data(obj, inclusions[0,1], included_entities) } } end end