diff options
88 files changed, 1466 insertions, 626 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7f1da58fa9d..8596037afa3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,7 +36,7 @@ _This notice should stay as the first item in the CONTRIBUTING.md file._ - [Release Scoping labels](#release-scoping-labels) - [Priority labels](#priority-labels) - [Severity labels](#severity-labels) - - [Severity impact guidance](#severity-impact-guidance) + - [Severity impact guidance](#severity-impact-guidance) - [Label for community contributors](#label-for-community-contributors) - [Implement design & UI elements](#implement-design--ui-elements) - [Issue tracker](#issue-tracker) @@ -70,7 +70,7 @@ to contribute to GitLab in a way that is easy for everyone. For a first-time step-by-step guide to the contribution process, please see ["Contributing to GitLab"](https://about.gitlab.com/contributing/). -Looking for something to work on? Look for issues with the label [Accepting Merge Requests](#i-want-to-contribute). +Looking for something to work on? Look for issues in the [Backlog (Accepting merge requests) milestone](#i-want-to-contribute). GitLab comes into two flavors, GitLab Community Edition (CE) our free and open source edition, and GitLab Enterprise Edition (EE) which is our commercial @@ -151,8 +151,8 @@ the remaining issues on the GitHub issue tracker. ## I want to contribute! -If you want to contribute to GitLab [issues with the label `Accepting Merge Requests` and small weight][accepting-mrs-weight] -is a great place to start. Issues with a lower weight (1 or 2) are deemed +If you want to contribute to GitLab, [issues in the Backlog (Accepting merge requests)](https://gitlab.com/gitlab-org/gitlab-ce/issues?scope=all&utf8=✓&state=opened&assignee_id=0&milestone_title=Backlog%20(Accepting%20merge%20requests)) +are a great place to start. Issues with a lower weight (1 or 2) are deemed suitable for beginners. These issues will be of reasonable size and challenge, for anyone to start contributing to GitLab. If you have any questions or need help visit [Getting Help](https://about.gitlab.com/getting-help/#discussion) to learn how to communicate with GitLab. If you're looking for a Gitter or Slack channel diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 48b1190fd56..aa6a32fa84e 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -128,7 +128,7 @@ GEM coderay (1.1.2) coercible (1.0.0) descendants_tracker (~> 0.0.1) - commonmarker (0.17.8) + commonmarker (0.17.13) ruby-enum (~> 0.5) concord (0.1.5) adamantium (~> 0.2.0) diff --git a/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js b/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js index cce2c07dc3e..6c9499864c5 100644 --- a/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js +++ b/app/assets/javascripts/filtered_search/issuable_filtered_search_token_keys.js @@ -1,6 +1,6 @@ import FilteredSearchTokenKeys from './filtered_search_token_keys'; -const tokenKeys = [{ +export const tokenKeys = [{ key: 'author', type: 'string', param: 'username', @@ -42,14 +42,14 @@ if (gon.current_user_id) { }); } -const alternativeTokenKeys = [{ +export const alternativeTokenKeys = [{ key: 'label', type: 'string', param: 'name', symbol: '~', }]; -const conditions = [{ +export const conditions = [{ url: 'assignee_id=0', tokenKey: 'assignee', value: 'none', diff --git a/app/assets/javascripts/ide/components/file_row_extra.vue b/app/assets/javascripts/ide/components/file_row_extra.vue new file mode 100644 index 00000000000..44a360ab909 --- /dev/null +++ b/app/assets/javascripts/ide/components/file_row_extra.vue @@ -0,0 +1,104 @@ +<script> +import { mapGetters } from 'vuex'; +import { n__, __, sprintf } from '~/locale'; +import tooltip from '~/vue_shared/directives/tooltip'; +import Icon from '~/vue_shared/components/icon.vue'; +import NewDropdown from './new_dropdown/index.vue'; +import ChangedFileIcon from './changed_file_icon.vue'; +import MrFileIcon from './mr_file_icon.vue'; + +export default { + name: 'FileRowExtra', + directives: { + tooltip, + }, + components: { + Icon, + NewDropdown, + ChangedFileIcon, + MrFileIcon, + }, + props: { + file: { + type: Object, + required: true, + }, + mouseOver: { + type: Boolean, + required: true, + }, + }, + computed: { + ...mapGetters([ + 'getChangesInFolder', + 'getUnstagedFilesCountForPath', + 'getStagedFilesCountForPath', + ]), + folderUnstagedCount() { + return this.getUnstagedFilesCountForPath(this.file.path); + }, + folderStagedCount() { + return this.getStagedFilesCountForPath(this.file.path); + }, + changesCount() { + return this.getChangesInFolder(this.file.path); + }, + folderChangesTooltip() { + if (this.changesCount === 0) return undefined; + + if (this.folderUnstagedCount > 0 && this.folderStagedCount === 0) { + return n__('%d unstaged change', '%d unstaged changes', this.folderUnstagedCount); + } else if (this.folderUnstagedCount === 0 && this.folderStagedCount > 0) { + return n__('%d staged change', '%d staged changes', this.folderStagedCount); + } + + return sprintf(__('%{unstaged} unstaged and %{staged} staged changes'), { + unstaged: this.folderUnstagedCount, + staged: this.folderStagedCount, + }); + }, + showTreeChangesCount() { + return this.file.type === 'tree' && this.changesCount > 0 && !this.file.opened; + }, + showChangedFileIcon() { + return this.file.changed || this.file.tempFile || this.file.staged; + }, + }, +}; +</script> + +<template> + <div class="float-right ide-file-icon-holder"> + <mr-file-icon + v-if="file.mrChange" + /> + <span + v-if="showTreeChangesCount" + class="ide-tree-changes" + > + {{ changesCount }} + <icon + v-tooltip + :title="folderChangesTooltip" + :size="12" + data-container="body" + data-placement="right" + name="file-modified" + css-classes="prepend-left-5 ide-file-modified" + /> + </span> + <changed-file-icon + v-else-if="showChangedFileIcon" + :file="file" + :show-tooltip="true" + :show-staged-icon="true" + :force-modified-icon="true" + /> + <new-dropdown + :type="file.type" + :path="file.path" + :mouse-over="mouseOver" + class="prepend-left-8" + /> + </div> +</template> diff --git a/app/assets/javascripts/ide/components/ide_tree_list.vue b/app/assets/javascripts/ide/components/ide_tree_list.vue index 00ae5ea2c15..e658d1bf956 100644 --- a/app/assets/javascripts/ide/components/ide_tree_list.vue +++ b/app/assets/javascripts/ide/components/ide_tree_list.vue @@ -2,15 +2,16 @@ import { mapActions, mapGetters, mapState } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; -import RepoFile from './repo_file.vue'; +import FileRow from '~/vue_shared/components/file_row.vue'; import NavDropdown from './nav_dropdown.vue'; +import FileRowExtra from './file_row_extra.vue'; export default { components: { Icon, - RepoFile, SkeletonLoadingContainer, NavDropdown, + FileRow, }, props: { viewerType: { @@ -34,8 +35,9 @@ export default { this.updateViewer(this.viewerType); }, methods: { - ...mapActions(['updateViewer']), + ...mapActions(['updateViewer', 'toggleTreeOpen']), }, + FileRowExtra, }; </script> @@ -63,11 +65,13 @@ export default { <div class="ide-tree-body h-100" > - <repo-file + <file-row v-for="file in currentTree.tree" :key="file.key" :file="file" :level="0" + :extra-component="$options.FileRowExtra" + @toggleTreeOpen="toggleTreeOpen" /> </div> </template> diff --git a/app/assets/javascripts/ide/components/repo_file.vue b/app/assets/javascripts/ide/components/repo_file.vue deleted file mode 100644 index 110eda83bb4..00000000000 --- a/app/assets/javascripts/ide/components/repo_file.vue +++ /dev/null @@ -1,227 +0,0 @@ -<script> -import { mapActions, mapGetters } from 'vuex'; -import { n__, __, sprintf } from '~/locale'; -import tooltip from '~/vue_shared/directives/tooltip'; -import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; -import Icon from '~/vue_shared/components/icon.vue'; -import FileIcon from '~/vue_shared/components/file_icon.vue'; -import router from '../ide_router'; -import NewDropdown from './new_dropdown/index.vue'; -import FileStatusIcon from './repo_file_status_icon.vue'; -import ChangedFileIcon from './changed_file_icon.vue'; -import MrFileIcon from './mr_file_icon.vue'; - -export default { - name: 'RepoFile', - directives: { - tooltip, - }, - components: { - SkeletonLoadingContainer, - NewDropdown, - FileStatusIcon, - FileIcon, - ChangedFileIcon, - MrFileIcon, - Icon, - }, - props: { - file: { - type: Object, - required: true, - }, - level: { - type: Number, - required: true, - }, - }, - data() { - return { - mouseOver: false, - }; - }, - computed: { - ...mapGetters([ - 'getChangesInFolder', - 'getUnstagedFilesCountForPath', - 'getStagedFilesCountForPath', - ]), - folderUnstagedCount() { - return this.getUnstagedFilesCountForPath(this.file.path); - }, - folderStagedCount() { - return this.getStagedFilesCountForPath(this.file.path); - }, - changesCount() { - return this.getChangesInFolder(this.file.path); - }, - folderChangesTooltip() { - if (this.changesCount === 0) return undefined; - - if (this.folderUnstagedCount > 0 && this.folderStagedCount === 0) { - return n__('%d unstaged change', '%d unstaged changes', this.folderUnstagedCount); - } else if (this.folderUnstagedCount === 0 && this.folderStagedCount > 0) { - return n__('%d staged change', '%d staged changes', this.folderStagedCount); - } - - return sprintf(__('%{unstaged} unstaged and %{staged} staged changes'), { - unstaged: this.folderUnstagedCount, - staged: this.folderStagedCount, - }); - }, - isTree() { - return this.file.type === 'tree'; - }, - isBlob() { - return this.file.type === 'blob'; - }, - levelIndentation() { - return { - marginLeft: `${this.level * 16}px`, - }; - }, - fileClass() { - return { - 'file-open': this.isBlob && this.file.opened, - 'file-active': this.isBlob && this.file.active, - folder: this.isTree, - 'is-open': this.file.opened, - }; - }, - showTreeChangesCount() { - return this.isTree && this.changesCount > 0 && !this.file.opened; - }, - showChangedFileIcon() { - return this.file.changed || this.file.tempFile || this.file.staged; - }, - }, - watch: { - 'file.active': function fileActiveWatch(active) { - if (this.file.type === 'blob' && active) { - this.scrollIntoView(); - } - }, - }, - mounted() { - if (this.hasPathAtCurrentRoute()) { - this.scrollIntoView(true); - } - }, - methods: { - ...mapActions(['toggleTreeOpen']), - clickFile() { - // Manual Action if a tree is selected/opened - if (this.isTree && this.hasUrlAtCurrentRoute()) { - this.toggleTreeOpen(this.file.path); - } - - router.push(`/project${this.file.url}`); - }, - scrollIntoView(isInit = false) { - const block = isInit && this.isTree ? 'center' : 'nearest'; - - this.$el.scrollIntoView({ - behavior: 'smooth', - block, - }); - }, - hasPathAtCurrentRoute() { - if (!this.$router || !this.$router.currentRoute) { - return false; - } - - // - strip route up to "/-/" and ending "/" - const routePath = this.$router.currentRoute.path - .replace(/^.*?[/]-[/]/g, '') - .replace(/[/]$/g, ''); - - // - strip ending "/" - const filePath = this.file.path.replace(/[/]$/g, ''); - - return filePath === routePath; - }, - hasUrlAtCurrentRoute() { - return this.$router.currentRoute.path === `/project${this.file.url}`; - }, - toggleHover(over) { - this.mouseOver = over; - }, - }, -}; -</script> - -<template> - <div> - <div - :class="fileClass" - class="file" - role="button" - @click="clickFile" - @mouseover="toggleHover(true)" - @mouseout="toggleHover(false)" - > - <div - class="file-name" - > - <span - :style="levelIndentation" - class="ide-file-name str-truncated" - > - <file-icon - :file-name="file.name" - :loading="file.loading" - :folder="isTree" - :opened="file.opened" - :size="16" - /> - {{ file.name }} - <file-status-icon - :file="file" - /> - </span> - <span class="float-right ide-file-icon-holder"> - <mr-file-icon - v-if="file.mrChange" - /> - <span - v-if="showTreeChangesCount" - class="ide-tree-changes" - > - {{ changesCount }} - <icon - v-tooltip - :title="folderChangesTooltip" - :size="12" - data-container="body" - data-placement="right" - name="file-modified" - css-classes="prepend-left-5 ide-file-modified" - /> - </span> - <changed-file-icon - v-else-if="showChangedFileIcon" - :file="file" - :show-tooltip="true" - :show-staged-icon="true" - :force-modified-icon="true" - class="float-right" - /> - </span> - <new-dropdown - :type="file.type" - :path="file.path" - :mouse-over="mouseOver" - class="float-right prepend-left-8" - /> - </div> - </div> - <template v-if="file.opened"> - <repo-file - v-for="childFile in file.tree" - :key="childFile.key" - :file="childFile" - :level="level + 1" - /> - </template> - </div> -</template> diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue new file mode 100644 index 00000000000..6f7bdbc2c4d --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -0,0 +1,210 @@ +<script> +import Icon from '~/vue_shared/components/icon.vue'; +import FileIcon from '~/vue_shared/components/file_icon.vue'; + +export default { + name: 'FileRow', + components: { + FileIcon, + Icon, + }, + props: { + file: { + type: Object, + required: true, + }, + level: { + type: Number, + required: true, + }, + extraComponent: { + type: Object, + required: false, + default: null, + }, + }, + data() { + return { + mouseOver: false, + }; + }, + computed: { + isTree() { + return this.file.type === 'tree'; + }, + isBlob() { + return this.file.type === 'blob'; + }, + levelIndentation() { + return { + marginLeft: `${this.level * 16}px`, + }; + }, + fileClass() { + return { + 'file-open': this.isBlob && this.file.opened, + 'is-active': this.isBlob && this.file.active, + folder: this.isTree, + 'is-open': this.file.opened, + }; + }, + }, + watch: { + 'file.active': function fileActiveWatch(active) { + if (this.file.type === 'blob' && active) { + this.scrollIntoView(); + } + }, + }, + mounted() { + if (this.hasPathAtCurrentRoute()) { + this.scrollIntoView(true); + } + }, + methods: { + toggleTreeOpen(path) { + this.$emit('toggleTreeOpen', path); + }, + clickFile() { + // Manual Action if a tree is selected/opened + if (this.isTree && this.hasUrlAtCurrentRoute()) { + this.toggleTreeOpen(this.file.path); + } + + if (this.$router) this.$router.push(`/project${this.file.url}`); + }, + scrollIntoView(isInit = false) { + const block = isInit && this.isTree ? 'center' : 'nearest'; + + this.$el.scrollIntoView({ + behavior: 'smooth', + block, + }); + }, + hasPathAtCurrentRoute() { + if (!this.$router || !this.$router.currentRoute) { + return false; + } + + // - strip route up to "/-/" and ending "/" + const routePath = this.$router.currentRoute.path + .replace(/^.*?[/]-[/]/g, '') + .replace(/[/]$/g, ''); + + // - strip ending "/" + const filePath = this.file.path.replace(/[/]$/g, ''); + + return filePath === routePath; + }, + hasUrlAtCurrentRoute() { + if (!this.$router || !this.$router.currentRoute) return true; + + return this.$router.currentRoute.path === `/project${this.file.url}`; + }, + toggleHover(over) { + this.mouseOver = over; + }, + }, +}; +</script> + +<template> + <div> + <div + :class="fileClass" + class="file-row" + role="button" + @click="clickFile" + @mouseover="toggleHover(true)" + @mouseout="toggleHover(false)" + > + <div + class="file-row-name-container" + > + <span + :style="levelIndentation" + class="file-row-name str-truncated" + > + <file-icon + :file-name="file.name" + :loading="file.loading" + :folder="isTree" + :opened="file.opened" + :size="16" + /> + {{ file.name }} + </span> + <component + v-if="extraComponent" + :is="extraComponent" + :file="file" + :mouse-over="mouseOver" + /> + </div> + </div> + <template v-if="file.opened"> + <file-row + v-for="childFile in file.tree" + :key="childFile.key" + :file="childFile" + :level="level + 1" + :extra-component="extraComponent" + @toggleTreeOpen="toggleTreeOpen" + /> + </template> + </div> +</template> + +<style> +.file-row { + display: flex; + align-items: center; + height: 32px; + padding: 4px 8px; + margin-left: -8px; + margin-right: -8px; + border-radius: 3px; + text-align: left; + cursor: pointer; +} + +.file-row:hover, +.file-row:focus { + background: #f2f2f2; +} + +.file-row:active { + background: #dfdfdf; +} + +.file-row.is-active { + background: #f2f2f2; +} + +.file-row-name-container { + display: flex; + width: 100%; + align-items: center; + overflow: visible; +} + +.file-row-name { + display: inline-block; + flex: 1; + max-width: inherit; + height: 18px; + line-height: 16px; + text-overflow: ellipsis; + white-space: nowrap; +} + +.file-row-name svg { + margin-right: 2px; + vertical-align: middle; +} + +.file-row-name .loading-container { + display: inline-block; + margin-right: 4px; +} +</style> diff --git a/app/assets/stylesheets/page_bundles/ide.scss b/app/assets/stylesheets/page_bundles/ide.scss index 45df8391f9a..65f0a0d18e2 100644 --- a/app/assets/stylesheets/page_bundles/ide.scss +++ b/app/assets/stylesheets/page_bundles/ide.scss @@ -53,83 +53,9 @@ $ide-commit-header-height: 48px; flex: 1; min-height: 0; // firefox fix - .file { - height: 32px; - cursor: pointer; - - &.file-active { - background: $theme-gray-100; - } - - .ide-file-name { - flex: 1; - white-space: nowrap; - text-overflow: ellipsis; - max-width: inherit; - line-height: 16px; - display: inline-block; - height: 18px; - - svg { - vertical-align: middle; - margin-right: 2px; - } - - .loading-container { - margin-right: 4px; - display: inline-block; - } - } - - .ide-file-icon-holder { - display: flex; - align-items: center; - color: $theme-gray-700; - } - - .ide-file-changed-icon { - margin-left: auto; - - > svg { - display: block; - } - } - - .ide-new-btn { - display: none; - - .btn { - padding: 2px 5px; - } - } - - &:hover, - &:focus { - .ide-new-btn { - display: block; - } - } - - .folder-icon { - fill: $gl-text-color-secondary; - } - } - a { color: $gl-text-color; } - - th { - position: sticky; - top: 0; - } -} - -.file-name { - display: flex; - overflow: visible; - align-items: center; - width: 100%; } .multi-file-loading-container { @@ -625,8 +551,7 @@ $ide-commit-header-height: 48px; } } -.multi-file-commit-list-path, -.ide-file-list .file { +.multi-file-commit-list-path { display: flex; align-items: center; margin-left: -$grid-size; @@ -634,28 +559,14 @@ $ide-commit-header-height: 48px; padding: $grid-size / 2 $grid-size; border-radius: $border-radius-default; text-align: left; - - &:hover, - &:focus { - background: $theme-gray-100; - } - - &:active { - background: $theme-gray-200; - } -} - -.multi-file-commit-list-path { cursor: pointer; height: $ide-commit-row-height; padding-right: 0; - &.is-active { - background-color: $white-normal; - } - &:hover, &:focus { + background: $theme-gray-100; + outline: 0; .multi-file-discard-btn { @@ -665,6 +576,14 @@ $ide-commit-header-height: 48px; } } + &:active { + background: $theme-gray-200; + } + + &.is-active { + background-color: $white-normal; + } + svg { min-width: 16px; vertical-align: middle; @@ -1398,9 +1317,17 @@ $ide-commit-header-height: 48px; } } -.ide-new-btn .dropdown.show .ide-entry-dropdown-toggle { - color: $white-normal; - background-color: $blue-500; +.ide-new-btn { + display: none; + + .btn { + padding: 2px 5px; + } + + .dropdown.show .ide-entry-dropdown-toggle { + color: $white-normal; + background-color: $blue-500; + } } .ide-preview-header { @@ -1465,3 +1392,28 @@ $ide-commit-header-height: 48px; width: $ide-commit-row-height; height: $ide-commit-row-height; } + +.ide-file-icon-holder { + display: flex; + align-items: center; + color: $theme-gray-700; +} + +.ide-file-changed-icon { + margin-left: auto; + + > svg { + display: block; + } +} + +.file-row:hover, +.file-row:focus { + .ide-new-btn { + display: block; + } + + .folder-icon { + fill: $gl-text-color-secondary; + } +} diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 17f34319050..caa839c32a5 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -279,6 +279,10 @@ table.u2f-registrations { } } +.codes { + padding-top: 14px; +} + .oauth-application-show { .scope-name { font-weight: $gl-font-weight-bold; diff --git a/app/finders/members_finder.rb b/app/finders/members_finder.rb index 48777838d60..f90a7868102 100644 --- a/app/finders/members_finder.rb +++ b/app/finders/members_finder.rb @@ -18,7 +18,7 @@ class MembersFinder group_members = GroupMembersFinder.new(group).execute(include_descendants: include_descendants) # rubocop: disable CodeReuse/Finder group_members = group_members.non_invite - union = Gitlab::SQL::Union.new([project_members, group_members], remove_duplicates: false) + union = Gitlab::SQL::Union.new([project_members, group_members], remove_duplicates: false) # rubocop: disable Gitlab/Union sql = distinct_on(union) diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index 715dffb972f..3528e4228b2 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -89,9 +89,7 @@ class SnippetsFinder < UnionFinder # We use a UNION here instead of OR clauses since this results in better # performance. - union = Gitlab::SQL::Union.new([authorized_projects.select('projects.id'), visible_projects.select('projects.id')]) - - Project.from("(#{union.to_sql}) AS #{Project.table_name}") + Project.from_union([authorized_projects, visible_projects]) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 0d9f16fdce7..74baf79e4f2 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -164,16 +164,13 @@ class TodosFinder # rubocop: disable CodeReuse/ActiveRecord def by_group(items) - if group? - groups = group.self_and_descendants - project_todos = items.where(project_id: Project.where(group: groups).select(:id)) - group_todos = items.where(group_id: groups.select(:id)) + return items unless group? - union = Gitlab::SQL::Union.new([project_todos, group_todos]) - items = Todo.from("(#{union.to_sql}) #{Todo.table_name}") - end + groups = group.self_and_descendants + project_todos = items.where(project_id: Project.where(group: groups).select(:id)) + group_todos = items.where(group_id: groups.select(:id)) - items + Todo.from_union([project_todos, group_todos]) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/finders/union_finder.rb b/app/finders/union_finder.rb index 798c3eba0f3..c3b02f7e52f 100644 --- a/app/finders/union_finder.rb +++ b/app/finders/union_finder.rb @@ -1,15 +1,15 @@ # frozen_string_literal: true class UnionFinder - # rubocop: disable CodeReuse/ActiveRecord def find_union(segments, klass) - if segments.length > 1 - union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) }) + unless klass < FromUnion + raise TypeError, "#{klass.inspect} must include the FromUnion module" + end - klass.where("#{klass.table_name}.id IN (#{union.to_sql})") + if segments.length > 1 + klass.from_union(segments) else segments.first end end - # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index f8d36dce45d..136772e1ec3 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -123,11 +123,6 @@ module CiStatusHelper render_status_with_link('pipeline', pipeline.status, path, tooltip_placement: tooltip_placement) end - def no_runners_for_project?(project) - project.runners.blank? && - Ci::Runner.instance_type.blank? - end - def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16) klass = "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}" title = "#{type.titleize}: #{ci_label_for_status(status)}" diff --git a/app/models/badge.rb b/app/models/badge.rb index 7e3b6b659e4..f016654206b 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Badge < ActiveRecord::Base + include FromUnion + # This structure sets the placeholders that the urls # can have. This hash also sets which action to ask when # the placeholder is found. diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index eabb41c29d7..3e815937f4b 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -7,6 +7,7 @@ module Ci include IgnorableColumn include RedisCacheable include ChronicDurationAttribute + include FromUnion RUNNER_QUEUE_EXPIRY_TIME = 60.minutes ONLINE_CONTACT_TIMEOUT = 1.hour @@ -57,18 +58,26 @@ module Ci } scope :owned_or_instance_wide, -> (project_id) do - union = Gitlab::SQL::Union.new( - [belonging_to_project(project_id), belonging_to_parent_group_of_project(project_id), instance_type], + from_union( + [ + belonging_to_project(project_id), + belonging_to_parent_group_of_project(project_id), + instance_type + ], remove_duplicates: false ) - from("(#{union.to_sql}) ci_runners") end scope :assignable_for, ->(project) do # FIXME: That `to_sql` is needed to workaround a weird Rails bug. # Without that, placeholders would miss one and couldn't match. + # + # We use "unscoped" here so that any current Ci::Runner filters don't + # apply to the inner query, which is not necessary. + exclude_runners = unscoped { project.runners.select(:id) }.to_sql + where(locked: false) - .where.not("ci_runners.id IN (#{project.runners.select(:id).to_sql})") + .where.not("ci_runners.id IN (#{exclude_runners})") .project_type end diff --git a/app/models/concerns/from_union.rb b/app/models/concerns/from_union.rb new file mode 100644 index 00000000000..9b8595b1211 --- /dev/null +++ b/app/models/concerns/from_union.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module FromUnion + extend ActiveSupport::Concern + + class_methods do + # Produces a query that uses a FROM to select data using a UNION. + # + # Using a FROM for a UNION has in the past lead to better query plans. As + # such, we generally recommend this pattern instead of using a WHERE IN. + # + # Example: + # users = User.from_union([User.where(id: 1), User.where(id: 2)]) + # + # This would produce the following SQL query: + # + # SELECT * + # FROM ( + # SELECT * + # FROM users + # WHERE id = 1 + # + # UNION + # + # SELECT * + # FROM users + # WHERE id = 2 + # ) users; + # + # members - An Array of ActiveRecord::Relation objects to use in the UNION. + # + # remove_duplicates - A boolean indicating if duplicate entries should be + # removed. Defaults to true. + # + # alias_as - The alias to use for the sub query. Defaults to the name of the + # table of the current model. + # rubocop: disable Gitlab/Union + def from_union(members, remove_duplicates: true, alias_as: table_name) + union = Gitlab::SQL::Union + .new(members, remove_duplicates: remove_duplicates) + .to_sql + + # This pattern is necessary as a bug in Rails 4 can cause the use of + # `from("string here").includes(:foo)` to break ActiveRecord. This is + # fixed in https://github.com/rails/rails/pull/25374, which is released as + # part of Rails 5. + from([Arel.sql("(#{union}) #{alias_as}")]) + end + # rubocop: enable Gitlab/Union + end +end diff --git a/app/models/event.rb b/app/models/event.rb index 041dac6941b..596155a9525 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -3,6 +3,7 @@ class Event < ActiveRecord::Base include Sortable include IgnorableColumn + include FromUnion default_scope { reorder(nil) } CREATED = 1 diff --git a/app/models/group.rb b/app/models/group.rb index 024e77188b8..62af20d2142 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -304,14 +304,12 @@ class Group < Namespace # 3. They belong to a sub-group or project in such sub-group # 4. They belong to an ancestor group def direct_and_indirect_users - union = Gitlab::SQL::Union.new([ + User.from_union([ User .where(id: direct_and_indirect_members.select(:user_id)) .reorder(nil), project_users_with_descendants ]) - - User.from("(#{union.to_sql}) #{User.table_name}") end # Returns all users that are members of projects diff --git a/app/models/label.rb b/app/models/label.rb index 8dc7ded53ad..9ef57a05b3e 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -7,6 +7,7 @@ class Label < ActiveRecord::Base include Gitlab::SQL::Pattern include OptionallySearch include Sortable + include FromUnion # Represents a "No Label" state used for filtering Issues and Merge # Requests that have no label assigned. diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e19bf62dcd0..dd5d494997d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -14,6 +14,7 @@ class MergeRequest < ActiveRecord::Base include Gitlab::Utils::StrongMemoize include LabelEventable include ReactiveCaching + include FromUnion self.reactive_cache_key = ->(model) { [model.project.id, model.iid] } self.reactive_cache_refresh_interval = 10.minutes @@ -237,11 +238,10 @@ class MergeRequest < ActiveRecord::Base def self.in_projects(relation) # unscoping unnecessary conditions that'll be applied # when executing `where("merge_requests.id IN (#{union.to_sql})")` - source = unscoped.where(source_project_id: relation).select(:id) - target = unscoped.where(target_project_id: relation).select(:id) - union = Gitlab::SQL::Union.new([source, target]) + source = unscoped.where(source_project_id: relation) + target = unscoped.where(target_project_id: relation) - where("merge_requests.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + from_union([source, target]) end # This is used after project import, to reset the IDs to the correct @@ -740,11 +740,8 @@ class MergeRequest < ActiveRecord::Base # compared to using OR statements. We're using UNION ALL since the queries # used won't produce any duplicates (e.g. a note for a commit can't also be # a note for an MR). - union = Gitlab::SQL::Union - .new([notes, commit_notes], remove_duplicates: false) - .to_sql - - Note.from("(#{union}) #{Note.table_name}") + Note + .from_union([notes, commit_notes], remove_duplicates: false) .includes(:noteable) end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 76920c3c039..0289f29211d 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -11,6 +11,7 @@ class Namespace < ActiveRecord::Base include Gitlab::SQL::Pattern include IgnorableColumn include FeatureGate + include FromUnion ignore_column :deleted_at diff --git a/app/models/note.rb b/app/models/note.rb index 4429c1dcb07..bea02d69b65 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -17,6 +17,7 @@ class Note < ActiveRecord::Base include Editable include Gitlab::SQL::Pattern include ThrottledTouch + include FromUnion module SpecialRole FIRST_TIME_CONTRIBUTOR = :first_time_contributor diff --git a/app/models/project.rb b/app/models/project.rb index c37915e111f..9e4c7f7a2d0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -29,6 +29,7 @@ class Project < ActiveRecord::Base include BatchDestroyDependentAssociations include FeatureGate include OptionallySearch + include FromUnion extend Gitlab::Cache::RequestCache extend Gitlab::ConfigHelper @@ -1493,8 +1494,7 @@ class Project < ActiveRecord::Base end def all_runners - union = Gitlab::SQL::Union.new([runners, group_runners, shared_runners]) - Ci::Runner.from("(#{union.to_sql}) ci_runners") + Ci::Runner.from_union([runners, group_runners, shared_runners]) end def active_runners @@ -2022,12 +2022,10 @@ class Project < ActiveRecord::Base def badges return project_badges unless group - group_badges_rel = GroupBadge.where(group: group.self_and_ancestors) - - union = Gitlab::SQL::Union.new([project_badges.select(:id), - group_badges_rel.select(:id)]) - - Badge.where("id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + Badge.from_union([ + project_badges, + GroupBadge.where(group: group.self_and_ancestors) + ]) end def merge_requests_allowing_push_to_user(user) diff --git a/app/models/project_authorization.rb b/app/models/project_authorization.rb index 746bb4584c9..2c590008db2 100644 --- a/app/models/project_authorization.rb +++ b/app/models/project_authorization.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ProjectAuthorization < ActiveRecord::Base + include FromUnion + belongs_to :user belongs_to :project @@ -8,9 +10,9 @@ class ProjectAuthorization < ActiveRecord::Base validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true validates :user, uniqueness: { scope: [:project, :access_level] }, presence: true - def self.select_from_union(union) - select(['project_id', 'MAX(access_level) AS access_level']) - .from("(#{union.to_sql}) #{ProjectAuthorization.table_name}") + def self.select_from_union(relations) + from_union(relations) + .select(['project_id', 'MAX(access_level) AS access_level']) .group(:project_id) end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 5b394e3fa79..e9533ee7c77 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -12,6 +12,7 @@ class Snippet < ActiveRecord::Base include Spammable include Editable include Gitlab::SQL::Pattern + include FromUnion cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description diff --git a/app/models/todo.rb b/app/models/todo.rb index 48d92ad04b3..265fb932f7c 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -2,6 +2,7 @@ class Todo < ActiveRecord::Base include Sortable + include FromUnion ASSIGNED = 1 MENTIONED = 2 diff --git a/app/models/user.rb b/app/models/user.rb index d68108a8e8e..eeac87e2e52 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -20,6 +20,7 @@ class User < ActiveRecord::Base include BlocksJsonSerialization include WithUploads include OptionallySearch + include FromUnion DEFAULT_NOTIFICATION_LEVEL = :participating @@ -286,11 +287,9 @@ class User < ActiveRecord::Base # user_id - The ID of the user to include. def self.union_with_user(user_id = nil) if user_id.present? - union = Gitlab::SQL::Union.new([all, User.unscoped.where(id: user_id)]) - # We use "unscoped" here so that any inner conditions are not repeated for # the outer query, which would be redundant. - User.unscoped.from("(#{union.to_sql}) #{User.table_name}") + User.unscoped.from_union([all, User.unscoped.where(id: user_id)]) else all end @@ -354,9 +353,8 @@ class User < ActiveRecord::Base emails = joins(:emails).where(emails: { email: email }) emails = emails.confirmed if confirmed - union = Gitlab::SQL::Union.new([users, emails]) - from("(#{union.to_sql}) #{table_name}") + from_union([users, emails]) end def filter(filter_name) @@ -635,7 +633,7 @@ class User < ActiveRecord::Base # possibility of the commit_email column not existing. def commit_email - return unless has_attribute?(:commit_email) + return self.email unless has_attribute?(:commit_email) # The commit email is the same as the primary email if undefined super.presence || self.email @@ -676,10 +674,10 @@ class User < ActiveRecord::Base # Returns the groups a user has access to, either through a membership or a project authorization def authorized_groups - union = Gitlab::SQL::Union - .new([groups.select(:id), authorized_projects.select(:namespace_id)]) - - Group.where("namespaces.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + Group.from_union([ + groups, + authorized_projects.joins(:namespace).select('namespaces.*') + ]) end # Returns the groups a user is a member of, either directly or through a parent group @@ -744,7 +742,15 @@ class User < ActiveRecord::Base end def owned_projects - @owned_projects ||= Project.from("(#{owned_projects_union.to_sql}) AS projects") + @owned_projects ||= Project.from_union( + [ + Project.where(namespace: namespace), + Project.joins(:project_authorizations) + .where("projects.namespace_id <> ?", namespace.id) + .where(project_authorizations: { user_id: id, access_level: Gitlab::Access::OWNER }) + ], + remove_duplicates: false + ) end # Returns projects which user can admin issues on (for example to move an issue to that project). @@ -1138,17 +1144,17 @@ class User < ActiveRecord::Base def ci_owned_runners @ci_owned_runners ||= begin - project_runner_ids = Ci::RunnerProject + project_runners = Ci::RunnerProject .where(project: authorized_projects(Gitlab::Access::MAINTAINER)) - .select(:runner_id) + .joins(:runner) + .select('ci_runners.*') - group_runner_ids = Ci::RunnerNamespace + group_runners = Ci::RunnerNamespace .where(namespace_id: owned_or_maintainers_groups.select(:id)) - .select(:runner_id) + .joins(:runner) + .select('ci_runners.*') - union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids]) - - Ci::Runner.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection + Ci::Runner.from_union([project_runners, group_runners]) end end @@ -1176,13 +1182,13 @@ class User < ActiveRecord::Base def assigned_open_merge_requests_count(force: false) Rails.cache.fetch(['users', id, 'assigned_open_merge_requests_count'], force: force, expires_in: 20.minutes) do - MergeRequestsFinder.new(self, assignee_id: self.id, state: 'opened').execute.count + MergeRequestsFinder.new(self, assignee_id: self.id, state: 'opened', non_archived: true).execute.count end end def assigned_open_issues_count(force: false) Rails.cache.fetch(['users', id, 'assigned_open_issues_count'], force: force, expires_in: 20.minutes) do - IssuesFinder.new(self, assignee_id: self.id, state: 'opened').execute.count + IssuesFinder.new(self, assignee_id: self.id, state: 'opened', non_archived: true).execute.count end end @@ -1380,15 +1386,6 @@ class User < ActiveRecord::Base Gitlab::CurrentSettings.usage_stats_set_by_user_id == self.id end - def owned_projects_union - Gitlab::SQL::Union.new([ - Project.where(namespace: namespace), - Project.joins(:project_authorizations) - .where("projects.namespace_id <> ?", namespace.id) - .where(project_authorizations: { user_id: id, access_level: Gitlab::Access::OWNER }) - ], remove_duplicates: false) - end - # Added according to https://github.com/plataformatec/devise/blob/7df57d5081f9884849ca15e4fde179ef164a575f/README.md#activejob-integration def send_devise_notification(notification, *args) return true unless can?(:receive_notifications) diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index ab9861c58c4..00a441a9a1e 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -79,6 +79,20 @@ class BuildDetailsEntity < JobEntity expose :trigger_variables, as: :variables, using: TriggerVariableEntity end + expose :runners do + expose :online do |build| + build.any_runners_online? + end + + expose :available do |build| + project.any_runners? + end + + expose :settings_path, if: -> (*) { can_admin_build? } do |build| + project_runners_path(project) + end + end + private def build_failed_issue_options @@ -97,4 +111,8 @@ class BuildDetailsEntity < JobEntity def can_create_build_terminal? can?(current_user, :create_build_terminal, build) && build.has_terminal? end + + def can_admin_build? + can?(request.current_user, :admin_build, project) + end end diff --git a/app/serializers/runner_entity.rb b/app/serializers/runner_entity.rb index 04ec80e0efa..97e5b336a35 100644 --- a/app/serializers/runner_entity.rb +++ b/app/serializers/runner_entity.rb @@ -5,8 +5,7 @@ class RunnerEntity < Grape::Entity expose :id, :description - expose :edit_path, - if: -> (*) { can?(request.current_user, :admin_build, project) && runner.project_type? } do |runner| + expose :edit_path, if: -> (*) { can_edit_runner? } do |runner| edit_project_runner_path(project, runner) end @@ -17,4 +16,8 @@ class RunnerEntity < Grape::Entity def project request.project end + + def can_edit_runner? + can?(request.current_user, :update_runner, runner) && runner.project_type? + end end diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 4e352f2dc63..0b69661bbd0 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -56,6 +56,7 @@ module Boards set_parent set_state set_scope + set_non_archived params end @@ -76,6 +77,10 @@ module Boards params[:include_subgroups] = board.group_board? end + def set_non_archived + params[:non_archived] = parent.is_a?(Group) + end + # rubocop: disable CodeReuse/ActiveRecord def board_label_ids @board_label_ids ||= board.lists.movable.pluck(:label_id) diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb index aec0282b31b..52360f775dc 100644 --- a/app/services/labels/transfer_service.rb +++ b/app/services/labels/transfer_service.rb @@ -34,13 +34,13 @@ module Labels # rubocop: disable CodeReuse/ActiveRecord def labels_to_transfer - label_ids = [] - label_ids << group_labels_applied_to_issues.select(:id) - label_ids << group_labels_applied_to_merge_requests.select(:id) - - union = Gitlab::SQL::Union.new(label_ids) - - Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq # rubocop:disable GitlabSecurity/SqlInjection + Label + .from_union([ + group_labels_applied_to_issues, + group_labels_applied_to_merge_requests + ]) + .reorder(nil) + .uniq end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb index faaf1283078..216acf79cbd 100644 --- a/app/validators/url_validator.rb +++ b/app/validators/url_validator.rb @@ -41,12 +41,13 @@ class UrlValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) @record = record - if value.present? - value.strip! - else + unless value.present? record.errors.add(attribute, 'must be a valid URL') + return end + value = strip_value!(record, attribute, value) + Gitlab::UrlBlocker.validate!(value, blocker_args) rescue Gitlab::UrlBlocker::BlockedUrlError => e record.errors.add(attribute, "is blocked: #{e.message}") @@ -54,6 +55,13 @@ class UrlValidator < ActiveModel::EachValidator private + def strip_value!(record, attribute, value) + new_value = value.strip + return value if new_value == value + + record.public_send("#{attribute}=", new_value) # rubocop:disable GitlabSecurity/PublicSend + end + def default_options # By default the validator doesn't block any url based on the ip address { diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 30e0e9fca27..7239780871c 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -158,7 +158,7 @@ - if project_nav_tab? :pipelines = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :artifacts]) do - = link_to project_pipelines_path(@project), class: 'shortcuts-pipelines' do + = link_to project_pipelines_path(@project), class: 'shortcuts-pipelines qa-link-pipelines' do .nav-icon-container = sprite_icon('rocket') %span.nav-item-name diff --git a/app/views/profiles/keys/_form.html.haml b/app/views/profiles/keys/_form.html.haml index 5207921d6fe..700c22e1652 100644 --- a/app/views/profiles/keys/_form.html.haml +++ b/app/views/profiles/keys/_form.html.haml @@ -5,10 +5,10 @@ .form-group = f.label :key, class: 'label-bold' %p= _("Paste your public SSH key, which is usually contained in the file '~/.ssh/id_rsa.pub' and begins with 'ssh-rsa'. Don't use your private SSH key.") - = f.text_area :key, class: "form-control js-add-ssh-key-validation-input", rows: 8, required: true, placeholder: s_('Profiles|Typically starts with "ssh-rsa …"') + = f.text_area :key, class: "form-control js-add-ssh-key-validation-input qa-key-public-key-field", rows: 8, required: true, placeholder: s_('Profiles|Typically starts with "ssh-rsa …"') .form-group = f.label :title, class: 'label-bold' - = f.text_field :title, class: "form-control input-lg", required: true, placeholder: s_('Profiles|e.g. My MacBook key') + = f.text_field :title, class: "form-control input-lg qa-key-title-field", required: true, placeholder: s_('Profiles|e.g. My MacBook key') %p.form-text.text-muted= _('Name your individual key via a title') .js-add-ssh-key-validation-warning.hide @@ -19,4 +19,4 @@ %button.btn.btn-create.js-add-ssh-key-validation-confirm-submit= _("Yes, add it") .prepend-top-default - = f.submit s_('Profiles|Add key'), class: "btn btn-create js-add-ssh-key-validation-original-submit" + = f.submit s_('Profiles|Add key'), class: "btn btn-create js-add-ssh-key-validation-original-submit qa-add-key-button" diff --git a/app/views/profiles/keys/_key_details.html.haml b/app/views/profiles/keys/_key_details.html.haml index 2ac514d3f6f..88473c7f72d 100644 --- a/app/views/profiles/keys/_key_details.html.haml +++ b/app/views/profiles/keys/_key_details.html.haml @@ -24,4 +24,4 @@ = @key.key .col-md-12 .float-right - = link_to 'Remove', path_to_key(@key, is_admin), data: {confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove delete-key" + = link_to 'Remove', path_to_key(@key, is_admin), data: {confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove delete-key qa-delete-key-button" diff --git a/app/views/profiles/two_factor_auths/_codes.html.haml b/app/views/profiles/two_factor_auths/_codes.html.haml index 93722d7b034..fb4fff12027 100644 --- a/app/views/profiles/two_factor_auths/_codes.html.haml +++ b/app/views/profiles/two_factor_auths/_codes.html.haml @@ -10,4 +10,6 @@ %li %span.monospace= code -= link_to 'Proceed', profile_account_path, class: 'btn btn-success' +.d-flex + = link_to 'Proceed', profile_account_path, class: 'btn btn-success append-right-10' + = link_to 'Download codes', "data:text/plain;charset=utf-8,#{CGI.escape(@codes.join("\n"))}", download: "gitlab-recovery-codes.txt", class: 'btn btn-default' diff --git a/app/views/projects/environments/terminal.html.haml b/app/views/projects/environments/terminal.html.haml index 5b680189bc8..e40d631a1a1 100644 --- a/app/views/projects/environments/terminal.html.haml +++ b/app/views/projects/environments/terminal.html.haml @@ -2,7 +2,7 @@ - page_title "Terminal for environment", @environment.name - content_for :page_specific_javascripts do - = stylesheet_link_tag "xterm/xterm" + = stylesheet_link_tag "xterm.css" %div{ class: container_class } .top-area diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml index 078f40c4477..cf8d42976f8 100644 --- a/app/views/projects/jobs/show.html.haml +++ b/app/views/projects/jobs/show.html.haml @@ -10,7 +10,7 @@ - unless @build.any_runners_online? .bs-callout.bs-callout-warning.js-build-stuck %p - - if no_runners_for_project?(@build.project) + - if @project.any_runners? This job is stuck, because the project doesn't have any runners online assigned to it. - elsif @build.tags.any? This job is stuck, because you don't have any active runners online with any of these tags assigned to them: diff --git a/changelogs/unreleased/45754-issue-mr-and-archived-projects.yml b/changelogs/unreleased/45754-issue-mr-and-archived-projects.yml new file mode 100644 index 00000000000..d81f47d9654 --- /dev/null +++ b/changelogs/unreleased/45754-issue-mr-and-archived-projects.yml @@ -0,0 +1,5 @@ +--- +title: Issue and MR count now ignores archived projects +merge_request: 21721 +author: +type: fixed diff --git a/changelogs/unreleased/45754-open-issues-from-archived-project-listed-in-group-issue-board.yml b/changelogs/unreleased/45754-open-issues-from-archived-project-listed-in-group-issue-board.yml new file mode 100644 index 00000000000..34394396020 --- /dev/null +++ b/changelogs/unreleased/45754-open-issues-from-archived-project-listed-in-group-issue-board.yml @@ -0,0 +1,5 @@ +--- +title: No longer show open issues from archived projects in group issue board +merge_request: 21721 +author: +type: fixed diff --git a/changelogs/unreleased/50956-web-terminal.yml b/changelogs/unreleased/50956-web-terminal.yml new file mode 100644 index 00000000000..73317b0224c --- /dev/null +++ b/changelogs/unreleased/50956-web-terminal.yml @@ -0,0 +1,5 @@ +--- +title: Include correct CSS file for xterm in environments page +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/51273-expose-runners-for-build-in-job-api.yml b/changelogs/unreleased/51273-expose-runners-for-build-in-job-api.yml new file mode 100644 index 00000000000..df43f1dfbae --- /dev/null +++ b/changelogs/unreleased/51273-expose-runners-for-build-in-job-api.yml @@ -0,0 +1,5 @@ +--- +title: Expose project runners in job API +merge_request: 21618 +author: +type: other diff --git a/changelogs/unreleased/add-2fa-button.yml b/changelogs/unreleased/add-2fa-button.yml new file mode 100644 index 00000000000..6cb71d52781 --- /dev/null +++ b/changelogs/unreleased/add-2fa-button.yml @@ -0,0 +1,5 @@ +--- +title: Add button to download 2FA codes +merge_request: +author: Luke Picciau +type: added diff --git a/changelogs/unreleased/osw-gitaly-diff-stats-client.yml b/changelogs/unreleased/osw-gitaly-diff-stats-client.yml new file mode 100644 index 00000000000..9f280162409 --- /dev/null +++ b/changelogs/unreleased/osw-gitaly-diff-stats-client.yml @@ -0,0 +1,5 @@ +--- +title: Add Gitaly diff stats RPC client +merge_request: 21732 +author: +type: changed diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 41de9a50efc..31a065bc196 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1231,13 +1231,16 @@ rspec: ``` The collected JUnit reports will be uploaded to GitLab as an artifact and will -be automatically [shown in merge requests](../junit_test_reports.md). +be automatically shown in merge requests. + +For more examples, see [JUnit test reports](../junit_test_reports.md). NOTE: **Note:** In case the JUnit tool you use exports to multiple XML files, you can specify -multiple test report paths within a single job -(`junit: [rspec-1.xml, rspec-2.xml, rspec-3.xml]`) and they will be automatically -concatenated into a single file. +multiple test report paths within a single job and they will be automatically +concatenated into a single file. Use a filename pattern (`junit: rspec-*.xml`), +an array of filenames (`junit: [rspec-1.xml, rspec-2.xml, rspec-3.xml]`), or a +combination thereof (`junit: [rspec.xml, test-results/TEST-*.xml]`). ## `dependencies` diff --git a/doc/install/requirements.md b/doc/install/requirements.md index 5531dcde4e9..df59ce9837c 100644 --- a/doc/install/requirements.md +++ b/doc/install/requirements.md @@ -27,7 +27,7 @@ Please see the [installation from source guide](installation.md) and the [instal ### Non-Unix operating systems such as Windows -GitLab is developed for Unix operating systems. +GitLab is developed for Unix operating systems. It does **not** run on Windows, and we have no plans to support it in the near future. For the latest development status view this [issue](https://gitlab.com/gitlab-org/gitlab-ce/issues/46567). Please consider using a virtual machine to run GitLab. @@ -103,19 +103,21 @@ features of GitLab work with MySQL/MariaDB: 1. MySQL support for subgroups was [dropped with GitLab 9.3][post]. See [issue #30472][30472] for more information. -1. GitLab Geo does [not support MySQL](https://docs.gitlab.com/ee/gitlab-geo/database.html#mysql-replication). -1. [Zero downtime migrations][zero] do not work with MySQL +1. Geo **[STARTER ONLY]** does [not support MySQL](https://docs.gitlab.com/ee/gitlab-geo/database.html#mysql-replication). This means no supported Disaster Recovery solution if using MySQL. +1. [Zero downtime migrations][../update/README.md#upgrading-without-downtime] do not work with MySQL. 1. GitLab [optimizes the loading of dashboard events](https://gitlab.com/gitlab-org/gitlab-ce/issues/31806) using [PostgreSQL LATERAL JOINs](https://blog.heapanalytics.com/postgresqls-powerful-new-join-type-lateral/). 1. In general, SQL optimized for PostgreSQL may run much slower in MySQL due to differences in query planners. For example, subqueries that work well in PostgreSQL - may not be [performant in MySQL](https://dev.mysql.com/doc/refman/5.7/en/optimizing-subqueries.html) + may not be [performant in MySQL](https://dev.mysql.com/doc/refman/5.7/en/optimizing-subqueries.html). +1. Binary column index length is limited to 20 bytes. This is accomplished with [a hack](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/initializers/mysql_set_length_for_binary_indexes.rb). +1. MySQL requires a variety of hacks to increase limits on various columns, [for example](https://gitlab.com/gitlab-org/gitlab-ce/issues/49583). +1. [The milestone filter runs slower queries on MySQL](https://gitlab.com/gitlab-org/gitlab-ce/issues/51173#note_99391731). 1. We expect this list to grow over time. Existing users using GitLab with MySQL/MariaDB are advised to [migrate to PostgreSQL](../update/mysql_to_postgresql.md) instead. [30472]: https://gitlab.com/gitlab-org/gitlab-ce/issues/30472 -[zero]: ../update/README.md#upgrading-without-downtime [post]: https://about.gitlab.com/2017/06/22/gitlab-9-3-released/#dropping-support-for-subgroups-in-mysql ### PostgreSQL Requirements diff --git a/lib/gitlab/contributions_calendar.rb b/lib/gitlab/contributions_calendar.rb index 3236abfa43f..1ffc2639237 100644 --- a/lib/gitlab/contributions_calendar.rb +++ b/lib/gitlab/contributions_calendar.rb @@ -30,8 +30,9 @@ module Gitlab note_events = event_counts(date_from, :merge_requests) .having(action: [Event::COMMENTED]) - union = Gitlab::SQL::Union.new([repo_events, issue_events, mr_events, note_events]) - events = Event.find_by_sql(union.to_sql).map(&:attributes) + events = Event + .from_union([repo_events, issue_events, mr_events, note_events]) + .map(&:attributes) @activity_dates = events.each_with_object(Hash.new {|h, k| h[k] = 0 }) do |event, activities| activities[event["date"]] += event["total_amount"] diff --git a/lib/gitlab/database/grant.rb b/lib/gitlab/database/grant.rb index d32837f5793..7d334a79009 100644 --- a/lib/gitlab/database/grant.rb +++ b/lib/gitlab/database/grant.rb @@ -2,6 +2,8 @@ module Gitlab module Database # Model that can be used for querying permissions of a SQL user. class Grant < ActiveRecord::Base + include FromUnion + self.table_name = if Database.postgresql? 'information_schema.role_table_grants' @@ -42,9 +44,7 @@ module Gitlab .where("GRANTEE = CONCAT('\\'', REPLACE(CURRENT_USER(), '@', '\\'@\\''), '\\'')") ] - union = SQL::Union.new(queries).to_sql - - Grant.from("(#{union}) privs").any? + Grant.from_union(queries, alias_as: 'privs').any? end end end diff --git a/lib/gitlab/git/diff_stats_collection.rb b/lib/gitlab/git/diff_stats_collection.rb new file mode 100644 index 00000000000..84d9e46f98e --- /dev/null +++ b/lib/gitlab/git/diff_stats_collection.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Gitlab + module Git + class DiffStatsCollection + include Enumerable + + def initialize(diff_stats) + @collection = diff_stats + end + + def each(&block) + @collection.each(&block) + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 61786f1d896..fa22294ac51 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -438,6 +438,16 @@ module Gitlab Gitlab::Git::DiffCollection.new(iterator, options) end + def diff_stats(left_id, right_id) + stats = wrapped_gitaly_errors do + gitaly_commit_client.diff_stats(left_id, right_id) + end + + Gitlab::Git::DiffStatsCollection.new(stats) + rescue CommandError + Gitlab::Git::DiffStatsCollection.new([]) + end + # Returns a RefName for a given SHA def ref_name_for_sha(ref_path, sha) raise ArgumentError, "sha can't be empty" unless sha.present? diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index f65d7383dc7..6c95abdcb4b 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -172,6 +172,17 @@ module Gitlab consume_commits_response(response) end + def diff_stats(left_commit_sha, right_commit_sha) + request = Gitaly::DiffStatsRequest.new( + repository: @gitaly_repo, + left_commit_id: left_commit_sha, + right_commit_id: right_commit_sha + ) + + response = GitalyClient.call(@repository.storage, :diff_service, :diff_stats, request, timeout: GitalyClient.medium_timeout) + response.flat_map(&:stats) + end + def find_all_commits(opts = {}) request = Gitaly::FindAllCommitsRequest.new( repository: @gitaly_repo, diff --git a/lib/gitlab/group_hierarchy.rb b/lib/gitlab/group_hierarchy.rb index b74e7b10448..8fbfa1a86bf 100644 --- a/lib/gitlab/group_hierarchy.rb +++ b/lib/gitlab/group_hierarchy.rb @@ -89,14 +89,14 @@ module Gitlab ancestors_table = ancestors.alias_to(groups_table) descendants_table = descendants.alias_to(groups_table) - union = SQL::Union.new([model.unscoped.from(ancestors_table), - model.unscoped.from(descendants_table)]) - relation = model .unscoped .with .recursive(ancestors.to_arel, descendants.to_arel) - .from("(#{union.to_sql}) #{model.table_name}") + .from_union([ + model.unscoped.from(ancestors_table), + model.unscoped.from(descendants_table) + ]) read_only(relation) end diff --git a/lib/gitlab/project_authorizations/with_nested_groups.rb b/lib/gitlab/project_authorizations/with_nested_groups.rb index e3da1634fa5..448c3f3a7d8 100644 --- a/lib/gitlab/project_authorizations/with_nested_groups.rb +++ b/lib/gitlab/project_authorizations/with_nested_groups.rb @@ -49,13 +49,11 @@ module Gitlab .where('p_ns.share_with_group_lock IS FALSE') ] - union = Gitlab::SQL::Union.new(relations) - ProjectAuthorization .unscoped .with .recursive(cte.to_arel) - .select_from_union(union) + .select_from_union(relations) end private diff --git a/lib/gitlab/project_authorizations/without_nested_groups.rb b/lib/gitlab/project_authorizations/without_nested_groups.rb index 7d0c00c7f36..ed2287dcc7e 100644 --- a/lib/gitlab/project_authorizations/without_nested_groups.rb +++ b/lib/gitlab/project_authorizations/without_nested_groups.rb @@ -24,11 +24,9 @@ module Gitlab user.groups.joins(:shared_projects).select_for_project_authorization ] - union = Gitlab::SQL::Union.new(relations) - ProjectAuthorization .unscoped - .select_from_union(union) + .select_from_union(relations) end end end diff --git a/lib/gitlab/user_extractor.rb b/lib/gitlab/user_extractor.rb index 09652a4fb5e..bd0d24e4369 100644 --- a/lib/gitlab/user_extractor.rb +++ b/lib/gitlab/user_extractor.rb @@ -18,7 +18,7 @@ module Gitlab def users return User.none unless @text.present? - @users ||= User.from("(#{union.to_sql}) users") + @users ||= User.from_union(union_relations) end # rubocop: enable CodeReuse/ActiveRecord @@ -43,13 +43,13 @@ module Gitlab private - def union + def union_relations relations = [] relations << User.by_any_email(emails) if emails.any? relations << User.by_username(usernames) if usernames.any? - Gitlab::SQL::Union.new(relations) + relations end end end @@ -57,6 +57,7 @@ module QA autoload :Wiki, 'qa/factory/resource/wiki' autoload :File, 'qa/factory/resource/file' autoload :Fork, 'qa/factory/resource/fork' + autoload :SSHKey, 'qa/factory/resource/ssh_key' end module Repository @@ -217,6 +218,7 @@ module QA module Profile autoload :PersonalAccessTokens, 'qa/page/profile/personal_access_tokens' + autoload :SSHKeys, 'qa/page/profile/ssh_keys' end module Issuable diff --git a/qa/qa/factory/repository/project_push.rb b/qa/qa/factory/repository/project_push.rb index 4f78098d348..167f47c9141 100644 --- a/qa/qa/factory/repository/project_push.rb +++ b/qa/qa/factory/repository/project_push.rb @@ -11,7 +11,9 @@ module QA factory.output end - product(:project) { |factory| factory.project } + product :project do |factory| + factory.project + end def initialize @file_name = 'file.txt' @@ -21,8 +23,8 @@ module QA @new_branch = true end - def repository_uri - @repository_uri ||= begin + def repository_http_uri + @repository_http_uri ||= begin project.visit! Page::Project::Show.act do choose_repository_clone_http @@ -30,6 +32,16 @@ module QA end end end + + def repository_ssh_uri + @repository_ssh_uri ||= begin + project.visit! + Page::Project::Show.act do + choose_repository_clone_ssh + repository_location.uri + end + end + end end end end diff --git a/qa/qa/factory/repository/push.rb b/qa/qa/factory/repository/push.rb index 5b7ebf6c41f..6c5088f1da5 100644 --- a/qa/qa/factory/repository/push.rb +++ b/qa/qa/factory/repository/push.rb @@ -5,8 +5,8 @@ module QA module Repository class Push < Factory::Base attr_accessor :file_name, :file_content, :commit_message, - :branch_name, :new_branch, :output, :repository_uri, - :user + :branch_name, :new_branch, :output, :repository_http_uri, + :repository_ssh_uri, :ssh_key, :user attr_writer :remote_branch @@ -16,7 +16,8 @@ module QA @commit_message = "This is a test commit" @branch_name = 'master' @new_branch = true - @repository_uri = "" + @repository_http_uri = "" + @ssh_key = nil end def remote_branch @@ -31,9 +32,14 @@ module QA def fabricate! Git::Repository.perform do |repository| - repository.uri = repository_uri + if ssh_key + repository.uri = repository_ssh_uri + repository.use_ssh_key(ssh_key) + else + repository.uri = repository_http_uri + repository.use_default_credentials + end - repository.use_default_credentials username = 'GitLab QA' email = 'root@gitlab.com' @@ -63,6 +69,8 @@ module QA repository.commit(commit_message) @output = repository.push_changes("#{branch_name}:#{remote_branch}") + + repository.delete_ssh_key end end end diff --git a/qa/qa/factory/repository/wiki_push.rb b/qa/qa/factory/repository/wiki_push.rb index fb7c2bb660d..ecc6cc18c88 100644 --- a/qa/qa/factory/repository/wiki_push.rb +++ b/qa/qa/factory/repository/wiki_push.rb @@ -16,8 +16,8 @@ module QA @new_branch = false end - def repository_uri - @repository_uri ||= begin + def repository_http_uri + @repository_http_uri ||= begin wiki.visit! Page::Project::Wiki::Show.act do go_to_clone_repository diff --git a/qa/qa/factory/resource/project.rb b/qa/qa/factory/resource/project.rb index 7fff22b5468..90db26ab3ab 100644 --- a/qa/qa/factory/resource/project.rb +++ b/qa/qa/factory/resource/project.rb @@ -20,6 +20,13 @@ module QA end end + product :repository_http_location do + Page::Project::Show.act do + choose_repository_clone_http + repository_location + end + end + def initialize @description = 'My awesome project' end diff --git a/qa/qa/factory/resource/ssh_key.rb b/qa/qa/factory/resource/ssh_key.rb new file mode 100644 index 00000000000..6c872f32d16 --- /dev/null +++ b/qa/qa/factory/resource/ssh_key.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module QA + module Factory + module Resource + class SSHKey < Factory::Base + extend Forwardable + + attr_accessor :title + attr_reader :private_key, :public_key, :fingerprint + def_delegators :key, :private_key, :public_key, :fingerprint + + product :private_key do |factory| + factory.private_key + end + + product :title do |factory| + factory.title + end + + product :fingerprint do |factory| + factory.fingerprint + end + + def key + @key ||= Runtime::Key::RSA.new + end + + def fabricate! + Page::Menu::Main.act { go_to_profile_settings } + Page::Menu::Profile.act { click_ssh_keys } + + Page::Profile::SSHKeys.perform do |page| + page.add_key(public_key, title) + end + end + end + end + end +end diff --git a/qa/qa/git/repository.rb b/qa/qa/git/repository.rb index faecacd45ec..14cb8125fdb 100644 --- a/qa/qa/git/repository.rb +++ b/qa/qa/git/repository.rb @@ -7,6 +7,10 @@ module QA class Repository include Scenario::Actable + def initialize + @ssh_cmd = "" + end + def self.perform(*args) Dir.mktmpdir do |dir| Dir.chdir(dir) { super } @@ -38,7 +42,7 @@ module QA end def clone(opts = '') - run_and_redact_credentials("git clone #{opts} #{@uri} ./") + run_and_redact_credentials(build_git_command("git clone #{opts} #{@uri} ./")) end def checkout(branch_name) @@ -58,6 +62,10 @@ module QA `git config user.email #{email}` end + def configure_ssh_command(command) + @ssh_cmd = "GIT_SSH_COMMAND='#{command}'" + end + def commit_file(name, contents, message) add_file(name, contents) commit(message) @@ -74,7 +82,7 @@ module QA end def push_changes(branch = 'master') - output, _ = run_and_redact_credentials("git push #{@uri} #{branch}") + output, _ = run_and_redact_credentials(build_git_command("git push #{@uri} #{branch}")) output end @@ -83,6 +91,31 @@ module QA `git log --oneline`.split("\n") end + def use_ssh_key(key) + @private_key_file = Tempfile.new("id_#{SecureRandom.hex(8)}") + File.binwrite(@private_key_file, key.private_key) + File.chmod(0700, @private_key_file) + + @known_hosts_file = Tempfile.new("known_hosts_#{SecureRandom.hex(8)}") + keyscan_params = ['-H'] + keyscan_params << "-p #{@uri.port}" if @uri.port + keyscan_params << @uri.host + run_and_redact_credentials("ssh-keyscan #{keyscan_params.join(' ')} >> #{@known_hosts_file.path}") + + configure_ssh_command("ssh -i #{@private_key_file.path} -o UserKnownHostsFile=#{@known_hosts_file.path}") + end + + def delete_ssh_key + return unless @private_key_file + + @private_key_file.close(true) + @known_hosts_file.close(true) + end + + def build_git_command(command_str) + [@ssh_cmd, command_str].compact.join(' ') + end + private # Since the remote URL contains the credentials, and git occasionally diff --git a/qa/qa/page/main/login.rb b/qa/qa/page/main/login.rb index 08cf8da34fd..4e9506a7dc8 100644 --- a/qa/qa/page/main/login.rb +++ b/qa/qa/page/main/login.rb @@ -66,6 +66,8 @@ module QA end using_wait_time 0 do + set_initial_password_if_present + sign_in_using_gitlab_credentials(admin) end diff --git a/qa/qa/page/menu/profile.rb b/qa/qa/page/menu/profile.rb index 95e88d863e4..7e24fa85c33 100644 --- a/qa/qa/page/menu/profile.rb +++ b/qa/qa/page/menu/profile.rb @@ -6,6 +6,7 @@ module QA element :access_token_link, 'link_to profile_personal_access_tokens_path' element :access_token_title, 'Access Tokens' element :top_level_items, '.sidebar-top-level-items' + element :ssh_keys, 'SSH Keys' end def click_access_tokens @@ -14,6 +15,12 @@ module QA end end + def click_ssh_keys + within_sidebar do + click_link('SSH Keys') + end + end + private def within_sidebar diff --git a/qa/qa/page/menu/side.rb b/qa/qa/page/menu/side.rb index 354ccec2a5a..a1eedfea42e 100644 --- a/qa/qa/page/menu/side.rb +++ b/qa/qa/page/menu/side.rb @@ -6,6 +6,7 @@ module QA element :settings_item element :settings_link, 'link_to edit_project_path' element :repository_link, "title: _('Repository')" + element :link_pipelines element :pipelines_settings_link, "title: _('CI / CD')" element :operations_kubernetes_link, "title: _('Kubernetes')" element :issues_link, /link_to.*shortcuts-issues/ @@ -49,7 +50,7 @@ module QA def click_ci_cd_pipelines within_sidebar do - click_link('CI / CD') + click_element :link_pipelines end end diff --git a/qa/qa/page/profile/ssh_keys.rb b/qa/qa/page/profile/ssh_keys.rb new file mode 100644 index 00000000000..ce1813b14d0 --- /dev/null +++ b/qa/qa/page/profile/ssh_keys.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module QA + module Page + module Profile + class SSHKeys < Page::Base + view 'app/views/profiles/keys/_form.html.haml' do + element :key_title_field + element :key_public_key_field + element :add_key_button + end + + view 'app/views/profiles/keys/_key_details.html.haml' do + element :delete_key_button + end + + def add_key(public_key, title) + fill_element :key_public_key_field, public_key + fill_element :key_title_field, title + + click_element :add_key_button + end + + def remove_key(title) + click_link(title) + + accept_alert do + click_element :delete_key_button + end + end + end + end + end +end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/add_ssh_key_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/add_ssh_key_spec.rb new file mode 100644 index 00000000000..84f663c4866 --- /dev/null +++ b/qa/qa/specs/features/browser_ui/3_create/repository/add_ssh_key_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module QA + context :create do + describe 'SSH keys support' do + let(:key_title) { "key for ssh tests #{Time.now.to_f}" } + + it 'user adds and then removes an SSH key' do + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.act { sign_in_using_credentials } + + key = Factory::Resource::SSHKey.fabricate! do |resource| + resource.title = key_title + end + + expect(page).to have_content("Title: #{key_title}") + expect(page).to have_content(key.fingerprint) + + Page::Menu::Main.act { go_to_profile_settings } + Page::Menu::Profile.act { click_ssh_keys } + + Page::Profile::SSHKeys.perform do |ssh_keys| + ssh_keys.remove_key(key_title) + end + + expect(page).not_to have_content("Title: #{key_title}") + expect(page).not_to have_content(key.fingerprint) + end + end + end +end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb new file mode 100644 index 00000000000..7c989bfd8cc --- /dev/null +++ b/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module QA + context :create do + describe 'SSH key support' do + # Note: If you run this test against GDK make sure you've enabled sshd + # See: https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/run_qa_against_gdk.md + + let(:key_title) { "key for ssh tests #{Time.now.to_f}" } + + it 'user adds an ssh key and pushes code to the repository' do + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.act { sign_in_using_credentials } + + key = Factory::Resource::SSHKey.fabricate! do |resource| + resource.title = key_title + end + + Factory::Repository::ProjectPush.fabricate! do |push| + push.ssh_key = key + push.file_name = 'README.md' + push.file_content = '# Test Use SSH Key' + push.commit_message = 'Add README.md' + end + + Page::Project::Show.act { wait_for_push } + + expect(page).to have_content('README.md') + expect(page).to have_content('Test Use SSH Key') + + Page::Menu::Main.act { go_to_profile_settings } + Page::Menu::Profile.act { click_ssh_keys } + + Page::Profile::SSHKeys.perform do |ssh_keys| + ssh_keys.remove_key(key_title) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/union.rb b/rubocop/cop/gitlab/union.rb new file mode 100644 index 00000000000..09541d8af3b --- /dev/null +++ b/rubocop/cop/gitlab/union.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true +require_relative '../../spec_helpers' + +module RuboCop + module Cop + module Gitlab + # Cop that disallows the use of `Gitlab::SQL::Union`, in favour of using + # the `FromUnion` module. + class Union < RuboCop::Cop::Cop + include SpecHelpers + + MSG = 'Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly' + + def_node_matcher :raw_union?, <<~PATTERN + (send (const (const (const nil? :Gitlab) :SQL) :Union) :new ...) + PATTERN + + def on_send(node) + return unless raw_union?(node) + return if in_spec?(node) + + add_offense(node, location: :expression) + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 46bd7d3bec6..9d6cc73fc3b 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -3,6 +3,7 @@ require_relative 'cop/gitlab/module_with_instance_variables' require_relative 'cop/gitlab/predicate_memoization' require_relative 'cop/gitlab/httparty' require_relative 'cop/gitlab/finder_with_find_by' +require_relative 'cop/gitlab/union' require_relative 'cop/include_sidekiq_worker' require_relative 'cop/avoid_return_from_blocks' require_relative 'cop/avoid_break_from_strong_memoize' diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 706f801a285..c82c85970dc 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -229,6 +229,114 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(json_response['deployment_status']["environment"]).not_to be_nil end end + + context 'when user can edit runner' do + context 'that belongs to the project' do + let(:runner) { create(:ci_runner, :project, projects: [project]) } + let(:job) { create(:ci_build, :success, pipeline: pipeline, runner: runner) } + + before do + project.add_maintainer(user) + sign_in(user) + + get_show(id: job.id, format: :json) + end + + it 'user can edit runner' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runner']).to have_key('edit_path') + end + end + + context 'that belongs to group' do + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } + let(:job) { create(:ci_build, :success, pipeline: pipeline, runner: runner) } + let(:user) { create(:user, :admin) } + + before do + project.add_maintainer(user) + sign_in(user) + + get_show(id: job.id, format: :json) + end + + it 'user can not edit runner' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runner']).not_to have_key('edit_path') + end + end + + context 'that belongs to instance' do + let(:runner) { create(:ci_runner, :instance) } + let(:job) { create(:ci_build, :success, pipeline: pipeline, runner: runner) } + let(:user) { create(:user, :admin) } + + before do + project.add_maintainer(user) + sign_in(user) + + get_show(id: job.id, format: :json) + end + + it 'user can not edit runner' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runner']).not_to have_key('edit_path') + end + end + end + + context 'when no runners are available' do + let(:runner) { create(:ci_runner, :instance, active: false) } + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner: runner) } + + it 'exposes needed information' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runners']['online']).to be false + expect(json_response['runners']['available']).to be false + end + end + + context 'when no runner is online' do + let(:runner) { create(:ci_runner, :instance) } + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner: runner) } + + it 'exposes needed information' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runners']['online']).to be false + expect(json_response['runners']['available']).to be true + end + end + + context 'settings_path' do + context 'when user is developer' do + it 'settings_path is not available' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runners']).not_to have_key('settings_path') + end + end + + context 'when user is maintainer' do + let(:user) { create(:user, :admin) } + + before do + project.add_maintainer(user) + sign_in(user) + end + + it 'settings_path is available' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('job/job_details') + expect(json_response['runners']['settings_path']).to match(/runners/) + end + end + end end context 'when requesting JSON job is triggered' do diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 2ba4d4918ff..e2f9e7e9cc5 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -533,7 +533,7 @@ describe 'File blob', :js do expect(page).to have_content('This project is licensed under the MIT License.') # shows a learn more link - expect(page).to have_link('Learn more', 'http://choosealicense.com/licenses/mit/') + expect(page).to have_link('Learn more', href: 'http://choosealicense.com/licenses/mit/') end end end @@ -566,10 +566,10 @@ describe 'File blob', :js do expect(page).to have_content('This project manages its dependencies using RubyGems and defines a gem named activerecord.') # shows a link to the gem - expect(page).to have_link('activerecord', 'https://rubygems.org/gems/activerecord') + expect(page).to have_link('activerecord', href: 'https://rubygems.org/gems/activerecord') # shows a learn more link - expect(page).to have_link('Learn more', 'http://choosealicense.com/licenses/mit/') + expect(page).to have_link('Learn more', href: 'https://rubygems.org/') end end end diff --git a/spec/fixtures/api/schemas/job/job_details.json b/spec/fixtures/api/schemas/job/job_details.json index ab0ade55bbe..cd67d3e4160 100644 --- a/spec/fixtures/api/schemas/job/job_details.json +++ b/spec/fixtures/api/schemas/job/job_details.json @@ -7,6 +7,8 @@ "artifact": { "$ref": "artifact.json" }, "terminal_path": { "type": "string" }, "trigger": { "$ref": "trigger.json" }, - "deployment_status": { "$ref": "deployment_status.json" } + "deployment_status": { "$ref": "deployment_status.json" }, + "runner": { "$ref": "runner.json" }, + "runners": { "type": "runners.json" } } } diff --git a/spec/fixtures/api/schemas/job/runner.json b/spec/fixtures/api/schemas/job/runner.json new file mode 100644 index 00000000000..acfeeeeb808 --- /dev/null +++ b/spec/fixtures/api/schemas/job/runner.json @@ -0,0 +1,17 @@ +{ + "oneOf": [ + { "type": "null" }, + { + "type": "object", + "required": [ + "id", + "description" + ], + "properties": { + "id": { "type": "integer" }, + "description": { "type": "string" }, + "edit_path": { "type": "string" } + } + } + ] +} diff --git a/spec/fixtures/api/schemas/job/runners.json b/spec/fixtures/api/schemas/job/runners.json new file mode 100644 index 00000000000..bebb0c88652 --- /dev/null +++ b/spec/fixtures/api/schemas/job/runners.json @@ -0,0 +1,13 @@ +{ + "type": "object", + "required": [ + "online", + "available" + ], + "properties": { + "online": { "type": "boolean" }, + "available": { "type": "boolean" }, + "settings_path": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/javascripts/ide/components/file_row_extra_spec.js b/spec/javascripts/ide/components/file_row_extra_spec.js new file mode 100644 index 00000000000..60dabe28045 --- /dev/null +++ b/spec/javascripts/ide/components/file_row_extra_spec.js @@ -0,0 +1,159 @@ +import Vue from 'vue'; +import { createStore } from '~/ide/stores'; +import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import FileRowExtra from '~/ide/components/file_row_extra.vue'; +import { file, resetStore } from '../helpers'; + +describe('IDE extra file row component', () => { + let Component; + let vm; + let unstagedFilesCount = 0; + let stagedFilesCount = 0; + let changesCount = 0; + + beforeAll(() => { + Component = Vue.extend(FileRowExtra); + }); + + beforeEach(() => { + vm = createComponentWithStore(Component, createStore(), { + file: { + ...file('test'), + }, + mouseOver: false, + }); + + spyOnProperty(vm, 'getUnstagedFilesCountForPath').and.returnValue(() => unstagedFilesCount); + spyOnProperty(vm, 'getStagedFilesCountForPath').and.returnValue(() => stagedFilesCount); + spyOnProperty(vm, 'getChangesInFolder').and.returnValue(() => changesCount); + + vm.$mount(); + }); + + afterEach(() => { + vm.$destroy(); + resetStore(vm.$store); + + stagedFilesCount = 0; + unstagedFilesCount = 0; + changesCount = 0; + }); + + describe('folderChangesTooltip', () => { + it('returns undefined when changes count is 0', () => { + expect(vm.folderChangesTooltip).toBe(undefined); + }); + + it('returns unstaged changes text', () => { + changesCount = 1; + unstagedFilesCount = 1; + + expect(vm.folderChangesTooltip).toBe('1 unstaged change'); + }); + + it('returns staged changes text', () => { + changesCount = 1; + stagedFilesCount = 1; + + expect(vm.folderChangesTooltip).toBe('1 staged change'); + }); + + it('returns staged and unstaged changes text', () => { + changesCount = 1; + stagedFilesCount = 1; + unstagedFilesCount = 1; + + expect(vm.folderChangesTooltip).toBe('1 unstaged and 1 staged changes'); + }); + }); + + describe('show tree changes count', () => { + it('does not show for blobs', () => { + vm.file.type = 'blob'; + + expect(vm.$el.querySelector('.ide-tree-changes')).toBe(null); + }); + + it('does not show when changes count is 0', () => { + vm.file.type = 'tree'; + + expect(vm.$el.querySelector('.ide-tree-changes')).toBe(null); + }); + + it('does not show when tree is open', done => { + vm.file.type = 'tree'; + vm.file.opened = true; + changesCount = 1; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-tree-changes')).toBe(null); + + done(); + }); + }); + + it('shows for trees with changes', done => { + vm.file.type = 'tree'; + vm.file.opened = false; + changesCount = 1; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-tree-changes')).not.toBe(null); + + done(); + }); + }); + }); + + describe('changes file icon', () => { + it('hides when file is not changed', () => { + expect(vm.$el.querySelector('.ide-file-changed-icon')).toBe(null); + }); + + it('shows when file is changed', done => { + vm.file.changed = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + + done(); + }); + }); + + it('shows when file is staged', done => { + vm.file.staged = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + + done(); + }); + }); + + it('shows when file is a tempFile', done => { + vm.file.tempFile = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ide-file-changed-icon')).not.toBe(null); + + done(); + }); + }); + }); + + describe('merge request icon', () => { + it('hides when not a merge request change', () => { + expect(vm.$el.querySelector('.ic-git-merge')).toBe(null); + }); + + it('shows when a merge request change', done => { + vm.file.mrChange = true; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.ic-git-merge')).not.toBe(null); + + done(); + }); + }); + }); +}); diff --git a/spec/javascripts/ide/components/repo_file_spec.js b/spec/javascripts/ide/components/repo_file_spec.js deleted file mode 100644 index fc639a672e2..00000000000 --- a/spec/javascripts/ide/components/repo_file_spec.js +++ /dev/null @@ -1,145 +0,0 @@ -import Vue from 'vue'; -import store from '~/ide/stores'; -import repoFile from '~/ide/components/repo_file.vue'; -import router from '~/ide/ide_router'; -import { createComponentWithStore } from '../../helpers/vue_mount_component_helper'; -import { file } from '../helpers'; - -describe('RepoFile', () => { - let vm; - - function createComponent(propsData) { - const RepoFile = Vue.extend(repoFile); - - vm = createComponentWithStore(RepoFile, store, propsData); - - vm.$mount(); - } - - afterEach(() => { - vm.$destroy(); - }); - - it('renders link, icon and name', () => { - createComponent({ - file: file('t4'), - level: 0, - }); - - const name = vm.$el.querySelector('.ide-file-name'); - - expect(name.href).toMatch(''); - expect(name.textContent.trim()).toEqual(vm.file.name); - }); - - it('fires clickFile when the link is clicked', done => { - spyOn(router, 'push'); - createComponent({ - file: file('t3'), - level: 0, - }); - - vm.$el.querySelector('.file-name').click(); - - setTimeout(() => { - expect(router.push).toHaveBeenCalledWith(`/project${vm.file.url}`); - - done(); - }); - }); - - describe('folder', () => { - it('renders changes count inside folder', () => { - const f = { - ...file('folder'), - path: 'testing', - type: 'tree', - branchId: 'master', - projectId: 'project', - }; - - store.state.changedFiles.push({ - ...file('fileName'), - path: 'testing/fileName', - }); - - createComponent({ - file: f, - level: 0, - }); - - const treeChangesEl = vm.$el.querySelector('.ide-tree-changes'); - - expect(treeChangesEl).not.toBeNull(); - expect(treeChangesEl.textContent).toContain('1'); - }); - - it('renders action dropdown', done => { - createComponent({ - file: { - ...file('t4'), - type: 'tree', - branchId: 'master', - projectId: 'project', - }, - level: 0, - }); - - setTimeout(() => { - expect(vm.$el.querySelector('.ide-new-btn')).not.toBeNull(); - - done(); - }); - }); - }); - - describe('locked file', () => { - let f; - - beforeEach(() => { - f = file('locked file'); - f.file_lock = { - user: { - name: 'testuser', - updated_at: new Date(), - }, - }; - - createComponent({ - file: f, - level: 0, - }); - }); - - it('renders lock icon', () => { - expect(vm.$el.querySelector('.file-status-icon')).not.toBeNull(); - }); - - it('renders a tooltip', () => { - expect( - vm.$el.querySelector('.ide-file-name span:nth-child(2)').dataset.originalTitle, - ).toContain('Locked by testuser'); - }); - }); - - it('calls scrollIntoView if made active', done => { - createComponent({ - file: { - ...file(), - type: 'blob', - active: false, - }, - level: 0, - }); - - spyOn(vm, 'scrollIntoView'); - - vm.file.active = true; - - vm.$nextTick(() => { - expect(vm.scrollIntoView).toHaveBeenCalled(); - - done(); - }); - }); -}); diff --git a/spec/javascripts/vue_shared/components/file_row_spec.js b/spec/javascripts/vue_shared/components/file_row_spec.js new file mode 100644 index 00000000000..9914c0b70f3 --- /dev/null +++ b/spec/javascripts/vue_shared/components/file_row_spec.js @@ -0,0 +1,74 @@ +import Vue from 'vue'; +import FileRow from '~/vue_shared/components/file_row.vue'; +import { file } from 'spec/ide/helpers'; +import mountComponent from '../../helpers/vue_mount_component_helper'; + +describe('RepoFile', () => { + let vm; + + function createComponent(propsData) { + const FileRowComponent = Vue.extend(FileRow); + + vm = mountComponent(FileRowComponent, propsData); + } + + afterEach(() => { + vm.$destroy(); + }); + + it('renders name', () => { + createComponent({ + file: file('t4'), + level: 0, + }); + + const name = vm.$el.querySelector('.file-row-name'); + + expect(name.textContent.trim()).toEqual(vm.file.name); + }); + + it('emits toggleTreeOpen on click', () => { + createComponent({ + file: { + ...file('t3'), + type: 'tree', + }, + level: 0, + }); + spyOn(vm, '$emit').and.stub(); + + vm.$el.querySelector('.file-row').click(); + + expect(vm.$emit).toHaveBeenCalledWith('toggleTreeOpen', vm.file.path); + }); + + it('calls scrollIntoView if made active', done => { + createComponent({ + file: { + ...file(), + type: 'blob', + active: false, + }, + level: 0, + }); + + spyOn(vm, 'scrollIntoView').and.stub(); + + vm.file.active = true; + + vm.$nextTick(() => { + expect(vm.scrollIntoView).toHaveBeenCalled(); + + done(); + }); + }); + + it('indents row based on level', () => { + createComponent({ + file: file('t4'), + level: 2, + }); + + expect(vm.$el.querySelector('.file-row-name').style.marginLeft).toBe('32px'); + }); +}); diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index ce4caf0c27d..58c260ee1f0 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1112,6 +1112,32 @@ describe Gitlab::Git::Repository, :seed_helper do end end + describe '#diff_stats' do + let(:left_commit_id) { 'feature' } + let(:right_commit_id) { 'master' } + + it 'returns a DiffStatsCollection' do + collection = repository.diff_stats(left_commit_id, right_commit_id) + + expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) + expect(collection).to be_a(Enumerable) + end + + it 'yields Gitaly::DiffStats objects' do + collection = repository.diff_stats(left_commit_id, right_commit_id) + + expect(collection.to_a).to all(be_a(Gitaly::DiffStats)) + end + + it 'returns no Gitaly::DiffStats when SHAs are invalid' do + collection = repository.diff_stats('foo', 'bar') + + expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) + expect(collection).to be_a(Enumerable) + expect(collection.to_a).to be_empty + end + end + describe "#ls_files" do let(:master_file_paths) { repository.ls_files("master") } let(:utf8_file_paths) { repository.ls_files("ls-files-utf8") } diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index bcdf12a00a0..d7bd757149d 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -118,6 +118,22 @@ describe Gitlab::GitalyClient::CommitService do end end + describe '#diff_stats' do + let(:left_commit_id) { 'master' } + let(:right_commit_id) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } + + it 'sends an RPC request' do + request = Gitaly::DiffStatsRequest.new(repository: repository_message, + left_commit_id: left_commit_id, + right_commit_id: right_commit_id) + + expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:diff_stats) + .with(request, kind_of(Hash)).and_return([]) + + described_class.new(repository).diff_stats(left_commit_id, right_commit_id) + end + end + describe '#tree_entries' do let(:path) { '/' } diff --git a/spec/models/concerns/from_union_spec.rb b/spec/models/concerns/from_union_spec.rb new file mode 100644 index 00000000000..ee427a667c6 --- /dev/null +++ b/spec/models/concerns/from_union_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe FromUnion do + describe '.from_union' do + let(:model) do + Class.new(ActiveRecord::Base) do + self.table_name = 'users' + + include FromUnion + end + end + + it 'selects from the results of the UNION' do + query = model.from_union([model.where(id: 1), model.where(id: 2)]) + + expect(query.to_sql).to match(/FROM \(SELECT.+UNION.+SELECT.+\) users/m) + end + + it 'supports the use of a custom alias for the sub query' do + query = model.from_union( + [model.where(id: 1), model.where(id: 2)], + alias_as: 'kittens' + ) + + expect(query.to_sql).to match(/FROM \(SELECT.+UNION.+SELECT.+\) kittens/m) + end + + it 'supports keeping duplicate rows' do + query = model.from_union( + [model.where(id: 1), model.where(id: 2)], + remove_duplicates: false + ) + + expect(query.to_sql) + .to match(/FROM \(SELECT.+UNION ALL.+SELECT.+\) users/m) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 27aec49e348..99d17f563d9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2543,6 +2543,34 @@ describe User do end end + describe '#assigned_open_merge_requests_count' do + it 'returns number of open merge requests from non-archived projects' do + user = create(:user) + project = create(:project, :public) + archived_project = create(:project, :public, :archived) + + create(:merge_request, source_project: project, author: user, assignee: user) + create(:merge_request, :closed, source_project: project, author: user, assignee: user) + create(:merge_request, source_project: archived_project, author: user, assignee: user) + + expect(user.assigned_open_merge_requests_count(force: true)).to eq 1 + end + end + + describe '#assigned_open_issues_count' do + it 'returns number of open issues from non-archived projects' do + user = create(:user) + project = create(:project, :public) + archived_project = create(:project, :public, :archived) + + create(:issue, project: project, author: user, assignees: [user]) + create(:issue, :closed, project: project, author: user, assignees: [user]) + create(:issue, project: archived_project, author: user, assignees: [user]) + + expect(user.assigned_open_issues_count(force: true)).to eq 1 + end + end + describe '#personal_projects_count' do it 'returns the number of personal projects using a single query' do user = build(:user) diff --git a/spec/rubocop/cop/gitlab/union_spec.rb b/spec/rubocop/cop/gitlab/union_spec.rb new file mode 100644 index 00000000000..5b06f30b25f --- /dev/null +++ b/spec/rubocop/cop/gitlab/union_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/union' + +describe RuboCop::Cop::Gitlab::Union do + include CopHelper + + subject(:cop) { described_class.new } + + it 'flags the use of Gitlab::SQL::Union.new' do + expect_offense(<<~SOURCE) + Gitlab::SQL::Union.new([foo]) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly + SOURCE + end + + it 'does not flag the use of Gitlab::SQL::Union in a spec' do + allow(cop).to receive(:in_spec?).and_return(true) + + expect_no_offenses('Gitlab::SQL::Union.new([foo])') + end +end diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index 27a7bf0e605..010679b5360 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -24,7 +24,7 @@ describe Boards::Issues::ListService do let!(:opened_issue1) { create(:labeled_issue, project: project, milestone: m1, title: 'Issue 1', labels: [bug]) } let!(:opened_issue2) { create(:labeled_issue, project: project, milestone: m2, title: 'Issue 2', labels: [p2]) } - let!(:reopened_issue1) { create(:issue, :opened, project: project, title: 'Issue 3' ) } + let!(:reopened_issue1) { create(:issue, :opened, project: project, title: 'Reopened Issue 1' ) } let!(:list1_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [p2, development]) } let!(:list1_issue2) { create(:labeled_issue, project: project, milestone: m2, labels: [development]) } @@ -44,12 +44,19 @@ describe Boards::Issues::ListService do end it_behaves_like 'issues list service' + + context 'when project is archived' do + let(:project) { create(:project, :archived) } + + it_behaves_like 'issues list service' + end end context 'when parent is a group' do let(:user) { create(:user) } let(:project) { create(:project, :empty_repo, namespace: group) } let(:project1) { create(:project, :empty_repo, namespace: group) } + let(:project_archived) { create(:project, :empty_repo, :archived, namespace: group) } let(:m1) { create(:milestone, group: group) } let(:m2) { create(:milestone, group: group) } @@ -77,7 +84,8 @@ describe Boards::Issues::ListService do let!(:opened_issue1) { create(:labeled_issue, project: project, milestone: m1, title: 'Issue 1', labels: [bug]) } let!(:opened_issue2) { create(:labeled_issue, project: project, milestone: m2, title: 'Issue 2', labels: [p2, p2_project]) } - let!(:reopened_issue1) { create(:issue, state: 'opened', project: project, title: 'Issue 3', closed_at: Time.now ) } + let!(:opened_issue3) { create(:labeled_issue, project: project_archived, milestone: m1, title: 'Issue 3', labels: [bug]) } + let!(:reopened_issue1) { create(:issue, state: 'opened', project: project, title: 'Reopened Issue 1', closed_at: Time.now ) } let!(:list1_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [p2, p2_project, development]) } let!(:list1_issue2) { create(:labeled_issue, project: project, milestone: m2, labels: [development]) } diff --git a/spec/support/helpers/markdown_feature.rb b/spec/support/helpers/markdown_feature.rb index 346f5b1cc4d..96401379cf0 100644 --- a/spec/support/helpers/markdown_feature.rb +++ b/spec/support/helpers/markdown_feature.rb @@ -10,6 +10,12 @@ class MarkdownFeature include FactoryBot::Syntax::Methods + attr_reader :fixture_path + + def initialize(fixture_path = Rails.root.join('spec/fixtures/markdown.md.erb')) + @fixture_path = fixture_path + end + def user @user ||= create(:user) end @@ -122,7 +128,7 @@ class MarkdownFeature end def raw_markdown - markdown = File.read(Rails.root.join('spec/fixtures/markdown.md.erb')) + markdown = File.read(fixture_path) ERB.new(markdown).result(binding) end end diff --git a/spec/validators/url_validator_spec.rb b/spec/validators/url_validator_spec.rb index 93fe013d11c..ab6100509a6 100644 --- a/spec/validators/url_validator_spec.rb +++ b/spec/validators/url_validator_spec.rb @@ -24,6 +24,21 @@ describe UrlValidator do expect(badge.errors.empty?).to be true end + + it 'strips urls' do + badge.link_url = "\n\r\n\nhttps://127.0.0.1\r\n\r\n\n\n\n" + + # It's unusual for a validator to modify its arguments. Some extensions, + # such as attr_encrypted, freeze the string to signal that modifications + # will not be persisted, so freeze this string to ensure the scheme is + # compatible with them. + badge.link_url.freeze + + subject + + expect(badge.errors).to be_empty + expect(badge.link_url).to eq('https://127.0.0.1') + end end context 'when allow_localhost is set to false' do diff --git a/spec/views/help/index.html.haml_spec.rb b/spec/views/help/index.html.haml_spec.rb index 836d452304c..4b4de540d9e 100644 --- a/spec/views/help/index.html.haml_spec.rb +++ b/spec/views/help/index.html.haml_spec.rb @@ -21,7 +21,7 @@ describe 'help/index' do render expect(rendered).to match '8.0.2' - expect(rendered).to have_link('abcdefg', 'https://gitlab.com/gitlab-org/gitlab-ce/commits/abcdefg') + expect(rendered).to have_link('abcdefg', href: 'https://gitlab.com/gitlab-org/gitlab-ce/commits/abcdefg') end end @@ -29,7 +29,7 @@ describe 'help/index' do it 'is visible to guests' do render - expect(rendered).to have_link(nil, help_instance_configuration_url) + expect(rendered).to have_link(nil, href: help_instance_configuration_url) end end |