diff options
50 files changed, 460 insertions, 385 deletions
diff --git a/.gitignore b/.gitignore index 51b77d5ac9e..21dc67384aa 100644 --- a/.gitignore +++ b/.gitignore @@ -76,3 +76,4 @@ eslint-report.html /.rspec /plugins/* /.gitlab_pages_secret +package-lock.json diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec904ada5bc..dcdf520ee6b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -306,14 +306,23 @@ To better understand the priority by which UX tackles issues, see the [UX sectio Once an issue has been worked on and is ready for development, a UXer removes the ~"UX" label and applies the ~"UX ready" label to that issue. The UX team has a special type label called ~"design artifact". This label indicates that the final output -for an issue is a UX solution/design. The solution will be developed by frontend and/or backend in a subsequent milestone. +for an issue is a UX solution/design. The solution will be developed by frontend and/or backend in a subsequent milestone. Any issue labeled ~"design artifact" should not also be labeled ~"frontend" or ~"backend" since no development is needed until the solution has been decided. ~"design artifact" issues are like any other issue and should contain a milestone label, ~"Deliverable" or ~"Stretch", when scheduled in the current milestone. -Once the ~"design artifact" issue has been completed, the UXer removes the ~"design artifact" label and applies the ~"UX ready" label. The Product Manager can use the -existing issue or decide to create a whole new issue for the purpose of development. +To prevent the misunderstanding that a feature will be be delivered in the +assigned milestone, when only UX design is planned for that milestone, the +Product Manager should create a separate issue for the ~"design artifact", +assign the ~UX, ~"design artifact" and ~"Deliverable" labels, add a milestone +and use a title that makes it clear that the scheduled issue is design only +(e.g. `Design exploration for XYZ`). + +When the ~"design artifact" issue has been completed, the UXer removes the ~UX +label, adds the ~"UX ready" label and closes the issue. This indicates the +design artifact is complete. The UXer will also copy the designs to related +issues for implementation in an upcoming milestone. ## Issue tracker diff --git a/app/assets/javascripts/ide/services/index.js b/app/assets/javascripts/ide/services/index.js index e8b51f2b516..da9de25302a 100644 --- a/app/assets/javascripts/ide/services/index.js +++ b/app/assets/javascripts/ide/services/index.js @@ -9,7 +9,7 @@ export default { return Vue.http.get(endpoint, { params: { format: 'json' } }); }, getFileData(endpoint) { - return Vue.http.get(endpoint, { params: { format: 'json' } }); + return Vue.http.get(endpoint, { params: { format: 'json', viewer: 'none' } }); }, getRawFileData(file) { if (file.tempFile) { diff --git a/app/assets/javascripts/ide/stores/modules/commit/actions.js b/app/assets/javascripts/ide/stores/modules/commit/actions.js index 0a0db4033c8..7219abc4185 100644 --- a/app/assets/javascripts/ide/stores/modules/commit/actions.js +++ b/app/assets/javascripts/ide/stores/modules/commit/actions.js @@ -49,31 +49,6 @@ export const setLastCommitMessage = ({ rootState, commit }, data) => { commit(rootTypes.SET_LAST_COMMIT_MSG, commitMsg, { root: true }); }; -export const checkCommitStatus = ({ rootState }) => - service - .getBranchData(rootState.currentProjectId, rootState.currentBranchId) - .then(({ data }) => { - const { id } = data.commit; - const selectedBranch = - rootState.projects[rootState.currentProjectId].branches[rootState.currentBranchId]; - - if (selectedBranch.workingReference !== id) { - return true; - } - - return false; - }) - .catch(() => - flash( - __('Error checking branch data. Please try again.'), - 'alert', - document, - null, - false, - true, - ), - ); - export const updateFilesAfterCommit = ({ commit, dispatch, rootState }, { data }) => { const selectedProject = rootState.projects[rootState.currentProjectId]; const lastCommit = { @@ -128,24 +103,17 @@ export const updateFilesAfterCommit = ({ commit, dispatch, rootState }, { data } export const commitChanges = ({ commit, state, getters, dispatch, rootState, rootGetters }) => { const newBranch = state.commitAction !== consts.COMMIT_TO_CURRENT_BRANCH; - const payload = createCommitPayload(getters.branchName, newBranch, state, rootState); - const getCommitStatus = newBranch ? Promise.resolve(false) : dispatch('checkCommitStatus'); + const payload = createCommitPayload({ + branch: getters.branchName, + newBranch, + state, + rootState, + }); commit(types.UPDATE_LOADING, true); - return getCommitStatus - .then( - branchChanged => - new Promise(resolve => { - if (branchChanged) { - // show the modal with a Bootstrap call - $('#ide-create-branch-modal').modal('show'); - } else { - resolve(); - } - }), - ) - .then(() => service.commit(rootState.currentProjectId, payload)) + return service + .commit(rootState.currentProjectId, payload) .then(({ data }) => { commit(types.UPDATE_LOADING, false); @@ -220,12 +188,16 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo ); }) .catch(err => { - let errMsg = __('Error committing changes. Please try again.'); - if (err.response.data && err.response.data.message) { - errMsg += ` (${stripHtml(err.response.data.message)})`; + if (err.response.status === 400) { + $('#ide-create-branch-modal').modal('show'); + } else { + let errMsg = __('Error committing changes. Please try again.'); + if (err.response.data && err.response.data.message) { + errMsg += ` (${stripHtml(err.response.data.message)})`; + } + flash(errMsg, 'alert', document, null, false, true); + window.dispatchEvent(new Event('resize')); } - flash(errMsg, 'alert', document, null, false, true); - window.dispatchEvent(new Event('resize')); commit(types.UPDATE_LOADING, false); }); diff --git a/app/assets/javascripts/ide/stores/mutations/file.js b/app/assets/javascripts/ide/stores/mutations/file.js index 5826f6cb828..46547820425 100644 --- a/app/assets/javascripts/ide/stores/mutations/file.js +++ b/app/assets/javascripts/ide/stores/mutations/file.js @@ -47,6 +47,7 @@ export default { baseRaw: null, html: data.html, size: data.size, + lastCommitSha: data.last_commit_sha, }); }, [types.SET_FILE_RAW_DATA](state, { file, raw }) { diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index a04a33cd12d..10368a4d97c 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -17,6 +17,7 @@ export const dataStructure = () => ({ changed: false, staged: false, lastCommitPath: '', + lastCommitSha: '', lastCommit: { id: '', url: '', @@ -104,7 +105,7 @@ export const setPageTitle = title => { document.title = title; }; -export const createCommitPayload = (branch, newBranch, state, rootState) => ({ +export const createCommitPayload = ({ branch, newBranch, state, rootState }) => ({ branch, commit_message: state.commitMessage, actions: rootState.stagedFiles.map(f => ({ @@ -112,6 +113,7 @@ export const createCommitPayload = (branch, newBranch, state, rootState) => ({ file_path: f.path, content: f.content, encoding: f.base64 ? 'base64' : 'text', + last_commit_id: newBranch ? undefined : f.lastCommitSha, })), start_branch: newBranch ? rootState.currentBranchId : undefined, }); diff --git a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js index 326d4523cce..491858c3602 100644 --- a/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js +++ b/app/assets/javascripts/merge_conflicts/merge_conflicts_bundle.js @@ -1,13 +1,9 @@ -/* eslint-disable new-cap, comma-dangle, no-new */ - import $ from 'jquery'; import Vue from 'vue'; -import Flash from '../flash'; +import createFlash from '../flash'; import initIssuableSidebar from '../init_issuable_sidebar'; import './merge_conflict_store'; import MergeConflictsService from './merge_conflict_service'; -import './mixins/line_conflict_utils'; -import './mixins/line_conflict_actions'; import './components/diff_file_editor'; import './components/inline_conflict_lines'; import './components/parallel_conflict_lines'; @@ -19,7 +15,7 @@ export default function initMergeConflicts() { const mergeConflictsStore = gl.mergeConflicts.mergeConflictsStore; const mergeConflictsService = new MergeConflictsService({ conflictsPath: conflictsEl.dataset.conflictsPath, - resolveConflictsPath: conflictsEl.dataset.resolveConflictsPath + resolveConflictsPath: conflictsEl.dataset.resolveConflictsPath, }); initIssuableSidebar(); @@ -29,17 +25,26 @@ export default function initMergeConflicts() { components: { 'diff-file-editor': gl.mergeConflicts.diffFileEditor, 'inline-conflict-lines': gl.mergeConflicts.inlineConflictLines, - 'parallel-conflict-lines': gl.mergeConflicts.parallelConflictLines + 'parallel-conflict-lines': gl.mergeConflicts.parallelConflictLines, }, data: mergeConflictsStore.state, computed: { - conflictsCountText() { return mergeConflictsStore.getConflictsCountText(); }, - readyToCommit() { return mergeConflictsStore.isReadyToCommit(); }, - commitButtonText() { return mergeConflictsStore.getCommitButtonText(); }, - showDiffViewTypeSwitcher() { return mergeConflictsStore.fileTextTypePresent(); } + conflictsCountText() { + return mergeConflictsStore.getConflictsCountText(); + }, + readyToCommit() { + return mergeConflictsStore.isReadyToCommit(); + }, + commitButtonText() { + return mergeConflictsStore.getCommitButtonText(); + }, + showDiffViewTypeSwitcher() { + return mergeConflictsStore.fileTextTypePresent(); + }, }, created() { - mergeConflictsService.fetchConflictsData() + mergeConflictsService + .fetchConflictsData() .then(({ data }) => { if (data.type === 'error') { mergeConflictsStore.setFailedRequest(data.message); @@ -87,9 +92,9 @@ export default function initMergeConflicts() { }) .catch(() => { mergeConflictsStore.setSubmitState(false); - new Flash('Failed to save merge conflicts resolutions. Please try again!'); + createFlash('Failed to save merge conflicts resolutions. Please try again!'); }); - } - } + }, + }, }); } diff --git a/app/assets/javascripts/pages/users/user_tabs.js b/app/assets/javascripts/pages/users/user_tabs.js index 9404b06615e..f88a6b27c18 100644 --- a/app/assets/javascripts/pages/users/user_tabs.js +++ b/app/assets/javascripts/pages/users/user_tabs.js @@ -180,7 +180,7 @@ export default class UserTabs { } toggleLoading(status) { - return this.$parentEl.find('.loading-status .loading').toggleClass('hidden', !status); + return this.$parentEl.find('.loading-status .loading').toggleClass('hide', !status); } setCurrentAction(source) { diff --git a/app/assets/javascripts/projects/project_new.js b/app/assets/javascripts/projects/project_new.js index 888b1d6ce33..002edb4663c 100644 --- a/app/assets/javascripts/projects/project_new.js +++ b/app/assets/javascripts/projects/project_new.js @@ -90,7 +90,7 @@ const bindEvents = () => { function chooseTemplate() { $('.template-option').hide(); $projectFieldsForm.addClass('selected'); - $selectedIcon.removeClass('active'); + $selectedIcon.removeClass('d-block'); const value = $(this).val(); const templates = { rails: { @@ -109,7 +109,7 @@ const bindEvents = () => { const selectedTemplate = templates[value]; $selectedTemplateText.text(selectedTemplate.text); - $(selectedTemplate.icon).addClass('active'); + $(selectedTemplate.icon).addClass('d-block'); $templateProjectNameInput.focus(); } diff --git a/app/assets/javascripts/single_file_diff.js b/app/assets/javascripts/single_file_diff.js index 1afff0dba38..ae27c676fa0 100644 --- a/app/assets/javascripts/single_file_diff.js +++ b/app/assets/javascripts/single_file_diff.js @@ -11,7 +11,7 @@ import syntaxHighlight from './syntax_highlight'; const WRAPPER = '<div class="diff-content"></div>'; const LOADING_HTML = '<i class="fa fa-spinner fa-spin"></i>'; const ERROR_HTML = '<div class="nothing-here-block"><i class="fa fa-warning"></i> Could not load diff</div>'; -const COLLAPSED_HTML = '<div class="nothing-here-block diff-collapsed">This diff is collapsed. <a class="click-to-expand">Click to expand it.</a></div>'; +const COLLAPSED_HTML = '<div class="nothing-here-block diff-collapsed">This diff is collapsed. <button class="click-to-expand btn btn-link">Click to expand it.</button></div>'; export default class SingleFileDiff { constructor(file) { diff --git a/app/assets/stylesheets/bootstrap_migration.scss b/app/assets/stylesheets/bootstrap_migration.scss index 810ed5bb0a6..5da0e672288 100644 --- a/app/assets/stylesheets/bootstrap_migration.scss +++ b/app/assets/stylesheets/bootstrap_migration.scss @@ -293,3 +293,9 @@ input[type=color].form-control { color: $gl-text-color-secondary; } } + +.project-templates-buttons { + .btn { + vertical-align: unset; + } +} diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index c5be27f2d29..0de05548c68 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -16,6 +16,7 @@ .click-to-expand { cursor: pointer; + vertical-align: initial; } } } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 0826bfd0035..04d2a049f7d 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -832,3 +832,5 @@ $input-border-color: $theme-gray-200; $input-color: $gl-text-color; $font-family-sans-serif: $regular_font; $font-family-monospace: $monospace_font; +$input-line-height: 20px; +$btn-line-height: 20px; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index caafda5fb05..7ac0eaec645 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -497,6 +497,12 @@ &:not(:first-child) { border-top: 1px solid $border-color; } + + .btn-template-icon { + position: absolute; + left: $gl-padding; + top: $gl-padding; + } } .template-title { @@ -514,12 +520,6 @@ } } - svg { - position: absolute; - left: $gl-padding; - top: $gl-padding; - } - .project-fields-form { display: none; @@ -530,34 +530,23 @@ } .template-input-group { - position: relative; - - @include media-breakpoint-up(sm) { - display: flex; - } - - .input-group-prepend, - .input-group-append { + .input-group-prepend { flex: 1; - text-align: left; - padding-left: ($gl-padding * 3); - background-color: $white-light; } - .selected-template { - line-height: 20px; + .input-group-text { + width: 100%; + background-color: $white-light; } .selected-icon { + padding-right: $gl-padding; + svg { display: none; top: 7px; height: 20px; width: 20px; - - &.active { - display: block; - } } } } diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 1f8e61257a9..4abb145067a 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -52,7 +52,7 @@ .settings-content { max-height: 1px; - overflow-y: scroll; + overflow-y: hidden; padding-right: 110px; animation: collapseMaxHeight 300ms ease-out; // Keep the section from expanding when we scroll over it diff --git a/app/controllers/concerns/internal_redirect.rb b/app/controllers/concerns/internal_redirect.rb index 7409b2e89a5..10b9852e329 100644 --- a/app/controllers/concerns/internal_redirect.rb +++ b/app/controllers/concerns/internal_redirect.rb @@ -23,6 +23,10 @@ module InternalRedirect nil end + def sanitize_redirect(url_or_path) + safe_redirect_path(url_or_path) || safe_redirect_path_for_url(url_or_path) + end + def host_allowed?(uri) uri.host == request.host && uri.port == request.port diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 0c1c286a0a4..7c65f1f5dfe 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -197,15 +197,14 @@ class Projects::BlobController < Projects::ApplicationController end def show_json - json = blob_json(@blob) - return render_404 unless json - + set_last_commit_sha path_segments = @path.split('/') path_segments.pop tree_path = path_segments.join('/') - render json: json.merge( + json = { id: @blob.id, + last_commit_sha: @last_commit_sha, path: blob.path, name: blob.name, extension: blob.extension, @@ -221,6 +220,10 @@ class Projects::BlobController < Projects::ApplicationController commits_path: project_commits_path(project, @id), tree_path: project_tree_path(project, File.join(@ref, tree_path)), permalink: project_blob_path(project, File.join(@commit.id, @path)) - ) + } + + json.merge!(blob_json(@blob) || {}) unless params[:viewer] == 'none' + + render json: json end end diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index 2494b56981d..f85dcfe6bfc 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -123,9 +123,9 @@ class Projects::MilestonesController < Projects::ApplicationController def search_params if request.format.json? && @project.group && can?(current_user, :read_group, @project.group) - groups = @project.group.self_and_ancestors + groups = @project.group.self_and_ancestors_ids end - params.permit(:state).merge(project_ids: @project.id, group_ids: groups&.select(:id)) + params.permit(:state).merge(project_ids: @project.id, group_ids: groups) end end diff --git a/app/graphql/resolvers/merge_request_resolver.rb b/app/graphql/resolvers/merge_request_resolver.rb index b1857ab09f7..9f2d348e95f 100644 --- a/app/graphql/resolvers/merge_request_resolver.rb +++ b/app/graphql/resolvers/merge_request_resolver.rb @@ -1,15 +1,14 @@ module Resolvers class MergeRequestResolver < BaseResolver - prepend FullPathResolver - - type Types::ProjectType, null: true - argument :iid, GraphQL::ID_TYPE, required: true, description: 'The IID of the merge request, e.g., "1"' - def resolve(full_path:, iid:) - project = model_by_full_path(Project, full_path) + type Types::MergeRequestType, null: true + + alias_method :project, :object + + def resolve(iid:) return unless project.present? BatchLoader.for(iid.to_s).batch(key: project.id) do |iids, loader| diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index 9e885d5845a..d9058ae7431 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -61,5 +61,12 @@ module Types field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true field :only_allow_merge_if_all_discussions_are_resolved, GraphQL::BOOLEAN_TYPE, null: true field :printing_merge_request_link_enabled, GraphQL::BOOLEAN_TYPE, null: true + + field :merge_request, + Types::MergeRequestType, + null: true, + resolver: Resolvers::MergeRequestResolver do + authorize :read_merge_request + end end end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index be79c78bf67..010ec2d7942 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -9,13 +9,6 @@ module Types authorize :read_project end - field :merge_request, Types::MergeRequestType, - null: true, - resolver: Resolvers::MergeRequestResolver, - description: "Find a merge request" do - authorize :read_merge_request - end - field :echo, GraphQL::STRING_TYPE, null: false, function: Functions::Echo.new end end diff --git a/app/views/projects/_project_templates.html.haml b/app/views/projects/_project_templates.html.haml index 9d27f51926e..d08807b5135 100644 --- a/app/views/projects/_project_templates.html.haml +++ b/app/views/projects/_project_templates.html.haml @@ -10,16 +10,18 @@ %a.btn.btn-default{ href: template.preview, rel: 'noopener noreferrer', target: '_blank' } Preview .project-fields-form - .form-group - %label.label-light - Template - .input-group.template-input-group - .input-group-prepend - .input-group-text - .selected-icon - - Gitlab::ProjectTemplate.all.each do |template| - = custom_icon(template.logo) - .selected-template - %button.btn.btn-default.change-template{ type: "button" } Change template + .row + .form-group.col-sm-12 + %label.label-light + Template + .input-group.template-input-group + .input-group-prepend + .input-group-text + .selected-icon + - Gitlab::ProjectTemplate.all.each do |template| + = custom_icon(template.logo) + .selected-template + .input-group-append + %button.btn.btn-default.change-template{ type: "button" } Change template = render 'new_project_fields', f: f, project_name_id: "template-project-name" diff --git a/app/views/projects/diffs/_collapsed.html.haml b/app/views/projects/diffs/_collapsed.html.haml index 5762f4d86d7..9bd1255fe00 100644 --- a/app/views/projects/diffs/_collapsed.html.haml +++ b/app/views/projects/diffs/_collapsed.html.haml @@ -2,4 +2,4 @@ - url = url_for(safe_params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier)) .nothing-here-block.diff-collapsed{ data: { diff_for_path: url } } This diff is collapsed. - %a.click-to-expand Click to expand it. + %button.click-to-expand.btn.btn-link Click to expand it. diff --git a/app/views/shared/builds/_build_output.html.haml b/app/views/shared/builds/_build_output.html.haml index 07f1501fadd..0e18128a8f1 100644 --- a/app/views/shared/builds/_build_output.html.haml +++ b/app/views/shared/builds/_build_output.html.haml @@ -1,3 +1,3 @@ %pre.build-trace#build-trace %code.bash.js-build-output - .build-loader-animation.js-build-refresh + .build-loader-animation.js-build-refresh diff --git a/changelogs/unreleased/blackst0ne-rails5-fix-optimistic-lock-values.yml b/changelogs/unreleased/blackst0ne-rails5-fix-optimistic-lock-values.yml new file mode 100644 index 00000000000..1915dff73ab --- /dev/null +++ b/changelogs/unreleased/blackst0ne-rails5-fix-optimistic-lock-values.yml @@ -0,0 +1,5 @@ +--- +title: "[Rails5] Fix optimistic lock value" +merge_request: 19878 +author: "@blackst0ne" +type: fixed diff --git a/changelogs/unreleased/bvl-graphql-nested-merge-request.yml b/changelogs/unreleased/bvl-graphql-nested-merge-request.yml new file mode 100644 index 00000000000..f0f0488d31a --- /dev/null +++ b/changelogs/unreleased/bvl-graphql-nested-merge-request.yml @@ -0,0 +1,5 @@ +--- +title: Allow querying a single merge request within a project +merge_request: 19853 +author: +type: changed diff --git a/changelogs/unreleased/rails5-fix-47836.yml b/changelogs/unreleased/rails5-fix-47836.yml new file mode 100644 index 00000000000..2aef2db607a --- /dev/null +++ b/changelogs/unreleased/rails5-fix-47836.yml @@ -0,0 +1,6 @@ +--- +title: Rails5 fix passing Group objects array into for_projects_and_groups milestone + scope +merge_request: 19863 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/safari-scrollbar-bug.yml b/changelogs/unreleased/safari-scrollbar-bug.yml new file mode 100644 index 00000000000..792a66d1ada --- /dev/null +++ b/changelogs/unreleased/safari-scrollbar-bug.yml @@ -0,0 +1,5 @@ +--- +title: Remove scrollbar in Safari in repo settings page +merge_request: 19809 +author: gfyoung +type: fixed diff --git a/config/initializers/active_record_locking.rb b/config/initializers/active_record_locking.rb index 3e7111fd063..0861544c5a4 100644 --- a/config/initializers/active_record_locking.rb +++ b/config/initializers/active_record_locking.rb @@ -1,73 +1,80 @@ # rubocop:disable Lint/RescueException -# Remove this entire initializer when we are at rails 5.0. -# This file fixes the bug (see below) which has been fixed in the upstream. -unless Gitlab.rails5? - # This patch fixes https://github.com/rails/rails/issues/26024 - # TODO: Remove it when it's no longer necessary - - module ActiveRecord - module Locking - module Optimistic - # We overwrite this method because we don't want to have default value - # for newly created records - def _create_record(attribute_names = self.attribute_names, *) # :nodoc: - super - end +# Remove this monkey-patch when all lock_version values are converted from NULLs to zeros. +# See https://gitlab.com/gitlab-org/gitlab-ce/issues/25228 +module ActiveRecord + module Locking + module Optimistic + # We overwrite this method because we don't want to have default value + # for newly created records + def _create_record(attribute_names = self.attribute_names, *) # :nodoc: + super + end - def _update_record(attribute_names = self.attribute_names) #:nodoc: - return super unless locking_enabled? - return 0 if attribute_names.empty? + def _update_record(attribute_names = self.attribute_names) #:nodoc: + return super unless locking_enabled? + return 0 if attribute_names.empty? - lock_col = self.class.locking_column + lock_col = self.class.locking_column - previous_lock_value = send(lock_col).to_i # rubocop:disable GitlabSecurity/PublicSend + previous_lock_value = send(lock_col).to_i # rubocop:disable GitlabSecurity/PublicSend - # This line is added as a patch - previous_lock_value = nil if previous_lock_value == '0' || previous_lock_value == 0 + # This line is added as a patch + previous_lock_value = nil if previous_lock_value == '0' || previous_lock_value == 0 - increment_lock + increment_lock - attribute_names += [lock_col] - attribute_names.uniq! + attribute_names += [lock_col] + attribute_names.uniq! - begin - relation = self.class.unscoped + begin + relation = self.class.unscoped - affected_rows = relation.where( - self.class.primary_key => id, - lock_col => previous_lock_value - ).update_all( - attributes_for_update(attribute_names).map do |name| - [name, _read_attribute(name)] - end.to_h - ) + affected_rows = relation.where( + self.class.primary_key => id, + lock_col => previous_lock_value + ).update_all( + attributes_for_update(attribute_names).map do |name| + [name, _read_attribute(name)] + end.to_h + ) - unless affected_rows == 1 - raise ActiveRecord::StaleObjectError.new(self, "update") - end + unless affected_rows == 1 + raise ActiveRecord::StaleObjectError.new(self, "update") + end - affected_rows + affected_rows - # If something went wrong, revert the version. - rescue Exception - send(lock_col + '=', previous_lock_value) # rubocop:disable GitlabSecurity/PublicSend - raise - end + # If something went wrong, revert the version. + rescue Exception + send(lock_col + '=', previous_lock_value) # rubocop:disable GitlabSecurity/PublicSend + raise end + end - # This is patched because we need it to query `lock_version IS NULL` - # rather than `lock_version = 0` whenever lock_version is NULL. - def relation_for_destroy - return super unless locking_enabled? + # This is patched because we need it to query `lock_version IS NULL` + # rather than `lock_version = 0` whenever lock_version is NULL. + def relation_for_destroy + return super unless locking_enabled? - column_name = self.class.locking_column - super.where(self.class.arel_table[column_name].eq(self[column_name])) - end + column_name = self.class.locking_column + super.where(self.class.arel_table[column_name].eq(self[column_name])) end + end + + # This is patched because we want `lock_version` default to `NULL` + # rather than `0` + if Gitlab.rails5? + class LockingType + def deserialize(value) + super + end - # This is patched because we want `lock_version` default to `NULL` - # rather than `0` + def serialize(value) + super + end + end + else class LockingType < SimpleDelegator def type_cast_from_database(value) super diff --git a/doc/api/README.md b/doc/api/README.md index 1c756dc855f..6267618d3bc 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -104,7 +104,7 @@ with a major point release of GitLab itself. All deprecations and changes between two versions should be listed in the documentation. For the changes between v3 and v4; please read the [v3 to v4 documentation](v3_to_v4.md) -#### Current status +### Current status Currently only API version v4 is available. Version v3 was removed in [GitLab 11.0](https://gitlab.com/gitlab-org/gitlab-ce/issues/36819). diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md index dcd5377284c..59e27922f64 100644 --- a/doc/api/graphql/index.md +++ b/doc/api/graphql/index.md @@ -29,9 +29,7 @@ curl --data "value=100" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://g ## Available queries -A first iteration of a GraphQL API includes only 2 queries: `project` and -`merge_request` and only returns scalar fields, or fields of the type `Project` -or `MergeRequest`. +A first iteration of a GraphQL API includes a query for: `project`. Within a project it is also possible to fetch a `mergeRequest` by IID. ## GraphiQL diff --git a/doc/integration/google.md b/doc/integration/google.md index ae1d848f439..8906f91b6b4 100644 --- a/doc/integration/google.md +++ b/doc/integration/google.md @@ -35,7 +35,12 @@ In Google's side: 1. You should now be able to see a Client ID and Client secret. Note them down or keep this page open as you will need them later. -1. From the **Dashboard** select **ENABLE APIS AND SERVICES > Compute > Google Kubernetes Engine API > Enable** +1. From the **Dashboard** select **ENABLE APIS AND SERVICES > Compute > Google+ API > Enable** +1. To enable projects to access [Google Kubernetes Engine](../user/project/clusters/index.md), you must also + enable these APIs: + - Google Kubernetes Engine API + - Cloud Resource Manager API + - Cloud Billing API On your GitLab server: diff --git a/doc/user/project/milestones/index.md b/doc/user/project/milestones/index.md index 64bb33be547..632253db94c 100644 --- a/doc/user/project/milestones/index.md +++ b/doc/user/project/milestones/index.md @@ -10,7 +10,6 @@ Milestones allow you to organize issues and merge requests into a cohesive group - **Project milestones** can be assigned to issues or merge requests in that project only. - **Group milestones** can be assigned to any issue or merge request of any project in that group. -- In the [future](https://gitlab.com/gitlab-org/gitlab-ce/issues/36862), you will be able to assign group milestones to issues and merge requests of projects in [subgroups](../../group/subgroups/index.md). ## Creating milestones diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 858e790005c..af8448937b3 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -65,7 +65,7 @@ module Banzai # We don't support IID lookups for group milestones, because IIDs can # clash between group and project milestones. if project.group && !params[:iid] - finder_params[:group_ids] = project.group.self_and_ancestors.select(:id) + finder_params[:group_ids] = project.group.self_and_ancestors_ids end MilestonesFinder.new(finder_params).find_by(params) diff --git a/spec/controllers/concerns/internal_redirect_spec.rb b/spec/controllers/concerns/internal_redirect_spec.rb index a0ee13b2352..7e23b56356e 100644 --- a/spec/controllers/concerns/internal_redirect_spec.rb +++ b/spec/controllers/concerns/internal_redirect_spec.rb @@ -54,6 +54,31 @@ describe InternalRedirect do end end + describe '#sanitize_redirect' do + let(:valid_path) { '/hello/world?hello=world' } + let(:valid_url) { "http://test.host#{valid_path}" } + + it 'returns `nil` for invalid paths' do + invalid_path = '//not/valid' + + expect(controller.sanitize_redirect(invalid_path)).to eq nil + end + + it 'returns `nil` for invalid urls' do + input = 'http://test.host:3000/invalid' + + expect(controller.sanitize_redirect(input)).to eq nil + end + + it 'returns input for valid paths' do + expect(controller.sanitize_redirect(valid_path)).to eq valid_path + end + + it 'returns path for valid urls' do + expect(controller.sanitize_redirect(valid_url)).to eq valid_path + end + end + describe '#host_allowed?' do it 'allows uris with the same host and port' do expect(controller.host_allowed?(URI('http://test.host/test'))).to be(true) diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 00a7df6ccc8..9e696e9cb29 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -55,6 +55,25 @@ describe Projects::BlobController do expect(json_response).to have_key 'raw_path' end end + + context "with viewer=none" do + let(:id) { 'master/README.md' } + + before do + get(:show, + namespace_id: project.namespace, + project_id: project, + id: id, + format: :json, + viewer: 'none') + end + + it do + expect(response).to be_ok + expect(json_response).not_to have_key 'html' + expect(json_response).to have_key 'raw_path' + end + end end context 'with tree path' do diff --git a/spec/features/issues/filtered_search/dropdown_assignee_spec.rb b/spec/features/issues/filtered_search/dropdown_assignee_spec.rb index cbd0949c192..c8115db9212 100644 --- a/spec/features/issues/filtered_search/dropdown_assignee_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_assignee_spec.rb @@ -31,7 +31,7 @@ describe 'Dropdown assignee', :js do describe 'behavior' do it 'opens when the search bar has assignee:' do - filtered_search.set('assignee:') + input_filtered_search('assignee:', submit: false, extra_space: false) expect(page).to have_css(js_dropdown_assignee, visible: true) end @@ -44,6 +44,7 @@ describe 'Dropdown assignee', :js do it 'should show loading indicator when opened' do slow_requests do + # We aren't using `input_filtered_search` because we want to see the loading indicator filtered_search.set('assignee:') expect(page).to have_css('#js-dropdown-assignee .filter-dropdown-loading', visible: true) @@ -51,19 +52,19 @@ describe 'Dropdown assignee', :js do end it 'should hide loading indicator when loaded' do - filtered_search.set('assignee:') + input_filtered_search('assignee:', submit: false, extra_space: false) expect(find(js_dropdown_assignee)).not_to have_css('.filter-dropdown-loading') end it 'should load all the assignees when opened' do - filtered_search.set('assignee:') + input_filtered_search('assignee:', submit: false, extra_space: false) expect(dropdown_assignee_size).to eq(4) end it 'shows current user at top of dropdown' do - filtered_search.set('assignee:') + input_filtered_search('assignee:', submit: false, extra_space: false) expect(filter_dropdown.first('.filter-dropdown-item')).to have_content(user.name) end @@ -71,7 +72,7 @@ describe 'Dropdown assignee', :js do describe 'filtering' do before do - filtered_search.set('assignee:') + input_filtered_search('assignee:', submit: false, extra_space: false) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_john.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) @@ -79,23 +80,21 @@ describe 'Dropdown assignee', :js do end it 'filters by name' do - filtered_search.send_keys('j') + input_filtered_search('jac', submit: false, extra_space: false) - expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_john.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_no_content(user.name) end it 'filters by case insensitive name' do - filtered_search.send_keys('J') + input_filtered_search('JAC', submit: false, extra_space: false) - expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_john.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_no_content(user.name) end it 'filters by username with symbol' do - filtered_search.send_keys('@ot') + input_filtered_search('@ott', submit: false, extra_space: false) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) @@ -103,7 +102,7 @@ describe 'Dropdown assignee', :js do end it 'filters by case insensitive username with symbol' do - filtered_search.send_keys('@OT') + input_filtered_search('@OTT', submit: false, extra_space: false) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) @@ -111,7 +110,9 @@ describe 'Dropdown assignee', :js do end it 'filters by username without symbol' do - filtered_search.send_keys('ot') + input_filtered_search('ott', submit: false, extra_space: false) + + wait_for_requests expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) @@ -119,7 +120,9 @@ describe 'Dropdown assignee', :js do end it 'filters by case insensitive username without symbol' do - filtered_search.send_keys('OT') + input_filtered_search('OTT', submit: false, extra_space: false) + + wait_for_requests expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) @@ -129,7 +132,7 @@ describe 'Dropdown assignee', :js do describe 'selecting from dropdown' do before do - filtered_search.set('assignee:') + input_filtered_search('assignee:', submit: false, extra_space: false) end it 'fills in the assignee username when the assignee has not been filtered' do @@ -143,7 +146,7 @@ describe 'Dropdown assignee', :js do end it 'fills in the assignee username when the assignee has been filtered' do - filtered_search.send_keys('roo') + input_filtered_search('roo', submit: false, extra_space: false) click_assignee(user.name) wait_for_requests @@ -165,7 +168,7 @@ describe 'Dropdown assignee', :js do describe 'selecting from dropdown without Ajax call' do before do Gitlab::Testing::RequestBlockerMiddleware.block_requests! - filtered_search.set('assignee:') + input_filtered_search('assignee:', submit: false, extra_space: false) end after do @@ -183,31 +186,31 @@ describe 'Dropdown assignee', :js do describe 'input has existing content' do it 'opens assignee dropdown with existing search term' do - filtered_search.set('searchTerm assignee:') + input_filtered_search('searchTerm assignee:', submit: false, extra_space: false) expect(page).to have_css(js_dropdown_assignee, visible: true) end it 'opens assignee dropdown with existing author' do - filtered_search.set('author:@user assignee:') + input_filtered_search('author:@user assignee:', submit: false, extra_space: false) expect(page).to have_css(js_dropdown_assignee, visible: true) end it 'opens assignee dropdown with existing label' do - filtered_search.set('label:~bug assignee:') + input_filtered_search('label:~bug assignee:', submit: false, extra_space: false) expect(page).to have_css(js_dropdown_assignee, visible: true) end it 'opens assignee dropdown with existing milestone' do - filtered_search.set('milestone:%v1.0 assignee:') + input_filtered_search('milestone:%v1.0 assignee:', submit: false, extra_space: false) expect(page).to have_css(js_dropdown_assignee, visible: true) end it 'opens assignee dropdown with existing my-reaction' do - filtered_search.set('my-reaction:star assignee:') + input_filtered_search('my-reaction:star assignee:', submit: false, extra_space: false) expect(page).to have_css(js_dropdown_assignee, visible: true) end @@ -215,8 +218,7 @@ describe 'Dropdown assignee', :js do describe 'caching requests' do it 'caches requests after the first load' do - filtered_search.set('assignee') - filtered_search.send_keys(':') + input_filtered_search('assignee:', submit: false, extra_space: false) initial_size = dropdown_assignee_size expect(initial_size).to be > 0 @@ -224,8 +226,7 @@ describe 'Dropdown assignee', :js do new_user = create(:user) project.add_master(new_user) find('.filtered-search-box .clear-search').click - filtered_search.set('assignee') - filtered_search.send_keys(':') + input_filtered_search('assignee:', submit: false, extra_space: false) expect(dropdown_assignee_size).to eq(initial_size) end diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb index dacca494755..17818beb947 100644 --- a/spec/features/issues/user_uses_slash_commands_spec.rb +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -226,7 +226,9 @@ feature 'Issues > User uses quick actions', :js do it 'does not move the issue' do add_note("/move #{project_unauthorized.full_path}") - expect(page).not_to have_content 'Commands applied' + wait_for_requests + + expect(page).to have_content 'Commands applied' expect(issue.reload).to be_open end end diff --git a/spec/features/projects/deploy_keys_spec.rb b/spec/features/projects/deploy_keys_spec.rb index 43a23c42f83..1552a3512dd 100644 --- a/spec/features/projects/deploy_keys_spec.rb +++ b/spec/features/projects/deploy_keys_spec.rb @@ -22,7 +22,8 @@ describe 'Project deploy keys', :js do accept_confirm { find('.ic-remove').click() } - expect(page).not_to have_selector('.fa-spinner', count: 0) + wait_for_requests + expect(page).to have_selector('.deploy-key', count: 0) end end diff --git a/spec/features/projects/diffs/diff_show_spec.rb b/spec/features/projects/diffs/diff_show_spec.rb index c1307ab640f..9bfcb1e816a 100644 --- a/spec/features/projects/diffs/diff_show_spec.rb +++ b/spec/features/projects/diffs/diff_show_spec.rb @@ -166,8 +166,7 @@ feature 'Diff file viewer', :js do context 'expanding the diff' do before do - # We can't use `click_link` because the "link" doesn't have an `href`. - find('a.click-to-expand').click + click_button 'Click to expand it.' wait_for_requests end diff --git a/spec/graphql/resolvers/merge_request_resolver_spec.rb b/spec/graphql/resolvers/merge_request_resolver_spec.rb index af015533209..73993b3a039 100644 --- a/spec/graphql/resolvers/merge_request_resolver_spec.rb +++ b/spec/graphql/resolvers/merge_request_resolver_spec.rb @@ -10,49 +10,36 @@ describe Resolvers::MergeRequestResolver do set(:other_project) { create(:project, :repository) } set(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) } - let(:full_path) { project.full_path } let(:iid_1) { merge_request_1.iid } let(:iid_2) { merge_request_2.iid } - let(:other_full_path) { other_project.full_path } let(:other_iid) { other_merge_request.iid } describe '#resolve' do it 'batch-resolves merge requests by target project full path and IID' do - path = full_path # avoid database query - result = batch(max_queries: 2) do - [resolve_mr(path, iid_1), resolve_mr(path, iid_2)] + [resolve_mr(project, iid_1), resolve_mr(project, iid_2)] end expect(result).to contain_exactly(merge_request_1, merge_request_2) end it 'can batch-resolve merge requests from different projects' do - path = project.full_path # avoid database queries - other_path = other_full_path - result = batch(max_queries: 3) do - [resolve_mr(path, iid_1), resolve_mr(path, iid_2), resolve_mr(other_path, other_iid)] + [resolve_mr(project, iid_1), resolve_mr(project, iid_2), resolve_mr(other_project, other_iid)] end expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request) end it 'resolves an unknown iid to nil' do - result = batch { resolve_mr(full_path, -1) } - - expect(result).to be_nil - end - - it 'resolves a known iid for an unknown full_path to nil' do - result = batch { resolve_mr('unknown/project', iid_1) } + result = batch { resolve_mr(project, -1) } expect(result).to be_nil end end - def resolve_mr(full_path, iid) - resolve(described_class, args: { full_path: full_path, iid: iid }) + def resolve_mr(project, iid) + resolve(described_class, obj: project, args: { iid: iid }) end end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index e0f89105b86..b4eeca2e3f1 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -2,4 +2,13 @@ require 'spec_helper' describe GitlabSchema.types['Project'] do it { expect(described_class.graphql_name).to eq('Project') } + + describe 'nested merge request' do + it { expect(described_class).to have_graphql_field(:merge_request) } + + it 'authorizes the merge request' do + expect(described_class.fields['mergeRequest']) + .to require_graphql_authorizations(:read_merge_request) + end + end end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index 8488252fd59..e1df6f9811d 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -5,7 +5,7 @@ describe GitlabSchema.types['Query'] do expect(described_class.graphql_name).to eq('Query') end - it { is_expected.to have_graphql_fields(:project, :merge_request, :echo) } + it { is_expected.to have_graphql_fields(:project, :echo) } describe 'project field' do subject { described_class.fields['project'] } @@ -20,18 +20,4 @@ describe GitlabSchema.types['Query'] do is_expected.to require_graphql_authorizations(:read_project) end end - - describe 'merge_request field' do - subject { described_class.fields['mergeRequest'] } - - it 'finds MRs by project and IID' do - is_expected.to have_graphql_arguments(:full_path, :iid) - is_expected.to have_graphql_type(Types::MergeRequestType) - is_expected.to have_graphql_resolver(Resolvers::MergeRequestResolver) - end - - it 'authorizes with read_merge_request' do - is_expected.to require_graphql_authorizations(:read_merge_request) - end - end end diff --git a/spec/javascripts/ide/stores/modules/commit/actions_spec.js b/spec/javascripts/ide/stores/modules/commit/actions_spec.js index a2869ff378b..133ad627f34 100644 --- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/actions_spec.js @@ -108,77 +108,6 @@ describe('IDE commit module actions', () => { }); }); - describe('checkCommitStatus', () => { - beforeEach(() => { - store.state.currentProjectId = 'abcproject'; - store.state.currentBranchId = 'master'; - store.state.projects.abcproject = { - branches: { - master: { - workingReference: '1', - }, - }, - }; - }); - - it('calls service', done => { - spyOn(service, 'getBranchData').and.returnValue( - Promise.resolve({ - data: { - commit: { id: '123' }, - }, - }), - ); - - store - .dispatch('commit/checkCommitStatus') - .then(() => { - expect(service.getBranchData).toHaveBeenCalledWith('abcproject', 'master'); - - done(); - }) - .catch(done.fail); - }); - - it('returns true if current ref does not equal returned ID', done => { - spyOn(service, 'getBranchData').and.returnValue( - Promise.resolve({ - data: { - commit: { id: '123' }, - }, - }), - ); - - store - .dispatch('commit/checkCommitStatus') - .then(val => { - expect(val).toBeTruthy(); - - done(); - }) - .catch(done.fail); - }); - - it('returns false if current ref equals returned ID', done => { - spyOn(service, 'getBranchData').and.returnValue( - Promise.resolve({ - data: { - commit: { id: '1' }, - }, - }), - ); - - store - .dispatch('commit/checkCommitStatus') - .then(val => { - expect(val).toBeFalsy(); - - done(); - }) - .catch(done.fail); - }); - }); - describe('updateFilesAfterCommit', () => { const data = { id: '123', @@ -314,6 +243,7 @@ describe('IDE commit module actions', () => { ...file('changed'), type: 'blob', active: true, + lastCommitSha: '123456789', }; store.state.stagedFiles.push(f); store.state.changedFiles = [ @@ -366,6 +296,7 @@ describe('IDE commit module actions', () => { file_path: jasmine.anything(), content: jasmine.anything(), encoding: jasmine.anything(), + last_commit_id: undefined, }, ], start_branch: 'master', @@ -376,6 +307,32 @@ describe('IDE commit module actions', () => { .catch(done.fail); }); + it('sends lastCommit ID when not creating new branch', done => { + store.state.commit.commitAction = '1'; + + store + .dispatch('commit/commitChanges') + .then(() => { + expect(service.commit).toHaveBeenCalledWith('abcproject', { + branch: jasmine.anything(), + commit_message: 'testing 123', + actions: [ + { + action: 'update', + file_path: jasmine.anything(), + content: jasmine.anything(), + encoding: jasmine.anything(), + last_commit_id: '123456789', + }, + ], + start_branch: undefined, + }); + + done(); + }) + .catch(done.fail); + }); + it('sets last Commit Msg', done => { store .dispatch('commit/commitChanges') diff --git a/spec/javascripts/ide/stores/utils_spec.js b/spec/javascripts/ide/stores/utils_spec.js index f38ac6dd82f..a7bd443af51 100644 --- a/spec/javascripts/ide/stores/utils_spec.js +++ b/spec/javascripts/ide/stores/utils_spec.js @@ -1,4 +1,5 @@ import * as utils from '~/ide/stores/utils'; +import { file } from '../helpers'; describe('Multi-file store utils', () => { describe('setPageTitle', () => { @@ -63,4 +64,59 @@ describe('Multi-file store utils', () => { expect(foundEntry).toBeUndefined(); }); }); + + describe('createCommitPayload', () => { + it('returns API payload', () => { + const state = { + commitMessage: 'commit message', + }; + const rootState = { + stagedFiles: [ + { + ...file('staged'), + path: 'staged', + content: 'updated file content', + lastCommitSha: '123456789', + }, + { + ...file('newFile'), + path: 'added', + tempFile: true, + content: 'new file content', + base64: true, + lastCommitSha: '123456789', + }, + ], + currentBranchId: 'master', + }; + const payload = utils.createCommitPayload({ + branch: 'master', + newBranch: false, + state, + rootState, + }); + + expect(payload).toEqual({ + branch: 'master', + commit_message: 'commit message', + actions: [ + { + action: 'update', + file_path: 'staged', + content: 'updated file content', + encoding: 'text', + last_commit_id: '123456789', + }, + { + action: 'create', + file_path: 'added', + content: 'new file content', + encoding: 'base64', + last_commit_id: '123456789', + }, + ], + start_branch: undefined, + }); + }); + }); }); diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index cbcf1e55979..b5a6d959ccb 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -54,14 +54,6 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do it { is_expected.to eq('Sample data in db') } end - - context 'when data_store is others' do - before do - build_trace_chunk.send(:write_attribute, :data_store, -1) - end - - it { expect { subject }.to raise_error('Unsupported data store') } - end end describe '#set_data' do diff --git a/spec/requests/api/graphql/merge_request_query_spec.rb b/spec/requests/api/graphql/merge_request_query_spec.rb deleted file mode 100644 index 12b1d5d18a2..00000000000 --- a/spec/requests/api/graphql/merge_request_query_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -require 'spec_helper' - -describe 'getting merge request information' do - include GraphqlHelpers - - let(:project) { create(:project, :repository) } - let(:merge_request) { create(:merge_request, source_project: project) } - let(:current_user) { create(:user) } - - let(:query) do - attributes = { - 'fullPath' => merge_request.project.full_path, - 'iid' => merge_request.iid - } - graphql_query_for('mergeRequest', attributes) - end - - context 'when the user has access to the merge request' do - before do - project.add_developer(current_user) - post_graphql(query, current_user: current_user) - end - - it 'returns the merge request' do - expect(graphql_data['mergeRequest']).not_to be_nil - end - - # This is a field coming from the `MergeRequestPresenter` - it 'includes a web_url' do - expect(graphql_data['mergeRequest']['webUrl']).to be_present - end - - it_behaves_like 'a working graphql query' - end - - context 'when the user does not have access to the merge request' do - before do - post_graphql(query, current_user: current_user) - end - - it 'returns an empty field' do - post_graphql(query, current_user: current_user) - - expect(graphql_data['mergeRequest']).to be_nil - end - - it_behaves_like 'a working graphql query' - end -end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index 8196bcfa87c..796ffc9d569 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -13,27 +13,76 @@ describe 'getting project information' do context 'when the user has access to the project' do before do project.add_developer(current_user) - post_graphql(query, current_user: current_user) end it 'includes the project' do + post_graphql(query, current_user: current_user) + expect(graphql_data['project']).not_to be_nil end - it_behaves_like 'a working graphql query' - end + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end - context 'when the user does not have access to the project' do - before do - post_graphql(query, current_user: current_user) + context 'when requesting a nested merge request' do + let(:merge_request) { create(:merge_request, source_project: project) } + let(:merge_request_graphql_data) { graphql_data['project']['mergeRequest'] } + + let(:query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('mergeRequest', iid: merge_request.iid) + ) + end + + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'contains merge request information' do + post_graphql(query, current_user: current_user) + + expect(merge_request_graphql_data).not_to be_nil + end + + # This is a field coming from the `MergeRequestPresenter` + it 'includes a web_url' do + post_graphql(query, current_user: current_user) + + expect(merge_request_graphql_data['webUrl']).to be_present + end + + context 'when the user does not have access to the merge request' do + let(:project) { create(:project, :public, :repository) } + + it 'returns nil' do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + + post_graphql(query) + + expect(merge_request_graphql_data).to be_nil + end + end end + end + context 'when the user does not have access to the project' do it 'returns an empty field' do post_graphql(query, current_user: current_user) expect(graphql_data['project']).to be_nil end - it_behaves_like 'a working graphql query' + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end end end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 30ff9a1196a..0930b9da368 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -34,14 +34,20 @@ module GraphqlHelpers end def graphql_query_for(name, attributes = {}, fields = nil) + <<~QUERY + { + #{query_graphql_field(name, attributes, fields)} + } + QUERY + end + + def query_graphql_field(name, attributes = {}, fields = nil) fields ||= all_graphql_fields_for(name.classify) attributes = attributes_to_graphql(attributes) <<~QUERY - { #{name}(#{attributes}) { #{fields} } - } QUERY end @@ -50,12 +56,15 @@ module GraphqlHelpers return "" unless type type.fields.map do |name, field| + # We can't guess arguments, so skip fields that require them + next if field.arguments.any? + if scalar?(field) name else "#{name} { #{all_graphql_fields_for(field_type(field))} }" end - end.join("\n") + end.compact.join("\n") end def attributes_to_graphql(attributes) diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb index ba7a1c8cde0..d23cbaf4beb 100644 --- a/spec/support/matchers/graphql_matchers.rb +++ b/spec/support/matchers/graphql_matchers.rb @@ -13,6 +13,12 @@ RSpec::Matchers.define :have_graphql_fields do |*expected| end end +RSpec::Matchers.define :have_graphql_field do |field_name| + match do |kls| + expect(kls.fields.keys).to include(GraphqlHelpers.fieldnamerize(field_name)) + end +end + RSpec::Matchers.define :have_graphql_arguments do |*expected| include GraphqlHelpers |