Merge branch '32057_support_ordering_project_notes_in_notes_api' into 'master'

support ordering of project notes in notes api

See merge request gitlab-org/gitlab-ce!15342

4 files changed, 156 insertions, 13 deletions

diff --git a/changelogs/unreleased/32057_support_ordering_project_notes_in_notes_api.yml b/changelogs/unreleased/32057_support_ordering_project_notes_in_notes_api.yml
new file mode 100644
index 00000000000..cd19d24e035
--- /dev/null
+++ b/changelogs/unreleased/32057_support_ordering_project_notes_in_notes_api.yml
@@ -0,0 +1,5 @@
+---
+title: added support for ordering and sorting in notes api
+merge_request: 15342
+author: haseebeqx
+type: added
diff --git a/doc/api/notes.md b/doc/api/notes.md
index e627369e17b..d02ef84d0bd 100644
--- a/doc/api/notes.md
+++ b/doc/api/notes.md
@@ -10,12 +10,15 @@ Gets a list of all notes for a single issue.
 
 ```
 GET /projects/:id/issues/:issue_iid/notes
+GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at
 ```
 
-Parameters:
-
-- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
-- `issue_iid` (required) - The IID of an issue
+| Attribute           | Type             | Required   | Description                                                                                                                                         |
+| ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `id`                | integer/string   | yes        | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
+| `issue_iid`         | integer          | yes        | The IID of an issue
+| `sort`              | string           | no         | Return issue notes sorted in `asc` or `desc` order. Default is `desc`
+| `order_by`          | string           | no         | Return issue notes ordered by `created_at` or `updated_at` fields. Default is `created_at`
 
 ```json
 [
@@ -133,12 +136,15 @@ Gets a list of all notes for a single snippet. Snippet notes are comments users
 
 ```
 GET /projects/:id/snippets/:snippet_id/notes
+GET /projects/:id/snippets/:snippet_id/notes?sort=asc&order_by=updated_at
 ```
 
-Parameters:
-
-- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
-- `snippet_id` (required) - The ID of a project snippet
+| Attribute           | Type             | Required   | Description                                                                                                                                         |
+| ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `id`                | integer/string   | yes        | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
+| `snippet_id`        | integer          | yes        | The ID of a project snippet
+| `sort`              | string           | no         | Return snippet notes sorted in `asc` or `desc` order. Default is `desc`
+| `order_by`          | string           | no         | Return snippet notes ordered by `created_at` or `updated_at` fields. Default is `created_at`
 
 ### Get single snippet note
 
@@ -231,12 +237,15 @@ Gets a list of all notes for a single merge request.
 
 ```
 GET /projects/:id/merge_requests/:merge_request_iid/notes
+GET /projects/:id/merge_requests/:merge_request_iid/notes?sort=asc&order_by=updated_at
 ```
 
-Parameters:
-
-- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
-- `merge_request_iid` (required) - The IID of a project merge request
+| Attribute           | Type             | Required   | Description                                                                                                                                         |
+| ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `id`                | integer/string   | yes        | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
+| `merge_request_iid` | integer          | yes        | The IID of a project merge request
+| `sort`              | string           | no         | Return merge request notes sorted in `asc` or `desc` order. Default is `desc`
+| `order_by`          | string           | no         | Return merge request notes ordered by `created_at` or `updated_at` fields. Default is `created_at`
 
 ### Get single merge request note
 
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index ceaaeca4046..3588dc85c9e 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -18,6 +18,10 @@ module API
         end
         params do
           requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
+          optional :order_by, type: String, values: %w[created_at updated_at], default: 'created_at',
+                              desc: 'Return notes ordered by `created_at` or `updated_at` fields.'
+          optional :sort, type: String, values: %w[asc desc], default: 'desc',
+                          desc: 'Return notes sorted in `asc` or `desc` order.'
           use :pagination
         end
         get ":id/#{noteables_str}/:noteable_id/notes" do
@@ -29,11 +33,12 @@ module API
             # at the DB query level (which we cannot in that case), the current
             # page can have less elements than :per_page even if
             # there's more than one page.
+            raw_notes = noteable.notes.with_metadata.reorder(params[:order_by] => params[:sort])
             notes =
               # paginate() only works with a relation. This could lead to a
               # mismatch between the pagination headers info and the actual notes
               # array returned, but this is really a edge-case.
-              paginate(noteable.notes.with_metadata)
+              paginate(raw_notes)
               .reject { |n| n.cross_reference_not_visible_for?(current_user) }
             present notes, with: Entities::Note
           else
diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb
index 784070db173..3bfb4c5506f 100644
--- a/spec/requests/api/notes_spec.rb
+++ b/spec/requests/api/notes_spec.rb
@@ -34,6 +34,48 @@ describe API::Notes do
 
   describe "GET /projects/:id/noteable/:noteable_id/notes" do
     context "when noteable is an Issue" do
+      context 'sorting' do
+        before do
+          create_list(:note, 3, noteable: issue, project: project, author: user)
+        end
+
+        it 'sorts by created_at in descending order by default' do
+          get api("/projects/#{project.id}/issues/#{issue.iid}/notes", user)
+
+          response_dates = json_response.map { |noteable| noteable['created_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        it 'sorts by ascending order when requested' do
+          get api("/projects/#{project.id}/issues/#{issue.iid}/notes?sort=asc", user)
+
+          response_dates = json_response.map { |noteable| noteable['created_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort)
+        end
+
+        it 'sorts by updated_at in descending order when requested' do
+          get api("/projects/#{project.id}/issues/#{issue.iid}/notes?order_by=updated_at", user)
+
+          response_dates = json_response.map { |noteable| noteable['updated_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        it 'sorts by updated_at in ascending order when requested' do
+          get api("/projects/#{project.id}/issues/#{issue.iid}/notes??order_by=updated_at&sort=asc", user)
+
+          response_dates = json_response.map { |noteable| noteable['updated_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort)
+        end
+      end
+
       it "returns an array of issue notes" do
         get api("/projects/#{project.id}/issues/#{issue.iid}/notes", user)
 
@@ -85,6 +127,47 @@ describe API::Notes do
     end
 
     context "when noteable is a Snippet" do
+      context 'sorting' do
+        before do
+          create_list(:note, 3, noteable: snippet, project: project, author: user)
+        end
+
+        it 'sorts by created_at in descending order by default' do
+          get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user)
+
+          response_dates = json_response.map { |noteable| noteable['created_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        it 'sorts by ascending order when requested' do
+          get api("/projects/#{project.id}/snippets/#{snippet.id}/notes?sort=asc", user)
+
+          response_dates = json_response.map { |noteable| noteable['created_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort)
+        end
+
+        it 'sorts by updated_at in descending order when requested' do
+          get api("/projects/#{project.id}/snippets/#{snippet.id}/notes?order_by=updated_at", user)
+
+          response_dates = json_response.map { |noteable| noteable['updated_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        it 'sorts by updated_at in ascending order when requested' do
+          get api("/projects/#{project.id}/snippets/#{snippet.id}/notes??order_by=updated_at&sort=asc", user)
+
+          response_dates = json_response.map { |noteable| noteable['updated_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort)
+        end
+      end
       it "returns an array of snippet notes" do
         get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user)
 
@@ -108,6 +191,47 @@ describe API::Notes do
     end
 
     context "when noteable is a Merge Request" do
+      context 'sorting' do
+        before do
+          create_list(:note, 3, noteable: merge_request, project: project, author: user)
+        end
+
+        it 'sorts by created_at in descending order by default' do
+          get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", user)
+
+          response_dates = json_response.map { |noteable| noteable['created_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        it 'sorts by ascending order when requested' do
+          get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes?sort=asc", user)
+
+          response_dates = json_response.map { |noteable| noteable['created_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort)
+        end
+
+        it 'sorts by updated_at in descending order when requested' do
+          get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes?order_by=updated_at", user)
+
+          response_dates = json_response.map { |noteable| noteable['updated_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        it 'sorts by updated_at in ascending order when requested' do
+          get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes??order_by=updated_at&sort=asc", user)
+
+          response_dates = json_response.map { |noteable| noteable['updated_at'] }
+
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort)
+        end
+      end
       it "returns an array of merge_requests notes" do
         get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", user)