diff options
| author | James Lopez <james@jameslopez.es> | 2016-02-01 15:49:03 +0100 |
|---|---|---|
| committer | James Lopez <james@jameslopez.es> | 2016-02-01 15:49:03 +0100 |
| commit | 850942ceba1e9f6918c1234e9b72c261af86de82 (patch) | |
| tree | 1d38d963657cf7fd559431e2e62962f8c95db8ac | |
| parent | 927ab48101d44976e174b13323d085aa5f846155 (diff) | |
| parent | da8e0f86595299740a344309cb5963854b61c4a6 (diff) | |
| download | gitlab-ce-850942ceba1e9f6918c1234e9b72c261af86de82.tar.gz | |
Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into fix/atom-url-issue
57 files changed, 433 insertions, 379 deletions
diff --git a/CHANGELOG b/CHANGELOG index 2e6e65e7ae2..a1ff098cbcb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,9 +15,19 @@ v 8.5.0 (unreleased) - Track project import failure - Fix visibility level text in admin area (Zeger-Jan van de Weg) - Update the ExternalIssue regex pattern (Blake Hitchcock) + - Revert "Add IP check against DNSBLs at account sign-up" - Deprecate API "merge_request/:merge_request_id/comments". Use "merge_requests/:merge_request_id/notes" instead - Deprecate API "merge_request/:merge_request_id/...". Use "merge_requests/:merge_request_id/..." instead - Prevent parse error when name of project ends with .atom and prevent path issues + - Mark inline difference between old and new paths when a file is renamed + +v 8.4.3 + - Increase lfs_objects size column to 8-byte integer to allow files larger + than 2.1GB + - Correctly highlight MR diff when MR has merge conflicts + - Fix highlighting in blame view + - Update sentry-raven gem to prevent "Not a git repository" console output + when running certain commands v 8.4.2 - Bump required gitlab-workhorse version to bring in a fix for missing @@ -28,7 +38,6 @@ v 8.4.2 improvement when checking if a repository was empty - Add instrumentation for Gitlab::Git::Repository instance methods so we can track them in Performance Monitoring. - - Correctly highlight MR diff when MR has merge conflicts - Increase contrast between highlighted code comments and inline diff marker - Fix method undefined when using external commit status in builds - Fix highlighting in blame view. @@ -38,6 +47,8 @@ v 8.4.1 and Nokogiri (1.6.7.2) - Fix redirect loop during import - Fix diff highlighting for all syntax themes + - Warn admin during OAuth of granting admin rights (Zeger-Jan van de Weg) + - Delete project and associations in a background worker v 8.4.0 - Allow LDAP users to change their email if it was not set by the LDAP server @@ -81,7 +92,7 @@ v 8.4.0 - Show 'All' tab by default in the builds page - Add Open Graph and Twitter Card data to all pages - Fix API project lookups when querying with a namespace with dots (Stan Hu) - - Enable forcing Two-Factor authentication sitewide, with optional grace period + - Enable forcing Two-factor authentication sitewide, with optional grace period - Import GitHub Pull Requests into GitLab - Change single user API endpoint to return more detailed data (Michael Potthoff) - Update version check images to use SVG diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1eabbdc5cad..e7659b06c71 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -147,7 +147,7 @@ sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true) sudo gitlab-rake gitlab:env:info) (For installations from source run and paste the output of: -sudo -u git -H bundle exec rake gitlab:env:info) +sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production) ## Possible fixes @@ -255,6 +255,8 @@ For examples of feedback on merge requests please look at already request feel free to mention one of the Merge Marshalls of the [core team][]. Please ensure that your merge request meets the contribution acceptance criteria. +When having your code reviewed and when reviewing merge requests please take the [thoughtbot code review guidelines](https://github.com/thoughtbot/guides/tree/master/code-review) into account. + ## Definition of done If you contribute to GitLab please know that changes involve more than just diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 1ef31c7700e..047df4786a9 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -4,6 +4,7 @@ class @AwardsHandler event.stopPropagation() event.preventDefault() $(".emoji-menu").show() + $("#emoji_search").focus() $("html").on 'click', (event) -> if !$(event.target).closest(".emoji-menu").length diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 00cb756b376..c7f3604850d 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -36,6 +36,20 @@ } } + .filename { + &.old { + span.idiff { + background-color: #f8cbcb; + } + } + + &.new { + span.idiff { + background-color: #a6f3a6; + } + } + } + .left-options { margin-top: -3px; } @@ -158,3 +172,15 @@ } } } + +span.idiff { + &.left { + border-top-left-radius: 2px; + border-bottom-left-radius: 2px; + } + + &.right { + border-top-right-radius: 2px; + border-bottom-right-radius: 2px; + } +} diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 094eef28a43..9943745208e 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -74,8 +74,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :metrics_timeout, :metrics_method_call_threshold, :metrics_sample_interval, - :ip_blocking_enabled, - :dnsbl_servers_list, :recaptcha_enabled, :recaptcha_site_key, :recaptcha_private_key, diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 4a2599dda37..1b9dd568043 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -106,7 +106,7 @@ class Projects::NotesController < Projects::ApplicationController { notes_left: [note], notes_right: [] } else { notes_left: [], notes_right: [note] } - end + end else template = "projects/notes/_diff_notes_with_reply" locals = { notes: [note] } diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 935f7d75c6a..4df5095bd94 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -93,6 +93,10 @@ class ProjectsController < ApplicationController return end + if @project.pending_delete? + flash[:alert] = "Project queued for delete." + end + respond_to do |format| format.html do if @project.repository_exists? @@ -120,8 +124,8 @@ class ProjectsController < ApplicationController def destroy return access_denied! unless can?(current_user, :remove_project, @project) - ::Projects::DestroyService.new(@project, current_user, {}).execute - flash[:alert] = "Project '#{@project.name}' was deleted." + ::Projects::DestroyService.new(@project, current_user, {}).pending_delete! + flash[:alert] = "Project '#{@project.name}' will be deleted." redirect_to dashboard_projects_path rescue Projects::DestroyService::DestroyError => ex diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 5efdd613e79..c48175a4c5a 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -8,11 +8,6 @@ class RegistrationsController < Devise::RegistrationsController def create if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha - if Gitlab::IpCheck.new(request.remote_ip).spam? - flash[:alert] = 'Could not create an account. This IP is listed for spam.' - return render action: 'new' - end - super else flash[:alert] = "There was an error with the reCAPTCHA code below. Please re-enter the code." diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 62971d1e14b..f9bacc8ba45 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -1,4 +1,13 @@ module DiffHelper + def mark_inline_diffs(old_line, new_line) + old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_line, new_line).inline_diffs + + marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs) + marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs) + + [marked_old_line, marked_new_line] + end + def diff_view params[:view] == 'parallel' ? 'parallel' : 'inline' end @@ -55,7 +64,7 @@ module DiffHelper if line.blank? " ".html_safe else - line.html_safe + line end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 2f3487b53ac..59563b8823c 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -43,8 +43,6 @@ # metrics_port :integer default(8089) # sentry_enabled :boolean default(FALSE) # sentry_dsn :string -# ip_blocking_enabled :boolean default(FALSE) -# dns_blacklist_threshold :float default(0.33) # class ApplicationSetting < ActiveRecord::Base diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb index 61119633717..8a0a8a4c2a9 100644 --- a/app/models/broadcast_message.rb +++ b/app/models/broadcast_message.rb @@ -26,7 +26,9 @@ class BroadcastMessage < ActiveRecord::Base default_value_for :font, '#FFFFFF' def self.current - where("ends_at > :now AND starts_at <= :now", now: Time.zone.now).last + Rails.cache.fetch("broadcast_message_current", expires_in: 1.minute) do + where("ends_at > :now AND starts_at <= :now", now: Time.zone.now).last + end end def active? diff --git a/app/models/commit.rb b/app/models/commit.rb index 0ba7b584d91..23b771aebb7 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -68,18 +68,18 @@ class Commit # Pattern used to extract commit references from text # - # The SHA can be between 6 and 40 hex characters. + # The SHA can be between 7 and 40 hex characters. # # This pattern supports cross-project references. def self.reference_pattern %r{ (?:#{Project.reference_pattern}#{reference_prefix})? - (?<commit>\h{6,40}) + (?<commit>\h{7,40}) }x end def self.link_reference_pattern - super("commit", /(?<commit>\h{6,40})/) + super("commit", /(?<commit>\h{7,40})/) end def to_reference(from_project = nil) diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb index 14e7971fa06..289dbc57287 100644 --- a/app/models/commit_range.rb +++ b/app/models/commit_range.rb @@ -32,8 +32,8 @@ class CommitRange PATTERN = /#{REF_PATTERN}\.{2,3}#{REF_PATTERN}/ # In text references, the beginning and ending refs can only be SHAs - # between 6 and 40 hex characters. - STRICT_PATTERN = /\h{6,40}\.{2,3}\h{6,40}/ + # between 7 and 40 hex characters. + STRICT_PATTERN = /\h{7,40}\.{2,3}\h{7,40}/ def self.reference_prefix '@' diff --git a/app/models/project.rb b/app/models/project.rb index 238932f59a7..043f08b9a13 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -36,6 +36,7 @@ # build_coverage_regex :string # build_allow_git_fetch :boolean default(TRUE), not null # build_timeout :integer default(3600), not null +# pending_delete :boolean # require 'carrierwave/orm/activerecord' diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 2a65f0431c4..dbd70dc5a44 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -110,7 +110,7 @@ class WikiPage # Returns boolean True or False if this instance # is an old version of the page. def historical? - @page.historical? + @page.historical? && versions.first.sha != version.sha end # Returns boolean True or False if this instance diff --git a/app/services/delete_user_service.rb b/app/services/delete_user_service.rb index e622fd5ea5d..173e50c9206 100644 --- a/app/services/delete_user_service.rb +++ b/app/services/delete_user_service.rb @@ -13,7 +13,7 @@ class DeleteUserService user.personal_projects.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute + ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete! end user.destroy diff --git a/app/services/destroy_group_service.rb b/app/services/destroy_group_service.rb index d929a676293..9189de390a2 100644 --- a/app/services/destroy_group_service.rb +++ b/app/services/destroy_group_service.rb @@ -9,7 +9,7 @@ class DestroyGroupService @group.projects.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute + ::Projects::DestroyService.new(project, current_user, skip_repo: true).pending_delete! end @group.destroy diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index a8486e6a5a1..8d9661167b5 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -6,27 +6,12 @@ module Notes note.system = false if note.save - notification_service.new_note(note) - - # Skip system notes, like status changes and cross-references and awards - unless note.system || note.is_award - event_service.leave_note(note, note.author) - note.create_cross_references! - execute_hooks(note) - end + # Finish the harder work in the background + NewNoteWorker.perform_in(2.seconds, note.id, params) end note end - def hook_data(note) - Gitlab::NoteDataBuilder.build(note, current_user) - end - - def execute_hooks(note) - note_data = hook_data(note) - note.project.execute_hooks(note_data, :note_hooks) - note.project.execute_services(note_data, :note_hooks) - end end end diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb new file mode 100644 index 00000000000..f37d3c50cdd --- /dev/null +++ b/app/services/notes/post_process_service.rb @@ -0,0 +1,30 @@ +module Notes + class PostProcessService + + attr_accessor :note + + def initialize(note) + @note = note + end + + def execute + # Skip system notes, like status changes and cross-references and awards + unless @note.system || @note.is_award + EventCreateService.new.leave_note(@note, @note.author) + @note.create_cross_references! + execute_note_hooks + end + end + + def hook_data + Gitlab::NoteDataBuilder.build(@note, @note.author) + end + + def execute_note_hooks + note_data = hook_data + @note.project.execute_hooks(note_data, :note_hooks) + @note.project.execute_services(note_data, :note_hooks) + end + + end +end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 28872c89259..294157b4f0e 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -6,6 +6,12 @@ module Projects DELETED_FLAG = '+deleted' + def pending_delete! + project.update_attribute(:pending_delete, true) + + ProjectDestroyWorker.perform_in(1.minute, project.id, current_user.id, params) + end + def execute return false unless can?(current_user, :remove_project, project) diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index baadca09518..b0f1a34cbec 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -105,14 +105,14 @@ = f.check_box :signin_enabled Sign-in enabled .form-group - = f.label :two_factor_authentication, 'Two-Factor authentication', class: 'control-label col-sm-2' + = f.label :two_factor_authentication, 'Two-factor authentication', class: 'control-label col-sm-2' .col-sm-10 .checkbox = f.label :require_two_factor_authentication do = f.check_box :require_two_factor_authentication - Require all users to setup Two-Factor authentication + Require all users to setup Two-factor authentication .form-group - = f.label :two_factor_authentication, 'Two-Factor grace period (hours)', class: 'control-label col-sm-2' + = f.label :two_factor_authentication, 'Two-factor grace period (hours)', class: 'control-label col-sm-2' .col-sm-10 = f.number_field :two_factor_grace_period, min: 0, class: 'form-control', placeholder: '0' .help-block Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication @@ -215,22 +215,6 @@ .form-group .col-sm-offset-2.col-sm-10 .checkbox - = f.label :ip_blocking_enabled do - = f.check_box :ip_blocking_enabled - Enable IP check against blacklist at sign-up - .help-block Helps preventing accounts creation from 'known spam sources' - - .form-group - = f.label :dnsbl_servers_list, class: 'control-label col-sm-2' do - DNSBL servers list - .col-sm-10 - = f.text_field :dnsbl_servers_list, class: 'form-control' - .help-block - Please enter DNSBL servers separated with comma - - .form-group - .col-sm-offset-2.col-sm-10 - .checkbox = f.label :recaptcha_enabled do = f.check_box :recaptcha_enabled Enable reCAPTCHA diff --git a/app/views/doorkeeper/authorizations/new.html.haml b/app/views/doorkeeper/authorizations/new.html.haml index 15f9ee266c1..eae80e5210f 100644 --- a/app/views/doorkeeper/authorizations/new.html.haml +++ b/app/views/doorkeeper/authorizations/new.html.haml @@ -4,6 +4,15 @@ Authorize %strong.text-info= @pre_auth.client.name to use your account? + + - if current_user.admin? + .text-warning.prepend-top-20 + %p + = icon("exclamation-triangle fw") + You are an admin, which means granting access to + %strong #{@pre_auth.client.name} + will allow them to interact with GitLab as an admin as well. Proceed with caution. + - if @pre_auth.scopes #oauth-permissions %p This application will be able to: @@ -25,4 +34,4 @@ = hidden_field_tag :state, @pre_auth.state = hidden_field_tag :response_type, @pre_auth.response_type = hidden_field_tag :scope, @pre_auth.scope - = submit_tag "Deny", class: "btn btn-danger prepend-left-10"
\ No newline at end of file + = submit_tag "Deny", class: "btn btn-danger prepend-left-10" diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml index 3dd2595f1ad..f2e405b14fd 100644 --- a/app/views/notify/repository_push_email.html.haml +++ b/app/views/notify/repository_push_email.html.haml @@ -18,7 +18,7 @@ %div %span by #{commit.author_name} %i at #{commit.committed_date.to_s(:iso8601)} - %pre.commit-message + %pre.commit-message = commit.safe_message %h4 #{pluralize @message.diffs_count, "changed file"}: diff --git a/app/views/profiles/two_factor_auths/new.html.haml b/app/views/profiles/two_factor_auths/new.html.haml index 1a5b6efce35..b2830aa0834 100644 --- a/app/views/profiles/two_factor_auths/new.html.haml +++ b/app/views/profiles/two_factor_auths/new.html.haml @@ -1,6 +1,6 @@ - page_title 'Two-factor Authentication', 'Account' -%h2.page-title Two-Factor Authentication (2FA) +%h2.page-title Two-factor Authentication (2FA) %p Download the Google Authenticator application from App Store for iOS or Google Play for Android and scan this code. diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index fc0eaef2286..3ac058a3bf8 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -7,16 +7,20 @@ = submodule_link(blob, @commit.id, project.repository) - else = blob_icon blob.mode, blob.name - = link_to "#diff-#{i}" do - %strong - = diff_file.new_path - - if diff_file.deleted_file - deleted - - elsif diff_file.renamed_file - renamed from - %strong - = diff_file.old_path + = link_to "#diff-#{i}" do + - if diff_file.renamed_file + - old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path) + %strong.filename.old + = old_path + → + %strong.filename.new + = new_path + - else + %strong + = diff_file.new_path + - if diff_file.deleted_file + deleted - if diff_file.mode_changed? %small diff --git a/app/views/votes/_votes_block.html.haml b/app/views/votes/_votes_block.html.haml index b1f8645eea0..91c5b7eac5e 100644 --- a/app/views/votes/_votes_block.html.haml +++ b/app/views/votes/_votes_block.html.haml @@ -7,7 +7,7 @@ - if current_user .awards-controls - %a.add-award{"data-toggle" => "dropdown", "data-target" => "#", "href" => "#"} + %a.add-award{"href" => "#"} = icon('smile-o') .emoji-menu .emoji-menu-content diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb new file mode 100644 index 00000000000..1b3232cd365 --- /dev/null +++ b/app/workers/new_note_worker.rb @@ -0,0 +1,12 @@ +class NewNoteWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform(note_id, note_params) + note = Note.find(note_id) + + NotificationService.new.new_note(note) + Notes::PostProcessService.new(note).execute + end +end diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb new file mode 100644 index 00000000000..d06e4480292 --- /dev/null +++ b/app/workers/project_destroy_worker.rb @@ -0,0 +1,17 @@ +class ProjectDestroyWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform(project_id, user_id, params) + begin + project = Project.find(project_id) + rescue ActiveRecord::RecordNotFound + return + end + + user = User.find(user_id) + + ::Projects::DestroyService.new(project, user, params).execute + end +end diff --git a/config/routes.rb b/config/routes.rb index fdfdb449085..54cc338b605 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -490,7 +490,7 @@ Rails.application.routes.draw do end resource :avatar, only: [:show, :destroy] - resources :commit, only: [:show], constraints: { id: /[[:alnum:]]{6,40}/ } do + resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do member do get :branches get :builds diff --git a/db/migrate/20160122185421_add_pending_delete_to_project.rb b/db/migrate/20160122185421_add_pending_delete_to_project.rb new file mode 100644 index 00000000000..046a5d8fc32 --- /dev/null +++ b/db/migrate/20160122185421_add_pending_delete_to_project.rb @@ -0,0 +1,5 @@ +class AddPendingDeleteToProject < ActiveRecord::Migration + def change + add_column :projects, :pending_delete, :boolean, default: false + end +end diff --git a/db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb b/db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb new file mode 100644 index 00000000000..41821cdcc42 --- /dev/null +++ b/db/migrate/20160128212447_remove_ip_blocking_settings_from_application_settings.rb @@ -0,0 +1,6 @@ +class RemoveIpBlockingSettingsFromApplicationSettings < ActiveRecord::Migration + def change + remove_column :application_settings, :ip_blocking_enabled, :boolean, default: false + remove_column :application_settings, :dnsbl_servers_list, :text + end +end diff --git a/db/migrate/20160128233227_change_lfs_objects_size_column.rb b/db/migrate/20160128233227_change_lfs_objects_size_column.rb new file mode 100644 index 00000000000..e7fd1f71777 --- /dev/null +++ b/db/migrate/20160128233227_change_lfs_objects_size_column.rb @@ -0,0 +1,5 @@ +class ChangeLfsObjectsSizeColumn < ActiveRecord::Migration + def change + change_column :lfs_objects, :size, :integer, limit: 8 + end +end diff --git a/db/schema.rb b/db/schema.rb index 97594011a02..2ad2c23fba5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160120172143) do +ActiveRecord::Schema.define(version: 20160128233227) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -64,8 +64,6 @@ ActiveRecord::Schema.define(version: 20160120172143) do t.integer "metrics_sample_interval", default: 15 t.boolean "sentry_enabled", default: false t.string "sentry_dsn" - t.boolean "ip_blocking_enabled", default: false - t.text "dnsbl_servers_list" end create_table "audit_events", force: :cascade do |t| @@ -447,8 +445,8 @@ ActiveRecord::Schema.define(version: 20160120172143) do add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree create_table "lfs_objects", force: :cascade do |t| - t.string "oid", null: false - t.integer "size", null: false + t.string "oid", null: false + t.integer "size", limit: 8, null: false t.datetime "created_at" t.datetime "updated_at" t.string "file" @@ -679,6 +677,7 @@ ActiveRecord::Schema.define(version: 20160120172143) do t.string "build_coverage_regex" t.boolean "build_allow_git_fetch", default: true, null: false t.integer "build_timeout", default: 3600, null: false + t.boolean "pending_delete", default: false end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree diff --git a/doc/api/builds.md b/doc/api/builds.md index ecb50754c88..6e64d096644 100644 --- a/doc/api/builds.md +++ b/doc/api/builds.md @@ -18,7 +18,7 @@ GET /projects/:id/builds ### Example of request ``` -curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds" +curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds" ``` ### Example of response @@ -123,7 +123,7 @@ GET /projects/:id/repository/commits/:sha/builds ### Example of request ``` -curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/repository/commits/0ff3ae198f8601a285adcf5c0fff204ee6fba5fd/builds" +curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/repository/commits/0ff3ae198f8601a285adcf5c0fff204ee6fba5fd/builds" ``` ### Example of response @@ -213,7 +213,7 @@ GET /projects/:id/builds/:build_id ### Example of request ``` -curl -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/8" +curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/8" ``` ### Example of response @@ -277,7 +277,7 @@ POST /projects/:id/builds/:build_id/cancel ### Example of request ``` -curl -X POST -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/cancel" +curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/cancel" ``` ### Example of response @@ -327,7 +327,7 @@ POST /projects/:id/builds/:build_id/retry ### Example of request ``` -curl -X POST -H "PRIVATE_TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/retry" +curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/retry" ``` ### Example of response diff --git a/doc/security/README.md b/doc/security/README.md index f34c792d005..be1abb88c3d 100644 --- a/doc/security/README.md +++ b/doc/security/README.md @@ -7,4 +7,4 @@ - [Reset your root password](reset_root_password.md) - [User File Uploads](user_file_uploads.md) - [How we manage the CRIME vulnerability](crime_vulnerability.md) -- [Enforce Two-Factor authentication](two_factor_authentication.md) +- [Enforce Two-factor authentication](two_factor_authentication.md) diff --git a/doc/workflow/gitlab_flow.md b/doc/workflow/gitlab_flow.md index 8965e5b3654..be32f0c720b 100644 --- a/doc/workflow/gitlab_flow.md +++ b/doc/workflow/gitlab_flow.md @@ -152,9 +152,10 @@ The name of this branch should start with the issue number, for example '15-requ When you are done or want to discuss the code you open a merge request. This is an online place to discuss the change and review the code. -Creating a branch is a manual action since you do not always want to merge a new branch you push, it could be a long-running environment or release branch. -If you create the merge request but do not assign it to anyone it is a 'work-in-process' merge request. +Opening a merge request is a manual action since you do not always want to merge a new branch you push, it could be a long-running environment or release branch. +If you open the merge request but do not assign it to anyone it is a 'Work In Progress' merge request. These are used to discuss the proposed implementation but are not ready for inclusion in the master branch yet. +_Pro tip:_ Start the title of the merge request with `[WIP]` or `WIP:` to prevent it from being merged before it's ready. When the author thinks the code is ready the merge request is assigned to reviewer. The reviewer presses the merge button when they think the code is ready for inclusion in the master branch. @@ -185,7 +186,7 @@ If you have an issue that spans across multiple repositories, the best thing is  -With git you can use an interactive rebase (rebase -i) to squash multiple commits into one and reorder them. +With git you can use an interactive rebase (`rebase -i`) to squash multiple commits into one and reorder them. This functionality is useful if you made a couple of commits for small changes during development and want to replace them with a single commit or if you want to make the order more logical. However you should never rebase commits you have pushed to a remote server. Somebody can have referred to the commits or cherry-picked them. diff --git a/features/project/issues/award_emoji.feature b/features/project/issues/award_emoji.feature index 9a06fdc2ee6..bfde89fd896 100644 --- a/features/project/issues/award_emoji.feature +++ b/features/project/issues/award_emoji.feature @@ -9,6 +9,7 @@ Feature: Award Emoji @javascript Scenario: I add and remove award in the issue Given I click to emoji-picker + Then The search field is focused And I click to emoji in the picker Then I have award added And I can remove it by clicking to icon @@ -16,11 +17,13 @@ Feature: Award Emoji @javascript Scenario: I can see the list of emoji categories Given I click to emoji-picker + Then The search field is focused Then I can see the activity and food categories @javascript Scenario: I can search emoji Given I click to emoji-picker + Then The search field is focused And I search "hand" Then I see search result for "hand" diff --git a/features/steps/project/issues/award_emoji.rb b/features/steps/project/issues/award_emoji.rb index 2c2ed08655e..69695d493f3 100644 --- a/features/steps/project/issues/award_emoji.rb +++ b/features/steps/project/issues/award_emoji.rb @@ -66,4 +66,8 @@ class Spinach::Features::AwardEmoji < Spinach::FeatureSteps expect(page).to have_selector '[data-emoji="raised_hand"]' end end + + step 'The search field is focused' do + page.evaluate_script("document.activeElement.id").should eq "emoji_search" + end end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 71bb342f844..1f991e600e3 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -187,7 +187,7 @@ module API else present @forked_project, with: Entities::Project, user_can_admin_project: can?(current_user, :admin_project, @forked_project) - end + end end # Update an existing project @@ -246,7 +246,7 @@ module API # DELETE /projects/:id delete ":id" do authorize! :remove_project, user_project - ::Projects::DestroyService.new(user_project, current_user, {}).execute + ::Projects::DestroyService.new(user_project, current_user, {}).pending_delete! end # Mark this project as forked from another diff --git a/lib/dnsxl_check.rb b/lib/dnsxl_check.rb deleted file mode 100644 index 1e506b2d9cb..00000000000 --- a/lib/dnsxl_check.rb +++ /dev/null @@ -1,105 +0,0 @@ -require 'resolv' - -class DNSXLCheck - - class Resolver - def self.search(query) - begin - Resolv.getaddress(query) - true - rescue Resolv::ResolvError - false - end - end - end - - IP_REGEXP = /\A(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\z/ - DEFAULT_THRESHOLD = 0.33 - - def self.create_from_list(list) - dnsxl_check = DNSXLCheck.new - - list.each do |entry| - dnsxl_check.add_list(entry.domain, entry.weight) - end - - dnsxl_check - end - - def test(ip) - if use_threshold? - test_with_threshold(ip) - else - test_strict(ip) - end - end - - def test_with_threshold(ip) - return false if lists.empty? - - search(ip) - final_score >= threshold - end - - def test_strict(ip) - return false if lists.empty? - - search(ip) - @score > 0 - end - - def use_threshold=(value) - @use_threshold = value == true - end - - def use_threshold? - @use_threshold &&= true - end - - def threshold=(threshold) - raise ArgumentError, "'threshold' value must be grather than 0 and less than or equal to 1" unless threshold > 0 && threshold <= 1 - @threshold = threshold - end - - def threshold - @threshold ||= DEFAULT_THRESHOLD - end - - def add_list(domain, weight) - @lists ||= [] - @lists << { domain: domain, weight: weight } - end - - def lists - @lists ||= [] - end - - private - - def search(ip) - raise ArgumentError, "'ip' value must be in #{IP_REGEXP} format" unless ip.match(IP_REGEXP) - - @score = 0 - - reversed = reverse_ip(ip) - search_in_rbls(reversed) - end - - def reverse_ip(ip) - ip.split('.').reverse.join('.') - end - - def search_in_rbls(reversed_ip) - lists.each do |rbl| - query = "#{reversed_ip}.#{rbl[:domain]}" - @score += rbl[:weight] if Resolver.search(query) - end - end - - def final_score - weights = lists.map{ |rbl| rbl[:weight] }.reduce(:+).to_i - return 0 if weights == 0 - - (@score.to_f / weights.to_f).round(2) - end -end diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb index a7f925ce134..9429b3ff88d 100644 --- a/lib/gitlab/diff/highlight.rb +++ b/lib/gitlab/diff/highlight.rb @@ -21,13 +21,13 @@ module Gitlab # ignore highlighting for "match" lines next diff_line if diff_line.type == 'match' || diff_line.type == 'nonewline' - rich_line = highlight_line(diff_line, i) + rich_line = highlight_line(diff_line) || diff_line.text if line_inline_diffs = inline_diffs[i] rich_line = InlineDiffMarker.new(diff_line.text, rich_line).mark(line_inline_diffs) end - diff_line.text = rich_line.html_safe + diff_line.text = rich_line diff_line end @@ -35,8 +35,8 @@ module Gitlab private - def highlight_line(diff_line, index) - return html_escape(diff_line.text) unless diff_file && diff_file.diff_refs + def highlight_line(diff_line) + return unless diff_file && diff_file.diff_refs line_prefix = diff_line.text.match(/\A(.)/) ? $1 : ' ' @@ -49,11 +49,11 @@ module Gitlab # Only update text if line is found. This will prevent # issues with submodules given the line only exists in diff content. - rich_line ? line_prefix + rich_line : html_escape(diff_line.text) + "#{line_prefix}#{rich_line}".html_safe if rich_line end def inline_diffs - @inline_diffs ||= InlineDiff.new(@raw_lines).inline_diffs + @inline_diffs ||= InlineDiff.for_lines(@raw_lines) end def old_lines @@ -72,11 +72,6 @@ module Gitlab [ref.project.repository, ref.id, path] end - - def html_escape(str) - replacements = { '&' => '&', '>' => '>', '<' => '<', '"' => '"', "'" => ''' } - str.gsub(/[&"'><]/, replacements) - end end end end diff --git a/lib/gitlab/diff/inline_diff.rb b/lib/gitlab/diff/inline_diff.rb index b8a61ad6115..789c14518b0 100644 --- a/lib/gitlab/diff/inline_diff.rb +++ b/lib/gitlab/diff/inline_diff.rb @@ -1,43 +1,58 @@ module Gitlab module Diff class InlineDiff - attr_accessor :lines + attr_accessor :old_line, :new_line, :offset - def initialize(lines) - @lines = lines - end + def self.for_lines(lines) + local_edit_indexes = self.find_local_edits(lines) - def inline_diffs inline_diffs = [] local_edit_indexes.each do |index| old_index = index new_index = index + 1 - old_line = @lines[old_index] - new_line = @lines[new_index] - - # Skip inline diff if empty line was replaced with content - next if old_line[1..-1] == "" - - # Add one, because this is based on the prefixless version - lcp = longest_common_prefix(old_line[1..-1], new_line[1..-1]) + 1 - lcs = longest_common_suffix(old_line[lcp..-1], new_line[lcp..-1]) + old_line = lines[old_index] + new_line = lines[new_index] - old_diff_range = lcp..(old_line.length - lcs - 1) - new_diff_range = lcp..(new_line.length - lcs - 1) + old_diffs, new_diffs = new(old_line, new_line, offset: 1).inline_diffs - inline_diffs[old_index] = [old_diff_range] if old_diff_range.begin <= old_diff_range.end - inline_diffs[new_index] = [new_diff_range] if new_diff_range.begin <= new_diff_range.end + inline_diffs[old_index] = old_diffs + inline_diffs[new_index] = new_diffs end inline_diffs end + def initialize(old_line, new_line, offset: 0) + @old_line = old_line[offset..-1] + @new_line = new_line[offset..-1] + @offset = offset + end + + def inline_diffs + # Skip inline diff if empty line was replaced with content + return if old_line == "" + + lcp = longest_common_prefix(old_line, new_line) + lcs = longest_common_suffix(old_line[lcp..-1], new_line[lcp..-1]) + + lcp += offset + old_length = old_line.length + offset + new_length = new_line.length + offset + + old_diff_range = lcp..(old_length - lcs - 1) + new_diff_range = lcp..(new_length - lcs - 1) + + old_diffs = [old_diff_range] if old_diff_range.begin <= old_diff_range.end + new_diffs = [new_diff_range] if new_diff_range.begin <= new_diff_range.end + + [old_diffs, new_diffs] + end + private - # Find runs of single line edits - def local_edit_indexes - line_prefixes = @lines.map { |line| line.match(/\A([+-])/) ? $1 : ' ' } + def self.find_local_edits(lines) + line_prefixes = lines.map { |line| line.match(/\A([+-])/) ? $1 : ' ' } joined_line_prefixes = " #{line_prefixes.join} " offset = 0 diff --git a/lib/gitlab/diff/inline_diff_marker.rb b/lib/gitlab/diff/inline_diff_marker.rb index 1d7fa1bce06..dccb717e95d 100644 --- a/lib/gitlab/diff/inline_diff_marker.rb +++ b/lib/gitlab/diff/inline_diff_marker.rb @@ -5,10 +5,12 @@ module Gitlab def initialize(raw_line, rich_line = raw_line) @raw_line = raw_line - @rich_line = rich_line + @rich_line = ERB::Util.html_escape(rich_line) end def mark(line_inline_diffs) + return rich_line unless line_inline_diffs + marker_ranges = [] line_inline_diffs.each do |inline_diff_range| # Map the inline-diff range based on the raw line to character positions in the rich line @@ -19,11 +21,15 @@ module Gitlab offset = 0 # Mark each range - marker_ranges.each do |range| - offset = insert_around_range(rich_line, range, "<span class='idiff'>", "</span>", offset) + marker_ranges.each_with_index do |range, i| + class_names = ["idiff"] + class_names << "left" if i == 0 + class_names << "right" if i == marker_ranges.length - 1 + + offset = insert_around_range(rich_line, range, "<span class='#{class_names.join(" ")}'>", "</span>", offset) end - rich_line + rich_line.html_safe end private diff --git a/lib/gitlab/ip_check.rb b/lib/gitlab/ip_check.rb deleted file mode 100644 index f2e9b50d225..00000000000 --- a/lib/gitlab/ip_check.rb +++ /dev/null @@ -1,34 +0,0 @@ -module Gitlab - class IpCheck - - def initialize(ip) - @ip = ip - - application_settings = ApplicationSetting.current - @ip_blocking_enabled = application_settings.ip_blocking_enabled - @dnsbl_servers_list = application_settings.dnsbl_servers_list - end - - def spam? - @ip_blocking_enabled && blacklisted? - end - - private - - def blacklisted? - on_dns_blacklist? - end - - def on_dns_blacklist? - dnsbl_check = DNSXLCheck.new - prepare_dnsbl_list(dnsbl_check) - dnsbl_check.test(@ip) - end - - def prepare_dnsbl_list(dnsbl_check) - @dnsbl_servers_list.split(',').map(&:strip).reject(&:empty?).each do |domain| - dnsbl_check.add_list(domain, 1) - end - end - end -end diff --git a/spec/fixtures/parallel_diff_result.yml b/spec/fixtures/parallel_diff_result.yml index a326b651aad..a8b7907d4ba 100644 --- a/spec/fixtures/parallel_diff_result.yml +++ b/spec/fixtures/parallel_diff_result.yml @@ -55,7 +55,7 @@ :type: new :number: 9 :text: | - +<span id="LC9" class="line"> <span class="k">raise</span> <span class="no"><span class='idiff'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff'> </span><span class="s2">"System commands must be given as an array of strings"</span></span> + +<span id="LC9" class="line"> <span class="k">raise</span> <span class="no"><span class='idiff left'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff right'> </span><span class="s2">"System commands must be given as an array of strings"</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9 - :left: :type: diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 955d2852cfd..14986a74c2e 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -104,8 +104,7 @@ describe DiffHelper do end end - describe 'diff_line_content' do - + describe '#diff_line_content' do it 'should return non breaking space when line is empty' do expect(diff_line_content(nil)).to eq(' ') end @@ -116,9 +115,19 @@ describe DiffHelper do expect(diff_line_content(diff_file.diff_lines.first.type)).to eq('match') expect(diff_file.diff_lines.first.new_pos).to eq(6) end + end + + describe "#mark_inline_diffs" do + let(:old_line) { %{abc 'def'} } + let(:new_line) { %{abc "def"} } + + it "returns strings with marked inline diffs" do + marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line) - it 'should return safe HTML' do - expect(diff_line_content(diff_file.diff_lines.first.text)).to be_html_safe + expect(marked_old_line).to eq("abc <span class='idiff left right'>'def'</span>") + expect(marked_old_line).to be_html_safe + expect(marked_new_line).to eq("abc <span class='idiff left right'>"def"</span>") + expect(marked_new_line).to be_html_safe end end end diff --git a/spec/lib/banzai/filter/commit_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_reference_filter_spec.rb index 473534ba68a..63a32d9d455 100644 --- a/spec/lib/banzai/filter/commit_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_reference_filter_spec.rb @@ -21,7 +21,7 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do let(:reference) { commit.id } # Let's test a variety of commit SHA sizes just to be paranoid - [6, 8, 12, 18, 20, 32, 40].each do |size| + [7, 8, 12, 18, 20, 32, 40].each do |size| it "links to a valid reference of #{size} characters" do doc = reference_filter("See #{reference[0...size]}") @@ -35,7 +35,7 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do doc = reference_filter("See #{commit.id}") expect(doc.text).to eq "See #{commit.short_id}" - doc = reference_filter("See #{commit.id[0...6]}") + doc = reference_filter("See #{commit.id[0...7]}") expect(doc.text).to eq "See #{commit.short_id}" end diff --git a/spec/lib/dnsxl_check_spec.rb b/spec/lib/dnsxl_check_spec.rb deleted file mode 100644 index a35a1be0c90..00000000000 --- a/spec/lib/dnsxl_check_spec.rb +++ /dev/null @@ -1,68 +0,0 @@ -require 'spec_helper' -require 'ostruct' - -describe 'DNSXLCheck', lib: true, no_db: true do - let(:spam_ip) { '127.0.0.2' } - let(:no_spam_ip) { '127.0.0.3' } - let(:invalid_ip) { 'a.b.c.d' } - let!(:dnsxl_check) { DNSXLCheck.create_from_list([OpenStruct.new({ domain: 'test', weight: 1 })]) } - - before(:context) do - class DNSXLCheck::Resolver - class << self - alias_method :old_search, :search - def search(query) - return false if query.match(/always\.failing\.domain\z/) - return true if query.match(/\A2\.0\.0\.127\./) - return false if query.match(/\A3\.0\.0\.127\./) - end - end - end - end - - describe '#test' do - before do - dnsxl_check.threshold = 0.75 - dnsxl_check.add_list('always.failing.domain', 1) - end - - context 'when threshold is used' do - before { dnsxl_check.use_threshold= true } - - it { expect(dnsxl_check.test(spam_ip)).to be_falsey } - end - - context 'when threshold is not used' do - before { dnsxl_check.use_threshold= false } - - it { expect(dnsxl_check.test(spam_ip)).to be_truthy } - end - end - - describe '#test_with_threshold' do - it { expect{ dnsxl_check.test_with_threshold(invalid_ip) }.to raise_error(ArgumentError) } - - it { expect(dnsxl_check.test_with_threshold(spam_ip)).to be_truthy } - it { expect(dnsxl_check.test_with_threshold(no_spam_ip)).to be_falsey } - end - - describe '#test_strict' do - before do - dnsxl_check.threshold = 1 - dnsxl_check.add_list('always.failing.domain', 1) - end - - it { expect{ dnsxl_check.test_strict(invalid_ip) }.to raise_error(ArgumentError) } - - it { expect(dnsxl_check.test_with_threshold(spam_ip)).to be_falsey } - it { expect(dnsxl_check.test_with_threshold(no_spam_ip)).to be_falsey } - it { expect(dnsxl_check.test_strict(spam_ip)).to be_truthy } - it { expect(dnsxl_check.test_strict(no_spam_ip)).to be_falsey } - end - - describe '#threshold=' do - it { expect{ dnsxl_check.threshold = 0 }.to raise_error(ArgumentError) } - it { expect{ dnsxl_check.threshold = 1.1 }.to raise_error(ArgumentError) } - it { expect{ dnsxl_check.threshold = 0.5 }.not_to raise_error } - end -end diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb index b84a57f357a..d19bf4ac84b 100644 --- a/spec/lib/gitlab/diff/highlight_spec.rb +++ b/spec/lib/gitlab/diff/highlight_spec.rb @@ -9,33 +9,69 @@ describe Gitlab::Diff::Highlight, lib: true do let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) } describe '#highlight' do - let(:diff_lines) { Gitlab::Diff::Highlight.new(diff_file).highlight } + context "with a diff file" do + let(:subject) { Gitlab::Diff::Highlight.new(diff_file).highlight } - it 'should return Gitlab::Diff::Line elements' do - expect(diff_lines.first).to be_an_instance_of(Gitlab::Diff::Line) - end + it 'should return Gitlab::Diff::Line elements' do + expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line) + end - it 'should not modify "match" lines' do - expect(diff_lines[0].text).to eq('@@ -6,12 +6,18 @@ module Popen') - expect(diff_lines[22].text).to eq('@@ -19,6 +25,7 @@ module Popen') - end + it 'should not modify "match" lines' do + expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen') + expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen') + end - it 'should highlight unchanged lines' do - code = %Q{ <span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>\n} + it 'highlights and marks unchanged lines' do + code = %Q{ <span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>\n} - expect(diff_lines[2].text).to eq(code) - end + expect(subject[2].text).to eq(code) + end - it 'should highlight removed lines' do - code = %Q{-<span id="LC9" class="line"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span>\n} + it 'highlights and marks removed lines' do + code = %Q{-<span id="LC9" class="line"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span>\n} - expect(diff_lines[4].text).to eq(code) + expect(subject[4].text).to eq(code) + end + + it 'highlights and marks added lines' do + code = %Q{+<span id="LC9" class="line"> <span class="k">raise</span> <span class="no"><span class='idiff left'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff right'> </span><span class="s2">"System commands must be given as an array of strings"</span></span>\n} + + expect(subject[5].text).to eq(code) + end end - it 'should highlight added lines' do - code = %Q{+<span id="LC9" class="line"> <span class="k">raise</span> <span class="no"><span class='idiff'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff'> </span><span class="s2">"System commands must be given as an array of strings"</span></span>\n} + context "with diff lines" do + let(:subject) { Gitlab::Diff::Highlight.new(diff_file.diff_lines).highlight } + + it 'should return Gitlab::Diff::Line elements' do + expect(subject.first).to be_an_instance_of(Gitlab::Diff::Line) + end + + it 'should not modify "match" lines' do + expect(subject[0].text).to eq('@@ -6,12 +6,18 @@ module Popen') + expect(subject[22].text).to eq('@@ -19,6 +25,7 @@ module Popen') + end + + it 'marks unchanged lines' do + code = %Q{ def popen(cmd, path=nil)} + + expect(subject[2].text).to eq(code) + expect(subject[2].text).not_to be_html_safe + end + + it 'marks removed lines' do + code = %Q{- raise "System commands must be given as an array of strings"} + + expect(subject[4].text).to eq(code) + expect(subject[4].text).not_to be_html_safe + end + + it 'marks added lines' do + code = %Q{+ raise <span class='idiff left right'>RuntimeError, </span>"System commands must be given as an array of strings"} - expect(diff_lines[5].text).to eq(code) + expect(subject[5].text).to eq(code) + expect(subject[5].text).to be_html_safe + end end end end diff --git a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb index 6f3276a8b53..ea5c31011f0 100644 --- a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb @@ -2,14 +2,28 @@ require 'spec_helper' describe Gitlab::Diff::InlineDiffMarker, lib: true do describe '#inline_diffs' do - let(:raw) { "abc 'def'" } - let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">'def'</span>} } - let(:inline_diffs) { [2..5] } - let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw, rich).mark(inline_diffs) } + context "when the rich text is html safe" do + let(:raw) { "abc 'def'" } + let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">'def'</span>}.html_safe } + let(:inline_diffs) { [2..5] } + let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw, rich).mark(inline_diffs) } - it 'marks the inline diffs' do - expect(subject).to eq(%{<span class="abc">ab<span class='idiff'>c</span></span><span class="space"><span class='idiff'> </span></span><span class="def"><span class='idiff'>'d</span>ef'</span>}) + it 'marks the inline diffs' do + expect(subject).to eq(%{<span class="abc">ab<span class='idiff left'>c</span></span><span class="space"><span class='idiff'> </span></span><span class="def"><span class='idiff right'>'d</span>ef'</span>}) + expect(subject).to be_html_safe + end + end + + context "when the text text is not html safe" do + let(:raw) { "abc 'def'" } + let(:inline_diffs) { [2..5] } + let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw).mark(inline_diffs) } + + it 'marks the inline diffs' do + expect(subject).to eq(%{ab<span class='idiff left right'>c 'd</span>ef'}) + expect(subject).to be_html_safe + end end end end diff --git a/spec/lib/gitlab/diff/inline_diff_spec.rb b/spec/lib/gitlab/diff/inline_diff_spec.rb index 056917df893..95a993d26cf 100644 --- a/spec/lib/gitlab/diff/inline_diff_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Diff::InlineDiff, lib: true do - describe '#inline_diffs' do + describe '.for_lines' do let(:diff) do <<eos class Test @@ -13,7 +13,7 @@ describe Gitlab::Diff::InlineDiff, lib: true do eos end - let(:subject) { Gitlab::Diff::InlineDiff.new(diff.lines).inline_diffs } + let(:subject) { described_class.for_lines(diff.lines) } it 'finds all inline diffs' do expect(subject[0]).to be_nil @@ -24,4 +24,17 @@ eos expect(subject[5]).to be_nil end end + + describe "#inline_diffs" do + let(:old_line) { "XXX def initialize(test = true)" } + let(:new_line) { "YYY def initialize(test = false)" } + let(:subject) { described_class.new(old_line, new_line, offset: 3).inline_diffs } + + it "finds the inline diff" do + old_diffs, new_diffs = subject + + expect(old_diffs).to eq([26..28]) + expect(new_diffs).to eq([26..29]) + end + end end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 138b87a9a06..fd1513cab1b 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -36,7 +36,7 @@ describe SystemHook, models: true do it "project_destroy hook" do user = create(:user) project = create(:empty_project, namespace: user.namespace) - Projects::DestroyService.new(project, user, {}).execute + Projects::DestroyService.new(project, user, {}).pending_delete! expect(WebMock).to have_requested(:post, @system_hook.url).with( body: /project_destroy/, headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } @@ -65,7 +65,7 @@ describe SystemHook, models: true do project = create(:project) project.team << [user, :master] expect(WebMock).to have_requested(:post, @system_hook.url).with( - body: /user_add_to_team/, + body: /user_add_to_team/, headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } ).once end @@ -76,7 +76,7 @@ describe SystemHook, models: true do project.team << [user, :master] project.project_members.destroy_all expect(WebMock).to have_requested(:post, @system_hook.url).with( - body: /user_remove_from_team/, + body: /user_remove_from_team/, headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook' } ).once end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index c1b03838aa9..ddc49495eda 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -189,6 +189,38 @@ describe WikiPage, models: true do end end + describe '#historical?' do + before do + create_page('Update', 'content') + @page = wiki.find_page('Update') + 3.times { |i| @page.update("content #{i}") } + end + + after do + destroy_page('Update') + end + + it 'returns true when requesting an old version' do + old_version = @page.versions.last.to_s + old_page = wiki.find_page('Update', old_version) + + expect(old_page.historical?).to eq true + end + + it 'returns false when requesting latest version' do + latest_version = @page.versions.first.to_s + latest_page = wiki.find_page('Update', latest_version) + + expect(latest_page.historical?).to eq false + end + + it 'returns false when version is nil' do + latest_page = wiki.find_page('Update', nil) + + expect(latest_page.historical?).to eq false + end + end + private def remove_temp_repo(path) diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 22937226fce..538f44e4f3f 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -321,12 +321,12 @@ describe Projects::HooksController, 'routing' do end end -# project_commit GET /:project_id/commit/:id(.:format) commit#show {id: /[[:alnum:]]{6,40}/, project_id: /[^\/]+/} +# project_commit GET /:project_id/commit/:id(.:format) commit#show {id: /\h{7,40}/, project_id: /[^\/]+/} describe Projects::CommitController, 'routing' do it 'to #show' do - expect(get('/gitlab/gitlabhq/commit/4246fb')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb') - expect(get('/gitlab/gitlabhq/commit/4246fb.diff')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb', format: 'diff') - expect(get('/gitlab/gitlabhq/commit/4246fb.patch')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb', format: 'patch') + expect(get('/gitlab/gitlabhq/commit/4246fbd')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd') + expect(get('/gitlab/gitlabhq/commit/4246fbd.diff')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'diff') + expect(get('/gitlab/gitlabhq/commit/4246fbd.patch')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'patch') expect(get('/gitlab/gitlabhq/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5') end end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index a797a2fe4aa..ff23f13e1cb 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -14,9 +14,7 @@ describe Notes::CreateService, services: true do noteable_type: 'Issue', noteable_id: issue.id } - - expect(project).to receive(:execute_hooks) - expect(project).to receive(:execute_services) + @note = Notes::CreateService.new(project, user, opts).execute end diff --git a/spec/services/notes/post_process_service_spec.rb b/spec/services/notes/post_process_service_spec.rb new file mode 100644 index 00000000000..1a3f339bd64 --- /dev/null +++ b/spec/services/notes/post_process_service_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Notes::PostProcessService, services: true do + let(:project) { create(:empty_project) } + let(:issue) { create(:issue, project: project) } + let(:user) { create(:user) } + + describe :execute do + before do + project.team << [user, :master] + note_opts = { + note: 'Awesome comment', + noteable_type: 'Issue', + noteable_id: issue.id + } + + @note = Notes::CreateService.new(project, user, note_opts).execute + end + + it do + expect(project).to receive(:execute_hooks) + expect(project).to receive(:execute_services) + Notes::PostProcessService.new(@note).execute + end + end +end diff --git a/spec/support/filter_spec_helper.rb b/spec/support/filter_spec_helper.rb index d6e03cbef3d..ef5ea7d626e 100644 --- a/spec/support/filter_spec_helper.rb +++ b/spec/support/filter_spec_helper.rb @@ -67,9 +67,9 @@ module FilterSpecHelper if reference =~ /\A(.+)?.\d+\z/ # Integer-based reference with optional project prefix reference.gsub(/\d+\z/) { |i| i.to_i + 1 } - elsif reference =~ /\A(.+@)?(\h{6,40}\z)/ + elsif reference =~ /\A(.+@)?(\h{7,40}\z)/ # SHA-based reference with optional prefix - reference.gsub(/\h{6,40}\z/) { |v| v.reverse } + reference.gsub(/\h{7,40}\z/) { |v| v.reverse } else reference.gsub(/\w+\z/) { |v| v.reverse } end |