Merge remote-tracking branch 'origin/master' into 8-10-stable

CHANGELOG

@@ -45,6 +45,7 @@ v 8.10.0 (unreleased)
   - RailsCache metris now includes fetch_hit/fetch_miss and read_hit/read_miss info.
   - Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w)
   - Set import_url validation to be more strict
+  - Memoize MR merged/closed events retrieval
   - Add basic system information like memory and disk usage to the admin panel
   - Don't garbage collect commits that have related DB records like comments
   - More descriptive message for git hooks and file locks
@@ -53,6 +54,7 @@ v 8.10.0 (unreleased)
   - Fix importer for GitHub Pull Requests when a branch was reused across Pull Requests
   - Add date when user joined the team on the member page
   - Fix 404 redirect after validation fails importing a GitLab project
+  - Added setting to set new users by default as external !4545 (Dravere)
 
 v 8.9.5
   - Add more debug info to import/export and memory killer. !5108

app/controllers/admin/application_settings_controller.rb

@@ -87,6 +87,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :version_check_enabled,
       :admin_notification_email,
       :user_oauth_applications,
+      :user_default_external,
       :shared_runners_enabled,
       :shared_runners_text,
       :max_artifacts_size,

app/models/application_setting.rb

@@ -142,6 +142,7 @@ class ApplicationSetting < ActiveRecord::Base
       send_user_confirmation_email: false,
       container_registry_token_expire_delay: 5,
       repository_storage: 'default',
+      user_default_external: false,
     )
   end
 

app/models/merge_request.rb

@@ -318,11 +318,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def merge_event
-    self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
+    @merge_event ||= target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
   end
 
   def closed_event
-    self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
+    @closed_event ||= target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
   end
 
   WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze

app/models/user.rb

@@ -15,7 +15,7 @@ class User < ActiveRecord::Base
   add_authentication_token_field :authentication_token
 
   default_value_for :admin, false
-  default_value_for :external, false
+  default_value_for(:external) { current_application_settings.user_default_external }
   default_value_for :can_create_group, gitlab_config.default_can_create_group
   default_value_for :can_create_team, false
   default_value_for :hide_no_ssh_key, false

app/views/admin/abuse_reports/_abuse_report.html.haml

@@ -3,14 +3,14 @@
 %tr
   %td
     - if user
-      = link_to user.name, [:admin, user]
+      = link_to user.name, user
       .light.small
         Joined #{time_ago_with_tooltip(user.created_at)}
     - else
       (removed)
   %td
     - if reporter
-      = link_to reporter.name, [:admin, reporter]
+      = link_to reporter.name, reporter
     - else
       (removed)
     .light.small

app/views/admin/application_settings/_form.html.haml

@@ -100,6 +100,13 @@
           = f.label :user_oauth_applications do
             = f.check_box :user_oauth_applications
             Allow users to register any application to use GitLab as an OAuth provider
+    .form-group
+      = f.label :user_default_external, 'New users set to external', class: 'control-label col-sm-2'
+      .col-sm-10
+        .checkbox
+          = f.label :user_default_external do
+            = f.check_box :user_default_external
+            Newly registered users will by default be external
 
   %fieldset
     %legend Sign-in Restrictions

app/views/users/show.html.haml

@@ -29,6 +29,11 @@
          
         = link_to user_path(@user, :atom, { private_token: current_user.private_token }), class: 'btn btn-gray' do
           = icon('rss')
+        - if current_user.admin?
+           
+          = link_to [:admin, @user], class: 'btn btn-gray', title: 'View user in admin area',
+            data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
+            = icon('users')
 
     .avatar-holder
       = link_to avatar_icon(@user, 400), target: '_blank' do

db/migrate/20160608211215_add_user_default_external_to_application_settings.rb

+class AddUserDefaultExternalToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:application_settings, :user_default_external, :boolean,
+                            default: false, allow_null: false)
+  end
+
+  def down
+    remove_column(:application_settings, :user_default_external)
+  end
+end

db/migrate/20160620110927_fix_no_validatable_import_url.rb

@@ -11,7 +11,7 @@ class FixNoValidatableImportUrl < ActiveRecord::Migration
 
     attr_reader :results, :query
 
-    def initialize(batch_size: 100, query:)
+    def initialize(batch_size: 1000, query:)
       @offset = 0
       @batch_size = batch_size
       @query = query
@@ -58,22 +58,38 @@ class FixNoValidatableImportUrl < ActiveRecord::Migration
       return
     end
 
+    say('Nullifying empty import URLs')
+
+    nullify_empty_urls
+
     say('Cleaning up invalid import URLs... This may take a few minutes if we have a large number of imported projects.')
 
-    invalid_import_url_project_ids.each { |project_id| cleanup_import_url(project_id) }
+    process_invalid_import_urls
   end
 
-  def invalid_import_url_project_ids
-    ids = []
+  def process_invalid_import_urls
     batches = SqlBatches.new(query: "SELECT id, import_url FROM projects WHERE import_url IS NOT NULL")
 
     while batches.next?
+      project_ids = []
+
       batches.results.each do |result|
-        ids << result['id'] unless valid_url?(result['import_url'])
+        project_ids << result['id'] unless valid_url?(result['import_url'])
       end
+
+      process_batch(project_ids)
     end
 
-    ids
+  end
+
+  def process_batch(project_ids)
+    Thread.new do
+      begin
+        project_ids.each { |project_id| cleanup_import_url(project_id) }
+      ensure
+        ActiveRecord::Base.connection.close
+      end
+    end.join
   end
 
   def valid_url?(url)
@@ -83,4 +99,8 @@ class FixNoValidatableImportUrl < ActiveRecord::Migration
   def cleanup_import_url(project_id)
     execute("UPDATE projects SET import_url = NULL WHERE id = #{project_id}")
   end
+
+  def nullify_empty_urls
+    execute("UPDATE projects SET import_url = NULL WHERE import_url = ''")
+  end
 end

db/schema.rb

@@ -84,6 +84,7 @@ ActiveRecord::Schema.define(version: 20160705163108) do
     t.string   "health_check_access_token"
     t.boolean  "send_user_confirmation_email",          default: false
     t.integer  "container_registry_token_expire_delay", default: 5
+    t.boolean  "user_default_external",                 default: false,        null: false
     t.text     "after_sign_up_text"
     t.string   "repository_storage",                    default: "default"
     t.string   "enabled_git_access_protocol"

doc/api/projects.md

@@ -713,7 +713,7 @@ have the proper access rights, code 403 is returned. Status 404 is returned if t
 doesn't exist, or is hidden to the user.
 
 ```
-POST /projects/:id/archive
+POST /projects/:id/unarchive
 ```
 
 | Attribute | Type | Required | Description |

doc/permissions/permissions.md

@@ -99,3 +99,6 @@ An administrator can flag a user as external [through the API](../api/users.md)
 or by checking the checkbox on the admin panel. As an administrator, navigate
 to **Admin > Users** to create a new user or edit an existing one. There, you
 will find the option to flag the user as external.
+
+By default new users are not set as external users. This behavior can be changed
+by an administrator under **Admin > Application Settings**.
\ No newline at end of file

lib/gitlab/current_settings.rb

@@ -48,6 +48,7 @@ module Gitlab
         akismet_enabled: false,
         repository_checks_enabled: true,
         container_registry_token_expire_delay: 5,
+        user_default_external: false,
       )
     end
 

spec/features/admin/admin_abuse_reports_spec.rb

+require 'spec_helper'
+
+describe "Admin::AbuseReports", feature: true, js: true  do
+  let(:user) { create(:user) }
+
+  context 'as an admin' do
+    describe 'if a user has been reported for abuse' do
+      before do
+        create(:abuse_report, user: user)
+        login_as :admin
+      end
+
+      describe 'in the abuse report view' do
+        it "should present a link to the user's profile" do
+          visit admin_abuse_reports_path
+
+          expect(page).to have_link user.name, href: user_path(user)
+        end
+      end
+
+      describe 'in the profile page of the user' do
+        it 'should show a link to the admin view of the user' do
+          visit user_path(user)
+
+          expect(page).to have_link '', href: admin_user_path(user)
+        end
+      end
+    end
+  end
+end

spec/models/user_spec.rb

@@ -446,6 +446,7 @@ describe User, models: true do
       it { expect(user.can_create_group?).to be_truthy }
       it { expect(user.can_create_project?).to be_truthy }
       it { expect(user.first_name).to eq('John') }
+      it { expect(user.external).to be_falsey }
     end
 
     describe 'with defaults' do
@@ -468,6 +469,26 @@ describe User, models: true do
         expect(user.theme_id).to eq(1)
       end
     end
+
+    context 'when current_application_settings.user_default_external is true' do
+      before do
+        stub_application_setting(user_default_external: true)
+      end
+
+      it "creates external user by default" do
+        user = build(:user)
+
+        expect(user.external).to be_truthy
+      end
+
+      describe 'with default overrides' do
+        it "creates a non-external user" do
+          user = build(:user, external: false)
+
+          expect(user.external).to be_falsey
+        end
+      end
+    end
   end
 
   describe '.find_by_any_email' do