Commit dbf521cf authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch '57538-normalize-users-private-profile-field' into 'master'

Migrate null values for users.private_profile column

See merge request gitlab-org/gitlab-ce!29888
parents d8f7017a 4959d8fd
......@@ -185,6 +185,7 @@ class User < ApplicationRecord
before_validation :set_notification_email, if: :new_record?
before_validation :set_public_email, if: :public_email_changed?
before_validation :set_commit_email, if: :commit_email_changed?
before_save :default_private_profile_to_false
before_save :set_public_email, if: :public_email_changed? # in case validation is skipped
before_save :set_commit_email, if: :commit_email_changed? # in case validation is skipped
before_save :ensure_incoming_email_token
......@@ -1491,6 +1492,12 @@ class User < ApplicationRecord
private
def default_private_profile_to_false
return unless private_profile_changed? && private_profile.nil?
self.private_profile = false
end
def has_current_license?
false
end
......
---
title: Migrate NULL values for users.private_profile column and update users API to reject null value for private_profile.
merge_request: 29888
author:
type: changed
# frozen_string_literal: true
class ChangeNullPrivateProfileToFalse < ActiveRecord::Migration[5.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
DELAY = 30.seconds.to_i
BATCH_SIZE = 1_000
disable_ddl_transaction!
class User < ActiveRecord::Base
self.table_name = 'users'
include ::EachBatch
end
def up
change_column_default :users, :private_profile, false
# Migration will take about 120 hours
User.where(private_profile: nil).each_batch(of: BATCH_SIZE) do |batch, index|
range = batch.pluck('MIN(id)', 'MAX(id)').first
delay = index * DELAY
BackgroundMigrationWorker.perform_in(delay.seconds, 'MigrateNullPrivateProfileToFalse', [*range])
end
end
def down
change_column_default :users, :private_profile, nil
end
end
......@@ -3391,7 +3391,7 @@ ActiveRecord::Schema.define(version: 2019_07_03_130053) do
t.integer "theme_id", limit: 2
t.integer "accepted_term_id"
t.string "feed_token"
t.boolean "private_profile"
t.boolean "private_profile", default: false
t.boolean "include_private_contributions"
t.string "commit_email"
t.boolean "auditor", default: false, null: false
......
......@@ -357,9 +357,9 @@ Parameters:
- `admin` (optional) - User is admin - true or false (default)
- `can_create_group` (optional) - User can create groups - true or false
- `skip_confirmation` (optional) - Skip confirmation - true or false (default)
- `external` (optional) - Flags the user as external - true or false(default)
- `external` (optional) - Flags the user as external - true or false (default)
- `avatar` (optional) - Image file for user's avatar
- `private_profile` (optional) - User's profile is private - true or false
- `private_profile` (optional) - User's profile is private - true or false (default)
- `shared_runners_minutes_limit` (optional) - Pipeline minutes quota for this user **(STARTER)**
- `extra_shared_runners_minutes_limit` (optional) - Extra pipeline minutes quota for this user **(STARTER)**
......@@ -392,11 +392,11 @@ Parameters:
- `admin` (optional) - User is admin - true or false (default)
- `can_create_group` (optional) - User can create groups - true or false
- `skip_reconfirmation` (optional) - Skip reconfirmation - true or false (default)
- `external` (optional) - Flags the user as external - true or false(default)
- `external` (optional) - Flags the user as external - true or false (default)
- `shared_runners_minutes_limit` (optional) - Pipeline minutes quota for this user
- `extra_shared_runners_minutes_limit` (optional) - Extra pipeline minutes quota for this user
- `avatar` (optional) - Image file for user's avatar
- `private_profile` (optional) - User's profile is private - true or false
- `private_profile` (optional) - User's profile is private - true or false (default)
- `shared_runners_minutes_limit` (optional) - Pipeline minutes quota for this user **(STARTER)**
- `extra_shared_runners_minutes_limit` (optional) - Extra pipeline minutes quota for this user **(STARTER)**
......
......@@ -51,7 +51,7 @@ module API
optional :can_create_group, type: Boolean, desc: 'Flag indicating the user can create groups'
optional :external, type: Boolean, desc: 'Flag indicating the user is an external user'
optional :avatar, type: File, desc: 'Avatar image for user'
optional :private_profile, type: Boolean, desc: 'Flag indicating the user has a private profile'
optional :private_profile, type: Boolean, default: false, desc: 'Flag indicating the user has a private profile'
all_or_none_of :extern_uid, :provider
use :optional_params_ee
......
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# This class is responsible for migrating a range of users with private_profile == NULL to false
class MigrateNullPrivateProfileToFalse
# Temporary AR class for users
class User < ActiveRecord::Base
self.table_name = 'users'
end
def perform(start_id, stop_id)
User.where(private_profile: nil, id: start_id..stop_id).update_all(private_profile: false)
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::BackgroundMigration::MigrateNullPrivateProfileToFalse, :migration, schema: 20190620105427 do
let(:users) { table(:users) }
it 'correctly migrates nil private_profile to false' do
private_profile_true = users.create!(private_profile: true, projects_limit: 1, email: 'a@b.com')
private_profile_false = users.create!(private_profile: false, projects_limit: 1, email: 'b@c.com')
private_profile_nil = users.create!(private_profile: nil, projects_limit: 1, email: 'c@d.com')
described_class.new.perform(private_profile_true.id, private_profile_nil.id)
private_profile_true.reload
private_profile_false.reload
private_profile_nil.reload
expect(private_profile_true.private_profile).to eq(true)
expect(private_profile_false.private_profile).to eq(false)
expect(private_profile_nil.private_profile).to eq(false)
end
end
......@@ -530,6 +530,17 @@ describe User do
end
describe 'before save hook' do
context '#default_private_profile_to_false' do
let(:user) { create(:user, private_profile: true) }
it 'converts nil to false' do
user.private_profile = nil
user.save!
expect(user.private_profile).to eq false
end
end
context 'when saving an external user' do
let(:user) { create(:user) }
let(:external_user) { create(:user, external: true) }
......@@ -1127,6 +1138,7 @@ describe User do
expect(user.can_create_group).to eq(Gitlab.config.gitlab.default_can_create_group)
expect(user.theme_id).to eq(Gitlab.config.gitlab.default_theme)
expect(user.external).to be_falsey
expect(user.private_profile).to eq false
end
end
......
......@@ -745,6 +745,14 @@ describe API::Users do
expect(user.reload.private_profile).to eq(true)
end
it "updates private profile when nil is given to false" do
admin.update(private_profile: true)
put api("/users/#{user.id}", admin), params: { private_profile: nil }
expect(user.reload.private_profile).to eq(false)
end
it "does not update admin status" do
put api("/users/#{admin_user.id}", admin), params: { can_create_group: false }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment