diff options
68 files changed, 806 insertions, 364 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a4cad70345..7da8f839a8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 9.5.2 (2017-08-28) + +- [FIXED] Fix signing in using LDAP when attribute mapping uses simple strings instead of arrays. +- [FIXED] Show un-highlighted text diffs when we do not have references to the correct blobs. +- [FIXED] Fix display of push events for removed refs. +- [FIXED] Testing of some integrations were broken due to missing ServiceHook record. +- [FIXED] Fire system hooks when a user is created via LDAP. +- [FIXED] Fix new project form not resetting the template value. + ## 9.5.1 (2017-08-23) - [FIXED] Fix merge request pipeline status when pipeline has errors. !13664 @@ -1 +1 @@ -9.5.1 +9.5.2 diff --git a/app/assets/javascripts/fly_out_nav.js b/app/assets/javascripts/fly_out_nav.js index adf397ca0fe..883ed63541d 100644 --- a/app/assets/javascripts/fly_out_nav.js +++ b/app/assets/javascripts/fly_out_nav.js @@ -1,21 +1,51 @@ +import Cookies from 'js-cookie'; import bp from './breakpoints'; -let headerHeight = 50; -let sidebar; +const HIDE_INTERVAL_TIMEOUT = 300; +const IS_OVER_CLASS = 'is-over'; +const IS_ABOVE_CLASS = 'is-above'; +const IS_SHOWING_FLY_OUT_CLASS = 'is-showing-fly-out'; +let currentOpenMenu = null; +let menuCornerLocs; +let timeoutId; -export const setSidebar = (el) => { sidebar = el; }; +export const mousePos = []; -export const getHeaderHeight = () => headerHeight; +export const setOpenMenu = (menu = null) => { currentOpenMenu = menu; }; + +export const slope = (a, b) => (b.y - a.y) / (b.x - a.x); export const canShowActiveSubItems = (el) => { - if (el.classList.contains('active') && (sidebar && !sidebar.classList.contains('sidebar-icons-only'))) { - return false; + const isHiddenByMedia = bp.getBreakpointSize() === 'sm' || bp.getBreakpointSize() === 'md'; + + if (el.classList.contains('active') && !isHiddenByMedia) { + return Cookies.get('sidebar_collapsed') === 'true'; } return true; }; + export const canShowSubItems = () => bp.getBreakpointSize() === 'sm' || bp.getBreakpointSize() === 'md' || bp.getBreakpointSize() === 'lg'; +export const getHideSubItemsInterval = () => { + if (!currentOpenMenu) return 0; + + const currentMousePos = mousePos[mousePos.length - 1]; + const prevMousePos = mousePos[0]; + const currentMousePosY = currentMousePos.y; + const [menuTop, menuBottom] = menuCornerLocs; + + if (currentMousePosY < menuTop.y || + currentMousePosY > menuBottom.y) return 0; + + if (slope(prevMousePos, menuBottom) < slope(currentMousePos, menuBottom) && + slope(prevMousePos, menuTop) > slope(currentMousePos, menuTop)) { + return HIDE_INTERVAL_TIMEOUT; + } + + return 0; +}; + export const calculateTop = (boundingRect, outerHeight) => { const windowHeight = window.innerHeight; const bottomOverflow = windowHeight - (boundingRect.top + outerHeight); @@ -24,51 +54,118 @@ export const calculateTop = (boundingRect, outerHeight) => { boundingRect.top; }; -export const showSubLevelItems = (el) => { - const subItems = el.querySelector('.sidebar-sub-level-items'); +export const hideMenu = (el) => { + if (!el) return; - if (!subItems || !canShowSubItems() || !canShowActiveSubItems(el)) return; + const parentEl = el.parentNode; - subItems.style.display = 'block'; - el.classList.add('is-showing-fly-out'); - el.classList.add('is-over'); + el.style.display = ''; // eslint-disable-line no-param-reassign + el.style.transform = ''; // eslint-disable-line no-param-reassign + el.classList.remove(IS_ABOVE_CLASS); + parentEl.classList.remove(IS_OVER_CLASS); + parentEl.classList.remove(IS_SHOWING_FLY_OUT_CLASS); + + setOpenMenu(); +}; +export const moveSubItemsToPosition = (el, subItems) => { const boundingRect = el.getBoundingClientRect(); const top = calculateTop(boundingRect, subItems.offsetHeight); const isAbove = top < boundingRect.top; subItems.classList.add('fly-out-list'); - subItems.style.transform = `translate3d(0, ${Math.floor(top) - headerHeight}px, 0)`; + subItems.style.transform = `translate3d(0, ${Math.floor(top)}px, 0)`; // eslint-disable-line no-param-reassign + + const subItemsRect = subItems.getBoundingClientRect(); + + menuCornerLocs = [ + { + x: subItemsRect.left, // left position of the sub items + y: subItemsRect.top, // top position of the sub items + }, + { + x: subItemsRect.left, // left position of the sub items + y: subItemsRect.top + subItemsRect.height, // bottom position of the sub items + }, + ]; if (isAbove) { - subItems.classList.add('is-above'); + subItems.classList.add(IS_ABOVE_CLASS); } }; -export const hideSubLevelItems = (el) => { +export const showSubLevelItems = (el) => { + const subItems = el.querySelector('.sidebar-sub-level-items'); + + if (!canShowSubItems() || !canShowActiveSubItems(el)) return; + + el.classList.add(IS_OVER_CLASS); + + if (!subItems) return; + + subItems.style.display = 'block'; + el.classList.add(IS_SHOWING_FLY_OUT_CLASS); + + setOpenMenu(subItems); + moveSubItemsToPosition(el, subItems); +}; + +export const mouseEnterTopItems = (el) => { + clearTimeout(timeoutId); + + timeoutId = setTimeout(() => { + if (currentOpenMenu) hideMenu(currentOpenMenu); + + showSubLevelItems(el); + }, getHideSubItemsInterval()); +}; + +export const mouseLeaveTopItem = (el) => { const subItems = el.querySelector('.sidebar-sub-level-items'); - if (!subItems || !canShowSubItems() || !canShowActiveSubItems(el)) return; + if (!canShowSubItems() || !canShowActiveSubItems(el) || + (subItems && subItems === currentOpenMenu)) return; - el.classList.remove('is-showing-fly-out'); - el.classList.remove('is-over'); - subItems.style.display = ''; - subItems.style.transform = ''; - subItems.classList.remove('is-above'); + el.classList.remove(IS_OVER_CLASS); +}; + +export const documentMouseMove = (e) => { + mousePos.push({ + x: e.clientX, + y: e.clientY, + }); + + if (mousePos.length > 6) mousePos.shift(); }; export default () => { - const items = [...document.querySelectorAll('.sidebar-top-level-items > li')] - .filter(el => el.querySelector('.sidebar-sub-level-items')); + const sidebar = document.querySelector('.sidebar-top-level-items'); - sidebar = document.querySelector('.nav-sidebar'); + if (!sidebar) return; - if (sidebar) { - headerHeight = sidebar.offsetTop; + const items = [...sidebar.querySelectorAll('.sidebar-top-level-items > li')]; - items.forEach((el) => { - el.addEventListener('mouseenter', e => showSubLevelItems(e.currentTarget)); - el.addEventListener('mouseleave', e => hideSubLevelItems(e.currentTarget)); - }); - } + sidebar.addEventListener('mouseleave', () => { + clearTimeout(timeoutId); + + timeoutId = setTimeout(() => { + if (currentOpenMenu) hideMenu(currentOpenMenu); + }, getHideSubItemsInterval()); + }); + + items.forEach((el) => { + const subItems = el.querySelector('.sidebar-sub-level-items'); + + if (subItems) { + subItems.addEventListener('mouseleave', () => { + clearTimeout(timeoutId); + hideMenu(currentOpenMenu); + }); + } + + el.addEventListener('mouseenter', e => mouseEnterTopItems(e.currentTarget)); + el.addEventListener('mouseleave', e => mouseLeaveTopItem(e.currentTarget)); + }); + + document.addEventListener('mousemove', documentMouseMove); }; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 5f590d3078c..91638e1cb6d 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -13,6 +13,9 @@ img { /*max-width: 100%;*/ margin: 0 0 8px; + } + + img.lazy { min-width: 200px; min-height: 100px; background-color: $gray-lightest; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 3c109a5a929..496489f8d6c 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -118,6 +118,7 @@ $gl-text-color-quaternary: #d6d6d6; $gl-text-color-inverted: rgba(255, 255, 255, 1.0); $gl-text-color-secondary-inverted: rgba(255, 255, 255, .85); $gl-text-green: $green-600; +$gl-text-green-hover: $green-700; $gl-text-red: $red-500; $gl-text-orange: $orange-600; $gl-link-color: $blue-600; diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index 70e59392bcd..faedd207e01 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -97,29 +97,21 @@ $new-sidebar-collapsed-width: 50px; top: $header-height; bottom: 0; left: 0; + overflow: auto; background-color: $gray-normal; box-shadow: inset -2px 0 0 $border-color; - transform: translate3d(0, 0, 0); &.sidebar-icons-only { width: $new-sidebar-collapsed-width; overflow-x: hidden; - .nav-sidebar-inner-scroll { - overflow-x: hidden; - } - .badge, .project-title { display: none; } .nav-item-name { - display: none; - } - - .sidebar-top-level-items > li > a { - min-height: 44px; + opacity: 0; } } @@ -184,12 +176,6 @@ $new-sidebar-collapsed-width: 50px; } } -.nav-sidebar-inner-scroll { - height: 100%; - width: 100%; - overflow: auto; -} - .with-performance-bar .nav-sidebar { top: $header-height + $performance-bar-height; } @@ -264,32 +250,13 @@ $new-sidebar-collapsed-width: 50px; position: absolute; top: -30px; bottom: -30px; - left: 0; + left: -10px; right: -30px; z-index: -1; } - &::after { - content: ""; - position: absolute; - top: 44px; - left: -30px; - right: 35px; - bottom: 0; - height: 100%; - max-height: 150px; - z-index: -1; - transform: skew(33deg); - } - &.is-above { margin-top: 1px; - - &::after { - top: auto; - bottom: 44px; - transform: skew(-30deg); - } } > .active { @@ -336,8 +303,7 @@ $new-sidebar-collapsed-width: 50px; } } - &:not(.active):hover > a, - > a:hover, + &.active > a:hover, &.is-over > a { background-color: $white-light; } diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 46fbfe5f91e..d0d11b4d71c 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -286,6 +286,9 @@ .gpg-status-box { + padding: 2px 10px; + margin-right: $gl-padding; + &:empty { display: none; } @@ -314,7 +317,6 @@ &.valid { svg { border: 1px solid $brand-success; - fill: $brand-success; } } @@ -322,7 +324,6 @@ &.invalid { svg { border: 1px solid $common-gray-light; - fill: $common-gray-light; } } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 0a194f3707f..fbfe5d3c682 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -766,17 +766,25 @@ ul.notes { background-color: transparent; border: none; outline: 0; + transition: color $general-hover-transition-duration $general-hover-transition-curve; &.is-disabled { cursor: default; } - &:not(.is-disabled):hover, + &:not(.is-disabled) { + &:hover, + &:focus { + color: $gl-text-green; + } + } + &.is-active { color: $gl-text-green; - svg { - fill: $gl-text-green; + &:hover, + &:focus { + color: $gl-text-green-hover; } } diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index d54a1111f11..daa5c88aae0 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -4,7 +4,6 @@ class Projects::ServicesController < Projects::ApplicationController # Authorize before_action :authorize_admin_project! before_action :service, only: [:edit, :update, :test] - before_action :update_service, only: [:update, :test] respond_to :html @@ -14,6 +13,8 @@ class Projects::ServicesController < Projects::ApplicationController end def update + @service.attributes = service_params[:service] + if @service.save(context: :manual_change) redirect_to(project_settings_integrations_path(@project), notice: success_message) else @@ -24,7 +25,7 @@ class Projects::ServicesController < Projects::ApplicationController def test message = {} - if @service.can_test? + if @service.can_test? && @service.update_attributes(service_params[:service]) data = @service.test_data(project, current_user) outcome = @service.test(data) @@ -50,10 +51,6 @@ class Projects::ServicesController < Projects::ApplicationController end end - def update_service - @service.assign_attributes(service_params[:service]) - end - def service @service ||= @project.find_or_initialize_service(params[:id]) end diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index 4b51269533c..a4c226a6aad 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -12,11 +12,18 @@ module AvatarsHelper avatar_size = options[:size] || 16 user_name = options[:user].try(:name) || options[:user_name] avatar_url = options[:url] || avatar_icon(options[:user] || options[:user_email], avatar_size) - data_attributes = { container: 'body' } + has_tooltip = options[:has_tooltip].nil? ? true : options[:has_tooltip] + data_attributes = {} + css_class = %W[avatar s#{avatar_size}].push(*options[:css_class]) + + if has_tooltip + css_class.push('has-tooltip') + data_attributes = { container: 'body' } + end image_tag( avatar_url, - class: %W[avatar has-tooltip s#{avatar_size}].push(*options[:css_class]), + class: css_class, alt: "#{user_name}'s avatar", title: user_name, data: data_attributes, diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 69220a1c0f6..cdc8c1664f1 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -114,7 +114,7 @@ module CommitsHelper end def commit_signature_badge_classes(additional_classes) - %w(btn status-box gpg-status-box) + Array(additional_classes) + %w(btn gpg-status-box) + Array(additional_classes) end protected diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index b6c2ed5bb60..2d042b2e567 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -181,6 +181,7 @@ module EventsHelper end def event_commit_title(message) + message ||= '' (message.split("\n").first || "").truncate(70) rescue "--broken encoding" diff --git a/app/models/event.rb b/app/models/event.rb index 15ee170ca75..996768a267b 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -406,7 +406,7 @@ class Event < ActiveRecord::Base def body? if push? - push_with_commits? || rm_ref? + push_with_commits? elsif note? true else diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index 418b42d8f1d..e45a729ad5e 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -19,46 +19,45 @@ class NotificationRecipient @notification_setting ||= find_notification_setting end - def raw_notification_level - notification_setting&.level&.to_sym - end - def notification_level - # custom is treated the same as watch if it's enabled - otherwise it's - # set to :custom, meaning to send exactly when our type is :participating - # or :mention. - @notification_level ||= - case raw_notification_level - when :custom - if @custom_action && notification_setting&.event_enabled?(@custom_action) - :watch - else - :custom - end - else - raw_notification_level - end + @notification_level ||= notification_setting&.level&.to_sym end def notifiable? return false unless has_access? return false if own_activity? - return true if @type == :subscription - - return false if notification_level.nil? || notification_level == :disabled - - return %i[participating mention].include?(@type) if notification_level == :custom + # even users with :disabled notifications receive manual subscriptions + return !unsubscribed? if @type == :subscription - return false if %i[watch participating].include?(notification_level) && excluded_watcher_action? - - return false unless NotificationSetting.levels[notification_level] <= NotificationSetting.levels[@type] + return false unless suitable_notification_level? + # check this last because it's expensive + # nobody should receive notifications if they've specifically unsubscribed return false if unsubscribed? true end + def suitable_notification_level? + case notification_level + when :disabled, nil + false + when :custom + custom_enabled? || %i[participating mention].include?(@type) + when :watch, :participating + !excluded_watcher_action? + when :mention + @type == :mention + else + false + end + end + + def custom_enabled? + @custom_action && notification_setting&.event_enabled?(@custom_action) + end + def unsubscribed? return false unless @target return false unless @target.respond_to?(:subscriptions) @@ -88,7 +87,7 @@ class NotificationRecipient def excluded_watcher_action? return false unless @custom_action - return false if raw_notification_level == :custom + return false if notification_level == :custom NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action) end diff --git a/app/services/concerns/users/new_user_notifier.rb b/app/services/concerns/users/new_user_notifier.rb new file mode 100644 index 00000000000..231693ce7a9 --- /dev/null +++ b/app/services/concerns/users/new_user_notifier.rb @@ -0,0 +1,9 @@ +module Users + module NewUserNotifier + def notify_new_user(user, reset_token) + log_info("User \"#{user.name}\" (#{user.email}) was created") + notification_service.new_user(user, reset_token) if reset_token + system_hook_service.execute_hooks_for(user, :create) + end + end +end diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 21c9c314a2a..c9f07c140f7 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -95,7 +95,7 @@ module NotificationRecipientService def add_participants(user) return unless target.respond_to?(:participants) - self << [target.participants(user), :watch] + self << [target.participants(user), :participating] end # Get project/group users with CUSTOM notification level diff --git a/app/services/test_hooks/system_service.rb b/app/services/test_hooks/system_service.rb index 76c3c19bd74..67552edefc9 100644 --- a/app/services/test_hooks/system_service.rb +++ b/app/services/test_hooks/system_service.rb @@ -2,47 +2,16 @@ module TestHooks class SystemService < TestHooks::BaseService private - def project - @project ||= begin - project = Project.first - - throw(:validation_error, 'Ensure that at least one project exists.') unless project - - project - end - end - def push_events_data - if project.empty_repo? - throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.") - end - - Gitlab::DataBuilder::Push.build_sample(project, current_user) + Gitlab::DataBuilder::Push.sample_data end def tag_push_events_data - if project.repository.tags.empty? - throw(:validation_error, "Ensure project \"#{project.human_name}\" has tags.") - end - - Gitlab::DataBuilder::Push.build_sample(project, current_user) + Gitlab::DataBuilder::Push.sample_data end def repository_update_events_data - commit = project.commit - ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}" - - unless commit - throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.") - end - - change = Gitlab::DataBuilder::Repository.single_change( - commit.parent_id || Gitlab::Git::BLANK_SHA, - commit.id, - ref - ) - - Gitlab::DataBuilder::Repository.update(project, current_user, [change], [ref]) + Gitlab::DataBuilder::Repository.sample_data end end end diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index 74abc017cea..c8a3c461d60 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -1,5 +1,7 @@ module Users class CreateService < BaseService + include NewUserNotifier + def initialize(current_user, params = {}) @current_user = current_user @params = params.dup @@ -10,11 +12,7 @@ module Users @reset_token = user.generate_reset_token if user.recently_sent_password_reset? - if user.save - log_info("User \"#{user.name}\" (#{user.email}) was created") - notification_service.new_user(user, @reset_token) if @reset_token - system_hook_service.execute_hooks_for(user, :create) - end + notify_new_user(user, @reset_token) if user.save user end diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index dfbd6016c3f..2f9855273dc 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -1,5 +1,7 @@ module Users class UpdateService < BaseService + include NewUserNotifier + def initialize(user, params = {}) @user = user @params = params.dup @@ -10,7 +12,11 @@ module Users assign_attributes(&block) + user_exists = @user.persisted? + if @user.save(validate: validate) + notify_new_user(@user, nil) unless user_exists + success else error(@user.errors.full_messages.uniq.join('. ')) diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml index bf655f9d21a..e3c5fd55f08 100644 --- a/app/views/events/_event_push.atom.haml +++ b/app/views/events/_event_push.atom.haml @@ -5,9 +5,10 @@ %i at = event.created_at.to_s(:short) - %blockquote= markdown(escape_once(event.commit_title), pipeline: :atom, project: event.project, author: event.author) - - if event.commits_count > 1 - %p - %i - \... and - = pluralize(event.commits_count - 1, "more commit") + - unless event.rm_ref? + %blockquote= markdown(escape_once(event.commit_title), pipeline: :atom, project: event.project, author: event.author) + - if event.commits_count > 1 + %p + %i + \... and + = pluralize(event.commits_count - 1, "more commit") diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 973c652ad88..53ebdd6d2ff 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -41,7 +41,3 @@ %li.commits-stat = link_to create_mr_path(project.default_branch, event.ref_name, project) do Create Merge Request -- elsif event.rm_ref? - .event-body - %ul.well-list.event_commits - = render "events/commit", project: project, event: event diff --git a/app/views/layouts/nav/_new_admin_sidebar.html.haml b/app/views/layouts/nav/_new_admin_sidebar.html.haml index 3cbcd841aff..1c3fd4a082c 100644 --- a/app/views/layouts/nav/_new_admin_sidebar.html.haml +++ b/app/views/layouts/nav/_new_admin_sidebar.html.haml @@ -4,7 +4,7 @@ = link_to admin_root_path, title: 'Admin Overview' do .avatar-container.s40.settings-avatar = icon('wrench') - .project-title Admin Area + .sidebar-context-title Admin Area %ul.sidebar-top-level-items = nav_link(controller: %w(dashboard admin projects users groups jobs runners cohorts), html_options: {class: 'home'}) do = link_to admin_root_path, title: 'Overview', class: 'shortcuts-tree' do diff --git a/app/views/layouts/nav/_new_group_sidebar.html.haml b/app/views/layouts/nav/_new_group_sidebar.html.haml index ed5793f09fe..d90aea2e361 100644 --- a/app/views/layouts/nav/_new_group_sidebar.html.haml +++ b/app/views/layouts/nav/_new_group_sidebar.html.haml @@ -4,7 +4,7 @@ = link_to group_path(@group), title: @group.name do .avatar-container.s40.group-avatar = image_tag group_icon(@group), class: "avatar s40 avatar-tile" - .group-title + .sidebar-context-title = @group.name %ul.sidebar-top-level-items = nav_link(path: ['groups#show', 'groups#activity', 'groups#subgroups'], html_options: { class: 'home' }) do diff --git a/app/views/layouts/nav/_new_profile_sidebar.html.haml b/app/views/layouts/nav/_new_profile_sidebar.html.haml index 4234df56d1d..85b2c7630c8 100644 --- a/app/views/layouts/nav/_new_profile_sidebar.html.haml +++ b/app/views/layouts/nav/_new_profile_sidebar.html.haml @@ -4,7 +4,7 @@ = link_to profile_path, title: 'Profile Settings' do .avatar-container.s40.settings-avatar = icon('user') - .project-title User Settings + .sidebar-context-title User Settings %ul.sidebar-top-level-items = nav_link(path: 'profiles#show', html_options: {class: 'home'}) do = link_to profile_path, title: 'Profile Settings' do diff --git a/app/views/layouts/nav/_new_project_sidebar.html.haml b/app/views/layouts/nav/_new_project_sidebar.html.haml index 0ef81375c3a..9c7d9826f56 100644 --- a/app/views/layouts/nav/_new_project_sidebar.html.haml +++ b/app/views/layouts/nav/_new_project_sidebar.html.haml @@ -5,7 +5,7 @@ = link_to project_path(@project), title: @project.name do .avatar-container.s40.project-avatar = project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile') - .project-title + .sidebar-context-title = @project.name %ul.sidebar-top-level-items = nav_link(path: ['projects#show', 'projects#activity', 'cycle_analytics#show'], html_options: { class: 'home' }) do diff --git a/app/views/projects/_project_templates.html.haml b/app/views/projects/_project_templates.html.haml index 97cf13df070..5638b7da1b0 100644 --- a/app/views/projects/_project_templates.html.haml +++ b/app/views/projects/_project_templates.html.haml @@ -1,6 +1,6 @@ .project-templates-buttons.import-buttons{ data: { toggle: "buttons" } } .btn.blank-option.active - %input{ type: "radio", autocomplete: "off", name: "project_templates", id: "blank", checked: "true" } + %input{ type: "radio", autocomplete: "off", name: "project[template_name]", id: "blank", checked: "true", value: "" } = icon('file-o', class: 'btn-template-icon') Blank - Gitlab::ProjectTemplate.all.each do |template| diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 7e8a5a38086..1214aabe837 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -37,14 +37,14 @@ .commit-actions.hidden-xs - - if commit.status(ref) - = render_commit_status(commit, ref: ref) - - if request.xhr? = render partial: 'projects/commit/signature', object: commit.signature - else = render partial: 'projects/commit/ajax_signature', locals: { commit: commit } + - if commit.status(ref) + = render_commit_status(commit, ref: ref) + = link_to commit.short_id, project_commit_path(project, commit), class: "commit-sha btn btn-transparent" = clipboard_button(text: commit.id, title: _("Copy commit SHA to clipboard")) = link_to_browse_code(project, commit) diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index ad5befc6ee5..de0f1de057d 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -32,7 +32,8 @@ %ul - if can_update_issue %li= link_to 'Edit', edit_project_issue_path(@project, @issue) - - unless current_user == @issue.author + / TODO: simplify condition back #36860 + - if @issue.author && current_user != @issue.author %li= link_to 'Report abuse', new_abuse_report_path(user_id: @issue.author.id, ref_url: issue_url(@issue)) - if can_update_issue %li= link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, format: 'json'), class: "btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml index cb737d129f0..b04f5efe1f9 100644 --- a/app/views/projects/notes/_actions.html.haml +++ b/app/views/projects/notes/_actions.html.haml @@ -26,8 +26,12 @@ ":title" => "buttonText", ":ref" => "'button'" } - = icon('spin spinner', 'v-show' => 'loading', class: 'loading', 'aria-hidden' => 'true', 'aria-label' => 'Loading') - %div{ 'v-show' => '!loading' }= render 'shared/icons/icon_status_success.svg' + = icon('spin spinner', 'v-if' => 'loading', class: 'loading', 'aria-hidden' => 'true', 'aria-label' => 'Loading') + %div{ 'v-else' => '' } + %template{ 'v-if' => 'isResolved' } + = render 'shared/icons/icon_status_success_solid.svg' + %template{ 'v-else' => '' } + = render 'shared/icons/icon_status_success.svg' - if current_user - if note.emoji_awardable? diff --git a/app/views/shared/icons/_icon_status_success.svg b/app/views/shared/icons/_icon_status_success.svg index eed5006bebe..845562e9320 100755 --- a/app/views/shared/icons/_icon_status_success.svg +++ b/app/views/shared/icons/_icon_status_success.svg @@ -1 +1 @@ -<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><g fill-rule="evenodd"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z"/><path d="M13 7A6 6 0 1 0 1 7a6 6 0 0 0 12 0z" fill="#FFF"/><path d="M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z"/></g></svg> +<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z M13 7A6 6 0 1 0 1 7a6 6 0 0 0 12 0z" fill-rule="evenodd"/><path d="M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z"/></svg> diff --git a/app/views/shared/icons/_icon_status_success_solid.svg b/app/views/shared/icons/_icon_status_success_solid.svg new file mode 100644 index 00000000000..0aac6d933e1 --- /dev/null +++ b/app/views/shared/icons/_icon_status_success_solid.svg @@ -0,0 +1 @@ +<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><path d="M0 7a7 7 0 1 1 14 0A7 7 0 0 1 0 7z M6.278 7.697L5.045 6.464a.296.296 0 0 0-.42-.002l-.613.614a.298.298 0 0 0 .002.42l1.91 1.909a.5.5 0 0 0 .703.005l.265-.265L9.997 6.04a.291.291 0 0 0-.009-.408l-.614-.614a.29.29 0 0 0-.408-.009L6.278 7.697z" fill-rule="evenodd"/></svg> diff --git a/app/views/shared/issuable/_close_reopen_button.html.haml b/app/views/shared/issuable/_close_reopen_button.html.haml index 8a1268a1c6d..f22b6c9a6c2 100644 --- a/app/views/shared/issuable/_close_reopen_button.html.haml +++ b/app/views/shared/issuable/_close_reopen_button.html.haml @@ -9,6 +9,7 @@ class: "hidden-xs hidden-sm btn btn-grouped btn-reopen #{issuable_button_visibility(issuable, false)}", title: "Reopen #{display_issuable_type}" - elsif can_update && !is_current_user = render 'shared/issuable/close_reopen_report_toggle', issuable: issuable -- else +- elsif issuable.author + / TODO: change back to else #36860 = link_to 'Report abuse', new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)), class: 'hidden-xs hidden-sm btn btn-grouped btn-close-color', title: 'Report abuse' diff --git a/app/views/shared/issuable/_close_reopen_report_toggle.html.haml b/app/views/shared/issuable/_close_reopen_report_toggle.html.haml index 6756a7f17fd..daa05990ae9 100644 --- a/app/views/shared/issuable/_close_reopen_report_toggle.html.haml +++ b/app/views/shared/issuable/_close_reopen_report_toggle.html.haml @@ -37,13 +37,15 @@ %li.divider.droplab-item-ignore - %li.report-item{ data: { text: 'Report abuse', url: new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)), - button_class: "#{button_class} btn-close-color", toggle_class: "#{toggle_class} btn-close-color", method: '' } } - %button.btn.btn-transparent - = icon('check', class: 'icon') - .description - %strong.title Report abuse - %p.text - Report - = display_issuable_type.pluralize - that are abusive, inappropriate or spam. + / TODO: remove condition #36860 + - if issuable.author + %li.report-item{ data: { text: 'Report abuse', url: new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)), + button_class: "#{button_class} btn-close-color", toggle_class: "#{toggle_class} btn-close-color", method: '' } } + %button.btn.btn-transparent + = icon('check', class: 'icon') + .description + %strong.title Report abuse + %p.text + Report + = display_issuable_type.pluralize + that are abusive, inappropriate or spam. diff --git a/app/views/shared/issuable/_user_dropdown_item.html.haml b/app/views/shared/issuable/_user_dropdown_item.html.haml index c18e4975bb8..48d04678d47 100644 --- a/app/views/shared/issuable/_user_dropdown_item.html.haml +++ b/app/views/shared/issuable/_user_dropdown_item.html.haml @@ -4,7 +4,7 @@ %li.filter-dropdown-item{ class: ('js-current-user' if user == current_user) } %button.btn.btn-link.dropdown-user{ type: :button } .avatar-container.s40 - = user_avatar_without_link(user: user, lazy: avatar[:lazy], url: avatar[:url], size: 40).gsub('/images/{{avatar_url}}','{{avatar_url}}').html_safe + = user_avatar_without_link(user: user, lazy: avatar[:lazy], url: avatar[:url], size: 40, has_tooltip: false).gsub('/images/{{avatar_url}}','{{avatar_url}}').html_safe .dropdown-user-details %span = user.name diff --git a/changelogs/unreleased/36061-mr-ref.yml b/changelogs/unreleased/36061-mr-ref.yml new file mode 100644 index 00000000000..039666070a7 --- /dev/null +++ b/changelogs/unreleased/36061-mr-ref.yml @@ -0,0 +1,5 @@ +--- +title: Instrument MergeRequest#ensure_ref_fetched +merge_request: +author: +type: other diff --git a/changelogs/unreleased/36582-fix-note-resolved-icon.yml b/changelogs/unreleased/36582-fix-note-resolved-icon.yml new file mode 100644 index 00000000000..758c0ecd212 --- /dev/null +++ b/changelogs/unreleased/36582-fix-note-resolved-icon.yml @@ -0,0 +1,5 @@ +--- +title: Update and fix resolvable note icons for easier recognition +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/36860-deleted-user-fix.yml b/changelogs/unreleased/36860-deleted-user-fix.yml new file mode 100644 index 00000000000..79e75441134 --- /dev/null +++ b/changelogs/unreleased/36860-deleted-user-fix.yml @@ -0,0 +1,5 @@ +--- +title: Fix failure when issue is authored by a deleted user +merge_request: 13807 +author: +type: fixed diff --git a/changelogs/unreleased/bugfix-notify-custom-participants.yml b/changelogs/unreleased/bugfix-notify-custom-participants.yml new file mode 100644 index 00000000000..04fcb95e18a --- /dev/null +++ b/changelogs/unreleased/bugfix-notify-custom-participants.yml @@ -0,0 +1,5 @@ +--- +title: Fixed: Notifications weren't sending to participating users with a `Custom` notification setting. +merge_request: 13680 +author: jneen +type: fixed diff --git a/changelogs/unreleased/revert-appearances-description-html-not-null.yml b/changelogs/unreleased/revert-appearances-description-html-not-null.yml new file mode 100644 index 00000000000..4e3c39cb5fd --- /dev/null +++ b/changelogs/unreleased/revert-appearances-description-html-not-null.yml @@ -0,0 +1,5 @@ +--- +title: Re-allow appearances.description_html to be NULL +merge_request: +author: +type: fixed diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb index 2aeb94d47cd..370a976b64a 100644 --- a/config/initializers/8_metrics.rb +++ b/config/initializers/8_metrics.rb @@ -119,6 +119,9 @@ def instrument_classes(instrumentation) # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/30224#note_32306159 instrumentation.instrument_instance_method(MergeRequestDiff, :load_commits) + + # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/36061 + instrumentation.instrument_instance_method(MergeRequest, :ensure_ref_fetched) end # rubocop:enable Metrics/AbcSize diff --git a/db/migrate/20170809142252_cleanup_appearances_schema.rb b/db/migrate/20170809142252_cleanup_appearances_schema.rb index 90d12925ba2..acf45060114 100644 --- a/db/migrate/20170809142252_cleanup_appearances_schema.rb +++ b/db/migrate/20170809142252_cleanup_appearances_schema.rb @@ -7,7 +7,7 @@ class CleanupAppearancesSchema < ActiveRecord::Migration # Set this constant to true if this migration requires downtime. DOWNTIME = false - NOT_NULL_COLUMNS = %i[title description description_html created_at updated_at] + NOT_NULL_COLUMNS = %i[title description created_at updated_at] TIME_COLUMNS = %i[created_at updated_at] diff --git a/db/migrate/20170824162758_allow_appearances_description_html_null.rb b/db/migrate/20170824162758_allow_appearances_description_html_null.rb new file mode 100644 index 00000000000..d7f481ee894 --- /dev/null +++ b/db/migrate/20170824162758_allow_appearances_description_html_null.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AllowAppearancesDescriptionHtmlNull < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def up + change_column_null :appearances, :description_html, true + end + + def down + # This column should not have a `NOT NULL` class, so we don't want to revert + # back to re-adding it. + end +end diff --git a/db/schema.rb b/db/schema.rb index 2ccf52b7b74..3e2c407ddfc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170820100558) do +ActiveRecord::Schema.define(version: 20170824162758) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -34,7 +34,7 @@ ActiveRecord::Schema.define(version: 20170820100558) do t.string "logo" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.text "description_html", null: false + t.text "description_html" t.integer "cached_markdown_version" end diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index 5c5f507d44d..4ab5b3455a5 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -3,6 +3,35 @@ module Gitlab module Push extend self + SAMPLE_DATA = + { + object_kind: "push", + event_name: "push", + before: "95790bf891e76fee5e1747ab589903a6a1f80f22", + after: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + ref: "refs/heads/master", + checkout_sha: "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + message: "Hello World", + user_id: 4, + user_name: "John Smith", + user_email: "john@example.com", + user_avatar: "https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80", + project_id: 15, + commits: [ + { + id: "c5feabde2d8cd023215af4d2ceeb7a64839fc428", + message: "Add simple search to projects in public area", + timestamp: "2013-05-13T18:18:08+00:00", + url: "https://test.example.com/gitlab/gitlabhq/commit/c5feabde2d8cd023215af4d2ceeb7a64839fc428", + author: { + name: "Test User", + email: "test@example.com" + } + } + ], + total_commits_count: 1 + }.freeze + # Produce a hash of post-receive data # # data = { @@ -74,6 +103,10 @@ module Gitlab build(project, user, commits.last&.id, commits.first&.id, ref, commits) end + def sample_data + SAMPLE_DATA + end + private def checkout_sha(repository, newrev, ref) diff --git a/lib/gitlab/data_builder/repository.rb b/lib/gitlab/data_builder/repository.rb index b42dc052949..c9c13ec6487 100644 --- a/lib/gitlab/data_builder/repository.rb +++ b/lib/gitlab/data_builder/repository.rb @@ -3,6 +3,23 @@ module Gitlab module Repository extend self + SAMPLE_DATA = { + event_name: 'repository_update', + user_id: 10, + user_name: 'john.doe', + user_email: 'test@example.com', + user_avatar: 'http://example.com/avatar/user.png', + project_id: 40, + changes: [ + { + before: "8205ea8d81ce0c6b90fbe8280d118cc9fdad6130", + after: "4045ea7a3df38697b3730a20fb73c8bed8a3e69e", + ref: "refs/heads/master" + } + ], + "refs": ["refs/heads/master"] + }.freeze + # Produce a hash of post-receive data def update(project, user, changes, refs) { @@ -30,6 +47,10 @@ module Gitlab ref: ref } end + + def sample_data + SAMPLE_DATA + end end end end diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 6d7de52cb80..1dabd4ebdd0 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -186,7 +186,10 @@ module Gitlab end def content_changed? - old_blob && new_blob && old_blob.id != new_blob.id + return blobs_changed? if diff_refs + return false if new_file? || deleted_file? || renamed_file? + + text? && diff_lines.any? end def different_type? @@ -225,6 +228,10 @@ module Gitlab private + def blobs_changed? + old_blob && new_blob && old_blob.id != new_blob.id + end + def simple_viewer_class return DiffViewer::NotDiffable unless diffable? @@ -250,6 +257,8 @@ module Gitlab DiffViewer::Renamed elsif mode_changed? DiffViewer::ModeChanged + else + DiffViewer::NoPreview end end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 24238786080..6ac2e13a004 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -158,8 +158,8 @@ module Gitlab def raw_blame(revision, path) request = Gitaly::RawBlameRequest.new( repository: @gitaly_repo, - revision: revision, - path: path + revision: GitalyClient.encode(revision), + path: GitalyClient.encode(path) ) response = GitalyClient.call(@repository.storage, :commit_service, :raw_blame, request) diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb index 8867a91c244..cd7e4ca7b7e 100644 --- a/lib/gitlab/ldap/adapter.rb +++ b/lib/gitlab/ldap/adapter.rb @@ -73,7 +73,7 @@ module Gitlab private def user_options(field, value, limit) - options = { attributes: user_attributes } + options = { attributes: Gitlab::LDAP::Person.ldap_attributes(config).compact.uniq } options[:size] = limit if limit if field.to_sym == :dn @@ -99,10 +99,6 @@ module Gitlab filter end end - - def user_attributes - %W(#{config.uid} cn dn) + config.attributes['username'] + config.attributes['email'] - end end end end diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb index 43eb73250b7..2427d79276b 100644 --- a/lib/gitlab/ldap/person.rb +++ b/lib/gitlab/ldap/person.rb @@ -21,6 +21,15 @@ module Gitlab adapter.dn_matches_filter?(dn, AD_USER_DISABLED) end + def self.ldap_attributes(config) + [ + 'dn', # Used in `dn` + config.uid, # Used in `uid` + *config.attributes['name'], # Used in `name` + *config.attributes['email'] # Used in `email` + ] + end + def initialize(entry, provider) Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" } @entry = entry diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 4e9b0c09ff2..efba9cc7306 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -10,9 +10,6 @@ describe Projects::ServicesController do before do sign_in(user) project.team << [user, :master] - - controller.instance_variable_set(:@project, project) - controller.instance_variable_set(:@service, service) end describe '#test' do @@ -20,7 +17,7 @@ describe Projects::ServicesController do it 'renders 404' do allow_any_instance_of(Service).to receive(:can_test?).and_return(false) - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param expect(response).to have_http_status(404) end @@ -36,7 +33,7 @@ describe Projects::ServicesController do it 'returns success' do allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true) - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param expect(response.status).to eq(200) end @@ -45,7 +42,7 @@ describe Projects::ServicesController do it 'returns success' do expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client) - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params expect(response.status).to eq(200) end @@ -54,17 +51,42 @@ describe Projects::ServicesController do it 'returns success' do expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_return(hipchat_client) - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params expect(response.status).to eq(200) end + + context 'when service is configured for the first time' do + before do + allow_any_instance_of(ServiceHook).to receive(:execute).and_return(true) + end + + it 'persist the object' do + do_put + + expect(BuildkiteService.first).to be_present + end + + it 'creates the ServiceHook object' do + do_put + + expect(BuildkiteService.first.service_hook).to be_present + end + + def do_put + put :test, namespace_id: project.namespace, + project_id: project, + id: 'buildkite', + service: { 'active' => '1', 'push_events' => '1', token: 'token', 'project_url' => 'http://test.com' } + end + end end context 'failure' do it 'returns success status code and the error message' do expect(HipChat::Client).to receive(:new).with('hipchat_token_p', anything).and_raise('Bad test') - put :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: service_params + put :test, namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params expect(response.status).to eq(200) expect(JSON.parse(response.body)) @@ -77,7 +99,7 @@ describe Projects::ServicesController do context 'when param `active` is set to true' do it 'activates the service and redirects to integrations paths' do put :update, - namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: true } + namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: true } expect(response).to redirect_to(project_settings_integrations_path(project)) expect(flash[:notice]).to eq 'HipChat activated.' @@ -87,7 +109,7 @@ describe Projects::ServicesController do context 'when param `active` is set to false' do it 'does not activate the service but saves the settings' do put :update, - namespace_id: project.namespace.id, project_id: project.id, id: service.id, service: { active: false } + namespace_id: project.namespace, project_id: project, id: service.to_param, service: { active: false } expect(flash[:notice]).to eq 'HipChat settings saved, but not activated.' end diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb index 28b636f9359..c470cb7c716 100644 --- a/spec/features/issues/issue_detail_spec.rb +++ b/spec/features/issues/issue_detail_spec.rb @@ -40,4 +40,18 @@ feature 'Issue Detail', :js do end end end + + context 'when authored by a user who is later deleted' do + before do + issue.update_attribute(:author_id, nil) + sign_in(user) + visit project_issue_path(project, issue) + end + + it 'shows the issue' do + page.within('.issuable-details') do + expect(find('h2')).to have_content(issue.title) + end + end + end end diff --git a/spec/features/projects/user_creates_files_spec.rb b/spec/features/projects/user_creates_files_spec.rb index 4b78cc4fc53..3d335687510 100644 --- a/spec/features/projects/user_creates_files_spec.rb +++ b/spec/features/projects/user_creates_files_spec.rb @@ -56,11 +56,10 @@ describe 'User creates files' do find('.add-to-tree').click click_link('New file') + expect(page).to have_selector('.file-editor') end it 'creates and commit a new file', js: true do - expect(page).to have_selector('.file-editor') - execute_script("ace.edit('editor').setValue('*.rbca')") fill_in(:file_name, with: 'not_a_file.md') fill_in(:commit_message, with: 'New commit message', visible: true) diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index d16fcf21e45..4632c679972 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -28,7 +28,7 @@ describe AvatarsHelper do it 'displays user avatar' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: 'avatar has-tooltip s16 lazy', + class: 'avatar s16 has-tooltip lazy', alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: avatar_icon(user, 16) } @@ -41,7 +41,7 @@ describe AvatarsHelper do it 'uses provided css_class' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: "avatar has-tooltip s16 #{options[:css_class]} lazy", + class: "avatar s16 #{options[:css_class]} has-tooltip lazy", alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: avatar_icon(user, 16) } @@ -55,7 +55,7 @@ describe AvatarsHelper do it 'uses provided size' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: "avatar has-tooltip s#{options[:size]} lazy", + class: "avatar s#{options[:size]} has-tooltip lazy", alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: avatar_icon(user, options[:size]) } @@ -69,7 +69,7 @@ describe AvatarsHelper do it 'uses provided url' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: 'avatar has-tooltip s16 lazy', + class: 'avatar s16 has-tooltip lazy', alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: options[:url] } @@ -77,6 +77,36 @@ describe AvatarsHelper do end end + context 'with has_tooltip parameter' do + context 'with has_tooltip set to true' do + let(:options) { { user: user, has_tooltip: true } } + + it 'adds has-tooltip' do + is_expected.to eq image_tag( + LazyImageTagHelper.placeholder_image, + class: 'avatar s16 has-tooltip lazy', + alt: "#{user.name}'s avatar", + title: user.name, + data: { container: 'body', src: avatar_icon(user, 16) } + ) + end + end + + context 'with has_tooltip set to false' do + let(:options) { { user: user, has_tooltip: false } } + + it 'does not add has-tooltip or data container' do + is_expected.to eq image_tag( + LazyImageTagHelper.placeholder_image, + class: 'avatar s16 lazy', + alt: "#{user.name}'s avatar", + title: user.name, + data: { src: avatar_icon(user, 16) } + ) + end + end + end + context 'with user_name parameter' do let(:options) { { user_name: 'Tinky Winky', user_email: 'no@f.un' } } @@ -86,7 +116,7 @@ describe AvatarsHelper do it 'prefers user parameter' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: 'avatar has-tooltip s16 lazy', + class: 'avatar s16 has-tooltip lazy', alt: "#{user.name}'s avatar", title: user.name, data: { container: 'body', src: avatar_icon(user, 16) } @@ -97,7 +127,7 @@ describe AvatarsHelper do it 'uses user_name and user_email parameter if user is not present' do is_expected.to eq image_tag( LazyImageTagHelper.placeholder_image, - class: 'avatar has-tooltip s16 lazy', + class: 'avatar s16 has-tooltip lazy', alt: "#{options[:user_name]}'s avatar", title: options[:user_name], data: { container: 'body', src: avatar_icon(options[:user_email], 16) } diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index 4b72dbb7964..d5536fcb22b 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -106,5 +106,9 @@ describe EventsHelper do it "handles empty strings" do expect(helper.event_commit_title("")).to eq("") end + + it 'handles nil values' do + expect(helper.event_commit_title(nil)).to eq('') + end end end diff --git a/spec/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index e13c4629214..65a7459c5ed 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -1,11 +1,15 @@ +import Cookies from 'js-cookie'; import { calculateTop, - hideSubLevelItems, showSubLevelItems, canShowSubItems, canShowActiveSubItems, - getHeaderHeight, - setSidebar, + mouseEnterTopItems, + mouseLeaveTopItem, + setOpenMenu, + mousePos, + getHideSubItemsInterval, + documentMouseMove, } from '~/fly_out_nav'; import bp from '~/breakpoints'; @@ -19,11 +23,14 @@ describe('Fly out sidebar navigation', () => { document.body.appendChild(el); spyOn(bp, 'getBreakpointSize').and.callFake(() => breakpointSize); + + setOpenMenu(null); }); afterEach(() => { - el.remove(); + document.body.innerHTML = ''; breakpointSize = 'lg'; + mousePos.length = 0; }); describe('calculateTop', () => { @@ -50,61 +57,152 @@ describe('Fly out sidebar navigation', () => { }); }); - describe('hideSubLevelItems', () => { + describe('getHideSubItemsInterval', () => { beforeEach(() => { - el.innerHTML = '<div class="sidebar-sub-level-items"></div>'; + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: fixed; top: 0; left: 100px; height: 50px;"></div>'; }); - it('hides subitems', () => { - hideSubLevelItems(el); + it('returns 0 if currentOpenMenu is nil', () => { + expect( + getHideSubItemsInterval(), + ).toBe(0); + }); + + it('returns 0 when mouse above sub-items', () => { + showSubLevelItems(el); + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top, + }); + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top - 50, + }); expect( - el.querySelector('.sidebar-sub-level-items').style.display, - ).toBe(''); + getHideSubItemsInterval(), + ).toBe(0); }); - it('does not hude subitems on mobile', () => { - breakpointSize = 'xs'; + it('returns 0 when mouse is below sub-items', () => { + const subItems = el.querySelector('.sidebar-sub-level-items'); - hideSubLevelItems(el); + showSubLevelItems(el); + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top, + }); + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: (el.getBoundingClientRect().top - subItems.getBoundingClientRect().height) + 50, + }); expect( - el.querySelector('.sidebar-sub-level-items').style.display, - ).not.toBe('none'); + getHideSubItemsInterval(), + ).toBe(0); + }); + + it('returns 300 when mouse is moved towards sub-items', () => { + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top, + }); + showSubLevelItems(el); + documentMouseMove({ + clientX: el.getBoundingClientRect().left + 20, + clientY: el.getBoundingClientRect().top + 10, + }); + + expect( + getHideSubItemsInterval(), + ).toBe(300); }); + }); - it('removes is-over class', () => { + describe('mouseLeaveTopItem', () => { + beforeEach(() => { spyOn(el.classList, 'remove'); + }); - hideSubLevelItems(el); + it('removes is-over class if currentOpenMenu is null', () => { + mouseLeaveTopItem(el); expect( el.classList.remove, ).toHaveBeenCalledWith('is-over'); }); - it('removes is-above class from sub-items', () => { - const subItems = el.querySelector('.sidebar-sub-level-items'); + it('removes is-over class if currentOpenMenu is null & there are sub-items', () => { + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute;"></div>'; - spyOn(subItems.classList, 'remove'); + mouseLeaveTopItem(el); - hideSubLevelItems(el); + expect( + el.classList.remove, + ).toHaveBeenCalledWith('is-over'); + }); + + it('does not remove is-over class if currentOpenMenu is the passed in sub-items', () => { + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute;"></div>'; + + setOpenMenu(el.querySelector('.sidebar-sub-level-items')); + mouseLeaveTopItem(el); expect( - subItems.classList.remove, - ).toHaveBeenCalledWith('is-above'); + el.classList.remove, + ).not.toHaveBeenCalled(); }); + }); - it('does nothing if el has no sub-items', () => { - el.innerHTML = ''; + describe('mouseEnterTopItems', () => { + beforeEach(() => { + jasmine.clock().install(); - spyOn(el.classList, 'remove'); + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute; top: 0; left: 100px; height: 200px;"></div>'; + }); + + afterEach(() => { + jasmine.clock().uninstall(); + }); - hideSubLevelItems(el); + it('shows sub-items after 0ms if no menu is open', () => { + mouseEnterTopItems(el); expect( - el.classList.remove, - ).not.toHaveBeenCalledWith(); + getHideSubItemsInterval(), + ).toBe(0); + + jasmine.clock().tick(0); + + expect( + el.querySelector('.sidebar-sub-level-items').style.display, + ).toBe('block'); + }); + + it('shows sub-items after 300ms if a menu is currently open', () => { + documentMouseMove({ + clientX: el.getBoundingClientRect().left, + clientY: el.getBoundingClientRect().top, + }); + + setOpenMenu(el.querySelector('.sidebar-sub-level-items')); + + documentMouseMove({ + clientX: el.getBoundingClientRect().left + 20, + clientY: el.getBoundingClientRect().top + 10, + }); + + mouseEnterTopItems(el); + + expect( + getHideSubItemsInterval(), + ).toBe(300); + + jasmine.clock().tick(300); + + expect( + el.querySelector('.sidebar-sub-level-items').style.display, + ).toBe('block'); }); }); @@ -133,7 +231,7 @@ describe('Fly out sidebar navigation', () => { ).not.toBe('block'); }); - it('does not shows sub-items', () => { + it('shows sub-items', () => { showSubLevelItems(el); expect( @@ -147,7 +245,7 @@ describe('Fly out sidebar navigation', () => { expect( subItems.style.transform, - ).toBe(`translate3d(0px, ${Math.floor(el.getBoundingClientRect().top) - getHeaderHeight()}px, 0px)`); + ).toBe(`translate3d(0px, ${Math.floor(el.getBoundingClientRect().top)}px, 0px)`); }); it('sets is-above when element is above', () => { @@ -183,7 +281,7 @@ describe('Fly out sidebar navigation', () => { describe('canShowActiveSubItems', () => { afterEach(() => { - setSidebar(null); + Cookies.remove('sidebar_collapsed'); }); it('returns true by default', () => { @@ -192,23 +290,36 @@ describe('Fly out sidebar navigation', () => { ).toBeTruthy(); }); - it('returns false when active & expanded sidebar', () => { - const sidebar = document.createElement('div'); + it('returns false when cookie is false & element is active', () => { + Cookies.set('sidebar_collapsed', 'false'); el.classList.add('active'); - setSidebar(sidebar); - expect( canShowActiveSubItems(el), ).toBeFalsy(); }); - it('returns true when active & collapsed sidebar', () => { - const sidebar = document.createElement('div'); - sidebar.classList.add('sidebar-icons-only'); + it('returns true when cookie is false & element is active', () => { + Cookies.set('sidebar_collapsed', 'true'); el.classList.add('active'); - setSidebar(sidebar); + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + + it('returns true when element is active & breakpoint is sm', () => { + breakpointSize = 'sm'; + el.classList.add('active'); + + expect( + canShowActiveSubItems(el), + ).toBeTruthy(); + }); + + it('returns true when element is active & breakpoint is md', () => { + breakpointSize = 'md'; + el.classList.add('active'); expect( canShowActiveSubItems(el), diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index d3d841b0668..c91895cedc3 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -15,6 +15,17 @@ describe Gitlab::Diff::File do it { expect(diff_lines.first).to be_kind_of(Gitlab::Diff::Line) } end + describe '#highlighted_diff_lines' do + it 'highlights the diff and memoises the result' do + expect(Gitlab::Diff::Highlight).to receive(:new) + .with(diff_file, repository: project.repository) + .once + .and_call_original + + diff_file.highlighted_diff_lines + end + end + describe '#mode_changed?' do it { expect(diff_file.mode_changed?).to be_falsey } end @@ -122,8 +133,20 @@ describe Gitlab::Diff::File do let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } let(:diff_file) { commit.diffs.diff_file_with_new_path('files/images/6049019_460s.jpg') } - it 'returns true' do - expect(diff_file.content_changed?).to be_truthy + context 'when the blobs are different' do + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end + end + + context 'when there are no diff refs' do + before do + allow(diff_file).to receive(:diff_refs).and_return(nil) + end + + it 'returns false' do + expect(diff_file.content_changed?).to be_falsey + end end end @@ -131,8 +154,20 @@ describe Gitlab::Diff::File do let(:commit) { project.commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') } let(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') } - it 'returns true' do - expect(diff_file.content_changed?).to be_truthy + context 'when the blobs are different' do + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end + end + + context 'when there are no diff refs' do + before do + allow(diff_file).to receive(:diff_refs).and_return(nil) + end + + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end end end end @@ -270,6 +305,21 @@ describe Gitlab::Diff::File do expect(diff_file.simple_viewer).to be_a(DiffViewer::ModeChanged) end end + + context 'when no other conditions apply' do + before do + allow(diff_file).to receive(:content_changed?).and_return(false) + allow(diff_file).to receive(:new_file?).and_return(false) + allow(diff_file).to receive(:deleted_file?).and_return(false) + allow(diff_file).to receive(:renamed_file?).and_return(false) + allow(diff_file).to receive(:mode_changed?).and_return(false) + allow(diff_file).to receive(:raw_text?).and_return(false) + end + + it 'returns a No Preview viewer' do + expect(diff_file.simple_viewer).to be_a(DiffViewer::NoPreview) + end + end end describe '#rich_viewer' do diff --git a/spec/lib/gitlab/git/storage/forked_storage_check_spec.rb b/spec/lib/gitlab/git/storage/forked_storage_check_spec.rb index 12366151f44..c708b15853a 100644 --- a/spec/lib/gitlab/git/storage/forked_storage_check_spec.rb +++ b/spec/lib/gitlab/git/storage/forked_storage_check_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Git::Storage::ForkedStorageCheck, skip_database_cleaner: true do +describe Gitlab::Git::Storage::ForkedStorageCheck, broken_storage: true, skip_database_cleaner: true do let(:existing_path) do existing_path = TestEnv.repos_path FileUtils.mkdir_p(existing_path) diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index d17d440d833..d9ddb4326be 100644 --- a/spec/lib/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/ldap/adapter_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::LDAP::Adapter do expect(adapter).to receive(:ldap_search) do |arg| expect(arg[:filter].to_s).to eq('(uid=johndoe)') expect(arg[:base]).to eq('dc=example,dc=com') - expect(arg[:attributes]).to match(%w{uid cn dn uid userid sAMAccountName mail email userPrincipalName}) + expect(arg[:attributes]).to match(%w{dn uid cn mail email userPrincipalName}) end.and_return({}) adapter.users('uid', 'johndoe') @@ -26,7 +26,7 @@ describe Gitlab::LDAP::Adapter do expect(adapter).to receive(:ldap_search).with( base: 'uid=johndoe,ou=users,dc=example,dc=com', scope: Net::LDAP::SearchScope_BaseObject, - attributes: %w{uid cn dn uid userid sAMAccountName mail email userPrincipalName}, + attributes: %w{dn uid cn mail email userPrincipalName}, filter: nil ).and_return({}) @@ -63,7 +63,7 @@ describe Gitlab::LDAP::Adapter do it 'uses the right uid attribute when non-default' do stub_ldap_config(uid: 'sAMAccountName') expect(adapter).to receive(:ldap_search).with( - hash_including(attributes: %w{sAMAccountName cn dn uid userid sAMAccountName mail email userPrincipalName}) + hash_including(attributes: %w{dn sAMAccountName cn mail email userPrincipalName}) ).and_return({}) adapter.users('sAMAccountName', 'johndoe') diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index e36d7a1800c..d5a067c7914 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -8,6 +8,25 @@ describe Notify do include_context 'gitlab email notification' + set(:user) { create(:user) } + set(:current_user) { create(:user, email: "current@email.com") } + set(:assignee) { create(:user, email: 'assignee@example.com', name: 'John Doe') } + + set(:merge_request) do + create(:merge_request, source_project: project, + target_project: project, + author: current_user, + assignee: assignee, + description: 'Awesome description') + end + + set(:issue) do + create(:issue, author: current_user, + assignees: [assignee], + project: project, + description: 'My awesome description!') + end + def have_referable_subject(referable, reply: false) prefix = referable.project.name if referable.project prefix = "Re: #{prefix}" if reply @@ -19,8 +38,6 @@ describe Notify do context 'for a project' do describe 'items that are assignable, the email' do - let(:current_user) { create(:user, email: "current@email.com") } - let(:assignee) { create(:user, email: 'assignee@example.com', name: 'John Doe') } let(:previous_assignee) { create(:user, name: 'Previous Assignee') } shared_examples 'an assignee email' do @@ -36,9 +53,6 @@ describe Notify do end context 'for issues' do - let(:issue) { create(:issue, author: current_user, assignees: [assignee], project: project) } - let(:issue_with_description) { create(:issue, author: current_user, assignees: [assignee], project: project, description: 'My awesome description') } - describe 'that are new' do subject { described_class.new_issue_email(issue.assignees.first.id, issue.id) } @@ -56,6 +70,10 @@ describe Notify do end end + it 'contains the description' do + is_expected.to have_html_escaped_body_text issue.description + end + context 'when enabled email_author_in_body' do before do stub_application_setting(email_author_in_body: true) @@ -68,16 +86,6 @@ describe Notify do end end - describe 'that are new with a description' do - subject { described_class.new_issue_email(issue_with_description.assignees.first.id, issue_with_description.id) } - - it_behaves_like 'it should show Gmail Actions View Issue link' - - it 'contains the description' do - is_expected.to have_html_escaped_body_text issue_with_description.description - end - end - describe 'that have been reassigned' do subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id) } @@ -197,11 +205,6 @@ describe Notify do end context 'for merge requests' do - let(:project) { create(:project, :repository) } - let(:merge_author) { create(:user) } - let(:merge_request) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project) } - let(:merge_request_with_description) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project, description: 'My awesome description') } - describe 'that are new' do subject { described_class.new_merge_request_email(merge_request.assignee_id, merge_request.id) } @@ -221,6 +224,10 @@ describe Notify do end end + it 'contains the description' do + is_expected.to have_html_escaped_body_text merge_request.description + end + context 'when enabled email_author_in_body' do before do stub_application_setting(email_author_in_body: true) @@ -233,17 +240,6 @@ describe Notify do end end - describe 'that are new with a description' do - subject { described_class.new_merge_request_email(merge_request_with_description.assignee_id, merge_request_with_description.id) } - - it_behaves_like 'it should show Gmail Actions View Merge request link' - it_behaves_like "an unsubscribeable thread" - - it 'contains the description' do - is_expected.to have_html_escaped_body_text merge_request_with_description.description - end - end - describe 'that are reassigned' do subject { described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) } @@ -321,6 +317,7 @@ describe Notify do end describe 'that are merged' do + let(:merge_author) { create(:user) } subject { described_class.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) } it_behaves_like 'a multiple recipients email' @@ -348,8 +345,6 @@ describe Notify do end describe 'project was moved' do - let(:project) { create(:project) } - let(:user) { create(:user) } subject { described_class.project_was_moved_email(project.id, user.id, "gitlab/gitlab") } it_behaves_like 'an email sent from GitLab' @@ -371,7 +366,6 @@ describe Notify do end end - let(:user) { create(:user) } let(:project_member) do project.request_access(user) project.requesters.find_by(user_id: user.id) @@ -398,7 +392,6 @@ describe Notify do let(:group_owner) { create(:user) } let(:group) { create(:group).tap { |g| g.add_owner(group_owner) } } let(:project) { create(:project, :public, :access_requestable, namespace: group) } - let(:user) { create(:user) } let(:project_member) do project.request_access(user) project.requesters.find_by(user_id: user.id) @@ -424,7 +417,6 @@ describe Notify do describe 'project access denied' do let(:project) { create(:project, :public, :access_requestable) } - let(:user) { create(:user) } let(:project_member) do project.request_access(user) project.requesters.find_by(user_id: user.id) @@ -445,7 +437,6 @@ describe Notify do describe 'project access changed' do let(:owner) { create(:user, name: "Chang O'Keefe") } let(:project) { create(:project, :public, :access_requestable, namespace: owner.namespace) } - let(:user) { create(:user) } let(:project_member) { create(:project_member, project: project, user: user) } subject { described_class.member_access_granted_email('project', project_member.id) } @@ -474,7 +465,6 @@ describe Notify do end describe 'project invitation' do - let(:project) { create(:project) } let(:master) { create(:user).tap { |u| project.team << [u, :master] } } let(:project_member) { invite_to_project(project, inviter: master) } @@ -494,7 +484,6 @@ describe Notify do end describe 'project invitation accepted' do - let(:project) { create(:project) } let(:invited_user) { create(:user, name: 'invited user') } let(:master) { create(:user).tap { |u| project.team << [u, :master] } } let(:project_member) do @@ -519,7 +508,6 @@ describe Notify do end describe 'project invitation declined' do - let(:project) { create(:project) } let(:master) { create(:user).tap { |u| project.team << [u, :master] } } let(:project_member) do invitee = invite_to_project(project, inviter: master) @@ -582,7 +570,6 @@ describe Notify do end describe 'on a commit' do - let(:project) { create(:project, :repository) } let(:commit) { project.commit } before do @@ -607,7 +594,6 @@ describe Notify do end describe 'on a merge request' do - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:note_on_merge_request_path) { project_merge_request_path(project, merge_request, anchor: "note_#{note.id}") } before do @@ -632,7 +618,6 @@ describe Notify do end describe 'on an issue' do - let(:issue) { create(:issue, project: project) } let(:note_on_issue_path) { project_issue_path(project, issue, anchor: "note_#{note.id}") } before do @@ -658,7 +643,6 @@ describe Notify do end context 'items that are noteable, the email for a discussion note' do - let(:project) { create(:project, :repository) } let(:note_author) { create(:user, name: 'author_name') } before :each do @@ -722,7 +706,6 @@ describe Notify do end describe 'on a merge request' do - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: note_author) } let(:note_on_merge_request_path) { project_merge_request_path(project, merge_request, anchor: "note_#{note.id}") } @@ -749,7 +732,6 @@ describe Notify do end describe 'on an issue' do - let(:issue) { create(:issue, project: project) } let(:note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: note_author) } let(:note_on_issue_path) { project_issue_path(project, issue, anchor: "note_#{note.id}") } @@ -835,7 +817,6 @@ describe Notify do end describe 'on a merge request' do - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:note) { create(:diff_note_on_merge_request) } subject { described_class.note_merge_request_email(recipient.id, note.id) } @@ -848,9 +829,10 @@ describe Notify do end context 'for a group' do + set(:group) { create(:group) } + describe 'group access requested' do let(:group) { create(:group, :public, :access_requestable) } - let(:user) { create(:user) } let(:group_member) do group.request_access(user) group.requesters.find_by(user_id: user.id) @@ -870,8 +852,6 @@ describe Notify do end describe 'group access denied' do - let(:group) { create(:group) } - let(:user) { create(:user) } let(:group_member) do group.request_access(user) group.requesters.find_by(user_id: user.id) @@ -890,8 +870,6 @@ describe Notify do end describe 'group access changed' do - let(:group) { create(:group) } - let(:user) { create(:user) } let(:group_member) { create(:group_member, group: group, user: user) } subject { described_class.member_access_granted_email('group', group_member.id) } @@ -921,7 +899,6 @@ describe Notify do end describe 'group invitation' do - let(:group) { create(:group) } let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } let(:group_member) { invite_to_group(group, inviter: owner) } @@ -941,7 +918,6 @@ describe Notify do end describe 'group invitation accepted' do - let(:group) { create(:group) } let(:invited_user) { create(:user, name: 'invited user') } let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } let(:group_member) do @@ -966,7 +942,6 @@ describe Notify do end describe 'group invitation declined' do - let(:group) { create(:group) } let(:owner) { create(:user).tap { |u| group.add_user(u, Gitlab::Access::OWNER) } } let(:group_member) do invitee = invite_to_group(group, inviter: owner) @@ -1020,7 +995,6 @@ describe Notify do describe 'email on push for a created branch' do let(:example_site_path) { root_path } - let(:user) { create(:user) } let(:tree_path) { project_tree_path(project, "empty-branch") } subject { described_class.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/empty-branch', action: :create) } @@ -1046,7 +1020,6 @@ describe Notify do describe 'email on push for a created tag' do let(:example_site_path) { root_path } - let(:user) { create(:user) } let(:tree_path) { project_tree_path(project, "v1.0") } subject { described_class.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :create) } @@ -1072,7 +1045,6 @@ describe Notify do describe 'email on push for a deleted branch' do let(:example_site_path) { root_path } - let(:user) { create(:user) } subject { described_class.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :delete) } @@ -1094,7 +1066,6 @@ describe Notify do describe 'email on push for a deleted tag' do let(:example_site_path) { root_path } - let(:user) { create(:user) } subject { described_class.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) } @@ -1115,9 +1086,7 @@ describe Notify do end describe 'email on push with multiple commits' do - let(:project) { create(:project, :repository) } let(:example_site_path) { root_path } - let(:user) { create(:user) } let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, sample_image_commit.id, sample_commit.id) } let(:compare) { Compare.decorate(raw_compare, project) } let(:commits) { compare.commits } @@ -1209,9 +1178,7 @@ describe Notify do end describe 'email on push with a single commit' do - let(:project) { create(:project, :repository) } let(:example_site_path) { root_path } - let(:user) { create(:user) } let(:raw_compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, sample_commit.parent_id, sample_commit.id) } let(:compare) { Compare.decorate(raw_compare, project) } let(:commits) { compare.commits } @@ -1242,8 +1209,6 @@ describe Notify do end describe 'HTML emails setting' do - let(:project) { create(:project) } - let(:user) { create(:user) } let(:multipart_mail) { described_class.project_was_moved_email(project.id, user.id, "gitlab/gitlab") } context 'when disabled' do diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index ff3224dd298..f55c161c821 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -304,6 +304,50 @@ describe Event do end end + describe '#body?' do + let(:push_event) do + event = build(:push_event) + + allow(event).to receive(:push?).and_return(true) + + event + end + + it 'returns true for a push event with commits' do + allow(push_event).to receive(:push_with_commits?).and_return(true) + + expect(push_event).to be_body + end + + it 'returns false for a push event without a valid commit range' do + allow(push_event).to receive(:push_with_commits?).and_return(false) + + expect(push_event).not_to be_body + end + + it 'returns true for a Note event' do + event = build(:event) + + allow(event).to receive(:note?).and_return(true) + + expect(event).to be_body + end + + it 'returns true if the target responds to #title' do + event = build(:event) + + allow(event).to receive(:target).and_return(double(:target, title: 'foo')) + + expect(event).to be_body + end + + it 'returns false for a regular event without a target' do + event = build(:event) + + expect(event).not_to be_body + end + end + def create_push_event(project, user) event = create(:push_event, project: project, author: user) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 6af5c79135d..2740757fabc 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -126,7 +126,18 @@ describe NotificationService, :mailer do project.add_master(issue.author) project.add_master(assignee) project.add_master(note.author) - create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy') + + @u_custom_off = create_user_with_notification(:custom, 'custom_off') + project.add_guest(@u_custom_off) + + create( + :note_on_issue, + author: @u_custom_off, + noteable: issue, + project_id: issue.project_id, + note: 'i think @subscribed_participant should see this' + ) + update_custom_notification(:new_note, @u_guest_custom, resource: project) update_custom_notification(:new_note, @u_custom_global) end @@ -136,8 +147,7 @@ describe NotificationService, :mailer do add_users_with_subscription(note.project, issue) reset_delivered_emails! - # Ensure create SentNotification by noteable = issue 6 times, not noteable = note - expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times + expect(SentNotification).to receive(:record).with(issue, any_args).exactly(9).times notification.new_note(note) @@ -149,6 +159,7 @@ describe NotificationService, :mailer do should_email(@subscriber) should_email(@watcher_and_subscriber) should_email(@subscribed_participant) + should_email(@u_custom_off) should_not_email(@u_guest_custom) should_not_email(@u_guest_watcher) should_not_email(note.author) diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb index 00d89924766..a15708bf82f 100644 --- a/spec/services/test_hooks/system_service_spec.rb +++ b/spec/services/test_hooks/system_service_spec.rb @@ -7,7 +7,6 @@ describe TestHooks::SystemService do let(:project) { create(:project, :repository) } let(:hook) { create(:system_hook) } let(:service) { described_class.new(hook, current_user, trigger) } - let(:sample_data) { { data: 'sample' }} let(:success_result) { { status: :success, http_status: 200, message: 'ok' } } before do @@ -26,18 +25,11 @@ describe TestHooks::SystemService do context 'push_events' do let(:trigger) { 'push_events' } - it 'returns error message if not enough data' do - allow(project).to receive(:empty_repo?).and_return(true) - - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) - end - it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) - allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end @@ -45,18 +37,11 @@ describe TestHooks::SystemService do context 'tag_push_events' do let(:trigger) { 'tag_push_events' } - it 'returns error message if not enough data' do - allow(project.repository).to receive(:tags).and_return([]) - - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has tags." }) - end - it 'executes hook' do allow(project.repository).to receive(:tags).and_return(['tag']) - allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end @@ -64,17 +49,11 @@ describe TestHooks::SystemService do context 'repository_update_events' do let(:trigger) { 'repository_update_events' } - it 'returns error message if not enough data' do - allow(project).to receive(:commit).and_return(nil) - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) - end - it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) - allow(Gitlab::DataBuilder::Repository).to receive(:update).and_return(sample_data) + expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 343804e3de0..256eb0de73a 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -24,7 +24,10 @@ describe Users::UpdateService do describe '#execute!' do it 'updates the name' do - result = update_user(user, name: 'New Name') + service = described_class.new(user, name: 'New Name') + expect(service).not_to receive(:notify_new_user) + + result = service.execute! expect(result).to be true expect(user.name).to eq('New Name') @@ -36,6 +39,18 @@ describe Users::UpdateService do end.to raise_error(ActiveRecord::RecordInvalid) end + it 'fires system hooks when a new user is saved' do + system_hook_service = spy(:system_hook_service) + user = build(:user) + service = described_class.new(user, name: 'John Doe') + expect(service).to receive(:notify_new_user).and_call_original + expect(service).to receive(:system_hook_service).and_return(system_hook_service) + + service.execute + + expect(system_hook_service).to have_received(:execute_hooks_for).with(user, :create) + end + def update_user(user, opts) described_class.new(user, opts).execute! end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0ba6ed56314..d35c82b293c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -97,9 +97,15 @@ RSpec.configure do |config| reset_delivered_emails! end - if ENV['CI'] - config.around(:each) do |ex| - ex.run_with_retry retry: 2 + # Stub the `ForkedStorageCheck.storage_available?` method unless + # `:broken_storage` metadata is defined + # + # This check can be slow and is unnecessary in a test environment where we + # know the storage is available, because we create it at runtime + config.before(:example) do |example| + unless example.metadata[:broken_storage] + allow(Gitlab::Git::Storage::ForkedStorageCheck) + .to receive(:storage_available?).and_return(true) end end diff --git a/spec/support/notify_shared_examples.rb b/spec/support/notify_shared_examples.rb index 136f92c6419..e2c23607406 100644 --- a/spec/support/notify_shared_examples.rb +++ b/spec/support/notify_shared_examples.rb @@ -1,9 +1,10 @@ shared_context 'gitlab email notification' do + set(:project) { create(:project, :repository) } + set(:recipient) { create(:user, email: 'recipient@example.com') } + let(:gitlab_sender_display_name) { Gitlab.config.gitlab.email_display_name } let(:gitlab_sender) { Gitlab.config.gitlab.email_from } let(:gitlab_sender_reply_to) { Gitlab.config.gitlab.email_reply_to } - let(:recipient) { create(:user, email: 'recipient@example.com') } - let(:project) { create(:project) } let(:new_user_address) { 'newguy@example.com' } before do diff --git a/vendor/project_templates/express.tar.gz b/vendor/project_templates/express.tar.gz Binary files differindex 6353f6605d5..302a74637b2 100644 --- a/vendor/project_templates/express.tar.gz +++ b/vendor/project_templates/express.tar.gz diff --git a/vendor/project_templates/rails.tar.gz b/vendor/project_templates/rails.tar.gz Binary files differindex 0509016f130..0f406705563 100644 --- a/vendor/project_templates/rails.tar.gz +++ b/vendor/project_templates/rails.tar.gz diff --git a/vendor/project_templates/spring.tar.gz b/vendor/project_templates/spring.tar.gz Binary files differindex d7c0ab74d01..02006b14406 100644 --- a/vendor/project_templates/spring.tar.gz +++ b/vendor/project_templates/spring.tar.gz |
