diff options
145 files changed, 2425 insertions, 817 deletions
diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 529a0de696b..6551f47d3be 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -139,7 +139,7 @@ setup-test-env: rspec unit pg: <<: *rspec-metadata-pg - parallel: 20 + parallel: 25 rspec integration pg: <<: *rspec-metadata-pg @@ -152,7 +152,7 @@ rspec system pg: rspec unit pg-10: <<: *rspec-metadata-pg-10 <<: *only-schedules-master - parallel: 20 + parallel: 25 rspec integration pg-10: <<: *rspec-metadata-pg-10 diff --git a/.gitlab/issue_templates/Database Reviewer.md b/.gitlab/issue_templates/Database Reviewer.md deleted file mode 100644 index acbaf5c1965..00000000000 --- a/.gitlab/issue_templates/Database Reviewer.md +++ /dev/null @@ -1,34 +0,0 @@ -#### Database Reviewer Checklist - -Thank you for becoming a ~database reviewer! Please work on the list -below to complete your setup. For any question, reach out to #database -an mention `@abrandl`. - -- [ ] Change issue title to include your name: `Database Reviewer Checklist: Your Name` -- [ ] Review general [code review guide](https://docs.gitlab.com/ee/development/code_review.html) -- [ ] Review [database review documentation](https://about.gitlab.com/handbook/engineering/workflow/code-review/database.html) -- [ ] Familiarize with [migration helpers](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/database/migration_helpers.rb) and review usage in existing migrations -- [ ] Read [database migration style guide](https://docs.gitlab.com/ee/development/migration_style_guide.html) -- [ ] Familiarize with best practices in [database guides](https://docs.gitlab.com/ee/development/#database-guides) -- [ ] Watch [Optimising Rails Database Queries: Episode 1](https://www.youtube.com/watch?v=79GurlaxhsI) -- [ ] Read [Understanding EXPLAIN plans](https://docs.gitlab.com/ee/development/understanding_explain_plans.html) -- [ ] Review [database best practices](https://docs.gitlab.com/ee/development/#best-practices) -- [ ] Review how we use [database instances restored from a backup](https://ops.gitlab.net/gitlab-com/gl-infra/gitlab-restore/postgres-gprd) for testing and make sure you're set up to execute pipelines (check [README.md](https://ops.gitlab.net/gitlab-com/gl-infra/gitlab-restore/postgres-gprd/blob/master/README.md) and reach out to @abrandl since this is currently subject to being changed) -- [ ] Get yourself added to [`@gl-database`](https://gitlab.com/groups/gl-database/-/group_members) group and respond to @-mentions to the group (reach out to any maintainer on the group to get added). You will get TODOs on gitlab.com for group mentions. -- [ ] Make sure you have proper access to at least a read-only replica in staging and production -- [ ] Indicate in `data/team.yml` your role as a database reviewer ([example MR](https://gitlab.com/gitlab-com/www-gitlab-com/merge_requests/19600/diffs)). Assign MR to your manager for merge. -- [ ] Send one MR to improve the [review documentation](https://about.gitlab.com/handbook/engineering/workflow/code-review/database.html) or the [issue template](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.gitlab/issue_templates/Database%20Reviewer.md) - -Note that *approving and accepting* merge requests is *restricted* to -Database Maintainers only. As a reviewer, pass the MR to a maintainer -for approval. - -You're all set! Watch out for TODOs on GitLab.com. - -###### Where to go for questions? - -Reach out to `#database` on Slack and mention `@abrandl` for any questions. - -cc @abrandl - -/label ~meta ~database diff --git a/Dangerfile b/Dangerfile index d0a605f8d8e..094d55e8652 100644 --- a/Dangerfile +++ b/Dangerfile @@ -19,4 +19,5 @@ unless helper.release_automation? danger.import_dangerfile(path: 'danger/single_codebase') danger.import_dangerfile(path: 'danger/gitlab_ui_wg') danger.import_dangerfile(path: 'danger/ce_ee_vue_templates') + danger.import_dangerfile(path: 'danger/only_documentation') end diff --git a/PROCESS.md b/PROCESS.md index 07b150ea463..22b68b0aaca 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -84,43 +84,28 @@ star, smile, etc.). Some good tips about code reviews can be found in our [Code Review Guidelines]: https://docs.gitlab.com/ce/development/code_review.html -## Feature freeze on the 7th for the release on the 22nd - -The feature freeze on the 7th has been discontinued. The [transition period overview](https://gitlab.com/gitlab-org/release/docs/blob/21cbd409dd5f157fe252f254f3e897f01908abe2/general/deploy/auto-deploy-transition.md#transition) -describes the change to this process. During the transition period, the only guarantee that -a change will be included in the release on the 22nd is if the change has been -deployed to GitLab.com prior to this date. +## Feature flags -### Feature flags +Overview and details of feature flag processes in development of GitLab itself is described in [feature flags process documentation](https://docs.gitlab.com/ee/development/feature_flags/process.html). -Merge requests that make changes hidden behind a feature flag, or remove an -existing feature flag because a feature is deemed stable, may be merged (and -picked into the stable branches) up to the 19th of the month. Such merge -requests should have the ~"feature flag" label assigned, and don't require a -corresponding exception request to be created. +Guides on how to include feature flags in your backend/frontend code while developing GitLab are described in [developing with feature flags documentation](https://docs.gitlab.com/ee/development/feature_flags/developing.html). -A level of common sense should be applied when deciding whether to have a feature -behind a feature flag off or on by default. +Getting access and how to expose the feature to users is detailed in [controlling feature flags documentation](https://docs.gitlab.com/ee/development/feature_flags/controls.html). -The following guidelines can be applied to help make this decision: +## Feature proposals from the 22nd to the 1st -* If the feature is not fully ready or functioning, the feature flag should be disabled by default. -* If the feature is ready but there are concerns about performance or impact, the feature flag should be enabled by default, but -disabled via chatops before deployment on GitLab.com environments. If the performance concern is confirmed, the final release should have the feature flag disabled by default. -* In most other cases, the feature flag can be enabled by default. +To allow the Product and Engineering teams time to discuss issues that will be placed into an upcoming milestone, +Product Managers must have their proposal for that milestone ready by the 22nd of each month. -For more information on rolling out changes using feature flags, read [through the documentation](https://docs.gitlab.com/ee/development/rolling_out_changes_using_feature_flags.html). +This proposal will be shared with Engineering for discussion, feedback, and planning. +The plan for the upcoming milestone must be finalized by the 1st of the month, one week before kickoff on the 8th. -In order to build the final package and present the feature for self-hosted -customers, the feature flag should be removed. This should happen before the -22nd, ideally _at least_ 2 days before. That means MRs with feature -flags being picked at the 19th would have quite a tight schedule, so picking -these _earlier_ is preferable. +## Feature freeze on the 7th for the release on the 22nd -While rare, release managers may decide to reject picking a change into a stable -branch, even when feature flags are used. This might be necessary if the changes -are deemed problematic, too invasive, or there simply isn't enough time to -properly test how the changes behave on GitLab.com. +The feature freeze on the 7th has been discontinued. [Transition period overview](https://gitlab.com/gitlab-org/release/docs/blob/21cbd409dd5f157fe252f254f3e897f01908abe2/general/deploy/auto-deploy-transition.md#transition) +describes the change to this process. During the transition period, the only guarantee that +a change will be included in the release on the 22nd is if the change has been +deployed to GitLab.com prior to this date. ### Between the 1st and the 7th diff --git a/app/assets/javascripts/monitoring/components/charts/area.vue b/app/assets/javascripts/monitoring/components/charts/area.vue index 9a3ce5174db..81773bd140e 100644 --- a/app/assets/javascripts/monitoring/components/charts/area.vue +++ b/app/assets/javascripts/monitoring/components/charts/area.vue @@ -1,5 +1,6 @@ <script> import { __ } from '~/locale'; +import { GlLink } from '@gitlab/ui'; import { GlAreaChart, GlChartSeriesLabel } from '@gitlab/ui/dist/charts'; import dateFormat from 'dateformat'; import { debounceByAnimationFrame, roundOffFloat } from '~/lib/utils/common_utils'; @@ -14,6 +15,7 @@ export default { components: { GlAreaChart, GlChartSeriesLabel, + GlLink, Icon, }, inheritAttrs: false, @@ -44,6 +46,10 @@ export default { required: false, default: () => [], }, + projectPath: { + type: String, + required: true, + }, thresholds: { type: Array, required: false, @@ -55,6 +61,7 @@ export default { tooltip: { title: '', content: [], + commitUrl: '', isDeployment: false, sha: '', }, @@ -195,12 +202,13 @@ export default { this.tooltip.title = dateFormat(params.value, 'dd mmm yyyy, h:MMTT'); this.tooltip.content = []; params.seriesData.forEach(seriesData => { - if (seriesData.componentSubType === graphTypes.deploymentData) { - this.tooltip.isDeployment = true; + this.tooltip.isDeployment = seriesData.componentSubType === graphTypes.deploymentData; + if (this.tooltip.isDeployment) { const [deploy] = this.recentDeployments.filter( deployment => deployment.createdAt === seriesData.value[0], ); this.tooltip.sha = deploy.sha.substring(0, 8); + this.tooltip.commitUrl = deploy.commitUrl; } else { const { seriesName, color } = seriesData; // seriesData.value contains the chart's [x, y] value pair @@ -259,7 +267,7 @@ export default { </template> <div slot="tooltipContent" class="d-flex align-items-center"> <icon name="commit" class="mr-2" /> - {{ tooltip.sha }} + <gl-link :href="tooltip.commitUrl">{{ tooltip.sha }}</gl-link> </div> </template> <template v-else> diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index 23687c54fd3..2cbda8ea05d 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -359,6 +359,7 @@ export default { <monitor-area-chart v-for="(graphData, graphIndex) in chartsWithData(groupData.metrics)" :key="graphIndex" + :project-path="projectPath" :graph-data="graphData" :deployment-data="deploymentData" :thresholds="getGraphAlertValues(graphData.queries)" diff --git a/app/assets/javascripts/monitoring/monitoring_bundle.js b/app/assets/javascripts/monitoring/monitoring_bundle.js index edbcf84b342..97d149e9ad5 100644 --- a/app/assets/javascripts/monitoring/monitoring_bundle.js +++ b/app/assets/javascripts/monitoring/monitoring_bundle.js @@ -8,10 +8,12 @@ export default (props = {}) => { const el = document.getElementById('prometheus-graphs'); if (el && el.dataset) { - store.dispatch('monitoringDashboard/setFeatureFlags', { - prometheusEndpointEnabled: gon.features.environmentMetricsUsePrometheusEndpoint, - multipleDashboardsEnabled: gon.features.environmentMetricsShowMultipleDashboards, - }); + if (gon.features) { + store.dispatch('monitoringDashboard/setFeatureFlags', { + prometheusEndpointEnabled: gon.features.environmentMetricsUsePrometheusEndpoint, + multipleDashboardsEnabled: gon.features.environmentMetricsShowMultipleDashboards, + }); + } const [currentDashboard] = getParameterValues('dashboard'); diff --git a/app/assets/javascripts/notes/components/discussion_actions.vue b/app/assets/javascripts/notes/components/discussion_actions.vue index 22cca756ef6..1357a5268d6 100644 --- a/app/assets/javascripts/notes/components/discussion_actions.vue +++ b/app/assets/javascripts/notes/components/discussion_actions.vue @@ -39,20 +39,27 @@ export default { </script> <template> - <div class="discussion-with-resolve-btn"> + <div class="discussion-with-resolve-btn clearfix"> <reply-placeholder class="qa-discussion-reply" @onClick="$emit('showReplyForm')" /> - <resolve-discussion-button - v-if="discussion.resolvable" - :is-resolving="isResolving" - :button-title="resolveButtonTitle" - @onClick="$emit('resolve')" - /> - <div v-if="discussion.resolvable" class="btn-group discussion-actions ml-sm-2" role="group"> - <resolve-with-issue-button v-if="resolveWithIssuePath" :url="resolveWithIssuePath" /> - <jump-to-next-discussion-button - v-if="shouldShowJumpToNextDiscussion" - @onClick="$emit('jumpToNextDiscussion')" + + <div class="btn-group discussion-actions" role="group"> + <resolve-discussion-button + v-if="discussion.resolvable" + :is-resolving="isResolving" + :button-title="resolveButtonTitle" + @onClick="$emit('resolve')" + /> + <resolve-with-issue-button + v-if="discussion.resolvable && resolveWithIssuePath" + :url="resolveWithIssuePath" /> </div> + + <div + v-if="discussion.resolvable && shouldShowJumpToNextDiscussion" + class="btn-group discussion-actions ml-sm-2" + > + <jump-to-next-discussion-button @onClick="$emit('jumpToNextDiscussion')" /> + </div> </div> </template> diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue index 228bb652597..30971ad5227 100644 --- a/app/assets/javascripts/notes/components/discussion_notes.vue +++ b/app/assets/javascripts/notes/components/discussion_notes.vue @@ -105,8 +105,8 @@ export default { :commit="commit" :help-page-path="helpPagePath" :show-reply-button="userCanReply" - @handle-delete-note="$emit('deleteNote')" - @start-replying="$emit('startReplying')" + @handleDeleteNote="$emit('deleteNote')" + @startReplying="$emit('startReplying')" > <note-edited-text v-if="discussion.resolved" @@ -132,7 +132,7 @@ export default { :note="componentData(note)" :help-page-path="helpPagePath" :line="line" - @handle-delete-note="$emit('deleteNote')" + @handleDeleteNote="$emit('deleteNote')" /> </template> </template> @@ -144,7 +144,7 @@ export default { :note="componentData(note)" :help-page-path="helpPagePath" :line="diffLine" - @handle-delete-note="$emit('deleteNote')" + @handleDeleteNote="$emit('deleteNote')" > <slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot> </component> diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 10b15a9c38c..b8eaff32cce 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -126,10 +126,7 @@ export default { return this.discussion.resolved_by_push ? __('Automatically resolved') : __('Resolved'); }, shouldShowJumpToNextDiscussion() { - return this.showJumpToNextDiscussion( - this.discussion.id, - this.discussionsByDiffOrder ? 'diff' : 'discussion', - ); + return this.showJumpToNextDiscussion(this.discussionsByDiffOrder ? 'diff' : 'discussion'); }, shouldRenderDiffs() { return this.discussion.diff_discussion && this.renderDiffFile; diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index d7982be3e4b..8aa8f5037b3 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -61,15 +61,13 @@ export const unresolvedDiscussionsCount = state => state.unresolvedDiscussionsCo export const resolvableDiscussionsCount = state => state.resolvableDiscussionsCount; export const hasUnresolvedDiscussions = state => state.hasUnresolvedDiscussions; -export const showJumpToNextDiscussion = (state, getters) => (discussionId, mode = 'discussion') => { +export const showJumpToNextDiscussion = (state, getters) => (mode = 'discussion') => { const orderedDiffs = mode !== 'discussion' ? getters.unresolvedDiscussionsIdsByDiff : getters.unresolvedDiscussionsIdsByDate; - const indexOf = orderedDiffs.indexOf(discussionId); - - return indexOf !== -1 && indexOf < orderedDiffs.length - 1; + return orderedDiffs.length > 1; }; export const isDiscussionResolved = (state, getters) => discussionId => diff --git a/app/assets/javascripts/performance_bar/components/detailed_metric.vue b/app/assets/javascripts/performance_bar/components/detailed_metric.vue index 8f3ba9779fb..d5f1cea8356 100644 --- a/app/assets/javascripts/performance_bar/components/detailed_metric.vue +++ b/app/assets/javascripts/performance_bar/components/detailed_metric.vue @@ -92,7 +92,9 @@ export default { </template> <template v-else> <tr> - <td>No {{ header.toLowerCase() }} for this request.</td> + <td> + {{ sprintf(__('No %{header} for this request.'), { header: header.toLowerCase() }) }} + </td> </tr> </template> </table> diff --git a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue index 48515cf785c..185003c306e 100644 --- a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue +++ b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue @@ -5,6 +5,7 @@ import { glEmojiTag } from '~/emoji'; import detailedMetric from './detailed_metric.vue'; import requestSelector from './request_selector.vue'; import simpleMetric from './simple_metric.vue'; +import { s__ } from '~/locale'; export default { components: { @@ -35,10 +36,10 @@ export default { }, }, detailedMetrics: [ - { metric: 'pg', header: 'SQL queries', details: 'queries', keys: ['sql'] }, + { metric: 'pg', header: s__('PerformanceBar|SQL queries'), details: 'queries', keys: ['sql'] }, { metric: 'gitaly', - header: 'Gitaly calls', + header: s__('PerformanceBar|Gitaly calls'), details: 'details', keys: ['feature', 'request'], }, @@ -99,7 +100,8 @@ export default { class="current-host" :class="{ canary: currentRequest.details.host.canary }" > - <span v-html="birdEmoji"></span> {{ currentRequest.details.host.hostname }} + <span v-html="birdEmoji"></span> + {{ currentRequest.details.host.hostname }} </span> </div> <detailed-metric @@ -118,9 +120,9 @@ export default { data-toggle="modal" data-target="#modal-peek-line-profile" > - profile + {{ s__('PerformanceBar|profile') }} </button> - <a v-else :href="profileUrl"> profile </a> + <a v-else :href="profileUrl">{{ s__('PerformanceBar|profile') }}</a> </div> <simple-metric v-for="metric in $options.simpleMetrics" @@ -139,7 +141,7 @@ export default { id="peek-view-trace" class="view" > - <a :href="currentRequest.details.tracing.tracing_url"> trace </a> + <a :href="currentRequest.details.tracing.tracing_url">{{ s__('PerformanceBar|trace') }}</a> </div> <request-selector v-if="currentRequest" diff --git a/app/assets/javascripts/reports/components/report_item.vue b/app/assets/javascripts/reports/components/report_item.vue index 01a30809e1a..2be9c37b00a 100644 --- a/app/assets/javascripts/reports/components/report_item.vue +++ b/app/assets/javascripts/reports/components/report_item.vue @@ -1,6 +1,6 @@ <script> import IssueStatusIcon from '~/reports/components/issue_status_icon.vue'; -import { components, componentNames } from '~/reports/components/issue_body'; +import { components, componentNames } from 'ee_else_ce/reports/components/issue_body'; export default { name: 'ReportItem', diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 41386178a1e..a79da476890 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -162,7 +162,8 @@ export default { removeWIPPath: store.removeWIPPath, sourceBranchPath: store.sourceBranchPath, ciEnvironmentsStatusPath: store.ciEnvironmentsStatusPath, - statusPath: store.statusPath, + mergeRequestBasicPath: store.mergeRequestBasicPath, + mergeRequestWidgetPath: store.mergeRequestWidgetPath, mergeActionsContentPath: store.mergeActionsContentPath, rebasePath: store.rebasePath, }; diff --git a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js index 0bb70bfd658..1dae53039d5 100644 --- a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js +++ b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js @@ -30,11 +30,11 @@ export default class MRWidgetService { } poll() { - return axios.get(`${this.endpoints.statusPath}?serializer=basic`); + return axios.get(this.endpoints.mergeRequestBasicPath); } checkStatus() { - return axios.get(`${this.endpoints.statusPath}?serializer=widget`); + return axios.get(this.endpoints.mergeRequestWidgetPath); } fetchMergeActionsContent() { diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index bfa3e7f4a59..581fee7477f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -86,7 +86,8 @@ export default class MergeRequestStore { this.mergePath = data.merge_path; this.ffOnlyEnabled = data.ff_only_enabled; this.shouldBeRebased = Boolean(data.should_be_rebased); - this.statusPath = data.status_path; + this.mergeRequestBasicPath = data.merge_request_basic_path; + this.mergeRequestWidgetPath = data.merge_request_widget_path; this.emailPatchesPath = data.email_patches_path; this.plainDiffPath = data.plain_diff_path; this.newBlobPath = data.new_blob_path; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 824edb2869f..e880b941d67 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -657,6 +657,10 @@ $note-form-margin-left: 72px; margin-left: -1px; } + .btn-group > .discussion-create-issue-btn { + margin-left: -2px; + } + svg { height: 15px; } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7321f719deb..75108bf2646 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -516,4 +516,10 @@ class ApplicationController < ActionController::Base def sentry_context Gitlab::Sentry.context(current_user) end + + def allow_gitaly_ref_name_caching + ::Gitlab::GitalyClient.allow_ref_name_caching do + yield + end + end end diff --git a/app/controllers/groups/clusters_controller.rb b/app/controllers/groups/clusters_controller.rb index b846fb21266..92602fd8096 100644 --- a/app/controllers/groups/clusters_controller.rb +++ b/app/controllers/groups/clusters_controller.rb @@ -4,7 +4,6 @@ class Groups::ClustersController < Clusters::ClustersController include ControllerWithCrossProjectAccessCheck prepend_before_action :group - prepend_before_action :check_group_clusters_feature_flag! requires_cross_project_access layout 'group' @@ -18,12 +17,4 @@ class Groups::ClustersController < Clusters::ClustersController def group @group ||= find_routable!(Group, params[:group_id] || params[:id]) end - - def check_group_clusters_feature_flag! - render_404 unless group_clusters_enabled? - end - - def group_clusters_enabled? - group.group_clusters_enabled? - end end diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 80e4f54bbf4..90396c15375 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -87,10 +87,4 @@ class Projects::ApplicationController < ApplicationController def check_issues_available! return render_404 unless @project.feature_available?(:issues, current_user) end - - def allow_gitaly_ref_name_caching - ::Gitlab::GitalyClient.allow_ref_name_caching do - yield - end - end end diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index f2a6268b3e9..dcc272aecff 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -51,4 +51,11 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont Ci::Pipeline.none end end + + def close_merge_request_if_no_source_project + return if @merge_request.source_project + return unless @merge_request.open? + + @merge_request.close + end end diff --git a/app/controllers/projects/merge_requests/content_controller.rb b/app/controllers/projects/merge_requests/content_controller.rb new file mode 100644 index 00000000000..6e026b83ee3 --- /dev/null +++ b/app/controllers/projects/merge_requests/content_controller.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class Projects::MergeRequests::ContentController < Projects::MergeRequests::ApplicationController + # @merge_request.check_mergeability is not executed here since + # widget serializer calls it via mergeable? method + # but we might want to call @merge_request.check_mergeability + # for other types of serialization + + before_action :close_merge_request_if_no_source_project + around_action :allow_gitaly_ref_name_caching + + def widget + respond_to do |format| + format.json do + Gitlab::PollingInterval.set_header(response, interval: 10_000) + + serializer = MergeRequestSerializer.new(current_user: current_user, project: merge_request.project) + render json: serializer.represent(merge_request, serializer: 'widget') + end + end + end +end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index fc37ce1dbc4..7ee8e0ea8f8 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -235,12 +235,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo params[:auto_merge_strategy].present? || params[:merge_when_pipeline_succeeds].present? end - def close_merge_request_if_no_source_project - if !@merge_request.source_project && @merge_request.open? - @merge_request.close - end - end - private def ci_environments_status_on_merge_result? diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index a80ab3bcd28..8c674be58c5 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -5,6 +5,8 @@ class SearchController < ApplicationController include SearchHelper include RendersCommits + around_action :allow_gitaly_ref_name_caching + skip_before_action :authenticate_user! requires_cross_project_access if: -> do search_term_present = params[:search].present? || params[:term].present? diff --git a/app/graphql/mutations/.keep b/app/graphql/mutations/.keep deleted file mode 100644 index e69de29bb2d..00000000000 --- a/app/graphql/mutations/.keep +++ /dev/null diff --git a/app/graphql/mutations/award_emojis/add.rb b/app/graphql/mutations/award_emojis/add.rb new file mode 100644 index 00000000000..8e050dd6d29 --- /dev/null +++ b/app/graphql/mutations/award_emojis/add.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Mutations + module AwardEmojis + class Add < Base + graphql_name 'AddAwardEmoji' + + def resolve(args) + awardable = authorized_find!(id: args[:awardable_id]) + + check_object_is_awardable!(awardable) + + # TODO this will be handled by AwardEmoji::AddService + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 + award = awardable.create_award_emoji(args[:name], current_user) + + { + award_emoji: (award if award.persisted?), + errors: errors_on_object(award) + } + end + end + end +end diff --git a/app/graphql/mutations/award_emojis/base.rb b/app/graphql/mutations/award_emojis/base.rb new file mode 100644 index 00000000000..d868db84f9d --- /dev/null +++ b/app/graphql/mutations/award_emojis/base.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Mutations + module AwardEmojis + class Base < BaseMutation + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorize :award_emoji + + argument :awardable_id, + GraphQL::ID_TYPE, + required: true, + description: 'The global id of the awardable resource' + + argument :name, + GraphQL::STRING_TYPE, + required: true, + description: copy_field_description(Types::AwardEmojis::AwardEmojiType, :name) + + field :award_emoji, + Types::AwardEmojis::AwardEmojiType, + null: true, + description: 'The award emoji after mutation' + + private + + def find_object(id:) + GitlabSchema.object_from_id(id) + end + + # Called by mutations methods after performing an authorization check + # of an awardable object. + def check_object_is_awardable!(object) + unless object.is_a?(Awardable) && object.emoji_awardable? + raise Gitlab::Graphql::Errors::ResourceNotAvailable, + 'Cannot award emoji to this resource' + end + end + end + end +end diff --git a/app/graphql/mutations/award_emojis/remove.rb b/app/graphql/mutations/award_emojis/remove.rb new file mode 100644 index 00000000000..3ba85e445b8 --- /dev/null +++ b/app/graphql/mutations/award_emojis/remove.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Mutations + module AwardEmojis + class Remove < Base + graphql_name 'RemoveAwardEmoji' + + def resolve(args) + awardable = authorized_find!(id: args[:awardable_id]) + + check_object_is_awardable!(awardable) + + # TODO this check can be removed once AwardEmoji services are available. + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 + unless awardable.awarded_emoji?(args[:name], current_user) + raise Gitlab::Graphql::Errors::ResourceNotAvailable, + 'You have not awarded emoji of type name to the awardable' + end + + # TODO this will be handled by AwardEmoji::DestroyService + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 + awardable.remove_award_emoji(args[:name], current_user) + + { + # Mutation response is always a `nil` award_emoji + errors: [] + } + end + end + end +end diff --git a/app/graphql/mutations/award_emojis/toggle.rb b/app/graphql/mutations/award_emojis/toggle.rb new file mode 100644 index 00000000000..c03902e8035 --- /dev/null +++ b/app/graphql/mutations/award_emojis/toggle.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Mutations + module AwardEmojis + class Toggle < Base + graphql_name 'ToggleAwardEmoji' + + field :toggledOn, + GraphQL::BOOLEAN_TYPE, + null: false, + description: 'True when the emoji was awarded, false when it was removed' + + def resolve(args) + awardable = authorized_find!(id: args[:awardable_id]) + + check_object_is_awardable!(awardable) + + # TODO this will be handled by AwardEmoji::ToggleService + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 + award = awardable.toggle_award_emoji(args[:name], current_user) + + # Destroy returns a collection :( + award = award.first if award.is_a?(Array) + + errors = errors_on_object(award) + + toggled_on = awardable.awarded_emoji?(args[:name], current_user) + + { + # For consistency with the AwardEmojis::Remove mutation, only return + # the AwardEmoji if it was created and not destroyed + award_emoji: (award if toggled_on), + errors: errors, + toggled_on: toggled_on + } + end + end + end +end diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index eb03dfe1624..08d2a1f18a3 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/app/graphql/mutations/base_mutation.rb @@ -2,6 +2,8 @@ module Mutations class BaseMutation < GraphQL::Schema::RelayClassicMutation + prepend Gitlab::Graphql::CopyFieldDescription + field :errors, [GraphQL::STRING_TYPE], null: false, description: "Reasons why the mutation failed." @@ -9,5 +11,10 @@ module Mutations def current_user context[:current_user] end + + # Returns Array of errors on an ActiveRecord object + def errors_on_object(record) + record.errors.full_messages + end end end diff --git a/app/graphql/types/award_emojis/award_emoji_type.rb b/app/graphql/types/award_emojis/award_emoji_type.rb new file mode 100644 index 00000000000..8daf699a112 --- /dev/null +++ b/app/graphql/types/award_emojis/award_emoji_type.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Types + module AwardEmojis + class AwardEmojiType < BaseObject + graphql_name 'AwardEmoji' + + authorize :read_emoji + + present_using AwardEmojiPresenter + + field :name, + GraphQL::STRING_TYPE, + null: false, + description: 'The emoji name' + + field :description, + GraphQL::STRING_TYPE, + null: false, + description: 'The emoji description' + + field :unicode, + GraphQL::STRING_TYPE, + null: false, + description: 'The emoji in unicode' + + field :emoji, + GraphQL::STRING_TYPE, + null: false, + description: 'The emoji as an icon' + + field :unicode_version, + GraphQL::STRING_TYPE, + null: false, + description: 'The unicode version for this emoji' + + field :user, + Types::UserType, + null: false, + description: 'The user who awarded the emoji', + resolve: -> (award_emoji, _args, _context) { + Gitlab::Graphql::Loaders::BatchModelLoader.new(User, award_emoji.user_id).find + } + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 2b4ef299296..6ef1d816b7c 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -6,6 +6,9 @@ module Types graphql_name "Mutation" + mount_mutation Mutations::AwardEmojis::Add + mount_mutation Mutations::AwardEmojis::Remove + mount_mutation Mutations::AwardEmojis::Toggle mount_mutation Mutations::MergeRequests::SetWip end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index a3f53ca8dd6..5aed7e313e6 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -142,7 +142,7 @@ module GroupsHelper can?(current_user, "read_group_#{resource}".to_sym, @group) end - if can?(current_user, :read_cluster, @group) && @group.group_clusters_enabled? + if can?(current_user, :read_cluster, @group) links << :kubernetes end diff --git a/app/models/board.rb b/app/models/board.rb index e08db764f65..50b6ca9b70f 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -4,11 +4,14 @@ class Board < ApplicationRecord belongs_to :group belongs_to :project - has_many :lists, -> { order(:list_type, :position) }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :lists, -> { ordered }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :destroyable_lists, -> { destroyable.ordered }, class_name: "List" validates :project, presence: true, if: :project_needed? validates :group, presence: true, unless: :project + scope :with_associations, -> { preload(:destroyable_lists) } + def project_needed? !group end diff --git a/app/models/clusters/instance.rb b/app/models/clusters/instance.rb index d8a888d53ba..f21dbdf7f26 100644 --- a/app/models/clusters/instance.rb +++ b/app/models/clusters/instance.rb @@ -9,9 +9,5 @@ module Clusters def feature_available?(feature) ::Feature.enabled?(feature, default_enabled: true) end - - def self.enabled? - ::Feature.enabled?(:instance_clusters, default_enabled: true) - end end end diff --git a/app/models/concerns/deployment_platform.rb b/app/models/concerns/deployment_platform.rb index 1bd8a799f0d..5a358ae2ef6 100644 --- a/app/models/concerns/deployment_platform.rb +++ b/app/models/concerns/deployment_platform.rb @@ -13,8 +13,8 @@ module DeploymentPlatform def find_deployment_platform(environment) find_cluster_platform_kubernetes(environment: environment) || - find_group_cluster_platform_kubernetes_with_feature_guard(environment: environment) || - find_instance_cluster_platform_kubernetes_with_feature_guard(environment: environment) + find_group_cluster_platform_kubernetes(environment: environment) || + find_instance_cluster_platform_kubernetes(environment: environment) end # EE would override this and utilize environment argument @@ -23,24 +23,12 @@ module DeploymentPlatform .last&.platform_kubernetes end - def find_group_cluster_platform_kubernetes_with_feature_guard(environment: nil) - return unless group_clusters_enabled? - - find_group_cluster_platform_kubernetes(environment: environment) - end - # EE would override this and utilize environment argument def find_group_cluster_platform_kubernetes(environment: nil) Clusters::Cluster.enabled.default_environment.ancestor_clusters_for_clusterable(self) .first&.platform_kubernetes end - def find_instance_cluster_platform_kubernetes_with_feature_guard(environment: nil) - return unless Clusters::Instance.enabled? - - find_instance_cluster_platform_kubernetes(environment: environment) - end - # EE would override this and utilize environment argument def find_instance_cluster_platform_kubernetes(environment: nil) Clusters::Instance.new.clusters.enabled.default_environment diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index 46d2c345758..22b6b1d720c 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -25,7 +25,7 @@ module RelativePositioning relative_position = position_between(max_relative_position, MAX_POSITION) object.relative_position = relative_position max_relative_position = relative_position - object.save + object.save(touch: false) end end end @@ -159,7 +159,7 @@ module RelativePositioning def save_positionable_neighbours return unless @positionable_neighbours - status = @positionable_neighbours.all?(&:save) + status = @positionable_neighbours.all? { |issue| issue.save(touch: false) } @positionable_neighbours = nil status diff --git a/app/models/group.rb b/app/models/group.rb index dbec211935d..8e89c7ecfb1 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -410,10 +410,6 @@ class Group < Namespace ensure_runners_token! end - def group_clusters_enabled? - Feature.enabled?(:group_clusters, root_ancestor, default_enabled: true) - end - def project_creation_level super || ::Gitlab::CurrentSettings.default_project_creation end diff --git a/app/models/list.rb b/app/models/list.rb index 17b1a8510cf..d28a9bda82d 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -16,6 +16,7 @@ class List < ApplicationRecord scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) } scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) } scope :preload_associations, -> { preload(:board, :label) } + scope :ordered, -> { order(:list_type, :position) } class << self def destroyable_types diff --git a/app/models/postgresql/replication_slot.rb b/app/models/postgresql/replication_slot.rb index 74ccf23cf69..7a123deb719 100644 --- a/app/models/postgresql/replication_slot.rb +++ b/app/models/postgresql/replication_slot.rb @@ -28,7 +28,7 @@ module Postgresql # We force the use of a transaction here so the query always goes to the # primary, even when using the EE DB load balancer. sizes = transaction { pluck(lag_function) } - too_great = sizes.count { |size| size >= max } + too_great = sizes.compact.count { |size| size >= max } # If too many replicas are falling behind too much, the availability of a # GitLab instance might suffer. To prevent this from happening we require diff --git a/app/models/project.rb b/app/models/project.rb index c642b65f674..b102e0580e7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -306,7 +306,6 @@ class Project < ApplicationRecord delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_role, to: :team delegate :add_master, to: :team # @deprecated delegate :group_runners_enabled, :group_runners_enabled=, :group_runners_enabled?, to: :ci_cd_settings - delegate :group_clusters_enabled?, to: :group, allow_nil: true delegate :root_ancestor, to: :namespace, allow_nil: true delegate :last_pipeline, to: :commit, allow_nil: true delegate :external_dashboard_url, to: :metrics_setting, allow_nil: true, prefix: true diff --git a/app/models/snippet.rb b/app/models/snippet.rb index f4fdac2558c..00931457344 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -194,6 +194,10 @@ class Snippet < ApplicationRecord 'snippet' end + def to_ability_name + model_name.singular + end + class << self # Searches for snippets with a matching title or file name. # diff --git a/app/policies/award_emoji_policy.rb b/app/policies/award_emoji_policy.rb new file mode 100644 index 00000000000..21e382e24b3 --- /dev/null +++ b/app/policies/award_emoji_policy.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AwardEmojiPolicy < BasePolicy + delegate { @subject.awardable if DeclarativePolicy.has_policy?(@subject.awardable) } + + condition(:can_read_awardable) do + can?(:"read_#{@subject.awardable.to_ability_name}") + end + + rule { can_read_awardable }.enable :read_emoji +end diff --git a/app/policies/clusters/instance_policy.rb b/app/policies/clusters/instance_policy.rb index e1045c85e6d..f72096e8fc6 100644 --- a/app/policies/clusters/instance_policy.rb +++ b/app/policies/clusters/instance_policy.rb @@ -6,9 +6,8 @@ module Clusters condition(:has_clusters, scope: :subject) { clusterable_has_clusters? } condition(:can_have_multiple_clusters) { multiple_clusters_available? } - condition(:instance_clusters_enabled) { Instance.enabled? } - rule { admin & instance_clusters_enabled }.policy do + rule { admin }.policy do enable :read_cluster enable :add_cluster enable :create_cluster diff --git a/app/presenters/award_emoji_presenter.rb b/app/presenters/award_emoji_presenter.rb new file mode 100644 index 00000000000..98713855d35 --- /dev/null +++ b/app/presenters/award_emoji_presenter.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class AwardEmojiPresenter < Gitlab::View::Presenter::Delegated + presents :award_emoji + + def description + as_emoji['description'] + end + + def unicode + as_emoji['unicode'] + end + + def emoji + as_emoji['moji'] + end + + def unicode_version + Gitlab::Emoji.emoji_unicode_version(award_emoji.name) + end + + private + + def as_emoji + @emoji ||= Gitlab::Emoji.emojis[award_emoji.name] || {} + end +end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 43aced598a9..fd2673fa0cc 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -217,8 +217,12 @@ class MergeRequestWidgetEntity < IssuableEntity project_merge_request_path(merge_request.project, merge_request, format: :diff) end - expose :status_path do |merge_request| - project_merge_request_path(merge_request.target_project, merge_request, format: :json) + expose :merge_request_basic_path do |merge_request| + project_merge_request_path(merge_request.target_project, merge_request, serializer: :basic, format: :json) + end + + expose :merge_request_widget_path do |merge_request| + widget_project_json_merge_request_path(merge_request.target_project, merge_request, format: :json) end expose :ci_environments_status_path do |merge_request| diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 26132f1824a..02de080e0ba 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -205,7 +205,7 @@ class IssuableBaseService < BaseService end if issuable.changed? || params.present? - issuable.assign_attributes(params.merge(updated_by: current_user)) + issuable.assign_attributes(params) if has_title_or_description_changed?(issuable) issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user) @@ -213,11 +213,16 @@ class IssuableBaseService < BaseService before_update(issuable) + # Do not touch when saving the issuable if only changes position within a list. We should call + # this method at this point to capture all possible changes. + should_touch = update_timestamp?(issuable) + + issuable.updated_by = current_user if should_touch # We have to perform this check before saving the issuable as Rails resets # the changed fields upon calling #save. update_project_counters = issuable.project && update_project_counter_caches?(issuable) - if issuable.with_transaction_returning_status { issuable.save } + if issuable.with_transaction_returning_status { issuable.save(touch: should_touch) } # We do not touch as it will affect a update on updated_at field ActiveRecord::Base.no_touching do Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: old_associations[:labels]) @@ -402,4 +407,8 @@ class IssuableBaseService < BaseService def ensure_milestone_available(issuable) issuable.milestone_id = nil unless issuable.milestone_available? end + + def update_timestamp?(issuable) + issuable.changes.keys != ["relative_position"] + end end diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index 15c13a452ad..8f52e9cb23f 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -63,12 +63,20 @@ module Users def assign_identity return unless identity_params.present? - identity = user.identities.find_or_create_by(provider: identity_params[:provider]) # rubocop: disable CodeReuse/ActiveRecord + identity = user.identities.find_or_create_by(provider_params) # rubocop: disable CodeReuse/ActiveRecord identity.update(identity_params) end def identity_attributes [:provider, :extern_uid] end + + def provider_attributes + [:provider] + end + + def provider_params + identity_params.slice(*provider_attributes) + end end end diff --git a/app/views/projects/_merge_request_settings_description_text.html.haml b/app/views/projects/_merge_request_settings_description_text.html.haml new file mode 100644 index 00000000000..42964c900b3 --- /dev/null +++ b/app/views/projects/_merge_request_settings_description_text.html.haml @@ -0,0 +1 @@ +%p= s_('ProjectSettings|Choose your merge method, merge options, and merge checks.') diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index c15b84d0aac..29b7c45201c 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -27,7 +27,7 @@ .settings-header %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Merge requests') %button.btn.btn-default.js-settings-toggle{ type: 'button' }= expanded ? _('Collapse') : _('Expand') - %p= _('Choose your merge method, options, checks, and set up a default merge request description template.') + = render_if_exists 'projects/merge_request_settings_description_text' .settings-content = render_if_exists 'shared/promotions/promote_mr_features' diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml index d59b2d4fb01..c13a47b0b09 100644 --- a/app/views/projects/environments/show.html.haml +++ b/app/views/projects/environments/show.html.haml @@ -31,21 +31,19 @@ = button_to stop_project_environment_path(@project, @environment), class: 'btn btn-danger has-tooltip', method: :post do = s_('Environments|Stop environment') - .row.top-area.adjust - .col-md-7 - %h3.page-title= @environment.name - .col-md-5 - .nav-controls - = render 'projects/environments/terminal_button', environment: @environment - = render 'projects/environments/external_url', environment: @environment - = render 'projects/environments/metrics_button', environment: @environment - - if can?(current_user, :update_environment, @environment) - = link_to _('Edit'), edit_project_environment_path(@project, @environment), class: 'btn' - - if can?(current_user, :stop_environment, @environment) - = button_tag class: 'btn btn-danger', type: 'button', data: { toggle: 'modal', - target: '#stop-environment-modal' } do - = sprite_icon('stop') - = s_('Environments|Stop') + .top-area + %h3.page-title= @environment.name + .nav-controls.ml-auto.my-2 + = render 'projects/environments/terminal_button', environment: @environment + = render 'projects/environments/external_url', environment: @environment + = render 'projects/environments/metrics_button', environment: @environment + - if can?(current_user, :update_environment, @environment) + = link_to _('Edit'), edit_project_environment_path(@project, @environment), class: 'btn' + - if can?(current_user, :stop_environment, @environment) + = button_tag class: 'btn btn-danger', type: 'button', data: { toggle: 'modal', + target: '#stop-environment-modal' } do + = sprite_icon('stop') + = s_('Environments|Stop') .environments-container - if @deployments.blank? diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index ea6349f2f57..1d0bc588c9c 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -76,6 +76,7 @@ #{ _('New tag') } .tree-controls + = render_if_exists 'projects/tree/lock_link' = link_to s_('Commits|History'), project_commits_path(@project, @id), class: 'btn' = render 'projects/find_file_link' diff --git a/app/views/shared/projects/_list.html.haml b/app/views/shared/projects/_list.html.haml index 13847cd9be1..576ec3e1782 100644 --- a/app/views/shared/projects/_list.html.haml +++ b/app/views/shared/projects/_list.html.haml @@ -28,7 +28,7 @@ .js-projects-list-holder - if any_projects?(projects) - - load_pipeline_status(projects) + - load_pipeline_status(projects) if pipeline_status %ul.projects-list{ class: css_classes } - projects.each_with_index do |project, i| - css_class = (i >= projects_limit) || project.pending_delete? ? 'hide' : nil diff --git a/changelogs/unreleased/11888-regression-deploy-correlation-markers-on-monitoring-graphs-not-clickable.yml b/changelogs/unreleased/11888-regression-deploy-correlation-markers-on-monitoring-graphs-not-clickable.yml new file mode 100644 index 00000000000..606abe818b4 --- /dev/null +++ b/changelogs/unreleased/11888-regression-deploy-correlation-markers-on-monitoring-graphs-not-clickable.yml @@ -0,0 +1,5 @@ +--- +title: Turn commit sha in monitor charts popover to link +merge_request: 29914 +author: +type: fixed diff --git a/changelogs/unreleased/44949-do-not-update-updated_at-on-an-issue-when-reordering-it.yml b/changelogs/unreleased/44949-do-not-update-updated_at-on-an-issue-when-reordering-it.yml new file mode 100644 index 00000000000..efc6af7845c --- /dev/null +++ b/changelogs/unreleased/44949-do-not-update-updated_at-on-an-issue-when-reordering-it.yml @@ -0,0 +1,5 @@ +--- +title: Will not update issue timestamps when changing positions in a list +merge_request: 29677 +author: +type: changed diff --git a/changelogs/unreleased/58689-regroup-jump-button-in-discussion.yml b/changelogs/unreleased/58689-regroup-jump-button-in-discussion.yml new file mode 100644 index 00000000000..bf6f314f0ce --- /dev/null +++ b/changelogs/unreleased/58689-regroup-jump-button-in-discussion.yml @@ -0,0 +1,6 @@ +--- +title: Improve discussion reply buttons layout and how jump to next discussion button + appears +merge_request: 29779 +author: +type: changed diff --git a/changelogs/unreleased/62826-graphql-emoji-mutations.yml b/changelogs/unreleased/62826-graphql-emoji-mutations.yml new file mode 100644 index 00000000000..0c0aaedf844 --- /dev/null +++ b/changelogs/unreleased/62826-graphql-emoji-mutations.yml @@ -0,0 +1,5 @@ +--- +title: GraphQL mutations for add, remove and toggle emoji +merge_request: 29919 +author: +type: added diff --git a/changelogs/unreleased/62968-environment-details-header-border-misaligned.yml b/changelogs/unreleased/62968-environment-details-header-border-misaligned.yml new file mode 100644 index 00000000000..749fe6a9cb0 --- /dev/null +++ b/changelogs/unreleased/62968-environment-details-header-border-misaligned.yml @@ -0,0 +1,5 @@ +--- +title: Resolve Environment details header border misaligned +merge_request: 30011 +author: +type: fixed diff --git a/changelogs/unreleased/63200-reply-button-broken.yml b/changelogs/unreleased/63200-reply-button-broken.yml new file mode 100644 index 00000000000..11f81dbd925 --- /dev/null +++ b/changelogs/unreleased/63200-reply-button-broken.yml @@ -0,0 +1,5 @@ +--- +title: Fix unresponsive reply button in discussions +merge_request: 29936 +author: +type: fixed diff --git a/changelogs/unreleased/add-metrics-dashboard-permission-check.yml b/changelogs/unreleased/add-metrics-dashboard-permission-check.yml new file mode 100644 index 00000000000..0ea2c4c8e41 --- /dev/null +++ b/changelogs/unreleased/add-metrics-dashboard-permission-check.yml @@ -0,0 +1,5 @@ +--- +title: Add permission check to metrics dashboards endpoint +merge_request: 30017 +author: +type: added diff --git a/changelogs/unreleased/ce-11098-update-merge-request-settings-description-text.yml b/changelogs/unreleased/ce-11098-update-merge-request-settings-description-text.yml new file mode 100644 index 00000000000..9f6a2040095 --- /dev/null +++ b/changelogs/unreleased/ce-11098-update-merge-request-settings-description-text.yml @@ -0,0 +1,5 @@ +--- +title: Update merge requests section description text on project settings page +merge_request: 27838 +author: +type: changed
\ No newline at end of file diff --git a/changelogs/unreleased/dohtaset.yml b/changelogs/unreleased/dohtaset.yml new file mode 100644 index 00000000000..5b917bd06d8 --- /dev/null +++ b/changelogs/unreleased/dohtaset.yml @@ -0,0 +1,5 @@ +--- +title: Fix charts on Cluster health page +merge_request: 30073 +author: +type: fixed diff --git a/changelogs/unreleased/id-extract-widget-into-different-request.yml b/changelogs/unreleased/id-extract-widget-into-different-request.yml new file mode 100644 index 00000000000..3b9f5fdd6bd --- /dev/null +++ b/changelogs/unreleased/id-extract-widget-into-different-request.yml @@ -0,0 +1,5 @@ +--- +title: Add a separate endpoint for fetching MRs serialized as widgets +merge_request: 29979 +author: +type: performance diff --git a/changelogs/unreleased/po-raw-changes-encoding.yml b/changelogs/unreleased/po-raw-changes-encoding.yml new file mode 100644 index 00000000000..051d18f50c7 --- /dev/null +++ b/changelogs/unreleased/po-raw-changes-encoding.yml @@ -0,0 +1,5 @@ +--- +title: Expect bytes from Gitaly RPC GetRawChanges +merge_request: 28164 +author: +type: fixed diff --git a/changelogs/unreleased/remove_group_and_instance_clusters_feature_flag.yml b/changelogs/unreleased/remove_group_and_instance_clusters_feature_flag.yml new file mode 100644 index 00000000000..fcc6c564345 --- /dev/null +++ b/changelogs/unreleased/remove_group_and_instance_clusters_feature_flag.yml @@ -0,0 +1,5 @@ +--- +title: Remove group and instance clusters feature flag +merge_request: 30124 +author: +type: changed diff --git a/changelogs/unreleased/sh-add-force-random-password-user-api.yml b/changelogs/unreleased/sh-add-force-random-password-user-api.yml new file mode 100644 index 00000000000..29f36978a0f --- /dev/null +++ b/changelogs/unreleased/sh-add-force-random-password-user-api.yml @@ -0,0 +1,5 @@ +--- +title: Add support for creating random passwords in user creation API +merge_request: 30138 +author: +type: changed diff --git a/changelogs/unreleased/sh-add-gitaly-ref-caching-search-controller.yml b/changelogs/unreleased/sh-add-gitaly-ref-caching-search-controller.yml new file mode 100644 index 00000000000..d4be28e9883 --- /dev/null +++ b/changelogs/unreleased/sh-add-gitaly-ref-caching-search-controller.yml @@ -0,0 +1,5 @@ +--- +title: Enable Gitaly ref caching for SearchController +merge_request: 30105 +author: +type: performance diff --git a/changelogs/unreleased/sh-avoid-loading-pipeline-status.yml b/changelogs/unreleased/sh-avoid-loading-pipeline-status.yml new file mode 100644 index 00000000000..2dead948786 --- /dev/null +++ b/changelogs/unreleased/sh-avoid-loading-pipeline-status.yml @@ -0,0 +1,5 @@ +--- +title: Avoid loading pipeline status in search results +merge_request: 30111 +author: +type: performance diff --git a/changelogs/unreleased/sh-handle-nil-replication-lag.yml b/changelogs/unreleased/sh-handle-nil-replication-lag.yml new file mode 100644 index 00000000000..5638d7e79e3 --- /dev/null +++ b/changelogs/unreleased/sh-handle-nil-replication-lag.yml @@ -0,0 +1,5 @@ +--- +title: Fix background migrations failing with unused replication slot +merge_request: 30042 +author: +type: fixed diff --git a/changelogs/unreleased/support-jsonb-default-value.yml b/changelogs/unreleased/support-jsonb-default-value.yml new file mode 100644 index 00000000000..d46156276f9 --- /dev/null +++ b/changelogs/unreleased/support-jsonb-default-value.yml @@ -0,0 +1,5 @@ +--- +title: Support jsonb default in add_column_with_default migration helper +merge_request: 29871 +author: +type: other diff --git a/config/routes/project.rb b/config/routes/project.rb index bcbbd7222e0..91613e3333f 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -261,6 +261,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :commits get :pipelines get :diffs, to: 'merge_requests/diffs#show' + get :widget, to: 'merge_requests/content#widget' end get :diff_for_path, controller: 'merge_requests/diffs' diff --git a/danger/only_documentation/Dangerfile b/danger/only_documentation/Dangerfile new file mode 100644 index 00000000000..8e4564f22b6 --- /dev/null +++ b/danger/only_documentation/Dangerfile @@ -0,0 +1,24 @@ +# rubocop:disable Style/SignalException +# frozen_string_literal: true + +has_only_docs_changes = helper.all_changed_files.all? { |file| file.start_with?('doc/') } +is_docs_only_branch = gitlab.branch_for_head =~ /(^docs[\/-].*|.*-docs$)/ + +if is_docs_only_branch && !has_only_docs_changes + fail "It seems like your branch name has a `docs` prefix or suffix. "\ + "The CI won't run the full pipeline, but you also have changed non-docs files. "\ + "Please recreate this MR with a new branch name." +end + +if has_only_docs_changes && !is_docs_only_branch + markdown(<<~MARKDOWN) + + ## Documentation only changes + + Hey! Seems your merge request contains only docs changes. + Tired of waiting 2 hours for the pipeline to finish? + Next time, prepend `docs-` to [your branch name](https://docs.gitlab.com/ee/development/documentation/#branch-naming) + and the pipeline will finish before you say GitLab (x300)! + + MARKDOWN +end diff --git a/doc/api/users.md b/doc/api/users.md index 4bc0335ae33..4667a985eb9 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -272,7 +272,14 @@ GET /users/:id?with_custom_attributes=true ## User creation -Creates a new user. Note only administrators can create new users. Either `password` or `reset_password` should be specified (`reset_password` takes priority). If `reset_password` is `false`, then `password` is required. +Creates a new user. Note only administrators can create new +users. Either `password`, `reset_password`, or `force_random_password` +must be specified. If `reset_password` and `force_random_password` are +both `false`, then `password` is required. + +Note that `force_random_password` and `reset_password` take priority +over `password`. In addition, `reset_password` and +`force_random_password` can be used together. ``` POST /users @@ -280,29 +287,30 @@ POST /users Parameters: -- `email` (required) - Email -- `password` (optional) - Password -- `reset_password` (optional) - Send user password reset link - true or false(default) -- `username` (required) - Username -- `name` (required) - Name -- `skype` (optional) - Skype ID -- `linkedin` (optional) - LinkedIn -- `twitter` (optional) - Twitter account -- `website_url` (optional) - Website URL -- `organization` (optional) - Organization name -- `projects_limit` (optional) - Number of projects user can create -- `extern_uid` (optional) - External UID -- `provider` (optional) - External provider name -- `group_id_for_saml` (optional) - ID of group where SAML has been configured -- `bio` (optional) - User's biography -- `location` (optional) - User's location -- `public_email` (optional) - The public email of the user -- `admin` (optional) - User is admin - true or false (default) -- `can_create_group` (optional) - User can create groups - true or false -- `skip_confirmation` (optional) - Skip confirmation - true or false (default) -- `external` (optional) - Flags the user as external - true or false(default) -- `avatar` (optional) - Image file for user's avatar -- `private_profile` (optional) - User's profile is private - true or false +- `email` (required) - Email +- `password` (optional) - Password +- `reset_password` (optional) - Send user password reset link - true or false (default) +- `force_random_password` (optional) - Set user password to a random value - true or false (default) +- `username` (required) - Username +- `name` (required) - Name +- `skype` (optional) - Skype ID +- `linkedin` (optional) - LinkedIn +- `twitter` (optional) - Twitter account +- `website_url` (optional) - Website URL +- `organization` (optional) - Organization name +- `projects_limit` (optional) - Number of projects user can create +- `extern_uid` (optional) - External UID +- `provider` (optional) - External provider name +- `group_id_for_saml` (optional) - ID of group where SAML has been configured +- `bio` (optional) - User's biography +- `location` (optional) - User's location +- `public_email` (optional) - The public email of the user +- `admin` (optional) - User is admin - true or false (default) +- `can_create_group` (optional) - User can create groups - true or false +- `skip_confirmation` (optional) - Skip confirmation - true or false (default) +- `external` (optional) - Flags the user as external - true or false(default) +- `avatar` (optional) - Image file for user's avatar +- `private_profile` (optional) - User's profile is private - true or false - `shared_runners_minutes_limit` (optional) - Pipeline minutes quota for this user - `extra_shared_runners_minutes_limit` (optional) - Extra pipeline minutes quota for this user diff --git a/doc/development/chatops_on_gitlabcom.md b/doc/development/chatops_on_gitlabcom.md index c63ec53414c..a7b402c3fb0 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/doc/development/chatops_on_gitlabcom.md @@ -1,12 +1,13 @@ # Chatops on GitLab.com -Chatops on GitLab.com allows GitLabbers to run various automation tasks on GitLab.com using Slack. +ChatOps on GitLab.com allows GitLab team members to run various automation tasks on GitLab.com using Slack. ## Requesting access -GitLabbers may need access to Chatops on GitLab.com for administration tasks such as: +GitLab team-members may need access to Chatops on GitLab.com for administration +tasks such as: -- Configuring feature flags on staging. +- Configuring feature flags. - Running `EXPLAIN` queries against the GitLab.com production replica. To request access to Chatops on GitLab.com: @@ -18,4 +19,4 @@ To request access to Chatops on GitLab.com: - [Chatops Usage](https://docs.gitlab.com/ee/ci/chatops/README.html) - [Understanding EXPLAIN plans](understanding_explain_plans.md) - - [Feature Groups](feature_flags.md#feature-groups) + - [Feature Groups](feature_flags/development.md#feature-groups) diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index 3d36a7bf3b1..2f1ad5fa910 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -65,10 +65,12 @@ The current team labels are: - ~Defend - ~Distribution - ~Documentation +- ~Ecosystem - ~Geo - ~Gitaly - ~Growth - ~Manage +- ~Memory - ~Monitor - ~Plan - ~Quality diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index f319d00d7fe..87e61a7476f 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -31,8 +31,8 @@ This is also the style used by linting tools such as [Return to Contributing documentation](index.md) -[rss-source]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#source-code-layout -[rss-naming]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#naming +[rss-source]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#source-code-layout +[rss-naming]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#naming-conventions [doc-guidelines]: ../documentation/index.md "Documentation guidelines" [js-styleguide]: ../fe_guide/style_guide_js.md "JavaScript styleguide" [scss-styleguide]: ../fe_guide/style_guide_scss.md "SCSS styleguide" diff --git a/doc/development/documentation/site_architecture/index.md b/doc/development/documentation/site_architecture/index.md index ee3a9caf9a0..6dd12b5efa7 100644 --- a/doc/development/documentation/site_architecture/index.md +++ b/doc/development/documentation/site_architecture/index.md @@ -11,8 +11,40 @@ and deploy it to <https://docs.gitlab.com>. While the source of the documentation content is stored in GitLab's respective product repositories, the source that is used to build the documentation site _from that content_ -is located at <https://gitlab.com/gitlab-com/gitlab-docs>. See the README there for -detailed information. +is located at <https://gitlab.com/gitlab-com/gitlab-docs>. + +The following diagram illustrates the relationship between the repositories +from where content is sourced, the `gitlab-docs` project, and the published output. + +```mermaid + graph LR + A[gitlab-ce/doc] + B[gitlab-ee/doc] + C[gitlab-runner/docs] + D[omnibus-gitlab/doc] + E[charts/doc] + F[gitlab-docs] + A --> F + B --> F + C --> F + D --> F + E --> F + F -- Build pipeline --> G + G[docs.gitlab.com] + H[/ce/] + I[/ee/] + J[/runner/] + K[/omnibus/] + L[/charts/] + G --> H + G --> I + G --> J + G --> K + G --> L +``` + +See the [README there](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md) +for detailed information. ## Assets @@ -22,9 +54,9 @@ the GitLab Documentation website. ### Libraries -- [Bootstrap 3.3 components](https://getbootstrap.com/docs/3.3/components/) -- [Bootstrap 3.3 JS](https://getbootstrap.com/docs/3.3/javascript/) -- [jQuery](https://jquery.com/) 3.2.1 +- [Bootstrap 4.3.1 components](https://getbootstrap.com/docs/4.3/components/) +- [Bootstrap 4.3.1 JS](https://getbootstrap.com/docs/4.3/getting-started/javascript/) +- [jQuery](https://jquery.com/) 3.3.1 - [Clipboard JS](https://clipboardjs.com/) - [Font Awesome 4.7.0](https://fontawesome.com/v4.7.0/icons/) diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index ff6dc16d1a0..7cd3d82ec4e 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -100,6 +100,13 @@ use regular Markdown markup, following the rules in the linked style guide. Note that Kramdown-specific markup (e.g., `{:.class}`) will not render properly on GitLab instances under [`/help`](index.md#gitlab-help). +Hard-coded HTML is valid, although it's discouraged to be used while we have `/help`. HTML is permitted as long as: + +- There's no equivalent markup in markdown. +- Advanced tables are necessary. +- Special styling is required. +- Reviewed and approved by a technical writer. + ## Structure ### Organize by topic, not by type @@ -143,7 +150,8 @@ The table below shows what kind of documentation goes where. a proper naming would be `import_projects_from_github.md`. The same rule applies to images. 1. For image files, do not exceed 100KB. -1. We do not yet support embedded videos. Please link out. +1. Do not upload video files to the product repositories. +[Link or embed videos](#videos) instead. 1. There are four main directories, `user`, `administration`, `api` and `development`. 1. The `doc/user/` directory has five main subdirectories: `project/`, `group/`, `profile/`, `dashboard/` and `admin_area/`. @@ -207,6 +215,7 @@ Do not include the same information in multiple places. [Link to a SSOT instead. ## Text +- [Write in markdown](#markdown). - Splitting long lines (preferably up to 100 characters) can make it easier to provide feedback on small chunks of text. - Insert an empty line for new paragraphs. - Use sentence case for titles, headings, labels, menu items, and buttons. @@ -430,7 +439,7 @@ To indicate the steps of navigation through the UI: - Images should be used (only when necessary) to _illustrate_ the description of a process, not to _replace_ it. - Max image size: 100KB (gifs included). -- The GitLab docs do not support videos yet. +- See also how to link and embed [videos](#videos) to illustrate the docs. Inside the document: @@ -455,6 +464,85 @@ directly to an HTML `img` tag: <img src="path/to/image.jpg" alt="Alt text (required)" class="image-noshadow"> ``` +## Videos + +Adding GitLab's existing YouTube video tutorials to the documentation is +highly encouraged, unless the video is outdated. Videos should not +replace documentation, but complement or illustrate it. If content in a video is +fundamental to a feature and its key use cases, but this is not adequately covered in the documentation, +add this detail to the documentation text or create an issue to review the video and do so. + +Do not upload videos to the product repositories. [Link](#link-to-video) or [embed](#embed-videos) them instead. + +### Link to video + +To link out to a video, include a YouTube icon so that readers can +quickly and easily scan the page for videos before reading: + +```md +<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> +For an overview, see [Video Title](link-to-video). +``` + +You can link any up-to-date video that is useful to the GitLab user. + +### Embed videos + +> [Introduced](https://gitlab.com/gitlab-com/gitlab-docs/merge_requests/472) in GitLab 12.1. + +GitLab docs (docs.gitlab.com) support embedded videos. + +You can only embed videos from +[GitLab's official YouTube account](https://www.youtube.com/channel/UCnMGQ8QHMAnVIsI3xJrihhg). +For videos from other sources, [link](#link-to-video) them instead. + +In most cases, it is better to [link to video](#link-to-video) instead, +because an embed takes up a lot of space on the page and can be distracting +to readers. + +To embed a video, follow the instructions below and make sure +you have your MR reviewed and approved by a technical writer. + +1. Copy the code below and paste it into your markdown file. + Leave a blank line above and below it. Do NOT edit the code + (don't remove or add any spaces, etc). +1. On YouTube, visit the video URL you want to display. Copy + the regular URL from your browser (`https://www.youtube.com/watch?v=VIDEO-ID`) + and replace the video title and link in the line under `<div class="video-fallback">`. +1. On YouTube, click **Share**, then **Embed**. +1. Copy the `<iframe>` source (`src`) **URL only** + (`https://www.youtube.com/embed/VIDEO-ID`), + and paste it, replacing the content of the `src` field in the + `iframe` tag. + +```html +leave a blank line here +<div class="video-fallback"> + See the video: [Video title](https://www.youtube.com/watch?v=MqL6BMOySIQ). +</div> +<figure class="video-container"> + <iframe src="https://www.youtube.com/embed/MqL6BMOySIQ" frameborder="0" allowfullscreen="true"> </iframe> +</figure> +leave a blank line here +``` + +This is how it renders on docs.gitlab.com: + +<div class="video-fallback"> + See the video: [What is GitLab](https://www.youtube.com/watch?v=enMumwvLAug). +</div> +<figure class="video-container"> + <iframe src="https://www.youtube.com/embed/MqL6BMOySIQ" frameborder="0" allowfullscreen="true"> </iframe> +</figure> + +> Notes: +> +> - The `figure` tag is required for semantic SEO and the `video_container` +class is necessary to make sure the video is responsive and displays +nicely on different mobile devices. +> - The `<div class="video-fallback">` is a fallback necessary for GitLab's +`/help`, as GitLab's markdown processor does not support iframes. It's hidden on the docs site but will be displayed on GitLab's `/help`. + ## Code blocks - Always wrap code added to a sentence in inline code blocks (``` ` ```). diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index df32242a522..64c793cfd64 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -5,8 +5,16 @@ [Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools] are useful for testing for potential accessibility problems in GitLab. -Accessibility best-practices and more in-depth information is available on -[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools. +The [axe][axe-website] browser extension (available for [Firefox][axe-firefox-extension] and [Chrome][axe-chrome-extension]) is +also a handy tool for running audits and getting feedback on markup, CSS and even potentially problematic color usages. + +Accessibility best-practices and more in-depth information are available on +[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools. The "[awesome a11y][awesome-a11y]" list is also a +useful compilation of accessibility-related material. [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules +[axe-website]: https://www.deque.com/axe/ +[axe-firefox-extension]: https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/ +[axe-chrome-extension]: https://chrome.google.com/webstore/detail/axe/lhdoppojpmngadmnindnejefpokejbdd +[awesome-a11y]: https://github.com/brunopulis/awesome-a11y diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index 13f0c5cc33e..6bad91d6287 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -1,127 +1 @@ -# Manage feature flags - -Starting from GitLab 9.3 we support feature flags for features in GitLab via -[Flipper](https://github.com/jnunemaker/flipper/). You should use the `Feature` -class (defined in `lib/feature.rb`) in your code to get, set and list feature -flags. - -During runtime you can set the values for the gates via the -[features API](../api/features.md) (accessible to admins only). - -## Feature groups - -Starting from GitLab 9.4 we support feature groups via -[Flipper groups](https://github.com/jnunemaker/flipper/blob/v0.10.2/docs/Gates.md#2-group). - -Feature groups must be defined statically in `lib/feature.rb` (in the -`.register_feature_groups` method), but their implementation can obviously be -dynamic (querying the DB etc.). - -Once defined in `lib/feature.rb`, you will be able to activate a -feature for a given feature group via the [`feature_group` param of the features API](../api/features.md#set-or-create-a-feature) - -For GitLab.com, [team members have access to feature flags through Chatops](chatops_on_gitlabcom.md). Only -percentage gates are supported at this time. Setting a feature to be used 50% of -the time, you should execute `/chatops run feature set my_feature_flag 50`. - -## Feature flags for user applications - -This document only covers feature flags used in the development of GitLab -itself. Feature flags in deployed user applications can be found at -[Feature Flags](../user/project/operations/feature_flags.md) - -## Developing with feature flags - -In general, it's better to have a group- or user-based gate, and you should prefer -it over the use of percentage gates. This would make debugging easier, as you -filter for example logs and errors based on actors too. Furthermore, this allows -for enabling for the `gitlab-org` group first, while the rest of the users -aren't impacted. - -```ruby -# Good -Feature.enabled?(:feature_flag, project) - -# Avoid, if possible -Feature.enabled?(:feature_flag) -``` - -To use feature gates based on actors, the model needs to respond to -`flipper_id`. For example, to enable for the Foo model: - -```ruby -class Foo < ActiveRecord::Base - include FeatureGate -end -``` - -Features that are developed and are intended to be merged behind a feature flag -should not include a changelog entry. The entry should be added in the merge -request removing the feature flags. - -In the rare case that you need the feature flag to be on automatically, use -`default_enabled: true` when checking: - -```ruby -Feature.enabled?(:feature_flag, project, default_enabled: true) -``` - -For more information about rolling out changes using feature flags, refer to the -[Rolling out changes using feature flags](rolling_out_changes_using_feature_flags.md) -guide. - -### Frontend - -For frontend code you can use the method `push_frontend_feature_flag`, which is -available to all controllers that inherit from `ApplicationController`. Using -this method you can expose the state of a feature flag as follows: - -```ruby -before_action do - push_frontend_feature_flag(:vim_bindings) -end - -def index - # ... -end - -def edit - # ... -end -``` - -You can then check for the state of the feature flag in JavaScript as follows: - -```javascript -if ( gon.features.vimBindings ) { - // ... -} -``` - -The name of the feature flag in JavaScript will always be camelCased, meaning -that checking for `gon.features.vim_bindings` would not work. - -### Specs - -In the test environment `Feature.enabled?` is stubbed to always respond to `true`, -so we make sure behavior under feature flag doesn't go untested in some non-specific -contexts. - -Whenever a feature flag is present, make sure to test _both_ states of the -feature flag. - -See the -[testing guide](testing_guide/best_practices.md#feature-flags-in-tests) -for information and examples on how to stub feature flags in tests. - -## Enabling a feature flag (in development) - -In the rails console (`rails c`), enter the following command to enable your feature flag - -```ruby -Feature.enable(:feature_flag_name) -``` - -## Enabling a feature flag (in production) - -Check how to [roll out changes using feature flags](rolling_out_changes_using_feature_flags.md). +This document was moved to [another location](feature_flags/index.md). diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md new file mode 100644 index 00000000000..c67467b7c11 --- /dev/null +++ b/doc/development/feature_flags/controls.md @@ -0,0 +1,123 @@ +# Access for enabling a feature flag in production + +In order to be able to turn on/off features behind feature flags in any of the +GitLab Inc. provided environments such as staging and production, you need to +have access to the chatops bot. Chatops bot is currently running on the ops instance, +which is different from GitLab.com or dev.gitlab.org. + +Follow the Chatops document to [request access](https://docs.gitlab.com/ee/development/chatops_on_gitlabcom.html#requesting-access). + +Once you are added to the project test if your access propagated, +run: + +``` +/chatops run feature --help +``` + +## Rolling out changes + +When the changes are deployed to the environments it is time to start +rolling out the feature to our users. The exact procedure of rolling out a +change is unspecified, as this can vary from change to change. However, in +general we recommend rolling out changes incrementally, instead of enabling them +for everybody right away. We also recommend you to _not_ enable a feature +_before_ the code is being deployed. +This allows you to separate rolling out a feature from a deploy, making it +easier to measure the impact of both separately. + +GitLab's feature library (using +[Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature +Flags process](process.md) guide) supports rolling out changes to a percentage of +users. This in turn can be controlled using [GitLab chatops](../../ci/chatops/README.md). + +For an up to date list of feature flag commands please see [the source +code](https://gitlab.com/gitlab-com/chatops/blob/master/lib/chatops/commands/feature.rb). +Note that all the examples in that file must be preceded by +`/chatops run`. + +If you get an error "Whoops! This action is not allowed. This incident +will be reported." that means your Slack account is not allowed to +change feature flags or you do not [have access](#access-for-enabling-a-feature-flag-in-production). + +### Enabling feature for staging and dev.gitlab.org + +As a first step in a feature rollout, you should enable the feature on <https://staging.gitlab.com> +and <https://dev.gitlab.org>. + +For example, to enable a feature for 25% of all users, run the following in +Slack: + +``` +/chatops run feature set new_navigation_bar 25 --dev +/chatops run feature set new_navigation_bar 25 --staging +``` + +These two environments have different scopes. +`dev.gitlab.org` is a production CE environment that has internal GitLab Inc. +traffic and is used for some development and other related work. +`staging.gitlab.com` has a smaller subset of GitLab.com database and repositories +and does not have regular traffic. Staging is an EE instance and can give you +a (very) rough estimate of how your feature will look/behave on GitLab.com. +Both of these instances are connected to Sentry so make sure you check the projects +there for any exceptions while testing your feature after enabling the feature flag. + +Once you are confident enough that these environments are in a good state with your +feature enabled, you can roll out the change to GitLab.com. + +## Enabling feature for GitLab.com + +Similar to above, to enable a feature for 25% of all users, run the following in +Slack: + +``` +/chatops run feature set new_navigation_bar 25 +``` + +This will enable the feature for GitLab.com, with `new_navigation_bar` being the +name of the feature. + +If you are not certain what percentages to use, simply use the following steps: + +1. 25% +1. 50% +1. 75% +1. 100% + +Between every step you'll want to wait a little while and monitor the +appropriate graphs on <https://dashboards.gitlab.net>. The exact time to wait +may differ. For some features a few minutes is enough, while for others you may +want to wait several hours or even days. This is entirely up to you, just make +sure it is clearly communicated to your team, and the Production team if you +anticipate any potential problems. + +Feature gates can also be actor based, for example a feature could first be +enabled for only the `gitlab-ce` project. The project is passed by supplying a +`--project` flag: + +``` +/chatops run feature set --project=gitlab-org/gitlab-ce some_feature true +``` + +For groups the `--group` flag is available: + +``` +/chatops run feature set --group=gitlab-org some_feature true +``` + +## Cleaning up + +Once the change is deemed stable, submit a new merge request to remove the +feature flag. This ensures the change is available to all users and self-hosted +instances. Make sure to add the ~"feature flag" label to this merge request so +release managers are aware the changes are hidden behind a feature flag. If the +merge request has to be picked into a stable branch, make sure to also add the +appropriate "Pick into X" label (e.g. "Pick into XX.X"). +See [the process document](https://docs.gitlab.com/ee/development/feature_flags/process.html#including-a-feature-behind-feature-flag-in-the-final-release) for further details. + +When a feature gate has been removed from the code base, the value still exists +in the database. +This can be removed through ChatOps: + +``` +/chatops run feature delete some_feature +``` diff --git a/doc/development/feature_flags/development.md b/doc/development/feature_flags/development.md new file mode 100644 index 00000000000..238052529d9 --- /dev/null +++ b/doc/development/feature_flags/development.md @@ -0,0 +1,131 @@ +# Developing with feature flags + +In general, it's better to have a group- or user-based gate, and you should prefer +it over the use of percentage gates. This would make debugging easier, as you +filter for example logs and errors based on actors too. Furthermore, this allows +for enabling for the `gitlab-org` or `gitlab-com` group first, while the rest of +the users aren't impacted. + +```ruby +# Good +Feature.enabled?(:feature_flag, project) + +# Avoid, if possible +Feature.enabled?(:feature_flag) +``` + +To use feature gates based on actors, the model needs to respond to +`flipper_id`. For example, to enable for the Foo model: + +```ruby +class Foo < ActiveRecord::Base + include FeatureGate +end +``` + +Features that are developed and are intended to be merged behind a feature flag +should not include a changelog entry. The entry should be added in the merge +request removing the feature flags. + +In the rare case that you need the feature flag to be on automatically, use +`default_enabled: true` when checking: + +```ruby +Feature.enabled?(:feature_flag, project, default_enabled: true) +``` + +The [`Project#feature_available?`][project-fa], +[`Namespace#feature_available?`][namespace-fa] (EE), and +[`License.feature_available?`][license-fa] (EE) methods all implicitly check for +a feature flag by the same name as the provided argument. + +For example if a feature is license-gated, there's no need to add an additional +explicit feature flag check since the flag will be checked as part of the +`License.feature_available?` call. Similarly, there's no need to "clean up" a +feature flag once the feature has reached general availability. + +You'd still want to use an explicit `Feature.enabled?` check if your new feature +isn't gated by a License or Plan. + +[project-fa]: https://gitlab.com/gitlab-org/gitlab-ee/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/app/models/project_feature.rb#L63-68 +[namespace-fa]: https://gitlab.com/gitlab-org/gitlab-ee/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/ee/namespace.rb#L71-85 +[license-fa]: https://gitlab.com/gitlab-org/gitlab-ee/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/license.rb#L293-300 + +An important side-effect of the implicit feature flags mentioned above is that +unless the feature is explicitly disabled or limited to a percentage of users, +the feature flag check will default to `true`. + +As an example, if you were to ship the backend half of a feature behind a flag, +you'd want to explicitly disable that flag until the frontend half is also ready +to be shipped. [You can do this via Chatops](https://docs.gitlab.com/ee/development/feature_flags/controls.html): + +``` +/chatops run feature set some_feature 0 +``` + +Note that you can do this at any time, even before the merge request using the +flag has been merged! + +## Feature groups + +Starting from GitLab 9.4 we support feature groups via +[Flipper groups](https://github.com/jnunemaker/flipper/blob/v0.10.2/docs/Gates.md#2-group). + +Feature groups must be defined statically in `lib/feature.rb` (in the +`.register_feature_groups` method), but their implementation can obviously be +dynamic (querying the DB etc.). + +Once defined in `lib/feature.rb`, you will be able to activate a +feature for a given feature group via the [`feature_group` param of the features API](../../api/features.md#set-or-create-a-feature) + +### Frontend + +For frontend code you can use the method `push_frontend_feature_flag`, which is +available to all controllers that inherit from `ApplicationController`. Using +this method you can expose the state of a feature flag as follows: + +```ruby +before_action do + push_frontend_feature_flag(:vim_bindings) +end + +def index + # ... +end + +def edit + # ... +end +``` + +You can then check for the state of the feature flag in JavaScript as follows: + +```javascript +if ( gon.features.vimBindings ) { + // ... +} +``` + +The name of the feature flag in JavaScript will always be camelCased, meaning +that checking for `gon.features.vim_bindings` would not work. + +### Specs + +In the test environment `Feature.enabled?` is stubbed to always respond to `true`, +so we make sure behavior under feature flag doesn't go untested in some non-specific +contexts. + +Whenever a feature flag is present, make sure to test _both_ states of the +feature flag. + +See the +[testing guide](../testing_guide/best_practices.md#feature-flags-in-tests) +for information and examples on how to stub feature flags in tests. + +### Enabling a feature flag (in development) + +In the rails console (`rails c`), enter the following command to enable your feature flag + +```ruby +Feature.enable(:feature_flag_name) +``` diff --git a/doc/development/feature_flags/index.md b/doc/development/feature_flags/index.md new file mode 100644 index 00000000000..56872f8c075 --- /dev/null +++ b/doc/development/feature_flags/index.md @@ -0,0 +1,12 @@ +# Feature flags in development of GitLab + +Feature flags can be used to gradually roll out changes, be +it a new feature, or a performance improvement. By using feature flags, we can +comfortably measure the impact of our changes, while still being able to easily +disable those changes, without having to revert an entire release. + +Before using feature flags for GitLab's development, read through the following: + +- [Process for using features flags](process.md). +- [Developing with feature flags documentation](development.md). +- [Controlling feature flags documentation](controls.md). diff --git a/doc/development/feature_flags/process.md b/doc/development/feature_flags/process.md new file mode 100644 index 00000000000..ee142b0da66 --- /dev/null +++ b/doc/development/feature_flags/process.md @@ -0,0 +1,130 @@ +# Feature flags process +## Feature flags for user applications + +This document only covers feature flags used in the development of GitLab +itself. Feature flags in deployed user applications can be found at +[Feature Flags feature documentation](../../user/project/operations/feature_flags.md). + +## Feature flags in GitLab development + +The following highlights should be considered when deciding if feature flags +should be leveraged: + +- By default, the feature flags should be **off**. +- Feature flags should remain in the codebase for as short period as possible +to reduce the need for feature flag accounting. +- The person operating with feature flags is responsible for clearly communicating +the status of a feature behind the feature flag with responsible stakeholders. +- Merge requests that make changes hidden behind a feature flag, or remove an +existing feature flag because a feature is deemed stable must have the +~"feature flag" label assigned. + +One might be tempted to think that feature flags will delay the release of a +feature by at least one month (= one release). This is not the case. A feature +flag does not have to stick around for a specific amount of time +(e.g. at least one release), instead they should stick around until the feature +is deemed stable. Stable means it works on GitLab.com without causing any +problems, such as outages. + +### When to use feature flags + +Starting with GitLab 11.4, developers are required to use feature flags for +non-trivial changes. Such changes include: + +- New features (e.g. a new merge request widget, epics, etc). +- Complex performance improvements that may require additional testing in + production, such as rewriting complex queries. +- Invasive changes to the user interface, such as a new navigation bar or the + removal of a sidebar. +- Adding support for importing projects from a third-party service. + +In all cases, those working on the changes can best decide if a feature flag is +necessary. For example, changing the color of a button doesn't need a feature +flag, while changing the navigation bar definitely needs one. In case you are +uncertain if a feature flag is necessary, simply ask about this in the merge +request, and those reviewing the changes will likely provide you with an answer. + +When using a feature flag for UI elements, make sure to _also_ use a feature +flag for the underlying backend code, if there is any. This ensures there is +absolutely no way to use the feature until it is enabled. + +### Including a feature behind feature flag in the final release + +In order to build a final release and present the feature for self-hosted +users, the feature flag should be at least defaulted to **on**. If the feature +is deemed stable and there is confidence that removing the feature flag is safe, +consider removing the feature flag altogether. Take into consideration that such +action can make the feature available on GitLab.com shortly after the change to +the feature flag is merged. + +Changing the default state or removing the feature flag has to be done before +the 22nd of the month, _at least_ 2 working days before, in order for the change +to be included in the final self-managed release. + +In addition to this, the feature behind feature flag should: + +- Run in all GitLab.com environments for a sufficient period of time. This time +period depends on the feature behind the feature flag, but as a general rule of +thumb 2-4 working days should be sufficient to gather enough feedback. +- The feature should be exposed to all users within the GitLab.com plan during +the above mentioned period of time. Exposing the feature to a smaller percentage +or only a group of users might not expose a sufficient amount of information to aid in +making a decision on feature stability. + +While rare, release managers may decide to reject picking or revert a change in +a stable branch, even when feature flags are used. This might be necessary if +the changes are deemed problematic, too invasive, or there simply isn't enough +time to properly measure how the changes behave on GitLab.com. + +### The cost of feature flags + +When reading the above, one might be tempted to think this procedure is going to +add a lot of work. Fortunately, this is not the case, and we'll show why. For +this example we'll specify the cost of the work to do as a number, ranging from +0 to infinity. The greater the number, the more expensive the work is. The cost +does _not_ translate to time, it's just a way of measuring complexity of one +change relative to another. + +Let's say we are building a new feature, and we have determined that the cost of +this is 10. We have also determined that the cost of adding a feature flag check +in a variety of places is 1. If we do not use feature flags, and our feature +works as intended, our total cost is 10. This however is the best case scenario. +Optimizing for the best case scenario is guaranteed to lead to trouble, whereas +optimizing for the worst case scenario is almost always better. + +To illustrate this, let's say our feature causes an outage, and there's no +immediate way to resolve it. This means we'd have to take the following steps to +resolve the outage: + +1. Revert the release. +1. Perform any cleanups that might be necessary, depending on the changes that + were made. +1. Revert the commit, ensuring the "master" branch remains stable. This is + especially necessary if solving the problem can take days or even weeks. +1. Pick the revert commit into the appropriate stable branches, ensuring we + don't block any future releases until the problem is resolved. + +As history has shown, these steps are time consuming, complex, often involve +many developers, and worst of all: our users will have a bad experience using +GitLab.com until the problem is resolved. + +Now let's say that all of this has an associated cost of 10. This means that in +the worst case scenario, which we should optimize for, our total cost is now 20. + +If we had used a feature flag, things would have been very different. We don't +need to revert a release, and because feature flags are disabled by default we +don't need to revert and pick any Git commits. In fact, all we have to do is +disable the feature, and in the worst case, perform cleanup. Let's say that +the cost of this is 2. In this case, our best case cost is 11: 10 to build the +feature, and 1 to add the feature flag. The worst case cost is now 13: 10 to +build the feature, 1 to add the feature flag, and 2 to disable and clean up. + +Here we can see that in the best case scenario the work necessary is only a tiny +bit more compared to not using a feature flag. Meanwhile, the process of +reverting our changes has been made significantly and reliably cheaper. + +In other words, feature flags do not slow down the development process. Instead, +they speed up the process as managing incidents now becomes _much_ easier. Once +continuous deployments are easier to perform, the time to iterate on a feature +is reduced even further, as you no longer need to wait weeks before your changes +are available on GitLab.com. diff --git a/doc/development/i18n/externalization.md b/doc/development/i18n/externalization.md index ce310672dad..17462887162 100644 --- a/doc/development/i18n/externalization.md +++ b/doc/development/i18n/externalization.md @@ -135,7 +135,7 @@ There is also and alternative method to [translate messages from validation erro ### Interpolation Placeholders in translated text should match the code style of the respective source file. -For example use `%{created_at}` in Ruby but `%{createdAt}` in JavaScript. +For example use `%{created_at}` in Ruby but `%{createdAt}` in JavaScript. Make sure to [avoid splitting sentences when adding links](#avoid-splitting-sentences-when-adding-links). - In Ruby/HAML: @@ -267,20 +267,41 @@ should be externalized as follows: This also applies when using links in between translated sentences, otherwise these texts are not translatable in certain languages. -Instead of: +- In Ruby/HAML, instead of: + + ```haml + - zones_link = link_to(s_('ClusterIntegration|zones'), 'https://cloud.google.com/compute/docs/regions-zones/regions-zones', target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Learn more about %{zones_link}').html_safe % { zones_link: zones_link } + ``` + + Set the link starting and ending HTML fragments as variables like so: + + ```haml + - zones_link_url = 'https://cloud.google.com/compute/docs/regions-zones/regions-zones' + - zones_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: zones_link_url } + = s_('ClusterIntegration|Learn more about %{zones_link_start}zones%{zones_link_end}').html_safe % { zones_link_start: zones_link_start, zones_link_end: '</a>'.html_safe } + ``` -```haml -- zones_link = link_to(s_('ClusterIntegration|zones'), 'https://cloud.google.com/compute/docs/regions-zones/regions-zones', target: '_blank', rel: 'noopener noreferrer') -= s_('ClusterIntegration|Learn more about %{zones_link}').html_safe % { zones_link: zones_link } -``` +- In JavaScript, instead of: -Set the link starting and ending HTML fragments as variables like so: + ```js + {{ + sprintf(s__("ClusterIntegration|Learn more about %{link}"), { + link: '<a href="https://cloud.google.com/compute/docs/regions-zones/regions-zones" target="_blank" rel="noopener noreferrer">zones</a>' + }) + }} + ``` -```haml -- zones_link_url = 'https://cloud.google.com/compute/docs/regions-zones/regions-zones' -- zones_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: zones_link_url } -= s_('ClusterIntegration|Learn more about %{zones_link_start}zones%{zones_link_end}').html_safe % { zones_link_start: zones_link_start, zones_link_end: '</a>'.html_safe } -``` + Set the link starting and ending HTML fragments as variables like so: + + ```js + {{ + sprintf(s__("ClusterIntegration|Learn more about %{linkStart}zones%{linkEnd}"), { + linkStart: '<a href="https://cloud.google.com/compute/docs/regions-zones/regions-zones" target="_blank" rel="noopener noreferrer">' + linkEnd: '</a>', + }) + }} + ``` The reasoning behind this is that in some languages words change depending on context. For example in Japanese は is added to the subject of a sentence and を to the object. This is impossible to translate correctly if we extract individual words from the sentence. diff --git a/doc/development/new_fe_guide/tips.md b/doc/development/new_fe_guide/tips.md index 4564f678ec0..879b54bd93c 100644 --- a/doc/development/new_fe_guide/tips.md +++ b/doc/development/new_fe_guide/tips.md @@ -10,16 +10,16 @@ yarn clean ## Creating feature flags in development -The process for creating a feature flag is the same as [enabling a feature flag in development](../feature_flags.md#enabling-a-feature-flag-in-development). +The process for creating a feature flag is the same as [enabling a feature flag in development](../feature_flags/development.md#enabling-a-feature-flag-in-development). Your feature flag can now be: -- [made available to the frontend](../feature_flags.md#frontend) via the `gon` -- queried in [tests](../feature_flags.md#specs) -- queried in HAML templates and ruby files via the `Feature.enabled?(:my_shiny_new_feature_flag)` method +- [Made available to the frontend](../feature_flags/development.md#frontend) via the `gon` +- Queried in [tests](../feature_flags/development.md#specs) +- Queried in HAML templates and ruby files via the `Feature.enabled?(:my_shiny_new_feature_flag)` method ### More on feature flags - [Deleting a feature flag](../../api/features.md#delete-a-feature) -- [Manage feature flags](../feature_flags.md) +- [Manage feature flags](../feature_flags/process.md) - [Feature flags API](../../api/features.md) diff --git a/doc/development/rolling_out_changes_using_feature_flags.md b/doc/development/rolling_out_changes_using_feature_flags.md index 84028b1b342..6bad91d6287 100644 --- a/doc/development/rolling_out_changes_using_feature_flags.md +++ b/doc/development/rolling_out_changes_using_feature_flags.md @@ -1,225 +1 @@ -# Rolling out changes using feature flags - -[Feature flags](feature_flags.md) can be used to gradually roll out changes, be -it a new feature, or a performance improvement. By using feature flags, we can -comfortably measure the impact of our changes, while still being able to easily -disable those changes, without having to revert an entire release. - -## When to use feature flags - -Starting with GitLab 11.4, developers are required to use feature flags for -non-trivial changes. Such changes include: - -- New features (e.g. a new merge request widget, epics, etc). -- Complex performance improvements that may require additional testing in - production, such as rewriting complex queries. -- Invasive changes to the user interface, such as a new navigation bar or the - removal of a sidebar. -- Adding support for importing projects from a third-party service. - -In all cases, those working on the changes can best decide if a feature flag is -necessary. For example, changing the color of a button doesn't need a feature -flag, while changing the navigation bar definitely needs one. In case you are -uncertain if a feature flag is necessary, simply ask about this in the merge -request, and those reviewing the changes will likely provide you with an answer. - -When using a feature flag for UI elements, make sure to _also_ use a feature -flag for the underlying backend code, if there is any. This ensures there is -absolutely no way to use the feature until it is enabled. - -## The cost of feature flags - -When reading the above, one might be tempted to think this procedure is going to -add a lot of work. Fortunately, this is not the case, and we'll show why. For -this example we'll specify the cost of the work to do as a number, ranging from -0 to infinity. The greater the number, the more expensive the work is. The cost -does _not_ translate to time, it's just a way of measuring complexity of one -change relative to another. - -Let's say we are building a new feature, and we have determined that the cost of -this is 10. We have also determined that the cost of adding a feature flag check -in a variety of places is 1. If we do not use feature flags, and our feature -works as intended, our total cost is 10. This however is the best case scenario. -Optimising for the best case scenario is guaranteed to lead to trouble, whereas -optimising for the worst case scenario is almost always better. - -To illustrate this, let's say our feature causes an outage, and there's no -immediate way to resolve it. This means we'd have to take the following steps to -resolve the outage: - -1. Revert the release. -1. Perform any cleanups that might be necessary, depending on the changes that - were made. -1. Revert the commit, ensuring the "master" branch remains stable. This is - especially necessary if solving the problem can take days or even weeks. -1. Pick the revert commit into the appropriate stable branches, ensuring we - don't block any future releases until the problem is resolved. - -As history has shown, these steps are time consuming, complex, often involve -many developers, and worst of all: our users will have a bad experience using -GitLab.com until the problem is resolved. - -Now let's say that all of this has an associated cost of 10. This means that in -the worst case scenario, which we should optimise for, our total cost is now 20. - -If we had used a feature flag, things would have been very different. We don't -need to revert a release, and because feature flags are disabled by default we -don't need to revert and pick any Git commits. In fact, all we have to do is -disable the feature, and in the worst case, perform cleanup. Let's say that -the cost of this is 2. In this case, our best case cost is 11: 10 to build the -feature, and 1 to add the feature flag. The worst case cost is now 13: 10 to -build the feature, 1 to add the feature flag, and 2 to disable and clean up. - -Here we can see that in the best case scenario the work necessary is only a tiny -bit more compared to not using a feature flag. Meanwhile, the process of -reverting our changes has been made significantly and reliably cheaper. - -In other words, feature flags do not slow down the development process. Instead, -they speed up the process as managing incidents now becomes _much_ easier. Once -continuous deployments are easier to perform, the time to iterate on a feature -is reduced even further, as you no longer need to wait weeks before your changes -are available on GitLab.com. - -## Rolling out changes - -The procedure of using feature flags is straightforward, and similar to not -using them. You add the necessary tests (make sure to test both the on and off -states of your feature flag(s)), make sure they all pass, have the code -reviewed, etc. You then submit your merge request, and add the ~"feature flag" -label. This label is used to signal to release managers that your changes are -hidden behind a feature flag and that it is safe to pick the MR into a stable -branch, without the need for an exception request. - -When the changes are deployed it is time to start rolling out the feature to our -users. The exact procedure of rolling out a change is unspecified, as this can -vary from change to change. However, in general we recommend rolling out changes -incrementally, instead of enabling them for everybody right away. We also -recommend you to _not_ enable a feature _before_ the code is being deployed. -This allows you to separate rolling out a feature from a deploy, making it -easier to measure the impact of both separately. - -GitLab's feature library (using -[Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature -Flags](feature_flags.md) guide) supports rolling out changes to a percentage of -users. This in turn can be controlled using [GitLab -chatops](../ci/chatops/README.md). - -For an up to date list of feature flag commands please see [the source -code](https://gitlab.com/gitlab-com/chatops/blob/master/lib/chatops/commands/feature.rb). -Note that all the examples in that file must be preceded by -`/chatops run`. - -If you get an error "Whoops! This action is not allowed. This incident -will be reported." that means your Slack account is not allowed to -change feature flags. To test if you are allowed to do anything at all, -run: - -``` -/chatops run feature --help -``` - -For example, to enable a feature for 25% of all users, run the following in -Slack: - -``` -/chatops run feature set new_navigation_bar 25 -``` - -This will enable the feature for GitLab.com, with `new_navigation_bar` being the -name of the feature. We can also enable the feature for <https://dev.gitlab.org> -or <https://staging.gitlab.com>: - -``` -/chatops run feature set new_navigation_bar 25 --dev -/chatops run feature set new_navigation_bar 25 --staging -``` - -If you are not certain what percentages to use, simply use the following steps: - -1. 25% -1. 50% -1. 75% -1. 100% - -Between every step you'll want to wait a little while and monitor the -appropriate graphs on <https://dashboards.gitlab.net>. The exact time to wait -may differ. For some features a few minutes is enough, while for others you may -want to wait several hours or even days. This is entirely up to you, just make -sure it is clearly communicated to your team, and the Production team if you -anticipate any potential problems. - -Feature gates can also be actor based, for example a feature could first be -enabled for only the `gitlab-ce` project. The project is passed by supplying a -`--project` flag: - -``` -/chatops run feature set --project=gitlab-org/gitlab-ce some_feature true -``` - -For groups the `--group` flag is available: - -``` -/chatops run feature set --group=gitlab-org some_feature true -``` - -Once a change is deemed stable, submit a new merge request to remove the -feature flag. This ensures the change is available to all users and self-hosted -instances. Make sure to add the ~"feature flag" label to this merge request so -release managers are aware the changes are hidden behind a feature flag. If the -merge request has to be picked into a stable branch (e.g. after the 7th), make -sure to also add the appropriate "Pick into X" label (e.g. "Pick into 11.4"). - -One might be tempted to think this will delay the release of a feature by at -least one month (= one release). This is not the case. A feature flag does not -have to stick around for a specific amount of time (e.g. at least one release), -instead they should stick around until the feature is deemed stable. Stable -means it works on GitLab.com without causing any problems, such as outages. In -most cases this will translate to a feature (with a feature flag) being shipped -in RC1, followed by the feature flag being removed in RC2. This in turn means -the feature will be stable by the time we publish a stable package around the -22nd of the month. - -## Implicit feature flags - -The [`Project#feature_available?`][project-fa], -[`Namespace#feature_available?`][namespace-fa] (EE), and -[`License.feature_available?`][license-fa] (EE) methods all implicitly check for -a feature flag by the same name as the provided argument. - -For example if a feature is license-gated, there's no need to add an additional -explicit feature flag check since the flag will be checked as part of the -`License.feature_available?` call. Similarly, there's no need to "clean up" a -feature flag once the feature has reached general availability. - -You'd still want to use an explicit `Feature.enabled?` check if your new feature -isn't gated by a License or Plan. - -[project-fa]: https://gitlab.com/gitlab-org/gitlab-ee/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/app/models/project_feature.rb#L63-68 -[namespace-fa]: https://gitlab.com/gitlab-org/gitlab-ee/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/ee/namespace.rb#L71-85 -[license-fa]: https://gitlab.com/gitlab-org/gitlab-ee/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/license.rb#L293-300 - -### Undefined feature flags default to "on" - -An important side-effect of the [implicit feature flags](#implicit-feature-flags) -mentioned above is that unless the feature is explicitly disabled or limited to a -percentage of users, the feature flag check will default to `true`. - -As an example, if you were to ship the backend half of a feature behind a flag, -you'd want to explicitly disable that flag until the frontend half is also ready -to be shipped. You can do this via ChatOps: - -``` -/chatops run feature set some_feature 0 -``` - -Note that you can do this at any time, even before the merge request using the -flag has been merged! - -### Cleaning up - -When a feature gate has been removed from the code base, the value still exists -in the database. This can be removed through ChatOps: - -``` -/chatops run feature delete some_feature -``` +This document was moved to [another location](feature_flags/index.md). diff --git a/doc/development/testing_guide/end_to_end/quick_start_guide.md b/doc/development/testing_guide/end_to_end/quick_start_guide.md index f96c85be1ba..041bdf716b3 100644 --- a/doc/development/testing_guide/end_to_end/quick_start_guide.md +++ b/doc/development/testing_guide/end_to_end/quick_start_guide.md @@ -242,7 +242,7 @@ module QA issue = Resource::Issue.fabricate_via_api! do |issue| issue.title = 'Issue to test the scoped labels' - issue.labels = @initial_label + issue.labels = [@initial_label] end [@new_label_same_scope, @new_label_different_scope].each do |label| @@ -365,6 +365,14 @@ Add the following `attribute :id` and `attribute :labels` right above the [`attr > We add the attributes above the existing attribute to keep them alphabetically organized. +Then, let's initialize an instance variable for labels to allow an empty array as default value when such information is not passed during the resource fabrication, since this optional. [Between the attributes and the `fabricate!` method](https://gitlab.com/gitlab-org/gitlab-ee/blob/1a1f1408728f19b2aa15887cd20bddab7e70c8bd/qa/qa/resource/issue.rb#L18), add the following: + +```ruby +def initialize + @labels = [] +end +``` + Next, add the following code right below the [`fabricate!`](https://gitlab.com/gitlab-org/gitlab-ee/blob/d3584e80b4236acdf393d815d604801573af72cc/qa/qa/resource/issue.rb#L27) method. ```ruby @@ -378,7 +386,7 @@ end def api_post_body { - labels: [labels], + labels: labels, title: title } end diff --git a/doc/gitlab-basics/command-line-commands.md b/doc/gitlab-basics/command-line-commands.md index 1cf883679d7..b7e6844f43a 100644 --- a/doc/gitlab-basics/command-line-commands.md +++ b/doc/gitlab-basics/command-line-commands.md @@ -29,7 +29,9 @@ git clone PASTE HTTPS OR SSH HERE A clone of the project will be created in your computer. ->**Note:** If you clone your project via a URL that contains special characters, make sure that characters are URL-encoded. +NOTE: **Note:** +If you clone your project via a URL that contains special characters, make sure +that characters are URL-encoded. ### Go into a project directory to work in it @@ -86,12 +88,18 @@ cat README.md ### Remove a file +DANGER: **Danger:** +This will permanently delete the file. + ``` rm NAME-OF-FILE ``` ### Remove a directory and all of its contents +DANGER: **Danger:** +This will permanently delete the directory and all of its contents. + ``` rm -r NAME-OF-DIRECTORY ``` @@ -113,9 +121,13 @@ history You will be asked for an administrator’s password. ``` -sudo +sudo COMMAND ``` +CAUTION: **Caution:** +Be careful of the commands you run with `sudo`. Certain commands may cause +damage to your data and system. + ### Show which directory I am in ``` diff --git a/doc/topics/application_development_platform/index.md b/doc/topics/application_development_platform/index.md index e8960a76dd9..8742606479d 100644 --- a/doc/topics/application_development_platform/index.md +++ b/doc/topics/application_development_platform/index.md @@ -1,12 +1,21 @@ # Application Development Platform -The GitLab Application Development Platform refers to the set of GitLab features that can be used by operations teams to -provide a full development environment to internal software development teams. +The GitLab Application Development Platform refers to the set of GitLab features used to create, configure, and manage +a complete software development environment. It provides development, operations, and security teams with a robust feature set aimed at supporting best practices out of the box. ## Overview -The GitLab Application Development Platform aims to reduce and even eliminate the time it takes for an Operations team -to provide a full environment for software developers. It comprises the following high-level elements: +The GitLab Application Development Platform aims to: + +- Reduce and even eliminate the time it takes for an Operations team + to provide a full environment for software developers. +- Get developers up and running fast so they can focus on writing + great applications with a robust development feature set. +- Provide best-of-breed security features so that applications developed + with GitLab are not affected by vulnerabilities that may lead to security + problems and unintended use. + +It is comprised of the following high-level elements: 1. Compute 1. Build, test, and deploy a wide range of applications diff --git a/doc/user/admin_area/index.md b/doc/user/admin_area/index.md index fa60ee96cf7..d2947ae3371 100644 --- a/doc/user/admin_area/index.md +++ b/doc/user/admin_area/index.md @@ -21,7 +21,7 @@ The Admin Area is made up of the following sections: | Section | Description | |:------------------------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | [Overview](#overview-section) | View your GitLab [Dashboard](#admin-dashboard), and administer [projects](#administering-projects), [users](#administering-users), [groups](#administering-groups), [jobs](#administering-jobs), [Runners](#administering-runners), and [Gitaly servers](#administering-gitaly-servers). | -| Monitoring | View GitLab system information, and information on background jobs, logs, [health checks](monitoring/health_check.md), request profiles, and audit logs. | +| Monitoring | View GitLab [system information](#system-info), and information on [background jobs](#background-jobs), [logs](#logs), [health checks](monitoring/health_check.md), [requests profiles](#requests-profiles), and [audit logs](#audit-log-premium-only). | | Messages | Send and manage [broadcast messages](broadcast_messages.md) for your users. | | System Hooks | Configure [system hooks](../../system_hooks/system_hooks.md) for many events. | | Applications | Create system [OAuth applications](../../integration/oauth_provider.md) for integrations with other services. | @@ -229,3 +229,66 @@ For each Gitaly server, the following details are listed: | Server version | Gitaly version | | Git version | Version of Git installed on the Gitaly server | | Up to date | Indicates if the Gitaly server version is the latest version available. A green dot indicates the server is up to date. | + +## Monitoring section + +The following topics document the **Monitoring** section of the Admin Area. + +### System Info + +The **System Info** page provides the following statistics: + +| Field | Description | +| :----------- | :---------- | +| CPU | Number of CPU cores available | +| Memory Usage | Memory in use, and total memory available | +| Disk Usage | Disk space in use, and total disk space available | +| Uptime | Approximate uptime of the GitLab instance | + +These statistics are updated only when you navigate to the **System Info** page, or you refresh the page in your browser. + +### Background Jobs + +The **Background Jobs** page displays the Sidekiq dashboard. Sidekiq is used by GitLab to +perform processing in the background. + +The Sidekiq dashboard consists of the following elements: + +- A tab per jobs' status. +- A breakdown of background job statistics. +- A live graph of **Processed** and **Failed** jobs, with a selectable polling interval. +- An historical graph of **Processed** and **Failed** jobs, with a selectable time span. +- Redis statistics, including: + - Version number + - Uptime, measured in days + - Number of connections + - Current memory usage, measured in MB + - Peak memory usage, measured in MB + +### Logs + +The **Logs** page provides access to the following log files: + +| Log file | Contents | +| :---------------------- | :------- | +| `application.log` | GitLab user activity | +| `githost.log` | Failed GitLab interaction with Git repositories | +| `production.log` | Requests received from Unicorn, and the actions taken to serve those requests | +| `sidekiq.log` | Background jobs | +| `repocheck.log` | Repository activity | +| `integrations_json.log` | Activity between GitLab and integrated systems | +| `kubernetes.log` | Kubernetes activity | + +The contents of these log files can be useful when troubleshooting a problem. Access is available to GitLab admins, without requiring direct access to the log files. + +For details of these log files and their contents, see [Log system](../../administration/logs.md). + +The content of each log file is listed in chronological order. To minimize performance issues, a maximum 2000 lines of each log file are shown. + +### Requests Profiles + +The **Requests Profiles** page contains the token required for profiling. For more details, see [Request Profiling](../../administration/monitoring/performance/request_profiling.md). + +### Audit Log **[PREMIUM ONLY]** + +The **Audit Log** page lists changes made within the GitLab server. With this information you can control, analyze, and track every change. diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 97d2dfc0f7e..c6ee168bad0 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -533,22 +533,20 @@ This job failed because the necessary resources were not successfully created. To find the cause of this error when creating a namespace and service account, check the [logs](../../../administration/logs.md#kuberneteslog). -NOTE: **NOTE:** -As of GitLab 12.1 we require [`cluster-admin`](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles) -tokens for all project level clusters unless you unselect the -[GitLab-managed cluster](#gitlab-managed-clusters) option. If you -want to manage namespaces and service accounts yourself and don't -want to provide a `cluster-admin` token to GitLab you must unselect this -option or you will get the above error. - -Common reasons for failure include: +Reasons for failure include: -- The token you gave GitLab did not have [`cluster-admin`](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles) +- The token you gave GitLab does not have [`cluster-admin`](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles) privileges required by GitLab. - Missing `KUBECONFIG` or `KUBE_TOKEN` variables. To be passed to your job, they must have a matching [`environment:name`](../../../ci/environments.md#defining-environments). If your job has no `environment:name` set, it will not be passed the Kubernetes credentials. +NOTE: **NOTE:** +Project-level clusters upgraded from GitLab 12.0 or older may be configured +in a way that causes this error. Ensure you deselect the +[GitLab-managed cluster](#gitlab-managed-clusters) option if you want to manage +namespaces and service accounts yourself. + ## Monitoring your Kubernetes cluster **[ULTIMATE]** > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4701) in [GitLab Ultimate][ee] 10.6. diff --git a/doc/user/project/clusters/serverless/img/function-endpoint.png b/doc/user/project/clusters/serverless/img/function-endpoint.png Binary files differnew file mode 100644 index 00000000000..f3c7ae7a00d --- /dev/null +++ b/doc/user/project/clusters/serverless/img/function-endpoint.png diff --git a/doc/user/project/clusters/serverless/index.md b/doc/user/project/clusters/serverless/index.md index 3be5bfeaddc..91f0e24b44e 100644 --- a/doc/user/project/clusters/serverless/index.md +++ b/doc/user/project/clusters/serverless/index.md @@ -325,3 +325,275 @@ loading or is not available at this time._ It will appear upon the first access page, but should go away after a few seconds. If the message does not disappear, then it is possible that GitLab is unable to connect to the Prometheus instance running on the cluster. + +## Enabling TLS for Knative services + +By default, a GitLab serverless deployment will be served over `http`. In order to serve over `https` you +must manually obtain and install TLS certificates. + +The simplest way to accomplish this is to +use [Certbot to manually obtain Let's Encrypt certificates](https://knative.dev/docs/serving/using-a-tls-cert/#using-certbot-to-manually-obtain-let-s-encrypt-certificates). Certbot is a free, open source software tool for automatically using Let’s Encrypt certificates on manually-administrated websites to enable HTTPS. + +NOTE: **Note:** +The instructions below relate to installing and running Certbot on a Linux server and may not work on other operating systems. + +1. Install Certbot by running the + [`certbot-auto` wrapper script](https://certbot.eff.org/docs/install.html#certbot-auto). + On the command line of your server, run the following commands: + + ```sh + wget https://dl.eff.org/certbot-auto + sudo mv certbot-auto /usr/local/bin/certbot-auto + sudo chown root /usr/local/bin/certbot-auto + chmod 0755 /usr/local/bin/certbot-auto + /usr/local/bin/certbot-auto --help + ``` + + To check the integrity of the `certbot-auto` script, run: + + ```sh + wget -N https://dl.eff.org/certbot-auto.asc + gpg2 --keyserver ipv4.pool.sks-keyservers.net --recv-key A2CFB51FA275A7286234E7B24D17C995CD9775F2 + gpg2 --trusted-key 4D17C995CD9775F2 --verify certbot-auto.asc /usr/local/bin/certbot-auto + ``` + + The output of the last command should look something like: + + ```sh + gpg: Signature made Mon 10 Jun 2019 06:24:40 PM EDT + gpg: using RSA key A2CFB51FA275A7286234E7B24D17C995CD9775F2 + gpg: key 4D17C995CD9775F2 marked as ultimately trusted + gpg: checking the trustdb + gpg: marginals needed: 3 completes needed: 1 trust model: pgp + gpg: depth: 0 valid: 1 signed: 0 trust: 0-, 0q, 0n, 0m, 0f, 1u + gpg: next trustdb check due at 2027-11-22 + gpg: Good signature from "Let's Encrypt Client Team <letsencrypt-client@eff.org>" [ultimate] + ``` + +1. Run the following command to use Certbot to request a certificate + using DNS challenge during authorization: + + + ```sh + ./certbot-auto certonly --manual --preferred-challenges dns -d '*.<namespace>.example.com' + ``` + + Where `<namespace>` is the namespace created by GitLab for your serverless project (composed of `<projectname+id>`) and + `example.com` is the domain being used for your project. If you are unsure what the namespace of your project is, navigate + to the **Operations > Serverless** page of your project and inspect + the endpoint provided for your function/app. + +  + + In the above image, the namespace for the project is `node-function-11909507` and the domain is `knative.info`, thus + certificate request line would look like this: + + ```sh + ./certbot-auto certonly --manual --preferred-challenges dns -d '*.node-function-11909507.knative.info' + ``` + + The Certbot tool walks you through the steps of validating that you own each domain that you specify by creating TXT records in those domains. + After this process is complete, the output should look something like this: + + ```sh + IMPORTANT NOTES: + - Congratulations! Your certificate and chain have been saved at: + /etc/letsencrypt/live/namespace.example.com/fullchain.pem + Your key file has been saved at: + /etc/letsencrypt/live/namespace.example/privkey.pem + Your cert will expire on 2019-09-19. To obtain a new or tweaked + version of this certificate in the future, simply run certbot-auto + again. To non-interactively renew *all* of your certificates, run + "certbot-auto renew" + -----BEGIN PRIVATE KEY----- + - Your account credentials have been saved in your Certbot + configuration directory at /etc/letsencrypt. You should make a + secure backup of this folder now. This configuration directory will + also contain certificates and private keys obtained by Certbot so + making regular backups of this folder is ideal. + ``` + +1. Create certificate and private key files. Using the contents of the files + returned by Certbot, we'll create two files in order to create the + Kubernetes secret: + + Run the following command to see the contents of `fullchain.pem`: + + ```sh + sudo cat /etc/letsencrypt/live/node-function-11909507.knative.info/fullchain.pem + ``` + + Output should look like this: + + ```sh + -----BEGIN CERTIFICATE----- + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b4ag== + -----END CERTIFICATE----- + -----BEGIN CERTIFICATE----- + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + K2fcb195768c39e9a94cec2c2e30Qg== + -----END CERTIFICATE----- + ``` + + Create a file with the name `cert.pem` with the contents of the entire output. + + Once `cert.pem` is created, run the following command to see the contents of `privkey.pem`: + + ```sh + sudo cat /etc/letsencrypt/live/namespace.example/privkey.pem + ``` + + Output should look like this: + + ```sh + -----BEGIN PRIVATE KEY----- + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + 2fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 04f294d1eaca42b8692017b426d53bbc8fe75f827734f0260710b83a556082df + -----BEGIN CERTIFICATE----- + fcb195768c39e9a94cec2c2e32c59c0aad7a3365c10892e8116b5d83d4096b6 + 4f294d1eaca42b8692017b4262== + -----END PRIVATE KEY----- + ``` + + Create a new file with the name `cert.pk` with the contents of the entire output. + +1. Create a Kubernetes secret to hold your TLS certificate, `cert.pem`, and + the private key `cert.pk`: + + NOTE: **Note:** + Running `kubectl` commands on your cluster requires setting up access to the cluster first. + For clusters created on GKE, see + [GKE Cluster Access](https://cloud.google.com/kubernetes-engine/docs/how-to/cluster-access-for-kubectl). + For other platforms, [install `kubectl`](https://kubernetes.io/docs/tasks/tools/install-kubectl/). + + ```sh + kubectl create --namespace istio-system secret tls istio-ingressgateway-certs \ + --key cert.pk \ + --cert cert.pem + ``` + + Where `cert.pem` and `cert.pk` are your certificate and private key files. Note that the `istio-ingressgateway-certs` secret name is required. + +1. Configure Knative to use the new secret that you created for HTTPS + connections. Run the + following command to open the Knative shared `gateway` in edit mode: + + ```sh + kubectl edit gateway knative-ingress-gateway --namespace knative-serving + ``` + + Update the gateway to include the following tls: section and configuration: + + ```sh + tls: + mode: SIMPLE + privateKey: /etc/istio/ingressgateway-certs/tls.key + serverCertificate: /etc/istio/ingressgateway-certs/tls.crt + ``` + + Example: + + ```sh + apiVersion: networking.istio.io/v1alpha3 + kind: Gateway + metadata: + # ... skipped ... + spec: + selector: + istio: ingressgateway + servers: + - hosts: + - "*" + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - "*" + port: + name: https + number: 443 + protocol: HTTPS + tls: + mode: SIMPLE + privateKey: /etc/istio/ingressgateway-certs/tls.key + serverCertificate: /etc/istio/ingressgateway-certs/tls.crt + ``` + + After your changes are running on your Knative cluster, you can begin using the HTTPS protocol for secure access your deployed Knative services. + In the event a mistake is made during this process and you need to update the cert, you will need to edit the gateway `knative-ingress-gateway` + to switch back to `PASSTHROUGH` mode. Once corrections are made, edit the file again so the gateway will use the new certificates.
\ No newline at end of file diff --git a/lib/api/boards.rb b/lib/api/boards.rb index b7c77730afb..4e31f74f18a 100644 --- a/lib/api/boards.rb +++ b/lib/api/boards.rb @@ -27,7 +27,7 @@ module API end get '/' do authorize!(:read_board, user_project) - present paginate(board_parent.boards), with: Entities::Board + present paginate(board_parent.boards.with_associations), with: Entities::Board end desc 'Find a project board' do diff --git a/lib/api/boards_responses.rb b/lib/api/boards_responses.rb index 86d9b24802f..68497a08fb8 100644 --- a/lib/api/boards_responses.rb +++ b/lib/api/boards_responses.rb @@ -11,7 +11,7 @@ module API end def board_lists - board.lists.destroyable + board.destroyable_lists end def create_list diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ead01dc53f7..d783591c238 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1101,7 +1101,7 @@ module API expose :project, using: Entities::BasicProjectDetails expose :lists, using: Entities::List do |board| - board.lists.destroyable + board.destroyable_lists end end diff --git a/lib/api/group_boards.rb b/lib/api/group_boards.rb index 9a20ee8c8b9..feb2254963e 100644 --- a/lib/api/group_boards.rb +++ b/lib/api/group_boards.rb @@ -37,7 +37,7 @@ module API use :pagination end get '/' do - present paginate(board_parent.boards), with: Entities::Board + present paginate(board_parent.boards.with_associations), with: Entities::Board end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 9ab5fa8d0bd..41418aa216c 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -158,6 +158,7 @@ module API at_least_one_of :password, :reset_password requires :name, type: String, desc: 'The name of the user' requires :username, type: String, desc: 'The username of the user' + optional :force_random_password, type: Boolean, desc: 'Flag indicating a random password will be set' use :optional_attributes end post do diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 0b12e862ded..e2cbf91f281 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -434,7 +434,8 @@ module Gitlab end begin - update_column_in_batches(table, column, default, &block) + default_after_type_cast = connection.type_cast(default, column_for(table, column)) + update_column_in_batches(table, column, default_after_type_cast, &block) change_column_null(table, column, false) unless allow_null # We want to rescue _all_ exceptions here, even those that don't inherit diff --git a/lib/gitlab/git/raw_diff_change.rb b/lib/gitlab/git/raw_diff_change.rb index e1002af40f6..9a41f04a4db 100644 --- a/lib/gitlab/git/raw_diff_change.rb +++ b/lib/gitlab/git/raw_diff_change.rb @@ -11,8 +11,8 @@ module Gitlab if raw_change.is_a?(Gitaly::GetRawChangesResponse::RawChange) @blob_id = raw_change.blob_id @blob_size = raw_change.size - @old_path = raw_change.old_path.presence - @new_path = raw_change.new_path.presence + @old_path = raw_change.old_path_bytes.presence + @new_path = raw_change.new_path_bytes.presence @operation = raw_change.operation&.downcase || :unknown else parse(raw_change) diff --git a/lib/gitlab/graphql/copy_field_description.rb b/lib/gitlab/graphql/copy_field_description.rb new file mode 100644 index 00000000000..edd73083ff2 --- /dev/null +++ b/lib/gitlab/graphql/copy_field_description.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module CopyFieldDescription + extend ActiveSupport::Concern + + class_methods do + # Returns the `description` for property of field `field_name` on type. + # This can be used to ensure, for example, that mutation argument descriptions + # are always identical to the corresponding query field descriptions. + # + # E.g.: + # argument :name, GraphQL::STRING_TYPE, description: copy_field_description(Types::UserType, :name) + def copy_field_description(type, field_name) + type.fields[field_name.to_s.camelize(:lower)].description + end + end + end + end +end diff --git a/lib/gitlab/graphql/errors.rb b/lib/gitlab/graphql/errors.rb index fe74549e322..40b90310e8b 100644 --- a/lib/gitlab/graphql/errors.rb +++ b/lib/gitlab/graphql/errors.rb @@ -6,6 +6,7 @@ module Gitlab BaseError = Class.new(GraphQL::ExecutionError) ArgumentError = Class.new(BaseError) ResourceNotAvailable = Class.new(BaseError) + MutationError = Class.new(BaseError) end end end diff --git a/lib/gitlab/metrics/dashboard/base_service.rb b/lib/gitlab/metrics/dashboard/base_service.rb index 90895eb237a..0628e82e592 100644 --- a/lib/gitlab/metrics/dashboard/base_service.rb +++ b/lib/gitlab/metrics/dashboard/base_service.rb @@ -10,6 +10,8 @@ module Gitlab NOT_FOUND_ERROR = Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError def get_dashboard + return error('Insufficient permissions.', :unauthorized) unless allowed? + success(dashboard: process_dashboard) rescue NOT_FOUND_ERROR error("#{dashboard_path} could not be found.", :not_found) @@ -30,6 +32,12 @@ module Gitlab private + # Determines whether users should be able to view + # dashboards at all. + def allowed? + Ability.allowed?(current_user, :read_environment, project) + end + # Returns a new dashboard Hash, supplemented with DB info def process_dashboard Gitlab::Metrics::Dashboard::Processor diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9ea368816f9..b8ce2c20563 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1991,9 +1991,6 @@ msgstr "" msgid "Choose visibility level, enable/disable project features (issues, repository, wiki, snippets) and set permissions." msgstr "" -msgid "Choose your merge method, options, checks, and set up a default merge request description template." -msgstr "" - msgid "CiStatusLabel|canceled" msgstr "" @@ -6652,6 +6649,9 @@ msgstr "" msgid "No" msgstr "" +msgid "No %{header} for this request." +msgstr "" + msgid "No %{providerTitle} repositories available to import" msgstr "" @@ -7137,6 +7137,18 @@ msgstr "" msgid "Performance optimization" msgstr "" +msgid "PerformanceBar|Gitaly calls" +msgstr "" + +msgid "PerformanceBar|SQL queries" +msgstr "" + +msgid "PerformanceBar|profile" +msgstr "" + +msgid "PerformanceBar|trace" +msgstr "" + msgid "Permissions" msgstr "" @@ -8058,6 +8070,9 @@ msgstr "" msgid "ProjectSettings|Badges" msgstr "" +msgid "ProjectSettings|Choose your merge method, merge options, and merge checks." +msgstr "" + msgid "ProjectSettings|Customize your project badges." msgstr "" diff --git a/spec/controllers/admin/clusters/applications_controller_spec.rb b/spec/controllers/admin/clusters/applications_controller_spec.rb index 76f261e7d3f..cf202d88acc 100644 --- a/spec/controllers/admin/clusters/applications_controller_spec.rb +++ b/spec/controllers/admin/clusters/applications_controller_spec.rb @@ -13,16 +13,6 @@ describe Admin::Clusters::ApplicationsController do it { expect { subject }.to be_allowed_for(:admin) } it { expect { subject }.to be_denied_for(:user) } it { expect { subject }.to be_denied_for(:external) } - - context 'when instance clusters are disabled' do - before do - stub_feature_flags(instance_clusters: false) - end - - it 'returns 404' do - is_expected.to have_http_status(:not_found) - end - end end let(:cluster) { create(:cluster, :instance, :provided_by_gcp) } diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index 7709f525119..e5501535875 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -17,66 +17,48 @@ describe Admin::ClustersController do get :index, params: params end - context 'when feature flag is not enabled' do - before do - stub_feature_flags(instance_clusters: false) - end + describe 'functionality' do + context 'when instance has one or more clusters' do + let!(:enabled_cluster) do + create(:cluster, :provided_by_gcp, :instance) + end - it 'responds with not found' do - get_index + let!(:disabled_cluster) do + create(:cluster, :disabled, :provided_by_gcp, :production_environment, :instance) + end - expect(response).to have_gitlab_http_status(404) - end - end + it 'lists available clusters' do + get_index - context 'when feature flag is enabled' do - before do - stub_feature_flags(instance_clusters: true) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index) + expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) + end - describe 'functionality' do - context 'when instance has one or more clusters' do - let!(:enabled_cluster) do - create(:cluster, :provided_by_gcp, :instance) - end + context 'when page is specified' do + let(:last_page) { Clusters::Cluster.instance_type.page.total_pages } - let!(:disabled_cluster) do - create(:cluster, :disabled, :provided_by_gcp, :production_environment, :instance) + before do + allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) + create_list(:cluster, 2, :provided_by_gcp, :production_environment, :instance) end - it 'lists available clusters' do - get_index + it 'redirects to the page' do + get_index(page: last_page) expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index) - expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) - end - - context 'when page is specified' do - let(:last_page) { Clusters::Cluster.instance_type.page.total_pages } - - before do - allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) - create_list(:cluster, 2, :provided_by_gcp, :production_environment, :instance) - end - - it 'redirects to the page' do - get_index(page: last_page) - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:clusters).current_page).to eq(last_page) - end + expect(assigns(:clusters).current_page).to eq(last_page) end end + end - context 'when instance does not have a cluster' do - it 'returns an empty state page' do - get_index + context 'when instance does not have a cluster' do + it 'returns an empty state page' do + get_index - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index, partial: :empty_state) - expect(assigns(:clusters)).to eq([]) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index, partial: :empty_state) + expect(assigns(:clusters)).to eq([]) end end end diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 2f64c7f3460..09677b42887 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -20,70 +20,52 @@ describe Groups::ClustersController do get :index, params: params.reverse_merge(group_id: group) end - context 'when feature flag is not enabled' do - before do - stub_feature_flags(group_clusters: false) - end + describe 'functionality' do + context 'when group has one or more clusters' do + let(:group) { create(:group) } - it 'renders 404' do - go + let!(:enabled_cluster) do + create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group]) + end - expect(response).to have_gitlab_http_status(404) - end - end + let!(:disabled_cluster) do + create(:cluster, :disabled, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group]) + end - context 'when feature flag is enabled' do - before do - stub_feature_flags(group_clusters: true) - end + it 'lists available clusters' do + go - describe 'functionality' do - context 'when group has one or more clusters' do - let(:group) { create(:group) } + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index) + expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) + end - let!(:enabled_cluster) do - create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group]) - end + context 'when page is specified' do + let(:last_page) { group.clusters.page.total_pages } - let!(:disabled_cluster) do - create(:cluster, :disabled, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group]) + before do + allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) + create_list(:cluster, 2, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group]) end - it 'lists available clusters' do - go + it 'redirects to the page' do + go(page: last_page) expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index) - expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) - end - - context 'when page is specified' do - let(:last_page) { group.clusters.page.total_pages } - - before do - allow(Clusters::Cluster).to receive(:paginates_per).and_return(1) - create_list(:cluster, 2, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group]) - end - - it 'redirects to the page' do - go(page: last_page) - - expect(response).to have_gitlab_http_status(:ok) - expect(assigns(:clusters).current_page).to eq(last_page) - end + expect(assigns(:clusters).current_page).to eq(last_page) end end + end - context 'when group does not have a cluster' do - let(:group) { create(:group) } + context 'when group does not have a cluster' do + let(:group) { create(:group) } - it 'returns an empty state page' do - go + it 'returns an empty state page' do + go - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:index, partial: :empty_state) - expect(assigns(:clusters)).to eq([]) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index, partial: :empty_state) + expect(assigns(:clusters)).to eq([]) end end end diff --git a/spec/controllers/projects/merge_requests/content_controller_spec.rb b/spec/controllers/projects/merge_requests/content_controller_spec.rb new file mode 100644 index 00000000000..2879e06aee4 --- /dev/null +++ b/spec/controllers/projects/merge_requests/content_controller_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::MergeRequests::ContentController do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, target_project: project, source_project: project) } + + before do + sign_in(user) + end + + def do_request + get :widget, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid, + format: :json + } + end + + describe 'GET widget' do + context 'user has access to the project' do + before do + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + + project.add_maintainer(user) + end + + it 'renders widget MR entity as json' do + do_request + + expect(response).to match_response_schema('entities/merge_request_widget') + end + + it 'checks whether the MR can be merged' do + controller.instance_variable_set(:@merge_request, merge_request) + + expect(merge_request).to receive(:check_mergeability) + + do_request + end + + it 'closes an MR with moved source project' do + merge_request.update_column(:source_project_id, nil) + + expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false) + end + end + + context 'user does not have access to the project' do + it 'renders widget MR entity as json' do + do_request + + expect(response).to have_http_status(:not_found) + end + end + end +end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 4634d1d4bb3..5a5c0a1f6ac 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -17,6 +17,10 @@ describe SearchController do set(:project) { create(:project, :public, :repository, :wiki_repo) } + before do + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + end + subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) } where(:partial, :scope) do @@ -35,6 +39,19 @@ describe SearchController do end end + context 'global search' do + render_views + + it 'omits pipeline status from load' do + project = create(:project, :public) + expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) + + get :show, params: { scope: 'projects', search: project.name } + + expect(assigns[:search_objects].first).to eq project + end + end + it 'finds issue comments' do project = create(:project, :public) note = create(:note_on_issue, project: project) diff --git a/spec/factories/award_emoji.rb b/spec/factories/award_emoji.rb index d37e2bf511e..43753fa650c 100644 --- a/spec/factories/award_emoji.rb +++ b/spec/factories/award_emoji.rb @@ -5,7 +5,7 @@ FactoryBot.define do awardable factory: :issue after(:create) do |award, evaluator| - award.awardable.project.add_guest(evaluator.user) + award.awardable.project&.add_guest(evaluator.user) end trait :upvote diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 08fa4a98feb..260eec7a9ed 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -362,14 +362,14 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end end - it 'shows jump to next discussion button except on last discussion' do + it 'shows jump to next discussion button on all discussions' do wait_for_requests all_discussion_replies = page.all('.discussion-reply-holder') expect(all_discussion_replies.count).to eq(2) expect(all_discussion_replies.first.all('.discussion-next-btn').count).to eq(1) - expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(0) + expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(1) end it 'displays next discussion even if hidden' do diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 7018cb9a305..eac1dbc6474 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -99,7 +99,8 @@ "revert_in_fork_path": { "type": ["string", "null"] }, "email_patches_path": { "type": "string" }, "plain_diff_path": { "type": "string" }, - "status_path": { "type": "string" }, + "merge_request_basic_path": { "type": "string" }, + "merge_request_widget_path": { "type": "string" }, "new_blob_path": { "type": ["string", "null"] }, "merge_check_path": { "type": "string" }, "ci_environments_status_path": { "type": "string" }, diff --git a/spec/frontend/notes/components/discussion_notes_spec.js b/spec/frontend/notes/components/discussion_notes_spec.js index c3204b3aaa0..394666403ee 100644 --- a/spec/frontend/notes/components/discussion_notes_spec.js +++ b/spec/frontend/notes/components/discussion_notes_spec.js @@ -112,6 +112,44 @@ describe('DiscussionNotes', () => { }); }); + describe('events', () => { + describe('with groupped notes and replies expanded', () => { + const findNoteAtIndex = index => wrapper.find(`.note:nth-of-type(${index + 1}`); + + beforeEach(() => { + createComponent({ shouldGroupReplies: true, isExpanded: true }); + }); + + it('emits deleteNote when first note emits handleDeleteNote', () => { + findNoteAtIndex(0).vm.$emit('handleDeleteNote'); + expect(wrapper.emitted().deleteNote).toBeTruthy(); + }); + + it('emits startReplying when first note emits startReplying', () => { + findNoteAtIndex(0).vm.$emit('startReplying'); + expect(wrapper.emitted().startReplying).toBeTruthy(); + }); + + it('emits deleteNote when second note emits handleDeleteNote', () => { + findNoteAtIndex(1).vm.$emit('handleDeleteNote'); + expect(wrapper.emitted().deleteNote).toBeTruthy(); + }); + }); + + describe('with ungroupped notes', () => { + let note; + beforeEach(() => { + createComponent(); + note = wrapper.find('.note'); + }); + + it('emits deleteNote when first note emits handleDeleteNote', () => { + note.vm.$emit('handleDeleteNote'); + expect(wrapper.emitted().deleteNote).toBeTruthy(); + }); + }); + }); + describe('componentData', () => { beforeEach(() => { createComponent(); diff --git a/spec/graphql/types/award_emojis/award_emoji_type_spec.rb b/spec/graphql/types/award_emojis/award_emoji_type_spec.rb new file mode 100644 index 00000000000..5663a3d7195 --- /dev/null +++ b/spec/graphql/types/award_emojis/award_emoji_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['AwardEmoji'] do + it { expect(described_class.graphql_name).to eq('AwardEmoji') } + + it { is_expected.to require_graphql_authorizations(:read_emoji) } + + it { expect(described_class).to have_graphql_fields(:description, :unicode_version, :emoji, :name, :unicode, :user) } +end diff --git a/spec/javascripts/monitoring/charts/area_spec.js b/spec/javascripts/monitoring/charts/area_spec.js index ac7e0bb12a1..d3a76f33679 100644 --- a/spec/javascripts/monitoring/charts/area_spec.js +++ b/spec/javascripts/monitoring/charts/area_spec.js @@ -1,14 +1,19 @@ import { shallowMount } from '@vue/test-utils'; +import { GlLink } from '@gitlab/ui'; import { GlAreaChart, GlChartSeriesLabel } from '@gitlab/ui/dist/charts'; import { shallowWrapperContainsSlotText } from 'spec/helpers/vue_test_utils_helper'; import Area from '~/monitoring/components/charts/area.vue'; import { createStore } from '~/monitoring/stores'; import * as types from '~/monitoring/stores/mutation_types'; +import { TEST_HOST } from 'spec/test_constants'; import MonitoringMock, { deploymentData } from '../mock_data'; describe('Area component', () => { + const mockSha = 'mockSha'; const mockWidgets = 'mockWidgets'; const mockSvgPathContent = 'mockSvgPathContent'; + const projectPath = `${TEST_HOST}/path/to/project`; + const commitUrl = `${projectPath}/commit/${mockSha}`; let mockGraphData; let areaChart; let spriteSpy; @@ -26,6 +31,7 @@ describe('Area component', () => { graphData: mockGraphData, containerWidth: 0, deploymentData: store.state.monitoringDashboard.deploymentData, + projectPath, }, slots: { default: mockWidgets, @@ -88,11 +94,14 @@ describe('Area component', () => { ); }); - it('renders commit sha in tooltip content', () => { - const mockSha = 'mockSha'; + it('renders clickable commit sha in tooltip content', () => { areaChart.vm.tooltip.sha = mockSha; + areaChart.vm.tooltip.commitUrl = commitUrl; - expect(shallowWrapperContainsSlotText(glAreaChart, 'tooltipContent', mockSha)).toBe(true); + const commitLink = areaChart.find(GlLink); + + expect(shallowWrapperContainsSlotText(commitLink, 'default', mockSha)).toBe(true); + expect(commitLink.attributes('href')).toEqual(commitUrl); }); }); }); diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js index 8f3c493dd4c..c3ed079e33b 100644 --- a/spec/javascripts/notes/stores/getters_spec.js +++ b/spec/javascripts/notes/stores/getters_spec.js @@ -32,6 +32,26 @@ describe('Getters Notes Store', () => { }; }); + describe('showJumpToNextDiscussion', () => { + it('should return true if there are 2 or more unresolved discussions', () => { + const localGetters = { + unresolvedDiscussionsIdsByDate: ['123', '456'], + allResolvableDiscussions: [], + }; + + expect(getters.showJumpToNextDiscussion(state, localGetters)()).toBe(true); + }); + + it('should return false if there are 1 or less unresolved discussions', () => { + const localGetters = { + unresolvedDiscussionsIdsByDate: ['123'], + allResolvableDiscussions: [], + }; + + expect(getters.showJumpToNextDiscussion(state, localGetters)()).toBe(false); + }); + }); + describe('discussions', () => { it('should return all discussions in the store', () => { expect(getters.discussions(state)).toEqual([individualNote]); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 48f812f0db4..253413ae43e 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -218,7 +218,8 @@ export default { '/root/acets-app/forks?continue%5Bnotice%5D=You%27re+not+allowed+to+make+changes+to+this+project+directly.+A+fork+of+this+project+has+been+created+that+you+can+make+changes+in%2C+so+you+can+submit+a+merge+request.+Try+to+cherry-pick+this+commit+again.&continue%5Bnotice_now%5D=You%27re+not+allowed+to+make+changes+to+this+project+directly.+A+fork+of+this+project+is+being+created+that+you+can+make+changes+in%2C+so+you+can+submit+a+merge+request.&continue%5Bto%5D=%2Froot%2Facets-app%2Fmerge_requests%2F22&namespace_key=1', email_patches_path: '/root/acets-app/merge_requests/22.patch', plain_diff_path: '/root/acets-app/merge_requests/22.diff', - status_path: '/root/acets-app/merge_requests/22.json', + merge_request_basic_path: '/root/acets-app/merge_requests/22.json?serializer=basic', + merge_request_widget_path: '/root/acets-app/merge_requests/22/widget.json', merge_check_path: '/root/acets-app/merge_requests/22/merge_check', ci_environments_status_url: '/root/acets-app/merge_requests/22/ci_environments_status', project_archived: false, diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index ac2fb16bd10..30e0504e4e1 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -473,7 +473,7 @@ describe('mrWidgetOptions', () => { vm.mr.relatedLinks = { assignToMe: null, closing: ` - <a class="close-related-link" href="#'> + <a class="close-related-link" href="#"> Close </a> `, diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 3cf3d032bf4..7409572288c 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -583,6 +583,24 @@ describe Gitlab::Database::MigrationHelpers do model.add_column_with_default(:projects, :foo, :integer, default: 10, limit: 8) end end + + it 'adds a column with an array default value for a jsonb type' do + create(:project) + allow(model).to receive(:transaction_open?).and_return(false) + allow(model).to receive(:transaction).and_yield + expect(model).to receive(:update_column_in_batches).with(:projects, :foo, '[{"foo":"json"}]').and_call_original + + model.add_column_with_default(:projects, :foo, :jsonb, default: [{ foo: "json" }]) + end + + it 'adds a column with an object default value for a jsonb type' do + create(:project) + allow(model).to receive(:transaction_open?).and_return(false) + allow(model).to receive(:transaction).and_yield + expect(model).to receive(:update_column_in_batches).with(:projects, :foo, '{"foo":"json"}').and_call_original + + model.add_column_with_default(:projects, :foo, :jsonb, default: { foo: "json" }) + end end context 'inside a transaction' do diff --git a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb index 20842f55014..50138d272c4 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb @@ -67,7 +67,7 @@ describe Gitlab::Graphql::Authorize::AuthorizeResource do end describe '#authorize!' do - it 'does not raise an error' do + it 'raises an error' do expect { loading_resource.authorize!(project) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) end end diff --git a/spec/lib/gitlab/graphql/copy_field_description_spec.rb b/spec/lib/gitlab/graphql/copy_field_description_spec.rb new file mode 100644 index 00000000000..e7462c5b954 --- /dev/null +++ b/spec/lib/gitlab/graphql/copy_field_description_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::CopyFieldDescription do + subject { Class.new.include(described_class) } + + describe '.copy_field_description' do + let(:type) do + Class.new(Types::BaseObject) do + graphql_name "TestType" + + field :field_name, GraphQL::STRING_TYPE, null: true, description: 'Foo' + end + end + + it 'returns the correct description' do + expect(subject.copy_field_description(type, :field_name)).to eq('Foo') + end + end +end diff --git a/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb index eecd257b38d..79a78df44ae 100644 --- a/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb @@ -6,13 +6,19 @@ describe Gitlab::Metrics::Dashboard::DynamicDashboardService, :use_clean_rails_m include MetricsDashboardHelpers set(:project) { build(:project) } + set(:user) { create(:user) } set(:environment) { create(:environment, project: project) } + before do + project.add_maintainer(user) + end + describe '#get_dashboard' do - let(:service_params) { [project, nil, { environment: environment, dashboard_path: nil }] } + let(:service_params) { [project, user, { environment: environment, dashboard_path: nil }] } let(:service_call) { described_class.new(*service_params).get_dashboard } it_behaves_like 'valid embedded dashboard service response' + it_behaves_like 'raises error for users with insufficient permissions' it 'caches the unprocessed dashboard for subsequent calls' do expect(YAML).to receive(:safe_load).once.and_call_original diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index b9a5ee9c2b3..d8ed54c0248 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -6,12 +6,17 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi include MetricsDashboardHelpers set(:project) { build(:project) } + set(:user) { create(:user) } set(:environment) { create(:environment, project: project) } let(:system_dashboard_path) { Gitlab::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH} + before do + project.add_maintainer(user) + end + describe '.find' do let(:dashboard_path) { '.gitlab/dashboards/test.yml' } - let(:service_call) { described_class.find(project, nil, environment, dashboard_path: dashboard_path) } + let(:service_call) { described_class.find(project, user, environment, dashboard_path: dashboard_path) } it_behaves_like 'misconfigured dashboard service response', :not_found @@ -41,13 +46,13 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi end context 'when no dashboard is specified' do - let(:service_call) { described_class.find(project, nil, environment) } + let(:service_call) { described_class.find(project, user, environment) } it_behaves_like 'valid dashboard service response' end context 'when the dashboard is expected to be embedded' do - let(:service_call) { described_class.find(project, nil, environment, dashboard_path: nil, embedded: true) } + let(:service_call) { described_class.find(project, user, environment, dashboard_path: nil, embedded: true) } it_behaves_like 'valid embedded dashboard service response' end diff --git a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb index 57d82421b5d..468e8ec9ef2 100644 --- a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb @@ -5,8 +5,8 @@ require 'rails_helper' describe Gitlab::Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_memory_store_caching do include MetricsDashboardHelpers - set(:user) { build(:user) } - set(:project) { build(:project) } + set(:user) { create(:user) } + set(:project) { create(:project) } set(:environment) { create(:environment, project: project) } before do @@ -22,6 +22,8 @@ describe Gitlab::Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_m it_behaves_like 'misconfigured dashboard service response', :not_found end + it_behaves_like 'raises error for users with insufficient permissions' + context 'when the dashboard exists' do let(:project) { project_with_dashboard(dashboard_path) } diff --git a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb index 2af745bd4d7..13f22dd01c5 100644 --- a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb @@ -5,15 +5,21 @@ require 'spec_helper' describe Gitlab::Metrics::Dashboard::SystemDashboardService, :use_clean_rails_memory_store_caching do include MetricsDashboardHelpers - set(:project) { build(:project) } + set(:user) { create(:user) } + set(:project) { create(:project) } set(:environment) { create(:environment, project: project) } + before do + project.add_maintainer(user) + end + describe 'get_dashboard' do let(:dashboard_path) { described_class::SYSTEM_DASHBOARD_PATH } - let(:service_params) { [project, nil, { environment: environment, dashboard_path: dashboard_path }] } + let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] } let(:service_call) { described_class.new(*service_params).get_dashboard } it_behaves_like 'valid dashboard service response' + it_behaves_like 'raises error for users with insufficient permissions' it 'caches the unprocessed dashboard for subsequent calls' do expect(YAML).to receive(:safe_load).once.and_call_original diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index 96465a51db2..2378f400540 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -82,16 +82,6 @@ describe DeploymentPlatform do end end end - - context 'feature flag disabled' do - before do - stub_feature_flags(group_clusters: false) - end - - it 'returns nil' do - is_expected.to be_nil - end - end end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d7accbef6bd..470ce65707d 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -866,35 +866,6 @@ describe Group do end end - describe '#group_clusters_enabled?' do - before do - # Override global stub in spec/spec_helper.rb - expect(Feature).to receive(:enabled?).and_call_original - end - - subject { group.group_clusters_enabled? } - - it { is_expected.to be_truthy } - - context 'explicitly disabled for root ancestor' do - before do - feature = Feature.get(:group_clusters) - feature.disable(group.root_ancestor) - end - - it { is_expected.to be_falsey } - end - - context 'explicitly disabled for root ancestor' do - before do - feature = Feature.get(:group_clusters) - feature.enable(group.root_ancestor) - end - - it { is_expected.to be_truthy } - end - end - describe '#first_auto_devops_config' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/postgresql/replication_slot_spec.rb b/spec/models/postgresql/replication_slot_spec.rb index e100af7ddc7..95ae204a8a8 100644 --- a/spec/models/postgresql/replication_slot_spec.rb +++ b/spec/models/postgresql/replication_slot_spec.rb @@ -47,5 +47,13 @@ describe Postgresql::ReplicationSlot, :postgresql do expect(described_class.lag_too_great?).to eq(false) end + + it 'returns false when there is a nil replication lag' do + expect(described_class) + .to receive(:pluck) + .and_return([0.megabytes, nil]) + + expect(described_class.lag_too_great?).to eq(false) + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cc0f5002a1e..1bc092fa41a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -497,7 +497,6 @@ describe Project do it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) } it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) } - it { is_expected.to delegate_method(:group_clusters_enabled?).to(:group).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) } it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) } end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index c5ab7e57272..0bd17dbacd7 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -844,6 +844,19 @@ describe Repository do end end + describe '#get_raw_changes' do + context `with non-UTF8 bytes in paths` do + let(:old_rev) { 'd0888d297eadcd7a345427915c309413b1231e65' } + let(:new_rev) { '19950f03c765f7ac8723a73a0599764095f52fc0' } + let(:changes) { repository.raw_changes_between(old_rev, new_rev) } + + it 'returns the changes' do + expect { changes }.not_to raise_error + expect(changes.first.new_path.bytes).to eq("hello\x80world".bytes) + end + end + end + describe '#create_ref' do it 'redirects the call to write_ref' do ref, ref_path = '1', '2' diff --git a/spec/policies/award_emoji_policy_spec.rb b/spec/policies/award_emoji_policy_spec.rb new file mode 100644 index 00000000000..2e3693c58d7 --- /dev/null +++ b/spec/policies/award_emoji_policy_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojiPolicy do + let(:user) { create(:user) } + let(:award_emoji) { create(:award_emoji, awardable: awardable) } + + subject { described_class.new(user, award_emoji) } + + shared_examples 'when the user can read the awardable' do + context do + let(:project) { create(:project, :public) } + + it { expect_allowed(:read_emoji) } + end + end + + shared_examples 'when the user cannot read the awardable' do + context do + let(:project) { create(:project, :private) } + + it { expect_disallowed(:read_emoji) } + end + end + + context 'when the awardable is an issue' do + let(:awardable) { create(:issue, project: project) } + + include_examples 'when the user can read the awardable' + include_examples 'when the user cannot read the awardable' + end + + context 'when the awardable is a merge request' do + let(:awardable) { create(:merge_request, source_project: project) } + + include_examples 'when the user can read the awardable' + include_examples 'when the user cannot read the awardable' + end + + context 'when the awardable is a note' do + let(:awardable) { create(:note_on_merge_request, project: project) } + + include_examples 'when the user can read the awardable' + include_examples 'when the user cannot read the awardable' + end + + context 'when the awardable is a snippet' do + let(:awardable) { create(:project_snippet, :public, project: project) } + + include_examples 'when the user can read the awardable' + include_examples 'when the user cannot read the awardable' + end +end diff --git a/spec/policies/clusters/instance_policy_spec.rb b/spec/policies/clusters/instance_policy_spec.rb index 9d755c6d29d..7b61819e079 100644 --- a/spec/policies/clusters/instance_policy_spec.rb +++ b/spec/policies/clusters/instance_policy_spec.rb @@ -16,21 +16,9 @@ describe Clusters::InstancePolicy do context 'when admin' do let(:user) { create(:admin) } - context 'with instance_level_clusters enabled' do - it { expect(policy).to be_allowed :read_cluster } - it { expect(policy).to be_allowed :update_cluster } - it { expect(policy).to be_allowed :admin_cluster } - end - - context 'with instance_level_clusters disabled' do - before do - stub_feature_flags(instance_clusters: false) - end - - it { expect(policy).to be_disallowed :read_cluster } - it { expect(policy).to be_disallowed :update_cluster } - it { expect(policy).to be_disallowed :admin_cluster } - end + it { expect(policy).to be_allowed :read_cluster } + it { expect(policy).to be_allowed :update_cluster } + it { expect(policy).to be_allowed :admin_cluster } end end end diff --git a/spec/presenters/award_emoji_presenter_spec.rb b/spec/presenters/award_emoji_presenter_spec.rb new file mode 100644 index 00000000000..e2ada2a3c93 --- /dev/null +++ b/spec/presenters/award_emoji_presenter_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojiPresenter do + let(:emoji_name) { 'thumbsup' } + let(:award_emoji) { build(:award_emoji, name: emoji_name) } + let(:presenter) { described_class.new(award_emoji) } + + describe '#description' do + it { expect(presenter.description).to eq Gitlab::Emoji.emojis[emoji_name]['description'] } + end + + describe '#unicode' do + it { expect(presenter.unicode).to eq Gitlab::Emoji.emojis[emoji_name]['unicode'] } + end + + describe '#unicode_version' do + it { expect(presenter.unicode_version).to eq Gitlab::Emoji.emoji_unicode_version(emoji_name) } + end + + describe '#emoji' do + it { expect(presenter.emoji).to eq Gitlab::Emoji.emojis[emoji_name]['moji'] } + end + + describe 'when presenting an award emoji with an invalid name' do + let(:emoji_name) { 'invalid-name' } + + it 'returns nil for all properties' do + expect(presenter.description).to be_nil + expect(presenter.emoji).to be_nil + expect(presenter.unicode).to be_nil + expect(presenter.unicode_version).to be_nil + end + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb new file mode 100644 index 00000000000..3982125a38a --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:mutation) do + variables = { + awardable_id: GitlabSchema.id_from_object(awardable).to_s, + name: emoji_name + } + + graphql_mutation(:add_award_emoji, variables) + end + + def mutation_response + graphql_mutation_response(:add_award_emoji) + end + + shared_examples 'a mutation that does not create an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + context 'when the user does not have permission' do + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + end + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do + let(:awardable) { create(:system_note) } + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable an Awardable' do + it 'creates an emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(1) + end + + it 'returns the emoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['awardEmoji']['name']).to eq(emoji_name) + end + + context 'when there were active record validation errors' do + before do + expect_next_instance_of(AwardEmoji) do |award| + expect(award).to receive(:valid?).at_least(:once).and_return(false) + expect(award).to receive_message_chain( + :errors, + :full_messages + ).and_return(['Error 1', 'Error 2']) + end + end + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Error 1', 'Error 2'] + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb new file mode 100644 index 00000000000..c78f0c7ca27 --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Removing an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:input) { { awardable_id: GitlabSchema.id_from_object(awardable).to_s, name: emoji_name } } + + let(:mutation) do + graphql_mutation(:remove_award_emoji, input) + end + + def mutation_response + graphql_mutation_response(:remove_award_emoji) + end + + def create_award_emoji(user) + create(:award_emoji, name: emoji_name, awardable: awardable, user: user ) + end + + shared_examples 'a mutation that does not destroy an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + shared_examples 'a mutation that does not authorize the user' do + it_behaves_like 'a mutation that does not destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + end + + context 'when the current_user does not own the award emoji' do + let!(:award_emoji) { create_award_emoji(create(:user)) } + + it_behaves_like 'a mutation that does not authorize the user' + end + + context 'when the current_user owns the award emoji' do + let!(:award_emoji) { create_award_emoji(current_user) } + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable' do + it 'removes the emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(-1) + end + + it 'returns no errors' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to be_nil + end + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb new file mode 100644 index 00000000000..31145730f10 --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Toggling an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:mutation) do + variables = { + awardable_id: GitlabSchema.id_from_object(awardable).to_s, + name: emoji_name + } + + graphql_mutation(:toggle_award_emoji, variables) + end + + def mutation_response + graphql_mutation_response(:toggle_award_emoji) + end + + shared_examples 'a mutation that does not create or destroy an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + def create_award_emoji(user) + create(:award_emoji, name: emoji_name, awardable: awardable, user: user ) + end + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do + let(:awardable) { create(:system_note) } + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable' do + context 'when no emoji has been awarded by the current_user yet' do + # Create an award emoji for another user. This therefore tests that + # toggling is correctly scoped to the user's emoji only. + let!(:award_emoji) { create_award_emoji(create(:user)) } + + it 'creates an emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(1) + end + + it 'returns the emoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['awardEmoji']['name']).to eq(emoji_name) + end + + it 'returns toggledOn as true' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['toggledOn']).to eq(true) + end + + context 'when there were active record validation errors' do + before do + expect_next_instance_of(AwardEmoji) do |award| + expect(award).to receive(:valid?).at_least(:once).and_return(false) + expect(award).to receive_message_chain(:errors, :full_messages).and_return(['Error 1', 'Error 2']) + end + end + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Error 1', 'Error 2'] + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end + + context 'when an emoji has been awarded by the current_user' do + let!(:award_emoji) { create_award_emoji(current_user) } + + it 'removes the emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(-1) + end + + it 'returns no errors' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to be_nil + end + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + + it 'returns toggledOn as false' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['toggledOn']).to eq(false) + end + end + end + end + + context 'when the user does not have permission' do + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + end +end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index bab1520b960..46925daf40a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -416,7 +416,6 @@ describe API::Users do expect(response).to have_gitlab_http_status(201) user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq(nil) expect(new_user.admin).to eq(true) expect(new_user.can_create_group).to eq(true) end @@ -435,7 +434,6 @@ describe API::Users do expect(response).to have_gitlab_http_status(201) user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq(nil) expect(new_user.admin).to eq(false) expect(new_user.can_create_group).to eq(false) end @@ -445,7 +443,6 @@ describe API::Users do expect(response).to have_gitlab_http_status(201) user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq(nil) expect(new_user.admin).to eq(false) end @@ -460,7 +457,6 @@ describe API::Users do user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq nil expect(new_user.external).to be_falsy end @@ -470,7 +466,6 @@ describe API::Users do user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq nil expect(new_user.external).to be_truthy end @@ -482,7 +477,19 @@ describe API::Users do user_id = json_response['id'] new_user = User.find(user_id) - expect(new_user).not_to eq(nil) + expect(new_user.recently_sent_password_reset?).to eq(true) + end + + it "creates user with random password" do + params = attributes_for(:user, force_random_password: true, reset_password: true) + post api('/users', admin), params: params + + expect(response).to have_gitlab_http_status(201) + + user_id = json_response['id'] + new_user = User.find(user_id) + + expect(new_user.valid_password?(params[:password])).to eq(false) expect(new_user.recently_sent_password_reset?).to eq(true) end diff --git a/spec/support/api/boards_shared_examples.rb b/spec/support/api/boards_shared_examples.rb index 592962ebf7c..3abb5096a7a 100644 --- a/spec/support/api/boards_shared_examples.rb +++ b/spec/support/api/boards_shared_examples.rb @@ -14,6 +14,16 @@ shared_examples_for 'group and project boards' do |route_definition, ee = false| end end + it 'avoids N+1 queries' do + pat = create(:personal_access_token, user: user) + control = ActiveRecord::QueryRecorder.new { get api(root_url, personal_access_token: pat) } + + create(:milestone, "#{board_parent.class.name.underscore}": board_parent) + create(:board, "#{board_parent.class.name.underscore}": board_parent) + + expect { get api(root_url, personal_access_token: pat) }.not_to exceed_query_limit(control) + end + describe "GET #{route_definition}" do context "when unauthenticated" do it "returns authentication error" do diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index bcf6669f37d..1a09d48f4cd 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -4,10 +4,7 @@ module GraphqlHelpers # makes an underscored string look like a fieldname # "merge_request" => "mergeRequest" def self.fieldnamerize(underscored_field_name) - graphql_field_name = underscored_field_name.to_s.camelize - graphql_field_name[0] = graphql_field_name[0].downcase - - graphql_field_name + underscored_field_name.to_s.camelize(:lower) end # Run a loader's named resolver diff --git a/spec/support/helpers/metrics_dashboard_helpers.rb b/spec/support/helpers/metrics_dashboard_helpers.rb index 6de00eea474..1511a2f6b49 100644 --- a/spec/support/helpers/metrics_dashboard_helpers.rb +++ b/spec/support/helpers/metrics_dashboard_helpers.rb @@ -50,4 +50,12 @@ module MetricsDashboardHelpers it_behaves_like 'valid dashboard service response for schema' end + + shared_examples_for 'raises error for users with insufficient permissions' do + context 'when the user does not have sufficient access' do + let(:user) { build(:user) } + + it_behaves_like 'misconfigured dashboard service response', :unauthorized + end + end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 77f22d9dd24..e63099d89b7 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -64,7 +64,8 @@ module TestEnv 'with-codeowners' => '219560e', 'submodule_inside_folder' => 'b491b92', 'png-lfs' => 'fe42f41', - 'sha-starting-with-large-number' => '8426165' + 'sha-starting-with-large-number' => '8426165', + 'invalid-utf8-diff-paths' => '99e4853' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily diff --git a/spec/support/shared_examples/graphql/mutation_shared_examples.rb b/spec/support/shared_examples/graphql/mutation_shared_examples.rb new file mode 100644 index 00000000000..022d41c0bdd --- /dev/null +++ b/spec/support/shared_examples/graphql/mutation_shared_examples.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# Shared example for expecting top-level errors. +# See https://graphql-ruby.org/mutations/mutation_errors#raising-errors +# +# { errors: [] } +# +# There must be a method or let called `mutation` defined that executes +# the mutation. +RSpec.shared_examples 'a mutation that returns top-level errors' do |errors:| + it do + post_graphql_mutation(mutation, current_user: current_user) + + error_messages = graphql_errors.map { |e| e['message'] } + + expect(error_messages).to eq(errors) + end +end + +# Shared example for expecting schema-level errors. +# See https://graphql-ruby.org/mutations/mutation_errors#errors-as-data +# +# { data: { mutationName: { errors: [] } } } +# +# There must be: +# - a method or let called `mutation` defined that executes the mutation +# - a `mutation_response` method defined that returns the data of the mutation response. +RSpec.shared_examples 'a mutation that returns errors in the response' do |errors:| + it do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['errors']).to eq(errors) + end +end diff --git a/spec/support/shared_examples/services/boards/issues_move_service.rb b/spec/support/shared_examples/services/boards/issues_move_service.rb index 9dbd1d8e867..5359831f8f8 100644 --- a/spec/support/shared_examples/services/boards/issues_move_service.rb +++ b/spec/support/shared_examples/services/boards/issues_move_service.rb @@ -1,8 +1,17 @@ shared_examples 'issues move service' do |group| + shared_examples 'updating timestamps' do + it 'updates updated_at' do + expect {described_class.new(parent, user, params).execute(issue)} + .to change {issue.reload.updated_at} + end + end + context 'when moving an issue between lists' do let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } } + it_behaves_like 'updating timestamps' + it 'delegates the label changes to Issues::UpdateService' do service = double(:service) expect(Issues::UpdateService).to receive(:new).and_return(service) @@ -24,6 +33,8 @@ shared_examples 'issues move service' do |group| let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression]) } let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: closed.id } } + it_behaves_like 'updating timestamps' + it 'delegates the close proceedings to Issues::CloseService' do expect_any_instance_of(Issues::CloseService).to receive(:execute).with(issue).once @@ -46,6 +57,8 @@ shared_examples 'issues move service' do |group| let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression], milestone: milestone) } let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: backlog.id } } + it_behaves_like 'updating timestamps' + it 'keeps labels and milestone' do described_class.new(parent, user, params).execute(issue) issue.reload @@ -59,6 +72,8 @@ shared_examples 'issues move service' do |group| let(:issue) { create(:labeled_issue, :closed, project: project, labels: [bug]) } let(:params) { { board_id: board1.id, from_list_id: closed.id, to_list_id: list2.id } } + it_behaves_like 'updating timestamps' + it 'delegates the re-open proceedings to Issues::ReopenService' do expect_any_instance_of(Issues::ReopenService).to receive(:execute).with(issue).once @@ -75,10 +90,13 @@ shared_examples 'issues move service' do |group| end context 'when moving to same list' do - let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:issue1) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:issue2) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } + let(:assignee) { create(:user) } + let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } + let(:issue1) { create(:labeled_issue, project: project, labels: [bug, development]) } + let(:issue2) { create(:labeled_issue, project: project, labels: [bug, development]) } + let(:issue) do + create(:labeled_issue, project: project, labels: [bug, development], assignees: [assignee]) + end it 'returns false' do expect(described_class.new(parent, user, params).execute(issue)).to eq false @@ -90,18 +108,36 @@ shared_examples 'issues move service' do |group| expect(issue.reload.labels).to contain_exactly(bug, development) end - it 'sorts issues' do - [issue, issue1, issue2].each do |issue| - issue.move_to_end && issue.save! - end + it 'keeps issues assignees' do + described_class.new(parent, user, params).execute(issue) + + expect(issue.reload.assignees).to contain_exactly(assignee) + end - params.merge!(move_after_id: issue1.id, move_before_id: issue2.id) + it 'sorts issues' do + reorder_issues(params, issues: [issue, issue1, issue2]) described_class.new(parent, user, params).execute(issue) expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) end + it 'does not update updated_at' do + reorder_issues(params, issues: [issue, issue1, issue2]) + + updated_at = issue.updated_at + updated_at1 = issue1.updated_at + updated_at2 = issue2.updated_at + + Timecop.travel(1.minute.from_now) do + described_class.new(parent, user, params).execute(issue) + end + + expect(issue.reload.updated_at.change(usec: 0)).to eq updated_at.change(usec: 0) + expect(issue1.reload.updated_at.change(usec: 0)).to eq updated_at1.change(usec: 0) + expect(issue2.reload.updated_at.change(usec: 0)).to eq updated_at2.change(usec: 0) + end + if group context 'when on a group board' do it 'sends the board_group_id parameter' do @@ -114,5 +150,13 @@ shared_examples 'issues move service' do |group| end end end + + def reorder_issues(params, issues: []) + issues.each do |issue| + issue.move_to_end && issue.save! + end + + params.merge!(move_after_id: issues[1].id, move_before_id: issues[2].id) + end end end |
