From fb91797934d4a9216e431f64c2b77b0dedad1b63 Mon Sep 17 00:00:00 2001 From: Bola Ahmed Buari Date: Fri, 2 Jul 2021 06:30:09 +0000 Subject: [PATCH] Fixes #3635 - REST doc of Online Notification controler is outdated/wrong and expand param is missing --- .../online_notifications_controller.rb | 65 +++-- app/models/online_notification.rb | 27 +- .../online_notifications_controller_spec.rb | 265 ++++++++++++++++++ 3 files changed, 329 insertions(+), 28 deletions(-) create mode 100644 spec/requests/online_notifications_controller_spec.rb diff --git a/app/controllers/online_notifications_controller.rb b/app/controllers/online_notifications_controller.rb index f0410d14f..9794772ae 100644 --- a/app/controllers/online_notifications_controller.rb +++ b/app/controllers/online_notifications_controller.rb @@ -11,14 +11,13 @@ JSON Example: { - "id":1, - "name":"some template", - "user_id": null, - "options":{"a":1,"b":2}, - "updated_at":"2012-09-14T17:51:53Z", - "created_at":"2012-09-14T17:51:53Z", - "updated_by_id":2. - "created_by_id":2, + "id": 123, + "o_id": 628, + "object": "Ticket", + "type": "escalation", + "seen": true, + "updated_at": "2016-08-16T07:55:42.119Z", + "created_at": "2016-08-16T07:55:42.119Z" } =end @@ -26,24 +25,28 @@ Example: =begin Resource: -GET /api/v1/templates.json +GET /api/v1/online_notifications Response: [ { "id": 1, - "name": "some_name1", + "object": "Ticket", + "type": "escalation", + "seen": true, ... }, { "id": 2, - "name": "some_name2", + "object": "Ticket", + "type": "escalation", + "seen": false, ... } ] Test: -curl http://localhost/api/v1/online_notifications.json -v -u #{login}:#{password} +curl http://localhost/api/v1/online_notifications -v -u #{login}:#{password} =end @@ -63,7 +66,7 @@ curl http://localhost/api/v1/online_notifications.json -v -u #{login}:#{password assets = {} item_ids = [] online_notifications.each do |item| - item_ids.push item.id + item_ids.push item['id'] assets = item.assets(assets) end render json: { @@ -83,18 +86,17 @@ curl http://localhost/api/v1/online_notifications.json -v -u #{login}:#{password =begin Resource: -GET /api/v1/online_notifications/{id} - -Payload: -{ - "id": "123", -} +GET /api/v1/online_notifications/#{id} Response: { - "id": 1, - "name": "some_name", - ... + "id": 123, + "o_id": 628, + "object": "Ticket", + "type": "escalation", + "seen": true, + "updated_at": "2016-08-16T07:55:42.119Z", + "created_at": "2016-08-16T07:55:42.119Z" } Test: @@ -113,15 +115,24 @@ PUT /api/v1/online_notifications/{id} Payload: { - "name": "some name", - "options":{"a":1,"b":2}, + "id": 123, + "o_id": 628, + "object": "Ticket", + "type": "escalation", + "seen": true, + "updated_at": "2016-08-16T07:55:42.119Z", + "created_at": "2016-08-16T07:55:42.119Z" } Response: { - "id": 1, - "name": "some_name", - ... + "id": 123, + "o_id": 628, + "object": "Ticket", + "type": "escalation", + "seen": true, + "updated_at": "2016-08-16T07:55:42.119Z", + "created_at": "2016-08-16T07:55:42.119Z" } Test: diff --git a/app/models/online_notification.rb b/app/models/online_notification.rb index 0154459a1..2ea27228c 100644 --- a/app/models/online_notification.rb +++ b/app/models/online_notification.rb @@ -100,11 +100,15 @@ return all online notifications of an user notifications = OnlineNotification.list(user, limit) +or with expand data + + notifications = OnlineNotification.list(user, limit, expand) + =end def self.list(user, limit) OnlineNotification.where(user_id: user.id) - .order(created_at: :desc) + .order('created_at DESC, id DESC') .limit(limit) end @@ -253,4 +257,25 @@ with dedicated times true end + # streamline structur + def attributes_with_association_names + attributes = super + map = { + 'object_lookup' => 'object', + 'object_lookup_id' => 'object_id', + 'type_lookup' => 'type', + 'type_lookup_id' => 'type_id', + } + map.each do |key, value| + next if !attributes.key?(key) + + attributes[value] = attributes[key] + attributes.delete(key) + end + + attributes['object'] = ObjectLookup.by_id(attributes['object_id']) if attributes['object_id'].present? + attributes['type'] = TypeLookup.by_id(attributes['type_id']) if attributes['type_id'].present? + + attributes + end end diff --git a/spec/requests/online_notifications_controller_spec.rb b/spec/requests/online_notifications_controller_spec.rb new file mode 100644 index 000000000..c93ede890 --- /dev/null +++ b/spec/requests/online_notifications_controller_spec.rb @@ -0,0 +1,265 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe OnlineNotification, type: :request do + + let(:admin) { create(:admin, groups: Group.all) } + let(:agent) { create(:agent, groups: Group.all) } + + let(:type_lookup_id) { TypeLookup.by_name('create') } + let(:object_lookup_id) { ObjectLookup.by_name('User') } + + describe 'request handling' do + + shared_examples 'for successful request' do + it 'has a good response' do + get path, params: {}, as: :json + + expect(response).to have_http_status(:ok) + end + end + + shared_examples 'for array response' do + it 'has an array response' do + get path, params: {}, as: :json + + expect(json_response).to be_a_kind_of(Array) + end + end + + shared_examples 'for hash response' do + it 'has hash response' do + get path, params: {}, as: :json + + expect(json_response).to be_a_kind_of(Hash) + end + end + + shared_examples 'for notification id' do + it 'has a notification id' do + get path, params: {}, as: :json + + expect(json_response[0]['id']).to be online_notification.id + end + end + + shared_examples 'for object attribute' do + it 'has a object attribute' do + get path, params: {}, as: :json + + expect(json_response[0]['object']).to be_a_kind_of(String) + end + end + + shared_examples 'for response with assests' do + it 'has an assests attribute' do + get path, params: {}, as: :json + + expect(json_response['assets']).to be_a_kind_of(Hash) + end + end + + shared_examples 'getting all associated online notifications' do + + before { online_notification && authenticated_as(user) } + + context 'when online notifications is requested' do + let(:path) { '/api/v1/online_notifications' } + + it 'has an object lookup id' do + get path, params: {}, as: :json + + expect(json_response[0]['object_lookup_id']).to be_a_kind_of(Integer) + end + + it 'has a type lookup id' do + get path, params: {}, as: :json + + expect(json_response[0]['type_lookup_id']).to be_a_kind_of(Integer) + end + + include_examples 'for successful request' + include_examples 'for array response' + include_examples 'for notification id' + end + + context 'when online notifications is requested with full params' do + let(:path) { '/api/v1/online_notifications?full=true' } + + it 'has a record_ids attribute' do + get path, params: {}, as: :json + + expect(json_response['record_ids']) + .to be_a_kind_of(Array) + .and include(online_notification.id) + end + + include_examples 'for successful request' + include_examples 'for hash response' + include_examples 'for response with assests' + end + + context 'when online notifications is requested with expand params' do + let(:path) { '/api/v1/online_notifications?expand=true' } + + it 'has a type attribute' do + get path, params: {}, as: :json + + expect(json_response[0]['type']).to be_a_kind_of(String) + end + + include_examples 'for successful request' + include_examples 'for array response' + include_examples 'for object attribute' + include_examples 'for notification id' + + end + end + + shared_examples 'getting specific associated online notification' do + before { authenticated_as(user) } + + context 'when specific online notifications is requested' do + let(:path) { "/api/v1/online_notifications/#{online_notification.id}" } + + it 'has a notification id' do + get path, params: {}, as: :json + + expect(json_response['id']).to be online_notification.id + end + + it 'has a type lookup id' do + get path, params: {}, as: :json + + expect(json_response['type_lookup_id']).to be_a_kind_of(Integer) + end + + it 'has an object lookup id' do + get path, params: {}, as: :json + + expect(json_response['object_lookup_id']).to be_a_kind_of(Integer) + end + + include_examples 'for successful request' + include_examples 'for hash response' + end + + context 'when specific online notifications is requested with full params' do + let(:path) { "/api/v1/online_notifications/#{online_notification.id}?full=true" } + + it 'has a notification id' do + get path, params: {}, as: :json + + expect(json_response['id']).to be online_notification.id + end + + include_examples 'for successful request' + include_examples 'for hash response' + include_examples 'for response with assests' + end + + context 'when specific online notifications is requested with expand params' do + let(:path) { "/api/v1/online_notifications/#{online_notification.id}?expand=true" } + + it 'has a type attribute' do + get path, params: {}, as: :json + + expect(json_response['type']).to be_a_kind_of(String) + end + + it 'has a object attribute' do + get path, params: {}, as: :json + + expect(json_response['object']).to be_a_kind_of(String) + end + + it 'has a notification id' do + get path, params: {}, as: :json + + expect(json_response['id']).to be online_notification.id + end + + include_examples 'for successful request' + include_examples 'for hash response' + end + + end + + shared_examples 'getting a different online notification' do + before { authenticated_as(user) } + + context 'when a different notification is request' do + let(:path) { "/api/v1/online_notifications/#{different_online_notification.id}" } + + it 'has a forbidden response' do + get path, params: {}, as: :json + + expect(response).to have_http_status(:forbidden) + end + + it 'has authorized error message' do + get path, params: {}, as: :json + + expect(json_response['error']).to eq('Not authorized') + end + + include_examples 'for hash response' + end + end + + context 'with authenticated admin' do + let(:user) { create(:admin, groups: Group.all) } + + let(:online_notification) do + create(:online_notification, o_id: admin.id, user_id: user.id, type_lookup_id: type_lookup_id, object_lookup_id: object_lookup_id) + end + + let(:different_online_notification) do + create(:online_notification, o_id: admin.id, user_id: agent.id, type_lookup_id: type_lookup_id, object_lookup_id: object_lookup_id) + end + + it_behaves_like 'getting all associated online notifications' + + it_behaves_like 'getting specific associated online notification' + + it_behaves_like 'getting a different online notification' + end + + context 'with authenticated agent' do + let(:user) { create(:agent, groups: Group.all) } + + let(:online_notification) do + create(:online_notification, o_id: admin.id, user_id: user.id, type_lookup_id: type_lookup_id, object_lookup_id: object_lookup_id) + end + + let(:different_online_notification) do + create(:online_notification, o_id: admin.id, user_id: admin.id, type_lookup_id: type_lookup_id, object_lookup_id: object_lookup_id) + end + + it_behaves_like 'getting all associated online notifications' + + it_behaves_like 'getting specific associated online notification' + + it_behaves_like 'getting a different online notification' + end + + context 'with authenticated customer' do + let(:user) { create(:customer) } + + let(:online_notification) do + create(:online_notification, o_id: user.id, user_id: user.id, type_lookup_id: type_lookup_id, object_lookup_id: object_lookup_id) + end + + let(:different_online_notification) do + create(:online_notification, o_id: admin.id, user_id: agent.id, type_lookup_id: type_lookup_id, object_lookup_id: object_lookup_id) + end + + it_behaves_like 'getting all associated online notifications' + + it_behaves_like 'getting specific associated online notification' + + it_behaves_like 'getting a different online notification' + end + end +end