From fbd3b3d8a245072121784df11b7b41d3257b989f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 12 May 2017 04:12:04 +0900 Subject: Add API support for pipeline schedule --- app/models/ci/pipeline_schedule.rb | 4 + lib/api/api.rb | 1 + lib/api/entities.rb | 8 + lib/api/pipeline_schedules.rb | 127 ++++++++++ spec/fixtures/api/schemas/pipeline_schedule.json | 64 +++++ spec/fixtures/api/schemas/pipeline_schedules.json | 4 + spec/requests/api/pipeline_schedules_spec.rb | 278 ++++++++++++++++++++++ 7 files changed, 486 insertions(+) create mode 100644 lib/api/pipeline_schedules.rb create mode 100644 spec/fixtures/api/schemas/pipeline_schedule.json create mode 100644 spec/fixtures/api/schemas/pipeline_schedules.json create mode 100644 spec/requests/api/pipeline_schedules_spec.rb diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index 07213ca608a..58cf62513d3 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -40,6 +40,10 @@ module Ci self.next_run_at = Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now) end + def last_pipeline + self.pipelines&.last + end + def schedule_next_run! save! # with set_next_run_at rescue ActiveRecord::RecordInvalid diff --git a/lib/api/api.rb b/lib/api/api.rb index ac113c5200d..bbdd2039f43 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -110,6 +110,7 @@ module API mount ::API::Notes mount ::API::NotificationSettings mount ::API::Pipelines + mount ::API::PipelineSchedules mount ::API::ProjectHooks mount ::API::Projects mount ::API::ProjectSnippets diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 8c5e5c91769..1f1942b2ec1 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -686,6 +686,14 @@ module API expose :coverage end + class PipelineSchedule < Grape::Entity + expose :id + expose :description, :ref, :cron, :cron_timezone, :next_run_at, :active + expose :created_at, :updated_at, :deleted_at + expose :last_pipeline, using: Entities::Pipeline, if: -> (pipeline_schedule, opts) { pipeline_schedule.last_pipeline.present? } + expose :owner, using: Entities::UserBasic + end + class EnvironmentBasic < Grape::Entity expose :id, :name, :slug, :external_url end diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb new file mode 100644 index 00000000000..32fa5a86fab --- /dev/null +++ b/lib/api/pipeline_schedules.rb @@ -0,0 +1,127 @@ +module API + class PipelineSchedules < Grape::API + include PaginationParams + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: { id: %r{[^/]+} } do + desc 'Get pipeline_schedules list' do + success Entities::PipelineSchedule + end + params do + use :pagination + end + get ':id/pipeline_schedules' do + authenticate! + authorize! :read_pipeline_schedule, user_project + + pipeline_schedules = user_project.pipeline_schedules + + present paginate(pipeline_schedules), with: Entities::PipelineSchedule + end + + desc 'Get specific pipeline_schedule of a project' do + success Entities::PipelineSchedule + end + params do + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline_schedule ID' + end + get ':id/pipeline_schedules/:pipeline_schedule_id' do + authenticate! + authorize! :read_pipeline_schedule, user_project + + pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) + return not_found!('PipelineSchedule') unless pipeline_schedule + + present pipeline_schedule, with: Entities::PipelineSchedule + end + + desc 'Create a pipeline_schedule' do + success Entities::PipelineSchedule + end + params do + requires :description, type: String, desc: 'The pipeline_schedule description' + requires :ref, type: String, desc: 'The pipeline_schedule ref' + requires :cron, type: String, desc: 'The pipeline_schedule cron' + requires :cron_timezone, type: String, desc: 'The pipeline_schedule cron_timezone' + requires :active, type: Boolean, desc: 'The pipeline_schedule active' + end + post ':id/pipeline_schedules' do + authenticate! + authorize! :create_pipeline_schedule, user_project + + pipeline_schedule = user_project.pipeline_schedules.create( + declared_params(include_missing: false).merge(owner: current_user)) + + if pipeline_schedule.valid? + present pipeline_schedule, with: Entities::PipelineSchedule + else + render_validation_error!(pipeline_schedule) + end + end + + desc 'Update a pipeline_schedule' do + success Entities::PipelineSchedule + end + params do + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline_schedule ID' + optional :description, type: String, desc: 'The pipeline_schedule description' + optional :ref, type: String, desc: 'The pipeline_schedule ref' + optional :cron, type: String, desc: 'The pipeline_schedule cron' + optional :cron_timezone, type: String, desc: 'The pipeline_schedule cron_timezone' + optional :active, type: Boolean, desc: 'The pipeline_schedule active' + end + put ':id/pipeline_schedules/:pipeline_schedule_id' do + authenticate! + authorize! :create_pipeline_schedule, user_project + + pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) + return not_found!('PipelineSchedule') unless pipeline_schedule + + if pipeline_schedule.update(declared_params(include_missing: false)) + present pipeline_schedule, with: Entities::PipelineSchedule + else + render_validation_error!(pipeline_schedule) + end + end + + desc 'Take ownership of pipeline_schedule' do + success Entities::PipelineSchedule + end + params do + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline_schedule ID' + end + post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do + authenticate! + authorize! :create_pipeline_schedule, user_project + + pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) + return not_found!('PipelineSchedule') unless pipeline_schedule + + if pipeline_schedule.update(owner: current_user) + status :ok + present pipeline_schedule, with: Entities::PipelineSchedule + else + render_validation_error!(pipeline_schedule) + end + end + + desc 'Delete a pipeline_schedule' do + success Entities::PipelineSchedule + end + params do + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline_schedule ID' + end + delete ':id/pipeline_schedules/:pipeline_schedule_id' do + authenticate! + authorize! :admin_pipeline_schedule, user_project + + pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) + return not_found!('PipelineSchedule') unless pipeline_schedule + + present pipeline_schedule.destroy, with: Entities::PipelineSchedule + end + end + end +end diff --git a/spec/fixtures/api/schemas/pipeline_schedule.json b/spec/fixtures/api/schemas/pipeline_schedule.json new file mode 100644 index 00000000000..46309b212a1 --- /dev/null +++ b/spec/fixtures/api/schemas/pipeline_schedule.json @@ -0,0 +1,64 @@ +{ + "type": "object", + "properties" : { + "id": { "type": "integer" }, + "description": { "type": "string" }, + "ref": { "type": "string" }, + "cron": { "type": "string" }, + "cron_timezone": { "type": "string" }, + "next_run_at": { "type": "date" }, + "active": { "type": "boolean" }, + "created_at": { "type": "date" }, + "updated_at": { "type": "date" }, + "deleted_at": { "type": "date" }, + "last_pipeline": { + "type": ["object", "null"], + "properties": { + "id": { "type": "integer" }, + "sha": { "type": "string" }, + "ref": { "type": "string" }, + "status": { "type": "string" }, + "before_sha": { "type": ["string", "null"] }, + "tag": { "type": ["boolean", "null"] }, + "yaml_errors": { "type": ["string", "null"] }, + "user": { + "type": ["object", "null"], + "properties": { + "name": { "type": "string" }, + "username": { "type": "string" }, + "id": { "type": "integer" }, + "state": { "type": "string" }, + "avatar_url": { "type": "uri" }, + "web_url": { "type": "uri" } + }, + "additionalProperties": false + }, + "created_at": { "type": "date" }, + "updated_at": { "type": "date" }, + "started_at": { "type": "date" }, + "finished_at": { "type": "date" }, + "committed_at": { "type": ["string", "null"] }, + "duration": { "type": ["integer", "null"] }, + "coverage": { "type": ["string", "null"] } + }, + "additionalProperties": false + }, + "owner": { + "type": "object", + "properties": { + "name": { "type": "string" }, + "username": { "type": "string" }, + "id": { "type": "integer" }, + "state": { "type": "string" }, + "avatar_url": { "type": "uri" }, + "web_url": { "type": "uri" } + }, + "additionalProperties": false + } + }, + "required": [ + "id", "description", "ref", "cron", "cron_timezone", "next_run_at", + "active", "created_at", "updated_at", "deleted_at", "owner" + ], + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/pipeline_schedules.json b/spec/fixtures/api/schemas/pipeline_schedules.json new file mode 100644 index 00000000000..173a28d2505 --- /dev/null +++ b/spec/fixtures/api/schemas/pipeline_schedules.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "pipeline_schedule.json" } +} diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb new file mode 100644 index 00000000000..31a2a4d576c --- /dev/null +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -0,0 +1,278 @@ +require 'spec_helper' + +describe API::PipelineSchedules do + let(:developer) { create(:user) } + let(:user) { create(:user) } + let!(:project) { create(:project, :repository) } + + before do + project.add_developer(developer) + end + + describe 'GET /projects/:id/pipeline_schedules' do + context 'authenticated user with valid permissions' do + before do + create(:ci_pipeline_schedule, project: project, owner: developer) + .tap do |pipeline_schedule| + pipeline_schedule.pipelines << build(:ci_pipeline, project: project) + end + end + + it 'returns list of pipeline_schedules' do + get api("/projects/#{project.id}/pipeline_schedules", developer) + + expect(response).to have_http_status(:ok) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('pipeline_schedules') + end + end + + context 'authenticated user with invalid permissions' do + it 'does not return pipeline_schedules list' do + get api("/projects/#{project.id}/pipeline_schedules", user) + + expect(response).to have_http_status(:not_found) + end + end + + context 'unauthenticated user' do + it 'does not return pipeline_schedules list' do + get api("/projects/#{project.id}/pipeline_schedules") + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'GET /projects/:id/pipeline_schedules/:pipeline_schedule_id' do + let(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: developer) + .tap do |pipeline_schedule| + pipeline_schedule.pipelines << build(:ci_pipeline, project: project) + end + end + + context 'authenticated user with valid permissions' do + it 'returns pipeline_schedule details' do + get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer) + + expect(response).to have_http_status(:ok) + expect(response).to match_response_schema('pipeline_schedule') + end + + it 'responds with 404 Not Found if requesting non-existing pipeline_schedule' do + get api("/projects/#{project.id}/pipeline_schedules/-5", developer) + + expect(response).to have_http_status(:not_found) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not return pipeline_schedules list' do + get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + + expect(response).to have_http_status(:not_found) + end + end + + context 'unauthenticated user' do + it 'does not return pipeline_schedules list' do + get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}") + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'POST /projects/:id/pipeline_schedules' do + let(:description) { 'pipeline_schedule' } + let(:ref) { 'master' } + let(:cron) { '* * * * *' } + let(:cron_timezone) { 'UTC' } + let(:active) { true } + + context 'authenticated user with valid permissions' do + context 'with required parameters' do + it 'creates pipeline_schedule' do + expect do + post api("/projects/#{project.id}/pipeline_schedules", developer), + description: description, ref: ref, cron: cron, + cron_timezone: cron_timezone, active: active + end + .to change{project.pipeline_schedules.count}.by(1) + + expect(response).to have_http_status(:created) + expect(response).to match_response_schema('pipeline_schedule') + expect(json_response['description']).to eq(description) + expect(json_response['ref']).to eq(ref) + expect(json_response['cron']).to eq(cron) + expect(json_response['cron_timezone']).to eq(cron_timezone) + expect(json_response['active']).to eq(active) + end + end + + context 'without required parameters' do + it 'does not create pipeline_schedule' do + post api("/projects/#{project.id}/pipeline_schedules", developer) + + expect(response).to have_http_status(:bad_request) + end + end + end + + context 'authenticated user with invalid permissions' do + it 'does not create pipeline_schedule' do + post api("/projects/#{project.id}/pipeline_schedules", user), + description: description, ref: ref, cron: cron, + cron_timezone: cron_timezone, active: active + + expect(response).to have_http_status(:not_found) + end + end + + context 'unauthenticated user' do + it 'does not create pipeline_schedule' do + post api("/projects/#{project.id}/pipeline_schedules"), + description: description, ref: ref, cron: cron, + cron_timezone: cron_timezone, active: active + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' do + let(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: developer) + end + + context 'authenticated user with valid permissions' do + let(:new_ref) { 'patch-x' } + + it 'updates ref' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer), + ref: new_ref + + expect(response).to have_http_status(:ok) + expect(response).to match_response_schema('pipeline_schedule') + expect(json_response['ref']).to eq(new_ref) + end + + let(:new_cron) { '1 2 3 4 *' } + + it 'updates cron' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer), + cron: new_cron + pipeline_schedule.reload + + expect(response).to have_http_status(:ok) + expect(response).to match_response_schema('pipeline_schedule') + expect(json_response['cron']).to eq(new_cron) + expect(pipeline_schedule.next_run_at.min).to eq(1) + expect(pipeline_schedule.next_run_at.hour).to eq(2) + expect(pipeline_schedule.next_run_at.day).to eq(3) + expect(pipeline_schedule.next_run_at.month).to eq(4) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user) + + expect(response).to have_http_status(:not_found) + end + end + + context 'unauthenticated user' do + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}") + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do + let(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: developer) + end + + context 'authenticated user with valid permissions' do + let(:developer2) { create(:user) } + + before do + project.add_developer(developer2) + end + + it 'updates owner' do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer2) + pipeline_schedule.reload + + expect(response).to have_http_status(:ok) + expect(response).to match_response_schema('pipeline_schedule') + expect(pipeline_schedule.owner).to eq(developer2) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not update owner' do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", user) + + expect(response).to have_http_status(:not_found) + end + end + + context 'unauthenticated user' do + it 'does not update owner' do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership") + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id' do + let(:master) { create(:user) } + + let!(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: developer) + end + + before do + project.add_master(master) + end + + context 'authenticated user with valid permissions' do + it 'deletes pipeline_schedule' do + expect do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", master) + end.to change{project.pipeline_schedules.count}.by(-1) + + expect(response).to have_http_status(:ok) + expect(response).to match_response_schema('pipeline_schedule') + end + + it 'responds with 404 Not Found if requesting non-existing pipeline_schedule' do + delete api("/projects/#{project.id}/pipeline_schedules/-5", master) + + expect(response).to have_http_status(:not_found) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not delete pipeline_schedule' do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer) + + expect(response).to have_http_status(403) + end + end + + context 'unauthenticated user' do + it 'does not delete pipeline_schedule' do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}") + + expect(response).to have_http_status(401) + end + end + end +end -- cgit v1.2.1 From 384fd190e513de41a36726e14bfc3651319c7324 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 12 May 2017 21:52:31 +0900 Subject: Change status number to ailias --- spec/requests/api/pipeline_schedules_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 31a2a4d576c..6a3cc89909d 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -263,7 +263,7 @@ describe API::PipelineSchedules do it 'does not delete pipeline_schedule' do delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer) - expect(response).to have_http_status(403) + expect(response).to have_http_status(:forbidden) end end @@ -271,7 +271,7 @@ describe API::PipelineSchedules do it 'does not delete pipeline_schedule' do delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}") - expect(response).to have_http_status(401) + expect(response).to have_http_status(:unauthorized) end end end -- cgit v1.2.1 From 5875d9969a31b754f2dda8b271f0fdddec5c3451 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 12 May 2017 21:53:31 +0900 Subject: Add changelog --- changelogs/unreleased/30892-add-api-support-for-pipeline-schedule.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/30892-add-api-support-for-pipeline-schedule.yml diff --git a/changelogs/unreleased/30892-add-api-support-for-pipeline-schedule.yml b/changelogs/unreleased/30892-add-api-support-for-pipeline-schedule.yml new file mode 100644 index 00000000000..26ce84697d0 --- /dev/null +++ b/changelogs/unreleased/30892-add-api-support-for-pipeline-schedule.yml @@ -0,0 +1,4 @@ +--- +title: Add API support for pipeline schedule +merge_request: 11307 +author: dosuken123 -- cgit v1.2.1 From 359b17611dbbfec8881319f29b81cf8f1c7bf06e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 01:37:01 +0900 Subject: Add doc --- doc/api/README.md | 1 + doc/api/pipeline_schedules.md | 253 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 254 insertions(+) create mode 100644 doc/api/pipeline_schedules.md diff --git a/doc/api/README.md b/doc/api/README.md index 1b0f6470b13..45579ccac4e 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -33,6 +33,7 @@ following locations: - [Notification settings](notification_settings.md) - [Pipelines](pipelines.md) - [Pipeline Triggers](pipeline_triggers.md) +- [Pipeline Schedules](pipeline_schedules.md) - [Projects](projects.md) including setting Webhooks - [Project Access Requests](access_requests.md) - [Project Members](members.md) diff --git a/doc/api/pipeline_schedules.md b/doc/api/pipeline_schedules.md new file mode 100644 index 00000000000..60b968822d8 --- /dev/null +++ b/doc/api/pipeline_schedules.md @@ -0,0 +1,253 @@ +# Pipeline schedules + +You can read more about [pipeline schedules](../ci/pipeline_schedules.md). + +## List Pipeline schedules + +Get a list of pipeline schedules. + +``` +GET /projects/:id/pipeline_schedules +``` + +| 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 | + +``` +curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules" +``` + +```json +[ + { + "id": 11, + "description": "Acceptance Test", + "ref": "master", + "cron": "0 4 * * *", + "cron_timezone": "America/Los_Angeles", + "next_run_at": "2017-05-13T11:00:00.000Z", + "active": true, + "created_at": "2017-05-12T13:10:34.497Z", + "updated_at": "2017-05-12T13:10:34.497Z", + "deleted_at": null, + "owner": { + "name": "Administrator", + "username": "root", + "id": 1, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "web_url": "http://192.168.10.5:3000/root" + } + } +] +``` + +## Single pipeline schedule + +Get a single pipeline schedule. + +``` +GET /projects/:id/pipeline_schedules/:pipeline_schedule_id +``` + +| 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 | +| `pipeline_schedule_id` | integer | yes | The pipeline schedule id | + +``` +curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules/11" +``` + +```json +{ + "id": 11, + "description": "Acceptance Test", + "ref": "master", + "cron": "0 4 * * *", + "cron_timezone": "America/Los_Angeles", + "next_run_at": "2017-05-13T11:00:00.000Z", + "active": true, + "created_at": "2017-05-12T13:10:34.497Z", + "updated_at": "2017-05-12T13:10:34.497Z", + "deleted_at": null, + "owner": { + "name": "Administrator", + "username": "root", + "id": 1, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "web_url": "http://192.168.10.5:3000/root" + } +} +``` + +## New pipeline schedule + +Creates a new pipeline schedule. + +``` +POST /projects/:id/pipeline_schedules +``` + +| 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 | +| `description` | string | yes | The description of pipeline schedule | +| `ref` | string | yes | The branch/tag name will be triggered | +| `cron ` | string | yes | The cron (e.g. `0 1 * * *`) ([Cron syntax](https://en.wikipedia.org/wiki/Cron)) | +| `cron_timezone ` | string | yes | The timezone supproted by `ActiveSupport::TimeZone` (e.g. `Pacific Time (US & Canada)`) or `TZInfo::Timezone` (e.g. `America/Los_Angeles`) | +| `active ` | boolean | yes | The activation of pipeline schedule. If false is set, the pipeline schedule will deactivated initially. | + +``` +curl --request POST --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form description="Build packages" --form ref="master" --form cron="0 1 * * 5" --form cron_timezone="UTC" --form active="true" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules" +``` + +```json +{ + "id": 12, + "description": "Build packages", + "ref": "master", + "cron": "0 1 * * 5", + "cron_timezone": "UTC", + "next_run_at": "2017-05-19T01:00:00.000Z", + "active": true, + "created_at": "2017-05-12T13:18:58.879Z", + "updated_at": "2017-05-12T13:18:58.879Z", + "deleted_at": null, + "owner": { + "name": "Administrator", + "username": "root", + "id": 1, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "web_url": "http://192.168.10.5:3000/root" + } +} +``` + +## Edit pipeline schedule + +Updates an existing pipeline schedule. Once the update is done, it will be rescheduled automatically. + +``` +PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id +``` + +| 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 | +| `pipeline_schedule_id` | integer | yes | The pipeline schedule id | +| `description` | string | no | The description of pipeline schedule | +| `ref` | string | no | The branch/tag name will be triggered | +| `cron ` | string | no | The cron (e.g. `0 1 * * *`) ([Cron syntax](https://en.wikipedia.org/wiki/Cron)) | +| `cron_timezone ` | string | no | The timezone supproted by `ActiveSupport::TimeZone` (e.g. `Pacific Time (US & Canada)`) or `TZInfo::Timezone` (e.g. `America/Los_Angeles`) | +| `active ` | boolean | no | The activation of pipeline schedule. If false is set, the pipeline schedule will deactivated initially. | + +``` +curl --request PUT --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form cron="0 2 * * *" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules/11" +``` + +```json +{ + "id": 11, + "description": "Acceptance Test", + "ref": "master", + "cron": "0 2 * * *", + "cron_timezone": "America/Los_Angeles", + "next_run_at": "2017-05-13T09:00:00.000Z", + "active": true, + "created_at": "2017-05-12T13:10:34.497Z", + "updated_at": "2017-05-12T13:22:07.798Z", + "deleted_at": null, + "owner": { + "name": "Administrator", + "username": "root", + "id": 1, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "web_url": "http://192.168.10.5:3000/root" + } +} +``` + +## Take ownership of a pipeline schedule + +Update an owner of a pipeline schedule. + +``` +POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/take_ownership +``` + +| 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 | +| `pipeline_schedule_id` | integer | yes | The pipeline schedule id | + +``` +curl --request POST --header "PRIVATE-TOKEN: hf2CvZXB9w8Uc5pZKpSB" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules/11/take_ownership" +``` + +```json +{ + "id": 11, + "description": "Acceptance Test", + "ref": "master", + "cron": "0 2 * * *", + "cron_timezone": "America/Los_Angeles", + "next_run_at": "2017-05-13T09:00:00.000Z", + "active": true, + "created_at": "2017-05-12T13:10:34.497Z", + "updated_at": "2017-05-12T13:26:12.191Z", + "deleted_at": null, + "owner": { + "name": "shinya", + "username": "maeda", + "id": 50, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/8ca0a796a679c292e3a11da50f99e801?s=80&d=identicon", + "web_url": "http://192.168.10.5:3000/maeda" + } +} +``` + +## Delete a pipeline schedule + +Delete a pipeline schedule. + +``` +DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id +``` + +| 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 | +| `pipeline_schedule_id` | integer | yes | The pipeline schedule id | + +``` +curl --request DELETE --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules/11" +``` + +```json +{ + "id": 11, + "description": "Acceptance Test", + "ref": "master", + "cron": "0 2 * * *", + "cron_timezone": "America/Los_Angeles", + "next_run_at": "2017-05-13T09:00:00.000Z", + "active": true, + "created_at": "2017-05-12T13:10:34.497Z", + "updated_at": "2017-05-12T13:26:12.191Z", + "deleted_at": "2017-05-12T13:27:38.529Z", + "owner": { + "name": "shinya", + "username": "maeda", + "id": 50, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/8ca0a796a679c292e3a11da50f99e801?s=80&d=identicon", + "web_url": "http://192.168.10.5:3000/maeda" + } +} +``` -- cgit v1.2.1 From 0a11ab48993d092bd730af9b378fbf665b255625 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 01:48:34 +0900 Subject: Reflect doc to api desc --- lib/api/pipeline_schedules.rb | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 32fa5a86fab..5b3af090e2b 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -6,7 +6,7 @@ module API requires :id, type: String, desc: 'The ID of a project' end resource :projects, requirements: { id: %r{[^/]+} } do - desc 'Get pipeline_schedules list' do + desc 'Get a list of pipeline schedules' do success Entities::PipelineSchedule end params do @@ -21,11 +21,11 @@ module API present paginate(pipeline_schedules), with: Entities::PipelineSchedule end - desc 'Get specific pipeline_schedule of a project' do + desc 'Get a single pipeline schedule' do success Entities::PipelineSchedule end params do - requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline_schedule ID' + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end get ':id/pipeline_schedules/:pipeline_schedule_id' do authenticate! @@ -37,15 +37,15 @@ module API present pipeline_schedule, with: Entities::PipelineSchedule end - desc 'Create a pipeline_schedule' do + desc 'Creates a new pipeline schedule' do success Entities::PipelineSchedule end params do - requires :description, type: String, desc: 'The pipeline_schedule description' - requires :ref, type: String, desc: 'The pipeline_schedule ref' - requires :cron, type: String, desc: 'The pipeline_schedule cron' - requires :cron_timezone, type: String, desc: 'The pipeline_schedule cron_timezone' - requires :active, type: Boolean, desc: 'The pipeline_schedule active' + requires :description, type: String, desc: 'The description of pipeline schedule' + requires :ref, type: String, desc: 'The branch/tag name will be triggered' + requires :cron, type: String, desc: 'The cron' + requires :cron_timezone, type: String, desc: 'The timezone' + requires :active, type: Boolean, desc: 'The activation of pipeline schedule' end post ':id/pipeline_schedules' do authenticate! @@ -61,16 +61,16 @@ module API end end - desc 'Update a pipeline_schedule' do + desc 'Updates an existing pipeline schedule' do success Entities::PipelineSchedule end params do - requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline_schedule ID' - optional :description, type: String, desc: 'The pipeline_schedule description' - optional :ref, type: String, desc: 'The pipeline_schedule ref' - optional :cron, type: String, desc: 'The pipeline_schedule cron' - optional :cron_timezone, type: String, desc: 'The pipeline_schedule cron_timezone' - optional :active, type: Boolean, desc: 'The pipeline_schedule active' + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' + optional :description, type: String, desc: 'The description of pipeline schedule' + optional :ref, type: String, desc: 'The branch/tag name will be triggered' + optional :cron, type: String, desc: 'The cron' + optional :cron_timezone, type: String, desc: 'The timezone' + optional :active, type: Boolean, desc: 'The activation of pipeline schedule' end put ':id/pipeline_schedules/:pipeline_schedule_id' do authenticate! @@ -86,11 +86,11 @@ module API end end - desc 'Take ownership of pipeline_schedule' do + desc 'Update an owner of a pipeline schedule' do success Entities::PipelineSchedule end params do - requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline_schedule ID' + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do authenticate! @@ -107,11 +107,11 @@ module API end end - desc 'Delete a pipeline_schedule' do + desc 'Delete a pipeline schedule' do success Entities::PipelineSchedule end params do - requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline_schedule ID' + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end delete ':id/pipeline_schedules/:pipeline_schedule_id' do authenticate! -- cgit v1.2.1 From fbb070646aeccab5e46f300d130c20fa5a0a22fa Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 02:10:14 +0900 Subject: Add validation spec --- spec/requests/api/pipeline_schedules_spec.rb | 32 ++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 6a3cc89909d..bf61c9de14d 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -118,6 +118,17 @@ describe API::PipelineSchedules do expect(response).to have_http_status(:bad_request) end end + + context 'when cron has validation error' do + it 'does not create pipeline_schedule' do + post api("/projects/#{project.id}/pipeline_schedules", developer), + description: description, ref: ref, cron: 'invalid-cron', + cron_timezone: cron_timezone, active: active + + expect(response).to have_http_status(:bad_request) + expect(json_response['message']).to have_key('cron') + end + end end context 'authenticated user with invalid permissions' do @@ -147,17 +158,6 @@ describe API::PipelineSchedules do end context 'authenticated user with valid permissions' do - let(:new_ref) { 'patch-x' } - - it 'updates ref' do - put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer), - ref: new_ref - - expect(response).to have_http_status(:ok) - expect(response).to match_response_schema('pipeline_schedule') - expect(json_response['ref']).to eq(new_ref) - end - let(:new_cron) { '1 2 3 4 *' } it 'updates cron' do @@ -173,6 +173,16 @@ describe API::PipelineSchedules do expect(pipeline_schedule.next_run_at.day).to eq(3) expect(pipeline_schedule.next_run_at.month).to eq(4) end + + context 'when cron has validation error' do + it 'does not update pipeline_schedule' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer), + cron: 'invalid-cron' + + expect(response).to have_http_status(:bad_request) + expect(json_response['message']).to have_key('cron') + end + end end context 'authenticated user with invalid permissions' do -- cgit v1.2.1 From bd7d7759d6e7f98d2d306495a8de5be44590b490 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 02:21:11 +0900 Subject: Use 'set' to top-level variables. Remove repository. --- spec/requests/api/pipeline_schedules_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index bf61c9de14d..9b8a96a8dc1 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' describe API::PipelineSchedules do - let(:developer) { create(:user) } - let(:user) { create(:user) } - let!(:project) { create(:project, :repository) } + set(:developer) { create(:user) } + set(:user) { create(:user) } + set(:project) { create(:project) } before do project.add_developer(developer) -- cgit v1.2.1 From 97bf2401991ab2e9cea956dfb7c9630e2a185683 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:06:05 +0900 Subject: Remove if from last_pipeline in entity --- lib/api/entities.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 1f1942b2ec1..b2bc0a17142 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -690,7 +690,7 @@ module API expose :id expose :description, :ref, :cron, :cron_timezone, :next_run_at, :active expose :created_at, :updated_at, :deleted_at - expose :last_pipeline, using: Entities::Pipeline, if: -> (pipeline_schedule, opts) { pipeline_schedule.last_pipeline.present? } + expose :last_pipeline, using: Entities::Pipeline expose :owner, using: Entities::UserBasic end -- cgit v1.2.1 From 8743f765abc2281c664792f5016747f54d0fb7aa Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:10:16 +0900 Subject: Use CreatePipelineScheduleService --- lib/api/pipeline_schedules.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 5b3af090e2b..6e6bbb29e3a 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -51,8 +51,9 @@ module API authenticate! authorize! :create_pipeline_schedule, user_project - pipeline_schedule = user_project.pipeline_schedules.create( - declared_params(include_missing: false).merge(owner: current_user)) + pipeline_schedule = Ci::CreatePipelineScheduleService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute if pipeline_schedule.valid? present pipeline_schedule, with: Entities::PipelineSchedule -- cgit v1.2.1 From f8cb5fd65a8ed00f38368a6a050c940e72cc6f3e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:37:09 +0900 Subject: Add own! method on PipleineSchedule --- app/models/ci/pipeline_schedule.rb | 4 ++++ lib/api/pipeline_schedules.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index 58cf62513d3..623275b7269 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -24,6 +24,10 @@ module Ci owner == current_user end + def own!(user) + update!(owner: user) + end + def inactive? !active? end diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 6e6bbb29e3a..81b88b17041 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -100,7 +100,7 @@ module API pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) return not_found!('PipelineSchedule') unless pipeline_schedule - if pipeline_schedule.update(owner: current_user) + if pipeline_schedule.own!(current_user) status :ok present pipeline_schedule, with: Entities::PipelineSchedule else -- cgit v1.2.1 From 9185241a1e591296c4592e19a056ec4761bd0b75 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:38:11 +0900 Subject: Use attributes_for --- spec/requests/api/pipeline_schedules_spec.rb | 40 +++++++++++----------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 9b8a96a8dc1..964c0bbaf04 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -85,29 +85,28 @@ describe API::PipelineSchedules do end describe 'POST /projects/:id/pipeline_schedules' do - let(:description) { 'pipeline_schedule' } - let(:ref) { 'master' } - let(:cron) { '* * * * *' } - let(:cron_timezone) { 'UTC' } - let(:active) { true } + let(:params) do + attributes_for(:ci_pipeline_schedule, + description: 'description', ref: 'master', cron: '* * * * *', + cron_timezone: 'UTC', active: true) + end context 'authenticated user with valid permissions' do context 'with required parameters' do it 'creates pipeline_schedule' do expect do post api("/projects/#{project.id}/pipeline_schedules", developer), - description: description, ref: ref, cron: cron, - cron_timezone: cron_timezone, active: active + params end .to change{project.pipeline_schedules.count}.by(1) expect(response).to have_http_status(:created) expect(response).to match_response_schema('pipeline_schedule') - expect(json_response['description']).to eq(description) - expect(json_response['ref']).to eq(ref) - expect(json_response['cron']).to eq(cron) - expect(json_response['cron_timezone']).to eq(cron_timezone) - expect(json_response['active']).to eq(active) + expect(json_response['description']).to eq(params[:description]) + expect(json_response['ref']).to eq(params[:ref]) + expect(json_response['cron']).to eq(params[:cron]) + expect(json_response['cron_timezone']).to eq(params[:cron_timezone]) + expect(json_response['active']).to eq(params[:active]) end end @@ -122,8 +121,7 @@ describe API::PipelineSchedules do context 'when cron has validation error' do it 'does not create pipeline_schedule' do post api("/projects/#{project.id}/pipeline_schedules", developer), - description: description, ref: ref, cron: 'invalid-cron', - cron_timezone: cron_timezone, active: active + params.tap { |_| params['cron'] = 'invalid-cron' } expect(response).to have_http_status(:bad_request) expect(json_response['message']).to have_key('cron') @@ -133,9 +131,7 @@ describe API::PipelineSchedules do context 'authenticated user with invalid permissions' do it 'does not create pipeline_schedule' do - post api("/projects/#{project.id}/pipeline_schedules", user), - description: description, ref: ref, cron: cron, - cron_timezone: cron_timezone, active: active + post api("/projects/#{project.id}/pipeline_schedules", user), params expect(response).to have_http_status(:not_found) end @@ -143,9 +139,7 @@ describe API::PipelineSchedules do context 'unauthenticated user' do it 'does not create pipeline_schedule' do - post api("/projects/#{project.id}/pipeline_schedules"), - description: description, ref: ref, cron: cron, - cron_timezone: cron_timezone, active: active + post api("/projects/#{project.id}/pipeline_schedules"), params expect(response).to have_http_status(:unauthorized) end @@ -158,16 +152,14 @@ describe API::PipelineSchedules do end context 'authenticated user with valid permissions' do - let(:new_cron) { '1 2 3 4 *' } - it 'updates cron' do put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer), - cron: new_cron + cron: '1 2 3 4 *' pipeline_schedule.reload expect(response).to have_http_status(:ok) expect(response).to match_response_schema('pipeline_schedule') - expect(json_response['cron']).to eq(new_cron) + expect(json_response['cron']).to eq('1 2 3 4 *') expect(pipeline_schedule.next_run_at.min).to eq(1) expect(pipeline_schedule.next_run_at.hour).to eq(2) expect(pipeline_schedule.next_run_at.day).to eq(3) -- cgit v1.2.1 From 18cf368ba0193e61d8b127c80f9b87de4121bf3c Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:39:32 +0900 Subject: Add space between curly brackets --- spec/requests/api/pipeline_schedules_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 964c0bbaf04..c250caa548f 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -98,7 +98,7 @@ describe API::PipelineSchedules do post api("/projects/#{project.id}/pipeline_schedules", developer), params end - .to change{project.pipeline_schedules.count}.by(1) + .to change { project.pipeline_schedules.count }.by(1) expect(response).to have_http_status(:created) expect(response).to match_response_schema('pipeline_schedule') -- cgit v1.2.1 From 427eac90a3b6965009dd853d7246c248988974e8 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:41:31 +0900 Subject: Concatinate dot behind end --- spec/requests/api/pipeline_schedules_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index c250caa548f..6b0310a6a2c 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -97,8 +97,7 @@ describe API::PipelineSchedules do expect do post api("/projects/#{project.id}/pipeline_schedules", developer), params - end - .to change { project.pipeline_schedules.count }.by(1) + end.to change { project.pipeline_schedules.count }.by(1) expect(response).to have_http_status(:created) expect(response).to match_response_schema('pipeline_schedule') -- cgit v1.2.1 From a104e7a2c92d9752e48b2ac776b547809b0036d5 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 16 May 2017 23:58:08 +0900 Subject: Move authenticate! to before --- lib/api/pipeline_schedules.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 81b88b17041..2319ff639e5 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -2,6 +2,8 @@ module API class PipelineSchedules < Grape::API include PaginationParams + before { authenticate! } + params do requires :id, type: String, desc: 'The ID of a project' end @@ -13,7 +15,6 @@ module API use :pagination end get ':id/pipeline_schedules' do - authenticate! authorize! :read_pipeline_schedule, user_project pipeline_schedules = user_project.pipeline_schedules @@ -28,7 +29,6 @@ module API requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end get ':id/pipeline_schedules/:pipeline_schedule_id' do - authenticate! authorize! :read_pipeline_schedule, user_project pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) @@ -48,7 +48,6 @@ module API requires :active, type: Boolean, desc: 'The activation of pipeline schedule' end post ':id/pipeline_schedules' do - authenticate! authorize! :create_pipeline_schedule, user_project pipeline_schedule = Ci::CreatePipelineScheduleService @@ -74,7 +73,6 @@ module API optional :active, type: Boolean, desc: 'The activation of pipeline schedule' end put ':id/pipeline_schedules/:pipeline_schedule_id' do - authenticate! authorize! :create_pipeline_schedule, user_project pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) @@ -94,7 +92,6 @@ module API requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do - authenticate! authorize! :create_pipeline_schedule, user_project pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) @@ -115,7 +112,6 @@ module API requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end delete ':id/pipeline_schedules/:pipeline_schedule_id' do - authenticate! authorize! :admin_pipeline_schedule, user_project pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) -- cgit v1.2.1 From e929897873267b0815baf28dfa6b19d49d695554 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 16 May 2017 23:58:22 +0900 Subject: Remove bang from update! --- app/models/ci/pipeline_schedule.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index 623275b7269..adac895ab7d 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -25,7 +25,7 @@ module Ci end def own!(user) - update!(owner: user) + update(owner: user) end def inactive? -- cgit v1.2.1 From 94f7595b9a8edcb4ac8480995a067eb4fa3dec7e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 00:20:11 +0900 Subject: Define last_pipeline in PipelineScheduleEntity --- app/models/ci/pipeline_schedule.rb | 4 ---- lib/api/entities.rb | 4 +++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index adac895ab7d..45d8cd34359 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -44,10 +44,6 @@ module Ci self.next_run_at = Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now) end - def last_pipeline - self.pipelines&.last - end - def schedule_next_run! save! # with set_next_run_at rescue ActiveRecord::RecordInvalid diff --git a/lib/api/entities.rb b/lib/api/entities.rb index b2bc0a17142..93f28e39f82 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -690,7 +690,9 @@ module API expose :id expose :description, :ref, :cron, :cron_timezone, :next_run_at, :active expose :created_at, :updated_at, :deleted_at - expose :last_pipeline, using: Entities::Pipeline + expose :last_pipeline, using: Entities::Pipeline do |pipeline_schedule| + pipeline_schedule.pipelines&.last + end expose :owner, using: Entities::UserBasic end -- cgit v1.2.1 From e828e835e435deab8a62c7c999dca0b774e7a7d6 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 01:33:09 +0900 Subject: avoids N + 1 queries --- lib/api/pipeline_schedules.rb | 2 +- spec/requests/api/pipeline_schedules_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 2319ff639e5..9429306fe20 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -17,7 +17,7 @@ module API get ':id/pipeline_schedules' do authorize! :read_pipeline_schedule, user_project - pipeline_schedules = user_project.pipeline_schedules + pipeline_schedules = user_project.pipeline_schedules.preload(:pipelines) present paginate(pipeline_schedules), with: Entities::PipelineSchedule end diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 6b0310a6a2c..07b078ee6e7 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -25,6 +25,18 @@ describe API::PipelineSchedules do expect(response).to include_pagination_headers expect(response).to match_response_schema('pipeline_schedules') end + + it 'avoids N + 1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + get api("/projects/#{project.id}/pipeline_schedules", developer) + end.count + + create_list(:ci_pipeline_schedule, 10, project: project, owner: developer) + + expect do + get api("/projects/#{project.id}/pipeline_schedules", developer) + end.not_to exceed_query_limit(control_count) + end end context 'authenticated user with invalid permissions' do -- cgit v1.2.1 From c7fc65e0672a3c88e15f033be3b4d4258bc36e2f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 21:16:54 +0900 Subject: zj keen eye --- spec/requests/api/pipeline_schedules_spec.rb | 38 +++++++++------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 07b078ee6e7..7b7bdbc7c74 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -11,11 +11,10 @@ describe API::PipelineSchedules do describe 'GET /projects/:id/pipeline_schedules' do context 'authenticated user with valid permissions' do + let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: developer) } + before do - create(:ci_pipeline_schedule, project: project, owner: developer) - .tap do |pipeline_schedule| - pipeline_schedule.pipelines << build(:ci_pipeline, project: project) - end + pipeline_schedule.pipelines << build(:ci_pipeline, project: project) end it 'returns list of pipeline_schedules' do @@ -57,11 +56,10 @@ describe API::PipelineSchedules do end describe 'GET /projects/:id/pipeline_schedules/:pipeline_schedule_id' do - let(:pipeline_schedule) do - create(:ci_pipeline_schedule, project: project, owner: developer) - .tap do |pipeline_schedule| - pipeline_schedule.pipelines << build(:ci_pipeline, project: project) - end + let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: developer) } + + before do + pipeline_schedule.pipelines << build(:ci_pipeline, project: project) end context 'authenticated user with valid permissions' do @@ -97,11 +95,7 @@ describe API::PipelineSchedules do end describe 'POST /projects/:id/pipeline_schedules' do - let(:params) do - attributes_for(:ci_pipeline_schedule, - description: 'description', ref: 'master', cron: '* * * * *', - cron_timezone: 'UTC', active: true) - end + let(:params) { attributes_for(:ci_pipeline_schedule) } context 'authenticated user with valid permissions' do context 'with required parameters' do @@ -117,7 +111,7 @@ describe API::PipelineSchedules do expect(json_response['ref']).to eq(params[:ref]) expect(json_response['cron']).to eq(params[:cron]) expect(json_response['cron_timezone']).to eq(params[:cron_timezone]) - expect(json_response['active']).to eq(params[:active]) + expect(json_response['owner']['id']).to eq(developer.id) end end @@ -132,7 +126,7 @@ describe API::PipelineSchedules do context 'when cron has validation error' do it 'does not create pipeline_schedule' do post api("/projects/#{project.id}/pipeline_schedules", developer), - params.tap { |_| params['cron'] = 'invalid-cron' } + params.merge('cron' => 'invalid-cron') expect(response).to have_http_status(:bad_request) expect(json_response['message']).to have_key('cron') @@ -211,19 +205,11 @@ describe API::PipelineSchedules do end context 'authenticated user with valid permissions' do - let(:developer2) { create(:user) } - - before do - project.add_developer(developer2) - end - it 'updates owner' do - post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer2) - pipeline_schedule.reload + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer) expect(response).to have_http_status(:ok) expect(response).to match_response_schema('pipeline_schedule') - expect(pipeline_schedule.owner).to eq(developer2) end end @@ -259,7 +245,7 @@ describe API::PipelineSchedules do it 'deletes pipeline_schedule' do expect do delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", master) - end.to change{project.pipeline_schedules.count}.by(-1) + end.to change { project.pipeline_schedules.count }.by(-1) expect(response).to have_http_status(:ok) expect(response).to match_response_schema('pipeline_schedule') -- cgit v1.2.1 From df6040bbd41a639b3d0aa339b2dc0e84d67b811b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 21:26:18 +0900 Subject: zj keen eye2 --- lib/api/pipeline_schedules.rb | 3 +-- spec/requests/api/pipeline_schedules_spec.rb | 7 +------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 9429306fe20..58888417888 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -54,7 +54,7 @@ module API .new(user_project, current_user, declared_params(include_missing: false)) .execute - if pipeline_schedule.valid? + if pipeline_schedule.persisted? present pipeline_schedule, with: Entities::PipelineSchedule else render_validation_error!(pipeline_schedule) @@ -98,7 +98,6 @@ module API return not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.own!(current_user) - status :ok present pipeline_schedule, with: Entities::PipelineSchedule else render_validation_error!(pipeline_schedule) diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 7b7bdbc7c74..34c4fcfcae1 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -160,15 +160,10 @@ describe API::PipelineSchedules do it 'updates cron' do put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer), cron: '1 2 3 4 *' - pipeline_schedule.reload expect(response).to have_http_status(:ok) expect(response).to match_response_schema('pipeline_schedule') expect(json_response['cron']).to eq('1 2 3 4 *') - expect(pipeline_schedule.next_run_at.min).to eq(1) - expect(pipeline_schedule.next_run_at.hour).to eq(2) - expect(pipeline_schedule.next_run_at.day).to eq(3) - expect(pipeline_schedule.next_run_at.month).to eq(4) end context 'when cron has validation error' do @@ -208,7 +203,7 @@ describe API::PipelineSchedules do it 'updates owner' do post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer) - expect(response).to have_http_status(:ok) + expect(response).to have_http_status(:created) expect(response).to match_response_schema('pipeline_schedule') end end -- cgit v1.2.1 From 17b9128b305aefc29fddae3b51d13c61ae7dfef9 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 22:58:23 +0900 Subject: includes last_pipeline --- lib/api/entities.rb | 4 +--- lib/api/pipeline_schedules.rb | 2 +- spec/requests/api/pipeline_schedules_spec.rb | 3 +++ 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 93f28e39f82..b2bc0a17142 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -690,9 +690,7 @@ module API expose :id expose :description, :ref, :cron, :cron_timezone, :next_run_at, :active expose :created_at, :updated_at, :deleted_at - expose :last_pipeline, using: Entities::Pipeline do |pipeline_schedule| - pipeline_schedule.pipelines&.last - end + expose :last_pipeline, using: Entities::Pipeline expose :owner, using: Entities::UserBasic end diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 58888417888..27be796356c 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -17,7 +17,7 @@ module API get ':id/pipeline_schedules' do authorize! :read_pipeline_schedule, user_project - pipeline_schedules = user_project.pipeline_schedules.preload(:pipelines) + pipeline_schedules = user_project.pipeline_schedules.includes(last_pipeline: {statuses: :latest}) present paginate(pipeline_schedules), with: Entities::PipelineSchedule end diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 34c4fcfcae1..1b44043907b 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -31,6 +31,9 @@ describe API::PipelineSchedules do end.count create_list(:ci_pipeline_schedule, 10, project: project, owner: developer) + .each do |pipeline_schedule| + pipeline_schedule.pipelines << build(:ci_pipeline, project: project) + end expect do get api("/projects/#{project.id}/pipeline_schedules", developer) -- cgit v1.2.1 From 20a07d26ff4be9c82271297e516df912109b05aa Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 18 May 2017 17:45:08 +0900 Subject: Include owner for pipeline_schedules. Improve N+1 spec. Use PipelineBasic for small payload. --- lib/api/entities.rb | 2 +- lib/api/pipeline_schedules.rb | 2 +- spec/fixtures/api/schemas/pipeline_schedule.json | 24 +----------------------- spec/requests/api/pipeline_schedules_spec.rb | 6 +++++- 4 files changed, 8 insertions(+), 26 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index b2bc0a17142..f0260cf03f3 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -690,7 +690,7 @@ module API expose :id expose :description, :ref, :cron, :cron_timezone, :next_run_at, :active expose :created_at, :updated_at, :deleted_at - expose :last_pipeline, using: Entities::Pipeline + expose :last_pipeline, using: Entities::PipelineBasic expose :owner, using: Entities::UserBasic end diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 27be796356c..c6c22f6e518 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -17,7 +17,7 @@ module API get ':id/pipeline_schedules' do authorize! :read_pipeline_schedule, user_project - pipeline_schedules = user_project.pipeline_schedules.includes(last_pipeline: {statuses: :latest}) + pipeline_schedules = user_project.pipeline_schedules.includes([:owner, last_pipeline: {statuses: :latest}]) present paginate(pipeline_schedules), with: Entities::PipelineSchedule end diff --git a/spec/fixtures/api/schemas/pipeline_schedule.json b/spec/fixtures/api/schemas/pipeline_schedule.json index 46309b212a1..9d7853d0f78 100644 --- a/spec/fixtures/api/schemas/pipeline_schedule.json +++ b/spec/fixtures/api/schemas/pipeline_schedule.json @@ -17,29 +17,7 @@ "id": { "type": "integer" }, "sha": { "type": "string" }, "ref": { "type": "string" }, - "status": { "type": "string" }, - "before_sha": { "type": ["string", "null"] }, - "tag": { "type": ["boolean", "null"] }, - "yaml_errors": { "type": ["string", "null"] }, - "user": { - "type": ["object", "null"], - "properties": { - "name": { "type": "string" }, - "username": { "type": "string" }, - "id": { "type": "integer" }, - "state": { "type": "string" }, - "avatar_url": { "type": "uri" }, - "web_url": { "type": "uri" } - }, - "additionalProperties": false - }, - "created_at": { "type": "date" }, - "updated_at": { "type": "date" }, - "started_at": { "type": "date" }, - "finished_at": { "type": "date" }, - "committed_at": { "type": ["string", "null"] }, - "duration": { "type": ["integer", "null"] }, - "coverage": { "type": ["string", "null"] } + "status": { "type": "string" } }, "additionalProperties": false }, diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 1b44043907b..74de2f0ba4a 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -30,8 +30,12 @@ describe API::PipelineSchedules do get api("/projects/#{project.id}/pipeline_schedules", developer) end.count - create_list(:ci_pipeline_schedule, 10, project: project, owner: developer) + create_list(:ci_pipeline_schedule, 10, project: project) .each do |pipeline_schedule| + create(:user).tap do |user| + project.add_developer(user) + pipeline_schedule.update_attributes(owner: user) + end pipeline_schedule.pipelines << build(:ci_pipeline, project: project) end -- cgit v1.2.1 From 8c40bbbe7d0d1bb930a590f5ca98570bd32ad1f0 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 18 May 2017 17:58:55 +0900 Subject: Switch to preload. Remove unncecessary associations. --- lib/api/pipeline_schedules.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index c6c22f6e518..3b9d5b0d7a6 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -17,7 +17,7 @@ module API get ':id/pipeline_schedules' do authorize! :read_pipeline_schedule, user_project - pipeline_schedules = user_project.pipeline_schedules.includes([:owner, last_pipeline: {statuses: :latest}]) + pipeline_schedules = user_project.pipeline_schedules.preload([:owner, :last_pipeline]) present paginate(pipeline_schedules), with: Entities::PipelineSchedule end -- cgit v1.2.1 From 92bc1ddaf5a256a5e6636bc4a6f8ebd8673ec719 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 18 May 2017 18:22:41 +0900 Subject: Dryup fetching pipeline_schedule with helper --- lib/api/pipeline_schedules.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 3b9d5b0d7a6..454237d3fbb 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -17,8 +17,6 @@ module API get ':id/pipeline_schedules' do authorize! :read_pipeline_schedule, user_project - pipeline_schedules = user_project.pipeline_schedules.preload([:owner, :last_pipeline]) - present paginate(pipeline_schedules), with: Entities::PipelineSchedule end @@ -31,7 +29,6 @@ module API get ':id/pipeline_schedules/:pipeline_schedule_id' do authorize! :read_pipeline_schedule, user_project - pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) return not_found!('PipelineSchedule') unless pipeline_schedule present pipeline_schedule, with: Entities::PipelineSchedule @@ -75,7 +72,6 @@ module API put ':id/pipeline_schedules/:pipeline_schedule_id' do authorize! :create_pipeline_schedule, user_project - pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) return not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.update(declared_params(include_missing: false)) @@ -94,7 +90,6 @@ module API post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do authorize! :create_pipeline_schedule, user_project - pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) return not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.own!(current_user) @@ -113,11 +108,24 @@ module API delete ':id/pipeline_schedules/:pipeline_schedule_id' do authorize! :admin_pipeline_schedule, user_project - pipeline_schedule = user_project.pipeline_schedules.find(params.delete(:pipeline_schedule_id)) return not_found!('PipelineSchedule') unless pipeline_schedule present pipeline_schedule.destroy, with: Entities::PipelineSchedule end end + + helpers do + def pipeline_schedules + @pipeline_schedules ||= + user_project.pipeline_schedules.preload([:owner, :last_pipeline]) + end + + def pipeline_schedule + @pipeline_schedule ||= + user_project.pipeline_schedules + .preload([:owner, :last_pipeline]) + .find(params.delete(:pipeline_schedule_id)) + end + end end end -- cgit v1.2.1 From f6a8894a5928e1cc37d8301c555fcfd5953cc180 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 19 May 2017 02:42:23 +0900 Subject: Expose last_pipeline only when detailed status --- lib/api/entities.rb | 2 +- lib/api/pipeline_schedules.rb | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index f0260cf03f3..d779cd2a294 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -690,7 +690,7 @@ module API expose :id expose :description, :ref, :cron, :cron_timezone, :next_run_at, :active expose :created_at, :updated_at, :deleted_at - expose :last_pipeline, using: Entities::PipelineBasic + expose :last_pipeline, using: Entities::PipelineBasic, if: { type: :full } expose :owner, using: Entities::UserBasic end diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 454237d3fbb..c013d6f316b 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -31,7 +31,7 @@ module API return not_found!('PipelineSchedule') unless pipeline_schedule - present pipeline_schedule, with: Entities::PipelineSchedule + present pipeline_schedule, with: Entities::PipelineSchedule, type: :full end desc 'Creates a new pipeline schedule' do @@ -52,7 +52,7 @@ module API .execute if pipeline_schedule.persisted? - present pipeline_schedule, with: Entities::PipelineSchedule + present pipeline_schedule, with: Entities::PipelineSchedule, type: :full else render_validation_error!(pipeline_schedule) end @@ -75,7 +75,7 @@ module API return not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.update(declared_params(include_missing: false)) - present pipeline_schedule, with: Entities::PipelineSchedule + present pipeline_schedule, with: Entities::PipelineSchedule, type: :full else render_validation_error!(pipeline_schedule) end @@ -93,7 +93,7 @@ module API return not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.own!(current_user) - present pipeline_schedule, with: Entities::PipelineSchedule + present pipeline_schedule, with: Entities::PipelineSchedule, type: :full else render_validation_error!(pipeline_schedule) end @@ -110,7 +110,7 @@ module API return not_found!('PipelineSchedule') unless pipeline_schedule - present pipeline_schedule.destroy, with: Entities::PipelineSchedule + present pipeline_schedule.destroy, with: Entities::PipelineSchedule, type: :full end end -- cgit v1.2.1 From bfa028e13a41b16b0da2d69359694edbebf9e455 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 19 May 2017 22:24:31 +0900 Subject: Remove deleted_at from Entity. Use find_by. Remove returns. --- lib/api/entities.rb | 2 +- lib/api/pipeline_schedules.rb | 10 +++++----- spec/fixtures/api/schemas/pipeline_schedule.json | 3 +-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d779cd2a294..40ef62fdb14 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -689,7 +689,7 @@ module API class PipelineSchedule < Grape::Entity expose :id expose :description, :ref, :cron, :cron_timezone, :next_run_at, :active - expose :created_at, :updated_at, :deleted_at + expose :created_at, :updated_at expose :last_pipeline, using: Entities::PipelineBasic, if: { type: :full } expose :owner, using: Entities::UserBasic end diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index c013d6f316b..367f6e2fbab 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -29,7 +29,7 @@ module API get ':id/pipeline_schedules/:pipeline_schedule_id' do authorize! :read_pipeline_schedule, user_project - return not_found!('PipelineSchedule') unless pipeline_schedule + not_found!('PipelineSchedule') unless pipeline_schedule present pipeline_schedule, with: Entities::PipelineSchedule, type: :full end @@ -72,7 +72,7 @@ module API put ':id/pipeline_schedules/:pipeline_schedule_id' do authorize! :create_pipeline_schedule, user_project - return not_found!('PipelineSchedule') unless pipeline_schedule + not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.update(declared_params(include_missing: false)) present pipeline_schedule, with: Entities::PipelineSchedule, type: :full @@ -90,7 +90,7 @@ module API post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do authorize! :create_pipeline_schedule, user_project - return not_found!('PipelineSchedule') unless pipeline_schedule + not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.own!(current_user) present pipeline_schedule, with: Entities::PipelineSchedule, type: :full @@ -108,7 +108,7 @@ module API delete ':id/pipeline_schedules/:pipeline_schedule_id' do authorize! :admin_pipeline_schedule, user_project - return not_found!('PipelineSchedule') unless pipeline_schedule + not_found!('PipelineSchedule') unless pipeline_schedule present pipeline_schedule.destroy, with: Entities::PipelineSchedule, type: :full end @@ -124,7 +124,7 @@ module API @pipeline_schedule ||= user_project.pipeline_schedules .preload([:owner, :last_pipeline]) - .find(params.delete(:pipeline_schedule_id)) + .find_by(id: params.delete(:pipeline_schedule_id)) end end end diff --git a/spec/fixtures/api/schemas/pipeline_schedule.json b/spec/fixtures/api/schemas/pipeline_schedule.json index 9d7853d0f78..f6346bd0fb6 100644 --- a/spec/fixtures/api/schemas/pipeline_schedule.json +++ b/spec/fixtures/api/schemas/pipeline_schedule.json @@ -10,7 +10,6 @@ "active": { "type": "boolean" }, "created_at": { "type": "date" }, "updated_at": { "type": "date" }, - "deleted_at": { "type": "date" }, "last_pipeline": { "type": ["object", "null"], "properties": { @@ -36,7 +35,7 @@ }, "required": [ "id", "description", "ref", "cron", "cron_timezone", "next_run_at", - "active", "created_at", "updated_at", "deleted_at", "owner" + "active", "created_at", "updated_at", "owner" ], "additionalProperties": false } -- cgit v1.2.1 From 0b2fd147eca68911342003333dd2d0daa558627b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 19 May 2017 22:48:36 +0900 Subject: Update doc with latest entity --- doc/api/pipeline_schedules.md | 115 ++++++++++++++++++++++++------------------ 1 file changed, 67 insertions(+), 48 deletions(-) diff --git a/doc/api/pipeline_schedules.md b/doc/api/pipeline_schedules.md index 60b968822d8..439d92eff98 100644 --- a/doc/api/pipeline_schedules.md +++ b/doc/api/pipeline_schedules.md @@ -15,22 +15,21 @@ GET /projects/:id/pipeline_schedules | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | ``` -curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules" +curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules" ``` ```json [ { - "id": 11, - "description": "Acceptance Test", + "id": 13, + "description": "Test schedule pipeline", "ref": "master", - "cron": "0 4 * * *", - "cron_timezone": "America/Los_Angeles", - "next_run_at": "2017-05-13T11:00:00.000Z", + "cron": "* * * * *", + "cron_timezone": "Asia/Tokyo", + "next_run_at": "2017-05-19T13:41:00.000Z", "active": true, - "created_at": "2017-05-12T13:10:34.497Z", - "updated_at": "2017-05-12T13:10:34.497Z", - "deleted_at": null, + "created_at": "2017-05-19T13:31:08.849Z", + "updated_at": "2017-05-19T13:40:17.727Z", "owner": { "name": "Administrator", "username": "root", @@ -57,21 +56,26 @@ GET /projects/:id/pipeline_schedules/:pipeline_schedule_id | `pipeline_schedule_id` | integer | yes | The pipeline schedule id | ``` -curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules/11" +curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules/13" ``` ```json { - "id": 11, - "description": "Acceptance Test", + "id": 13, + "description": "Test schedule pipeline", "ref": "master", - "cron": "0 4 * * *", - "cron_timezone": "America/Los_Angeles", - "next_run_at": "2017-05-13T11:00:00.000Z", + "cron": "* * * * *", + "cron_timezone": "Asia/Tokyo", + "next_run_at": "2017-05-19T13:41:00.000Z", "active": true, - "created_at": "2017-05-12T13:10:34.497Z", - "updated_at": "2017-05-12T13:10:34.497Z", - "deleted_at": null, + "created_at": "2017-05-19T13:31:08.849Z", + "updated_at": "2017-05-19T13:40:17.727Z", + "last_pipeline": { + "id": 332, + "sha": "0e788619d0b5ec17388dffb973ecd505946156db", + "ref": "master", + "status": "pending" + }, "owner": { "name": "Administrator", "username": "root", @@ -101,21 +105,21 @@ POST /projects/:id/pipeline_schedules | `active ` | boolean | yes | The activation of pipeline schedule. If false is set, the pipeline schedule will deactivated initially. | ``` -curl --request POST --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form description="Build packages" --form ref="master" --form cron="0 1 * * 5" --form cron_timezone="UTC" --form active="true" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules" +curl --request POST --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form description="Build packages" --form ref="master" --form cron="0 1 * * 5" --form cron_timezone="UTC" --form active="true" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules" ``` ```json { - "id": 12, + "id": 14, "description": "Build packages", "ref": "master", "cron": "0 1 * * 5", "cron_timezone": "UTC", - "next_run_at": "2017-05-19T01:00:00.000Z", + "next_run_at": "2017-05-26T01:00:00.000Z", "active": true, - "created_at": "2017-05-12T13:18:58.879Z", - "updated_at": "2017-05-12T13:18:58.879Z", - "deleted_at": null, + "created_at": "2017-05-19T13:43:08.169Z", + "updated_at": "2017-05-19T13:43:08.169Z", + "last_pipeline": null, "owner": { "name": "Administrator", "username": "root", @@ -146,21 +150,26 @@ PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id | `active ` | boolean | no | The activation of pipeline schedule. If false is set, the pipeline schedule will deactivated initially. | ``` -curl --request PUT --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form cron="0 2 * * *" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules/11" +curl --request PUT --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form cron="0 2 * * *" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules/13" ``` ```json { - "id": 11, - "description": "Acceptance Test", + "id": 13, + "description": "Test schedule pipeline", "ref": "master", "cron": "0 2 * * *", - "cron_timezone": "America/Los_Angeles", - "next_run_at": "2017-05-13T09:00:00.000Z", + "cron_timezone": "Asia/Tokyo", + "next_run_at": "2017-05-19T17:00:00.000Z", "active": true, - "created_at": "2017-05-12T13:10:34.497Z", - "updated_at": "2017-05-12T13:22:07.798Z", - "deleted_at": null, + "created_at": "2017-05-19T13:31:08.849Z", + "updated_at": "2017-05-19T13:44:16.135Z", + "last_pipeline": { + "id": 332, + "sha": "0e788619d0b5ec17388dffb973ecd505946156db", + "ref": "master", + "status": "pending" + }, "owner": { "name": "Administrator", "username": "root", @@ -186,21 +195,26 @@ POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/take_ownership | `pipeline_schedule_id` | integer | yes | The pipeline schedule id | ``` -curl --request POST --header "PRIVATE-TOKEN: hf2CvZXB9w8Uc5pZKpSB" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules/11/take_ownership" +curl --request POST --header "PRIVATE-TOKEN: hf2CvZXB9w8Uc5pZKpSB" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules/13/take_ownership" ``` ```json { - "id": 11, - "description": "Acceptance Test", + "id": 13, + "description": "Test schedule pipeline", "ref": "master", "cron": "0 2 * * *", - "cron_timezone": "America/Los_Angeles", - "next_run_at": "2017-05-13T09:00:00.000Z", + "cron_timezone": "Asia/Tokyo", + "next_run_at": "2017-05-19T17:00:00.000Z", "active": true, - "created_at": "2017-05-12T13:10:34.497Z", - "updated_at": "2017-05-12T13:26:12.191Z", - "deleted_at": null, + "created_at": "2017-05-19T13:31:08.849Z", + "updated_at": "2017-05-19T13:46:37.468Z", + "last_pipeline": { + "id": 332, + "sha": "0e788619d0b5ec17388dffb973ecd505946156db", + "ref": "master", + "status": "pending" + }, "owner": { "name": "shinya", "username": "maeda", @@ -226,21 +240,26 @@ DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id | `pipeline_schedule_id` | integer | yes | The pipeline schedule id | ``` -curl --request DELETE --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/28/pipeline_schedules/11" +curl --request DELETE --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules/13" ``` ```json { - "id": 11, - "description": "Acceptance Test", + "id": 13, + "description": "Test schedule pipeline", "ref": "master", "cron": "0 2 * * *", - "cron_timezone": "America/Los_Angeles", - "next_run_at": "2017-05-13T09:00:00.000Z", + "cron_timezone": "Asia/Tokyo", + "next_run_at": "2017-05-19T17:00:00.000Z", "active": true, - "created_at": "2017-05-12T13:10:34.497Z", - "updated_at": "2017-05-12T13:26:12.191Z", - "deleted_at": "2017-05-12T13:27:38.529Z", + "created_at": "2017-05-19T13:31:08.849Z", + "updated_at": "2017-05-19T13:46:37.468Z", + "last_pipeline": { + "id": 332, + "sha": "0e788619d0b5ec17388dffb973ecd505946156db", + "ref": "master", + "status": "pending" + }, "owner": { "name": "shinya", "username": "maeda", -- cgit v1.2.1 From 20c26eaf9371dcc89cd3a403f79882e6c2dbf6ec Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 22 May 2017 15:52:08 +0900 Subject: FIx doc lint --- doc/api/pipeline_schedules.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/pipeline_schedules.md b/doc/api/pipeline_schedules.md index 439d92eff98..a8b5f5228f1 100644 --- a/doc/api/pipeline_schedules.md +++ b/doc/api/pipeline_schedules.md @@ -1,6 +1,6 @@ # Pipeline schedules -You can read more about [pipeline schedules](../ci/pipeline_schedules.md). +You can read more about [pipeline schedules](../user/project/pipelines/schedules.md). ## List Pipeline schedules -- cgit v1.2.1 From c91292b61f80626b91d41cc17d0a662f302d6ecd Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 24 May 2017 19:25:13 +0900 Subject: Improve document --- doc/api/pipeline_schedules.md | 56 +++++++++++++++++++++---------------------- lib/api/pipeline_schedules.rb | 8 +++---- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/doc/api/pipeline_schedules.md b/doc/api/pipeline_schedules.md index a8b5f5228f1..73b4f345a62 100644 --- a/doc/api/pipeline_schedules.md +++ b/doc/api/pipeline_schedules.md @@ -2,9 +2,9 @@ You can read more about [pipeline schedules](../user/project/pipelines/schedules.md). -## List Pipeline schedules +## Get all pipeline schedules -Get a list of pipeline schedules. +Get a list of the pipeline schedules of a project. ``` GET /projects/:id/pipeline_schedules @@ -14,8 +14,8 @@ GET /projects/:id/pipeline_schedules |-----------|---------|----------|---------------------| | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | -``` -curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules" +```sh +curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules" ``` ```json @@ -36,15 +36,15 @@ curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/ap "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://192.168.10.5:3000/root" + "web_url": "https://gitlab.example.com/root" } } ] ``` -## Single pipeline schedule +## Get a single pipeline schedule -Get a single pipeline schedule. +Get the pipeline schedule of a project. ``` GET /projects/:id/pipeline_schedules/:pipeline_schedule_id @@ -55,8 +55,8 @@ GET /projects/:id/pipeline_schedules/:pipeline_schedule_id | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | | `pipeline_schedule_id` | integer | yes | The pipeline schedule id | -``` -curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules/13" +```sh +curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules/13" ``` ```json @@ -82,14 +82,14 @@ curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/ap "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://192.168.10.5:3000/root" + "web_url": "https://gitlab.example.com/root" } } ``` -## New pipeline schedule +## Create a new pipeline schedule -Creates a new pipeline schedule. +Create a new pipeline schedule of a project. ``` POST /projects/:id/pipeline_schedules @@ -104,8 +104,8 @@ POST /projects/:id/pipeline_schedules | `cron_timezone ` | string | yes | The timezone supproted by `ActiveSupport::TimeZone` (e.g. `Pacific Time (US & Canada)`) or `TZInfo::Timezone` (e.g. `America/Los_Angeles`) | | `active ` | boolean | yes | The activation of pipeline schedule. If false is set, the pipeline schedule will deactivated initially. | -``` -curl --request POST --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form description="Build packages" --form ref="master" --form cron="0 1 * * 5" --form cron_timezone="UTC" --form active="true" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules" +```sh +curl --request POST --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form description="Build packages" --form ref="master" --form cron="0 1 * * 5" --form cron_timezone="UTC" --form active="true" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules" ``` ```json @@ -126,14 +126,14 @@ curl --request POST --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form descri "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://192.168.10.5:3000/root" + "web_url": "https://gitlab.example.com/root" } } ``` -## Edit pipeline schedule +## Edit a pipeline schedule -Updates an existing pipeline schedule. Once the update is done, it will be rescheduled automatically. +Updates the pipeline schedule of a project. Once the update is done, it will be rescheduled automatically. ``` PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id @@ -149,8 +149,8 @@ PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id | `cron_timezone ` | string | no | The timezone supproted by `ActiveSupport::TimeZone` (e.g. `Pacific Time (US & Canada)`) or `TZInfo::Timezone` (e.g. `America/Los_Angeles`) | | `active ` | boolean | no | The activation of pipeline schedule. If false is set, the pipeline schedule will deactivated initially. | -``` -curl --request PUT --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form cron="0 2 * * *" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules/13" +```sh +curl --request PUT --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form cron="0 2 * * *" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules/13" ``` ```json @@ -176,14 +176,14 @@ curl --request PUT --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form cron="0 "id": 1, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://192.168.10.5:3000/root" + "web_url": "https://gitlab.example.com/root" } } ``` ## Take ownership of a pipeline schedule -Update an owner of a pipeline schedule. +Update the owner of the pipeline schedule of a project. ``` POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/take_ownership @@ -194,8 +194,8 @@ POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/take_ownership | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | | `pipeline_schedule_id` | integer | yes | The pipeline schedule id | -``` -curl --request POST --header "PRIVATE-TOKEN: hf2CvZXB9w8Uc5pZKpSB" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules/13/take_ownership" +```sh +curl --request POST --header "PRIVATE-TOKEN: hf2CvZXB9w8Uc5pZKpSB" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules/13/take_ownership" ``` ```json @@ -221,14 +221,14 @@ curl --request POST --header "PRIVATE-TOKEN: hf2CvZXB9w8Uc5pZKpSB" "http://192.1 "id": 50, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/8ca0a796a679c292e3a11da50f99e801?s=80&d=identicon", - "web_url": "http://192.168.10.5:3000/maeda" + "web_url": "https://gitlab.example.com/maeda" } } ``` ## Delete a pipeline schedule -Delete a pipeline schedule. +Delete the pipeline schedule of a project. ``` DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id @@ -239,8 +239,8 @@ DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | | `pipeline_schedule_id` | integer | yes | The pipeline schedule id | -``` -curl --request DELETE --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192.168.10.5:3000/api/v4/projects/29/pipeline_schedules/13" +```sh +curl --request DELETE --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules/13" ``` ```json @@ -266,7 +266,7 @@ curl --request DELETE --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "http://192 "id": 50, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/8ca0a796a679c292e3a11da50f99e801?s=80&d=identicon", - "web_url": "http://192.168.10.5:3000/maeda" + "web_url": "https://gitlab.example.com/maeda" } } ``` diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 367f6e2fbab..4b7dd487186 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -8,7 +8,7 @@ module API requires :id, type: String, desc: 'The ID of a project' end resource :projects, requirements: { id: %r{[^/]+} } do - desc 'Get a list of pipeline schedules' do + desc 'Get all pipeline schedules' do success Entities::PipelineSchedule end params do @@ -34,7 +34,7 @@ module API present pipeline_schedule, with: Entities::PipelineSchedule, type: :full end - desc 'Creates a new pipeline schedule' do + desc 'Create a new pipeline schedule' do success Entities::PipelineSchedule end params do @@ -58,7 +58,7 @@ module API end end - desc 'Updates an existing pipeline schedule' do + desc 'Edit a pipeline schedule' do success Entities::PipelineSchedule end params do @@ -81,7 +81,7 @@ module API end end - desc 'Update an owner of a pipeline schedule' do + desc 'Take ownership of a pipeline schedule' do success Entities::PipelineSchedule end params do -- cgit v1.2.1 From b17c8d67d8811e0a440338dc25464d8c90e81179 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 27 May 2017 21:29:01 +0900 Subject: Use PipelineScheduleDetails --- lib/api/entities.rb | 5 ++++- lib/api/pipeline_schedules.rb | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 40ef62fdb14..e10bd230ae2 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -690,10 +690,13 @@ module API expose :id expose :description, :ref, :cron, :cron_timezone, :next_run_at, :active expose :created_at, :updated_at - expose :last_pipeline, using: Entities::PipelineBasic, if: { type: :full } expose :owner, using: Entities::UserBasic end + class PipelineScheduleDetails < PipelineSchedule + expose :last_pipeline, using: Entities::PipelineBasic + end + class EnvironmentBasic < Grape::Entity expose :id, :name, :slug, :external_url end diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 4b7dd487186..5269239b7ed 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -21,7 +21,7 @@ module API end desc 'Get a single pipeline schedule' do - success Entities::PipelineSchedule + success Entities::PipelineScheduleDetails end params do requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' @@ -31,11 +31,11 @@ module API not_found!('PipelineSchedule') unless pipeline_schedule - present pipeline_schedule, with: Entities::PipelineSchedule, type: :full + present pipeline_schedule, with: Entities::PipelineScheduleDetails end desc 'Create a new pipeline schedule' do - success Entities::PipelineSchedule + success Entities::PipelineScheduleDetails end params do requires :description, type: String, desc: 'The description of pipeline schedule' @@ -52,14 +52,14 @@ module API .execute if pipeline_schedule.persisted? - present pipeline_schedule, with: Entities::PipelineSchedule, type: :full + present pipeline_schedule, with: Entities::PipelineScheduleDetails else render_validation_error!(pipeline_schedule) end end desc 'Edit a pipeline schedule' do - success Entities::PipelineSchedule + success Entities::PipelineScheduleDetails end params do requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' @@ -75,14 +75,14 @@ module API not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.update(declared_params(include_missing: false)) - present pipeline_schedule, with: Entities::PipelineSchedule, type: :full + present pipeline_schedule, with: Entities::PipelineScheduleDetails else render_validation_error!(pipeline_schedule) end end desc 'Take ownership of a pipeline schedule' do - success Entities::PipelineSchedule + success Entities::PipelineScheduleDetails end params do requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' @@ -93,14 +93,14 @@ module API not_found!('PipelineSchedule') unless pipeline_schedule if pipeline_schedule.own!(current_user) - present pipeline_schedule, with: Entities::PipelineSchedule, type: :full + present pipeline_schedule, with: Entities::PipelineScheduleDetails else render_validation_error!(pipeline_schedule) end end desc 'Delete a pipeline schedule' do - success Entities::PipelineSchedule + success Entities::PipelineScheduleDetails end params do requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' @@ -110,7 +110,7 @@ module API not_found!('PipelineSchedule') unless pipeline_schedule - present pipeline_schedule.destroy, with: Entities::PipelineSchedule, type: :full + present pipeline_schedule.destroy, with: Entities::PipelineScheduleDetails end end -- cgit v1.2.1 From 63ca126e977666335d7e5f70665815d1289a6f34 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 27 May 2017 22:22:33 +0900 Subject: Improve API with optinal and default. Allow to use scope as a parameter. --- lib/api/pipeline_schedules.rb | 10 +++++++--- spec/requests/api/pipeline_schedules_spec.rb | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 5269239b7ed..52ad682b972 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -13,11 +13,15 @@ module API end params do use :pagination + optional :scope, type: String, values: %w[active inactive], + desc: 'The scope of pipeline schedules' end get ':id/pipeline_schedules' do authorize! :read_pipeline_schedule, user_project - present paginate(pipeline_schedules), with: Entities::PipelineSchedule + schedules = PipelineSchedulesFinder.new(user_project).execute(scope: params[:scope]) + .preload([:owner, :last_pipeline]) + present paginate(schedules), with: Entities::PipelineSchedule end desc 'Get a single pipeline schedule' do @@ -41,8 +45,8 @@ module API requires :description, type: String, desc: 'The description of pipeline schedule' requires :ref, type: String, desc: 'The branch/tag name will be triggered' requires :cron, type: String, desc: 'The cron' - requires :cron_timezone, type: String, desc: 'The timezone' - requires :active, type: Boolean, desc: 'The activation of pipeline schedule' + optional :cron_timezone, type: String, default: 'UTC', desc: 'The timezone' + optional :active, type: Boolean, default: true, desc: 'The activation of pipeline schedule' end post ':id/pipeline_schedules' do authorize! :create_pipeline_schedule, user_project diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 74de2f0ba4a..77bf377884d 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -43,6 +43,24 @@ describe API::PipelineSchedules do get api("/projects/#{project.id}/pipeline_schedules", developer) end.not_to exceed_query_limit(control_count) end + + %w[active inactive].each do |target| + context "when scope is #{target}" do + before do + create(:ci_pipeline_schedule, project: project, active: active?(target)) + end + + it 'returns matched pipeline schedules' do + get api("/projects/#{project.id}/pipeline_schedules", developer), scope: target + + expect(json_response.map{ |r| r['active'] }).to all(eq(active?(target))) + end + end + + def active?(str) + (str == 'active') ? true : false + end + end end context 'authenticated user with invalid permissions' do -- cgit v1.2.1 From 3b61451c46f462b392783b282fe63dbddd8b6c2e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 27 May 2017 22:31:22 +0900 Subject: Return 202 for destory. Remove []. Remove def pipeline_schedules from helper. --- lib/api/pipeline_schedules.rb | 8 ++------ spec/requests/api/pipeline_schedules_spec.rb | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index 52ad682b972..d9509375698 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -114,20 +114,16 @@ module API not_found!('PipelineSchedule') unless pipeline_schedule + status :accepted present pipeline_schedule.destroy, with: Entities::PipelineScheduleDetails end end helpers do - def pipeline_schedules - @pipeline_schedules ||= - user_project.pipeline_schedules.preload([:owner, :last_pipeline]) - end - def pipeline_schedule @pipeline_schedule ||= user_project.pipeline_schedules - .preload([:owner, :last_pipeline]) + .preload(:owner, :last_pipeline) .find_by(id: params.delete(:pipeline_schedule_id)) end end diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 77bf377884d..85d11deb26f 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -267,7 +267,7 @@ describe API::PipelineSchedules do delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", master) end.to change { project.pipeline_schedules.count }.by(-1) - expect(response).to have_http_status(:ok) + expect(response).to have_http_status(:accepted) expect(response).to match_response_schema('pipeline_schedule') end -- cgit v1.2.1 From 0e0e278196801cb9b42791ad4ef9707ad52c4896 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 29 May 2017 15:03:07 +0900 Subject: Fix document according to the new change --- doc/api/pipeline_schedules.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/api/pipeline_schedules.md b/doc/api/pipeline_schedules.md index 73b4f345a62..433654c18cc 100644 --- a/doc/api/pipeline_schedules.md +++ b/doc/api/pipeline_schedules.md @@ -13,6 +13,7 @@ GET /projects/:id/pipeline_schedules | 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 | +| `scope` | string | no | The scope of pipeline schedules, one of: `active`, `inactive` | ```sh curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules" @@ -101,8 +102,8 @@ POST /projects/:id/pipeline_schedules | `description` | string | yes | The description of pipeline schedule | | `ref` | string | yes | The branch/tag name will be triggered | | `cron ` | string | yes | The cron (e.g. `0 1 * * *`) ([Cron syntax](https://en.wikipedia.org/wiki/Cron)) | -| `cron_timezone ` | string | yes | The timezone supproted by `ActiveSupport::TimeZone` (e.g. `Pacific Time (US & Canada)`) or `TZInfo::Timezone` (e.g. `America/Los_Angeles`) | -| `active ` | boolean | yes | The activation of pipeline schedule. If false is set, the pipeline schedule will deactivated initially. | +| `cron_timezone ` | string | no | The timezone supproted by `ActiveSupport::TimeZone` (e.g. `Pacific Time (US & Canada)`) (default: `'UTC'`) | +| `active ` | boolean | no | The activation of pipeline schedule. If false is set, the pipeline schedule will deactivated initially (default: `true`) | ```sh curl --request POST --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form description="Build packages" --form ref="master" --form cron="0 1 * * 5" --form cron_timezone="UTC" --form active="true" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules" -- cgit v1.2.1 From 2da420be04ae1fa00b06149c0293a0349a085e99 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 31 May 2017 00:06:37 +0900 Subject: Use update_pipeline_schedule --- lib/api/pipeline_schedules.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index d9509375698..93d89209934 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -74,7 +74,7 @@ module API optional :active, type: Boolean, desc: 'The activation of pipeline schedule' end put ':id/pipeline_schedules/:pipeline_schedule_id' do - authorize! :create_pipeline_schedule, user_project + authorize! :update_pipeline_schedule, user_project not_found!('PipelineSchedule') unless pipeline_schedule @@ -92,7 +92,7 @@ module API requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' end post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do - authorize! :create_pipeline_schedule, user_project + authorize! :update_pipeline_schedule, user_project not_found!('PipelineSchedule') unless pipeline_schedule -- cgit v1.2.1 From 98043aeedf1fe3241d8362d8036f28b709e6d468 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 22 May 2017 16:42:10 +0200 Subject: Copy `filter_projects` helper to V3 The helper will be modified in V4, so copy the original to V4 to keep the current behavior in V3. --- lib/api/v3/helpers.rb | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/api/v3/helpers.rb b/lib/api/v3/helpers.rb index 0f234d4cdad..06af286ef50 100644 --- a/lib/api/v3/helpers.rb +++ b/lib/api/v3/helpers.rb @@ -14,6 +14,33 @@ module API authorize! access_level, merge_request merge_request end + + # project helpers + + def filter_projects(projects) + if params[:membership] + projects = projects.merge(current_user.authorized_projects) + end + + if params[:owned] + projects = projects.merge(current_user.owned_projects) + end + + if params[:starred] + projects = projects.merge(current_user.starred_projects) + end + + if params[:search].present? + projects = projects.search(params[:search]) + end + + if params[:visibility].present? + projects = projects.search_by_visibility(params[:visibility]) + end + + projects = projects.where(archived: params[:archived]) + projects.reorder(params[:order_by] => params[:sort]) + end end end end -- cgit v1.2.1 From 44fdf0a1e314da517d189e356b0e44bb63cf0f4b Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 22 May 2017 16:47:16 +0200 Subject: Move ProjectsFinder to `present_projects` for simplification To avoid passing parameters double, move all filtering to the `present_projects` helper. --- lib/api/projects.rb | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/api/projects.rb b/lib/api/projects.rb index d4fe5c023bf..fce496308c3 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -67,20 +67,18 @@ module API optional :import_url, type: String, desc: 'URL from which the project is imported' end - def present_projects(projects, options = {}) + def present_projects(options = {}) options = options.reverse_merge( - with: Entities::Project, - current_user: current_user, - simple: params[:simple], - with_issues_enabled: params[:with_issues_enabled], - with_merge_requests_enabled: params[:with_merge_requests_enabled] + with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, + current_user: current_user ) + projects = ProjectsFinder.new(current_user: current_user).execute projects = filter_projects(projects) - projects = projects.with_statistics if options[:statistics] - projects = projects.with_issues_enabled if options[:with_issues_enabled] - projects = projects.with_merge_requests_enabled if options[:with_merge_requests_enabled] - options[:with] = Entities::BasicProjectDetails if options[:simple] + projects = projects.with_statistics if params[:statistics] + projects = projects.with_issues_enabled if params[:with_issues_enabled] + projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] + options[:with] = Entities::BasicProjectDetails if params[:simple] present paginate(projects), options end @@ -94,8 +92,7 @@ module API use :statistics_params end get do - entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails - present_projects ProjectsFinder.new(current_user: current_user).execute, with: entity, statistics: params[:statistics] + present_projects end desc 'Create new project' do -- cgit v1.2.1 From 4fda13b68eb7da27be5e74cac6d3d537502b19f7 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 22 May 2017 16:48:38 +0200 Subject: Build options hash after finding the list of projects Because this order makes more sense and makes the code easier to read. --- lib/api/projects.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/api/projects.rb b/lib/api/projects.rb index fce496308c3..ef09dfa0102 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -68,16 +68,17 @@ module API end def present_projects(options = {}) - options = options.reverse_merge( - with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, - current_user: current_user - ) - projects = ProjectsFinder.new(current_user: current_user).execute projects = filter_projects(projects) projects = projects.with_statistics if params[:statistics] projects = projects.with_issues_enabled if params[:with_issues_enabled] projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] + + options = options.reverse_merge( + with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, + statistics: params[:statistics], + current_user: current_user + ) options[:with] = Entities::BasicProjectDetails if params[:simple] present paginate(projects), options -- cgit v1.2.1 From 07fc79e7c53a4fa7c4dd33835b905dfa8a609ff8 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 23 May 2017 15:48:13 +0200 Subject: Handle `membership` in ProjectFinder The ProjectFinder supports the `non_public` parameter. This can be used to find only projects the user is member of. --- lib/api/helpers.rb | 4 ---- lib/api/projects.rb | 5 ++++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 226a7ddd50e..0dc33eb2097 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -257,10 +257,6 @@ module API # project helpers def filter_projects(projects) - if params[:membership] - projects = projects.merge(current_user.authorized_projects) - end - if params[:owned] projects = projects.merge(current_user.owned_projects) end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index ef09dfa0102..bb03480f1ee 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -68,7 +68,10 @@ module API end def present_projects(options = {}) - projects = ProjectsFinder.new(current_user: current_user).execute + finder_params = {} + finder_params[:non_public] = true if params[:membership].present? + + projects = ProjectsFinder.new(current_user: current_user, params: finder_params).execute projects = filter_projects(projects) projects = projects.with_statistics if params[:statistics] projects = projects.with_issues_enabled if params[:with_issues_enabled] -- cgit v1.2.1 From 8e72ad70bd2479ae5a465eac1df74f99f03ea731 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 23 May 2017 22:40:07 +0200 Subject: Add starred_by scope to Project Add a scope to search for the projects that are starred by a certain user. --- app/models/project.rb | 3 ++- app/models/user.rb | 2 +- spec/models/project_spec.rb | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index a59095cb25c..963fd594d46 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -242,6 +242,7 @@ class Project < ActiveRecord::Base scope :in_namespace, ->(namespace_ids) { where(namespace_id: namespace_ids) } scope :personal, ->(user) { where(namespace_id: user.namespace_id) } scope :joined, ->(user) { where('namespace_id != ?', user.namespace_id) } + scope :starred_by, ->(user) { joins(:users_star_projects).where('users_star_projects.user_id': user.id) } scope :visible_to_user, ->(user) { where(id: user.authorized_projects.select(:id).reorder(nil)) } scope :non_archived, -> { where(archived: false) } scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct } @@ -350,7 +351,7 @@ class Project < ActiveRecord::Base where("projects.id IN (#{union.to_sql})") end - def search_by_visibility(level) + def search_by_visibility(level) # DEPRECATED: remove with API V3 where(visibility_level: Gitlab::VisibilityLevel.string_options[level]) end diff --git a/app/models/user.rb b/app/models/user.rb index 3f816a250c2..20894ce269a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -557,7 +557,7 @@ class User < ActiveRecord::Base authorized_projects(Gitlab::Access::REPORTER).where(id: projects) end - def viewable_starred_projects + def viewable_starred_projects # DEPRECATED: Use ProjectFinder instead. Remove together with API V3 starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (?)", [Project::PUBLIC, Project::INTERNAL], authorized_projects.select(:project_id)) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2ef3d5654d5..da1b29a2bda 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -948,6 +948,20 @@ describe Project, models: true do end end + describe '.starred_by' do + it 'returns only projects starred by the given user' do + user1 = create(:user) + user2 = create(:user) + project1 = create(:empty_project) + project2 = create(:empty_project) + create(:empty_project) + user1.toggle_star(project1) + user2.toggle_star(project2) + + expect(Project.starred_by(user1)).to contain_exactly(project1) + end + end + describe '.visible_to_user' do let!(:project) { create(:empty_project, :private) } let!(:user) { create(:user) } -- cgit v1.2.1 From 0725050802dd30d4c235b6a2d28dd494d2d7429b Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 23 May 2017 22:38:12 +0200 Subject: Change ProjectFinder so starred can be combined with other filters The `starred` parameter couldn't be used in combination with `trending` or `non_public`. But this is changed now. --- app/finders/projects_finder.rb | 7 +++++-- spec/finders/projects_finder_spec.rb | 8 +++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index f6d8226bf3f..588406982d7 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -31,6 +31,7 @@ class ProjectsFinder < UnionFinder items = by_ids(items) items = union(items) items = by_personal(items) + items = by_starred(items) items = by_visibilty_level(items) items = by_tags(items) items = by_search(items) @@ -45,8 +46,6 @@ class ProjectsFinder < UnionFinder if params[:trending].present? projects << Project.trending - elsif params[:starred].present? && current_user - projects << current_user.viewable_starred_projects else projects << current_user.authorized_projects if current_user projects << Project.unscoped.public_to_user(current_user) unless params[:non_public].present? @@ -67,6 +66,10 @@ class ProjectsFinder < UnionFinder (params[:personal].present? && current_user) ? items.personal(current_user) : items end + def by_starred(items) + (params[:starred].present? && current_user) ? items.starred_by(current_user) : items + end + def by_visibilty_level(items) params[:visibility_level].present? ? items.where(visibility_level: params[:visibility_level]) : items end diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 148adcffe3b..077f2624388 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -146,13 +146,19 @@ describe ProjectsFinder do it { is_expected.to eq([private_project]) } end - describe 'filter by viewable_starred_projects' do + describe 'filter by starred' do let(:params) { { starred: true } } before do current_user.toggle_star(public_project) end it { is_expected.to eq([public_project]) } + + it 'returns only projects the user has access to' do + current_user.toggle_star(private_project) + + is_expected.to eq([public_project]) + end end describe 'sorting' do -- cgit v1.2.1 From a1deed629e03d8db47deb1bcf795ae8abaf2c847 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 23 May 2017 22:40:39 +0200 Subject: Use ProjectFinder to filter the projects Instead of trying to do the heavy lifting in the API itself, use the existing features of the ProjectFinder. --- lib/api/helpers.rb | 13 ------------- lib/api/projects.rb | 4 ++++ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 0dc33eb2097..2855bd7385d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -261,19 +261,6 @@ module API projects = projects.merge(current_user.owned_projects) end - if params[:starred] - projects = projects.merge(current_user.starred_projects) - end - - if params[:search].present? - projects = projects.search(params[:search]) - end - - if params[:visibility].present? - projects = projects.search_by_visibility(params[:visibility]) - end - - projects = projects.where(archived: params[:archived]) projects.reorder(params[:order_by] => params[:sort]) end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index bb03480f1ee..7610e3cbacc 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -70,6 +70,10 @@ module API def present_projects(options = {}) finder_params = {} finder_params[:non_public] = true if params[:membership].present? + finder_params[:starred] = true if params[:starred].present? + finder_params[:visibility_level] = Gitlab::VisibilityLevel.level_value(params[:visibility]) if params[:visibility] + finder_params[:archived] = params[:archived] + finder_params[:search] = params[:search] if params[:search] projects = ProjectsFinder.new(current_user: current_user, params: finder_params).execute projects = filter_projects(projects) -- cgit v1.2.1 From 01c6323d238706bc8d0acb0273552a094c996ca0 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Wed, 24 May 2017 15:03:45 +0200 Subject: UNION of SELECT/WHERE is faster than WHERE on UNION Instead of applying WHERE on a UNION, apply the WHERE on each of the seperate SELECT statements, and do UNION on that. Local tests with about 2_000_000 projects: - 1_500_000 private projects - 40_000 internal projects - 400_000 public projects For the API endpoint `/api/v4/projects?visibility=private` the slowest query was: ```sql SELECT "projects".* FROM "projects" WHERE ... ``` The original query took 1073.8ms. The query refactored to UNION of SELECT/WHERE took 2.3ms. The original query was: ```sql SELECT "projects".* FROM "projects" WHERE "projects"."pending_delete" = $1 AND (projects.id IN (SELECT "projects"."id" FROM "projects" INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id" WHERE "projects"."pending_delete" = 'f' AND "project_authorizations"."user_id" = 23 UNION SELECT "projects"."id" FROM "projects" WHERE "projects"."visibility_level" IN (20, 10))) AND "projects"."visibility_level" = $2 AND "projects"."archived" = $3 ORDER BY "projects"."created_at" DESC LIMIT 20 OFFSET 0 [["pending_delete", "f"], ["visibility_level", 0], ["archived", "f"]] ``` The refactored query: ```sql SELECT "projects".* FROM "projects" WHERE "projects"."pending_delete" = $1 AND (projects.id IN (SELECT "projects"."id" FROM "projects" INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id" WHERE "projects"."pending_delete" = 'f' AND "project_authorizations"."user_id" = 23 AND "projects"."visibility_level" = 0 AND "projects"."archived" = 'f' UNION SELECT "projects"."id" FROM "projects" WHERE "projects"."visibility_level" IN (20, 10) AND "projects"."visibility_level" = 0 AND "projects"."archived" = 'f')) ORDER BY "projects"."created_at" DESC LIMIT 20 OFFSET 0 [["pending_delete", "f"]] ``` --- app/finders/projects_finder.rb | 18 ++++++++++-------- changelogs/unreleased/tc-improve-project-api-perf.yml | 4 ++++ 2 files changed, 14 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/tc-improve-project-api-perf.yml diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 588406982d7..1d365e7cd7d 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -28,14 +28,16 @@ class ProjectsFinder < UnionFinder def execute items = init_collection - items = by_ids(items) + items = items.map do |item| + item = by_ids(item) + item = by_personal(item) + item = by_starred(item) + item = by_visibilty_level(item) + item = by_tags(item) + item = by_search(item) + by_archived(item) + end items = union(items) - items = by_personal(items) - items = by_starred(items) - items = by_visibilty_level(items) - items = by_tags(items) - items = by_search(items) - items = by_archived(items) sort(items) end @@ -55,7 +57,7 @@ class ProjectsFinder < UnionFinder end def by_ids(items) - project_ids_relation ? items.map { |item| item.where(id: project_ids_relation) } : items + project_ids_relation ? items.where(id: project_ids_relation) : items end def union(items) diff --git a/changelogs/unreleased/tc-improve-project-api-perf.yml b/changelogs/unreleased/tc-improve-project-api-perf.yml new file mode 100644 index 00000000000..7e88466c058 --- /dev/null +++ b/changelogs/unreleased/tc-improve-project-api-perf.yml @@ -0,0 +1,4 @@ +--- +title: Improve performance of ProjectFinder used in /projects API endpoint +merge_request: 11666 +author: -- cgit v1.2.1 From 0f0b9a8466747f69e210fc27778f96ab8ef628bc Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Wed, 24 May 2017 22:12:40 +0200 Subject: Use helper to construct Finder params The ProjectsFinder and GroupFinder both support the same set of params. And the `/api/v4/projects` and `/api/v4/group/:id/projects` also support the same set of params. But they do not match the Finder params. So use a helper method to transform them. --- lib/api/groups.rb | 2 +- lib/api/helpers.rb | 10 ++++++++++ lib/api/projects.rb | 9 +-------- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/api/groups.rb b/lib/api/groups.rb index ee85b777aff..aacc3356a0e 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -151,7 +151,7 @@ module API end get ":id/projects" do group = find_group!(params[:id]) - projects = GroupProjectsFinder.new(group: group, current_user: current_user).execute + projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute projects = filter_projects(projects) entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project present paginate(projects), with: entity, current_user: current_user diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2855bd7385d..17f57cfb8d7 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -264,6 +264,16 @@ module API projects.reorder(params[:order_by] => params[:sort]) end + def project_finder_params + finder_params = {} + finder_params[:non_public] = true if params[:membership].present? + finder_params[:starred] = true if params[:starred].present? + finder_params[:visibility_level] = Gitlab::VisibilityLevel.level_value(params[:visibility]) if params[:visibility] + finder_params[:archived] = params[:archived] + finder_params[:search] = params[:search] if params[:search] + finder_params + end + # file helpers def uploaded_file(field, uploads_path) diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 7610e3cbacc..267dd2a74d7 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -68,14 +68,7 @@ module API end def present_projects(options = {}) - finder_params = {} - finder_params[:non_public] = true if params[:membership].present? - finder_params[:starred] = true if params[:starred].present? - finder_params[:visibility_level] = Gitlab::VisibilityLevel.level_value(params[:visibility]) if params[:visibility] - finder_params[:archived] = params[:archived] - finder_params[:search] = params[:search] if params[:search] - - projects = ProjectsFinder.new(current_user: current_user, params: finder_params).execute + projects = ProjectsFinder.new(current_user: current_user, params: project_finder_params).execute projects = filter_projects(projects) projects = projects.with_statistics if params[:statistics] projects = projects.with_issues_enabled if params[:with_issues_enabled] -- cgit v1.2.1 From 5654ac877df5b6007606e0e1827d965bdf8e552b Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 26 May 2017 15:39:38 +0200 Subject: Make it possible to combine :trending with other params Now it is possible to combine the :non_public parameter. This might be useful when a user wants to know the trending projects they are member of. --- app/finders/projects_finder.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 1d365e7cd7d..866d95ea1d7 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -32,6 +32,7 @@ class ProjectsFinder < UnionFinder item = by_ids(item) item = by_personal(item) item = by_starred(item) + item = by_trending(item) item = by_visibilty_level(item) item = by_tags(item) item = by_search(item) @@ -46,12 +47,8 @@ class ProjectsFinder < UnionFinder def init_collection projects = [] - if params[:trending].present? - projects << Project.trending - else - projects << current_user.authorized_projects if current_user - projects << Project.unscoped.public_to_user(current_user) unless params[:non_public].present? - end + projects << current_user.authorized_projects if current_user + projects << Project.unscoped.public_to_user(current_user) unless params[:non_public].present? projects end @@ -72,6 +69,10 @@ class ProjectsFinder < UnionFinder (params[:starred].present? && current_user) ? items.starred_by(current_user) : items end + def by_trending(items) + params[:trending].present? ? items.trending : items + end + def by_visibilty_level(items) params[:visibility_level].present? ? items.where(visibility_level: params[:visibility_level]) : items end -- cgit v1.2.1 From db679788e46d55984a4af71034c6db11aed919e4 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 26 May 2017 16:31:37 +0200 Subject: Add :owned param to ProjectFinder And use it in the API. --- app/finders/projects_finder.rb | 9 +++++++-- lib/api/groups.rb | 2 +- lib/api/helpers.rb | 7 ++----- lib/api/projects.rb | 2 +- spec/finders/projects_finder_spec.rb | 7 +++++++ 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 866d95ea1d7..5bf722d1ec6 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -7,6 +7,7 @@ # project_ids_relation: int[] - project ids to use # params: # trending: boolean +# owned: boolean # non_public: boolean # starred: boolean # sort: string @@ -47,8 +48,12 @@ class ProjectsFinder < UnionFinder def init_collection projects = [] - projects << current_user.authorized_projects if current_user - projects << Project.unscoped.public_to_user(current_user) unless params[:non_public].present? + if params[:owned].present? + projects << current_user.owned_projects if current_user + else + projects << current_user.authorized_projects if current_user + projects << Project.unscoped.public_to_user(current_user) unless params[:non_public].present? + end projects end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index aacc3356a0e..e14a988a153 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -152,7 +152,7 @@ module API get ":id/projects" do group = find_group!(params[:id]) projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute - projects = filter_projects(projects) + projects = reorder_projects(projects) entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project present paginate(projects), with: entity, current_user: current_user end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 17f57cfb8d7..d61450f8258 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -256,16 +256,13 @@ module API # project helpers - def filter_projects(projects) - if params[:owned] - projects = projects.merge(current_user.owned_projects) - end - + def reorder_projects(projects) projects.reorder(params[:order_by] => params[:sort]) end def project_finder_params finder_params = {} + finder_params[:owned] = true if params[:owned].present? finder_params[:non_public] = true if params[:membership].present? finder_params[:starred] = true if params[:starred].present? finder_params[:visibility_level] = Gitlab::VisibilityLevel.level_value(params[:visibility]) if params[:visibility] diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 267dd2a74d7..1356f959e70 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -69,7 +69,7 @@ module API def present_projects(options = {}) projects = ProjectsFinder.new(current_user: current_user, params: project_finder_params).execute - projects = filter_projects(projects) + projects = reorder_projects(projects) projects = projects.with_statistics if params[:statistics] projects = projects.with_issues_enabled if params[:with_issues_enabled] projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 077f2624388..03d98459e8c 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -137,6 +137,13 @@ describe ProjectsFinder do it { is_expected.to eq([public_project]) } end + describe 'filter by owned' do + let(:params) { { owned: true } } + let!(:owned_project) { create(:empty_project, :private, namespace: current_user.namespace) } + + it { is_expected.to eq([owned_project]) } + end + describe 'filter by non_public' do let(:params) { { non_public: true } } before do -- cgit v1.2.1 From 1e5506d01619780da68fc51ada58188a9070255b Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Tue, 30 May 2017 23:24:17 +0200 Subject: Remove some deprecated methods To avoid the use of slow queries, remove some deprecated methods and encourage the use of ProjectFinder to find projects. --- app/controllers/dashboard_controller.rb | 2 +- app/models/project.rb | 4 ---- app/models/user.rb | 6 ------ lib/api/v3/helpers.rb | 2 +- lib/api/v3/projects.rb | 2 +- spec/models/user_spec.rb | 19 ------------------- 6 files changed, 3 insertions(+), 32 deletions(-) diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 6195121b931..f9c31920302 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -24,7 +24,7 @@ class DashboardController < Dashboard::ApplicationController def load_events projects = if params[:filter] == "starred" - current_user.viewable_starred_projects + ProjectsFinder.new(current_user: current_user, params: { starred: true }).execute else current_user.authorized_projects end diff --git a/app/models/project.rb b/app/models/project.rb index 963fd594d46..457399cb60e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -351,10 +351,6 @@ class Project < ActiveRecord::Base where("projects.id IN (#{union.to_sql})") end - def search_by_visibility(level) # DEPRECATED: remove with API V3 - where(visibility_level: Gitlab::VisibilityLevel.string_options[level]) - end - def search_by_title(query) pattern = "%#{query}%" table = Project.arel_table diff --git a/app/models/user.rb b/app/models/user.rb index 20894ce269a..9aad327b592 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -557,12 +557,6 @@ class User < ActiveRecord::Base authorized_projects(Gitlab::Access::REPORTER).where(id: projects) end - def viewable_starred_projects # DEPRECATED: Use ProjectFinder instead. Remove together with API V3 - starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (?)", - [Project::PUBLIC, Project::INTERNAL], - authorized_projects.select(:project_id)) - end - def owned_projects @owned_projects ||= Project.where('namespace_id IN (?) OR namespace_id = ?', diff --git a/lib/api/v3/helpers.rb b/lib/api/v3/helpers.rb index 06af286ef50..d9e76560d03 100644 --- a/lib/api/v3/helpers.rb +++ b/lib/api/v3/helpers.rb @@ -35,7 +35,7 @@ module API end if params[:visibility].present? - projects = projects.search_by_visibility(params[:visibility]) + projects = projects.where(visibility_level: Gitlab::VisibilityLevel.level_value(params[:visibility])) end projects = projects.where(archived: params[:archived]) diff --git a/lib/api/v3/projects.rb b/lib/api/v3/projects.rb index 164612cb8dd..896c00b88e7 100644 --- a/lib/api/v3/projects.rb +++ b/lib/api/v3/projects.rb @@ -147,7 +147,7 @@ module API get '/starred' do authenticate! - present_projects current_user.viewable_starred_projects + present_projects ProjectsFinder.new(current_user: current_user, params: { starred: true }).execute end desc 'Get all projects for admin user' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9edf34b05ad..fe9df3360ff 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1496,25 +1496,6 @@ describe User, models: true do end end - describe '#viewable_starred_projects' do - let(:user) { create(:user) } - let(:public_project) { create(:empty_project, :public) } - let(:private_project) { create(:empty_project, :private) } - let(:private_viewable_project) { create(:empty_project, :private) } - - before do - private_viewable_project.team << [user, Gitlab::Access::MASTER] - - [public_project, private_project, private_viewable_project].each do |project| - user.toggle_star(project) - end - end - - it 'returns only starred projects the user can view' do - expect(user.viewable_starred_projects).not_to include(private_project) - end - end - describe '#projects_with_reporter_access_limited_to' do let(:project1) { create(:empty_project) } let(:project2) { create(:empty_project) } -- cgit v1.2.1 From 870a8bbbdd7456af03b5b4159c6dee4941edb759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 30 May 2017 22:16:43 -0400 Subject: Allow PostReceivePack to be enabled with Gitaly --- lib/gitlab/workhorse.rb | 3 +-- spec/lib/gitlab/workhorse_spec.rb | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index fe37e4da94f..18d8b4f4744 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -31,8 +31,7 @@ module Gitlab feature_enabled = case action.to_s when 'git_receive_pack' - # Disabled for now, see https://gitlab.com/gitlab-org/gitaly/issues/172 - false + Gitlab::GitalyClient.feature_enabled?(:post_receive_pack) when 'git_upload_pack' Gitlab::GitalyClient.feature_enabled?(:post_upload_pack) when 'info_refs' diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index fdbb55fc874..b1999409170 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -244,7 +244,7 @@ describe Gitlab::Workhorse, lib: true do context "when git_receive_pack action is passed" do let(:action) { 'git_receive_pack' } - it { expect(subject).not_to include(gitaly_params) } + it { expect(subject).to include(gitaly_params) } end context "when info_refs action is passed" do -- cgit v1.2.1 From 0684073d1ea5de0c7e0052b3ccc0cca77f661b56 Mon Sep 17 00:00:00 2001 From: vanadium23 Date: Tue, 30 May 2017 20:41:43 +0300 Subject: Add tag_list param to project api --- changelogs/unreleased/33000-tag-list-in-project-create-api.yml | 4 ++++ doc/api/projects.md | 3 +++ lib/api/projects.rb | 2 ++ spec/requests/api/projects_spec.rb | 8 ++++++++ 4 files changed, 17 insertions(+) create mode 100644 changelogs/unreleased/33000-tag-list-in-project-create-api.yml diff --git a/changelogs/unreleased/33000-tag-list-in-project-create-api.yml b/changelogs/unreleased/33000-tag-list-in-project-create-api.yml new file mode 100644 index 00000000000..b0d0d3cbeba --- /dev/null +++ b/changelogs/unreleased/33000-tag-list-in-project-create-api.yml @@ -0,0 +1,4 @@ +--- +title: Add tag_list param to project api +merge_request: 11799 +author: Ivan Chernov diff --git a/doc/api/projects.md b/doc/api/projects.md index 345f93a6017..62c78ddc32e 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -473,6 +473,7 @@ Parameters: | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | +| `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | ### Create project for user @@ -506,6 +507,7 @@ Parameters: | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | +| `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | ### Edit project @@ -538,6 +540,7 @@ Parameters: | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | +| `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | ### Fork project diff --git a/lib/api/projects.rb b/lib/api/projects.rb index d4fe5c023bf..a827fb26b98 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -21,6 +21,7 @@ module API optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access' optional :only_allow_merge_if_pipeline_succeeds, type: Boolean, desc: 'Only allow to merge if builds succeed' optional :only_allow_merge_if_all_discussions_are_resolved, type: Boolean, desc: 'Only allow to merge if all discussions are resolved' + optional :tag_list, type: Array[String], desc: 'The list of tags for a project' end params :optional_params do @@ -231,6 +232,7 @@ module API :request_access_enabled, :shared_runners_enabled, :snippets_enabled, + :tag_list, :visibility, :wiki_enabled ] diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index f95a287a184..3d98551628b 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -390,6 +390,14 @@ describe API::Projects do expect(json_response['visibility']).to eq('private') end + it 'sets tag list to a project' do + project = attributes_for(:project, tag_list: %w[tagFirst tagSecond]) + + post api('/projects', user), project + + expect(json_response['tag_list']).to eq(%w[tagFirst tagSecond]) + end + it 'sets a project as allowing merge even if build fails' do project = attributes_for(:project, { only_allow_merge_if_pipeline_succeeds: false }) post api('/projects', user), project -- cgit v1.2.1 From 16368e6551dcc716f89be6471da1171b996f4885 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 23 May 2017 16:33:27 +0200 Subject: Check only a merge ability for protected actions --- app/policies/ci/build_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index d4af4490608..2d7405dc240 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -23,7 +23,7 @@ module Ci !::Gitlab::UserAccess .new(user, project: build.project) - .can_push_to_branch?(build.ref) + .can_merge_to_branch?(build.ref) end end end -- cgit v1.2.1 From 0f52544baddd570646f8ae4c1d00eed1cdb6b59d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 10:37:19 +0200 Subject: Fix build entity specs related to protected actions --- spec/serializers/build_entity_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/build_entity_spec.rb index b5eb84ae43b..6d5e1046e86 100644 --- a/spec/serializers/build_entity_spec.rb +++ b/spec/serializers/build_entity_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe BuildEntity do let(:user) { create(:user) } let(:build) { create(:ci_build) } + let(:project) { build.project } let(:request) { double('request') } before do @@ -52,7 +53,10 @@ describe BuildEntity do context 'when user is allowed to trigger action' do before do - build.project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end it 'contains path to play action' do -- cgit v1.2.1 From f8eb8feae3aa146c41433308abd2e184d73688e5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 10:40:52 +0200 Subject: Fix pipeline processing specs related to protected actions --- spec/services/ci/process_pipeline_service_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index fc5de5d069a..1557cb3c938 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -333,10 +333,11 @@ describe Ci::ProcessPipelineService, '#execute', :services do context 'when pipeline is promoted sequentially up to the end' do before do - # We are using create(:empty_project), and users has to be master in - # order to execute manual action when repository does not exist. + # Users need ability to merge into a branch in order to trigger + # protected manual actions. # - project.add_master(user) + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end it 'properly processes entire pipeline' do -- cgit v1.2.1 From 20047e72d814e1d5fd744a0eda843019eb4e0b3f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:07:12 +0200 Subject: Fix environment specs related to protected actions --- app/views/projects/deployments/_actions.haml | 1 + app/views/projects/environments/show.html.haml | 2 +- .../projects/environments/environment_spec.rb | 57 ++++++++++++++-------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/app/views/projects/deployments/_actions.haml b/app/views/projects/deployments/_actions.haml index 506246f2ee6..e2baaa625ae 100644 --- a/app/views/projects/deployments/_actions.haml +++ b/app/views/projects/deployments/_actions.haml @@ -8,6 +8,7 @@ = icon('caret-down') %ul.dropdown-menu.dropdown-menu-align-right - actions.each do |action| + - next unless can?(current_user, :update_build, action) %li = link_to [:play, @project.namespace.becomes(Namespace), @project, action], method: :post, rel: 'nofollow' do = custom_icon('icon_play') diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml index 7315e671056..9e221240cf2 100644 --- a/app/views/projects/environments/show.html.haml +++ b/app/views/projects/environments/show.html.haml @@ -13,7 +13,7 @@ = render 'projects/environments/metrics_button', environment: @environment - if can?(current_user, :update_environment, @environment) = link_to 'Edit', edit_namespace_project_environment_path(@project.namespace, @project, @environment), class: 'btn' - - if can?(current_user, :create_deployment, @environment) && @environment.can_stop? + - if can?(current_user, :stop_environment, @environment) = link_to 'Stop', stop_namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to stop this environment?' }, class: 'btn btn-danger', method: :post .environments-container diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb index 86ce50c976f..18b608c863e 100644 --- a/spec/features/projects/environments/environment_spec.rb +++ b/spec/features/projects/environments/environment_spec.rb @@ -12,6 +12,7 @@ feature 'Environment', :feature do feature 'environment details page' do given!(:environment) { create(:environment, project: project) } + given!(:permissions) { } given!(:deployment) { } given!(:action) { } @@ -62,20 +63,31 @@ feature 'Environment', :feature do name: 'deploy to production') end - given(:role) { :master } + context 'when user has ability to trigger deployment' do + given(:permissions) do + create(:protected_branch, :developers_can_merge, + name: action.ref, project: project) + end - scenario 'does show a play button' do - expect(page).to have_link(action.name.humanize) - end + it 'does show a play button' do + expect(page).to have_link(action.name.humanize) + end + + it 'does allow to play manual action' do + expect(action).to be_manual - scenario 'does allow to play manual action' do - expect(action).to be_manual + expect { click_link(action.name.humanize) } + .not_to change { Ci::Pipeline.count } - expect { click_link(action.name.humanize) } - .not_to change { Ci::Pipeline.count } + expect(page).to have_content(action.name) + expect(action.reload).to be_pending + end + end - expect(page).to have_content(action.name) - expect(action.reload).to be_pending + context 'when user has no ability to trigger a deployment' do + it 'does not show a play button' do + expect(page).not_to have_link(action.name.humanize) + end end context 'with external_url' do @@ -134,12 +146,23 @@ feature 'Environment', :feature do on_stop: 'close_app') end - given(:role) { :master } + context 'when user has ability to stop environment' do + given(:permissions) do + create(:protected_branch, :developers_can_merge, + name: action.ref, project: project) + end - scenario 'does allow to stop environment' do - click_link('Stop') + it 'allows to stop environment' do + click_link('Stop') - expect(page).to have_content('close_app') + expect(page).to have_content('close_app') + end + end + + context 'when user has no ability to stop environment' do + it 'does not allow to stop environment' do + expect(page).to have_no_link('Stop') + end end context 'for reporter' do @@ -150,12 +173,6 @@ feature 'Environment', :feature do end end end - - context 'without stop action' do - scenario 'does allow to stop environment' do - click_link('Stop') - end - end end context 'when environment is stopped' do -- cgit v1.2.1 From 1984359de3371b5f0c8ddbfad757a32d3013c252 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:09:29 +0200 Subject: Fix deploy chat command specs for protected actions --- spec/lib/gitlab/chat_commands/deploy_spec.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/chat_commands/deploy_spec.rb b/spec/lib/gitlab/chat_commands/deploy_spec.rb index b33389d959e..46dbdeae37c 100644 --- a/spec/lib/gitlab/chat_commands/deploy_spec.rb +++ b/spec/lib/gitlab/chat_commands/deploy_spec.rb @@ -7,7 +7,12 @@ describe Gitlab::ChatCommands::Deploy, service: true do let(:regex_match) { described_class.match('deploy staging to production') } before do - project.add_master(user) + # Make it possible to trigger protected manual actions for developers. + # + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end subject do -- cgit v1.2.1 From 13f10a96e345535cc8946bf73f85de0063d80739 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:11:46 +0200 Subject: Fix play status specs related to protected actions --- spec/lib/gitlab/ci/status/build/play_spec.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index f5d0f977768..0e15a5f3c6b 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Play do let(:user) { create(:user) } + let(:project) { build.project } let(:build) { create(:ci_build, :manual) } let(:status) { Gitlab::Ci::Status::Core.new(build, user) } @@ -15,8 +16,13 @@ describe Gitlab::Ci::Status::Build::Play do describe '#has_action?' do context 'when user is allowed to update build' do - context 'when user can push to branch' do - before { build.project.add_master(user) } + context 'when user is allowed to trigger protected action' do + before do + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) + end it { is_expected.to have_action } end -- cgit v1.2.1 From c970702517558ee6c1ba454572a20ee57fadf285 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:15:25 +0200 Subject: Fix job play service specs related to protected actions --- spec/services/ci/play_build_service_spec.rb | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb index d6f9fa42045..ea211de1f82 100644 --- a/spec/services/ci/play_build_service_spec.rb +++ b/spec/services/ci/play_build_service_spec.rb @@ -13,8 +13,11 @@ describe Ci::PlayBuildService, '#execute', :services do context 'when project does not have repository yet' do let(:project) { create(:empty_project) } - it 'allows user with master role to play build' do - project.add_master(user) + it 'allows user to play build if protected branch rules are met' do + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) service.execute(build) @@ -45,7 +48,10 @@ describe Ci::PlayBuildService, '#execute', :services do let(:build) { create(:ci_build, :manual, pipeline: pipeline) } before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end it 'enqueues the build' do @@ -64,7 +70,10 @@ describe Ci::PlayBuildService, '#execute', :services do let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) } before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end it 'duplicates the build' do -- cgit v1.2.1 From e3cb71442e96921ae67a443a62d8f27f6df02216 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:18:15 +0200 Subject: Fix build factory specs related to protected actions --- spec/lib/gitlab/ci/status/build/factory_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index 185bb9098da..3f30b2c38f2 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -224,7 +224,10 @@ describe Gitlab::Ci::Status::Build::Factory do context 'when user has ability to play action' do before do - build.project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end it 'fabricates status that has action' do -- cgit v1.2.1 From fb1b7b00f3f76e0c0ace91b5dfea63408c24de08 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:20:36 +0200 Subject: Fix environment model specs related to protected actions --- spec/models/environment_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 12519de8636..9fbe19b04d5 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -227,7 +227,10 @@ describe Environment, models: true do context 'when user is allowed to stop environment' do before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) end context 'when action did not yet finish' do -- cgit v1.2.1 From c9b653b4a0981647270c70122b01bfa6744c451b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:22:12 +0200 Subject: Fix pipeline retry specs related to protected actions --- spec/services/ci/retry_pipeline_service_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index d941d56c0d8..3e860203063 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -6,9 +6,12 @@ describe Ci::RetryPipelineService, '#execute', :services do let(:pipeline) { create(:ci_pipeline, project: project) } let(:service) { described_class.new(project, user) } - context 'when user has ability to modify pipeline' do + context 'when user has full ability to modify pipeline' do before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: pipeline.ref, project: project) end context 'when there are already retried jobs present' do -- cgit v1.2.1 From 9a6b4c80892750c60933082def7ad958f61f46df Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:24:17 +0200 Subject: Fix builds controller specs related to protected actions --- spec/controllers/projects/builds_controller_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb index f41503fd34e..932276ce380 100644 --- a/spec/controllers/projects/builds_controller_spec.rb +++ b/spec/controllers/projects/builds_controller_spec.rb @@ -234,7 +234,11 @@ describe Projects::BuildsController do describe 'POST play' do before do - project.add_master(user) + project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: 'master', project: project) + sign_in(user) post_play -- cgit v1.2.1 From 37694858223f217c90cbdb794b85b59ece69b284 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 11:25:55 +0200 Subject: Fix chat commands specs related to protected actions --- spec/lib/gitlab/chat_commands/command_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index eb4f06b371c..13e6953147b 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -58,9 +58,12 @@ describe Gitlab::ChatCommands::Command, service: true do end end - context 'and user does have deployment permission' do + context 'and user has deployment permission' do before do - build.project.add_master(user) + build.project.add_developer(user) + + create(:protected_branch, :developers_can_merge, + name: build.ref, project: project) end it 'returns action' do -- cgit v1.2.1 From 0a2648065082b9a351a76dba77181bcd78bb9dce Mon Sep 17 00:00:00 2001 From: winh Date: Wed, 31 May 2017 12:11:30 +0200 Subject: Center loading spinner in issuable filters --- app/assets/stylesheets/framework/filters.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index d191bbb226c..90051ffe753 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -475,4 +475,5 @@ .filter-dropdown-loading { padding: 8px 16px; + text-align: center; } -- cgit v1.2.1 From 9dd1ef182bd0cf1a0af2073e6ede4e332995d127 Mon Sep 17 00:00:00 2001 From: Mark Fletcher Date: Wed, 31 May 2017 18:35:24 +0800 Subject: Ask for an example project for bug reports * Bugs reported in the GitLab Issue trackers should be reproducible * Asking reporters to reproduce on GitLab.com is reasonable * It lets us know whether the issue still exists in latest * It ensures that the steps to reproduce are solid * It gives Developers a live example to work from * Reporter can verify the fix in the example project once shipped [skip ci] --- .gitlab/issue_templates/Bug.md | 6 ++++++ .../unreleased/issue-template-reproduce-in-example-project.yml | 4 ++++ 2 files changed, 10 insertions(+) create mode 100644 changelogs/unreleased/issue-template-reproduce-in-example-project.yml diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index 58af062e75e..9d53a48409a 100644 --- a/.gitlab/issue_templates/Bug.md +++ b/.gitlab/issue_templates/Bug.md @@ -20,6 +20,12 @@ Please remove this notice if you're confident your issue isn't a duplicate. (How one can reproduce the issue - this is very important) +### Example Project + +(If possible, please create an example project here on GitLab.com that exhibits the problematic behaviour, and link to it here in the bug report) + +(If you are using an older version of GitLab, this will also determine whether the bug has been fixed in a more recent version) + ### What is the current *bug* behavior? (What actually happens) diff --git a/changelogs/unreleased/issue-template-reproduce-in-example-project.yml b/changelogs/unreleased/issue-template-reproduce-in-example-project.yml new file mode 100644 index 00000000000..8116007b459 --- /dev/null +++ b/changelogs/unreleased/issue-template-reproduce-in-example-project.yml @@ -0,0 +1,4 @@ +--- +title: Ask for an example project for bug reports +merge_request: +author: -- cgit v1.2.1 From c791245cbf833abeadba214ddef219b8ac20102f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 13:12:35 +0200 Subject: Add changelog for protected branches abilities fix --- .../fix-gb-use-merge-ability-for-protected-manual-actions.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml diff --git a/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml b/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml new file mode 100644 index 00000000000..43c18502cd6 --- /dev/null +++ b/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml @@ -0,0 +1,4 @@ +--- +title: Respect merge, instead of push, permissions for protected actions +merge_request: 11648 +author: -- cgit v1.2.1 From 6914aeaeb78015f2178dd1a612f90ddb150408d8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 31 May 2017 13:13:37 +0200 Subject: Update docs related to protected actions --- doc/ci/yaml/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index da20076da52..2df03196f80 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -591,7 +591,7 @@ Optional manual actions have `allow_failure: true` set by default. **Manual actions are considered to be write actions, so permissions for protected branches are used when user wants to trigger an action. In other words, in order to trigger a manual action assigned to a branch that the -pipeline is running for, user needs to have ability to push to this branch.** +pipeline is running for, user needs to have ability to merge to this branch.** ### environment -- cgit v1.2.1