From 4c788f43cbcd70bcceb4e40891d329952aa016d0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 28 May 2020 09:08:05 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .editorconfig | 3 + app/assets/javascripts/boards/models/list.js | 8 +- .../javascripts/boards/stores/boards_store.js | 9 + app/assets/javascripts/logs/constants.js | 7 + .../javascripts/logs/logs_tracking_helper.js | 18 + app/assets/javascripts/logs/stores/actions.js | 18 +- .../explorer/components/image_list.vue | 92 +--- .../explorer/components/image_list_row.vue | 136 ++++++ .../framework/broadcast_messages.scss | 1 - app/controllers/admin/users_controller.rb | 3 +- .../merge_requests/diffs_controller.rb | 11 +- .../merge_requests/drafts_controller.rb | 129 +++++ app/helpers/notes_helper.rb | 4 +- app/policies/draft_note_policy.rb | 13 + app/serializers/draft_note_entity.rb | 39 ++ app/serializers/draft_note_serializer.rb | 4 + app/services/users/build_service.rb | 3 +- app/views/admin/users/_admin_notes.html.haml | 7 + app/views/admin/users/_form.html.haml | 2 +- app/views/admin/users/_user_detail.html.haml | 2 +- .../admin/users/_user_detail_note.html.haml | 7 + .../admin/users/_user_listing_note.html.haml | 3 + app/views/admin/users/show.html.haml | 2 +- .../projects/commit/_commit_box.html.haml | 4 +- app/views/projects/diffs/_file.html.haml | 2 +- app/views/projects/pipelines/charts.html.haml | 2 +- app/workers/new_note_worker.rb | 7 +- ...ge-repository-list-view-of-the-contain.yml | 5 + .../218165-add-note-no-extend-ecs.yml | 5 + .../28154-move-controllers-outside-ee.yml | 5 + ...destroy-function-logic-from-list-model.yml | 5 + .../unreleased/fox-comment-icons-commits.yml | 5 + .../unreleased/jivanvl-add-snowplow-logs.yml | 5 + ...dular-broadcast-notification-close-btn.yml | 5 + changelogs/unreleased/notes-ee-feature.yml | 5 + config/routes/merge_requests.rb | 9 + .../geo/disaster_recovery/index.md | 6 + doc/administration/gitaly/praefect.md | 7 +- .../graphql/reference/gitlab_schema.graphql | 10 +- doc/api/graphql/reference/gitlab_schema.json | 112 ++--- doc/api/users.md | 12 +- doc/ci/cloud_deployment/index.md | 11 + doc/topics/autodevops/index.md | 9 + lib/api/entities/user_with_admin.rb | 3 +- lib/api/users.rb | 1 + .../templates/Jobs/Deploy/ECS.gitlab-ci.yml | 10 + lib/gitlab/gl_repository.rb | 17 +- lib/gitlab/gl_repository/identifier.rb | 74 +++ lib/gitlab/gl_repository/repo_type.rb | 27 +- .../quick_actions/merge_request_actions.rb | 17 + locale/gitlab.pot | 24 +- .../admin/users_controller_spec.rb | 26 + .../merge_requests/drafts_controller_spec.rb | 455 ++++++++++++++++++ spec/factories/notes.rb | 4 + spec/frontend/logs/stores/actions_spec.js | 73 ++- .../components/image_list_row_spec.js | 140 ++++++ .../explorer/components/image_list_spec.js | 43 +- .../gitlab/gl_repository/identifier_spec.rb | 82 ++++ .../gitlab/gl_repository/repo_type_spec.rb | 10 +- spec/lib/gitlab/gl_repository_spec.rb | 2 +- spec/requests/api/users_spec.rb | 171 +++++++ .../quick_actions/interpret_service_spec.rb | 23 + .../gitlab/gl_repository_shared_examples.rb | 12 + .../lib/gitlab/repo_type_shared_examples.rb | 20 - spec/workers/new_note_worker_spec.rb | 10 + spec/workers/post_receive_spec.rb | 2 +- 66 files changed, 1714 insertions(+), 284 deletions(-) create mode 100644 app/assets/javascripts/logs/logs_tracking_helper.js create mode 100644 app/assets/javascripts/registry/explorer/components/image_list_row.vue create mode 100644 app/controllers/projects/merge_requests/drafts_controller.rb create mode 100644 app/policies/draft_note_policy.rb create mode 100644 app/serializers/draft_note_entity.rb create mode 100644 app/serializers/draft_note_serializer.rb create mode 100644 app/views/admin/users/_admin_notes.html.haml create mode 100644 app/views/admin/users/_user_detail_note.html.haml create mode 100644 app/views/admin/users/_user_listing_note.html.haml create mode 100644 changelogs/unreleased/216757-include-tag-count-in-the-image-repository-list-view-of-the-contain.yml create mode 100644 changelogs/unreleased/218165-add-note-no-extend-ecs.yml create mode 100644 changelogs/unreleased/28154-move-controllers-outside-ee.yml create mode 100644 changelogs/unreleased/Remove-destroy-function-logic-from-list-model.yml create mode 100644 changelogs/unreleased/fox-comment-icons-commits.yml create mode 100644 changelogs/unreleased/jivanvl-add-snowplow-logs.yml create mode 100644 changelogs/unreleased/nicolasdular-broadcast-notification-close-btn.yml create mode 100644 changelogs/unreleased/notes-ee-feature.yml create mode 100644 lib/gitlab/gl_repository/identifier.rb create mode 100644 spec/controllers/projects/merge_requests/drafts_controller_spec.rb create mode 100644 spec/frontend/registry/explorer/components/image_list_row_spec.js create mode 100644 spec/lib/gitlab/gl_repository/identifier_spec.rb create mode 100644 spec/support/shared_examples/lib/gitlab/gl_repository_shared_examples.rb diff --git a/.editorconfig b/.editorconfig index d704f20c726..56ce6d87145 100644 --- a/.editorconfig +++ b/.editorconfig @@ -13,3 +13,6 @@ indent_size = 2 [*.{js,json,vue,scss,rb,haml,yml,md}] indent_style = space charset = utf-8 + +[*.{md,markdown}] +trim_trailing_whitespace = false diff --git a/app/assets/javascripts/boards/models/list.js b/app/assets/javascripts/boards/models/list.js index c2166de351b..bf5aee2a2a4 100644 --- a/app/assets/javascripts/boards/models/list.js +++ b/app/assets/javascripts/boards/models/list.js @@ -83,13 +83,7 @@ class List { } destroy() { - const index = boardsStore.state.lists.indexOf(this); - boardsStore.state.lists.splice(index, 1); - boardsStore.updateNewListDropdown(this.id); - - boardsStore.destroyList(this.id).catch(() => { - // TODO: handle request error - }); + boardsStore.destroy(this); } update() { diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 6f66bc5d9b7..cde7256a258 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -547,6 +547,15 @@ const boardsStore = { destroyList(id) { return axios.delete(`${this.state.endpoints.listsEndpoint}/${id}`); }, + destroy(list) { + const index = this.state.lists.indexOf(list); + this.state.lists.splice(index, 1); + this.updateNewListDropdown(list.id); + + this.destroyList(list.id).catch(() => { + // TODO: handle request error + }); + }, saveList(list) { const entity = list.label || list.assignee || list.milestone; diff --git a/app/assets/javascripts/logs/constants.js b/app/assets/javascripts/logs/constants.js index 51770aa7a1c..2bca1ecfd38 100644 --- a/app/assets/javascripts/logs/constants.js +++ b/app/assets/javascripts/logs/constants.js @@ -1,3 +1,10 @@ export const dateFormatMask = 'mmm dd HH:MM:ss.l'; export const TOKEN_TYPE_POD_NAME = 'TOKEN_TYPE_POD_NAME'; + +export const tracking = { + USED_SEARCH_BAR: 'used_search_bar', + POD_LOG_CHANGED: 'pod_log_changed', + TIME_RANGE_SET: 'time_range_set', + ENVIRONMENT_SELECTED: 'environment_selected', +}; diff --git a/app/assets/javascripts/logs/logs_tracking_helper.js b/app/assets/javascripts/logs/logs_tracking_helper.js new file mode 100644 index 00000000000..91b0392f71f --- /dev/null +++ b/app/assets/javascripts/logs/logs_tracking_helper.js @@ -0,0 +1,18 @@ +import Tracking from '~/tracking'; + +/** + * The value of 1 in count, means there was one action performed + * related to the tracked action, in either of the following categories + * 1. Refreshing the logs + * 2. Select an environment + * 3. Change the time range + * 4. Use the search bar + */ +const trackLogs = label => + Tracking.event(document.body.dataset.page, 'logs_view', { + label, + property: 'count', + value: 1, + }); + +export default trackLogs; diff --git a/app/assets/javascripts/logs/stores/actions.js b/app/assets/javascripts/logs/stores/actions.js index a86d3c775a9..79fde1c7f2b 100644 --- a/app/assets/javascripts/logs/stores/actions.js +++ b/app/assets/javascripts/logs/stores/actions.js @@ -2,7 +2,8 @@ import { backOff } from '~/lib/utils/common_utils'; import httpStatusCodes from '~/lib/utils/http_status'; import axios from '~/lib/utils/axios_utils'; import { convertToFixedRange } from '~/lib/utils/datetime_range'; -import { TOKEN_TYPE_POD_NAME } from '../constants'; +import { TOKEN_TYPE_POD_NAME, tracking } from '../constants'; +import trackLogs from '../logs_tracking_helper'; import * as types from './mutation_types'; @@ -81,22 +82,22 @@ export const showFilteredLogs = ({ dispatch, commit }, filters = []) => { commit(types.SET_CURRENT_POD_NAME, podName); commit(types.SET_SEARCH, search); - dispatch('fetchLogs'); + dispatch('fetchLogs', tracking.USED_SEARCH_BAR); }; export const showPodLogs = ({ dispatch, commit }, podName) => { commit(types.SET_CURRENT_POD_NAME, podName); - dispatch('fetchLogs'); + dispatch('fetchLogs', tracking.POD_LOG_CHANGED); }; export const setTimeRange = ({ dispatch, commit }, timeRange) => { commit(types.SET_TIME_RANGE, timeRange); - dispatch('fetchLogs'); + dispatch('fetchLogs', tracking.TIME_RANGE_SET); }; export const showEnvironment = ({ dispatch, commit }, environmentName) => { commit(types.SET_PROJECT_ENVIRONMENT, environmentName); - dispatch('fetchLogs'); + dispatch('fetchLogs', tracking.ENVIRONMENT_SELECTED); }; /** @@ -111,19 +112,22 @@ export const fetchEnvironments = ({ commit, dispatch }, environmentsPath) => { .get(environmentsPath) .then(({ data }) => { commit(types.RECEIVE_ENVIRONMENTS_DATA_SUCCESS, data.environments); - dispatch('fetchLogs'); + dispatch('fetchLogs', tracking.ENVIRONMENT_SELECTED); }) .catch(() => { commit(types.RECEIVE_ENVIRONMENTS_DATA_ERROR); }); }; -export const fetchLogs = ({ commit, state }) => { +export const fetchLogs = ({ commit, state }, trackingLabel) => { commit(types.REQUEST_LOGS_DATA); return requestLogsUntilData({ commit, state }) .then(({ data }) => { const { pod_name, pods, logs, cursor } = data; + if (logs && logs.length > 0) { + trackLogs(trackingLabel); + } commit(types.RECEIVE_LOGS_DATA_SUCCESS, { logs, cursor }); commit(types.SET_CURRENT_POD_NAME, pod_name); commit(types.RECEIVE_PODS_DATA_SUCCESS, pods); diff --git a/app/assets/javascripts/registry/explorer/components/image_list.vue b/app/assets/javascripts/registry/explorer/components/image_list.vue index bc209b12738..9d48769cbad 100644 --- a/app/assets/javascripts/registry/explorer/components/image_list.vue +++ b/app/assets/javascripts/registry/explorer/components/image_list.vue @@ -1,24 +1,12 @@ diff --git a/app/assets/javascripts/registry/explorer/components/image_list_row.vue b/app/assets/javascripts/registry/explorer/components/image_list_row.vue new file mode 100644 index 00000000000..1b631ca36de --- /dev/null +++ b/app/assets/javascripts/registry/explorer/components/image_list_row.vue @@ -0,0 +1,136 @@ + + + diff --git a/app/assets/stylesheets/framework/broadcast_messages.scss b/app/assets/stylesheets/framework/broadcast_messages.scss index 9903d10d27c..6b5d1794f9a 100644 --- a/app/assets/stylesheets/framework/broadcast_messages.scss +++ b/app/assets/stylesheets/framework/broadcast_messages.scss @@ -42,7 +42,6 @@ } .broadcast-message-dismiss { - height: 100%; color: $gray-800; } } diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index ee42baa8326..fc0acd8f99a 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -241,7 +241,8 @@ class Admin::UsersController < Admin::ApplicationController :theme_id, :twitter, :username, - :website_url + :website_url, + :note ] end diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 2331674f42c..1bf143c9a91 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -162,8 +162,13 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic def renderable_notes define_diff_comment_vars unless @notes - @notes + draft_notes = + if current_user + merge_request.draft_notes.authored_by(current_user) + else + [] + end + + @notes.concat(draft_notes) end end - -Projects::MergeRequests::DiffsController.prepend_if_ee('EE::Projects::MergeRequests::DiffsController') diff --git a/app/controllers/projects/merge_requests/drafts_controller.rb b/app/controllers/projects/merge_requests/drafts_controller.rb new file mode 100644 index 00000000000..f4846b1aa81 --- /dev/null +++ b/app/controllers/projects/merge_requests/drafts_controller.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +class Projects::MergeRequests::DraftsController < Projects::MergeRequests::ApplicationController + include Gitlab::Utils::StrongMemoize + + respond_to :json + + before_action :authorize_create_note!, only: [:create, :publish] + before_action :authorize_admin_draft!, only: [:update, :destroy] + before_action :authorize_admin_draft!, if: -> { action_name == 'publish' && params[:id].present? } + + def index + drafts = prepare_notes_for_rendering(draft_notes) + render json: DraftNoteSerializer.new(current_user: current_user).represent(drafts) + end + + def create + create_params = draft_note_params.merge(in_reply_to_discussion_id: params[:in_reply_to_discussion_id]) + create_service = DraftNotes::CreateService.new(merge_request, current_user, create_params) + + draft_note = create_service.execute + + prepare_notes_for_rendering(draft_note) + + render json: DraftNoteSerializer.new(current_user: current_user).represent(draft_note) + end + + def update + draft_note.update!(draft_note_params) + + prepare_notes_for_rendering(draft_note) + + render json: DraftNoteSerializer.new(current_user: current_user).represent(draft_note) + end + + def destroy + DraftNotes::DestroyService.new(merge_request, current_user).execute(draft_note) + + head :ok + end + + def publish + result = DraftNotes::PublishService.new(merge_request, current_user).execute(draft_note(allow_nil: true)) + + if result[:status] == :success + head :ok + else + render json: { message: result[:message] }, status: result[:status] + end + end + + def discard + DraftNotes::DestroyService.new(merge_request, current_user).execute + + head :ok + end + + private + + def draft_note(allow_nil: false) + strong_memoize(:draft_note) do + draft_notes.find(params[:id]) + end + rescue ActiveRecord::RecordNotFound => ex + # draft_note is allowed to be nil in #publish + raise ex unless allow_nil + end + + def draft_notes + return unless current_user + + strong_memoize(:draft_notes) do + merge_request.draft_notes.authored_by(current_user) + end + end + + # rubocop: disable CodeReuse/ActiveRecord + def merge_request + @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).find_by!(iid: params[:merge_request_id]) + end + # rubocop: enable CodeReuse/ActiveRecord + + def draft_note_params + params.require(:draft_note).permit( + :commit_id, + :note, + :position, + :resolve_discussion + ).tap do |h| + # Old FE version will still be sending `draft_note[commit_id]` as 'undefined'. + # That can result to having a note linked to a commit with 'undefined' ID + # which is non-existent. + h[:commit_id] = nil if h[:commit_id] == 'undefined' + end + end + + def prepare_notes_for_rendering(notes) + return [] unless notes + + notes = Array.wrap(notes) + + # Preload author and access-level information + DraftNote.preload_author(notes) + user_ids = notes.map(&:author_id) + project.team.max_member_access_for_user_ids(user_ids) + + notes.map(&method(:render_draft_note)) + end + + def render_draft_note(note) + params = { target_id: merge_request.id, target_type: 'MergeRequest', text: note.note } + result = PreviewMarkdownService.new(@project, current_user, params).execute + markdown_params = { markdown_engine: result[:markdown_engine], issuable_state_filter_enabled: true } + + note.rendered_note = view_context.markdown(result[:text], markdown_params) + note.users_referenced = result[:users] + note.commands_changes = view_context.markdown(result[:commands]) + + note + end + + def authorize_admin_draft! + access_denied! unless can?(current_user, :admin_note, draft_note) + end + + def authorize_create_note! + access_denied! unless can?(current_user, :create_note, merge_request) + end +end diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index bb9892ad596..3a752521e4b 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -61,8 +61,8 @@ module NotesHelper class: 'add-diff-note js-add-diff-note-button', type: 'submit', name: 'button', data: diff_view_line_data(line_code, position, line_type), - title: 'Add a comment to this line' do - icon('comment-o') + title: _('Add a comment to this line') do + sprite_icon('comment', size: 12) end end diff --git a/app/policies/draft_note_policy.rb b/app/policies/draft_note_policy.rb new file mode 100644 index 00000000000..be99d12c5f8 --- /dev/null +++ b/app/policies/draft_note_policy.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class DraftNotePolicy < BasePolicy + delegate { @subject.merge_request } + + condition(:is_author) { @user && @subject.author == @user } + + rule { is_author }.policy do + enable :read_note + enable :admin_note + enable :resolve_note + end +end diff --git a/app/serializers/draft_note_entity.rb b/app/serializers/draft_note_entity.rb new file mode 100644 index 00000000000..cab4849ebc9 --- /dev/null +++ b/app/serializers/draft_note_entity.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true +class DraftNoteEntity < Grape::Entity + include RequestAwareEntity + + expose :id + expose :author, using: NoteUserEntity + expose :merge_request_id + expose :position, if: -> (note, _) { note.on_diff? } + expose :line_code + expose :file_identifier_hash + expose :file_hash + expose :file_path + expose :note + expose :rendered_note, as: :note_html + expose :references + expose :discussion_id + expose :resolve_discussion + expose :noteable_type + + expose :current_user do + expose :can_edit do |note| + can?(current_user, :admin_note, note) + end + + expose :can_award_emoji do |note| + note.emoji_awardable? + end + + expose :can_resolve do |note| + note.resolvable? && can?(current_user, :resolve_note, note) + end + end + + private + + def current_user + request.current_user + end +end diff --git a/app/serializers/draft_note_serializer.rb b/app/serializers/draft_note_serializer.rb new file mode 100644 index 00000000000..282d7f9bdda --- /dev/null +++ b/app/serializers/draft_note_serializer.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true +class DraftNoteSerializer < BaseSerializer + entity DraftNoteEntity +end diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 3938d675596..f06f00a5c3f 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -82,7 +82,8 @@ module Users :organization, :location, :public_email, - :user_type + :user_type, + :note ] end diff --git a/app/views/admin/users/_admin_notes.html.haml b/app/views/admin/users/_admin_notes.html.haml new file mode 100644 index 00000000000..5d91ba1a1ca --- /dev/null +++ b/app/views/admin/users/_admin_notes.html.haml @@ -0,0 +1,7 @@ +%fieldset + %legend= _('Admin notes') + .form-group.row + .col-sm-2.col-form-label.text-right + = f.label :note, s_('AdminNote|Note') + .col-sm-10 + = f.text_area :note, class: 'form-control' diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml index 3281718071c..38c6c8b2a62 100644 --- a/app/views/admin/users/_form.html.haml +++ b/app/views/admin/users/_form.html.haml @@ -83,7 +83,7 @@ .col-sm-10 = f.text_field :website_url, class: 'form-control' - = render_if_exists 'admin/users/admin_notes', f: f + = render 'admin/users/admin_notes', f: f .form-actions - if @user.new_record? diff --git a/app/views/admin/users/_user_detail.html.haml b/app/views/admin/users/_user_detail.html.haml index a29f369b9de..3839231cb95 100644 --- a/app/views/admin/users/_user_detail.html.haml +++ b/app/views/admin/users/_user_detail.html.haml @@ -6,7 +6,7 @@ = image_tag avatar_icon_for_user(user), class: 'avatar s16 d-xs-flex d-md-none mr-1 gl-mt-2', alt: _('Avatar for %{name}') % { name: sanitize_name(user.name) } = link_to user.name, admin_user_path(user), class: 'text-plain js-user-link', data: { user_id: user.id, qa_selector: 'username_link' } - = render_if_exists 'admin/users/user_listing_note', user: user + = render 'admin/users/user_listing_note', user: user - user_badges_in_admin_section(user).each do |badge| - css_badge = "badge badge-#{badge[:variant]}" if badge[:variant].present? diff --git a/app/views/admin/users/_user_detail_note.html.haml b/app/views/admin/users/_user_detail_note.html.haml new file mode 100644 index 00000000000..4f2a682c5ca --- /dev/null +++ b/app/views/admin/users/_user_detail_note.html.haml @@ -0,0 +1,7 @@ +- if @user.note.present? + - text = @user.note + .card.border-info + .card-header.bg-info.text-white + = _('Admin Note') + .card-body + %p= text diff --git a/app/views/admin/users/_user_listing_note.html.haml b/app/views/admin/users/_user_listing_note.html.haml new file mode 100644 index 00000000000..df4af009c5c --- /dev/null +++ b/app/views/admin/users/_user_listing_note.html.haml @@ -0,0 +1,3 @@ +- if user.note.present? + %span.has-tooltip.user-note{ title: user.note } + = icon("sticky-note-o cgrey") diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index cd07fee8e59..fa707b73d3e 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -154,7 +154,7 @@ %br = link_to 'Confirm user', confirm_admin_user_path(@user), method: :put, class: "btn btn-info", data: { confirm: 'Are you sure?', qa_selector: 'confirm_user_button' } - = render_if_exists 'admin/users/user_detail_note' + = render 'admin/users/user_detail_note' - if @user.deactivated? .card.border-info diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 88d1ec54cb0..4442bdcdf1d 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -22,8 +22,8 @@ .header-action-buttons - if defined?(@notes_count) && @notes_count > 0 - %span.btn.disabled.btn-grouped.d-none.d-sm-block.append-right-10 - = icon('comment') + %span.btn.disabled.btn-grouped.d-none.d-sm-block.append-right-10.has-tooltip{ title: n_("%d comment on this commit", "%d comments on this commit", @notes_count) % @notes_count } + = sprite_icon('comment') = @notes_count = link_to project_tree_path(@project, @commit), class: "btn btn-default append-right-10 d-none d-sm-none d-md-inline" do #{ _('Browse files') } diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 855b719dc45..7395c16c38b 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -14,7 +14,7 @@ .file-actions.d-none.d-sm-block - if blob&.readable_text? = link_to '#', class: 'js-toggle-diff-comments btn active has-tooltip', title: _("Toggle comments for this file"), disabled: @diff_notes_disabled do - = icon('comment') + = sprite_icon('comment', size: 16) \ - if editable_diff?(diff_file) - link_opts = @merge_request.persisted? ? { from_merge_request_iid: @merge_request.iid } : {} diff --git a/app/views/projects/pipelines/charts.html.haml b/app/views/projects/pipelines/charts.html.haml index 7496ca97d56..55f1b9098c3 100644 --- a/app/views/projects/pipelines/charts.html.haml +++ b/app/views/projects/pipelines/charts.html.haml @@ -1,4 +1,4 @@ -- page_title _('CI / CD Charts') +- page_title _('CI / CD Analytics') #js-project-pipelines-charts-app{ data: { counts: @counts, success_ratio: success_ratio(@counts), times_chart: { labels: @charts[:pipeline_times].labels, values: @charts[:pipeline_times].pipeline_times }, diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb index ee1d2237001..b31311b0e44 100644 --- a/app/workers/new_note_worker.rb +++ b/app/workers/new_note_worker.rb @@ -19,14 +19,11 @@ class NewNoteWorker # rubocop:disable Scalability/IdempotentWorker Gitlab::AppLogger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job") end end + # rubocop: enable CodeReuse/ActiveRecord private - # EE-only method def skip_notification?(note) - false + note.review.present? end - # rubocop: enable CodeReuse/ActiveRecord end - -NewNoteWorker.prepend_if_ee('EE::NewNoteWorker') diff --git a/changelogs/unreleased/216757-include-tag-count-in-the-image-repository-list-view-of-the-contain.yml b/changelogs/unreleased/216757-include-tag-count-in-the-image-repository-list-view-of-the-contain.yml new file mode 100644 index 00000000000..c9b7a195002 --- /dev/null +++ b/changelogs/unreleased/216757-include-tag-count-in-the-image-repository-list-view-of-the-contain.yml @@ -0,0 +1,5 @@ +--- +title: Include tag count in the image repository list +merge_request: 33027 +author: +type: changed diff --git a/changelogs/unreleased/218165-add-note-no-extend-ecs.yml b/changelogs/unreleased/218165-add-note-no-extend-ecs.yml new file mode 100644 index 00000000000..7952028f97e --- /dev/null +++ b/changelogs/unreleased/218165-add-note-no-extend-ecs.yml @@ -0,0 +1,5 @@ +--- +title: Add note to ECS CI template +merge_request: 32597 +author: +type: added diff --git a/changelogs/unreleased/28154-move-controllers-outside-ee.yml b/changelogs/unreleased/28154-move-controllers-outside-ee.yml new file mode 100644 index 00000000000..3fa60392c65 --- /dev/null +++ b/changelogs/unreleased/28154-move-controllers-outside-ee.yml @@ -0,0 +1,5 @@ +--- +title: Move review related controllers/workers outside EE +merge_request: 32663 +author: +type: changed diff --git a/changelogs/unreleased/Remove-destroy-function-logic-from-list-model.yml b/changelogs/unreleased/Remove-destroy-function-logic-from-list-model.yml new file mode 100644 index 00000000000..2832db23bed --- /dev/null +++ b/changelogs/unreleased/Remove-destroy-function-logic-from-list-model.yml @@ -0,0 +1,5 @@ +--- +title: Remove destroy function logic from list model +merge_request: 32237 +author: nuwe1 +type: other diff --git a/changelogs/unreleased/fox-comment-icons-commits.yml b/changelogs/unreleased/fox-comment-icons-commits.yml new file mode 100644 index 00000000000..9eb198629ae --- /dev/null +++ b/changelogs/unreleased/fox-comment-icons-commits.yml @@ -0,0 +1,5 @@ +--- +title: Use sprites for comment icons on Commits +merge_request: 31696 +author: +type: changed diff --git a/changelogs/unreleased/jivanvl-add-snowplow-logs.yml b/changelogs/unreleased/jivanvl-add-snowplow-logs.yml new file mode 100644 index 00000000000..bc0d3592e74 --- /dev/null +++ b/changelogs/unreleased/jivanvl-add-snowplow-logs.yml @@ -0,0 +1,5 @@ +--- +title: Add snowplow tracking for logs page +merge_request: 32704 +author: +type: other diff --git a/changelogs/unreleased/nicolasdular-broadcast-notification-close-btn.yml b/changelogs/unreleased/nicolasdular-broadcast-notification-close-btn.yml new file mode 100644 index 00000000000..f7eb40584af --- /dev/null +++ b/changelogs/unreleased/nicolasdular-broadcast-notification-close-btn.yml @@ -0,0 +1,5 @@ +--- +title: Move broadcast notification dismiss button to the top +merge_request: 33174 +author: +type: changed diff --git a/changelogs/unreleased/notes-ee-feature.yml b/changelogs/unreleased/notes-ee-feature.yml new file mode 100644 index 00000000000..38d9471ce39 --- /dev/null +++ b/changelogs/unreleased/notes-ee-feature.yml @@ -0,0 +1,5 @@ +--- +title: Move Admin note feature to GitLab Core +merge_request: 31457 +author: Rajendra +type: added diff --git a/config/routes/merge_requests.rb b/config/routes/merge_requests.rb index f6c45081ce0..b2635a7fa74 100644 --- a/config/routes/merge_requests.rb +++ b/config/routes/merge_requests.rb @@ -55,6 +55,15 @@ resources :merge_requests, concerns: :awardable, except: [:new, :create, :show], delete :resolve, action: :unresolve end end + + scope module: :merge_requests do + resources :drafts, only: [:index, :update, :create, :destroy] do + collection do + post :publish + delete :discard + end + end + end end scope path: 'merge_requests', controller: 'merge_requests/creations' do diff --git a/doc/administration/geo/disaster_recovery/index.md b/doc/administration/geo/disaster_recovery/index.md index b7a02eab784..9095935e6dc 100644 --- a/doc/administration/geo/disaster_recovery/index.md +++ b/doc/administration/geo/disaster_recovery/index.md @@ -121,6 +121,12 @@ Note the following when promoting a secondary: gitlab-ctl promote-to-primary-node ``` + If you have already run the [preflight checks](planned_failover.md#preflight-checks), you can skip them with: + + ```shell + gitlab-ctl promote-to-primary-node --skip-preflight-check + ``` + 1. Verify you can connect to the newly promoted **primary** node using the URL used previously for the **secondary** node. 1. If successful, the **secondary** node has now been promoted to the **primary** node. diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md index 4582c720c0d..e5044d8874f 100644 --- a/doc/administration/gitaly/praefect.md +++ b/doc/administration/gitaly/praefect.md @@ -784,9 +784,7 @@ If the time frame is not specified, dead replication jobs from the last six hour sudo /opt/gitlab/embedded/bin/praefect -config /var/opt/gitlab/praefect/config.toml dataloss Failed replication jobs between [2020-01-02 00:00:00 +0000 UTC, 2020-01-02 06:00:00 +0000 UTC): -example/repository-1: 1 jobs -example/repository-2: 4 jobs -example/repository-3: 2 jobs +@hashed/fa/53/fa539965395b8382145f8370b34eab249cf610d2d6f2943c95b9b9d08a63d4a3.git: 2 jobs ``` To specify a time frame in UTC, run: @@ -797,7 +795,8 @@ sudo /opt/gitlab/embedded/bin/praefect -config /var/opt/gitlab/praefect/config.t ### Checking repository checksums -To check a project's checksums across all nodes, the Praefect replicas Rake task can be used: +To check a project's repository checksums across on all Gitaly nodes, the +replicas Rake task can be run on the main GitLab node: ```shell sudo gitlab-rake "gitlab:praefect:replicas[project_id]" diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 4ddd2d6a63c..8e881a124a6 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -8309,11 +8309,6 @@ type Project { """ after: String - """ - Filter requirements by author username - """ - authorUsername: [String!] - """ Returns the elements in the list that come before the specified cursor. """ @@ -8455,6 +8450,11 @@ type Project { """ after: String + """ + Filter requirements by author username + """ + authorUsername: [String!] + """ Returns the elements in the list that come before the specified cursor. """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 8d0192c17b0..6cebd565ae7 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -24398,34 +24398,6 @@ "ofType": null }, "defaultValue": null - }, - { - "name": "search", - "description": "Filter requirements by title search", - "type": { - "kind": "SCALAR", - "name": "String", - "ofType": null - }, - "defaultValue": null - }, - { - "name": "authorUsername", - "description": "Filter requirements by author username", - "type": { - "kind": "LIST", - "name": null, - "ofType": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "SCALAR", - "name": "String", - "ofType": null - } - } - }, - "defaultValue": null } ], "type": { @@ -24488,34 +24460,6 @@ }, "defaultValue": null }, - { - "name": "search", - "description": "Filter requirements by title search", - "type": { - "kind": "SCALAR", - "name": "String", - "ofType": null - }, - "defaultValue": null - }, - { - "name": "authorUsername", - "description": "Filter requirements by author username", - "type": { - "kind": "LIST", - "name": null, - "ofType": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "SCALAR", - "name": "String", - "ofType": null - } - } - }, - "defaultValue": null - }, { "name": "after", "description": "Returns the elements in the list that come after the specified cursor.", @@ -24766,6 +24710,34 @@ "ofType": null }, "defaultValue": null + }, + { + "name": "search", + "description": "Filter requirements by title search", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "authorUsername", + "description": "Filter requirements by author username", + "type": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + } + }, + "defaultValue": null } ], "type": { @@ -24842,6 +24814,34 @@ }, "defaultValue": null }, + { + "name": "search", + "description": "Filter requirements by title search", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "authorUsername", + "description": "Filter requirements by author username", + "type": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + } + }, + "defaultValue": null + }, { "name": "after", "description": "Returns the elements in the list that come after the specified cursor.", diff --git a/doc/api/users.md b/doc/api/users.md index b27e37c96a3..cb0d24d8566 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -104,6 +104,7 @@ GET /users "color_scheme_id": 2, "projects_limit": 100, "current_sign_in_at": "2012-06-02T06:36:55Z", + "note": "DMCA Request: 2018-11-05 | DMCA Violation | Abuse | https://gitlab.zendesk.com/agent/tickets/123", "identities": [ {"provider": "github", "extern_uid": "2435223452345"}, {"provider": "bitbucket", "extern_uid": "john.smith"}, @@ -154,7 +155,7 @@ GET /users ] ``` -Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) will also see the `shared_runners_minutes_limit`, `extra_shared_runners_minutes_limit`, and `note` parameters. +Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) will also see the `shared_runners_minutes_limit`, and `extra_shared_runners_minutes_limit` parameters. ```json [ @@ -163,7 +164,6 @@ Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) ... "shared_runners_minutes_limit": 133, "extra_shared_runners_minutes_limit": 133, - "note": "DMCA Request: 2018-11-05 | DMCA Violation | Abuse | https://gitlab.zendesk.com/agent/tickets/123", ... } ] @@ -296,6 +296,7 @@ Example Responses: "color_scheme_id": 2, "projects_limit": 100, "current_sign_in_at": "2012-06-02T06:36:55Z", + "note": "DMCA Request: 2018-11-05 | DMCA Violation | Abuse | https://gitlab.zendesk.com/agent/tickets/123", "identities": [ {"provider": "github", "extern_uid": "2435223452345"}, {"provider": "bitbucket", "extern_uid": "john.smith"}, @@ -316,7 +317,7 @@ Example Responses: NOTE: **Note:** The `plan` and `trial` parameters are only available on GitLab Enterprise Edition. Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) will also see -the `shared_runners_minutes_limit`, `extra_shared_runners_minutes_limit`, and `note` parameters. +the `shared_runners_minutes_limit`, and `extra_shared_runners_minutes_limit` parameters. ```json { @@ -324,7 +325,6 @@ the `shared_runners_minutes_limit`, `extra_shared_runners_minutes_limit`, and `n "username": "john_smith", "shared_runners_minutes_limit": 133, "extra_shared_runners_minutes_limit": 133, - "note": "DMCA Request: 2018-11-05 | DMCA Violation | Abuse | https://gitlab.zendesk.com/agent/tickets/123", ... } ``` @@ -338,7 +338,6 @@ see the `group_saml` option: "username": "john_smith", "shared_runners_minutes_limit": 133, "extra_shared_runners_minutes_limit": 133, - "note": "DMCA Request: 2018-11-05 | DMCA Violation | Abuse | https://gitlab.zendesk.com/agent/tickets/123", "identities": [ {"provider": "github", "extern_uid": "2435223452345"}, {"provider": "bitbucket", "extern_uid": "john.smith"}, @@ -391,6 +390,7 @@ Parameters: | `linkedin` | No | LinkedIn | | `location` | No | User's location | | `name` | Yes | Name | +| `note` | No | Admin notes for this user | | `organization` | No | Organization name | | `password` | No | Password | | `private_profile` | No | User's profile is private - true, false (default), or null (will be converted to false) | @@ -432,7 +432,7 @@ Parameters: | `linkedin` | No | LinkedIn | | `location` | No | User's location | | `name` | No | Name | -| `note` | No | Admin notes for this user **(STARTER)** | +| `note` | No | Admin notes for this user | | `organization` | No | Organization name | | `password` | No | Password | | `private_profile` | No | User's profile is private - true, false (default), or null (will be converted to false) | diff --git a/doc/ci/cloud_deployment/index.md b/doc/ci/cloud_deployment/index.md index 97f5b8f103b..e59846e0f12 100644 --- a/doc/ci/cloud_deployment/index.md +++ b/doc/ci/cloud_deployment/index.md @@ -123,6 +123,17 @@ After you're all set up on AWS ECS, follow these steps: task definition, making the cluster pull the newest version of your application. +CAUTION: **Warning:** +The [`Deploy-ECS.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Deploy-ECS.gitlab-ci.yml) +template includes both the [`Jobs/Build.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml) +and [`Jobs/Deploy/ECS.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Jobs/Deploy/ECS.gitlab-ci.yml) +"sub-templates". Do not include these "sub-templates" on their own, and only include the main +`Deploy-ECS.gitlab-ci.yml` template. The "sub-templates" are designed to only be +used along with the main template. They may move or change unexpectedly causing your +pipeline to fail if you didn't include the main template. Also, the job names within +these templates may change. Do not override these jobs names in your own pipeline, +as the override will stop working when the name changes. + Alternatively, if you don't wish to use the `Deploy-ECS.gitlab-ci.yml` template to deploy to AWS ECS, you can always use our `aws-base` Docker image to run your own [AWS CLI commands for ECS](https://docs.aws.amazon.com/cli/latest/reference/ecs/index.html#cli-aws-ecs). diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 1e3e3e6cd44..351619e59ca 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -232,6 +232,15 @@ NOTE: **Note:** If you have both a valid `AUTO_DEVOPS_PLATFORM_TARGET` variable and a Kubernetes cluster tied to your project, only the deployment to Kubernetes will run. +CAUTION: **Warning:** +Setting the `AUTO_DEVOPS_PLATFORM_TARGET` variable to `ECS` will trigger jobs +defined in the [`Jobs/Deploy/ECS.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Jobs/Deploy/ECS.gitlab-ci.yml). +However, it is not recommended to [include](../../ci/yaml/README.md#includetemplate) +it on its own. This template is designed to be used with Auto DevOps only. It may change +unexpectedly causing your pipeline to fail if included on its own. Also, the job +names within this template may also change. Do not override these jobs names in your +own pipeline, as the override will stop working when the name changes. + ## Auto DevOps base domain The Auto DevOps base domain is required to use diff --git a/lib/api/entities/user_with_admin.rb b/lib/api/entities/user_with_admin.rb index d3df12200ff..c225ade6eb6 100644 --- a/lib/api/entities/user_with_admin.rb +++ b/lib/api/entities/user_with_admin.rb @@ -4,8 +4,7 @@ module API module Entities class UserWithAdmin < UserPublic expose :admin?, as: :is_admin + expose :note end end end - -API::Entities::UserWithAdmin.prepend_if_ee('EE::API::Entities::UserWithAdmin', with_descendants: true) diff --git a/lib/api/users.rb b/lib/api/users.rb index c986414c223..8457ebb00ed 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -55,6 +55,7 @@ module API optional :theme_id, type: Integer, desc: 'The GitLab theme for the user' optional :color_scheme_id, type: Integer, desc: 'The color scheme for the file viewer' optional :private_profile, type: Boolean, desc: 'Flag indicating the user has a private profile' + optional :note, type: String, desc: 'Admin note for this user' all_or_none_of :extern_uid, :provider use :optional_params_ee diff --git a/lib/gitlab/ci/templates/Jobs/Deploy/ECS.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy/ECS.gitlab-ci.yml index 642f0ebeaf7..cba7746cf8d 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy/ECS.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy/ECS.gitlab-ci.yml @@ -1,3 +1,13 @@ +# WARNING (post-GitLab 13.0): +# +# This CI template should NOT be included in your own CI configuration files: +# 'review_ecs' and 'production_ecs' are two temporary names given to the jobs below. +# +# Should this template be included in your CI configuration, the upcoming name changes could +# then result in potentially breaking your future pipelines. +# +# More about including CI templates: https://docs.gitlab.com/ee/ci/yaml/#includetemplate + .deploy_to_ecs: image: 'registry.gitlab.com/gitlab-org/cloud-deploy/aws-ecs:latest' script: diff --git a/lib/gitlab/gl_repository.rb b/lib/gitlab/gl_repository.rb index 8abefad1ef3..b75b4be9a4e 100644 --- a/lib/gitlab/gl_repository.rb +++ b/lib/gitlab/gl_repository.rb @@ -12,14 +12,15 @@ module Gitlab WIKI = RepoType.new( name: :wiki, access_checker_class: Gitlab::GitAccessWiki, - repository_resolver: -> (project) { project&.wiki&.repository }, + repository_resolver: -> (container) { container&.wiki&.repository }, + project_resolver: -> (container) { container.is_a?(Project) ? container : nil }, suffix: :wiki ).freeze SNIPPET = RepoType.new( name: :snippet, access_checker_class: Gitlab::GitAccessSnippet, repository_resolver: -> (snippet) { snippet&.repository }, - container_resolver: -> (id) { Snippet.find_by_id(id) }, + container_class: Snippet, project_resolver: -> (snippet) { snippet&.project }, guest_read_ability: :read_snippet ).freeze @@ -42,16 +43,12 @@ module Gitlab end def self.parse(gl_repository) - type_name, _id = gl_repository.split('-').first - type = types[type_name] + result = ::Gitlab::GlRepository::Identifier.new(gl_repository) - unless type - raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\"" - end + repo_type = result.repo_type + container = result.fetch_container! - container = type.fetch_container!(gl_repository) - - [container, type.project_for(container), type] + [container, repo_type.project_for(container), repo_type] end def self.default_type diff --git a/lib/gitlab/gl_repository/identifier.rb b/lib/gitlab/gl_repository/identifier.rb new file mode 100644 index 00000000000..dc3e7931696 --- /dev/null +++ b/lib/gitlab/gl_repository/identifier.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module Gitlab + class GlRepository + class Identifier + attr_reader :gl_repository, :repo_type + + def initialize(gl_repository) + @gl_repository = gl_repository + @segments = gl_repository.split('-') + + raise_error if segments.size > 3 + + @repo_type = find_repo_type + @container_id = find_container_id + @container_class = find_container_class + end + + def fetch_container! + container_class.find_by_id(container_id) + end + + private + + attr_reader :segments, :container_class, :container_id + + def find_repo_type + type_name = three_segments_format? ? segments.last : segments.first + type = Gitlab::GlRepository.types[type_name] + + raise_error unless type + + type + end + + def find_container_class + if three_segments_format? + case segments[0] + when 'project' + Project + when 'group' + Group + else + raise_error + end + else + repo_type.container_class + end + end + + def find_container_id + id = Integer(segments[1], 10, exception: false) + + raise_error unless id + + id + end + + # gl_repository can either have 2 or 3 segments: + # "wiki-1" is the older 2-segment format, where container is implied. + # "group-1-wiki" is the newer 3-segment format, including container information. + # + # TODO: convert all 2-segment format to 3-segment: + # https://gitlab.com/gitlab-org/gitlab/-/issues/219192 + def three_segments_format? + segments.size == 3 + end + + def raise_error + raise ArgumentError, "Invalid GL Repository \"#{gl_repository}\"" + end + end + end +end diff --git a/lib/gitlab/gl_repository/repo_type.rb b/lib/gitlab/gl_repository/repo_type.rb index 64c329b15ae..2b482ee3d2d 100644 --- a/lib/gitlab/gl_repository/repo_type.rb +++ b/lib/gitlab/gl_repository/repo_type.rb @@ -6,7 +6,7 @@ module Gitlab attr_reader :name, :access_checker_class, :repository_resolver, - :container_resolver, + :container_class, :project_resolver, :guest_read_ability, :suffix @@ -15,36 +15,27 @@ module Gitlab name:, access_checker_class:, repository_resolver:, - container_resolver: default_container_resolver, + container_class: default_container_class, project_resolver: nil, guest_read_ability: :download_code, suffix: nil) @name = name @access_checker_class = access_checker_class @repository_resolver = repository_resolver - @container_resolver = container_resolver + @container_class = container_class @project_resolver = project_resolver @guest_read_ability = guest_read_ability @suffix = suffix end def identifier_for_container(container) + if container.is_a?(Group) + return "#{container.class.name.underscore}-#{container.id}-#{name}" + end + "#{name}-#{container.id}" end - def fetch_id(identifier) - match = /\A#{name}-(?\d+)\z/.match(identifier) - match[:id] if match - end - - def fetch_container!(identifier) - id = fetch_id(identifier) - - raise ArgumentError, "Invalid GL Repository \"#{identifier}\"" unless id - - container_resolver.call(id) - end - def wiki? self == WIKI end @@ -85,8 +76,8 @@ module Gitlab private - def default_container_resolver - -> (id) { Project.find_by_id(id) } + def default_container_class + Project end end end diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index 7c06698ffec..98db8ff761e 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -104,6 +104,23 @@ module Gitlab command :target_branch do |branch_name| @updates[:target_branch] = branch_name if project.repository.branch_exists?(branch_name) end + + desc _('Submit a review') + explanation _('Submit the current review.') + types MergeRequest + condition do + quick_action_target.persisted? + end + command :submit_review do + next if params[:review_id] + + result = DraftNotes::PublishService.new(quick_action_target, current_user).execute + @execution_message[:submit_review] = if result[:status] == :success + _('Submitted the current review.') + else + result[:message] + end + end end def merge_orchestration_service diff --git a/locale/gitlab.pot b/locale/gitlab.pot index dbafd0f050c..28c88b14686 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -96,6 +96,11 @@ msgid_plural "%d comments" msgstr[0] "" msgstr[1] "" +msgid "%d comment on this commit" +msgid_plural "%d comments on this commit" +msgstr[0] "" +msgstr[1] "" + msgid "%d commit" msgid_plural "%d commits" msgstr[0] "" @@ -1218,6 +1223,9 @@ msgstr "" msgid "Add a bullet list" msgstr "" +msgid "Add a comment to this line" +msgstr "" + msgid "Add a general comment to this %{noteableDisplayName}." msgstr "" @@ -1419,6 +1427,9 @@ msgstr "" msgid "Admin Area" msgstr "" +msgid "Admin Note" +msgstr "" + msgid "Admin Overview" msgstr "" @@ -3605,7 +3616,7 @@ msgstr "" msgid "CI / CD" msgstr "" -msgid "CI / CD Charts" +msgid "CI / CD Analytics" msgstr "" msgid "CI / CD Settings" @@ -5838,6 +5849,11 @@ msgid_plural "ContainerRegistry|%{count} Image repositories" msgstr[0] "" msgstr[1] "" +msgid "ContainerRegistry|%{count} Tag" +msgid_plural "ContainerRegistry|%{count} Tags" +msgstr[0] "" +msgstr[1] "" + msgid "ContainerRegistry|%{imageName} tags" msgstr "" @@ -6407,6 +6423,9 @@ msgstr "" msgid "Create issue" msgstr "" +msgid "Create iteration" +msgstr "" + msgid "Create lists from labels. Issues with that label appear in that list." msgstr "" @@ -23333,6 +23352,9 @@ msgstr "" msgid "Unable to resolve" msgstr "" +msgid "Unable to save iteration. Please try again" +msgstr "" + msgid "Unable to save your changes. Please try again." msgstr "" diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 7a7201a6454..1576f6abf5e 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -254,6 +254,18 @@ describe Admin::UsersController do errors = assigns[:user].errors expect(errors).to contain_exactly(errors.full_message(:email, I18n.t('errors.messages.invalid'))) end + + context 'admin notes' do + it 'creates the user with note' do + note = '2020-05-12 | Note | DCMA | Link' + user_params = attributes_for(:user, note: note) + + expect { post :create, params: { user: user_params } }.to change { User.count }.by(1) + + new_user = User.last + expect(new_user.note).to eq(note) + end + end end describe 'POST update' do @@ -338,6 +350,20 @@ describe Admin::UsersController do end end end + + context 'admin notes' do + it 'updates the note for the user' do + note = '2020-05-12 | Note | DCMA | Link' + params = { + id: user.to_param, + user: { + note: note + } + } + + expect { post :update, params: params }.to change { user.reload.note }.to(note) + end + end end describe "DELETE #remove_email" do diff --git a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb new file mode 100644 index 00000000000..067c111cb49 --- /dev/null +++ b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb @@ -0,0 +1,455 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Projects::MergeRequests::DraftsController do + include RepoHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } + let(:user) { project.owner } + let(:user2) { create(:user) } + + let(:params) do + { + namespace_id: project.namespace.to_param, + project_id: project.to_param, + merge_request_id: merge_request.iid + } + end + + before do + sign_in(user) + stub_licensed_features(multiple_merge_request_assignees: true) + stub_commonmark_sourcepos_disabled + end + + describe 'GET #index' do + let!(:draft_note) { create(:draft_note, merge_request: merge_request, author: user) } + + it 'list merge request draft notes for current user' do + get :index, params: params + + expect(json_response.first['merge_request_id']).to eq(merge_request.id) + expect(json_response.first['author']['id']).to eq(user.id) + expect(json_response.first['note_html']).not_to be_empty + end + end + + describe 'POST #create' do + def create_draft_note(draft_overrides: {}, overrides: {}) + post_params = params.merge({ + draft_note: { + note: 'This is a unpublished comment' + }.merge(draft_overrides) + }.merge(overrides)) + + post :create, params: post_params + end + + context 'without permissions' do + let(:project) { create(:project, :private) } + + before do + sign_in(user2) + end + + it 'does not allow draft note creation' do + expect { create_draft_note }.to change { DraftNote.count }.by(0) + expect(response).to have_gitlab_http_status(:not_found) + end + end + + it 'creates a draft note' do + expect { create_draft_note }.to change { DraftNote.count }.by(1) + end + + it 'creates draft note with position' do + diff_refs = project.commit(sample_commit.id).try(:diff_refs) + + position = Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: diff_refs + ) + + create_draft_note(draft_overrides: { position: position.to_json }) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['position']).to be_present + expect(json_response['file_hash']).to be_present + expect(json_response['file_identifier_hash']).to be_present + expect(json_response['line_code']).to match(/\w+_\d+_\d+/) + expect(json_response['note_html']).to eq('

This is a unpublished comment

') + end + + it 'creates a draft note with quick actions' do + create_draft_note(draft_overrides: { note: "#{user2.to_reference}\n/assign #{user.to_reference}" }) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['note_html']).to match(/#{user2.to_reference}/) + expect(json_response['references']['commands']).to match(/Assigns/) + expect(json_response['references']['users']).to include(user2.username) + end + + context 'in a thread' do + let(:discussion) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion } + + it 'creates draft note as a reply' do + expect do + create_draft_note(overrides: { in_reply_to_discussion_id: discussion.reply_id }) + end.to change { DraftNote.count }.by(1) + + draft_note = DraftNote.last + + expect(draft_note).to be_valid + expect(draft_note.discussion_id).to eq(discussion.reply_id) + end + + it 'creates a draft note that will resolve a thread' do + expect do + create_draft_note( + overrides: { in_reply_to_discussion_id: discussion.reply_id }, + draft_overrides: { resolve_discussion: true } + ) + end.to change { DraftNote.count }.by(1) + + draft_note = DraftNote.last + + expect(draft_note).to be_valid + expect(draft_note.discussion_id).to eq(discussion.reply_id) + expect(draft_note.resolve_discussion).to eq(true) + end + + it 'cannot create more than one draft note per thread' do + expect do + create_draft_note( + overrides: { in_reply_to_discussion_id: discussion.reply_id }, + draft_overrides: { resolve_discussion: true } + ) + end.to change { DraftNote.count }.by(1) + + expect do + create_draft_note( + overrides: { in_reply_to_discussion_id: discussion.reply_id }, + draft_overrides: { resolve_discussion: true, note: 'A note' } + ) + end.to change { DraftNote.count }.by(0) + end + end + + context 'commit_id is present' do + let(:commit) { project.commit(sample_commit.id) } + + let(:position) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: commit.diff_refs + ) + end + + before do + create_draft_note(draft_overrides: { commit_id: commit_id, position: position.to_json }) + end + + context 'value is a commit sha' do + let(:commit_id) { commit.id } + + it 'creates the draft note with commit ID' do + expect(DraftNote.last.commit_id).to eq(commit_id) + end + end + + context 'value is "undefined"' do + let(:commit_id) { 'undefined' } + + it 'creates the draft note with nil commit ID' do + expect(DraftNote.last.commit_id).to be_nil + end + end + end + end + + describe 'PUT #update' do + let(:draft) { create(:draft_note, merge_request: merge_request, author: user) } + + def update_draft_note(overrides = {}) + put_params = params.merge({ + id: draft.id, + draft_note: { + note: 'This is an updated unpublished comment' + }.merge(overrides) + }) + + put :update, params: put_params + end + + context 'without permissions' do + before do + sign_in(user2) + project.add_developer(user2) + end + + it 'does not allow editing draft note belonging to someone else' do + update_draft_note + + expect(response).to have_gitlab_http_status(:not_found) + expect(draft.reload.note).not_to eq('This is an updated unpublished comment') + end + end + + it 'updates the draft' do + expect(draft.note).not_to be_empty + + expect { update_draft_note }.not_to change { DraftNote.count } + + draft.reload + + expect(draft.note).to eq('This is an updated unpublished comment') + expect(json_response['note_html']).not_to be_empty + end + end + + describe 'POST #publish' do + context 'without permissions' do + shared_examples_for 'action that does not allow publishing draft note' do + it 'does not allow publishing draft note' do + expect { action } + .to not_change { Note.count } + .and not_change { DraftNote.count } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + before do + sign_in(user2) + end + + context 'when note belongs to someone else' do + before do + project.add_developer(user2) + end + + it_behaves_like 'action that does not allow publishing draft note' do + let!(:draft) { create(:draft_note, merge_request: merge_request, author: user) } + let(:action) { post :publish, params: params.merge(id: draft.id) } + end + end + + context 'when merge request discussion is locked' do + let(:project) { create(:project, :public, :merge_requests_public, :repository) } + + before do + create(:draft_note, merge_request: merge_request, author: user2) + merge_request.update!(discussion_locked: true) + end + + it_behaves_like 'action that does not allow publishing draft note' do + let(:action) { post :publish, params: params } + end + end + end + + context 'when PublishService errors' do + it 'returns message and 500 response' do + create(:draft_note, merge_request: merge_request, author: user) + error_message = "Something went wrong" + + expect_next_instance_of(DraftNotes::PublishService) do |service| + allow(service).to receive(:execute).and_return({ message: error_message, status: :error }) + end + + post :publish, params: params + + expect(response).to have_gitlab_http_status(:error) + expect(json_response["message"]).to include(error_message) + end + end + + it 'publishes draft notes with position' do + diff_refs = project.commit(sample_commit.id).try(:diff_refs) + + position = Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: diff_refs + ) + + draft = create(:draft_note_on_text_diff, merge_request: merge_request, author: user, position: position) + + expect { post :publish, params: params }.to change { Note.count }.by(1) + .and change { DraftNote.count }.by(-1) + + note = merge_request.notes.reload.last + + expect(note.note).to eq(draft.note) + expect(note.position).to eq(draft.position) + end + + it 'does nothing if there are no draft notes' do + expect { post :publish, params: params }.to change { Note.count }.by(0).and change { DraftNote.count }.by(0) + end + + it 'publishes a draft note with quick actions and applies them' do + project.add_developer(user2) + create(:draft_note, merge_request: merge_request, author: user, + note: "/assign #{user2.to_reference}") + + expect(merge_request.assignees).to be_empty + + expect { post :publish, params: params }.to change { Note.count }.by(1) + .and change { DraftNote.count }.by(-1) + + expect(response).to have_gitlab_http_status(:ok) + expect(merge_request.reload.assignee_ids).to match_array([user2.id]) + expect(Note.last.system?).to be true + end + + it 'publishes all draft notes for an MR' do + draft_params = { merge_request: merge_request, author: user } + + drafts = create_list(:draft_note, 4, draft_params) + + note = create(:discussion_note_on_merge_request, noteable: merge_request, project: project) + draft_reply = create(:draft_note, draft_params.merge(discussion_id: note.discussion_id)) + + diff_note = create(:diff_note_on_merge_request, noteable: merge_request, project: project) + diff_draft_reply = create(:draft_note, draft_params.merge(discussion_id: diff_note.discussion_id)) + + expect { post :publish, params: params }.to change { Note.count }.by(6) + .and change { DraftNote.count }.by(-6) + + expect(response).to have_gitlab_http_status(:ok) + + notes = merge_request.notes.reload + + expect(notes.pluck(:note)).to include(*drafts.map(&:note)) + expect(note.discussion.notes.last.note).to eq(draft_reply.note) + expect(diff_note.discussion.notes.last.note).to eq(diff_draft_reply.note) + end + + it 'can publish just a single draft note' do + draft_params = { merge_request: merge_request, author: user } + + drafts = create_list(:draft_note, 4, draft_params) + + expect { post :publish, params: params.merge(id: drafts.first.id) }.to change { Note.count }.by(1) + .and change { DraftNote.count }.by(-1) + end + + context 'when publishing drafts in a thread' do + let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + def create_reply(discussion_id, resolves: false) + create(:draft_note, + merge_request: merge_request, + author: user, + discussion_id: discussion_id, + resolve_discussion: resolves + ) + end + + it 'resolves a thread if the draft note resolves it' do + draft_reply = create_reply(note.discussion_id, resolves: true) + + post :publish, params: params + + discussion = note.discussion + + expect(discussion.notes.last.note).to eq(draft_reply.note) + expect(discussion.resolved?).to eq(true) + expect(discussion.resolved_by.id).to eq(user.id) + end + + it 'unresolves a thread if the draft note unresolves it' do + note.discussion.resolve!(user) + expect(note.discussion.resolved?).to eq(true) + + draft_reply = create_reply(note.discussion_id, resolves: false) + + post :publish, params: params + + discussion = note.discussion + + expect(discussion.notes.last.note).to eq(draft_reply.note) + expect(discussion.resolved?).to eq(false) + end + end + end + + describe 'DELETE #destroy' do + let(:draft) { create(:draft_note, merge_request: merge_request, author: user) } + + def create_draft + create(:draft_note, merge_request: merge_request, author: user) + end + + context 'without permissions' do + before do + sign_in(user2) + project.add_developer(user2) + end + + it 'does not allow destroying a draft note belonging to someone else' do + draft = create(:draft_note, merge_request: merge_request, author: user) + + expect { post :destroy, params: params.merge(id: draft.id) } + .not_to change { DraftNote.count } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + it 'destroys the draft note when ID is given' do + draft = create_draft + + expect { delete :destroy, params: params.merge(id: draft.id) }.to change { DraftNote.count }.by(-1) + expect(response).to have_gitlab_http_status(:ok) + end + + context 'without permissions' do + before do + sign_in(user2) + end + + it 'does not allow editing draft note belonging to someone else' do + draft = create_draft + + expect { delete :destroy, params: params.merge(id: draft.id) }.to change { DraftNote.count }.by(0) + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + describe 'DELETE #discard' do + it 'deletes all DraftNotes belonging to a user in a Merge Request' do + create_list(:draft_note, 6, merge_request: merge_request, author: user) + + expect { delete :discard, params: params }.to change { DraftNote.count }.by(-6) + expect(response).to have_gitlab_http_status(:ok) + end + + context 'without permissions' do + before do + sign_in(user2) + project.add_developer(user2) + end + + it 'does not destroys a draft note belonging to someone else' do + create(:draft_note, merge_request: merge_request, author: user) + + expect { post :discard, params: params } + .not_to change { DraftNote.count } + + expect(response).to have_gitlab_http_status(:ok) + end + end + end +end diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 7c3ba122b5a..a7d4b4eb57a 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -183,6 +183,10 @@ FactoryBot.define do confidential { true } end + trait :with_review do + review + end + transient do in_reply_to { nil } end diff --git a/spec/frontend/logs/stores/actions_spec.js b/spec/frontend/logs/stores/actions_spec.js index 6199c400e16..e2e3c3d23c6 100644 --- a/spec/frontend/logs/stores/actions_spec.js +++ b/spec/frontend/logs/stores/actions_spec.js @@ -1,6 +1,6 @@ import MockAdapter from 'axios-mock-adapter'; - import testAction from 'helpers/vuex_action_helper'; +import Tracking from '~/tracking'; import * as types from '~/logs/stores/mutation_types'; import { convertToFixedRange } from '~/lib/utils/datetime_range'; import logsPageState from '~/logs/stores/state'; @@ -104,7 +104,7 @@ describe('Logs Store actions', () => { { type: types.SET_CURRENT_POD_NAME, payload: null }, { type: types.SET_SEARCH, payload: '' }, ], - [{ type: 'fetchLogs' }], + [{ type: 'fetchLogs', payload: 'used_search_bar' }], )); it('text search should filter with a search term', () => @@ -116,7 +116,7 @@ describe('Logs Store actions', () => { { type: types.SET_CURRENT_POD_NAME, payload: null }, { type: types.SET_SEARCH, payload: mockSearch }, ], - [{ type: 'fetchLogs' }], + [{ type: 'fetchLogs', payload: 'used_search_bar' }], )); it('pod search should filter with a search term', () => @@ -128,7 +128,7 @@ describe('Logs Store actions', () => { { type: types.SET_CURRENT_POD_NAME, payload: mockPodName }, { type: types.SET_SEARCH, payload: '' }, ], - [{ type: 'fetchLogs' }], + [{ type: 'fetchLogs', payload: 'used_search_bar' }], )); it('pod search should filter with a pod selection and a search term', () => @@ -140,7 +140,7 @@ describe('Logs Store actions', () => { { type: types.SET_CURRENT_POD_NAME, payload: mockPodName }, { type: types.SET_SEARCH, payload: mockSearch }, ], - [{ type: 'fetchLogs' }], + [{ type: 'fetchLogs', payload: 'used_search_bar' }], )); it('pod search should filter with a pod selection and two search terms', () => @@ -152,7 +152,7 @@ describe('Logs Store actions', () => { { type: types.SET_CURRENT_POD_NAME, payload: null }, { type: types.SET_SEARCH, payload: `term1 term2` }, ], - [{ type: 'fetchLogs' }], + [{ type: 'fetchLogs', payload: 'used_search_bar' }], )); it('pod search should filter with a pod selection and a search terms before and after', () => @@ -168,7 +168,7 @@ describe('Logs Store actions', () => { { type: types.SET_CURRENT_POD_NAME, payload: mockPodName }, { type: types.SET_SEARCH, payload: `term1 term2` }, ], - [{ type: 'fetchLogs' }], + [{ type: 'fetchLogs', payload: 'used_search_bar' }], )); }); @@ -179,7 +179,7 @@ describe('Logs Store actions', () => { mockPodName, state, [{ type: types.SET_CURRENT_POD_NAME, payload: mockPodName }], - [{ type: 'fetchLogs' }], + [{ type: 'fetchLogs', payload: 'pod_log_changed' }], )); }); @@ -198,7 +198,7 @@ describe('Logs Store actions', () => { { type: types.REQUEST_ENVIRONMENTS_DATA }, { type: types.RECEIVE_ENVIRONMENTS_DATA_SUCCESS, payload: mockEnvironments }, ], - [{ type: 'fetchLogs' }], + [{ type: 'fetchLogs', payload: 'environment_selected' }], ); }); @@ -471,3 +471,58 @@ describe('Logs Store actions', () => { }); }); }); + +describe('Tracking user interaction', () => { + let commit; + let dispatch; + let state; + let mock; + + beforeEach(() => { + jest.spyOn(Tracking, 'event'); + commit = jest.fn(); + dispatch = jest.fn(); + state = logsPageState(); + state.environments.options = mockEnvironments; + state.environments.current = mockEnvName; + + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.reset(); + }); + + describe('Logs with data', () => { + beforeEach(() => { + mock.onGet(mockLogsEndpoint).reply(200, mockResponse); + mock.onGet(mockLogsEndpoint).replyOnce(202); // mock reactive cache + }); + + it('tracks fetched logs with data', () => { + return fetchLogs({ state, commit, dispatch }, 'environment_selected').then(() => { + expect(Tracking.event).toHaveBeenCalledWith(document.body.dataset.page, 'logs_view', { + label: 'environment_selected', + property: 'count', + value: 1, + }); + }); + }); + }); + + describe('Logs without data', () => { + beforeEach(() => { + mock.onGet(mockLogsEndpoint).reply(200, { + ...mockResponse, + logs: [], + }); + mock.onGet(mockLogsEndpoint).replyOnce(202); // mock reactive cache + }); + + it('does not track empty log responses', () => { + return fetchLogs({ state, commit, dispatch }).then(() => { + expect(Tracking.event).not.toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/spec/frontend/registry/explorer/components/image_list_row_spec.js b/spec/frontend/registry/explorer/components/image_list_row_spec.js new file mode 100644 index 00000000000..a6c5d485051 --- /dev/null +++ b/spec/frontend/registry/explorer/components/image_list_row_spec.js @@ -0,0 +1,140 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlIcon, GlSprintf } from '@gitlab/ui'; +import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; +import Component from '~/registry/explorer/components/image_list_row.vue'; +import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; +import { + ROW_SCHEDULED_FOR_DELETION, + LIST_DELETE_BUTTON_DISABLED, +} from '~/registry/explorer/constants'; +import { RouterLink } from '../stubs'; +import { imagesListResponse } from '../mock_data'; + +describe('Image List Row', () => { + let wrapper; + const item = imagesListResponse.data[0]; + const findDeleteBtn = () => wrapper.find('[data-testid="deleteImageButton"]'); + const findDetailsLink = () => wrapper.find('[data-testid="detailsLink"]'); + const findTagsCount = () => wrapper.find('[data-testid="tagsCount"]'); + const findDeleteButtonWrapper = () => wrapper.find('[data-testid="deleteButtonWrapper"]'); + const findClipboardButton = () => wrapper.find(ClipboardButton); + + const mountComponent = props => { + wrapper = shallowMount(Component, { + stubs: { + RouterLink, + GlSprintf, + }, + propsData: { + item, + ...props, + }, + directives: { + GlTooltip: createMockDirective(), + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('main tooltip', () => { + it(`the title is ${ROW_SCHEDULED_FOR_DELETION}`, () => { + mountComponent(); + const tooltip = getBinding(wrapper.element, 'gl-tooltip'); + expect(tooltip).toBeDefined(); + expect(tooltip.value.title).toBe(ROW_SCHEDULED_FOR_DELETION); + }); + + it('is disabled when item is being deleted', () => { + mountComponent({ item: { ...item, deleting: true } }); + const tooltip = getBinding(wrapper.element, 'gl-tooltip'); + expect(tooltip.value.disabled).toBe(false); + }); + }); + + describe('image title and path', () => { + it('contains a link to the details page', () => { + mountComponent(); + const link = findDetailsLink(); + expect(link.html()).toContain(item.path); + expect(link.props('to').name).toBe('details'); + }); + + it('contains a clipboard button', () => { + mountComponent(); + const button = findClipboardButton(); + expect(button.exists()).toBe(true); + expect(button.props('text')).toBe(item.location); + expect(button.props('title')).toBe(item.location); + }); + }); + + describe('delete button wrapper', () => { + it('has a tooltip', () => { + mountComponent(); + const tooltip = getBinding(findDeleteButtonWrapper().element, 'gl-tooltip'); + expect(tooltip).toBeDefined(); + expect(tooltip.value.title).toBe(LIST_DELETE_BUTTON_DISABLED); + }); + it('tooltip is enabled when destroy_path is falsy', () => { + mountComponent({ item: { ...item, destroy_path: null } }); + const tooltip = getBinding(findDeleteButtonWrapper().element, 'gl-tooltip'); + expect(tooltip.value.disabled).toBeFalsy(); + }); + }); + + describe('delete button', () => { + it('exists', () => { + mountComponent(); + expect(findDeleteBtn().exists()).toBe(true); + }); + + it('emits a delete event', () => { + mountComponent(); + findDeleteBtn().vm.$emit('click'); + expect(wrapper.emitted('delete')).toEqual([[item]]); + }); + + it.each` + destroy_path | deleting | state + ${null} | ${null} | ${'true'} + ${null} | ${true} | ${'true'} + ${'foo'} | ${true} | ${'true'} + ${'foo'} | ${false} | ${undefined} + `( + 'disabled is $state when destroy_path is $destroy_path and deleting is $deleting', + ({ destroy_path, deleting, state }) => { + mountComponent({ item: { ...item, destroy_path, deleting } }); + expect(findDeleteBtn().attributes('disabled')).toBe(state); + }, + ); + }); + + describe('tags count', () => { + it('exists', () => { + mountComponent(); + expect(findTagsCount().exists()).toBe(true); + }); + + it('contains a tag icon', () => { + mountComponent(); + const icon = findTagsCount().find(GlIcon); + expect(icon.exists()).toBe(true); + expect(icon.props('name')).toBe('tag'); + }); + + describe('tags count text', () => { + it('with one tag in the image', () => { + mountComponent({ item: { ...item, tags_count: 1 } }); + expect(findTagsCount().text()).toMatchInterpolatedText('1 Tag'); + }); + it('with more than one tag in the image', () => { + mountComponent({ item: { ...item, tags_count: 3 } }); + expect(findTagsCount().text()).toMatchInterpolatedText('3 Tags'); + }); + }); + }); +}); diff --git a/spec/frontend/registry/explorer/components/image_list_spec.js b/spec/frontend/registry/explorer/components/image_list_spec.js index 12f0fbe0c87..f849e60a749 100644 --- a/spec/frontend/registry/explorer/components/image_list_spec.js +++ b/spec/frontend/registry/explorer/components/image_list_spec.js @@ -1,26 +1,18 @@ import { shallowMount } from '@vue/test-utils'; import { GlPagination } from '@gitlab/ui'; import Component from '~/registry/explorer/components/image_list.vue'; -import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; -import { RouterLink } from '../stubs'; +import ImageListRow from '~/registry/explorer/components/image_list_row.vue'; + import { imagesListResponse, imagePagination } from '../mock_data'; describe('Image List', () => { let wrapper; - const firstElement = imagesListResponse.data[0]; - - const findDeleteBtn = () => wrapper.find('[data-testid="deleteImageButton"]'); - const findRowItems = () => wrapper.findAll('[data-testid="rowItem"]'); - const findDetailsLink = () => wrapper.find('[data-testid="detailsLink"]'); - const findClipboardButton = () => wrapper.find(ClipboardButton); + const findRow = () => wrapper.findAll(ImageListRow); const findPagination = () => wrapper.find(GlPagination); const mountComponent = () => { wrapper = shallowMount(Component, { - stubs: { - RouterLink, - }, propsData: { images: imagesListResponse.data, pagination: imagePagination, @@ -32,26 +24,17 @@ describe('Image List', () => { mountComponent(); }); - it('contains one list element for each image', () => { - expect(findRowItems().length).toBe(imagesListResponse.data.length); - }); + describe('list', () => { + it('contains one list element for each image', () => { + expect(findRow().length).toBe(imagesListResponse.data.length); + }); - it('contains a link to the details page', () => { - const link = findDetailsLink(); - expect(link.html()).toContain(firstElement.path); - expect(link.props('to').name).toBe('details'); - }); - - it('contains a clipboard button', () => { - const button = findClipboardButton(); - expect(button.exists()).toBe(true); - expect(button.props('text')).toBe(firstElement.location); - expect(button.props('title')).toBe(firstElement.location); - }); - - it('should be possible to delete a repo', () => { - const deleteBtn = findDeleteBtn(); - expect(deleteBtn.exists()).toBe(true); + it('when delete event is emitted on the row it emits up a delete event', () => { + findRow() + .at(0) + .vm.$emit('delete', 'foo'); + expect(wrapper.emitted('delete')).toEqual([['foo']]); + }); }); describe('pagination', () => { diff --git a/spec/lib/gitlab/gl_repository/identifier_spec.rb b/spec/lib/gitlab/gl_repository/identifier_spec.rb new file mode 100644 index 00000000000..c36f296702e --- /dev/null +++ b/spec/lib/gitlab/gl_repository/identifier_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::GlRepository::Identifier do + let_it_be(:project) { create(:project) } + let_it_be(:personal_snippet) { create(:personal_snippet, author: project.owner) } + let_it_be(:project_snippet) { create(:project_snippet, project: project, author: project.owner) } + + describe 'project repository' do + it_behaves_like 'parsing gl_repository identifier' do + let(:record_id) { project.id } + let(:identifier) { "project-#{record_id}" } + let(:expected_container) { project } + let(:expected_type) { Gitlab::GlRepository::PROJECT } + end + end + + describe 'wiki' do + it_behaves_like 'parsing gl_repository identifier' do + let(:record_id) { project.id } + let(:identifier) { "wiki-#{record_id}" } + let(:expected_container) { project } + let(:expected_type) { Gitlab::GlRepository::WIKI } + end + end + + describe 'snippet' do + context 'when PersonalSnippet' do + it_behaves_like 'parsing gl_repository identifier' do + let(:record_id) { personal_snippet.id } + let(:identifier) { "snippet-#{record_id}" } + let(:expected_container) { personal_snippet } + let(:expected_type) { Gitlab::GlRepository::SNIPPET } + end + end + + context 'when ProjectSnippet' do + it_behaves_like 'parsing gl_repository identifier' do + let(:record_id) { project_snippet.id } + let(:identifier) { "snippet-#{record_id}" } + let(:expected_container) { project_snippet } + let(:expected_type) { Gitlab::GlRepository::SNIPPET } + end + end + end + + describe 'design' do + it_behaves_like 'parsing gl_repository identifier' do + let(:record_id) { project.id } + let(:identifier) { "design-#{project.id}" } + let(:expected_container) { project } + let(:expected_type) { Gitlab::GlRepository::DESIGN } + end + end + + describe 'incorrect format' do + def expect_error_raised_for(identifier) + expect { described_class.new(identifier) }.to raise_error(ArgumentError) + end + + it 'raises error for incorrect id' do + expect_error_raised_for('wiki-noid') + end + + it 'raises error for incorrect type' do + expect_error_raised_for('foo-2') + end + + it 'raises error for incorrect three-segment container' do + expect_error_raised_for('snippet-2-wiki') + end + + it 'raises error for one segment' do + expect_error_raised_for('snippet') + end + + it 'raises error for more than three segments' do + expect_error_raised_for('project-1-wiki-bar') + end + end +end diff --git a/spec/lib/gitlab/gl_repository/repo_type_spec.rb b/spec/lib/gitlab/gl_repository/repo_type_spec.rb index bf6df55b71e..f5270104d2f 100644 --- a/spec/lib/gitlab/gl_repository/repo_type_spec.rb +++ b/spec/lib/gitlab/gl_repository/repo_type_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::GlRepository::RepoType do describe Gitlab::GlRepository::PROJECT do it_behaves_like 'a repo type' do - let(:expected_id) { project.id.to_s } + let(:expected_id) { project.id } let(:expected_identifier) { "project-#{expected_id}" } let(:expected_suffix) { '' } let(:expected_container) { project } @@ -42,7 +42,7 @@ describe Gitlab::GlRepository::RepoType do describe Gitlab::GlRepository::WIKI do it_behaves_like 'a repo type' do - let(:expected_id) { project.id.to_s } + let(:expected_id) { project.id } let(:expected_identifier) { "wiki-#{expected_id}" } let(:expected_suffix) { '.wiki' } let(:expected_container) { project } @@ -72,7 +72,7 @@ describe Gitlab::GlRepository::RepoType do describe Gitlab::GlRepository::SNIPPET do context 'when PersonalSnippet' do it_behaves_like 'a repo type' do - let(:expected_id) { personal_snippet.id.to_s } + let(:expected_id) { personal_snippet.id } let(:expected_identifier) { "snippet-#{expected_id}" } let(:expected_suffix) { '' } let(:expected_repository) { personal_snippet.repository } @@ -101,7 +101,7 @@ describe Gitlab::GlRepository::RepoType do context 'when ProjectSnippet' do it_behaves_like 'a repo type' do - let(:expected_id) { project_snippet.id.to_s } + let(:expected_id) { project_snippet.id } let(:expected_identifier) { "snippet-#{expected_id}" } let(:expected_suffix) { '' } let(:expected_repository) { project_snippet.repository } @@ -131,7 +131,7 @@ describe Gitlab::GlRepository::RepoType do describe Gitlab::GlRepository::DESIGN do it_behaves_like 'a repo type' do let(:expected_identifier) { "design-#{project.id}" } - let(:expected_id) { project.id.to_s } + let(:expected_id) { project.id } let(:expected_suffix) { '.design' } let(:expected_repository) { project.design_repository } let(:expected_container) { project } diff --git a/spec/lib/gitlab/gl_repository_spec.rb b/spec/lib/gitlab/gl_repository_spec.rb index 5f5244b7116..413540b4db8 100644 --- a/spec/lib/gitlab/gl_repository_spec.rb +++ b/spec/lib/gitlab/gl_repository_spec.rb @@ -11,7 +11,7 @@ describe ::Gitlab::GlRepository do expect(described_class.parse("project-#{project.id}")).to eq([project, project, Gitlab::GlRepository::PROJECT]) end - it 'parses a wiki gl_repository' do + it 'parses a project wiki gl_repository' do expect(described_class.parse("wiki-#{project.id}")).to eq([project, project, Gitlab::GlRepository::WIKI]) end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 4a0f0eea088..05d2e4bb5a2 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -15,6 +15,177 @@ describe API::Users, :do_not_mock_admin_mode do let(:not_existing_pat_id) { (PersonalAccessToken.maximum('id') || 0 ) + 10 } let(:private_user) { create(:user, private_profile: true) } + context 'admin notes' do + let(:admin) { create(:admin, note: '2019-10-06 | 2FA added | user requested | www.gitlab.com') } + let(:user) { create(:user, note: '2018-11-05 | 2FA removed | user requested | www.gitlab.com') } + + describe 'POST /users' do + context 'when unauthenticated' do + it 'return authentication error' do + post api('/users') + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authenticated' do + context 'as an admin' do + it 'contains the note of the user' do + optional_attributes = { note: 'Awesome Note' } + attributes = attributes_for(:user).merge(optional_attributes) + + post api('/users', admin), params: attributes + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['note']).to eq(optional_attributes[:note]) + end + end + + context 'as a regular user' do + it 'does not allow creating new user' do + post api('/users', user), params: attributes_for(:user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + describe 'GET /users/:id' do + context 'when unauthenticated' do + it 'does not contain the note of the user' do + get api("/users/#{user.id}") + + expect(json_response).not_to have_key('note') + end + end + + context 'when authenticated' do + context 'as an admin' do + it 'contains the note of the user' do + get api("/users/#{user.id}", admin) + + expect(json_response).to have_key('note') + expect(json_response['note']).to eq(user.note) + end + end + + context 'as a regular user' do + it 'does not contain the note of the user' do + get api("/users/#{user.id}", user) + + expect(json_response).not_to have_key('note') + end + end + end + end + + describe "PUT /users/:id" do + context 'when user is an admin' do + it "updates note of the user" do + new_note = '2019-07-07 | Email changed | user requested | www.gitlab.com' + + expect do + put api("/users/#{user.id}", admin), params: { note: new_note } + end.to change { user.reload.note } + .from('2018-11-05 | 2FA removed | user requested | www.gitlab.com') + .to(new_note) + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['note']).to eq(new_note) + end + end + + context 'when user is not an admin' do + it "cannot update their own note" do + expect do + put api("/users/#{user.id}", user), params: { note: 'new note' } + end.not_to change { user.reload.note } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + + describe 'GET /users/' do + context 'when unauthenticated' do + it "does not contain the note of users" do + get api("/users"), params: { username: user.username } + + expect(json_response.first).not_to have_key('note') + end + end + + context 'when authenticated' do + context 'as a regular user' do + it 'does not contain the note of users' do + get api("/users", user), params: { username: user.username } + + expect(json_response.first).not_to have_key('note') + end + end + + context 'as an admin' do + it 'contains the note of users' do + get api("/users", admin), params: { username: user.username } + + expect(response).to have_gitlab_http_status(:success) + expect(json_response.first).to have_key('note') + expect(json_response.first['note']).to eq '2018-11-05 | 2FA removed | user requested | www.gitlab.com' + end + end + end + end + + describe 'GET /user' do + context 'when authenticated' do + context 'as an admin' do + context 'accesses their own profile' do + it 'contains the note of the user' do + get api("/user", admin) + + expect(json_response).to have_key('note') + expect(json_response['note']).to eq(admin.note) + end + end + + context 'sudo' do + let(:admin_personal_access_token) { create(:personal_access_token, user: admin, scopes: %w[api sudo]).token } + + context 'accesses the profile of another regular user' do + it 'does not contain the note of the user' do + get api("/user?private_token=#{admin_personal_access_token}&sudo=#{user.id}") + + expect(json_response['id']).to eq(user.id) + expect(json_response).not_to have_key('note') + end + end + + context 'accesses the profile of another admin' do + let(:admin_2) {create(:admin, note: '2010-10-10 | 2FA added | admin requested | www.gitlab.com')} + + it 'contains the note of the user' do + get api("/user?private_token=#{admin_personal_access_token}&sudo=#{admin_2.id}") + + expect(json_response['id']).to eq(admin_2.id) + expect(json_response).to have_key('note') + expect(json_response['note']).to eq(admin_2.note) + end + end + end + end + + context 'as a regular user' do + it 'does not contain the note of the user' do + get api("/user", user) + + expect(json_response).not_to have_key('note') + end + end + end + end + end + shared_examples 'rendering user status' do it 'returns the status if there was one' do create(:user_status, user: user) diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index a9de0a747f6..1bd402e38be 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -1621,6 +1621,29 @@ describe QuickActions::InterpretService do expect(message).to eq("Created branch '#{branch_name}' and a merge request to resolve this issue.") end end + + context 'submit_review command' do + using RSpec::Parameterized::TableSyntax + + where(:note) do + [ + 'I like it', + '/submit_review' + ] + end + + with_them do + let(:content) { '/submit_review' } + let!(:draft_note) { create(:draft_note, note: note, merge_request: merge_request, author: developer) } + + it 'submits the users current review' do + _, _, message = service.execute(content, merge_request) + + expect { draft_note.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(message).to eq('Submitted the current review.') + end + end + end end describe '#explain' do diff --git a/spec/support/shared_examples/lib/gitlab/gl_repository_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/gl_repository_shared_examples.rb new file mode 100644 index 00000000000..97f4341340d --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/gl_repository_shared_examples.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'parsing gl_repository identifier' do + subject { described_class.new(identifier) } + + it 'returns correct information' do + aggregate_failures do + expect(subject.repo_type).to eq(expected_type) + expect(subject.fetch_container!).to eq(expected_container) + end + end +end diff --git a/spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb index 69ae9339f10..4aeae788114 100644 --- a/spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb @@ -7,26 +7,6 @@ RSpec.shared_examples 'a repo type' do it { is_expected.to eq(expected_identifier) } end - describe '#fetch_id' do - it 'finds an id match in the identifier' do - expect(described_class.fetch_id(expected_identifier)).to eq(expected_id) - end - - it 'does not break on other identifiers' do - expect(described_class.fetch_id('wiki-noid')).to eq(nil) - end - end - - describe '#fetch_container!' do - it 'returns the container' do - expect(described_class.fetch_container!(expected_identifier)).to eq expected_container - end - - it 'raises an exception if the identifier is invalid' do - expect { described_class.fetch_container!('project-noid') }.to raise_error ArgumentError - end - end - describe '#path_suffix' do subject { described_class.path_suffix } diff --git a/spec/workers/new_note_worker_spec.rb b/spec/workers/new_note_worker_spec.rb index cf350fbcf2a..57269355180 100644 --- a/spec/workers/new_note_worker_spec.rb +++ b/spec/workers/new_note_worker_spec.rb @@ -49,4 +49,14 @@ describe NewNoteWorker do described_class.new.perform(unexistent_note_id) end end + + context 'when note is with review' do + it 'does not create a new note notification' do + note = create(:note, :with_review) + + expect_any_instance_of(NotificationService).not_to receive(:new_note) + + subject.perform(note.id) + end + end end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index aab7a36189a..18e06332eb3 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -355,7 +355,7 @@ describe PostReceive do context "webhook" do it "fetches the correct project" do - expect(Project).to receive(:find_by).with(id: project.id.to_s) + expect(Project).to receive(:find_by).with(id: project.id) perform end