diff options
Diffstat (limited to 'app')
47 files changed, 296 insertions, 204 deletions
diff --git a/app/assets/javascripts/create_label.js.es6 b/app/assets/javascripts/create_label.js.es6 index 947c129d5b5..85384d98126 100644 --- a/app/assets/javascripts/create_label.js.es6 +++ b/app/assets/javascripts/create_label.js.es6 @@ -107,9 +107,9 @@ if (typeof label.message === 'string') { errors = label.message; } else { - errors = label.message.map(function (value, key) { - return key + " " + value[0]; - }).join("<br/>"); + errors = Object.keys(label.message).map(key => + `${gl.text.humanize(key)} ${label.message[key].join(', ')}` + ).join("<br/>"); } this.$newLabelError diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 45aa6050aed..f55db02f0fd 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -74,7 +74,7 @@ const ShortcutsBlob = require('./shortcuts_blob'); case 'projects:merge_requests:index': case 'projects:issues:index': if (gl.FilteredSearchManager) { - new gl.FilteredSearchManager(); + new gl.FilteredSearchManager(page === 'projects:issues:index' ? 'issues' : 'merge_requests'); } Issuable.init(); new gl.IssuableBulkActions({ diff --git a/app/assets/javascripts/filtered_search/dropdown_hint.js.es6 b/app/assets/javascripts/filtered_search/dropdown_hint.js.es6 index 572c221929a..9e92d544bef 100644 --- a/app/assets/javascripts/filtered_search/dropdown_hint.js.es6 +++ b/app/assets/javascripts/filtered_search/dropdown_hint.js.es6 @@ -37,23 +37,18 @@ require('./filtered_search_dropdown'); } renderContent() { - const dropdownData = [{ - icon: 'fa-pencil', - hint: 'author:', - tag: '<@author>', - }, { - icon: 'fa-user', - hint: 'assignee:', - tag: '<@assignee>', - }, { - icon: 'fa-clock-o', - hint: 'milestone:', - tag: '<%milestone>', - }, { - icon: 'fa-tag', - hint: 'label:', - tag: '<~label>', - }]; + const dropdownData = []; + + [].forEach.call(this.input.parentElement.querySelectorAll('.dropdown-menu'), (dropdownMenu) => { + const { icon, hint, tag } = dropdownMenu.dataset; + if (icon && hint && tag) { + dropdownData.push({ + icon: `fa-${icon}`, + hint, + tag: `<${tag}>`, + }); + } + }); this.droplab.changeHookList(this.hookId, this.dropdown, [droplabFilter], this.config); this.droplab.setData(this.hookId, dropdownData); diff --git a/app/assets/javascripts/filtered_search/filtered_search_dropdown.js.es6 b/app/assets/javascripts/filtered_search/filtered_search_dropdown.js.es6 index e8c2df03a46..fbc72a3001a 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_dropdown.js.es6 +++ b/app/assets/javascripts/filtered_search/filtered_search_dropdown.js.es6 @@ -52,8 +52,9 @@ } renderContent(forceShowList = false) { - if (forceShowList && this.getCurrentHook().list.hidden) { - this.getCurrentHook().list.show(); + const currentHook = this.getCurrentHook(); + if (forceShowList && currentHook && currentHook.list.hidden) { + currentHook.list.show(); } } @@ -92,18 +93,24 @@ } hideDropdown() { - this.getCurrentHook().list.hide(); + const currentHook = this.getCurrentHook(); + if (currentHook) { + currentHook.list.hide(); + } } resetFilters() { const hook = this.getCurrentHook(); - const data = hook.list.data; - const results = data.map((o) => { - const updated = o; - updated.droplab_hidden = false; - return updated; - }); - hook.list.render(results); + + if (hook) { + const data = hook.list.data; + const results = data.map((o) => { + const updated = o; + updated.droplab_hidden = false; + return updated; + }); + hook.list.render(results); + } } } diff --git a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js.es6 b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js.es6 index 8ce4cf4fc36..cecd3518ce3 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js.es6 +++ b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js.es6 @@ -2,10 +2,12 @@ (() => { class FilteredSearchDropdownManager { - constructor(baseEndpoint = '') { + constructor(baseEndpoint = '', page) { this.baseEndpoint = baseEndpoint.replace(/\/$/, ''); this.tokenizer = gl.FilteredSearchTokenizer; + this.filteredSearchTokenKeys = gl.FilteredSearchTokenKeys; this.filteredSearchInput = document.querySelector('.filtered-search'); + this.page = page; this.setupMapping(); @@ -150,7 +152,7 @@ this.droplab = new DropLab(); } - const match = gl.FilteredSearchTokenKeys.searchByKey(dropdownName.toLowerCase()); + const match = this.filteredSearchTokenKeys.searchByKey(dropdownName.toLowerCase()); const shouldOpenFilterDropdown = match && this.currentDropdown !== match.key && this.mapping[match.key]; const shouldOpenHintDropdown = !match && this.currentDropdown !== 'hint'; diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js.es6 b/app/assets/javascripts/filtered_search/filtered_search_manager.js.es6 index 13a9bf59246..bbafead0305 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js.es6 +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js.es6 @@ -1,12 +1,13 @@ (() => { class FilteredSearchManager { - constructor() { + constructor(page) { this.filteredSearchInput = document.querySelector('.filtered-search'); this.clearSearchButton = document.querySelector('.clear-search'); + this.filteredSearchTokenKeys = gl.FilteredSearchTokenKeys; if (this.filteredSearchInput) { this.tokenizer = gl.FilteredSearchTokenizer; - this.dropdownManager = new gl.FilteredSearchDropdownManager(this.filteredSearchInput.getAttribute('data-base-endpoint') || ''); + this.dropdownManager = new gl.FilteredSearchDropdownManager(this.filteredSearchInput.getAttribute('data-base-endpoint') || '', page); this.bindEvents(); this.loadSearchParamsFromURL(); @@ -117,8 +118,8 @@ const keyParam = decodeURIComponent(split[0]); const value = split[1]; - // Check if it matches edge conditions listed in gl.FilteredSearchTokenKeys - const condition = gl.FilteredSearchTokenKeys.searchByConditionUrl(p); + // Check if it matches edge conditions listed in this.filteredSearchTokenKeys + const condition = this.filteredSearchTokenKeys.searchByConditionUrl(p); if (condition) { inputValues.push(`${condition.tokenKey}:${condition.value}`); @@ -126,7 +127,7 @@ // Sanitize value since URL converts spaces into + // Replace before decode so that we know what was originally + versus the encoded + const sanitizedValue = value ? decodeURIComponent(value.replace(/\+/g, ' ')) : value; - const match = gl.FilteredSearchTokenKeys.searchByKeyParam(keyParam); + const match = this.filteredSearchTokenKeys.searchByKeyParam(keyParam); if (match) { const indexOf = keyParam.indexOf('_'); @@ -171,9 +172,9 @@ paths.push(`state=${currentState}`); tokens.forEach((token) => { - const condition = gl.FilteredSearchTokenKeys + const condition = this.filteredSearchTokenKeys .searchByConditionKeyValue(token.key, token.value.toLowerCase()); - const { param } = gl.FilteredSearchTokenKeys.searchByKey(token.key) || {}; + const { param } = this.filteredSearchTokenKeys.searchByKey(token.key) || {}; const keyParam = param ? `${token.key}_${param}` : token.key; let tokenPath = ''; diff --git a/app/assets/javascripts/header.js b/app/assets/javascripts/header.js index fa85f9a6c86..a853c3aeb1f 100644 --- a/app/assets/javascripts/header.js +++ b/app/assets/javascripts/header.js @@ -2,7 +2,7 @@ (function() { $(document).on('todo:toggle', function(e, count) { var $todoPendingCount = $('.todos-pending-count'); - $todoPendingCount.text(gl.text.addDelimiter(count)); + $todoPendingCount.text(gl.text.highCountTrim(count)); $todoPendingCount.toggleClass('hidden', count === 0); }); })(); diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index f755d212b3c..579d322e3fb 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -14,6 +14,9 @@ require('vendor/latinise'); gl.text.addDelimiter = function(text) { return text ? text.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ",") : text; }; + gl.text.highCountTrim = function(count) { + return count > 99 ? '99+' : count; + }; gl.text.randomString = function() { return Math.random().toString(36).substring(7); }; diff --git a/app/assets/javascripts/search_autocomplete.js.es6 b/app/assets/javascripts/search_autocomplete.js.es6 index 6250e75d407..6fd5345a0a6 100644 --- a/app/assets/javascripts/search_autocomplete.js.es6 +++ b/app/assets/javascripts/search_autocomplete.js.es6 @@ -169,10 +169,10 @@ url: issuesPath + "/?author_username=" + userName }, 'separator', { text: 'Merge requests assigned to me', - url: mrPath + "/?assignee_id=" + userId + url: mrPath + "/?assignee_username=" + userName }, { text: "Merge requests I've created", - url: mrPath + "/?author_id=" + userId + url: mrPath + "/?author_username=" + userName } ]; if (!name) { diff --git a/app/assets/stylesheets/pages/todos.scss b/app/assets/stylesheets/pages/todos.scss index bac831ec315..af9ddb9ff80 100644 --- a/app/assets/stylesheets/pages/todos.scss +++ b/app/assets/stylesheets/pages/todos.scss @@ -6,6 +6,8 @@ .navbar-nav { li { .badge.todos-pending-count { + position: inherit; + top: -6px; margin-top: -5px; font-weight: normal; background: $todo-alert-blue; diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bf6be3d516b..5e7af3bff0d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -74,7 +74,7 @@ class ApplicationController < ActionController::Base def authenticate_user!(*args) if redirect_to_home_page_url? - redirect_to current_application_settings.home_page_url and return + return redirect_to current_application_settings.home_page_url end super(*args) @@ -131,7 +131,7 @@ class ApplicationController < ActionController::Base headers['X-UA-Compatible'] = 'IE=edge' headers['X-Content-Type-Options'] = 'nosniff' # Enabling HSTS for non-standard ports would send clients to the wrong port - if Gitlab.config.gitlab.https and Gitlab.config.gitlab.port == 443 + if Gitlab.config.gitlab.https && Gitlab.config.gitlab.port == 443 headers['Strict-Transport-Security'] = 'max-age=31536000' end end @@ -152,7 +152,7 @@ class ApplicationController < ActionController::Base def check_password_expiration if current_user && current_user.password_expires_at && current_user.password_expires_at < Time.now && !current_user.ldap_user? - redirect_to new_profile_password_path and return + return redirect_to new_profile_password_path end end @@ -218,7 +218,7 @@ class ApplicationController < ActionController::Base def require_email if current_user && current_user.temp_oauth_email? && session[:impersonator_id].nil? - redirect_to profile_path, notice: 'Please complete your profile with email address' and return + return redirect_to profile_path, notice: 'Please complete your profile with email address' end end diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb index a6891149bfa..da225d8f1c7 100644 --- a/app/controllers/concerns/spammable_actions.rb +++ b/app/controllers/concerns/spammable_actions.rb @@ -17,13 +17,31 @@ module SpammableActions private - def recaptcha_params - return {} unless params[:recaptcha_verification] && Gitlab::Recaptcha.load_configurations! && verify_recaptcha + def recaptcha_check_with_fallback(&fallback) + if spammable.valid? + redirect_to spammable + elsif render_recaptcha? + if params[:recaptcha_verification] + flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' + end + + render :verify + else + fallback.call + end + end + + def spammable_params + default_params = { request: request } + + recaptcha_check = params[:recaptcha_verification] && + Gitlab::Recaptcha.load_configurations! && + verify_recaptcha + + return default_params unless recaptcha_check - { - recaptcha_verified: true, - spam_log_id: params[:spam_log_id] - } + { recaptcha_verified: true, + spam_log_id: params[:spam_log_id] }.merge(default_params) end def spammable diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 4e61b0886d8..5848ca62777 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -1,4 +1,6 @@ class Dashboard::TodosController < Dashboard::ApplicationController + include ActionView::Helpers::NumberHelper + before_action :find_todos, only: [:index, :destroy_all] def index @@ -35,6 +37,11 @@ class Dashboard::TodosController < Dashboard::ApplicationController render json: todos_counts end + # Used in TodosHelper also + def self.todos_count_format(count) + count >= 100 ? '99+' : count + end + private def find_todos @@ -43,8 +50,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController def todos_counts { - count: current_user.todos_pending_count, - done_count: current_user.todos_done_count + count: number_with_delimiter(current_user.todos_pending_count), + done_count: number_with_delimiter(current_user.todos_done_count) } end end diff --git a/app/controllers/import/fogbugz_controller.rb b/app/controllers/import/fogbugz_controller.rb index 99b10b2f9b3..5df6bd34185 100644 --- a/app/controllers/import/fogbugz_controller.rb +++ b/app/controllers/import/fogbugz_controller.rb @@ -29,7 +29,7 @@ class Import::FogbugzController < Import::BaseController unless user_map.is_a?(Hash) && user_map.all? { |k, v| !v[:name].blank? } flash.now[:alert] = 'All users must have a name.' - render 'new_user_map' and return + return render 'new_user_map' end session[:fogbugz_user_map] = user_map diff --git a/app/controllers/import/google_code_controller.rb b/app/controllers/import/google_code_controller.rb index 8d0de158f98..7d7f13ce5d5 100644 --- a/app/controllers/import/google_code_controller.rb +++ b/app/controllers/import/google_code_controller.rb @@ -44,13 +44,13 @@ class Import::GoogleCodeController < Import::BaseController rescue flash.now[:alert] = "The entered user map is not a valid JSON user map." - render "new_user_map" and return + return render "new_user_map" end unless user_map.is_a?(Hash) && user_map.all? { |k, v| k.is_a?(String) && v.is_a?(String) } flash.now[:alert] = "The entered user map is not a valid JSON user map." - render "new_user_map" and return + return render "new_user_map" end # This is the default, so let's not save it into the database. diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 58964a0e65d..7625187c7be 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -42,9 +42,7 @@ class InvitesController < ApplicationController @token = params[:id] @member = Member.find_by_invite_token(@token) - unless @member - render_404 and return - end + return render_404 unless @member @member end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 3ab7e6e0658..58d50ad647b 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -122,7 +122,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController else error_message = @user.errors.full_messages.to_sentence - redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return + return redirect_to omniauth_error_path(oauth['provider'], error: error_message) end end diff --git a/app/controllers/profiles/keys_controller.rb b/app/controllers/profiles/keys_controller.rb index 830e0b9591b..c8663a3c38e 100644 --- a/app/controllers/profiles/keys_controller.rb +++ b/app/controllers/profiles/keys_controller.rb @@ -45,13 +45,13 @@ class Profiles::KeysController < Profiles::ApplicationController if user.present? render text: user.all_ssh_keys.join("\n"), content_type: "text/plain" else - render_404 and return + return render_404 end rescue => e render text: e.message end else - render_404 and return + return render_404 end end diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index a1db856dcfb..39ba815cfca 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -95,7 +95,7 @@ class Projects::BlobController < Projects::ApplicationController else if tree = @repository.tree(@commit.id, @path) if tree.entries.any? - redirect_to namespace_project_tree_path(@project.namespace, @project, File.join(@ref, @path)) and return + return redirect_to namespace_project_tree_path(@project.namespace, @project, File.join(@ref, @path)) end end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 744a4af1c51..6ef36771ac1 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -94,15 +94,15 @@ class Projects::IssuesController < Projects::ApplicationController end def create - extra_params = { request: request, - merge_request_for_resolving_discussions: merge_request_for_resolving_discussions } - extra_params.merge!(recaptcha_params) + create_params = issue_params + .merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions) + .merge(spammable_params) - @issue = Issues::CreateService.new(project, current_user, issue_params.merge(extra_params)).execute + @issue = Issues::CreateService.new(project, current_user, create_params).execute respond_to do |format| format.html do - html_response_create + recaptcha_check_with_fallback { render :new } end format.js do @link = @issue.attachment.url.to_js @@ -111,7 +111,9 @@ class Projects::IssuesController < Projects::ApplicationController end def update - @issue = Issues::UpdateService.new(project, current_user, issue_params).execute(issue) + update_params = issue_params.merge(spammable_params) + + @issue = Issues::UpdateService.new(project, current_user, update_params).execute(issue) if params[:move_to_project_id].to_i > 0 new_project = Project.find(params[:move_to_project_id]) @@ -123,11 +125,7 @@ class Projects::IssuesController < Projects::ApplicationController respond_to do |format| format.html do - if @issue.valid? - redirect_to issue_path(@issue) - else - render :edit - end + recaptcha_check_with_fallback { render :edit } end format.json do @@ -179,20 +177,6 @@ class Projects::IssuesController < Projects::ApplicationController protected - def html_response_create - if @issue.valid? - redirect_to issue_path(@issue) - elsif render_recaptcha? - if params[:recaptcha_verification] - flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' - end - - render :verify - else - render :new - end - end - def issue # The Sortable default scope causes performance issues when used with find_by @noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take || redirect_old diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 2bf3542d089..75971faa93e 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -50,6 +50,17 @@ class Projects::MergeRequestsController < Projects::ApplicationController @labels = LabelsFinder.new(current_user, labels_params).execute end + @users = [] + if params[:assignee_id].present? + assignee = User.find_by_id(params[:assignee_id]) + @users.push(assignee) if assignee + end + + if params[:author_id].present? + author = User.find_by_id(params[:author_id]) + @users.push(author) if author + end + respond_to do |format| format.html format.json do diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index ef5d3d242eb..ea1a97b7cf0 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -38,24 +38,19 @@ class Projects::SnippetsController < Projects::ApplicationController end def create - create_params = snippet_params.merge(request: request) + create_params = snippet_params.merge(spammable_params) + @snippet = CreateSnippetService.new(@project, current_user, create_params).execute - if @snippet.valid? - respond_with(@snippet, - location: namespace_project_snippet_path(@project.namespace, - @project, @snippet)) - else - render :new - end + recaptcha_check_with_fallback { render :new } end def update - UpdateSnippetService.new(project, current_user, @snippet, - snippet_params).execute - respond_with(@snippet, - location: namespace_project_snippet_path(@project.namespace, - @project, @snippet)) + update_params = snippet_params.merge(spammable_params) + + UpdateSnippetService.new(project, current_user, @snippet, update_params).execute + + recaptcha_check_with_fallback { render :edit } end def show diff --git a/app/controllers/projects/tree_controller.rb b/app/controllers/projects/tree_controller.rb index cb3ed0f6f9c..4f094146348 100644 --- a/app/controllers/projects/tree_controller.rb +++ b/app/controllers/projects/tree_controller.rb @@ -15,10 +15,10 @@ class Projects::TreeController < Projects::ApplicationController if tree.entries.empty? if @repository.blob_at(@commit.id, @path) - redirect_to( + return redirect_to( namespace_project_blob_path(@project.namespace, @project, File.join(@ref, @path)) - ) and return + ) elsif @path.present? return render_404 end diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 366804ab17e..2d26718873f 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -23,7 +23,7 @@ class SnippetsController < ApplicationController if params[:username].present? @user = User.find_by(username: params[:username]) - render_404 and return unless @user + return render_404 unless @user @snippets = SnippetsFinder.new.execute(current_user, { filter: :by_user, @@ -42,16 +42,19 @@ class SnippetsController < ApplicationController end def create - create_params = snippet_params.merge(request: request) + create_params = snippet_params.merge(spammable_params) + @snippet = CreateSnippetService.new(nil, current_user, create_params).execute - respond_with @snippet.becomes(Snippet) + recaptcha_check_with_fallback { render :new } end def update - UpdateSnippetService.new(nil, current_user, @snippet, - snippet_params).execute - respond_with @snippet.becomes(Snippet) + update_params = snippet_params.merge(spammable_params) + + UpdateSnippetService.new(nil, current_user, @snippet, update_params).execute + + recaptcha_check_with_fallback { render :edit } end def show diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 845f1a0e840..c52afd6db1c 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -3,6 +3,10 @@ module TodosHelper @todos_pending_count ||= current_user.todos_pending_count end + def todos_count_format(count) + count > 99 ? '99+' : count + end + def todos_done_count @todos_done_count ||= current_user.todos_done_count end diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb index 79adc77c9e4..107e6764ba2 100644 --- a/app/models/concerns/spammable.rb +++ b/app/models/concerns/spammable.rb @@ -13,7 +13,7 @@ module Spammable attr_accessor :spam attr_accessor :spam_log - after_validation :check_for_spam, on: :create + after_validation :check_for_spam, on: [:create, :update] cattr_accessor :spammable_attrs, instance_accessor: false do [] diff --git a/app/models/project.rb b/app/models/project.rb index fc5b1a66910..411299eef63 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -552,7 +552,7 @@ class Project < ActiveRecord::Base end def check_limit - unless creator.can_create_project? or namespace.kind == 'group' + unless creator.can_create_project? || namespace.kind == 'group' projects_limit = creator.projects_limit if projects_limit == 0 diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb index 942ec9371e5..1ad9efac196 100644 --- a/app/models/project_services/drone_ci_service.rb +++ b/app/models/project_services/drone_ci_service.rb @@ -52,7 +52,7 @@ class DroneCiService < CiService response = HTTParty.get(commit_status_path(sha, ref), verify: enable_ssl_verification) status = - if response.code == 200 and response['status'] + if response.code == 200 && response['status'] case response['status'] when 'killed' :canceled diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb index 5d93064f9b3..5d6862d9faa 100644 --- a/app/models/project_services/irker_service.rb +++ b/app/models/project_services/irker_service.rb @@ -96,7 +96,7 @@ class IrkerService < Service rescue URI::InvalidURIError end - unless uri.present? and default_irc_uri.nil? + unless uri.present? && default_irc_uri.nil? begin new_recipient = URI.join(default_irc_uri, '/', recipient).to_s uri = consider_uri(URI.parse(new_recipient)) diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb index 9bb456eee24..25b5d777641 100644 --- a/app/models/project_snippet.rb +++ b/app/models/project_snippet.rb @@ -9,8 +9,4 @@ class ProjectSnippet < Snippet participant :author participant :notes_with_associations - - def check_for_spam? - super && project.public? - end end diff --git a/app/services/create_snippet_service.rb b/app/services/create_snippet_service.rb index 14f5ba064ff..40286dbf3bf 100644 --- a/app/services/create_snippet_service.rb +++ b/app/services/create_snippet_service.rb @@ -1,7 +1,8 @@ class CreateSnippetService < BaseService + include SpamCheckService + def execute - request = params.delete(:request) - api = params.delete(:api) + filter_spam_check_params snippet = if project project.snippets.build(params) @@ -15,10 +16,11 @@ class CreateSnippetService < BaseService end snippet.author = current_user - snippet.spam = SpamService.new(snippet, request).check(api) + + spam_check(snippet, current_user) if snippet.save - UserAgentDetailService.new(snippet, request).create + UserAgentDetailService.new(snippet, @request).create end snippet diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 5f3ced49665..9500faf2862 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -191,14 +191,12 @@ class IssuableBaseService < BaseService # To be overridden by subclasses end - def after_update(issuable) + def before_update(issuable) # To be overridden by subclasses end - def update_issuable(issuable, attributes) - issuable.with_transaction_returning_status do - issuable.update(attributes.merge(updated_by: current_user)) - end + def after_update(issuable) + # To be overridden by subclasses end def update(issuable) @@ -212,16 +210,22 @@ class IssuableBaseService < BaseService label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) - if params.present? && update_issuable(issuable, params) - # We do not touch as it will affect a update on updated_at field - ActiveRecord::Base.no_touching do - handle_common_system_notes(issuable, old_labels: old_labels) - end + if params.present? + issuable.assign_attributes(params.merge(updated_by: current_user)) + + before_update(issuable) - handle_changes(issuable, old_labels: old_labels, old_mentioned_users: old_mentioned_users) - after_update(issuable) - issuable.create_new_cross_references!(current_user) - execute_hooks(issuable, 'update') + if issuable.with_transaction_returning_status { issuable.save } + # We do not touch as it will affect a update on updated_at field + ActiveRecord::Base.no_touching do + handle_common_system_notes(issuable, old_labels: old_labels) + end + + handle_changes(issuable, old_labels: old_labels, old_mentioned_users: old_mentioned_users) + after_update(issuable) + issuable.create_new_cross_references!(current_user) + execute_hooks(issuable, 'update') + end end issuable diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 961605a1005..366b3572738 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -1,10 +1,9 @@ module Issues class CreateService < Issues::BaseService + include SpamCheckService + def execute - @request = params.delete(:request) - @api = params.delete(:api) - @recaptcha_verified = params.delete(:recaptcha_verified) - @spam_log_id = params.delete(:spam_log_id) + filter_spam_check_params issue_attributes = params.merge(merge_request_for_resolving_discussions: merge_request_for_resolving_discussions) @issue = BuildService.new(project, current_user, issue_attributes).execute @@ -12,14 +11,8 @@ module Issues create(@issue) end - def before_create(issuable) - if @recaptcha_verified - spam_log = current_user.spam_logs.find_by(id: @spam_log_id, title: issuable.title) - spam_log&.update!(recaptcha_verified: true) - else - issuable.spam = spam_service.check(@api) - issuable.spam_log = spam_service.spam_log - end + def before_create(issue) + spam_check(issue, current_user) end def after_create(issuable) @@ -42,10 +35,6 @@ module Issues private - def spam_service - @spam_service ||= SpamService.new(@issue, @request) - end - def user_agent_detail_service UserAgentDetailService.new(@issue, @request) end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 78cbf94ec69..22e32b13259 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -1,9 +1,17 @@ module Issues class UpdateService < Issues::BaseService + include SpamCheckService + def execute(issue) + filter_spam_check_params + update(issue) end + def before_update(issue) + spam_check(issue, current_user) + end + def handle_changes(issue, old_labels: [], old_mentioned_users: []) if has_changes?(issue, old_labels: old_labels) todo_service.mark_pending_todos_as_done(issue, current_user) diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index f4d52e3ebbd..9d4739e37bb 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -2,18 +2,14 @@ module MergeRequests class BuildService < MergeRequests::BaseService def execute self.merge_request = MergeRequest.new(params) - merge_request.can_be_created = true merge_request.compare_commits = [] merge_request.source_project = find_source_project merge_request.target_project = find_target_project merge_request.target_branch = find_target_branch + merge_request.can_be_created = branches_valid? && source_branch_specified? && target_branch_specified? - if branches_specified? && branches_valid? - compare_branches - assign_title_and_description - else - merge_request.can_be_created = false - end + compare_branches if branches_present? + assign_title_and_description if merge_request.can_be_created merge_request end @@ -37,11 +33,17 @@ module MergeRequests target_branch || target_project.default_branch end - def branches_specified? - params[:source_branch] && params[:target_branch] + def source_branch_specified? + params[:source_branch].present? + end + + def target_branch_specified? + params[:target_branch].present? end def branches_valid? + return false unless source_branch_specified? || target_branch_specified? + validate_branches errors.blank? end @@ -55,8 +57,10 @@ module MergeRequests target_branch ) - merge_request.compare_commits = compare.commits - merge_request.compare = compare + if compare + merge_request.compare_commits = compare.commits + merge_request.compare = compare + end end def validate_branches diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb index 012e82a7704..be34d4fa9b8 100644 --- a/app/services/projects/upload_service.rb +++ b/app/services/projects/upload_service.rb @@ -5,7 +5,7 @@ module Projects end def execute - return nil unless @file and @file.size <= max_attachment_size + return nil unless @file && @file.size <= max_attachment_size uploader = FileUploader.new(@project) uploader.store!(@file) diff --git a/app/services/spam_check_service.rb b/app/services/spam_check_service.rb new file mode 100644 index 00000000000..023e0824e85 --- /dev/null +++ b/app/services/spam_check_service.rb @@ -0,0 +1,24 @@ +# SpamCheckService +# +# Provide helper methods for checking if a given spammable object has +# potential spam data. +# +# Dependencies: +# - params with :request +# +module SpamCheckService + def filter_spam_check_params + @request = params.delete(:request) + @api = params.delete(:api) + @recaptcha_verified = params.delete(:recaptcha_verified) + @spam_log_id = params.delete(:spam_log_id) + end + + def spam_check(spammable, user) + spam_service = SpamService.new(spammable, @request) + + spam_service.when_recaptcha_verified(@recaptcha_verified, @api) do + user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true) + end + end +end diff --git a/app/services/spam_service.rb b/app/services/spam_service.rb index 024a7c19d33..3e65b7d31a3 100644 --- a/app/services/spam_service.rb +++ b/app/services/spam_service.rb @@ -17,15 +17,6 @@ class SpamService end end - def check(api = false) - return false unless request && check_for_spam? - - return false unless akismet.is_spam? - - create_spam_log(api) - true - end - def mark_as_spam! return false unless spammable.submittable_as_spam? @@ -36,8 +27,30 @@ class SpamService end end + def when_recaptcha_verified(recaptcha_verified, api = false) + # In case it's a request which is already verified through recaptcha, yield + # block. + if recaptcha_verified + yield + else + # Otherwise, it goes to Akismet and check if it's a spam. If that's the + # case, it assigns spammable record as "spam" and create a SpamLog record. + spammable.spam = check(api) + spammable.spam_log = spam_log + end + end + private + def check(api) + return false unless request && check_for_spam? + + return false unless akismet.is_spam? + + create_spam_log(api) + true + end + def akismet @akismet ||= AkismetService.new( spammable_owner, diff --git a/app/services/update_snippet_service.rb b/app/services/update_snippet_service.rb index a6bb36821c3..358bca73aec 100644 --- a/app/services/update_snippet_service.rb +++ b/app/services/update_snippet_service.rb @@ -1,4 +1,6 @@ class UpdateSnippetService < BaseService + include SpamCheckService + attr_accessor :snippet def initialize(project, user, snippet, params) @@ -9,7 +11,7 @@ class UpdateSnippetService < BaseService def execute # check that user is allowed to set specified visibility_level new_visibility = params[:visibility_level] - + if new_visibility && new_visibility.to_i != snippet.visibility_level unless Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) deny_visibility_level(snippet, new_visibility) @@ -17,6 +19,10 @@ class UpdateSnippetService < BaseService end end - snippet.update_attributes(params) + filter_spam_check_params + snippet.assign_attributes(params) + spam_check(snippet, current_user) + + snippet.save end end diff --git a/app/views/layouts/_recaptcha_verification.html.haml b/app/views/layouts/_recaptcha_verification.html.haml new file mode 100644 index 00000000000..77c77dc6754 --- /dev/null +++ b/app/views/layouts/_recaptcha_verification.html.haml @@ -0,0 +1,23 @@ +- humanized_resource_name = spammable.class.model_name.human.downcase +- resource_name = spammable.class.model_name.singular + +%h3.page-title + Anti-spam verification +%hr + +%p + #{"We detected potential spam in the #{humanized_resource_name}. Please solve the reCAPTCHA to proceed."} + += form_for form do |f| + .recaptcha + - params[resource_name].each do |field, value| + = hidden_field(resource_name, field, value: value) + = hidden_field_tag(:spam_log_id, spammable.spam_log.id) + = hidden_field_tag(:recaptcha_verification, true) + = recaptcha_tags + + -# Yields a block with given extra params. + = yield + + .row-content-block.footer-block + = f.submit "Submit #{humanized_resource_name}", class: 'btn btn-create' diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 60b9b8bdbc4..f8986893776 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -35,7 +35,7 @@ = link_to dashboard_todos_path, title: 'Todos', aria: { label: "Todos" }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do = icon('bell fw') %span.badge.todos-pending-count{ class: ("hidden" if todos_pending_count == 0) } - = todos_pending_count + = todos_count_format(todos_pending_count) - if Gitlab::Sherlock.enabled? %li = link_to sherlock_transactions_path, title: 'Sherlock Transactions', diff --git a/app/views/projects/issues/verify.html.haml b/app/views/projects/issues/verify.html.haml index 1934b18c086..09aa401e44a 100644 --- a/app/views/projects/issues/verify.html.haml +++ b/app/views/projects/issues/verify.html.haml @@ -1,20 +1,4 @@ -- page_title "Anti-spam verification" +- form = [@project.namespace.becomes(Namespace), @project, @issue] -%h3.page-title - Anti-spam verification -%hr - -%p - We detected potential spam in the issue description. Please verify that you are not a robot to submit the issue. - -= form_for [@project.namespace.becomes(Namespace), @project, @issue] do |f| - .recaptcha - - params[:issue].each do |field, value| - = hidden_field(:issue, field, value: value) - = hidden_field_tag(:merge_request_for_resolving_discussions, params[:merge_request_for_resolving_discussions]) - = hidden_field_tag(:spam_log_id, @issue.spam_log.id) - = hidden_field_tag(:recaptcha_verification, true) - = recaptcha_tags - - .row-content-block.footer-block - = f.submit "Submit #{@issue.class.model_name.human.downcase}", class: 'btn btn-create' += render layout: 'layouts/recaptcha_verification', locals: { spammable: @issue, form: form } do + = hidden_field_tag(:merge_request_for_resolving_discussions, params[:merge_request_for_resolving_discussions]) diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml index 144b3a9c8c8..83e6c026ba7 100644 --- a/app/views/projects/merge_requests/index.html.haml +++ b/app/views/projects/merge_requests/index.html.haml @@ -5,18 +5,19 @@ = render "projects/issues/head" = render 'projects/last_push' +- content_for :page_specific_javascripts do + = page_specific_javascript_bundle_tag('filtered_search') + %div{ class: container_class } .top-area = render 'shared/issuable/nav', type: :merge_requests .nav-controls - = render 'shared/issuable/search_form', path: namespace_project_merge_requests_path(@project.namespace, @project) - - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project)) - if merge_project = link_to new_namespace_project_merge_request_path(merge_project.namespace, merge_project), class: "btn btn-new", title: "New Merge Request" do New Merge Request - = render 'shared/issuable/filter', type: :merge_requests + = render 'shared/issuable/search_bar', type: :merge_requests .merge-requests-holder = render 'merge_requests' diff --git a/app/views/projects/snippets/verify.html.haml b/app/views/projects/snippets/verify.html.haml new file mode 100644 index 00000000000..eb56f03b3f4 --- /dev/null +++ b/app/views/projects/snippets/verify.html.haml @@ -0,0 +1,4 @@ +- form = [@project.namespace.becomes(Namespace), @project, @snippet.becomes(Snippet)] + += render 'layouts/recaptcha_verification', spammable: @snippet, form: form + diff --git a/app/views/search/_results.html.haml b/app/views/search/_results.html.haml index 7fe2bce3e7c..22004ecacbc 100644 --- a/app/views/search/_results.html.haml +++ b/app/views/search/_results.html.haml @@ -11,7 +11,7 @@ .results.prepend-top-10 - if @scope == 'commits' - %ul.list-unstyled + %ul.content-list.commit-list.table-list.table-wide = render partial: "search/results/commit", collection: @search_objects - else .search-results diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index 6e417aa2251..8e04b50bb8a 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -32,7 +32,7 @@ {{hint}} %span.js-filter-tag.dropdown-light-content {{tag}} - #js-dropdown-author.dropdown-menu + #js-dropdown-author.dropdown-menu{ data: { icon: 'pencil', hint: 'author', tag: '@author' } } %ul.filter-dropdown{ 'data-dynamic' => true, 'data-dropdown' => true } %li.filter-dropdown-item %button.btn.btn-link.dropdown-user @@ -42,7 +42,7 @@ {{name}} %span.dropdown-light-content @{{username}} - #js-dropdown-assignee.dropdown-menu + #js-dropdown-assignee.dropdown-menu{ data: { icon: 'user', hint: 'assignee', tag: '@assignee' } } %ul{ 'data-dropdown' => true } %li.filter-dropdown-item{ 'data-value' => 'none' } %button.btn.btn-link @@ -57,7 +57,7 @@ {{name}} %span.dropdown-light-content @{{username}} - #js-dropdown-milestone.dropdown-menu{ 'data-dropdown' => true } + #js-dropdown-milestone.dropdown-menu{ data: { icon: 'clock-o', hint: 'milestone', tag: '%milestone' } } %ul{ 'data-dropdown' => true } %li.filter-dropdown-item{ 'data-value' => 'none' } %button.btn.btn-link @@ -70,7 +70,7 @@ %li.filter-dropdown-item %button.btn.btn-link.js-data-value {{title}} - #js-dropdown-label.dropdown-menu{ 'data-dropdown' => true } + #js-dropdown-label.dropdown-menu{ data: { icon: 'tag', hint: 'label', tag: '~label' } } %ul{ 'data-dropdown' => true } %li.filter-dropdown-item{ 'data-value' => 'none' } %button.btn.btn-link diff --git a/app/views/snippets/verify.html.haml b/app/views/snippets/verify.html.haml new file mode 100644 index 00000000000..cb623ccab57 --- /dev/null +++ b/app/views/snippets/verify.html.haml @@ -0,0 +1,4 @@ +- form = [@snippet.becomes(Snippet)] + += render 'layouts/recaptcha_verification', spammable: @snippet, form: form + |