Merge branch 'master' into do-sync-samples

.rubocop.yml

@@ -153,3 +153,12 @@ Style/SymbolArray:
 Style/WordArray:
   Description: Use %w or %W for arrays of words.
   Enabled: false
+
+
+####################
+#  Rails
+####################
+
+Rails/CreateTableWithTimestamps:
+  Description: Create new tables with created_at and updated_at timestamps
+  Enabled: false

NEWS.md

@@ -5,12 +5,13 @@ Development
 - None yet
 
 ### Features
-- None yet
+* Email notifications toggle API endpoint [#15930](https://github.com/CartoDB/cartodb/pull/15930)
 
 ### Bug fixes / enhancements
 * Fix BigQuery connector not importing 0-bytes-processed datasets [#15916](https://github.com/CartoDB/cartodb/pull/15916)
 * Error importing geopackage files with multiple layers [#15907](https://github.com/CartoDB/cartodb/pull/15907)
 * Add DO notification in dashboard [#15929](https://github.com/CartoDB/cartodb/pull/15929)
+* Data loss on table rename due to GhostTablesManager [#15935](https://github.com/CartoDB/cartodb/pull/15935)
 
 4.43.0 (2020-11-06)
 -------------------

app/controllers/carto/api/email_notifications_controller.rb

@@ -0,0 +1,38 @@
+module Carto
+  module Api
+    class EmailNotificationsController < ::Api::ApplicationController
+
+      ssl_required :update
+      before_action :load_notifications
+
+      rescue_from StandardError, with: :rescue_from_standard_error
+
+      def show
+        render_jsonp({ notifications: decorate_notifications }, 200)
+      end
+
+      def update
+        notifications = params.require(:notifications)
+        @carto_user.email_notifications = notifications
+
+        render_jsonp({}, 204)
+      end
+
+      private
+
+      def load_notifications
+        @carto_user = Carto::User.find(current_user.id)
+        @notifications = @carto_user.email_notifications
+      end
+
+      def decorate_notifications
+        payload = {}
+        @notifications.each do |notification|
+          payload[notification.notification] = notification.enabled
+        end
+        payload
+      end
+
+    end
+  end
+end

app/models/carto/user.rb

@@ -38,6 +38,7 @@ class Carto::User < ActiveRecord::Base
   belongs_to :rate_limit
   has_one :owned_organization, class_name: Carto::Organization, inverse_of: :owner, foreign_key: :owner_id
   has_one :static_notifications, class_name: Carto::UserNotification, inverse_of: :user
+  has_many :email_notifications, class_name: 'Carto::UserEmailNotification', inverse_of: :user, dependent: :destroy
 
   has_many :self_feature_flags_user, dependent: :destroy, foreign_key: :user_id, inverse_of: :user, class_name: Carto::FeatureFlagsUser
   has_many :self_feature_flags, through: :self_feature_flags_user, source: :feature_flag
@@ -293,6 +294,28 @@ class Carto::User < ActiveRecord::Base
     reload.dbdirect_ip
   end
 
+  # @param [Hash] notifications_hash A notification list with this format: {"notif_a" => true, "notif_b" => false}
+  def email_notifications=(notifications_hash)
+    notifications_hash.each do |key, value|
+      Carto::UserEmailNotification
+        .find_or_initialize_by(user_id: id, notification: key)
+        .update!(enabled: value)
+    end
+    reload
+  rescue StandardError => e
+    log_warning(message: 'Tried to create an invalid notification', exception: e, current_user: self)
+    raise e
+  end
+
+  def email_notification_enabled?(notification)
+    # By default, the email notifications are enabled if the row is missing.
+    email_notification = email_notifications.find { |n| n.notification == notification }
+
+    return true if email_notification.nil?
+
+    email_notification.enabled
+  end
+
   private
 
   def set_database_host

app/models/carto/user_email_notification.rb

@@ -0,0 +1,27 @@
+module Carto
+  class UserEmailNotification < ActiveRecord::Base
+
+    belongs_to :user, class_name: 'Carto::User', inverse_of: :email_notifications
+
+    validates :user, presence: true
+    validate  :valid_notification
+
+    NOTIFICATION_DO_SUBSCRIPTIONS = 'do_subscriptions'.freeze
+    VALID_NOTIFICATIONS = [NOTIFICATION_DO_SUBSCRIPTIONS].freeze
+
+    def self.new_do_subscriptions(user_id)
+      Carto::UserEmailNotification.create!(
+        user_id: user_id,
+        notification: NOTIFICATION_DO_SUBSCRIPTIONS,
+        enabled: true
+      )
+    end
+
+    private
+
+    def valid_notification
+      errors.add(:notification, 'Invalid notification') unless VALID_NOTIFICATIONS.include?(notification)
+    end
+
+  end
+end

app/models/table/user_table.rb

@@ -201,7 +201,12 @@ class UserTable < Sequel::Model
   end
 
   def destroy
-    Carto::UserTable.find_by(id: id)&.destroy
+    ar_user_table = Carto::UserTable.find_by(id: id)
+
+    return if ar_user_table.nil?
+
+    ar_user_table.set_service(service)
+    ar_user_table.destroy
   end
 
   def before_update

config/routes.rb

@@ -691,6 +691,9 @@ CartoDB::Application.routes.draw do
 
       put 'notifications/:category', to: 'static_notifications#update', as: :api_v3_static_notifications_update
 
+      get 'email_notifications', to: 'email_notifications#show', as: :api_v3_email_notifications_show
+      put 'email_notifications', to: 'email_notifications#update', as: :api_v3_email_notifications_update
+
       resources :organizations, only: [] do
         resources :notifications, only: [:create, :destroy],
                                   controller: :organization_notifications,

db/migrate/20201111102307_create_user_email_notifications.rb

@@ -0,0 +1,20 @@
+require 'carto/db/migration_helper'
+
+include Carto::Db::MigrationHelper
+
+migration(
+  proc do
+    create_table :user_email_notifications do
+      Uuid :id, primary_key: true, default: Sequel.lit('uuid_generate_v4()')
+      foreign_key :user_id, :users, type: :uuid, null: false, on_delete: :cascade
+      String :notification, null: false
+      Boolean :enabled
+      DateTime :created_at, null: false
+      DateTime :updated_at, null: false
+      index [:user_id, :notification], unique: true
+    end
+  end,
+  proc do
+    drop_table :user_email_notifications
+  end
+)

spec/models/carto/user_spec.rb

@@ -101,4 +101,95 @@ describe Carto::User do
       @user.password_reset_sent_at.to_s.should eql now.to_s
     end
   end
+
+  describe '#is_email_notification_enabled' do
+    before(:all) do
+      @carto_user = FactoryGirl.create(:carto_user)
+    end
+
+    it 'returns the notification flag if it exists' do
+      notification_type = Carto::UserEmailNotification::NOTIFICATION_DO_SUBSCRIPTIONS
+      email_notification = Carto::UserEmailNotification.create(
+        user_id: @carto_user.id,
+        notification: notification_type,
+        enabled: false
+      )
+
+      expect(@carto_user.email_notification_enabled?(notification_type)).to be_false
+      email_notification.destroy
+    end
+
+    it 'returns true as default if notification is missing' do
+      expect(@carto_user.email_notification_enabled?('missing')).to be_true
+    end
+
+    it 'cascade delete notifications if the user is destroyed' do
+      notification_type = Carto::UserEmailNotification::NOTIFICATION_DO_SUBSCRIPTIONS
+      email_notification = Carto::UserEmailNotification.create(
+        user_id: @carto_user.id,
+        notification: notification_type,
+        enabled: true
+      )
+
+      @carto_user.destroy
+      expect do
+        Carto::UserEmailNotification.find(email_notification.id)
+      end.to raise_error ActiveRecord::RecordNotFound
+    end
+
+    it 'does not create object if an invalid type is provided' do
+      email_notification = Carto::UserEmailNotification.new(
+        user_id: @carto_user.id,
+        notification: 'invalid_type',
+        enabled: true
+      )
+      expect(email_notification.valid?).to be_false
+    end
+  end
+
+  describe '#email_notification=' do
+    before(:all) do
+      @carto_user = FactoryGirl.create(:carto_user)
+    end
+
+    it 'creates a fresh set of notifications' do
+      Carto::UserEmailNotification.any_instance.stubs(:valid_notification).returns(true)
+      @carto_user.email_notifications = {
+        notif_a: true,
+        notif_b: false
+      }
+
+      expect(@carto_user.email_notifications.length).to eq 2
+      expect(@carto_user.email_notification_enabled?('notif_a')).to be_true
+      expect(@carto_user.email_notification_enabled?('notif_b')).to be_false
+    end
+
+    it 'updates notifications if they already exist' do
+      Carto::UserEmailNotification.any_instance.stubs(:valid_notification).returns(true)
+      Carto::UserEmailNotification.create(
+        user_id: @carto_user.id,
+        notification: 'existing_notification',
+        enabled: true
+      )
+
+      @carto_user.email_notifications = {
+        notif_a: true,
+        notif_b: false,
+        existing_notification: false
+      }
+
+      expect(@carto_user.email_notifications.length).to eq 3
+      expect(@carto_user.email_notification_enabled?('notif_a')).to be_true
+      expect(@carto_user.email_notification_enabled?('notif_b')).to be_false
+      expect(@carto_user.email_notification_enabled?('existing_notification')).to be_false
+    end
+
+    it 'raises an error if an invalid notification is set' do
+      expect do
+        @carto_user.email_notifications = {
+          notif_a: true
+        }
+      end.to raise_error StandardError
+    end
+  end
 end

spec/requests/carto/api/email_notifications_controller_spec.rb

@@ -0,0 +1,53 @@
+require 'spec_helper_min'
+require 'support/helpers'
+
+describe Carto::Api::EmailNotificationsController do
+  include HelperMethods
+
+  before(:all) do
+    @carto_user = FactoryGirl.create(:carto_user)
+    @carto_user.email_notifications = {
+      do_subscriptions: true
+    }
+  end
+
+  let(:auth_params) do
+    { user_domain: @carto_user.username, api_key: @carto_user.api_key }
+  end
+
+  describe '#show' do
+    it 'list the current notifications' do
+      get_json(api_v3_email_notifications_show_url(auth_params)) do |response|
+        response.status.should eq 200
+        response.body.should eq({ notifications: { do_subscriptions: true } })
+      end
+    end
+
+    it 'return error if unauthenticated' do
+      get_json(api_v3_email_notifications_show_url({})) do |response|
+        response.status.should eq 401
+      end
+    end
+  end
+
+  describe '#update' do
+    it 'return error if unauthenticated' do
+      put_json(api_v3_email_notifications_update_url({})) do |response|
+        response.status.should eq 401
+      end
+    end
+
+    it 'returns error if an invalid notification is provided' do
+      put_json(api_v3_email_notifications_update_url(auth_params), { notifications: { invalid: true } }) do |response|
+        response.status.should eq 500
+      end
+    end
+
+    it 'successfully updates notifications' do
+      params = { notifications: { do_subscriptions: false } }
+      put_json(api_v3_email_notifications_update_url(auth_params), params) do |response|
+        response.status.should eq 204
+      end
+    end
+  end
+end