From 9aa32e23fe017c7b511d3ce0f269d5088ec390cf Mon Sep 17 00:00:00 2001 From: Antti Hukkanen Date: Thu, 15 Jun 2023 16:39:02 +0300 Subject: [PATCH] Backport 'Fix notifications page when vapid is not available' to v0.27 (#10940) * Prepare 0.27.3 release * Update the setup-chromedriver GitHub action to latest v1 * Fix notifications page when vapid is not available * User's group endorsement no longer disappears after personal endorsement removed * Fixed group endorsement removal when personal endorsement removed & tests * test-fixes * Fix the notification settings when vapid keys are not present --------- Co-authored-by: JoonasAapro <110532525+JoonasAapro@users.noreply.github.com> * Revert changes --------- Co-authored-by: Alexandru Emil Lupu Co-authored-by: JoonasAapro <110532525+JoonasAapro@users.noreply.github.com> --- .../decidim/notifications_settings_form.rb | 2 +- .../decidim/send_push_notification.rb | 2 +- .../forms/notifications_settings_form_spec.rb | 14 ++++++++++++-- .../decidim/send_push_notification_spec.rb | 19 +++++++++++++++++-- decidim-core/spec/system/account_spec.rb | 18 ++++++++++++++++-- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/decidim-core/app/forms/decidim/notifications_settings_form.rb b/decidim-core/app/forms/decidim/notifications_settings_form.rb index 2783d6456ac8..418bff43246e 100644 --- a/decidim-core/app/forms/decidim/notifications_settings_form.rb +++ b/decidim-core/app/forms/decidim/notifications_settings_form.rb @@ -53,7 +53,7 @@ def user_is_moderator?(user) end def meet_push_notifications_requirements? - Rails.application.secrets.vapid[:enabled] + Rails.application.secrets.dig(:vapid, :enabled) || false end end end diff --git a/decidim-core/app/services/decidim/send_push_notification.rb b/decidim-core/app/services/decidim/send_push_notification.rb index 75405579288f..8d3d8f437ef2 100644 --- a/decidim-core/app/services/decidim/send_push_notification.rb +++ b/decidim-core/app/services/decidim/send_push_notification.rb @@ -16,7 +16,7 @@ class SendPushNotification # # Returns the result of the dispatch or nil if user or subscription are empty def perform(notification) - return unless Rails.application.secrets.vapid[:enabled] + return unless Rails.application.secrets.dig(:vapid, :enabled) I18n.with_locale(notification.user.locale || notification.user.organization.default_locale) do notification.user.notifications_subscriptions.values.map do |subscription| diff --git a/decidim-core/spec/forms/notifications_settings_form_spec.rb b/decidim-core/spec/forms/notifications_settings_form_spec.rb index d625c8e45597..e640659f3467 100644 --- a/decidim-core/spec/forms/notifications_settings_form_spec.rb +++ b/decidim-core/spec/forms/notifications_settings_form_spec.rb @@ -188,7 +188,7 @@ module Decidim describe "#meet_push_notifications_requirements?" do context "when the notifications requirements are met" do before do - allow(Rails.application.secrets).to receive("vapid").and_return({ enabled: true }) + Rails.application.secrets[:vapid] = { enabled: true } end it "returns true" do @@ -196,9 +196,19 @@ module Decidim end end + context "when vapid secrets are not present" do + before do + Rails.application.secrets.delete(:vapid) + end + + it "returns false" do + expect(subject.meet_push_notifications_requirements?).to be false + end + end + context "when the notifications requirements aren't met" do before do - allow(Rails.application.secrets).to receive("vapid").and_return({ enabled: false }) + Rails.application.secrets[:vapid] = { enabled: false } end it "returns false" do diff --git a/decidim-core/spec/services/decidim/send_push_notification_spec.rb b/decidim-core/spec/services/decidim/send_push_notification_spec.rb index 1dcf55f30c01..2727da182ac6 100644 --- a/decidim-core/spec/services/decidim/send_push_notification_spec.rb +++ b/decidim-core/spec/services/decidim/send_push_notification_spec.rb @@ -15,12 +15,27 @@ end before do - allow(Rails.application.secrets).to receive("vapid").and_return({ enabled: true, public_key: "public_key", private_key: "private_key" }) + Rails.application.secrets[:vapid] = { enabled: true, public_key: "public_key", private_key: "private_key" } end context "without vapid settings config" do before do - allow(Rails.application.secrets).to receive("vapid").and_return({ enabled: false }) + Rails.application.secrets.delete(:vapid) + end + + describe "#perform" do + let(:user) { create(:user) } + let(:notification) { create :notification, user: user } + + it "returns false" do + expect(subject.perform(notification)).to be_falsy + end + end + end + + context "without vapid enabled" do + before do + Rails.application.secrets[:vapid] = { enabled: false } end describe "#perform" do diff --git a/decidim-core/spec/system/account_spec.rb b/decidim-core/spec/system/account_spec.rb index dc3037d25964..58409606c1b3 100644 --- a/decidim-core/spec/system/account_spec.rb +++ b/decidim-core/spec/system/account_spec.rb @@ -308,7 +308,7 @@ context "when VAPID keys are set" do before do - allow(Rails.application.secrets).to receive("vapid").and_return(vapid_keys) + Rails.application.secrets[:vapid] = vapid_keys driven_by(:pwa_chrome) switch_to_host(organization.host) login_as user, scope: :user @@ -339,9 +339,23 @@ end end + context "when VAPID is disabled" do + before do + Rails.application.secrets[:vapid] = { enabled: false } + driven_by(:pwa_chrome) + switch_to_host(organization.host) + login_as user, scope: :user + visit decidim.notifications_settings_path + end + + it "does not show the push notifications switch" do + expect(page).to have_no_selector(".push-notifications") + end + end + context "when VAPID keys are not set" do before do - allow(Rails.application.secrets).to receive("vapid").and_return({}) + Rails.application.secrets.delete(:vapid) driven_by(:pwa_chrome) switch_to_host(organization.host) login_as user, scope: :user