Commit 3b8b1417 authored by GitLab Bot's avatar GitLab Bot

Automatic merge of gitlab-org/gitlab master

parents a7c77ef7 7335c312
......@@ -257,17 +257,6 @@ static-analysis:
- run_timed_command "retry yarn install --frozen-lockfile"
- scripts/static-analysis
downtime_check:
extends:
- .rails-job-base
- .rails:rules:downtime_check
needs: []
stage: test
variables:
SETUP_DB: "false"
script:
- bundle exec rake downtime_check
rspec migration pg11:
extends:
- .rspec-base-pg11
......
......@@ -880,11 +880,6 @@
changes: *code-backstage-patterns
when: on_failure
.rails:rules:downtime_check:
rules:
- <<: *if-merge-request
changes: *code-backstage-patterns
.rails:rules:deprecations:
rules:
- <<: *if-not-ee
......
......@@ -697,7 +697,6 @@ RSpec/EmptyLineAfterFinalLetItBe:
- ee/spec/services/ee/notification_service_spec.rb
- ee/spec/services/ee/resource_events/merge_into_notes_service_spec.rb
- ee/spec/services/ee/todos/destroy/entity_leave_service_spec.rb
- ee/spec/services/epics/create_service_spec.rb
- ee/spec/services/external_approval_rules/create_service_spec.rb
- ee/spec/services/external_approval_rules/destroy_service_spec.rb
- ee/spec/services/external_approval_rules/update_service_spec.rb
......
import Vue from 'vue';
import SortDropdown from './components/sort_dropdown.vue';
const mountDropdownApp = (el) => {
const { mode, projectBranchesFilteredPath, sortOptions } = el.dataset;
return new Vue({
el,
name: 'SortBranchesDropdownApp',
components: {
SortDropdown,
},
provide: {
mode,
projectBranchesFilteredPath,
sortOptions: JSON.parse(sortOptions),
},
render: (createElement) => createElement(SortDropdown),
});
};
export default () => {
const el = document.getElementById('js-branches-sort-dropdown');
return el ? mountDropdownApp(el) : null;
};
<script>
import { GlDropdown, GlDropdownItem, GlSearchBoxByClick } from '@gitlab/ui';
import { mergeUrlParams, visitUrl, getParameterValues } from '~/lib/utils/url_utility';
import { s__ } from '~/locale';
const OVERVIEW_MODE = 'overview';
export default {
i18n: {
searchPlaceholder: s__('Branches|Filter by branch name'),
},
components: {
GlDropdown,
GlDropdownItem,
GlSearchBoxByClick,
},
inject: ['projectBranchesFilteredPath', 'sortOptions', 'mode'],
data() {
return {
selectedKey: 'updated_desc',
searchTerm: '',
};
},
computed: {
shouldShowDropdown() {
return this.mode !== OVERVIEW_MODE;
},
selectedSortMethodName() {
return this.sortOptions[this.selectedKey];
},
},
created() {
const sortValue = getParameterValues('sort');
const searchValue = getParameterValues('search');
if (sortValue.length > 0) {
[this.selectedKey] = sortValue;
}
if (searchValue.length > 0) {
[this.searchTerm] = searchValue;
}
},
methods: {
isSortMethodSelected(sortKey) {
return sortKey === this.selectedKey;
},
visitUrlFromOption(sortKey) {
this.selectedKey = sortKey;
const urlParams = {};
if (this.mode !== OVERVIEW_MODE) {
urlParams.sort = sortKey;
}
urlParams.search = this.searchTerm.length > 0 ? this.searchTerm : null;
const newUrl = mergeUrlParams(urlParams, this.projectBranchesFilteredPath);
visitUrl(newUrl);
},
},
};
</script>
<template>
<div class="gl-display-flex gl-pr-4">
<gl-search-box-by-click
v-model="searchTerm"
:placeholder="$options.i18n.searchPlaceholder"
class="gl-pr-4"
data-testid="branch-search"
@submit="visitUrlFromOption(selectedKey)"
/>
<gl-dropdown
v-if="shouldShowDropdown"
:text="selectedSortMethodName"
data-testid="branches-dropdown"
>
<gl-dropdown-item
v-for="(value, key) in sortOptions"
:key="key"
:is-checked="isSortMethodSelected(key)"
is-check-item
@click="visitUrlFromOption(key)"
>{{ value }}</gl-dropdown-item
>
</gl-dropdown>
</div>
</template>
import AjaxLoadingSpinner from '~/branches/ajax_loading_spinner';
import BranchSortDropdown from '~/branches/branch_sort_dropdown';
import DeleteModal from '~/branches/branches_delete_modal';
import initDiverganceGraph from '~/branches/divergence_graph';
AjaxLoadingSpinner.init();
new DeleteModal(); // eslint-disable-line no-new
initDiverganceGraph(document.querySelector('.js-branch-list').dataset.divergingCountsEndpoint);
BranchSortDropdown();
......@@ -12,6 +12,9 @@ class Projects::BranchesController < Projects::ApplicationController
# Support legacy URLs
before_action :redirect_for_legacy_index_sort_or_search, only: [:index]
before_action :limit_diverging_commit_counts!, only: [:diverging_commit_counts]
before_action do
push_frontend_feature_flag(:gldropdown_branches)
end
feature_category :source_code_management
......
......@@ -27,6 +27,15 @@ module Types
end
end
# Helper to define an enum member for each element of a Rails AR enum
def from_rails_enum(enum, description:)
enum.each_key do |name|
value name.to_s.upcase,
value: name,
description: format(description, name: name)
end
end
def value(*args, **kwargs, &block)
enum[args[0].downcase] = kwargs[:value] || args[0]
gitlab_deprecation(kwargs)
......
# frozen_string_literal: true
module FindClosest
# Find the closest node of a given type above this node, and return the domain object
def closest_parent(type, parent)
parent = parent.try(:parent) while parent && parent.object.class != type
return unless parent
parent.object.object
end
end
......@@ -23,7 +23,7 @@ module Types
A global identifier.
A global identifier represents an object uniquely across the application.
An example of such an identifier is "gid://gitlab/User/1".
An example of such an identifier is `"gid://gitlab/User/1"`.
Global identifiers are encoded as strings.
DESC
......
# frozen_string_literal: true
module Types
class MergeRequestReviewStateEnum < BaseEnum
graphql_name 'MergeRequestReviewState'
description 'State of a review of a GitLab merge request.'
from_rails_enum(::MergeRequestReviewer.states,
description: "The merge request is %{name}.")
end
end
......@@ -132,7 +132,10 @@ module Types
description: 'The milestone of the merge request.'
field :assignees, Types::UserType.connection_type, null: true, complexity: 5,
description: 'Assignees of the merge request.'
field :reviewers, Types::UserType.connection_type, null: true, complexity: 5,
field :reviewers,
type: Types::MergeRequests::ReviewerType.connection_type,
null: true,
complexity: 5,
description: 'Users from whom a review has been requested.'
field :author, Types::UserType, null: true,
description: 'User who created this merge request.'
......
# frozen_string_literal: true
module Types
module MergeRequests
class ReviewerType < ::Types::UserType
include FindClosest
graphql_name 'MergeRequestReviewer'
description 'A user from whom a merge request review has been requested.'
authorize :read_user
field :merge_request_interaction,
type: ::Types::UserMergeRequestInteractionType,
null: true,
extras: [:parent],
description: "Details of this user's interactions with the merge request."
def merge_request_interaction(parent:)
merge_request = closest_parent(::Types::MergeRequestType, parent)
return unless merge_request
Users::MergeRequestInteraction.new(user: object, merge_request: merge_request)
end
end
end
end
# frozen_string_literal: true
module Types
class UserMergeRequestInteractionType < BaseObject
graphql_name 'UserMergeRequestInteraction'
description <<~MD
Information about a merge request given a specific user.
This object has two parts to its state: a `User` and a `MergeRequest`. All
fields relate to interactions between the two entities.
MD
authorize :read_merge_request
field :can_merge,
type: ::GraphQL::BOOLEAN_TYPE,
null: false,
calls_gitaly: true,
method: :can_merge?,
description: 'Whether this user can merge this merge request.'
field :can_update,
type: ::GraphQL::BOOLEAN_TYPE,
null: false,
method: :can_update?,
description: 'Whether this user can update this merge request.'
field :review_state,
::Types::MergeRequestReviewStateEnum,
null: true,
description: 'The state of the review by this user.'
field :reviewed,
type: ::GraphQL::BOOLEAN_TYPE,
null: false,
method: :reviewed?,
description: 'Whether this user has provided a review for this merge request.'
field :approved,
type: ::GraphQL::BOOLEAN_TYPE,
null: false,
method: :approved?,
description: 'Whether this user has approved this merge request.'
end
end
::Types::UserMergeRequestInteractionType.prepend_if_ee('EE::Types::UserMergeRequestInteractionType')
......@@ -20,6 +20,10 @@ module BranchesHelper
end
end
end
def gldropdrown_branches_enabled?
Feature.enabled?(:gldropdown_branches)
end
end
BranchesHelper.prepend_if_ee('EE::BranchesHelper')
# frozen_string_literal: true
module Users
class MergeRequestInteraction
def initialize(user:, merge_request:)
@user = user
@merge_request = merge_request
end
def declarative_policy_subject
merge_request
end
def can_merge?
merge_request.can_be_merged_by?(user)
end
def can_update?
user.can?(:update_merge_request, merge_request)
end
def review_state
reviewer&.state
end
def reviewed?
reviewer&.reviewed? == true
end
def approved?
merge_request.approvals.any? { |app| app.user_id == user.id }
end
private
def reviewer
@reviewer ||= merge_request.merge_request_reviewers.find { |r| r.user_id == user.id }
end
attr_reader :user, :merge_request
end
end
::Users::MergeRequestInteraction.prepend_if_ee('EE::Users::MergeRequestInteraction')
......@@ -13,7 +13,11 @@ class UserPresenter < Gitlab::View::Presenter::Delegated
private
def can?(*args)
user.can?(*args)
end
def should_be_private?
!can?(current_user, :read_user_profile, user)
!Ability.allowed?(current_user, :read_user_profile, user)
end
end
......@@ -16,21 +16,25 @@
= link_to s_('Branches|All'), project_branches_filtered_path(@project, state: 'all'), title: s_('Branches|Show all branches')
.nav-controls
= form_tag(project_branches_filtered_path(@project, state: 'all'), method: :get) do
= search_field_tag :search, params[:search], { placeholder: s_('Branches|Filter by branch name'), id: 'branch-search', class: 'form-control search-text-input input-short', spellcheck: false }
- if !gldropdrown_branches_enabled?
= form_tag(project_branches_filtered_path(@project, state: 'all'), method: :get) do
= search_field_tag :search, params[:search], { placeholder: s_('Branches|Filter by branch name'), id: 'branch-search', class: 'form-control search-text-input input-short', spellcheck: false }
- unless @mode == 'overview'
.dropdown.inline>
%button.dropdown-menu-toggle{ type: 'button', 'data-toggle' => 'dropdown' }
%span.light
= branches_sort_options_hash[@sort]
= sprite_icon('chevron-down', css_class: "dropdown-menu-toggle-icon gl-top-3")
%ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable
%li.dropdown-header
= s_('Branches|Sort by')
- branches_sort_options_hash.each do |value, title|
%li
= link_to title, project_branches_filtered_path(@project, state: 'all', search: params[:search], sort: value), class: ("is-active" if @sort == value)
- unless @mode == 'overview'
.dropdown.inline>
%button.dropdown-menu-toggle{ type: 'button', 'data-toggle' => 'dropdown' }
%span.light
= branches_sort_options_hash[@sort]
= sprite_icon('chevron-down', css_class: "dropdown-menu-toggle-icon gl-top-3")
%ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable
%li.dropdown-header
= s_('Branches|Sort by')
- branches_sort_options_hash.each do |value, title|
%li
= link_to title, project_branches_filtered_path(@project, state: 'all', search: params[:search], sort: value), class: ("is-active" if @sort == value)
- else
#js-branches-sort-dropdown{ data: { project_branches_filtered_path: project_branches_path(@project, state: 'all'), sort_options: branches_sort_options_hash.to_json, mode: @mode } }
- if can? current_user, :push_code, @project
= link_to project_merged_branches_path(@project),
......
---
title: Add user-merge request interaction type
merge_request: 54588
author:
type: added
---
name: gldropdown_branches
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57760
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326549
milestone: '13.11'
type: development
group: group::continuous integration
default_enabled: false
......@@ -692,6 +692,16 @@ An API Fuzzing scan profile.
| `name` | [`String`](#string) | The unique name of the profile. |
| `yaml` | [`String`](#string) | A syntax highlit HTML representation of the YAML. |
### `ApprovalRule`
Describes a rule for who can approve merge requests.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `id` | [`GlobalID!`](#globalid) | ID of the rule. |
| `name` | [`String`](#string) | Name of the rule. |
| `type` | [`ApprovalRuleType`](#approvalruletype) | Type of the rule. |
### `AwardEmoji`
An emoji awarded by a user.
......@@ -3916,7 +3926,7 @@ An edge in a connection.
| `rebaseCommitSha` | [`String`](#string) | Rebase commit SHA of the merge request. |
| `rebaseInProgress` | [`Boolean!`](#boolean) | Indicates if there is a rebase currently in progress for the merge request. |
| `reference` | [`String!`](#string) | Internal reference of the merge request. Returned in shortened format by default. |
| `reviewers` | [`UserConnection`](#userconnection) | Users from whom a review has been requested. |
| `reviewers` | [`MergeRequestReviewerConnection`](#mergerequestreviewerconnection) | Users from whom a review has been requested. |
| `securityAutoFix` | [`Boolean`](#boolean) | Indicates if the merge request is created by @GitLab-Security-Bot. |
| `securityReportsUpToDateOnTargetBranch` | [`Boolean!`](#boolean) | Indicates if the target branch security reports are out of date. |
| `shouldBeRebased` | [`Boolean!`](#boolean) | Indicates if the merge request will be rebased. |
......@@ -4038,6 +4048,56 @@ Check permissions for the current user on a merge request.
| `revertOnCurrentMergeRequest` | [`Boolean!`](#boolean) | Indicates the user can perform `revert_on_current_merge_request` on this resource. |
| `updateMergeRequest` | [`Boolean!`](#boolean) | Indicates the user can perform `update_merge_request` on this resource. |
### `MergeRequestReviewer`
A user from whom a merge request review has been requested.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `assignedMergeRequests` | [`MergeRequestConnection`](#mergerequestconnection) | Merge requests assigned to the user. |
| `authoredMergeRequests` | [`MergeRequestConnection`](#mergerequestconnection) | Merge requests authored by the user. |
| `avatarUrl` | [`String`](#string) | URL of the user's avatar. |
| `bot` | [`Boolean!`](#boolean) | Indicates if the user is a bot. |
| `callouts` | [`UserCalloutConnection`](#usercalloutconnection) | User callouts that belong to the user. |
| `email` **{warning-solid}** | [`String`](#string) | **Deprecated** in 13.7. This was renamed. Use: `User.publicEmail`. |
| `groupCount` | [`Int`](#int) | Group count for the user. Available only when feature flag `user_group_counts` is enabled. |
| `groupMemberships` | [`GroupMemberConnection`](#groupmemberconnection) | Group memberships of the user. |
| `id` | [`ID!`](#id) | ID of the user. |
| `location` | [`String`](#string) | The location of the user. |
| `mergeRequestInteraction` | [`UserMergeRequestInteraction`](#usermergerequestinteraction) | Details of this user's interactions with the merge request. |
| `name` | [`String!`](#string) | Human-readable name of the user. |
| `projectMemberships` | [`ProjectMemberConnection`](#projectmemberconnection) | Project memberships of the user. |
| `publicEmail` | [`String`](#string) | User's public email. |
| `reviewRequestedMergeRequests` | [`MergeRequestConnection`](#mergerequestconnection) | Merge requests assigned to the user for review. |
| `snippets` | [`SnippetConnection`](#snippetconnection) | Snippets authored by the user. |
| `starredProjects` | [`ProjectConnection`](#projectconnection) | Projects starred by the user. |
| `state` | [`UserState!`](#userstate) | State of the user. |
| `status` | [`UserStatus`](#userstatus) | User status. |
| `todos` | [`TodoConnection`](#todoconnection) | To-do items of the user. |
| `userPermissions` | [`UserPermissions!`](#userpermissions) | Permissions for the current user on the resource. |
| `username` | [`String!`](#string) | Username of the user. Unique within this instance of GitLab. |
| `webPath` | [`String!`](#string) | Web path of the user. |
| `webUrl` | [`String!`](#string) | Web URL of the user. |
### `MergeRequestReviewerConnection`
The connection type for MergeRequestReviewer.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `edges` | [`[MergeRequestReviewerEdge]`](#mergerequestrevieweredge) | A list of edges. |
| `nodes` | [`[MergeRequestReviewer]`](#mergerequestreviewer) | A list of nodes. |
| `pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. |
### `MergeRequestReviewerEdge`
An edge in a connection.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `cursor` | [`String!`](#string) | A cursor for use in pagination. |
| `node` | [`MergeRequestReviewer`](#mergerequestreviewer) | The item at the end of the edge. |
### `MergeRequestReviewerRereviewPayload`
Autogenerated return type of MergeRequestReviewerRereview.
......@@ -6623,6 +6683,22 @@ An edge in a connection.
| `cursor` | [`String!`](#string) | A cursor for use in pagination. |
| `node` | [`User`](#user) | The item at the end of the edge. |
### `UserMergeRequestInteraction`
Information about a merge request given a specific user.
This object has two parts to its state: a `User` and a `MergeRequest`. All
fields relate to interactions between the two entities.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `applicableApprovalRules` | [`[ApprovalRule!]`](#approvalrule) | Approval rules that apply to this user for this merge request. |
| `approved` | [`Boolean!`](#boolean) | Whether this user has approved this merge request. |
| `canMerge` | [`Boolean!`](#boolean) | Whether this user can merge this merge request. |
| `canUpdate` | [`Boolean!`](#boolean) | Whether this user can update this merge request. |
| `reviewState` | [`MergeRequestReviewState`](#mergerequestreviewstate) | The state of the review by this user. |
| `reviewed` | [`Boolean!`](#boolean) | Whether this user has provided a review for this merge request. |
### `UserPermissions`
| Field | Type | Description |
......@@ -7320,6 +7396,17 @@ All possible ways to specify the API surface for an API fuzzing scan.
| `OPENAPI` | The API surface is specified by a OPENAPI file. |
| `POSTMAN` | The API surface is specified by a POSTMAN file. |
### `ApprovalRuleType`
The kind of an approval rule.
| Value | Description |
| ----- | ----------- |
| `ANY_APPROVER` | A `any_approver` approval rule. |
| `CODE_OWNER` | A `code_owner` approval rule. |
| `REGULAR` | A `regular` approval rule. |
| `REPORT_APPROVER` | A `report_approver` approval rule. |
### `AvailabilityEnum`
User availability status.
......@@ -7802,6 +7889,15 @@ New state to apply to a merge request.
| `CLOSED` | Close the merge request if it is open. |
| `OPEN` | Open the merge request if it is closed. |
### `MergeRequestReviewState`
State of a review of a GitLab merge request.
| Value | Description |
| ----- | ----------- |
| `REVIEWED` | The merge request is reviewed. |
| `UNREVIEWED` | The merge request is unreviewed. |
### `MergeRequestSort`
Values for sorting merge requests.
......@@ -8513,6 +8609,15 @@ A `GitlabErrorTrackingDetailedErrorID` is a global ID. It is encoded as a string
An example `GitlabErrorTrackingDetailedErrorID` is: `"gid://gitlab/Gitlab::ErrorTracking::DetailedError/1"`.
### `GlobalID`
A global identifier.
A global identifier represents an object uniquely across the application.
An example of such an identifier is `"gid://gitlab/User/1"`.
Global identifiers are encoded as strings.
### `GroupID`
A `GroupID` is a global ID. It is encoded as a string.
......
......@@ -43,8 +43,6 @@ It's recommended to create two separate migration script files.
class InsertProjectHooksPlanLimits < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
create_or_update_plan_limit('project_hooks', 'default', 0)
create_or_update_plan_limit('project_hooks', 'free', 10)
......
......@@ -4,12 +4,13 @@ group: Database
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
---
# What requires downtime?
# Avoiding downtime in migrations
When working with a database certain operations can be performed without taking
GitLab offline, others do require a downtime period. This guide describes
various operations, their impact, and how to perform them without requiring
downtime.
When working with a database certain operations may require downtime. Since we
cannot have downtime in migrations we need to use a set of steps to get the
same end result without downtime. This guide describes various operations that
may appear to need downtime, their impact, and how to perform them without
requiring downtime.
## Dropping Columns
......
......@@ -69,8 +69,6 @@ Migration file for adding `NOT VALID` foreign key:
class AddNotValidForeignKeyToEmailsUser < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
# safe to use: it requires short lock on the table since we don't validate the foreign key
add_foreign_key :emails, :users, on_delete: :cascade, validate: false
......@@ -97,8 +95,6 @@ Example for cleaning up records in the `emails` table within a database migratio
class RemoveRecordsWithoutUserFromEmailsTable < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
class Email < ActiveRecord::Base
......@@ -133,8 +129,6 @@ Migration file for validating the foreign key:
class ValidateForeignKeyOnEmailUsers < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
validate_foreign_key :emails, :user_id
end
......
......@@ -66,7 +66,7 @@ Finally, you can find various guides in the [Database guides](index.md) page tha
topics and use cases. The most frequently required during database reviewing are the following:
- [Migrations style guide](../migration_style_guide.md) for creating safe SQL migrations.
- [What requires downtime?](../what_requires_downtime.md).
- [Avoiding downtime in migrations](../avoiding_downtime_in_migrations.md).
- [SQL guidelines](../sql.md) for working with SQL queries.
## How to apply for becoming a database maintainer
......
......@@ -22,7 +22,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
## Migrations
- [What requires downtime?](../what_requires_downtime.md)
- [Avoiding downtime in migrations](../avoiding_downtime_in_migrations.md)
- [SQL guidelines](../sql.md) for working with SQL queries
- [Migrations style guide](../migration_style_guide.md) for creating safe SQL migrations
- [Testing Rails migrations](../testing_guide/testing_migrations_guide.md) guide
......
......@@ -26,8 +26,6 @@ For example, consider a migration that creates a table with two `NOT NULL` colum
```ruby
class CreateDbGuides < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
create_table :db_guides do |t|
t.bigint :stars, default: 0, null: false
......@@ -47,8 +45,6 @@ For example, consider a migration that adds a new `NOT NULL` column `active` to
```ruby
class AddExtendedTitleToSprints < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :db_guides, :active, :boolean, default: true, null: false
end
......@@ -117,7 +113,6 @@ with `validate: false` in a post-deployment migration,
```ruby
class AddNotNullConstraintToEpicsDescription < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
......@@ -191,7 +186,6 @@ migration helper in a final post-deployment migration,
```ruby
class ValidateNotNullConstraintOnEpicsDescription < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
......
......@@ -52,8 +52,6 @@ For example, consider a migration that creates a table with two text columns,
class CreateDbGuides < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
create_table_with_constraints :db_guides do |t|
t.bigint :stars, default: 0, null: false
......@@ -89,7 +87,6 @@ For example, consider a migration that adds a new text column `extended_title` t
```ruby
class AddExtendedTitleToSprints < ActiveRecord::Migration[6.0]
DOWNTIME = false
# rubocop:disable Migration/AddLimitToTextColumns
# limit is added in 20200501000002_add_text_limit_to_sprints_extended_title
......@@ -106,8 +103,6 @@ A second migration should follow the first one with a limit added to `extended_t
```ruby
class AddTextLimitToSprintsExtendedTitle < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -185,8 +180,6 @@ in a post-deployment migration,
class AddTextLimitMigration < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -266,7 +259,6 @@ helper in a final post-deployment migration,
```ruby
class ValidateTextLimitMigration < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
......
......@@ -176,7 +176,7 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac
#### Preparation when removing columns, tables, indexes, or other structures
- Follow the [guidelines on dropping columns](what_requires_downtime.md#dropping-columns).
- Follow the [guidelines on dropping columns](avoiding_downtime_in_migrations.md#dropping-columns).
- Generally it's best practice (but not a hard rule) to remove indexes and foreign keys in a post-deployment migration.
- Exceptions include removing indexes and foreign keys for small tables.
- If you're adding a composite index, another index might become redundant, so remove that in the same migration.
......@@ -199,7 +199,7 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac
- Check that the relevant version files under `db/schema_migrations` were added or removed.
- Check queries timing (If any): In a single transaction, cumulative query time executed in a migration
needs to fit comfortably within `15s` - preferably much less than that - on GitLab.com.
- For column removals, make sure the column has been [ignored in a previous release](what_requires_downtime.md#dropping-columns)
- For column removals, make sure the column has been [ignored in a previous release](avoiding_downtime_in_migrations.md#dropping-columns)
- Check [background migrations](background_migrations.md):
- Establish a time estimate for execution on GitLab.com. For historical purposes,
it's highly recommended to include this estimation on the merge request description.
......
......@@ -266,8 +266,6 @@ For example, to add support for files referenced by a `Widget` model with a
class CreateWidgetRegistry < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -399,8 +397,6 @@ can track verification state.
# frozen_string_literal: true
class AddVerificationStateToWidgets < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
change_table(:widgets) do |t|
t.integer :verification_state, default: 0, limit: 2, null: false
......@@ -427,8 +423,6 @@ can track verification state.
class AddVerificationFailureLimitToWidgets < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
CONSTRAINT_NAME = 'widget_verification_failure_text_limit'
......@@ -453,8 +447,6 @@ can track verification state.
class AddVerificationIndexesToWidgets < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
VERIFICATION_STATE_INDEX_NAME = "index_widgets_verification_state"
PENDING_VERIFICATION_INDEX_NAME = "index_widgets_pending_verification"
FAILED_VERIFICATION_INDEX_NAME = "index_widgets_failed_verification"
NEEDS_VERIFICATION_INDEX_NAME = "index_widgets_needs_verification"
......@@ -497,8 +489,6 @@ can track verification state.
class CreateWidgetStates < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -926,8 +916,6 @@ For example, to add support for files referenced by a `Gizmos` model with a
class CreateGizmoRegistry < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
......
......@@ -18,7 +18,7 @@ To measure the impact of a merge request you can use
the following guides:
- [Performance Guidelines](performance.md)
- [What requires downtime?](what_requires_downtime.md)
- [Avoiding downtime in migrations](avoiding_downtime_in_migrations.md)
## Definition
......
......@@ -16,16 +16,12 @@ migrations are written carefully, can be applied online, and adhere to the style
guide below.
Migrations are **not** allowed to require GitLab installations to be taken
offline unless _absolutely necessary_.
When downtime is necessary the migration has to be approved by:
1. The VP of Engineering
1. A Backend Maintainer
1. A Database Maintainer
An up-to-date list of people holding these titles can be found at
<https://about.gitlab.com/company/team/>.
offline ever. Migrations always must be written in such a way to avoid
downtime. In the past we had a process for defining migrations that allowed for
downtime by setting a `DOWNTIME` constant. You may see this when looking at
older migrations. This process was in place for 4 years without every being
used and as such we've learnt we can always figure out how to write a migration
differently to avoid downtime.
When writing your migrations, also consider that databases might have stale data
or inconsistencies and guard for that. Try to make as few assumptions as
......@@ -65,47 +61,16 @@ scripts/regenerate-schema
TARGET=12-9-stable-ee scripts/regenerate-schema
```
## What Requires Downtime?
The document ["What Requires Downtime?"](what_requires_downtime.md) specifies
various database operations, such as
- [dropping and renaming columns](what_requires_downtime.md#dropping-columns)
- [changing column constraints and types](what_requires_downtime.md#changing-column-constraints)
- [adding and dropping indexes, tables, and foreign keys](what_requires_downtime.md#adding-indexes)
and whether they require downtime and how to work around that whenever possible.
## Downtime Tagging
Every migration must specify if it requires downtime or not, and if it should
require downtime it must also specify a reason for this. This is required even
if 99% of the migrations don't require downtime as this makes it easier to find
the migrations that _do_ require downtime.
To tag a migration, add the following two constants to the migration class'
body:
- `DOWNTIME`: a boolean that when set to `true` indicates the migration requires
downtime.
- `DOWNTIME_REASON`: a String containing the reason for the migration requiring
downtime. This constant **must** be set when `DOWNTIME` is set to `true`.
## Avoiding downtime
For example:
The document ["Avoiding downtime in migrations"](avoiding_downtime_in_migrations.md) specifies
various database operations, such as:
```ruby
class MyMigration < ActiveRecord::Migration[6.0]
DOWNTIME = true
DOWNTIME_REASON = 'This migration requires downtime because ...'
def change
...
end
end
```
- [dropping and renaming columns](avoiding_downtime_in_migrations.md#dropping-columns)
- [changing column constraints and types](avoiding_downtime_in_migrations.md#changing-column-constraints)
- [adding and dropping indexes, tables, and foreign keys](avoiding_downtime_in_migrations.md#adding-indexes)
It is an error (that is, CI fails) if the `DOWNTIME` constant is missing
from a migration class.
and explains how to perform them without requiring downtime.
## Reversibility
......@@ -512,8 +477,6 @@ class like so:
class MyMigration < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
INDEX_NAME = 'index_name'
......@@ -640,8 +603,6 @@ Take the following migration as an example:
```ruby
class DefaultRequestAccessGroups < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
change_column_default(:namespaces, :request_access_enabled, from: false, to: true)
end
......@@ -849,8 +810,6 @@ Example migration adding this column:
```ruby
class AddOptionsToBuildMetadata < ActiveRecord::Migration[5.0]
DOWNTIME = false
def change
add_column :ci_builds_metadata, :config_options, :jsonb
end
......
......@@ -39,8 +39,6 @@ Or, you can create a database migration:
class ImportCommonMetrics < ActiveRecord::Migration[4.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
end
......
......@@ -36,7 +36,7 @@ application starts, Rails queries the database schema, caching the tables and
column types for the data requested. Because of this schema cache, dropping a
column or table while the application is running can produce 500 errors to the
user. This is why we have a [process for dropping columns and other
no-downtime changes](what_requires_downtime.md).
no-downtime changes](avoiding_downtime_in_migrations.md).
#### Multi-tenancy
......
......@@ -831,8 +831,6 @@ as shown in this example:
class MigrateTheRenamedSidekiqQueue < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
sidekiq_queue_migrate 'old_queue_name', to: 'new_queue_name'
end
......
......@@ -176,7 +176,7 @@ The following table depicts the various user permission levels in a project.
| Delete wiki pages | | | | ✓ | ✓ |
| View project Audit Events | | | ✓ (*11*) | ✓ | ✓ |
| Manage [push rules](../push_rules/push_rules.md) | | | | ✓ | ✓ |
| Manage [project access tokens](project/settings/project_access_tokens.md) **(FREE SELF)** | | | | ✓ | ✓ |
| Manage [project access tokens](project/settings/project_access_tokens.md) **(FREE SELF)** **(PREMIUM SAAS)** (*12*) | | | | ✓ | ✓ |
| View 2FA status of members | | | | ✓ | ✓ |
| Switch visibility level | | | | | ✓ |
| Transfer project to another namespace | | | | | ✓ |
......@@ -204,6 +204,8 @@ The following table depicts the various user permission levels in a project.
[Eligible approvers](project/merge_requests/merge_request_approvals.md#eligible-approvers).
1. Applies only to comments on [Design Management](project/issues/design_management.md) designs.
1. Users can only view events based on their individual actions.
1. Project access tokens are supported for self-managed instances on Free and above. They are also
supported on GitLab SaaS Premium and above (excluding [trial licenses](https://about.gitlab.com/free-trial)).
## Project features permissions
......
# frozen_string_literal: true
module EE
module Types
module UserMergeRequestInteractionType
extend ActiveSupport::Concern
prepended do
field :applicable_approval_rules,
[::Types::ApprovalRuleType],
null: true,
description: 'Approval rules that apply to this user for this merge request.'
end
end
end
end
# frozen_string_literal: true
module Types
class ApprovalRuleType < BaseObject
graphql_name 'ApprovalRule'
description 'Describes a rule for who can approve merge requests.'
authorize :read_approval_rule
field :id,
type: ::Types::GlobalIDType,
null: false,
description: 'ID of the rule.'
field :name,
type: GraphQL::STRING_TYPE,
null: true,
description: 'Name of the rule.'
field :type,
type: ::Types::ApprovalRuleTypeEnum,
null: true,
method: :rule_type,
description: 'Type of the rule.'
end
end
# frozen_string_literal: true
module Types
class ApprovalRuleTypeEnum < BaseEnum
# See: ApprovalMergeRequestRule, and ApprovalProjectRule
graphql_name 'ApprovalRuleType'
description 'The kind of an approval rule.'
from_rails_enum(
ApprovalProjectRule.rule_types.merge(ApprovalMergeRequestRule.rule_types),
description: 'A `%{name}` approval rule.'
)
end
end
......@@ -24,7 +24,7 @@ class ApprovalWrappedRule
def_delegators(:@approval_rule,
:regular?, :any_approver?, :code_owner?, :report_approver?,
:overridden?, :id, :name, :users, :groups, :code_owner,
:source_rule, :rule_type, :approvals_required, :section)
:source_rule, :rule_type, :approvals_required, :section, :to_global_id)
def self.wrap(merge_request, rule)
if rule.any_approver?
......@@ -51,6 +51,10 @@ class ApprovalWrappedRule
end
end
def declarative_policy_delegate
@approval_rule
end
# @return [Array<User>] of users who have approved the merge request
#
# This is dynamically calculated unless it is persisted as `approved_approvers`.
......
# frozen_string_literal: true
module EE
module Users
module MergeRequestInteraction
def applicable_approval_rules
return [] unless merge_request.project.licensed_feature_available?(:merge_request_approvers)
merge_request.applicable_approval_rules_for_user(user.id)
end
end
end
end
......@@ -8,4 +8,6 @@ class ApprovalMergeRequestRulePolicy < BasePolicy
end
rule { editable }.enable :edit_approval_rule
rule { can?(:read_merge_request) }.enable :read_approval_rule
end
......@@ -8,4 +8,6 @@ class ApprovalProjectRulePolicy < BasePolicy
end
rule { editable }.enable :edit_approval_rule
rule { can?(:read_project) }.enable :read_approval_rule
end
---
title: Fix RSpec/EmptyLineAfterFinalLetItBe rubocop offenses in ee/spec/services/epics
merge_request: 58350
author: Abdul Wadood @abdulwd
type: fixed
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['UserMergeRequestInteraction'] do
include GraphqlHelpers
let_it_be(:user) { create(:user) }
let_it_be(:current_user) { create(:user) }
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:merge_request) { create(:merge_request, source_project: project) }
let(:interaction) { ::Users::MergeRequestInteraction.new(user: user, merge_request: merge_request.reset) }
it 'has the expected fields' do
expected_fields = %w[
applicable_approval_rules
]
expect(described_class).to have_graphql_fields(*expected_fields).at_least
end
def resolve(field_name)
resolve_field(field_name, interaction, current_user: current_user)
end
describe '#applicable_approval_rules' do
subject { resolve(:applicable_approval_rules) }
before do
merge_request.clear_memoization(:approval_state)
end
context 'when there are no approval rules' do
it { is_expected.to be_empty }
end
context 'when there are approval rules' do
before do
create(:approval_merge_request_rule, merge_request: merge_request)
create(:code_owner_rule, merge_request: merge_request)
create(:any_approver_rule, merge_request: merge_request)
end
context 'when the feature is not available' do
it { is_expected.to be_empty }
end
context 'when the feature is available' do
before do
stub_licensed_features(merge_request_approvers: true)
end
it { is_expected.to be_empty }
context 'when the user is associated with a rule' do
let(:rule) { create(:code_owner_rule, merge_request: merge_request) }
before do
rule.users << user
end
it { is_expected.to contain_exactly(have_attributes(approval_rule: rule)) }
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['ApprovalRuleType'] do
it 'has the correct members' do
expect(described_class.values).to match(
'REGULAR' => have_attributes(
description: 'A `regular` approval rule.',
value: 'regular'
),
'CODE_OWNER' => have_attributes(
description: 'A `code_owner` approval rule.',
value: 'code_owner'
),
'REPORT_APPROVER' => have_attributes(
description: 'A `report_approver` approval rule.',
value: 'report_approver'
),
'ANY_APPROVER' => have_attributes(
description: 'A `any_approver` approval rule.',
value: 'any_approver'
)
)
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['ApprovalRule'] do
let(:fields) { %i[id name type] }
it { expect(described_class).to have_graphql_fields(fields) }
it { expect(described_class).to require_graphql_authorizations(:read_approval_rule) }
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ::Users::MergeRequestInteraction do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:merge_request) { create(:merge_request, source_project: project) }
subject { described_class.new(user: user, merge_request: merge_request) }
describe '#applicable_approval_rules' do
before do
merge_request.reset
merge_request.clear_memoization(:approval_state)
end
context 'when there are no approval rules' do
it { is_expected.to have_attributes(applicable_approval_rules: be_empty) }
end
context 'when there are approval rules' do
before do
create(:approval_merge_request_rule, merge_request: merge_request)
create(:code_owner_rule, merge_request: merge_request)
create(:any_approver_rule, merge_request: merge_request)
end
context 'when the feature is not available' do
it { is_expected.to have_attributes(applicable_approval_rules: be_empty) }
end
context 'when the feature is available' do
before do
stub_licensed_features(merge_request_approvers: true)
end
it { is_expected.to have_attributes(applicable_approval_rules: be_empty) }
context 'when the user is associated with a rule' do
let(:rule) { create(:code_owner_rule, merge_request: merge_request) }
before do
create(:code_owner_rule, merge_request: merge_request) # irrelevant rule
rule.users << user
end
specify do
is_expected.to have_attributes(
applicable_approval_rules: contain_exactly(
have_attributes(approval_rule: rule)
)
)
end
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'MergeRequestReviewer' do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) }
context 'when requesting information about MR interactions' do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:merge_request) { create(:merge_request, source_project: project) }
let(:query) do
graphql_query_for(
:project,
{ full_path: project.full_path },
query_graphql_field(
:merge_request,
{ iid: merge_request.iid.to_s },
query_nodes(
:reviewers,
"mergeRequestInteraction { #{all_graphql_fields_for('UserMergeRequestInteraction')} }"
)
)
)
end
let(:interaction) do
graphql_data_at(:project,
:merge_request,
:reviewers,
:nodes, 0,
:merge_request_interaction)
end
before do
merge_request.reviewers << user
end
context 'when the user does not have any applicable rules' do
it 'returns null data' do
post_graphql(query)
expect(interaction).to include(
'applicableApprovalRules' => []
)
end
end
context 'when the user has interacted' do
let(:rule) { create(:code_owner_rule, merge_request: merge_request) }
before do
stub_licensed_features(merge_request_approvers: true)
rule.users << user
end
it 'returns appropriate data' do
the_rule = eq(
'id' => global_id_of(rule),
'name' => rule.name,
'type' => 'CODE_OWNER'
)
post_graphql(query)
expect(interaction['applicableApprovalRules']).to contain_exactly(the_rule)
end
end
end
end
......@@ -6,6 +6,7 @@ RSpec.describe Epics::CreateService do
let_it_be(:group) { create(:group, :internal)}
let_it_be(:user) { create(:user) }
let_it_be(:parent_epic) { create(:epic, group: group) }
let(:params) { { title: 'new epic', description: 'epic description', parent_id: parent_epic.id, confidential: true } }
subject { described_class.new(group, user, params).execute }
......
......@@ -6,14 +6,6 @@
class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Migration.current_version %>]
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
# DOWNTIME_REASON = ''
# When using the methods "add_concurrent_index" or "remove_concurrent_index"
# you must disable the use of transactions
# as these methods can not run in an existing transaction.
......
......@@ -7,14 +7,6 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi
# Uncomment the following include if you require helper functions:
# include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
# DOWNTIME_REASON = ''
# When using the methods "add_concurrent_index" or "remove_concurrent_index"
# you must disable the use of transactions
# as these methods can not run in an existing transaction.
......
......@@ -7,8 +7,6 @@ class <%= migration_class_name %> < ActiveRecord::Migration[<%= ActiveRecord::Mi
# Uncomment the following include if you require helper functions:
# include Gitlab::Database::MigrationHelpers
DOWNTIME = false
# When using the methods "add_concurrent_index" or "remove_concurrent_index"
# you must disable the use of transactions
# as these methods can not run in an existing transaction.
......
# frozen_string_literal: true
module Gitlab
# Checks if a set of migrations requires downtime or not.
class DowntimeCheck
# The constant containing the boolean that indicates if downtime is needed
# or not.
DOWNTIME_CONST = :DOWNTIME
# The constant that specifies the reason for the migration requiring
# downtime.
DOWNTIME_REASON_CONST = :DOWNTIME_REASON
# Checks the given migration paths and returns an Array of
# `Gitlab::DowntimeCheck::Message` instances.
#
# migrations - The migration file paths to check.
def check(migrations)
migrations.map do |path|
require(path)
migration_class = class_for_migration_file(path)
unless migration_class.const_defined?(DOWNTIME_CONST)
raise "The migration in #{path} does not specify if it requires " \
"downtime or not"
end
if online?(migration_class)
Message.new(path)
else
reason = downtime_reason(migration_class)
unless reason
raise "The migration in #{path} requires downtime but no reason " \
"was given"
end
Message.new(path, true, reason)
end
end
end
# Checks the given migrations and prints the results to STDOUT/STDERR.
#
# migrations - The migration file paths to check.
def check_and_print(migrations)
check(migrations).each do |message|
puts message.to_s # rubocop: disable Rails/Output
end
end
# Returns the class for the given migration file path.
def class_for_migration_file(path)
File.basename(path, File.extname(path)).split('_', 2).last.camelize
.constantize
end
# Returns true if the given migration can be performed without downtime.
def online?(migration)
migration.const_get(DOWNTIME_CONST, false) == false
end
# Returns the downtime reason, or nil if none was defined.
def downtime_reason(migration)
if migration.const_defined?(DOWNTIME_REASON_CONST)
migration.const_get(DOWNTIME_REASON_CONST, false)
else
nil
end
end
end
end
# frozen_string_literal: true
module Gitlab
class DowntimeCheck
class Message
attr_reader :path, :offline
OFFLINE = "\e[31moffline\e[0m"
ONLINE = "\e[32monline\e[0m"
# path - The file path of the migration.
# offline - When set to `true` the migration will require downtime.
# reason - The reason as to why the migration requires downtime.
def initialize(path, offline = false, reason = nil)
@path = path
@offline = offline
@reason = reason
end
def to_s
label = offline ? OFFLINE : ONLINE
message = ["[#{label}]: #{path}"]
if reason?
message << ":\n\n#{reason}\n\n"
end
message.join
end
def reason?
@reason.present?
end
def reason
@reason.strip.lines.map(&:strip).join("\n")
end
end
end
end
......@@ -21,8 +21,9 @@ module Gitlab
def ok?(object, current_user)
return true if none?
subject = object.try(:declarative_policy_subject) || object
abilities.all? do |ability|
Ability.allowed?(current_user, ability, object)
Ability.allowed?(current_user, ability, subject)
end
end
end
......
......@@ -93,7 +93,7 @@ module Gitlab
end
def cursor_attributes_for_node(node)
column_definitions.each_with_object({}) do |column_definition, hash|
column_definitions.each_with_object({}.with_indifferent_access) do |column_definition, hash|
field_value = node[column_definition.attribute_name]
hash[column_definition.attribute_name] = if field_value.is_a?(Time)
field_value.strftime('%Y-%m-%d %H:%M:%S.%N %Z')
......@@ -162,7 +162,7 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord
def apply_cursor_conditions(scope, values = {})
scope = apply_custom_projections(scope)
scope.where(build_where_values(values))
scope.where(build_where_values(values.with_indifferent_access))
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -20,7 +20,7 @@ task 'db:obsolete_ignored_columns' => :environment do
WARNING: Removing columns is tricky because running GitLab processes may still be using the columns.
See also https://docs.gitlab.com/ee/development/what_requires_downtime.html#dropping-columns
See also https://docs.gitlab.com/ee/development/avoiding_downtime_in_migrations.html#dropping-columns
TEXT
end
end
# frozen_string_literal: true
desc 'Checks if migrations in a branch require downtime'
task downtime_check: :environment do
repo = if defined?(Gitlab::License)
'gitlab'
else
'gitlab-foss'
end
`git fetch https://gitlab.com/gitlab-org/#{repo}.git --depth 1`
Rake::Task['gitlab:db:downtime_check'].invoke('FETCH_HEAD')
end
......@@ -80,22 +80,6 @@ namespace :gitlab do
end
end
desc 'GitLab | DB | Checks if migrations require downtime or not'
task :downtime_check, [:ref] => :environment do |_, args|
abort 'You must specify a Git reference to compare with' unless args[:ref]
require 'shellwords'
ref = Shellwords.escape(args[:ref])
migrations = `git diff #{ref}.. --diff-filter=A --name-only -- db/migrate`.lines
.map { |file| Rails.root.join(file.strip).to_s }
.select { |file| File.file?(file) }
.select { |file| /\A[0-9]+.*\.rb\z/ =~ File.basename(file) }
Gitlab::DowntimeCheck.new.check_and_print(migrations)
end
desc 'GitLab | DB | Sets up EE specific database functionality'
if Gitlab.ee?
......
......@@ -9,11 +9,12 @@ RSpec.describe "User deletes branch", :js do
before do
project.add_developer(user)
sign_in(user)
visit(project_branches_path(project))
end
it "deletes branch" do
stub_feature_flags(gldropdown_branches: false)
visit(project_branches_path(project))
fill_in("branch-search", with: "improve/awesome").native.send_keys(:enter)
page.within(".js-branch-improve\\/awesome") do
......@@ -24,4 +25,23 @@ RSpec.describe "User deletes branch", :js do
expect(page).to have_css(".js-branch-improve\\/awesome", visible: :hidden)
end
context 'with gldropdown_branches enabled' do
it "deletes branch" do
visit(project_branches_path(project))
branch_search = find('input[data-testid="branch-search"]')
branch_search.set('improve/awesome')
branch_search.native.send_keys(:enter)
page.within(".js-branch-improve\\/awesome") do
accept_alert { find(".btn-danger").click }
end
wait_for_requests
expect(page).to have_css(".js-branch-improve\\/awesome", visible: :hidden)
end
end
end
......@@ -86,6 +86,7 @@ RSpec.describe 'Branches' do
describe 'Find branches' do
it 'shows filtered branches', :js do
stub_feature_flags(gldropdown_branches: false)
visit project_branches_path(project)
fill_in 'branch-search', with: 'fix'
......@@ -94,6 +95,20 @@ RSpec.describe 'Branches' do
expect(page).to have_content('fix')
expect(find('.all-branches')).to have_selector('li', count: 1)
end
context 'with gldropdown_branches enabled' do
it 'shows filtered branches', :js do
visit project_branches_path(project)
branch_search = find('input[data-testid="branch-search"]')
branch_search.set('fix')
branch_search.native.send_keys(:enter)
expect(page).to have_content('fix')
expect(find('.all-branches')).to have_selector('li', count: 1)
end
end
end
describe 'Delete unprotected branch on Overview' do
......@@ -115,6 +130,7 @@ RSpec.describe 'Branches' do
end
it 'sorts the branches by name' do
stub_feature_flags(gldropdown_branches: false)
visit project_branches_filtered_path(project, state: 'all')
click_button "Last updated" # Open sorting dropdown
......@@ -123,7 +139,21 @@ RSpec.describe 'Branches' do
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :name))
end
context 'with gldropdown_branches enabled' do
it 'sorts the branches by name', :js do
visit project_branches_filtered_path(project, state: 'all')
click_button "Last updated" # Open sorting dropdown
within '[data-testid="branches-dropdown"]' do
find('p', text: 'Name').click
end
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :name))
end
end
it 'sorts the branches by oldest updated' do
stub_feature_flags(gldropdown_branches: false)
visit project_branches_filtered_path(project, state: 'all')
click_button "Last updated" # Open sorting dropdown
......@@ -132,6 +162,19 @@ RSpec.describe 'Branches' do
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :updated_asc))
end
context 'with gldropdown_branches enabled' do
it 'sorts the branches by oldest updated', :js do
visit project_branches_filtered_path(project, state: 'all')
click_button "Last updated" # Open sorting dropdown
within '[data-testid="branches-dropdown"]' do
find('p', text: 'Oldest updated').click
end
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :updated_asc))
end
end
it 'avoids a N+1 query in branches index' do
control_count = ActiveRecord::QueryRecorder.new { visit project_branches_path(project) }.count
......@@ -143,6 +186,7 @@ RSpec.describe 'Branches' do
describe 'Find branches on All branches' do
it 'shows filtered branches', :js do
stub_feature_flags(gldropdown_branches: false)
visit project_branches_filtered_path(project, state: 'all')
fill_in 'branch-search', with: 'fix'
......@@ -151,10 +195,25 @@ RSpec.describe 'Branches' do
expect(page).to have_content('fix')
expect(find('.all-branches')).to have_selector('li', count: 1)
end
context 'with gldropdown_branches enabled' do
it 'shows filtered branches', :js do
visit project_branches_filtered_path(project, state: 'all')
branch_search = find('input[data-testid="branch-search"]')
branch_search.set('fix')
branch_search.native.send_keys(:enter)
expect(page).to have_content('fix')
expect(find('.all-branches')).to have_selector('li', count: 1)
end
end
end
describe 'Delete unprotected branch on All branches' do
it 'removes branch after confirmation', :js do
stub_feature_flags(gldropdown_branches: false)
visit project_branches_filtered_path(project, state: 'all')
fill_in 'branch-search', with: 'fix'
......@@ -168,6 +227,24 @@ RSpec.describe 'Branches' do
expect(page).not_to have_content('fix')
expect(find('.all-branches')).to have_selector('li', count: 0)
end
context 'with gldropdown_branches enabled' do
it 'removes branch after confirmation', :js do
visit project_branches_filtered_path(project, state: 'all')
branch_search = find('input[data-testid="branch-search"]')
branch_search.set('fix')
branch_search.native.send_keys(:enter)
expect(page).to have_content('fix')
expect(find('.all-branches')).to have_selector('li', count: 1)
accept_confirm { find('.js-branch-fix .btn-danger').click }
expect(page).not_to have_content('fix')
expect(find('.all-branches')).to have_selector('li', count: 0)
end
end
end
context 'on project with 0 branch' do
......
......@@ -22,6 +22,7 @@ RSpec.describe 'Protected Branches', :js do
end
it 'does not allow developer to removes protected branch' do
stub_feature_flags(gldropdown_branches: false)
visit project_branches_path(project)
fill_in 'branch-search', with: 'fix'
......@@ -29,6 +30,17 @@ RSpec.describe 'Protected Branches', :js do
expect(page).to have_css('.btn-danger.disabled')
end
context 'with gldropdown_branches enabled' do
it 'does not allow developer to removes protected branch' do
visit project_branches_path(project)
find('input[data-testid="branch-search"]').set('fix')
find('input[data-testid="branch-search"]').native.send_keys(:enter)
expect(page).to have_css('.btn-danger.disabled')
end
end
end
end
......@@ -45,6 +57,7 @@ RSpec.describe 'Protected Branches', :js do
end
it 'removes branch after modal confirmation' do
stub_feature_flags(gldropdown_branches: false)
visit project_branches_path(project)
fill_in 'branch-search', with: 'fix'
......@@ -63,6 +76,28 @@ RSpec.describe 'Protected Branches', :js do
expect(page).to have_content('No branches to show')
end
context 'with gldropdown_branches enabled' do
it 'removes branch after modal confirmation' do
visit project_branches_path(project)
find('input[data-testid="branch-search"]').set('fix')
find('input[data-testid="branch-search"]').native.send_keys(:enter)
expect(page).to have_content('fix')
expect(find('.all-branches')).to have_selector('li', count: 1)
page.find('[data-target="#modal-delete-branch"]').click
expect(page).to have_css('.js-delete-branch[disabled]')
fill_in 'delete_branch_input', with: 'fix'
click_link 'Delete protected branch'
find('input[data-testid="branch-search"]').set('fix')
find('input[data-testid="branch-search"]').native.send_keys(:enter)
expect(page).to have_content('No branches to show')
end
end
end
end
......
import { GlSearchBoxByClick } from '@gitlab/ui';
import { mount } from '@vue/test-utils';
import { extendedWrapper } from 'helpers/vue_test_utils_helper';
import SortDropdown from '~/branches/components/sort_dropdown.vue';
import * as urlUtils from '~/lib/utils/url_utility';
describe('Branches Sort Dropdown', () => {
let wrapper;
const createWrapper = (props = {}) => {
return extendedWrapper(
mount(SortDropdown, {
provide: {
mode: 'overview',
projectBranchesFilteredPath: '/root/ci-cd-project-demo/-/branches?state=all',
sortOptions: {
name_asc: 'Name',
updated_asc: 'Oldest updated',
updated_desc: 'Last updated',
},
...props,
},
}),
);
};
const findSearchBox = () => wrapper.findComponent(GlSearchBoxByClick);
const findBranchesDropdown = () => wrapper.findByTestId('branches-dropdown');
afterEach(() => {
if (wrapper) {
wrapper.destroy();
}
});
describe('When in overview mode', () => {
beforeEach(() => {
wrapper = createWrapper();
});
it('should have a search box with a placeholder', () => {
const searchBox = findSearchBox();
expect(searchBox.exists()).toBe(true);
expect(searchBox.find('input').attributes('placeholder')).toBe('Filter by branch name');
});
it('should not have a branches dropdown when in overview mode', () => {
const branchesDropdown = findBranchesDropdown();
expect(branchesDropdown.exists()).toBe(false);
});
});
describe('when in All branches mode', () => {
beforeEach(() => {
wrapper = createWrapper({ mode: 'all' });
});
it('should have a search box with a placeholder', () => {
const searchBox = findSearchBox();
expect(searchBox.exists()).toBe(true);
expect(searchBox.find('input').attributes('placeholder')).toBe('Filter by branch name');
});
it('should have a branches dropdown when in all branches mode', () => {
const branchesDropdown = findBranchesDropdown();
expect(branchesDropdown.exists()).toBe(true);
});
});
describe('when submitting a search term', () => {
beforeEach(() => {
urlUtils.visitUrl = jest.fn();
wrapper = createWrapper();
});
it('should call visitUrl', () => {
const searchBox = findSearchBox();
searchBox.vm.$emit('submit');
expect(urlUtils.visitUrl).toHaveBeenCalledWith(
'/root/ci-cd-project-demo/-/branches?state=all',
);
});
});
});
......@@ -3,6 +3,38 @@
require 'spec_helper'
RSpec.describe Types::BaseEnum do
describe '.from_rails_enum' do
let(:enum_type) { Class.new(described_class) }
let(:template) { "The name is '%{name}', James %{name}." }
let(:enum) do
{
'foo' => 1,
'bar' => 2,
'baz' => 100
}
end
it 'contructs the correct values' do
enum_type.from_rails_enum(enum, description: template)
expect(enum_type.values).to match(
'FOO' => have_attributes(
description: "The name is 'foo', James foo.",
value: 'foo'
),
'BAR' => have_attributes(
description: "The name is 'bar', James bar.",
value: 'bar'
),
'BAZ' => have_attributes(
description: "The name is 'baz', James baz.",
value: 'baz'
)
)
end
end
describe '.declarative_enum' do
let(:use_name) { true }
let(:use_description) { true }
......@@ -26,12 +58,15 @@ RSpec.describe Types::BaseEnum do
end
end
subject(:set_declarative_enum) { enum_type.declarative_enum(enum_module, use_name: use_name, use_description: use_description) }
subject(:set_declarative_enum) do
enum_type.declarative_enum(enum_module, use_name: use_name, use_description: use_description)
end
describe '#graphql_name' do
context 'when the use_name is `true`' do
it 'changes the graphql_name' do
expect { set_declarative_enum }.to change { enum_type.graphql_name }.from('OriginalName').to('Name')
expect { set_declarative_enum }
.to change(enum_type, :graphql_name).from('OriginalName').to('Name')
end
end
......@@ -39,7 +74,8 @@ RSpec.describe Types::BaseEnum do
let(:use_name) { false }
it 'does not change the graphql_name' do
expect { set_declarative_enum }.not_to change { enum_type.graphql_name }.from('OriginalName')
expect { set_declarative_enum }
.not_to change(enum_type, :graphql_name).from('OriginalName')
end
end
end
......@@ -47,7 +83,8 @@ RSpec.describe Types::BaseEnum do
describe '#description' do
context 'when the use_description is `true`' do
it 'changes the description' do
expect { set_declarative_enum }.to change { enum_type.description }.from('Original description').to('Description')
expect { set_declarative_enum }
.to change(enum_type, :description).from('Original description').to('Description')
end
end
......@@ -55,7 +92,8 @@ RSpec.describe Types::BaseEnum do
let(:use_description) { false }
it 'does not change the description' do
expect { set_declarative_enum }.not_to change { enum_type.description }.from('Original description')
expect { set_declarative_enum }
.not_to change(enum_type, :description).from('Original description')
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['MergeRequestReviewState'] do
it 'the correct enum members' do
expect(described_class.values).to match(
'REVIEWED' => have_attributes(
description: 'The merge request is reviewed.',
value: 'reviewed'
),
'UNREVIEWED' => have_attributes(
description: 'The merge request is unreviewed.',
value: 'unreviewed'
)
)
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['MergeRequestReviewer'] do
specify { expect(described_class).to require_graphql_authorizations(:read_user) }
it 'has the expected fields' do
expected_fields = %w[
id
bot
user_permissions
snippets
name
username
email
publicEmail
avatarUrl
webUrl
webPath
todos
state
status
location
authoredMergeRequests
assignedMergeRequests
reviewRequestedMergeRequests
groupMemberships
groupCount
projectMemberships
starredProjects
callouts
merge_request_interaction
]
expect(described_class).to have_graphql_fields(*expected_fields)
end
describe '#merge_request_interaction' do
subject { described_class.fields['mergeRequestInteraction'] }
it 'returns the correct type' do
is_expected.to have_graphql_type(Types::UserMergeRequestInteractionType)
end
it 'has the correct arguments' do
is_expected.to have_attributes(arguments: be_empty)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GitlabSchema.types['UserMergeRequestInteraction'] do
include GraphqlHelpers
let_it_be(:user) { create(:user) }
let_it_be(:current_user) { create(:user) }
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:merge_request) { create(:merge_request, source_project: project) }
let(:interaction) { ::Users::MergeRequestInteraction.new(user: user, merge_request: merge_request.reset) }
specify { expect(described_class).to require_graphql_authorizations(:read_merge_request) }
it 'has the expected fields' do
expected_fields = %w[
can_merge
can_update
review_state
reviewed
approved
]
expect(described_class).to have_graphql_fields(*expected_fields).at_least
end
def resolve(field_name)
resolve_field(field_name, interaction, current_user: current_user)
end
describe '#can_merge' do
subject { resolve(:can_merge) }
context 'when the user cannot merge' do
it { is_expected.to be false }
end
context 'when the user can merge' do
before do
project.add_maintainer(user)
end
it { is_expected.to be true }
end
end
describe '#can_update' do
subject { resolve(:can_update) }
context 'when the user cannot update the MR' do
it { is_expected.to be false }
end
context 'when the user can update the MR' do
before do
project.add_developer(user)
end
it { is_expected.to be true }
end
end
describe '#review_state' do
subject { resolve(:review_state) }
context 'when the user has not been asked to review the MR' do
it { is_expected.to be_nil }
it 'implies not reviewed' do
expect(resolve(:reviewed)).to be false
end
end
context 'when the user has been asked to review the MR' do
before do
merge_request.reviewers << user
end
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['UNREVIEWED'].value) }
it 'implies not reviewed' do
expect(resolve(:reviewed)).to be false
end
end
context 'when the user has provided a review' do
before do
merge_request.merge_request_reviewers.create!(reviewer: user, state: MergeRequestReviewer.states['reviewed'])
end
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['REVIEWED'].value) }
it 'implies reviewed' do
expect(resolve(:reviewed)).to be true
end
end
end
describe '#approved' do
subject { resolve(:approved) }
context 'when the user has not approved the MR' do
it { is_expected.to be false }
end
context 'when the user has approved the MR' do
before do
merge_request.approved_by_users << user
end
it { is_expected.to be true }
end
end
end
......@@ -47,4 +47,19 @@ RSpec.describe BranchesHelper do
end
end
end
describe '#gl_dropdown_branches_enabled?' do
context 'when the feature is enabled' do
it 'returns true' do
expect(helper.gldropdrown_branches_enabled?).to be_truthy
end
end
context 'when the feature is disabled' do
it 'returns false' do
stub_feature_flags(gldropdown_branches: false)
expect(helper.gldropdrown_branches_enabled?).to be_falsy
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::DowntimeCheck::Message do
describe '#to_s' do
it 'returns an ANSI formatted String for an offline migration' do
message = described_class.new('foo.rb', true, 'hello')
expect(message.to_s).to eq("[\e[31moffline\e[0m]: foo.rb:\n\nhello\n\n")
end
it 'returns an ANSI formatted String for an online migration' do
message = described_class.new('foo.rb')
expect(message.to_s).to eq("[\e[32monline\e[0m]: foo.rb")
end
end
describe '#reason?' do
it 'returns false when no reason is specified' do
message = described_class.new('foo.rb')
expect(message.reason?).to eq(false)
end
it 'returns true when a reason is specified' do
message = described_class.new('foo.rb', true, 'hello')
expect(message.reason?).to eq(true)
end
end
describe '#reason' do
it 'strips excessive whitespace from the returned String' do
message = described_class.new('foo.rb', true, " hello\n world\n\n foo")
expect(message.reason).to eq("hello\nworld\n\nfoo")
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::DowntimeCheck do
subject { described_class.new }
let(:path) { 'foo.rb' }
describe '#check' do
before do
expect(subject).to receive(:require).with(path)
end
context 'when a migration does not specify if downtime is required' do
it 'raises RuntimeError' do
expect(subject).to receive(:class_for_migration_file)
.with(path)
.and_return(Class.new)
expect { subject.check([path]) }
.to raise_error(RuntimeError, /it requires downtime/)
end
end
context 'when a migration requires downtime' do
context 'when no reason is specified' do
it 'raises RuntimeError' do
stub_const('TestMigration::DOWNTIME', true)
expect(subject).to receive(:class_for_migration_file)
.with(path)
.and_return(TestMigration)
expect { subject.check([path]) }
.to raise_error(RuntimeError, /no reason was given/)
end
end
context 'when a reason is specified' do
it 'returns an Array of messages' do
stub_const('TestMigration::DOWNTIME', true)
stub_const('TestMigration::DOWNTIME_REASON', 'foo')
expect(subject).to receive(:class_for_migration_file)
.with(path)
.and_return(TestMigration)
messages = subject.check([path])
expect(messages).to be_an_instance_of(Array)
expect(messages[0]).to be_an_instance_of(Gitlab::DowntimeCheck::Message)
message = messages[0]
expect(message.path).to eq(path)
expect(message.offline).to eq(true)
expect(message.reason).to eq('foo')
end
end
end
end
describe '#check_and_print' do
it 'checks the migrations and prints the results to STDOUT' do
stub_const('TestMigration::DOWNTIME', true)
stub_const('TestMigration::DOWNTIME_REASON', 'foo')
expect(subject).to receive(:require).with(path)
expect(subject).to receive(:class_for_migration_file)
.with(path)
.and_return(TestMigration)
expect(subject).to receive(:puts).with(an_instance_of(String))
subject.check_and_print([path])
end
end
describe '#class_for_migration_file' do
it 'returns the class for a migration file path' do
expect(subject.class_for_migration_file('123_string.rb')).to eq(String)
end
end
describe '#online?' do
it 'returns true when a migration can be performed online' do
stub_const('TestMigration::DOWNTIME', false)
expect(subject.online?(TestMigration)).to eq(true)
end
it 'returns false when a migration can not be performed online' do
stub_const('TestMigration::DOWNTIME', true)
expect(subject.online?(TestMigration)).to eq(false)
end
end
describe '#downtime_reason' do
context 'when a reason is defined' do
it 'returns the downtime reason' do
stub_const('TestMigration::DOWNTIME_REASON', 'hello')
expect(subject.downtime_reason(TestMigration)).to eq('hello')
end
end
context 'when a reason is not defined' do
it 'returns nil' do
expect(subject.downtime_reason(Class.new)).to be_nil
end
end
end
end
# frozen_string_literal: true
require 'fast_spec_helper'
RSpec.describe ::Gitlab::Graphql::Authorize::ObjectAuthorization do
describe '#ok?' do
subject { described_class.new(%i[go_fast go_slow]) }
let(:user) { double(:User, id: 10001) }
let(:policy) do
Class.new(::DeclarativePolicy::Base) do
condition(:fast, scope: :subject) { @subject.x >= 10 }
condition(:slow, scope: :subject) { @subject.y >= 10 }
rule { fast }.policy do
enable :go_fast
end
rule { slow }.policy do
enable :go_slow
end
end
end
before do
stub_const('Foo', Struct.new(:x, :y))
stub_const('FooPolicy', policy)
end
context 'when there are no abilities' do
subject { described_class.new([]) }
it { is_expected.to be_ok(double, double) }
end
context 'when no ability should be allowed' do
let(:object) { Foo.new(0, 0) }
it { is_expected.not_to be_ok(object, user) }
end
context 'when go_fast should be allowed' do
let(:object) { Foo.new(100, 0) }
it { is_expected.not_to be_ok(object, user) }
end
context 'when go_fast and go_slow should be allowed' do
let(:object) { Foo.new(100, 100) }
it { is_expected.to be_ok(object, user) }
end
context 'when the object delegates to another subject' do
def proxy(foo)
double(:Proxy, declarative_policy_subject: foo)
end
it { is_expected.to be_ok(proxy(Foo.new(100, 100)), user) }
it { is_expected.not_to be_ok(proxy(Foo.new(0, 100)), user) }
end
end
end
......@@ -417,4 +417,59 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do
end
end
end
context 'extract and apply cursor attributes' do
let(:model) { Project.new(id: 100) }
let(:scope) { Project.all }
shared_examples 'cursor attribute examples' do
describe '#cursor_attributes_for_node' do
it { expect(order.cursor_attributes_for_node(model)).to eq({ id: '100' }.with_indifferent_access) }
end
describe '#apply_cursor_conditions' do
context 'when params with string keys are passed' do
subject(:sql) { order.apply_cursor_conditions(scope, { 'id' => '100' }).to_sql }
it { is_expected.to include('"projects"."id" < 100)') }
end
context 'when params with symbol keys are passed' do
subject(:sql) { order.apply_cursor_conditions(scope, { id: '100' }).to_sql }
it { is_expected.to include('"projects"."id" < 100)') }
end
end
end
context 'when string attribute name is given' do
let(:order) do
Gitlab::Pagination::Keyset::Order.build([
Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
attribute_name: 'id',
order_expression: Project.arel_table['id'].desc,
nullable: :not_nullable,
distinct: true
)
])
end
it_behaves_like 'cursor attribute examples'
end
context 'when symbol attribute name is given' do
let(:order) do
Gitlab::Pagination::Keyset::Order.build([
Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
attribute_name: :id,
order_expression: Project.arel_table['id'].desc,
nullable: :not_nullable,
distinct: true
)
])
end
it_behaves_like 'cursor attribute examples'
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ::Users::MergeRequestInteraction do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:merge_request) { create(:merge_request, source_project: project) }
subject(:interaction) do
::Users::MergeRequestInteraction.new(user: user, merge_request: merge_request.reset)
end
describe 'declarative policy delegation' do
it 'delegates to the merge request' do
expect(subject.declarative_policy_subject).to eq(merge_request)
end
end
describe '#can_merge?' do
context 'when the user cannot merge' do
it { is_expected.not_to be_can_merge }
end
context 'when the user can merge' do
before do
project.add_maintainer(user)
end
it { is_expected.to be_can_merge }
end
end
describe '#can_update?' do
context 'when the user cannot update the MR' do
it { is_expected.not_to be_can_update }
end
context 'when the user can update the MR' do
before do
project.add_developer(user)
end
it { is_expected.to be_can_update }
end
end
describe '#review_state' do
subject { interaction.review_state }
context 'when the user has not been asked to review the MR' do
it { is_expected.to be_nil }
it 'implies not reviewed' do
expect(interaction).not_to be_reviewed
end
end
context 'when the user has been asked to review the MR' do
before do
merge_request.reviewers << user
end
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['UNREVIEWED'].value) }
it 'implies not reviewed' do
expect(interaction).not_to be_reviewed
end
end
context 'when the user has provided a review' do
before do
merge_request.merge_request_reviewers.create!(reviewer: user, state: MergeRequestReviewer.states['reviewed'])
end
it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['REVIEWED'].value) }
it 'implies reviewed' do
expect(interaction).to be_reviewed
end
end
end
describe '#approved?' do
context 'when the user has not approved the MR' do
it { is_expected.not_to be_approved }
end
context 'when the user has approved the MR' do
before do
merge_request.approved_by_users << user
end
it { is_expected.to be_approved }
end
end
end
......@@ -42,7 +42,13 @@ RSpec.describe 'User' do
end
context 'when username and id parameter are used' do
let_it_be(:query) { graphql_query_for(:user, { id: current_user.to_global_id.to_s, username: current_user.username }, 'id') }
let_it_be(:query) do
graphql_query_for(
:user,
{ id: current_user.to_global_id.to_s, username: current_user.username },
'id'
)
end
it 'displays an error' do
post_graphql(query)
......
......@@ -17,7 +17,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
it 'registers an offense' do
expect_offense(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -54,7 +53,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -90,8 +88,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :test_text_limits, id: false do |t|
t.integer :test_id, null: false
......@@ -113,7 +109,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
it 'registers an offense' do
expect_offense(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -146,7 +141,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
it 'registers no offenses' do
expect_no_offenses(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -168,8 +162,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
drop_table :no_offense_on_down
end
......@@ -194,7 +186,6 @@ RSpec.describe RuboCop::Cop::Migration::AddLimitToTextColumns do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
......
......@@ -9,8 +9,6 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
let(:migration_with_add_timestamps) do
%q(
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_timestamps(:users)
......@@ -22,8 +20,6 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
let(:migration_without_add_timestamps) do
%q(
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column(:users, :username, :text)
end
......@@ -34,8 +30,6 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
let(:migration_with_add_timestamps_with_timezone) do
%q(
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_timestamps_with_timezone(:users)
......@@ -52,8 +46,6 @@ RSpec.describe RuboCop::Cop::Migration::AddTimestamps do
it 'registers an offense when the "add_timestamps" method is used' do
expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_timestamps(:users)
......
......@@ -18,8 +18,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
it 'registers an offense' do
expect_offense(<<~RUBY)
class TestComplexIndexes < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :test_table do |t|
t.integer :column1, null: false
......@@ -46,8 +44,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class TestComplexIndexes < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :test_table do |t|
t.integer :column1, null: false
......@@ -74,8 +70,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
it 'registers an offense' do
expect_offense(<<~RUBY)
class TestComplexIndexes < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
......@@ -101,8 +95,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
it 'registers no offenses' do
expect_no_offenses(<<~RUBY)
class TestComplexIndexes < ActiveRecord::Migration[6.0]
DOWNTIME = false
INDEX_NAME = 'my_test_name'
disable_ddl_transaction!
......@@ -135,8 +127,6 @@ RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName do
it 'registers no offenses' do
expect_no_offenses(<<~RUBY)
class TestComplexIndexes < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
......
......@@ -9,8 +9,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
let(:create_table_migration_without_datetime) do
%q(
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
......@@ -24,8 +22,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
let(:create_table_migration_with_datetime_with_timezone) do
%q(
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
......@@ -39,8 +35,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
let(:add_column_migration_with_datetime) do
%q(
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_column(:users, :last_sign_in, :datetime)
......@@ -52,8 +46,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
let(:add_column_migration_with_timestamp) do
%q(
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_column(:users, :last_sign_in, :timestamp)
......@@ -65,8 +57,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
let(:add_column_migration_without_datetime) do
%q(
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column(:users, :username, :text)
end
......@@ -77,8 +67,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
let(:add_column_migration_with_datetime_with_timezone) do
%q(
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_column(:users, :last_sign_in, :datetime_with_timezone)
......@@ -95,8 +83,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
it 'registers an offense when the ":datetime" data type is used on create_table' do
expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
......@@ -111,8 +97,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
it 'registers an offense when the ":timestamp" data type is used on create_table' do
expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
......@@ -135,8 +119,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
it 'registers an offense when the ":datetime" data type is used on add_column' do
expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_column(:users, :last_sign_in, :datetime)
......@@ -149,8 +131,6 @@ RSpec.describe RuboCop::Cop::Migration::Datetime do
it 'registers an offense when the ":timestamp" data type is used on add_column' do
expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column(:users, :username, :text)
add_column(:users, :last_sign_in, :timestamp)
......
......@@ -15,8 +15,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
it 'registers an offense' do
expect_offense(<<~RUBY, msg: "Do not use the `string` data type, use `text` instead.[...]")
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :users do |t|
t.string :username, null: false
......@@ -46,8 +44,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :users do |t|
t.integer :not_a_string, null: false
......@@ -65,8 +61,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :users do |t|
t.text :username, null: false
......@@ -87,8 +81,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class TestStringArrays < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :test_string_arrays, id: false do |t|
t.integer :test_id, null: false
......@@ -108,8 +100,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
remove_column :users, :bio
remove_column :users, :url
......@@ -137,8 +127,6 @@ RSpec.describe RuboCop::Cop::Migration::PreventStrings do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class Users < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :users do |t|
t.string :username, null: false
......
......@@ -15,8 +15,6 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do
it 'registers an offense' do
expect_offense(<<~RUBY, msg: 'migration methods that refer to existing indexes must do so by name')
class TestReferToIndexByName < ActiveRecord::Migration[6.0]
DOWNTIME = false
INDEX_NAME = 'my_test_name'
disable_ddl_transaction!
......@@ -63,8 +61,6 @@ RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName do
it 'registers no offenses' do
expect_no_offenses(<<~RUBY)
class TestReferToIndexByName < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
......
......@@ -9,8 +9,6 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
let(:migration_with_timestamps) do
%q(
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
......@@ -25,8 +23,6 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
let(:migration_without_timestamps) do
%q(
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
......@@ -40,8 +36,6 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
let(:migration_with_timestamps_with_timezone) do
%q(
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
......@@ -61,8 +55,6 @@ RSpec.describe RuboCop::Cop::Migration::Timestamps do
it 'registers an offense when the "timestamps" method is used' do
expect_offense(<<~RUBY)
class Users < ActiveRecord::Migration[4.2]
DOWNTIME = false
def change
create_table :users do |t|
t.string :username, null: false
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment