use paginated collection for activities#index

Oftentimes, the journal is checked for visiblility meaning whether a user has permissions in a project.

app/models/journal/aggregated_journal.rb

@@ -120,7 +120,7 @@ class Journal::AggregatedJournal
                                 .group_by(&:journal_id)
                               end
 
-      attachable_journals = if includes.include?(:customizable_journals)
+      attachable_journals = if includes.include?(:attachable_journals)
                               Journal::AttachableJournal
                               .where(journal_id: journal_ids)
                               .all

app/models/queries/journals/journal_query.rb

@@ -0,0 +1,37 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+class Queries::Journals::JournalQuery < Queries::BaseQuery
+  def self.model
+    Journal
+  end
+
+  def default_scope
+    super.where(journable_type: WorkPackage.name)
+  end
+end

db/migrate/20210614125212_add_wp_journals_project_id_index.rb

@@ -0,0 +1,5 @@
+class AddWpJournalsProjectIdIndex < ActiveRecord::Migration[6.1]
+  def change
+    add_index :work_package_journals, :project_id
+  end
+end

docs/api/apiv3/endpoints/activities.apib

@@ -1,5 +1,22 @@
 # Group Activities
 
+Activities are changes by users made on some resource in the OpenProject instance. This includes but is not limited to:
+
+* Updating a work package
+* Inviting a user to become a member in the project
+* Creating a wiki page.
+
+Because it is the byproduct of a users action on a resource, an activity cannot be created explicitly.
+
+However, not every activity is necessarily a change on a resource. Sometimes, a user simply comments on a resource. This is also tracked as an activity.
+
+## Linked Properties
+|  Link                              | Description                                                         | Type                        | Constraints       | Supported operations |
+|:-------------------:               |-----------------------------------------                            | -------------               | ----------------- | -------------------- |
+| self                               | This activity                                                       | Activity                    | not null          | READ                 |
+| user                               | The user who made the change                                        | User                        | not null          | READ                 |
+| workPackage (to be generalized )   | The resource (e.g. work package) on which the change was performed  | Resource (e.g. WorkPackage) | not null          | READ                 |
+
 ## Local Properties
 | Property    | Description      | Type                 | Constraints | Supported operations |
 | :---------: | -------------    | ----                 | ----------- | -------------------- |
@@ -11,7 +28,7 @@
 
 Activity can be either _type Activity or _type Activity::Comment.
 
-## Activity [/api/v3/activities/{id}]
+## View activity [/api/v3/activities/{id}]
 
 + Model
     + Body
@@ -56,11 +73,14 @@ Activity can be either _type Activity or _type Activity::Comment.
 
 + Response 200 (application/hal+json)
 
-    [Activity][]
+    [View activity][]
+
+## Update activity [/api/v3/activities/{id}]
 
 ## Update activity [PATCH]
 
-Updates an activity's comment and, on success, returns the updated activity.
+Updates an activity's comment and, on success, returns the updated activity. The other properties of the activity
+cannot be altered.
 
 + Parameters
     + id (required, integer, `1`) ... Activity id
@@ -73,7 +93,7 @@ Updates an activity's comment and, on success, returns the updated activity.
 
 + Response 200 (application/hal+json)
 
-    [Activity][]
+    [View activity][]
 
 + Response 400 (application/hal+json)
 
@@ -113,3 +133,77 @@ Updates an activity's comment and, on success, returns the updated activity.
                 "errorIdentifier": "urn:openproject-org:api:v3:errors:PropertyIsReadOnly",
                 "message": "The ID of an activity can't be changed."
             }
+
+## List activities [/api/v3/activities{?filters,sortBy}]
+
++ Model
+    + Body
+
+            {
+                "_links": {
+                    "self": { "href": "/api/v3/activities" }
+                },
+                "total": 2,
+                "count": 2,
+                "_type": "Collection",
+                "_embedded":
+                {
+                    "elements": [
+                        {
+                            "_type": "Activity::Comment",
+                            "_links": {
+                                "self": {
+                                    "href": "/api/v3/activity/1",
+                                    "title": "Priority changed from High to Low"
+                                },
+                                "workPackage": {
+                                    "href": "/api/v3/work_packages/1",
+                                    "title": "quis numquam qui voluptatum quia praesentium blanditiis nisi"
+                                },
+                                "user": {
+                                    "href": "/api/v3/users/1",
+                                    "title": "John Sheppard - admin"
+                                }
+                            },
+                            "id": 1,
+                            "details": [
+                                {
+                                    "format": "markdown",
+                                    "raw": "Lorem ipsum dolor sit amet.",
+                                    "html": "<p>Lorem ipsum dolor sit amet.</p>"
+                                }
+                            ],
+                            "comment": {
+                                "format": "markdown",
+                                "raw": "Lorem ipsum dolor sit amet.",
+                                "html": "<p>Lorem ipsum dolor sit amet.</p>"
+                            },
+                            "createdAt": "2014-05-21T08:51:20Z",
+                            "version": 31
+                        },
+                        <-- omitted for brevity -->
+                    ]
+                }
+            }
+
+## List activities [GET]
+
+Returns a collection of activities. The client can choose to filter the activities similar to how work packages are filtered.
+In addition to the provided filters, the server will reduce the result set to only contain activities, for which the requesting client has sufficient permissions.
+The required permissions depend on the resource the activity is associated with.
+E.g. in case of a work package, the client needs the view_work_packages permission in the project of the altered work package.
+
++ Parameters
+    + filters (optional, string, `[{ "created_at": { "operator": "<>d", "values": ["2021-06-08T02:00:00+02:00"] }" }]`) ... JSON specifying filter conditions.
+    Accepts the same format as returned by the [queries](#queries) endpoint.
+    Currently supported filters are:
+      + created_at: filters activities based on creation time.
+
+    + sortBy = ["id", "asc"] (optional, string, `[["id", "asc"]]`) ... JSON specifying sort criteria.
+    Accepts the same format as returned by the [queries](#queries) endpoint. Currently supported sorts are:
+      + id: Sort by primary key
+      + created_at: Sort by activity creation datetime
+
++ Response 200 (application/hal+json)
+
+    [List activities][]

lib/api/v3/activities/activities_api.rb

@@ -33,6 +33,11 @@ module API
     module Activities
       class ActivitiesAPI < ::API::OpenProjectAPI
         resources :activities do
+          get &::API::V3::Utilities::Endpoints::Index.new(model: ::Journal,
+                                                          api_name: 'Activity',
+                                                          render_representer: ::API::V3::Activities::PaginatedActivityCollectionRepresenter)
+                                                     .mount
+
           route_param :id, type: Integer, desc: 'Activity ID' do
             after_validation do
               @activity = Journal.find(declared_params[:id])

lib/api/v3/activities/activity_representer.rb

@@ -39,7 +39,7 @@ module API
         include API::Decorators::FormattableProperty
 
         self_link path: :activity,
-                  id_attribute: :notes_id,
+                  id_attribute: ->(*) { represented_id },
                   title_getter: ->(*) { nil }
 
         link :workPackage do
@@ -59,13 +59,14 @@ module API
           next unless current_user_allowed_to_edit?
 
           {
-            href: api_v3_paths.activity(represented.notes_id),
+            href: api_v3_paths.activity(represented_id),
             method: :patch
           }
         end
 
-        property :id,
-                 getter: ->(*) { notes_id },
+        property :represented_id,
+                 as: :id,
+                 exec_context: :decorator,
                  render_nil: true
 
         formattable_property :notes,
@@ -105,6 +106,10 @@ module API
           end
         end
 
+        def represented_id
+          represented.respond_to?(:notes_id) ? represented.notes_id : represented.id
+        end
+
         private
 
         def current_user_allowed_to_edit?

lib/api/v3/activities/paginated_activity_collection_representer.rb

@@ -0,0 +1,41 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2021 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+module API
+  module V3
+    module Activities
+      class PaginatedActivityCollectionRepresenter < ::API::Decorators::OffsetPaginatedCollection
+        def element_decorator
+          ActivityRepresenter
+        end
+      end
+    end
+  end
+end

lib/api/v3/utilities/endpoints/index.rb

@@ -62,11 +62,6 @@ module API
             api_name.underscore.pluralize
           end
 
-          attr_accessor :model,
-                        :api_name,
-                        :scope,
-                        :render_representer
-
           private
 
           def render_success(query, params, self_path, base_scope)
@@ -127,14 +122,6 @@ module API
             api_name.pluralize
           end
 
-          def model_class(scope)
-            if scope.is_a? Class
-              scope
-            else
-              scope.model
-            end
-          end
-
           def merge_scopes(scope_a, scope_b)
             if scope_a.is_a? Class
               scope_b

spec/lib/api/v3/activities/activity_representer_rendering_spec.rb

@@ -28,7 +28,9 @@
 
 require 'spec_helper'
 
-describe ::API::V3::Activities::ActivityRepresenter do
+describe ::API::V3::Activities::ActivityRepresenter, 'rendering' do
+  include ::API::V3::Utilities::PathHelper
+
   let(:current_user) do
     FactoryBot.build_stubbed(:user).tap do |u|
       allow(u)
@@ -40,11 +42,15 @@ describe ::API::V3::Activities::ActivityRepresenter do
   let(:other_user) { FactoryBot.build_stubbed(:user) }
   let(:work_package) { journal.journable }
   let(:notes) { "My notes" }
+  let(:notes_id) { 123 }
   let(:journal) do
-    FactoryBot.build_stubbed(:work_package_journal, notes: notes).tap do |journal|
-      allow(journal)
-        .to receive(:notes_id)
-        .and_return(journal.id)
+    FactoryBot.build_stubbed(:work_package_journal, notes: notes, user: other_user).tap do |journal|
+      if notes_id
+        allow(journal)
+          .to receive(:notes_id)
+          .and_return(notes_id)
+      end
+
       allow(journal)
         .to receive(:get_changes)
         .and_return(changes)
@@ -58,37 +64,63 @@ describe ::API::V3::Activities::ActivityRepresenter do
     login_as(current_user)
   end
 
-  context 'generation' do
-    subject(:generated) { representer.to_json }
+  subject(:generated) { representer.to_json }
 
+  describe 'properties' do
     describe 'type' do
-      it { is_expected.to be_json_eql('Activity::Comment'.to_json).at_path('_type') }
+      context 'with notes' do
+        let(:notes) { 'Some notes' }
 
-      context 'if notes are empty' do
+        it_behaves_like 'property', :_type do
+          let(:value) { 'Activity::Comment' }
+        end
+      end
+
+      context 'with empty notes' do
         let(:notes) { '' }
 
-        it { is_expected.to be_json_eql('Activity'.to_json).at_path('_type') }
+        it_behaves_like 'property', :_type do
+          let(:value) { 'Activity' }
+        end
       end
 
-      context 'if notes and changes are empty' do
+      context 'with empty notes and empty changes' do
         let(:notes) { '' }
         let(:changes) { {} }
 
-        it { is_expected.to be_json_eql('Activity::Comment'.to_json).at_path('_type') }
+        it_behaves_like 'property', :_type do
+          let(:value) { 'Activity::Comment' }
+        end
       end
     end
 
-    it { is_expected.to have_json_type(Object).at_path('_links') }
-    it 'should link to self' do
-      expect(subject).to have_json_path('_links/self/href')
+    describe 'id' do
+      context 'with the journal having notes_id' do
+        it_behaves_like 'property', :id do
+          let(:value) { notes_id }
+        end
+      end
+
+      context 'without the journal having notes_id' do
+        let(:notes_id) { nil }
+
+        it_behaves_like 'property', :id do
+          let(:value) { journal.id }
+        end
+      end
     end
 
-    it { is_expected.to have_json_path('id') }
-    it { is_expected.to have_json_path('version') }
+    describe 'createdAt' do
+      it_behaves_like 'has UTC ISO 8601 date and time' do
+        let(:date) { journal.created_at }
+        let(:json_path) { 'createdAt' }
+      end
+    end
 
-    it_behaves_like 'has UTC ISO 8601 date and time' do
-      let(:date) { journal.created_at }
-      let(:json_path) { 'createdAt' }
+    describe 'version' do
+      it_behaves_like 'property', :version do
+        let(:value) { journal.version }
+      end
     end
 
     describe 'comment' do
@@ -115,7 +147,7 @@ describe ::API::V3::Activities::ActivityRepresenter do
 
       it { is_expected.to have_json_size(journal.details.count).at_path('details') }
 
-      it 'should render all details as formattable' do
+      it 'renders all details as formattable' do
         (0..journal.details.count - 1).each do |x|
           is_expected.to be_json_eql('custom'.to_json).at_path("details/#{x}/format")
           is_expected.to have_json_path("details/#{x}/raw")
@@ -123,33 +155,56 @@ describe ::API::V3::Activities::ActivityRepresenter do
         end
       end
     end
+  end
+
+  describe '_links' do
+    describe 'self' do
+      context 'with the journal having notes_id' do
+        it_behaves_like 'has an untitled link' do
+          let(:link) { 'self' }
+          let(:href) { api_v3_paths.activity notes_id }
+        end
+      end
 
-    it 'should link to work package' do
-      expect(subject).to have_json_path('_links/workPackage/href')
+      context 'without the journal having notes_id' do
+        let(:notes_id) { nil }
+
+        it_behaves_like 'has an untitled link' do
+          let(:link) { 'self' }
+          let(:href) { api_v3_paths.activity journal.id }
+        end
+      end
     end
 
-    it 'should link to user' do
-      expect(subject).to have_json_path('_links/user/href')
+    describe 'workPackage' do
+      it_behaves_like 'has a titled link' do
+        let(:link) { 'workPackage' }
+        let(:href) { api_v3_paths.work_package work_package.id }
+        let(:title) { work_package.subject }
+      end
     end
 
-    it 'should link to update' do
-      expect(subject).to have_json_path('_links/update/href')
+    describe 'user' do
+      it_behaves_like 'has an untitled link' do
+        let(:link) { 'user' }
+        let(:href) { api_v3_paths.user other_user.id }
+      end
     end
 
-    context 'for a non own journal' do
-      context 'when having edit_work_package_notes' do
-        it 'should link to update' do
-          expect(subject).to have_json_path('_links/update/href')
-        end
+    describe 'update' do
+      let(:link) { 'update' }
+      let(:href) { api_v3_paths.activity(notes_id) }
+
+      it_behaves_like 'has an untitled link'
+
+      context 'with a non own journal having edit_work_package_notes permission' do
+        it_behaves_like 'has an untitled link'
       end
 
-      context 'when only having edit_own_work_package_notes' do
+      context 'with a non own journal having only edit_own work_package_notes permission' do
         let(:permissions) { %i(edit_own_work_package_notes) }
 
-        it 'has no update link' do
-          expect(subject)
-            .not_to have_json_path('_links/update/href')
-        end
+        it_behaves_like 'has no link'
       end
     end
   end

spec/requests/api/v3/activities_api_spec.rb

@@ -33,6 +33,8 @@ describe API::V3::Activities::ActivitiesAPI, type: :request, content_type: :json
   include Rack::Test::Methods
   include API::V3::Utilities::PathHelper
 
+  subject(:response) { last_response }
+
   let(:current_user) do
     FactoryBot.create(:user,
                       member_in_project: project,
@@ -47,8 +49,6 @@ describe API::V3::Activities::ActivitiesAPI, type: :request, content_type: :json
   let(:comment) { 'This is a new test comment!' }
 
   shared_examples_for 'valid activity request' do |type|
-    subject { last_response }
-
     it 'returns an activity of the correct type' do
       expect(subject.body).to be_json_eql(type.to_json).at_path('_type')
       expect(subject.body).to be_json_eql(activity.id.to_json).at_path('id')
@@ -69,6 +69,27 @@ describe API::V3::Activities::ActivitiesAPI, type: :request, content_type: :json
     end
   end
 
+  describe 'GET /api/v3/activities' do
+    let(:path) { api_v3_paths.activities }
+
+    before do
+      work_package
+      login_as(current_user)
+
+      get path
+    end
+
+    it 'responds 200 OK' do
+      expect(subject.status).to eq(200)
+    end
+
+    it 'returns work package activities the user is allowed to see' do
+      expect(subject.body)
+        .to be_json_eql(1.to_json)
+              .at_path('total')
+    end
+  end
+
   describe 'PATCH /api/v3/activities/:activityId' do
     let(:params) { { comment: comment } }
     before do
@@ -140,7 +161,7 @@ describe API::V3::Activities::ActivitiesAPI, type: :request, content_type: :json
     end
   end
 
-  describe '#get api' do
+  describe 'GET /api/v3/activities/:id' do
     let(:get_path) { api_v3_paths.activity activity.id }
 
     before do
@@ -166,7 +187,7 @@ describe API::V3::Activities::ActivitiesAPI, type: :request, content_type: :json
         it_behaves_like 'valid activity request', 'Activity::Comment'
       end
 
-      context 'for an aggregated journal when requesting by the notes_id (which is not the aggregated journal`s id)`' do
+      context 'for an aggregated journal when requesting by the notes_id (which is not the aggregated journal`s id)' do
         let(:activity) do
           work_package.journals.first.tap do |journal|
             journal.update_column(:notes, comment)