From cbfdc37a241fd2a8851cf9fa85d817e44e8436e0 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Fri, 14 Oct 2016 10:57:10 -0500 Subject: Smaller min-width for MR pipeline table --- app/assets/stylesheets/pages/pipelines.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 7b71876b822..21b58b90a5f 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -634,6 +634,10 @@ &.pipelines { + .ci-table { + min-width: 900px; + } + .content-list.pipelines { overflow: auto; } -- cgit v1.2.1 From baa3d0b3744b6ee7ef067ec39238257748fde71b Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Sun, 18 Sep 2016 17:20:40 +0100 Subject: Added tooltip with jobs full name to build items in graph Added status to build tooltip and removed status icon tooltip Added Pipelines class to force tooltips ontop of the dropdown, we cannot do this with data attributes because dropdown toggle is already set Corrected dispatcher invocation --- app/assets/javascripts/pipelines.js.es6 | 11 +++++++++++ app/helpers/ci_status_helper.rb | 4 ++-- app/views/projects/commit/_pipeline_stage.html.haml | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/pipelines.js.es6 b/app/assets/javascripts/pipelines.js.es6 index a7624de6089..dd320c52e44 100644 --- a/app/assets/javascripts/pipelines.js.es6 +++ b/app/assets/javascripts/pipelines.js.es6 @@ -4,6 +4,17 @@ constructor() { $(document).off('click', '.toggle-pipeline-btn').on('click', '.toggle-pipeline-btn', this.toggleGraph); this.addMarginToBuildColumns(); + this.initGroupedPipelineTooltips(); + } + + initGroupedPipelineTooltips() { + $('.dropdown-menu-toggle', $('.grouped-pipeline-dropdown').parent()).each(function() { + const $this = $(this); + $this.tooltip({ + title: $this.data('tooltip-title'), + placement: 'bottom' + }); + }); } toggleGraph() { diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index b7f48630bd4..0e727f3dcf0 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -71,10 +71,10 @@ module CiStatusHelper Ci::Runner.shared.blank? end - def render_status_with_link(type, status, path = nil, tooltip_placement: 'auto left', cssclass: '', container: 'body') + def render_status_with_link(type, status, path = nil, tooltip_placement: 'auto left', cssclass: '', container: 'body', show_tooltip: true) klass = "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}" title = "#{type.titleize}: #{ci_label_for_status(status)}" - data = { toggle: 'tooltip', placement: tooltip_placement, container: container } + data = { toggle: 'tooltip', placement: tooltip_placement, container: container } if show_tooltip if path link_to ci_icon_for_status(status), path, diff --git a/app/views/projects/commit/_pipeline_stage.html.haml b/app/views/projects/commit/_pipeline_stage.html.haml index 289aa5178b1..993fc89d23d 100644 --- a/app/views/projects/commit/_pipeline_stage.html.haml +++ b/app/views/projects/commit/_pipeline_stage.html.haml @@ -5,7 +5,7 @@ - is_playable = status.playable? && can?(current_user, :update_build, @project) %li.build{ class: ("playable" if is_playable) } .curve - .build-content + .build-content{ { data: { toggle: 'tooltip', title: "#{group_name} - #{status.status}", placement: 'bottom' } } } = render "projects/#{status.to_partial_path}_pipeline", subject: status - else %li.build -- cgit v1.2.1 From bb56450acc8d9a8c88aec8abf68da548bdfdb4da Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 18 Oct 2016 17:03:36 +0100 Subject: Fixed height of issue board blank state --- app/assets/stylesheets/pages/boards.scss | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 6e81c12aa55..d8fabbdcebe 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -1,4 +1,3 @@ -lex [v-cloak] { display: none; } @@ -132,7 +131,7 @@ lex } .board-blank-state { - height: 100%; + height: calc(100% - 49px); padding: $gl-padding; background-color: #fff; } -- cgit v1.2.1 From f744a5c12f0c82573f5247b9e7580c3eeb7768cf Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Fri, 7 Oct 2016 19:30:34 +0100 Subject: Added dyanmic position adjustment Added tooltips for dropdown items Reverted pretty much everything in favour of a DOM approach Simplified JS --- app/assets/javascripts/pipelines.js.es6 | 11 ----------- app/assets/stylesheets/pages/pipelines.scss | 14 ++++++++++---- app/helpers/ci_status_helper.rb | 4 ++-- app/views/projects/ci/builds/_build_pipeline.html.haml | 4 ++-- app/views/projects/commit/_pipeline_stage.html.haml | 2 +- app/views/projects/commit/_pipeline_status_group.html.haml | 2 +- .../_generic_commit_status_pipeline.html.haml | 13 +++++++------ 7 files changed, 23 insertions(+), 27 deletions(-) diff --git a/app/assets/javascripts/pipelines.js.es6 b/app/assets/javascripts/pipelines.js.es6 index dd320c52e44..a7624de6089 100644 --- a/app/assets/javascripts/pipelines.js.es6 +++ b/app/assets/javascripts/pipelines.js.es6 @@ -4,17 +4,6 @@ constructor() { $(document).off('click', '.toggle-pipeline-btn').on('click', '.toggle-pipeline-btn', this.toggleGraph); this.addMarginToBuildColumns(); - this.initGroupedPipelineTooltips(); - } - - initGroupedPipelineTooltips() { - $('.dropdown-menu-toggle', $('.grouped-pipeline-dropdown').parent()).each(function() { - const $this = $(this); - $this.tooltip({ - title: $this.data('tooltip-title'), - placement: 'bottom' - }); - }); } toggleGraph() { diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 7b71876b822..46308742aaf 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -360,10 +360,6 @@ &:hover { background-color: $gray-lighter; - - .dropdown-menu-toggle { - background-color: transparent; - } } &.playable { @@ -393,6 +389,15 @@ } } + .tooltip { + white-space: nowrap; + + .tooltip-inner { + overflow: hidden; + text-overflow: ellipsis; + } + } + .ci-status-text { width: 135px; white-space: nowrap; @@ -410,6 +415,7 @@ } .dropdown-menu-toggle { + background-color: transparent; border: none; width: auto; padding: 0; diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 0e727f3dcf0..b7f48630bd4 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -71,10 +71,10 @@ module CiStatusHelper Ci::Runner.shared.blank? end - def render_status_with_link(type, status, path = nil, tooltip_placement: 'auto left', cssclass: '', container: 'body', show_tooltip: true) + def render_status_with_link(type, status, path = nil, tooltip_placement: 'auto left', cssclass: '', container: 'body') klass = "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}" title = "#{type.titleize}: #{ci_label_for_status(status)}" - data = { toggle: 'tooltip', placement: tooltip_placement, container: container } if show_tooltip + data = { toggle: 'tooltip', placement: tooltip_placement, container: container } if path link_to ci_icon_for_status(status), path, diff --git a/app/views/projects/ci/builds/_build_pipeline.html.haml b/app/views/projects/ci/builds/_build_pipeline.html.haml index 017d3ff6af2..55965172d3f 100644 --- a/app/views/projects/ci/builds/_build_pipeline.html.haml +++ b/app/views/projects/ci/builds/_build_pipeline.html.haml @@ -1,10 +1,10 @@ - is_playable = subject.playable? && can?(current_user, :update_build, @project) - if is_playable - = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, title: 'Play' do + = link_to play_namespace_project_build_path(subject.project.namespace, subject.project, subject, return_to: request.original_url), method: :post, data: { toggle: 'tooltip', title: "#{subject.name} - play", container: '.pipeline-graph', placement: 'bottom' } do = render_status_with_link('build', 'play') .ci-status-text= subject.name - elsif can?(current_user, :read_build, @project) - = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject) do + = link_to namespace_project_build_path(subject.project.namespace, subject.project, subject), data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } do %span.ci-status-icon = render_status_with_link('build', subject.status) .ci-status-text= subject.name diff --git a/app/views/projects/commit/_pipeline_stage.html.haml b/app/views/projects/commit/_pipeline_stage.html.haml index 993fc89d23d..289aa5178b1 100644 --- a/app/views/projects/commit/_pipeline_stage.html.haml +++ b/app/views/projects/commit/_pipeline_stage.html.haml @@ -5,7 +5,7 @@ - is_playable = status.playable? && can?(current_user, :update_build, @project) %li.build{ class: ("playable" if is_playable) } .curve - .build-content{ { data: { toggle: 'tooltip', title: "#{group_name} - #{status.status}", placement: 'bottom' } } } + .build-content = render "projects/#{status.to_partial_path}_pipeline", subject: status - else %li.build diff --git a/app/views/projects/commit/_pipeline_status_group.html.haml b/app/views/projects/commit/_pipeline_status_group.html.haml index 5d0d5ba0262..f2d71fa6989 100644 --- a/app/views/projects/commit/_pipeline_status_group.html.haml +++ b/app/views/projects/commit/_pipeline_status_group.html.haml @@ -1,5 +1,5 @@ - group_status = CommitStatus.where(id: subject).status -%button.dropdown-menu-toggle{ type: 'button', data: { toggle: 'dropdown' } } +%button.dropdown-menu-toggle.has-tooltip{ type: 'button', data: { toggle: 'dropdown', title: "#{name} - #{group_status}" } } %span.ci-status-icon = render_status_with_link('build', group_status) %span.ci-status-text diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml index 0a66d60accc..c45b73e4225 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status_pipeline.html.haml @@ -1,9 +1,10 @@ -- if subject.target_url - = link_to subject.target_url do +%a{ data: { toggle: 'tooltip', title: "#{subject.name} - #{subject.status}", container: '.pipeline-graph', placement: 'bottom' } } + - if subject.target_url + = link_to subject.target_url do + %span.ci-status-icon + = render_status_with_link('commit status', subject.status) + %span.ci-status-text= subject.name + - else %span.ci-status-icon = render_status_with_link('commit status', subject.status) %span.ci-status-text= subject.name -- else - %span.ci-status-icon - = render_status_with_link('commit status', subject.status) - %span.ci-status-text= subject.name -- cgit v1.2.1 From 325a5bafcc8a37b7d2ed1416b2952862066a914f Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Wed, 19 Oct 2016 21:58:12 +0200 Subject: Simpler arguments passed to named_route on toggle_award_url helper method --- CHANGELOG.md | 1 + app/helpers/award_emoji_helper.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e185d030ed5..421f0496025 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) + - Simpler arguments passed to named_route on toggle_award_url helper method ## 8.13.0 (2016-10-22) diff --git a/app/helpers/award_emoji_helper.rb b/app/helpers/award_emoji_helper.rb index 592ffe7b89f..167b09e678f 100644 --- a/app/helpers/award_emoji_helper.rb +++ b/app/helpers/award_emoji_helper.rb @@ -3,8 +3,8 @@ module AwardEmojiHelper return url_for([:toggle_award_emoji, awardable]) unless @project if awardable.is_a?(Note) - # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (6.5x) - toggle_award_emoji_namespace_project_note_url(namespace_id: @project.namespace, project_id: @project, id: awardable.id) + # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (4.5x) + toggle_award_emoji_namespace_project_note_url(@project.namespace, @project, awardable.id) else url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable]) end -- cgit v1.2.1 From ed9838cd292ce78ae98079f513d0d1648b7f49f0 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 14 Oct 2016 19:38:41 -0300 Subject: Create project feature when project is created --- CHANGELOG.md | 1 + app/models/project.rb | 7 +----- ...213545_generate_project_feature_for_projects.rb | 28 ++++++++++++++++++++++ db/schema.rb | 2 +- spec/models/project_spec.rb | 14 ++++++++--- 5 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20161019213545_generate_project_feature_for_projects.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b018fc0d57..e9d07ed0927 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Add an example for testing a phoenix application with Gitlab CI in the docs (Manthan Mallikarjun) - Cancelled pipelines could be retried. !6927 - Updating verbiage on git basics to be more intuitive + - Fix project_feature record not generated on project creation - Clarify documentation for Runners API (Gennady Trafimenkov) - The instrumentation for Banzai::Renderer has been restored - Change user & group landing page routing from /u/:username to /:username diff --git a/app/models/project.rb b/app/models/project.rb index 653c38322c5..6685baab699 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -32,8 +32,8 @@ class Project < ActiveRecord::Base default_value_for(:shared_runners_enabled) { current_application_settings.shared_runners_enabled } after_create :ensure_dir_exist + after_create :create_project_feature, unless: :project_feature after_save :ensure_dir_exist, if: :namespace_id_changed? - after_initialize :setup_project_feature # set last_activity_at to the same as created_at after_create :set_last_activity_at @@ -1310,11 +1310,6 @@ class Project < ActiveRecord::Base "projects/#{id}/pushes_since_gc" end - # Prevents the creation of project_feature record for every project - def setup_project_feature - build_project_feature unless project_feature - end - def default_branch_protected? current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_FULL || current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE diff --git a/db/migrate/20161019213545_generate_project_feature_for_projects.rb b/db/migrate/20161019213545_generate_project_feature_for_projects.rb new file mode 100644 index 00000000000..4554e14b0df --- /dev/null +++ b/db/migrate/20161019213545_generate_project_feature_for_projects.rb @@ -0,0 +1,28 @@ +class GenerateProjectFeatureForProjects < ActiveRecord::Migration + DOWNTIME = true + + DOWNTIME_REASON = <<-HEREDOC + Application was eager loading project_feature for all projects generating an extra query + everytime a project was fetched. We removed that behavior to avoid the extra query, this migration + makes sure all projects have a project_feature record associated. + HEREDOC + + def up + # Generate enabled values for each project feature 20, 20, 20, 20, 20 + # All features are enabled by default + enabled_values = [ProjectFeature::ENABLED] * 5 + + execute <<-EOF.strip_heredoc + INSERT INTO project_features + (project_id, merge_requests_access_level, builds_access_level, + issues_access_level, snippets_access_level, wiki_access_level) + (SELECT projects.id, #{enabled_values.join(',')} FROM projects LEFT OUTER JOIN project_features + ON project_features.project_id = projects.id + WHERE project_features.id IS NULL) + EOF + end + + def down + "Not needed" + end +end diff --git a/db/schema.rb b/db/schema.rb index 65f55aa109b..a3c7fc2fd57 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161018024550) do +ActiveRecord::Schema.define(version: 20161019213545) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e6d98e25d0b..f4dda1ee558 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -67,6 +67,14 @@ describe Project, models: true do it { is_expected.to have_many(:notification_settings).dependent(:destroy) } it { is_expected.to have_many(:forks).through(:forked_project_links) } + context 'after create' do + it "creates project feature" do + project = FactoryGirl.build(:project) + + expect { project.save }.to change{ project.project_feature.present? }.from(false).to(true) + end + end + describe '#members & #requesters' do let(:project) { create(:project, :public) } let(:requester) { create(:user) } @@ -531,9 +539,9 @@ describe Project, models: true do end describe '#has_wiki?' do - let(:no_wiki_project) { build(:project, wiki_enabled: false, has_external_wiki: false) } - let(:wiki_enabled_project) { build(:project) } - let(:external_wiki_project) { build(:project, has_external_wiki: true) } + let(:no_wiki_project) { create(:project, wiki_access_level: ProjectFeature::DISABLED, has_external_wiki: false) } + let(:wiki_enabled_project) { create(:project) } + let(:external_wiki_project) { create(:project, has_external_wiki: true) } it 'returns true if project is wiki enabled or has external wiki' do expect(wiki_enabled_project).to have_wiki -- cgit v1.2.1 From af8e06ee4c1ca54e7e05a40433d09253ba55fbbb Mon Sep 17 00:00:00 2001 From: Takuya Noguchi Date: Thu, 20 Oct 2016 08:57:23 +0900 Subject: Fix a broken table in Project API doc --- CHANGELOG.md | 1 + doc/api/projects.md | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b018fc0d57..a0e7dff7e1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Delete dynamic environments - Fix buggy iOS tooltip layering behavior. - Make guests unable to view MRs on private projects + - Fix broken Project API docs (Takuya Noguchi) ## 8.12.7 diff --git a/doc/api/projects.md b/doc/api/projects.md index b7791b4748a..77d6bd6b5c2 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1333,8 +1333,8 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `query` (required) - A string contained in the project name -| `per_page` (optional) - number of projects to return per page -| `page` (optional) - the page to retrieve -| `order_by` (optional) - Return requests ordered by `id`, `name`, `created_at` or `last_activity_at` fields +| `query` | string | yes | A string contained in the project name | +| `per_page` | integer | no | number of projects to return per page | +| `page` | integer | no | the page to retrieve | +| `order_by` | string | no | Return requests ordered by `id`, `name`, `created_at` or `last_activity_at` fields | | `sort` | string | no | Return requests sorted in `asc` or `desc` order | -- cgit v1.2.1 From a8bbe53c0c15c0924bce971e1ed853630d2b6d56 Mon Sep 17 00:00:00 2001 From: Takuya Noguchi Date: Thu, 20 Oct 2016 08:58:35 +0900 Subject: Remove pagination description from individual doc --- doc/api/projects.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/api/projects.md b/doc/api/projects.md index 77d6bd6b5c2..b69db90e70d 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1334,7 +1334,5 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `query` | string | yes | A string contained in the project name | -| `per_page` | integer | no | number of projects to return per page | -| `page` | integer | no | the page to retrieve | | `order_by` | string | no | Return requests ordered by `id`, `name`, `created_at` or `last_activity_at` fields | | `sort` | string | no | Return requests sorted in `asc` or `desc` order | -- cgit v1.2.1 From 9124310f2871117acaac98781be84c9fc016e2ad Mon Sep 17 00:00:00 2001 From: Callum Dryden Date: Thu, 6 Oct 2016 15:19:27 +0000 Subject: Differentiate the expire from leave event At the moment we cannot see weather a user left a project due to their membership expiring of if they themselves opted to leave the project. This adds a new event type that allows us to make this differentiation. Note that is not really feasable to go back and reliably fix up the previous events. As a result the events for previous expire removals will remain the same however events of this nature going forward will be correctly represented. --- CHANGELOG.md | 1 + app/models/concerns/expirable.rb | 6 +++- app/models/event.rb | 9 ++++- app/models/members/project_member.rb | 6 +++- app/services/event_create_service.rb | 4 +++ features/dashboard/dashboard.feature | 13 ------- features/steps/dashboard/dashboard.rb | 27 -------------- lib/event_filter.rb | 11 ++++-- .../project_member_activity_index_spec.rb | 41 ++++++++++++++++++++++ spec/models/concerns/expirable_spec.rb | 31 ++++++++++++++++ spec/models/event_spec.rb | 27 ++++++++++++++ spec/models/members/project_member_spec.rb | 11 ++++++ spec/services/event_create_service_spec.rb | 19 ++++++++++ 13 files changed, 161 insertions(+), 45 deletions(-) create mode 100644 spec/features/dashboard/project_member_activity_index_spec.rb create mode 100644 spec/models/concerns/expirable_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b018fc0d57..8cb848c3957 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) + - Adds user project membership expired event to clarify why user was removed (Callum Dryden) ## 8.13.0 (2016-10-22) diff --git a/app/models/concerns/expirable.rb b/app/models/concerns/expirable.rb index be93435453b..b66ba08dc59 100644 --- a/app/models/concerns/expirable.rb +++ b/app/models/concerns/expirable.rb @@ -5,11 +5,15 @@ module Expirable scope :expired, -> { where('expires_at <= ?', Time.current) } end + def expired? + expires? && expires_at <= Time.current + end + def expires? expires_at.present? end def expires_soon? - expires_at < 7.days.from_now + expires? && expires_at < 7.days.from_now end end diff --git a/app/models/event.rb b/app/models/event.rb index 0764cb8cabd..3993b35f96d 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -12,6 +12,7 @@ class Event < ActiveRecord::Base JOINED = 8 # User joined project LEFT = 9 # User left project DESTROYED = 10 + EXPIRED = 11 # User left project due to expiry RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour @@ -115,6 +116,10 @@ class Event < ActiveRecord::Base action == LEFT end + def expired? + action == EXPIRED + end + def destroyed? action == DESTROYED end @@ -124,7 +129,7 @@ class Event < ActiveRecord::Base end def membership_changed? - joined? || left? + joined? || left? || expired? end def created_project? @@ -184,6 +189,8 @@ class Event < ActiveRecord::Base 'joined' elsif left? 'left' + elsif expired? + 'removed due to membership expiration from' elsif destroyed? 'destroyed' elsif commented? diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 125f26369d7..e4880973117 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -121,7 +121,11 @@ class ProjectMember < Member end def post_destroy_hook - event_service.leave_project(self.project, self.user) + if expired? + event_service.expired_leave_project(self.project, self.user) + else + event_service.leave_project(self.project, self.user) + end super end diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 07fc77001a5..e24cc66e0fe 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -62,6 +62,10 @@ class EventCreateService create_event(project, current_user, Event::LEFT) end + def expired_leave_project(project, current_user) + create_event(project, current_user, Event::EXPIRED) + end + def create_project(project, current_user) create_event(project, current_user, Event::CREATED) end diff --git a/features/dashboard/dashboard.feature b/features/dashboard/dashboard.feature index 1f4c9020731..b1d5e4a7acb 100644 --- a/features/dashboard/dashboard.feature +++ b/features/dashboard/dashboard.feature @@ -31,19 +31,6 @@ Feature: Dashboard And I click "Create Merge Request" link Then I see prefilled new Merge Request page - @javascript - Scenario: I should see User joined Project event - Given user with name "John Doe" joined project "Shop" - When I visit dashboard activity page - Then I should see "John Doe joined project Shop" event - - @javascript - Scenario: I should see User left Project event - Given user with name "John Doe" joined project "Shop" - And user with name "John Doe" left project "Shop" - When I visit dashboard activity page - Then I should see "John Doe left project Shop" event - @javascript Scenario: Sorting Issues Given I visit dashboard issues page diff --git a/features/steps/dashboard/dashboard.rb b/features/steps/dashboard/dashboard.rb index a7d61bc28e0..b2bec369e0f 100644 --- a/features/steps/dashboard/dashboard.rb +++ b/features/steps/dashboard/dashboard.rb @@ -33,33 +33,6 @@ class Spinach::Features::Dashboard < Spinach::FeatureSteps expect(find("input#merge_request_target_branch").value).to eq "master" end - step 'user with name "John Doe" joined project "Shop"' do - user = create(:user, { name: "John Doe" }) - project.team << [user, :master] - Event.create( - project: project, - author_id: user.id, - action: Event::JOINED - ) - end - - step 'I should see "John Doe joined project Shop" event' do - expect(page).to have_content "John Doe joined project #{project.name_with_namespace}" - end - - step 'user with name "John Doe" left project "Shop"' do - user = User.find_by(name: "John Doe") - Event.create( - project: project, - author_id: user.id, - action: Event::LEFT - ) - end - - step 'I should see "John Doe left project Shop" event' do - expect(page).to have_content "John Doe left project #{project.name_with_namespace}" - end - step 'I have group with projects' do @group = create(:group) @project = create(:project, namespace: @group) diff --git a/lib/event_filter.rb b/lib/event_filter.rb index 96e70e37e8f..21f6a9a762b 100644 --- a/lib/event_filter.rb +++ b/lib/event_filter.rb @@ -45,9 +45,16 @@ class EventFilter when EventFilter.comments actions = [Event::COMMENTED] when EventFilter.team - actions = [Event::JOINED, Event::LEFT] + actions = [Event::JOINED, Event::LEFT, Event::EXPIRED] when EventFilter.all - actions = [Event::PUSHED, Event::MERGED, Event::COMMENTED, Event::JOINED, Event::LEFT] + actions = [ + Event::PUSHED, + Event::MERGED, + Event::COMMENTED, + Event::JOINED, + Event::LEFT, + Event::EXPIRED + ] end events.where(action: actions) diff --git a/spec/features/dashboard/project_member_activity_index_spec.rb b/spec/features/dashboard/project_member_activity_index_spec.rb new file mode 100644 index 00000000000..ba77093a6d4 --- /dev/null +++ b/spec/features/dashboard/project_member_activity_index_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +feature 'Project member activity', feature: true, js: true do + include WaitForAjax + + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public, name: 'x', namespace: user.namespace) } + + before do + project.team << [user, :master] + end + + def visit_activities_and_wait_with_event(event_type) + Event.create(project: project, author_id: user.id, action: event_type) + visit activity_namespace_project_path(project.namespace.path, project.path) + wait_for_ajax + end + + subject { page.find(".event-title").text } + + context 'when a user joins the project' do + before { visit_activities_and_wait_with_event(Event::JOINED) } + + it { is_expected.to eq("#{user.name} joined project") } + end + + context 'when a user leaves the project' do + before { visit_activities_and_wait_with_event(Event::LEFT) } + + it { is_expected.to eq("#{user.name} left project") } + end + + context 'when a users membership expires for the project' do + before { visit_activities_and_wait_with_event(Event::EXPIRED) } + + it "presents the correct message" do + message = "#{user.name} removed due to membership expiration from project" + is_expected.to eq(message) + end + end +end diff --git a/spec/models/concerns/expirable_spec.rb b/spec/models/concerns/expirable_spec.rb new file mode 100644 index 00000000000..f7b436f32e6 --- /dev/null +++ b/spec/models/concerns/expirable_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Expirable do + describe 'ProjectMember' do + let(:no_expire) { create(:project_member) } + let(:expire_later) { create(:project_member, expires_at: Time.current + 6.days) } + let(:expired) { create(:project_member, expires_at: Time.current - 6.days) } + + describe '.expired' do + it { expect(ProjectMember.expired).to match_array([expired]) } + end + + describe '#expired?' do + it { expect(no_expire.expired?).to eq(false) } + it { expect(expire_later.expired?).to eq(false) } + it { expect(expired.expired?).to eq(true) } + end + + describe '#expires?' do + it { expect(no_expire.expires?).to eq(false) } + it { expect(expire_later.expires?).to eq(true) } + it { expect(expired.expires?).to eq(true) } + end + + describe '#expires_soon?' do + it { expect(no_expire.expires_soon?).to eq(false) } + it { expect(expire_later.expires_soon?).to eq(true) } + it { expect(expired.expires_soon?).to eq(true) } + end + end +end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 733b79079ed..aca49be2942 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -40,6 +40,33 @@ describe Event, models: true do end end + describe '#membership_changed?' do + context "created" do + subject { build(:event, action: Event::CREATED).membership_changed? } + it { is_expected.to be_falsey } + end + + context "updated" do + subject { build(:event, action: Event::UPDATED).membership_changed? } + it { is_expected.to be_falsey } + end + + context "expired" do + subject { build(:event, action: Event::EXPIRED).membership_changed? } + it { is_expected.to be_truthy } + end + + context "left" do + subject { build(:event, action: Event::LEFT).membership_changed? } + it { is_expected.to be_truthy } + end + + context "joined" do + subject { build(:event, action: Event::JOINED).membership_changed? } + it { is_expected.to be_truthy } + end + end + describe '#note?' do subject { Event.new(project: target.project, target: target) } diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index d85a1c1e3b2..b2fe96e2e02 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -54,6 +54,17 @@ describe ProjectMember, models: true do master_todos end + it "creates an expired event when left due to expiry" do + expired = create(:project_member, project: project, expires_at: Time.now - 6.days) + expired.destroy + expect(Event.first.action).to eq(Event::EXPIRED) + end + + it "creates a left event when left due to leave" do + master.destroy + expect(Event.first.action).to eq(Event::LEFT) + end + it "destroys itself and delete associated todos" do expect(owner.user.todos.size).to eq(2) expect(master.user.todos.size).to eq(3) diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index 16a9956fe7f..b7dc99ed887 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -110,4 +110,23 @@ describe EventCreateService, services: true do end end end + + describe 'Project' do + let(:user) { create :user } + let(:project) { create(:empty_project) } + + describe '#join_project' do + subject { service.join_project(project, user) } + + it { is_expected.to be_truthy } + it { expect { subject }.to change { Event.count }.from(0).to(1) } + end + + describe '#expired_leave_project' do + subject { service.expired_leave_project(project, user) } + + it { is_expected.to be_truthy } + it { expect { subject }.to change { Event.count }.from(0).to(1) } + end + end end -- cgit v1.2.1 From e6f515ecbed85b204e8f7bc9934b0bf208a0ea4c Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 19 Oct 2016 16:53:12 +0100 Subject: Revert "Add #closed_without_source_project?" This reverts commit 31c37c6c38258684fc92e0d91119c33872e39034. See #23341 --- app/models/merge_request.rb | 10 ++----- app/views/projects/merge_requests/_show.html.haml | 31 ++++++++++------------ spec/models/merge_request_spec.rb | 32 ----------------------- 3 files changed, 16 insertions(+), 57 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0cc0b3c2a0e..d32bc9f882f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -333,11 +333,7 @@ class MergeRequest < ActiveRecord::Base end def closed_without_fork? - closed? && forked_source_project_missing? - end - - def closed_without_source_project? - closed? && !source_project + closed? && (forked_source_project_missing? || !source_project) end def forked_source_project_missing? @@ -348,9 +344,7 @@ class MergeRequest < ActiveRecord::Base end def reopenable? - return false if closed_without_fork? || closed_without_source_project? || merged? - - closed? + source_branch_exists? && closed? end def ensure_merge_request_diff diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index cd98aaf8d75..fe90383b00f 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -26,19 +26,17 @@ %ul.dropdown-menu.dropdown-menu-align-right %li= link_to "Email Patches", merge_request_path(@merge_request, format: :patch) %li= link_to "Plain Diff", merge_request_path(@merge_request, format: :diff) - - unless @merge_request.closed_without_fork? - .normal - %span Request to merge - %span.label-branch= source_branch_with_namespace(@merge_request) - %span into - %span.label-branch - = link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch) - - if @merge_request.open? && @merge_request.diverged_from_target_branch? - %span (#{pluralize(@merge_request.diverged_commits_count, 'commit')} behind) + .normal + %span Request to merge + %span.label-branch= source_branch_with_namespace(@merge_request) + %span into + %span.label-branch + = link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch) + - if @merge_request.open? && @merge_request.diverged_from_target_branch? + %span (#{pluralize(@merge_request.diverged_commits_count, 'commit')} behind) - - unless @merge_request.closed_without_source_project? - = render "projects/merge_requests/show/how_to_merge" - = render "projects/merge_requests/widget/show.html.haml" + = render "projects/merge_requests/show/how_to_merge" + = render "projects/merge_requests/widget/show.html.haml" - if @merge_request.source_branch_exists? && @merge_request.mergeable? && @merge_request.can_be_merged_by?(current_user) .light.prepend-top-default.append-bottom-default @@ -52,11 +50,10 @@ = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do Discussion %span.badge= @merge_request.mr_and_commit_notes.user.count - - unless @merge_request.closed_without_source_project? - %li.commits-tab - = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do - Commits - %span.badge= @commits_count + %li.commits-tab + = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do + Commits + %span.badge= @commits_count - if @pipeline %li.pipelines-tab = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6db5e7f7d80..ee003a9d18f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1274,38 +1274,6 @@ describe MergeRequest, models: true do end end - describe '#closed_without_source_project?' do - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:fork_project) { create(:project, forked_from_project: project, namespace: user.namespace) } - let(:destroy_service) { Projects::DestroyService.new(fork_project, user) } - - context 'when the merge request is closed' do - let(:closed_merge_request) do - create(:closed_merge_request, - source_project: fork_project, - target_project: project) - end - - it 'returns false if the source project exists' do - expect(closed_merge_request.closed_without_source_project?).to be_falsey - end - - it 'returns true if the source project does not exist' do - destroy_service.execute - closed_merge_request.reload - - expect(closed_merge_request.closed_without_source_project?).to be_truthy - end - end - - context 'when the merge request is open' do - it 'returns false' do - expect(subject.closed_without_source_project?).to be_falsey - end - end - end - describe '#reopenable?' do context 'when the merge request is closed' do it 'returns true' do -- cgit v1.2.1 From a4ef4244d4b9b54246f53bba5c02746542034da5 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 20 Oct 2016 16:16:57 +0800 Subject: Preserve note_type and position for notes from emails Closes #23208 --- lib/gitlab/email/handler/create_note_handler.rb | 4 +++- spec/fixtures/emails/commands_in_reply.eml | 2 -- spec/fixtures/emails/commands_only_reply.eml | 2 -- .../email/handler/create_note_handler_spec.rb | 24 ++++++++++++---------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb index 06dae31cc27..447c7a6a6b9 100644 --- a/lib/gitlab/email/handler/create_note_handler.rb +++ b/lib/gitlab/email/handler/create_note_handler.rb @@ -46,7 +46,9 @@ module Gitlab noteable_type: sent_notification.noteable_type, noteable_id: sent_notification.noteable_id, commit_id: sent_notification.commit_id, - line_code: sent_notification.line_code + line_code: sent_notification.line_code, + position: sent_notification.position, + type: sent_notification.note_type ).execute end end diff --git a/spec/fixtures/emails/commands_in_reply.eml b/spec/fixtures/emails/commands_in_reply.eml index 06bf60ab734..712f6f797b4 100644 --- a/spec/fixtures/emails/commands_in_reply.eml +++ b/spec/fixtures/emails/commands_in_reply.eml @@ -23,8 +23,6 @@ Cool! /close /todo -/due tomorrow - On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta wrote: diff --git a/spec/fixtures/emails/commands_only_reply.eml b/spec/fixtures/emails/commands_only_reply.eml index aed64224b06..2d2e2f94290 100644 --- a/spec/fixtures/emails/commands_only_reply.eml +++ b/spec/fixtures/emails/commands_only_reply.eml @@ -21,8 +21,6 @@ X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 /close /todo -/due tomorrow - On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta wrote: diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index 4909fed6b77..48660d1dd1b 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -12,10 +12,13 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do let(:email_raw) { fixture_file('emails/valid_reply.eml') } let(:project) { create(:project, :public) } - let(:noteable) { create(:issue, project: project) } let(:user) { create(:user) } + let(:note) { create(:diff_note_on_merge_request, project: project) } + let(:noteable) { note.noteable } - let!(:sent_notification) { SentNotification.record(noteable, user.id, mail_key) } + let!(:sent_notification) do + SentNotification.record_note(note, user.id, mail_key) + end context "when the recipient address doesn't include a mail key" do let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "") } @@ -82,7 +85,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do expect { receiver.execute }.to change { noteable.notes.count }.by(1) expect(noteable.reload).to be_closed - expect(noteable.due_date).to eq(Date.tomorrow) expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy end end @@ -100,7 +102,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do expect { receiver.execute }.to change { noteable.notes.count }.by(1) expect(noteable.reload).to be_open - expect(noteable.due_date).to be_nil expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy end end @@ -117,7 +118,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do expect { receiver.execute }.to change { noteable.notes.count }.by(2) expect(noteable.reload).to be_closed - expect(noteable.due_date).to eq(Date.tomorrow) expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy end end @@ -138,10 +138,11 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do it "creates a comment" do expect { receiver.execute }.to change { noteable.notes.count }.by(1) - note = noteable.notes.last + new_note = noteable.notes.last - expect(note.author).to eq(sent_notification.recipient) - expect(note.note).to include("I could not disagree more.") + expect(new_note.author).to eq(sent_notification.recipient) + expect(new_note.position).to eq(note.position) + expect(new_note.note).to include("I could not disagree more.") end it "adds all attachments" do @@ -160,10 +161,11 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do shared_examples 'an email that contains a mail key' do |header| it "fetches the mail key from the #{header} header and creates a comment" do expect { receiver.execute }.to change { noteable.notes.count }.by(1) - note = noteable.notes.last + new_note = noteable.notes.last - expect(note.author).to eq(sent_notification.recipient) - expect(note.note).to include('I could not disagree more.') + expect(new_note.author).to eq(sent_notification.recipient) + expect(new_note.position).to eq(note.position) + expect(new_note.note).to include('I could not disagree more.') end end -- cgit v1.2.1 From 80ca78fa47068d71e866b58d546d0cd5059d96d9 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 20 Oct 2016 16:24:37 +0800 Subject: Add CHANGELOG entry [ci skip] --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b018fc0d57..d6e6690ced1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) + - Fix discussion thread from emails for merge requests. !7010 + ## 8.13.0 (2016-10-22) - Fix save button on project pipeline settings page. (!6955) -- cgit v1.2.1 From fd2c3a3da0302a474d7c1adbd409aedea2a41053 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 19 Oct 2016 19:43:04 +0300 Subject: Refactoring find_commits functionality --- app/controllers/projects/commits_controller.rb | 2 +- app/models/repository.rb | 9 ++++++--- lib/gitlab/project_search_results.rb | 6 +----- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index a52c614b259..c2e7bf1ffec 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -13,7 +13,7 @@ class Projects::CommitsController < Projects::ApplicationController @commits = if search.present? - @repository.find_commits_by_message(search, @ref, @path, @limit, @offset).compact + @repository.find_commits_by_message(search, @ref, @path, @limit, @offset) else @repository.commits(@ref, path: @path, limit: @limit, offset: @offset) end diff --git a/app/models/repository.rb b/app/models/repository.rb index 72e473871fa..1b7f20a2134 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -109,6 +109,10 @@ class Repository end def find_commits_by_message(query, ref = nil, path = nil, limit = 1000, offset = 0) + unless exists? && has_visible_content? && query.present? + return [] + end + ref ||= root_ref args = %W( @@ -117,9 +121,8 @@ class Repository ) args = args.concat(%W(-- #{path})) if path.present? - git_log_results = Gitlab::Popen.popen(args, path_to_repo).first.lines.map(&:chomp) - commits = git_log_results.map { |c| commit(c) } - commits + git_log_results = Gitlab::Popen.popen(args, path_to_repo).first.lines + git_log_results.map { |c| commit(c.chomp) }.compact end def find_branch(name, fresh_repo: true) diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 5b9cfaeb2f8..24733435a5a 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -73,11 +73,7 @@ module Gitlab end def commits - if project.empty_repo? || query.blank? - [] - else - project.repository.find_commits_by_message(query).compact - end + project.repository.find_commits_by_message(query) end def project_ids_relation -- cgit v1.2.1 From c9108442b5ac33514801c38412bed5e7b13677b8 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 19 Oct 2016 14:48:37 +0200 Subject: Update duration at the end of pipeline --- CHANGELOG.md | 1 + app/models/ci/pipeline.rb | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfde2cc81e5..cb43cb4b307 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Add `/projects/visible` API endpoint (Ben Boeckel) - Fix centering of custom header logos (Ashley Dumaine) - Keep around commits only pipeline creation as pipeline data doesn't change over time + - Update duration at the end of pipeline - ExpireBuildArtifactsWorker query builds table without ordering enqueuing one job per build to cleanup - Add group level labels. (!6425) - Add an example for testing a phoenix application with Gitlab CI in the docs (Manthan Mallikarjun) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index e84c91b417d..d5c1e03b461 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -59,9 +59,6 @@ module Ci before_transition any => [:success, :failed, :canceled] do |pipeline| pipeline.finished_at = Time.now - end - - before_transition do |pipeline| pipeline.update_duration end -- cgit v1.2.1 From d9d0b81bc6117f9c01c28c1fab597f556b56f369 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 20 Oct 2016 12:10:27 +0200 Subject: Make label API spec independent of order --- spec/requests/api/labels_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 1da9988978b..867bc615b97 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -22,8 +22,7 @@ describe API::API, api: true do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.size).to eq(2) - expect(json_response.first['name']).to eq(group_label.name) - expect(json_response.second['name']).to eq(label1.name) + expect(json_response.map { |l| l['name'] }).to match_array([group_label.name, label1.name]) end end -- cgit v1.2.1 From 50288a6ace0e75aa3e6c09c534f0be9800c95836 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 20 Oct 2016 10:27:30 +0100 Subject: Removed code from project members controller This code was meant to be added to another branch as an expirement, but instead was commited to wrong branch --- app/controllers/projects/project_members_controller.rb | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 37a86ed0523..2a07d154853 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -32,21 +32,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController current_user: current_user ) - if params[:group_ids].present? - group_ids = params[:group_ids].split(',') - groups = Group.where(id: group_ids) - - groups.each do |group| - next unless can?(current_user, :read_group, group) - - project.project_group_links.create( - group: group, - group_access: params[:access_level], - expires_at: params[:expires_at] - ) - end - end - redirect_to namespace_project_project_members_path(@project.namespace, @project) end -- cgit v1.2.1 From bc31a489dd8c329cf011ac898498735809c319dd Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 20 Oct 2016 12:59:39 +0200 Subject: Restrict ProjectCacheWorker jobs to one per 15 min This ensures ProjectCacheWorker jobs for a given project are performed at most once per 15 minutes. This should reduce disk load a bit in cases where there are multiple pushes happening (which should schedule multiple ProjectCacheWorker jobs). --- CHANGELOG.md | 3 ++- app/workers/project_cache_worker.rb | 27 ++++++++++++++++++++++ spec/workers/project_cache_worker_spec.rb | 38 +++++++++++++++++++++++-------- 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 399a7f65267..ba5d72d3ef5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) - Adds user project membership expired event to clarify why user was removed (Callum Dryden) - - Simpler arguments passed to named_route on toggle_award_url helper method + - Simpler arguments passed to named_route on toggle_award_url helper method ## 8.13.0 (2016-10-22) @@ -35,6 +35,7 @@ Please view this file on the master branch, on stable branches it's out of date. - AbstractReferenceFilter caches project_refs on RequestStore when active - Replaced the check sign to arrow in the show build view. !6501 - Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar) + - ProjectCacheWorker updates caches at most once per 15 minutes per project - Fix Error 500 when viewing old merge requests with bad diff data - Create a new /templates namespace for the /licenses, /gitignores and /gitlab_ci_ymls API endpoints. !5717 (tbalthazar) - Speed-up group milestones show page diff --git a/app/workers/project_cache_worker.rb b/app/workers/project_cache_worker.rb index ccefd0f71a0..0d524e88dc3 100644 --- a/app/workers/project_cache_worker.rb +++ b/app/workers/project_cache_worker.rb @@ -1,9 +1,30 @@ +# Worker for updating any project specific caches. +# +# This worker runs at most once every 15 minutes per project. This is to ensure +# that multiple instances of jobs for this worker don't hammer the underlying +# storage engine as much. class ProjectCacheWorker include Sidekiq::Worker sidekiq_options queue: :default + LEASE_TIMEOUT = 15.minutes.to_i + def perform(project_id) + if try_obtain_lease_for(project_id) + Rails.logger. + info("Obtained ProjectCacheWorker lease for project #{project_id}") + else + Rails.logger. + info("Could not obtain ProjectCacheWorker lease for project #{project_id}") + + return + end + + update_caches(project_id) + end + + def update_caches(project_id) project = Project.find(project_id) return unless project.repository.exists? @@ -15,4 +36,10 @@ class ProjectCacheWorker project.repository.build_cache end end + + def try_obtain_lease_for(project_id) + Gitlab::ExclusiveLease. + new("project_cache_worker:#{project_id}", timeout: LEASE_TIMEOUT). + try_obtain + end end diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb index 5785a6a06ff..f5b60b90d11 100644 --- a/spec/workers/project_cache_worker_spec.rb +++ b/spec/workers/project_cache_worker_spec.rb @@ -6,21 +6,39 @@ describe ProjectCacheWorker do subject { described_class.new } describe '#perform' do - it 'updates project cache data' do - expect_any_instance_of(Repository).to receive(:size) - expect_any_instance_of(Repository).to receive(:commit_count) + context 'when an exclusive lease can be obtained' do + before do + allow(subject).to receive(:try_obtain_lease_for).with(project.id). + and_return(true) + end - expect_any_instance_of(Project).to receive(:update_repository_size) - expect_any_instance_of(Project).to receive(:update_commit_count) + it 'updates project cache data' do + expect_any_instance_of(Repository).to receive(:size) + expect_any_instance_of(Repository).to receive(:commit_count) - subject.perform(project.id) + expect_any_instance_of(Project).to receive(:update_repository_size) + expect_any_instance_of(Project).to receive(:update_commit_count) + + subject.perform(project.id) + end + + it 'handles missing repository data' do + expect_any_instance_of(Repository).to receive(:exists?).and_return(false) + expect_any_instance_of(Repository).not_to receive(:size) + + subject.perform(project.id) + end end - it 'handles missing repository data' do - expect_any_instance_of(Repository).to receive(:exists?).and_return(false) - expect_any_instance_of(Repository).not_to receive(:size) + context 'when an exclusive lease can not be obtained' do + it 'does nothing' do + allow(subject).to receive(:try_obtain_lease_for).with(project.id). + and_return(false) + + expect(subject).not_to receive(:update_caches) - subject.perform(project.id) + subject.perform(project.id) + end end end end -- cgit v1.2.1 From 374071321d0cfb7a161ec38e85e27e1d46ae7c9a Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 19 Oct 2016 17:13:04 +0100 Subject: Fix the merge request view when source projects or branches are removed --- app/controllers/projects/merge_requests_controller.rb | 2 +- app/helpers/merge_requests_helper.rb | 10 +++++++--- app/models/merge_request.rb | 15 ++++++--------- app/views/projects/merge_requests/_show.html.haml | 13 ++++++++----- spec/features/merge_requests/created_from_fork_spec.rb | 14 ++++++++++++++ 5 files changed, 36 insertions(+), 18 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 0f593d1a936..55ea31e48a0 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -554,7 +554,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_pipelines_vars @pipelines = @merge_request.all_pipelines - if @pipelines.any? + if @pipelines.present? @pipeline = @pipelines.first @statuses = @pipeline.statuses.relevant end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 249cb44e9d5..a6659ea2fd1 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -86,11 +86,15 @@ module MergeRequestsHelper end def source_branch_with_namespace(merge_request) - branch = link_to(merge_request.source_branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch)) + namespace = merge_request.source_project_namespace + branch = merge_request.source_branch + + if merge_request.source_branch_exists? + namespace = link_to(namespace, project_path(merge_request.source_project)) + branch = link_to(branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch)) + end if merge_request.for_fork? - namespace = link_to(merge_request.source_project_namespace, - project_path(merge_request.source_project)) namespace + ":" + branch else branch diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index d32bc9f882f..45fa8af60e5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -333,7 +333,7 @@ class MergeRequest < ActiveRecord::Base end def closed_without_fork? - closed? && (forked_source_project_missing? || !source_project) + closed? && forked_source_project_missing? end def forked_source_project_missing? @@ -344,7 +344,7 @@ class MergeRequest < ActiveRecord::Base end def reopenable? - source_branch_exists? && closed? + closed? && !source_project_missing? && source_branch_exists? end def ensure_merge_request_diff @@ -656,7 +656,7 @@ class MergeRequest < ActiveRecord::Base end def has_ci? - source_project.ci_service && commits.any? + source_project.try(:ci_service) && commits.any? end def branch_missing? @@ -688,12 +688,9 @@ class MergeRequest < ActiveRecord::Base @environments ||= begin - environments = source_project.environments_for( - source_branch, diff_head_commit) - environments += target_project.environments_for( - target_branch, diff_head_commit, with_tags: true) - - environments.uniq + envs = target_project.environments_for(target_branch, diff_head_commit, with_tags: true) + envs.concat(source_project.environments_for(source_branch, diff_head_commit)) if source_project + envs.uniq end end diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index fe90383b00f..0e19d224fcd 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -35,7 +35,9 @@ - if @merge_request.open? && @merge_request.diverged_from_target_branch? %span (#{pluralize(@merge_request.diverged_commits_count, 'commit')} behind) - = render "projects/merge_requests/show/how_to_merge" + - if @merge_request.source_branch_exists? + = render "projects/merge_requests/show/how_to_merge" + = render "projects/merge_requests/widget/show.html.haml" - if @merge_request.source_branch_exists? && @merge_request.mergeable? && @merge_request.can_be_merged_by?(current_user) @@ -50,10 +52,11 @@ = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do Discussion %span.badge= @merge_request.mr_and_commit_notes.user.count - %li.commits-tab - = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do - Commits - %span.badge= @commits_count + - if @merge_request.source_project + %li.commits-tab + = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do + Commits + %span.badge= @commits_count - if @pipeline %li.pipelines-tab = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb index a506624b30d..cfc1244429f 100644 --- a/spec/features/merge_requests/created_from_fork_spec.rb +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -25,6 +25,20 @@ feature 'Merge request created from fork' do expect(page).to have_content 'Test merge request' end + context 'source project is deleted' do + background do + MergeRequests::MergeService.new(project, user).execute(merge_request) + fork_project.destroy! + end + + scenario 'user can access merge request' do + visit_merge_request(merge_request) + + expect(page).to have_content 'Test merge request' + expect(page).to have_content "(removed):#{merge_request.source_branch}" + end + end + context 'pipeline present in source project' do include WaitForAjax -- cgit v1.2.1 From 6f846fcbe8fa129e240ae58c8d4bbb1e1a82bbef Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 19 Oct 2016 17:48:30 +0100 Subject: Fix two CI endpoints for MRs where the source project is deleted --- app/controllers/projects/merge_requests_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 55ea31e48a0..2ee53f7ceda 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -398,7 +398,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController status ||= "preparing" else - ci_service = @merge_request.source_project.ci_service + ci_service = @merge_request.source_project.try(:ci_service) status = ci_service.commit_status(merge_request.diff_head_sha, merge_request.source_branch) if ci_service if ci_service.respond_to?(:commit_coverage) -- cgit v1.2.1 From 61536ed2cff454bb2e3db8b7ca9ee09cc407461f Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 19 Oct 2016 19:33:51 +0100 Subject: Rename forked_source_project_missing? to source_project_missing? --- app/models/merge_request.rb | 6 +++--- spec/models/merge_request_spec.rb | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 45fa8af60e5..c476a3bb14e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -326,17 +326,17 @@ class MergeRequest < ActiveRecord::Base def validate_fork return true unless target_project && source_project return true if target_project == source_project - return true unless forked_source_project_missing? + return true unless source_project_missing? errors.add :validate_fork, 'Source project is not a fork of the target project' end def closed_without_fork? - closed? && forked_source_project_missing? + closed? && source_project_missing? end - def forked_source_project_missing? + def source_project_missing? return false unless for_fork? return true unless source_project diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ee003a9d18f..6e5137602aa 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1198,7 +1198,7 @@ describe MergeRequest, models: true do end end - describe "#forked_source_project_missing?" do + describe "#source_project_missing?" do let(:project) { create(:project) } let(:fork_project) { create(:project, forked_from_project: project) } let(:user) { create(:user) } @@ -1211,13 +1211,13 @@ describe MergeRequest, models: true do target_project: project) end - it { expect(merge_request.forked_source_project_missing?).to be_falsey } + it { expect(merge_request.source_project_missing?).to be_falsey } end context "when the source project is the same as the target project" do let(:merge_request) { create(:merge_request, source_project: project) } - it { expect(merge_request.forked_source_project_missing?).to be_falsey } + it { expect(merge_request.source_project_missing?).to be_falsey } end context "when the fork does not exist" do @@ -1231,7 +1231,7 @@ describe MergeRequest, models: true do unlink_project.execute merge_request.reload - expect(merge_request.forked_source_project_missing?).to be_truthy + expect(merge_request.source_project_missing?).to be_truthy end end end -- cgit v1.2.1 From 0bbfdde64d3de61df1f87e2394c0f459512c7f51 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 19 Oct 2016 18:09:33 +0100 Subject: Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b018fc0d57..edaa1432757 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar) - Fix Error 500 when viewing old merge requests with bad diff data - Create a new /templates namespace for the /licenses, /gitignores and /gitlab_ci_ymls API endpoints. !5717 (tbalthazar) + - Fix viewing merged MRs when the source project has been removed !6991 - Speed-up group milestones show page - Fix inconsistent options dropdown caret on mobile viewports (ClemMakesApps) - Extract project#update_merge_requests and SystemHooks to its own worker from GitPushService -- cgit v1.2.1 From 8815748feb0c9bbef9b04f07f09645ba94ea4d63 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Thu, 13 Oct 2016 13:58:31 -0500 Subject: Change input order on Sign In form for better tabbing. This *unreverts* 8751491b, which was mistakenly reverted in !6328. It also changes the implementation of the original commit to work with the new login styling and markup. cc: @ClemMakesApps --- app/assets/stylesheets/pages/login.scss | 17 +++++++++++++++++ app/views/devise/sessions/_new_base.html.haml | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index e6d9be5185d..bdb13bee178 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -53,6 +53,7 @@ margin: 0 0 10px; } + .login-footer { margin-top: 10px; @@ -246,3 +247,19 @@ padding: 65px; // height of footer + bottom padding of email confirmation link } } + +// For sign in pane only, to improve tab order, the following removes the submit button from +// normal document flow and pins it to the bottom of the form. For context, see !6867 & !6928 + +.login-box { + .new_user { + position: relative; + padding-bottom: 35px; + } + + .move-submit-down { + position: absolute; + width: 100%; + bottom: 0; + } +} diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index a96b579c593..525e7d99d71 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -5,6 +5,8 @@ %div.form-group = f.label :password = f.password_field :password, class: "form-control bottom", required: true, title: "This field is required." + %div.submit-container.move-submit-down + = f.submit "Sign in", class: "btn btn-save" - if devise_mapping.rememberable? .remember-me.checkbox %label{for: "user_remember_me"} @@ -12,5 +14,3 @@ %span Remember me .pull-right = link_to "Forgot your password?", new_password_path(resource_name) - %div.submit-container - = f.submit "Sign in", class: "btn btn-save" -- cgit v1.2.1 From c1212beaa495b5b0c3b805bb1b2a7bdc6d1a941b Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Wed, 19 Oct 2016 14:49:09 +0200 Subject: Use deployment IID when saving refs --- app/models/deployment.rb | 2 +- app/models/environment.rb | 4 ++-- spec/controllers/projects/merge_requests_controller_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 1f8c5fb3d85..c843903877b 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -102,6 +102,6 @@ class Deployment < ActiveRecord::Base private def ref_path - File.join(environment.ref_path, 'deployments', id.to_s) + File.join(environment.ref_path, 'deployments', iid.to_s) end end diff --git a/app/models/environment.rb b/app/models/environment.rb index d575f1dc73a..73f415c0ef0 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -71,8 +71,8 @@ class Environment < ActiveRecord::Base return nil unless ref - deployment_id = ref.split('/').last - deployments.find(deployment_id) + deployment_iid = ref.split('/').last + deployments.find_by(iid: deployment_iid) end def ref_path diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index d6980471ea4..940d54f8686 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -913,7 +913,7 @@ describe Projects::MergeRequestsController do end describe 'GET ci_environments_status' do - context 'when the environment is from a forked project' do + context 'the environment is from a forked project' do let!(:forked) { create(:project) } let!(:environment) { create(:environment, project: forked) } let!(:deployment) { create(:deployment, environment: environment, sha: forked.commit.id, ref: 'master') } -- cgit v1.2.1 From 55365d368a069d6598f58d71d8891d6c8f06ea66 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Thu, 20 Oct 2016 14:21:40 +0200 Subject: Only create refs for new deployments This patch makes sure GitLab does not save the refs to the filesystem each time the deployment is updated. This will save some IO although I expect the impact to be minimal. --- app/models/deployment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/deployment.rb b/app/models/deployment.rb index c843903877b..91d85c2279b 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -11,7 +11,7 @@ class Deployment < ActiveRecord::Base delegate :name, to: :environment, prefix: true - after_save :create_ref + after_create :create_ref def commit project.commit(sha) -- cgit v1.2.1 From b5d210c9d6d44fc664b006567b8f488e198ad04f Mon Sep 17 00:00:00 2001 From: Airat Shigapov Date: Thu, 15 Sep 2016 18:45:22 +0300 Subject: Render hipchat notification descriptions as HTML instead of raw markdown --- app/models/project_services/hipchat_service.rb | 9 +++------ spec/models/project_services/hipchat_service_spec.rb | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index afebd3b6a12..98f0312d84e 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -144,8 +144,7 @@ class HipchatService < Service message = "#{user_name} #{state} #{issue_link} in #{project_link}: #{title}" if description - description = format_body(description) - message << description + message << format_body(Banzai::Filter::MarkdownFilter.renderer.render(description)) end message @@ -167,8 +166,7 @@ class HipchatService < Service "#{project_link}: #{title}" if description - description = format_body(description) - message << description + message << format_body(Banzai::Filter::MarkdownFilter.renderer.render(description)) end message @@ -219,8 +217,7 @@ class HipchatService < Service message << title if note - note = format_body(note) - message << note + message << format_body(Banzai::Filter::MarkdownFilter.renderer.render(note)) end message diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 26dd95bdfec..90066f01f6f 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -117,7 +117,7 @@ describe HipchatService, models: true do end context 'issue events' do - let(:issue) { create(:issue, title: 'Awesome issue', description: 'please fix') } + let(:issue) { create(:issue, title: 'Awesome issue', description: '**please** fix') } let(:issue_service) { Issues::CreateService.new(project, user) } let(:issues_sample_data) { issue_service.hook_data(issue, 'open') } @@ -135,12 +135,12 @@ describe HipchatService, models: true do "issue ##{obj_attr["iid"]} in " \ "#{project_name}: " \ "Awesome issue" \ - "
please fix
") + "

please fix

\n
") end end context 'merge request events' do - let(:merge_request) { create(:merge_request, description: 'please fix', title: 'Awesome merge request', target_project: project, source_project: project) } + let(:merge_request) { create(:merge_request, description: '**please** fix', title: 'Awesome merge request', target_project: project, source_project: project) } let(:merge_service) { MergeRequests::CreateService.new(project, user) } let(:merge_sample_data) { merge_service.hook_data(merge_request, 'open') } @@ -159,7 +159,7 @@ describe HipchatService, models: true do "merge request !#{obj_attr["iid"]} in " \ "#{project_name}: " \ "Awesome merge request" \ - "
please fix
") + "

please fix

\n
") end end @@ -190,7 +190,7 @@ describe HipchatService, models: true do "commit #{commit_id} in " \ "#{project_name}: " \ "#{title}" \ - "
a comment on a commit
") + "

a comment on a commit

\n
") end end @@ -203,7 +203,7 @@ describe HipchatService, models: true do let(:merge_request_note) do create(:note_on_merge_request, noteable: merge_request, project: project, - note: "merge request note") + note: "merge request **note**") end it "calls Hipchat API for merge request comment events" do @@ -222,7 +222,7 @@ describe HipchatService, models: true do "merge request !#{merge_id} in " \ "#{project_name}: " \ "#{title}" \ - "
merge request note
") + "

merge request note

\n
") end end @@ -230,7 +230,7 @@ describe HipchatService, models: true do let(:issue) { create(:issue, project: project) } let(:issue_note) do create(:note_on_issue, noteable: issue, project: project, - note: "issue note") + note: "issue **note**") end it "calls Hipchat API for issue comment events" do @@ -247,7 +247,7 @@ describe HipchatService, models: true do "issue ##{issue_id} in " \ "#{project_name}: " \ "#{title}" \ - "
issue note
") + "

issue note

\n
") end end @@ -275,7 +275,7 @@ describe HipchatService, models: true do "snippet ##{snippet_id} in " \ "#{project_name}: " \ "#{title}" \ - "
snippet note
") + "

snippet note

\n
") end end end -- cgit v1.2.1 From 3d034c15017b45611cb3e52744f09cc45c1c7bbe Mon Sep 17 00:00:00 2001 From: David Eisner Date: Tue, 4 Oct 2016 12:28:42 +0100 Subject: Full Banzai rendering for HipChat notifications --- app/models/project_services/hipchat_service.rb | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 98f0312d84e..f4edbbbceb2 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -121,14 +121,6 @@ class HipchatService < Service message end - def format_body(body) - if body - body = body.truncate(200, separator: ' ', omission: '...') - end - - "
#{body}
" - end - def create_issue_message(data) user_name = data[:user][:name] @@ -144,7 +136,7 @@ class HipchatService < Service message = "#{user_name} #{state} #{issue_link} in #{project_link}: #{title}" if description - message << format_body(Banzai::Filter::MarkdownFilter.renderer.render(description)) + message << Banzai.render(note, project: project) end message @@ -166,7 +158,7 @@ class HipchatService < Service "#{project_link}: #{title}" if description - message << format_body(Banzai::Filter::MarkdownFilter.renderer.render(description)) + message << Banzai.render(note, project: project) end message @@ -217,7 +209,7 @@ class HipchatService < Service message << title if note - message << format_body(Banzai::Filter::MarkdownFilter.renderer.render(note)) + message << Banzai.render(note, project: project) end message -- cgit v1.2.1 From 8e0c868436db61be8020356f4792cc79c2964363 Mon Sep 17 00:00:00 2001 From: David Eisner Date: Tue, 4 Oct 2016 12:38:31 +0100 Subject: Also render commit titles in HipChat notifications --- app/models/project_services/hipchat_service.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index f4edbbbceb2..7d70452a70b 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -88,6 +88,10 @@ class HipchatService < Service end end + def render_line(text) + Banzai.render(text.lines.first.chomp, project: project, pipeline: :single_line) if text + end + def create_push_message(push) ref_type = Gitlab::Git.tag_ref?(push[:ref]) ? 'tag' : 'branch' ref = Gitlab::Git.ref_name(push[:ref]) @@ -110,7 +114,7 @@ class HipchatService < Service message << "(Compare changes)" push[:commits].take(MAX_COMMITS).each do |commit| - message << "
- #{commit[:message].lines.first} (#{commit[:id][0..5]})" + message << "
- #{render_line(commit[:message])} (#{commit[:id][0..5]})" end if push[:commits].count > MAX_COMMITS @@ -126,7 +130,7 @@ class HipchatService < Service obj_attr = data[:object_attributes] obj_attr = HashWithIndifferentAccess.new(obj_attr) - title = obj_attr[:title] + title = render_line(obj_attr[:title]) state = obj_attr[:state] issue_iid = obj_attr[:iid] issue_url = obj_attr[:url] @@ -150,7 +154,7 @@ class HipchatService < Service merge_request_id = obj_attr[:iid] state = obj_attr[:state] description = obj_attr[:description] - title = obj_attr[:title] + title = render_line(obj_attr[:title]) merge_request_url = "#{project_url}/merge_requests/#{merge_request_id}" merge_request_link = "merge request !#{merge_request_id}" @@ -165,7 +169,7 @@ class HipchatService < Service end def format_title(title) - "" + title.lines.first.chomp + "" + "#{render_line(title)}" end def create_note_message(data) -- cgit v1.2.1 From eb074021b0bfeed139e098d06d45b562b98f6214 Mon Sep 17 00:00:00 2001 From: David Eisner Date: Tue, 4 Oct 2016 16:23:16 +0100 Subject: Absolute URLs for Banzai HTML for HipChat Using "pipeline: :email" gets "only_path: false" into the context to produce full URLs instead of /namespace/project/... --- app/models/project_services/hipchat_service.rb | 32 +++++++++++++++++--------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 7d70452a70b..9d52a1423b7 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -89,7 +89,7 @@ class HipchatService < Service end def render_line(text) - Banzai.render(text.lines.first.chomp, project: project, pipeline: :single_line) if text + markdown(text.lines.first.chomp, pipeline: :single_line) if text end def create_push_message(push) @@ -125,6 +125,20 @@ class HipchatService < Service message end + def markdown(text, context = {}) + if text + context = ({ + project: project, + pipeline: :email + }).merge(context) + + html = Banzai.render(text, context) + html = Banzai.post_process(html, context) + else + "" + end + end + def create_issue_message(data) user_name = data[:user][:name] @@ -139,9 +153,7 @@ class HipchatService < Service issue_link = "issue ##{issue_iid}" message = "#{user_name} #{state} #{issue_link} in #{project_link}: #{title}" - if description - message << Banzai.render(note, project: project) - end + message << markdown(description) message end @@ -161,9 +173,7 @@ class HipchatService < Service message = "#{user_name} #{state} #{merge_request_link} in " \ "#{project_link}: #{title}" - if description - message << Banzai.render(note, project: project) - end + message << markdown(description) message end @@ -180,11 +190,13 @@ class HipchatService < Service note = obj_attr[:note] note_url = obj_attr[:url] noteable_type = obj_attr[:noteable_type] + commit_id = nil case noteable_type when "Commit" commit_attr = HashWithIndifferentAccess.new(data[:commit]) - subject_desc = commit_attr[:id] + commit_id = commit_attr[:id] + subject_desc = commit_id subject_desc = Commit.truncate_sha(subject_desc) subject_type = "commit" title = format_title(commit_attr[:message]) @@ -212,9 +224,7 @@ class HipchatService < Service message = "#{user_name} commented on #{subject_html} in #{project_link}: " message << title - if note - message << Banzai.render(note, project: project) - end + message << markdown(note, ref: commit_id) message end -- cgit v1.2.1 From b434b75fd0a5486325dabcf0a2edf652c959675b Mon Sep 17 00:00:00 2001 From: David Eisner Date: Tue, 4 Oct 2016 16:25:45 +0100 Subject: Ensure absolute URLs for single lines from Banzai for HipChat "pipeline: :single_line" leaves the protocol/host part out of the URLs and caches them that way. To avoid giving those out to HipChat, markdown is always rendered with "pipeline: :email" first. There must be a better way to do this, but I can't see how to avoid the link caching. --- app/models/project_services/hipchat_service.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 9d52a1423b7..ce4a2a96015 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -125,12 +125,16 @@ class HipchatService < Service message end - def markdown(text, context = {}) + def markdown(text, options = {}) if text - context = ({ + context = { project: project, pipeline: :email - }).merge(context) + } + + Banzai.render(text, context) + + context.merge!(options) html = Banzai.render(text, context) html = Banzai.post_process(html, context) -- cgit v1.2.1 From aa2406e0f821e217ed5e0c59a212cecd73227509 Mon Sep 17 00:00:00 2001 From: David Eisner Date: Tue, 4 Oct 2016 16:27:40 +0100 Subject: Clean up Banzai HTML for HipChat The `class` and `data-*` attributes are meaningless in HipChat, and it would probably be better to limit the tags, too. For example, we could avoid block-level elements in `render_line`. --- app/models/project_services/hipchat_service.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index ce4a2a96015..8988a7b905e 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -1,4 +1,6 @@ class HipchatService < Service + include ActionView::Helpers::SanitizeHelper + MAX_COMMITS = 3 prop_accessor :token, :room, :server, :notify, :color, :api_version @@ -138,6 +140,7 @@ class HipchatService < Service html = Banzai.render(text, context) html = Banzai.post_process(html, context) + sanitize html, attributes: %w(href title alt) else "" end -- cgit v1.2.1 From 32cf2e5f77c24c27782adc37d4635b06dfada060 Mon Sep 17 00:00:00 2001 From: David Eisner Date: Wed, 5 Oct 2016 13:38:08 +0100 Subject: Tests for markdown HipChat notifications --- spec/models/project_services/hipchat_service_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 90066f01f6f..1029b6d2459 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -135,7 +135,7 @@ describe HipchatService, models: true do "issue ##{obj_attr["iid"]} in " \ "#{project_name}: " \ "Awesome issue" \ - "

please fix

\n
") + "

please fix

") end end @@ -159,7 +159,7 @@ describe HipchatService, models: true do "merge request !#{obj_attr["iid"]} in " \ "#{project_name}: " \ "Awesome merge request" \ - "

please fix

\n
") + "

please fix

") end end @@ -190,7 +190,7 @@ describe HipchatService, models: true do "commit #{commit_id} in " \ "#{project_name}: " \ "#{title}" \ - "

a comment on a commit

\n
") + "

a comment on a commit

") end end @@ -222,7 +222,7 @@ describe HipchatService, models: true do "merge request !#{merge_id} in " \ "#{project_name}: " \ "#{title}" \ - "

merge request note

\n
") + "

merge request note

") end end @@ -247,7 +247,7 @@ describe HipchatService, models: true do "issue ##{issue_id} in " \ "#{project_name}: " \ "#{title}" \ - "

issue note

\n
") + "

issue note

") end end @@ -275,7 +275,7 @@ describe HipchatService, models: true do "snippet ##{snippet_id} in " \ "#{project_name}: " \ "#{title}" \ - "

snippet note

\n
") + "

snippet note

") end end end -- cgit v1.2.1 From 7b90680c2dd78d88d747fff545d69b1908ced7ae Mon Sep 17 00:00:00 2001 From: Airat Shigapov Date: Wed, 12 Oct 2016 09:51:02 +0300 Subject: Use guard clause instead of if-else statement --- app/models/project_services/hipchat_service.rb | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index 8988a7b905e..e7a77070b9f 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -128,22 +128,21 @@ class HipchatService < Service end def markdown(text, options = {}) - if text - context = { - project: project, - pipeline: :email - } + return "" unless text - Banzai.render(text, context) + context = { + project: project, + pipeline: :email + } - context.merge!(options) + Banzai.render(text, context) - html = Banzai.render(text, context) - html = Banzai.post_process(html, context) - sanitize html, attributes: %w(href title alt) - else - "" - end + context.merge!(options) + + html = Banzai.render(text, context) + html = Banzai.post_process(html, context) + + sanitize html, attributes: %w(href title alt) end def create_issue_message(data) -- cgit v1.2.1 From 257d15a67007f7c8750ca6d00a7c13f8e8a9f974 Mon Sep 17 00:00:00 2001 From: Airat Shigapov Date: Wed, 19 Oct 2016 22:51:15 +0300 Subject: Return truncation for notification descriptions, fix minor bugs with rendering --- app/models/project_services/hipchat_service.rb | 17 +++++++++++------ spec/models/project_services/hipchat_service_spec.rb | 12 ++++++------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index e7a77070b9f..660a8ae3421 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -2,6 +2,11 @@ class HipchatService < Service include ActionView::Helpers::SanitizeHelper MAX_COMMITS = 3 + HIPCHAT_ALLOWED_TAGS = %w[ + a b i strong em br img pre code + table th tr td caption colgroup col thead tbody tfoot + ul ol li dl dt dd + ] prop_accessor :token, :room, :server, :notify, :color, :api_version boolean_accessor :notify_only_broken_builds @@ -139,10 +144,10 @@ class HipchatService < Service context.merge!(options) - html = Banzai.render(text, context) - html = Banzai.post_process(html, context) + html = Banzai.post_process(Banzai.render(text, context), context) + sanitized_html = sanitize(html, tags: HIPCHAT_ALLOWED_TAGS, attributes: %w[href title alt]) - sanitize html, attributes: %w(href title alt) + sanitized_html.truncate(200, separator: ' ', omission: '...') end def create_issue_message(data) @@ -159,7 +164,7 @@ class HipchatService < Service issue_link = "issue ##{issue_iid}" message = "#{user_name} #{state} #{issue_link} in #{project_link}: #{title}" - message << markdown(description) + message << "
#{markdown(description)}
" message end @@ -179,7 +184,7 @@ class HipchatService < Service message = "#{user_name} #{state} #{merge_request_link} in " \ "#{project_link}: #{title}" - message << markdown(description) + message << "
#{markdown(description)}
" message end @@ -230,7 +235,7 @@ class HipchatService < Service message = "#{user_name} commented on #{subject_html} in #{project_link}: " message << title - message << markdown(note, ref: commit_id) + message << "
#{markdown(note, ref: commit_id)}
" message end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 1029b6d2459..2da3a9cb09f 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -135,7 +135,7 @@ describe HipchatService, models: true do "issue ##{obj_attr["iid"]} in " \ "#{project_name}: " \ "Awesome issue" \ - "

please fix

") + "
please fix
") end end @@ -159,7 +159,7 @@ describe HipchatService, models: true do "merge request !#{obj_attr["iid"]} in " \ "#{project_name}: " \ "Awesome merge request" \ - "

please fix

") + "
please fix
") end end @@ -190,7 +190,7 @@ describe HipchatService, models: true do "commit #{commit_id} in " \ "#{project_name}: " \ "#{title}" \ - "

a comment on a commit

") + "
a comment on a commit
") end end @@ -222,7 +222,7 @@ describe HipchatService, models: true do "merge request !#{merge_id} in " \ "#{project_name}: " \ "#{title}" \ - "

merge request note

") + "
merge request note
") end end @@ -247,7 +247,7 @@ describe HipchatService, models: true do "issue ##{issue_id} in " \ "#{project_name}: " \ "#{title}" \ - "

issue note

") + "
issue note
") end end @@ -275,7 +275,7 @@ describe HipchatService, models: true do "snippet ##{snippet_id} in " \ "#{project_name}: " \ "#{title}" \ - "

snippet note

") + "
snippet note
") end end end -- cgit v1.2.1 From db7c227fc64de0221d061d58387a93c6925cded7 Mon Sep 17 00:00:00 2001 From: Airat Shigapov Date: Thu, 20 Oct 2016 15:12:39 +0300 Subject: Add CHANGELOG.md entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98cffab8c03..9135b851f96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) - Adds user project membership expired event to clarify why user was removed (Callum Dryden) + - Fix HipChat notifications rendering (airatshigapov, eisnerd) - Simpler arguments passed to named_route on toggle_award_url helper method ## 8.13.0 (2016-10-22) -- cgit v1.2.1 From d1f6dfc863f1560f7e6e1a30e846304979679a61 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Thu, 20 Oct 2016 20:47:21 +0800 Subject: We want to release this in 8.13.0 --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6e6690ced1..4fd0b763e48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,6 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.14.0 (2016-11-22) - - Fix discussion thread from emails for merge requests. !7010 - ## 8.13.0 (2016-10-22) - Fix save button on project pipeline settings page. (!6955) @@ -40,6 +38,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Speed-up group milestones show page - Fix inconsistent options dropdown caret on mobile viewports (ClemMakesApps) - Extract project#update_merge_requests and SystemHooks to its own worker from GitPushService + - Fix discussion thread from emails for merge requests. !7010 - Don't include archived projects when creating group milestones. !4940 (Jeroen Jacobs) - Add tag shortcut from the Commit page. !6543 - Keep refs for each deployment -- cgit v1.2.1 From 2e411b5ea0faf733aa457ecc28064bb48f07deed Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Wed, 19 Oct 2016 15:35:38 +0200 Subject: Test GitLab project import for a user with only their default namespace. Refactor the spec file: - remove hardcoded record IDs - avoid top-level let if not used in all scenarios - prefer expect { ... }.to change { ... }.from(0).to(1) over checking that there are no records at the beginning of the test --- .../projects/import_export/import_file_spec.rb | 53 ++++++++++++---------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index f32834801a0..3015576f6f8 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -3,13 +3,8 @@ require 'spec_helper' feature 'Import/Export - project import integration test', feature: true, js: true do include Select2Helper - let(:admin) { create(:admin) } - let(:normal_user) { create(:user) } - let!(:namespace) { create(:namespace, name: "asd", owner: admin) } let(:file) { File.join(Rails.root, 'spec', 'features', 'projects', 'import_export', 'test_project_export.tar.gz') } let(:export_path) { "#{Dir::tmpdir}/import_file_spec" } - let(:project) { Project.last } - let(:project_hook) { Gitlab::Git::Hook.new('post-receive', project.repository.path) } background do allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) @@ -19,41 +14,43 @@ feature 'Import/Export - project import integration test', feature: true, js: tr FileUtils.rm_rf(export_path, secure: true) end - context 'admin user' do + context 'when selecting the namespace' do + let(:user) { create(:admin) } + let!(:namespace) { create(:namespace, name: "asd", owner: user) } + before do - login_as(admin) + login_as(user) end scenario 'user imports an exported project successfully' do - expect(Project.all.count).to be_zero - visit new_project_path - select2('2', from: '#project_namespace_id') + select2(namespace.id, from: '#project_namespace_id') fill_in :project_path, with: 'test-project-path', visible: true click_link 'GitLab export' expect(page).to have_content('GitLab project export') - expect(URI.parse(current_url).query).to eq('namespace_id=2&path=test-project-path') + expect(URI.parse(current_url).query).to eq("namespace_id=#{namespace.id}&path=test-project-path") attach_file('file', file) - click_on 'Import project' # import starts + expect { click_on 'Import project' }.to change { Project.count }.from(0).to(1) + project = Project.last expect(project).not_to be_nil expect(project.issues).not_to be_empty expect(project.merge_requests).not_to be_empty - expect(project_hook).to exist - expect(wiki_exists?).to be true + expect(project_hook_exists?(project)).to be true + expect(wiki_exists?(project)).to be true expect(project.import_status).to eq('finished') end scenario 'invalid project' do - project = create(:project, namespace_id: 2) + project = create(:project, namespace: namespace) visit new_project_path - select2('2', from: '#project_namespace_id') + select2(namespace.id, from: '#project_namespace_id') fill_in :project_path, with: project.name, visible: true click_link 'GitLab export' @@ -66,11 +63,11 @@ feature 'Import/Export - project import integration test', feature: true, js: tr end scenario 'project with no name' do - create(:project, namespace_id: 2) + create(:project, namespace: namespace) visit new_project_path - select2('2', from: '#project_namespace_id') + select2(namespace.id, from: '#project_namespace_id') # click on disabled element find(:link, 'GitLab export').trigger('click') @@ -81,24 +78,30 @@ feature 'Import/Export - project import integration test', feature: true, js: tr end end - context 'normal user' do + context 'when limited to the default user namespace' do + let(:user) { create(:user) } before do - login_as(normal_user) + login_as(user) end - scenario 'non-admin user is allowed to import a project' do - expect(Project.all.count).to be_zero - + scenario 'passes correct namespace ID in the URL' do visit new_project_path fill_in :project_path, with: 'test-project-path', visible: true - expect(page).to have_content('GitLab export') + click_link 'GitLab export' + + expect(page).to have_content('GitLab project export') + expect(URI.parse(current_url).query).to eq("namespace_id=#{user.namespace.id}&path=test-project-path") end end - def wiki_exists? + def wiki_exists?(project) wiki = ProjectWiki.new(project) File.exist?(wiki.repository.path_to_repo) && !wiki.repository.empty? end + + def project_hook_exists?(project) + Gitlab::Git::Hook.new('post-receive', project.repository.path).exists? + end end -- cgit v1.2.1 From 474313e44034882122c60329b7c72b0f31cac45b Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Wed, 19 Oct 2016 20:42:35 +0200 Subject: Fix GitLab project import when a user has access only to their default namespace. Render a hidden field with namespace ID so it can be read by JavaScript and passed to "/import/gitlab_project/new" screen. --- app/views/projects/new.html.haml | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index cc8cb134fb8..399ccf15b7f 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -27,6 +27,7 @@ - else .input-group-addon.static-namespace #{root_url}#{current_user.username}/ + = f.hidden_field :namespace_id, value: current_user.namespace_id .form-group.col-xs-12.col-sm-6.project-path = f.label :namespace_id, class: 'label-light' do %span -- cgit v1.2.1 From fab393984a4685164886c974747d312da21e1798 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 20 Oct 2016 14:15:39 -0200 Subject: [ci skip] Add a comment explaining validate_board_limit callback Callback associations are not common to see around. We want to make clear that the `before_add` callback uses the number before the addition, in this particular case 1. --- app/models/project.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/models/project.rb b/app/models/project.rb index 6685baab699..af117f0acb0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1339,6 +1339,13 @@ class Project < ActiveRecord::Base shared_projects.any? end + # Similar to the normal callbacks that hook into the life cycle of an + # Active Record object, you can also define callbacks that get triggered + # when you add an object to an association collection. If any of these + # callbacks throw an exception, the object will not be added to the + # collection. Before you add a new board to the boards collection if you + # already have 1, 2, or n it will fail, but it if you have 0 that is lower + # than the number of permitted boards per project it won't fail. def validate_board_limit(board) raise BoardLimitExceeded, 'Number of permitted boards exceeded' if boards.size >= NUMBER_OF_PERMITTED_BOARDS end -- cgit v1.2.1 From 3bace66970ff5cd71107f24511447d8c5a72715f Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 14 Oct 2016 17:25:12 -0500 Subject: Create protected branches bundle --- .../protected_branch_access_dropdown.js.es6 | 28 -------- .../javascripts/protected_branch_create.js.es6 | 54 --------------- .../javascripts/protected_branch_dropdown.js.es6 | 76 ---------------------- .../javascripts/protected_branch_edit.js.es6 | 65 ------------------ .../javascripts/protected_branch_edit_list.js.es6 | 17 ----- .../protected_branch_access_dropdown.js.es6 | 28 ++++++++ .../protected_branch_create.js.es6 | 54 +++++++++++++++ .../protected_branch_dropdown.js.es6 | 76 ++++++++++++++++++++++ .../protected_branch_edit.js.es6 | 65 ++++++++++++++++++ .../protected_branch_edit_list.js.es6 | 17 +++++ .../protected_branches_bundle.js | 1 + .../projects/protected_branches/index.html.haml | 2 + config/application.rb | 1 + 13 files changed, 244 insertions(+), 240 deletions(-) delete mode 100644 app/assets/javascripts/protected_branch_access_dropdown.js.es6 delete mode 100644 app/assets/javascripts/protected_branch_create.js.es6 delete mode 100644 app/assets/javascripts/protected_branch_dropdown.js.es6 delete mode 100644 app/assets/javascripts/protected_branch_edit.js.es6 delete mode 100644 app/assets/javascripts/protected_branch_edit_list.js.es6 create mode 100644 app/assets/javascripts/protected_branches/protected_branch_access_dropdown.js.es6 create mode 100644 app/assets/javascripts/protected_branches/protected_branch_create.js.es6 create mode 100644 app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 create mode 100644 app/assets/javascripts/protected_branches/protected_branch_edit.js.es6 create mode 100644 app/assets/javascripts/protected_branches/protected_branch_edit_list.js.es6 create mode 100644 app/assets/javascripts/protected_branches/protected_branches_bundle.js diff --git a/app/assets/javascripts/protected_branch_access_dropdown.js.es6 b/app/assets/javascripts/protected_branch_access_dropdown.js.es6 deleted file mode 100644 index 7aeb5f92514..00000000000 --- a/app/assets/javascripts/protected_branch_access_dropdown.js.es6 +++ /dev/null @@ -1,28 +0,0 @@ -(global => { - global.gl = global.gl || {}; - - gl.ProtectedBranchAccessDropdown = class { - constructor(options) { - const { $dropdown, data, onSelect } = options; - - $dropdown.glDropdown({ - data: data, - selectable: true, - inputId: $dropdown.data('input-id'), - fieldName: $dropdown.data('field-name'), - toggleLabel(item, el) { - if (el.is('.is-active')) { - return item.text; - } else { - return 'Select'; - } - }, - clicked(item, $el, e) { - e.preventDefault(); - onSelect(); - } - }); - } - } - -})(window); diff --git a/app/assets/javascripts/protected_branch_create.js.es6 b/app/assets/javascripts/protected_branch_create.js.es6 deleted file mode 100644 index 46beca469b9..00000000000 --- a/app/assets/javascripts/protected_branch_create.js.es6 +++ /dev/null @@ -1,54 +0,0 @@ -(global => { - global.gl = global.gl || {}; - - gl.ProtectedBranchCreate = class { - constructor() { - this.$wrap = this.$form = $('#new_protected_branch'); - this.buildDropdowns(); - } - - buildDropdowns() { - const $allowedToMergeDropdown = this.$wrap.find('.js-allowed-to-merge'); - const $allowedToPushDropdown = this.$wrap.find('.js-allowed-to-push'); - - // Cache callback - this.onSelectCallback = this.onSelect.bind(this); - - // Allowed to Merge dropdown - new gl.ProtectedBranchAccessDropdown({ - $dropdown: $allowedToMergeDropdown, - data: gon.merge_access_levels, - onSelect: this.onSelectCallback - }); - - // Allowed to Push dropdown - new gl.ProtectedBranchAccessDropdown({ - $dropdown: $allowedToPushDropdown, - data: gon.push_access_levels, - onSelect: this.onSelectCallback - }); - - // Select default - $allowedToPushDropdown.data('glDropdown').selectRowAtIndex(0); - $allowedToMergeDropdown.data('glDropdown').selectRowAtIndex(0); - - // Protected branch dropdown - new ProtectedBranchDropdown({ - $dropdown: this.$wrap.find('.js-protected-branch-select'), - onSelect: this.onSelectCallback - }); - } - - // This will run after clicked callback - onSelect() { - - // Enable submit button - const $branchInput = this.$wrap.find('input[name="protected_branch[name]"]'); - const $allowedToMergeInput = this.$wrap.find('input[name="protected_branch[merge_access_levels_attributes][0][access_level]"]'); - const $allowedToPushInput = this.$wrap.find('input[name="protected_branch[push_access_levels_attributes][0][access_level]"]'); - - this.$form.find('input[type="submit"]').attr('disabled', !($branchInput.val() && $allowedToMergeInput.length && $allowedToPushInput.length)); - } - } - -})(window); diff --git a/app/assets/javascripts/protected_branch_dropdown.js.es6 b/app/assets/javascripts/protected_branch_dropdown.js.es6 deleted file mode 100644 index 983322cbecc..00000000000 --- a/app/assets/javascripts/protected_branch_dropdown.js.es6 +++ /dev/null @@ -1,76 +0,0 @@ -class ProtectedBranchDropdown { - constructor(options) { - this.onSelect = options.onSelect; - this.$dropdown = options.$dropdown; - this.$dropdownContainer = this.$dropdown.parent(); - this.$dropdownFooter = this.$dropdownContainer.find('.dropdown-footer'); - this.$protectedBranch = this.$dropdownContainer.find('.create-new-protected-branch'); - - this.buildDropdown(); - this.bindEvents(); - - // Hide footer - this.$dropdownFooter.addClass('hidden'); - } - - buildDropdown() { - this.$dropdown.glDropdown({ - data: this.getProtectedBranches.bind(this), - filterable: true, - remote: false, - search: { - fields: ['title'] - }, - selectable: true, - toggleLabel(selected) { - return (selected && 'id' in selected) ? selected.title : 'Protected Branch'; - }, - fieldName: 'protected_branch[name]', - text(protectedBranch) { - return _.escape(protectedBranch.title); - }, - id(protectedBranch) { - return _.escape(protectedBranch.id); - }, - onFilter: this.toggleCreateNewButton.bind(this), - clicked: (item, $el, e) => { - e.preventDefault(); - this.onSelect(); - } - }); - } - - bindEvents() { - this.$protectedBranch.on('click', this.onClickCreateWildcard.bind(this)); - } - - onClickCreateWildcard() { - // Refresh the dropdown's data, which ends up calling `getProtectedBranches` - this.$dropdown.data('glDropdown').remote.execute(); - this.$dropdown.data('glDropdown').selectRowAtIndex(0); - } - - getProtectedBranches(term, callback) { - if (this.selectedBranch) { - callback(gon.open_branches.concat(this.selectedBranch)); - } else { - callback(gon.open_branches); - } - } - - toggleCreateNewButton(branchName) { - this.selectedBranch = { - title: branchName, - id: branchName, - text: branchName - }; - - if (branchName) { - this.$dropdownContainer - .find('.create-new-protected-branch code') - .text(branchName); - } - - this.$dropdownFooter.toggleClass('hidden', !branchName); - } -} diff --git a/app/assets/javascripts/protected_branch_edit.js.es6 b/app/assets/javascripts/protected_branch_edit.js.es6 deleted file mode 100644 index 15a6dca2875..00000000000 --- a/app/assets/javascripts/protected_branch_edit.js.es6 +++ /dev/null @@ -1,65 +0,0 @@ -(global => { - global.gl = global.gl || {}; - - gl.ProtectedBranchEdit = class { - constructor(options) { - this.$wrap = options.$wrap; - this.$allowedToMergeDropdown = this.$wrap.find('.js-allowed-to-merge'); - this.$allowedToPushDropdown = this.$wrap.find('.js-allowed-to-push'); - - this.buildDropdowns(); - } - - buildDropdowns() { - - // Allowed to merge dropdown - new gl.ProtectedBranchAccessDropdown({ - $dropdown: this.$allowedToMergeDropdown, - data: gon.merge_access_levels, - onSelect: this.onSelect.bind(this) - }); - - // Allowed to push dropdown - new gl.ProtectedBranchAccessDropdown({ - $dropdown: this.$allowedToPushDropdown, - data: gon.push_access_levels, - onSelect: this.onSelect.bind(this) - }); - } - - onSelect() { - const $allowedToMergeInput = this.$wrap.find(`input[name="${this.$allowedToMergeDropdown.data('fieldName')}"]`); - const $allowedToPushInput = this.$wrap.find(`input[name="${this.$allowedToPushDropdown.data('fieldName')}"]`); - - // Do not update if one dropdown has not selected any option - if (!($allowedToMergeInput.length && $allowedToPushInput.length)) return; - - $.ajax({ - type: 'POST', - url: this.$wrap.data('url'), - dataType: 'json', - data: { - _method: 'PATCH', - protected_branch: { - merge_access_levels_attributes: [{ - id: this.$allowedToMergeDropdown.data('access-level-id'), - access_level: $allowedToMergeInput.val() - }], - push_access_levels_attributes: [{ - id: this.$allowedToPushDropdown.data('access-level-id'), - access_level: $allowedToPushInput.val() - }] - } - }, - success: () => { - this.$wrap.effect('highlight'); - }, - error() { - $.scrollTo(0); - new Flash('Failed to update branch!'); - } - }); - } - } - -})(window); diff --git a/app/assets/javascripts/protected_branch_edit_list.js.es6 b/app/assets/javascripts/protected_branch_edit_list.js.es6 deleted file mode 100644 index 9ff0fd12c76..00000000000 --- a/app/assets/javascripts/protected_branch_edit_list.js.es6 +++ /dev/null @@ -1,17 +0,0 @@ -(global => { - global.gl = global.gl || {}; - - gl.ProtectedBranchEditList = class { - constructor() { - this.$wrap = $('.protected-branches-list'); - - // Build edit forms - this.$wrap.find('.js-protected-branch-edit-form').each((i, el) => { - new gl.ProtectedBranchEdit({ - $wrap: $(el) - }); - }); - } - } - -})(window); diff --git a/app/assets/javascripts/protected_branches/protected_branch_access_dropdown.js.es6 b/app/assets/javascripts/protected_branches/protected_branch_access_dropdown.js.es6 new file mode 100644 index 00000000000..7aeb5f92514 --- /dev/null +++ b/app/assets/javascripts/protected_branches/protected_branch_access_dropdown.js.es6 @@ -0,0 +1,28 @@ +(global => { + global.gl = global.gl || {}; + + gl.ProtectedBranchAccessDropdown = class { + constructor(options) { + const { $dropdown, data, onSelect } = options; + + $dropdown.glDropdown({ + data: data, + selectable: true, + inputId: $dropdown.data('input-id'), + fieldName: $dropdown.data('field-name'), + toggleLabel(item, el) { + if (el.is('.is-active')) { + return item.text; + } else { + return 'Select'; + } + }, + clicked(item, $el, e) { + e.preventDefault(); + onSelect(); + } + }); + } + } + +})(window); diff --git a/app/assets/javascripts/protected_branches/protected_branch_create.js.es6 b/app/assets/javascripts/protected_branches/protected_branch_create.js.es6 new file mode 100644 index 00000000000..46beca469b9 --- /dev/null +++ b/app/assets/javascripts/protected_branches/protected_branch_create.js.es6 @@ -0,0 +1,54 @@ +(global => { + global.gl = global.gl || {}; + + gl.ProtectedBranchCreate = class { + constructor() { + this.$wrap = this.$form = $('#new_protected_branch'); + this.buildDropdowns(); + } + + buildDropdowns() { + const $allowedToMergeDropdown = this.$wrap.find('.js-allowed-to-merge'); + const $allowedToPushDropdown = this.$wrap.find('.js-allowed-to-push'); + + // Cache callback + this.onSelectCallback = this.onSelect.bind(this); + + // Allowed to Merge dropdown + new gl.ProtectedBranchAccessDropdown({ + $dropdown: $allowedToMergeDropdown, + data: gon.merge_access_levels, + onSelect: this.onSelectCallback + }); + + // Allowed to Push dropdown + new gl.ProtectedBranchAccessDropdown({ + $dropdown: $allowedToPushDropdown, + data: gon.push_access_levels, + onSelect: this.onSelectCallback + }); + + // Select default + $allowedToPushDropdown.data('glDropdown').selectRowAtIndex(0); + $allowedToMergeDropdown.data('glDropdown').selectRowAtIndex(0); + + // Protected branch dropdown + new ProtectedBranchDropdown({ + $dropdown: this.$wrap.find('.js-protected-branch-select'), + onSelect: this.onSelectCallback + }); + } + + // This will run after clicked callback + onSelect() { + + // Enable submit button + const $branchInput = this.$wrap.find('input[name="protected_branch[name]"]'); + const $allowedToMergeInput = this.$wrap.find('input[name="protected_branch[merge_access_levels_attributes][0][access_level]"]'); + const $allowedToPushInput = this.$wrap.find('input[name="protected_branch[push_access_levels_attributes][0][access_level]"]'); + + this.$form.find('input[type="submit"]').attr('disabled', !($branchInput.val() && $allowedToMergeInput.length && $allowedToPushInput.length)); + } + } + +})(window); diff --git a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 new file mode 100644 index 00000000000..983322cbecc --- /dev/null +++ b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js.es6 @@ -0,0 +1,76 @@ +class ProtectedBranchDropdown { + constructor(options) { + this.onSelect = options.onSelect; + this.$dropdown = options.$dropdown; + this.$dropdownContainer = this.$dropdown.parent(); + this.$dropdownFooter = this.$dropdownContainer.find('.dropdown-footer'); + this.$protectedBranch = this.$dropdownContainer.find('.create-new-protected-branch'); + + this.buildDropdown(); + this.bindEvents(); + + // Hide footer + this.$dropdownFooter.addClass('hidden'); + } + + buildDropdown() { + this.$dropdown.glDropdown({ + data: this.getProtectedBranches.bind(this), + filterable: true, + remote: false, + search: { + fields: ['title'] + }, + selectable: true, + toggleLabel(selected) { + return (selected && 'id' in selected) ? selected.title : 'Protected Branch'; + }, + fieldName: 'protected_branch[name]', + text(protectedBranch) { + return _.escape(protectedBranch.title); + }, + id(protectedBranch) { + return _.escape(protectedBranch.id); + }, + onFilter: this.toggleCreateNewButton.bind(this), + clicked: (item, $el, e) => { + e.preventDefault(); + this.onSelect(); + } + }); + } + + bindEvents() { + this.$protectedBranch.on('click', this.onClickCreateWildcard.bind(this)); + } + + onClickCreateWildcard() { + // Refresh the dropdown's data, which ends up calling `getProtectedBranches` + this.$dropdown.data('glDropdown').remote.execute(); + this.$dropdown.data('glDropdown').selectRowAtIndex(0); + } + + getProtectedBranches(term, callback) { + if (this.selectedBranch) { + callback(gon.open_branches.concat(this.selectedBranch)); + } else { + callback(gon.open_branches); + } + } + + toggleCreateNewButton(branchName) { + this.selectedBranch = { + title: branchName, + id: branchName, + text: branchName + }; + + if (branchName) { + this.$dropdownContainer + .find('.create-new-protected-branch code') + .text(branchName); + } + + this.$dropdownFooter.toggleClass('hidden', !branchName); + } +} diff --git a/app/assets/javascripts/protected_branches/protected_branch_edit.js.es6 b/app/assets/javascripts/protected_branches/protected_branch_edit.js.es6 new file mode 100644 index 00000000000..15a6dca2875 --- /dev/null +++ b/app/assets/javascripts/protected_branches/protected_branch_edit.js.es6 @@ -0,0 +1,65 @@ +(global => { + global.gl = global.gl || {}; + + gl.ProtectedBranchEdit = class { + constructor(options) { + this.$wrap = options.$wrap; + this.$allowedToMergeDropdown = this.$wrap.find('.js-allowed-to-merge'); + this.$allowedToPushDropdown = this.$wrap.find('.js-allowed-to-push'); + + this.buildDropdowns(); + } + + buildDropdowns() { + + // Allowed to merge dropdown + new gl.ProtectedBranchAccessDropdown({ + $dropdown: this.$allowedToMergeDropdown, + data: gon.merge_access_levels, + onSelect: this.onSelect.bind(this) + }); + + // Allowed to push dropdown + new gl.ProtectedBranchAccessDropdown({ + $dropdown: this.$allowedToPushDropdown, + data: gon.push_access_levels, + onSelect: this.onSelect.bind(this) + }); + } + + onSelect() { + const $allowedToMergeInput = this.$wrap.find(`input[name="${this.$allowedToMergeDropdown.data('fieldName')}"]`); + const $allowedToPushInput = this.$wrap.find(`input[name="${this.$allowedToPushDropdown.data('fieldName')}"]`); + + // Do not update if one dropdown has not selected any option + if (!($allowedToMergeInput.length && $allowedToPushInput.length)) return; + + $.ajax({ + type: 'POST', + url: this.$wrap.data('url'), + dataType: 'json', + data: { + _method: 'PATCH', + protected_branch: { + merge_access_levels_attributes: [{ + id: this.$allowedToMergeDropdown.data('access-level-id'), + access_level: $allowedToMergeInput.val() + }], + push_access_levels_attributes: [{ + id: this.$allowedToPushDropdown.data('access-level-id'), + access_level: $allowedToPushInput.val() + }] + } + }, + success: () => { + this.$wrap.effect('highlight'); + }, + error() { + $.scrollTo(0); + new Flash('Failed to update branch!'); + } + }); + } + } + +})(window); diff --git a/app/assets/javascripts/protected_branches/protected_branch_edit_list.js.es6 b/app/assets/javascripts/protected_branches/protected_branch_edit_list.js.es6 new file mode 100644 index 00000000000..9ff0fd12c76 --- /dev/null +++ b/app/assets/javascripts/protected_branches/protected_branch_edit_list.js.es6 @@ -0,0 +1,17 @@ +(global => { + global.gl = global.gl || {}; + + gl.ProtectedBranchEditList = class { + constructor() { + this.$wrap = $('.protected-branches-list'); + + // Build edit forms + this.$wrap.find('.js-protected-branch-edit-form').each((i, el) => { + new gl.ProtectedBranchEdit({ + $wrap: $(el) + }); + }); + } + } + +})(window); diff --git a/app/assets/javascripts/protected_branches/protected_branches_bundle.js b/app/assets/javascripts/protected_branches/protected_branches_bundle.js new file mode 100644 index 00000000000..15b3affd469 --- /dev/null +++ b/app/assets/javascripts/protected_branches/protected_branches_bundle.js @@ -0,0 +1 @@ +/*= require_tree . */ diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml index 49dcc9a6ba4..42e9bdbd30e 100644 --- a/app/views/projects/protected_branches/index.html.haml +++ b/app/views/projects/protected_branches/index.html.haml @@ -1,4 +1,6 @@ - page_title "Protected branches" +- content_for :page_specific_javascripts do + = page_specific_javascript_tag('protected_branches/protected_branches_bundle.js') .row.prepend-top-default.append-bottom-default .col-lg-3 diff --git a/config/application.rb b/config/application.rb index 8a9c539cb43..f3337b00dc6 100644 --- a/config/application.rb +++ b/config/application.rb @@ -87,6 +87,7 @@ module Gitlab config.assets.precompile << "users/users_bundle.js" config.assets.precompile << "network/network_bundle.js" config.assets.precompile << "profile/profile_bundle.js" + config.assets.precompile << "protected_branches/protected_branches_bundle.js" config.assets.precompile << "diff_notes/diff_notes_bundle.js" config.assets.precompile << "boards/boards_bundle.js" config.assets.precompile << "merge_conflicts/merge_conflicts_bundle.js" -- cgit v1.2.1 From b4359fb24e599c278edeae843bd6a25c980b1243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 20 Oct 2016 18:51:03 +0200 Subject: Don't use Hash#slice since it's not supported in Ruby 2.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/commit_statuses.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index f282a3b9cd6..f54d4f06627 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -67,9 +67,14 @@ module API pipeline = @project.ensure_pipeline(ref, commit.sha, current_user) status = GenericCommitStatus.running_or_pending.find_or_initialize_by( - project: @project, pipeline: pipeline, - user: current_user, name: name, ref: ref) - status.attributes = declared(params).slice(:target_url, :description) + project: @project, + pipeline: pipeline, + user: current_user, + name: name, + ref: ref, + target_url: params[:target_url], + description: params[:description] + ) begin case params[:state].to_s -- cgit v1.2.1