diff options
32 files changed, 495 insertions, 25 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a3ce1de50c2..8466edb1981 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -362,6 +362,7 @@ db:migrate:reset-mysql: - git fetch origin v8.14.10 - git checkout -f FETCH_HEAD - bundle install $BUNDLE_INSTALL_FLAGS + - cp config/gitlab.yml.example config/gitlab.yml - bundle exec rake db:drop db:create db:schema:load db:seed_fu - git checkout $CI_COMMIT_SHA - bundle install $BUNDLE_INSTALL_FLAGS diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index 17f23f7fce3..f9ceaec9418 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -31,6 +31,12 @@ $new-sidebar-width: 220px; &:hover { background-color: $border-color; } + + .project-title, + .group-title { + overflow: hidden; + text-overflow: ellipsis; + } } .settings-avatar { diff --git a/app/models/concerns/each_batch.rb b/app/models/concerns/each_batch.rb new file mode 100644 index 00000000000..6ddbb8da1a9 --- /dev/null +++ b/app/models/concerns/each_batch.rb @@ -0,0 +1,81 @@ +module EachBatch + extend ActiveSupport::Concern + + module ClassMethods + # Iterates over the rows in a relation in batches, similar to Rails' + # `in_batches` but in a more efficient way. + # + # Unlike `in_batches` provided by Rails this method does not support a + # custom start/end range, nor does it provide support for the `load:` + # keyword argument. + # + # This method will yield an ActiveRecord::Relation to the supplied block, or + # return an Enumerator if no block is given. + # + # Example: + # + # User.each_batch do |relation| + # relation.update_all(updated_at: Time.now) + # end + # + # The supplied block is also passed an optional batch index: + # + # User.each_batch do |relation, index| + # puts index # => 1, 2, 3, ... + # end + # + # You can also specify an alternative column to use for ordering the rows: + # + # User.each_batch(column: :created_at) do |relation| + # ... + # end + # + # This will produce SQL queries along the lines of: + # + # User Load (0.7ms) SELECT "users"."id" FROM "users" WHERE ("users"."id" >= 41654) ORDER BY "users"."id" ASC LIMIT 1 OFFSET 1000 + # (0.7ms) SELECT COUNT(*) FROM "users" WHERE ("users"."id" >= 41654) AND ("users"."id" < 42687) + # + # of - The number of rows to retrieve per batch. + # column - The column to use for ordering the batches. + def each_batch(of: 1000, column: primary_key) + unless column + raise ArgumentError, + 'the column: argument must be set to a column name to use for ordering rows' + end + + start = except(:select) + .select(column) + .reorder(column => :asc) + .take + + return unless start + + start_id = start[column] + arel_table = self.arel_table + + 1.step do |index| + stop = except(:select) + .select(column) + .where(arel_table[column].gteq(start_id)) + .reorder(column => :asc) + .offset(of) + .limit(1) + .take + + relation = where(arel_table[column].gteq(start_id)) + + if stop + stop_id = stop[column] + start_id = stop_id + relation = relation.where(arel_table[column].lt(stop_id)) + end + + # Any ORDER BYs are useless for this relation and can lead to less + # efficient UPDATE queries, hence we get rid of it. + yield relation.except(:order), index + + break unless stop + end + end + end +end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index c5f959a1874..bc4a13cf4bc 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -35,11 +35,12 @@ module MergeRequests # target branch manually def close_merge_requests commit_ids = @commits.map(&:id) - merge_requests = @project.merge_requests.opened.where(target_branch: @branch_name).to_a + merge_requests = @project.merge_requests.preload(:merge_request_diff).opened.where(target_branch: @branch_name).to_a merge_requests = merge_requests.select(&:diff_head_commit) merge_requests = merge_requests.select do |merge_request| - commit_ids.include?(merge_request.diff_head_sha) + commit_ids.include?(merge_request.diff_head_sha) && + merge_request.merge_request_diff.state != 'empty' end filter_merge_requests(merge_requests).each do |merge_request| diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 1a9f5401a78..cc9219cb6fe 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -12,10 +12,12 @@ .content-wrapper{ class: "#{(layout_nav_class unless show_new_nav?)}" } .alert-wrapper = render "layouts/broadcast" + - if show_new_nav? + - if content_for?(:new_global_flash) + = yield :new_global_flash + = render "layouts/nav/breadcrumbs" = render "layouts/flash" = yield :flash_message - - if show_new_nav? - = render "layouts/nav/breadcrumbs" %div{ class: "#{(container_class unless @no_container)} #{@content_class}" } .content{ id: "content-body" } = yield diff --git a/app/views/layouts/nav/_new_admin_sidebar.html.haml b/app/views/layouts/nav/_new_admin_sidebar.html.haml index 40c1ca7b53e..d7a9e530983 100644 --- a/app/views/layouts/nav/_new_admin_sidebar.html.haml +++ b/app/views/layouts/nav/_new_admin_sidebar.html.haml @@ -4,7 +4,7 @@ = icon('wrench') .project-title Admin Area %ul.sidebar-top-level-items - = nav_link(controller: %w(dashboard admin projects users groups builds runners cohorts), html_options: {class: 'home'}) do + = nav_link(controller: %w(dashboard admin projects users groups jobs runners cohorts), html_options: {class: 'home'}) do = link_to admin_root_path, title: 'Overview', class: 'shortcuts-tree' do %span Overview @@ -26,7 +26,7 @@ = link_to admin_groups_path, title: 'Groups' do %span Groups - = nav_link path: 'builds#index' do + = nav_link path: 'jobs#index' do = link_to admin_jobs_path, title: 'Jobs' do %span Jobs diff --git a/app/views/layouts/nav/_new_group_sidebar.html.haml b/app/views/layouts/nav/_new_group_sidebar.html.haml index b7ac04cc3e5..7b68d11041e 100644 --- a/app/views/layouts/nav/_new_group_sidebar.html.haml +++ b/app/views/layouts/nav/_new_group_sidebar.html.haml @@ -1,5 +1,5 @@ .nav-sidebar - = link_to group_path(@group), title: 'Group', class: 'context-header' do + = link_to group_path(@group), title: @group.name, class: 'context-header' do .avatar-container.s40.group-avatar = image_tag group_icon(@group), class: "avatar s40 avatar-tile" .group-title diff --git a/app/views/layouts/nav/_new_project_sidebar.html.haml b/app/views/layouts/nav/_new_project_sidebar.html.haml index 6e483353a2d..cc731db3cc1 100644 --- a/app/views/layouts/nav/_new_project_sidebar.html.haml +++ b/app/views/layouts/nav/_new_project_sidebar.html.haml @@ -1,6 +1,6 @@ .nav-sidebar - can_edit = can?(current_user, :admin_project, @project) - = link_to project_path(@project), title: 'Project', class: 'context-header' do + = link_to project_path(@project), title: @project.name, class: 'context-header' do .avatar-container.s40.project-avatar = project_icon(@project, alt: @project.name, class: 'avatar s40 avatar-tile') .project-title diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml index 50e0bad3ccf..0f132a68ce1 100644 --- a/app/views/projects/empty.html.haml +++ b/app/views/projects/empty.html.haml @@ -1,6 +1,7 @@ - @no_container = true +- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message -= content_for :flash_message do += content_for flash_message_container do - if current_user && can?(current_user, :download_code, @project) = render 'shared/no_ssh' = render 'shared/no_password' diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index ea780b1cb83..d413c4619be 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -1,9 +1,10 @@ - @no_container = true +- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message = content_for :meta_tags do = auto_discovery_link_tag(:atom, project_path(@project, rss_url_options), title: "#{@project.name} activity") -= content_for :flash_message do += content_for flash_message_container do - if current_user && can?(current_user, :download_code, @project) = render 'shared/no_ssh' = render 'shared/no_password' diff --git a/changelogs/unreleased/feature-user-agent-details-api.yml b/changelogs/unreleased/feature-user-agent-details-api.yml new file mode 100644 index 00000000000..839ec7d21cd --- /dev/null +++ b/changelogs/unreleased/feature-user-agent-details-api.yml @@ -0,0 +1,4 @@ +--- +title: Allow admins to retrieve user agent details for an issue or snippet +merge_request: 12655 +author: diff --git a/changelogs/unreleased/fix-mrs-merged-immediately.yml b/changelogs/unreleased/fix-mrs-merged-immediately.yml new file mode 100644 index 00000000000..41c06614e6d --- /dev/null +++ b/changelogs/unreleased/fix-mrs-merged-immediately.yml @@ -0,0 +1,4 @@ +--- +title: Don't mark empty MRs as merged on push to the target branch +merge_request: +author: diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 75d03de18a1..221e3d6e03b 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -657,7 +657,10 @@ test: client_id: 'YOUR_AUTH0_CLIENT_ID', client_secret: 'YOUR_AUTH0_CLIENT_SECRET', namespace: 'YOUR_AUTH0_DOMAIN' } } - + - { name: 'authentiq', + app_id: 'YOUR_CLIENT_ID', + app_secret: 'YOUR_CLIENT_SECRET', + args: { scope: 'aq:name email~rs address aq:push' } } ldap: enabled: false servers: diff --git a/doc/api/issues.md b/doc/api/issues.md index df5666bb7b6..a00a63bad4b 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -964,3 +964,30 @@ Example response: ## Comments on issues Comments are done via the [notes](notes.md) resource. + +## Get user agent details + +Available only for admins. + +``` +GET /projects/:id/issues/:issue_iid/user_agent_detail +``` + +| Attribute | Type | Required | Description | +|-------------|---------|----------|--------------------------------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `issue_iid` | integer | yes | The internal ID of a project's issue | + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/93/user_agent_detail +``` + +Example response: + +```json +{ + "user_agent": "AppleWebKit/537.36", + "ip_address": "127.0.0.1", + "akismet_submitted": false +} +``` diff --git a/doc/api/project_snippets.md b/doc/api/project_snippets.md index 92491de4daa..d74398c6e65 100644 --- a/doc/api/project_snippets.md +++ b/doc/api/project_snippets.md @@ -119,3 +119,35 @@ Parameters: - `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user - `snippet_id` (required) - The ID of a project's snippet + +## Get user agent details + +> **Notes:** +> [Introduced][ce-29508] in GitLab 9.4. + + +Available only for admins. + +``` +GET /projects/:id/snippets/:snippet_id/user_agent_detail +``` + +| Attribute | Type | Required | Description | +|-------------|---------|----------|--------------------------------------| +| `id` | Integer | yes | The ID of a snippet | + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/1/snippets/1/user_agent_detail +``` + +Example response: + +```json +{ + "user_agent": "AppleWebKit/537.36", + "ip_address": "127.0.0.1", + "akismet_submitted": false +} +``` + +[ce-[ce-29508]: https://gitlab.com/gitlab-org/gitlab-ce/issues/29508]: https://gitlab.com/gitlab-org/gitlab-ce/issues/29508 diff --git a/doc/api/snippets.md b/doc/api/snippets.md index efaab712367..fdafbfb5b9e 100644 --- a/doc/api/snippets.md +++ b/doc/api/snippets.md @@ -234,3 +234,35 @@ Example response: } ] ``` + +## Get user agent details + +> **Notes:** +> [Introduced][ce-29508] in GitLab 9.4. + + +Available only for admins. + +``` +GET /snippets/:id/user_agent_detail +``` + +| Attribute | Type | Required | Description | +|-------------|---------|----------|--------------------------------------| +| `id` | Integer | yes | The ID of a snippet | + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/snippets/1/user_agent_detail +``` + +Example response: + +```json +{ + "user_agent": "AppleWebKit/537.36", + "ip_address": "127.0.0.1", + "akismet_submitted": false +} +``` + +[ce-[ce-29508]: https://gitlab.com/gitlab-org/gitlab-ce/issues/29508]: https://gitlab.com/gitlab-org/gitlab-ce/issues/29508 diff --git a/doc/development/README.md b/doc/development/README.md index a2a07c37ced..58993c52dcd 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -55,6 +55,7 @@ - [Single Table Inheritance](single_table_inheritance.md) - [Background Migrations](background_migrations.md) - [Storing SHA1 Hashes As Binary](sha1_as_binary.md) +- [Iterating Tables In Batches](iterating_tables_in_batches.md) ## i18n diff --git a/doc/development/iterating_tables_in_batches.md b/doc/development/iterating_tables_in_batches.md new file mode 100644 index 00000000000..590c8cbba2d --- /dev/null +++ b/doc/development/iterating_tables_in_batches.md @@ -0,0 +1,37 @@ +# Iterating Tables In Batches + +Rails provides a method called `in_batches` that can be used to iterate over +rows in batches. For example: + +```ruby +User.in_batches(of: 10) do |relation| + relation.update_all(updated_at: Time.now) +end +``` + +Unfortunately this method is implemented in a way that is not very efficient, +both query and memory usage wise. + +To work around this you can include the `EachBatch` module into your models, +then use the `each_batch` class method. For example: + +```ruby +class User < ActiveRecord::Base + include EachBatch +end + +User.each_batch(of: 10) do |relation| + relation.update_all(updated_at: Time.now) +end +``` + +This will end up producing queries such as: + +``` +User Load (0.7ms) SELECT "users"."id" FROM "users" WHERE ("users"."id" >= 41654) ORDER BY "users"."id" ASC LIMIT 1 OFFSET 1000 + (0.7ms) SELECT COUNT(*) FROM "users" WHERE ("users"."id" >= 41654) AND ("users"."id" < 42687) +``` + +The API of this method is similar to `in_batches`, though it doesn't support +all of the arguments that `in_batches` supports. You should always use +`each_batch` _unless_ you have a specific need for `in_batches`. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index fdc0c562248..f4796f311a5 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -888,5 +888,11 @@ module API expose :dependencies, using: Dependency end end + + class UserAgentDetail < Grape::Entity + expose :user_agent + expose :ip_address + expose :submitted, as: :akismet_submitted + end end end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 09dca0dff8b..64be08094ed 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -241,6 +241,22 @@ module API present paginate(merge_requests), with: Entities::MergeRequestBasic, current_user: current_user, project: user_project end + + desc 'Get the user agent details for an issue' do + success Entities::UserAgentDetail + end + params do + requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue' + end + get ":id/issues/:issue_iid/user_agent_detail" do + authenticated_as_admin! + + issue = find_project_issue(params[:issue_iid]) + + return not_found!('UserAgentDetail') unless issue.user_agent_detail + + present issue.user_agent_detail, with: Entities::UserAgentDetail + end end end end diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index 64efe82a937..3320eadff0d 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -131,6 +131,22 @@ module API content_type 'text/plain' present snippet.content end + + desc 'Get the user agent details for a project snippet' do + success Entities::UserAgentDetail + end + params do + requires :snippet_id, type: Integer, desc: 'The ID of a project snippet' + end + get ":id/snippets/:snippet_id/user_agent_detail" do + authenticated_as_admin! + + snippet = Snippet.find_by!(id: params[:id]) + + return not_found!('UserAgentDetail') unless snippet.user_agent_detail + + present snippet.user_agent_detail, with: Entities::UserAgentDetail + end end end end diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index c630c24c339..fd634037a77 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -140,6 +140,22 @@ module API content_type 'text/plain' present snippet.content end + + desc 'Get the user agent details for a snippet' do + success Entities::UserAgentDetail + end + params do + requires :id, type: Integer, desc: 'The ID of a snippet' + end + get ":id/user_agent_detail" do + authenticated_as_admin! + + snippet = Snippet.find_by!(id: params[:id]) + + return not_found!('UserAgentDetail') unless snippet.user_agent_detail + + present snippet.user_agent_detail, with: Entities::UserAgentDetail + end end end end diff --git a/lib/gitlab/otp_key_rotator.rb b/lib/gitlab/otp_key_rotator.rb index 0d541935bc6..22332474945 100644 --- a/lib/gitlab/otp_key_rotator.rb +++ b/lib/gitlab/otp_key_rotator.rb @@ -34,7 +34,7 @@ module Gitlab write_csv do |csv| ActiveRecord::Base.transaction do - User.with_two_factor.in_batches do |relation| + User.with_two_factor.in_batches do |relation| # rubocop: disable Cop/InBatches rows = relation.pluck(:id, :encrypted_otp_secret, :encrypted_otp_secret_iv, :encrypted_otp_secret_salt) rows.each do |row| user = %i[id ciphertext iv salt].zip(row).to_h diff --git a/rubocop/cop/in_batches.rb b/rubocop/cop/in_batches.rb new file mode 100644 index 00000000000..c0240187e66 --- /dev/null +++ b/rubocop/cop/in_batches.rb @@ -0,0 +1,16 @@ +require_relative '../model_helpers' + +module RuboCop + module Cop + # Cop that prevents the use of `in_batches` + class InBatches < RuboCop::Cop::Cop + MSG = 'Do not use `in_batches`, use `each_batch` from the EachBatch module instead'.freeze + + def on_send(node) + return unless node.children[1] == :in_batches + + add_offense(node, :selector) + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 1e70314598a..f76144275c9 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -5,6 +5,7 @@ require_relative 'cop/redirect_with_status' require_relative 'cop/polymorphic_associations' require_relative 'cop/project_path_helper' require_relative 'cop/active_record_dependent' +require_relative 'cop/in_batches' require_relative 'cop/migration/add_column' require_relative 'cop/migration/add_column_with_default_to_large_table' require_relative 'cop/migration/add_concurrent_foreign_key' diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb index 1b6d1f3415f..42764e808e6 100644 --- a/spec/features/oauth_login_spec.rb +++ b/spec/features/oauth_login_spec.rb @@ -13,7 +13,7 @@ feature 'OAuth Login', js: true do end providers = [:github, :twitter, :bitbucket, :gitlab, :google_oauth2, - :facebook, :cas3, :auth0] + :facebook, :cas3, :auth0, :authentiq] before(:all) do # The OmniAuth `full_host` parameter doesn't get set correctly (it gets set to something like `http://localhost` diff --git a/spec/models/concerns/each_batch_spec.rb b/spec/models/concerns/each_batch_spec.rb new file mode 100644 index 00000000000..951690a217b --- /dev/null +++ b/spec/models/concerns/each_batch_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe EachBatch do + describe '.each_batch' do + let(:model) do + Class.new(ActiveRecord::Base) do + include EachBatch + + self.table_name = 'users' + end + end + + before do + 5.times { create(:user, updated_at: 1.day.ago) } + end + + it 'yields an ActiveRecord::Relation when a block is given' do + model.each_batch do |relation| + expect(relation).to be_a_kind_of(ActiveRecord::Relation) + end + end + + it 'yields a batch index as the second argument' do + model.each_batch do |_, index| + expect(index).to eq(1) + end + end + + it 'accepts a custom batch size' do + amount = 0 + + model.each_batch(of: 1) { amount += 1 } + + expect(amount).to eq(5) + end + + it 'does not include ORDER BYs in the yielded relations' do + model.each_batch do |relation| + expect(relation.to_sql).not_to include('ORDER BY') + end + end + + it 'allows updating of the yielded relations' do + time = Time.now + + model.each_batch do |relation| + relation.update_all(updated_at: time) + end + + expect(model.where(updated_at: time).count).to eq(5) + end + end +end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 9b53164b4a2..9837fedb522 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1462,6 +1462,25 @@ describe API::Issues do end end + describe "GET /projects/:id/issues/:issue_iid/user_agent_detail" do + let!(:user_agent_detail) { create(:user_agent_detail, subject: issue) } + + it 'exposes known attributes' do + get api("/projects/#{project.id}/issues/#{issue.iid}/user_agent_detail", admin) + + expect(response).to have_http_status(200) + expect(json_response['user_agent']).to eq(user_agent_detail.user_agent) + expect(json_response['ip_address']).to eq(user_agent_detail.ip_address) + expect(json_response['akismet_submitted']).to eq(user_agent_detail.submitted) + end + + it "returns unautorized for non-admin users" do + get api("/projects/#{project.id}/issues/#{issue.iid}/user_agent_detail", user) + + expect(response).to have_http_status(403) + end + end + def expect_paginated_array_response(size: nil) expect(response).to have_http_status(200) expect(response).to include_pagination_headers diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 518639f45a2..f220972bae3 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -5,6 +5,26 @@ describe API::ProjectSnippets do let(:user) { create(:user) } let(:admin) { create(:admin) } + describe "GET /projects/:project_id/snippets/:id/user_agent_detail" do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:user_agent_detail) { create(:user_agent_detail, subject: snippet) } + + it 'exposes known attributes' do + get api("/projects/#{project.id}/snippets/#{snippet.id}/user_agent_detail", admin) + + expect(response).to have_http_status(200) + expect(json_response['user_agent']).to eq(user_agent_detail.user_agent) + expect(json_response['ip_address']).to eq(user_agent_detail.ip_address) + expect(json_response['akismet_submitted']).to eq(user_agent_detail.submitted) + end + + it "returns unautorized for non-admin users" do + get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/user_agent_detail", user) + + expect(response).to have_http_status(403) + end + end + describe 'GET /projects/:project_id/snippets/' do let(:user) { create(:user) } @@ -20,7 +40,7 @@ describe API::ProjectSnippets do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.size).to eq(3) - expect(json_response.map{ |snippet| snippet['id']} ).to include(public_snippet.id, internal_snippet.id, private_snippet.id) + expect(json_response.map { |snippet| snippet['id'] }).to include(public_snippet.id, internal_snippet.id, private_snippet.id) expect(json_response.last).to have_key('web_url') end @@ -38,7 +58,7 @@ describe API::ProjectSnippets do describe 'GET /projects/:project_id/snippets/:id' do let(:user) { create(:user) } - let(:snippet) { create(:project_snippet, :public, project: project) } + let(:snippet) { create(:project_snippet, :public, project: project) } it 'returns snippet json' do get api("/projects/#{project.id}/snippets/#{snippet.id}", user) diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index b20a187acfe..373fab4d98a 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -271,4 +271,25 @@ describe API::Snippets do expect(json_response['message']).to eq('404 Snippet Not Found') end end + + describe "GET /snippets/:id/user_agent_detail" do + let(:admin) { create(:admin) } + let(:snippet) { create(:personal_snippet, :public, author: user) } + let!(:user_agent_detail) { create(:user_agent_detail, subject: snippet) } + + it 'exposes known attributes' do + get api("/snippets/#{snippet.id}/user_agent_detail", admin) + + expect(response).to have_http_status(200) + expect(json_response['user_agent']).to eq(user_agent_detail.user_agent) + expect(json_response['ip_address']).to eq(user_agent_detail.ip_address) + expect(json_response['akismet_submitted']).to eq(user_agent_detail.submitted) + end + + it "returns unautorized for non-admin users" do + get api("/snippets/#{snippet.id}/user_agent_detail", user) + + expect(response).to have_http_status(403) + end + end end diff --git a/spec/rubocop/cop/in_batches_spec.rb b/spec/rubocop/cop/in_batches_spec.rb new file mode 100644 index 00000000000..072481984c6 --- /dev/null +++ b/spec/rubocop/cop/in_batches_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../rubocop/cop/in_batches' + +describe RuboCop::Cop::InBatches do + include CopHelper + + subject(:cop) { described_class.new } + + it 'registers an offense when in_batches is used' do + inspect_source(cop, 'foo.in_batches do; end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end +end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 671a932441e..74dcf152cb8 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -98,18 +98,52 @@ describe MergeRequests::RefreshService, services: true do end context 'push to origin repo target branch' do - before do - service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') - reload_mrs + context 'when all MRs to the target branch had diffs' do + before do + service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + reload_mrs + end + + it 'updates the merge state' do + expect(@merge_request.notes.last.note).to include('merged') + expect(@merge_request).to be_merged + expect(@fork_merge_request).to be_merged + expect(@fork_merge_request.notes.last.note).to include('merged') + expect(@build_failed_todo).to be_done + expect(@fork_build_failed_todo).to be_done + end end - it 'updates the merge state' do - expect(@merge_request.notes.last.note).to include('merged') - expect(@merge_request).to be_merged - expect(@fork_merge_request).to be_merged - expect(@fork_merge_request.notes.last.note).to include('merged') - expect(@build_failed_todo).to be_done - expect(@fork_build_failed_todo).to be_done + context 'when an MR to be closed was empty already' do + let!(:empty_fork_merge_request) do + create(:merge_request, + source_project: @fork_project, + source_branch: 'master', + target_branch: 'master', + target_project: @project) + end + + before do + # This spec already has a fake push, so pretend that we were targeting + # feature all along. + empty_fork_merge_request.update_columns(target_branch: 'feature') + + service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + reload_mrs + empty_fork_merge_request.reload + end + + it 'only updates the non-empty MRs' do + expect(@merge_request).to be_merged + expect(@merge_request.notes.last.note).to include('merged') + + expect(@fork_merge_request).to be_merged + expect(@fork_merge_request.notes.last.note).to include('merged') + + expect(empty_fork_merge_request).to be_open + expect(empty_fork_merge_request.merge_request_diff.state).to eq('empty') + expect(empty_fork_merge_request.notes).to be_empty + end end end |