diff options
author | Brian Neel <brian@gitlab.com> | 2017-09-05 02:18:53 +0300 |
---|---|---|
committer | Brian Neel <brian@gitlab.com> | 2017-09-05 02:18:53 +0300 |
commit | 1e878fd7b8ebbaa2d4f32fba96773bd51f7d3542 (patch) | |
tree | e34557e3225db97ad915a26d1e14e6dcccfc6aa3 | |
parent | f161f1adb18d4d593a9597d50ac72a814cde0517 (diff) | |
parent | 789cc6787fff872693625565732cbbc18009ed2f (diff) |
Merge branch '9-5-stable' into 'security-9-5'
Merge 9-5-stable into security-9-5
See merge request gitlab/gitlabhq!2184
90 files changed, 856 insertions, 540 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 7da8f839a8e..7ee2a2cae21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 9.5.3 (2017-09-03) + +- [SECURITY] Filter additional secrets from Rails logs. +- [FIXED] Make username update fail if the namespace update fails. !13642 +- [FIXED] Fix failure when issue is authored by a deleted user. !13807 +- [FIXED] Reverts changes made to signin_enabled. !13956 +- [FIXED] Fix Merge when pipeline succeeds button dropdown caret icon horizontal alignment. +- [FIXED] Fixed diff changes bar buttons from showing/hiding whilst scrolling. +- [FIXED] Fix events error importing GitLab projects. +- [FIXED] Fix pipeline trigger via API fails with 500 Internal Server Error in 9.5. +- [FIXED] Fixed fly-out nav flashing in & out. +- [FIXED] Remove closing external issues by reference error. +- [FIXED] Re-allow appearances.description_html to be NULL. +- [CHANGED] Update and fix resolvable note icons for easier recognition. +- [OTHER] Eager load head pipeline projects for MRs index. +- [OTHER] Instrument MergeRequest#fetch_ref. +- [OTHER] Instrument MergeRequest#ensure_ref_fetched. + ## 9.5.2 (2017-08-28) - [FIXED] Fix signing in using LDAP when attribute mapping uses simple strings instead of arrays. diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index be386c9ede3..7b52f5e5178 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.33.0 +0.35.0 @@ -289,7 +289,7 @@ group :metrics do gem 'influxdb', '~> 0.2', require: false # Prometheus - gem 'prometheus-client-mmap', '~>0.7.0.beta11' + gem 'prometheus-client-mmap', '~>0.7.0.beta14' gem 'raindrops', '~> 0.18' end diff --git a/Gemfile.lock b/Gemfile.lock index 38944248f95..98edefd79fc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -610,7 +610,12 @@ GEM premailer-rails (1.9.7) actionmailer (>= 3, < 6) premailer (~> 1.7, >= 1.7.9) - prometheus-client-mmap (0.7.0.beta11) + proc_to_ast (0.1.0) + coderay + parser + unparser + procto (0.0.3) + prometheus-client-mmap (0.7.0.beta14) mmap2 (~> 2.2, >= 2.2.7) pry (0.10.4) coderay (~> 1.1.0) @@ -1066,7 +1071,7 @@ DEPENDENCIES pg (~> 0.18.2) poltergeist (~> 1.9.0) premailer-rails (~> 1.9.7) - prometheus-client-mmap (~> 0.7.0.beta11) + prometheus-client-mmap (~> 0.7.0.beta14) pry-byebug (~> 3.4.1) pry-rails (~> 0.3.4) rack-attack (~> 4.4.1) @@ -1 +1 @@ -9.5.2 +9.5.3 diff --git a/app/assets/javascripts/fly_out_nav.js b/app/assets/javascripts/fly_out_nav.js index adf397ca0fe..a65e69527c8 100644 --- a/app/assets/javascripts/fly_out_nav.js +++ b/app/assets/javascripts/fly_out_nav.js @@ -1,9 +1,23 @@ import bp from './breakpoints'; -let headerHeight = 50; +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; let sidebar; +export const mousePos = []; + export const setSidebar = (el) => { sidebar = el; }; +export const getOpenMenu = () => currentOpenMenu; +export const setOpenMenu = (menu = null) => { currentOpenMenu = menu; }; + +export const slope = (a, b) => (b.y - a.y) / (b.x - a.x); + +let headerHeight = 50; export const getHeaderHeight = () => headerHeight; @@ -14,8 +28,28 @@ export const canShowActiveSubItems = (el) => { 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 +58,125 @@ 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) - headerHeight}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 default () => { - const items = [...document.querySelectorAll('.sidebar-top-level-items > li')] - .filter(el => el.querySelector('.sidebar-sub-level-items')); +export const documentMouseMove = (e) => { + mousePos.push({ + x: e.clientX, + y: e.clientY, + }); - sidebar = document.querySelector('.nav-sidebar'); + if (mousePos.length > 6) mousePos.shift(); +}; - if (sidebar) { - headerHeight = sidebar.offsetTop; +export const subItemsMouseLeave = (relatedTarget) => { + clearTimeout(timeoutId); - items.forEach((el) => { - el.addEventListener('mouseenter', e => showSubLevelItems(e.currentTarget)); - el.addEventListener('mouseleave', e => hideSubLevelItems(e.currentTarget)); - }); + if (!relatedTarget.closest(`.${IS_OVER_CLASS}`)) { + hideMenu(currentOpenMenu); } }; + +export default () => { + sidebar = document.querySelector('.nav-sidebar'); + + if (!sidebar) return; + + const items = [...sidebar.querySelectorAll('.sidebar-top-level-items > li')]; + + sidebar.querySelector('.sidebar-top-level-items').addEventListener('mouseleave', () => { + clearTimeout(timeoutId); + + timeoutId = setTimeout(() => { + if (currentOpenMenu) hideMenu(currentOpenMenu); + }, getHideSubItemsInterval()); + }); + + headerHeight = document.querySelector('.nav-sidebar').offsetTop; + + items.forEach((el) => { + const subItems = el.querySelector('.sidebar-sub-level-items'); + + if (subItems) { + subItems.addEventListener('mouseleave', e => subItemsMouseLeave(e.relatedTarget)); + } + + el.addEventListener('mouseenter', e => mouseEnterTopItems(e.currentTarget)); + el.addEventListener('mouseleave', e => mouseLeaveTopItem(e.currentTarget)); + }); + + document.addEventListener('mousemove', documentMouseMove); +}; diff --git a/app/assets/javascripts/lib/utils/sticky.js b/app/assets/javascripts/lib/utils/sticky.js index ff2b66046b4..283c0ec0410 100644 --- a/app/assets/javascripts/lib/utils/sticky.js +++ b/app/assets/javascripts/lib/utils/sticky.js @@ -1,5 +1,5 @@ export const isSticky = (el, scrollY, stickyTop) => { - const top = el.offsetTop - scrollY; + const top = Math.floor(el.offsetTop - scrollY); if (top <= stickyTop) { el.classList.add('is-stuck'); diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 5a9b3d19f84..3b3620fe61b 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -253,6 +253,7 @@ import bp from './breakpoints'; loadDiff(source) { if (this.diffsLoaded) { + document.dispatchEvent(new CustomEvent('scroll')); return; } diff --git a/app/assets/javascripts/new_sidebar.js b/app/assets/javascripts/new_sidebar.js index 2d1ed9e4076..b18d12b48b5 100644 --- a/app/assets/javascripts/new_sidebar.js +++ b/app/assets/javascripts/new_sidebar.js @@ -47,7 +47,6 @@ export default class NewNavSidebar { if (this.$sidebar.length) { this.$sidebar.toggleClass('sidebar-icons-only', collapsed); - this.$page.toggleClass('page-with-new-sidebar', !collapsed); this.$page.toggleClass('page-with-icon-sidebar', breakpoint === 'sm' ? true : collapsed); } NewNavSidebar.setCollapsedCookie(collapsed); diff --git a/app/assets/javascripts/project_select_combo_button.js b/app/assets/javascripts/project_select_combo_button.js index f799d9d619a..ba3c45ff553 100644 --- a/app/assets/javascripts/project_select_combo_button.js +++ b/app/assets/javascripts/project_select_combo_button.js @@ -57,10 +57,10 @@ export default class ProjectSelectComboButton { setNewItemBtnAttributes(project) { if (project) { this.newItemBtn.attr('href', project.url); - this.newItemBtn.text(`${this.newItemBtnBaseText} in ${project.name}`); + this.newItemBtn.text(`New ${this.deriveItemTypeFromLabel()} in ${project.name}`); this.newItemBtn.enable(); } else { - this.newItemBtn.text(`Select project to create ${this.itemType}`); + this.newItemBtn.text(`Select project to create ${this.deriveItemTypeFromLabel()}`); this.newItemBtn.disable(); } } diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 02e0ba74158..5bc1db4b284 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -189,7 +189,7 @@ width: auto; top: 100%; left: 0; - z-index: 9; + z-index: 200; min-width: 240px; max-width: 500px; margin-top: 2px; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 91638e1cb6d..a532616ff43 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -26,6 +26,7 @@ min-width: inherit; min-height: inherit; background-color: inherit; + max-width: 100%; } p a:not(.no-attachment-icon) img { 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 0d13c742502..faedd207e01 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -70,7 +70,8 @@ $new-sidebar-collapsed-width: 50px; background-color: $white-light; } - .sidebar-context-title { + .project-title, + .group-title { overflow: hidden; text-overflow: ellipsis; } @@ -96,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, - .sidebar-context-title { + .project-title { display: none; } .nav-item-name { - display: none; - } - - .sidebar-top-level-items > li > a { - min-height: 44px; + opacity: 0; } } @@ -183,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; } @@ -263,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 { @@ -335,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/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 6bb013cca85..31fce49e1f8 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -291,6 +291,7 @@ .dropdown-toggle { .fa { + margin-left: 0; color: inherit; } } 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/application_controller.rb b/app/controllers/application_controller.rb index 5b448008a1b..ee9924bf2de 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -202,7 +202,7 @@ class ApplicationController < ActionController::Base end def check_password_expiration - if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && current_user.allow_password_authentication? + if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && !current_user.ldap_user? return redirect_to new_profile_password_path end end diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index b43b2c5621f..a34a82b7ba6 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -15,7 +15,17 @@ module IssuableCollections end def merge_requests_collection - merge_requests_finder.execute.preload(:source_project, :target_project, :author, :assignee, :labels, :milestone, :head_pipeline, target_project: :namespace, merge_request_diff: :merge_request_diff_commits) + merge_requests_finder.execute.preload( + :source_project, + :target_project, + :author, + :assignee, + :labels, + :milestone, + head_pipeline: :project, + target_project: :namespace, + merge_request_diff: :merge_request_diff_commits + ) end def issues_finder diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index aa8cf630032..fda944adecd 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -1,8 +1,6 @@ class PasswordsController < Devise::PasswordsController - include Gitlab::CurrentSettings - before_action :resource_from_email, only: [:create] - before_action :check_password_authentication_available, only: [:create] + before_action :prevent_ldap_reset, only: [:create] before_action :throttle_reset, only: [:create] def edit @@ -40,11 +38,11 @@ class PasswordsController < Devise::PasswordsController self.resource = resource_class.find_by_email(email) end - def check_password_authentication_available - return if current_application_settings.password_authentication_enabled? && (resource.nil? || resource.allow_password_authentication?) + def prevent_ldap_reset + return unless resource&.ldap_user? redirect_to after_sending_reset_password_instructions_path_for(resource_name), - alert: "Password authentication is unavailable." + alert: "Cannot reset password for LDAP user." end def throttle_reset diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb index c423761ab24..7beb52dd8e8 100644 --- a/app/controllers/profiles/passwords_controller.rb +++ b/app/controllers/profiles/passwords_controller.rb @@ -77,7 +77,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController end def authorize_change_password! - render_404 unless @user.allow_password_authentication? + render_404 if @user.ldap_user? end def user_params diff --git a/app/models/commit.rb b/app/models/commit.rb index 37379ed0da6..71aa93d0979 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -55,7 +55,8 @@ class Commit end def from_hash(hash, project) - new(Gitlab::Git::Commit.new(hash), project) + raw_commit = Gitlab::Git::Commit.new(project.repository.raw, hash) + new(raw_commit, project) end def valid_hash?(key) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index e83b11f7668..9fc526ec3ef 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -598,6 +598,8 @@ class MergeRequest < ActiveRecord::Base self.merge_requests_closing_issues.delete_all closes_issues(current_user).each do |issue| + next if issue.is_a?(ExternalIssue) + self.merge_requests_closing_issues.create!(issue: issue) end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index d9d746ccf41..58050e1f438 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -282,9 +282,7 @@ class MergeRequestDiff < ActiveRecord::Base def load_commits commits = st_commits.presence || merge_request_diff_commits - commits.map do |commit| - Commit.new(Gitlab::Git::Commit.new(commit.to_hash), merge_request.source_project) - end + commits.map { |commit| Commit.from_hash(commit.to_hash, project) } end def save_diffs 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/models/repository.rb b/app/models/repository.rb index 019c40bf0a5..ff82b958255 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -64,6 +64,8 @@ class Repository @raw_repository ||= initialize_raw_repository end + alias_method :raw, :raw_repository + # Return absolute path to repository def path_to_repo @path_to_repo ||= File.expand_path( diff --git a/app/models/user.rb b/app/models/user.rb index 192eed81735..d09e8478b69 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -598,7 +598,7 @@ class User < ActiveRecord::Base end def require_personal_access_token_creation_for_git_auth? - return false if allow_password_authentication? || ldap_user? + return false if current_application_settings.password_authentication_enabled? || ldap_user? PersonalAccessTokensFinder.new(user: self, impersonation: false, state: 'active').execute.none? end @@ -834,7 +834,12 @@ class User < ActiveRecord::Base create_namespace!(path: username, name: username) unless namespace if username_changed? - namespace.update_attributes(path: username, name: username) + unless namespace.update_attributes(path: username, name: username) + namespace.errors.each do |attribute, message| + self.errors.add(:"namespace_#{attribute}", message) + end + raise ActiveRecord::RecordInvalid.new(namespace) + end end end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index d0ba9f89460..de2cd7e87be 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -15,7 +15,7 @@ module Ci pipeline_schedule: schedule ) - result = validate(current_user || trigger_request.trigger.owner, + result = validate(current_user, ignore_skip_ci: ignore_skip_ci, save_on_errors: save_on_errors) 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/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 672d04a8af5..36de3073347 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -147,7 +147,7 @@ .checkbox = f.label :password_authentication_enabled do = f.check_box :password_authentication_enabled - Password authentication enabled + Sign-in enabled - if omniauth_enabled? && button_based_providers.any? .form-group = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2' diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 26d9640e98a..448f6abedf2 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -29,7 +29,7 @@ = link_to profile_emails_path, title: 'Emails' do %span Emails - - if current_user.allow_password_authentication? + - unless current_user.ldap_user? = nav_link(controller: :passwords) do = link_to edit_profile_password_path, title: 'Password' do %span 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/config/application.rb b/config/application.rb index 47887bf8596..c09b9168267 100644 --- a/config/application.rb +++ b/config/application.rb @@ -51,31 +51,24 @@ module Gitlab # Configure sensitive parameters which will be filtered from the log file. # # Parameters filtered: - # - Password (:password, :password_confirmation) - # - Private tokens + # - Any parameter ending with `_token` + # - Any parameter containing `password` + # - Any parameter containing `secret` # - Two-factor tokens (:otp_attempt) # - Repo/Project Import URLs (:import_url) # - Build variables (:variables) # - GitLab Pages SSL cert/key info (:certificate, :encrypted_key) # - Webhook URLs (:hook) - # - GitLab-shell secret token (:secret_token) # - Sentry DSN (:sentry_dsn) # - Deploy keys (:key) + config.filter_parameters += [/_token$/, /password/, /secret/] config.filter_parameters += %i( - authentication_token certificate encrypted_key hook import_url - incoming_email_token - rss_token key otp_attempt - password - password_confirmation - private_token - runners_token - secret_token sentry_dsn variables ) diff --git a/config/initializers/7_prometheus_metrics.rb b/config/initializers/7_prometheus_metrics.rb index 54c797e0714..31839297523 100644 --- a/config/initializers/7_prometheus_metrics.rb +++ b/config/initializers/7_prometheus_metrics.rb @@ -1,4 +1,5 @@ require 'prometheus/client' +require 'prometheus/client/support/unicorn' Prometheus::Client.configure do |config| config.logger = Rails.logger @@ -9,6 +10,8 @@ Prometheus::Client.configure do |config| if Rails.env.development? || Rails.env.test? config.multiprocess_files_dir ||= Rails.root.join('tmp/prometheus_multiproc_dir') end + + config.pid_provider = Prometheus::Client::Support::Unicorn.method(:worker_pid_provider) end Sidekiq.configure_server do |config| diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb index 2aeb94d47cd..5b455a8065a 100644 --- a/config/initializers/8_metrics.rb +++ b/config/initializers/8_metrics.rb @@ -119,6 +119,10 @@ 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) + instrumentation.instrument_instance_method(MergeRequest, :fetch_ref) 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/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index 10f5ab3370d..ae69d7f92f2 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -144,9 +144,8 @@ gitlab_rails['backup_upload_connection'] = { 'region' => 'eu-west-1', 'aws_access_key_id' => 'AKIAKIAKI', 'aws_secret_access_key' => 'secret123' - # If using an IAM Profile, leave aws_access_key_id & aws_secret_access_key empty - # ie. 'aws_access_key_id' => '', - # 'use_iam_profile' => 'true' + # If using an IAM Profile, don't configure aws_access_key_id & aws_secret_access_key + # 'use_iam_profile' => true } gitlab_rails['backup_upload_remote_directory'] = 'my.s3.bucket' ``` diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 7d3aa532750..4befcd5a290 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -48,10 +48,6 @@ module Gitlab # Avoid resource intensive login checks if password is not provided return unless password.present? - # Nothing to do here if internal auth is disabled and LDAP is - # not configured - return unless current_application_settings.password_authentication_enabled? || Gitlab::LDAP::Config.enabled? - Gitlab::Auth::UniqueIpsLimiter.limit_user! do user = User.by_login(login) diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb index b260822788d..2479b4a7706 100644 --- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -54,7 +54,7 @@ module Gitlab end def serialize_commit(event, commit, query) - commit = Commit.new(Gitlab::Git::Commit.new(commit.to_hash), @project) + commit = Commit.from_hash(commit.to_hash, @project) AnalyticsCommitSerializer.new(project: @project, total_time: event['total_time']).represent(commit) 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/git/blame.rb b/lib/gitlab/git/blame.rb index 8dbe25e55f6..31effdba292 100644 --- a/lib/gitlab/git/blame.rb +++ b/lib/gitlab/git/blame.rb @@ -16,7 +16,7 @@ module Gitlab def each @blames.each do |blame| yield( - Gitlab::Git::Commit.new(blame.commit), + Gitlab::Git::Commit.new(@repo, blame.commit), blame.line ) end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 2ea4619fe2a..60e6e45007c 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -51,7 +51,7 @@ module Gitlab # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/321 def find(repo, commit_id = "HEAD") return commit_id if commit_id.is_a?(Gitlab::Git::Commit) - return decorate(commit_id) if commit_id.is_a?(Rugged::Commit) + return decorate(repo, commit_id) if commit_id.is_a?(Rugged::Commit) obj = if commit_id.is_a?(String) repo.rev_parse_target(commit_id) @@ -61,7 +61,7 @@ module Gitlab return nil unless obj.is_a?(Rugged::Commit) - decorate(obj) + decorate(repo, obj) rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, Gitlab::Git::Repository::NoRepository nil end @@ -102,7 +102,7 @@ module Gitlab if is_enabled repo.gitaly_commit_client.between(base, head) else - repo.rugged_commits_between(base, head).map { |c| decorate(c) } + repo.rugged_commits_between(base, head).map { |c| decorate(repo, c) } end end rescue Rugged::ReferenceError @@ -169,7 +169,7 @@ module Gitlab offset = actual_options[:skip] limit = actual_options[:max_count] walker.each(offset: offset, limit: limit) do |commit| - commits.push(decorate(commit)) + commits.push(decorate(repo, commit)) end walker.reset @@ -183,8 +183,8 @@ module Gitlab Gitlab::GitalyClient::CommitService.new(repo).find_all_commits(options) end - def decorate(commit, ref = nil) - Gitlab::Git::Commit.new(commit, ref) + def decorate(repository, commit, ref = nil) + Gitlab::Git::Commit.new(repository, commit, ref) end # Returns a diff object for the changes introduced by +rugged_commit+. @@ -231,7 +231,7 @@ module Gitlab end end - def initialize(raw_commit, head = nil) + def initialize(repository, raw_commit, head = nil) raise "Nil as raw commit passed" unless raw_commit case raw_commit @@ -239,12 +239,13 @@ module Gitlab init_from_hash(raw_commit) when Rugged::Commit init_from_rugged(raw_commit) - when Gitlab::GitalyClient::Commit + when Gitaly::GitCommit init_from_gitaly(raw_commit) else raise "Invalid raw commit type: #{raw_commit.class}" end + @repository = repository @head = head end @@ -319,14 +320,7 @@ module Gitlab end def parents - case raw_commit - when Rugged::Commit - raw_commit.parents.map { |c| Gitlab::Git::Commit.new(c) } - when Gitlab::GitalyClient::Commit - parent_ids.map { |oid| self.class.find(raw_commit.repository, oid) }.compact - else - raise NotImplementedError, "commit source doesn't support #parents" - end + parent_ids.map { |oid| self.class.find(@repository, oid) }.compact end def stats diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb index 87ed9c3ea26..6a601561c2a 100644 --- a/lib/gitlab/git/diff_collection.rb +++ b/lib/gitlab/git/diff_collection.rb @@ -28,7 +28,6 @@ module Gitlab @limits = self.class.collection_limits(options) @enforce_limits = !!options.fetch(:limits, true) @expanded = !!options.fetch(:expanded, true) - @from_gitaly = options.fetch(:from_gitaly, false) @line_count = 0 @byte_count = 0 @@ -44,7 +43,7 @@ module Gitlab return if @iterator.nil? Gitlab::GitalyClient.migrate(:commit_raw_diffs) do |is_enabled| - if is_enabled && @from_gitaly + if is_enabled && @iterator.is_a?(Gitlab::GitalyClient::DiffStitcher) each_gitaly_patch(&block) else each_rugged_patch(&block) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 6cb63a1c7dc..1dd03cb8679 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -321,7 +321,7 @@ module Gitlab options[:limit] ||= 0 options[:offset] ||= 0 - raw_log(options).map { |c| Commit.decorate(c) } + raw_log(options).map { |c| Commit.decorate(self, c) } end def count_commits(options) diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb deleted file mode 100644 index 61fe462d762..00000000000 --- a/lib/gitlab/gitaly_client/commit.rb +++ /dev/null @@ -1,14 +0,0 @@ -module Gitlab - module GitalyClient - class Commit - attr_reader :repository, :gitaly_commit - - delegate :id, :subject, :body, :author, :committer, :parent_ids, to: :gitaly_commit - - def initialize(repository, gitaly_commit) - @repository = repository - @gitaly_commit = gitaly_commit - end - end - end -end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 19827a123de..6ac2e13a004 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -107,8 +107,7 @@ module Gitlab gitaly_commit = GitalyClient.call(@repository.storage, :commit_service, :last_commit_for_path, request).commit return unless gitaly_commit - commit = GitalyClient::Commit.new(@repository, gitaly_commit) - Gitlab::Git::Commit.new(commit) + Gitlab::Git::Commit.new(@repository, gitaly_commit) end def between(from, to) @@ -170,7 +169,7 @@ module Gitlab private def commit_diff_request_params(commit, options = {}) - parent_id = commit.parents[0]&.id || EMPTY_TREE_ID + parent_id = commit.parent_ids.first || EMPTY_TREE_ID { repository: @gitaly_repo, @@ -183,8 +182,7 @@ module Gitlab def consume_commits_response(response) response.flat_map do |message| message.commits.map do |gitaly_commit| - commit = GitalyClient::Commit.new(@repository, gitaly_commit) - Gitlab::Git::Commit.new(commit) + Gitlab::Git::Commit.new(@repository, gitaly_commit) end end end diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index b0f7548b7dc..919fb68b8c7 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -16,8 +16,7 @@ module Gitlab response.flat_map do |message| message.branches.map do |branch| - gitaly_commit = GitalyClient::Commit.new(@repository, branch.target) - target_commit = Gitlab::Git::Commit.decorate(gitaly_commit) + target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target) Gitlab::Git::Branch.new(@repository, branch.name, branch.target.id, target_commit) end end @@ -102,8 +101,7 @@ module Gitlab response.flat_map do |message| message.tags.map do |gitaly_tag| if gitaly_tag.target_commit.present? - commit = GitalyClient::Commit.new(@repository, gitaly_tag.target_commit) - gitaly_commit = Gitlab::Git::Commit.new(commit) + gitaly_commit = Gitlab::Git::Commit.decorate(@repository, gitaly_tag.target_commit) end Gitlab::Git::Tag.new( @@ -141,7 +139,7 @@ module Gitlab committer_email: response.commit_committer.email.dup } - Gitlab::Git::Commit.decorate(hash) + Gitlab::Git::Commit.decorate(@repository, hash) end end end diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 9d9ebcb389a..6a561f84d74 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -135,3 +135,7 @@ methods: - :diff_head_sha project: - :description_html + events: + - :action + push_event_payload: + - :action
\ No newline at end of file diff --git a/locale/zh_TW/gitlab.po b/locale/zh_TW/gitlab.po index 4fd728659c6..11ee5dc2893 100644 --- a/locale/zh_TW/gitlab.po +++ b/locale/zh_TW/gitlab.po @@ -1111,31 +1111,17 @@ msgstr "因該階段的資料不足而無法顯示相關資訊" msgid "Withdraw Access Request" msgstr "取消權限申請" -msgid "" -"You are going to remove %{group_name}.\n" -"Removed groups CANNOT be restored!\n" -"Are you ABSOLUTELY sure?" -msgstr "即將要刪除 %{group_name}。\n" -"被刪除的群組完全無法救回來喔!\n" -"真的「100%確定」要這麼做嗎?" +msgid "You are going to remove %{group_name}. Removed groups CANNOT be restored! Are you ABSOLUTELY sure?" +msgstr "將要刪除 %{group_name}。被刪除的群組無法復原!真的「確定」要這麼做嗎?" -msgid "" -"You are going to remove %{project_name_with_namespace}.\n" -"Removed project CANNOT be restored!\n" -"Are you ABSOLUTELY sure?" -msgstr "即將要刪除 %{project_name_with_namespace}。被刪除的專案完全無法救回來喔!真的「100%確定」要這麼做嗎?" +msgid "You are going to remove %{project_name_with_namespace}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?" +msgstr "將要刪除 %{project_name_with_namespace}。被刪除的專案無法復原!真的「確定」要這麼做嗎?" -msgid "" -"You are going to remove the fork relationship to source project " -"%{forked_from_project}. Are you ABSOLUTELY sure?" -msgstr "" -"將要刪除本分支專案與主幹的所有關聯 (fork relationship) 。 %{forked_from_project} " -"真的「100%確定」要這麼做嗎?" +msgid "You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?" +msgstr "將要刪除本分支專案與主幹 %{forked_from_project} 的所有關聯。 真的「確定」要這麼做嗎?" -msgid "" -"You are going to transfer %{project_name_with_namespace} to another owner. " -"Are you ABSOLUTELY sure?" -msgstr "將要把 %{project_name_with_namespace} 的所有權轉移給另一個人。真的「100%確定」要這麼做嗎?" +msgid "You are going to transfer %{project_name_with_namespace} to another owner. Are you ABSOLUTELY sure?" +msgstr "將要把 %{project_name_with_namespace} 的所有權轉移給另一個人。真的「確定」要這麼做嗎?" msgid "You can only add files when you are on a branch" msgstr "只能在分支 (branch) 上建立檔案" diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 331903a5543..59a6cfbf4f5 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -8,34 +8,43 @@ describe ApplicationController do it 'redirects if the user is over their password expiry' do user.password_expires_at = Time.new(2002) + expect(user.ldap_user?).to be_falsey allow(controller).to receive(:current_user).and_return(user) expect(controller).to receive(:redirect_to) expect(controller).to receive(:new_profile_password_path) + controller.send(:check_password_expiration) end it 'does not redirect if the user is under their password expiry' do user.password_expires_at = Time.now + 20010101 + expect(user.ldap_user?).to be_falsey allow(controller).to receive(:current_user).and_return(user) expect(controller).not_to receive(:redirect_to) + controller.send(:check_password_expiration) end it 'does not redirect if the user is over their password expiry but they are an ldap user' do user.password_expires_at = Time.new(2002) + allow(user).to receive(:ldap_user?).and_return(true) allow(controller).to receive(:current_user).and_return(user) expect(controller).not_to receive(:redirect_to) + controller.send(:check_password_expiration) end - it 'does not redirect if the user is over their password expiry but sign-in is disabled' do + it 'redirects if the user is over their password expiry and sign-in is disabled' do stub_application_setting(password_authentication_enabled: false) user.password_expires_at = Time.new(2002) + + expect(user.ldap_user?).to be_falsey allow(controller).to receive(:current_user).and_return(user) - expect(controller).not_to receive(:redirect_to) + expect(controller).to receive(:redirect_to) + expect(controller).to receive(:new_profile_password_path) controller.send(:check_password_expiration) end diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index 2955d01fad0..cdaa88bbf5d 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -1,18 +1,18 @@ require 'spec_helper' describe PasswordsController do - describe '#check_password_authentication_available' do + describe '#prevent_ldap_reset' do before do @request.env["devise.mapping"] = Devise.mappings[:user] end context 'when password authentication is disabled' do - it 'prevents a password reset' do + it 'allows password reset' do stub_application_setting(password_authentication_enabled: false) post :create - expect(flash[:alert]).to eq 'Password authentication is unavailable.' + expect(response).to have_http_status(302) end end @@ -22,7 +22,7 @@ describe PasswordsController do it 'prevents a password reset' do post :create, user: { email: user.email } - expect(flash[:alert]).to eq 'Password authentication is unavailable.' + expect(flash[:alert]).to eq('Cannot reset password for LDAP user.') end end end diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index cd2cbf4bfe7..207c25beb0e 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -101,14 +101,6 @@ describe 'Filter issues', js: true do expect_issues_list_count(5) expect_filtered_search_input_empty end - - it 'filters issues by invalid author' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple authors' do - skip('to be tested, issue #26546') - end end context 'author with other filters' do @@ -158,10 +150,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - it 'sorting' do - skip('to be tested, issue #26546') - end end describe 'filter issues by assignee' do @@ -181,14 +169,6 @@ describe 'Filter issues', js: true do expect_issues_list_count(8, 1) expect_filtered_search_input_empty end - - it 'filters issues by invalid assignee' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple assignees' do - skip('to be tested, issue #26546') - end end context 'assignee with other filters' do @@ -238,12 +218,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by label' do @@ -266,10 +240,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end - it 'filters issues by invalid label' do - skip('to be tested, issue #26546') - end - it 'filters issues by multiple labels' do input_filtered_search("label:~#{bug_label.title} label:~#{caps_sensitive_label.title}") @@ -471,12 +441,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by milestone' do @@ -513,14 +477,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end - it 'filters issues by invalid milestones' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple milestones' do - skip('to be tested, issue #26546') - end - it 'filters issues by milestone containing special characters' do special_milestone = create(:milestone, title: '!@\#{$%^&*()}', project: project) create(:issue, title: "Issue with special character milestone", project: project, milestone: special_milestone) @@ -590,12 +546,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by text' do 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/profiles/password_spec.rb b/spec/features/profiles/password_spec.rb index 2c757f99a27..225d4c16841 100644 --- a/spec/features/profiles/password_spec.rb +++ b/spec/features/profiles/password_spec.rb @@ -53,12 +53,12 @@ describe 'Profile > Password' do context 'Regular user' do let(:user) { create(:user) } - it 'renders 404 when sign-in is disabled' do + it 'renders 200 when sign-in is disabled' do stub_application_setting(password_authentication_enabled: false) visit edit_profile_password_path - expect(page).to have_http_status(404) + expect(page).to have_http_status(200) 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/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index e13c4629214..4588bf3d971 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -1,11 +1,18 @@ import { calculateTop, - hideSubLevelItems, showSubLevelItems, canShowSubItems, canShowActiveSubItems, + mouseEnterTopItems, + mouseLeaveTopItem, + getOpenMenu, + setOpenMenu, + mousePos, + getHideSubItemsInterval, + documentMouseMove, getHeaderHeight, setSidebar, + subItemsMouseLeave, } from '~/fly_out_nav'; import bp from '~/breakpoints'; @@ -19,11 +26,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 +60,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: 150px;"></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 +234,7 @@ describe('Fly out sidebar navigation', () => { ).not.toBe('block'); }); - it('does not shows sub-items', () => { + it('shows sub-items', () => { showSubLevelItems(el); expect( @@ -215,4 +316,29 @@ describe('Fly out sidebar navigation', () => { ).toBeTruthy(); }); }); + + describe('subItemsMouseLeave', () => { + beforeEach(() => { + el.innerHTML = '<div class="sidebar-sub-level-items" style="position: absolute;"></div>'; + + setOpenMenu(el.querySelector('.sidebar-sub-level-items')); + }); + + it('hides subMenu if element is not hovered', () => { + subItemsMouseLeave(el); + + expect( + getOpenMenu(), + ).toBeNull(); + }); + + it('does not hide subMenu if element is hovered', () => { + el.classList.add('is-over'); + subItemsMouseLeave(el); + + expect( + getOpenMenu(), + ).not.toBeNull(); + }); + }); }); diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index dc40244c20e..8830a2d29e5 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -295,6 +295,17 @@ import 'vendor/jquery.scrollTo'; this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); }); + it('triggers scroll event when diff already loaded', function () { + spyOn(document, 'dispatchEvent'); + + this.class.diffsLoaded = true; + this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); + + expect( + document.dispatchEvent, + ).toHaveBeenCalledWith(new CustomEvent('scroll')); + }); + describe('with inline diff', () => { let noteId; let noteLineNumId; diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 4a498e79c87..f685bb83d0d 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -279,16 +279,6 @@ describe Gitlab::Auth do gl_auth.find_with_user_password('ldap_user', 'password') end end - - context "with sign-in disabled" do - before do - stub_application_setting(password_authentication_enabled: false) - end - - it "does not find user by valid login/password" do - expect(gl_auth.find_with_user_password(username, password)).to be_nil - end - end end private diff --git a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb index 87f45619e7a..0d5fffa38ff 100644 --- a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb @@ -210,7 +210,11 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads::Event do end end -describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads do +## +# The background migration relies on a temporary table, hence we're migrating +# to a specific version of the database where said table is still present. +# +describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migration, schema: 20170608152748 do let(:migration) { described_class.new } let(:project) { create(:project_empty_repo) } let(:author) { create(:user) } @@ -229,21 +233,6 @@ describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads do ) end - # The background migration relies on a temporary table, hence we're migrating - # to a specific version of the database where said table is still present. - before :all do - ActiveRecord::Migration.verbose = false - - ActiveRecord::Migrator - .migrate(ActiveRecord::Migrator.migrations_paths, 20170608152748) - end - - after :all do - ActiveRecord::Migrator.migrate(ActiveRecord::Migrator.migrations_paths) - - ActiveRecord::Migration.verbose = true - end - describe '#perform' do it 'returns if data should not be migrated' do allow(migration).to receive(:migrate?).and_return(false) diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 1b30b24c9ce..c37508402c4 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Commit, seed_helper: true do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } - let(:commit) { Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID) } + let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } let(:rugged_commit) do repository.rugged.lookup(SeedRepo::Commit::ID) end @@ -24,7 +24,7 @@ describe Gitlab::Git::Commit, seed_helper: true do } @parents = [repo.head.target] - @gitlab_parents = @parents.map { |c| Gitlab::Git::Commit.decorate(c) } + @gitlab_parents = @parents.map { |c| described_class.decorate(repository, c) } @tree = @parents.first.tree sha = Rugged::Commit.create( @@ -38,7 +38,7 @@ describe Gitlab::Git::Commit, seed_helper: true do ) @raw_commit = repo.lookup(sha) - @commit = Gitlab::Git::Commit.new(@raw_commit) + @commit = described_class.new(repository, @raw_commit) end it { expect(@commit.short_id).to eq(@raw_commit.oid[0..10]) } @@ -91,7 +91,7 @@ describe Gitlab::Git::Commit, seed_helper: true do committer: committer ) end - let(:commit) { described_class.new(Gitlab::GitalyClient::Commit.new(repository, gitaly_commit)) } + let(:commit) { described_class.new(repository, gitaly_commit) } it { expect(commit.short_id).to eq(id[0..10]) } it { expect(commit.id).to eq(id) } @@ -113,45 +113,45 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'Class methods' do describe '.find' do it "should return first head commit if without params" do - expect(Gitlab::Git::Commit.last(repository).id).to eq( + expect(described_class.last(repository).id).to eq( repository.rugged.head.target.oid ) end it "should return valid commit" do - expect(Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID)).to be_valid_commit + expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_valid_commit end it "should return valid commit for tag" do - expect(Gitlab::Git::Commit.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') + expect(described_class.find(repository, 'v1.0.0').id).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') end it "should return nil for non-commit ids" do blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") - expect(Gitlab::Git::Commit.find(repository, blob.id)).to be_nil + expect(described_class.find(repository, blob.id)).to be_nil end it "should return nil for parent of non-commit object" do blob = Gitlab::Git::Blob.find(repository, SeedRepo::Commit::ID, "files/ruby/popen.rb") - expect(Gitlab::Git::Commit.find(repository, "#{blob.id}^")).to be_nil + expect(described_class.find(repository, "#{blob.id}^")).to be_nil end it "should return nil for nonexisting ids" do - expect(Gitlab::Git::Commit.find(repository, "+123_4532530XYZ")).to be_nil + expect(described_class.find(repository, "+123_4532530XYZ")).to be_nil end context 'with broken repo' do let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH) } it 'returns nil' do - expect(Gitlab::Git::Commit.find(repository, SeedRepo::Commit::ID)).to be_nil + expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil end end end describe '.last_for_path' do context 'no path' do - subject { Gitlab::Git::Commit.last_for_path(repository, 'master') } + subject { described_class.last_for_path(repository, 'master') } describe '#id' do subject { super().id } @@ -160,7 +160,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'path' do - subject { Gitlab::Git::Commit.last_for_path(repository, 'master', 'files/ruby') } + subject { described_class.last_for_path(repository, 'master', 'files/ruby') } describe '#id' do subject { super().id } @@ -169,7 +169,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'ref + path' do - subject { Gitlab::Git::Commit.last_for_path(repository, SeedRepo::Commit::ID, 'encoding') } + subject { described_class.last_for_path(repository, SeedRepo::Commit::ID, 'encoding') } describe '#id' do subject { super().id } @@ -181,7 +181,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe '.where' do context 'path is empty string' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: '', @@ -199,7 +199,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'path is nil' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: nil, @@ -217,7 +217,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is branch name' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'master', path: 'files', @@ -237,7 +237,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is commit id' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: "874797c3a73b60d2187ed6e2fcabd289ff75171e", path: 'files', @@ -257,7 +257,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref is tag' do subject do - commits = Gitlab::Git::Commit.where( + commits = described_class.where( repo: repository, ref: 'v1.0.0', path: 'files', @@ -278,7 +278,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe '.between' do subject do - commits = Gitlab::Git::Commit.between(repository, SeedRepo::Commit::PARENT_ID, SeedRepo::Commit::ID) + commits = described_class.between(repository, SeedRepo::Commit::PARENT_ID, SeedRepo::Commit::ID) commits.map { |c| c.id } end @@ -294,12 +294,12 @@ describe Gitlab::Git::Commit, seed_helper: true do it 'should return a return a collection of commits' do commits = described_class.find_all(repository) - expect(commits).to all( be_a_kind_of(Gitlab::Git::Commit) ) + expect(commits).to all( be_a_kind_of(described_class) ) end context 'max_count' do subject do - commits = Gitlab::Git::Commit.find_all( + commits = described_class.find_all( repository, max_count: 50 ) @@ -322,7 +322,7 @@ describe Gitlab::Git::Commit, seed_helper: true do context 'ref + max_count + skip' do subject do - commits = Gitlab::Git::Commit.find_all( + commits = described_class.find_all( repository, ref: 'master', max_count: 50, @@ -374,7 +374,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#init_from_rugged' do - let(:gitlab_commit) { Gitlab::Git::Commit.new(rugged_commit) } + let(:gitlab_commit) { described_class.new(repository, rugged_commit) } subject { gitlab_commit } describe '#id' do @@ -384,7 +384,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#init_from_hash' do - let(:commit) { Gitlab::Git::Commit.new(sample_commit_hash) } + let(:commit) { described_class.new(repository, sample_commit_hash) } subject { commit } describe '#id' do @@ -451,7 +451,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end describe '#ref_names' do - let(:commit) { Gitlab::Git::Commit.find(repository, 'master') } + let(:commit) { described_class.find(repository, 'master') } subject { commit.ref_names(repository) } it 'has 1 element' do @@ -461,6 +461,17 @@ describe Gitlab::Git::Commit, seed_helper: true do it { is_expected.not_to include("feature") } end + describe '#rugged_commit' do + let(:raw_commit) { { id: SeedRepo::Commit::ID } } + let(:commit) { described_class.decorate(repository, raw_commit) } + subject { commit.send(:rugged_commit) } + + it 'returns a rugged commit based on the id of a non-rugged raw commit' do + expect(subject.class).to be(Rugged::Commit) + expect(subject).to eq(rugged_commit) + end + end + def sample_commit_hash { author_email: "dmitriy.zaporozhets@gmail.com", diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 558efeb0ca7..7b2ac9c0f53 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -511,17 +511,22 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe "#log" do - commit_with_old_name = nil - commit_with_new_name = nil - rename_commit = nil + let(:commit_with_old_name) do + Gitlab::Git::Commit.decorate(repository, @commit_with_old_name_id) + end + let(:commit_with_new_name) do + Gitlab::Git::Commit.decorate(repository, @commit_with_new_name_id) + end + let(:rename_commit) do + Gitlab::Git::Commit.decorate(repository, @rename_commit_id) + end before(:context) do # Add new commits so that there's a renamed file in the commit history repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged - - commit_with_old_name = Gitlab::Git::Commit.decorate(new_commit_edit_old_file(repo)) - rename_commit = Gitlab::Git::Commit.decorate(new_commit_move_file(repo)) - commit_with_new_name = Gitlab::Git::Commit.decorate(new_commit_edit_new_file(repo)) + @commit_with_old_name_id = new_commit_edit_old_file(repo) + @rename_commit_id = new_commit_move_file(repo) + @commit_with_new_name_id = new_commit_edit_new_file(repo) end after(:context) do diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 12580ad0112..01faeddc6ec 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::GitalyClient::CommitService do context 'when a commit does not have a parent' do it 'sends an RPC request with empty tree ref as left commit' do - initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') + initial_commit = project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863').raw request = Gitaly::CommitDiffRequest.new( repository: repository_message, left_commit_id: '4b825dc642cb6eb9a060e54bf8d69288fbee4904', diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 956f1d56eb4..107313386e3 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -79,6 +79,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(event).not_to be_nil end + it 'has the action' do + expect(event.action).not_to be_nil + end + it 'event belongs to note, belongs to merge request, belongs to a project' do expect(event.note.noteable.project).not_to be_nil end 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/migrations/README.md b/spec/migrations/README.md index 05d4f35db72..45cf25b96de 100644 --- a/spec/migrations/README.md +++ b/spec/migrations/README.md @@ -28,6 +28,14 @@ The `after` hook will migrate the database **up** and reinstitutes the latest schema version, so that the process does not affect subsequent specs and ensures proper isolation. +## Testing a class that is not an ActiveRecord::Migration + +In order to test a class that is not a migration itself, you will need to +manually provide a required schema version. Please add a `schema` tag to a +context that you want to switch the database schema within. + +Example: `describe SomeClass, :migration, schema: 20170608152748`. + ## Available helpers Use `table` helper to create a temporary `ActiveRecord::Base` derived model @@ -80,8 +88,6 @@ end ## Best practices -1. Use only one test example per migration unless there is a good reason to -use more. 1. Note that this type of tests do not run within the transaction, we use a truncation database cleanup strategy. Do not depend on transaction being present. diff --git a/spec/migrations/remove_dot_git_from_usernames_spec.rb b/spec/migrations/remove_dot_git_from_usernames_spec.rb index 8737e00eaeb..129374cb38c 100644 --- a/spec/migrations/remove_dot_git_from_usernames_spec.rb +++ b/spec/migrations/remove_dot_git_from_usernames_spec.rb @@ -51,7 +51,6 @@ describe RemoveDotGitFromUsernames do namespace.path = path namespace.save!(validate: false) - user.username = path - user.save!(validate: false) + user.update_column(:username, path) end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 08693b5da33..1d38f8e5a6d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -33,7 +33,6 @@ describe Commit do describe '#to_reference' do let(:project) { create(:project, :repository, path: 'sample-project') } - let(:commit) { project.commit } it 'returns a String reference to the object' do expect(commit.to_reference).to eq commit.id @@ -47,7 +46,6 @@ describe Commit do describe '#reference_link_text' do let(:project) { create(:project, :repository, path: 'sample-project') } - let(:commit) { project.commit } it 'returns a String reference to the object' do expect(commit.reference_link_text).to eq commit.short_id diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a1a3e70a7d2..5baa7c81ecc 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -159,6 +159,7 @@ describe MergeRequest do before do subject.project.has_external_issue_tracker = true subject.project.save! + create(:jira_service, project: subject.project) end it 'does not cache issues from external trackers' do @@ -166,6 +167,7 @@ describe MergeRequest do commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") allow(subject).to receive(:commits).and_return([commit]) + expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to raise_error expect { subject.cache_merge_request_closes_issues!(subject.author) }.not_to change(subject.merge_requests_closing_issues, :count) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 284fa68e795..e25ffa4d228 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2013,4 +2013,65 @@ describe User do expect(user.projects_limit_left).to eq(5) end end + + describe '#ensure_namespace_correct' do + context 'for a new user' do + let(:user) { build(:user) } + + it 'creates the namespace' do + expect(user.namespace).to be_nil + user.save! + expect(user.namespace).not_to be_nil + end + end + + context 'for an existing user' do + let(:username) { 'foo' } + let(:user) { create(:user, username: username) } + + context 'when the user is updated' do + context 'when the username is changed' do + let(:new_username) { 'bar' } + + it 'changes the namespace (just to compare to when username is not changed)' do + expect do + user.update_attributes!(username: new_username) + end.to change { user.namespace.updated_at } + end + + it 'updates the namespace name' do + user.update_attributes!(username: new_username) + expect(user.namespace.name).to eq(new_username) + end + + it 'updates the namespace path' do + user.update_attributes!(username: new_username) + expect(user.namespace.path).to eq(new_username) + end + + context 'when there is a validation error (namespace name taken) while updating namespace' do + let!(:conflicting_namespace) { create(:group, name: new_username, path: 'quz') } + + it 'causes the user save to fail' do + expect(user.update_attributes(username: new_username)).to be_falsey + expect(user.namespace.errors.messages[:name].first).to eq('has already been taken') + end + + it 'adds the namespace errors to the user' do + user.update_attributes(username: new_username) + expect(user.errors.full_messages.first).to eq('Namespace name has already been taken') + end + end + end + + context 'when the username is not changed' do + it 'does not change the namespace' do + expect do + user.update_attributes!(email: 'asdf@asdf.com') + end.not_to change { user.namespace.updated_at } + end + end + end + end + end end diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index d5c53b703dd..4f80953a694 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -85,6 +85,22 @@ describe API::Triggers do expect(pipeline.variables.map { |v| { v.key => v.value } }.last).to eq(variables) end end + + context 'when legacy trigger' do + before do + trigger.update(owner: nil) + end + + it 'creates pipeline' do + post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'master') + + expect(response).to have_http_status(201) + expect(json_response).to include('id' => pipeline.id) + pipeline.builds.reload + expect(pipeline.builds.pending.size).to eq(2) + expect(pipeline.builds.size).to eq(5) + end + end end context 'when triggering a pipeline from a trigger token' do diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 8465a6f99bd..a4535a97b05 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -413,14 +413,12 @@ describe Ci::CreatePipelineService do end context 'when trigger belongs to a developer' do - let(:user) {} + let(:user) { create(:user) } + let(:trigger) { create(:ci_trigger, owner: user) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } - let(:trigger_request) do - create(:ci_trigger_request).tap do |request| - user = create(:user) - project.add_developer(user) - request.trigger.update(owner: user) - end + before do + project.add_developer(user) end it 'does not create a pipeline' do @@ -431,17 +429,15 @@ describe Ci::CreatePipelineService do end context 'when trigger belongs to a master' do - let(:user) {} + let(:user) { create(:user) } + let(:trigger) { create(:ci_trigger, owner: user) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } - let(:trigger_request) do - create(:ci_trigger_request).tap do |request| - user = create(:user) - project.add_master(user) - request.trigger.update(owner: user) - end + before do + project.add_master(user) end - it 'does not create a pipeline' do + it 'creates a pipeline' do expect(execute_service(trigger_request: trigger_request)) .to be_persisted expect(Ci::Pipeline.count).to eq(1) 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 256eb0de73a..6ee35a33b2d 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -12,9 +12,22 @@ describe Users::UpdateService do end it 'returns an error result when record cannot be updated' do + result = {} expect do - update_user(user, { email: 'invalid' }) + result = update_user(user, { email: 'invalid' }) end.not_to change { user.reload.email } + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq('Email is invalid') + end + + it 'includes namespace error messages' do + create(:group, name: 'taken', path: 'something_else') + result = {} + expect do + result = update_user(user, { username: 'taken' }) + end.not_to change { user.reload.username } + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq('Namespace name has already been taken') end def update_user(user, opts) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d35c82b293c..2580776afb0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -136,17 +136,12 @@ RSpec.configure do |config| Sidekiq.redis(&:flushall) end - config.before(:example, :migration) do - ActiveRecord::Migrator - .migrate(migrations_paths, previous_migration.version) - - reset_column_in_migration_models + config.before(:each, :migration) do + schema_migrate_down! end - config.after(:example, :migration) do - ActiveRecord::Migrator.migrate(migrations_paths) - - reset_column_in_migration_models + config.after(:context, :migration) do + schema_migrate_up! end config.around(:each, :nested_groups) do |example| diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 7f5769209bb..b0f520d08e8 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -20,7 +20,7 @@ RSpec.configure do |config| end config.before(:each, :migration) do - DatabaseCleaner.strategy = :truncation + DatabaseCleaner.strategy = :truncation, { cache_tables: false } end config.before(:each) do diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb index 2011408be93..562423afc2a 100644 --- a/spec/support/import_export/export_file_helper.rb +++ b/spec/support/import_export/export_file_helper.rb @@ -35,7 +35,8 @@ module ExportFileHelper project: project, commit_id: ci_pipeline.sha) - create(:event, :created, target: milestone, project: project, author: user) + event = create(:event, :created, target: milestone, project: project, author: user, action: 5) + create(:push_event_payload, event: event) create(:project_member, :master, user: user, project: project) create(:ci_variable, project: project) create(:ci_trigger, project: project) diff --git a/spec/support/migrations_helpers.rb b/spec/support/migrations_helpers.rb index aabdad13047..4ca019c1b05 100644 --- a/spec/support/migrations_helpers.rb +++ b/spec/support/migrations_helpers.rb @@ -16,6 +16,8 @@ module MigrationsHelpers end def reset_column_in_migration_models + ActiveRecord::Base.clear_cache! + described_class.constants.sort.each do |name| const = described_class.const_get(name) @@ -31,6 +33,35 @@ module MigrationsHelpers end end + def migration_schema_version + self.class.metadata[:schema] || previous_migration.version + end + + def schema_migrate_down! + disable_migrations_output do + ActiveRecord::Migrator.migrate(migrations_paths, + migration_schema_version) + end + + reset_column_in_migration_models + end + + def schema_migrate_up! + disable_migrations_output do + ActiveRecord::Migrator.migrate(migrations_paths) + end + + reset_column_in_migration_models + end + + def disable_migrations_output + ActiveRecord::Migration.verbose = false + + yield + ensure + ActiveRecord::Migration.verbose = true + end + def migrate! ActiveRecord::Migrator.up(migrations_paths) do |migration| migration.name == described_class.name 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 |