From 6d92cd3e836f2252b660479f5b33d15e6456b04d Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Mon, 1 Aug 2016 20:12:30 +0200 Subject: [PATCH 001/144] WIP --- .../projects/project_members_controller.rb | 2 +- app/models/member.rb | 6 ++-- app/models/members/project_member.rb | 4 +-- app/models/project_team.rb | 5 ++-- app/models/user.rb | 2 +- .../_new_project_member.html.haml | 5 ++++ ...20160801163421_add_expires_at_to_member.rb | 29 +++++++++++++++++++ db/schema.rb | 3 +- 8 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20160801163421_add_expires_at_to_member.rb diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 3435a118964..0fe96b66fa2 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -36,7 +36,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def create - @project.team.add_users(params[:user_ids].split(','), params[:access_level], current_user) + @project.team.add_users(params[:user_ids].split(','), params[:access_level], current_user, params[:expires_at]) redirect_to namespace_project_project_members_path(@project.namespace, @project) end diff --git a/app/models/member.rb b/app/models/member.rb index 24ab1276ee9..998144330b1 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -31,6 +31,7 @@ class Member < ActiveRecord::Base scope :non_invite, -> { where(invite_token: nil) } scope :request, -> { where.not(requested_at: nil) } scope :has_access, -> { where('access_level > 0') } + scope :still_active, -> { where('expires_at IS NULL OR expires_at > ?', Time.current) } scope :guests, -> { where(access_level: GUEST) } scope :reporters, -> { where(access_level: REPORTER) } @@ -54,7 +55,7 @@ class Member < ActiveRecord::Base class << self def access_for_user_ids(user_ids) - where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h + where(user_id: user_ids).has_access.still_active.pluck(:user_id, :access_level).to_h end def find_by_invite_token(invite_token) @@ -73,7 +74,7 @@ class Member < ActiveRecord::Base user end - def add_user(members, user_id, access_level, current_user = nil) + def add_user(members, user_id, access_level, current_user = nil, expires_at = nil) user = user_for_id(user_id) # `user` can be either a User object or an email to be invited @@ -87,6 +88,7 @@ class Member < ActiveRecord::Base if can_update_member?(current_user, member) || project_creator?(member, access_level) member.created_by ||= current_user member.access_level = access_level + member.expires_at = expires_at member.save end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index f39afc61ce9..b86b536005b 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -33,7 +33,7 @@ class ProjectMember < Member # :master # ) # - def add_users_into_projects(project_ids, user_ids, access, current_user = nil) + def add_users_into_projects(project_ids, user_ids, access, current_user = nil, expires_at = nil) access_level = if roles_hash.has_key?(access) roles_hash[access] elsif roles_hash.values.include?(access.to_i) @@ -49,7 +49,7 @@ class ProjectMember < Member project = Project.find(project_id) users.each do |user| - Member.add_user(project.project_members, user, access_level, current_user) + Member.add_user(project.project_members, user, access_level, current_user, expires_at) end end end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index fdfaf052730..cf1d2396974 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -33,12 +33,13 @@ class ProjectTeam member end - def add_users(users, access, current_user = nil) + def add_users(users, access, current_user = nil, expires_at = nil) ProjectMember.add_users_into_projects( [project.id], users, access, - current_user + current_user, + expires_at ) end diff --git a/app/models/user.rb b/app/models/user.rb index db747434959..2995db9a5f2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -66,7 +66,7 @@ class User < ActiveRecord::Base # Projects has_many :groups_projects, through: :groups, source: :projects has_many :personal_projects, through: :namespace, source: :projects - has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, class_name: 'ProjectMember' + has_many :project_members, -> { where(requested_at: nil).still_active }, dependent: :destroy, class_name: 'ProjectMember' has_many :projects, through: :project_members has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' has_many :users_star_projects, dependent: :destroy diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml index 978c4dfc5ec..f84aa9e5423 100644 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ b/app/views/projects/project_members/_new_project_member.html.haml @@ -14,5 +14,10 @@ Read more about role permissions %strong= link_to "here", help_page_path("user/permissions"), class: "vlink" + .form-group + = f.label :expires_at, "Membership expires at", class: 'control-label' + .col-sm-10 + = text_field_tag :expires_at, nil, class: "datepicker form-control", placeholder: "Select expires at" + .form-actions = f.submit 'Add users to project', class: "btn btn-create" diff --git a/db/migrate/20160801163421_add_expires_at_to_member.rb b/db/migrate/20160801163421_add_expires_at_to_member.rb new file mode 100644 index 00000000000..9cd37da6818 --- /dev/null +++ b/db/migrate/20160801163421_add_expires_at_to_member.rb @@ -0,0 +1,29 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddExpiresAtToMember < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :members, :expires_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 2d2ae5fd840..d1ebbe082d1 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: 20160722221922) do +ActiveRecord::Schema.define(version: 20160801163421) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -581,6 +581,7 @@ ActiveRecord::Schema.define(version: 20160722221922) do t.string "invite_token" t.datetime "invite_accepted_at" t.datetime "requested_at" + t.datetime "expires_at" end add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree From 420e59030b151a5634e31beaa1d80f1eac079295 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Tue, 2 Aug 2016 18:54:50 +0200 Subject: [PATCH 002/144] Initialize the datepicker - the ugly way :( --- .../project_members/_new_project_member.html.haml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml index f84aa9e5423..7f935522bed 100644 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ b/app/views/projects/project_members/_new_project_member.html.haml @@ -15,9 +15,16 @@ %strong= link_to "here", help_page_path("user/permissions"), class: "vlink" .form-group - = f.label :expires_at, "Membership expires at", class: 'control-label' + = f.label :expires_at, "Access expiration date", class: 'control-label' .col-sm-10 - = text_field_tag :expires_at, nil, class: "datepicker form-control", placeholder: "Select expires at" + = text_field_tag :expires_at, nil, class: "datepicker form-control", placeholder: "Select access expiration date" + .help-block + Leave it empty if you do not want this user's access to expire. .form-actions = f.submit 'Add users to project', class: "btn btn-create" + +:javascript + $(".datepicker").datepicker({ + dateFormat: "yy-mm-dd" + }); From b2c8dc6f35ceb08e23422a356831070b5136809d Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Tue, 2 Aug 2016 20:37:22 +0200 Subject: [PATCH 003/144] Replace optional parameters with keyword arguments. --- .../projects/project_members_controller.rb | 7 ++++++- app/models/group.rb | 2 +- app/models/member.rb | 2 +- app/models/members/project_member.rb | 10 ++++++++-- app/models/project_team.rb | 10 +++++----- spec/mailers/notify_spec.rb | 14 ++++++++++++-- spec/models/member_spec.rb | 14 ++++++++++++-- 7 files changed, 45 insertions(+), 14 deletions(-) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 0fe96b66fa2..7b84be54a3a 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -36,7 +36,12 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def create - @project.team.add_users(params[:user_ids].split(','), params[:access_level], current_user, params[:expires_at]) + @project.team.add_users( + params[:user_ids].split(','), + params[:access_level], + expires_at: params[:expires_at], + current_user: current_user + ) redirect_to namespace_project_project_members_path(@project.namespace, @project) end diff --git a/app/models/group.rb b/app/models/group.rb index 37631b99701..11c39bbdfe4 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -97,7 +97,7 @@ class Group < Namespace def add_users(user_ids, access_level, current_user = nil) user_ids.each do |user_id| - Member.add_user(self.group_members, user_id, access_level, current_user) + Member.add_user(self.group_members, user_id, access_level, current_user: current_user) end end diff --git a/app/models/member.rb b/app/models/member.rb index 998144330b1..6b9206163c4 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -74,7 +74,7 @@ class Member < ActiveRecord::Base user end - def add_user(members, user_id, access_level, current_user = nil, expires_at = nil) + def add_user(members, user_id, access_level, current_user: nil, expires_at: nil) user = user_for_id(user_id) # `user` can be either a User object or an email to be invited diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index b86b536005b..d5666de79cc 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -33,7 +33,7 @@ class ProjectMember < Member # :master # ) # - def add_users_into_projects(project_ids, user_ids, access, current_user = nil, expires_at = nil) + def add_users_into_projects(project_ids, user_ids, access, current_user: nil, expires_at: nil) access_level = if roles_hash.has_key?(access) roles_hash[access] elsif roles_hash.values.include?(access.to_i) @@ -49,7 +49,13 @@ class ProjectMember < Member project = Project.find(project_id) users.each do |user| - Member.add_user(project.project_members, user, access_level, current_user, expires_at) + Member.add_user( + project.project_members, + user, + access_level, + current_user: current_user, + expires_at: expires_at + ) end end end diff --git a/app/models/project_team.rb b/app/models/project_team.rb index cf1d2396974..1865c5253e2 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -15,7 +15,7 @@ class ProjectTeam users, access, current_user = *args if users.respond_to?(:each) - add_users(users, access, current_user) + add_users(users, access, current_user: current_user) else add_user(users, access, current_user) end @@ -33,18 +33,18 @@ class ProjectTeam member end - def add_users(users, access, current_user = nil, expires_at = nil) + def add_users(users, access, current_user: nil, expires_at: nil) ProjectMember.add_users_into_projects( [project.id], users, access, - current_user, - expires_at + current_user: current_user, + expires_at: expires_at ) end def add_user(user, access, current_user = nil) - add_users([user], access, current_user) + add_users([user], access, current_user: current_user) end # Remove all users from project team diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 3685b2b17b5..828d2845988 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -493,7 +493,12 @@ describe Notify do end def invite_to_project(project:, email:, inviter:) - ProjectMember.add_user(project.project_members, 'toto@example.com', Gitlab::Access::DEVELOPER, inviter) + Member.add_user( + project.project_members, + 'toto@example.com', + Gitlab::Access::DEVELOPER, + current_user: inviter + ) project.project_members.invite.last end @@ -740,7 +745,12 @@ describe Notify do end def invite_to_group(group:, email:, inviter:) - GroupMember.add_user(group.group_members, 'toto@example.com', Gitlab::Access::DEVELOPER, inviter) + Member.add_user( + group.group_members, + 'toto@example.com', + Gitlab::Access::DEVELOPER, + current_user: inviter + ) group.group_members.invite.last end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 44cd3c08718..2f6cccdc417 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -65,11 +65,21 @@ describe Member, models: true do @master_user = create(:user).tap { |u| project.team << [u, :master] } @master = project.members.find_by(user_id: @master_user.id) - ProjectMember.add_user(project.members, 'toto1@example.com', Gitlab::Access::DEVELOPER, @master_user) + Member.add_user( + project.members, + 'toto1@example.com', + Gitlab::Access::DEVELOPER, + current_user: @master_user + ) @invited_member = project.members.invite.find_by_invite_email('toto1@example.com') accepted_invite_user = build(:user) - ProjectMember.add_user(project.members, 'toto2@example.com', Gitlab::Access::DEVELOPER, @master_user) + Member.add_user( + project.members, + 'toto2@example.com', + Gitlab::Access::DEVELOPER, + current_user: @master_user + ) @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) } requested_user = create(:user).tap { |u| project.request_access(u) } From d12570280095cf26fc4a019b85579ecd3efdb7ec Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Wed, 3 Aug 2016 20:57:09 +0200 Subject: [PATCH 004/144] Add test for a member with the expiration date. --- ...r_adds_member_with_expiration_date_spec.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb new file mode 100644 index 00000000000..87d9208465e --- /dev/null +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +feature 'Projects > Members > Master adds member with expiration date', feature: true, js: true do + include Select2Helper + + let!(:master) { create(:user) } + let!(:project) { create(:project) } + let!(:new_member) { create(:user) } + + background do + project.team << [master, :master] + login_as(master) + visit namespace_project_project_members_path(project.namespace, project) + end + + scenario 'expiration date is displayed in the members list' do + page.within ".users-project-form" do + select2(new_member.id, from: "#user_ids", multiple: true) + fill_in "Access expiration date", with: "2016-08-02" + click_on "Add users to project" + end + + page.within ".project_member:first-child" do + expect(page).to have_content("Access expires Aug 2, 2016") + end + end +end From 736ff98199941f0438d8425cded9e4f547538d71 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Wed, 3 Aug 2016 20:57:54 +0200 Subject: [PATCH 005/144] Display expiration date in the project member list. --- app/views/shared/members/_member.html.haml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 5ae485f36ba..aea08eac086 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -59,6 +59,9 @@ = time_ago_with_tooltip(member.requested_at) - else Joined #{time_ago_with_tooltip(member.created_at)} + - if member.expires_at.present? + %span.prepend-left-20 + Access expires #{member.expires_at.to_s(:medium)} - else = image_tag avatar_icon(member.invite_email, 40), class: "avatar s40", alt: '' From 28fc9088558300bbb671412baf0c3915421a0028 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Wed, 3 Aug 2016 20:58:18 +0200 Subject: [PATCH 006/144] Fix label so it's linked to the input. --- .../projects/project_members/_new_project_member.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml index 7f935522bed..5aa0950523b 100644 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ b/app/views/projects/project_members/_new_project_member.html.haml @@ -15,7 +15,7 @@ %strong= link_to "here", help_page_path("user/permissions"), class: "vlink" .form-group - = f.label :expires_at, "Access expiration date", class: 'control-label' + = label_tag :expires_at, "Access expiration date", class: 'control-label' .col-sm-10 = text_field_tag :expires_at, nil, class: "datepicker form-control", placeholder: "Select access expiration date" .help-block From d0aa98bc5dc7228415f1eba2f61f75fa6406180a Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 4 Aug 2016 07:56:06 +0200 Subject: [PATCH 007/144] Add Member#expires? method. --- app/models/member.rb | 4 ++++ app/views/shared/members/_member.html.haml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/member.rb b/app/models/member.rb index 6b9206163c4..9d913805ad2 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -125,6 +125,10 @@ class Member < ActiveRecord::Base invite? || request? end + def expires? + expires_at.present? + end + def accept_request return false unless request? diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index aea08eac086..88e9522b9cc 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -59,7 +59,7 @@ = time_ago_with_tooltip(member.requested_at) - else Joined #{time_ago_with_tooltip(member.created_at)} - - if member.expires_at.present? + - if member.expires? %span.prepend-left-20 Access expires #{member.expires_at.to_s(:medium)} From 419990ebd2a9f4781d625b1f234be04271c36ac6 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 4 Aug 2016 07:59:14 +0200 Subject: [PATCH 008/144] Remove scopes. --- app/models/member.rb | 3 +-- app/models/user.rb | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/models/member.rb b/app/models/member.rb index 9d913805ad2..bcb6e32d53a 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -31,7 +31,6 @@ class Member < ActiveRecord::Base scope :non_invite, -> { where(invite_token: nil) } scope :request, -> { where.not(requested_at: nil) } scope :has_access, -> { where('access_level > 0') } - scope :still_active, -> { where('expires_at IS NULL OR expires_at > ?', Time.current) } scope :guests, -> { where(access_level: GUEST) } scope :reporters, -> { where(access_level: REPORTER) } @@ -55,7 +54,7 @@ class Member < ActiveRecord::Base class << self def access_for_user_ids(user_ids) - where(user_id: user_ids).has_access.still_active.pluck(:user_id, :access_level).to_h + where(user_id: user_ids).has_access.pluck(:user_id, :access_level).to_h end def find_by_invite_token(invite_token) diff --git a/app/models/user.rb b/app/models/user.rb index 2995db9a5f2..db747434959 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -66,7 +66,7 @@ class User < ActiveRecord::Base # Projects has_many :groups_projects, through: :groups, source: :projects has_many :personal_projects, through: :namespace, source: :projects - has_many :project_members, -> { where(requested_at: nil).still_active }, dependent: :destroy, class_name: 'ProjectMember' + has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, class_name: 'ProjectMember' has_many :projects, through: :project_members has_many :created_projects, foreign_key: :creator_id, class_name: 'Project' has_many :users_star_projects, dependent: :destroy From d95fd3cb9927d41b33e6991377ec060f2a12f4d5 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 4 Aug 2016 08:07:23 +0200 Subject: [PATCH 009/144] Use Date instead of Datetime. Nobody cares about the exact hour. --- db/migrate/20160801163421_add_expires_at_to_member.rb | 2 +- db/schema.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20160801163421_add_expires_at_to_member.rb b/db/migrate/20160801163421_add_expires_at_to_member.rb index 9cd37da6818..8db0fc60c4b 100644 --- a/db/migrate/20160801163421_add_expires_at_to_member.rb +++ b/db/migrate/20160801163421_add_expires_at_to_member.rb @@ -24,6 +24,6 @@ class AddExpiresAtToMember < ActiveRecord::Migration # disable_ddl_transaction! def change - add_column :members, :expires_at, :datetime + add_column :members, :expires_at, :date end end diff --git a/db/schema.rb b/db/schema.rb index d1ebbe082d1..4792e13cd69 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -581,7 +581,7 @@ ActiveRecord::Schema.define(version: 20160801163421) do t.string "invite_token" t.datetime "invite_accepted_at" t.datetime "requested_at" - t.datetime "expires_at" + t.date "expires_at" end add_index "members", ["access_level"], name: "index_members_on_access_level", using: :btree From 9adddebc6deacd05e2de146ede640f690a17d7bf Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 4 Aug 2016 09:41:29 +0200 Subject: [PATCH 010/144] Add worker which removes expired members. --- app/workers/remove_expired_members_worker.rb | 13 +++++++++++++ config/initializers/1_settings.rb | 3 +++ 2 files changed, 16 insertions(+) create mode 100644 app/workers/remove_expired_members_worker.rb diff --git a/app/workers/remove_expired_members_worker.rb b/app/workers/remove_expired_members_worker.rb new file mode 100644 index 00000000000..30486b9e8a3 --- /dev/null +++ b/app/workers/remove_expired_members_worker.rb @@ -0,0 +1,13 @@ +class RemoveExpiredMembersWorker + include Sidekiq::Worker + + def perform + Member.includes(:created_by).where("expires_at <= ?", Time.current).find_each do |member| + begin + Members::DestroyService.new(member, member.created_by).execute + rescue => ex + logger.error("Expired Member ID=#{member.id} cannot be removed - #{ex}") + end + end + end +end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 49130f37b31..78e4b2b1bd3 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -293,6 +293,9 @@ Settings.cron_jobs['gitlab_remove_project_export_worker']['job_class'] = 'Gitlab Settings.cron_jobs['requests_profiles_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['requests_profiles_worker']['cron'] ||= '0 0 * * *' Settings.cron_jobs['requests_profiles_worker']['job_class'] = 'RequestsProfilesWorker' +Settings.cron_jobs['remove_expired_members_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['remove_expired_members_worker']['cron'] ||= '10 0 * * *' +Settings.cron_jobs['remove_expired_members_worker']['job_class'] = 'RemoveExpiredMembersWorker' # # GitLab Shell From b4b51441aa096db9f953eb8e907dd68e75227c62 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 4 Aug 2016 23:34:57 +0200 Subject: [PATCH 011/144] Extract Members::AuthorizedDestroyService from Members::DestroyService. --- .../members/authorized_destroy_service.rb | 17 +++++++++++++++++ app/services/members/destroy_service.rb | 7 +------ 2 files changed, 18 insertions(+), 6 deletions(-) create mode 100644 app/services/members/authorized_destroy_service.rb diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb new file mode 100644 index 00000000000..c23f90a6a10 --- /dev/null +++ b/app/services/members/authorized_destroy_service.rb @@ -0,0 +1,17 @@ +module Members + class AuthorizedDestroyService < BaseService + attr_accessor :member, :user + + def initialize(member, user = nil) + @member, @user = member, user + end + + def execute + member.destroy + + if member.request? && member.user != user + notification_service.decline_access_request(member) + end + end + end +end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 15358f80208..5228da4d457 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -10,12 +10,7 @@ module Members unless member && can?(current_user, "destroy_#{member.type.underscore}".to_sym, member) raise Gitlab::Access::AccessDeniedError end - - member.destroy - - if member.request? && member.user != current_user - notification_service.decline_access_request(member) - end + AuthorizedDestroyService.new(member, current_user).execute end end end From 8354810f103a8e82cb5d2b77049b5b3c9b298fa9 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 4 Aug 2016 23:35:28 +0200 Subject: [PATCH 012/144] Use Members::AuthorizedDestroyService in RemoveExpiredMembersWorker. --- app/workers/remove_expired_members_worker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/workers/remove_expired_members_worker.rb b/app/workers/remove_expired_members_worker.rb index 30486b9e8a3..027abf50587 100644 --- a/app/workers/remove_expired_members_worker.rb +++ b/app/workers/remove_expired_members_worker.rb @@ -2,9 +2,9 @@ class RemoveExpiredMembersWorker include Sidekiq::Worker def perform - Member.includes(:created_by).where("expires_at <= ?", Time.current).find_each do |member| + Member.where("expires_at <= ?", Time.current).find_each do |member| begin - Members::DestroyService.new(member, member.created_by).execute + Members::AuthorizedDestroyService.new(member).execute rescue => ex logger.error("Expired Member ID=#{member.id} cannot be removed - #{ex}") end From 671d247eaad5536962201a942b9af6b26e02299d Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Thu, 4 Aug 2016 23:36:50 +0200 Subject: [PATCH 013/144] Write test for RemoveExpiredMembersWorker. --- .../remove_expired_members_worker_spec.rb | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 spec/workers/remove_expired_members_worker_spec.rb diff --git a/spec/workers/remove_expired_members_worker_spec.rb b/spec/workers/remove_expired_members_worker_spec.rb new file mode 100644 index 00000000000..086f6e7c301 --- /dev/null +++ b/spec/workers/remove_expired_members_worker_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe RemoveExpiredMembersWorker do + let!(:worker) { RemoveExpiredMembersWorker.new } + let!(:expired_member) { create(:project_member, expires_at: 1.hour.ago) } + let!(:member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now) } + let!(:non_expiring_member) { create(:project_member, expires_at: nil) } + + describe "#perform" do + it "removes expired members" do + expect { worker.perform }.to change { Member.count }.by(-1) + expect(Member.find_by(id: expired_member.id)).to be_nil + end + + it "leaves members who expire in the future" do + worker.perform + expect(member_expiring_in_future.reload).to be_present + end + + it "leaves members who do not expire at all" do + worker.perform + expect(non_expiring_member.reload).to be_present + end + end +end From 895c3c55dc960e226b8d5415a4533e5faf696a0a Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Fri, 5 Aug 2016 07:28:39 +0200 Subject: [PATCH 014/144] Better presentation of member expiration. --- app/helpers/members_helper.rb | 4 ++++ app/views/shared/members/_member.html.haml | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb index ec106418f2d..183d3b77a3f 100644 --- a/app/helpers/members_helper.rb +++ b/app/helpers/members_helper.rb @@ -42,4 +42,8 @@ module MembersHelper "Are you sure you want to leave the " \ "\"#{member_source.human_name}\" #{member_source.class.to_s.humanize(capitalize: false)}?" end + + def member_expires_soon?(member) + member.expires_at < 7.days.from_now + end end diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 88e9522b9cc..74af31f3252 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -60,8 +60,9 @@ - else Joined #{time_ago_with_tooltip(member.created_at)} - if member.expires? - %span.prepend-left-20 - Access expires #{member.expires_at.to_s(:medium)} + · + %span{ class: ("text-warning" if member_expires_soon?(member)) } + Expires in #{distance_of_time_in_words_to_now(member.expires_at)} - else = image_tag avatar_icon(member.invite_email, 40), class: "avatar s40", alt: '' From 245f237dc8675e43f229ddf74c70d1f72156bc3e Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Fri, 5 Aug 2016 07:52:21 +0200 Subject: [PATCH 015/144] Update spec for adding member. --- .../members/master_adds_member_with_expiration_date_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index 87d9208465e..242919ee113 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -16,12 +16,12 @@ feature 'Projects > Members > Master adds member with expiration date', feature: scenario 'expiration date is displayed in the members list' do page.within ".users-project-form" do select2(new_member.id, from: "#user_ids", multiple: true) - fill_in "Access expiration date", with: "2016-08-02" + fill_in "Access expiration date", with: 4.days.from_now click_on "Add users to project" end page.within ".project_member:first-child" do - expect(page).to have_content("Access expires Aug 2, 2016") + expect(page).to have_content("Expires in 4 days") end end end From f564535579af71af599696a039801195a5c30f78 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Fri, 5 Aug 2016 08:36:09 +0200 Subject: [PATCH 016/144] Allow to edit access expiration date. --- .../projects/project_members_controller.rb | 2 +- app/views/shared/members/_member.html.haml | 4 ++- features/steps/group/members.rb | 2 +- features/steps/project/team_management.rb | 2 +- ...r_adds_member_with_expiration_date_spec.rb | 27 ++++++++++++++----- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 7b84be54a3a..42a7e5a2c30 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -99,7 +99,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController protected def member_params - params.require(:project_member).permit(:user_id, :access_level) + params.require(:project_member).permit(:user_id, :access_level, :expires_at) end # MembershipActions concern diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 74af31f3252..ae487f86e50 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -16,7 +16,7 @@ = button_tag icon('pencil'), type: 'button', class: 'btn inline js-toggle-button', - title: 'Edit access level' + title: 'Edit' - if member.request? = link_to icon('check inverse'), polymorphic_path([:approve_access_request, member]), @@ -80,5 +80,7 @@ = form_for member, remote: true do |f| .prepend-top-10 = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control' + .prepend-top-10 + = f.text_field :expires_at, class: 'form-control', placeholder: 'Access expiration date' .prepend-top-10 = f.submit 'Save', class: 'btn btn-save btn-sm' diff --git a/features/steps/group/members.rb b/features/steps/group/members.rb index dfa2fa75def..956a0026e1a 100644 --- a/features/steps/group/members.rb +++ b/features/steps/group/members.rb @@ -116,7 +116,7 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps member = mary_jane_member page.within "#group_member_#{member.id}" do - click_button "Edit access level" + click_button 'Edit' select 'Developer', from: 'group_member_access_level' click_on 'Save' end diff --git a/features/steps/project/team_management.rb b/features/steps/project/team_management.rb index f32576d2cb1..334ce7dd3db 100644 --- a/features/steps/project/team_management.rb +++ b/features/steps/project/team_management.rb @@ -65,7 +65,7 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps user = User.find_by(name: 'Dmitriy') project_member = project.project_members.find_by(user_id: user.id) page.within "#project_member_#{project_member.id}" do - click_button "Edit access level" + click_button 'Edit' select "Reporter", from: "project_member_access_level" click_button "Save" end diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index 242919ee113..8d706f015cf 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -10,18 +10,31 @@ feature 'Projects > Members > Master adds member with expiration date', feature: background do project.team << [master, :master] login_as(master) - visit namespace_project_project_members_path(project.namespace, project) end scenario 'expiration date is displayed in the members list' do - page.within ".users-project-form" do - select2(new_member.id, from: "#user_ids", multiple: true) - fill_in "Access expiration date", with: 4.days.from_now - click_on "Add users to project" + visit namespace_project_project_members_path(project.namespace, project) + + page.within '.users-project-form' do + select2(new_member.id, from: '#user_ids', multiple: true) + fill_in 'Access expiration date', with: 4.days.from_now + click_on 'Add users to project' end - page.within ".project_member:first-child" do - expect(page).to have_content("Expires in 4 days") + page.within '.project_member:first-child' do + expect(page).to have_content('Expires in 4 days') + end + end + + scenario 'change expiration date' do + project.team.add_users([new_member.id], :developer, expires_at: 1.month.from_now) + visit namespace_project_project_members_path(project.namespace, project) + + page.within '.project_member:first-child' do + click_on 'Edit' + fill_in 'Access expiration date', with: 2.days.from_now + click_on 'Save' + expect(page).to have_content('Expires in 2 days') end end end From 9accb302f6c1887374f4147c5316091b48c68f25 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Sat, 6 Aug 2016 19:09:04 +0200 Subject: [PATCH 017/144] Set explicit time in tests so they return consistent results regardless of the hour of execution. --- ...r_adds_member_with_expiration_date_spec.rb | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index 8d706f015cf..400c7e477d2 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' feature 'Projects > Members > Master adds member with expiration date', feature: true, js: true do include Select2Helper + include ActiveSupport::Testing::TimeHelpers let!(:master) { create(:user) } let!(:project) { create(:project) } @@ -13,28 +14,32 @@ feature 'Projects > Members > Master adds member with expiration date', feature: end scenario 'expiration date is displayed in the members list' do - visit namespace_project_project_members_path(project.namespace, project) + travel_to Time.zone.parse("2016-08-06 08:00") do + visit namespace_project_project_members_path(project.namespace, project) - page.within '.users-project-form' do - select2(new_member.id, from: '#user_ids', multiple: true) - fill_in 'Access expiration date', with: 4.days.from_now - click_on 'Add users to project' - end + page.within '.users-project-form' do + select2(new_member.id, from: '#user_ids', multiple: true) + fill_in 'Access expiration date', with: "2016-08-10" + click_on 'Add users to project' + end - page.within '.project_member:first-child' do - expect(page).to have_content('Expires in 4 days') + page.within '.project_member:first-child' do + expect(page).to have_content('Expires in 4 days') + end end end scenario 'change expiration date' do - project.team.add_users([new_member.id], :developer, expires_at: 1.month.from_now) - visit namespace_project_project_members_path(project.namespace, project) + travel_to Time.zone.parse("2016-08-06 08:00") do + project.team.add_users([new_member.id], :developer, expires_at: "2016-09-06") + visit namespace_project_project_members_path(project.namespace, project) - page.within '.project_member:first-child' do - click_on 'Edit' - fill_in 'Access expiration date', with: 2.days.from_now - click_on 'Save' - expect(page).to have_content('Expires in 2 days') + page.within '.project_member:first-child' do + click_on 'Edit' + fill_in 'Access expiration date', with: "2016-08-09" + click_on 'Save' + expect(page).to have_content('Expires in 3 days') + end end end end From 6d892244580f0176b6ec33d8437844570c815c71 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Sat, 6 Aug 2016 21:06:30 +0200 Subject: [PATCH 018/144] Add API support for expires_at. --- lib/api/entities.rb | 3 +++ lib/api/project_members.rb | 9 ++++++--- spec/requests/api/project_members_spec.rb | 11 +++++++++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 4eb95d8a215..595e634b7cb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -95,6 +95,9 @@ module API expose :access_level do |user, options| options[:project].project_members.find_by(user_id: user.id).access_level end + expose :expires_at do |user, options| + options[:project].project_members.find_by(user_id: user.id).expires_at + end end class Group < Grape::Entity diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb index 6a0b3e7d134..590658c5023 100644 --- a/lib/api/project_members.rb +++ b/lib/api/project_members.rb @@ -38,6 +38,7 @@ module API # id (required) - The ID of a project # user_id (required) - The ID of a user # access_level (required) - Project access level + # expires_at (optional) - Date string in the format YEAR-MONTH-DAY # Example Request: # POST /projects/:id/members post ":id/members" do @@ -49,7 +50,8 @@ module API if project_member.nil? project_member = user_project.project_members.new( user_id: params[:user_id], - access_level: params[:access_level] + access_level: params[:access_level], + expires_at: params[:expires_at] ) end @@ -67,16 +69,17 @@ module API # id (required) - The ID of a project # user_id (required) - The ID of a team member # access_level (required) - Project access level + # expires_at (optional) - Date string in the format YEAR-MONTH-DAY # Example Request: # PUT /projects/:id/members/:user_id put ":id/members/:user_id" do authorize! :admin_project, user_project required_attributes! [:access_level] - + attrs = attributes_for_keys [:access_level, :expires_at] project_member = user_project.project_members.find_by(user_id: params[:user_id]) not_found!("User can not be found") if project_member.nil? - if project_member.update_attributes(access_level: params[:access_level]) + if project_member.update_attributes(attrs) @member = project_member.user present @member, with: Entities::ProjectMember, project: user_project else diff --git a/spec/requests/api/project_members_spec.rb b/spec/requests/api/project_members_spec.rb index 9a7c1da4401..360bf205762 100644 --- a/spec/requests/api/project_members_spec.rb +++ b/spec/requests/api/project_members_spec.rb @@ -54,12 +54,16 @@ describe API::API, api: true do describe "POST /projects/:id/members" do it "should add user to project team" do expect do - post api("/projects/#{project.id}/members", user), user_id: user2.id, access_level: ProjectMember::DEVELOPER + post api("/projects/#{project.id}/members", user), + user_id: user2.id, + access_level: ProjectMember::DEVELOPER, + expires_at: '2016-08-05' end.to change { ProjectMember.count }.by(1) expect(response).to have_http_status(201) expect(json_response['username']).to eq(user2.username) expect(json_response['access_level']).to eq(ProjectMember::DEVELOPER) + expect(json_response['expires_at']).to eq('2016-08-05') end it "should return a 201 status if user is already project member" do @@ -95,10 +99,13 @@ describe API::API, api: true do before { project_member2 } it "should update project team member" do - put api("/projects/#{project.id}/members/#{user3.id}", user), access_level: ProjectMember::MASTER + put api("/projects/#{project.id}/members/#{user3.id}", user), + access_level: ProjectMember::MASTER, + expires_at: '2016-08-05' expect(response).to have_http_status(200) expect(json_response['username']).to eq(user3.username) expect(json_response['access_level']).to eq(ProjectMember::MASTER) + expect(json_response['expires_at']).to eq('2016-08-05') end it "should return a 404 error if user_id is not found" do From 0c3ac8276811ec17608acc56f02a03a0890f809a Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Sat, 6 Aug 2016 22:53:48 +0200 Subject: [PATCH 019/144] Remove inline JavaScript. Add datepicker to the edit form. --- app/assets/javascripts/project_members.js | 7 +++++++ .../project_members/_new_project_member.html.haml | 9 ++------- app/views/projects/project_members/index.html.haml | 2 +- app/views/shared/members/_member.html.haml | 5 +++-- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/project_members.js b/app/assets/javascripts/project_members.js index f6a796b325a..8171fa8d533 100644 --- a/app/assets/javascripts/project_members.js +++ b/app/assets/javascripts/project_members.js @@ -4,6 +4,13 @@ $('li.project_member').bind('ajax:success', function() { return $(this).fadeOut(); }); + + $('.js-project-members-page').on('focus', '.js-access-expiration-date', function() { + $(this).datepicker({ + dateFormat: 'yy-mm-dd', + minDate: 1 + }); + }); } return ProjectMembers; diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml index 5aa0950523b..0545ed07d62 100644 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ b/app/views/projects/project_members/_new_project_member.html.haml @@ -15,16 +15,11 @@ %strong= link_to "here", help_page_path("user/permissions"), class: "vlink" .form-group - = label_tag :expires_at, "Access expiration date", class: 'control-label' + = label_tag :expires_at, 'Access expiration date', class: 'control-label' .col-sm-10 - = text_field_tag :expires_at, nil, class: "datepicker form-control", placeholder: "Select access expiration date" + = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date' .help-block Leave it empty if you do not want this user's access to expire. .form-actions = f.submit 'Add users to project', class: "btn btn-create" - -:javascript - $(".datepicker").datepicker({ - dateFormat: "yy-mm-dd" - }); diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index 9031f01b496..9d063b3081f 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -1,6 +1,6 @@ - page_title "Members" -.project-members-page.prepend-top-default +.project-members-page.js-project-members-page.prepend-top-default - if can?(current_user, :admin_project_member, @project) .panel.panel-default .panel-heading diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index ae487f86e50..c215445f123 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -80,7 +80,8 @@ = form_for member, remote: true do |f| .prepend-top-10 = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control' - .prepend-top-10 - = f.text_field :expires_at, class: 'form-control', placeholder: 'Access expiration date' + - if member.type == 'ProjectMember' + .prepend-top-10 + = f.text_field :expires_at, class: 'form-control js-access-expiration-date', placeholder: 'Access expiration date', id: "member_expires_at_#{member.id}" .prepend-top-10 = f.submit 'Save', class: 'btn btn-save btn-sm' From 245d4abbbf3d8a1a74b730f4b95a29218de73ee3 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Sun, 7 Aug 2016 13:42:54 +0200 Subject: [PATCH 020/144] Single quotes all the way. --- app/views/shared/members/_member.html.haml | 2 +- app/workers/remove_expired_members_worker.rb | 2 +- .../master_adds_member_with_expiration_date_spec.rb | 10 +++++----- spec/workers/remove_expired_members_worker_spec.rb | 8 ++++---- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index c215445f123..0db7ecb61bb 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -61,7 +61,7 @@ Joined #{time_ago_with_tooltip(member.created_at)} - if member.expires? · - %span{ class: ("text-warning" if member_expires_soon?(member)) } + %span{ class: ('text-warning' if member_expires_soon?(member)) } Expires in #{distance_of_time_in_words_to_now(member.expires_at)} - else diff --git a/app/workers/remove_expired_members_worker.rb b/app/workers/remove_expired_members_worker.rb index 027abf50587..2d773ec72df 100644 --- a/app/workers/remove_expired_members_worker.rb +++ b/app/workers/remove_expired_members_worker.rb @@ -2,7 +2,7 @@ class RemoveExpiredMembersWorker include Sidekiq::Worker def perform - Member.where("expires_at <= ?", Time.current).find_each do |member| + Member.where('expires_at <= ?', Time.current).find_each do |member| begin Members::AuthorizedDestroyService.new(member).execute rescue => ex diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index 400c7e477d2..47a72a82cf3 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -14,12 +14,12 @@ feature 'Projects > Members > Master adds member with expiration date', feature: end scenario 'expiration date is displayed in the members list' do - travel_to Time.zone.parse("2016-08-06 08:00") do + travel_to Time.zone.parse('2016-08-06 08:00') do visit namespace_project_project_members_path(project.namespace, project) page.within '.users-project-form' do select2(new_member.id, from: '#user_ids', multiple: true) - fill_in 'Access expiration date', with: "2016-08-10" + fill_in 'Access expiration date', with: '2016-08-10' click_on 'Add users to project' end @@ -30,13 +30,13 @@ feature 'Projects > Members > Master adds member with expiration date', feature: end scenario 'change expiration date' do - travel_to Time.zone.parse("2016-08-06 08:00") do - project.team.add_users([new_member.id], :developer, expires_at: "2016-09-06") + travel_to Time.zone.parse('2016-08-06 08:00') do + project.team.add_users([new_member.id], :developer, expires_at: '2016-09-06') visit namespace_project_project_members_path(project.namespace, project) page.within '.project_member:first-child' do click_on 'Edit' - fill_in 'Access expiration date', with: "2016-08-09" + fill_in 'Access expiration date', with: '2016-08-09' click_on 'Save' expect(page).to have_content('Expires in 3 days') end diff --git a/spec/workers/remove_expired_members_worker_spec.rb b/spec/workers/remove_expired_members_worker_spec.rb index 086f6e7c301..75a5bee7ab4 100644 --- a/spec/workers/remove_expired_members_worker_spec.rb +++ b/spec/workers/remove_expired_members_worker_spec.rb @@ -6,18 +6,18 @@ describe RemoveExpiredMembersWorker do let!(:member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now) } let!(:non_expiring_member) { create(:project_member, expires_at: nil) } - describe "#perform" do - it "removes expired members" do + describe '#perform' do + it 'removes expired members' do expect { worker.perform }.to change { Member.count }.by(-1) expect(Member.find_by(id: expired_member.id)).to be_nil end - it "leaves members who expire in the future" do + it 'leaves members who expire in the future' do worker.perform expect(member_expiring_in_future.reload).to be_present end - it "leaves members who do not expire at all" do + it 'leaves members who do not expire at all' do worker.perform expect(non_expiring_member.reload).to be_present end From 19c9dbf73b3f7bf406781ddd4e04b8e98e71fc44 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Sun, 7 Aug 2016 14:24:58 +0200 Subject: [PATCH 021/144] Update API documentation. --- doc/api/projects.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/doc/api/projects.md b/doc/api/projects.md index 0ba0bffb4ac..454129b0234 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -894,10 +894,13 @@ Parameters: "name": "John Smith", "state": "active", "created_at": "2012-05-23T08:00:58Z", - "access_level": 40 + "access_level": 40, + "expires_at": null } ``` +>**Note:** The expiration date feature is not released yet. + ### Add project team member Adds a user to a project team. This is an idempotent method and can be called multiple times @@ -913,6 +916,9 @@ Parameters: - `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project - `user_id` (required) - The ID of a user to add - `access_level` (required) - Project access level +- `expires_at` (optional) - Date string in the format YEAR-MONTH-DAY + +>**Note:** Setting the expiration date is not released yet. ### Edit project team member @@ -927,6 +933,9 @@ Parameters: - `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project - `user_id` (required) - The ID of a team member - `access_level` (required) - Project access level +- `expires_at` (optional) - Date string in the format YEAR-MONTH-DAY + +>**Note:** Setting the expiration date is not released yet. ### Remove project team member From 23c1f9f4cd5ad2e29e975de9716e2ce0c7dcc623 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Sun, 7 Aug 2016 14:45:45 +0200 Subject: [PATCH 022/144] Add CHANGELOG entry. --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 9b66108c160..7d62e44698f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -36,6 +36,7 @@ v 8.11.0 (unreleased) - Make error pages responsive (Takuya Noguchi) - Change requests_profiles resource constraint to catch virtually any file - Reduce number of queries made for merge_requests/:id/diffs + - Add the option to set the expiration date for the project membership when giving a user access to a project. !5599 (Adam Niedzielski) v 8.10.3 (unreleased) - Fix hooks missing on imported GitLab projects From 3861a8c5fca96bf374e1f210a4e2082be01485f7 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Mon, 8 Aug 2016 10:41:15 -0500 Subject: [PATCH 023/144] Fix button missing type --- CHANGELOG | 1 + app/views/projects/issues/show.html.haml | 2 +- app/views/projects/merge_requests/show/_mr_title.html.haml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 77bcea54cf9..a21f92bb40b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -56,6 +56,7 @@ v 8.11.0 (unreleased) - Add the `sprockets-es6` gem - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska) - Profile requests when a header is passed + - Fix button missing type (ClemMakesApps) - Avoid calculation of line_code and position for _line partial when showing diff notes on discussion tab. - Speedup DiffNote#active? on discussions, preloading noteables and avoid touching git repository to return diff_refs when possible - Add commit stats in commit api. !5517 (dixpac) diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index e5cce16a171..2f248fbfc1d 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -22,7 +22,7 @@ - if can?(current_user, :create_issue, @project) || can?(current_user, :update_issue, @issue) .issuable-actions .clearfix.issue-btn-group.dropdown - %button.btn.btn-default.pull-left.hidden-md.hidden-lg{ data: { toggle: "dropdown" } } + %button.btn.btn-default.pull-left.hidden-md.hidden-lg{ type: "button", data: { toggle: "dropdown" } } %span.caret Options .dropdown-menu.dropdown-menu-align-right.hidden-lg diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml index b24bdf22ceb..098ce19da21 100644 --- a/app/views/projects/merge_requests/show/_mr_title.html.haml +++ b/app/views/projects/merge_requests/show/_mr_title.html.haml @@ -14,7 +14,7 @@ - if can?(current_user, :update_merge_request, @merge_request) .issuable-actions .clearfix.issue-btn-group.dropdown - %button.btn.btn-default.pull-left.hidden-md.hidden-lg{ data: { toggle: "dropdown" } } + %button.btn.btn-default.pull-left.hidden-md.hidden-lg{ type: "button", data: { toggle: "dropdown" } } %span.caret Options .dropdown-menu.dropdown-menu-align-right.hidden-lg From c887d2131f02ffe88722d276e3a8cf3bcf9a74f2 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 8 Aug 2016 10:36:32 -0600 Subject: [PATCH 024/144] Fix small image previews in the file viewer. See also https://gitlab.com/gitlab-org/gitlab-ce/issues/20505#note_13670085 --- app/assets/stylesheets/framework/files.scss | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 407f1873431..1ccc331458b 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -63,9 +63,10 @@ &.image_file { background: #eee; text-align: center; + img { - padding: 100px; - max-width: 50%; + padding: 20px; + max-width: 100%; } } From 5643cd6b497c2850fb585cf263d3019953c282f5 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 9 Aug 2016 18:42:42 +0800 Subject: [PATCH 025/144] Show wall-clock time when showing pipeline instead of: cumulative builds time. Closes #17007 --- app/helpers/time_helper.rb | 5 +++++ app/models/ci/pipeline.rb | 4 ++++ app/views/projects/ci/pipelines/_pipeline.html.haml | 4 ++-- app/views/projects/pipelines/_info.html.haml | 4 ++-- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/helpers/time_helper.rb b/app/helpers/time_helper.rb index 790001222f1..a031abb8676 100644 --- a/app/helpers/time_helper.rb +++ b/app/helpers/time_helper.rb @@ -17,6 +17,11 @@ module TimeHelper def duration_in_numbers(finished_at, started_at) interval = interval_in_seconds(started_at, finished_at) + + duration_in_numbers_from_interval(interval) + end + + def duration_in_numbers_from_interval(interval) time_format = interval < 1.hour ? "%M:%S" : "%H:%M:%S" Time.at(interval).utc.strftime(time_format) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index bce6a992af6..db29df668bf 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -213,6 +213,10 @@ module Ci ] end + def wall_clock_duration + finished_at.to_i - started_at.to_i if finished_at && started_at + end + private def build_builds_for_stages(stages, user, status, trigger_request) diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 9a594877803..60c3c27bd1a 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -46,10 +46,10 @@ \- %td - - if pipeline.started_at && pipeline.finished_at + - if pipeline.wall_clock_duration %p.duration = custom_icon("icon_timer") - = duration_in_numbers(pipeline.finished_at, pipeline.started_at) + = duration_in_numbers_from_interval(pipeline.wall_clock_duration) - if pipeline.finished_at %p.finished-at = icon("calendar") diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index 8289aefcde7..a0f6c19350d 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -7,9 +7,9 @@ - if @pipeline.ref for = link_to @pipeline.ref, namespace_project_commits_path(@project.namespace, @project, @pipeline.ref), class: "monospace" - - if @pipeline.duration + - if @pipeline.wall_clock_duration in - = time_interval_in_words @pipeline.duration + = time_interval_in_words(@pipeline.wall_clock_duration) .pull-right = link_to namespace_project_pipeline_path(@project.namespace, @project, @pipeline), class: "ci-status ci-#{@pipeline.status}" do From 831b6c8fda1adec32c0719500ad3e830284fb2af Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 9 Aug 2016 19:20:31 +0800 Subject: [PATCH 026/144] Add a changelog entry --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 7bfeff2a4ec..7db10bdfe5b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ v 8.11.0 (unreleased) - Expand commit message width in repo view (ClemMakesApps) - Cache highlighted diff lines for merge requests - Fix of 'Commits being passed to custom hooks are already reachable when using the UI' + - Show wall clock time when showing a pipeline. !5734 - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable - Optimize maximum user access level lookup in loading of notes - Add "No one can push" as an option for protected branches. !5081 From 08ecf0e78e1f0f3073fb0d2fa3dfa8d29acbbd04 Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Tue, 9 Aug 2016 17:45:22 +0200 Subject: [PATCH 027/144] Use "is_a?" because it is more readable than checking the "type" field. --- app/views/shared/members/_member.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 0db7ecb61bb..7dde9442e24 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -80,7 +80,7 @@ = form_for member, remote: true do |f| .prepend-top-10 = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control' - - if member.type == 'ProjectMember' + - if member.is_a?(ProjectMember) .prepend-top-10 = f.text_field :expires_at, class: 'form-control js-access-expiration-date', placeholder: 'Access expiration date', id: "member_expires_at_#{member.id}" .prepend-top-10 From c90d167b5ec020138f7d72cb1006d1436c980a2a Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Tue, 9 Aug 2016 18:39:13 +0200 Subject: [PATCH 028/144] Create Member.expired scope. --- app/models/member.rb | 1 + app/workers/remove_expired_members_worker.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/member.rb b/app/models/member.rb index bcb6e32d53a..84bbbffe718 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -31,6 +31,7 @@ class Member < ActiveRecord::Base scope :non_invite, -> { where(invite_token: nil) } scope :request, -> { where.not(requested_at: nil) } scope :has_access, -> { where('access_level > 0') } + scope :expired, -> { where('expires_at <= ?', Time.current) } scope :guests, -> { where(access_level: GUEST) } scope :reporters, -> { where(access_level: REPORTER) } diff --git a/app/workers/remove_expired_members_worker.rb b/app/workers/remove_expired_members_worker.rb index 2d773ec72df..cf765af97ce 100644 --- a/app/workers/remove_expired_members_worker.rb +++ b/app/workers/remove_expired_members_worker.rb @@ -2,7 +2,7 @@ class RemoveExpiredMembersWorker include Sidekiq::Worker def perform - Member.where('expires_at <= ?', Time.current).find_each do |member| + Member.expired.find_each do |member| begin Members::AuthorizedDestroyService.new(member).execute rescue => ex From 21fdc1edaecf228c1fdc540375bbdad0fd69164a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 10 Aug 2016 22:45:30 +0800 Subject: [PATCH 029/144] Cleanup the use of duration and optimize some queries --- app/helpers/time_helper.rb | 22 +++---------------- app/models/ci/pipeline.rb | 10 ++++----- app/models/commit_status.rb | 14 ++++++------ app/models/concerns/statuseable.rb | 15 ++++++++----- app/views/admin/builds/_build.html.haml | 2 +- app/views/projects/ci/builds/_build.html.haml | 2 +- .../projects/ci/pipelines/_pipeline.html.haml | 4 ++-- app/views/projects/pipelines/_info.html.haml | 4 ++-- spec/helpers/time_helper_spec.rb | 16 +++++++------- 9 files changed, 39 insertions(+), 50 deletions(-) diff --git a/app/helpers/time_helper.rb b/app/helpers/time_helper.rb index a031abb8676..271e839692a 100644 --- a/app/helpers/time_helper.rb +++ b/app/helpers/time_helper.rb @@ -15,25 +15,9 @@ module TimeHelper "#{from.to_s(:short)} - #{to.to_s(:short)}" end - def duration_in_numbers(finished_at, started_at) - interval = interval_in_seconds(started_at, finished_at) + def duration_in_numbers(duration) + time_format = duration < 1.hour ? "%M:%S" : "%H:%M:%S" - duration_in_numbers_from_interval(interval) - end - - def duration_in_numbers_from_interval(interval) - time_format = interval < 1.hour ? "%M:%S" : "%H:%M:%S" - - Time.at(interval).utc.strftime(time_format) - end - - private - - def interval_in_seconds(started_at, finished_at = nil) - if started_at && finished_at - finished_at.to_i - started_at.to_i - elsif started_at - Time.now.to_i - started_at.to_i - end + Time.at(duration).utc.strftime(time_format) end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index db29df668bf..8b1d80299cc 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -34,6 +34,10 @@ module Ci CommitStatus.where(pipeline: pluck(:id)).stages end + def self.duration + where.not(duration: nil).pluck(:duration).inject(0, &:+) + end + def project_id project.id end @@ -213,10 +217,6 @@ module Ci ] end - def wall_clock_duration - finished_at.to_i - started_at.to_i if finished_at && started_at - end - private def build_builds_for_stages(stages, user, status, trigger_request) @@ -240,7 +240,7 @@ module Ci end self.started_at = statuses.started_at self.finished_at = statuses.finished_at - self.duration = statuses.latest.duration + self.duration = calculate_duration.to_i save end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 2d185c28809..5d0a28ecfcc 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -21,6 +21,7 @@ class CommitStatus < ActiveRecord::Base where(id: max_id.group(:name, :commit_id)) end + scope :retried, -> { where.not(id: latest) } scope :ordered, -> { order(:name) } scope :ignored, -> { where(allow_failure: true, status: [:failed, :canceled]) } @@ -83,18 +84,17 @@ class CommitStatus < ActiveRecord::Base end end + def self.duration + select(:started_at, :finished_at).all. + map(&:duration).compact.inject(0, &:+) + end + def ignored? allow_failure? && (failed? || canceled?) end def duration - duration = - if started_at && finished_at - finished_at - started_at - elsif started_at - Time.now - started_at - end - duration + calculate_duration end def stuck? diff --git a/app/models/concerns/statuseable.rb b/app/models/concerns/statuseable.rb index 44c6b30f278..37efc44b79b 100644 --- a/app/models/concerns/statuseable.rb +++ b/app/models/concerns/statuseable.rb @@ -31,11 +31,6 @@ module Statuseable all.pluck(self.status_sql).first end - def duration - duration_array = all.map(&:duration).compact - duration_array.reduce(:+) - end - def started_at all.minimum(:started_at) end @@ -78,4 +73,14 @@ module Statuseable def complete? canceled? || success? || failed? end + + private + + def calculate_duration + if started_at && finished_at + finished_at - started_at + elsif started_at + Time.now - started_at + end + end end diff --git a/app/views/admin/builds/_build.html.haml b/app/views/admin/builds/_build.html.haml index 352adbedee4..f29d9c94441 100644 --- a/app/views/admin/builds/_build.html.haml +++ b/app/views/admin/builds/_build.html.haml @@ -51,7 +51,7 @@ - if build.duration %p.duration = custom_icon("icon_timer") - = duration_in_numbers(build.finished_at, build.started_at) + = duration_in_numbers(build.duration) - if build.finished_at %p.finished-at diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 91081435220..1fdf32466f2 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -63,7 +63,7 @@ - if build.duration %p.duration = custom_icon("icon_timer") - = duration_in_numbers(build.finished_at, build.started_at) + = duration_in_numbers(build.duration) - if build.finished_at %p.finished-at = icon("calendar") diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 60c3c27bd1a..c2fecbf9d4c 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -46,10 +46,10 @@ \- %td - - if pipeline.wall_clock_duration + - if pipeline.duration %p.duration = custom_icon("icon_timer") - = duration_in_numbers_from_interval(pipeline.wall_clock_duration) + = duration_in_numbers(pipeline.duration) - if pipeline.finished_at %p.finished-at = icon("calendar") diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index a0f6c19350d..063e83a407a 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -7,9 +7,9 @@ - if @pipeline.ref for = link_to @pipeline.ref, namespace_project_commits_path(@project.namespace, @project, @pipeline.ref), class: "monospace" - - if @pipeline.wall_clock_duration + - if @pipeline.duration in - = time_interval_in_words(@pipeline.wall_clock_duration) + = time_interval_in_words(@pipeline.duration) .pull-right = link_to namespace_project_pipeline_path(@project.namespace, @project, @pipeline), class: "ci-status ci-#{@pipeline.status}" do diff --git a/spec/helpers/time_helper_spec.rb b/spec/helpers/time_helper_spec.rb index bf3ed5c094c..21f35585367 100644 --- a/spec/helpers/time_helper_spec.rb +++ b/spec/helpers/time_helper_spec.rb @@ -19,16 +19,16 @@ describe TimeHelper do describe "#duration_in_numbers" do it "returns minutes and seconds" do - duration_in_numbers = { - [100, 0] => "01:40", - [121, 0] => "02:01", - [3721, 0] => "01:02:01", - [0, 0] => "00:00", - [nil, Time.now.to_i - 42] => "00:42" + durations_and_expectations = { + 100 => "01:40", + 121 => "02:01", + 3721 => "01:02:01", + 0 => "00:00", + 42 => "00:42" } - duration_in_numbers.each do |interval, expectation| - expect(duration_in_numbers(*interval)).to eq(expectation) + durations_and_expectations.each do |duration, expectation| + expect(duration_in_numbers(duration)).to eq(expectation) end end end From 9cb3fcda281853f5bdbb4c3a716a875ce72928e4 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Tue, 9 Aug 2016 11:58:42 -0500 Subject: [PATCH 030/144] Fix spacing and vertical alignment on build status icon on commits page --- CHANGELOG | 1 + app/assets/stylesheets/pages/commit.scss | 20 +++++++++++++++++++ .../projects/commit/_commit_box.html.haml | 3 ++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 77bcea54cf9..62b8c9e0ac1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -52,6 +52,7 @@ v 8.11.0 (unreleased) - Gitlab::Metrics.current_transaction needs to be public for RailsQueueDuration - Fix search for notes which belongs to deleted objects - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska) + - Fix spacing and vertical alignment on build status icon on commits page (ClemMakesApps) - Allow branch names ending with .json for graph and network page !5579 (winniehell) - Add the `sprockets-es6` gem - Multiple trigger variables show in separate lines (Katarzyna Kobierska Ula Budziszewska) diff --git a/app/assets/stylesheets/pages/commit.scss b/app/assets/stylesheets/pages/commit.scss index bbe0c6c5f1f..fe8a8025038 100644 --- a/app/assets/stylesheets/pages/commit.scss +++ b/app/assets/stylesheets/pages/commit.scss @@ -66,6 +66,26 @@ margin-left: 8px; } } + + .ci-status-link { + margin-left: 2px; + + svg { + vertical-align: middle; + } + + .ci-status-label { + display: inline-block; + } + + &:hover { + text-decoration: none; + + .ci-status-label { + text-decoration: underline; + } + } + } } .ci-status-link { diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 3ad866bb2f1..14adee7a6e9 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -56,7 +56,8 @@ = pluralize(@commit.pipelines.count, 'pipeline') = link_to builds_namespace_project_commit_path(@project.namespace, @project, @commit.id), class: "ci-status-link ci-status-icon-#{@commit.status}" do = ci_icon_for_status(@commit.status) - = ci_label_for_status(@commit.status) + %span.ci-status-label + = ci_label_for_status(@commit.status) - if @commit.pipelines.duration in = time_interval_in_words @commit.pipelines.duration From 761031c974de2c389ddc589a16776acd0b293030 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 12 Aug 2016 11:02:46 -0600 Subject: [PATCH 031/144] Decrease max-width for image previews to 80%. --- app/assets/stylesheets/framework/files.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 1ccc331458b..d3e3fc50736 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -66,7 +66,7 @@ img { padding: 20px; - max-width: 100%; + max-width: 80%; } } From 3eba2e4f70f18f2a86d0134e264e963d67cbac45 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Thu, 4 Aug 2016 14:29:45 -0500 Subject: [PATCH 032/144] Wrap single lines of code; horizontally scroll multi-line code blocks --- CHANGELOG | 1 + app/assets/stylesheets/framework/typography.scss | 10 +++++++++- app/assets/stylesheets/pages/detail_page.scss | 8 ++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 96965a20f69..11ca5a414bc 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -95,6 +95,7 @@ v 8.11.0 (unreleased) - Sensible state specific default sort order for issues and merge requests !5453 (tomb0y) - Fix bug where destroying a namespace would not always destroy projects - Fix RequestProfiler::Middleware error when code is reloaded in development + - Allow horizontal scrolling of code blocks in issue body - Catch what warden might throw when profiling requests to re-throw it - Avoid commit lookup on diff_helper passing existing local variable to the helper method - Add description to new_issue email and new_merge_request_email in text/plain content type. !5663 (dixpac) diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 8659604cb8b..06874a993fa 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -14,12 +14,20 @@ margin-top: 0; } + // Single code lines should wrap code { font-family: $monospace_font; - white-space: pre; + white-space: pre-wrap; word-wrap: normal; } + // Multi-line code blocks should scroll horizontally + pre { + code { + white-space: pre; + } + } + kbd { display: inline-block; padding: 3px 5px; diff --git a/app/assets/stylesheets/pages/detail_page.scss b/app/assets/stylesheets/pages/detail_page.scss index 1b389d83525..742984d6c7a 100644 --- a/app/assets/stylesheets/pages/detail_page.scss +++ b/app/assets/stylesheets/pages/detail_page.scss @@ -36,9 +36,17 @@ } .wiki { + // Single lines of code should wrap code { white-space: pre-wrap; word-break: keep-all; } + + // Code blocks should scroll horizontally + pre { + code { + white-space: pre; + } + } } } From 4d757ea5020dbace177bb0e9773046ee939396de Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Thu, 4 Aug 2016 14:57:29 -0500 Subject: [PATCH 033/144] Remove unneeded code --- app/assets/stylesheets/pages/detail_page.scss | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/app/assets/stylesheets/pages/detail_page.scss b/app/assets/stylesheets/pages/detail_page.scss index 742984d6c7a..4d9c73c6840 100644 --- a/app/assets/stylesheets/pages/detail_page.scss +++ b/app/assets/stylesheets/pages/detail_page.scss @@ -34,19 +34,4 @@ } } } - - .wiki { - // Single lines of code should wrap - code { - white-space: pre-wrap; - word-break: keep-all; - } - - // Code blocks should scroll horizontally - pre { - code { - white-space: pre; - } - } - } } From 47ed793af9430eb9dd6b2ae957e16d07cdd360b4 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 15 Aug 2016 18:47:26 +0800 Subject: [PATCH 034/144] Update duration for wall-clock time --- app/models/ci/pipeline.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index fb88f6d811d..581acdf8e51 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -256,7 +256,7 @@ module Ci end def update_duration - self.duration = statuses.latest.duration + self.duration = calculate_duration end def execute_hooks From 7d9bdac6cef6c2e59c8d8753cd54bf5516bfdefc Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 15 Aug 2016 18:50:02 +0800 Subject: [PATCH 035/144] We could just sum with SQL --- app/models/ci/pipeline.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 581acdf8e51..a57ae0085ca 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -79,7 +79,7 @@ module Ci end def self.duration - where.not(duration: nil).pluck(:duration).inject(0, &:+) + where.not(duration: nil).sum(:duration) end def project_id From 5e7d99d3dc3cc7c5cf644f15610ed9813b45cb41 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 15 Aug 2016 18:54:38 +0800 Subject: [PATCH 036/144] Rename to total_duration and we're not using CommitStatus.duration --- app/models/ci/pipeline.rb | 2 +- app/models/commit_status.rb | 5 ----- app/views/projects/commit/_commit_box.html.haml | 5 ++--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index a57ae0085ca..19335ccea57 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -78,7 +78,7 @@ module Ci CommitStatus.where(pipeline: pluck(:id)).stages end - def self.duration + def self.total_duration where.not(duration: nil).sum(:duration) end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index ac589fa2e31..9e1e9b76ce4 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -103,11 +103,6 @@ class CommitStatus < ActiveRecord::Base end end - def self.duration - select(:started_at, :finished_at).all. - map(&:duration).compact.inject(0, &:+) - end - def ignored? allow_failure? && (failed? || canceled?) end diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 3ad866bb2f1..f733d49224e 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -57,9 +57,8 @@ = link_to builds_namespace_project_commit_path(@project.namespace, @project, @commit.id), class: "ci-status-link ci-status-icon-#{@commit.status}" do = ci_icon_for_status(@commit.status) = ci_label_for_status(@commit.status) - - if @commit.pipelines.duration - in - = time_interval_in_words @commit.pipelines.duration + in + = time_interval_in_words @commit.pipelines.total_duration .commit-box.content-block %h3.commit-title From ce641335167230d288fbb2ec277548acbaff9dd7 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 15 Aug 2016 21:32:36 +0800 Subject: [PATCH 037/144] Introduce Gitlab::Utils.now so that it's easier to stub --- app/models/ci/pipeline.rb | 4 ++-- app/models/concerns/statuseable.rb | 2 +- lib/gitlab/utils.rb | 7 ++++++- spec/models/ci/pipeline_spec.rb | 9 ++++++--- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 5b9754b5c37..72c2dc469a9 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -48,11 +48,11 @@ module Ci end before_transition [:created, :pending] => :running do |pipeline| - pipeline.started_at = Time.now + pipeline.started_at = Gitlab::Utils.now end before_transition any => [:success, :failed, :canceled] do |pipeline| - pipeline.finished_at = Time.now + pipeline.finished_at = Gitlab::Utils.now end before_transition do |pipeline| diff --git a/app/models/concerns/statuseable.rb b/app/models/concerns/statuseable.rb index 750f937b724..9f73c5c9467 100644 --- a/app/models/concerns/statuseable.rb +++ b/app/models/concerns/statuseable.rb @@ -87,7 +87,7 @@ module Statuseable if started_at && finished_at finished_at - started_at elsif started_at - Time.now - started_at + Gitlab::Utils.now - started_at end end end diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index d13fe0ef8a9..4d1bd16eb95 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -7,11 +7,16 @@ module Gitlab # @param cmd [Array] # @return [Boolean] def system_silent(cmd) - Popen::popen(cmd).last.zero? + Popen.popen(cmd).last.zero? end def force_utf8(str) str.force_encoding(Encoding::UTF_8) end + + # The same as Time.now but using this would make it easier to test + def now + Time.now + end end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 8137e9f8f71..67bd23a1ccb 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -129,12 +129,15 @@ describe Ci::Pipeline, models: true do describe '#duration' do before do - build.skip - build2.skip + allow(Gitlab::Utils).to receive(:now). + and_return(current - 120, current) + + pipeline.run + pipeline.succeed end it 'matches sum of builds duration' do - expect(pipeline.reload.duration).to eq(build.duration + build2.duration) + expect(pipeline.reload.duration).to eq(120) end end From 74c5f82fb034f3d25f4c32fb0fac93beb6b4623f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 15 Aug 2016 21:51:13 +0800 Subject: [PATCH 038/144] pipeline duration no longer depends on builds --- spec/models/ci/pipeline_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 67bd23a1ccb..9478b011a71 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -124,8 +124,7 @@ describe Ci::Pipeline, models: true do describe 'state machine' do let(:current) { Time.now.change(usec: 0) } - let(:build) { create :ci_build, name: 'build1', pipeline: pipeline, started_at: current - 60, finished_at: current } - let(:build2) { create :ci_build, name: 'build2', pipeline: pipeline, started_at: current - 60, finished_at: current } + let(:build) { create :ci_build, name: 'build1', pipeline: pipeline } describe '#duration' do before do From 15bbab880eb9673b8120f6a34f3c145a88a58a53 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 Aug 2016 15:43:04 +0000 Subject: [PATCH 039/144] Add test requirement for new files --- CONTRIBUTING.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fbc8e15bebf..b393e4157a9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -387,7 +387,9 @@ description area. Copy-paste it to retain the markdown format. 1. The change is as small as possible 1. Include proper tests and make all tests pass (unless it contains a test - exposing a bug in existing code) + exposing a bug in existing code). Every new file with code you created + must have a corresponging file with tests even if functionality is + already tested somewhere else. 1. If you suspect a failing CI build is unrelated to your contribution, you may try and restart the failing CI job or ask a developer to fix the aforementioned failing test From fdf283ab2e504a40aaac3ff5c38a8445d2047c44 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Mon, 15 Aug 2016 13:41:55 -0500 Subject: [PATCH 040/144] Fix commit mention font inconsistency --- CHANGELOG | 1 + app/assets/stylesheets/framework/gfm.scss | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index ccc60846787..28bae17e5c3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,7 @@ v 8.11.0 (unreleased) - Ignore URLs starting with // in Markdown links !5677 (winniehell) - Fix CI status icon link underline (ClemMakesApps) - The Repository class is now instrumented + - Fix commit mention font inconsistency (ClemMakesApps) - Fix filter label tooltip HTML rendering (ClemMakesApps) - Cache the commit author in RequestStore to avoid extra lookups in PostReceive - Expand commit message width in repo view (ClemMakesApps) diff --git a/app/assets/stylesheets/framework/gfm.scss b/app/assets/stylesheets/framework/gfm.scss index f4d35c4b4b1..c0de09f3968 100644 --- a/app/assets/stylesheets/framework/gfm.scss +++ b/app/assets/stylesheets/framework/gfm.scss @@ -2,7 +2,7 @@ * Styles that apply to all GFM related forms. */ -.gfm-commit, .gfm-commit_range { +.gfm-commit_range { font-family: $monospace_font; font-size: 90%; } From 054d8fba0347c37c1c85629e31aca89833b9ee57 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Fri, 5 Aug 2016 14:51:29 -0500 Subject: [PATCH 041/144] Fix branch title trailing space on hover --- CHANGELOG | 1 + app/views/projects/branches/_branch.html.haml | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e9c8a4895e1..35cff98afd0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -44,6 +44,7 @@ v 8.11.0 (unreleased) - Add hover state to todos !5361 (winniehell) - Limit git rev-list output count to one in forced push check - Show deployment status on merge requests with external URLs + - Fix branch title trailing space on hover (ClemMakesApps) - Clean up unused routes (Josef Strzibny) - Fix issue on empty project to allow developers to only push to protected branches if given permission - Add green outline to New Branch button. !5447 (winniehell) diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 4bd85061240..6192ccb710b 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -5,8 +5,8 @@ - number_commits_ahead = diverging_commit_counts[:ahead] %li(class="js-branch-#{branch.name}") %div - = link_to namespace_project_tree_path(@project.namespace, @project, branch.name) do - %span.item-title.str-truncated= branch.name + = link_to namespace_project_tree_path(@project.namespace, @project, branch.name), class: 'item-title str-truncated' do + = branch.name   - if branch.name == @repository.root_ref %span.label.label-primary default From 41d598ce5004d23c6af7961284cfd9055297ba09 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 1 Aug 2016 13:56:56 -0600 Subject: [PATCH 042/144] Initial implementation of an async branch dropdown for Revert and Cherry Pick. --- app/assets/stylesheets/framework/modal.scss | 1 - app/controllers/projects/branches_controller.rb | 7 +++++++ app/views/projects/commit/_change.html.haml | 9 ++++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss index 26ad2870aa0..8374f30d0b2 100644 --- a/app/assets/stylesheets/framework/modal.scss +++ b/app/assets/stylesheets/framework/modal.scss @@ -1,6 +1,5 @@ .modal-body { position: relative; - overflow-y: auto; padding: 15px; .form-actions { diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 48fe81b0d74..462d4e461e1 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -15,6 +15,13 @@ class Projects::BranchesController < Projects::ApplicationController diverging_commit_counts = repository.diverging_commit_counts(branch) [memo, diverging_commit_counts[:behind], diverging_commit_counts[:ahead]].max end + + respond_to do |format| + format.html + format.json do + render json: @repository.branch_names.to_json + end + end end def recent diff --git a/app/views/projects/commit/_change.html.haml b/app/views/projects/commit/_change.html.haml index d9b800a4ded..f5407bdbc7b 100644 --- a/app/views/projects/commit/_change.html.haml +++ b/app/views/projects/commit/_change.html.haml @@ -17,7 +17,14 @@ .form-group.branch = label_tag 'target_branch', target_label, class: 'control-label' .col-sm-10 - = select_tag "target_branch", project_branches, class: "select2 select2-sm js-target-branch" + .dropdown + = dropdown_toggle @project.default_branch, { toggle: "dropdown", selected: @project.default_branch, ref: @ref, refs_url: branches_namespace_project_path(@project.namespace, @project) }, { toggle_class: "js-project-refs-dropdown js-target-branch" } + .dropdown-menu.dropdown-menu-selectable + = dropdown_title "Switch branch" + = dropdown_filter "Search branches" + = dropdown_content + = dropdown_loading + - if can?(current_user, :push_code, @project) .js-create-merge-request-container .checkbox From f4eda673c5d7f98919e74a4542e13ac1bcfdd76d Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 1 Aug 2016 16:07:08 -0600 Subject: [PATCH 043/144] Make the ref-selector function work properly for branch selection. --- app/assets/javascripts/project.js | 5 +++-- app/views/projects/commit/_change.html.haml | 9 ++------- app/views/shared/_ref_switcher.html.haml | 2 +- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index b97f6d22715..4e1de4dfb72 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -65,7 +65,8 @@ url: $dropdown.data('refs-url'), data: { ref: $dropdown.data('ref') - } + }, + dataType: "json" }).done(function(refs) { return callback(refs); }); @@ -73,7 +74,7 @@ selectable: true, filterable: true, filterByText: true, - fieldName: 'ref', + fieldName: $dropdown.data('field-name'), renderRow: function(ref) { var link; if (ref.header != null) { diff --git a/app/views/projects/commit/_change.html.haml b/app/views/projects/commit/_change.html.haml index f5407bdbc7b..6faf8267d4e 100644 --- a/app/views/projects/commit/_change.html.haml +++ b/app/views/projects/commit/_change.html.haml @@ -17,13 +17,8 @@ .form-group.branch = label_tag 'target_branch', target_label, class: 'control-label' .col-sm-10 - .dropdown - = dropdown_toggle @project.default_branch, { toggle: "dropdown", selected: @project.default_branch, ref: @ref, refs_url: branches_namespace_project_path(@project.namespace, @project) }, { toggle_class: "js-project-refs-dropdown js-target-branch" } - .dropdown-menu.dropdown-menu-selectable - = dropdown_title "Switch branch" - = dropdown_filter "Search branches" - = dropdown_content - = dropdown_loading + = hidden_field_tag :target_branch, @project.default_branch, id: 'target_branch' + = dropdown_tag(@project.default_branch, options: { title: "Switch branch", filter: true, placeholder: "Search branches", toggle_class: 'js-project-refs-dropdown js-target-branch', dropdown_class: 'dropdown-menu-selectable', data: { field_name: "target_branch", selected: @project.default_branch, target_branch: @project.default_branch, refs_url: namespace_project_branches_path(@project.namespace, @project), submit_form_on_click: false }}) - if can?(current_user, :push_code, @project) .js-create-merge-request-container diff --git a/app/views/shared/_ref_switcher.html.haml b/app/views/shared/_ref_switcher.html.haml index ea7162d4d63..9a8252ab087 100644 --- a/app/views/shared/_ref_switcher.html.haml +++ b/app/views/shared/_ref_switcher.html.haml @@ -6,7 +6,7 @@ - @options && @options.each do |key, value| = hidden_field_tag key, value, id: nil .dropdown - = dropdown_toggle dropdown_toggle_text, { toggle: "dropdown", selected: dropdown_toggle_text, ref: @ref, refs_url: refs_namespace_project_path(@project.namespace, @project) }, { toggle_class: "js-project-refs-dropdown" } + = dropdown_toggle dropdown_toggle_text, { toggle: "dropdown", selected: dropdown_toggle_text, ref: @ref, refs_url: refs_namespace_project_path(@project.namespace, @project), field_name: 'ref', submit_form_on_click: true }, { toggle_class: "js-project-refs-dropdown" } .dropdown-menu.dropdown-menu-selectable{ class: ("dropdown-menu-align-right" if local_assigns[:align_right]) } = dropdown_title "Switch branch/tag" = dropdown_filter "Search branches and tags" From 0bfef4bd83ee62076b4c22ca56d4c1d751ebfc98 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 2 Aug 2016 10:01:44 -0600 Subject: [PATCH 044/144] Add dynamic sizing for dropdown toggle, update Changelog. Also resolve feedback. --- CHANGELOG | 1 + app/assets/stylesheets/framework/dropdowns.scss | 9 +++++++++ app/views/projects/commit/_change.html.haml | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 837e9e27aba..29e3fa0c112 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -57,6 +57,7 @@ v 8.11.0 (unreleased) - Trigram indexes for the "ci_runners" table have been removed to speed up UPDATE queries - Fix devise deprecation warnings. - Update version_sorter and use new interface for faster tag sorting + - Load branches asynchronously in Cherry Pick and Revert dialogs. - Optimize checking if a user has read access to a list of issues !5370 - Store all DB secrets in secrets.yml, under descriptive names !5274 - Nokogiri's various parsing methods are now instrumented diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index f1635a53763..7d3a063d6c2 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -84,6 +84,15 @@ width: 100%; } } + + // Allows dynamic-width text in the dropdown toggle. + // Resizes to allow long text without overflowing the container. + &.dynamic { + width: auto; + min-width: 160px; + max-width: 100%; + padding-right: 25px; + } } .dropdown-menu, diff --git a/app/views/projects/commit/_change.html.haml b/app/views/projects/commit/_change.html.haml index 6faf8267d4e..e4cd55b9f7a 100644 --- a/app/views/projects/commit/_change.html.haml +++ b/app/views/projects/commit/_change.html.haml @@ -18,7 +18,7 @@ = label_tag 'target_branch', target_label, class: 'control-label' .col-sm-10 = hidden_field_tag :target_branch, @project.default_branch, id: 'target_branch' - = dropdown_tag(@project.default_branch, options: { title: "Switch branch", filter: true, placeholder: "Search branches", toggle_class: 'js-project-refs-dropdown js-target-branch', dropdown_class: 'dropdown-menu-selectable', data: { field_name: "target_branch", selected: @project.default_branch, target_branch: @project.default_branch, refs_url: namespace_project_branches_path(@project.namespace, @project), submit_form_on_click: false }}) + = dropdown_tag(@project.default_branch, options: { title: "Switch branch", filter: true, placeholder: "Search branches", toggle_class: 'js-project-refs-dropdown js-target-branch dynamic', dropdown_class: 'dropdown-menu-selectable', data: { field_name: "target_branch", selected: @project.default_branch, target_branch: @project.default_branch, refs_url: namespace_project_branches_path(@project.namespace, @project), submit_form_on_click: false }}) - if can?(current_user, :push_code, @project) .js-create-merge-request-container From 3babc959db12efd794d3620ae7ec051216a8ac5d Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Tue, 2 Aug 2016 10:53:10 -0600 Subject: [PATCH 045/144] Fix tests. --- app/controllers/projects/branches_controller.rb | 2 +- spec/features/projects/branches_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 462d4e461e1..2de8ada3e29 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -19,7 +19,7 @@ class Projects::BranchesController < Projects::ApplicationController respond_to do |format| format.html format.json do - render json: @repository.branch_names.to_json + render json: @repository.branch_names end end end diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb index 79abba21854..1b14945bf0a 100644 --- a/spec/features/projects/branches_spec.rb +++ b/spec/features/projects/branches_spec.rb @@ -20,7 +20,7 @@ describe 'Branches', feature: true do describe 'Find branches' do it 'shows filtered branches', js: true do - visit namespace_project_branches_path(project.namespace, project, project.id) + visit namespace_project_branches_path(project.namespace, project) fill_in 'branch-search', with: 'fix' find('#branch-search').native.send_keys(:enter) From 9a8b29e3ee3fc88f536f104a4ff962c0e435f3f4 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Fri, 5 Aug 2016 15:07:11 -0600 Subject: [PATCH 046/144] Add a test for the cherry pick dropdown. --- .../projects/commits/cherry_pick_spec.rb | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/spec/features/projects/commits/cherry_pick_spec.rb b/spec/features/projects/commits/cherry_pick_spec.rb index 1b4ff6b6f1b..e45e3a36d01 100644 --- a/spec/features/projects/commits/cherry_pick_spec.rb +++ b/spec/features/projects/commits/cherry_pick_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +include WaitForAjax describe 'Cherry-pick Commits' do let(:project) { create(:project) } @@ -8,12 +9,11 @@ describe 'Cherry-pick Commits' do before do login_as :user project.team << [@user, :master] - visit namespace_project_commits_path(project.namespace, project, project.repository.root_ref, { limit: 5 }) + visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) end context "I cherry-pick a commit" do it do - visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) find("a[href='#modal-cherry-pick-commit']").click expect(page).not_to have_content('v1.0.0') # Only branches, not tags page.within('#modal-cherry-pick-commit') do @@ -26,7 +26,6 @@ describe 'Cherry-pick Commits' do context "I cherry-pick a merge commit" do it do - visit namespace_project_commit_path(project.namespace, project, master_pickable_merge.id) find("a[href='#modal-cherry-pick-commit']").click page.within('#modal-cherry-pick-commit') do uncheck 'create_merge_request' @@ -38,7 +37,6 @@ describe 'Cherry-pick Commits' do context "I cherry-pick a commit that was previously cherry-picked" do it do - visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) find("a[href='#modal-cherry-pick-commit']").click page.within('#modal-cherry-pick-commit') do uncheck 'create_merge_request' @@ -56,7 +54,6 @@ describe 'Cherry-pick Commits' do context "I cherry-pick a commit in a new merge request" do it do - visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) find("a[href='#modal-cherry-pick-commit']").click page.within('#modal-cherry-pick-commit') do click_button 'Cherry-pick' @@ -64,4 +61,28 @@ describe 'Cherry-pick Commits' do expect(page).to have_content('The commit has been successfully cherry-picked. You can now submit a merge request to get this change into the original branch.') end end + + context "I cherry-pick a commit from a different branch", js: true do + it do + find('.commit-action-buttons a.dropdown-toggle').click + find(:css, "a[href='#modal-cherry-pick-commit']").click + + page.within('#modal-cherry-pick-commit') do + click_button 'master' + end + + wait_for_ajax + + page.within('#modal-cherry-pick-commit .dropdown-menu .dropdown-content') do + click_link 'feature' + end + + page.within('#modal-cherry-pick-commit') do + uncheck 'create_merge_request' + click_button 'Cherry-pick' + end + + expect(page).to have_content('The commit has been successfully cherry-picked.') + end + end end From 7fdd8613b72e74b91b9508ff79e6092e8b530dbe Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Wed, 17 Aug 2016 15:28:47 -0500 Subject: [PATCH 047/144] Fix badge count alignment --- CHANGELOG | 1 + app/assets/stylesheets/framework/nav.scss | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 61342926836..820d6e7a370 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -16,6 +16,7 @@ v 8.11.0 (unreleased) - API: List access requests, request access, approve, and deny access requests to a project or a group. !4833 - Use long options for curl examples in documentation !5703 (winniehell) - Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell) + - Fix badge count alignment (ClemMakesApps) - GitLab Performance Monitoring can now track custom events such as the number of tags pushed to a repository - Add support for relative links starting with ./ or / to RelativeLinkFilter (winniehell) - Ignore URLs starting with // in Markdown links !5677 (winniehell) diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index 7852fc9a424..9e924f99e9c 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -72,6 +72,7 @@ font-weight: normal; background-color: #eee; color: #78a; + vertical-align: baseline; } } From dd4784f3b16218cadfa3f73dce2250d4276ffb1f Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 18 Aug 2016 14:12:48 +0100 Subject: [PATCH 048/144] Add MemberExpirationDate JS helper This helper adds a datepicker to all `js-access-expiration-date` elements. If that element is a child of a `clearable-input` element and has a sibling `js-clear-input` element, then it will show a working clear button to the right of the input field. --- app/assets/javascripts/dispatcher.js | 1 + .../javascripts/member_expiration_date.js | 32 +++++++++++++++++++ app/assets/javascripts/project_members.js | 10 ------ .../_new_project_member.html.haml | 8 ++--- app/views/shared/members/_member.html.haml | 16 +++++++--- 5 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 app/assets/javascripts/member_expiration_date.js diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 9e6901962c6..846162a5a74 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -126,6 +126,7 @@ new UsersSelect(); break; case 'projects:project_members:index': + new MemberExpirationDate(); new ProjectMembers(); new UsersSelect(); break; diff --git a/app/assets/javascripts/member_expiration_date.js b/app/assets/javascripts/member_expiration_date.js new file mode 100644 index 00000000000..fdcf3b115b7 --- /dev/null +++ b/app/assets/javascripts/member_expiration_date.js @@ -0,0 +1,32 @@ +(function() { + // Add datepickers to all `js-access-expiration-date` elements. If those elements are + // children of an element with the `clearable-input` class, and have a sibling + // `js-clear-input` element, then show that element when there is a value in the + // datepicker, and make clicking on that element clear the field. + // + this.MemberExpirationDate = function() { + $('.js-access-expiration-date').each(function(i, element) { + var expirationDateInput = $(element); + + function toggleClearInput() { + expirationDateInput.parent().toggleClass('has-value', !!expirationDateInput.val()); + } + + expirationDateInput.datepicker({ + dateFormat: 'yy-mm-dd', + minDate: 1, + onSelect: toggleClearInput + }); + + expirationDateInput.on('blur', toggleClearInput); + + toggleClearInput(); + + expirationDateInput.next('.js-clear-input').on('click', function(event) { + event.preventDefault(); + expirationDateInput.datepicker('setDate', null); + toggleClearInput(); + }); + }); + }; +}).call(this); diff --git a/app/assets/javascripts/project_members.js b/app/assets/javascripts/project_members.js index 8171fa8d533..78f7b48bc7d 100644 --- a/app/assets/javascripts/project_members.js +++ b/app/assets/javascripts/project_members.js @@ -4,17 +4,7 @@ $('li.project_member').bind('ajax:success', function() { return $(this).fadeOut(); }); - - $('.js-project-members-page').on('focus', '.js-access-expiration-date', function() { - $(this).datepicker({ - dateFormat: 'yy-mm-dd', - minDate: 1 - }); - }); } - return ProjectMembers; - })(); - }).call(this); diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml index 0545ed07d62..96d05c7fd65 100644 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ b/app/views/projects/project_members/_new_project_member.html.haml @@ -15,11 +15,11 @@ %strong= link_to "here", help_page_path("user/permissions"), class: "vlink" .form-group - = label_tag :expires_at, 'Access expiration date', class: 'control-label' + = f.label :expires_at, 'Access expiration date', class: 'control-label' .col-sm-10 - = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date' - .help-block - Leave it empty if you do not want this user's access to expire. + .clearable-input + = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date' + %i.clear-icon.js-clear-input .form-actions = f.submit 'Add users to project', class: "btn btn-create" diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 7dde9442e24..c959c41b221 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -77,11 +77,17 @@ - if show_roles .edit-member.hide.js-toggle-content %br - = form_for member, remote: true do |f| - .prepend-top-10 - = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control' + = form_for member, remote: true, html: { class: 'form-horizontal' } do |f| + .form-group + = label_tag :expires_at, 'Project access', class: 'control-label' + .col-sm-10 + = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control' - if member.is_a?(ProjectMember) - .prepend-top-10 - = f.text_field :expires_at, class: 'form-control js-access-expiration-date', placeholder: 'Access expiration date', id: "member_expires_at_#{member.id}" + .form-group + = label_tag :expires_at, 'Access expiration date', class: 'control-label' + .col-sm-10 + .clearable-input + = f.text_field :expires_at, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date', id: "member_expires_at_#{member.id}" + %i.clear-icon.js-clear-input .prepend-top-10 = f.submit 'Save', class: 'btn btn-save btn-sm' From ff903e645335901355ab837accc995408f79e96a Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Fri, 5 Aug 2016 15:29:20 +0200 Subject: [PATCH 049/144] Move to project dropdown with infinite scroll for better performance Use just SQL to check is a user can admin_issue on a project Using offset pagination instead pages to avoid a count query Tradeoff - we duplicate how we check admin_issue in a SQL relation in the Ability class --- CHANGELOG | 1 + app/assets/javascripts/issuable_form.js | 24 +++++++-- app/assets/stylesheets/framework/selects.scss | 3 +- app/finders/move_to_project_finder.rb | 6 +++ app/views/shared/issuable/_form.html.haml | 2 +- .../autocomplete_controller_spec.rb | 50 +++++++++++++++++++ spec/finders/move_to_project_finder_spec.rb | 22 ++++++++ 7 files changed, 101 insertions(+), 7 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 61c654002ad..a3186a8afb9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -105,6 +105,7 @@ v 8.11.0 (unreleased) - Fix merge request new view not changing code view rendering style - Make error pages responsive (Takuya Noguchi) - The performance of the project dropdown used for moving issues has been improved + - Move to project dropdown with infinite scroll for better performance - Fix skip_repo parameter being ignored when destroying a namespace - Add all builds into stage/job dropdowns on builds page - Change requests_profiles resource constraint to catch virtually any file diff --git a/app/assets/javascripts/issuable_form.js b/app/assets/javascripts/issuable_form.js index 297d4f029f0..b7f92ae9883 100644 --- a/app/assets/javascripts/issuable_form.js +++ b/app/assets/javascripts/issuable_form.js @@ -102,20 +102,34 @@ }; IssuableForm.prototype.initMoveDropdown = function() { - var $moveDropdown; + var $moveDropdown, pageSize; $moveDropdown = $('.js-move-dropdown'); if ($moveDropdown.length) { + pageSize = $moveDropdown.data('page-size'); return $('.js-move-dropdown').select2({ ajax: { url: $moveDropdown.data('projects-url'), - results: function(data) { + quietMillis: 125, + data: function(term, page, context) { return { - results: data + search: term, + offset_id: context }; }, - data: function(query) { + results: function(data) { + var context, + more; + + if (data.length >= pageSize) + more = true; + + if (data[data.length - 1]) + context = data[data.length - 1].id; + return { - search: query + results: data, + more: more, + context: context }; } }, diff --git a/app/assets/stylesheets/framework/selects.scss b/app/assets/stylesheets/framework/selects.scss index 21d87cc9d34..b2e22b60440 100644 --- a/app/assets/stylesheets/framework/selects.scss +++ b/app/assets/stylesheets/framework/selects.scss @@ -45,7 +45,8 @@ min-width: 175px; } -.select2-results .select2-result-label { +.select2-results .select2-result-label, +.select2-more-results { padding: 10px 15px; } diff --git a/app/finders/move_to_project_finder.rb b/app/finders/move_to_project_finder.rb index 3334b8556df..79eb45568be 100644 --- a/app/finders/move_to_project_finder.rb +++ b/app/finders/move_to_project_finder.rb @@ -1,4 +1,6 @@ class MoveToProjectFinder + PAGE_SIZE = 50 + def initialize(user) @user = user end @@ -8,6 +10,10 @@ class MoveToProjectFinder projects = projects.search(search) if search.present? projects = projects.excluding_project(from_project) + # infinite scroll using offset + projects = projects.where('projects.id < ?', offset_id) if offset_id.present? + projects = projects.limit(PAGE_SIZE) + # to ask for Project#name_with_namespace projects.includes(namespace: :owner) end diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 9e2e096d5f9..901351f2620 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -120,7 +120,7 @@ = label_tag :move_to_project_id, 'Move', class: 'control-label' .col-sm-10 .issuable-form-select-holder - = hidden_field_tag :move_to_project_id, nil, class: 'js-move-dropdown', data: { placeholder: 'Select project', projects_url: autocomplete_projects_path(project_id: @project.id) } + = hidden_field_tag :move_to_project_id, nil, class: 'js-move-dropdown', data: { placeholder: 'Select project', projects_url: autocomplete_projects_path(project_id: @project.id), page_size: MoveToProjectFinder::PAGE_SIZE }   %span{ data: { toggle: 'tooltip', placement: 'auto top' }, style: 'cursor: default', title: 'Moving an issue will copy the discussion to a different project and close it here. All participants will be notified of the new location.' } diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 44128a43362..a121cb2fc97 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -237,6 +237,56 @@ describe AutocompleteController do end end + context 'authorized projects apply limit' do + before do + authorized_project2 = create(:project) + authorized_project3 = create(:project) + + authorized_project.team << [user, :master] + authorized_project2.team << [user, :master] + authorized_project3.team << [user, :master] + + stub_const 'MoveToProjectFinder::PAGE_SIZE', 2 + end + + describe 'GET #projects with project ID' do + before do + get(:projects, project_id: project.id) + end + + let(:body) { JSON.parse(response.body) } + + it do + expect(body).to be_kind_of(Array) + expect(body.size).to eq 3 # Of a total of 4 + end + end + end + + context 'authorized projects with offset' do + before do + authorized_project2 = create(:project) + authorized_project3 = create(:project) + + authorized_project.team << [user, :master] + authorized_project2.team << [user, :master] + authorized_project3.team << [user, :master] + end + + describe 'GET #projects with project ID and offset_id' do + before do + get(:projects, project_id: project.id, offset_id: authorized_project.id) + end + + let(:body) { JSON.parse(response.body) } + + it do + expect(body.detect { |item| item['id'] == 0 }).to be_nil # 'No project' is not there + expect(body.detect { |item| item['id'] == authorized_project.id }).to be_nil # Offset project is not there either + end + end + end + context 'authorized projects without admin_issue ability' do before(:each) do authorized_project.team << [user, :guest] diff --git a/spec/finders/move_to_project_finder_spec.rb b/spec/finders/move_to_project_finder_spec.rb index 4f3304f7b6d..fdce4e714ff 100644 --- a/spec/finders/move_to_project_finder_spec.rb +++ b/spec/finders/move_to_project_finder_spec.rb @@ -51,6 +51,28 @@ describe MoveToProjectFinder do expect(subject.execute(project).to_a).to eq([other_reporter_project]) end + + it 'returns a page of projects ordered by id in descending order' do + stub_const 'MoveToProjectFinder::PAGE_SIZE', 2 + + reporter_project.team << [user, :reporter] + developer_project.team << [user, :developer] + master_project.team << [user, :master] + + expect(subject.execute(project).to_a).to eq([master_project, developer_project]) + end + + it 'returns projects after the given offset id' do + stub_const 'MoveToProjectFinder::PAGE_SIZE', 2 + + reporter_project.team << [user, :reporter] + developer_project.team << [user, :developer] + master_project.team << [user, :master] + + expect(subject.execute(project, search: nil, offset_id: master_project.id).to_a).to eq([developer_project, reporter_project]) + expect(subject.execute(project, search: nil, offset_id: developer_project.id).to_a).to eq([reporter_project]) + expect(subject.execute(project, search: nil, offset_id: reporter_project.id).to_a).to be_empty + end end context 'search' do From d649370ac19ce621a098b862008382e5ab2cb7fc Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 18 Aug 2016 15:01:21 +0100 Subject: [PATCH 050/144] Only use let! where needed --- .../members/master_adds_member_with_expiration_date_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb index 47a72a82cf3..5dd38de34d2 100644 --- a/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb +++ b/spec/features/projects/members/master_adds_member_with_expiration_date_spec.rb @@ -4,8 +4,8 @@ feature 'Projects > Members > Master adds member with expiration date', feature: include Select2Helper include ActiveSupport::Testing::TimeHelpers - let!(:master) { create(:user) } - let!(:project) { create(:project) } + let(:master) { create(:user) } + let(:project) { create(:project) } let!(:new_member) { create(:user) } background do From 21a73302e8a8b9f22e51f1707a306f04d3faad07 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 18 Aug 2016 14:12:48 +0100 Subject: [PATCH 051/144] Add MemberExpirationDate JS helper This helper adds a datepicker to all `js-access-expiration-date` elements. If that element is a child of a `clearable-input` element and has a sibling `js-clear-input` element, then it will show a working clear button to the right of the input field. --- .../javascripts/member_expiration_date.js | 2 ++ app/assets/stylesheets/pages/projects.scss | 26 +++++++++++++++++++ .../projects/project_members/update.js.haml | 1 + app/views/shared/members/_member.html.haml | 6 ++--- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/member_expiration_date.js b/app/assets/javascripts/member_expiration_date.js index fdcf3b115b7..93c34d5ccd7 100644 --- a/app/assets/javascripts/member_expiration_date.js +++ b/app/assets/javascripts/member_expiration_date.js @@ -8,6 +8,8 @@ $('.js-access-expiration-date').each(function(i, element) { var expirationDateInput = $(element); + if (expirationDateInput.hasClass('hasDatepicker')) { return; } + function toggleClearInput() { expirationDateInput.parent().toggleClass('has-value', !!expirationDateInput.val()); } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 4409477916f..abda079ad4c 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -714,3 +714,29 @@ pre.light-well { width: 300px; } } + +.clearable-input { + position: relative; + + .clear-icon { + @extend .fa-times; + display: none; + position: absolute; + right: 7px; + top: 7px; + color: $location-icon-color; + + &:before { + font-family: FontAwesome; + font-weight: normal; + font-style: normal; + } + } + + &.has-value { + .clear-icon { + cursor: pointer; + display: block; + } + } +} diff --git a/app/views/projects/project_members/update.js.haml b/app/views/projects/project_members/update.js.haml index 45f8ef89060..833954bc039 100644 --- a/app/views/projects/project_members/update.js.haml +++ b/app/views/projects/project_members/update.js.haml @@ -1,2 +1,3 @@ :plain $("##{dom_id(@project_member)}").replaceWith('#{escape_javascript(render('shared/members/member', member: @project_member))}'); + new MemberExpirationDate(); diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index c959c41b221..1c081bcdc17 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -79,12 +79,12 @@ %br = form_for member, remote: true, html: { class: 'form-horizontal' } do |f| .form-group - = label_tag :expires_at, 'Project access', class: 'control-label' + = label_tag "member_access_level_#{member.id}", 'Project access', class: 'control-label' .col-sm-10 - = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control' + = f.select :access_level, options_for_select(member.class.access_level_roles, member.access_level), {}, class: 'form-control', id: "member_access_level_#{member.id}" - if member.is_a?(ProjectMember) .form-group - = label_tag :expires_at, 'Access expiration date', class: 'control-label' + = label_tag "member_expires_at_#{member.id}", 'Access expiration date', class: 'control-label' .col-sm-10 .clearable-input = f.text_field :expires_at, class: 'form-control js-access-expiration-date', placeholder: 'Select access expiration date', id: "member_expires_at_#{member.id}" From 42496ecef778ad6d3500babbf33d350b6ab984d7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 11:37:07 +0200 Subject: [PATCH 052/144] Render coverage badge for latest successful pipeline --- lib/gitlab/badge/coverage/report.rb | 3 +- spec/lib/gitlab/badge/coverage/report_spec.rb | 77 +++++++++++-------- 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/lib/gitlab/badge/coverage/report.rb b/lib/gitlab/badge/coverage/report.rb index 3d56ea3e47a..95d925dc7f3 100644 --- a/lib/gitlab/badge/coverage/report.rb +++ b/lib/gitlab/badge/coverage/report.rb @@ -13,8 +13,7 @@ module Gitlab @job = job @pipeline = @project.pipelines - .where(ref: @ref) - .where(sha: @project.commit(@ref).try(:sha)) + .latest_successful_for(@ref) .first end diff --git a/spec/lib/gitlab/badge/coverage/report_spec.rb b/spec/lib/gitlab/badge/coverage/report_spec.rb index 1ff49602486..ab0cce6e091 100644 --- a/spec/lib/gitlab/badge/coverage/report_spec.rb +++ b/spec/lib/gitlab/badge/coverage/report_spec.rb @@ -44,45 +44,49 @@ describe Gitlab::Badge::Coverage::Report do end end - context 'pipeline exists' do - let!(:pipeline) do - create(:ci_pipeline, project: project, - sha: project.commit.id, - ref: 'master') - end - - context 'builds exist' do - before do - create(:ci_build, name: 'first', pipeline: pipeline, coverage: 40) - create(:ci_build, pipeline: pipeline, coverage: 60) + context 'when latest successful pipeline exists' do + before do + create_pipeline do |pipeline| + create(:ci_build, :success, pipeline: pipeline, name: 'first', coverage: 40) + create(:ci_build, :success, pipeline: pipeline, coverage: 60) end - context 'particular job specified' do - let(:job_name) { 'first' } - - it 'returns coverage for the particular job' do - expect(badge.status).to eq 40 - end - end - - context 'particular job not specified' do - let(:job_name) { '' } - - it 'returns arithemetic mean for the pipeline' do - expect(badge.status).to eq 50 - end + create_pipeline do |pipeline| + create(:ci_build, :failed, pipeline: pipeline, coverage: 10) end end - context 'builds do not exist' do - it_behaves_like 'unknown coverage report' + context 'when particular job specified' do + let(:job_name) { 'first' } - context 'particular job specified' do - let(:job_name) { 'nonexistent' } + it 'returns coverage for the particular job' do + expect(badge.status).to eq 40 + end + end - it 'retruns nil' do - expect(badge.status).to be_nil - end + context 'when particular job not specified' do + let(:job_name) { '' } + + it 'returns arithemetic mean for the pipeline' do + expect(badge.status).to eq 50 + end + end + end + + context 'when only failed pipeline exists' do + before do + create_pipeline do |pipeline| + create(:ci_build, :failed, pipeline: pipeline, coverage: 10) + end + end + + it_behaves_like 'unknown coverage report' + + context 'particular job specified' do + let(:job_name) { 'nonexistent' } + + it 'retruns nil' do + expect(badge.status).to be_nil end end end @@ -90,4 +94,13 @@ describe Gitlab::Badge::Coverage::Report do context 'pipeline does not exist' do it_behaves_like 'unknown coverage report' end + + def create_pipeline + opts = { project: project, sha: project.commit.id, ref: 'master' } + + create(:ci_pipeline, opts).tap do |pipeline| + yield pipeline + pipeline.build_updated + end + end end From cfbdf7420cd1ad5733d7e0da12eb28ffcbaf0b0d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 11:47:46 +0200 Subject: [PATCH 053/144] Update documentation for test coverage report badge --- doc/ci/pipelines.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index d90d7aca4fd..20cd88c8d20 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -67,6 +67,8 @@ use following Markdown code to embed the est coverage report into `README.md`: ![coverage](http://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg?job=coverage) ``` +The latest successful pipeline will be used to read the test coverage value. + [builds]: #builds [jobs]: yaml/README.md#jobs [stages]: yaml/README.md#stages From 804474d78c1cc0440f29424b0b6bbe2f5f77eab7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 11:49:26 +0200 Subject: [PATCH 054/144] Add Changelog entry for coverage badge improvements --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 61c654002ad..18d9a301a25 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.11.0 (unreleased) + - Use test coverage value from the latest successful pipeline in badge. !5862 - Add test coverage report badge. !5708 - Remove the http_parser.rb dependency by removing the tinder gem. !5758 (tbalthazar) - Ability to specify branches for Pivotal Tracker integration (Egor Lynko) From 8527f3fa4641cee3c56e8eef7c07efc06aa7bdc5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 12:38:14 +0200 Subject: [PATCH 055/144] Update coverage report badge feature tests --- .../features/projects/badges/coverage_spec.rb | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/spec/features/projects/badges/coverage_spec.rb b/spec/features/projects/badges/coverage_spec.rb index af86d3c338a..5972e7f31c2 100644 --- a/spec/features/projects/badges/coverage_spec.rb +++ b/spec/features/projects/badges/coverage_spec.rb @@ -4,12 +4,6 @@ feature 'test coverage badge' do given!(:user) { create(:user) } given!(:project) { create(:project, :private) } - given!(:pipeline) do - create(:ci_pipeline, project: project, - ref: 'master', - sha: project.commit.id) - end - context 'when user has access to view badge' do background do project.team << [user, :developer] @@ -17,8 +11,10 @@ feature 'test coverage badge' do end scenario 'user requests coverage badge image for pipeline' do - create_job(coverage: 100, name: 'test:1') - create_job(coverage: 90, name: 'test:2') + create_pipeline do |pipeline| + create_build(pipeline, coverage: 100, name: 'test:1') + create_build(pipeline, coverage: 90, name: 'test:2') + end show_test_coverage_badge @@ -26,9 +22,11 @@ feature 'test coverage badge' do end scenario 'user requests coverage badge for specific job' do - create_job(coverage: 50, name: 'test:1') - create_job(coverage: 50, name: 'test:2') - create_job(coverage: 85, name: 'coverage') + create_pipeline do |pipeline| + create_build(pipeline, coverage: 50, name: 'test:1') + create_build(pipeline, coverage: 50, name: 'test:2') + create_build(pipeline, coverage: 85, name: 'coverage') + end show_test_coverage_badge(job: 'coverage') @@ -36,7 +34,9 @@ feature 'test coverage badge' do end scenario 'user requests coverage badge for pipeline without coverage' do - create_job(coverage: nil, name: 'test') + create_pipeline do |pipeline| + create_build(pipeline, coverage: nil, name: 'test') + end show_test_coverage_badge @@ -54,10 +54,19 @@ feature 'test coverage badge' do end end - def create_job(coverage:, name:) - create(:ci_build, name: name, - coverage: coverage, - pipeline: pipeline) + def create_pipeline + opts = { project: project, ref: 'master', sha: project.commit.id } + + create(:ci_pipeline, opts).tap do |pipeline| + yield pipeline + pipeline.build_updated + end + end + + def create_build(pipeline, coverage:, name:) + opts = { pipeline: pipeline, coverage: coverage, name: name } + + create(:ci_build, :success, opts) end def show_test_coverage_badge(job: nil) From d3af645e6aee2430e0882a25dc7ad11c80f23aea Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 17 Aug 2016 13:30:36 +0200 Subject: [PATCH 056/144] Refactor pipeline fixtures for dev env a little --- db/fixtures/development/14_builds.rb | 51 +++++++++++++++------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/db/fixtures/development/14_builds.rb b/db/fixtures/development/14_builds.rb index 069d9dd6226..3b02dddc1fd 100644 --- a/db/fixtures/development/14_builds.rb +++ b/db/fixtures/development/14_builds.rb @@ -34,33 +34,38 @@ class Gitlab::Seeder::Builds end end + private + def pipelines - master_pipelines + merge_request_pipelines + create_master_pipelines + create_merge_request_pipelines end - def master_pipelines - create_pipelines_for(@project, 'master') - rescue - [] - end - - def merge_request_pipelines - @project.merge_requests.last(5).map do |merge_request| - create_pipelines(merge_request.source_project, merge_request.source_branch, merge_request.commits.last(5)) - end.flatten - rescue - [] - end - - def create_pipelines_for(project, ref) - commits = project.repository.commits(ref, limit: 5) - create_pipelines(project, ref, commits) - end - - def create_pipelines(project, ref, commits) - commits.map do |commit| - project.pipelines.create(sha: commit.id, ref: ref) + def create_master_pipelines + @project.repository.commits('master', limit: 5).map do |commit| + create_pipeline!(@project, 'master', commit) end + rescue + [] + end + + def create_merge_request_pipelines + pipelines = @project.merge_requests.first(5).map do |merge_request| + project = merge_request.source_project + branch = merge_request.source_branch + + merge_request.commits.last(5).map do |commit| + create_pipeline!(project, branch, commit) + end + end + + pipelines.flatten + rescue + [] + end + + + def create_pipeline!(project, ref, commit) + project.pipelines.create(sha: commit.id, ref: ref) end def build_create!(pipeline, opts = {}) From 312c1dce925e97319063e9e9b052607a8a26c5ed Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 09:00:08 +0200 Subject: [PATCH 057/144] Fix build logs in development environment fixtures --- db/fixtures/development/14_builds.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/db/fixtures/development/14_builds.rb b/db/fixtures/development/14_builds.rb index 3b02dddc1fd..903b75c924b 100644 --- a/db/fixtures/development/14_builds.rb +++ b/db/fixtures/development/14_builds.rb @@ -41,7 +41,7 @@ class Gitlab::Seeder::Builds end def create_master_pipelines - @project.repository.commits('master', limit: 5).map do |commit| + @project.repository.commits('master', limit: 4).map do |commit| create_pipeline!(@project, 'master', commit) end rescue @@ -49,11 +49,11 @@ class Gitlab::Seeder::Builds end def create_merge_request_pipelines - pipelines = @project.merge_requests.first(5).map do |merge_request| + pipelines = @project.merge_requests.first(3).map do |merge_request| project = merge_request.source_project branch = merge_request.source_branch - merge_request.commits.last(5).map do |commit| + merge_request.commits.last(4).map do |commit| create_pipeline!(project, branch, commit) end end @@ -71,7 +71,7 @@ class Gitlab::Seeder::Builds def build_create!(pipeline, opts = {}) attributes = build_attributes_for(pipeline, opts) - Ci::Build.create!(attributes) do |build| + Ci::Build.create!(attributes).tap do |build| if opts[:name].start_with?('build') artifacts_cache_file(artifacts_archive_path) do |file| build.artifacts_file = file @@ -82,8 +82,12 @@ class Gitlab::Seeder::Builds end end + ## + # We need to set build trace after saving a build (id required) + # That is why we need `#tap` method instead of passing block + # directly to `Ci::Build#create!`. + # if %w(running success failed).include?(build.status) - # We need to set build trace after saving a build (id required) build.trace = FFaker::Lorem.paragraphs(6).join("\n\n") end end From dda60230623844b2615742de8d96b49f90ac8a87 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 09:39:44 +0200 Subject: [PATCH 058/144] Fix build artifacts in fixtures in development env --- db/fixtures/development/14_builds.rb | 52 ++++++++++++++++------------ 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/db/fixtures/development/14_builds.rb b/db/fixtures/development/14_builds.rb index 903b75c924b..55af91c1cb8 100644 --- a/db/fixtures/development/14_builds.rb +++ b/db/fixtures/development/14_builds.rb @@ -69,46 +69,52 @@ class Gitlab::Seeder::Builds end def build_create!(pipeline, opts = {}) - attributes = build_attributes_for(pipeline, opts) + attributes = job_attributes(pipeline, opts) + .merge(commands: '$ build command') Ci::Build.create!(attributes).tap do |build| - if opts[:name].start_with?('build') - artifacts_cache_file(artifacts_archive_path) do |file| - build.artifacts_file = file - end + setup_artifacts(build) + setup_build_log(build) + build.save + end + end - artifacts_cache_file(artifacts_metadata_path) do |file| - build.artifacts_metadata = file - end - end + def setup_artifacts(build) + return unless %w[build test].include?(build.stage) - ## - # We need to set build trace after saving a build (id required) - # That is why we need `#tap` method instead of passing block - # directly to `Ci::Build#create!`. - # - if %w(running success failed).include?(build.status) - build.trace = FFaker::Lorem.paragraphs(6).join("\n\n") - end + artifacts_cache_file(artifacts_archive_path) do |file| + build.artifacts_file = file + end + + artifacts_cache_file(artifacts_metadata_path) do |file| + build.artifacts_metadata = file + end + end + + def setup_build_log(build) + ## + # We need to set build trace after saving a build (id required) + # That is why we need `#tap` method instead of passing block + # directly to `Ci::Build#create!`. + # + if %w(running success failed).include?(build.status) + build.trace = FFaker::Lorem.paragraphs(6).join("\n\n") end end def commit_status_create!(pipeline, opts = {}) - attributes = commit_status_attributes_for(pipeline, opts) + attributes = job_attributes(pipeline, opts) + GenericCommitStatus.create!(attributes) end - def commit_status_attributes_for(pipeline, opts) + def job_attributes(pipeline, opts) { name: 'test build', stage: 'test', stage_idx: stage_index(opts[:stage]), ref: 'master', tag: false, user: build_user, project: @project, pipeline: pipeline, created_at: Time.now, updated_at: Time.now }.merge(opts) end - def build_attributes_for(pipeline, opts) - commit_status_attributes_for(pipeline, opts).merge(commands: '$ build command') - end - def build_user @project.team.users.sample end From 34960c299c256bc8470333d63ed1a4946064d6b6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 10:15:21 +0200 Subject: [PATCH 059/144] Reorder failed stages and improve build statuses --- db/fixtures/development/14_builds.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/db/fixtures/development/14_builds.rb b/db/fixtures/development/14_builds.rb index 55af91c1cb8..a6117c12930 100644 --- a/db/fixtures/development/14_builds.rb +++ b/db/fixtures/development/14_builds.rb @@ -7,11 +7,12 @@ class Gitlab::Seeder::Builds { name: 'rspec:windows', stage: 'test', status: :success }, { name: 'rspec:windows', stage: 'test', status: :success }, { name: 'rspec:osx', stage: 'test', status_event: :success }, - { name: 'spinach:linux', stage: 'test', status: :pending }, - { name: 'spinach:osx', stage: 'test', status: :canceled }, - { name: 'cucumber:linux', stage: 'test', status: :running }, - { name: 'cucumber:osx', stage: 'test', status: :failed }, - { name: 'staging', stage: 'deploy', environment: 'staging', status: :success }, + { name: 'spinach:linux', stage: 'test', status: :success }, + { name: 'spinach:osx', stage: 'test', status: :failed, allow_failure: true}, + { name: 'env:alpha', stage: 'deploy', environment: 'alpha', status: :pending }, + { name: 'env:beta', stage: 'deploy', environment: 'beta', status: :running }, + { name: 'env:gamma', stage: 'deploy', environment: 'gamma', status: :canceled }, + { name: 'staging', stage: 'deploy', environment: 'staging', status_event: :success }, { name: 'production', stage: 'deploy', environment: 'production', when: 'manual', status: :skipped }, { name: 'slack', stage: 'notify', when: 'manual', status: :created }, ] From b16f5319146ef2a47ff785ca87f7b2257dd3636b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 10:15:56 +0200 Subject: [PATCH 060/144] Reduce the number of build fixtures created in dev --- db/fixtures/development/14_builds.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/fixtures/development/14_builds.rb b/db/fixtures/development/14_builds.rb index a6117c12930..9a3d54fb56c 100644 --- a/db/fixtures/development/14_builds.rb +++ b/db/fixtures/development/14_builds.rb @@ -147,7 +147,7 @@ class Gitlab::Seeder::Builds end Gitlab::Seeder.quiet do - Project.all.sample(10).each do |project| + Project.all.sample(5).each do |project| project_builds = Gitlab::Seeder::Builds.new(project) project_builds.seed! end From c3ce005e66832587efd228e355daab0d3d08e7dd Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 10:17:28 +0200 Subject: [PATCH 061/144] Rename file that holds pipeline fixtures in dev env --- db/fixtures/development/{14_builds.rb => 14_pipelines.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename db/fixtures/development/{14_builds.rb => 14_pipelines.rb} (100%) diff --git a/db/fixtures/development/14_builds.rb b/db/fixtures/development/14_pipelines.rb similarity index 100% rename from db/fixtures/development/14_builds.rb rename to db/fixtures/development/14_pipelines.rb From 7450fe78000ac32ede0e363373585a846b4f0a0d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 18 Aug 2016 10:28:12 +0200 Subject: [PATCH 062/144] Change name of the class that with pipeline seeds --- db/fixtures/development/14_pipelines.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb index 9a3d54fb56c..36b0d16bfb9 100644 --- a/db/fixtures/development/14_pipelines.rb +++ b/db/fixtures/development/14_pipelines.rb @@ -1,4 +1,4 @@ -class Gitlab::Seeder::Builds +class Gitlab::Seeder::Pipelines STAGES = %w[build test deploy notify] BUILDS = [ { name: 'build:linux', stage: 'build', status: :success }, @@ -148,7 +148,7 @@ end Gitlab::Seeder.quiet do Project.all.sample(5).each do |project| - project_builds = Gitlab::Seeder::Builds.new(project) + project_builds = Gitlab::Seeder::Pipelines.new(project) project_builds.seed! end end From b4398de5c5381a81f225c390e32f99d4e0e7d627 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Sun, 15 May 2016 00:50:15 +0100 Subject: [PATCH 063/144] Added new non-selectable selector exclusions to fix arrow key events, fixed the simulated clicking of a row and fixed the conflict between enter key form submit and enter key row selection Added bootstrap dropdown event triggers to invoke the open and close methods of the dropdown, allowing for the binding of array key events Added #17465 fix entry to CHANGELOG Fixed multi-dropdown selected row index conflict Fixed whitespace diff Added padding to the dropdown content iterative scroll as well as new conditional scrolls to scroll all the way to the top when the first item of a list is selected and to scroll all the way to the bottom when the last item of a list is selected Added conditionals to the enable and disable autocomplete methods to stop multiple invocations without any enabled/disabled state change Fixes some incorrect firing of requests. The dropdown box was invoking a new query every time it closed and the GitLabDropdownRemote callback was invoking a new query which was causing the dropdown double render issue. Added .selectable css class to dropdown list items that are not dividers or headers and altered selectors to account for that. Moved scroll padding Number to variable. Removed unused method Started Dropdown tests Added fixture and began first test Almost finished, navigation done, action and close needed YAY. TESTS DONE. Altered test and fixed click started removing selectable class use Fixed as reviewed altered selection method Fixed autocomplete shutting dropdown on arrow key use patched XSS vulns updated tests f Added click fixes --- app/assets/javascripts/gl_dropdown.js.coffee | 657 ++++++++++++++++++ .../javascripts/search_autocomplete.js.coffee | 349 ++++++++++ .../fixtures/gl_dropdown.html.haml | 16 + spec/javascripts/gl_dropdown_spec.js.coffee | 96 +++ 4 files changed, 1118 insertions(+) create mode 100644 app/assets/javascripts/gl_dropdown.js.coffee create mode 100644 app/assets/javascripts/search_autocomplete.js.coffee create mode 100644 spec/javascripts/fixtures/gl_dropdown.html.haml create mode 100644 spec/javascripts/gl_dropdown_spec.js.coffee diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee new file mode 100644 index 00000000000..e096effaade --- /dev/null +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -0,0 +1,657 @@ +class GitLabDropdownFilter + BLUR_KEYCODES = [27, 40] + ARROW_KEY_CODES = [38, 40] + HAS_VALUE_CLASS = "has-value" + + constructor: (@input, @options) -> + { + @filterInputBlur = true + } = @options + + $inputContainer = @input.parent() + $clearButton = $inputContainer.find('.js-dropdown-input-clear') + + @indeterminateIds = [] + + # Clear click + $clearButton.on 'click', (e) => + e.preventDefault() + e.stopPropagation() + @input + .val('') + .trigger('keyup') + .focus() + + # Key events + timeout = "" + @input.on "keyup", (e) => + keyCode = e.which + + return if ARROW_KEY_CODES.indexOf(keyCode) >= 0 + + if @input.val() isnt "" and !$inputContainer.hasClass HAS_VALUE_CLASS + $inputContainer.addClass HAS_VALUE_CLASS + else if @input.val() is "" and $inputContainer.hasClass HAS_VALUE_CLASS + $inputContainer.removeClass HAS_VALUE_CLASS + + if keyCode is 13 + return false + + # Only filter asynchronously only if option remote is set + if @options.remote + clearTimeout timeout + timeout = setTimeout => + blur_field = @shouldBlur keyCode + + if blur_field and @filterInputBlur + @input.blur() + + @options.query @input.val(), (data) => + @options.callback(data) + , 250 + else + @filter @input.val() + + shouldBlur: (keyCode) -> + return BLUR_KEYCODES.indexOf(keyCode) >= 0 + + filter: (search_text) -> + @options.onFilter(search_text) if @options.onFilter + data = @options.data() + + if data? and not @options.filterByText + results = data + + if search_text isnt '' + # When data is an array of objects therefore [object Array] e.g. + # [ + # { prop: 'foo' }, + # { prop: 'baz' } + # ] + if _.isArray(data) + results = fuzzaldrinPlus.filter(data, search_text, + key: @options.keys + ) + else + # If data is grouped therefore an [object Object]. e.g. + # { + # groupName1: [ + # { prop: 'foo' }, + # { prop: 'baz' } + # ], + # groupName2: [ + # { prop: 'abc' }, + # { prop: 'def' } + # ] + # } + if gl.utils.isObject data + results = {} + for key, group of data + tmp = fuzzaldrinPlus.filter(group, search_text, + key: @options.keys + ) + + if tmp.length + results[key] = tmp.map (item) -> item + + @options.callback results + else + elements = @options.elements() + + if search_text + elements.each -> + $el = $(@) + matches = fuzzaldrinPlus.match($el.text().trim(), search_text) + + unless $el.is('.dropdown-header') + if matches.length + $el.show() + else + $el.hide() + else + elements.show() + +class GitLabDropdownRemote + constructor: (@dataEndpoint, @options) -> + + execute: -> + if typeof @dataEndpoint is "string" + @fetchData() + else if typeof @dataEndpoint is "function" + if @options.beforeSend + @options.beforeSend() + + # Fetch the data by calling the data funcfion + @dataEndpoint "", (data) => + if @options.success + @options.success(data) + + if @options.beforeSend + @options.beforeSend() + + # Fetch the data through ajax if the data is a string + fetchData: -> + $.ajax( + url: @dataEndpoint, + dataType: @options.dataType, + beforeSend: => + if @options.beforeSend + @options.beforeSend() + success: (data) => + if @options.success + @options.success(data) + ) + +class GitLabDropdown + LOADING_CLASS = "is-loading" + PAGE_TWO_CLASS = "is-page-two" + ACTIVE_CLASS = "is-active" + INDETERMINATE_CLASS = "is-indeterminate" + NON_SELECTABLE_CLASSES = '.divider, .separator, .dropdown-header, .dropdown-menu-empty-link' + SELECTABLE_CLASSES = ".dropdown-content li:not(#{NON_SELECTABLE_CLASSES})" + FILTER_INPUT = '.dropdown-input .dropdown-input-field' + currentIndex = -1 + CURSOR_SELECT_SCROLL_PADDING = 5 + + constructor: (@el, @options) -> + self = @ + selector = $(@el).data "target" + @dropdown = if selector? then $(selector) else $(@el).parent() + + # Set Defaults + { + # If no input is passed create a default one + @filterInput = @getElement(FILTER_INPUT) + @highlight = false + @filterInputBlur = true + } = @options + + self = @ + + # If selector was passed + if _.isString(@filterInput) + @filterInput = @getElement(@filterInput) + + searchFields = if @options.search then @options.search.fields else [] + + if @options.data + # If we provided data + # data could be an array of objects or a group of arrays + if _.isObject(@options.data) and not _.isFunction(@options.data) + @fullData = @options.data + @parseData @options.data + else + # Remote data + @remote = new GitLabDropdownRemote @options.data, { + dataType: @options.dataType, + beforeSend: @toggleLoading.bind(@) + success: (data) => + @fullData = data + + # Reset selected row index on new data + currentIndex = -1 + @parseData @fullData + + @filter.input.trigger('keyup') if @options.filterable and @filter and @filter.input + } + + # Init filterable + if @options.filterable + @filter = new GitLabDropdownFilter @filterInput, + filterInputBlur: @filterInputBlur + filterByText: @options.filterByText + onFilter: @options.onFilter + remote: @options.filterRemote + query: @options.data + keys: searchFields + elements: => + selector = SELECTABLE_CLASSES + + if @dropdown.find('.dropdown-toggle-page').length + selector = ".dropdown-page-one #{selector}" + + return $(selector) + data: => + return @fullData + callback: (data) => + @parseData data + + unless @filterInput.val() is '' + selector = '.dropdown-content li:not(.divider):visible' + + if @dropdown.find('.dropdown-toggle-page').length + selector = ".dropdown-page-one #{selector}" + + $(selector, @dropdown) + .first() + .find('a') + .addClass('is-focused') + + currentIndex = 0 + + + # Event listeners + + @dropdown.on "shown.bs.dropdown", @opened + @dropdown.on "hidden.bs.dropdown", @hidden + $(@el).on "update.label", @updateLabel + @dropdown.on "click", ".dropdown-menu, .dropdown-menu-close", @shouldPropagate + @dropdown.on 'keyup', (e) => + if e.which is 27 # Escape key + $('.dropdown-menu-close', @dropdown).trigger 'click' + @dropdown.on 'blur', 'a', (e) => + if e.relatedTarget? + $relatedTarget = $(e.relatedTarget) + $dropdownMenu = $relatedTarget.closest('.dropdown-menu') + + if $dropdownMenu.length is 0 + @dropdown.removeClass('open') + + if @dropdown.find(".dropdown-toggle-page").length + @dropdown.find(".dropdown-toggle-page, .dropdown-menu-back").on "click", (e) => + e.preventDefault() + e.stopPropagation() + + @togglePage() + + if @options.selectable + selector = ".dropdown-content a" + + if @dropdown.find(".dropdown-toggle-page").length + selector = ".dropdown-page-one .dropdown-content a" + + @dropdown.on "click", selector, (e) -> + $el = $(@) + selected = self.rowClicked $el + + if self.options.clicked + self.options.clicked(selected, $el, e) + + $el.trigger('blur') + + # Finds an element inside wrapper element + getElement: (selector) -> + @dropdown.find selector + + toggleLoading: -> + $('.dropdown-menu', @dropdown).toggleClass LOADING_CLASS + + togglePage: -> + menu = $('.dropdown-menu', @dropdown) + + if menu.hasClass(PAGE_TWO_CLASS) + if @remote + @remote.execute() + + menu.toggleClass PAGE_TWO_CLASS + + # Focus first visible input on active page + @dropdown.find('[class^="dropdown-page-"]:visible :text:visible:first').focus() + + parseData: (data) -> + @renderedData = data + + if @options.filterable and data.length is 0 + # render no matching results + html = [@noResults()] + else + # Handle array groups + if gl.utils.isObject data + html = [] + for name, groupData of data + # Add header for each group + html.push(@renderItem(header: name, name)) + + @renderData(groupData, name) + .map (item) -> + html.push item + else + # Render each row + html = @renderData(data) + + # Render the full menu + full_html = @renderMenu(html) + + @appendMenu(full_html) + + renderData: (data, group = false) -> + data.map (obj, index) => + return @renderItem(obj, group, index) + + shouldPropagate: (e) => + if @options.multiSelect + $target = $(e.target) + + if not $target.hasClass('dropdown-menu-close') and not $target.hasClass('dropdown-menu-close-icon') and not $target.data('is-link') + e.stopPropagation() + return false + else + return true + + opened: => + @resetRows() + @addArrowKeyEvent() + + if @options.setIndeterminateIds + @options.setIndeterminateIds.call(@) + + if @options.setActiveIds + @options.setActiveIds.call(@) + + # Makes indeterminate items effective + if @fullData and @dropdown.find('.dropdown-menu-toggle').hasClass('js-filter-bulk-update') + @parseData @fullData + + contentHtml = $('.dropdown-content', @dropdown).html() + if @remote && contentHtml is "" + @remote.execute() + + if @options.filterable + @filterInput.focus() + + @dropdown.trigger('shown.gl.dropdown') + + hidden: (e) => + @resetRows() + @removeArrayKeyEvent() + + $input = @dropdown.find(".dropdown-input-field") + + if @options.filterable + $input + .blur() + .val("") + + # Triggering 'keyup' will re-render the dropdown which is not always required + # specially if we want to keep the state of the dropdown needed for bulk-assignment + if not @options.persistWhenHide + $input.trigger("keyup") + + if @dropdown.find(".dropdown-toggle-page").length + $('.dropdown-menu', @dropdown).removeClass PAGE_TWO_CLASS + + if @options.hidden + @options.hidden.call(@,e) + + @dropdown.trigger('hidden.gl.dropdown') + + + # Render the full menu + renderMenu: (html) -> + menu_html = "" + + if @options.renderMenu + menu_html = @options.renderMenu(html) + else + menu_html = $('