From b67b45248bbf2ef2658bfdba370d129d219be006 Mon Sep 17 00:00:00 2001 From: Quentin Champenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Fri, 8 Mar 2024 11:25:43 +0100 Subject: [PATCH] feat: Backport language selector and allow to modify notification sending frequency (#505) * feat: Backport Extra User Fields account specs * fix: Extra Signup Fields form account * fix: Friendly Signup initializer * fix: Add account specs * feat: Disable locale selector in my account * fix: Create registration spec * ci: Add CA ES locales in CI * fix: Add translations keys * fix: Deprecated spe * feat: Allow to modify default notification sending frequency --- .env-example | 3 + .github/workflows/ci_cd.yml | 4 +- Gemfile | 2 +- Gemfile.lock | 22 +- app/views/decidim/account/show.html.erb | 70 ++++ config/i18n-tasks.yml | 3 + config/initializers/decidim.rb | 2 +- config/initializers/friendly_signup.rb | 2 + .../decidim/create_registration_spec.rb | 1 + .../k8s/organization_exporter_spec.rb | 2 +- spec/system/account_spec.rb | 301 +++++++++++++++++- spec/system/extra_user_fields/account_spec.rb | 149 +++++++++ 12 files changed, 541 insertions(+), 20 deletions(-) create mode 100644 app/views/decidim/account/show.html.erb create mode 100644 spec/system/extra_user_fields/account_spec.rb diff --git a/.env-example b/.env-example index c6eb40c9da..5a83d84cf8 100644 --- a/.env-example +++ b/.env-example @@ -81,3 +81,6 @@ DECIDIM_ADMIN_PASSWORD_STRONG="false" # VAPID_PUBLIC_KEY # VAPID_PRIVATE_KEY RAILS_LOG_LEVEL=warn + +# Default notifications sending frequency : (daily, weekly, none, real_time) +# NOTIFICATIONS_SENDING_FREQUENCY=daily \ No newline at end of file diff --git a/.github/workflows/ci_cd.yml b/.github/workflows/ci_cd.yml index 86b55a8772..ef745bc125 100644 --- a/.github/workflows/ci_cd.yml +++ b/.github/workflows/ci_cd.yml @@ -10,8 +10,8 @@ env: NODE_VERSION: 16.9.1 RUBYOPT: '-W:no-deprecated' # Set locales available for i18n tasks - ENFORCED_LOCALES: "en,fr" - AVAILABLE_LOCALES: "en,fr" + ENFORCED_LOCALES: "en,fr,ca,es" + AVAILABLE_LOCALES: "en,fr,ca,es" jobs: todo: diff --git a/Gemfile b/Gemfile index e10d0cedb6..9e440854c6 100644 --- a/Gemfile +++ b/Gemfile @@ -18,7 +18,7 @@ gem "decidim-cache_cleaner" gem "decidim-decidim_awesome" gem "decidim-extended_socio_demographic_authorization_handler", git: "https://github.com/OpenSourcePolitics/decidim-module-extended_socio_demographic_authorization_handler.git", branch: DECIDIM_BRANCH -gem "decidim-extra_user_fields", git: "https://github.com/PopulateTools/decidim-module-extra_user_fields.git", branch: "release/0.27-stable" +gem "decidim-extra_user_fields", git: "https://github.com/OpenSourcePolitics/decidim-module-extra_user_fields.git", branch: "release/0.27-stable" gem "decidim-friendly_signup", git: "https://github.com/OpenSourcePolitics/decidim-module-friendly_signup.git" gem "decidim-gallery", git: "https://github.com/OpenSourcePolitics/decidim-module-gallery.git", branch: "fix/nokogiri_deps" gem "decidim-homepage_interactive_map", git: "https://github.com/OpenSourcePolitics/decidim-module-homepage_interactive_map.git", branch: DECIDIM_BRANCH diff --git a/Gemfile.lock b/Gemfile.lock index 79289a69b5..39e841e4c5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -14,6 +14,16 @@ GIT decidim-extended_socio_demographic_authorization_handler (0.27.0) decidim-core (~> 0.27) +GIT + remote: https://github.com/OpenSourcePolitics/decidim-module-extra_user_fields.git + revision: 0716b8d9e89db89ff3b182ad1bb63e92b952d39c + branch: release/0.27-stable + specs: + decidim-extra_user_fields (0.27.2) + country_select (~> 4.0) + decidim-core (>= 0.27.0, < 0.28) + deface (~> 1.5) + GIT remote: https://github.com/OpenSourcePolitics/decidim-module-friendly_signup.git revision: 1a054faf964cc6ed07f1bec47cb92f0083a5bcb4 @@ -84,16 +94,6 @@ GIT omniauth (~> 2.0) omniauth-oauth2 (>= 1.7.2, < 2.0) -GIT - remote: https://github.com/PopulateTools/decidim-module-extra_user_fields.git - revision: ef262d6619ef254de1379278f56c4c6af789e54c - branch: release/0.27-stable - specs: - decidim-extra_user_fields (0.27.2) - country_select (~> 4.0) - decidim-core (>= 0.27.0, < 0.28) - deface (~> 1.5) - GIT remote: https://github.com/sgruhier/foundation_rails_helper.git revision: bc33600db7a2d16ce3cdc1f8369d0d7e7c4245b5 @@ -721,6 +721,8 @@ GEM nio4r (2.7.0) nokogiri (1.13.4-arm64-darwin) racc (~> 1.4) + nokogiri (1.13.4-x86_64-darwin) + racc (~> 1.4) nokogiri (1.13.4-x86_64-linux) racc (~> 1.4) oauth (1.1.0) diff --git a/app/views/decidim/account/show.html.erb b/app/views/decidim/account/show.html.erb new file mode 100644 index 0000000000..160d55ab69 --- /dev/null +++ b/app/views/decidim/account/show.html.erb @@ -0,0 +1,70 @@ +<%= alert_box("", "account-notification hide", true) %> +<% add_decidim_page_title(t("profile", scope: "layouts.decidim.user_menu")) %> +<% content_for(:subtitle) { t("profile", scope: "layouts.decidim.user_menu") } %> + +
+ <%= decidim_form_for(@account, url: account_path, method: :put, html: { autocomplete: "on" }) do |f| %> +
+ <%= f.upload :avatar %> +
+ +
+ <% if current_user.unconfirmed_email.present? %> +
+

<%= t("decidim.account.email_change.title") %>

+

<%= t("decidim.account.email_change.body1", unconfirmed_email: current_user.unconfirmed_email) %>

+

+ <%== t( + "decidim.account.email_change.body2", + resend_link: link_to(t("decidim.account.email_change.send_again"), resend_confirmation_instructions_account_path, role: :button, method: :post, remote: true), + cancel_link: link_to(t("decidim.account.email_change.cancel"), cancel_email_change_account_path, role: :button, method: :post, remote: true)) %> +

+
+ <% end %> + + <%= form_required_explanation %> + + <%= f.text_field :name, autocomplete: "name" %> + <%= f.text_field :nickname, autocomplete: "nickname" %> + <%= f.email_field :email, disabled: current_user.unconfirmed_email.present?, autocomplete: "email" %> + <%= f.url_field :personal_url, autocomplete: "url" %> + <%= f.text_area :about, rows: 5 %> + + <% if @account.organization.available_locales.size > 1 %> + <%= f.collection_select( + :locale, + @account.organization.available_locales, + :to_s, + ->(locale) {locale_name(locale) } + ) %> + <% else %> + <%= f.collection_select( + :locale, + @account.organization.available_locales, + :to_s, + ->(locale) {locale_name(locale) }, + {}, + { disabled: true } + ) + %> + <% end %> + +

<%= t(".available_locales_helper") %>

+ + <% if @account.errors[:password].any? || @account.errors[:password_confirmation].any? %> + <%= render partial: "password_fields", locals: { form: f } %> + <% else %> + <% if current_organization.sign_in_enabled? %> +

+ +

+
+ <%= render partial: "password_fields", locals: { form: f } %> +
+ <% end %> + <% end %> + + <%= f.submit t(".update_account") %> +
+ <% end %> +
\ No newline at end of file diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index de39cf5c66..4202be3b4f 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -105,6 +105,9 @@ ignore_missing: - decidim.admin.titles.initiatives - decidim.admin.models.initiatives.fields.* - decidim.admin.actions.configure + - decidim.account.email_change.* + - decidim.account.show.* + - layouts.decidim.user_menu.profile # Consider these keys used: ignore_unused: diff --git a/config/initializers/decidim.rb b/config/initializers/decidim.rb index ef776d6100..1e6e751753 100644 --- a/config/initializers/decidim.rb +++ b/config/initializers/decidim.rb @@ -13,7 +13,7 @@ config.available_locales = ENV.fetch("AVAILABLE_LOCALES", "fr").split(",").map(&:to_sym) else config.default_locale = ENV.fetch("DEFAULT_LOCALE", "en").to_sym - config.available_locales = ENV.fetch("AVAILABLE_LOCALES", "en,fr").split(",").map(&:to_sym) + config.available_locales = ENV.fetch("AVAILABLE_LOCALES", "en,fr,ca,es").split(",").map(&:to_sym) end # Timeout session diff --git a/config/initializers/friendly_signup.rb b/config/initializers/friendly_signup.rb index 460c589058..a22291a441 100644 --- a/config/initializers/friendly_signup.rb +++ b/config/initializers/friendly_signup.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +return unless defined?(Decidim::FriendlySignup) + Decidim::FriendlySignup.configure do |config| # Override password views or leave the originals (default is true): config.override_passwords = ENV.fetch("FRIENDLY_SIGNUP_OVERRIDE_PASSWORDS", "1") == "1" diff --git a/spec/commands/decidim/create_registration_spec.rb b/spec/commands/decidim/create_registration_spec.rb index cc17492ad9..ba6d4343a2 100644 --- a/spec/commands/decidim/create_registration_spec.rb +++ b/spec/commands/decidim/create_registration_spec.rb @@ -86,6 +86,7 @@ module Comments expect(User).to receive(:create!).with( name: form.name, nickname: form.nickname, + notifications_sending_frequency: "daily", email: form.email, password: form.password, password_confirmation: form.password_confirmation, diff --git a/spec/lib/decidim_app/k8s/organization_exporter_spec.rb b/spec/lib/decidim_app/k8s/organization_exporter_spec.rb index 7ffa77c337..daa3fe7061 100644 --- a/spec/lib/decidim_app/k8s/organization_exporter_spec.rb +++ b/spec/lib/decidim_app/k8s/organization_exporter_spec.rb @@ -176,7 +176,7 @@ it "returns the organization columns" do expect(subject.organization_columns).to eq({ "available_authorizations" => [], - "available_locales" => %w(en fr), + "available_locales" => %w(en fr ca es), "default_locale" => "en", "file_upload_settings" => { "allowed_content_types" => { diff --git a/spec/system/account_spec.rb b/spec/system/account_spec.rb index d3e9e6e52f..d99d4f3cdb 100644 --- a/spec/system/account_spec.rb +++ b/spec/system/account_spec.rb @@ -1,16 +1,307 @@ # frozen_string_literal: true require "spec_helper" -require_relative "examples/account_password_examples" -describe "Registration", type: :system do - let(:organization) { create(:organization) } - let(:user) { create(:user, :confirmed, organization: organization) } +describe "Account", type: :system do + let(:user) { create(:user, :confirmed, password: password, password_confirmation: password) } + let(:password) { "dqCFgjfDbC7dPbrv" } + let(:organization) { user.organization } before do + allow(Decidim::FriendlySignup).to receive(:override_passwords).and_return(false) switch_to_host(organization.host) login_as user, scope: :user end - it_behaves_like "on/off account passwords" + describe "navigation" do + it "shows the account form when clicking on the menu" do + visit decidim.root_path + + within_user_menu do + find("a", text: "account").click + end + + expect(page).to have_css("form.edit_user") + end + end + + context "when on the account page" do + before do + visit decidim.account_path + end + + it_behaves_like "accessible page" + + describe "update avatar" do + it "can update avatar" do + dynamically_attach_file(:user_avatar, Decidim::Dev.asset("avatar.jpg"), remove_before: true) + + within "form.edit_user" do + find("*[type=submit]").click + end + + expect(page).to have_css(".flash.success") + end + + it "shows error when image is too big" do + find("#user_avatar_button").click + + within ".upload-modal" do + find(".remove-upload-item").click + input_element = find("input[type='file']", visible: :all) + input_element.attach_file(Decidim::Dev.asset("5000x5000.png")) + + expect(page).to have_content("File resolution is too large", count: 1) + expect(page).to have_css(".upload-errors .form-error", count: 1) + end + end + end + + describe "update locales" do + context "when the organization has one locale" do + let(:organization) { create(:organization, available_locales: ["en"]) } + + it "is not possible to change locales, #user_locale is disabled" do + expect(page).to have_css("#user_locale", text: "English") + expect(find("#user_locale")).to be_disabled + end + end + + context "when the organization has more than one locale" do + it "shows the list of locales" do + find("#user_locale").click + expect(page).to have_css("option", count: organization.available_locales.size) + end + end + end + + describe "updating personal data" do + let!(:encrypted_password) { user.encrypted_password } + + it "updates the user's data" do + within "form.edit_user" do + select "Castellano", from: :user_locale + fill_in :user_name, with: "Nikola Tesla" + fill_in :user_personal_url, with: "https://example.org" + fill_in :user_about, with: "A Serbian-American inventor, electrical engineer, mechanical engineer, physicist, and futurist." + find("*[type=submit]").click + end + + within_flash_messages do + expect(page).to have_content("successfully") + end + + within ".title-bar" do + expect(page).to have_content("Nikola Tesla") + end + + user.reload + + within_user_menu do + find("a", text: "perfil público").click + end + + expect(page).to have_content("example.org") + expect(page).to have_content("Serbian-American") + + # The user's password should not change when they did not update it + expect(user.reload.encrypted_password).to eq(encrypted_password) + end + end + + describe "updating locale" do + context "when the organization has more than one locale" do + it "switches the locale to french" do + within "form.edit_user" do + find("#user_locale").click + find("option", text: "Français").select_option + find("*[type=submit]").click + end + + within_flash_messages do + expect(page).to have_content("successfully") + end + + within "#user_locale" do + expect(page).to have_content("Français") + end + expect(page).to have_css(".help-text") + end + end + end + + describe "updating the password" do + context "when password and confirmation match" do + it "updates the password successfully" do + within "form.edit_user" do + page.find(".change-password").click + + fill_in :user_password, with: "sekritpass123" + fill_in :user_password_confirmation, with: "sekritpass123" + + find("*[type=submit]").click + end + + within_flash_messages do + expect(page).to have_content("successfully") + end + + expect(user.reload.valid_password?("sekritpass123")).to be(true) + end + end + + context "when passwords don't match" do + it "doesn't update the password" do + within "form.edit_user" do + page.find(".change-password").click + + fill_in :user_password, with: "sekritpass123" + fill_in :user_password_confirmation, with: "oopseytypo" + + find("*[type=submit]").click + end + + within_flash_messages do + expect(page).to have_content("There was a problem") + end + + expect(user.reload.valid_password?("sekritpass123")).to be(false) + end + end + end + + context "when updating the email" do + let(:pending_email) { "foo@bar.com" } + + before do + within "form.edit_user" do + fill_in :user_email, with: pending_email + + perform_enqueued_jobs { find("*[type=submit]").click } + end + + within_flash_messages do + expect(page).to have_content("You'll receive an email to confirm your new email address") + end + end + + after do + clear_enqueued_jobs + end + + it "tells user to confirm new email" do + expect(page).to have_content("Email change verification") + expect(page).to have_selector("#user_email[disabled='disabled']") + expect(page).to have_content("We've sent an email to #{pending_email} to verify your new email address") + end + + it "resend confirmation" do + within "#email-change-pending" do + click_link "Send again" + end + expect(page).to have_content("Confirmation email resent successfully to #{pending_email}") + perform_enqueued_jobs + perform_enqueued_jobs + + expect(emails.count).to eq(2) + visit last_email_link + expect(page).to have_content("Your email address has been successfully confirmed") + end + + it "cancels the email change" do + expect(Decidim::User.find(user.id).unconfirmed_email).to eq(pending_email) + within "#email-change-pending" do + click_link "cancel" + end + + expect(page).to have_content("Email change cancelled successfully") + expect(page).not_to have_content("Email change verification") + expect(Decidim::User.find(user.id).unconfirmed_email).to be_nil + end + end + + context "when on the notifications settings page" do + before do + visit decidim.notifications_settings_path + end + + it "updates the user's notifications" do + within ".switch.newsletter_notifications" do + page.find(".switch-paddle").click + end + + within "form.edit_user" do + find("*[type=submit]").click + end + + within_flash_messages do + expect(page).to have_content("successfully") + end + end + + context "when the user is an admin" do + let!(:user) { create(:user, :confirmed, :admin, password: password, password_confirmation: password) } + + before do + login_as user, scope: :user + visit decidim.notifications_settings_path + end + + it "updates the administrator's notifications" do + within ".switch.email_on_moderations" do + page.find(".switch-paddle").click + end + + within ".switch.notification_settings" do + page.find(".switch-paddle").click + end + + within "form.edit_user" do + find("*[type=submit]").click + end + + within_flash_messages do + expect(page).to have_content("successfully") + end + end + end + end + + context "when on the interests page" do + before do + visit decidim.user_interests_path + end + + it "doesn't find any scopes" do + expect(page).to have_content("My interests") + expect(page).to have_content("This organization doesn't have any scope yet") + end + + context "when scopes are defined" do + let!(:scopes) { create_list(:scope, 3, organization: organization) } + let!(:subscopes) { create_list(:subscope, 3, parent: scopes.first) } + + before do + visit decidim.user_interests_path + end + + it "display translated scope name" do + label_field = "label[for='user_scopes_#{scopes.first.id}_checked']" + expect(page).to have_content("My interests") + expect(find("#{label_field} > span.switch-label").text).to eq(translated(scopes.first.name)) + end + + it "allows to choose interests" do + label_field = "label[for='user_scopes_#{scopes.first.id}_checked']" + expect(page).to have_content("My interests") + find(label_field).click + click_button "Update my interests" + + within_flash_messages do + expect(page).to have_content("Your interests have been successfully updated.") + end + end + end + end + end end diff --git a/spec/system/extra_user_fields/account_spec.rb b/spec/system/extra_user_fields/account_spec.rb new file mode 100644 index 0000000000..44ca322d83 --- /dev/null +++ b/spec/system/extra_user_fields/account_spec.rb @@ -0,0 +1,149 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe "Account", type: :system do + shared_examples_for "does not display extra user field" do |field, label| + it "does not display field '#{field}'" do + expect(page).not_to have_content(label) + end + end + + let(:organization) { create(:organization, extra_user_fields: extra_user_fields) } + let(:user) { create(:user, :confirmed, organization: organization, password: password, password_confirmation: password) } + let(:password) { "dqCFgjfDbC7dPbrv" } + # rubocop:disable Style/TrailingCommaInHashLiteral + let(:extra_user_fields) do + { + "enabled" => true, + "date_of_birth" => date_of_birth, + "postal_code" => postal_code, + "gender" => gender, + "country" => country, + "phone_number" => phone_number, + "location" => location, + # Block ExtraUserFields ExtraUserFields + + # EndBlock + } + end + # rubocop:enable Style/TrailingCommaInHashLiteral + + let(:date_of_birth) do + { "enabled" => true } + end + + let(:postal_code) do + { "enabled" => true } + end + + let(:country) do + { "enabled" => true } + end + + let(:gender) do + { "enabled" => true } + end + + let(:phone_number) do + { "enabled" => true } + end + + let(:location) do + { "enabled" => true } + end + + # Block ExtraUserFields RspecVar + + # EndBlock + + before do + switch_to_host(organization.host) + login_as user, scope: :user + end + + context "when on the account page" do + before do + visit decidim.account_path + end + + describe "updating personal data" do + it "updates the user's data" do + within "form.edit_user" do + select "Castellano", from: :user_locale + fill_in :user_name, with: "Nikola Tesla" + fill_in :user_personal_url, with: "https://example.org" + fill_in :user_about, with: "A Serbian-American inventor, electrical engineer, mechanical engineer, physicist, and futurist." + + fill_in :user_date_of_birth, with: "01/01/2000" + select "Other", from: :user_gender + select "Argentina", from: :user_country + fill_in :user_postal_code, with: "00000" + fill_in :user_phone_number, with: "0123456789" + fill_in :user_location, with: "Cahors" + # Block ExtraUserFields FillFieldSpec + + # EndBlock + + find("*[type=submit]").click + end + + within_flash_messages do + expect(page).to have_content("successfully") + end + + within ".title-bar" do + expect(page).to have_content("Nikola Tesla") + end + end + end + + context "when date_of_birth is not enabled" do + let(:date_of_birth) do + { "enabled" => false } + end + + it_behaves_like "does not display extra user field", "date_of_birth", "Date of birth" + end + + context "when postal_code is not enabled" do + let(:postal_code) do + { "enabled" => false } + end + + it_behaves_like "does not display extra user field", "postal_code", "Postal code" + end + + context "when country is not enabled" do + let(:country) do + { "enabled" => false } + end + + it_behaves_like "does not display extra user field", "country", "Country" + end + + context "when gender is not enabled" do + let(:gender) do + { "enabled" => false } + end + + it_behaves_like "does not display extra user field", "gender", "Gender" + end + + context "when phone number is not enabled" do + let(:phone_number) do + { "enabled" => false } + end + + it_behaves_like "does not display extra user field", "phone number", "Phone number" + end + + context "when location is not enabled" do + let(:location) do + { "enabled" => false } + end + + it_behaves_like "does not display extra user field", "location", "Location" + end + end +end