diff options
78 files changed, 1140 insertions, 733 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1ccaa2e095c..50c2a7b5adf 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -298,9 +298,8 @@ review-docs-deploy: - gem install gitlab --no-ri --no-rdoc - ./$SCRIPT_NAME deploy only: - - /(^docs[\/-].*|.*-docs$)/ - - branches@gitlab-org/gitlab-ce - - branches@gitlab-org/gitlab-ee + - /(^docs[\/-].*|.*-docs$)/@gitlab-org/gitlab-ce + - /(^docs[\/-].*|.*-docs$)/@gitlab-org/gitlab-ee <<: *except-qa # Cleanup remote environment of gitlab-docs diff --git a/app/assets/javascripts/pages/dashboard/todos/index/todos.js b/app/assets/javascripts/pages/dashboard/todos/index/todos.js index ff19b9a9c30..9aa83ce6269 100644 --- a/app/assets/javascripts/pages/dashboard/todos/index/todos.js +++ b/app/assets/javascripts/pages/dashboard/todos/index/todos.js @@ -39,6 +39,7 @@ export default class Todos { } initFilters() { + this.initFilterDropdown($('.js-group-search'), 'group_id', ['text']); this.initFilterDropdown($('.js-project-search'), 'project_id', ['text']); this.initFilterDropdown($('.js-type-search'), 'type'); this.initFilterDropdown($('.js-action-search'), 'action_id'); @@ -53,7 +54,16 @@ export default class Todos { filterable: searchFields ? true : false, search: { fields: searchFields }, data: $dropdown.data('data'), - clicked: () => $dropdown.closest('form.filter-form').submit(), + clicked: () => { + const $formEl = $dropdown.closest('form.filter-form'); + const mutexDropdowns = { + group_id: 'project_id', + project_id: 'group_id', + }; + + $formEl.find(`input[name="${mutexDropdowns[fieldName]}"]`).remove(); + $formEl.submit(); + }, }); } diff --git a/app/assets/javascripts/pages/projects/clusters/gcp/login/index.js b/app/assets/javascripts/pages/projects/clusters/gcp/login/index.js deleted file mode 100644 index 0c2d7d7c96a..00000000000 --- a/app/assets/javascripts/pages/projects/clusters/gcp/login/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import gcpSignupOffer from '~/clusters/components/gcp_signup_offer'; - -gcpSignupOffer(); diff --git a/app/assets/javascripts/pages/projects/clusters/new/index.js b/app/assets/javascripts/pages/projects/clusters/new/index.js deleted file mode 100644 index 0c2d7d7c96a..00000000000 --- a/app/assets/javascripts/pages/projects/clusters/new/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import gcpSignupOffer from '~/clusters/components/gcp_signup_offer'; - -gcpSignupOffer(); diff --git a/app/assets/javascripts/pages/projects/index.js b/app/assets/javascripts/pages/projects/index.js index de1e13de7e9..0658ae8ec23 100644 --- a/app/assets/javascripts/pages/projects/index.js +++ b/app/assets/javascripts/pages/projects/index.js @@ -1,7 +1,21 @@ +import gcpSignupOffer from '~/clusters/components/gcp_signup_offer'; +import initGkeDropdowns from '~/projects/gke_cluster_dropdowns'; import Project from './project'; import ShortcutsNavigation from '../../shortcuts_navigation'; document.addEventListener('DOMContentLoaded', () => { + const page = document.body.dataset.page; + const newClusterViews = [ + 'projects:clusters:new', + 'projects:clusters:create_gcp', + 'projects:clusters:create_user', + ]; + + if (newClusterViews.indexOf(page) > -1) { + gcpSignupOffer(); + initGkeDropdowns(); + } + new Project(); // eslint-disable-line no-new new ShortcutsNavigation(); // eslint-disable-line no-new }); diff --git a/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue new file mode 100644 index 00000000000..ffaed9c7193 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue @@ -0,0 +1,98 @@ +<script> +import { __ } from '~/locale'; +import tooltip from '~/vue_shared/directives/tooltip'; + +import Icon from '~/vue_shared/components/icon.vue'; +import LoadingIcon from '~/vue_shared/components/loading_icon.vue'; + +const MARK_TEXT = __('Mark todo as done'); +const TODO_TEXT = __('Add todo'); + +export default { + directives: { + tooltip, + }, + components: { + Icon, + LoadingIcon, + }, + props: { + issuableId: { + type: Number, + required: true, + }, + issuableType: { + type: String, + required: true, + }, + isTodo: { + type: Boolean, + required: false, + default: true, + }, + isActionActive: { + type: Boolean, + required: false, + default: false, + }, + collapsed: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + buttonClasses() { + return this.collapsed ? + 'btn-blank btn-todo sidebar-collapsed-icon dont-change-state' : + 'btn btn-default btn-todo issuable-header-btn float-right'; + }, + buttonLabel() { + return this.isTodo ? MARK_TEXT : TODO_TEXT; + }, + collapsedButtonIconClasses() { + return this.isTodo ? 'todo-undone' : ''; + }, + collapsedButtonIcon() { + return this.isTodo ? 'todo-done' : 'todo-add'; + }, + }, + methods: { + handleButtonClick() { + this.$emit('toggleTodo'); + }, + }, +}; +</script> + +<template> + <button + v-tooltip + :class="buttonClasses" + :title="buttonLabel" + :aria-label="buttonLabel" + :data-issuable-id="issuableId" + :data-issuable-type="issuableType" + type="button" + data-container="body" + data-placement="left" + data-boundary="viewport" + @click="handleButtonClick" + > + <icon + v-show="collapsed" + :css-classes="collapsedButtonIconClasses" + :name="collapsedButtonIcon" + /> + <span + v-show="!collapsed" + class="issuable-todo-inner" + > + {{ buttonLabel }} + </span> + <loading-icon + v-show="isActionActive" + :inline="true" + /> + </button> +</template> diff --git a/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue b/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue index ac2e99abe77..80dc7d3557c 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue @@ -12,6 +12,11 @@ export default { type: Boolean, required: true, }, + cssClasses: { + type: String, + required: false, + default: '', + }, }, computed: { tooltipLabel() { @@ -30,10 +35,12 @@ export default { <button v-tooltip :title="tooltipLabel" + :class="cssClasses" type="button" class="btn btn-blank gutter-toggle btn-sidebar-action" data-container="body" data-placement="left" + data-boundary="viewport" @click="toggle" > <i diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index f9fd9f1ab8b..f6617380cc0 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -449,6 +449,7 @@ .todo-undone { color: $gl-link-color; + fill: $gl-link-color; } .author { diff --git a/app/assets/stylesheets/pages/todos.scss b/app/assets/stylesheets/pages/todos.scss index e5d7dd13915..010a2c05a1c 100644 --- a/app/assets/stylesheets/pages/todos.scss +++ b/app/assets/stylesheets/pages/todos.scss @@ -174,6 +174,18 @@ } } +@include media-breakpoint-down(lg) { + .todos-filters { + .filter-categories { + width: 75%; + + .filter-item { + margin-bottom: 10px; + } + } + } +} + @include media-breakpoint-down(xs) { .todo { .avatar { @@ -199,6 +211,10 @@ } .todos-filters { + .filter-categories { + width: auto; + } + .dropdown-menu-toggle { width: 100%; } diff --git a/app/controllers/concerns/todos_actions.rb b/app/controllers/concerns/todos_actions.rb new file mode 100644 index 00000000000..c0acdb3498d --- /dev/null +++ b/app/controllers/concerns/todos_actions.rb @@ -0,0 +1,12 @@ +module TodosActions + extend ActiveSupport::Concern + + def create + todo = TodoService.new.mark_todo(issuable, current_user) + + render json: { + count: TodosFinder.new(current_user, state: :pending).execute.count, + delete_path: dashboard_todo_path(todo) + } + end +end diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index f9e8fe624e8..bd7111e28bc 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -70,7 +70,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController end def todo_params - params.permit(:action_id, :author_id, :project_id, :type, :sort, :state) + params.permit(:action_id, :author_id, :project_id, :type, :sort, :state, :group_id) end def redirect_out_of_range(todos) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb deleted file mode 100644 index c2c5ad61e01..00000000000 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ /dev/null @@ -1,76 +0,0 @@ -class Projects::Clusters::GcpController < Projects::ApplicationController - before_action :authorize_read_cluster! - before_action :authorize_create_cluster!, only: [:new, :create] - before_action :authorize_google_api, except: :login - helper_method :token_in_session - - def login - begin - state = generate_session_key_redirect(gcp_new_namespace_project_clusters_path.to_s) - - @authorize_url = GoogleApi::CloudPlatform::Client.new( - nil, callback_google_api_auth_url, - state: state).authorize_url - rescue GoogleApi::Auth::ConfigMissingError - # no-op - end - end - - def new - @cluster = ::Clusters::Cluster.new.tap do |cluster| - cluster.build_provider_gcp - end - end - - def create - @cluster = ::Clusters::CreateService - .new(project, current_user, create_params) - .execute(token_in_session) - - if @cluster.persisted? - redirect_to project_cluster_path(project, @cluster) - else - render :new - end - end - - private - - def create_params - params.require(:cluster).permit( - :enabled, - :name, - :environment_scope, - provider_gcp_attributes: [ - :gcp_project_id, - :zone, - :num_nodes, - :machine_type - ]).merge( - provider_type: :gcp, - platform_type: :kubernetes - ) - end - - def authorize_google_api - unless GoogleApi::CloudPlatform::Client.new(token_in_session, nil) - .validate_token(expires_at_in_session) - redirect_to action: 'login' - end - end - - def token_in_session - session[GoogleApi::CloudPlatform::Client.session_key_for_token] - end - - def expires_at_in_session - @expires_at_in_session ||= - session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] - end - - def generate_session_key_redirect(uri) - GoogleApi::CloudPlatform::Client.new_session_key_for_redirect_uri do |key| - session[key] = uri - end - end -end diff --git a/app/controllers/projects/clusters/user_controller.rb b/app/controllers/projects/clusters/user_controller.rb deleted file mode 100644 index d0db64b2fa9..00000000000 --- a/app/controllers/projects/clusters/user_controller.rb +++ /dev/null @@ -1,40 +0,0 @@ -class Projects::Clusters::UserController < Projects::ApplicationController - before_action :authorize_read_cluster! - before_action :authorize_create_cluster!, only: [:new, :create] - - def new - @cluster = ::Clusters::Cluster.new.tap do |cluster| - cluster.build_platform_kubernetes - end - end - - def create - @cluster = ::Clusters::CreateService - .new(project, current_user, create_params) - .execute - - if @cluster.persisted? - redirect_to project_cluster_path(project, @cluster) - else - render :new - end - end - - private - - def create_params - params.require(:cluster).permit( - :enabled, - :name, - :environment_scope, - platform_kubernetes_attributes: [ - :namespace, - :api_url, - :token, - :ca_cert - ]).merge( - provider_type: :user, - platform_type: :kubernetes - ) - end -end diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index d58039b7d42..62193257940 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -1,10 +1,15 @@ class Projects::ClustersController < Projects::ApplicationController - before_action :cluster, except: [:index, :new] + before_action :cluster, except: [:index, :new, :create_gcp, :create_user] before_action :authorize_read_cluster! + before_action :generate_gcp_authorize_url, only: [:new] + before_action :validate_gcp_token, only: [:new] + before_action :gcp_cluster, only: [:new] + before_action :user_cluster, only: [:new] before_action :authorize_create_cluster!, only: [:new] before_action :authorize_update_cluster!, only: [:update] before_action :authorize_admin_cluster!, only: [:destroy] before_action :update_applications_status, only: [:status] + helper_method :token_in_session STATUS_POLLING_INTERVAL = 10_000 @@ -64,6 +69,38 @@ class Projects::ClustersController < Projects::ApplicationController end end + def create_gcp + @gcp_cluster = ::Clusters::CreateService + .new(project, current_user, create_gcp_cluster_params) + .execute(token_in_session) + + if @gcp_cluster.persisted? + redirect_to project_cluster_path(project, @gcp_cluster) + else + generate_gcp_authorize_url + validate_gcp_token + user_cluster + + render :new, locals: { active_tab: 'gcp' } + end + end + + def create_user + @user_cluster = ::Clusters::CreateService + .new(project, current_user, create_user_cluster_params) + .execute(token_in_session) + + if @user_cluster.persisted? + redirect_to project_cluster_path(project, @user_cluster) + else + generate_gcp_authorize_url + validate_gcp_token + gcp_cluster + + render :new, locals: { active_tab: 'user' } + end + end + private def cluster @@ -95,6 +132,80 @@ class Projects::ClustersController < Projects::ApplicationController end end + def create_gcp_cluster_params + params.require(:cluster).permit( + :enabled, + :name, + :environment_scope, + provider_gcp_attributes: [ + :gcp_project_id, + :zone, + :num_nodes, + :machine_type + ]).merge( + provider_type: :gcp, + platform_type: :kubernetes + ) + end + + def create_user_cluster_params + params.require(:cluster).permit( + :enabled, + :name, + :environment_scope, + platform_kubernetes_attributes: [ + :namespace, + :api_url, + :token, + :ca_cert + ]).merge( + provider_type: :user, + platform_type: :kubernetes + ) + end + + def generate_gcp_authorize_url + state = generate_session_key_redirect(new_project_cluster_path(@project).to_s) + + @authorize_url = GoogleApi::CloudPlatform::Client.new( + nil, callback_google_api_auth_url, + state: state).authorize_url + rescue GoogleApi::Auth::ConfigMissingError + # no-op + end + + def gcp_cluster + @gcp_cluster = ::Clusters::Cluster.new.tap do |cluster| + cluster.build_provider_gcp + end + end + + def user_cluster + @user_cluster = ::Clusters::Cluster.new.tap do |cluster| + cluster.build_platform_kubernetes + end + end + + def validate_gcp_token + @valid_gcp_token = GoogleApi::CloudPlatform::Client.new(token_in_session, nil) + .validate_token(expires_at_in_session) + end + + def token_in_session + session[GoogleApi::CloudPlatform::Client.session_key_for_token] + end + + def expires_at_in_session + @expires_at_in_session ||= + session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] + end + + def generate_session_key_redirect(uri) + GoogleApi::CloudPlatform::Client.new_session_key_for_redirect_uri do |key| + session[key] = uri + end + end + def authorize_update_cluster! access_denied! unless can?(current_user, :update_cluster, cluster) end diff --git a/app/controllers/projects/todos_controller.rb b/app/controllers/projects/todos_controller.rb index a41fcb85c40..93fb9da6510 100644 --- a/app/controllers/projects/todos_controller.rb +++ b/app/controllers/projects/todos_controller.rb @@ -1,19 +1,13 @@ class Projects::TodosController < Projects::ApplicationController - before_action :authenticate_user!, only: [:create] - - def create - todo = TodoService.new.mark_todo(issuable, current_user) + include Gitlab::Utils::StrongMemoize + include TodosActions - render json: { - count: TodosFinder.new(current_user, state: :pending).execute.count, - delete_path: dashboard_todo_path(todo) - } - end + before_action :authenticate_user!, only: [:create] private def issuable - @issuable ||= begin + strong_memoize(:issuable) do case params[:issuable_type] when "issue" IssuesFinder.new(current_user, project_id: @project.id).find(params[:issuable_id]) diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 09e2c586f2a..2156413fb26 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -15,6 +15,7 @@ class TodosFinder prepend FinderWithCrossProjectAccess include FinderMethods + include Gitlab::Utils::StrongMemoize requires_cross_project_access unless: -> { project? } @@ -34,9 +35,11 @@ class TodosFinder items = by_author(items) items = by_state(items) items = by_type(items) + items = by_group(items) # Filtering by project HAS TO be the last because we use # the project IDs yielded by the todos query thus far items = by_project(items) + items = visible_to_user(items) sort(items) end @@ -82,6 +85,10 @@ class TodosFinder params[:project_id].present? end + def group? + params[:group_id].present? + end + def project return @project if defined?(@project) @@ -100,18 +107,14 @@ class TodosFinder @project end - def project_ids(items) - ids = items.except(:order).select(:project_id) - if Gitlab::Database.mysql? - # To make UPDATE work on MySQL, wrap it in a SELECT with an alias - ids = Todo.except(:order).select('*').from("(#{ids.to_sql}) AS t") + def group + strong_memoize(:group) do + Group.find(params[:group_id]) end - - ids end def type? - type.present? && %w(Issue MergeRequest).include?(type) + type.present? && %w(Issue MergeRequest Epic).include?(type) end def type @@ -148,12 +151,37 @@ class TodosFinder def by_project(items) if project? - items.where(project: project) - else - projects = Project.public_or_visible_to_user(current_user) + items = items.where(project: project) + end + + items + end - items.joins(:project).merge(projects) + def by_group(items) + if group? + groups = group.self_and_descendants + items = items.where( + 'project_id IN (?) OR group_id IN (?)', + Project.where(group: groups).select(:id), + groups.select(:id) + ) end + + items + end + + def visible_to_user(items) + projects = Project.public_or_visible_to_user(current_user) + groups = Group.public_or_visible_to_user(current_user) + + items + .joins('LEFT JOIN namespaces ON namespaces.id = todos.group_id') + .joins('LEFT JOIN projects ON projects.id = todos.project_id') + .where( + 'project_id IN (?) OR group_id IN (?)', + projects.select(:id), + groups.select(:id) + ) end def by_state(items) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 9f501ea55fb..353479776b8 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -131,6 +131,19 @@ module IssuablesHelper end end + def group_dropdown_label(group_id, default_label) + return default_label if group_id.nil? + return "Any group" if group_id == "0" + + group = ::Group.find_by(id: group_id) + + if group + group.full_name + else + default_label + end + end + def milestone_dropdown_label(milestone_title, default_label = "Milestone") title = case milestone_title diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index f7620e0b6b8..7cd74358168 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -43,7 +43,7 @@ module TodosHelper project_commit_path(todo.project, todo.target, anchor: anchor) else - path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target] + path = [todo.parent, todo.target] path.unshift(:pipelines) if todo.build_failed? @@ -167,4 +167,12 @@ module TodosHelper def show_todo_state?(todo) (todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && %w(closed merged).include?(todo.target.state) end + + def todo_group_options + groups = current_user.authorized_groups.map do |group| + { id: group.id, text: group.full_name } + end + + groups.unshift({ id: '', text: 'Any Group' }).to_json + end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index b93c1145f82..7a459078151 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -243,6 +243,12 @@ module Issuable opened? end + def overdue? + return false unless respond_to?(:due_date) + + due_date.try(:past?) || false + end + def user_notes_count if notes.loaded? # Use the in-memory association to select and count to avoid hitting the db diff --git a/app/models/group.rb b/app/models/group.rb index 9c171de7fc3..b0392774379 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -39,6 +39,8 @@ class Group < Namespace has_many :boards has_many :badges, class_name: 'GroupBadge' + has_many :todos + accepts_nested_attributes_for :variables, allow_destroy: true validate :visibility_level_allowed_by_projects @@ -82,6 +84,12 @@ class Group < Namespace where(id: user.authorized_groups.select(:id).reorder(nil)) end + def public_or_visible_to_user(user) + where('id IN (?) OR namespaces.visibility_level IN (?)', + user.authorized_groups.select(:id), + Gitlab::VisibilityLevel.levels_for_user(user)) + end + def select_for_project_authorization if current_scope.joins_values.include?(:shared_projects) joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id') diff --git a/app/models/issue.rb b/app/models/issue.rb index 4715d942c8d..983684a5e05 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -275,10 +275,6 @@ class Issue < ActiveRecord::Base user ? readable_by?(user) : publicly_visible? end - def overdue? - due_date.try(:past?) || false - end - def check_for_spam? project.public? && (title_changed? || description_changed?) end diff --git a/app/models/note.rb b/app/models/note.rb index abc40d9016e..3918bbee194 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -229,6 +229,10 @@ class Note < ActiveRecord::Base !for_personal_snippet? end + def for_issuable? + for_issue? || for_merge_request? + end + def skip_project_check? !for_project_noteable? end diff --git a/app/models/todo.rb b/app/models/todo.rb index a2ab405fdbe..942cbb754e3 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -22,15 +22,18 @@ class Todo < ActiveRecord::Base belongs_to :author, class_name: "User" belongs_to :note belongs_to :project + belongs_to :group belongs_to :target, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations belongs_to :user delegate :name, :email, to: :author, prefix: true, allow_nil: true - validates :action, :project, :target_type, :user, presence: true + validates :action, :target_type, :user, presence: true validates :author, presence: true validates :target_id, presence: true, unless: :for_commit? validates :commit_id, presence: true, if: :for_commit? + validates :project, presence: true, unless: :group_id + validates :group, presence: true, unless: :project_id scope :pending, -> { with_state(:pending) } scope :done, -> { with_state(:done) } @@ -44,7 +47,7 @@ class Todo < ActiveRecord::Base state :done end - after_save :keep_around_commit + after_save :keep_around_commit, if: :commit_id class << self # Priority sorting isn't displayed in the dropdown, because we don't show @@ -79,6 +82,10 @@ class Todo < ActiveRecord::Base end end + def parent + project + end + def unmergeable? action == UNMERGEABLE end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index f91cd03bf5c..46f12086555 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -260,15 +260,15 @@ class TodoService end end - def create_mention_todos(project, target, author, note = nil, skip_users = []) + def create_mention_todos(parent, target, author, note = nil, skip_users = []) # Create Todos for directly addressed users - directly_addressed_users = filter_directly_addressed_users(project, note || target, author, skip_users) - attributes = attributes_for_todo(project, target, author, Todo::DIRECTLY_ADDRESSED, note) + directly_addressed_users = filter_directly_addressed_users(parent, note || target, author, skip_users) + attributes = attributes_for_todo(parent, target, author, Todo::DIRECTLY_ADDRESSED, note) create_todos(directly_addressed_users, attributes) # Create Todos for mentioned users - mentioned_users = filter_mentioned_users(project, note || target, author, skip_users) - attributes = attributes_for_todo(project, target, author, Todo::MENTIONED, note) + mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users) + attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note) create_todos(mentioned_users, attributes) end @@ -299,36 +299,36 @@ class TodoService def attributes_for_todo(project, target, author, action, note = nil) attributes_for_target(target).merge!( - project_id: project.id, + project_id: project&.id, author_id: author.id, action: action, note: note ) end - def filter_todo_users(users, project, target) - reject_users_without_access(users, project, target).uniq + def filter_todo_users(users, parent, target) + reject_users_without_access(users, parent, target).uniq end - def filter_mentioned_users(project, target, author, skip_users = []) + def filter_mentioned_users(parent, target, author, skip_users = []) mentioned_users = target.mentioned_users(author) - skip_users - filter_todo_users(mentioned_users, project, target) + filter_todo_users(mentioned_users, parent, target) end - def filter_directly_addressed_users(project, target, author, skip_users = []) + def filter_directly_addressed_users(parent, target, author, skip_users = []) directly_addressed_users = target.directly_addressed_users(author) - skip_users - filter_todo_users(directly_addressed_users, project, target) + filter_todo_users(directly_addressed_users, parent, target) end - def reject_users_without_access(users, project, target) - if target.is_a?(Note) && (target.for_issue? || target.for_merge_request?) + def reject_users_without_access(users, parent, target) + if target.is_a?(Note) && target.for_issuable? target = target.noteable end if target.is_a?(Issuable) select_users(users, :"read_#{target.to_ability_name}", target) else - select_users(users, :read_project, project) + select_users(users, :read_project, parent) end end diff --git a/app/validators/abstract_path_validator.rb b/app/validators/abstract_path_validator.rb index e43b66cbe3a..45ac695c5ec 100644 --- a/app/validators/abstract_path_validator.rb +++ b/app/validators/abstract_path_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AbstractPathValidator < ActiveModel::EachValidator extend Gitlab::EncodingHelper diff --git a/app/validators/certificate_fingerprint_validator.rb b/app/validators/certificate_fingerprint_validator.rb index 17df756183a..79d78653ec7 100644 --- a/app/validators/certificate_fingerprint_validator.rb +++ b/app/validators/certificate_fingerprint_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class CertificateFingerprintValidator < ActiveModel::EachValidator FINGERPRINT_PATTERN = /\A([a-zA-Z0-9]{2}[\s\-:]?){16,}\z/.freeze diff --git a/app/validators/certificate_key_validator.rb b/app/validators/certificate_key_validator.rb index 8c7bb750339..5b2bbffc066 100644 --- a/app/validators/certificate_key_validator.rb +++ b/app/validators/certificate_key_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # UrlValidator # # Custom validator for private keys. diff --git a/app/validators/certificate_validator.rb b/app/validators/certificate_validator.rb index b0c9a1b92a4..de8bb179dfb 100644 --- a/app/validators/certificate_validator.rb +++ b/app/validators/certificate_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # UrlValidator # # Custom validator for private keys. diff --git a/app/validators/cluster_name_validator.rb b/app/validators/cluster_name_validator.rb index e7d32550176..85fd63f08e5 100644 --- a/app/validators/cluster_name_validator.rb +++ b/app/validators/cluster_name_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # ClusterNameValidator # # Custom validator for ClusterName. diff --git a/app/validators/color_validator.rb b/app/validators/color_validator.rb index 571d0007aa2..1932d042e83 100644 --- a/app/validators/color_validator.rb +++ b/app/validators/color_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # ColorValidator # # Custom validator for web color codes. It requires the leading hash symbol and diff --git a/app/validators/cron_timezone_validator.rb b/app/validators/cron_timezone_validator.rb index 542c7d006ad..c5f51d65060 100644 --- a/app/validators/cron_timezone_validator.rb +++ b/app/validators/cron_timezone_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # CronTimezoneValidator # # Custom validator for CronTimezone. diff --git a/app/validators/cron_validator.rb b/app/validators/cron_validator.rb index 981fade47a6..bd48a7a6efb 100644 --- a/app/validators/cron_validator.rb +++ b/app/validators/cron_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # CronValidator # # Custom validator for Cron. diff --git a/app/validators/duration_validator.rb b/app/validators/duration_validator.rb index 10ff44031c6..811828169ca 100644 --- a/app/validators/duration_validator.rb +++ b/app/validators/duration_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # DurationValidator # # Validate the format conforms with ChronicDuration diff --git a/app/validators/email_validator.rb b/app/validators/email_validator.rb index aab07a7ece4..9459edb7515 100644 --- a/app/validators/email_validator.rb +++ b/app/validators/email_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class EmailValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) record.errors.add(attribute, :invalid) unless value =~ Devise.email_regexp diff --git a/app/validators/key_restriction_validator.rb b/app/validators/key_restriction_validator.rb index 204be827941..891d13b1596 100644 --- a/app/validators/key_restriction_validator.rb +++ b/app/validators/key_restriction_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class KeyRestrictionValidator < ActiveModel::EachValidator FORBIDDEN = -1 diff --git a/app/validators/line_code_validator.rb b/app/validators/line_code_validator.rb index ed29e5aeb67..a351180790e 100644 --- a/app/validators/line_code_validator.rb +++ b/app/validators/line_code_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # LineCodeValidator # # Custom validator for GitLab line codes. diff --git a/app/validators/namespace_name_validator.rb b/app/validators/namespace_name_validator.rb index 2e51af2982d..fb1c241037c 100644 --- a/app/validators/namespace_name_validator.rb +++ b/app/validators/namespace_name_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # NamespaceNameValidator # # Custom validator for GitLab namespace name strings. diff --git a/app/validators/namespace_path_validator.rb b/app/validators/namespace_path_validator.rb index 7b0ae4db5d4..c078b272b2f 100644 --- a/app/validators/namespace_path_validator.rb +++ b/app/validators/namespace_path_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class NamespacePathValidator < AbstractPathValidator extend Gitlab::EncodingHelper diff --git a/app/validators/project_path_validator.rb b/app/validators/project_path_validator.rb index 424fd77a6a3..aea0a68e7cf 100644 --- a/app/validators/project_path_validator.rb +++ b/app/validators/project_path_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectPathValidator < AbstractPathValidator extend Gitlab::EncodingHelper diff --git a/app/validators/public_url_validator.rb b/app/validators/public_url_validator.rb index 1e8118fccbb..3ff880deedd 100644 --- a/app/validators/public_url_validator.rb +++ b/app/validators/public_url_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # PublicUrlValidator # # Custom validator for URLs. This validator works like UrlValidator but diff --git a/app/validators/top_level_group_validator.rb b/app/validators/top_level_group_validator.rb index 7e2e735e0cf..b50c9dca154 100644 --- a/app/validators/top_level_group_validator.rb +++ b/app/validators/top_level_group_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class TopLevelGroupValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) if value&.subgroup? diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb index 6854fec582e..faaf1283078 100644 --- a/app/validators/url_validator.rb +++ b/app/validators/url_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # UrlValidator # # Custom validator for URLs. diff --git a/app/validators/variable_duplicates_validator.rb b/app/validators/variable_duplicates_validator.rb index 72660be6c43..90193e85f2a 100644 --- a/app/validators/variable_duplicates_validator.rb +++ b/app/validators/variable_duplicates_validator.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # VariableDuplicatesValidator # # This validator is designed for especially the following condition @@ -22,8 +24,8 @@ class VariableDuplicatesValidator < ActiveModel::EachValidator def validate_duplicates(record, attribute, values) duplicates = values.reject(&:marked_for_destruction?).group_by(&:key).select { |_, v| v.many? }.map(&:first) if duplicates.any? - error_message = "have duplicate values (#{duplicates.join(", ")})" - error_message += " for #{values.first.send(options[:scope])} scope" if options[:scope] # rubocop:disable GitlabSecurity/PublicSend + error_message = +"have duplicate values (#{duplicates.join(", ")})" + error_message << " for #{values.first.send(options[:scope])} scope" if options[:scope] # rubocop:disable GitlabSecurity/PublicSend record.errors.add(attribute, error_message) end end diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index d5a9cc646a6..8b3974d97f8 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -30,27 +30,33 @@ .todos-filters .row-content-block.second-block - = form_tag todos_filter_path(without: [:project_id, :author_id, :type, :action_id]), method: :get, class: 'filter-form' do - .filter-item.inline - - if params[:project_id].present? - = hidden_field_tag(:project_id, params[:project_id]) - = dropdown_tag(project_dropdown_label(params[:project_id], 'Project'), options: { toggle_class: 'js-project-search js-filter-submit', title: 'Filter by project', filter: true, filterInput: 'input#project-search', dropdown_class: 'dropdown-menu-selectable dropdown-menu-project js-filter-submit', - placeholder: 'Search projects', data: { data: todo_projects_options, default_label: 'Project', display: 'static' } }) - .filter-item.inline - - if params[:author_id].present? - = hidden_field_tag(:author_id, params[:author_id]) - = dropdown_tag(user_dropdown_label(params[:author_id], 'Author'), options: { toggle_class: 'js-user-search js-filter-submit js-author-search', title: 'Filter by author', filter: true, filterInput: 'input#author-search', dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-author js-filter-submit', - placeholder: 'Search authors', data: { any_user: 'Any Author', first_user: (current_user.username if current_user), project_id: (@project.id if @project), selected: params[:author_id], field_name: 'author_id', default_label: 'Author', todo_filter: true, todo_state_filter: params[:state] || 'pending' } }) - .filter-item.inline - - if params[:type].present? - = hidden_field_tag(:type, params[:type]) - = dropdown_tag(todo_types_dropdown_label(params[:type], 'Type'), options: { toggle_class: 'js-type-search js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-type js-filter-submit', - data: { data: todo_types_options, default_label: 'Type' } }) - .filter-item.inline.actions-filter - - if params[:action_id].present? - = hidden_field_tag(:action_id, params[:action_id]) - = dropdown_tag(todo_actions_dropdown_label(params[:action_id], 'Action'), options: { toggle_class: 'js-action-search js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-action js-filter-submit', - data: { data: todo_actions_options, default_label: 'Action' } }) + = form_tag todos_filter_path(without: [:project_id, :author_id, :type, :action_id]), method: :get, class: 'filter-form d-sm-flex' do + .filter-categories.flex-fill + .filter-item.inline + - if params[:group_id].present? + = hidden_field_tag(:group_id, params[:group_id]) + = dropdown_tag(group_dropdown_label(params[:group_id], 'Group'), options: { toggle_class: 'js-group-search js-filter-submit', title: 'Filter by group', filter: true, filterInput: 'input#group-search', dropdown_class: 'dropdown-menu-selectable dropdown-menu-group js-filter-submit', + placeholder: 'Search groups', data: { data: todo_group_options, default_label: 'Group', display: 'static' } }) + .filter-item.inline + - if params[:project_id].present? + = hidden_field_tag(:project_id, params[:project_id]) + = dropdown_tag(project_dropdown_label(params[:project_id], 'Project'), options: { toggle_class: 'js-project-search js-filter-submit', title: 'Filter by project', filter: true, filterInput: 'input#project-search', dropdown_class: 'dropdown-menu-selectable dropdown-menu-project js-filter-submit', + placeholder: 'Search projects', data: { data: todo_projects_options, default_label: 'Project', display: 'static' } }) + .filter-item.inline + - if params[:author_id].present? + = hidden_field_tag(:author_id, params[:author_id]) + = dropdown_tag(user_dropdown_label(params[:author_id], 'Author'), options: { toggle_class: 'js-user-search js-filter-submit js-author-search', title: 'Filter by author', filter: true, filterInput: 'input#author-search', dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-author js-filter-submit', + placeholder: 'Search authors', data: { any_user: 'Any Author', first_user: (current_user.username if current_user), project_id: (@project.id if @project), selected: params[:author_id], field_name: 'author_id', default_label: 'Author', todo_filter: true, todo_state_filter: params[:state] || 'pending' } }) + .filter-item.inline + - if params[:type].present? + = hidden_field_tag(:type, params[:type]) + = dropdown_tag(todo_types_dropdown_label(params[:type], 'Type'), options: { toggle_class: 'js-type-search js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-type js-filter-submit', + data: { data: todo_types_options, default_label: 'Type' } }) + .filter-item.inline.actions-filter + - if params[:action_id].present? + = hidden_field_tag(:action_id, params[:action_id]) + = dropdown_tag(todo_actions_dropdown_label(params[:action_id], 'Action'), options: { toggle_class: 'js-action-search js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-action js-filter-submit', + data: { data: todo_actions_options, default_label: 'Action' } }) .filter-item.sort-filter .dropdown %button.dropdown-menu-toggle.dropdown-menu-toggle-sort{ type: 'button', 'data-toggle' => 'dropdown' } diff --git a/app/views/projects/clusters/_dropdown.html.haml b/app/views/projects/clusters/_dropdown.html.haml deleted file mode 100644 index d55a9c60b64..00000000000 --- a/app/views/projects/clusters/_dropdown.html.haml +++ /dev/null @@ -1,12 +0,0 @@ -%h4.prepend-top-0= s_('ClusterIntegration|Choose how to set up Kubernetes cluster integration') - -.dropdown.clusters-dropdown - %button.dropdown-menu-toggle.dropdown-menu-full-width{ type: 'button', data: { toggle: 'dropdown' }, 'aria-haspopup': true, 'aria-expanded': false } - %span.dropdown-toggle-text - = dropdown_text - = icon('chevron-down') - %ul.dropdown-menu.clusters-dropdown-menu.dropdown-menu-full-width - %li - = link_to(s_('ClusterIntegration|Create Kubernetes cluster on Google Kubernetes Engine'), gcp_new_namespace_project_clusters_path(@project.namespace, @project)) - %li - = link_to(s_('ClusterIntegration|Add an existing Kubernetes cluster'), user_new_namespace_project_clusters_path(@project.namespace, @project)) diff --git a/app/views/projects/clusters/gcp/_form.html.haml b/app/views/projects/clusters/gcp/_form.html.haml index b8e40b0a38b..0a2e320556d 100644 --- a/app/views/projects/clusters/gcp/_form.html.haml +++ b/app/views/projects/clusters/gcp/_form.html.haml @@ -10,8 +10,10 @@ - link_to_help_page = link_to(s_('ClusterIntegration|help page'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer') = s_('ClusterIntegration|Read our %{link_to_help_page} on Kubernetes cluster integration.').html_safe % { link_to_help_page: link_to_help_page} -= form_for @cluster, html: { class: 'js-gke-cluster-creation prepend-top-20', data: { token: token_in_session } }, url: gcp_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field| - = form_errors(@cluster) +%p= link_to('Select a different Google account', @authorize_url) + += form_for @gcp_cluster, html: { class: 'js-gke-cluster-creation prepend-top-20', data: { token: token_in_session } }, url: create_gcp_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field| + = form_errors(@gcp_cluster) .form-group = field.label :name, s_('ClusterIntegration|Kubernetes cluster name'), class: 'label-light' = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Kubernetes cluster name') @@ -19,7 +21,7 @@ = field.label :environment_scope, s_('ClusterIntegration|Environment scope'), class: 'label-light' = field.text_field :environment_scope, class: 'form-control', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope') - = field.fields_for :provider_gcp, @cluster.provider_gcp do |provider_gcp_field| + = field.fields_for :provider_gcp, @gcp_cluster.provider_gcp do |provider_gcp_field| .form-group = provider_gcp_field.label :gcp_project_id, s_('ClusterIntegration|Google Cloud Platform project'), class: 'label-light' .js-gcp-project-id-dropdown-entry-point{ data: { docsUrl: 'https://console.cloud.google.com/home/dashboard' } } diff --git a/app/views/projects/clusters/gcp/_header.html.haml b/app/views/projects/clusters/gcp/_header.html.haml index fa989943492..a2ad3cd64df 100644 --- a/app/views/projects/clusters/gcp/_header.html.haml +++ b/app/views/projects/clusters/gcp/_header.html.haml @@ -1,4 +1,4 @@ -%h4.prepend-top-20 +%h4 = s_('ClusterIntegration|Enter the details for your Kubernetes cluster') %p = s_('ClusterIntegration|Please make sure that your Google account meets the following requirements:') diff --git a/app/views/projects/clusters/gcp/login.html.haml b/app/views/projects/clusters/gcp/login.html.haml deleted file mode 100644 index 96c7a648676..00000000000 --- a/app/views/projects/clusters/gcp/login.html.haml +++ /dev/null @@ -1,21 +0,0 @@ -- breadcrumb_title 'Kubernetes' -- page_title _("Login") - -= render_gcp_signup_offer - -.row.prepend-top-default - .col-sm-4 - = render 'projects/clusters/sidebar' - .col-sm-8 - = render 'projects/clusters/dropdown', dropdown_text: s_('ClusterIntegration|Create Kubernetes cluster on Google Kubernetes Engine') - = render 'header' -.row - .col-sm-8.offset-sm-4.signin-with-google - - if @authorize_url - = link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px'), @authorize_url) - = _('or') - = link_to('create a new Google account', 'https://accounts.google.com/SignUpWithoutGmail?service=cloudconsole&continue=https%3A%2F%2Fconsole.cloud.google.com%2Ffreetrial%3Futm_campaign%3D2018_cpanel%26utm_source%3Dgitlab%26utm_medium%3Dreferral', target: '_blank', rel: 'noopener noreferrer') - - else - .settings-message.text-center - - link = link_to(s_('ClusterIntegration|properly configured'), help_page_path("integration/google"), target: '_blank', rel: 'noopener noreferrer') - = s_('Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service.').html_safe % { link_to_documentation: link } diff --git a/app/views/projects/clusters/gcp/new.html.haml b/app/views/projects/clusters/gcp/new.html.haml deleted file mode 100644 index ea78d66d883..00000000000 --- a/app/views/projects/clusters/gcp/new.html.haml +++ /dev/null @@ -1,10 +0,0 @@ -- breadcrumb_title 'Kubernetes' -- page_title _("New Kubernetes Cluster") - -.row.prepend-top-default - .col-sm-4 - = render 'projects/clusters/sidebar' - .col-sm-8 - = render 'projects/clusters/dropdown', dropdown_text: s_('ClusterIntegration|Create Kubernetes cluster on Google Kubernetes Engine') - = render 'header' - = render 'form' diff --git a/app/views/projects/clusters/new.html.haml b/app/views/projects/clusters/new.html.haml index 828e2a84753..a38003f5750 100644 --- a/app/views/projects/clusters/new.html.haml +++ b/app/views/projects/clusters/new.html.haml @@ -1,15 +1,36 @@ - breadcrumb_title 'Kubernetes' - page_title _("Kubernetes Cluster") +- active_tab = local_assigns.fetch(:active_tab, 'gcp') += javascript_include_tag 'https://apis.google.com/js/api.js' = render_gcp_signup_offer .row.prepend-top-default - .col-sm-4 + .col-md-3 = render 'sidebar' - .col-sm-8 - %h4.prepend-top-0= s_('ClusterIntegration|Choose how to set up Kubernetes cluster integration') + .col-md-9.js-toggle-container + %ul.nav-links.nav-tabs.gitlab-tabs.nav{ role: 'tablist' } + %li.nav-item{ role: 'presentation' } + %a.nav-link{ href: '#create-gcp-cluster-pane', id: 'create-gcp-cluster-tab', class: active_when(active_tab == 'gcp'), data: { toggle: 'tab' }, role: 'tab' } + %span Create new Cluster on GKE + %li.nav-item{ role: 'presentation' } + %a.nav-link{ href: '#add-user-cluster-pane', id: 'add-user-cluster-tab', class: active_when(active_tab == 'user'), data: { toggle: 'tab' }, role: 'tab' } + %span Add existing cluster - %p= s_('ClusterIntegration|Create a new Kubernetes cluster on Google Kubernetes Engine right from GitLab') - = link_to s_('ClusterIntegration|Create on Google Kubernetes Engine'), gcp_new_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' - %p= s_('ClusterIntegration|Enter the details for an existing Kubernetes cluster') - = link_to s_('ClusterIntegration|Add an existing Kubernetes cluster'), user_new_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' + .tab-content.gitlab-tab-content + .tab-pane{ id: 'create-gcp-cluster-pane', class: active_when(active_tab == 'gcp'), role: 'tabpanel' } + = render 'projects/clusters/gcp/header' + - if @valid_gcp_token + = render 'projects/clusters/gcp/form' + - elsif @authorize_url + .signin-with-google + = link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px'), @authorize_url) + = _('or') + = link_to('create a new Google account', 'https://accounts.google.com/SignUpWithoutGmail?service=cloudconsole&continue=https%3A%2F%2Fconsole.cloud.google.com%2Ffreetrial%3Futm_campaign%3D2018_cpanel%26utm_source%3Dgitlab%26utm_medium%3Dreferral', target: '_blank', rel: 'noopener noreferrer') + - else + - link = link_to(s_('ClusterIntegration|properly configured'), help_page_path("integration/google"), target: '_blank', rel: 'noopener noreferrer') + = s_('Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service.').html_safe % { link_to_documentation: link } + + .tab-pane{ id: 'add-user-cluster-pane', class: active_when(active_tab == 'user'), role: 'tabpanel' } + = render 'projects/clusters/user/header' + = render 'projects/clusters/user/form' diff --git a/app/views/projects/clusters/user/_form.html.haml b/app/views/projects/clusters/user/_form.html.haml index d45ae6ec91f..3006bb5073e 100644 --- a/app/views/projects/clusters/user/_form.html.haml +++ b/app/views/projects/clusters/user/_form.html.haml @@ -1,5 +1,5 @@ -= form_for @cluster, url: user_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field| - = form_errors(@cluster) += form_for @user_cluster, url: create_user_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field| + = form_errors(@user_cluster) .form-group = field.label :name, s_('ClusterIntegration|Kubernetes cluster name'), class: 'label-light' = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Kubernetes cluster name') @@ -8,7 +8,7 @@ = field.label :environment_scope, s_('ClusterIntegration|Environment scope'), class: 'label-light' = field.text_field :environment_scope, class: 'form-control', placeholder: s_('ClusterIntegration|Environment scope') - = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| + = field.fields_for :platform_kubernetes, @user_cluster.platform_kubernetes do |platform_kubernetes_field| .form-group = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL'), class: 'label-light' = platform_kubernetes_field.text_field :api_url, class: 'form-control', placeholder: s_('ClusterIntegration|API URL') diff --git a/app/views/projects/clusters/user/_header.html.haml b/app/views/projects/clusters/user/_header.html.haml index 37f6a788518..749177fa6c1 100644 --- a/app/views/projects/clusters/user/_header.html.haml +++ b/app/views/projects/clusters/user/_header.html.haml @@ -1,4 +1,4 @@ -%h4.prepend-top-20 +%h4 = s_('ClusterIntegration|Enter the details for your Kubernetes cluster') %p - link_to_help_page = link_to(s_('ClusterIntegration|documentation'), help_page_path('user/project/clusters/index', anchor: 'adding-an-existing-kubernetes-cluster'), target: '_blank', rel: 'noopener noreferrer') diff --git a/app/views/projects/clusters/user/new.html.haml b/app/views/projects/clusters/user/new.html.haml deleted file mode 100644 index 7fb75cd9cc7..00000000000 --- a/app/views/projects/clusters/user/new.html.haml +++ /dev/null @@ -1,11 +0,0 @@ -- breadcrumb_title 'Kubernetes' -- page_title _("New Kubernetes cluster") - -.row.prepend-top-default - .col-sm-4 - = render 'projects/clusters/sidebar' - .col-sm-8 - = render 'projects/clusters/dropdown', dropdown_text: s_('ClusterIntegration|Add an existing Kubernetes cluster') - = render 'header' - .prepend-top-20 - = render 'form' diff --git a/app/workers/project_cache_worker.rb b/app/workers/project_cache_worker.rb index abe86066fb4..b0e1d8837d9 100644 --- a/app/workers/project_cache_worker.rb +++ b/app/workers/project_cache_worker.rb @@ -3,7 +3,6 @@ # Worker for updating any project specific caches. class ProjectCacheWorker include ApplicationWorker - include ExclusiveLeaseGuard LEASE_TIMEOUT = 15.minutes.to_i @@ -14,30 +13,30 @@ class ProjectCacheWorker # statistics - An Array containing columns from ProjectStatistics to # refresh, if empty all columns will be refreshed def perform(project_id, files = [], statistics = []) - @project = Project.find_by(id: project_id) - return unless @project&.repository&.exists? + project = Project.find_by(id: project_id) - update_statistics(statistics) + return unless project && project.repository.exists? - @project.repository.refresh_method_caches(files.map(&:to_sym)) + update_statistics(project, statistics.map(&:to_sym)) - @project.cleanup + project.repository.refresh_method_caches(files.map(&:to_sym)) + + project.cleanup end - private + def update_statistics(project, statistics = []) + return unless try_obtain_lease_for(project.id, :update_statistics) - def update_statistics(statistics = []) - try_obtain_lease do - Rails.logger.info("Updating statistics for project #{@project.id}") - @project.statistics.refresh!(only: statistics.to_a.map(&:to_sym)) - end - end + Rails.logger.info("Updating statistics for project #{project.id}") - def lease_timeout - LEASE_TIMEOUT + project.statistics.refresh!(only: statistics) end - def lease_key - "project_cache_worker:#{@project.id}:update_statistics" + private + + def try_obtain_lease_for(project_id, section) + Gitlab::ExclusiveLease + .new("project_cache_worker:#{project_id}:#{section}", timeout: LEASE_TIMEOUT) + .try_obtain end end diff --git a/changelogs/unreleased/43446-new-cluster-page-tabs.yml b/changelogs/unreleased/43446-new-cluster-page-tabs.yml new file mode 100644 index 00000000000..e8c73257b16 --- /dev/null +++ b/changelogs/unreleased/43446-new-cluster-page-tabs.yml @@ -0,0 +1,5 @@ +--- +title: Create new or add existing Kubernetes cluster from a single page +merge_request: 18963 +author: +type: changed diff --git a/changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml b/changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml deleted file mode 100644 index bae6c2a8987..00000000000 --- a/changelogs/unreleased/44726-cancel_lease_upon_completion_in_project_cache_worker.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Cancel ExclusiveLease upon completion in ProjectCacheWorker -merge_request: 20103 -author: -type: fixed diff --git a/changelogs/unreleased/frozen-string-enable-app-validators.yml b/changelogs/unreleased/frozen-string-enable-app-validators.yml new file mode 100644 index 00000000000..db480b06d9b --- /dev/null +++ b/changelogs/unreleased/frozen-string-enable-app-validators.yml @@ -0,0 +1,5 @@ +--- +title: Enable frozen string in apps/validators/*.rb +merge_request: 20220 +author: gfyoung +type: other diff --git a/config/routes/project.rb b/config/routes/project.rb index b689b9800e6..a9cff7ff028 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -206,14 +206,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do resources :clusters, except: [:edit, :create] do collection do - scope :providers do - get '/user/new', to: 'clusters/user#new' - post '/user', to: 'clusters/user#create' - - get '/gcp/new', to: 'clusters/gcp#new' - get '/gcp/login', to: 'clusters/gcp#login' - post '/gcp', to: 'clusters/gcp#create' - end + post :create_gcp + post :create_user end member do diff --git a/db/migrate/20180608091413_add_group_to_todos.rb b/db/migrate/20180608091413_add_group_to_todos.rb new file mode 100644 index 00000000000..af3ee48b29d --- /dev/null +++ b/db/migrate/20180608091413_add_group_to_todos.rb @@ -0,0 +1,32 @@ +class AddGroupToTodos < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column :todos, :group_id, :integer + add_concurrent_foreign_key :todos, :namespaces, column: :group_id, on_delete: :cascade + add_concurrent_index :todos, :group_id + + change_column_null :todos, :project_id, true + end + + def down + return unless group_id_exists? + + remove_foreign_key :todos, column: :group_id + remove_index :todos, :group_id if index_exists?(:todos, :group_id) + remove_column :todos, :group_id + + execute "DELETE FROM todos WHERE project_id IS NULL" + change_column_null :todos, :project_id, false + end + + private + + def group_id_exists? + column_exists?(:todos, :group_id) + end +end diff --git a/db/schema.rb b/db/schema.rb index 5d917abdbb5..9a4e3fe5555 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1930,7 +1930,7 @@ ActiveRecord::Schema.define(version: 20180629191052) do create_table "todos", force: :cascade do |t| t.integer "user_id", null: false - t.integer "project_id", null: false + t.integer "project_id" t.integer "target_id" t.string "target_type", null: false t.integer "author_id", null: false @@ -1940,10 +1940,12 @@ ActiveRecord::Schema.define(version: 20180629191052) do t.datetime "updated_at" t.integer "note_id" t.string "commit_id" + t.integer "group_id" end add_index "todos", ["author_id"], name: "index_todos_on_author_id", using: :btree add_index "todos", ["commit_id"], name: "index_todos_on_commit_id", using: :btree + add_index "todos", ["group_id"], name: "index_todos_on_group_id", using: :btree add_index "todos", ["note_id"], name: "index_todos_on_note_id", using: :btree add_index "todos", ["project_id"], name: "index_todos_on_project_id", using: :btree add_index "todos", ["target_type", "target_id"], name: "index_todos_on_target_type_and_target_id", using: :btree @@ -2315,6 +2317,7 @@ ActiveRecord::Schema.define(version: 20180629191052) do add_foreign_key "term_agreements", "users", on_delete: :cascade add_foreign_key "timelogs", "issues", name: "fk_timelogs_issues_issue_id", on_delete: :cascade add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade + add_foreign_key "todos", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "todos", "notes", name: "fk_91d1f47b13", on_delete: :cascade add_foreign_key "todos", "projects", name: "fk_45054f9c45", on_delete: :cascade add_foreign_key "todos", "users", column: "author_id", name: "fk_ccf0373936", on_delete: :cascade diff --git a/doc/api/todos.md b/doc/api/todos.md index 27e623007cc..0843e4eedc6 100644 --- a/doc/api/todos.md +++ b/doc/api/todos.md @@ -18,6 +18,7 @@ Parameters: | `action` | string | no | The action to be filtered. Can be `assigned`, `mentioned`, `build_failed`, `marked`, `approval_required`, `unmergeable` or `directly_addressed`. | | `author_id` | integer | no | The ID of an author | | `project_id` | integer | no | The ID of a project | +| `group_id` | integer | no | The ID of a group | | `state` | string | no | The state of the todo. Can be either `pending` or `done` | | `type` | string | no | The type of a todo. Can be either `Issue` or `MergeRequest` | diff --git a/doc/workflow/todos.md b/doc/workflow/todos.md index 760cd87d4cc..dda82352c67 100644 --- a/doc/workflow/todos.md +++ b/doc/workflow/todos.md @@ -109,6 +109,7 @@ There are four kinds of filters you can use on your Todos dashboard. | Filter | Description | | ------- | ----------- | | Project | Filter by project | +| Group | Filter by group | | Author | Filter by the author that triggered the Todo | | Type | Filter by issue or merge request | | Action | Filter by the action that triggered the Todo | diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d04f92885a3..930b5ef37a3 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -769,28 +769,33 @@ module API class Todo < Grape::Entity expose :id - expose :project, using: Entities::BasicProjectDetails + expose :project, using: Entities::ProjectIdentity, if: -> (todo, _) { todo.project_id } + expose :group, using: 'API::Entities::NamespaceBasic', if: -> (todo, _) { todo.group_id } expose :author, using: Entities::UserBasic expose :action_name expose :target_type expose :target do |todo, options| - Entities.const_get(todo.target_type).represent(todo.target, options) + todo_target_class(todo.target_type).represent(todo.target, options) end expose :target_url do |todo, options| target_type = todo.target_type.underscore - target_url = "namespace_project_#{target_type}_url" + target_url = "#{todo.parent.class.to_s.underscore}_#{target_type}_url" target_anchor = "note_#{todo.note_id}" if todo.note_id? Gitlab::Routing .url_helpers - .public_send(target_url, todo.project.namespace, todo.project, todo.target, anchor: target_anchor) # rubocop:disable GitlabSecurity/PublicSend + .public_send(target_url, todo.parent, todo.target, anchor: target_anchor) # rubocop:disable GitlabSecurity/PublicSend end expose :body expose :state expose :created_at + + def todo_target_class(target_type) + ::API::Entities.const_get(target_type) + end end class NamespaceBasic < Grape::Entity diff --git a/qa/qa/page/project/operations/kubernetes/add.rb b/qa/qa/page/project/operations/kubernetes/add.rb index 9b3c482fa6c..11ebe10fb18 100644 --- a/qa/qa/page/project/operations/kubernetes/add.rb +++ b/qa/qa/page/project/operations/kubernetes/add.rb @@ -5,11 +5,11 @@ module QA module Kubernetes class Add < Page::Base view 'app/views/projects/clusters/new.html.haml' do - element :add_kubernetes_cluster_button, "link_to s_('ClusterIntegration|Add an existing Kubernetes cluster')" + element :add_existing_cluster_button, "Add existing cluster" end def add_existing_cluster - click_on 'Add an existing Kubernetes cluster' + click_on 'Add existing cluster' end end end diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb deleted file mode 100644 index 271ba37aed4..00000000000 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ /dev/null @@ -1,182 +0,0 @@ -require 'spec_helper' - -describe Projects::Clusters::GcpController do - include AccessMatchersForController - include GoogleApi::CloudPlatformHelpers - - set(:project) { create(:project) } - - describe 'GET login' do - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - context 'when omniauth has been configured' do - let(:key) { 'secret-key' } - let(:session_key_for_redirect_uri) do - GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(key) - end - - before do - allow(SecureRandom).to receive(:hex).and_return(key) - end - - it 'has authorize_url' do - go - - expect(assigns(:authorize_url)).to include(key) - expect(session[session_key_for_redirect_uri]).to eq(gcp_new_project_clusters_path(project)) - end - end - - context 'when omniauth has not configured' do - before do - stub_omniauth_setting(providers: []) - end - - it 'does not have authorize_url' do - go - - expect(assigns(:authorize_url)).to be_nil - end - end - end - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - get :login, namespace_id: project.namespace, project_id: project - end - end - - describe 'GET new' do - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - context 'when access token is valid' do - before do - stub_google_api_validate_token - end - - it 'has new object' do - go - - expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) - end - end - - context 'when access token is expired' do - before do - stub_google_api_expired_token - end - - it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } - end - - context 'when access token is not stored in session' do - it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } - end - end - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - get :new, namespace_id: project.namespace, project_id: project - end - end - - describe 'POST create' do - let(:params) do - { - cluster: { - name: 'new-cluster', - provider_gcp_attributes: { - gcp_project_id: '111' - } - } - } - end - - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - context 'when access token is valid' do - before do - stub_google_api_validate_token - end - - it 'creates a new cluster' do - expect(ClusterProvisionWorker).to receive(:perform_async) - expect { go }.to change { Clusters::Cluster.count } - .and change { Clusters::Providers::Gcp.count } - expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) - expect(project.clusters.first).to be_gcp - expect(project.clusters.first).to be_kubernetes - end - end - - context 'when access token is expired' do - before do - stub_google_api_expired_token - end - - it 'redirects to login page' do - expect(go).to redirect_to(gcp_login_project_clusters_path(project)) - end - end - - context 'when access token is not stored in session' do - it 'redirects to login page' do - expect(go).to redirect_to(gcp_login_project_clusters_path(project)) - end - end - end - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - post :create, params.merge(namespace_id: project.namespace, project_id: project) - end - end -end diff --git a/spec/controllers/projects/clusters/user_controller_spec.rb b/spec/controllers/projects/clusters/user_controller_spec.rb deleted file mode 100644 index 913976d187f..00000000000 --- a/spec/controllers/projects/clusters/user_controller_spec.rb +++ /dev/null @@ -1,89 +0,0 @@ -require 'spec_helper' - -describe Projects::Clusters::UserController do - include AccessMatchersForController - - set(:project) { create(:project) } - - describe 'GET new' do - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - it 'has new object' do - go - - expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) - end - end - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - get :new, namespace_id: project.namespace, project_id: project - end - end - - describe 'POST create' do - let(:params) do - { - cluster: { - name: 'new-cluster', - platform_kubernetes_attributes: { - api_url: 'http://my-url', - token: 'test', - namespace: 'aaa' - } - } - } - end - - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - context 'when creates a cluster' do - it 'creates a new cluster' do - expect(ClusterProvisionWorker).to receive(:perform_async) - expect { go }.to change { Clusters::Cluster.count } - .and change { Clusters::Platforms::Kubernetes.count } - expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) - expect(project.clusters.first).to be_user - expect(project.clusters.first).to be_kubernetes - end - end - end - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - post :create, params.merge(namespace_id: project.namespace, project_id: project) - end - end -end diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 380e50c8cac..e47ccdc9aea 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Projects::ClustersController do include AccessMatchersForController + include GoogleApi::CloudPlatformHelpers set(:project) { create(:project) } @@ -73,6 +74,231 @@ describe Projects::ClustersController do end end + describe 'GET new' do + describe 'functionality for new cluster' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'when omniauth has been configured' do + let(:key) { 'secret-key' } + let(:session_key_for_redirect_uri) do + GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(key) + end + + before do + allow(SecureRandom).to receive(:hex).and_return(key) + end + + it 'has authorize_url' do + go + + expect(assigns(:authorize_url)).to include(key) + expect(session[session_key_for_redirect_uri]).to eq(new_project_cluster_path(project)) + end + end + + context 'when omniauth has not configured' do + before do + stub_omniauth_setting(providers: []) + end + + it 'does not have authorize_url' do + go + + expect(assigns(:authorize_url)).to be_nil + end + end + + context 'when access token is valid' do + before do + stub_google_api_validate_token + end + + it 'has new object' do + go + + expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::Cluster) + end + end + + context 'when access token is expired' do + before do + stub_google_api_expired_token + end + + it { expect(@valid_gcp_token).to be_falsey } + end + + context 'when access token is not stored in session' do + it { expect(@valid_gcp_token).to be_falsey } + end + end + + describe 'functionality for existing cluster' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + it 'has new object' do + go + + expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::Cluster) + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + get :new, namespace_id: project.namespace, project_id: project + end + end + + describe 'POST create for new cluster' do + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_gcp_attributes: { + gcp_project_id: 'gcp-project-12345' + } + } + } + end + + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'when access token is valid' do + before do + stub_google_api_validate_token + end + + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { go }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Gcp.count } + expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) + expect(project.clusters.first).to be_gcp + expect(project.clusters.first).to be_kubernetes + end + end + + context 'when access token is expired' do + before do + stub_google_api_expired_token + end + + it { expect(@valid_gcp_token).to be_falsey } + end + + context 'when access token is not stored in session' do + it { expect(@valid_gcp_token).to be_falsey } + end + end + + describe 'security' do + before do + allow_any_instance_of(described_class) + .to receive(:token_in_session).and_return('token') + allow_any_instance_of(described_class) + .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s) + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_clusters_create) do + OpenStruct.new( + self_link: 'projects/gcp-project-12345/zones/us-central1-a/operations/ope-123', + status: 'RUNNING' + ) + end + + allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) + end + + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + post :create_gcp, params.merge(namespace_id: project.namespace, project_id: project) + end + end + + describe 'POST create for existing cluster' do + let(:params) do + { + cluster: { + name: 'new-cluster', + platform_kubernetes_attributes: { + api_url: 'http://my-url', + token: 'test', + namespace: 'aaa' + } + } + } + end + + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'when creates a cluster' do + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { go }.to change { Clusters::Cluster.count } + .and change { Clusters::Platforms::Kubernetes.count } + expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) + expect(project.clusters.first).to be_user + expect(project.clusters.first).to be_kubernetes + end + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + post :create_user, params.merge(namespace_id: project.namespace, project_id: project) + end + end + describe 'GET status' do let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb index 1ce7e84bef9..58f2817c7cc 100644 --- a/spec/controllers/projects/todos_controller_spec.rb +++ b/spec/controllers/projects/todos_controller_spec.rb @@ -5,10 +5,29 @@ describe Projects::TodosController do let(:project) { create(:project) } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, source_project: project) } + let(:parent) { project } + + shared_examples 'project todos actions' do + it_behaves_like 'todos actions' + + context 'when not authorized for resource' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + sign_in(user) + end + + it "doesn't create todo" do + expect { post_create }.not_to change { user.todos.count } + expect(response).to have_gitlab_http_status(404) + end + end + end context 'Issues' do describe 'POST create' do - def go + def post_create post :create, namespace_id: project.namespace, project_id: project, @@ -17,66 +36,13 @@ describe Projects::TodosController do format: 'html' end - context 'when authorized' do - before do - sign_in(user) - project.add_developer(user) - end - - it 'creates todo for issue' do - expect do - go - end.to change { user.todos.count }.by(1) - - expect(response).to have_gitlab_http_status(200) - end - - it 'returns todo path and pending count' do - go - - expect(response).to have_gitlab_http_status(200) - expect(json_response['count']).to eq 1 - expect(json_response['delete_path']).to match(%r{/dashboard/todos/\d{1}}) - end - end - - context 'when not authorized for project' do - it 'does not create todo for issue that user has no access to' do - sign_in(user) - expect do - go - end.to change { user.todos.count }.by(0) - - expect(response).to have_gitlab_http_status(404) - end - - it 'does not create todo for issue when user not logged in' do - expect do - go - end.to change { user.todos.count }.by(0) - - expect(response).to have_gitlab_http_status(302) - end - end - - context 'when not authorized for issue' do - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) - sign_in(user) - end - - it "doesn't create todo" do - expect { go }.not_to change { user.todos.count } - expect(response).to have_gitlab_http_status(404) - end - end + it_behaves_like 'project todos actions' end end context 'Merge Requests' do describe 'POST create' do - def go + def post_create post :create, namespace_id: project.namespace, project_id: project, @@ -85,60 +51,7 @@ describe Projects::TodosController do format: 'html' end - context 'when authorized' do - before do - sign_in(user) - project.add_developer(user) - end - - it 'creates todo for merge request' do - expect do - go - end.to change { user.todos.count }.by(1) - - expect(response).to have_gitlab_http_status(200) - end - - it 'returns todo path and pending count' do - go - - expect(response).to have_gitlab_http_status(200) - expect(json_response['count']).to eq 1 - expect(json_response['delete_path']).to match(%r{/dashboard/todos/\d{1}}) - end - end - - context 'when not authorized for project' do - it 'does not create todo for merge request user has no access to' do - sign_in(user) - expect do - go - end.to change { user.todos.count }.by(0) - - expect(response).to have_gitlab_http_status(404) - end - - it 'does not create todo for merge request user has no access to' do - expect do - go - end.to change { user.todos.count }.by(0) - - expect(response).to have_gitlab_http_status(302) - end - end - - context 'when not authorized for merge_request' do - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) - sign_in(user) - end - - it "doesn't create todo" do - expect { go }.not_to change { user.todos.count } - expect(response).to have_gitlab_http_status(404) - end - end + it_behaves_like 'project todos actions' end end end diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb index 94f8caedfa6..14486c80341 100644 --- a/spec/factories/todos.rb +++ b/spec/factories/todos.rb @@ -1,8 +1,8 @@ FactoryBot.define do factory :todo do project - author { project.creator } - user { project.creator } + author { project&.creator || user } + user { project&.creator || user } target factory: :issue action { Todo::ASSIGNED } diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 3db384e5b65..df21a42c3d3 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -16,9 +16,9 @@ feature 'Gcp Cluster', :js do let(:project_id) { 'test-project-1234' } before do - allow_any_instance_of(Projects::Clusters::GcpController) + allow_any_instance_of(Projects::ClustersController) .to receive(:token_in_session).and_return('token') - allow_any_instance_of(Projects::Clusters::GcpController) + allow_any_instance_of(Projects::ClustersController) .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s) end @@ -27,7 +27,7 @@ feature 'Gcp Cluster', :js do visit project_clusters_path(project) click_link 'Add Kubernetes cluster' - click_link 'Create on Google Kubernetes Engine' + click_link 'Create new Cluster on GKE' end context 'when user filled form with valid parameters' do @@ -148,7 +148,7 @@ feature 'Gcp Cluster', :js do visit project_clusters_path(project) click_link 'Add Kubernetes cluster' - click_link 'Create on Google Kubernetes Engine' + click_link 'Create new Cluster on GKE' end it 'user sees a login page' do @@ -187,7 +187,7 @@ feature 'Gcp Cluster', :js do it 'user sees offer on cluster GCP login page' do click_link 'Add Kubernetes cluster' - click_link 'Create on Google Kubernetes Engine' + click_link 'Create new Cluster on GKE' expect(page).to have_css('.gcp-signup-offer') end diff --git a/spec/features/projects/clusters/user_spec.rb b/spec/features/projects/clusters/user_spec.rb index 698b64a659c..766ea58cc17 100644 --- a/spec/features/projects/clusters/user_spec.rb +++ b/spec/features/projects/clusters/user_spec.rb @@ -17,7 +17,7 @@ feature 'User Cluster', :js do visit project_clusters_path(project) click_link 'Add Kubernetes cluster' - click_link 'Add an existing Kubernetes cluster' + click_link 'Add existing cluster' end context 'when user filled form with valid parameters' do diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index a251a2f4e52..64241102c8b 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -83,7 +83,7 @@ feature 'Clusters', :js do visit project_clusters_path(project) click_link 'Add Kubernetes cluster' - click_link 'Create on Google Kubernetes Engine' + click_link 'Create new Cluster on GKE' end it 'user sees a login page' do diff --git a/spec/finders/todos_finder_spec.rb b/spec/finders/todos_finder_spec.rb index 9747b9402a7..6061021d3b0 100644 --- a/spec/finders/todos_finder_spec.rb +++ b/spec/finders/todos_finder_spec.rb @@ -5,12 +5,76 @@ describe TodosFinder do let(:user) { create(:user) } let(:group) { create(:group) } let(:project) { create(:project, namespace: group) } + let(:issue) { create(:issue, project: project) } + let(:merge_request) { create(:merge_request, source_project: project) } let(:finder) { described_class } before do group.add_developer(user) end + describe '#execute' do + context 'visibility' do + let(:private_group_access) { create(:group, :private) } + let(:private_group_hidden) { create(:group, :private) } + let(:public_project) { create(:project, :public) } + let(:private_project_hidden) { create(:project) } + let(:public_group) { create(:group) } + + let!(:todo1) { create(:todo, user: user, project: project, group: nil) } + let!(:todo2) { create(:todo, user: user, project: public_project, group: nil) } + let!(:todo3) { create(:todo, user: user, project: private_project_hidden, group: nil) } + let!(:todo4) { create(:todo, user: user, project: nil, group: group) } + let!(:todo5) { create(:todo, user: user, project: nil, group: private_group_access) } + let!(:todo6) { create(:todo, user: user, project: nil, group: private_group_hidden) } + let!(:todo7) { create(:todo, user: user, project: nil, group: public_group) } + + before do + private_group_access.add_developer(user) + end + + it 'returns only todos with a target a user has access to' do + todos = finder.new(user).execute + + expect(todos).to match_array([todo1, todo2, todo4, todo5, todo7]) + end + end + + context 'filtering' do + let!(:todo1) { create(:todo, user: user, project: project, target: issue) } + let!(:todo2) { create(:todo, user: user, group: group, target: merge_request) } + + it 'returns correct todos when filtered by a project' do + todos = finder.new(user, { project_id: project.id }).execute + + expect(todos).to match_array([todo1]) + end + + it 'returns correct todos when filtered by a group' do + todos = finder.new(user, { group_id: group.id }).execute + + expect(todos).to match_array([todo1, todo2]) + end + + it 'returns correct todos when filtered by a type' do + todos = finder.new(user, { type: 'Issue' }).execute + + expect(todos).to match_array([todo1]) + end + + context 'with subgroups', :nested_groups do + let(:subgroup) { create(:group, parent: group) } + let!(:todo3) { create(:todo, user: user, group: subgroup, target: issue) } + + it 'returns todos from subgroups when filtered by a group' do + todos = finder.new(user, { group_id: group.id }).execute + + expect(todos).to match_array([todo1, todo2, todo3]) + end + end + end + end + describe '#sort' do context 'by date' do let!(:todo1) { create(:todo, user: user, project: project) } diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index cddb49b320f..d246beb9888 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -21,6 +21,27 @@ describe IssuablesHelper do end end + describe '#group_dropdown_label' do + let(:group) { create(:group) } + let(:default) { 'default label' } + + it 'returns default group label when group_id is nil' do + expect(group_dropdown_label(nil, default)).to eq('default label') + end + + it 'returns "any group" when group_id is 0' do + expect(group_dropdown_label('0', default)).to eq('Any group') + end + + it 'returns group full path when a group was found for the provided id' do + expect(group_dropdown_label(group.id, default)).to eq(group.full_name) + end + + it 'returns default label when a group was not found for the provided id' do + expect(group_dropdown_label(9999, default)).to eq('default label') + end + end + describe '#issuable_labels_tooltip' do it 'returns label text with no labels' do expect(issuable_labels_tooltip([])).to eq("Labels") diff --git a/spec/javascripts/sidebar/todo_spec.js b/spec/javascripts/sidebar/todo_spec.js new file mode 100644 index 00000000000..a929b804a29 --- /dev/null +++ b/spec/javascripts/sidebar/todo_spec.js @@ -0,0 +1,158 @@ +import Vue from 'vue'; + +import SidebarTodos from '~/sidebar/components/todo_toggle/todo.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; + +const createComponent = ({ + issuableId = 1, + issuableType = 'epic', + isTodo, + isActionActive, + collapsed, +}) => { + const Component = Vue.extend(SidebarTodos); + + return mountComponent(Component, { + issuableId, + issuableType, + isTodo, + isActionActive, + collapsed, + }); +}; + +describe('SidebarTodo', () => { + let vm; + + beforeEach(() => { + vm = createComponent({}); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('computed', () => { + describe('buttonClasses', () => { + it('returns todo button classes for when `collapsed` prop is `false`', () => { + expect(vm.buttonClasses).toBe('btn btn-default btn-todo issuable-header-btn float-right'); + }); + + it('returns todo button classes for when `collapsed` prop is `true`', done => { + vm.collapsed = true; + Vue.nextTick() + .then(() => { + expect(vm.buttonClasses).toBe('btn-blank btn-todo sidebar-collapsed-icon dont-change-state'); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('buttonLabel', () => { + it('returns todo button text for marking todo as done when `isTodo` prop is `true`', () => { + expect(vm.buttonLabel).toBe('Mark todo as done'); + }); + + it('returns todo button text for add todo when `isTodo` prop is `false`', done => { + vm.isTodo = false; + Vue.nextTick() + .then(() => { + expect(vm.buttonLabel).toBe('Add todo'); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('collapsedButtonIconClasses', () => { + it('returns collapsed button icon class when `isTodo` prop is `true`', () => { + expect(vm.collapsedButtonIconClasses).toBe('todo-undone'); + }); + + it('returns empty string when `isTodo` prop is `false`', done => { + vm.isTodo = false; + Vue.nextTick() + .then(() => { + expect(vm.collapsedButtonIconClasses).toBe(''); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('collapsedButtonIcon', () => { + it('returns button icon name when `isTodo` prop is `true`', () => { + expect(vm.collapsedButtonIcon).toBe('todo-done'); + }); + + it('returns button icon name when `isTodo` prop is `false`', done => { + vm.isTodo = false; + Vue.nextTick() + .then(() => { + expect(vm.collapsedButtonIcon).toBe('todo-add'); + }) + .then(done) + .catch(done.fail); + }); + }); + }); + + describe('methods', () => { + describe('handleButtonClick', () => { + it('emits `toggleTodo` event on component', () => { + spyOn(vm, '$emit'); + vm.handleButtonClick(); + expect(vm.$emit).toHaveBeenCalledWith('toggleTodo'); + }); + }); + }); + + describe('template', () => { + it('renders component container element', () => { + const dataAttributes = { + issuableId: '1', + issuableType: 'epic', + originalTitle: 'Mark todo as done', + placement: 'left', + container: 'body', + boundary: 'viewport', + }; + expect(vm.$el.nodeName).toBe('BUTTON'); + + const elDataAttrs = vm.$el.dataset; + Object.keys(elDataAttrs).forEach((attr) => { + expect(elDataAttrs[attr]).toBe(dataAttributes[attr]); + }); + }); + + it('renders button label element when `collapsed` prop is `false`', () => { + const buttonLabelEl = vm.$el.querySelector('span.issuable-todo-inner'); + expect(buttonLabelEl).not.toBeNull(); + expect(buttonLabelEl.innerText.trim()).toBe('Mark todo as done'); + }); + + it('renders button icon when `collapsed` prop is `true`', done => { + vm.collapsed = true; + Vue.nextTick() + .then(() => { + const buttonIconEl = vm.$el.querySelector('svg'); + expect(buttonIconEl).not.toBeNull(); + expect(buttonIconEl.querySelector('use').getAttribute('xlink:href')).toContain('todo-done'); + }) + .then(done) + .catch(done.fail); + }); + + it('renders loading icon when `isActionActive` prop is true', done => { + vm.isActionActive = true; + Vue.nextTick() + .then(() => { + const loadingEl = vm.$el.querySelector('span.loading-container'); + expect(loadingEl).not.toBeNull(); + }) + .then(done) + .catch(done.fail); + }); + }); +}); diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index bd498269798..f29abcf536e 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -7,6 +7,7 @@ describe Todo do it { is_expected.to belong_to(:author).class_name("User") } it { is_expected.to belong_to(:note) } it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:target).touch(true) } it { is_expected.to belong_to(:user) } end diff --git a/spec/support/shared_examples/controllers/todos_shared_examples.rb b/spec/support/shared_examples/controllers/todos_shared_examples.rb new file mode 100644 index 00000000000..bafd9bac8d0 --- /dev/null +++ b/spec/support/shared_examples/controllers/todos_shared_examples.rb @@ -0,0 +1,43 @@ +shared_examples 'todos actions' do + context 'when authorized' do + before do + sign_in(user) + parent.add_developer(user) + end + + it 'creates todo' do + expect do + post_create + end.to change { user.todos.count }.by(1) + + expect(response).to have_gitlab_http_status(200) + end + + it 'returns todo path and pending count' do + post_create + + expect(response).to have_gitlab_http_status(200) + expect(json_response['count']).to eq 1 + expect(json_response['delete_path']).to match(%r{/dashboard/todos/\d{1}}) + end + end + + context 'when not authorized for project/group' do + it 'does not create todo for resource that user has no access to' do + sign_in(user) + expect do + post_create + end.to change { user.todos.count }.by(0) + + expect(response).to have_gitlab_http_status(404) + end + + it 'does not create todo when user is not logged in' do + expect do + post_create + end.to change { user.todos.count }.by(0) + + expect(response).to have_gitlab_http_status(parent.is_a?(Group) ? 401 : 302) + end + end +end diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb index 8c4daac5f80..b9b5445562f 100644 --- a/spec/workers/project_cache_worker_spec.rb +++ b/spec/workers/project_cache_worker_spec.rb @@ -9,50 +9,44 @@ describe ProjectCacheWorker do let(:lease_key) { "project_cache_worker:#{project.id}:update_statistics" } let(:lease_timeout) { ProjectCacheWorker::LEASE_TIMEOUT } - before do - stub_exclusive_lease(lease_key, timeout: lease_timeout) - - allow(Project).to receive(:find_by) - .with(id: project.id) - .and_return(project) - end - describe '#perform' do - context 'with a non-existing project' do - it 'does not update statistic' do - allow(Project).to receive(:find_by).with(id: -1).and_return(nil) + before do + stub_exclusive_lease(lease_key, timeout: lease_timeout) + end - expect(subject).not_to receive(:update_statistics) + context 'with a non-existing project' do + it 'does nothing' do + expect(worker).not_to receive(:update_statistics) - subject.perform(-1) + worker.perform(-1) end end context 'with an existing project without a repository' do - it 'does not update statistics' do - allow(project.repository).to receive(:exists?).and_return(false) + it 'does nothing' do + allow_any_instance_of(Repository).to receive(:exists?).and_return(false) - expect(subject).not_to receive(:update_statistics) + expect(worker).not_to receive(:update_statistics) - subject.perform(project.id) + worker.perform(project.id) end end context 'with an existing project' do it 'updates the project statistics' do - expect(subject).to receive(:update_statistics) - .with(%w(repository_size)) - .and_call_original + expect(worker).to receive(:update_statistics) + .with(kind_of(Project), %i(repository_size)) + .and_call_original - subject.perform(project.id, [], %w(repository_size)) + worker.perform(project.id, [], %w(repository_size)) end it 'refreshes the method caches' do - expect(project.repository).to receive(:refresh_method_caches) - .with(%i(readme)) - .and_call_original + expect_any_instance_of(Repository).to receive(:refresh_method_caches) + .with(%i(readme)) + .and_call_original - subject.perform(project.id, %w(readme)) + worker.perform(project.id, %w(readme)) end context 'with plain readme' do @@ -60,22 +54,23 @@ describe ProjectCacheWorker do allow(MarkupHelper).to receive(:gitlab_markdown?).and_return(false) allow(MarkupHelper).to receive(:plain?).and_return(true) - expect(project.repository).to receive(:refresh_method_caches) - .with(%i(readme)) - .and_call_original - - subject.perform(project.id, %w(readme)) + expect_any_instance_of(Repository).to receive(:refresh_method_caches) + .with(%i(readme)) + .and_call_original + worker.perform(project.id, %w(readme)) end end end + end + describe '#update_statistics' do context 'when a lease could not be obtained' do it 'does not update the repository size' do stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) - expect(project.statistics).not_to receive(:refresh!) + expect(statistics).not_to receive(:refresh!) - subject.perform(project.id, [], %w(repository_size)) + worker.update_statistics(project) end end @@ -83,17 +78,11 @@ describe ProjectCacheWorker do it 'updates the project statistics' do stub_exclusive_lease(lease_key, timeout: lease_timeout) - expect(project.statistics).to receive(:refresh!) - .with(only: %i(repository_size)) - .and_call_original - - subject.perform(project.id, [], %i(repository_size)) - end - - it 'cancels the lease after statistics has been updated' do - expect(subject).to receive(:release_lease).with('uuid') + expect(statistics).to receive(:refresh!) + .with(only: %i(repository_size)) + .and_call_original - subject.perform(project.id, [], %i(repository_size)) + worker.update_statistics(project, %i(repository_size)) end end end |