Merge pull request #15315 from CartoDB/do-regular-licensing

[DO] Regular licensing

Makefile

@@ -358,6 +358,7 @@ SPEC_HELPER_MIN_SPECS = \
 	spec/requests/carto/api/public/data_observatory_controller_spec.rb \
 	spec/lib/tasks/data_observatory_rake_spec.rb \
 	spec/services/carto/do_licensing_service_spec.rb \
+	spec/mailers/data_observatory_mailer_spec.rb \
 	$(NULL)
 
 # This class must be tested isolated as pollutes namespace

NEWS.md

@@ -5,9 +5,10 @@ Development
 - None yet
 
 ### Features
-* BigQuery Connector beta release
-* Add new parameter `import_as` to odbc connectors ([#15266](https://github.com/CartoDB/cartodb/pull/15266))
-* Add support for Storage API to BigQuery connector, make it public, and allow separate a billing project
+- Regular licensing for Data Observatory ([#15315](https://github.com/CartoDB/cartodb/pull/15315))
+- BigQuery Connector beta release
+- Add new parameter `import_as` to odbc connectors ([#15266](https://github.com/CartoDB/cartodb/pull/15266))
+- Add support for Storage API to BigQuery connector, make it public, and allow separate a billing project
   ([#15266](https://github.com/CartoDB/cartodb/pull/15266))
 * Split into Upload/connect tabs in new Connectors UI ([#15207](https://github.com/CartoDB/cartodb/issues/15207))
 * New Connectors UI layout([#15194](https://github.com/CartoDB/cartodb/issues/15194))

app/controllers/carto/api/public/data_observatory_controller.rb

@@ -13,7 +13,7 @@ module Carto
         before_action :load_id, only: [:subscription_info, :subscribe, :unsubscribe]
         before_action :load_type, only: [:subscription_info, :subscribe]
         before_action :check_api_key_permissions
-        before_action :check_licensing_enabled, only: [:subscription_info, :subscribe, :unsubscribe]
+        before_action :check_do_enabled, only: [:subscription_info, :subscriptions]
 
         setup_default_rescues
 
@@ -26,6 +26,7 @@ module Carto
         METADATA_FIELDS = %i(id estimated_delivery_days subscription_list_price tos tos_link licenses licenses_link
                              rights type).freeze
         TABLES_BY_TYPE = { 'dataset' => 'datasets', 'geography' => 'geographies' }.freeze
+        REQUIRED_METADATA_FIELDS = %i(available_in estimated_delivery_days subscription_list_price).freeze
 
         def token
           response = Cartodb::Central.new.get_do_token(@user.username)
@@ -46,19 +47,27 @@ module Carto
 
         def subscribe
           metadata = subscription_metadata
+
+          instant_licensing_available?(metadata) ? instant_license(metadata) : regular_license(metadata)
+
           response = present_metadata(metadata)
+          render(json: response)
+        end
 
-          return render(json: response) if metadata[:estimated_delivery_days].positive?
+        def instant_licensing_available?(metadata)
+          @user.has_feature_flag?('do-instant-licensing') &&
+            REQUIRED_METADATA_FIELDS.all? { |field| metadata[field].present? } &&
+            metadata[:estimated_delivery_days].zero?
+        end
 
-          license_info = {
-            dataset_id: metadata[:id],
-            available_in: metadata[:available_in],
-            price: metadata[:subscription_list_price],
-            expires_at: Time.now.round + 1.year
-          }
-          Carto::DoLicensingService.new(@user.username).subscribe([license_info])
+        def instant_license(metadata)
+          licensing_service = Carto::DoLicensingService.new(@user.username)
+          licensing_service.subscribe([license_info(metadata)])
+        end
 
-          render(json: response)
+        def regular_license(metadata)
+          DataObservatoryMailer.user_request(@user, metadata[:id], metadata[:name]).deliver_now
+          DataObservatoryMailer.carto_request(@user, metadata[:id], metadata[:estimated_delivery_days]).deliver_now
         end
 
         def unsubscribe
@@ -97,8 +106,8 @@ module Carto
           raise UnauthorizedError unless api_key&.master? || api_key&.data_observatory_permissions?
         end
 
-        def check_licensing_enabled
-          raise UnauthorizedError.new('DO licensing not enabled') unless @user.has_feature_flag?('do-licensing')
+        def check_do_enabled
+          Cartodb::Central.new.check_do_enabled(@user.username)
         end
 
         def rescue_from_central_error(exception)
@@ -137,21 +146,19 @@ module Carto
           query = "SELECT *, '#{@type}' as type FROM #{TABLES_BY_TYPE[@type]} WHERE id = '#{@id}'"
 
           result = metadata_user.in_database[query].first
-          validate_metadata(result)
-        end
-
-        def validate_metadata(result)
           raise Carto::LoadError.new("No metadata found for #{@id}") unless result
 
-          valid_data = result[:available_in].present? && result[:estimated_delivery_days].present? &&
-                       result[:subscription_list_price].present?
-          unless valid_data
-            CartoDB::Logger.info(message: 'Incomplete DO metadata', id: @id)
-            raise Carto::LoadError.new("Incomplete metadata found for #{@id}")
-          end
-
           result
         end
+
+        def license_info(metadata)
+          {
+            dataset_id: metadata[:id],
+            available_in: metadata[:available_in],
+            price: metadata[:subscription_list_price],
+            expires_at: Time.now.round + 1.year
+          }
+        end
       end
     end
   end

app/mailers/data_observatory_mailer.rb

@@ -0,0 +1,26 @@
+class DataObservatoryMailer < ActionMailer::Base
+
+  CARTO_REQUEST_RECIPIENT = 'dataobservatory@carto.com'.freeze
+
+  default from: Cartodb.get_config(:mailer, 'from')
+  layout 'mail'
+
+  def user_request(user, dataset_id, dataset_name)
+    subject = 'Your dataset request to CARTO'
+    @user_name = user.name
+    @dataset_id = dataset_id
+    @dataset_name = dataset_name
+
+    mail to: user.email, subject: subject
+  end
+
+  def carto_request(user, dataset_id, delivery_days)
+    subject = 'Dataset request'
+    @user_name = user.name
+    @user_email = user.email
+    @dataset_id = dataset_id
+    @delivery_days = delivery_days
+
+    mail to: CARTO_REQUEST_RECIPIENT, subject: subject
+  end
+end

app/views/data_observatory_mailer/carto_request.html.erb

@@ -0,0 +1,11 @@
+<tr>
+  <td align="left" style="font-size: 14px; line-height: 25px; font-family: 'Open Sans', Helvetica, Arial, sans-serif; color: #647083;">
+    <p>Hi!</p><br>
+    <p>There is a new dataset request for the Data Observatory.</p><br>
+    <p>Customer name: <%= @user_name %></p>
+    <p>Customer email: <a href="mailto:<%= @user_email %>"><%= @user_email %></a></p>
+    <p>Dataset: <%= @dataset_id %></p>
+    <p>Estimated delivery time: <%= @delivery_days %> days</p><br>
+    <p>VAMOS!</p>
+  </td>
+</tr>

app/views/data_observatory_mailer/user_request.html.erb

@@ -0,0 +1,9 @@
+<tr>
+  <td align="left" style="font-size: 14px; line-height: 25px; font-family: 'Open Sans', Helvetica, Arial, sans-serif; color: #647083;">
+    <p>Hi <%= @user_name %>,</p><br>
+    <p>Thanks for requesting the <%= @dataset_name %> dataset (id: <%= @dataset_id %>).</p>
+    <p>We are already working on it and we will get in touch with you shortly.</p><br>
+    <p>Best regards,</p>
+    <p>The CARTO team.</p>
+  </td>
+</tr>

lib/cartodb/central.rb

@@ -90,6 +90,10 @@ module Cartodb
       send_request("api/users/#{username}", nil, :delete, [204, 404])
     end
 
+    def check_do_enabled(username)
+      send_request("api/users/#{username}/do_status", nil, :get, [200])
+    end
+
     def get_do_token(username)
       send_request("api/users/#{username}/do_token", nil, :get, [200])
     end

spec/mailers/data_observatory_mailer_spec.rb

@@ -0,0 +1,97 @@
+require 'spec_helper_min'
+
+describe DataObservatoryMailer do
+
+  describe ".user_request" do
+    before(:all) do
+      @user = FactoryGirl.create(:carto_user, email: 'fulano@example.com', name: 'Fulano')
+    end
+
+    before(:each) do
+      @mailer = DataObservatoryMailer.user_request(@user, 'carto.open-data.demographics', 'Demographics')
+    end
+
+    after(:each) do
+      ActionMailer::Base.deliveries = []
+    end
+
+    after(:all) do
+      @user.destroy
+    end
+
+    it "delivers the mail" do
+      expect { @mailer.deliver_now }.to change(ActionMailer::Base.deliveries, :size).by(1)
+    end
+
+    it "delivers with the expected subject" do
+      @mailer.deliver_now
+
+      mail = ActionMailer::Base.deliveries.first
+      expect(mail.subject).to eql('Your dataset request to CARTO')
+    end
+
+    it "delivers to the expected recipients" do
+      @mailer.deliver_now
+
+      mail = ActionMailer::Base.deliveries.first
+      expect(mail.to).to eql([@user.email])
+    end
+
+    it "delivers a text including the requested dataset id" do
+      @mailer.deliver_now
+
+      mail = ActionMailer::Base.deliveries.first
+      expect(mail.body).to include('carto.open-data.demographics')
+    end
+
+    it "delivers a text including the requested dataset name" do
+      @mailer.deliver_now
+
+      mail = ActionMailer::Base.deliveries.first
+      expect(mail.body).to include('Demographics')
+    end
+  end
+
+  describe ".carto_request" do
+    before(:all) do
+      @user = FactoryGirl.create(:carto_user, email: 'fulano@example.com', name: 'Fulano')
+    end
+
+    before(:each) do
+      @mailer = DataObservatoryMailer.carto_request(@user, 'carto.open-data.demographics', 3)
+    end
+
+    after(:each) do
+      ActionMailer::Base.deliveries = []
+    end
+
+    after(:all) do
+      @user.destroy
+    end
+
+    it "delivers the mail" do
+      expect { @mailer.deliver_now }.to change(ActionMailer::Base.deliveries, :size).by(1)
+    end
+
+    it "delivers with the expected subject" do
+      @mailer.deliver_now
+
+      mail = ActionMailer::Base.deliveries.first
+      expect(mail.subject).to eql('Dataset request')
+    end
+
+    it "delivers to the expected recipients" do
+      @mailer.deliver_now
+
+      mail = ActionMailer::Base.deliveries.first
+      expect(mail.to).to eql(['dataobservatory@carto.com'])
+    end
+
+    it "delivers a text including the requested dataset id" do
+      @mailer.deliver_now
+
+      mail = ActionMailer::Base.deliveries.first
+      expect(mail.body).to include('carto.open-data.demographics')
+    end
+  end
+end

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

@@ -14,7 +14,7 @@ describe Carto::Api::Public::DataObservatoryController do
     @granted_token = @user1.api_keys.create_regular_key!(name: 'do', grants: do_grants).token
     @headers = { 'CONTENT_TYPE' => 'application/json' }
     populate_do_metadata
-    @feature_flag = FactoryGirl.create(:feature_flag, name: 'do-licensing', restricted: true)
+    @feature_flag = FactoryGirl.create(:feature_flag, name: 'do-instant-licensing', restricted: true)
     Carto::FeatureFlagsUser.create(user_id: @user1.id, feature_flag_id: @feature_flag.id)
   end
 
@@ -120,8 +120,24 @@ describe Carto::Api::Public::DataObservatoryController do
       $users_metadata.del(@redis_key)
     end
 
+    before(:each) do
+      Cartodb::Central.any_instance.stubs(:check_do_enabled).returns(true)
+    end
+
+    after(:each) do
+      Cartodb::Central.any_instance.unstub(:check_do_enabled)
+    end
+
     it_behaves_like 'an endpoint validating a DO API key'
 
+    it 'checks if DO is enabled' do
+      central_mock = mock
+      Cartodb::Central.stubs(:new).returns(central_mock)
+      central_mock.expects(:check_do_enabled).once.returns(true)
+
+      get_json endpoint_url(api_key: @master), @headers
+    end
+
     it 'returns 200 with the non expired subscriptions' do
       expected_dataset = { project: 'carto', dataset: 'abc', table: 'table2', id: 'carto.abc.table2', type: 'dataset' }
       get_json endpoint_url(api_key: @master), @headers do |response|
@@ -212,6 +228,14 @@ describe Carto::Api::Public::DataObservatoryController do
   end
 
   describe 'subscription_info' do
+    before(:each) do
+      Cartodb::Central.any_instance.stubs(:check_do_enabled).returns(true)
+    end
+
+    after(:each) do
+      Cartodb::Central.any_instance.unstub(:check_do_enabled)
+    end
+
     before(:all) do
       @url_helper = 'api_v4_do_subscription_info_url'
       @params = { id: 'carto.abc.dataset1', type: 'dataset' }
@@ -219,6 +243,14 @@ describe Carto::Api::Public::DataObservatoryController do
 
     it_behaves_like 'an endpoint validating a DO API key'
 
+    it 'checks if DO is enabled' do
+      central_mock = mock
+      Cartodb::Central.stubs(:new).returns(central_mock)
+      central_mock.expects(:check_do_enabled).once.returns(true)
+
+      get_json endpoint_url(api_key: @master, id: 'carto.abc.dataset1', type: 'dataset'), @headers
+    end
+
     it 'returns 400 if the id param is not valid' do
       get_json endpoint_url(api_key: @master, id: 'wrong'), @headers do |response|
         expect(response.status).to eq(400)
@@ -237,15 +269,6 @@ describe Carto::Api::Public::DataObservatoryController do
       end
     end
 
-    it 'returns 403 if the feature flag is not enabled for the user' do
-      with_feature_flag @user1, 'do-licensing', false do
-        get_json endpoint_url(api_key: @master, id: 'carto.abc.dataset1', type: 'dataset'), @headers do |response|
-          expect(response.status).to eq(403)
-          expect(response.body).to eq(errors: "DO licensing not enabled", errors_cause: nil)
-        end
-      end
-    end
-
     it 'returns 404 if the metadata user does not exist' do
       Carto::User.find_by_username('do-metadata').destroy
 
@@ -326,15 +349,6 @@ describe Carto::Api::Public::DataObservatoryController do
       end
     end
 
-    it 'returns 403 if the feature flag is not enabled for the user' do
-      with_feature_flag @user1, 'do-licensing', false do
-        post_json endpoint_url(api_key: @master), @payload do |response|
-          expect(response.status).to eq(403)
-          expect(response.body).to eq(errors: "DO licensing not enabled", errors_cause: nil)
-        end
-      end
-    end
-
     it 'returns 404 if the metadata user does not exist' do
       Carto::User.find_by_username('do-metadata').destroy
 
@@ -353,13 +367,6 @@ describe Carto::Api::Public::DataObservatoryController do
       end
     end
 
-    it 'returns 404 if the dataset metadata is incomplete' do
-      post_json endpoint_url(api_key: @master), id: 'carto.abc.incomplete', type: 'dataset' do |response|
-        expect(response.status).to eq(404)
-        expect(response.body).to eq(errors: "Incomplete metadata found for carto.abc.incomplete", errors_cause: nil)
-      end
-    end
-
     it 'returns 500 with an explicit message if the central call fails' do
       central_response = OpenStruct.new(code: 500, body: { errors: ['boom'] }.to_json)
       central_error = CartoDB::CentralCommunicationFailure.new(central_response)
@@ -371,59 +378,91 @@ describe Carto::Api::Public::DataObservatoryController do
       end
     end
 
-    it 'returns 200 with the dataset metadata and calls the DoLicensingService with the expected params' do
-      expected_params = [{
-        dataset_id: 'carto.abc.dataset1',
-        available_in: ['bq'],
-        price: 100.0,
-        expires_at: Time.parse('2019/01/01 00:00:00')
-      }]
+    it 'sends request emails if the dataset metadata is incomplete' do
+      mailer_mock = stub(:deliver_now)
+      dataset_id = 'carto.abc.incomplete'
+      dataset_name = 'Incomplete dataset'
+      DataObservatoryMailer.expects(:user_request).with(@carto_user1, dataset_id, dataset_name).once
+                           .returns(mailer_mock)
+      DataObservatoryMailer.expects(:carto_request).with(@carto_user1, dataset_id, 0.0).once.returns(mailer_mock)
 
-      mock_service = mock
-      mock_service.expects(:subscribe).with(expected_params).once
-      Carto::DoLicensingService.expects(:new).with(@user1.username).once.returns(mock_service)
+      post_json endpoint_url(api_key: @master), id: dataset_id, type: 'dataset' do |response|
+        expect(response.status).to eq(200)
+      end
+    end
 
-      Delorean.time_travel_to '2018/01/01 00:00:00' do
+    it 'sends request emails if instant licensing is not enabled for the user' do
+      mailer_mock = stub(:deliver_now)
+      dataset_id = @payload[:id]
+      dataset_name = 'CARTO dataset 1'
+      DataObservatoryMailer.expects(:user_request).with(@carto_user1, dataset_id, dataset_name).once
+                           .returns(mailer_mock)
+      DataObservatoryMailer.expects(:carto_request).with(@carto_user1, dataset_id, 0.0).once.returns(mailer_mock)
+
+      with_feature_flag @user1, 'do-instant-licensing', false do
         post_json endpoint_url(api_key: @master), @payload do |response|
           expect(response.status).to eq(200)
+        end
+      end
+    end
+
+    it 'sends request emails if the delivery time is not 0' do
+      mailer_mock = stub(:deliver_now)
+      dataset_id = 'carto.abc.geography1'
+      dataset_name = 'CARTO geography 1'
+      DataObservatoryMailer.expects(:user_request).with(@carto_user1, dataset_id, dataset_name).once.returns(mailer_mock)
+      DataObservatoryMailer.expects(:carto_request).with(@carto_user1, dataset_id, 3.0).once.returns(mailer_mock)
+      Carto::DoLicensingService.expects(:new).never
+
+      Delorean.time_travel_to '2018/01/01 00:00:00' do
+        post_json endpoint_url(api_key: @master), id: 'carto.abc.geography1', type: 'geography' do |response|
+          expect(response.status).to eq(200)
           expected_response = {
-            estimated_delivery_days: 0.0,
-            id: 'carto.abc.dataset1',
+            estimated_delivery_days: 3.0,
+            id: 'carto.abc.geography1',
             licenses: 'licenses',
             licenses_link: 'licenses_link',
             rights: 'rights',
-            subscription_list_price: 100.0,
+            subscription_list_price: 90.0,
             tos: 'tos',
             tos_link: 'tos_link',
-            type: 'dataset'
+            type: 'geography'
           }
           expect(response.body).to eq expected_response
         end
       end
     end
 
-    it 'returns 200 with the dataset metadata without calling DoLicensingService when the delivery time is not 0' do
-      Carto::DoLicensingService.expects(:new).never
+    it 'returns 200 with the dataset metadata and calls the DoLicensingService with the expected params' do
+      expected_params = [{
+        dataset_id: 'carto.abc.dataset1',
+        available_in: ['bq'],
+        price: 100.0,
+        expires_at: Time.parse('2019/01/01 00:00:00')
+      }]
+
+      mock_service = mock
+      mock_service.expects(:subscribe).with(expected_params).once
+      Carto::DoLicensingService.expects(:new).with(@user1.username).once.returns(mock_service)
 
       Delorean.time_travel_to '2018/01/01 00:00:00' do
-        post_json endpoint_url(api_key: @master), id: 'carto.abc.geography1', type: 'geography' do |response|
+        post_json endpoint_url(api_key: @master), @payload do |response|
           expect(response.status).to eq(200)
           expected_response = {
-            estimated_delivery_days: 3.0,
-            id: 'carto.abc.geography1',
+            estimated_delivery_days: 0.0,
+            id: 'carto.abc.dataset1',
             licenses: 'licenses',
             licenses_link: 'licenses_link',
             rights: 'rights',
-            subscription_list_price: 90.0,
+            subscription_list_price: 100.0,
             tos: 'tos',
             tos_link: 'tos_link',
-            type: 'geography'
+            type: 'dataset'
           }
           expect(response.body).to eq expected_response
         end
       end
     end
-
   end
 
   describe 'unsubscribe' do
@@ -439,15 +478,6 @@ describe Carto::Api::Public::DataObservatoryController do
       end
     end
 
-    it 'returns 403 if the feature flag is not enabled for the user' do
-      with_feature_flag @user1, 'do-licensing', false do
-        delete_json endpoint_url(@params) do |response|
-          expect(response.status).to eq(403)
-          expect(response.body).to eq(errors: "DO licensing not enabled", errors_cause: nil)
-        end
-      end
-    end
-
     it 'returns 204 calls the DoLicensingService with the expected params' do
       mock_service = mock
       mock_service.expects(:unsubscribe).with('carto.abc.dataset1').once
@@ -463,15 +493,17 @@ describe Carto::Api::Public::DataObservatoryController do
     metadata_user = FactoryGirl.create(:user, username: 'do-metadata')
     db_seed = %{
       CREATE TABLE datasets(id text, estimated_delivery_days numeric, subscription_list_price numeric, tos text,
-                            tos_link text, licenses text, licenses_link text, rights text, available_in text[]);
+                            tos_link text, licenses text, licenses_link text, rights text, available_in text[], 
+                            name text);
       INSERT INTO datasets VALUES ('carto.abc.dataset1', 0.0, 100.0, 'tos', 'tos_link', 'licenses', 'licenses_link',
-                                   'rights', '{bq}');
+                                   'rights', '{bq}', 'CARTO dataset 1');
       INSERT INTO datasets VALUES ('carto.abc.incomplete', 0.0, 100.0, 'tos', 'tos_link', 'licenses', 'licenses_link',
-                                   'rights', NULL);
+                                   'rights', NULL, 'Incomplete dataset');
       CREATE TABLE geographies(id text, estimated_delivery_days numeric, subscription_list_price numeric, tos text,
-                               tos_link text, licenses text, licenses_link text, rights text, available_in text[]);
+                               tos_link text, licenses text, licenses_link text, rights text, available_in text[],
+                               name text);
       INSERT INTO geographies VALUES ('carto.abc.geography1', 3.0, 90.0, 'tos', 'tos_link', 'licenses', 'licenses_link',
-                                      'rights', '{bq}');
+                                      'rights', '{bq}', 'CARTO geography 1');
     }
     metadata_user.in_database.run(db_seed)
   end