diff options
112 files changed, 1171 insertions, 696 deletions
diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 8d83554d813..478269f3fcf 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -143,20 +143,48 @@ } } +@mixin dropdown-item-hover { + background-color: $dropdown-item-hover-bg; + color: $gl-text-color; + outline: 0; + + // make sure the text color is not overriden + &.text-danger { + color: $brand-danger; + } + + .avatar { + border-color: $white-light; + } +} + @mixin dropdown-link { + background: transparent; + border: 0; + border-radius: 0; + box-shadow: none; display: block; + font-weight: $gl-font-weight-normal; position: relative; - padding: 5px 8px; + padding: 8px 16px; color: $gl-text-color; - line-height: initial; - border-radius: 2px; - white-space: nowrap; + line-height: normal; + white-space: normal; overflow: hidden; + text-align: left; + width: 100%; + + // make sure the text color is not overriden + &.text-danger { + color: $brand-danger; + } &:hover, + &:active, &:focus, &.is-focused { - background-color: $dropdown-link-hover-bg; + @include dropdown-item-hover; + text-decoration: none; .badge { @@ -166,6 +194,13 @@ &.dropdown-menu-user-link { line-height: 16px; + padding-top: 10px; + padding-bottom: 7px; + white-space: nowrap; + + .dropdown-menu-user-username { + display: block; + } } .icon-play { @@ -187,8 +222,8 @@ z-index: 300; min-width: 240px; max-width: 500px; - margin-top: 2px; - margin-bottom: 2px; + margin-top: $dropdown-vertical-offset; + margin-bottom: 24px; font-size: 14px; font-weight: $gl-font-weight-normal; padding: 8px 0; @@ -197,6 +232,10 @@ border-radius: $border-radius-base; box-shadow: 0 2px 4px $dropdown-shadow-color; + &.dropdown-open-top { + margin-bottom: $dropdown-vertical-offset; + } + &.dropdown-open-left { right: 0; left: auto; @@ -227,16 +266,27 @@ } li { + display: block; text-align: left; list-style: none; - padding: 0 10px; + padding: 0 1px; + + a, + button, + .menu-item { + @include dropdown-link; + } } .divider { height: 1px; - margin: 6px 10px; + margin: 6px 0; padding: 0; background-color: $dropdown-divider-color; + + &:hover { + background-color: $dropdown-divider-color; + } } .separator { @@ -247,10 +297,6 @@ background-color: $dropdown-divider-color; } - a { - @include dropdown-link; - } - .dropdown-menu-empty-item a { &:hover, &:focus { @@ -262,7 +308,7 @@ color: $gl-text-color-secondary; font-size: 13px; line-height: 22px; - padding: 0 16px; + padding: 8px 16px; } &.capitalize-header .dropdown-header { @@ -277,7 +323,7 @@ .separator + .dropdown-header, .separator + .dropdown-bold-header { - padding-top: 2px; + padding-top: 10px; } .unclickable { @@ -298,48 +344,28 @@ } .dropdown-menu li { - padding: $gl-btn-padding; cursor: pointer; + &.droplab-item-active button { + @include dropdown-item-hover; + } + > a, > button { display: flex; margin: 0; - padding: 0; - border-radius: 0; text-overflow: inherit; - background-color: inherit; - color: inherit; - border: inherit; text-align: left; - &:hover, - &:focus { - background-color: inherit; - color: inherit; - } - &.btn .fa:not(:last-child) { margin-left: 5px; } } - &:hover, - &:focus { - background-color: $dropdown-hover-color; - color: $white-light; - } - &.droplab-item-selected i { visibility: visible; } - &.divider { - margin: 0 8px; - padding: 0; - border-top: $gray-darkest; - } - .icon { visibility: hidden; } @@ -431,11 +457,6 @@ } } -.dropdown-menu-user-link { - padding-top: 10px; - padding-bottom: 7px; -} - .dropdown-menu-user-full-name { display: block; font-weight: $gl-font-weight-normal; @@ -464,41 +485,44 @@ .dropdown-menu-align-right { left: auto; right: 0; - margin-top: -5px; } .dropdown-menu-selectable { - a { - padding-left: 26px; - position: relative; + li { + a { + padding: 8px 40px; + position: relative; + + &.is-indeterminate, + &.is-active { + color: $gl-text-color; + + &::before { + position: absolute; + left: 16px; + top: 16px; + transform: translateY(-50%); + font: normal normal normal 14px/1 FontAwesome; + font-size: inherit; + text-rendering: auto; + -webkit-font-smoothing: antialiased; + -moz-osx-font-smoothing: grayscale; + } - &.is-indeterminate, - &.is-active { - font-weight: $gl-font-weight-bold; - color: $gl-text-color; - - &::before { - position: absolute; - left: 6px; - top: 50%; - transform: translateY(-50%); - font: normal normal normal 14px/1 FontAwesome; - font-size: inherit; - text-rendering: auto; - -webkit-font-smoothing: antialiased; - -moz-osx-font-smoothing: grayscale; + &.dropdown-menu-user-link { + &::before { + top: 50%; + } + } } - } - &.is-indeterminate::before { - content: "\f068"; - } + &.is-indeterminate::before { + content: "\f068"; + } - &.is-active::before { - content: "\f00c"; - position: absolute; - top: 50%; - transform: translateY(-50%); + &.is-active::before { + content: "\f00c"; + } } } } @@ -735,136 +759,6 @@ } } -@mixin dropdown-item-hover { - background-color: $dropdown-item-hover-bg; - color: $gl-text-color; -} - -// TODO: change global style and remove mixin -@mixin new-style-dropdown($selector: '') { - #{$selector}.dropdown-menu, - #{$selector}.dropdown-menu-nav { - margin-bottom: 24px; - - &.dropdown-open-top { - margin-bottom: $dropdown-vertical-offset; - } - - li { - display: block; - padding: 0 1px; - - &:hover { - background-color: transparent; - } - - &.divider { - margin: 6px 0; - - &:hover { - background-color: $dropdown-divider-color; - } - } - - &.dropdown-header { - padding: 8px 16px; - } - - &.droplab-item-active button { - @include dropdown-item-hover; - } - - a, - button, - .menu-item { - margin-bottom: 0; - border-radius: 0; - box-shadow: none; - padding: 8px 16px; - text-align: left; - white-space: normal; - width: 100%; - font-weight: $gl-font-weight-normal; - line-height: normal; - - &.dropdown-menu-user-link { - white-space: nowrap; - - .dropdown-menu-user-username { - display: block; - } - } - - // make sure the text color is not overriden - &.text-danger { - color: $brand-danger; - } - - &.is-focused, - &:hover, - &:active, - &:focus { - @include dropdown-item-hover; - - background-color: $dropdown-item-hover-bg; - color: $gl-text-color; - - // make sure the text color is not overriden - &.text-danger { - color: $brand-danger; - } - } - - &.is-active { - font-weight: inherit; - - &::before { - top: 16px; - } - - &.dropdown-menu-user-link::before { - top: 50%; - transform: translateY(-50%); - } - } - } - - &.dropdown-menu-empty-item a { - &:hover, - &:focus { - background-color: transparent; - } - } - } - - &.dropdown-menu-selectable { - li { - a { - padding: 8px 40px; - - &.is-indeterminate::before, - &.is-active::before { - left: 16px; - } - } - } - } - } - - #{$selector}.dropdown-menu-align-right { - margin-top: 2px; - } - - .open { - #{$selector}.dropdown-menu, - #{$selector}.dropdown-menu-nav { - @media (max-width: $screen-xs-max) { - max-width: 100%; - } - } - } -} - @media (max-width: $screen-xs-max) { .navbar-gitlab { li.header-projects, @@ -891,9 +785,6 @@ } } -@include new-style-dropdown('.breadcrumbs-list .dropdown '); -@include new-style-dropdown('.js-namespace-select + '); - header.header-content .dropdown-menu.projects-dropdown-menu { padding: 0; } diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index cec38eea464..2d7465401f1 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -50,8 +50,6 @@ } .filtered-search-wrapper { - @include new-style-dropdown; - display: -webkit-flex; display: flex; @@ -165,16 +163,6 @@ } } -.droplab-dropdown li.filtered-search-token { - padding: 0; - - &:hover, - &:focus { - background-color: inherit; - color: inherit; - } -} - .filtered-search-term { .name { background-color: inherit; @@ -336,21 +324,12 @@ .filtered-search-history-dropdown-content { max-height: none; -} - -.filtered-search-history-dropdown-item, -.filtered-search-history-clear-button { - @include dropdown-link; - - overflow: hidden; - width: 100%; - margin: 0.5em 0; - background-color: transparent; - border: 0; - text-align: left; - white-space: nowrap; - text-overflow: ellipsis; + .filtered-search-history-dropdown-item, + .filtered-search-history-clear-button { + white-space: nowrap; + text-overflow: ellipsis; + } } .filtered-search-history-dropdown-token { @@ -402,24 +381,9 @@ } } -%filter-dropdown-item-btn-hover { - text-decoration: none; - outline: 0; - - .avatar { - border-color: $white-light; - } -} - .droplab-dropdown .dropdown-menu .filter-dropdown-item { .btn { - border: 0; - width: 100%; - text-align: left; - padding: 8px 16px; text-overflow: ellipsis; - overflow: hidden; - border-radius: 0; .fa { width: 15px; @@ -434,11 +398,6 @@ height: 17px; top: 0; } - - &:hover, - &:focus { - @extend %filter-dropdown-item-btn-hover; - } } .dropdown-light-content { @@ -459,17 +418,9 @@ word-break: break-all; } } - - &.droplab-item-active .btn { - @extend %filter-dropdown-item-btn-hover; - } } .filter-dropdown-loading { padding: 8px 16px; text-align: center; } - -.issues-details-filters { - @include new-style-dropdown; -} diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index f985a3aea5c..2ce4de6a2d5 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -3,8 +3,6 @@ } .navbar-gitlab { - @include new-style-dropdown; - &.navbar-gitlab { padding: 0 16px; z-index: 1000; diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index e6e6c4c3963..f79a71221c4 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -132,8 +132,6 @@ ul.content-list { } .controls { - @include new-style-dropdown; - float: right; > .control-text { diff --git a/app/assets/stylesheets/framework/secondary-navigation-elements.scss b/app/assets/stylesheets/framework/secondary-navigation-elements.scss index 8498b37abe4..5f67126bafa 100644 --- a/app/assets/stylesheets/framework/secondary-navigation-elements.scss +++ b/app/assets/stylesheets/framework/secondary-navigation-elements.scss @@ -86,8 +86,6 @@ } .nav-controls { - @include new-style-dropdown; - display: inline-block; float: right; text-align: right; diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 3ebba4f9efb..0742c0a2a09 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -144,10 +144,6 @@ } } -.issuable-sidebar { - @include new-style-dropdown; -} - .pikaday-container { .pika-single { margin-top: 2px; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 0817cce114c..11c1aeea871 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -343,8 +343,6 @@ a > code { @extend .ref-name; } -@include new-style-dropdown('.git-revision-dropdown'); - /** * Apply Markdown typography * diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index f139f4ab650..98d460339cd 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -323,8 +323,6 @@ } .build-dropdown { - @include new-style-dropdown; - margin: $gl-padding 0; padding: 0; diff --git a/app/assets/stylesheets/pages/clusters.scss b/app/assets/stylesheets/pages/clusters.scss index c303f016ff9..88d44131d5b 100644 --- a/app/assets/stylesheets/pages/clusters.scss +++ b/app/assets/stylesheets/pages/clusters.scss @@ -13,8 +13,6 @@ max-width: 100%; } -@include new-style-dropdown('.clusters-dropdown '); - .clusters-container { .nav-bar-right { padding: $gl-padding-top $gl-padding; diff --git a/app/assets/stylesheets/pages/cycle_analytics.scss b/app/assets/stylesheets/pages/cycle_analytics.scss index 292e0ad394b..3b35beb7695 100644 --- a/app/assets/stylesheets/pages/cycle_analytics.scss +++ b/app/assets/stylesheets/pages/cycle_analytics.scss @@ -1,6 +1,4 @@ #cycle-analytics { - @include new-style-dropdown; - max-width: 1000px; margin: 24px auto 0; position: relative; diff --git a/app/assets/stylesheets/pages/detail_page.scss b/app/assets/stylesheets/pages/detail_page.scss index 52e4d904b9b..2f2c04206e2 100644 --- a/app/assets/stylesheets/pages/detail_page.scss +++ b/app/assets/stylesheets/pages/detail_page.scss @@ -32,8 +32,6 @@ } .detail-page-header-actions { - @include new-style-dropdown; - align-self: center; flex-shrink: 0; flex: 0 0 auto; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 71a6c7a2bf9..60b07537799 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -581,8 +581,6 @@ } .commit-stat-summary { - @include new-style-dropdown; - @media (min-width: $screen-sm-min) { margin-left: -$gl-padding; padding-left: $gl-padding; diff --git a/app/assets/stylesheets/pages/editor.scss b/app/assets/stylesheets/pages/editor.scss index c586dab4cf2..8ecda50602d 100644 --- a/app/assets/stylesheets/pages/editor.scss +++ b/app/assets/stylesheets/pages/editor.scss @@ -204,8 +204,6 @@ .gitlab-ci-yml-selector, .dockerfile-selector, .template-type-selector { - @include new-style-dropdown; - display: inline-block; vertical-align: top; font-family: $regular_font; diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index a5a6b7461a3..f4882305c57 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -12,8 +12,6 @@ .environments-container { .ci-table { - @include new-style-dropdown; - .deployment-column { > span { word-break: break-all; diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 2e26f2b5bbd..8f8f11e3857 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -488,12 +488,6 @@ } } - .dropdown-content { - a:hover { - color: inherit; - } - } - .dropdown-menu-toggle { width: 100%; padding-top: 6px; @@ -512,10 +506,6 @@ } } -.sidebar-move-issue-dropdown { - @include new-style-dropdown; -} - .sidebar-move-issue-confirmation-button { width: 100%; diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index af1df8b8802..c48e58af691 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -142,8 +142,6 @@ ul.related-merge-requests > li { } .issue-form { - @include new-style-dropdown; - .select2-container { width: 250px !important; } diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 92abe82df4c..e8cd8a4905c 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/app/assets/stylesheets/pages/labels.scss @@ -116,8 +116,6 @@ } .manage-labels-list { - @include new-style-dropdown; - > li:not(.empty-message):not(.is-not-draggable) { background-color: $white-light; cursor: move; diff --git a/app/assets/stylesheets/pages/members.scss b/app/assets/stylesheets/pages/members.scss index 18c48405ecd..3422829de58 100644 --- a/app/assets/stylesheets/pages/members.scss +++ b/app/assets/stylesheets/pages/members.scss @@ -58,8 +58,6 @@ } .member-form-control { - @include new-style-dropdown; - @media (max-width: $screen-xs-max) { padding-bottom: 5px; margin-left: 0; @@ -73,8 +71,6 @@ } .member-search-form { - @include new-style-dropdown; - position: relative; @media (min-width: $screen-sm-min) { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 2afb17334e3..e75a35d78ad 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -485,8 +485,6 @@ } .mr-source-target { - @include new-style-dropdown; - display: flex; flex-wrap: wrap; justify-content: space-between; @@ -608,8 +606,6 @@ } .mr-version-controls { - @include new-style-dropdown; - position: relative; background: $gray-light; color: $gl-text-color; @@ -727,7 +723,3 @@ font-size: 16px; } } - -.merge-request-form { - @include new-style-dropdown; -} diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index ebb5d121433..6d4ccd53e12 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -23,8 +23,6 @@ .new-note, .note-edit-form { .note-form-actions { - @include new-style-dropdown; - position: relative; margin: $gl-padding 0 0; } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 4d5613c292b..26e6e8688b6 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -490,8 +490,6 @@ ul.notes { } .note-actions { - @include new-style-dropdown; - align-self: flex-start; flex-shrink: 0; display: inline-flex; diff --git a/app/assets/stylesheets/pages/notifications.scss b/app/assets/stylesheets/pages/notifications.scss index c28b1e68008..bdf07a99daf 100644 --- a/app/assets/stylesheets/pages/notifications.scss +++ b/app/assets/stylesheets/pages/notifications.scss @@ -14,7 +14,3 @@ font-size: 18px; } } - -.notification-form { - @include new-style-dropdown; -} diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index cb24274c612..9805fc4f882 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -286,8 +286,6 @@ // Pipeline visualization .pipeline-actions { - @include new-style-dropdown; - border-bottom: 0; } @@ -703,9 +701,6 @@ button.mini-pipeline-graph-dropdown-toggle { } } -@include new-style-dropdown('.big-pipeline-graph-dropdown-menu'); -@include new-style-dropdown('.mini-pipeline-graph-dropdown-menu'); - // dropdown content for big and mini pipeline .big-pipeline-graph-dropdown-menu, .mini-pipeline-graph-dropdown-menu { @@ -804,7 +799,6 @@ button.mini-pipeline-graph-dropdown-toggle { font-weight: normal; line-height: $line-height-base; white-space: nowrap; - border-radius: 3px; .ci-job-name-component { align-items: center; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 674588752d2..6f4c678c4b8 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -323,8 +323,6 @@ } .project-repo-buttons { - @include new-style-dropdown; - .project-action-button .dropdown-menu { max-height: 250px; overflow-y: auto; @@ -898,8 +896,6 @@ pre.light-well { .new-protected-branch, .new-protected-tag { - @include new-style-dropdown; - label { margin-top: 6px; font-weight: $gl-font-weight-normal; @@ -919,8 +915,6 @@ pre.light-well { .protected-branches-list, .protected-tags-list { - @include new-style-dropdown; - margin-bottom: 30px; .settings-message { diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index fe455a04960..49c8e546bf2 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -116,11 +116,6 @@ input[type="checkbox"]:hover { opacity: 0; display: block; left: -5px; - padding: 0; - - ul { - padding: 10px 0; - } } .dropdown-content { @@ -185,8 +180,6 @@ input[type="checkbox"]:hover { } .search-holder { - @include new-style-dropdown; - @media (min-width: $screen-sm-min) { display: -webkit-flex; display: flex; diff --git a/app/assets/stylesheets/pages/todos.scss b/app/assets/stylesheets/pages/todos.scss index 2139a029fc7..a79772ea37b 100644 --- a/app/assets/stylesheets/pages/todos.scss +++ b/app/assets/stylesheets/pages/todos.scss @@ -265,7 +265,3 @@ font-weight: $gl-font-weight-bold; } } - -.todos-filters { - @include new-style-dropdown; -} diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index 5d14323e4bc..e0ee7e9aa3d 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -1,6 +1,4 @@ .tree-holder { - @include new-style-dropdown; - .nav-block { margin: 10px 0; diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 2ce26de1768..a94726887d9 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -1,4 +1,6 @@ class Admin::GroupsController < Admin::ApplicationController + include MembersPresentation + before_action :group, only: [:edit, :update, :destroy, :project_update, :members_update] def index @@ -10,8 +12,10 @@ class Admin::GroupsController < Admin::ApplicationController def show @group = Group.with_statistics.joins(:route).group('routes.path').find_by_full_path(params[:id]) - @members = @group.members.order("access_level DESC").page(params[:members_page]) - @requesters = AccessRequestsFinder.new(@group).execute(current_user) + @members = present_members( + @group.members.order("access_level DESC").page(params[:members_page])) + @requesters = present_members( + AccessRequestsFinder.new(@group).execute(current_user)) @projects = @group.projects.with_statistics.page(params[:projects_page]) end diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb index 50cf2643390..3afe66c3566 100644 --- a/app/controllers/admin/projects_controller.rb +++ b/app/controllers/admin/projects_controller.rb @@ -1,4 +1,6 @@ class Admin::ProjectsController < Admin::ApplicationController + include MembersPresentation + before_action :project, only: [:show, :transfer, :repository_check] before_action :group, only: [:show, :transfer] @@ -19,11 +21,14 @@ class Admin::ProjectsController < Admin::ApplicationController def show if @group - @group_members = @group.members.order("access_level DESC").page(params[:group_members_page]) + @group_members = present_members( + @group.members.order("access_level DESC").page(params[:group_members_page])) end - @project_members = @project.members.page(params[:project_members_page]) - @requesters = AccessRequestsFinder.new(@project).execute(current_user) + @project_members = present_members( + @project.members.page(params[:project_members_page])) + @requesters = present_members( + AccessRequestsFinder.new(@project).execute(current_user)) end def transfer diff --git a/app/controllers/concerns/members_presentation.rb b/app/controllers/concerns/members_presentation.rb new file mode 100644 index 00000000000..c0622516fd3 --- /dev/null +++ b/app/controllers/concerns/members_presentation.rb @@ -0,0 +1,11 @@ +module MembersPresentation + extend ActiveSupport::Concern + + def present_members(members) + Gitlab::View::Presenter::Factory.new( + members, + current_user: current_user, + presenter_class: MembersPresenter + ).fabricate! + end +end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 5919bf54468..21e77431176 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -1,5 +1,6 @@ class Groups::GroupMembersController < Groups::ApplicationController include MembershipActions + include MembersPresentation include SortingHelper # Authorize @@ -14,15 +15,17 @@ class Groups::GroupMembersController < Groups::ApplicationController @members = @members.search(params[:search]) if params[:search].present? @members = @members.sort(@sort) @members = @members.page(params[:page]).per(50) - @members.includes(:user) + @members = present_members(@members.includes(:user)) - @requesters = AccessRequestsFinder.new(@group).execute(current_user) + @requesters = present_members( + AccessRequestsFinder.new(@group).execute(current_user)) @group_member = @group.group_members.new end def update @group_member = @group.members_and_requesters.find(params[:id]) + .present(current_user: current_user) return render_403 unless can?(current_user, :update_group_member, @group_member) diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index 0907daacbc3..4a7879db313 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -8,11 +8,8 @@ class Projects::ClustersController < Projects::ApplicationController STATUS_POLLING_INTERVAL = 10_000 def index - @scope = params[:scope] || 'all' - @clusters = ClustersFinder.new(project, current_user, @scope).execute.page(params[:page]) - @active_count = ClustersFinder.new(project, current_user, :active).execute.count - @inactive_count = ClustersFinder.new(project, current_user, :inactive).execute.count - @all_count = @active_count + @inactive_count + clusters = ClustersFinder.new(project, current_user, :all).execute + @clusters = clusters.page(params[:page]).per(20) end def new diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 5a01a59481b..d7372beb9d3 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -1,5 +1,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController include MembershipActions + include MembersPresentation include SortingHelper # Authorize @@ -20,13 +21,14 @@ class Projects::ProjectMembersController < Projects::ApplicationController @group_links = @group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id)) end - @project_members = @project_members.sort(@sort).page(params[:page]) - @requesters = AccessRequestsFinder.new(@project).execute(current_user) + @project_members = present_members(@project_members.sort(@sort).page(params[:page])) + @requesters = present_members(AccessRequestsFinder.new(@project).execute(current_user)) @project_member = @project.project_members.new end def update @project_member = @project.members_and_requesters.find(params[:id]) + .present(current_user: current_user) return render_403 unless can?(current_user, :update_project_member, @project_member) diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index e1ba7898ee6..c1c19062c91 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -1,6 +1,13 @@ module LabelsHelper include ActionView::Helpers::TagHelper + def show_label_issuables_link?(label, issuables_type, current_user: nil, project: nil) + return true if label.is_a?(GroupLabel) + return true unless project + + project.feature_available?(issuables_type, current_user) + end + # Link to a Label # # label - Label object to link to diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb index 41d471cc92f..a3129cac2b1 100644 --- a/app/helpers/members_helper.rb +++ b/app/helpers/members_helper.rb @@ -1,11 +1,4 @@ module MembersHelper - # Returns a `<action>_<source>_member` association, e.g.: - # - admin_project_member, update_project_member, destroy_project_member - # - admin_group_member, update_group_member, destroy_group_member - def action_member_permission(action, member) - "#{action}_#{member.type.underscore}".to_sym - end - def remove_member_message(member, user: nil) user = current_user if defined?(current_user) diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb index 98776eab424..90ad644ce34 100644 --- a/app/models/concerns/cache_markdown_field.rb +++ b/app/models/concerns/cache_markdown_field.rb @@ -85,8 +85,7 @@ module CacheMarkdownField def cached_html_up_to_date?(markdown_field) html_field = cached_markdown_fields.html_field(markdown_field) - cached = cached_html_for(markdown_field).present? && __send__(markdown_field).present? # rubocop:disable GitlabSecurity/PublicSend - return false unless cached + return false if cached_html_for(markdown_field).nil? && !__send__(markdown_field).nil? # rubocop:disable GitlabSecurity/PublicSend markdown_changed = attribute_changed?(markdown_field) || false html_changed = attribute_changed?(html_field) || false diff --git a/app/models/member.rb b/app/models/member.rb index 2fe5fda985f..c47145667b5 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -4,6 +4,7 @@ class Member < ActiveRecord::Base include Importable include Expirable include Gitlab::Access + include Presentable attr_accessor :raw_invite_token diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 422f138c4ea..26a3388602a 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -8,6 +8,7 @@ class MergeRequest < ActiveRecord::Base include ManualInverseAssociation include EachBatch include ThrottledTouch + include Gitlab::Utils::StrongMemoize ignore_column :locked_at, :ref_fetched @@ -53,6 +54,7 @@ class MergeRequest < ActiveRecord::Base after_create :ensure_merge_request_diff, unless: :importing? after_update :reload_diff_if_branch_changed + after_update :clear_memoized_shas # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests @@ -387,13 +389,17 @@ class MergeRequest < ActiveRecord::Base end def source_branch_head - return unless source_project - - source_project.repository.commit(source_branch_ref) if source_branch_ref + strong_memoize(:source_branch_head) do + if source_project && source_branch_ref + source_project.repository.commit(source_branch_ref) + end + end end def target_branch_head - target_project.repository.commit(target_branch_ref) + strong_memoize(:target_branch_head) do + target_project.repository.commit(target_branch_ref) + end end def branch_merge_base_commit @@ -525,6 +531,13 @@ class MergeRequest < ActiveRecord::Base end end + def clear_memoized_shas + @target_branch_sha = @source_branch_sha = nil + + clear_memoization(:source_branch_head) + clear_memoization(:target_branch_head) + end + def reload_diff_if_branch_changed if (source_branch_changed? || target_branch_changed?) && (source_branch_head && target_branch_head) diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index c37aa0a594b..e35de9b97ee 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -104,19 +104,19 @@ class MergeRequestDiff < ActiveRecord::Base def base_commit return unless base_commit_sha - project.commit(base_commit_sha) + project.commit_by(oid: base_commit_sha) end def start_commit return unless start_commit_sha - project.commit(start_commit_sha) + project.commit_by(oid: start_commit_sha) end def head_commit return unless head_commit_sha - project.commit(head_commit_sha) + project.commit_by(oid: head_commit_sha) end def commit_shas diff --git a/app/models/note.rb b/app/models/note.rb index c4c2ab8e67d..02f9fd61e49 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -401,6 +401,9 @@ class Note < ActiveRecord::Base end noteable_object&.touch + + # We return the noteable object so we can re-use it in EE for ElasticSearch. + noteable_object end def banzai_render_context(field) diff --git a/app/models/project.rb b/app/models/project.rb index 6ae15a0a50f..5183a216c53 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -659,7 +659,8 @@ class Project < ActiveRecord::Base end def import_started? - import? && import_status == 'started' + # import? does SQL work so only run it if it looks like there's an import running + import_status == 'started' && import? end def import_scheduled? @@ -1147,7 +1148,7 @@ class Project < ActiveRecord::Base def change_head(branch) if repository.branch_exists?(branch) repository.before_change_head - repository.write_ref('HEAD', "refs/heads/#{branch}") + repository.write_ref('HEAD', "refs/heads/#{branch}", force: true) repository.copy_gitattributes(branch) repository.after_change_head reload_default_branch diff --git a/app/models/repository.rb b/app/models/repository.rb index d1ae3304e4a..c0e31eca8da 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -19,6 +19,7 @@ class Repository attr_accessor :full_path, :disk_path, :project, :is_wiki delegate :ref_name_for_sha, to: :raw_repository + delegate :write_ref, to: :raw_repository CreateTreeError = Class.new(StandardError) @@ -237,11 +238,10 @@ class Repository # This will still fail if the file is corrupted (e.g. 0 bytes) begin - write_ref(keep_around_ref_name(sha), sha) - rescue Rugged::ReferenceError => ex - Rails.logger.error "Unable to create #{REF_KEEP_AROUND} reference for repository #{path}: #{ex}" - rescue Rugged::OSError => ex - raise unless ex.message =~ /Failed to create locked file/ && ex.message =~ /File exists/ + write_ref(keep_around_ref_name(sha), sha, force: true) + rescue Gitlab::Git::Repository::GitError => ex + # Necessary because https://gitlab.com/gitlab-org/gitlab-ce/issues/20156 + return true if ex.message =~ /Failed to create locked file/ && ex.message =~ /File exists/ Rails.logger.error "Unable to create #{REF_KEEP_AROUND} reference for repository #{path}: #{ex}" end @@ -251,10 +251,6 @@ class Repository ref_exists?(keep_around_ref_name(sha)) end - def write_ref(ref_path, sha) - rugged.references.create(ref_path, sha, force: true) - end - def diverging_commit_counts(branch) root_ref_hash = raw_repository.commit(root_ref).id cache.fetch(:"diverging_commit_counts_#{branch.name}") do @@ -994,7 +990,7 @@ class Repository end def create_ref(ref, ref_path) - raw_repository.write_ref(ref_path, ref) + write_ref(ref_path, ref) end def ls_files(ref) diff --git a/app/presenters/group_member_presenter.rb b/app/presenters/group_member_presenter.rb new file mode 100644 index 00000000000..8f53dfa105e --- /dev/null +++ b/app/presenters/group_member_presenter.rb @@ -0,0 +1,15 @@ +class GroupMemberPresenter < MemberPresenter + private + + def admin_member_permission + :admin_group_member + end + + def update_member_permission + :update_group_member + end + + def destroy_member_permission + :destroy_group_member + end +end diff --git a/app/presenters/member_presenter.rb b/app/presenters/member_presenter.rb new file mode 100644 index 00000000000..7d2f9303b8f --- /dev/null +++ b/app/presenters/member_presenter.rb @@ -0,0 +1,38 @@ +class MemberPresenter < Gitlab::View::Presenter::Delegated + presents :member + + def access_level_roles + member.class.access_level_roles + end + + def can_resend_invite? + invite? && + can?(current_user, admin_member_permission, source) + end + + def can_update? + can?(current_user, update_member_permission, member) + end + + def can_remove? + can?(current_user, destroy_member_permission, member) + end + + def can_approve? + request? && can_update? + end + + private + + def admin_member_permission + raise NotImplementedError + end + + def update_member_permission + raise NotImplementedError + end + + def destroy_member_permission + raise NotImplementedError + end +end diff --git a/app/presenters/members_presenter.rb b/app/presenters/members_presenter.rb new file mode 100644 index 00000000000..e4aba37b69e --- /dev/null +++ b/app/presenters/members_presenter.rb @@ -0,0 +1,15 @@ +class MembersPresenter < Gitlab::View::Presenter::Delegated + include Enumerable + + presents :members + + def to_ary + to_a + end + + def each + members.each do |member| + yield member.present(current_user: current_user) + end + end +end diff --git a/app/presenters/project_member_presenter.rb b/app/presenters/project_member_presenter.rb new file mode 100644 index 00000000000..7f42d2b70df --- /dev/null +++ b/app/presenters/project_member_presenter.rb @@ -0,0 +1,15 @@ +class ProjectMemberPresenter < MemberPresenter + private + + def admin_member_permission + :admin_project_member + end + + def update_member_permission + :update_project_member + end + + def destroy_member_permission + :destroy_project_member + end +end diff --git a/app/services/members/approve_access_request_service.rb b/app/services/members/approve_access_request_service.rb index c13f289f61e..2a2bb0cae5b 100644 --- a/app/services/members/approve_access_request_service.rb +++ b/app/services/members/approve_access_request_service.rb @@ -35,8 +35,17 @@ module Members def can_update_access_requester?(access_requester, opts = {}) access_requester && ( opts[:force] || - can?(current_user, action_member_permission(:update, access_requester), access_requester) + can?(current_user, update_member_permission(access_requester), access_requester) ) end + + def update_member_permission(member) + case member + when GroupMember + :update_group_member + when ProjectMember + :update_project_member + end + end end end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 46c505baf8b..05b93ac8fdb 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -36,7 +36,16 @@ module Members end def can_destroy_member?(member) - member && can?(current_user, action_member_permission(:destroy, member), member) + member && can?(current_user, destroy_member_permission(member), member) + end + + def destroy_member_permission(member) + case member + when GroupMember + :destroy_group_member + when ProjectMember + :destroy_project_member + end end end end diff --git a/app/views/projects/blob/_template_selectors.html.haml b/app/views/projects/blob/_template_selectors.html.haml index 2a178325041..5b092427496 100644 --- a/app/views/projects/blob/_template_selectors.html.haml +++ b/app/views/projects/blob/_template_selectors.html.haml @@ -3,15 +3,15 @@ Template .template-selector-dropdowns-wrap .template-type-selector.js-template-type-selector-wrap.hidden - = dropdown_tag("Choose type", options: { toggle_class: 'btn js-template-type-selector', title: "Choose a template type" } ) + = dropdown_tag("Choose type", options: { toggle_class: 'js-template-type-selector', title: "Choose a template type" } ) .license-selector.js-license-selector-wrap.js-template-selector-wrap.hidden - = dropdown_tag("Apply a license template", options: { toggle_class: 'btn js-license-selector', title: "Apply a license", filter: true, placeholder: "Filter", data: { data: licenses_for_select, project: @project.name, fullname: @project.namespace.human_name } } ) + = dropdown_tag("Apply a license template", options: { toggle_class: 'js-license-selector', title: "Apply a license", filter: true, placeholder: "Filter", data: { data: licenses_for_select, project: @project.name, fullname: @project.namespace.human_name } } ) .gitignore-selector.js-gitignore-selector-wrap.js-template-selector-wrap.hidden - = dropdown_tag("Apply a .gitignore template", options: { toggle_class: 'btn js-gitignore-selector', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: gitignore_names } } ) + = dropdown_tag("Apply a .gitignore template", options: { toggle_class: 'js-gitignore-selector', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: gitignore_names } } ) .gitlab-ci-yml-selector.js-gitlab-ci-yml-selector-wrap.js-template-selector-wrap.hidden - = dropdown_tag("Apply a GitLab CI Yaml template", options: { toggle_class: 'btn js-gitlab-ci-yml-selector', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: gitlab_ci_ymls } } ) + = dropdown_tag("Apply a GitLab CI Yaml template", options: { toggle_class: 'js-gitlab-ci-yml-selector', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: gitlab_ci_ymls } } ) .dockerfile-selector.js-dockerfile-selector-wrap.js-template-selector-wrap.hidden - = dropdown_tag("Apply a Dockerfile template", options: { toggle_class: 'btn js-dockerfile-selector', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: dockerfile_names } } ) + = dropdown_tag("Apply a Dockerfile template", options: { toggle_class: 'js-dockerfile-selector', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: dockerfile_names } } ) .template-selectors-undo-menu.hidden %span.text-info Template applied %button.btn.btn-sm.btn-info Undo diff --git a/app/views/projects/branches/new.html.haml b/app/views/projects/branches/new.html.haml index 2baaaf6ac5b..e9d8fc75142 100644 --- a/app/views/projects/branches/new.html.haml +++ b/app/views/projects/branches/new.html.haml @@ -20,7 +20,7 @@ .col-sm-10.create-from .dropdown = hidden_field_tag :ref, default_ref - = button_tag type: 'button', title: default_ref, class: 'dropdown-menu-toggle wide form-control js-branch-select git-revision-dropdown-toggle', required: true, data: { toggle: 'dropdown', selected: default_ref, field_name: 'ref' } do + = button_tag type: 'button', title: default_ref, class: 'dropdown-menu-toggle wide js-branch-select git-revision-dropdown-toggle', required: true, data: { toggle: 'dropdown', selected: default_ref, field_name: 'ref' } do .text-left.dropdown-toggle-text= default_ref = icon('chevron-down') = render 'shared/ref_dropdown', dropdown_class: 'wide' diff --git a/app/views/projects/clusters/_tabs.html.haml b/app/views/projects/clusters/_tabs.html.haml deleted file mode 100644 index c8120e806fa..00000000000 --- a/app/views/projects/clusters/_tabs.html.haml +++ /dev/null @@ -1,16 +0,0 @@ -.top-area.scrolling-tabs-container.inner-page-scroll-tabs - .fade-left= icon("angle-left") - .fade-right= icon("angle-right") - %ul.nav-links.scrolling-tabs - %li{ class: ('active' if @scope == 'active') }> - = link_to project_clusters_path(@project, scope: :active), class: "js-active-tab" do - = s_("ClusterIntegration|Active") - %span.badge= @active_count - %li{ class: ('active' if @scope == 'inactive') }> - = link_to project_clusters_path(@project, scope: :inactive), class: "js-inactive-tab" do - = s_("ClusterIntegration|Inactive") - %span.badge= @inactive_count - %li{ class: ('active' if @scope.nil? || @scope == 'all') }> - = link_to project_clusters_path(@project), class: "js-all-tab" do - = s_("ClusterIntegration|All") - %span.badge= @all_count diff --git a/app/views/projects/clusters/index.html.haml b/app/views/projects/clusters/index.html.haml index 104e39b0e06..bec512be91c 100644 --- a/app/views/projects/clusters/index.html.haml +++ b/app/views/projects/clusters/index.html.haml @@ -2,8 +2,12 @@ - page_title "Clusters" .clusters-container - - if !@clusters.empty? - = render "tabs" + - if @clusters.empty? + = render "empty_state" + - else + .top-area.adjust + .nav-text + = s_("ClusterIntegration|Clusters can be used to deploy applications and to provide Review Apps for this project") .ci-table.js-clusters-list .gl-responsive-table-row.table-row-header{ role: "row" } .table-section.section-30{ role: "rowheader" } @@ -16,9 +20,3 @@ - @clusters.each do |cluster| = render "cluster", cluster: cluster.present(current_user: current_user) = paginate @clusters, theme: "gitlab" - - elsif @scope == 'all' - = render "empty_state" - - else - = render "tabs" - .prepend-top-20.text-center - = s_("ClusterIntegration|There are no clusters to show") diff --git a/app/views/projects/environments/metrics.html.haml b/app/views/projects/environments/metrics.html.haml index 56cf16c3778..ad94113fffd 100644 --- a/app/views/projects/environments/metrics.html.haml +++ b/app/views/projects/environments/metrics.html.haml @@ -15,9 +15,9 @@ #prometheus-graphs{ data: { "settings-path": edit_project_service_path(@project, 'prometheus'), "documentation-path": help_page_path('administration/monitoring/prometheus/index.md'), - "empty-getting-started-svg-path": image_path('illustrations/monitoring/getting_started'), - "empty-loading-svg-path": image_path('illustrations/monitoring/loading'), - "empty-unable-to-connect-svg-path": image_path('illustrations/monitoring/unable_to_connect'), + "empty-getting-started-svg-path": image_path('illustrations/monitoring/getting_started.svg'), + "empty-loading-svg-path": image_path('illustrations/monitoring/loading.svg'), + "empty-unable-to-connect-svg-path": image_path('illustrations/monitoring/unable_to_connect.svg'), "additional-metrics": additional_metrics_project_environment_path(@project, @environment, format: :json), "project-path": project_path(@project), "tags-path": project_tags_path(@project), diff --git a/app/views/projects/merge_requests/_mr_title.html.haml b/app/views/projects/merge_requests/_mr_title.html.haml index 9df14265737..bc91758110e 100644 --- a/app/views/projects/merge_requests/_mr_title.html.haml +++ b/app/views/projects/merge_requests/_mr_title.html.haml @@ -27,7 +27,7 @@ .dropdown-menu.dropdown-menu-align-right.hidden-lg %ul - if can_update_merge_request - %li= link_to 'Edit', edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'js-issuable-edit' + %li= link_to 'Edit', edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request) - unless current_user == @merge_request.author %li= link_to 'Report abuse', new_abuse_report_path(user_id: @merge_request.author.id, ref_url: merge_request_url(@merge_request)) - if can_update_merge_request diff --git a/app/views/projects/project_members/_team.html.haml b/app/views/projects/project_members/_team.html.haml index e71d58ec26d..16bcf671c25 100644 --- a/app/views/projects/project_members/_team.html.haml +++ b/app/views/projects/project_members/_team.html.haml @@ -1,11 +1,13 @@ +- project = local_assigns.fetch(:project) +- members = local_assigns.fetch(:members) + .panel.panel-default .panel-heading.flex-project-members-panel %span.flex-project-title Members of - %strong - #{@project.name} - %span.badge= @project_members.total_count - = form_tag project_project_members_path(@project), method: :get, class: 'form-inline member-search-form flex-project-members-form' do + %strong= project.name + %span.badge= members.total_count + = form_tag project_project_members_path(project), method: :get, class: 'form-inline member-search-form flex-project-members-form' do .form-group = search_field_tag :search, params[:search], { placeholder: 'Find existing members by name', class: 'form-control', spellcheck: false } %button.member-search-btn{ type: "submit", "aria-label" => "Submit search" } diff --git a/app/views/projects/project_members/index.html.haml b/app/views/projects/project_members/index.html.haml index fd5d3ec56da..d81103c3a92 100644 --- a/app/views/projects/project_members/index.html.haml +++ b/app/views/projects/project_members/index.html.haml @@ -37,5 +37,5 @@ - if @group_links.any? = render 'projects/project_members/groups', group_links: @group_links - = render 'projects/project_members/team', members: @project_members + = render 'projects/project_members/team', project: @project, members: @project_members = paginate @project_members, theme: "gitlab" diff --git a/app/views/projects/tags/new.html.haml b/app/views/projects/tags/new.html.haml index 031efa903c5..6e105a5521a 100644 --- a/app/views/projects/tags/new.html.haml +++ b/app/views/projects/tags/new.html.haml @@ -14,13 +14,13 @@ .form-group = label_tag :tag_name, nil, class: 'control-label' .col-sm-10 - = text_field_tag :tag_name, params[:tag_name], required: true, tabindex: 1, autofocus: true, class: 'form-control' + = text_field_tag :tag_name, params[:tag_name], required: true, autofocus: true, class: 'form-control' .form-group = label_tag :ref, 'Create from', class: 'control-label' .col-sm-10.create-from .dropdown = hidden_field_tag :ref, default_ref - = button_tag type: 'button', title: default_ref, class: 'dropdown-menu-toggle wide form-control js-branch-select', required: true, data: { toggle: 'dropdown', selected: default_ref, field_name: 'ref' } do + = button_tag type: 'button', title: default_ref, class: 'dropdown-menu-toggle wide js-branch-select', required: true, data: { toggle: 'dropdown', selected: default_ref, field_name: 'ref' } do .text-left.dropdown-toggle-text= default_ref = render 'shared/ref_dropdown', dropdown_class: 'wide' .help-block @@ -28,7 +28,7 @@ .form-group = label_tag :message, nil, class: 'control-label' .col-sm-10 - = text_area_tag :message, @message, required: false, tabindex: 3, class: 'form-control', rows: 5 + = text_area_tag :message, @message, required: false, class: 'form-control', rows: 5 .help-block = s_('TagsPage|Optionally, add a message to the tag.') %hr @@ -41,6 +41,6 @@ .help-block = s_('TagsPage|Optionally, add release notes to the tag. They will be stored in the GitLab database and displayed on the tags page.') .form-actions - = button_tag s_('TagsPage|Create tag'), class: 'btn btn-create', tabindex: 3 + = button_tag s_('TagsPage|Create tag'), class: 'btn btn-create' = link_to s_('TagsPage|Cancel'), project_tags_path(@project), class: 'btn btn-cancel' %script#availableRefs{ type: "application/json" }= @project.repository.ref_names.to_json.html_safe diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml index 3fcc33044e9..81d07074325 100644 --- a/app/views/shared/_label.html.haml +++ b/app/views/shared/_label.html.haml @@ -2,6 +2,8 @@ - status = label_subscription_status(label, @project).inquiry if current_user - subject = local_assigns[:subject] - toggle_subscription_path = toggle_subscription_label_path(label, @project) if current_user +- show_label_merge_requests_link = show_label_issuables_link?(label, :merge_requests, project: @project) +- show_label_issues_link = show_label_issuables_link?(label, :issues, project: @project) %li{ id: label_css_id, data: { id: label.id } } = render "shared/label_row", label: label @@ -12,12 +14,14 @@ = icon('caret-down') .dropdown-menu.dropdown-menu-align-right %ul - %li - = link_to_label(label, subject: subject, type: :merge_request) do - View merge requests - %li - = link_to_label(label, subject: subject) do - View open issues + - if show_label_merge_requests_link + %li + = link_to_label(label, subject: subject, type: :merge_request) do + View merge requests + - if show_label_issues_link + %li + = link_to_label(label, subject: subject) do + View open issues - if current_user %li.label-subscription - if can_subscribe_to_label_in_different_levels?(label) @@ -35,13 +39,20 @@ %li = link_to 'Edit', edit_label_path(label) %li - = link_to 'Delete', destroy_label_path(label), title: 'Delete', method: :delete, data: {confirm: 'Remove this label? Are you sure?'} + = link_to 'Delete', + destroy_label_path(label), + title: 'Delete', + method: :delete, + data: {confirm: 'Remove this label? Are you sure?'}, + class: 'text-danger' .pull-right.hidden-xs.hidden-sm.hidden-md - = link_to_label(label, subject: subject, type: :merge_request, css_class: 'btn btn-transparent btn-action btn-link') do - view merge requests - = link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action btn-link') do - view open issues + - if show_label_merge_requests_link + = link_to_label(label, subject: subject, type: :merge_request, css_class: 'btn btn-transparent btn-action btn-link') do + view merge requests + - if show_label_issues_link + = link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action btn-link') do + view open issues - if current_user .label-subscription.inline diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 2c27dd638a7..71878e93255 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -1,9 +1,9 @@ - show_roles = local_assigns.fetch(:show_roles, true) - show_controls = local_assigns.fetch(:show_controls, true) - force_mobile_view = local_assigns.fetch(:force_mobile_view, false) +- member = local_assigns.fetch(:member) - user = local_assigns.fetch(:user, member.user) - source = member.source -- can_admin_member = can?(current_user, action_member_permission(:update, member), member) %li.member{ class: dom_class(member), id: dom_id(member) } %span.list-item-name @@ -50,18 +50,17 @@ .controls.member-controls - if show_controls && member.source == current_resource - - if member.invite? && can?(current_user, action_member_permission(:admin, member), member.source) + - if member.can_resend_invite? = link_to icon('paper-plane'), polymorphic_path([:resend_invite, member]), method: :post, class: 'btn btn-default prepend-left-10 hidden-xs', title: 'Resend invite' - - if user != current_user && can_admin_member + - if user != current_user && member.can_update? = form_for member, remote: true, html: { class: 'form-horizontal js-edit-member-form' } do |f| = f.hidden_field :access_level .member-form-control.dropdown.append-right-5 %button.dropdown-menu-toggle.js-member-permissions-dropdown{ type: "button", - disabled: !can_admin_member, data: { toggle: "dropdown", field_name: "#{f.object_name}[access_level]" } } %span.dropdown-toggle-text = member.human_access @@ -70,23 +69,22 @@ = dropdown_title("Change permissions") .dropdown-content %ul - - member.class.access_level_roles.each do |role, role_id| + - member.access_level_roles.each do |role, role_id| %li = link_to role, "javascript:void(0)", class: ("is-active" if member.access_level == role_id), data: { id: role_id, el_id: dom_id(member) } .prepend-left-5.clearable-input.member-form-control - = f.text_field :expires_at, class: 'form-control js-access-expiration-date js-member-update-control', placeholder: 'Expiration date', id: "member_expires_at_#{member.id}", disabled: !can_admin_member, data: { el_id: dom_id(member) } + = f.text_field :expires_at, + class: 'form-control js-access-expiration-date js-member-update-control', + placeholder: 'Expiration date', + id: "member_expires_at_#{member.id}", + data: { el_id: dom_id(member) } %i.clear-icon.js-clear-input - else %span.member-access-text= member.human_access - - if member.invite? && can?(current_user, action_member_permission(:admin, member), member.source) - = link_to 'Resend invite', polymorphic_path([:resend_invite, member]), - method: :post, - class: 'btn btn-default prepend-left-10 visible-xs-block' - - - elsif member.request? && can_admin_member + - if member.can_approve? = link_to polymorphic_path([:approve_access_request, member]), method: :post, class: 'btn btn-success prepend-left-10', @@ -96,7 +94,7 @@ - unless force_mobile_view = icon('check inverse', class: 'hidden-xs') - - if can?(current_user, action_member_permission(:destroy, member), member) + - if member.can_remove? - if current_user == user = link_to icon('sign-out', text: 'Leave'), polymorphic_path([:leave, member.source, :members]), method: :delete, diff --git a/app/views/shared/members/_requests.html.haml b/app/views/shared/members/_requests.html.haml index 09b9944082f..1fbd6bcc4cb 100644 --- a/app/views/shared/members/_requests.html.haml +++ b/app/views/shared/members/_requests.html.haml @@ -1,10 +1,13 @@ +- membership_source = local_assigns.fetch(:membership_source) +- requesters = local_assigns.fetch(:requesters) - force_mobile_view = local_assigns.fetch(:force_mobile_view, false) -- if requesters.any? - .panel.panel-default.prepend-top-default{ class: ('panel-mobile' if force_mobile_view ) } - .panel-heading - Users requesting access to - %strong= membership_source.name - %span.badge= requesters.size - %ul.content-list.members-list - = render partial: 'shared/members/member', collection: requesters, as: :member, locals: { force_mobile_view: force_mobile_view } +- return if requesters.empty? + +.panel.panel-default.prepend-top-default{ class: ('panel-mobile' if force_mobile_view ) } + .panel-heading + Users requesting access to + %strong= membership_source.name + %span.badge= requesters.size + %ul.content-list.members-list + = render partial: 'shared/members/member', collection: requesters, as: :member, locals: { force_mobile_view: force_mobile_view } diff --git a/changelogs/unreleased/28004-consider-refactoring-member-view-by-using-presenter.yml b/changelogs/unreleased/28004-consider-refactoring-member-view-by-using-presenter.yml new file mode 100644 index 00000000000..0e91d4ae403 --- /dev/null +++ b/changelogs/unreleased/28004-consider-refactoring-member-view-by-using-presenter.yml @@ -0,0 +1,4 @@ +--- +title: Refactor member view using a Presenter +merge_request: 9645 +author: TM Lee diff --git a/changelogs/unreleased/40285-prometheus-loading-screen-no-longer-seems-to-appear.yml b/changelogs/unreleased/40285-prometheus-loading-screen-no-longer-seems-to-appear.yml new file mode 100644 index 00000000000..978930a5b8c --- /dev/null +++ b/changelogs/unreleased/40285-prometheus-loading-screen-no-longer-seems-to-appear.yml @@ -0,0 +1,5 @@ +--- +title: Fix broken illustration images for monitoring page empty states +merge_request: 15889 +author: +type: fixed diff --git a/changelogs/unreleased/multiple-clusters-single-list.yml b/changelogs/unreleased/multiple-clusters-single-list.yml new file mode 100644 index 00000000000..55743f3c00e --- /dev/null +++ b/changelogs/unreleased/multiple-clusters-single-list.yml @@ -0,0 +1,5 @@ +--- +title: Present multiple clusters in a single list instead of a tabbed view +merge_request: 15669 +author: +type: changed diff --git a/changelogs/unreleased/optimize-issues-avoid-noop-empty-cache-updates2.yml b/changelogs/unreleased/optimize-issues-avoid-noop-empty-cache-updates2.yml new file mode 100644 index 00000000000..e0c3136be69 --- /dev/null +++ b/changelogs/unreleased/optimize-issues-avoid-noop-empty-cache-updates2.yml @@ -0,0 +1,6 @@ +--- +title: Treat empty markdown and html strings as valid cached text, not missing cache + that needs to be updated +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/optimize-projects-for-imported-projects.yml b/changelogs/unreleased/optimize-projects-for-imported-projects.yml new file mode 100644 index 00000000000..13186fa36d5 --- /dev/null +++ b/changelogs/unreleased/optimize-projects-for-imported-projects.yml @@ -0,0 +1,6 @@ +--- +title: check the import_status field before doing SQL operations to check the import + url +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/remove-tabindexes-from-tag-form.yml b/changelogs/unreleased/remove-tabindexes-from-tag-form.yml new file mode 100644 index 00000000000..a15bf2a7a4f --- /dev/null +++ b/changelogs/unreleased/remove-tabindexes-from-tag-form.yml @@ -0,0 +1,5 @@ +--- +title: removed tabindexes from tag form +merge_request: +author: Marcus Amargi +type: changed diff --git a/changelogs/unreleased/sophie-h-gitlab-ce-patch-15.yml b/changelogs/unreleased/sophie-h-gitlab-ce-patch-15.yml new file mode 100644 index 00000000000..b5e3210c737 --- /dev/null +++ b/changelogs/unreleased/sophie-h-gitlab-ce-patch-15.yml @@ -0,0 +1,5 @@ +--- +title: Hide link to issues/MRs from labels list if issues/MRs are disabled. +merge_request: 15863 +author: Sophie Herold +type: fixed diff --git a/changelogs/unreleased/zj-memoization-mr-commits.yml b/changelogs/unreleased/zj-memoization-mr-commits.yml new file mode 100644 index 00000000000..59dfc6d6049 --- /dev/null +++ b/changelogs/unreleased/zj-memoization-mr-commits.yml @@ -0,0 +1,5 @@ +--- +title: Cache commits for MergeRequest diffs +merge_request: +author: +type: performance diff --git a/db/migrate/20171122131600_add_new_project_guidelines_to_appearances.rb b/db/migrate/20171122131600_add_new_project_guidelines_to_appearances.rb index f141c442d97..328cc65a549 100644 --- a/db/migrate/20171122131600_add_new_project_guidelines_to_appearances.rb +++ b/db/migrate/20171122131600_add_new_project_guidelines_to_appearances.rb @@ -4,6 +4,12 @@ class AddNewProjectGuidelinesToAppearances < ActiveRecord::Migration DOWNTIME = false def change + # Clears the current Appearance cache otherwise it breaks since + # new_project_guidelines_html would be missing. See + # https://gitlab.com/gitlab-org/gitlab-ce/issues/41041 + # We're not using Appearance#flush_redis_cache on purpose here. + Rails.cache.delete('current_appearance') + change_table :appearances do |t| t.text :new_project_guidelines t.text :new_project_guidelines_html diff --git a/doc/ci/docker/using_docker_images.md b/doc/ci/docker/using_docker_images.md index ecb8f15c851..fb5bfe26bb0 100644 --- a/doc/ci/docker/using_docker_images.md +++ b/doc/ci/docker/using_docker_images.md @@ -319,45 +319,62 @@ As you can see, the syntax of `command` is similar to [Dockerfile's `CMD`][cmd]. > Introduced in GitLab and GitLab Runner 9.4. Read more about the [extended configuration options](#extended-docker-configuration-options). +Before showing the available entrypoint override methods, let's describe shortly +how the Runner starts and uses a Docker image for the containers used in the +CI jobs: + +1. The Runner starts a Docker container using the defined entrypoint (default + from `Dockerfile` that may be overridden in `.gitlab-ci.yml`) +1. The Runner attaches itself to a running container. +1. The Runner prepares a script (the combination of + [`before_script`](../yaml/README.md#before_script), + [`script`](../yaml/README.md#script), + and [`after_script`](../yaml/README.md#after_script)). +1. The Runner sends the script to the container's shell STDIN and receives the + output. + +To override the entrypoint of a Docker image, the recommended solution is to +define an empty `entrypoint` in `.gitlab-ci.yml`, so the Runner doesn't start +a useless shell layer. However, that will not work for all Docker versions, and +you should check which one your Runner is using. Specifically: + +- If Docker 17.06 or later is used, the `entrypoint` can be set to an empty value. +- If Docker 17.03 or previous versions are used, the `entrypoint` can be set to + `/bin/sh -c`, `/bin/bash -c` or an equivalent shell available in the image. + +The syntax of `image:entrypoint` is similar to [Dockerfile's `ENTRYPOINT`][entrypoint]. + +---- + Let's assume you have a `super/sql:experimental` image with some SQL database inside it and you would like to use it as a base image for your job because you want to execute some tests with this database binary. Let's also assume that this image is configured with `/usr/bin/super-sql run` as an entrypoint. That -means, that when starting the container without additional options, it will run +means that when starting the container without additional options, it will run the database's process, while Runner expects that the image will have no -entrypoint or at least will start with a shell as its entrypoint. - -Before the new extended Docker configuration options, you would need to create -your own image based on the `super/sql:experimental` image, set the entrypoint -to a shell and then use it in job's configuration, like: +entrypoint or that the entrypoint is prepared to start a shell command. -```Dockerfile -# my-super-sql:experimental image's Dockerfile +With the extended Docker configuration options, instead of creating your +own image based on `super/sql:experimental`, setting the `ENTRYPOINT` +to a shell, and then using the new image in your CI job, you can now simply +define an `entrypoint` in `.gitlab-ci.yml`. -FROM super/sql:experimental -ENTRYPOINT ["/bin/sh"] -``` +**For Docker 17.06+:** ```yaml -# .gitlab-ci.yml - -image: my-super-sql:experimental +image: + name: super/sql:experimental + entrypoint: [""] ``` -After the new extended Docker configuration options, you can now simply -set an `entrypoint` in `.gitlab-ci.yml`, like: +**For Docker =< 17.03:** ```yaml -# .gitlab-ci.yml - image: name: super/sql:experimental - entrypoint: ["/bin/sh"] + entrypoint: ["/bin/sh", "-c"] ``` -As you can see the syntax of `entrypoint` is similar to -[Dockerfile's `ENTRYPOINT`][entrypoint]. - ## Define image and services in `config.toml` Look for the `[runners.docker]` section: diff --git a/doc/development/fe_guide/dropdowns.md b/doc/development/fe_guide/dropdowns.md index e1660ac5caa..6314f8f38d2 100644 --- a/doc/development/fe_guide/dropdowns.md +++ b/doc/development/fe_guide/dropdowns.md @@ -4,15 +4,15 @@ ## How to style a bootstrap dropdown 1. Use the HTML structure provided by the [docs][bootstrap-dropdowns] 1. Add a specific class to the top level `.dropdown` element - - + + ```Haml .dropdown.my-dropdown %button{ type: 'button', data: { toggle: 'dropdown' }, 'aria-haspopup': true, 'aria-expanded': false } %span.dropdown-toggle-text Toggle Dropdown = icon('chevron-down') - + %ul.dropdown-menu %li %a @@ -29,10 +29,4 @@ item! ``` -1. Include the mixin in CSS - - ```SCSS - @include new-style-dropdown('.my-dropdown '); - ``` - [bootstrap-dropdowns]: https://getbootstrap.com/docs/3.3/javascript/#dropdowns diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index d76ea259301..b5c3f74a113 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -138,7 +138,8 @@ You can create a new merge request by sending an email to a user-specific email address. The address can be obtained on the merge requests page by clicking on a **Email a new merge request to this project** button. The subject will be used as the source branch name for the new merge request and the target branch -will be the default branch for the project. +will be the default branch for the project. The message body (if not empty) +will be used as the merge request description. ## Revert changes diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index fb28e80ff73..b9099ce256a 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -19,6 +19,8 @@ module Gitlab commit_message: commit_message || default_commit_message } resolver.resolve_conflicts(user, files, args) + ensure + @merge_request.clear_memoized_shas end def files diff --git a/lib/gitlab/email/handler/create_merge_request_handler.rb b/lib/gitlab/email/handler/create_merge_request_handler.rb index c63666b98c1..e2f7c1d0257 100644 --- a/lib/gitlab/email/handler/create_merge_request_handler.rb +++ b/lib/gitlab/email/handler/create_merge_request_handler.rb @@ -55,11 +55,13 @@ module Gitlab end def merge_request_params - { + params = { source_project_id: project.id, source_branch: mail.subject, target_project_id: project.id } + params[:description] = message if message.present? + params end end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index fbdd646ea13..0e0a1987c7d 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1073,12 +1073,17 @@ module Gitlab end end - def write_ref(ref_path, ref) + def write_ref(ref_path, ref, force: false) raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ') raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00") - input = "update #{ref_path}\x00#{ref}\x00\x00" - run_git!(%w[update-ref --stdin -z]) { |stdin| stdin.write(input) } + ref = "refs/heads/#{ref}" unless ref.start_with?("refs") || ref =~ /\A[a-f0-9]+\z/i + + rugged.references.create(ref_path, ref, force: force) + rescue Rugged::ReferenceError => ex + raise GitError, "could not create ref #{ref_path}: #{ex}" + rescue Rugged::OSError => ex + raise GitError, "could not create ref #{ref_path}: #{ex}" end def fetch_ref(source_repository, source_ref:, target_ref:) diff --git a/lib/gitlab/utils/strong_memoize.rb b/lib/gitlab/utils/strong_memoize.rb index a2ac9285b56..fe091f4611b 100644 --- a/lib/gitlab/utils/strong_memoize.rb +++ b/lib/gitlab/utils/strong_memoize.rb @@ -11,6 +11,8 @@ module Gitlab # # We could write it like: # + # include Gitlab::Utils::StrongMemoize + # # def trigger_from_token # strong_memoize(:trigger) do # Ci::Trigger.find_by_token(params[:token].to_s) @@ -18,14 +20,22 @@ module Gitlab # end # def strong_memoize(name) - ivar_name = "@#{name}" - - if instance_variable_defined?(ivar_name) - instance_variable_get(ivar_name) + if instance_variable_defined?(ivar(name)) + instance_variable_get(ivar(name)) else - instance_variable_set(ivar_name, yield) + instance_variable_set(ivar(name), yield) end end + + def clear_memoization(name) + remove_instance_variable(ivar(name)) if instance_variable_defined?(ivar(name)) + end + + private + + def ivar(name) + "@#{name}" + end end end end diff --git a/lib/gitlab/view/presenter/factory.rb b/lib/gitlab/view/presenter/factory.rb index d172d61e2c9..570f0723e39 100644 --- a/lib/gitlab/view/presenter/factory.rb +++ b/lib/gitlab/view/presenter/factory.rb @@ -16,7 +16,7 @@ module Gitlab attr_reader :subject, :attributes def presenter_class - "#{subject.class.name}Presenter".constantize + attributes.delete(:presenter_class) { "#{subject.class.name}Presenter".constantize } end end end @@ -9,6 +9,7 @@ module QA autoload :User, 'qa/runtime/user' autoload :Namespace, 'qa/runtime/namespace' autoload :Scenario, 'qa/runtime/scenario' + autoload :Browser, 'qa/runtime/browser' end ## @@ -69,7 +70,6 @@ module QA autoload :Base, 'qa/page/base' module Main - autoload :Entry, 'qa/page/main/entry' autoload :Login, 'qa/page/main/login' autoload :Menu, 'qa/page/main/menu' autoload :OAuth, 'qa/page/main/oauth' diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index f9a93ef051e..99eba02b6e3 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -10,6 +10,18 @@ module QA visit current_url end + def wait(css = '.application', time: 60) + Time.now.tap do |start| + while Time.now - start < time + break if page.has_css?(css, wait: 5) + + refresh + end + end + + yield if block_given? + end + def scroll_to(selector, text: nil) page.execute_script <<~JS var elements = Array.from(document.querySelectorAll('#{selector}')); @@ -24,6 +36,10 @@ module QA page.within(selector) { yield } if block_given? end + + def self.path + raise NotImplementedError + end end end end diff --git a/qa/qa/page/main/entry.rb b/qa/qa/page/main/entry.rb deleted file mode 100644 index ae6484b4bfe..00000000000 --- a/qa/qa/page/main/entry.rb +++ /dev/null @@ -1,26 +0,0 @@ -module QA - module Page - module Main - class Entry < Page::Base - def visit_login_page - visit("#{Runtime::Scenario.gitlab_address}/users/sign_in") - wait_for_instance_to_be_ready - end - - private - - def wait_for_instance_to_be_ready - # This resolves cold boot / background tasks problems - # - start = Time.now - - while Time.now - start < 240 - break if page.has_css?('.application', wait: 10) - - refresh - end - end - end - end - end -end diff --git a/qa/qa/page/main/login.rb b/qa/qa/page/main/login.rb index 8b0111a78a2..f88325f408b 100644 --- a/qa/qa/page/main/login.rb +++ b/qa/qa/page/main/login.rb @@ -2,6 +2,10 @@ module QA module Page module Main class Login < Page::Base + def initialize + wait('.application', time: 500) + end + def sign_in_using_credentials if page.has_content?('Change your password') fill_in :user_password, with: Runtime::User.password @@ -13,6 +17,10 @@ module QA fill_in :user_password, with: Runtime::User.password click_button 'Sign in' end + + def self.path + '/users/sign_in' + end end end end diff --git a/qa/qa/page/mattermost/login.rb b/qa/qa/page/mattermost/login.rb index 42ab9c6f675..8ffd4fdad13 100644 --- a/qa/qa/page/mattermost/login.rb +++ b/qa/qa/page/mattermost/login.rb @@ -2,10 +2,6 @@ module QA module Page module Mattermost class Login < Page::Base - def initialize - visit(Runtime::Scenario.mattermost_address + '/login') - end - def sign_in_using_oauth click_link class: 'btn btn-custom-login gitlab' @@ -13,6 +9,10 @@ module QA click_button 'Authorize' end end + + def self.path + '/login' + end end end end diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb new file mode 100644 index 00000000000..6fb37fdfc7f --- /dev/null +++ b/qa/qa/runtime/browser.rb @@ -0,0 +1,109 @@ +require 'rspec/core' +require 'capybara/rspec' +require 'capybara-screenshot/rspec' +require 'selenium-webdriver' + +module QA + module Runtime + class Browser + include QA::Scenario::Actable + + def initialize + self.class.configure! + end + + ## + # Visit a page that belongs to a GitLab instance under given address. + # + # Example: + # + # visit(:gitlab, Page::Main::Login) + # visit('http://gitlab.example/users/sign_in') + # + # In case of an address that is a symbol we will try to guess address + # based on `Runtime::Scenario#something_address`. + # + def visit(address, page, &block) + Browser::Session.new(address, page).tap do |session| + session.perform(&block) + end + end + + def self.visit(address, page, &block) + new.visit(address, page, &block) + end + + def self.configure! + return if Capybara.drivers.include?(:chrome) + + Capybara.register_driver :chrome do |app| + capabilities = Selenium::WebDriver::Remote::Capabilities.chrome( + 'chromeOptions' => { + 'args' => %w[headless no-sandbox disable-gpu window-size=1280,1680] + } + ) + + Capybara::Selenium::Driver + .new(app, browser: :chrome, desired_capabilities: capabilities) + end + + Capybara::Screenshot.register_driver(:chrome) do |driver, path| + driver.browser.save_screenshot(path) + end + + Capybara.configure do |config| + config.default_driver = :chrome + config.javascript_driver = :chrome + config.default_max_wait_time = 10 + # https://github.com/mattheworiordan/capybara-screenshot/issues/164 + config.save_path = 'tmp' + end + end + + class Session + include Capybara::DSL + + def initialize(instance, page = nil) + @instance = instance + @address = host + page&.path + end + + def host + if @instance.is_a?(Symbol) + Runtime::Scenario.send("#{@instance}_address") + else + @instance.to_s + end + end + + def perform(&block) + visit(@address) + + yield if block_given? + rescue + raise if block.nil? + + # RSpec examples will take care of screenshots on their own + # + unless block.binding.receiver.is_a?(RSpec::Core::ExampleGroup) + screenshot_and_save_page + end + + raise + ensure + clear! if block_given? + end + + ## + # Selenium allows to reset session cookies for current domain only. + # + # See gitlab-org/gitlab-qa#102 + # + def clear! + visit(@address) + reset_session! + end + end + end + end +end diff --git a/qa/qa/scenario/entrypoint.rb b/qa/qa/scenario/entrypoint.rb index b9d924651a0..ae099fd911e 100644 --- a/qa/qa/scenario/entrypoint.rb +++ b/qa/qa/scenario/entrypoint.rb @@ -8,7 +8,6 @@ module QA include Bootable def perform(address, *files) - Specs::Config.act { configure_capybara! } Runtime::Scenario.define(:gitlab_address, address) ## diff --git a/qa/qa/scenario/gitlab/admin/hashed_storage.rb b/qa/qa/scenario/gitlab/admin/hashed_storage.rb index ac2cd549829..44604c6bc66 100644 --- a/qa/qa/scenario/gitlab/admin/hashed_storage.rb +++ b/qa/qa/scenario/gitlab/admin/hashed_storage.rb @@ -6,7 +6,6 @@ module QA def perform(*traits) raise ArgumentError unless traits.include?(:enabled) - Page::Main::Entry.act { visit_login_page } Page::Main::Login.act { sign_in_using_credentials } Page::Main::Menu.act { go_to_admin_area } Page::Admin::Menu.act { go_to_settings } diff --git a/qa/qa/specs/config.rb b/qa/qa/specs/config.rb deleted file mode 100644 index bce7923e52d..00000000000 --- a/qa/qa/specs/config.rb +++ /dev/null @@ -1,63 +0,0 @@ -require 'rspec/core' -require 'capybara/rspec' -require 'capybara-screenshot/rspec' -require 'selenium-webdriver' - -# rubocop:disable Metrics/MethodLength -# rubocop:disable Metrics/LineLength - -module QA - module Specs - class Config < Scenario::Template - include Scenario::Actable - - def perform - configure_rspec! - configure_capybara! - end - - def configure_rspec! - RSpec.configure do |config| - config.expect_with :rspec do |expectations| - expectations.include_chain_clauses_in_custom_matcher_descriptions = true - end - - config.mock_with :rspec do |mocks| - mocks.verify_partial_doubles = true - end - - config.order = :random - Kernel.srand config.seed - config.formatter = :documentation - config.color = true - end - end - - def configure_capybara! - Capybara.register_driver :chrome do |app| - capabilities = Selenium::WebDriver::Remote::Capabilities.chrome( - 'chromeOptions' => { - 'args' => %w[headless no-sandbox disable-gpu window-size=1280,1680] - } - ) - - Capybara::Selenium::Driver - .new(app, browser: :chrome, desired_capabilities: capabilities) - end - - Capybara::Screenshot.register_driver(:chrome) do |driver, path| - driver.browser.save_screenshot(path) - end - - Capybara.configure do |config| - config.default_driver = :chrome - config.javascript_driver = :chrome - config.default_max_wait_time = 10 - - # https://github.com/mattheworiordan/capybara-screenshot/issues/164 - config.save_path = 'tmp' - end - end - end - end -end diff --git a/qa/qa/specs/features/login/standard_spec.rb b/qa/qa/specs/features/login/standard_spec.rb index b155708c387..9eaa2b772e6 100644 --- a/qa/qa/specs/features/login/standard_spec.rb +++ b/qa/qa/specs/features/login/standard_spec.rb @@ -1,7 +1,7 @@ module QA - feature 'standard root login', :core do + feature 'standard user login', :core do scenario 'user logs in using credentials' do - Page::Main::Entry.act { visit_login_page } + Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.act { sign_in_using_credentials } # TODO, since `Signed in successfully` message was removed diff --git a/qa/qa/specs/features/mattermost/group_create_spec.rb b/qa/qa/specs/features/mattermost/group_create_spec.rb index 853a9a6a4f4..b3dbe44bf6e 100644 --- a/qa/qa/specs/features/mattermost/group_create_spec.rb +++ b/qa/qa/specs/features/mattermost/group_create_spec.rb @@ -1,7 +1,7 @@ module QA feature 'create a new group', :mattermost do scenario 'creating a group with a mattermost team' do - Page::Main::Entry.act { visit_login_page } + Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.act { sign_in_using_credentials } Page::Main::Menu.act { go_to_groups } diff --git a/qa/qa/specs/features/mattermost/login_spec.rb b/qa/qa/specs/features/mattermost/login_spec.rb index 1fde3f89a99..637bbdd643a 100644 --- a/qa/qa/specs/features/mattermost/login_spec.rb +++ b/qa/qa/specs/features/mattermost/login_spec.rb @@ -1,24 +1,17 @@ module QA feature 'logging in to Mattermost', :mattermost do scenario 'can use gitlab oauth' do - Page::Main::Entry.act { visit_login_page } - Page::Main::Login.act { sign_in_using_credentials } - Page::Mattermost::Login.act { sign_in_using_oauth } + Runtime::Browser.visit(:gitlab, Page::Main::Login) do + Page::Main::Login.act { sign_in_using_credentials } - Page::Mattermost::Main.perform do |page| - expect(page).to have_content(/(Welcome to: Mattermost|Logout GitLab Mattermost)/) - end - end + Runtime::Browser.visit(:mattermost, Page::Mattermost::Login) do + Page::Mattermost::Login.act { sign_in_using_oauth } - ## - # TODO, temporary workaround for gitlab-org/gitlab-qa#102. - # - after do - visit Runtime::Scenario.mattermost_address - reset_session! - - visit Runtime::Scenario.gitlab_address - reset_session! + Page::Mattermost::Main.perform do |page| + expect(page).to have_content(/(Welcome to: Mattermost|Logout GitLab Mattermost)/) + end + end + end end end end diff --git a/qa/qa/specs/features/project/create_spec.rb b/qa/qa/specs/features/project/create_spec.rb index aba0c2b4c14..0b3accb848d 100644 --- a/qa/qa/specs/features/project/create_spec.rb +++ b/qa/qa/specs/features/project/create_spec.rb @@ -1,7 +1,7 @@ module QA feature 'create a new project', :core do scenario 'user creates a new project' do - Page::Main::Entry.act { visit_login_page } + Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.act { sign_in_using_credentials } Scenario::Gitlab::Project::Create.perform do |project| diff --git a/qa/qa/specs/features/repository/clone_spec.rb b/qa/qa/specs/features/repository/clone_spec.rb index 5cc3b3b9c1b..c5c24622657 100644 --- a/qa/qa/specs/features/repository/clone_spec.rb +++ b/qa/qa/specs/features/repository/clone_spec.rb @@ -9,7 +9,7 @@ module QA end before do - Page::Main::Entry.act { visit_login_page } + Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.act { sign_in_using_credentials } Scenario::Gitlab::Project::Create.perform do |scenario| diff --git a/qa/qa/specs/features/repository/push_spec.rb b/qa/qa/specs/features/repository/push_spec.rb index 5b930b9818a..ef29dfa2d2f 100644 --- a/qa/qa/specs/features/repository/push_spec.rb +++ b/qa/qa/specs/features/repository/push_spec.rb @@ -2,7 +2,7 @@ module QA feature 'push code to repository', :core do context 'with regular account over http' do scenario 'user pushes code to the repository' do - Page::Main::Entry.act { visit_login_page } + Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.act { sign_in_using_credentials } Scenario::Gitlab::Project::Create.perform do |scenario| diff --git a/qa/qa/specs/runner.rb b/qa/qa/specs/runner.rb index f98b8f88e9a..3f7b75df986 100644 --- a/qa/qa/specs/runner.rb +++ b/qa/qa/specs/runner.rb @@ -17,7 +17,7 @@ module QA tags.to_a.each { |tag| args.push(['-t', tag.to_s]) } args.push(files) - Specs::Config.perform + Runtime::Browser.configure! RSpec::Core::Runner.run(args.flatten, $stderr, $stdout).tap do |status| abort if status.nonzero? diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 280b7e4d8b9..a3b13647c92 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -27,13 +27,6 @@ describe Projects::ClustersController do expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster]) end - it 'assigns counters to correct values' do - go - - expect(assigns(:active_count)).to eq(1) - expect(assigns(:inactive_count)).to eq(1) - end - context 'when page is specified' do let(:last_page) { project.clusters.page.total_pages } @@ -48,20 +41,6 @@ describe Projects::ClustersController do expect(assigns(:clusters).current_page).to eq(last_page) end end - - context 'when only enabled clusters are requested' do - it 'returns only enabled clusters' do - get :index, namespace_id: project.namespace, project_id: project, scope: 'active' - expect(assigns(:clusters)).to all(have_attributes(enabled: true)) - end - end - - context 'when only disabled clusters are requested' do - it 'returns only disabled clusters' do - get :index, namespace_id: project.namespace, project_id: project, scope: 'inactive' - expect(assigns(:clusters)).to all(have_attributes(enabled: false)) - end - end end context 'when project does not have a cluster' do @@ -74,13 +53,6 @@ describe Projects::ClustersController do expect(response).to render_template(:index, partial: :empty_state) expect(assigns(:clusters)).to eq([]) end - - it 'assigns counters to zero' do - go - - expect(assigns(:active_count)).to eq(0) - expect(assigns(:inactive_count)).to eq(0) - end end end diff --git a/spec/features/groups/labels/user_sees_links_to_issuables.rb b/spec/features/groups/labels/user_sees_links_to_issuables.rb new file mode 100644 index 00000000000..5d6290d2109 --- /dev/null +++ b/spec/features/groups/labels/user_sees_links_to_issuables.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +feature 'Groups > Labels > User sees links to issuables' do + set(:group) { create(:group, :public) } + + before do + create(:group_label, group: group, title: 'bug') + visit group_labels_path(group) + end + + scenario 'shows links to MRs and issues' do + expect(page).to have_link('view merge requests') + expect(page).to have_link('view open issues') + end +end diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index 008bdf2044b..93929bf6814 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -35,17 +35,6 @@ feature 'Clusters', :js do expect(page).to have_selector('.gl-responsive-table-row', count: 2) end - it 'user sees navigation tabs' do - expect(page.find('.js-active-tab').text).to include('Active') - expect(page.find('.js-active-tab .badge').text).to include('1') - - expect(page.find('.js-inactive-tab').text).to include('Inactive') - expect(page.find('.js-inactive-tab .badge').text).to include('0') - - expect(page.find('.js-all-tab').text).to include('All') - expect(page.find('.js-all-tab .badge').text).to include('1') - end - context 'inline update of cluster' do it 'user can update cluster' do expect(page).to have_selector('.js-toggle-cluster-list') diff --git a/spec/features/projects/labels/user_sees_links_to_issuables.rb b/spec/features/projects/labels/user_sees_links_to_issuables.rb new file mode 100644 index 00000000000..aa56fd7f74e --- /dev/null +++ b/spec/features/projects/labels/user_sees_links_to_issuables.rb @@ -0,0 +1,75 @@ +require 'spec_helper' + +feature 'Projects > Labels > User sees links to issuables' do + set(:user) { create(:user) } + + before do + label # creates the label + project.add_developer(user) + sign_in user + visit project_labels_path(project) + end + + context 'with a project label' do + let(:label) { create(:label, project: project, title: 'bug') } + + context 'when merge requests and issues are enabled for the project' do + let(:project) { create(:project, :public) } + + scenario 'shows links to MRs and issues' do + expect(page).to have_link('view merge requests') + expect(page).to have_link('view open issues') + end + end + + context 'when issues are disabled for the project' do + let(:project) { create(:project, :public, issues_access_level: ProjectFeature::DISABLED) } + + scenario 'shows links to MRs but not to issues' do + expect(page).to have_link('view merge requests') + expect(page).not_to have_link('view open issues') + end + end + + context 'when merge requests are disabled for the project' do + let(:project) { create(:project, :public, merge_requests_access_level: ProjectFeature::DISABLED) } + + scenario 'shows links to issues but not to MRs' do + expect(page).not_to have_link('view merge requests') + expect(page).to have_link('view open issues') + end + end + end + + context 'with a group label' do + set(:group) { create(:group) } + let(:label) { create(:group_label, group: group, title: 'bug') } + + context 'when merge requests and issues are enabled for the project' do + let(:project) { create(:project, :public, namespace: group) } + + scenario 'shows links to MRs and issues' do + expect(page).to have_link('view merge requests') + expect(page).to have_link('view open issues') + end + end + + context 'when issues are disabled for the project' do + let(:project) { create(:project, :public, namespace: group, issues_access_level: ProjectFeature::DISABLED) } + + scenario 'shows links to MRs and issues' do + expect(page).to have_link('view merge requests') + expect(page).to have_link('view open issues') + end + end + + context 'when merge requests are disabled for the project' do + let(:project) { create(:project, :public, namespace: group, merge_requests_access_level: ProjectFeature::DISABLED) } + + scenario 'shows links to MRs and issues' do + expect(page).to have_link('view merge requests') + expect(page).to have_link('view open issues') + end + end + end +end diff --git a/spec/fixtures/emails/valid_new_merge_request.eml b/spec/fixtures/emails/valid_new_merge_request.eml index 480675a6d7e..729df674604 100644 --- a/spec/fixtures/emails/valid_new_merge_request.eml +++ b/spec/fixtures/emails/valid_new_merge_request.eml @@ -16,3 +16,5 @@ X-Sieve: CMU Sieve 2.2 X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, 13 Jun 2013 14:03:48 -0700 (PDT) X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + +Merge request description diff --git a/spec/fixtures/emails/valid_new_merge_request_no_description.eml b/spec/fixtures/emails/valid_new_merge_request_no_description.eml new file mode 100644 index 00000000000..480675a6d7e --- /dev/null +++ b/spec/fixtures/emails/valid_new_merge_request_no_description.eml @@ -0,0 +1,18 @@ +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +To: incoming+gitlabhq/gitlabhq+merge-request+auth_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: feature +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 4ac4302adfd..0286d36952c 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -1,6 +1,69 @@ require 'spec_helper' describe LabelsHelper do + describe '#show_label_issuables_link?' do + shared_examples 'a valid response to show_label_issuables_link?' do |issuables_type, when_enabled = true, when_disabled = false| + let(:context_project) { project } + + context "when asking for a #{issuables_type} link" do + subject { show_label_issuables_link?(label, issuables_type, project: context_project) } + + context "when #{issuables_type} are enabled for the project" do + let(:project) { create(:project, "#{issuables_type}_access_level": ProjectFeature::ENABLED) } + + it { is_expected.to be(when_enabled) } + end + + context "when #{issuables_type} are disabled for the project" do + let(:project) { create(:project, :public, "#{issuables_type}_access_level": ProjectFeature::DISABLED) } + + it { is_expected.to be(when_disabled) } + end + end + end + + context 'with a project label' do + let(:label) { create(:label, project: project, title: 'bug') } + + context 'when asking for an issue link' do + it_behaves_like 'a valid response to show_label_issuables_link?', :issues + end + + context 'when asking for a merge requests link' do + it_behaves_like 'a valid response to show_label_issuables_link?', :merge_requests + end + end + + context 'with a group label' do + set(:group) { create(:group) } + let(:label) { create(:group_label, group: group, title: 'bug') } + + context 'when asking for an issue link' do + context 'in the context of a project' do + it_behaves_like 'a valid response to show_label_issuables_link?', :issues, true, true + end + + context 'in the context of a group' do + let(:context_project) { nil } + + it_behaves_like 'a valid response to show_label_issuables_link?', :issues, true, true + end + end + + context 'when asking for a merge requests link' do + context 'in the context of a project' do + it_behaves_like 'a valid response to show_label_issuables_link?', :merge_requests, true, true + end + + context 'in the context of a group' do + let(:context_project) { nil } + + it_behaves_like 'a valid response to show_label_issuables_link?', :merge_requests, true, true + end + end + end + end + describe 'link_to_label' do let(:project) { create(:project) } let(:label) { create(:label, project: project) } diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb index 33186cf50d5..45ffbeb27a4 100644 --- a/spec/helpers/members_helper_spec.rb +++ b/spec/helpers/members_helper_spec.rb @@ -1,14 +1,6 @@ require 'spec_helper' describe MembersHelper do - describe '#action_member_permission' do - let(:project_member) { build(:project_member) } - let(:group_member) { build(:group_member) } - - it { expect(action_member_permission(:admin, project_member)).to eq :admin_project_member } - it { expect(action_member_permission(:admin, group_member)).to eq :admin_group_member } - end - describe '#remove_member_message' do let(:requester) { create(:user) } let(:project) { create(:project, :public, :access_requestable) } diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb index b80df6956b0..be45c00dfe6 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -182,13 +182,22 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq do end context 'for a pre-Markdown Note attachment file path' do - class Note < ActiveRecord::Base - has_many :uploads, as: :model, dependent: :destroy + let(:model) { create(:note, :with_attachment) } + let!(:expected_upload_attrs) { Upload.where(model_type: 'Note', model_id: model.id).first.attributes.slice('path', 'uploader', 'size', 'checksum') } + let!(:untracked_file) { untracked_files_for_uploads.create!(path: expected_upload_attrs['path']) } + + before do + Upload.where(model_type: 'Note', model_id: model.id).delete_all end - let(:model) { create(:note, :with_attachment) } + # Can't use the shared example because Note doesn't have an `uploads` association + it 'creates an Upload record' do + expect do + subject.perform(1, untracked_files_for_uploads.last.id) + end.to change { Upload.where(model_type: 'Note', model_id: model.id).count }.from(0).to(1) - it_behaves_like 'non_markdown_file' + expect(Upload.where(model_type: 'Note', model_id: model.id).first.attributes).to include(expected_upload_attrs) + end end context 'for a user avatar file path' do diff --git a/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb index 51ce3116880..dc1a93367a4 100644 --- a/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb @@ -49,6 +49,7 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do expect(merge_request.author).to eq(user) expect(merge_request.source_branch).to eq('feature') expect(merge_request.title).to eq('Feature added') + expect(merge_request.description).to eq('Merge request description') expect(merge_request.target_branch).to eq(project.default_branch) end end @@ -79,6 +80,17 @@ describe Gitlab::Email::Handler::CreateMergeRequestHandler do expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidMergeRequestError) end end + + context "when the message body is blank" do + let(:email_raw) { fixture_file("emails/valid_new_merge_request_no_description.eml") } + + it "creates a new merge request with description set from the last commit" do + expect { receiver.execute }.to change { project.merge_requests.count }.by(1) + merge_request = project.merge_requests.last + + expect(merge_request.description).to eq('Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>') + end + end end end end diff --git a/spec/lib/gitlab/utils/strong_memoize_spec.rb b/spec/lib/gitlab/utils/strong_memoize_spec.rb index 4a104ab6d97..473f8100771 100644 --- a/spec/lib/gitlab/utils/strong_memoize_spec.rb +++ b/spec/lib/gitlab/utils/strong_memoize_spec.rb @@ -49,4 +49,16 @@ describe Gitlab::Utils::StrongMemoize do end end end + + describe '#clear_memoization' do + let(:value) { 'mepmep' } + + it 'removes the instance variable' do + object.method_name + + object.clear_memoization(:method_name) + + expect(object.instance_variable_defined?(:@method_name)).to be(false) + end + end end diff --git a/spec/lib/gitlab/view/presenter/factory_spec.rb b/spec/lib/gitlab/view/presenter/factory_spec.rb index 70d2e22b48f..6120bafb2e3 100644 --- a/spec/lib/gitlab/view/presenter/factory_spec.rb +++ b/spec/lib/gitlab/view/presenter/factory_spec.rb @@ -27,5 +27,13 @@ describe Gitlab::View::Presenter::Factory do expect(presenter).to be_a(Ci::BuildPresenter) end + + it 'uses the presenter_class if given on #initialize' do + MyCustomPresenter = Class.new(described_class) + + presenter = described_class.new(build, presenter_class: MyCustomPresenter).fabricate! + + expect(presenter).to be_a(MyCustomPresenter) + end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index a6258676767..c5e23532aa5 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -132,11 +132,9 @@ describe Ci::Build do end describe '#artifacts?' do - context 'when new artifacts are used' do - let(:build) { create(:ci_build, :artifacts) } - - subject { build.artifacts? } + subject { build.artifacts? } + context 'when new artifacts are used' do context 'artifacts archive does not exist' do let(:build) { create(:ci_build) } @@ -144,25 +142,19 @@ describe Ci::Build do end context 'artifacts archive exists' do + let(:build) { create(:ci_build, :artifacts) } + it { is_expected.to be_truthy } context 'is expired' do - let!(:build) { create(:ci_build, :artifacts, :expired) } + let(:build) { create(:ci_build, :artifacts, :expired) } it { is_expected.to be_falsy } end - - context 'is not expired' do - it { is_expected.to be_truthy } - end end end context 'when legacy artifacts are used' do - let(:build) { create(:ci_build, :legacy_artifacts) } - - subject { build.artifacts? } - context 'artifacts archive does not exist' do let(:build) { create(:ci_build) } @@ -170,17 +162,15 @@ describe Ci::Build do end context 'artifacts archive exists' do + let(:build) { create(:ci_build, :legacy_artifacts) } + it { is_expected.to be_truthy } context 'is expired' do - let!(:build) { create(:ci_build, :legacy_artifacts, :expired) } + let(:build) { create(:ci_build, :legacy_artifacts, :expired) } it { is_expected.to be_falsy } end - - context 'is not expired' do - it { is_expected.to be_truthy } - end end end end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 129dfa07f15..3c7f578975b 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -102,6 +102,26 @@ describe CacheMarkdownField do it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } end + context 'when a markdown field is set repeatedly to an empty string' do + it do + expect(thing).to receive(:refresh_markdown_cache).once + thing.foo = '' + thing.save + thing.foo = '' + thing.save + end + end + + context 'when a markdown field is set repeatedly to a string which renders as empty html' do + it do + expect(thing).to receive(:refresh_markdown_cache).once + thing.foo = '[//]: # (This is also a comment.)' + thing.save + thing.foo = '[//]: # (This is also a comment.)' + thing.save + end + end + context 'a non-markdown field changed' do before do thing.bar = 'OK' diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 30a5a3bbff7..6b98d013ded 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -124,6 +124,7 @@ describe MergeRequest do context 'when the target branch does not exist' do before do project.repository.rm_branch(subject.author, subject.target_branch) + subject.clear_memoized_shas end it 'returns nil' do @@ -733,7 +734,7 @@ describe MergeRequest do before do project.repository.raw_repository.delete_branch(subject.target_branch) - subject.reload + subject.clear_memoized_shas end it 'does not crash' do @@ -1468,6 +1469,7 @@ describe MergeRequest do context 'when the target branch does not exist' do before do subject.project.repository.rm_branch(subject.author, subject.target_branch) + subject.clear_memoized_shas end it 'returns nil' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f4699fd243d..dd9e8498519 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1863,10 +1863,11 @@ describe Project do project.change_head(project.default_branch) end - it 'creates the new reference with rugged' do - expect(project.repository.rugged.references).to receive(:create).with('HEAD', + it 'creates the new reference' do + expect(project.repository.raw_repository).to receive(:write_ref).with('HEAD', "refs/heads/#{project.default_branch}", force: true) + project.change_head(project.default_branch) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 358bc3dfb94..129fce74f45 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1916,6 +1916,23 @@ describe Repository do File.delete(path) end + + it "attempting to call keep_around when exists a lock does not fail" do + ref = repository.send(:keep_around_ref_name, sample_commit.id) + path = File.join(repository.path, ref) + lock_path = "#{path}.lock" + + FileUtils.mkdir_p(File.dirname(path)) + File.open(lock_path, 'w') { |f| f.write('') } + + begin + expect { repository.keep_around(sample_commit.id) }.not_to raise_error(Gitlab::Git::Repository::GitError) + + expect(File.exist?(lock_path)).to be_falsey + ensure + File.delete(path) + end + end end describe '#update_ref' do diff --git a/spec/presenters/group_member_presenter_spec.rb b/spec/presenters/group_member_presenter_spec.rb new file mode 100644 index 00000000000..c00e41725d9 --- /dev/null +++ b/spec/presenters/group_member_presenter_spec.rb @@ -0,0 +1,138 @@ +require 'spec_helper' + +describe GroupMemberPresenter do + let(:user) { double(:user) } + let(:group) { double(:group) } + let(:group_member) { double(:group_member, source: group) } + let(:presenter) { described_class.new(group_member, current_user: user) } + + describe '#can_resend_invite?' do + context 'when group_member is invited' do + before do + expect(group_member).to receive(:invite?).and_return(true) + end + + context 'and user can admin_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :admin_group_member, group).and_return(true) + end + + it { expect(presenter.can_resend_invite?).to eq(true) } + end + + context 'and user cannot admin_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :admin_group_member, group).and_return(false) + end + + it { expect(presenter.can_resend_invite?).to eq(false) } + end + end + + context 'when group_member is not invited' do + before do + expect(group_member).to receive(:invite?).and_return(false) + end + + context 'and user can admin_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :admin_group_member, group).and_return(true) + end + + it { expect(presenter.can_resend_invite?).to eq(false) } + end + + context 'and user cannot admin_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :admin_group_member, group).and_return(false) + end + + it { expect(presenter.can_resend_invite?).to eq(false) } + end + end + end + + describe '#can_update?' do + context 'when user can update_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_group_member, presenter).and_return(true) + end + + it { expect(presenter.can_update?).to eq(true) } + end + + context 'when user cannot update_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_group_member, presenter).and_return(false) + allow(presenter).to receive(:can?).with(user, :override_group_member, presenter).and_return(false) + end + + it { expect(presenter.can_update?).to eq(false) } + end + end + + describe '#can_remove?' do + context 'when user can destroy_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :destroy_group_member, presenter).and_return(true) + end + + it { expect(presenter.can_remove?).to eq(true) } + end + + context 'when user cannot destroy_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :destroy_group_member, presenter).and_return(false) + end + + it { expect(presenter.can_remove?).to eq(false) } + end + end + + describe '#can_approve?' do + context 'when group_member has request an invite' do + before do + expect(group_member).to receive(:request?).and_return(true) + end + + context 'when user can update_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_group_member, presenter).and_return(true) + end + + it { expect(presenter.can_approve?).to eq(true) } + end + + context 'when user cannot update_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_group_member, presenter).and_return(false) + allow(presenter).to receive(:can?).with(user, :override_group_member, presenter).and_return(false) + end + + it { expect(presenter.can_approve?).to eq(false) } + end + end + + context 'when group_member did not request an invite' do + before do + expect(group_member).to receive(:request?).and_return(false) + end + + context 'when user can update_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_group_member, presenter).and_return(true) + end + + it { expect(presenter.can_approve?).to eq(false) } + end + + context 'when user cannot update_group_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_group_member, presenter).and_return(false) + end + + it { expect(presenter.can_approve?).to eq(false) } + end + end + end +end diff --git a/spec/presenters/project_member_presenter_spec.rb b/spec/presenters/project_member_presenter_spec.rb new file mode 100644 index 00000000000..83db5c56cdf --- /dev/null +++ b/spec/presenters/project_member_presenter_spec.rb @@ -0,0 +1,138 @@ +require 'spec_helper' + +describe ProjectMemberPresenter do + let(:user) { double(:user) } + let(:project) { double(:project) } + let(:project_member) { double(:project_member, source: project) } + let(:presenter) { described_class.new(project_member, current_user: user) } + + describe '#can_resend_invite?' do + context 'when project_member is invited' do + before do + expect(project_member).to receive(:invite?).and_return(true) + end + + context 'and user can admin_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :admin_project_member, project).and_return(true) + end + + it { expect(presenter.can_resend_invite?).to eq(true) } + end + + context 'and user cannot admin_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :admin_project_member, project).and_return(false) + end + + it { expect(presenter.can_resend_invite?).to eq(false) } + end + end + + context 'when project_member is not invited' do + before do + expect(project_member).to receive(:invite?).and_return(false) + end + + context 'and user can admin_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :admin_project_member, project).and_return(true) + end + + it { expect(presenter.can_resend_invite?).to eq(false) } + end + + context 'and user cannot admin_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :admin_project_member, project).and_return(false) + end + + it { expect(presenter.can_resend_invite?).to eq(false) } + end + end + end + + describe '#can_update?' do + context 'when user can update_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(true) + end + + it { expect(presenter.can_update?).to eq(true) } + end + + context 'when user cannot update_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(false) + allow(presenter).to receive(:can?).with(user, :override_project_member, presenter).and_return(false) + end + + it { expect(presenter.can_update?).to eq(false) } + end + end + + describe '#can_remove?' do + context 'when user can destroy_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :destroy_project_member, presenter).and_return(true) + end + + it { expect(presenter.can_remove?).to eq(true) } + end + + context 'when user cannot destroy_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :destroy_project_member, presenter).and_return(false) + end + + it { expect(presenter.can_remove?).to eq(false) } + end + end + + describe '#can_approve?' do + context 'when project_member has request an invite' do + before do + expect(project_member).to receive(:request?).and_return(true) + end + + context 'and user can update_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(true) + end + + it { expect(presenter.can_approve?).to eq(true) } + end + + context 'and user cannot update_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(false) + allow(presenter).to receive(:can?).with(user, :override_project_member, presenter).and_return(false) + end + + it { expect(presenter.can_approve?).to eq(false) } + end + end + + context 'when project_member did not request an invite' do + before do + expect(project_member).to receive(:request?).and_return(false) + end + + context 'and user can update_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(true) + end + + it { expect(presenter.can_approve?).to eq(false) } + end + + context 'and user cannot update_project_member' do + before do + allow(presenter).to receive(:can?).with(user, :update_project_member, presenter).and_return(false) + end + + it { expect(presenter.can_approve?).to eq(false) } + end + end + end +end |