diff options
author | Kamil Trzcinski <ayufan@ayufan.eu> | 2017-10-05 15:27:42 +0300 |
---|---|---|
committer | Kamil Trzcinski <ayufan@ayufan.eu> | 2017-10-05 15:27:42 +0300 |
commit | aba71835f310d5aad9c2af9e22c81bd44e6f0686 (patch) | |
tree | 40abb714359b8bed16bc5f43b7c4a085eb12d473 | |
parent | b229637b0837065a9993d06573b41218ab7e9dfb (diff) | |
parent | 1fe7a42afca83fbbda638d9569d58a87039cd8f2 (diff) |
Merge remote-tracking branch 'origin/master' into feature/sm/35954-create-kubernetes-cluster-on-gke-from-k8s-service
110 files changed, 952 insertions, 448 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 15d9117976a..efd32d44890 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 10.0.3 (2017-10-05) + +- [FIXED] find_user Users helper method no longer overrides find_user API helper method. !14418 +- [FIXED] Fix CSRF validation issue when closing/opening merge requests from the UI. !14555 +- [FIXED] Kubernetes integration: ensure v1.8.0 compatibility. !14635 +- [FIXED] Fixes data parameter not being sent in ajax request for jobs log. +- [FIXED] Improves UX of autodevops popover to match gpg one. +- [FIXED] Fixed commenting on side-by-side commit diff. +- [FIXED] Make sure API responds with 401 when invalid authentication info is provided. +- [FIXED] Fix merge request counter updates after merge. +- [FIXED] Fix gitlab-rake gitlab:import:repos task failing. +- [FIXED] Fix pushes to an empty repository not invalidating has_visible_content? cache. +- [FIXED] Ensure all refs are restored on a restore from backup. +- [FIXED] Gitaly RepositoryExists remains opt-in for all method calls. +- [FIXED] Fix 500 error on merged merge requests when GitLab is restored from a backup. +- [FIXED] Adjust MRs being stuck on "process of being merged" for more than 2 hours. + ## 10.0.2 (2017-09-27) - [FIXED] Notes will not show an empty bubble when the author isn't a member. !14450 @@ -195,6 +212,10 @@ entry. - Added type to CHANGELOG entries. (Jacopo Beschi @jacopo-beschi) - [BUGIFX] Improves subgroup creation permissions. !13418 +## 9.5.8 (2017-10-04) + +- [FIXED] Fixed fork button being disabled for users who can fork to a group. + ## 9.5.7 (2017-10-03) - Fix gitlab rake:import:repos task. diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index f80a26b3fd4..442ed86d50c 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -29,6 +29,7 @@ showEmptyState: true, updateAspectRatio: false, updatedAspectRatios: 0, + hoverData: {}, resizeThrottled: {}, }; }, @@ -64,6 +65,10 @@ this.updatedAspectRatios = 0; } }, + + hoverChanged(data) { + this.hoverData = data; + }, }, created() { @@ -72,10 +77,12 @@ deploymentEndpoint: this.deploymentEndpoint, }); eventHub.$on('toggleAspectRatio', this.toggleAspectRatio); + eventHub.$on('hoverChanged', this.hoverChanged); }, beforeDestroy() { eventHub.$off('toggleAspectRatio', this.toggleAspectRatio); + eventHub.$off('hoverChanged', this.hoverChanged); window.removeEventListener('resize', this.resizeThrottled, false); }, @@ -102,6 +109,7 @@ v-for="(graphData, index) in groupData.metrics" :key="index" :graph-data="graphData" + :hover-data="hoverData" :update-aspect-ratio="updateAspectRatio" :deployment-data="store.deploymentData" /> diff --git a/app/assets/javascripts/monitoring/components/empty_state.vue b/app/assets/javascripts/monitoring/components/empty_state.vue index a7b483f6786..a18164482a2 100644 --- a/app/assets/javascripts/monitoring/components/empty_state.vue +++ b/app/assets/javascripts/monitoring/components/empty_state.vue @@ -73,34 +73,22 @@ <template> <div class="prometheus-state"> - <div class="row"> - <div class="col-md-4 col-md-offset-4 state-svg svg-content"> - <img :src="currentState.svgUrl"/> - </div> + <div class="state-svg svg-content"> + <img :src="currentState.svgUrl"/> </div> - <div class="row"> - <div class="col-md-6 col-md-offset-3"> - <h4 class="text-center state-title"> - {{currentState.title}} - </h4> - </div> - </div> - <div class="row"> - <div class="col-md-6 col-md-offset-3"> - <div class="description-text text-center state-description"> - {{currentState.description}} - <a v-if="showButtonDescription" :href="settingsPath"> - Prometheus server - </a> - </div> - </div> - </div> - <div class="row state-button-section"> - <div class="col-md-4 col-md-offset-4 text-center state-button"> - <a class="btn btn-success" :href="buttonPath"> - {{currentState.buttonText}} - </a> - </div> + <h4 class="state-title"> + {{currentState.title}} + </h4> + <p class="state-description"> + {{currentState.description}} + <a v-if="showButtonDescription" :href="settingsPath"> + Prometheus server + </a> + </p> + <div class="state-button"> + <a class="btn btn-success" :href="buttonPath"> + {{currentState.buttonText}} + </a> </div> </div> </template> diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index 6b3e341f936..33611c408ef 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -7,12 +7,10 @@ import MonitoringMixin from '../mixins/monitoring_mixins'; import eventHub from '../event_hub'; import measurements from '../utils/measurements'; - import { timeScaleFormat } from '../utils/date_time_formatters'; + import { timeScaleFormat, bisectDate } from '../utils/date_time_formatters'; import createTimeSeries from '../utils/multiple_time_series'; import bp from '../../breakpoints'; - const bisectDate = d3.bisector(d => d.time).left; - export default { props: { graphData: { @@ -27,6 +25,11 @@ type: Array, required: true, }, + hoverData: { + type: Object, + required: false, + default: () => ({}), + }, }, mixins: [MonitoringMixin], @@ -52,6 +55,7 @@ currentXCoordinate: 0, currentFlagPosition: 0, showFlag: false, + showFlagContent: false, showDeployInfo: true, timeSeries: [], }; @@ -122,22 +126,14 @@ const d1 = firstTimeSeries.values[overlayIndex]; if (d0 === undefined || d1 === undefined) return; const evalTime = timeValueOverlay - d0[0] > d1[0] - timeValueOverlay; - this.currentData = evalTime ? d1 : d0; - this.currentDataIndex = evalTime ? overlayIndex : (overlayIndex - 1); - this.currentXCoordinate = Math.floor(firstTimeSeries.timeSeriesScaleX(this.currentData.time)); + const hoveredDataIndex = evalTime ? overlayIndex : (overlayIndex - 1); + const hoveredDate = firstTimeSeries.values[hoveredDataIndex].time; const currentDeployXPos = this.mouseOverDeployInfo(point.x); - if (this.currentXCoordinate > (this.graphWidth - 200)) { - this.currentFlagPosition = this.currentXCoordinate - 103; - } else { - this.currentFlagPosition = this.currentXCoordinate; - } - - if (currentDeployXPos) { - this.showFlag = false; - } else { - this.showFlag = true; - } + eventHub.$emit('hoverChanged', { + hoveredDate, + currentDeployXPos, + }); }, renderAxesPaths() { @@ -194,6 +190,10 @@ eventHub.$emit('toggleAspectRatio'); } }, + + hoverData() { + this.positionFlag(); + }, }, mounted() { @@ -203,7 +203,10 @@ </script> <template> - <div class="prometheus-graph"> + <div + class="prometheus-graph" + @mouseover="showFlagContent = true" + @mouseleave="showFlagContent = false"> <h5 class="text-center graph-title"> {{graphData.title}} </h5> @@ -247,6 +250,7 @@ <graph-deployment :show-deploy-info="showDeployInfo" :deployment-data="reducedDeploymentData" + :graph-width="graphWidth" :graph-height="graphHeight" :graph-height-offset="graphHeightOffset" /> @@ -257,6 +261,7 @@ :current-flag-position="currentFlagPosition" :graph-height="graphHeight" :graph-height-offset="graphHeightOffset" + :show-flag-content="showFlagContent" /> <rect class="prometheus-graph-overlay" diff --git a/app/assets/javascripts/monitoring/components/graph/deployment.vue b/app/assets/javascripts/monitoring/components/graph/deployment.vue index 3623d2ed946..e3b8be0c7fb 100644 --- a/app/assets/javascripts/monitoring/components/graph/deployment.vue +++ b/app/assets/javascripts/monitoring/components/graph/deployment.vue @@ -19,6 +19,10 @@ type: Number, required: true, }, + graphWidth: { + type: Number, + required: true, + }, }, computed: { @@ -47,6 +51,14 @@ transformDeploymentGroup(deployment) { return `translate(${Math.floor(deployment.xPos) + 1}, 20)`; }, + + positionFlag(deployment) { + let xPosition = 3; + if (deployment.xPos > (this.graphWidth - 200)) { + xPosition = -97; + } + return xPosition; + }, }, }; </script> @@ -77,7 +89,7 @@ <svg v-if="deployment.showDeploymentFlag" class="js-deploy-info-box" - x="3" + :x="positionFlag(deployment)" y="0" width="92" height="60"> diff --git a/app/assets/javascripts/monitoring/components/graph/flag.vue b/app/assets/javascripts/monitoring/components/graph/flag.vue index a98e3d06c18..10fb7ff6803 100644 --- a/app/assets/javascripts/monitoring/components/graph/flag.vue +++ b/app/assets/javascripts/monitoring/components/graph/flag.vue @@ -23,6 +23,10 @@ type: Number, required: true, }, + showFlagContent: { + type: Boolean, + required: true, + }, }, data() { @@ -57,6 +61,7 @@ transform="translate(-5, 20)"> </line> <svg + v-if="showFlagContent" class="rect-text-metric" :x="currentFlagPosition" y="0"> diff --git a/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js b/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js index 345a0b37a76..31f38aca5d6 100644 --- a/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js +++ b/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js @@ -1,3 +1,5 @@ +import { bisectDate } from '../utils/date_time_formatters'; + const mixins = { methods: { mouseOverDeployInfo(mouseXPos) { @@ -18,6 +20,7 @@ const mixins = { return dataFound; }, + formatDeployments() { this.reducedDeploymentData = this.deploymentData.reduce((deploymentDataArray, deployment) => { const time = new Date(deployment.created_at); @@ -40,6 +43,25 @@ const mixins = { return deploymentDataArray; }, []); }, + + positionFlag() { + const timeSeries = this.timeSeries[0]; + const hoveredDataIndex = bisectDate(timeSeries.values, this.hoverData.hoveredDate, 1); + this.currentData = timeSeries.values[hoveredDataIndex]; + this.currentDataIndex = hoveredDataIndex; + this.currentXCoordinate = Math.floor(timeSeries.timeSeriesScaleX(this.currentData.time)); + if (this.currentXCoordinate > (this.graphWidth - 200)) { + this.currentFlagPosition = this.currentXCoordinate - 103; + } else { + this.currentFlagPosition = this.currentXCoordinate; + } + + if (this.hoverData.currentDeployXPos) { + this.showFlag = false; + } else { + this.showFlag = true; + } + }, }, }; diff --git a/app/assets/javascripts/monitoring/monitoring_bundle.js b/app/assets/javascripts/monitoring/monitoring_bundle.js index ef280e02092..104432ef5de 100644 --- a/app/assets/javascripts/monitoring/monitoring_bundle.js +++ b/app/assets/javascripts/monitoring/monitoring_bundle.js @@ -3,8 +3,5 @@ import Dashboard from './components/dashboard.vue'; document.addEventListener('DOMContentLoaded', () => new Vue({ el: '#prometheus-graphs', - components: { - Dashboard, - }, - render: createElement => createElement('dashboard'), + render: createElement => createElement(Dashboard), })); diff --git a/app/assets/javascripts/monitoring/stores/monitoring_store.js b/app/assets/javascripts/monitoring/stores/monitoring_store.js index 7592af5878e..854636e9a89 100644 --- a/app/assets/javascripts/monitoring/stores/monitoring_store.js +++ b/app/assets/javascripts/monitoring/stores/monitoring_store.js @@ -13,7 +13,7 @@ function normalizeMetrics(metrics) { ...result, values: result.values.map(([timestamp, value]) => ({ time: new Date(timestamp * 1000), - value, + value: Number(value), })), })), })), diff --git a/app/assets/javascripts/monitoring/utils/date_time_formatters.js b/app/assets/javascripts/monitoring/utils/date_time_formatters.js index 26bcaa02511..c4c6b1ac1f5 100644 --- a/app/assets/javascripts/monitoring/utils/date_time_formatters.js +++ b/app/assets/javascripts/monitoring/utils/date_time_formatters.js @@ -2,6 +2,7 @@ import d3 from 'd3'; export const dateFormat = d3.time.format('%b %-d, %Y'); export const timeFormat = d3.time.format('%-I:%M%p'); +export const bisectDate = d3.bisector(d => d.time).left; export const timeScaleFormat = d3.time.format.multi([ ['.%L', d => d.getMilliseconds()], diff --git a/app/assets/javascripts/repo/components/repo_sidebar.vue b/app/assets/javascripts/repo/components/repo_sidebar.vue index 1e40814b95f..685f6ff806f 100644 --- a/app/assets/javascripts/repo/components/repo_sidebar.vue +++ b/app/assets/javascripts/repo/components/repo_sidebar.vue @@ -74,8 +74,8 @@ export default { <thead v-if="!isMini"> <tr> <th class="name">Name</th> - <th class="hidden-sm hidden-xs last-commit">Last Commit</th> - <th class="hidden-xs last-update text-right">Last Update</th> + <th class="hidden-sm hidden-xs last-commit">Last commit</th> + <th class="hidden-xs last-update text-right">Last update</th> </tr> </thead> <tbody> diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 9362d80d4e6..6a9c2578d95 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -207,10 +207,13 @@ } .prometheus-state { - margin-top: 10px; + max-width: 430px; + margin: 10px auto; + text-align: center; - .state-button-section { - margin-top: 10px; + .state-svg { + max-width: 80vw; + margin: 0 auto; } } diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 67abe6e88ed..6c521bb06ee 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -392,11 +392,11 @@ table.u2f-registrations { } } -.gpg-email-badge { +.email-badge { display: inline; margin-right: $gl-padding / 2; - .gpg-email-badge-email { + .email-badge-email { display: inline; margin-right: $gl-padding / 4; } diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 676a7203c7d..156a8e2c515 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -155,7 +155,7 @@ class Admin::UsersController < Admin::ApplicationController def remove_email email = user.emails.find(params[:email_id]) - success = Emails::DestroyService.new(current_user, user: user, email: email.email).execute + success = Emails::DestroyService.new(current_user, user: user).execute(email) respond_to do |format| if success diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 771c6f3034a..967fe39256a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -85,12 +85,21 @@ class ApplicationController < ActionController::Base super payload[:remote_ip] = request.remote_ip - if current_user.present? - payload[:user_id] = current_user.id - payload[:username] = current_user.username + logged_user = auth_user + + if logged_user.present? + payload[:user_id] = logged_user.try(:id) + payload[:username] = logged_user.try(:username) end end + # Controllers such as GitHttpController may use alternative methods + # (e.g. tokens) to authenticate the user, whereas Devise sets current_user + def auth_user + return current_user if current_user.present? + return try(:authenticated_user) + end + # This filter handles both private tokens and personal access tokens def authenticate_user_from_private_token! token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 0c2646d7bf0..80ab681ed87 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -10,13 +10,14 @@ class ConfirmationsController < Devise::ConfirmationsController users_almost_there_path end - def after_confirmation_path_for(resource_name, resource) - if signed_in?(resource_name) + def after_confirmation_path_for(_resource_name, resource) + # incoming resource can either be a :user or an :email + if signed_in?(:user) after_sign_in(resource) else Gitlab::AppLogger.info("Email Confirmed: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip}") flash[:notice] += " Please sign in." - new_session_path(resource_name) + new_session_path(:user) end end diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb index 97db84b92d4..bbd7ba49d77 100644 --- a/app/controllers/profiles/emails_controller.rb +++ b/app/controllers/profiles/emails_controller.rb @@ -1,15 +1,14 @@ class Profiles::EmailsController < Profiles::ApplicationController + before_action :find_email, only: [:destroy, :resend_confirmation_instructions] + def index - @primary = current_user.email + @primary_email = current_user.email @emails = current_user.emails.order_id_desc end def create @email = Emails::CreateService.new(current_user, email_params.merge(user: current_user)).execute - - if @email.errors.blank? - NotificationService.new.new_email(@email) - else + unless @email.errors.blank? flash[:alert] = @email.errors.full_messages.first end @@ -17,9 +16,7 @@ class Profiles::EmailsController < Profiles::ApplicationController end def destroy - @email = current_user.emails.find(params[:id]) - - Emails::DestroyService.new(current_user, user: current_user, email: @email.email).execute + Emails::DestroyService.new(current_user, user: current_user).execute(@email) respond_to do |format| format.html { redirect_to profile_emails_url, status: 302 } @@ -27,9 +24,23 @@ class Profiles::EmailsController < Profiles::ApplicationController end end + def resend_confirmation_instructions + if Emails::ConfirmService.new(current_user, user: current_user).execute(@email) + flash[:notice] = "Confirmation email sent to #{@email.email}" + else + flash[:alert] = "There was a problem sending the confirmation email" + end + + redirect_to profile_emails_url + end + private def email_params params.require(:email).permit(:email) end + + def find_email + @email = current_user.emails.find(params[:id]) + end end diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 7d0e2b3e2ef..95d7a02e9e9 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -9,6 +9,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController delegate :actor, :authentication_abilities, to: :authentication_result, allow_nil: true alias_method :user, :actor + alias_method :authenticated_user, :actor # Git clients will not know what authenticity token to send along skip_before_action :verify_authenticity_token diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index c401030e34a..4f5edeb9bda 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -7,12 +7,6 @@ module Emails mail(to: @user.notification_email, subject: subject("Account was created for you")) end - def new_email_email(email_id) - @email = Email.find(email_id) - @current_user = @user = @email.user - mail(to: @user.notification_email, subject: subject("Email was added to your account")) - end - def new_ssh_key_email(key_id) @key = Key.find_by(id: key_id) diff --git a/app/models/email.rb b/app/models/email.rb index 826d4f16edb..384f38f2db7 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -7,6 +7,13 @@ class Email < ActiveRecord::Base validates :email, presence: true, uniqueness: true, email: true validate :unique_email, if: ->(email) { email.email_changed? } + scope :confirmed, -> { where.not(confirmed_at: nil) } + + after_commit :update_invalid_gpg_signatures, if: -> { previous_changes.key?('confirmed_at') } + + devise :confirmable + self.reconfirmable = false # currently email can't be changed, no need to reconfirm + def email=(value) write_attribute(:email, value.downcase.strip) end @@ -14,4 +21,9 @@ class Email < ActiveRecord::Base def unique_email self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email) end + + # once email is confirmed, update the gpg signatures + def update_invalid_gpg_signatures + user.update_invalid_gpg_signatures if confirmed? + end end diff --git a/app/models/user.rb b/app/models/user.rb index 4e71a3e11c2..4ba9130a75a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -163,15 +163,16 @@ class User < ActiveRecord::Base before_validation :sanitize_attrs before_validation :set_notification_email, if: :email_changed? before_validation :set_public_email, if: :public_email_changed? - - after_update :update_emails_with_primary_email, if: :email_changed? before_save :ensure_authentication_token, :ensure_incoming_email_token before_save :ensure_user_rights_and_limits, if: :external_changed? before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) } + before_save :check_for_verified_email, if: ->(user) { user.email_changed? && !user.new_record? } after_save :ensure_namespace_correct + after_destroy :post_destroy_hook + after_commit :update_emails_with_primary_email, on: :update, if: -> { previous_changes.key?('email') } after_commit :update_invalid_gpg_signatures, on: :update, if: -> { previous_changes.key?('email') } + after_initialize :set_projects_limit - after_destroy :post_destroy_hook # User's Layout preference enum layout: [:fixed, :fluid] @@ -525,12 +526,24 @@ class User < ActiveRecord::Base errors.add(:public_email, "is not an email you own") unless all_emails.include?(public_email) end + # see if the new email is already a verified secondary email + def check_for_verified_email + skip_reconfirmation! if emails.confirmed.where(email: self.email).any? + end + + # Note: the use of the Emails services will cause `saves` on the user object, running + # through the callbacks again and can have side effects, such as the `previous_changes` + # hash and `_was` variables getting munged. + # By using an `after_commit` instead of `after_update`, we avoid the recursive callback + # scenario, though it then requires us to use the `previous_changes` hash def update_emails_with_primary_email + previous_email = previous_changes[:email][0] # grab this before the DestroyService is called primary_email_record = emails.find_by(email: email) - if primary_email_record - Emails::DestroyService.new(self, user: self, email: email).execute - Emails::CreateService.new(self, user: self, email: email_was).execute - end + Emails::DestroyService.new(self, user: self).execute(primary_email_record) if primary_email_record + + # the original primary email was confirmed, and we want that to carry over. We don't + # have access to the original confirmation values at this point, so just set confirmed_at + Emails::CreateService.new(self, user: self, email: previous_email).execute(confirmed_at: confirmed_at) end def update_invalid_gpg_signatures @@ -816,6 +829,10 @@ class User < ActiveRecord::Base avatar_path(args) || GravatarService.new.execute(email, size, scale, username: username) end + def primary_email_verified? + confirmed? && !temp_oauth_email? + end + def all_emails all_emails = [] all_emails << email unless temp_oauth_email? @@ -823,6 +840,18 @@ class User < ActiveRecord::Base all_emails end + def verified_emails + verified_emails = [] + verified_emails << email if primary_email_verified? + verified_emails.concat(emails.confirmed.pluck(:email)) + verified_emails + end + + def verified_email?(check_email) + downcased = check_email.downcase + email == downcased ? primary_email_verified? : emails.confirmed.where(email: downcased).exists? + end + def hook_attrs { name: name, @@ -1047,10 +1076,6 @@ class User < ActiveRecord::Base ensure_rss_token! end - def verified_email?(email) - self.email == email - end - def sync_attribute?(attribute) return true if ldap_user? && attribute == :email diff --git a/app/serializers/commit_entity.rb b/app/serializers/commit_entity.rb index e4e9d8ef90a..c8dd98cc04d 100644 --- a/app/serializers/commit_entity.rb +++ b/app/serializers/commit_entity.rb @@ -1,4 +1,4 @@ -class CommitEntity < API::Entities::RepoCommit +class CommitEntity < API::Entities::Commit include RequestAwareEntity expose :author, using: UserEntity diff --git a/app/services/emails/base_service.rb b/app/services/emails/base_service.rb index 7f591c89411..5bbceeb3b3f 100644 --- a/app/services/emails/base_service.rb +++ b/app/services/emails/base_service.rb @@ -1,9 +1,8 @@ module Emails class BaseService - def initialize(current_user, opts) - @current_user = current_user - @user = opts.delete(:user) - @email = opts[:email] + def initialize(current_user, params = {}) + @current_user, @params = current_user, params.dup + @user = params.delete(:user) end end end diff --git a/app/services/emails/confirm_service.rb b/app/services/emails/confirm_service.rb new file mode 100644 index 00000000000..b5301bf2b82 --- /dev/null +++ b/app/services/emails/confirm_service.rb @@ -0,0 +1,7 @@ +module Emails + class ConfirmService < ::Emails::BaseService + def execute(email) + email.resend_confirmation_instructions + end + end +end diff --git a/app/services/emails/create_service.rb b/app/services/emails/create_service.rb index b6491ee9804..94a841af7c3 100644 --- a/app/services/emails/create_service.rb +++ b/app/services/emails/create_service.rb @@ -1,7 +1,7 @@ module Emails class CreateService < ::Emails::BaseService - def execute - @user.emails.create(email: @email) + def execute(extra_params = {}) + @user.emails.create(@params.merge(extra_params)) end end end diff --git a/app/services/emails/destroy_service.rb b/app/services/emails/destroy_service.rb index 44011cc36c8..1ed131fe326 100644 --- a/app/services/emails/destroy_service.rb +++ b/app/services/emails/destroy_service.rb @@ -1,7 +1,7 @@ module Emails class DestroyService < ::Emails::BaseService - def execute - update_secondary_emails! if Email.find_by_email!(@email).destroy + def execute(email) + email.destroy && update_secondary_emails! end private diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index e2a80db06a6..8d5da459882 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -31,13 +31,6 @@ class NotificationService end end - # Always notify user about email added to profile - def new_email(email) - if email.user&.can?(:receive_notifications) - mailer.new_email_email(email.id).deliver_later - end - end - # When create an issue we should send an email to: # # * issue assignee if their notification level is not Disabled diff --git a/app/views/devise/mailer/_confirmation_instructions_account.html.haml b/app/views/devise/mailer/_confirmation_instructions_account.html.haml new file mode 100644 index 00000000000..65565b7b8a8 --- /dev/null +++ b/app/views/devise/mailer/_confirmation_instructions_account.html.haml @@ -0,0 +1,16 @@ +- confirmation_link = confirmation_url(@resource, confirmation_token: @token) +- if @resource.unconfirmed_email.present? + #content + = email_default_heading(@resource.unconfirmed_email) + %p Click the link below to confirm your email address. + #cta + = link_to 'Confirm your email address', confirmation_link +- else + #content + - if Gitlab.com? + = email_default_heading('Thanks for signing up to GitLab!') + - else + = email_default_heading("Welcome, #{@resource.name}!") + %p To get started, click the link below to confirm your account. + #cta + = link_to 'Confirm your account', confirmation_link diff --git a/app/views/devise/mailer/_confirmation_instructions_account.text.erb b/app/views/devise/mailer/_confirmation_instructions_account.text.erb new file mode 100644 index 00000000000..01f09aa763d --- /dev/null +++ b/app/views/devise/mailer/_confirmation_instructions_account.text.erb @@ -0,0 +1,14 @@ +<% if @resource.unconfirmed_email.present? %> +<%= @resource.unconfirmed_email %>, + +Use the link below to confirm your email address. +<% else %> + <% if Gitlab.com? %> +Thanks for signing up to GitLab! + <% else %> +Welcome, <%= @resource.name %>! + <% end %> +To get started, use the link below to confirm your account. +<% end %> + +<%= confirmation_url(@resource, confirmation_token: @token) %> diff --git a/app/views/devise/mailer/_confirmation_instructions_secondary.html.haml b/app/views/devise/mailer/_confirmation_instructions_secondary.html.haml new file mode 100644 index 00000000000..3d0a1f622a5 --- /dev/null +++ b/app/views/devise/mailer/_confirmation_instructions_secondary.html.haml @@ -0,0 +1,8 @@ +#content + = email_default_heading("#{@resource.user.name}, you've added an additional email!") + %p Click the link below to confirm your email address (#{@resource.email}) + #cta + = link_to 'Confirm your email address', confirmation_url(@resource, confirmation_token: @token) + %p + If this email was added in error, you can remove it here: + = link_to "Emails", profile_emails_url diff --git a/app/views/devise/mailer/_confirmation_instructions_secondary.text.erb b/app/views/devise/mailer/_confirmation_instructions_secondary.text.erb new file mode 100644 index 00000000000..a3b28cb0b84 --- /dev/null +++ b/app/views/devise/mailer/_confirmation_instructions_secondary.text.erb @@ -0,0 +1,7 @@ +<%= @resource.user.name %>, you've added an additional email! + +Use the link below to confirm your email address (<%= @resource.email %>) + +<%= confirmation_url(@resource, confirmation_token: @token) %> + +If this email was added in error, you can remove it here: <%= profile_emails_url %> diff --git a/app/views/devise/mailer/confirmation_instructions.html.haml b/app/views/devise/mailer/confirmation_instructions.html.haml index 4d1037807be..50ee7b53d8f 100644 --- a/app/views/devise/mailer/confirmation_instructions.html.haml +++ b/app/views/devise/mailer/confirmation_instructions.html.haml @@ -1,16 +1 @@ -- confirmation_link = confirmation_url(@resource, confirmation_token: @token) -- if @resource.unconfirmed_email.present? - #content - = email_default_heading(@resource.unconfirmed_email) - %p Click the link below to confirm your email address. - #cta - = link_to confirmation_link, confirmation_link -- else - #content - - if Gitlab.com? - = email_default_heading('Thanks for signing up to GitLab!') - - else - = email_default_heading("Welcome, #{@resource.name}!") - %p To get started, click the link below to confirm your account. - #cta - = link_to confirmation_link, confirmation_link += render partial: "confirmation_instructions_#{@resource.is_a?(User) ? 'account' : 'secondary'}" diff --git a/app/views/devise/mailer/confirmation_instructions.text.erb b/app/views/devise/mailer/confirmation_instructions.text.erb index 9f76edb76a4..05fddddf415 100644 --- a/app/views/devise/mailer/confirmation_instructions.text.erb +++ b/app/views/devise/mailer/confirmation_instructions.text.erb @@ -1,9 +1 @@ -Welcome, <%= @resource.name %>! - -<% if @resource.unconfirmed_email.present? %> -You can confirm your email (<%= @resource.unconfirmed_email %>) through the link below: -<% else %> -You can confirm your account through the link below: -<% end %> - -<%= confirmation_url(@resource, confirmation_token: @token) %> +<%= render partial: "confirmation_instructions_#{@resource.is_a?(User) ? 'account' : 'secondary'}" %>
\ No newline at end of file diff --git a/app/views/groups/milestones/_header_title.html.haml b/app/views/groups/milestones/_header_title.html.haml index d7fabf53587..24eb39b8e2f 100644 --- a/app/views/groups/milestones/_header_title.html.haml +++ b/app/views/groups/milestones/_header_title.html.haml @@ -1 +1,2 @@ -- header_title group_title(@group, "Milestones", group_milestones_path(@group)) +- breadcrumb_title @milestone.title +- add_to_breadcrumbs "Milestones", group_milestones_path(@group) diff --git a/app/views/notify/new_email_email.html.haml b/app/views/notify/new_email_email.html.haml deleted file mode 100644 index 4a0448a573c..00000000000 --- a/app/views/notify/new_email_email.html.haml +++ /dev/null @@ -1,10 +0,0 @@ -%p - Hi #{@user.name}! -%p - A new email was added to your account: -%p - email: - %code= @email.email -%p - If this email was added in error, you can remove it here: - = link_to "Emails", profile_emails_url diff --git a/app/views/notify/new_email_email.text.erb b/app/views/notify/new_email_email.text.erb deleted file mode 100644 index 51cba99ad0d..00000000000 --- a/app/views/notify/new_email_email.text.erb +++ /dev/null @@ -1,7 +0,0 @@ -Hi <%= @user.name %>! - -A new email was added to your account: - -email.................. <%= @email.email %> - -If this email was added in error, you can remove it here: <%= profile_emails_url %> diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index 612ecbbb96a..df1df4f5d72 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -32,19 +32,25 @@ All email addresses will be used to identify your commits. %ul.well-list %li - = @primary + = render partial: 'shared/email_with_badge', locals: { email: @primary_email, verified: current_user.confirmed? } %span.pull-right %span.label.label-success Primary email - - if @primary === current_user.public_email + - if @primary_email === current_user.public_email %span.label.label-info Public email - - if @primary === current_user.notification_email + - if @primary_email === current_user.notification_email %span.label.label-info Notification email - @emails.each do |email| %li - = email.email + = render partial: 'shared/email_with_badge', locals: { email: email.email, verified: email.confirmed? } %span.pull-right - if email.email === current_user.public_email %span.label.label-info Public email - if email.email === current_user.notification_email %span.label.label-info Notification email - = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-warning prepend-left-10' + - unless email.confirmed? + - confirm_title = "#{email.confirmation_sent_at ? 'Resend' : 'Send'} confirmation email" + = link_to confirm_title, resend_confirmation_instructions_profile_email_path(email), method: :put, class: 'btn btn-sm btn-warning prepend-left-10' + + = link_to profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-danger prepend-left-10' do + %span.sr-only Remove + = icon('trash') diff --git a/app/views/profiles/gpg_keys/_key.html.haml b/app/views/profiles/gpg_keys/_key.html.haml index b04981f90e3..970e92aadaa 100644 --- a/app/views/profiles/gpg_keys/_key.html.haml +++ b/app/views/profiles/gpg_keys/_key.html.haml @@ -3,7 +3,7 @@ = icon 'key', class: "settings-list-icon hidden-xs" .key-list-item-info - key.emails_with_verified_status.map do |email, verified| - = render partial: 'email_with_badge', locals: { email: email, verified: verified } + = render partial: 'shared/email_with_badge', locals: { email: email, verified: verified } .description %code= key.fingerprint diff --git a/app/views/projects/tree/_old_tree_content.html.haml b/app/views/projects/tree/_old_tree_content.html.haml index 820b947804e..6ea78851b8d 100644 --- a/app/views/projects/tree/_old_tree_content.html.haml +++ b/app/views/projects/tree/_old_tree_content.html.haml @@ -6,7 +6,7 @@ %th= s_('ProjectFileTree|Name') %th.hidden-xs .pull-left= _('Last commit') - %th.text-right= _('Last Update') + %th.text-right= _('Last update') - if @path.present? %tr.tree-item %td.tree-item-file-name diff --git a/app/views/profiles/gpg_keys/_email_with_badge.html.haml b/app/views/shared/_email_with_badge.html.haml index 5f7844584e1..b7bbc109238 100644 --- a/app/views/profiles/gpg_keys/_email_with_badge.html.haml +++ b/app/views/shared/_email_with_badge.html.haml @@ -2,7 +2,7 @@ - css_classes << (verified ? 'verified': 'unverified') - text = verified ? 'Verified' : 'Unverified' -.gpg-email-badge - .gpg-email-badge-email= email +.email-badge + .email-badge-email= email %div{ class: css_classes } = text diff --git a/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml b/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml deleted file mode 100644 index 1984ec6e81c..00000000000 --- a/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: find_user Users helper method no longer overrides find_user API helper method. -merge_request: 14418 -author: -type: fixed diff --git a/changelogs/unreleased/38031-monitoring-hover-info-is-clipped.yml b/changelogs/unreleased/38031-monitoring-hover-info-is-clipped.yml new file mode 100644 index 00000000000..8b3fae2c103 --- /dev/null +++ b/changelogs/unreleased/38031-monitoring-hover-info-is-clipped.yml @@ -0,0 +1,6 @@ +--- +title: Move the deployment flag content to the left when deployment marker is near + the end +merge_request: 14514 +author: +type: fixed diff --git a/changelogs/unreleased/38036-hover-and-legend-data-should-be-linked.yml b/changelogs/unreleased/38036-hover-and-legend-data-should-be-linked.yml new file mode 100644 index 00000000000..591e542cd17 --- /dev/null +++ b/changelogs/unreleased/38036-hover-and-legend-data-should-be-linked.yml @@ -0,0 +1,5 @@ +--- +title: Sync up hover and legend data across all graphs for the prometheus dashboard +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml b/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml deleted file mode 100644 index 43dec51029b..00000000000 --- a/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Adjust MRs being stuck on "process of being merged" for more than 2 hours -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/38528-build-url.yml b/changelogs/unreleased/38528-build-url.yml deleted file mode 100644 index 357b9aacea8..00000000000 --- a/changelogs/unreleased/38528-build-url.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fixes data parameter not being sent in ajax request for jobs log -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/38582-popover-badge.yml b/changelogs/unreleased/38582-popover-badge.yml deleted file mode 100644 index ccec679a13f..00000000000 --- a/changelogs/unreleased/38582-popover-badge.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improves UX of autodevops popover to match gpg one -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/38789-prometheus-graphs-occasionally-have-incorrect-y-scale.yml b/changelogs/unreleased/38789-prometheus-graphs-occasionally-have-incorrect-y-scale.yml new file mode 100644 index 00000000000..bbfe5d49a3e --- /dev/null +++ b/changelogs/unreleased/38789-prometheus-graphs-occasionally-have-incorrect-y-scale.yml @@ -0,0 +1,5 @@ +--- +title: Fix broken Y-axis scaling in some Prometheus graphs +merge_request: 14693 +author: +type: fixed diff --git a/changelogs/unreleased/bvl-fix-close-issuable-link.yml b/changelogs/unreleased/bvl-fix-close-issuable-link.yml deleted file mode 100644 index 140a9d35cc1..00000000000 --- a/changelogs/unreleased/bvl-fix-close-issuable-link.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix CSRF validation issue when closing/opening merge requests from the UI -merge_request: 14555 -author: -type: fixed diff --git a/changelogs/unreleased/commit-side-by-side-comment.yml b/changelogs/unreleased/commit-side-by-side-comment.yml deleted file mode 100644 index f9bea285a77..00000000000 --- a/changelogs/unreleased/commit-side-by-side-comment.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fixed commenting on side-by-side commit diff -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/declarative-policy-optimisations.yml b/changelogs/unreleased/declarative-policy-optimisations.yml new file mode 100644 index 00000000000..dc51c89d575 --- /dev/null +++ b/changelogs/unreleased/declarative-policy-optimisations.yml @@ -0,0 +1,5 @@ +--- +title: Speed up permission checks +merge_request: +author: +type: other diff --git a/changelogs/unreleased/dm-api-unauthorized.yml b/changelogs/unreleased/dm-api-unauthorized.yml deleted file mode 100644 index 26b45bd4c40..00000000000 --- a/changelogs/unreleased/dm-api-unauthorized.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Make sure API responds with 401 when invalid authentication info is provided -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/feature-verify_secondary_emails.yml b/changelogs/unreleased/feature-verify_secondary_emails.yml new file mode 100644 index 00000000000..e1ecc527f85 --- /dev/null +++ b/changelogs/unreleased/feature-verify_secondary_emails.yml @@ -0,0 +1,5 @@ +--- +title: A confirmation email is now sent when adding a secondary email address +merge_request: +author: digitalmoksha +type: added diff --git a/changelogs/unreleased/fix-kubectl-180.yml b/changelogs/unreleased/fix-kubectl-180.yml deleted file mode 100644 index beb71cecd57..00000000000 --- a/changelogs/unreleased/fix-kubectl-180.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: 'Kubernetes integration: ensure v1.8.0 compatibility' -merge_request: 14635 -author: -type: fixed diff --git a/changelogs/unreleased/fix-mr-sidebar-counter-after-merge.yml b/changelogs/unreleased/fix-mr-sidebar-counter-after-merge.yml deleted file mode 100644 index 22a3efb8b1e..00000000000 --- a/changelogs/unreleased/fix-mr-sidebar-counter-after-merge.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix merge request counter updates after merge -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/fork-btn-enabled-user-groups.yml b/changelogs/unreleased/fork-btn-enabled-user-groups.yml deleted file mode 100644 index 3bd7581a961..00000000000 --- a/changelogs/unreleased/fork-btn-enabled-user-groups.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fixed fork button being disabled for users who can fork to a group -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/group-milestones-breadcrumb.yml b/changelogs/unreleased/group-milestones-breadcrumb.yml new file mode 100644 index 00000000000..87085759fda --- /dev/null +++ b/changelogs/unreleased/group-milestones-breadcrumb.yml @@ -0,0 +1,5 @@ +--- +title: Fixed milestone breadcrumb links +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/remove_repo_prefix_from_api.yml b/changelogs/unreleased/remove_repo_prefix_from_api.yml new file mode 100644 index 00000000000..bf2075e529c --- /dev/null +++ b/changelogs/unreleased/remove_repo_prefix_from_api.yml @@ -0,0 +1,5 @@ +--- +title: Remove 'Repo' prefix from API entites +merge_request: 14694 +author: Vitaliy @blackst0ne Klachkov +type: other diff --git a/changelogs/unreleased/sh-fix-import-repos.yml b/changelogs/unreleased/sh-fix-import-repos.yml deleted file mode 100644 index 5764b3bdc01..00000000000 --- a/changelogs/unreleased/sh-fix-import-repos.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix gitlab-rake gitlab:import:repos task failing -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/sh-fix-issue-38646.yml b/changelogs/unreleased/sh-fix-issue-38646.yml deleted file mode 100644 index 5c205775662..00000000000 --- a/changelogs/unreleased/sh-fix-issue-38646.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix pushes to an empty repository not invalidating has_visible_content? cache -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/sh-fix-username-logging.yml b/changelogs/unreleased/sh-fix-username-logging.yml new file mode 100644 index 00000000000..dadf3fb6729 --- /dev/null +++ b/changelogs/unreleased/sh-fix-username-logging.yml @@ -0,0 +1,5 @@ +--- +title: Fix username and ID not logging in production_json.log for Git activity +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sh-restore-all-refs-backup.yml b/changelogs/unreleased/sh-restore-all-refs-backup.yml deleted file mode 100644 index eaac0c71dd0..00000000000 --- a/changelogs/unreleased/sh-restore-all-refs-backup.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Ensure all refs are restored on a restore from backup -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/zj-repo-gitaly.yml b/changelogs/unreleased/zj-repo-gitaly.yml deleted file mode 100644 index 634f6ba1b8b..00000000000 --- a/changelogs/unreleased/zj-repo-gitaly.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Gitaly RepositoryExists remains opt-in for all method calls -merge_request: -author: -type: fixed diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 0ba0d791054..c6ec0aeda7b 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -175,7 +175,7 @@ Devise.setup do |config| # Configure the default scope given to Warden. By default it's the first # devise role declared in your routes (usually :user). - # config.default_scope = :user + config.default_scope = :user # now have an :email scope as well, so set the default # Configure sign_out behavior. # Sign_out action can be scoped (i.e. /users/sign_out affects only :user scope). diff --git a/config/routes/profile.rb b/config/routes/profile.rb index 3e4e6111ab8..ddc852f0132 100644 --- a/config/routes/profile.rb +++ b/config/routes/profile.rb @@ -1,3 +1,6 @@ +# for secondary email confirmations - uses the same confirmation controller as :users +devise_for :emails, path: 'profile/emails', controllers: { confirmations: :confirmations } + resource :profile, only: [:show, :update] do member do get :audit_log @@ -28,7 +31,11 @@ resource :profile, only: [:show, :update] do put :revoke end end - resources :emails, only: [:index, :create, :destroy] + resources :emails, only: [:index, :create, :destroy] do + member do + put :resend_confirmation_instructions + end + end resources :chat_names, only: [:index, :new, :create, :destroy] do collection do delete :deny diff --git a/db/migrate/20170904092148_add_email_confirmation.rb b/db/migrate/20170904092148_add_email_confirmation.rb new file mode 100644 index 00000000000..17ff424b319 --- /dev/null +++ b/db/migrate/20170904092148_add_email_confirmation.rb @@ -0,0 +1,33 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddEmailConfirmation < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :emails, :confirmation_token, :string + add_column :emails, :confirmed_at, :datetime_with_timezone + add_column :emails, :confirmation_sent_at, :datetime_with_timezone + end +end diff --git a/db/migrate/20170909090114_add_email_confirmation_index.rb b/db/migrate/20170909090114_add_email_confirmation_index.rb new file mode 100644 index 00000000000..a8c1023c482 --- /dev/null +++ b/db/migrate/20170909090114_add_email_confirmation_index.rb @@ -0,0 +1,36 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddEmailConfirmationIndex < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + disable_ddl_transaction! + + # Not necessary to remove duplicates, as :confirmation_token is a new column + def up + add_concurrent_index :emails, :confirmation_token, unique: true + end + + def down + remove_concurrent_index :emails, :confirmation_token if index_exists?(:emails, :confirmation_token) + end +end diff --git a/db/schema.rb b/db/schema.rb index e7391ac9826..00a52c1dffe 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -514,8 +514,12 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.string "email", null: false t.datetime "created_at" t.datetime "updated_at" + t.string "confirmation_token" + t.datetime "confirmed_at" + t.datetime "confirmation_sent_at" end + add_index "emails", ["confirmation_token"], name: "index_emails_on_confirmation_token", unique: true, using: :btree add_index "emails", ["email"], name: "index_emails_on_email", unique: true, using: :btree add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree diff --git a/doc/administration/job_artifacts.md b/doc/administration/job_artifacts.md index 3587696225c..86b436d89dd 100644 --- a/doc/administration/job_artifacts.md +++ b/doc/administration/job_artifacts.md @@ -142,9 +142,9 @@ and [projects APIs](../api/projects.md). ## Implementation details When GitLab receives an artifacts archive, an archive metadata file is also -generated. This metadata file describes all the entries that are located in the -artifacts archive itself. The metadata file is in a binary format, with -additional GZIP compression. +generated by [GitLab Workhorse]. This metadata file describes all the entries +that are located in the artifacts archive itself. +The metadata file is in a binary format, with additional GZIP compression. GitLab does not extract the artifacts archive in order to save space, memory and disk I/O. It instead inspects the metadata file which contains all the diff --git a/doc/user/project/import/github.md b/doc/user/project/import/github.md index 016f98966e3..6423beefc77 100644 --- a/doc/user/project/import/github.md +++ b/doc/user/project/import/github.md @@ -42,6 +42,11 @@ The importer will create any new namespaces (groups) if they don't exist or in the case the namespace is taken, the repository will be imported under the user's namespace that started the import process. +The importer will also import branches on forks of projects related to open pull +requests. These branches will be imported with a naming scheume similar to +GH-SHA-Username/Pull-Request-number/fork-name/branch. This may lead to a discrepency +in branches compared to the GitHub Repository. + ## Importing your GitHub repositories The importer page is visible when you create a new project. diff --git a/doc/user/project/repository/gpg_signed_commits/index.md b/doc/user/project/repository/gpg_signed_commits/index.md index 29e04a0ccf0..6b9976d133c 100644 --- a/doc/user/project/repository/gpg_signed_commits/index.md +++ b/doc/user/project/repository/gpg_signed_commits/index.md @@ -26,7 +26,7 @@ to be uploaded to GitLab. For a signature to be verified three conditions need to be met: 1. The public key needs to be added your GitLab account -1. One of the emails in the GPG key matches your **primary** email +1. One of the emails in the GPG key matches a **verified** email address you use in GitLab 1. The committer's email matches the verified email from the gpg key ## Generating a GPG key @@ -94,7 +94,7 @@ started: ``` 1. Enter you real name, the email address to be associated with this key (should - match the primary email address you use in GitLab) and an optional comment + match a verified email address you use in GitLab) and an optional comment (press <kbd>Enter</kbd> to skip): ``` diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 643c8e6fb8e..61a2d688282 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -13,7 +13,7 @@ module API end resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do desc 'Get a project repository branches' do - success Entities::RepoBranch + success Entities::Branch end params do use :pagination @@ -23,13 +23,13 @@ module API # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37442 Gitlab::GitalyClient.allow_n_plus_1_calls do - present paginate(branches), with: Entities::RepoBranch, project: user_project + present paginate(branches), with: Entities::Branch, project: user_project end end resource ':id/repository/branches/:branch', requirements: BRANCH_ENDPOINT_REQUIREMENTS do desc 'Get a single branch' do - success Entities::RepoBranch + success Entities::Branch end params do requires :branch, type: String, desc: 'The name of the branch' @@ -41,7 +41,7 @@ module API branch = user_project.repository.find_branch(params[:branch]) not_found!('Branch') unless branch - present branch, with: Entities::RepoBranch, project: user_project + present branch, with: Entities::Branch, project: user_project end end @@ -50,7 +50,7 @@ module API # in `gitlab-org/gitlab-ce!5081`. The API interface has not been changed (to maintain compatibility), # but it works with the changed data model to infer `developers_can_merge` and `developers_can_push`. desc 'Protect a single branch' do - success Entities::RepoBranch + success Entities::Branch end params do requires :branch, type: String, desc: 'The name of the branch' @@ -80,7 +80,7 @@ module API end if protected_branch.valid? - present branch, with: Entities::RepoBranch, project: user_project + present branch, with: Entities::Branch, project: user_project else render_api_error!(protected_branch.errors.full_messages, 422) end @@ -88,7 +88,7 @@ module API # Note: This API will be deprecated in favor of the protected branches API. desc 'Unprotect a single branch' do - success Entities::RepoBranch + success Entities::Branch end params do requires :branch, type: String, desc: 'The name of the branch' @@ -101,11 +101,11 @@ module API protected_branch = user_project.protected_branches.find_by(name: branch.name) protected_branch&.destroy - present branch, with: Entities::RepoBranch, project: user_project + present branch, with: Entities::Branch, project: user_project end desc 'Create branch' do - success Entities::RepoBranch + success Entities::Branch end params do requires :branch, type: String, desc: 'The name of the branch' @@ -119,7 +119,7 @@ module API if result[:status] == :success present result[:branch], - with: Entities::RepoBranch, + with: Entities::Branch, project: user_project else render_api_error!(result[:message], 400) diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 4b8d248f5f7..4af37a2ad1d 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -13,7 +13,7 @@ module API end resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do desc 'Get a project repository commits' do - success Entities::RepoCommit + success Entities::Commit end params do optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used' @@ -46,11 +46,11 @@ module API paginated_commits = Kaminari.paginate_array(commits, total_count: commit_count) - present paginate(paginated_commits), with: Entities::RepoCommit + present paginate(paginated_commits), with: Entities::Commit end desc 'Commit multiple file changes as one commit' do - success Entities::RepoCommitDetail + success Entities::CommitDetail detail 'This feature was introduced in GitLab 8.13' end params do @@ -72,14 +72,14 @@ module API if result[:status] == :success commit_detail = user_project.repository.commit(result[:result]) - present commit_detail, with: Entities::RepoCommitDetail + present commit_detail, with: Entities::CommitDetail else render_api_error!(result[:message], 400) end end desc 'Get a specific commit of a project' do - success Entities::RepoCommitDetail + success Entities::CommitDetail failure [[404, 'Commit Not Found']] end params do @@ -90,7 +90,7 @@ module API not_found! 'Commit' unless commit - present commit, with: Entities::RepoCommitDetail + present commit, with: Entities::CommitDetail end desc 'Get the diff for a specific commit of a project' do @@ -104,7 +104,7 @@ module API not_found! 'Commit' unless commit - present commit.raw_diffs.to_a, with: Entities::RepoDiff + present commit.raw_diffs.to_a, with: Entities::Diff end desc "Get a commit's comments" do @@ -126,7 +126,7 @@ module API desc 'Cherry pick commit into a branch' do detail 'This feature was introduced in GitLab 8.15' - success Entities::RepoCommit + success Entities::Commit end params do requires :sha, type: String, desc: 'A commit sha, or the name of a branch or tag to be cherry picked' @@ -151,7 +151,7 @@ module API if result[:status] == :success branch = user_project.repository.find_branch(params[:branch]) - present user_project.repository.commit(branch.dereferenced_target), with: Entities::RepoCommit + present user_project.repository.commit(branch.dereferenced_target), with: Entities::Commit else render_api_error!(result[:message], 400) end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 7082f31b5b8..86dec3ca9f1 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -220,7 +220,7 @@ module API expose :shared_projects, using: Entities::Project end - class RepoCommit < Grape::Entity + class Commit < Grape::Entity expose :id, :short_id, :title, :created_at expose :parent_ids expose :safe_message, as: :message @@ -228,20 +228,20 @@ module API expose :committer_name, :committer_email, :committed_date end - class RepoCommitStats < Grape::Entity + class CommitStats < Grape::Entity expose :additions, :deletions, :total end - class RepoCommitDetail < RepoCommit - expose :stats, using: Entities::RepoCommitStats + class CommitDetail < Commit + expose :stats, using: Entities::CommitStats expose :status expose :last_pipeline, using: 'API::Entities::PipelineBasic' end - class RepoBranch < Grape::Entity + class Branch < Grape::Entity expose :name - expose :commit, using: Entities::RepoCommit do |repo_branch, options| + expose :commit, using: Entities::Commit do |repo_branch, options| options[:project].repository.commit(repo_branch.dereferenced_target) end @@ -265,7 +265,7 @@ module API end end - class RepoTreeObject < Grape::Entity + class TreeObject < Grape::Entity expose :id, :name, :type, :path expose :mode do |obj, options| @@ -305,7 +305,7 @@ module API expose :state, :created_at, :updated_at end - class RepoDiff < Grape::Entity + class Diff < Grape::Entity expose :old_path, :new_path, :a_mode, :b_mode expose :new_file?, as: :new_file expose :renamed_file?, as: :renamed_file @@ -483,7 +483,7 @@ module API end class MergeRequestChanges < MergeRequest - expose :diffs, as: :changes, using: Entities::RepoDiff do |compare, _| + expose :diffs, as: :changes, using: Entities::Diff do |compare, _| compare.raw_diffs(limits: false).to_a end end @@ -494,9 +494,9 @@ module API end class MergeRequestDiffFull < MergeRequestDiff - expose :commits, using: Entities::RepoCommit + expose :commits, using: Entities::Commit - expose :diffs, using: Entities::RepoDiff do |compare, _| + expose :diffs, using: Entities::Diff do |compare, _| compare.raw_diffs(limits: false).to_a end end @@ -592,8 +592,7 @@ module API expose :target_type expose :target do |todo, options| - target = todo.target_type == 'Commit' ? 'RepoCommit' : todo.target_type - Entities.const_get(target).represent(todo.target, options) + Entities.const_get(todo.target_type).represent(todo.target, options) end expose :target_url do |todo, options| @@ -729,15 +728,15 @@ module API end class Compare < Grape::Entity - expose :commit, using: Entities::RepoCommit do |compare, options| - Commit.decorate(compare.commits, nil).last + expose :commit, using: Entities::Commit do |compare, options| + ::Commit.decorate(compare.commits, nil).last end - expose :commits, using: Entities::RepoCommit do |compare, options| - Commit.decorate(compare.commits, nil) + expose :commits, using: Entities::Commit do |compare, options| + ::Commit.decorate(compare.commits, nil) end - expose :diffs, using: Entities::RepoDiff do |compare, options| + expose :diffs, using: Entities::Diff do |compare, options| compare.diffs(limits: false).to_a end @@ -773,10 +772,10 @@ module API expose :description end - class RepoTag < Grape::Entity + class Tag < Grape::Entity expose :name, :message - expose :commit, using: Entities::RepoCommit do |repo_tag, options| + expose :commit, using: Entities::Commit do |repo_tag, options| options[:project].repository.commit(repo_tag.dereferenced_target) end @@ -827,7 +826,7 @@ module API expose :created_at, :started_at, :finished_at expose :user, with: User expose :artifacts_file, using: JobArtifactFile, if: -> (job, opts) { job.artifacts? } - expose :commit, with: RepoCommit + expose :commit, with: Commit expose :runner, with: Runner expose :pipeline, with: PipelineBasic end @@ -880,7 +879,7 @@ module API expose :deployable, using: Entities::Job end - class RepoLicense < Grape::Entity + class License < Grape::Entity expose :key, :name, :nickname expose :featured, as: :popular expose :url, as: :html_url diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 8aa1e0216ee..828bc48383e 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -183,13 +183,13 @@ module API end desc 'Get the commits of a merge request' do - success Entities::RepoCommit + success Entities::Commit end get ':id/merge_requests/:merge_request_iid/commits' do merge_request = find_merge_request_with_access(params[:merge_request_iid]) commits = ::Kaminari.paginate_array(merge_request.commits) - present paginate(commits), with: Entities::RepoCommit + present paginate(commits), with: Entities::Commit end desc 'Show the merge request changes' do diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 2255fb1b70d..ceee3226732 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -35,7 +35,7 @@ module API end desc 'Get a project repository tree' do - success Entities::RepoTreeObject + success Entities::TreeObject end params do optional :ref, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used' @@ -52,7 +52,7 @@ module API tree = user_project.repository.tree(commit.id, path, recursive: params[:recursive]) entries = ::Kaminari.paginate_array(tree.sorted_entries) - present paginate(entries), with: Entities::RepoTreeObject + present paginate(entries), with: Entities::TreeObject end desc 'Get raw blob contents from the repository' diff --git a/lib/api/tags.rb b/lib/api/tags.rb index 912415e3a7f..0d394a7b441 100644 --- a/lib/api/tags.rb +++ b/lib/api/tags.rb @@ -11,18 +11,18 @@ module API end resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do desc 'Get a project repository tags' do - success Entities::RepoTag + success Entities::Tag end params do use :pagination end get ':id/repository/tags' do tags = ::Kaminari.paginate_array(user_project.repository.tags.sort_by(&:name).reverse) - present paginate(tags), with: Entities::RepoTag, project: user_project + present paginate(tags), with: Entities::Tag, project: user_project end desc 'Get a single repository tag' do - success Entities::RepoTag + success Entities::Tag end params do requires :tag_name, type: String, desc: 'The name of the tag' @@ -31,11 +31,11 @@ module API tag = user_project.repository.find_tag(params[:tag_name]) not_found!('Tag') unless tag - present tag, with: Entities::RepoTag, project: user_project + present tag, with: Entities::Tag, project: user_project end desc 'Create a new repository tag' do - success Entities::RepoTag + success Entities::Tag end params do requires :tag_name, type: String, desc: 'The name of the tag' @@ -51,7 +51,7 @@ module API if result[:status] == :success present result[:tag], - with: Entities::RepoTag, + with: Entities::Tag, project: user_project else render_api_error!(result[:message], 400) diff --git a/lib/api/templates.rb b/lib/api/templates.rb index f70bc0622b7..6550b331fb8 100644 --- a/lib/api/templates.rb +++ b/lib/api/templates.rb @@ -49,7 +49,7 @@ module API desc 'Get the list of the available license template' do detail 'This feature was introduced in GitLab 8.7.' - success ::API::Entities::RepoLicense + success ::API::Entities::License end params do optional :popular, type: Boolean, desc: 'If passed, returns only popular licenses' @@ -60,12 +60,12 @@ module API featured: declared(params)[:popular].present? ? true : nil } licences = ::Kaminari.paginate_array(Licensee::License.all(options)) - present paginate(licences), with: Entities::RepoLicense + present paginate(licences), with: Entities::License end desc 'Get the text for a specific license' do detail 'This feature was introduced in GitLab 8.7.' - success ::API::Entities::RepoLicense + success ::API::Entities::License end params do requires :name, type: String, desc: 'The name of the template' @@ -75,7 +75,7 @@ module API template = parsed_license_template - present template, with: ::API::Entities::RepoLicense + present template, with: ::API::Entities::License end GLOBAL_TEMPLATE_TYPES.each do |template_type, properties| diff --git a/lib/api/users.rb b/lib/api/users.rb index d07dc302717..b6f97a1eac2 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -331,7 +331,6 @@ module API email = Emails::CreateService.new(current_user, declared_params(include_missing: false).merge(user: user)).execute if email.errors.blank? - NotificationService.new.new_email(email) present email, with: Entities::Email else render_validation_error!(email) @@ -369,10 +368,8 @@ module API not_found!('Email') unless email destroy_conditionally!(email) do |email| - Emails::DestroyService.new(current_user, user: user, email: email.email).execute + Emails::DestroyService.new(current_user, user: user).execute(email) end - - user.update_secondary_emails! end desc 'Delete a user. Available only for admins.' do @@ -677,7 +674,6 @@ module API email = Emails::CreateService.new(current_user, declared_params.merge(user: current_user)).execute if email.errors.blank? - NotificationService.new.new_email(email) present email, with: Entities::Email else render_validation_error!(email) @@ -693,10 +689,8 @@ module API not_found!('Email') unless email destroy_conditionally!(email) do |email| - Emails::DestroyService.new(current_user, user: current_user, email: email.email).execute + Emails::DestroyService.new(current_user, user: current_user).execute(email) end - - current_user.update_secondary_emails! end desc 'Get a list of user activities' diff --git a/lib/api/v3/branches.rb b/lib/api/v3/branches.rb index 81b13249892..69cd12de72c 100644 --- a/lib/api/v3/branches.rb +++ b/lib/api/v3/branches.rb @@ -11,12 +11,12 @@ module API end resource :projects, requirements: { id: %r{[^/]+} } do desc 'Get a project repository branches' do - success ::API::Entities::RepoBranch + success ::API::Entities::Branch end get ":id/repository/branches" do branches = user_project.repository.branches.sort_by(&:name) - present branches, with: ::API::Entities::RepoBranch, project: user_project + present branches, with: ::API::Entities::Branch, project: user_project end desc 'Delete a branch' @@ -47,7 +47,7 @@ module API end desc 'Create branch' do - success ::API::Entities::RepoBranch + success ::API::Entities::Branch end params do requires :branch_name, type: String, desc: 'The name of the branch' @@ -60,7 +60,7 @@ module API if result[:status] == :success present result[:branch], - with: ::API::Entities::RepoBranch, + with: ::API::Entities::Branch, project: user_project else render_api_error!(result[:message], 400) diff --git a/lib/api/v3/commits.rb b/lib/api/v3/commits.rb index 5936f4700aa..345cb7e7c11 100644 --- a/lib/api/v3/commits.rb +++ b/lib/api/v3/commits.rb @@ -13,7 +13,7 @@ module API end resource :projects, requirements: { id: %r{[^/]+} } do desc 'Get a project repository commits' do - success ::API::Entities::RepoCommit + success ::API::Entities::Commit end params do optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used' @@ -34,11 +34,11 @@ module API after: params[:since], before: params[:until]) - present commits, with: ::API::Entities::RepoCommit + present commits, with: ::API::Entities::Commit end desc 'Commit multiple file changes as one commit' do - success ::API::Entities::RepoCommitDetail + success ::API::Entities::CommitDetail detail 'This feature was introduced in GitLab 8.13' end params do @@ -59,14 +59,14 @@ module API if result[:status] == :success commit_detail = user_project.repository.commits(result[:result], limit: 1).first - present commit_detail, with: ::API::Entities::RepoCommitDetail + present commit_detail, with: ::API::Entities::CommitDetail else render_api_error!(result[:message], 400) end end desc 'Get a specific commit of a project' do - success ::API::Entities::RepoCommitDetail + success ::API::Entities::CommitDetail failure [[404, 'Not Found']] end params do @@ -77,7 +77,7 @@ module API not_found! "Commit" unless commit - present commit, with: ::API::Entities::RepoCommitDetail + present commit, with: ::API::Entities::CommitDetail end desc 'Get the diff for a specific commit of a project' do @@ -113,7 +113,7 @@ module API desc 'Cherry pick commit into a branch' do detail 'This feature was introduced in GitLab 8.15' - success ::API::Entities::RepoCommit + success ::API::Entities::Commit end params do requires :sha, type: String, desc: 'A commit sha to be cherry picked' @@ -138,7 +138,7 @@ module API if result[:status] == :success branch = user_project.repository.find_branch(params[:branch]) - present user_project.repository.commit(branch.dereferenced_target), with: ::API::Entities::RepoCommit + present user_project.repository.commit(branch.dereferenced_target), with: ::API::Entities::Commit else render_api_error!(result[:message], 400) end diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index c928ce5265b..afdd7b83998 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -220,7 +220,7 @@ module API expose :created_at, :started_at, :finished_at expose :user, with: ::API::Entities::User expose :artifacts_file, using: ::API::Entities::JobArtifactFile, if: -> (build, opts) { build.artifacts? } - expose :commit, with: ::API::Entities::RepoCommit + expose :commit, with: ::API::Entities::Commit expose :runner, with: ::API::Entities::Runner expose :pipeline, with: ::API::Entities::PipelineBasic end @@ -237,7 +237,7 @@ module API end class MergeRequestChanges < MergeRequest - expose :diffs, as: :changes, using: ::API::Entities::RepoDiff do |compare, _| + expose :diffs, as: :changes, using: ::API::Entities::Diff do |compare, _| compare.raw_diffs(limits: false).to_a end end diff --git a/lib/api/v3/merge_requests.rb b/lib/api/v3/merge_requests.rb index b6b7254ae29..1d6d823f32b 100644 --- a/lib/api/v3/merge_requests.rb +++ b/lib/api/v3/merge_requests.rb @@ -135,12 +135,12 @@ module API end desc 'Get the commits of a merge request' do - success ::API::Entities::RepoCommit + success ::API::Entities::Commit end get "#{path}/commits" do merge_request = find_merge_request_with_access(params[:merge_request_id]) - present merge_request.commits, with: ::API::Entities::RepoCommit + present merge_request.commits, with: ::API::Entities::Commit end desc 'Show the merge request changes' do diff --git a/lib/api/v3/repositories.rb b/lib/api/v3/repositories.rb index 0eaa0de2eef..41a7c6b83ae 100644 --- a/lib/api/v3/repositories.rb +++ b/lib/api/v3/repositories.rb @@ -19,7 +19,7 @@ module API end desc 'Get a project repository tree' do - success ::API::Entities::RepoTreeObject + success ::API::Entities::TreeObject end params do optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used' @@ -35,7 +35,7 @@ module API tree = user_project.repository.tree(commit.id, path, recursive: params[:recursive]) - present tree.sorted_entries, with: ::API::Entities::RepoTreeObject + present tree.sorted_entries, with: ::API::Entities::TreeObject end desc 'Get a raw file contents' diff --git a/lib/api/v3/tags.rb b/lib/api/v3/tags.rb index 7e5875cd030..6e37d31d153 100644 --- a/lib/api/v3/tags.rb +++ b/lib/api/v3/tags.rb @@ -8,11 +8,11 @@ module API end resource :projects, requirements: { id: %r{[^/]+} } do desc 'Get a project repository tags' do - success ::API::Entities::RepoTag + success ::API::Entities::Tag end get ":id/repository/tags" do tags = user_project.repository.tags.sort_by(&:name).reverse - present tags, with: ::API::Entities::RepoTag, project: user_project + present tags, with: ::API::Entities::Tag, project: user_project end desc 'Delete a repository tag' diff --git a/lib/api/v3/templates.rb b/lib/api/v3/templates.rb index 2a2fb59045c..7298203df10 100644 --- a/lib/api/v3/templates.rb +++ b/lib/api/v3/templates.rb @@ -52,7 +52,7 @@ module API detailed_desc = 'This feature was introduced in GitLab 8.7.' detailed_desc << DEPRECATION_MESSAGE unless status == :ok detail detailed_desc - success ::API::Entities::RepoLicense + success ::API::Entities::License end params do optional :popular, type: Boolean, desc: 'If passed, returns only popular licenses' @@ -61,7 +61,7 @@ module API options = { featured: declared(params)[:popular].present? ? true : nil } - present Licensee::License.all(options), with: ::API::Entities::RepoLicense + present Licensee::License.all(options), with: ::API::Entities::License end end @@ -70,7 +70,7 @@ module API detailed_desc = 'This feature was introduced in GitLab 8.7.' detailed_desc << DEPRECATION_MESSAGE unless status == :ok detail detailed_desc - success ::API::Entities::RepoLicense + success ::API::Entities::License end params do requires :name, type: String, desc: 'The name of the template' @@ -80,7 +80,7 @@ module API template = parsed_license_template - present template, with: ::API::Entities::RepoLicense + present template, with: ::API::Entities::License end end diff --git a/lib/declarative_policy/rule.rb b/lib/declarative_policy/rule.rb index bfcec241489..7cfa82a9a9f 100644 --- a/lib/declarative_policy/rule.rb +++ b/lib/declarative_policy/rule.rb @@ -206,11 +206,13 @@ module DeclarativePolicy end def cached_pass?(context) - passes = @rules.map { |r| r.cached_pass?(context) } - return false if passes.any? { |p| p == false } - return true if passes.all? { |p| p == true } + @rules.each do |rule| + pass = rule.cached_pass?(context) - nil + return pass if pass.nil? || pass == false + end + + true end def repr @@ -245,11 +247,13 @@ module DeclarativePolicy end def cached_pass?(context) - passes = @rules.map { |r| r.cached_pass?(context) } - return true if passes.any? { |p| p == true } - return false if passes.all? { |p| p == false } + @rules.each do |rule| + pass = rule.cached_pass?(context) - nil + return pass if pass.nil? || pass == true + end + + false end def score(context) diff --git a/lib/declarative_policy/runner.rb b/lib/declarative_policy/runner.rb index 56afd1f1392..45ff2ef9ced 100644 --- a/lib/declarative_policy/runner.rb +++ b/lib/declarative_policy/runner.rb @@ -107,7 +107,7 @@ module DeclarativePolicy end # This is the core spot where all those `#score` methods matter. - # It is critcal for performance to run steps in the correct order, + # It is critical for performance to run steps in the correct order, # so that we don't compute expensive conditions (potentially n times # if we're called on, say, a large list of users). # @@ -139,30 +139,39 @@ module DeclarativePolicy return end - steps = Set.new(@steps) - remaining_enablers = steps.count { |s| s.enable? } + remaining_steps = Set.new(@steps) + remaining_enablers, remaining_preventers = remaining_steps.partition(&:enable?).map { |s| Set.new(s) } loop do - return if steps.empty? + if @state.enabled? + # Once we set this, we never need to unset it, because a single + # prevent will stop this from being enabled + remaining_steps = remaining_preventers + else + # if the permission hasn't yet been enabled and we only have + # prevent steps left, we short-circuit the state here + @state.prevent! if remaining_enablers.empty? + end - # if the permission hasn't yet been enabled and we only have - # prevent steps left, we short-circuit the state here - @state.prevent! if !@state.enabled? && remaining_enablers == 0 + return if remaining_steps.empty? lowest_score = Float::INFINITY next_step = nil - steps.each do |step| + remaining_steps.each do |step| score = step.score + if score < lowest_score next_step = step lowest_score = score end - end - steps.delete(next_step) + break if lowest_score.zero? + end - remaining_enablers -= 1 if next_step.enable? + [remaining_steps, remaining_enablers, remaining_preventers].each do |set| + set.delete(next_step) + end yield next_step, lowest_score end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 89b654253cb..e42bbb659b4 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -990,7 +990,7 @@ module Gitlab tmp_ref = fetch_ref( start_repository, source_ref: "#{Gitlab::Git::BRANCH_REF_PREFIX}#{start_branch_name}", - target_ref: "refs/tmp/#{SecureRandom.hex}/head" + target_ref: "refs/tmp/#{SecureRandom.hex}" ) yield commit(sha) diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 68815be4d13..47c2a422387 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -64,6 +64,8 @@ module Gitlab protected def add_or_update_user_identities + return unless gl_user + # find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved. identity = gl_user.identities.find { |identity| identity.provider == auth_hash.provider } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 74e59294f08..5c26f942b02 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -852,9 +852,6 @@ msgstr[1] "" msgid "Last Pipeline" msgstr "" -msgid "Last Update" -msgstr "" - msgid "Last commit" msgstr "" @@ -864,6 +861,9 @@ msgstr "" msgid "Last edited by %{name}" msgstr "" +msgid "Last update" +msgstr "" + msgid "Last updated" msgstr "" diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index 95aeef10752..22d12b479cb 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -25,9 +25,9 @@ GEM mime-types (3.1) mime-types-data (~> 3.2015) mime-types-data (3.2016.0521) - mini_portile2 (2.1.0) - nokogiri (1.7.0.1) - mini_portile2 (~> 2.1.0) + mini_portile2 (2.3.0) + nokogiri (1.8.1) + mini_portile2 (~> 2.3.0) pry (0.10.4) coderay (~> 1.1.0) method_source (~> 0.8.1) @@ -75,4 +75,4 @@ DEPENDENCIES selenium-webdriver (~> 2.53) BUNDLED WITH - 1.14.6 + 1.15.4 diff --git a/qa/README.md b/qa/README.md index e0ebb53a2e9..1cfbbdd9d42 100644 --- a/qa/README.md +++ b/qa/README.md @@ -1,10 +1,10 @@ -## Integration tests for GitLab +# GitLab QA - Integration tests for GitLab This directory contains integration tests for GitLab. -It is part of [GitLab QA project](https://gitlab.com/gitlab-org/gitlab-qa). +It is part of the [GitLab QA project](https://gitlab.com/gitlab-org/gitlab-qa). -## What GitLab QA is? +## What is it? GitLab QA is an integration tests suite for GitLab. @@ -20,18 +20,34 @@ against any existing instance. ## How can I use it? You can use GitLab QA to exercise tests on any live instance! For example, the -follow call would login to the local GitLab instance and run all specs in +following call would login to a local [GDK] instance and run all specs in `qa/specs/features`: ``` -GITLAB_USERNAME='root' GITLAB_PASSWORD='5iveL!fe' bin/qa Test::Instance http://localhost +bin/qa Test::Instance http://localhost:3000 ``` -You can also supply a specific tests to run as another parameter. For example, to +### Running specific tests + +You can also supply specific tests to run as another parameter. For example, to test the EE license specs, you can run: ``` -EE_LICENSE="<YOUR LICENSE KEY>" GITLAB_USERNAME='root' GITLAB_PASSWORD='5iveL!fe' bin/qa Test::Instance http://localhost qa/ee +EE_LICENSE="<YOUR LICENSE KEY>" bin/qa Test::Instance http://localhost qa/ee +``` + +### Overriding the authenticated user + +Unless told otherwise, the QA tests will run as the default `root` user seeded +by the GDK. + +If you need to authenticate as a different user, you can provide the +`GITLAB_USERNAME` and `GITLAB_PASSWORD` environment variables: + +``` +GITLAB_USERNAME=jsmith GITLAB_PASSWORD=password bin/qa Test::Instance https://gitlab.example.com ``` All [supported environment variables are here](https://gitlab.com/gitlab-org/gitlab-qa#supported-environment-variables). + +[GDK]: https://gitlab.com/gitlab-org/gitlab-development-kit/ diff --git a/qa/qa/runtime/user.rb b/qa/qa/runtime/user.rb index 12ceda015f0..60027c89ab1 100644 --- a/qa/qa/runtime/user.rb +++ b/qa/qa/runtime/user.rb @@ -8,7 +8,7 @@ module QA end def password - ENV['GITLAB_PASSWORD'] || 'test1234' + ENV['GITLAB_PASSWORD'] || '5iveL!fe' end end end diff --git a/spec/controllers/profiles/emails_controller_spec.rb b/spec/controllers/profiles/emails_controller_spec.rb new file mode 100644 index 00000000000..ecf14aad54f --- /dev/null +++ b/spec/controllers/profiles/emails_controller_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Profiles::EmailsController do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe '#create' do + let(:email_params) { { email: "add_email@example.com" } } + + it 'sends an email confirmation' do + expect { post(:create, { email: email_params }) }.to change { ActionMailer::Base.deliveries.size } + expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]] + expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions" + end + end + + describe '#resend_confirmation_instructions' do + let(:email_params) { { email: "add_email@example.com" } } + + it 'resends an email confirmation' do + email = user.emails.create(email: 'add_email@example.com') + + expect { put(:resend_confirmation_instructions, { id: email }) }.to change { ActionMailer::Base.deliveries.size } + expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]] + expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions" + end + + it 'unable to resend an email confirmation' do + expect { put(:resend_confirmation_instructions, { id: 1 }) }.not_to change { ActionMailer::Base.deliveries.size } + end + end +end diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index b52b63e05a4..ce5040ff02b 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -15,6 +15,20 @@ describe ProfilesController do expect(user.unconfirmed_email).to eq('john@gmail.com') end + it "allows an email update without confirmation if existing verified email" do + user = create(:user) + create(:email, :confirmed, user: user, email: 'john@gmail.com') + sign_in(user) + + put :update, + user: { email: "john@gmail.com", name: "John" } + + user.reload + + expect(response.status).to eq(302) + expect(user.unconfirmed_email).to eq nil + end + it "ignores an email update from a user with an external email address" do stub_omniauth_setting(sync_profile_from_provider: ['ldap']) stub_omniauth_setting(sync_profile_attributes: true) diff --git a/spec/factories/emails.rb b/spec/factories/emails.rb index 8303861bcfe..c9ab87a15aa 100644 --- a/spec/factories/emails.rb +++ b/spec/factories/emails.rb @@ -2,5 +2,7 @@ FactoryGirl.define do factory :email do user email { generate(:email_alias) } + + trait(:confirmed) { confirmed_at Time.now } end end diff --git a/spec/features/profiles/emails_spec.rb b/spec/features/profiles/emails_spec.rb new file mode 100644 index 00000000000..11cc8aae6f3 --- /dev/null +++ b/spec/features/profiles/emails_spec.rb @@ -0,0 +1,71 @@ +require 'rails_helper' + +feature 'Profile > Emails' do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe 'User adds an email' do + before do + visit profile_emails_path + end + + scenario 'saves the new email' do + fill_in('Email', with: 'my@email.com') + click_button('Add email address') + + expect(page).to have_content('my@email.com Unverified') + expect(page).to have_content("#{user.email} Verified") + expect(page).to have_content('Resend confirmation email') + end + + scenario 'does not add a duplicate email' do + fill_in('Email', with: user.email) + click_button('Add email address') + + email = user.emails.find_by(email: user.email) + expect(email).to be_nil + expect(page).to have_content('Email has already been taken') + end + end + + scenario 'User removes email' do + user.emails.create(email: 'my@email.com') + visit profile_emails_path + expect(page).to have_content("my@email.com") + + click_link('Remove') + expect(page).not_to have_content("my@email.com") + end + + scenario 'User confirms email' do + email = user.emails.create(email: 'my@email.com') + visit profile_emails_path + expect(page).to have_content("#{email.email} Unverified") + + email.confirm + expect(email.confirmed?).to be_truthy + + visit profile_emails_path + expect(page).to have_content("#{email.email} Verified") + end + + scenario 'User re-sends confirmation email' do + email = user.emails.create(email: 'my@email.com') + visit profile_emails_path + + expect { click_link("Resend confirmation email") }.to change { ActionMailer::Base.deliveries.size } + expect(page).to have_content("Confirmation email sent to #{email.email}") + end + + scenario 'old unconfirmed emails show Send Confirmation button' do + email = user.emails.create(email: 'my@email.com') + email.update_attribute(:confirmation_sent_at, nil) + visit profile_emails_path + + expect(page).not_to have_content('Resend confirmation email') + expect(page).to have_content('Send confirmation email') + end +end diff --git a/spec/features/profiles/gpg_keys_spec.rb b/spec/features/profiles/gpg_keys_spec.rb index 623e4f341c5..b0f6848bc4b 100644 --- a/spec/features/profiles/gpg_keys_spec.rb +++ b/spec/features/profiles/gpg_keys_spec.rb @@ -4,7 +4,7 @@ feature 'Profile > GPG Keys' do let(:user) { create(:user, email: GpgHelpers::User2.emails.first) } before do - login_as(user) + sign_in(user) end describe 'User adds a key' do diff --git a/spec/javascripts/monitoring/graph/deployment_spec.js b/spec/javascripts/monitoring/graph/deployment_spec.js index c2ff38ffab9..dea42d755d4 100644 --- a/spec/javascripts/monitoring/graph/deployment_spec.js +++ b/spec/javascripts/monitoring/graph/deployment_spec.js @@ -21,6 +21,7 @@ describe('MonitoringDeployment', () => { const component = createComponent({ showDeployInfo: false, deploymentData: reducedDeploymentData, + graphWidth: 440, graphHeight: 300, graphHeightOffset: 120, }); @@ -36,6 +37,7 @@ describe('MonitoringDeployment', () => { showDeployInfo: false, deploymentData: reducedDeploymentData, graphHeight: 300, + graphWidth: 440, graphHeightOffset: 120, }); @@ -49,6 +51,7 @@ describe('MonitoringDeployment', () => { showDeployInfo: false, deploymentData: reducedDeploymentData, graphHeight: 300, + graphWidth: 440, graphHeightOffset: 120, }); @@ -62,6 +65,7 @@ describe('MonitoringDeployment', () => { showDeployInfo: false, deploymentData: reducedDeploymentData, graphHeight: 300, + graphWidth: 440, graphHeightOffset: 120, }); @@ -75,6 +79,7 @@ describe('MonitoringDeployment', () => { const component = createComponent({ showDeployInfo: true, deploymentData: reducedDeploymentData, + graphWidth: 440, graphHeight: 300, graphHeightOffset: 120, }); @@ -82,12 +87,29 @@ describe('MonitoringDeployment', () => { expect(component.$el.querySelector('.js-deploy-info-box')).toBeNull(); }); + it('positions the flag to the left when the xPos is too far right', () => { + reducedDeploymentData[0].showDeploymentFlag = false; + reducedDeploymentData[0].xPos = 250; + const component = createComponent({ + showDeployInfo: true, + deploymentData: reducedDeploymentData, + graphWidth: 440, + graphHeight: 300, + graphHeightOffset: 120, + }); + + expect( + component.positionFlag(reducedDeploymentData[0]), + ).toBeLessThan(0); + }); + it('shows the deployment flag', () => { reducedDeploymentData[0].showDeploymentFlag = true; const component = createComponent({ showDeployInfo: true, deploymentData: reducedDeploymentData, graphHeight: 300, + graphWidth: 440, graphHeightOffset: 120, }); @@ -102,6 +124,7 @@ describe('MonitoringDeployment', () => { showDeployInfo: true, deploymentData: reducedDeploymentData, graphHeight: 300, + graphWidth: 440, graphHeightOffset: 120, }); @@ -115,6 +138,7 @@ describe('MonitoringDeployment', () => { showDeployInfo: true, deploymentData: reducedDeploymentData, graphHeight: 300, + graphWidth: 440, graphHeightOffset: 120, }); @@ -127,6 +151,7 @@ describe('MonitoringDeployment', () => { showDeployInfo: true, deploymentData: reducedDeploymentData, graphHeight: 300, + graphWidth: 440, graphHeightOffset: 120, }); diff --git a/spec/javascripts/monitoring/graph/flag_spec.js b/spec/javascripts/monitoring/graph/flag_spec.js index 14794cbfd50..8ee1171419d 100644 --- a/spec/javascripts/monitoring/graph/flag_spec.js +++ b/spec/javascripts/monitoring/graph/flag_spec.js @@ -14,19 +14,22 @@ function getCoordinate(component, selector, coordinate) { return parseInt(coordinateVal, 10); } +const defaultValuesComponent = { + currentXCoordinate: 200, + currentYCoordinate: 100, + currentFlagPosition: 100, + currentData: { + time: new Date('2017-06-04T18:17:33.501Z'), + value: '1.49609375', + }, + graphHeight: 300, + graphHeightOffset: 120, + showFlagContent: true, +}; + describe('GraphFlag', () => { it('has a line and a circle located at the currentXCoordinate and currentYCoordinate', () => { - const component = createComponent({ - currentXCoordinate: 200, - currentYCoordinate: 100, - currentFlagPosition: 100, - currentData: { - time: new Date('2017-06-04T18:17:33.501Z'), - value: '1.49609375', - }, - graphHeight: 300, - graphHeightOffset: 120, - }); + const component = createComponent(defaultValuesComponent); expect(getCoordinate(component, '.selected-metric-line', 'x1')) .toEqual(component.currentXCoordinate); @@ -35,17 +38,7 @@ describe('GraphFlag', () => { }); it('has a SVG with the class rect-text-metric at the currentFlagPosition', () => { - const component = createComponent({ - currentXCoordinate: 200, - currentYCoordinate: 100, - currentFlagPosition: 100, - currentData: { - time: new Date('2017-06-04T18:17:33.501Z'), - value: '1.49609375', - }, - graphHeight: 300, - graphHeightOffset: 120, - }); + const component = createComponent(defaultValuesComponent); const svg = component.$el.querySelector('.rect-text-metric'); expect(svg.tagName).toEqual('svg'); @@ -54,17 +47,7 @@ describe('GraphFlag', () => { describe('Computed props', () => { it('calculatedHeight', () => { - const component = createComponent({ - currentXCoordinate: 200, - currentYCoordinate: 100, - currentFlagPosition: 100, - currentData: { - time: new Date('2017-06-04T18:17:33.501Z'), - value: '1.49609375', - }, - graphHeight: 300, - graphHeightOffset: 120, - }); + const component = createComponent(defaultValuesComponent); expect(component.calculatedHeight).toEqual(180); }); diff --git a/spec/javascripts/monitoring/graph_spec.js b/spec/javascripts/monitoring/graph_spec.js index 7d8b0744af1..7213b3bfa30 100644 --- a/spec/javascripts/monitoring/graph_spec.js +++ b/spec/javascripts/monitoring/graph_spec.js @@ -86,4 +86,22 @@ describe('Graph', () => { expect(component.yAxisLabel).toEqual(component.graphData.y_label); expect(component.legendTitle).toEqual(component.graphData.queries[0].label); }); + + it('sets the currentData object based on the hovered data index', () => { + const component = createComponent({ + graphData: convertedMetrics[1], + classType: 'col-md-6', + updateAspectRatio: false, + deploymentData, + graphIdentifier: 0, + hoverData: { + hoveredDate: new Date('Sun Aug 27 2017 06:11:51 GMT-0500 (CDT)'), + currentDeployXPos: null, + }, + }); + + component.positionFlag(); + expect(component.currentData).toBe(component.timeSeries[0].values[10]); + expect(component.currentDataIndex).toEqual(10); + }); }); diff --git a/spec/javascripts/repo/components/repo_sidebar_spec.js b/spec/javascripts/repo/components/repo_sidebar_spec.js index db9911c7a2c..23c10ea022e 100644 --- a/spec/javascripts/repo/components/repo_sidebar_spec.js +++ b/spec/javascripts/repo/components/repo_sidebar_spec.js @@ -23,8 +23,8 @@ describe('RepoSidebar', () => { expect(vm.$el.id).toEqual('sidebar'); expect(vm.$el.classList.contains('sidebar-mini')).toBeFalsy(); expect(thead.querySelector('.name').textContent).toEqual('Name'); - expect(thead.querySelector('.last-commit').textContent).toEqual('Last Commit'); - expect(thead.querySelector('.last-update').textContent).toEqual('Last Update'); + expect(thead.querySelector('.last-commit').textContent).toEqual('Last commit'); + expect(thead.querySelector('.last-update').textContent).toEqual('Last update'); expect(tbody.querySelector('.repo-file-options')).toBeFalsy(); expect(tbody.querySelector('.prev-directory')).toBeFalsy(); expect(tbody.querySelector('.loading-file')).toBeFalsy(); diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index 09e5094cf84..1f7be415e35 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -120,29 +120,4 @@ describe Emails::Profile do it { expect { Notify.new_gpg_key_email('foo') }.not_to raise_error } end end - - describe 'user added email' do - let(:email) { create(:email) } - - subject { Notify.new_email_email(email.id) } - - it_behaves_like 'it should not have Gmail Actions links' - it_behaves_like 'a user cannot unsubscribe through footer link' - - it 'is sent to the new user' do - is_expected.to deliver_to email.user.email - end - - it 'has the correct subject' do - is_expected.to have_subject /^Email was added to your account$/i - end - - it 'contains the new email address' do - is_expected.to have_body_text /#{email.email}/ - end - - it 'includes a link to emails page' do - is_expected.to have_body_text /#{profile_emails_path}/ - end - end end diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index 1d6fabe48b1..b32dd31ae6d 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -11,4 +11,33 @@ describe Email do expect(described_class.new(email: ' inFO@exAMPLe.com ').email) .to eq 'info@example.com' end + + describe '#update_invalid_gpg_signatures' do + let(:user) do + create(:user, email: 'tula.torphy@abshire.ca').tap do |user| + user.skip_reconfirmation! + end + end + let(:user) { create(:user) } + + it 'synchronizes the gpg keys when the email is updated' do + email = user.emails.create(email: 'new@email.com') + + expect(user).to receive(:update_invalid_gpg_signatures) + + email.confirm + end + end + + describe 'scopes' do + let(:user) { create(:user) } + + it 'scopes confirmed emails' do + create(:email, :confirmed, user: user) + create(:email, user: user) + + expect(user.emails.count).to eq 2 + expect(user.emails.confirmed.count).to eq 1 + end + end end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 4a4d079b721..743f2cfcab5 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -90,11 +90,20 @@ describe GpgKey do it 'email is verified if the user has the matching email' do user = create :user, email: 'bette.cartwright@example.com' gpg_key = create :gpg_key, key: GpgHelpers::User2.public_key, user: user + create :email, user: user + user.reload expect(gpg_key.emails_with_verified_status).to eq( 'bette.cartwright@example.com' => true, 'bette.cartwright@example.net' => false ) + + create :email, :confirmed, user: user, email: 'bette.cartwright@example.net' + user.reload + expect(gpg_key.emails_with_verified_status).to eq( + 'bette.cartwright@example.com' => true, + 'bette.cartwright@example.net' => true + ) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 62890dd5002..9f517e4af72 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -360,9 +360,22 @@ describe User do expect(external_user.projects_limit).to be 0 end end + + describe '#check_for_verified_email' do + let(:user) { create(:user) } + let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } + + it 'allows a verfied secondary email to be used as the primary without needing reconfirmation' do + user.update_attributes!(email: secondary.email) + user.reload + expect(user.email).to eq secondary.email + expect(user.unconfirmed_email).to eq nil + expect(user.confirmed?).to be_truthy + end + end end - describe 'after update hook' do + describe 'after commit hook' do describe '.update_invalid_gpg_signatures' do let(:user) do create(:user, email: 'tula.torphy@abshire.ca').tap do |user| @@ -376,10 +389,50 @@ describe User do end it 'synchronizes the gpg keys when the email is updated' do - expect(user).to receive(:update_invalid_gpg_signatures) + expect(user).to receive(:update_invalid_gpg_signatures).at_most(:twice) user.update_attributes!(email: 'shawnee.ritchie@denesik.com') end end + + describe '#update_emails_with_primary_email' do + before do + @user = create(:user, email: 'primary@example.com').tap do |user| + user.skip_reconfirmation! + end + @secondary = create :email, email: 'secondary@example.com', user: @user + @user.reload + end + + it 'gets called when email updated' do + expect(@user).to receive(:update_emails_with_primary_email) + + @user.update_attributes!(email: 'new_primary@example.com') + end + + it 'adds old primary to secondary emails when secondary is a new email ' do + @user.update_attributes!(email: 'new_primary@example.com') + @user.reload + + expect(@user.emails.count).to eq 2 + expect(@user.emails.pluck(:email)).to match_array([@secondary.email, 'primary@example.com']) + end + + it 'adds old primary to secondary emails if secondary is becoming a primary' do + @user.update_attributes!(email: @secondary.email) + @user.reload + + expect(@user.emails.count).to eq 1 + expect(@user.emails.first.email).to eq 'primary@example.com' + end + + it 'transfers old confirmation values into new secondary' do + @user.update_attributes!(email: @secondary.email) + @user.reload + + expect(@user.emails.count).to eq 1 + expect(@user.emails.first.confirmed_at).not_to eq nil + end + end end describe '#update_tracked_fields!', :clean_gitlab_redis_shared_state do @@ -467,6 +520,7 @@ describe User do describe '#generate_password' do it "does not generate password by default" do user = create(:user, password: 'abcdefghe') + expect(user.password).to eq('abcdefghe') end end @@ -474,6 +528,7 @@ describe User do describe 'authentication token' do it "has authentication token" do user = create(:user) + expect(user.authentication_token).not_to be_blank end end @@ -481,6 +536,7 @@ describe User do describe 'ensure incoming email token' do it 'has incoming email token' do user = create(:user) + expect(user.incoming_email_token).not_to be_blank end end @@ -523,6 +579,7 @@ describe User do it 'ensures an rss token on read' do user = create(:user, rss_token: nil) rss_token = user.rss_token + expect(rss_token).not_to be_blank expect(user.reload.rss_token).to eq rss_token end @@ -633,6 +690,7 @@ describe User do it "blocks user" do user.block + expect(user.blocked?).to be_truthy end end @@ -966,6 +1024,7 @@ describe User do it 'is case-insensitive' do user = create(:user, username: 'JohnDoe') + expect(described_class.find_by_username('JOHNDOE')).to eq user end end @@ -978,6 +1037,7 @@ describe User do it 'is case-insensitive' do user = create(:user, username: 'JohnDoe') + expect(described_class.find_by_username!('JOHNDOE')).to eq user end end @@ -1067,11 +1127,13 @@ describe User do it 'is true if avatar is image' do user.update_attribute(:avatar, 'uploads/avatar.png') + expect(user.avatar_type).to be_truthy end it 'is false if avatar is html page' do user.update_attribute(:avatar, 'uploads/avatar.html') + expect(user.avatar_type).to eq(['only images allowed']) end end @@ -1094,6 +1156,50 @@ describe User do end end + describe '#all_emails' do + let(:user) { create(:user) } + + it 'returns all emails' do + email_confirmed = create :email, user: user, confirmed_at: Time.now + email_unconfirmed = create :email, user: user + user.reload + + expect(user.all_emails).to match_array([user.email, email_unconfirmed.email, email_confirmed.email]) + end + end + + describe '#verified_emails' do + let(:user) { create(:user) } + + it 'returns only confirmed emails' do + email_confirmed = create :email, user: user, confirmed_at: Time.now + create :email, user: user + user.reload + + expect(user.verified_emails).to match_array([user.email, email_confirmed.email]) + end + end + + describe '#verified_email?' do + let(:user) { create(:user) } + + it 'returns true when the email is verified/confirmed' do + email_confirmed = create :email, user: user, confirmed_at: Time.now + create :email, user: user + user.reload + + expect(user.verified_email?(user.email)).to be_truthy + expect(user.verified_email?(email_confirmed.email.titlecase)).to be_truthy + end + + it 'returns false when the email is not verified/confirmed' do + email_unconfirmed = create :email, user: user + user.reload + + expect(user.verified_email?(email_unconfirmed.email)).to be_falsy + end + end + describe '#requires_ldap_check?' do let(:user) { described_class.new } @@ -1101,6 +1207,7 @@ describe User do # Create a condition which would otherwise cause 'true' to be returned allow(user).to receive(:ldap_user?).and_return(true) user.last_credential_check_at = nil + expect(user.requires_ldap_check?).to be_falsey end @@ -1111,6 +1218,7 @@ describe User do it 'is false for non-LDAP users' do allow(user).to receive(:ldap_user?).and_return(false) + expect(user.requires_ldap_check?).to be_falsey end @@ -1121,11 +1229,13 @@ describe User do it 'is true when the user has never had an LDAP check before' do user.last_credential_check_at = nil + expect(user.requires_ldap_check?).to be_truthy end it 'is true when the last LDAP check happened over 1 hour ago' do user.last_credential_check_at = 2.hours.ago + expect(user.requires_ldap_check?).to be_truthy end end @@ -1136,16 +1246,19 @@ describe User do describe '#ldap_user?' do it 'is true if provider name starts with ldap' do user = create(:omniauth_user, provider: 'ldapmain') + expect(user.ldap_user?).to be_truthy end it 'is false for other providers' do user = create(:omniauth_user, provider: 'other-provider') + expect(user.ldap_user?).to be_falsey end it 'is false if no extern_uid is provided' do user = create(:omniauth_user, extern_uid: nil) + expect(user.ldap_user?).to be_falsey end end @@ -1153,6 +1266,7 @@ describe User do describe '#ldap_identity' do it 'returns ldap identity' do user = create :omniauth_user + expect(user.ldap_identity.provider).not_to be_empty end end @@ -1162,6 +1276,7 @@ describe User do it 'blocks user flaging the action caming from ldap' do user.ldap_block + expect(user.blocked?).to be_truthy expect(user.ldap_blocked?).to be_truthy end @@ -1234,18 +1349,22 @@ describe User do expect(user.starred?(project2)).to be_falsey star1 = UsersStarProject.create!(project: project1, user: user) + expect(user.starred?(project1)).to be_truthy expect(user.starred?(project2)).to be_falsey star2 = UsersStarProject.create!(project: project2, user: user) + expect(user.starred?(project1)).to be_truthy expect(user.starred?(project2)).to be_truthy star1.destroy + expect(user.starred?(project1)).to be_falsey expect(user.starred?(project2)).to be_truthy star2.destroy + expect(user.starred?(project1)).to be_falsey expect(user.starred?(project2)).to be_falsey end @@ -1257,9 +1376,13 @@ describe User do project = create(:project, :public) expect(user.starred?(project)).to be_falsey + user.toggle_star(project) + expect(user.starred?(project)).to be_truthy + user.toggle_star(project) + expect(user.starred?(project)).to be_falsey end end @@ -1438,9 +1561,11 @@ describe User do user = create(:user) member = group.add_developer(user) + expect(user.authorized_projects).to include(project) member.destroy + expect(user.authorized_projects).not_to include(project) end @@ -1461,9 +1586,11 @@ describe User do project = create(:project, :private, namespace: user1.namespace) project.team << [user2, Gitlab::Access::DEVELOPER] + expect(user2.authorized_projects).to include(project) project.destroy + expect(user2.authorized_projects).not_to include(project) end @@ -1473,9 +1600,11 @@ describe User do user = create(:user) group.add_developer(user) + expect(user.authorized_projects).to include(project) group.destroy + expect(user.authorized_projects).not_to include(project) end end @@ -2019,7 +2148,9 @@ describe User do it 'creates the namespace' do expect(user.namespace).to be_nil + user.save! + expect(user.namespace).not_to be_nil end end @@ -2040,11 +2171,13 @@ describe User do 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 @@ -2058,6 +2191,7 @@ describe User do 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 @@ -2074,56 +2208,49 @@ describe User do end end - describe '#verified_email?' do - it 'returns true when the email is the primary email' do - user = build :user, email: 'email@example.com' - - expect(user.verified_email?('email@example.com')).to be true - end - - it 'returns false when the email is not the primary email' do - user = build :user, email: 'email@example.com' - - expect(user.verified_email?('other_email@example.com')).to be false - end - end - describe '#sync_attribute?' do let(:user) { described_class.new } context 'oauth user' do it 'returns true if name can be synced' do stub_omniauth_setting(sync_profile_attributes: %w(name location)) + expect(user.sync_attribute?(:name)).to be_truthy end it 'returns true if email can be synced' do stub_omniauth_setting(sync_profile_attributes: %w(name email)) + expect(user.sync_attribute?(:email)).to be_truthy end it 'returns true if location can be synced' do stub_omniauth_setting(sync_profile_attributes: %w(location email)) + expect(user.sync_attribute?(:email)).to be_truthy end it 'returns false if name can not be synced' do stub_omniauth_setting(sync_profile_attributes: %w(location email)) + expect(user.sync_attribute?(:name)).to be_falsey end it 'returns false if email can not be synced' do stub_omniauth_setting(sync_profile_attributes: %w(location email)) + expect(user.sync_attribute?(:name)).to be_falsey end it 'returns false if location can not be synced' do stub_omniauth_setting(sync_profile_attributes: %w(location email)) + expect(user.sync_attribute?(:name)).to be_falsey end it 'returns true for all syncable attributes if all syncable attributes can be synced' do stub_omniauth_setting(sync_profile_attributes: true) + expect(user.sync_attribute?(:name)).to be_truthy expect(user.sync_attribute?(:email)).to be_truthy expect(user.sync_attribute?(:location)).to be_truthy @@ -2139,6 +2266,7 @@ describe User do context 'ldap user' do it 'returns true for email if ldap user' do allow(user).to receive(:ldap_user?).and_return(true) + expect(user.sync_attribute?(:name)).to be_falsey expect(user.sync_attribute?(:email)).to be_truthy expect(user.sync_attribute?(:location)).to be_falsey @@ -2147,6 +2275,7 @@ describe User do it 'returns true for email and location if ldap user and location declared as syncable' do allow(user).to receive(:ldap_user?).and_return(true) stub_omniauth_setting(sync_profile_attributes: %w(location)) + expect(user.sync_attribute?(:name)).to be_falsey expect(user.sync_attribute?(:email)).to be_truthy expect(user.sync_attribute?(:location)).to be_truthy diff --git a/spec/services/emails/confirm_service_spec.rb b/spec/services/emails/confirm_service_spec.rb new file mode 100644 index 00000000000..2b2c31e2521 --- /dev/null +++ b/spec/services/emails/confirm_service_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe Emails::ConfirmService do + let(:user) { create(:user) } + + subject(:service) { described_class.new(user) } + + describe '#execute' do + it 'sends a confirmation email again' do + email = user.emails.create(email: 'new@email.com') + mail = service.execute(email) + expect(mail.subject).to eq('Confirmation instructions') + end + end +end diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 75812c17309..54692c88623 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -12,6 +12,11 @@ describe Emails::CreateService do expect(Email.where(opts)).not_to be_empty end + it 'creates an email with additional attributes' do + expect { service.execute(confirmation_token: 'abc') }.to change { Email.count }.by(1) + expect(Email.where(opts).first.confirmation_token).to eq 'abc' + end + it 'has the right user association' do service.execute diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 7726fc0ef81..c3204fac3df 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -4,11 +4,11 @@ describe Emails::DestroyService do let!(:user) { create(:user) } let!(:email) { create(:email, user: user) } - subject(:service) { described_class.new(user, user: user, email: email.email) } + subject(:service) { described_class.new(user, user: user) } describe '#execute' do it 'removes an email' do - expect { service.execute }.to change { user.emails.count }.by(-1) + expect { service.execute(email) }.to change { user.emails.count }.by(-1) end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index f4b36eb7eeb..b64ca5be8fc 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -105,18 +105,6 @@ describe NotificationService, :mailer do end end - describe 'Email' do - describe '#new_email' do - let!(:email) { create(:email) } - - it { expect(notification.new_email(email)).to be_truthy } - - it 'sends email to email owner' do - expect { notification.new_email(email) }.to change { ActionMailer::Base.deliveries.size }.by(1) - end - end - end - describe 'Notes' do context 'issue note' do let(:project) { create(:project, :private) } |