From fbd3b3d8a245072121784df11b7b41d3257b989f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 12 May 2017 04:12:04 +0900 Subject: [PATCH 01/33] 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 ++++++++ .../api/schemas/pipeline_schedule.json | 64 ++++ .../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 From 384fd190e513de41a36726e14bfc3651319c7324 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 12 May 2017 21:52:31 +0900 Subject: [PATCH 02/33] 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 From 5875d9969a31b754f2dda8b271f0fdddec5c3451 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 12 May 2017 21:53:31 +0900 Subject: [PATCH 03/33] Add changelog --- .../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 From 359b17611dbbfec8881319f29b81cf8f1c7bf06e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 01:37:01 +0900 Subject: [PATCH 04/33] 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" + } +} +``` From 0a11ab48993d092bd730af9b378fbf665b255625 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 01:48:34 +0900 Subject: [PATCH 05/33] 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! From fbb070646aeccab5e46f300d130c20fa5a0a22fa Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 02:10:14 +0900 Subject: [PATCH 06/33] 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 From bd7d7759d6e7f98d2d306495a8de5be44590b490 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 02:21:11 +0900 Subject: [PATCH 07/33] 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) From 97bf2401991ab2e9cea956dfb7c9630e2a185683 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:06:05 +0900 Subject: [PATCH 08/33] 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 From 8743f765abc2281c664792f5016747f54d0fb7aa Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:10:16 +0900 Subject: [PATCH 09/33] 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 From f8cb5fd65a8ed00f38368a6a050c940e72cc6f3e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:37:09 +0900 Subject: [PATCH 10/33] 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 From 9185241a1e591296c4592e19a056ec4761bd0b75 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:38:11 +0900 Subject: [PATCH 11/33] 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) From 18cf368ba0193e61d8b127c80f9b87de4121bf3c Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:39:32 +0900 Subject: [PATCH 12/33] 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') From 427eac90a3b6965009dd853d7246c248988974e8 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 13 May 2017 03:41:31 +0900 Subject: [PATCH 13/33] 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') From a104e7a2c92d9752e48b2ac776b547809b0036d5 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 16 May 2017 23:58:08 +0900 Subject: [PATCH 14/33] 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)) From e929897873267b0815baf28dfa6b19d49d695554 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 16 May 2017 23:58:22 +0900 Subject: [PATCH 15/33] 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? From 94f7595b9a8edcb4ac8480995a067eb4fa3dec7e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 00:20:11 +0900 Subject: [PATCH 16/33] 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 From e828e835e435deab8a62c7c999dca0b774e7a7d6 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 01:33:09 +0900 Subject: [PATCH 17/33] 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 From c7fc65e0672a3c88e15f033be3b4d4258bc36e2f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 21:16:54 +0900 Subject: [PATCH 18/33] 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') From df6040bbd41a639b3d0aa339b2dc0e84d67b811b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 21:26:18 +0900 Subject: [PATCH 19/33] 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 From 17b9128b305aefc29fddae3b51d13c61ae7dfef9 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 17 May 2017 22:58:23 +0900 Subject: [PATCH 20/33] 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) From 20a07d26ff4be9c82271297e516df912109b05aa Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 18 May 2017 17:45:08 +0900 Subject: [PATCH 21/33] 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 +- .../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 From 8c40bbbe7d0d1bb930a590f5ca98570bd32ad1f0 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 18 May 2017 17:58:55 +0900 Subject: [PATCH 22/33] 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 From 92bc1ddaf5a256a5e6636bc4a6f8ebd8673ec719 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 18 May 2017 18:22:41 +0900 Subject: [PATCH 23/33] 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 From f6a8894a5928e1cc37d8301c555fcfd5953cc180 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 19 May 2017 02:42:23 +0900 Subject: [PATCH 24/33] 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 From bfa028e13a41b16b0da2d69359694edbebf9e455 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 19 May 2017 22:24:31 +0900 Subject: [PATCH 25/33] 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 } From 0b2fd147eca68911342003333dd2d0daa558627b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 19 May 2017 22:48:36 +0900 Subject: [PATCH 26/33] 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", From 20c26eaf9371dcc89cd3a403f79882e6c2dbf6ec Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 22 May 2017 15:52:08 +0900 Subject: [PATCH 27/33] 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 From c91292b61f80626b91d41cc17d0a662f302d6ecd Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 24 May 2017 19:25:13 +0900 Subject: [PATCH 28/33] 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 From b17c8d67d8811e0a440338dc25464d8c90e81179 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 27 May 2017 21:29:01 +0900 Subject: [PATCH 29/33] 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 From 63ca126e977666335d7e5f70665815d1289a6f34 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 27 May 2017 22:22:33 +0900 Subject: [PATCH 30/33] 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 From 3b61451c46f462b392783b282fe63dbddd8b6c2e Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Sat, 27 May 2017 22:31:22 +0900 Subject: [PATCH 31/33] 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 From 0e0e278196801cb9b42791ad4ef9707ad52c4896 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 29 May 2017 15:03:07 +0900 Subject: [PATCH 32/33] 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" From 2da420be04ae1fa00b06149c0293a0349a085e99 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 31 May 2017 00:06:37 +0900 Subject: [PATCH 33/33] 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