Fixes #3635 - REST doc of Online Notification controler is outdated/wrong and expand param is missing

app/controllers/online_notifications_controller.rb

@@ -11,14 +11,13 @@ JSON
 
 Example:
 {
-  "id":1,
+  "id": 123,
-  "name":"some template",
+  "o_id": 628,
-  "user_id": null,
+  "object": "Ticket",
-  "options":{"a":1,"b":2},
+  "type": "escalation",
-  "updated_at":"2012-09-14T17:51:53Z",
+  "seen": true,
-  "created_at":"2012-09-14T17:51:53Z",
+  "updated_at": "2016-08-16T07:55:42.119Z",
-  "updated_by_id":2.
+  "created_at": "2016-08-16T07:55:42.119Z"
-  "created_by_id":2,
 }
 
 =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}
+GET /api/v1/online_notifications/#{id}
-
-Payload:
-{
-  "id": "123",
-}
 
 Response:
 {
-  "id": 1,
+  "id": 123,
-  "name": "some_name",
+  "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",
+  "id": 123,
-  "options":{"a":1,"b":2},
+  "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,
+  "id": 123,
-  "name": "some_name",
+  "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:

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

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