From c9e3fc7b715234b4cc7a666f41195362b5c55516 Mon Sep 17 00:00:00 2001 From: Alexandru Emil Lupu Date: Wed, 27 Nov 2024 21:38:49 +0200 Subject: [PATCH 1/3] Optimize the proposals --- .../app/cells/decidim/activity_cell.rb | 3 -- .../app/cells/decidim/author/show.erb | 7 ++-- decidim-core/app/cells/decidim/author_cell.rb | 23 ++++++++++++ .../decidim/profiles_controller.rb | 4 +++ decidim-core/app/packs/src/decidim/index.js | 3 ++ .../app/packs/src/decidim/remote_tooltips.js | 36 +++++++++++++++++++ .../views/decidim/profiles/tooltip.html.erb | 3 ++ decidim-core/config/routes.rb | 1 + .../decidim/proposals/proposal_g_cell.rb | 0 .../decidim/proposals/proposal_l_cell.rb | 35 +++++++++--------- .../decidim/proposals/proposals_controller.rb | 14 ++++---- 11 files changed, 96 insertions(+), 33 deletions(-) create mode 100644 decidim-core/app/packs/src/decidim/remote_tooltips.js create mode 100644 decidim-core/app/views/decidim/profiles/tooltip.html.erb create mode 100644 decidim-proposals/app/cells/decidim/proposals/proposal_g_cell.rb diff --git a/decidim-core/app/cells/decidim/activity_cell.rb b/decidim-core/app/cells/decidim/activity_cell.rb index f47eaead01fde..6532ad2d43ebe 100644 --- a/decidim-core/app/cells/decidim/activity_cell.rb +++ b/decidim-core/app/cells/decidim/activity_cell.rb @@ -107,9 +107,6 @@ def cache_hash hash << I18n.locale.to_s hash << model.class.name.underscore hash << model.cache_key_with_version - if (author_cell = author) - hash.push(Digest::MD5.hexdigest(author_cell.send(:cache_hash))) - end hash.join(Decidim.cache_key_separator) end diff --git a/decidim-core/app/cells/decidim/author/show.erb b/decidim-core/app/cells/decidim/author/show.erb index d6cbfd05138fb..97c06b03c91ee 100644 --- a/decidim-core/app/cells/decidim/author/show.erb +++ b/decidim-core/app/cells/decidim/author/show.erb @@ -1,6 +1,5 @@ -<% data = has_tooltip? ? { tooltip: render(:profile_minicard).html_safe } : nil %> - - <%= content_tag :span, class: "author__container#{" is-compact" if layout == :compact}", data: do %> +<%= content_tag(:span, class: :author, data: ) do %> + <%= content_tag :span, class: "author__container#{" is-compact" if layout == :compact}" do %> <% if layout == :compact %> <%= render :avatar %> @@ -24,4 +23,4 @@ <%= render action %> <% end %> <% end %> - +<% end %> diff --git a/decidim-core/app/cells/decidim/author_cell.rb b/decidim-core/app/cells/decidim/author_cell.rb index b1087104d39e5..aaf883d58acd7 100644 --- a/decidim-core/app/cells/decidim/author_cell.rb +++ b/decidim-core/app/cells/decidim/author_cell.rb @@ -49,8 +49,23 @@ def context_actions_options @context_actions_options ||= options[:context_actions].map(&:to_sym) end + def profile_minicard + render + end + private + def data + @data ||= begin + internal_data = { author: true } + if has_tooltip? + internal_data["remote_tooltip"] = true + internal_data["tooltip-url"] = decidim.profile_tooltip_path(raw_model.nickname) + end + internal_data + end + end + def layout @layout ||= LAYOUTS.include?(options[:layout]) ? options[:layout] : :default end @@ -162,5 +177,13 @@ def resource_i18n_scope def resource_name @resource_name ||= from_context.class.name.demodulize.underscore end + + def has_tooltip? + return false if model.deleted? + return false if model.respond_to?(:blocked?) && model.blocked? + return options[:tooltip] if options.has_key?(:tooltip) + + model.has_tooltip? + end end end diff --git a/decidim-core/app/controllers/decidim/profiles_controller.rb b/decidim-core/app/controllers/decidim/profiles_controller.rb index 2f57ddbd93113..ba9560d7ef5c7 100644 --- a/decidim-core/app/controllers/decidim/profiles_controller.rb +++ b/decidim-core/app/controllers/decidim/profiles_controller.rb @@ -27,6 +27,10 @@ def show redirect_to profile_activity_path(nickname: params[:nickname]) end + def tooltip + render json: { data: cell("decidim/author", profile_holder.presenter).profile_minicard } + end + def following @content_cell = "decidim/following" @title_key = "following" diff --git a/decidim-core/app/packs/src/decidim/index.js b/decidim-core/app/packs/src/decidim/index.js index 482e1d3018ff8..73e1e1470c63d 100644 --- a/decidim-core/app/packs/src/decidim/index.js +++ b/decidim-core/app/packs/src/decidim/index.js @@ -68,6 +68,7 @@ import markAsReadNotifications from "src/decidim/notifications" import RemoteModal from "src/decidim/remote_modal" import selectActiveIdentity from "src/decidim/identity_selector_dialog" import createTooltip from "src/decidim/tooltips" +import fetchRemoteTooltip from "src/decidim/remote_tooltips" import createToggle from "src/decidim/toggle" import { createAccordion, @@ -189,6 +190,8 @@ const initializer = (element = document) => { // Initialize data-toggles element.querySelectorAll("[data-toggle]").forEach((elem) => createToggle(elem)) + element.querySelectorAll("[data-remote-tooltip]").forEach((elem) => fetchRemoteTooltip(elem)) + element.querySelectorAll(".new_report").forEach((elem) => changeReportFormBehavior(elem)) } diff --git a/decidim-core/app/packs/src/decidim/remote_tooltips.js b/decidim-core/app/packs/src/decidim/remote_tooltips.js new file mode 100644 index 0000000000000..9c453fd83fc98 --- /dev/null +++ b/decidim-core/app/packs/src/decidim/remote_tooltips.js @@ -0,0 +1,36 @@ +import createTooltip from "src/decidim/tooltips" + +/** + * This function is parsing the DOM and tries to attach and initialize a tooltip starting from an element + * containing the following structure: + * + * + * + * + * @param {HTMLElement} node The element holding the initialization data + * @returns {void} + */ +export default async function(node) { + const container = node.firstElementChild; + + if (container) { + + try { + const response = await fetch(node.dataset.tooltipUrl, { + headers: { + "Content-Type": "application/json" + } + }); + if (!response.ok) { + throw new Error(`Response status: ${response.status}`); + } + + const json = await response.json(); + + container.dataset.tooltip = json.data; + createTooltip(container); + } catch (error) { + console.error(error); + } + } +} diff --git a/decidim-core/app/views/decidim/profiles/tooltip.html.erb b/decidim-core/app/views/decidim/profiles/tooltip.html.erb new file mode 100644 index 0000000000000..38ffe6e9e2b16 --- /dev/null +++ b/decidim-core/app/views/decidim/profiles/tooltip.html.erb @@ -0,0 +1,3 @@ +{ + "data": <%= cell("decidim/author", profile_holder.presenter).profile_minicard.to_json %> +} diff --git a/decidim-core/config/routes.rb b/decidim-core/config/routes.rb index a17f4955df5d0..89b743a4db239 100644 --- a/decidim-core/config/routes.rb +++ b/decidim-core/config/routes.rb @@ -137,6 +137,7 @@ get "group_members", to: "profiles#group_members", as: "profile_group_members" get "group_admins", to: "profiles#group_admins", as: "profile_group_admins" get "activity", to: "user_activities#index", as: "profile_activity" + get "tooltip", to: "profiles#tooltip", as: "profile_tooltip" resources :conversations, except: [:destroy], controller: "user_conversations", as: "profile_conversations" end diff --git a/decidim-proposals/app/cells/decidim/proposals/proposal_g_cell.rb b/decidim-proposals/app/cells/decidim/proposals/proposal_g_cell.rb new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/decidim-proposals/app/cells/decidim/proposals/proposal_l_cell.rb b/decidim-proposals/app/cells/decidim/proposals/proposal_l_cell.rb index 955e87fafcda2..47fc052498961 100644 --- a/decidim-proposals/app/cells/decidim/proposals/proposal_l_cell.rb +++ b/decidim-proposals/app/cells/decidim/proposals/proposal_l_cell.rb @@ -22,25 +22,24 @@ def metadata_cell end def cache_hash - hash = [] - hash << I18n.locale.to_s - hash << model.cache_key_with_version - hash << model.proposal_votes_count - hash << model.endorsements_count - hash << model.comments_count - hash << Digest::MD5.hexdigest(model.component.cache_key_with_version) - hash << Digest::MD5.hexdigest(resource_image_url) if resource_image_url - hash << render_space? ? 1 : 0 - if current_user - hash << current_user.cache_key_with_version - hash << current_user.follows?(model) ? 1 : 0 + @cache_hash ||= begin + hash = [] + hash << I18n.locale.to_s + hash << self.class.name.demodulize.underscore + hash << model.cache_key_with_version + hash << model.proposal_votes_count + hash << model.endorsements_count + hash << model.comments_count + hash << Digest::MD5.hexdigest(model.component.cache_key_with_version) + hash << Digest::MD5.hexdigest(resource_image_url) if resource_image_url + hash << render_space? ? 1 : 0 + hash << model.follows_count + hash << Digest::MD5.hexdigest(model.authors.map(&:cache_key_with_version).to_s) + hash << (model.must_render_translation?(model.organization) ? 1 : 0) if model.respond_to?(:must_render_translation?) + hash << model.component.participatory_space.active_step.id if model.component.participatory_space.try(:active_step) + + hash.join(Decidim.cache_key_separator) end - hash << model.follows_count - hash << Digest::MD5.hexdigest(model.authors.map(&:cache_key_with_version).to_s) - hash << (model.must_render_translation?(model.organization) ? 1 : 0) if model.respond_to?(:must_render_translation?) - hash << model.component.participatory_space.active_step.id if model.component.participatory_space.try(:active_step) - - hash.join(Decidim.cache_key_separator) end end end diff --git a/decidim-proposals/app/controllers/decidim/proposals/proposals_controller.rb b/decidim-proposals/app/controllers/decidim/proposals/proposals_controller.rb index 2fcdf47a4f297..5978682a38871 100644 --- a/decidim-proposals/app/controllers/decidim/proposals/proposals_controller.rb +++ b/decidim-proposals/app/controllers/decidim/proposals/proposals_controller.rb @@ -43,13 +43,13 @@ def index .order(position: :asc) render "decidim/proposals/proposals/participatory_texts/participatory_text" else - @base_query = search - .result - .published - .not_hidden + @proposals = search.result - @proposals = @base_query.includes(:component, :coauthorships, :attachments) - @all_geocoded_proposals = @base_query.geocoded + @all_geocoded_proposals = @proposals.geocoded + + @proposals = reorder(@proposals) + @proposals = paginate(@proposals) + @proposals = @proposals.includes(:component, :coauthorships, :attachments) @voted_proposals = if current_user ProposalVote.where( @@ -59,8 +59,6 @@ def index else [] end - @proposals = reorder(@proposals) - @proposals = paginate(@proposals) end end From 6fe60fee0ddcfeee79ed5cfa82f22064a5cf0259 Mon Sep 17 00:00:00 2001 From: Alexandru Emil Lupu Date: Thu, 28 Nov 2024 22:58:32 +0200 Subject: [PATCH 2/3] Fix failing specs --- .../app/cells/decidim/author/show.erb | 3 ++- decidim-core/app/cells/decidim/author_cell.rb | 5 +++-- .../app/packs/src/decidim/remote_tooltips.js | 19 ++++++---------- .../views/decidim/profiles/tooltip.html.erb | 3 --- .../test/shared_examples/comments_examples.rb | 3 +-- .../spec/cells/decidim/activity_cell_spec.rb | 20 ----------------- .../spec/system/last_activity_spec.rb | 3 ++- .../spec/system/user_activity_spec.rb | 2 +- .../decidim/proposals/proposal_l_cell_spec.rb | 22 ++++++++++--------- 9 files changed, 28 insertions(+), 52 deletions(-) delete mode 100644 decidim-core/app/views/decidim/profiles/tooltip.html.erb diff --git a/decidim-core/app/cells/decidim/author/show.erb b/decidim-core/app/cells/decidim/author/show.erb index 97c06b03c91ee..362651b624c80 100644 --- a/decidim-core/app/cells/decidim/author/show.erb +++ b/decidim-core/app/cells/decidim/author/show.erb @@ -1,5 +1,6 @@ +<% tooltip = options.has_key?(:demo) ? { tooltip: render(:profile_minicard).html_safe } : nil %> <%= content_tag(:span, class: :author, data: ) do %> - <%= content_tag :span, class: "author__container#{" is-compact" if layout == :compact}" do %> + <%= content_tag :span, class: "author__container#{" is-compact" if layout == :compact}", data: tooltip do %> <% if layout == :compact %> <%= render :avatar %> diff --git a/decidim-core/app/cells/decidim/author_cell.rb b/decidim-core/app/cells/decidim/author_cell.rb index aaf883d58acd7..5066eb93ac15a 100644 --- a/decidim-core/app/cells/decidim/author_cell.rb +++ b/decidim-core/app/cells/decidim/author_cell.rb @@ -58,10 +58,11 @@ def profile_minicard def data @data ||= begin internal_data = { author: true } - if has_tooltip? + if has_tooltip? && !options.has_key?(:demo) internal_data["remote_tooltip"] = true internal_data["tooltip-url"] = decidim.profile_tooltip_path(raw_model.nickname) end + internal_data end end @@ -181,7 +182,7 @@ def resource_name def has_tooltip? return false if model.deleted? return false if model.respond_to?(:blocked?) && model.blocked? - return options[:tooltip] if options.has_key?(:tooltip) + return true if options.has_key?(:tooltip) model.has_tooltip? end diff --git a/decidim-core/app/packs/src/decidim/remote_tooltips.js b/decidim-core/app/packs/src/decidim/remote_tooltips.js index 9c453fd83fc98..f01d2fa5bbe45 100644 --- a/decidim-core/app/packs/src/decidim/remote_tooltips.js +++ b/decidim-core/app/packs/src/decidim/remote_tooltips.js @@ -14,23 +14,18 @@ export default async function(node) { const container = node.firstElementChild; if (container) { - - try { - const response = await fetch(node.dataset.tooltipUrl, { - headers: { - "Content-Type": "application/json" - } - }); - if (!response.ok) { - throw new Error(`Response status: ${response.status}`); + const response = await fetch(node.dataset.tooltipUrl, { + headers: { + "Content-Type": "application/json" } - + }); + if (response.ok) { const json = await response.json(); container.dataset.tooltip = json.data; createTooltip(container); - } catch (error) { - console.error(error); + } else { + console.error(response.status, response.statusText); } } } diff --git a/decidim-core/app/views/decidim/profiles/tooltip.html.erb b/decidim-core/app/views/decidim/profiles/tooltip.html.erb deleted file mode 100644 index 38ffe6e9e2b16..0000000000000 --- a/decidim-core/app/views/decidim/profiles/tooltip.html.erb +++ /dev/null @@ -1,3 +0,0 @@ -{ - "data": <%= cell("decidim/author", profile_holder.presenter).profile_minicard.to_json %> -} diff --git a/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb b/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb index 649386924e877..bf5ed3ef61ec5 100644 --- a/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb +++ b/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb @@ -613,7 +613,7 @@ end context "when the user has verified organizations" do - let(:user_group) { create(:user_group, :verified) } + let(:user_group) { create(:user_group, :verified, organization:) } let(:content) { "This is a new comment" } before do @@ -1025,7 +1025,6 @@ end context "when authenticated" do - let!(:organization) { create(:organization) } let!(:user) { create(:user, :confirmed, organization:) } let!(:comments) { create_list(:comment, 3, commentable:) } diff --git a/decidim-core/spec/cells/decidim/activity_cell_spec.rb b/decidim-core/spec/cells/decidim/activity_cell_spec.rb index 983da9c26644f..e445176413148 100644 --- a/decidim-core/spec/cells/decidim/activity_cell_spec.rb +++ b/decidim-core/spec/cells/decidim/activity_cell_spec.rb @@ -87,26 +87,6 @@ allow(controller).to receive(:current_user).and_return(nil) end - context "when the author is shown" do - let(:show_author) { true } - - context "and the user is updated" do - let!(:original_hash) { subject.send(:cache_hash) } - - before do - # rubocop:disable Rails/SkipsModelValidations - resource.normalized_author.touch - # rubocop:enable Rails/SkipsModelValidations - - subject.user.reload - end - - it "changes the cache hash" do - expect(subject.send(:cache_hash)).not_to eq(original_hash) - end - end - end - context "when the author is hidden" do context "and the user is updated" do let!(:original_hash) { subject.send(:cache_hash) } diff --git a/decidim-core/spec/system/last_activity_spec.rb b/decidim-core/spec/system/last_activity_spec.rb index 8975e470bf4ff..7a37f7b89f486 100644 --- a/decidim-core/spec/system/last_activity_spec.rb +++ b/decidim-core/spec/system/last_activity_spec.rb @@ -31,13 +31,14 @@ organization:) end let(:long_body_comment) { "This is my very long comment for Last Activity card that must be shorten up because is more than 100 chars" } - let(:another_comment) { create(:comment, body: long_body_comment) } + let(:another_comment) { create(:comment, body: long_body_comment, commentable: second_commentable) } let(:component) do create(:component, :published, organization:) end let(:resource) do create(:dummy_resource, component:, published_at: Time.current) end + let(:second_commentable) { create(:dummy_resource, component:) } before do allow(Decidim::ActionLog).to receive(:public_resource_types).and_return( diff --git a/decidim-core/spec/system/user_activity_spec.rb b/decidim-core/spec/system/user_activity_spec.rb index c56f05fc446bd..485b8d70a56c0 100644 --- a/decidim-core/spec/system/user_activity_spec.rb +++ b/decidim-core/spec/system/user_activity_spec.rb @@ -4,7 +4,7 @@ describe "User activity" do let(:organization) { create(:organization) } - let(:comment) { create(:comment) } + let(:comment) { create(:comment, commentable: resource) } let(:user) { create(:user, :confirmed, organization:) } let!(:action_log) do diff --git a/decidim-proposals/spec/cells/decidim/proposals/proposal_l_cell_spec.rb b/decidim-proposals/spec/cells/decidim/proposals/proposal_l_cell_spec.rb index 9ecb8e236652b..1f61b231a23da 100644 --- a/decidim-proposals/spec/cells/decidim/proposals/proposal_l_cell_spec.rb +++ b/decidim-proposals/spec/cells/decidim/proposals/proposal_l_cell_spec.rb @@ -85,6 +85,7 @@ module Decidim::Proposals old_hash = my_cell.send(:cache_hash) allow(I18n).to receive(:locale).and_return(alt_locale) + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -94,6 +95,7 @@ module Decidim::Proposals old_hash = my_cell.send(:cache_hash) proposal.update!(title: { en: "New title" }) + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -103,6 +105,7 @@ module Decidim::Proposals old_hash = my_cell.send(:cache_hash) create(:endorsement, resource: proposal, author: build(:user, organization: proposal.participatory_space.organization)) + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -113,6 +116,7 @@ module Decidim::Proposals create(:proposal_vote, proposal:) my_cell.model.reload + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -124,21 +128,12 @@ module Decidim::Proposals component.settings = { foo: "bar" } component.save! + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) - component.settings = component_settings end end - context "when no current user" do - it "generate a different hash" do - old_hash = my_cell.send(:cache_hash) - allow(controller).to receive(:current_user).and_return(nil) - - expect(my_cell.send(:cache_hash)).not_to eq(old_hash) - end - end - context "when followers changes" do let(:another_user) { create(:user, organization: proposal.participatory_space.organization) } @@ -146,6 +141,7 @@ module Decidim::Proposals old_hash = my_cell.send(:cache_hash) create(:follow, followable: proposal, user: another_user) + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -155,6 +151,7 @@ module Decidim::Proposals old_hash = my_cell.send(:cache_hash) create(:follow, followable: proposal, user:) + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -165,6 +162,7 @@ module Decidim::Proposals old_hash = my_cell.send(:cache_hash) model.add_coauthor(user) + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end @@ -173,6 +171,7 @@ module Decidim::Proposals old_hash = my_cell.send(:cache_hash) model.authors.first.update(personal_url: "new personal url") + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -196,6 +195,7 @@ module Decidim::Proposals old_hash = my_cell.send(:cache_hash) my_cell.context.merge!({ show_space: true }) + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -226,6 +226,7 @@ module Decidim::Proposals step2.update!(active: true) proposal.reload + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end @@ -241,6 +242,7 @@ module Decidim::Proposals step3.update!(active: true) proposal.reload + my_cell.remove_instance_variable(:@cache_hash) expect(my_cell.send(:cache_hash)).not_to eq(old_hash) end end From 408d2943a858edc2c333bed2e3c1bfe9c9f2b3f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Bol=C3=ADvar?= Date: Fri, 29 Nov 2024 12:16:44 +0100 Subject: [PATCH 3/3] Remove empty file --- decidim-proposals/app/cells/decidim/proposals/proposal_g_cell.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 decidim-proposals/app/cells/decidim/proposals/proposal_g_cell.rb diff --git a/decidim-proposals/app/cells/decidim/proposals/proposal_g_cell.rb b/decidim-proposals/app/cells/decidim/proposals/proposal_g_cell.rb deleted file mode 100644 index e69de29bb2d1d..0000000000000