From 2b0a812543bc563589d235c73151df84a66107fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?=
<15726+Senen@users.noreply.github.com>
Date: Wed, 24 May 2023 16:29:05 +0200
Subject: [PATCH 1/2] Add method to retrieve translations in one database query
This methods performs much better when we need to load a lot of
globalized models translations and returns the best fallback translation
for the current language.
---
app/models/concerns/globalizable.rb | 17 ++++++++
spec/models/concerns/globalizable.rb | 65 +++++++++++++++++++++++++---
2 files changed, 77 insertions(+), 5 deletions(-)
diff --git a/app/models/concerns/globalizable.rb b/app/models/concerns/globalizable.rb
index c2e04554f..0b995350d 100644
--- a/app/models/concerns/globalizable.rb
+++ b/app/models/concerns/globalizable.rb
@@ -100,5 +100,22 @@ module Globalizable
def translation_class_delegate(method)
translation_class.instance_eval { delegate method, to: :globalized_model }
end
+
+ def with_fallback_translation
+ translations_foreign_key = reflect_on_association(:translations).foreign_key
+ fallbacks = Globalize.fallbacks(Globalize.locale)
+
+ fallbacks_with_order = fallbacks.map.with_index do |locale, order|
+ "('#{locale}', #{order})"
+ end.join(", ")
+
+ translations_ids = translation_class
+ .select("DISTINCT ON (#{translations_foreign_key}) id")
+ .where(locale: fallbacks)
+ .joins("LEFT JOIN (VALUES #{fallbacks_with_order}) AS locales(name, ordering) ON locale = locales.name")
+ .order(translations_foreign_key, "locales.ordering")
+
+ with_translations(fallbacks).where("#{translations_table_name}.id": translations_ids)
+ end
end
end
diff --git a/spec/models/concerns/globalizable.rb b/spec/models/concerns/globalizable.rb
index 0eca3b268..fd74d09c1 100644
--- a/spec/models/concerns/globalizable.rb
+++ b/spec/models/concerns/globalizable.rb
@@ -15,10 +15,7 @@ shared_examples_for "globalizable" do |factory_name|
before do
record.update!(attribute => "In English")
- I18n.with_locale(:es) do
- record.update!(required_fields.index_with("En español"))
- record.update!(attribute => "En español")
- end
+ add_translation(record, :es, "En español")
record.reload
end
@@ -98,7 +95,7 @@ shared_examples_for "globalizable" do |factory_name|
end
it "Does not automatically add a translation for the current locale" do
- record.translations.find_by(locale: :en).destroy!
+ destroy_translation(record, :en)
record.reload
record.update!(translations_attributes: [
@@ -188,4 +185,62 @@ shared_examples_for "globalizable" do |factory_name|
expect(record.send(attribute)).to eq "Deutsche Sprache"
end
end
+
+ describe ".with_fallback_translation" do
+ before do
+ fallbacks = { fr: %i[fr en es], es: %i[es fr en], en: %i[en es fr] }
+ allow(I18n).to receive(:fallbacks).and_return(I18n.fallbacks.merge(fallbacks))
+ Globalize.set_fallbacks_to_all_available_locales
+ end
+
+ it "returns records with the best fallback translation available for the current locale" do
+ record.update!(attribute => "Content in English")
+ add_translation(record, :es, "Contenido en español")
+ add_translation(record, :fr, "Contenu en français")
+
+ I18n.with_locale(:es) do
+ expect(attribute_with_fallback_translation(record, attribute)).to eq "Contenido en español"
+
+ destroy_translation(record, :es)
+
+ expect(attribute_with_fallback_translation(record, attribute)).to eq "Contenu en français"
+
+ destroy_translation(record, :fr)
+
+ expect(attribute_with_fallback_translation(record, attribute)).to eq "Content in English"
+ end
+
+ record.reload
+
+ add_translation(record, :es, "Contenido en español")
+ add_translation(record, :fr, "Contenu en français")
+
+ I18n.with_locale(:fr) do
+ expect(attribute_with_fallback_translation(record, attribute)).to eq "Contenu en français"
+
+ destroy_translation(record, :fr)
+
+ expect(attribute_with_fallback_translation(record, attribute)).to eq "Content in English"
+
+ destroy_translation(record, :en)
+
+ expect(attribute_with_fallback_translation(record, attribute)).to eq "Contenido en español"
+ end
+ end
+ end
+end
+
+def add_translation(record, locale, text)
+ I18n.with_locale(locale) do
+ record.update!(required_fields.index_with(text))
+ record.update!(attribute => text)
+ end
+end
+
+def destroy_translation(record, locale)
+ record.translations.find_by(locale: locale).destroy!
+end
+
+def attribute_with_fallback_translation(record, attribute)
+ record.class.with_fallback_translation.where(id: record.id).pick(attribute)
end
From 3fa3c90db6cca50e8f4cf944921e75c31af4983c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?=
<15726+Senen@users.noreply.github.com>
Date: Fri, 16 Jun 2023 15:48:57 +0200
Subject: [PATCH 2/2] Fix map markers navigation for keyboard users
By using the bindPopup function instead of the click event
popups work when using the keyboard.
Note that now we are loading all the map markers in the first
request in a single query to the database (needed when there
is a lot or markers to show). Because of that we removed the
AJAX endpoint.
---
app/assets/javascripts/map.js | 15 ++----------
.../budgets/investments/map_component.rb | 2 +-
app/components/budgets/map_component.rb | 2 +-
.../budgets/investments_controller.rb | 24 ++++---------------
app/models/abilities/everyone.rb | 2 +-
app/models/map_location.rb | 19 +++++++++++++++
config/routes/budget.rb | 3 ---
7 files changed, 28 insertions(+), 39 deletions(-)
diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js
index 4b2e4a5a2..396ac5418 100644
--- a/app/assets/javascripts/map.js
+++ b/app/assets/javascripts/map.js
@@ -172,7 +172,7 @@
if (App.Map.validCoordinates(coordinates)) {
marker = createMarker(coordinates.lat, coordinates.long);
marker.options.id = coordinates.investment_id;
- marker.on("click", App.Map.openMarkerPopup);
+ marker.bindPopup(App.Map.getPopupContent(coordinates));
}
});
}
@@ -217,19 +217,8 @@
polygon.addTo(map);
},
- openMarkerPopup: function(e) {
- var marker = e.target;
- $.ajax("/investments/" + marker.options.id + "/json_data", {
- type: "GET",
- dataType: "json",
- success: function(data) {
- e.target.bindPopup(App.Map.getPopupContent(data)).openPopup();
- }
- });
- },
getPopupContent: function(data) {
- return "" +
- data.investment_title + "";
+ return "" + data.title + "";
},
validZoom: function(zoom) {
return App.Map.isNumeric(zoom);
diff --git a/app/components/budgets/investments/map_component.rb b/app/components/budgets/investments/map_component.rb
index 897524ca2..43253dfed 100644
--- a/app/components/budgets/investments/map_component.rb
+++ b/app/components/budgets/investments/map_component.rb
@@ -18,7 +18,7 @@ class Budgets::Investments::MapComponent < ApplicationComponent
end
def coordinates
- MapLocation.where(investment: investments).map(&:json_data)
+ MapLocation.investments_json_data(investments.unscope(:order))
end
def geozones_data
diff --git a/app/components/budgets/map_component.rb b/app/components/budgets/map_component.rb
index dcbc427d6..ccf3719d1 100644
--- a/app/components/budgets/map_component.rb
+++ b/app/components/budgets/map_component.rb
@@ -19,7 +19,7 @@ class Budgets::MapComponent < ApplicationComponent
investments = budget.investments
end
- MapLocation.where(investment_id: investments).map(&:json_data)
+ MapLocation.investments_json_data(investments)
end
def geozones_data
diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb
index 84277042f..c192827c4 100644
--- a/app/controllers/budgets/investments_controller.rb
+++ b/app/controllers/budgets/investments_controller.rb
@@ -11,12 +11,11 @@ module Budgets
PER_PAGE = 10
- before_action :authenticate_user!, except: [:index, :show, :json_data]
- before_action :load_budget, except: :json_data
+ before_action :authenticate_user!, except: [:index, :show]
+ before_action :load_budget
- authorize_resource :budget, except: :json_data
- load_and_authorize_resource :investment, through: :budget, class: "Budget::Investment",
- except: :json_data
+ authorize_resource :budget
+ load_and_authorize_resource :investment, through: :budget, class: "Budget::Investment"
before_action :load_ballot, only: [:index, :show]
before_action :load_heading, only: [:index, :show]
@@ -25,8 +24,6 @@ module Budgets
before_action :set_default_investment_filter, only: :index
before_action :set_view, only: :index
- skip_authorization_check only: :json_data
-
feature_flag :budgets
has_orders %w[most_voted newest oldest], only: :show
@@ -91,19 +88,6 @@ module Budgets
super
end
- def json_data
- investment = Budget::Investment.find(params[:id])
- data = {
- investment_id: investment.id,
- investment_title: investment.title,
- budget_id: investment.budget.id
- }.to_json
-
- respond_to do |format|
- format.json { render json: data }
- end
- end
-
private
def resource_model
diff --git a/app/models/abilities/everyone.rb b/app/models/abilities/everyone.rb
index 7052c48d5..252ce3896 100644
--- a/app/models/abilities/everyone.rb
+++ b/app/models/abilities/everyone.rb
@@ -14,7 +14,7 @@ module Abilities
can [:read, :welcome], Budget
can [:read], Budget
can [:read], Budget::Group
- can [:read, :print, :json_data], Budget::Investment
+ can [:read, :print], Budget::Investment
can :read_results, Budget, id: Budget.finished.results_enabled.ids
can :read_stats, Budget, id: Budget.valuating_or_later.stats_enabled.ids
can :read_executions, Budget, phase: "finished"
diff --git a/app/models/map_location.rb b/app/models/map_location.rb
index 6ed4fc9cc..7adc6d639 100644
--- a/app/models/map_location.rb
+++ b/app/models/map_location.rb
@@ -24,4 +24,23 @@ class MapLocation < ApplicationRecord
longitude: (heading.longitude.to_f if heading.longitude.present?)
)
end
+
+ def self.investments_json_data(investments)
+ return [] unless investments.any?
+
+ budget_id = investments.first.budget_id
+
+ data = investments.joins(:map_location)
+ .with_fallback_translation
+ .pluck(:id, :title, :latitude, :longitude)
+
+ data.map do |values|
+ {
+ title: values[1],
+ link: "/budgets/#{budget_id}/investments/#{values[0]}",
+ lat: values[2],
+ long: values[3]
+ }
+ end
+ end
end
diff --git a/config/routes/budget.rb b/config/routes/budget.rb
index 6d173af87..f6054d95f 100644
--- a/config/routes/budget.rb
+++ b/config/routes/budget.rb
@@ -23,6 +23,3 @@ end
resolve "Budget::Investment" do |investment, options|
[investment.budget, :investment, options.merge(id: investment)]
end
-
-get "investments/:id/json_data", action: :json_data, controller: "budgets/investments"
-get "/budgets/:budget_id/investments/:id/json_data", action: :json_data, controller: "budgets/investments"