Simplify API for activating featur flags. Fix rake tasks.

app/models/carto/helpers/user_commons.rb

@@ -321,4 +321,10 @@ module Carto::UserCommons
     feature_flags.exists?(name: feature_flag_name)
   end
 
+  def activate_feature_flag!(feature_flag)
+    return if Carto::FeatureFlagsUser.exists?(feature_flag: feature_flag, user_id: id)
+
+    Carto::FeatureFlagsUser.create!(feature_flag: feature_flag, user_id: id)
+  end
+
 end

lib/tasks/acceptance/ghost_tables.rake

@@ -6,7 +6,7 @@ namespace :cartodb do
     desc 'Acceptance tests regarding Ghost Tables trigger'
     task :ghost_tables, [:username, :sleep_time] => :environment do |_t, args|
       raise 'usage: rake cartodb:acceptance:ghost_tables[username]' if args[:username].blank?
-      @user = User.where(username: args[:username]).first
+      @user = Carto::User.find_by(username: args[:username])
       raise "user with username '#{args[:username]}' not found" unless @user
       args.with_defaults(sleep_time: DEFAULT_SLEEP_TIME)
       @sleep_time = args[:sleep_time].to_i
@@ -62,7 +62,7 @@ namespace :cartodb do
 
     def enable_feature_flag
       ff = Carto::FeatureFlag.find_by(name: 'ghost_tables_trigger_disabled')
-      Carto::FeatureFlagsUser.create(feature_flag: ff, user: @user)
+      @user.activate_feature_flag!(ff)
       @user.reload
     end
 

lib/tasks/enable_feature.rake

@@ -9,9 +9,9 @@ namespace :cartodb do
       ff = Carto::FeatureFlag.find_by(name: args[:feature])
       raise "[ERROR]  Feature '#{args[:feature]}' does not exist" if ff.nil?
 
-      ::User.all.each do |user|
+      Carto::User.find_each do |user|
         if !Carto::FeatureFlagsUser.exists?(feature_flag: ff, user: user)
-          Carto::FeatureFlagsUser.create(feature_flag: ff, user: user)
+          user.activate_feature_flag!(ff)
           track_feature_flag_state(user.id, args[:feature], 'enabled')
         end
       end
@@ -25,11 +25,11 @@ namespace :cartodb do
       ff = Carto::FeatureFlag.find_by(name: args[:feature])
       raise "[ERROR]  Feature '#{args[:feature]}' does not exist" if ff.nil?
 
-      user = ::User[username: args[:username]]
+      user = Carto::User.find_by(username: args[:username])
       raise "[ERROR]  User '#{args[:username]}' does not exist" if user.nil?
 
       if !Carto::FeatureFlagsUser.exists?(feature_flag: ff, user: user)
-        Carto::FeatureFlagsUser.create(feature_flag: ff, user: user)
+        user.activate_feature_flag!(ff)
         track_feature_flag_state(user.id, args[:feature], 'enabled')
       else
         puts "[INFO]  Feature '#{args[:feature]}' was already enabled for user '#{args[:username]}'"
@@ -44,12 +44,12 @@ namespace :cartodb do
       ff = Carto::FeatureFlag.find_by(name: args[:feature])
       raise "[ERROR]  Feature '#{args[:feature]}' does not exist" if ff.nil?
 
-      organization = Organization[name: args[:org_name]]
+      organization = Carto::Organization.find_by(name: args[:org_name])
       raise "[ERROR]  Organization '#{args[:org_name]}' does not exist" if organization.nil?
 
       organization.users.each do |user|
         if !Carto::FeatureFlagsUser.exists?(feature_flag: ff, user: user)
-          Carto::FeatureFlagsUser.create(feature_flag: ff, user: user)
+          user.activate_feature_flag!(ff)
           track_feature_flag_state(user.id, args[:feature], 'enabled')
         end
       end
@@ -67,7 +67,7 @@ namespace :cartodb do
       if ffus.nil?
         puts "[INFO]  No users had feature '#{args[:feature]}' enabled"
       else
-        ffus.destroy
+        ffus.destroy_all
       end
     end
 
@@ -79,7 +79,7 @@ namespace :cartodb do
       ff = Carto::FeatureFlag.find_by(name: args[:feature])
       raise "[ERROR]  Feature '#{args[:feature]}' does not exist" if ff.nil?
 
-      user = ::User[username: args[:username]]
+      user = Carto::User.find_by(username: args[:username])
       raise "[ERROR]  User '#{args[:username]}' does not exist" if user.nil?
 
       if !Carto::FeatureFlagsUser.exists?(feature_flag: ff, user: user)
@@ -98,7 +98,7 @@ namespace :cartodb do
       ff = Carto::FeatureFlag.find_by(name: args[:feature])
       raise "[ERROR]  Feature '#{args[:feature]}' does not exist" if ff.nil?
 
-      organization = Organization[name: args[:org_name]]
+      organization = Carto::Organization.find_by(name: args[:org_name])
       raise "[ERROR]  Organization '#{args[:org_name]}' does not exist" if organization.nil?
 
       organization.users.each do |user|
@@ -170,7 +170,7 @@ namespace :cartodb do
       if ffus.nil?
         puts "[INFO]  No users had feature '#{args[:feature]}' enabled"
       else
-        ffus.destroy
+        ffus.destroy_all
       end
 
       ff.destroy()
@@ -182,7 +182,7 @@ namespace :cartodb do
     task :list_all_features => :environment do
 
       puts "Available features:"
-      Carto::FeatureFlag.all.each do |feature|
+      Carto::FeatureFlag.find_each do |feature|
         puts "  - #{feature.name}"
       end
     end

lib/tasks/google_services.rake

@@ -23,7 +23,7 @@ namespace :cartodb do
 
           return if Carto::FeatureFlagsUser.where(feature_flag_id: ff.id, user_id: user.id).exists?
 
-          Carto::FeatureFlagsUser.create(feature_flag_id: ff.id, user_id: user.id)
+          user.activate_feature_flag!(ff)
         end
 
         def remove_feature_flag_from_user(feature_name, user)

spec/helpers/feature_flag_helper.rb

@@ -8,7 +8,7 @@ module FeatureFlagHelper
       ffu = Carto::FeatureFlagsUser.find_by(feature_flag: ff, user: user)
       if state
         unless ffu
-          Carto::FeatureFlagsUser.create(feature_flag: ff, user: user)
+          user.activate_feature_flag!(ff)
         end
       else
         ff.update restricted: false unless ff.restricted

spec/helpers/user_commons_spec.rb

@@ -40,7 +40,7 @@ describe Carto::UserCommons do
   describe 'feature flags' do
     let(:feature_flag) { create(:feature_flag) }
 
-    before { user.self_feature_flags_user.create!(feature_flag: feature_flag) }
+    before { user.activate_feature_flag!(feature_flag) }
 
     describe '#self_feature_flags_user' do
       subject { user.self_feature_flags_user }
@@ -78,7 +78,7 @@ describe Carto::UserCommons do
       it 'returns feature flags inherited from organization' do
         organization.update!(inherit_owner_ffs: true)
         user.update!(organization: organization)
-        organization_owner.self_feature_flags_user.create!(feature_flag: organization_feature_flag)
+        organization_owner.activate_feature_flag!(organization_feature_flag)
 
         expect(subject).to include(organization_feature_flag)
       end
@@ -114,5 +114,27 @@ describe Carto::UserCommons do
         expect(subject).to eq(sequel_user.has_feature_flag?(feature_flag.name))
       end
     end
+
+    describe '#activate_feature_flag!' do
+      let(:new_feature_flag) { create(:feature_flag) }
+
+      context 'when not active yet' do
+        it 'activates it' do
+          user.activate_feature_flag!(new_feature_flag)
+          user.reload
+
+          expect(user.has_feature_flag?(new_feature_flag.name)).to be_true
+        end
+      end
+
+      context 'when already active' do
+        it 'preserves it' do
+          user.activate_feature_flag!(feature_flag)
+          user.reload
+
+          expect(user.has_feature_flag?(feature_flag.name)).to be_true
+        end
+      end
+    end
   end
 end

spec/models/user_part_crud_spec.rb

@@ -27,7 +27,7 @@ describe User do
       let(:new_feature_flag) { create(:feature_flag) }
       let(:user) { create(:user) }
 
-      before { user.self_feature_flags_user.create!(feature_flag: feature_flag) }
+      before { user.activate_feature_flag!(feature_flag) }
 
       it 'updates feature flags' do
         expect(user.feature_flags).to include(feature_flag)

spec/requests/carto/api/public/federated_tables_controller_spec.rb

@@ -11,7 +11,7 @@ describe Carto::Api::Public::FederatedTablesController do
     host! "#{@user1.username}.localhost.lan"
 
     @feature_flag = FactoryGirl.create(:feature_flag, name: 'federated_tables', restricted: true)
-    Carto::FeatureFlagsUser.create(user_id: @user1.id, feature_flag_id: @feature_flag.id)
+    @user1.activate_feature_flag!(@feature_flag)
 
     puts "Starting remote server"
     @dir = Cartodb.get_config(:federated_server, 'dir')

spec/requests/superadmin/feature_flag_spec.rb

@@ -48,7 +48,7 @@ describe Carto::FeatureFlag do
     end
 
     it 'should destroy feature flag user relations' do
-      user.self_feature_flags_user.create!(feature_flag: feature_flag)
+      user.activate_feature_flag!(feature_flag)
 
       expect {
         delete superadmin_feature_flag_url(feature_flag.id), payload, superadmin_headers

spec/requests/superadmin/users_spec.rb

@@ -550,7 +550,7 @@ feature "Superadmin's users API" do
     it 'should create user feature_flag relation' do
       user = create(:user)
       feature_flag = create(:feature_flag)
-      user.self_feature_flags_user.create!(feature_flag: feature_flag)
+      user.activate_feature_flag!(feature_flag)
       new_feature_flag = create(:feature_flag)
 
       payload = {

spec/services/carto/user_metadata_export_service_spec.rb

@@ -37,7 +37,7 @@ describe Carto::UserMetadataExportService do
 
     @asset = FactoryGirl.create(:carto_asset, user: @user)
 
-    Carto::FeatureFlagsUser.create(feature_flag: @feature_flag, user: @user)
+    @user.activate_feature_flag!(@feature_flag)
 
     CartoDB::GeocoderUsageMetrics.new(@user.username).incr(:geocoder_here, :success_responses)