Add latest changes from gitlab-org/gitlab@master

app/finders/projects_finder.rb

@@ -142,8 +142,8 @@ class ProjectsFinder < UnionFinder
   # rubocop: disable CodeReuse/ActiveRecord
   def by_ids(items)
     items = items.where(id: project_ids_relation) if project_ids_relation
-    items = items.where('id > ?', params[:id_after]) if params[:id_after]
-    items = items.where('id < ?', params[:id_before]) if params[:id_before]
+    items = items.where('projects.id > ?', params[:id_after]) if params[:id_after]
+    items = items.where('projects.id < ?', params[:id_before]) if params[:id_before]
     items
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/graphql/resolvers/merge_requests_resolver.rb

@@ -4,11 +4,11 @@ module Resolvers
   class MergeRequestsResolver < BaseResolver
     argument :iid, GraphQL::STRING_TYPE,
               required: false,
-              description: 'The IID of the merge request, e.g., "1"'
+              description: 'IID of the merge request, for example `1`'
 
     argument :iids, [GraphQL::STRING_TYPE],
               required: false,
-              description: 'The list of IIDs of issues, e.g., [1, 2]'
+              description: 'Array of IIDs of merge requests, for example `[1, 2]`'
 
     type Types::MergeRequestType, null: true
 

changelogs/unreleased/ab-keyset-ambig-bug.yml

+---
+title: Fully qualify id columns for keyset pagination (Projects API)
+merge_request: 29026
+author:
+type: fixed

changelogs/unreleased/remove_api_activity_logging_feature_flag.yml

+---
+title: Enable last user activity logging on the REST API
+merge_request: 28755
+author:
+type: added

db/migrate/20171230123729_init_schema.rb

@@ -6,6 +6,7 @@
 # rubocop:disable Migration/AddConcurrentForeignKey
 # rubocop:disable Style/WordArray
 # rubocop:disable Migration/AddLimitToStringColumns
+# rubocop:disable Migration/Datetime
 
 class InitSchema < ActiveRecord::Migration[4.2]
   DOWNTIME = false

db/migrate/20180116193854_create_lfs_file_locks.rb

@@ -9,7 +9,7 @@ class CreateLfsFileLocks < ActiveRecord::Migration[4.2]
     create_table :lfs_file_locks do |t|
       t.references :project, null: false, foreign_key: { on_delete: :cascade }
       t.references :user, null: false, index: true, foreign_key: { on_delete: :cascade }
-      t.datetime :created_at, null: false
+      t.datetime :created_at, null: false # rubocop:disable Migration/Datetime
       t.string :path, limit: 511
     end
 

db/migrate/20180503131624_create_remote_mirrors.rb

@@ -14,9 +14,9 @@ class CreateRemoteMirrors < ActiveRecord::Migration[4.2]
       t.string :url
       t.boolean :enabled, default: true
       t.string :update_status
-      t.datetime :last_update_at
-      t.datetime :last_successful_update_at
-      t.datetime :last_update_started_at
+      t.datetime :last_update_at # rubocop:disable Migration/Datetime
+      t.datetime :last_successful_update_at # rubocop:disable Migration/Datetime
+      t.datetime :last_update_started_at # rubocop:disable Migration/Datetime
       t.string :last_error
       t.boolean :only_protected_branches, default: false, null: false
       t.string :remote_name

db/migrate/20200215225103_drop_forked_project_links_table.rb

 # frozen_string_literal: true
 
+# rubocop:disable Migration/Datetime
+
 class DropForkedProjectLinksTable < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 

doc/api/graphql/reference/gitlab_schema.graphql

@@ -6147,12 +6147,12 @@ type Project {
   """
   mergeRequest(
     """
-    The IID of the merge request, e.g., "1"
+    IID of the merge request, for example `1`
     """
     iid: String
 
     """
-    The list of IIDs of issues, e.g., [1, 2]
+    Array of IIDs of merge requests, for example `[1, 2]`
     """
     iids: [String!]
   ): MergeRequest
@@ -6177,12 +6177,12 @@ type Project {
     first: Int
 
     """
-    The IID of the merge request, e.g., "1"
+    IID of the merge request, for example `1`
     """
     iid: String
 
     """
-    The list of IIDs of issues, e.g., [1, 2]
+    Array of IIDs of merge requests, for example `[1, 2]`
     """
     iids: [String!]
 

doc/api/graphql/reference/gitlab_schema.json

@@ -18471,7 +18471,7 @@
               "args": [
                 {
                   "name": "iid",
-                  "description": "The IID of the merge request, e.g., \"1\"",
+                  "description": "IID of the merge request, for example `1`",
                   "type": {
                     "kind": "SCALAR",
                     "name": "String",
@@ -18481,7 +18481,7 @@
                 },
                 {
                   "name": "iids",
-                  "description": "The list of IIDs of issues, e.g., [1, 2]",
+                  "description": "Array of IIDs of merge requests, for example `[1, 2]`",
                   "type": {
                     "kind": "LIST",
                     "name": null,
@@ -18512,7 +18512,7 @@
               "args": [
                 {
                   "name": "iid",
-                  "description": "The IID of the merge request, e.g., \"1\"",
+                  "description": "IID of the merge request, for example `1`",
                   "type": {
                     "kind": "SCALAR",
                     "name": "String",
@@ -18522,7 +18522,7 @@
                 },
                 {
                   "name": "iids",
-                  "description": "The list of IIDs of issues, e.g., [1, 2]",
+                  "description": "Array of IIDs of merge requests, for example `[1, 2]`",
                   "type": {
                     "kind": "LIST",
                     "name": null,

doc/user/project/import/svn.md

@@ -100,9 +100,6 @@ Running SubGit in a mirror mode requires a
 [registration](https://subgit.com/pricing). Registration is free for open
 source, academic and startup projects.
 
-We're currently working on deeper GitLab/SubGit integration. You may track our
-progress at [this issue](https://gitlab.com/gitlab-org/gitlab/issues/990).
-
 ### SubGit support
 
 For any questions related to SVN to GitLab migration with SubGit, you can

lib/api/api.rb

@@ -105,7 +105,7 @@ module API
 
     namespace do
       after do
-        ::Users::ActivityService.new(@current_user).execute if Feature.enabled?(:api_activity_logging)
+        ::Users::ActivityService.new(@current_user).execute
       end
 
       # Keep in alphabetical order

lib/gitlab/background_migration/link_lfs_objects.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module BackgroundMigration
-    # Create missing LfsObjectsProject records for forks
-    class LinkLfsObjects
-      # Model definition used for migration
-      class ForkNetworkMember < ActiveRecord::Base
-        self.table_name = 'fork_network_members'
-
-        def self.with_non_existing_lfs_objects
-          joins('JOIN lfs_objects_projects lop ON fork_network_members.forked_from_project_id = lop.project_id')
-            .where(
-              <<~SQL
-                NOT EXISTS (
-                  SELECT 1
-                  FROM lfs_objects_projects
-                  WHERE lfs_objects_projects.project_id = fork_network_members.project_id
-                  AND lfs_objects_projects.lfs_object_id = lop.lfs_object_id
-                )
-              SQL
-            )
-        end
-      end
-
-      def perform(start_id, end_id)
-        # no-op as some queries times out
-      end
-    end
-  end
-end

lib/gitlab/experimentation.rb

@@ -20,7 +20,7 @@ module Gitlab
       paid_signup_flow: {
         feature_toggle: :paid_signup_flow,
         environment: ::Gitlab.dev_env_or_com?,
-        enabled_ratio: 0.5,
+        enabled_ratio: 1,
         tracking_category: 'Growth::Acquisition::Experiment::PaidSignUpFlow'
       },
       suggest_pipeline: {

rubocop/cop/migration/datetime.rb

@@ -14,7 +14,7 @@ module RuboCop
           return unless in_migration?(node)
 
           node.each_descendant(:send) do |send_node|
-            method_name = node.children[1]
+            method_name = send_node.children[1]
 
             if method_name == :datetime || method_name == :timestamp
               add_offense(send_node, location: :selector, message: format(MSG, method_name))

spec/finders/projects_finder_spec.rb

@@ -85,6 +85,24 @@ describe ProjectsFinder, :do_not_mock_admin_mode do
         end
       end
 
+      describe 'regression: Combination of id_before/id_after and joins requires fully qualified column names' do
+        context 'only returns projects with a project id less than given and matching search' do
+          subject { finder.execute.joins(:route) }
+
+          let(:params) { { id_before: public_project.id }}
+
+          it { is_expected.to eq([internal_project]) }
+        end
+
+        context 'only returns projects with a project id greater than given and matching search' do
+          subject { finder.execute.joins(:route) }
+
+          let(:params) { { id_after: internal_project.id }}
+
+          it { is_expected.to eq([public_project]) }
+        end
+      end
+
       describe 'filter by visibility_level' do
         before do
           private_project.add_maintainer(user)

spec/requests/api/api_spec.rb

@@ -13,13 +13,5 @@ describe API::API do
     it 'updates the users last_activity_on date' do
       expect { get api('/groups', user) }.to change { user.reload.last_activity_on }.to(Date.today)
     end
-
-    context 'when the the api_activity_logging feature is disabled' do
-      it 'does not touch last_activity_on' do
-        stub_feature_flags(api_activity_logging: false)
-
-        expect { get api('/groups', user) }.not_to change { user.reload.last_activity_on }
-      end
-    end
   end
 end

spec/rubocop/cop/migration/datetime_spec.rb

@@ -12,9 +12,69 @@ describe RuboCop::Cop::Migration::Datetime do
 
   subject(:cop) { described_class.new }
 
-  let(:migration_with_datetime) do
+  let(:create_table_migration_with_datetime) do
     %q(
-      class Users < ActiveRecord::Migration[4.2]
+      class Users < ActiveRecord::Migration[6.0]
+        DOWNTIME = false
+
+        def change
+          create_table :users do |t|
+            t.string :username, null: false
+            t.datetime :last_sign_in
+          end
+        end
+      end
+    )
+  end
+
+  let(:create_table_migration_with_timestamp) do
+    %q(
+      class Users < ActiveRecord::Migration[6.0]
+        DOWNTIME = false
+
+        def change
+          create_table :users do |t|
+            t.string :username, null: false
+            t.timestamp :last_sign_in
+          end
+        end
+      end
+    )
+  end
+
+  let(:create_table_migration_without_datetime) do
+    %q(
+      class Users < ActiveRecord::Migration[6.0]
+        DOWNTIME = false
+
+        def change
+          create_table :users do |t|
+            t.string :username, null: false
+            t.string :password
+          end
+        end
+      end
+    )
+  end
+
+  let(:create_table_migration_with_datetime_with_timezone) do
+    %q(
+      class Users < ActiveRecord::Migration[6.0]
+        DOWNTIME = false
+
+        def change
+          create_table :users do |t|
+            t.string :username, null: false
+            t.datetime_with_timezone :last_sign_in
+          end
+        end
+      end
+    )
+  end
+
+  let(:add_column_migration_with_datetime) do
+    %q(
+      class Users < ActiveRecord::Migration[6.0]
         DOWNTIME = false
 
         def change
@@ -25,9 +85,9 @@ describe RuboCop::Cop::Migration::Datetime do
     )
   end
 
-  let(:migration_with_timestamp) do
+  let(:add_column_migration_with_timestamp) do
     %q(
-      class Users < ActiveRecord::Migration[4.2]
+      class Users < ActiveRecord::Migration[6.0]
         DOWNTIME = false
 
         def change
@@ -38,9 +98,9 @@ describe RuboCop::Cop::Migration::Datetime do
     )
   end
 
-  let(:migration_without_datetime) do
+  let(:add_column_migration_without_datetime) do
     %q(
-      class Users < ActiveRecord::Migration[4.2]
+      class Users < ActiveRecord::Migration[6.0]
         DOWNTIME = false
 
         def change
@@ -50,9 +110,9 @@ describe RuboCop::Cop::Migration::Datetime do
     )
   end
 
-  let(:migration_with_datetime_with_timezone) do
+  let(:add_column_migration_with_datetime_with_timezone) do
     %q(
-      class Users < ActiveRecord::Migration[4.2]
+      class Users < ActiveRecord::Migration[6.0]
         DOWNTIME = false
 
         def change
@@ -68,18 +128,54 @@ describe RuboCop::Cop::Migration::Datetime do
       allow(cop).to receive(:in_migration?).and_return(true)
     end
 
-    it 'registers an offense when the ":datetime" data type is used' do
-      inspect_source(migration_with_datetime)
+    it 'registers an offense when the ":datetime" data type is used on create_table' do
+      inspect_source(create_table_migration_with_datetime)
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([8])
+        expect(cop.offenses.first.message).to include('`datetime`')
+      end
+    end
+
+    it 'registers an offense when the ":timestamp" data type is used on create_table' do
+      inspect_source(create_table_migration_with_timestamp)
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([8])
+        expect(cop.offenses.first.message).to include('timestamp')
+      end
+    end
+
+    it 'does not register an offense when the ":datetime" data type is not used on create_table' do
+      inspect_source(create_table_migration_without_datetime)
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(0)
+      end
+    end
+
+    it 'does not register an offense when the ":datetime_with_timezone" data type is used on create_table' do
+      inspect_source(create_table_migration_with_datetime_with_timezone)
+
+      aggregate_failures do
+        expect(cop.offenses.size).to eq(0)
+      end
+    end
+
+    it 'registers an offense when the ":datetime" data type is used on add_column' do
+      inspect_source(add_column_migration_with_datetime)
 
       aggregate_failures do
         expect(cop.offenses.size).to eq(1)
         expect(cop.offenses.map(&:line)).to eq([7])
-        expect(cop.offenses.first.message).to include('datetime')
+        expect(cop.offenses.first.message).to include('`datetime`')
       end
     end
 
-    it 'registers an offense when the ":timestamp" data type is used' do
-      inspect_source(migration_with_timestamp)
+    it 'registers an offense when the ":timestamp" data type is used on add_column' do
+      inspect_source(add_column_migration_with_timestamp)
 
       aggregate_failures do
         expect(cop.offenses.size).to eq(1)
@@ -88,16 +184,16 @@ describe RuboCop::Cop::Migration::Datetime do
       end
     end
 
-    it 'does not register an offense when the ":datetime" data type is not used' do
-      inspect_source(migration_without_datetime)
+    it 'does not register an offense when the ":datetime" data type is not used on add_column' do
+      inspect_source(add_column_migration_without_datetime)
 
       aggregate_failures do
         expect(cop.offenses.size).to eq(0)
       end
     end
 
-    it 'does not register an offense when the ":datetime_with_timezone" data type is used' do
-      inspect_source(migration_with_datetime_with_timezone)
+    it 'does not register an offense when the ":datetime_with_timezone" data type is used on add_column' do
+      inspect_source(add_column_migration_with_datetime_with_timezone)
 
       aggregate_failures do
         expect(cop.offenses.size).to eq(0)
@@ -107,10 +203,10 @@ describe RuboCop::Cop::Migration::Datetime do
 
   context 'outside of migration' do
     it 'registers no offense' do
-      inspect_source(migration_with_datetime)
-      inspect_source(migration_with_timestamp)
-      inspect_source(migration_without_datetime)
-      inspect_source(migration_with_datetime_with_timezone)
+      inspect_source(add_column_migration_with_datetime)
+      inspect_source(add_column_migration_with_timestamp)
+      inspect_source(add_column_migration_without_datetime)
+      inspect_source(add_column_migration_with_datetime_with_timezone)
 
       expect(cop.offenses.size).to eq(0)
     end