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 0000000000000000000000000000000000000000..cd19d24e0357d6888a0e1ac1f2cb28fb6e03a2ce --- /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 e627369e17b1dca05af32a778a9ced2dd7976cf8..d02ef84d0bdaeb366a1e82d3529bc62a821a9785 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 ceaaeca4046015117291f25844ffa0adeb97e868..3588dc85c9e70afda263734330d01dcfc5905483 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 784070db173226d7bd63fba2688ce3546df68f17..3bfb4c5506f195538ff351208c8ffb2465680ea8 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)