From d14a5ab099371774d832f8b9f82e65fb8bb30816 Mon Sep 17 00:00:00 2001
From: Finn Bacall <finn.bacall@manchester.ac.uk>
Date: Fri, 4 Aug 2023 11:11:30 +0100
Subject: [PATCH] Hide map on index page if no API key. Fixes #879

Also tweak custom Config getters to be dynamic.
---
 app/models/event.rb                           |  4 +-
 app/views/events/index.html.erb               |  4 +-
 config/application.rb                         | 31 ++++++++++----
 test/config/test_secrets.yml                  |  2 +-
 test/controllers/events_controller_test.rb    | 41 +++++++++++++++++++
 test/fixtures/edit_suggestions.yml            |  7 +++-
 .../cookie_consent_integration_test.rb        | 14 +++----
 test/models/event_test.rb                     | 14 +++++++
 8 files changed, 94 insertions(+), 23 deletions(-)

diff --git a/app/models/event.rb b/app/models/event.rb
index 2a54da1d4..1b5d0a419 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -229,9 +229,7 @@ def to_ical_event
   end
 
   def show_map?
-    Rails.application.secrets.google_maps_api_key.present? && # !self.online? &&
-      ((latitude.present? && longitude.present?) ||
-        (suggested_latitude.present? && suggested_longitude.present?))
+    TeSS::Config.map_enabled && ((latitude.present? && longitude.present?) || (suggested_latitude.present? && suggested_longitude.present?))
   end
 
   def all_day?
diff --git a/app/views/events/index.html.erb b/app/views/events/index.html.erb
index 11ac9831f..62f1b77dc 100644
--- a/app/views/events/index.html.erb
+++ b/app/views/events/index.html.erb
@@ -21,7 +21,7 @@
     <% content_for :display_options do %>
       <ul class="nav nav-xs nav-pills index-display-options">
         <%= tab('List', 'fa fa-list', 'home', active: true) %>
-        <% if !TeSS::Config.feature['disabled'].include? 'events_map' %>
+        <% if TeSS::Config.map_enabled %>
           <%= tab('Map', 'fa fa-globe', 'map',
                   disabled: { check: (search_and_facet_params[:online] == 'true'),
                               message: 'Only showing online events.' }) %>
@@ -42,7 +42,7 @@
           <%= render partial: "search/common/pagination_bar", locals: { resources: @events } %>
         </div>
 
-        <% unless TeSS::Config.feature['disabled'].include?('events_map') %>
+        <% if TeSS::Config.map_enabled %>
           <div id="map" class="tab-pane fade">
             <div id="map-count" class="search-results-count"></div>
             <div id="map-content">
diff --git a/config/application.rb b/config/application.rb
index b99020dbe..59d0fbe52 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -72,18 +72,31 @@ def self.merge_config(default_config, config, current_path = '')
 
   merge_config(Rails.configuration.tess_defaults.with_indifferent_access, tess_config)
 
-  Config = OpenStruct.new(tess_config)
+  class TessConfig < OpenStruct
+    def redis_url
+      if Rails.env.test?
+        ENV.fetch('REDIS_TEST_URL') { 'redis://localhost:6379/0' }
+      else
+        ENV.fetch('REDIS_URL') { 'redis://localhost:6379/1' }
+      end
+    end
 
-  Config.redis_url = if Rails.env.test?
-                       ENV.fetch('REDIS_TEST_URL') { 'redis://localhost:6379/0' }
-                     else
-                       ENV.fetch('REDIS_URL') { 'redis://localhost:6379/1' }
-                     end
+    def ingestion
+      return @ingestion if @ingestion
+      config_file = File.join(Rails.root, 'config', 'ingestion.yml')
+      @ingestion = YAML.safe_load(File.read(config_file)).deep_symbolize_keys! if File.exist?(config_file)
+    end
 
-  config_file = File.join(Rails.root, 'config', 'ingestion.yml')
-  Config.ingestion = YAML.safe_load(File.read(config_file)).deep_symbolize_keys! if File.exist?(config_file)
+    def analytics_enabled
+      force_analytics_enabled || (Rails.application.secrets.google_analytics_code.present? && Rails.env.production?)
+    end
+
+    def map_enabled
+      !feature['disabled'].include?('events_map') && Rails.application.secrets.google_maps_api_key.present?
+    end
+  end
 
-  Config.analytics_enabled = Rails.application.secrets.google_analytics_code.present? && Rails.env.production?
+  Config = TessConfig.new(tess_config)
 
   tess_base_uri = URI.parse(TeSS::Config.base_url)
   Rails.application.default_url_options = {
diff --git a/test/config/test_secrets.yml b/test/config/test_secrets.yml
index fefc1dfc8..d80a913e1 100644
--- a/test/config/test_secrets.yml
+++ b/test/config/test_secrets.yml
@@ -1,4 +1,4 @@
-google_maps_api_key:
+google_maps_api_key: 123xyz
 recaptcha:
   sitekey: test
   secret: test
diff --git a/test/controllers/events_controller_test.rb b/test/controllers/events_controller_test.rb
index 0c6fd7d63..3792439eb 100644
--- a/test/controllers/events_controller_test.rb
+++ b/test/controllers/events_controller_test.rb
@@ -1324,6 +1324,9 @@ class EventsControllerTest < ActionController::TestCase
   end
 
   test 'should hide map tab if disabled' do
+    assert Rails.application.secrets.google_maps_api_key.present?
+    assert TeSS::Config.map_enabled
+
     get :index
     assert_response :success
     assert_select '#content .nav' do
@@ -1331,6 +1334,22 @@ class EventsControllerTest < ActionController::TestCase
     end
 
     with_settings(feature: { disabled: ['events_map'] }) do
+      assert Rails.application.secrets.google_maps_api_key.present?
+      refute TeSS::Config.map_enabled
+
+      get :index
+      assert_response :success
+      assert_select '#content .nav' do
+        assert_select 'li a[href=?]', '#map', count: 0
+      end
+    end
+  end
+
+  test 'should hide map tab if no API key' do
+    Rails.application.secrets.stub(:google_maps_api_key, nil) do
+      refute Rails.application.secrets.google_maps_api_key.present?
+      refute TeSS::Config.map_enabled
+
       get :index
       assert_response :success
       assert_select '#content .nav' do
@@ -1354,4 +1373,26 @@ class EventsControllerTest < ActionController::TestCase
     get :clone, params: { id: @event }
     assert_response :forbidden
   end
+
+  test 'should show map if location data' do
+    get :show, params: { id: events(:one) }
+    assert_response :success
+    assert_select '#map'
+  end
+
+  test 'should not show map if no location data' do
+    get :show, params: { id: events(:portal_event) }
+    assert_response :success
+    assert_select '#map', count: 0
+  end
+
+  test 'should not show map if disabled' do
+    with_settings(feature: { disabled: ['events_map'] }) do
+      assert Rails.application.secrets.google_maps_api_key.present?
+      refute TeSS::Config.map_enabled
+      get :show, params: { id: events(:one) }
+      assert_response :success
+      assert_select '#map', count: 0
+    end
+  end
 end
diff --git a/test/fixtures/edit_suggestions.yml b/test/fixtures/edit_suggestions.yml
index ba752a3dd..32e5b81a2 100644
--- a/test/fixtures/edit_suggestions.yml
+++ b/test/fixtures/edit_suggestions.yml
@@ -16,4 +16,9 @@ multiple_fields:
   suggestible: iann_event (Event)
   data_fields:
     title: banana
-    latitude: 15
\ No newline at end of file
+    latitude: 15
+
+location_suggestions:
+  suggestible: kilburn (Event)
+  data_fields:
+    geographic_coordinates: [43, 43]
diff --git a/test/integration/cookie_consent_integration_test.rb b/test/integration/cookie_consent_integration_test.rb
index d2ac2734d..899599aa7 100644
--- a/test/integration/cookie_consent_integration_test.rb
+++ b/test/integration/cookie_consent_integration_test.rb
@@ -16,7 +16,7 @@ class CookieConsentIntegrationTest < ActionDispatch::IntegrationTest
   end
 
   test 'cookie consent banner shown with tracking option if analytics enabled' do
-    with_settings({ require_cookie_consent: true, analytics_enabled: true }) do
+    with_settings({ require_cookie_consent: true, force_analytics_enabled: true }) do
       get root_path
 
       assert_select '#cookie-banner' do
@@ -51,7 +51,7 @@ class CookieConsentIntegrationTest < ActionDispatch::IntegrationTest
   end
 
   test 'analytics code not present if only necessary cookies allowed' do
-    with_settings({ require_cookie_consent: true, analytics_enabled: true }) do
+    with_settings({ require_cookie_consent: true, force_analytics_enabled: true }) do
       post cookies_consent_path, params: { allow: 'necessary' }
 
       get root_path
@@ -62,7 +62,7 @@ class CookieConsentIntegrationTest < ActionDispatch::IntegrationTest
   end
 
   test 'analytics code present if only all cookies allowed' do
-    with_settings({ require_cookie_consent: true, analytics_enabled: true }) do
+    with_settings({ require_cookie_consent: true, force_analytics_enabled: true }) do
       post cookies_consent_path, params: { allow: all_options }
 
       get root_path
@@ -73,7 +73,7 @@ class CookieConsentIntegrationTest < ActionDispatch::IntegrationTest
   end
 
   test 'analytics code present if cookie consent not required' do
-    with_settings({ require_cookie_consent: false, analytics_enabled: true }) do
+    with_settings({ require_cookie_consent: false, force_analytics_enabled: true }) do
       post cookies_consent_path, params: { allow: 'necessary' }
 
       get root_path
@@ -86,7 +86,7 @@ class CookieConsentIntegrationTest < ActionDispatch::IntegrationTest
   end
 
   test 'can access and use cookie consent page as anonymous user' do
-    with_settings({ require_cookie_consent: true, analytics_enabled: true }) do
+    with_settings({ require_cookie_consent: true, force_analytics_enabled: true }) do
       get cookies_consent_path
 
       assert_nil User.current_user
@@ -122,7 +122,7 @@ class CookieConsentIntegrationTest < ActionDispatch::IntegrationTest
     user = users(:regular_user)
     post '/users/sign_in', params: { 'user[login]' => user.username, 'user[password]' => 'hello' }
 
-    with_settings({ require_cookie_consent: true, analytics_enabled: true }) do
+    with_settings({ require_cookie_consent: true, force_analytics_enabled: true }) do
       get cookies_consent_path
 
       assert_equal user, User.current_user
@@ -152,7 +152,7 @@ class CookieConsentIntegrationTest < ActionDispatch::IntegrationTest
   end
 
   test 'revoke consent' do
-    with_settings({ require_cookie_consent: true, analytics_enabled: true }) do
+    with_settings({ require_cookie_consent: true, force_analytics_enabled: true }) do
       post cookies_consent_path, params: { allow: 'necessary' }
       follow_redirect!
       assert_select '#flash-container .alert-danger', count: 0
diff --git a/test/models/event_test.rb b/test/models/event_test.rb
index 922c18ccc..19bd1348d 100644
--- a/test/models/event_test.rb
+++ b/test/models/event_test.rb
@@ -543,4 +543,18 @@ class EventTest < ActiveSupport::TestCase
     assert_equal 'Event  Title', @event.title
     assert_equal 'https://event.com', @event.url
   end
+
+  test 'show_map?' do
+    assert TeSS::Config.map_enabled
+
+    assert events(:one).show_map?
+    refute events(:portal_event).show_map?
+    assert events(:kilburn).suggested_latitude
+    assert events(:kilburn).show_map?
+
+    with_settings(feature: { disabled: ['events_map'] }) do
+      refute TeSS::Config.map_enabled
+      refute events(:one).show_map?
+    end
+  end
 end