From 5b78bbd1385a84f009910a79360edc5d6bb9915d Mon Sep 17 00:00:00 2001 From: Jan Cernik <66427541+jancernik@users.noreply.github.com> Date: Mon, 12 Aug 2024 16:02:00 -0500 Subject: [PATCH] DEV: Convert account activation pages to use Ember (#28206) --- .../discourse/app/routes/activate-account.js | 8 ++ .../discourse/app/routes/app-route-map.js | 1 + .../app/templates/activate-account.gjs | 128 ++++++++++++++++++ .../discourse/scripts/activate-account.js | 27 ---- .../discourse/scripts/auto-redirect.js | 6 - app/controllers/users_controller.rb | 17 ++- app/views/users/activate_account.html.erb | 19 --- .../users/perform_account_activation.html.erb | 27 ---- config/locales/client.en.yml | 9 ++ config/locales/server.en.yml | 4 - spec/requests/users_controller_spec.rb | 26 ++-- spec/system/login_spec.rb | 31 ++++- .../page_objects/pages/activate_account.rb | 20 +++ spec/system/page_objects/pages/invite_form.rb | 40 ++++++ spec/system/signup_spec.rb | 56 +++++++- 15 files changed, 313 insertions(+), 106 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/routes/activate-account.js create mode 100644 app/assets/javascripts/discourse/app/templates/activate-account.gjs delete mode 100644 app/assets/javascripts/discourse/scripts/activate-account.js delete mode 100644 app/assets/javascripts/discourse/scripts/auto-redirect.js delete mode 100644 app/views/users/activate_account.html.erb delete mode 100644 app/views/users/perform_account_activation.html.erb create mode 100644 spec/system/page_objects/pages/activate_account.rb create mode 100644 spec/system/page_objects/pages/invite_form.rb diff --git a/app/assets/javascripts/discourse/app/routes/activate-account.js b/app/assets/javascripts/discourse/app/routes/activate-account.js new file mode 100644 index 00000000000..5d5af2de174 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/activate-account.js @@ -0,0 +1,8 @@ +import DiscourseRoute from "discourse/routes/discourse"; +import I18n from "discourse-i18n"; + +export default class ActivateAccountRoute extends DiscourseRoute { + titleToken() { + return I18n.t("login.activate_account"); + } +} diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index 7e741df35c0..eaecbc2b8a0 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -110,6 +110,7 @@ export default function () { this.route("resent"); this.route("edit-email"); }); + this.route("activate-account", { path: "/u/activate-account/:token" }); this.route("confirm-new-email", { path: "/u/confirm-new-email/:token" }); this.route("confirm-old-email", { path: "/u/confirm-old-email/:token" }); this.route( diff --git a/app/assets/javascripts/discourse/app/templates/activate-account.gjs b/app/assets/javascripts/discourse/app/templates/activate-account.gjs new file mode 100644 index 00000000000..92e1b0e5381 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/activate-account.gjs @@ -0,0 +1,128 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; +import RouteTemplate from "ember-route-template"; +import DButton from "discourse/components/d-button"; +import hideApplicationHeaderButtons from "discourse/helpers/hide-application-header-buttons"; +import hideApplicationSidebar from "discourse/helpers/hide-application-sidebar"; +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import { wavingHandURL } from "discourse/lib/waving-hand-url"; +import i18n from "discourse-common/helpers/i18n"; + +export default RouteTemplate( + class extends Component { + @service siteSettings; + + @tracked accountActivated = false; + @tracked isLoading = false; + @tracked needsApproval = false; + @tracked errorMessage = null; + + @action + async activate() { + this.isLoading = true; + + let hp; + try { + const response = await fetch("/session/hp", { + headers: { + Accept: "application/json", + }, + }); + hp = await response.json(); + } catch (error) { + this.isLoading = false; + popupAjaxError(error); + return; + } + + try { + const response = await ajax( + `/u/activate-account/${this.args.model.token}.json`, + { + type: "PUT", + data: { + password_confirmation: hp.value, + challenge: hp.challenge.split("").reverse().join(""), + }, + } + ); + + if (!response.success) { + this.errorMessage = i18n("user.activate_account.already_done"); + return; + } + + this.accountActivated = true; + + if (response.redirect_to) { + window.location.href = response.redirect_to; + } else if (response.needs_approval) { + this.needsApproval = true; + } else { + setTimeout(() => (window.location.href = "/"), 2000); + } + } catch (error) { + this.errorMessage = i18n("user.activate_account.already_done"); + } + } + + + } +); diff --git a/app/assets/javascripts/discourse/scripts/activate-account.js b/app/assets/javascripts/discourse/scripts/activate-account.js deleted file mode 100644 index 0581a40707e..00000000000 --- a/app/assets/javascripts/discourse/scripts/activate-account.js +++ /dev/null @@ -1,27 +0,0 @@ -(function () { - const activateButton = document.querySelector("#activate-account-button"); - activateButton.addEventListener("click", async function () { - activateButton.setAttribute("disabled", true); - const hpPath = document.getElementById("data-activate-account").dataset - .path; - - try { - const response = await fetch(hpPath, { - headers: { - Accept: "application/json", - }, - }); - const hp = await response.json(); - - document.querySelector("#password_confirmation").value = hp.value; - document.querySelector("#challenge").value = hp.challenge - .split("") - .reverse() - .join(""); - document.querySelector("#activate-account-form").submit(); - } catch (e) { - activateButton.removeAttribute("disabled"); - throw e; - } - }); -})(); diff --git a/app/assets/javascripts/discourse/scripts/auto-redirect.js b/app/assets/javascripts/discourse/scripts/auto-redirect.js deleted file mode 100644 index ae6255bb63f..00000000000 --- a/app/assets/javascripts/discourse/scripts/auto-redirect.js +++ /dev/null @@ -1,6 +0,0 @@ -(function () { - const path = document.getElementById("data-auto-redirect").dataset.path; - setTimeout(function () { - window.location.href = path; - }, 2000); -})(); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f16a2c47aac..984f4ada620 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1100,7 +1100,11 @@ class UsersController < ApplicationController def activate_account expires_now - render layout: "no_ember" + + respond_to do |format| + format.html { render "default/empty" } + format.json { render json: success_json } + end end def perform_account_activation @@ -1130,21 +1134,22 @@ class UsersController < ApplicationController end if Wizard.user_requires_completion?(@user) - return redirect_to(wizard_path) + @redirect_to = wizard_path elsif destination_url.present? - return redirect_to(destination_url, allow_other_host: true) + @redirect_to = destination_url elsif SiteSetting.enable_discourse_connect_provider && payload = cookies.delete(:sso_payload) - return redirect_to(session_sso_provider_url + "?" + payload) + @redirect_to = session_sso_provider_url + "?" + payload end else @needs_approval = true end else - flash.now[:error] = I18n.t("activation.already_done") + return render_json_error(I18n.t("activation.already_done")) end - render layout: "no_ember" + render json: + success_json.merge(redirect_to: @redirect_to, needs_approval: @needs_approval || false) end def update_activation_email diff --git a/app/views/users/activate_account.html.erb b/app/views/users/activate_account.html.erb deleted file mode 100644 index 5c7f8b75a47..00000000000 --- a/app/views/users/activate_account.html.erb +++ /dev/null @@ -1,19 +0,0 @@ -
-
-

<%= t 'activation.welcome_to', site_name: SiteSetting.title %> " alt="" class="waving-hand">

-
- - - <%= form_tag(perform_activate_account_path, method: :put, id: 'activate-account-form') do %> - <%= hidden_field_tag 'password_confirmation' %> - <%= hidden_field_tag 'challenge' %> - <% end %> -
-
- -<%- content_for(:no_ember_head) do %> - <%= render_google_universal_analytics_code %> - <%= tag.meta id: 'data-activate-account', data: { path: path('/session/hp') } %> -<%- end %> - -<%= preload_script "activate-account" %> diff --git a/app/views/users/perform_account_activation.html.erb b/app/views/users/perform_account_activation.html.erb deleted file mode 100644 index 8cb03aeaa65..00000000000 --- a/app/views/users/perform_account_activation.html.erb +++ /dev/null @@ -1,27 +0,0 @@ -
- <%if flash[:error]%> -
- <%=flash[:error]%> -
- <%else%> -
-

<%= t 'activation.welcome_to', site_name: SiteSetting.title %> " alt="" class="waving-hand">

-
-
-
- tada emoji -
- <% if @needs_approval %> -

<%= t 'activation.approval_required' %>

- <% else %> -

<%= t('activation.please_continue') %>

-

"><%= t('activation.continue_button', site_name: SiteSetting.title) -%>

- <%- content_for(:no_ember_head) do %> - <%= tag.meta id: 'data-auto-redirect', data: { path: path('/') } %> - <%- end %> - <%= preload_script 'auto-redirect' %> - <% end %> -
-
- <%end%> -
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5b927aaf0b1..d7609c85d79 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1745,6 +1745,14 @@ en: account_specific: "Your %{provider} account '%{account_description}' will be used for authentication." generic: "Your %{provider} account will be used for authentication." + activate_account: + action: "Click here to activate your account" + already_done: "Sorry, this account confirmation link is no longer valid. Perhaps your account is already active?" + please_continue: "Your new account is confirmed; you will be redirected to the home page." + continue_button: "Continue to %{site_name}" + welcome_to: "Welcome to %{site_name}!" + approval_required: "A moderator must manually approve your new account before you can access this forum. You'll get an email when your account is approved!" + name: title: "Name" instructions: "Your full name (optional)." @@ -2387,6 +2395,7 @@ en: resend_activation_email: "Click here to send the activation email again." omniauth_disallow_totp: "Your account has two-factor authentication enabled. Please log in with your password." + activate_account: "Activate Account" resend_title: "Resend Activation Email" change_email: "Change Email Address" provide_new_email: "Provide a new address and we'll resend your confirmation email." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ed20eb9ce05..3e7bbd70734 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1046,11 +1046,7 @@ en: connected: "(connected)" activation: - action: "Click here to activate your account" already_done: "Sorry, this account confirmation link is no longer valid. Perhaps your account is already active?" - please_continue: "Your new account is confirmed; you will be redirected to the home page." - continue_button: "Continue to %{site_name}" - welcome_to: "Welcome to %{site_name}!" approval_required: "A moderator must manually approve your new account before you can access this forum. You'll get an email when your account is approved!" missing_session: "We cannot detect if your account was created, please ensure you have cookies enabled." activated: "Sorry, this account has already been activated." diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 33a2448985a..dc7670918b4 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -60,9 +60,8 @@ RSpec.describe UsersController do context "with invalid token" do it "return success" do - put "/u/activate-account/invalid-tooken" - expect(response.status).to eq(200) - expect(flash[:error]).to be_present + put "/u/activate-account/invalid-token" + expect(response.status).to eq(422) end end @@ -108,12 +107,11 @@ RSpec.describe UsersController do ) expect(response.status).to eq(200) - expect(flash[:error]).to be_blank - expect(session[:current_user_id]).to be_present - expect(CGI.unescapeHTML(response.body)).to_not include( - I18n.t("activation.approval_required"), - ) + data = JSON.parse(response.body) + expect(data["needs_approval"]).to eq(false) + + expect(session[:current_user_id]).to be_present end end @@ -124,11 +122,9 @@ RSpec.describe UsersController do put "/u/activate-account/#{email_token.token}" expect(response.status).to eq(200) - expect(CGI.unescapeHTML(response.body)).to include(I18n.t("activation.approval_required")) + data = JSON.parse(response.body) + expect(data["needs_approval"]).to eq(true) - expect(response.body).to_not have_tag(:script, with: { src: "/assets/application.js" }) - - expect(flash[:error]).to be_blank expect(session[:current_user_id]).to be_blank end end @@ -141,7 +137,8 @@ RSpec.describe UsersController do put "/u/activate-account/#{email_token.token}" - expect(response).to redirect_to(destination_url) + expect(response.status).to eq(200) + expect(response.parsed_body["redirect_to"]).to eq(destination_url) end end @@ -158,7 +155,8 @@ RSpec.describe UsersController do it "should redirect to the topic" do put "/u/activate-account/#{email_token.token}" - expect(response).to redirect_to(topic.relative_url) + expect(response.status).to eq(200) + expect(response.parsed_body["redirect_to"]).to eq(topic.relative_url) end end end diff --git a/spec/system/login_spec.rb b/spec/system/login_spec.rb index ba485319a27..a2036935cde 100644 --- a/spec/system/login_spec.rb +++ b/spec/system/login_spec.rb @@ -4,8 +4,10 @@ require "rotp" shared_examples "login scenarios" do let(:login_modal) { PageObjects::Modals::Login.new } + let(:activate_account) { PageObjects::Pages::ActivateAccount.new } let(:user_preferences_security_page) { PageObjects::Pages::UserPreferencesSecurity.new } fab!(:user) { Fabricate(:user, username: "john", password: "supersecurepassword") } + fab!(:admin) { Fabricate(:admin, username: "admin", password: "supersecurepassword") } let(:user_menu) { PageObjects::Components::UserMenu.new } before { Jobs.run_immediately! } @@ -43,12 +45,39 @@ shared_examples "login scenarios" do activation_link = wait_for_email_link(user, :activation) visit activation_link - find("#activate-account-button").click + activate_account.click_activate_account + activate_account.click_continue expect(page).to have_current_path("/") expect(page).to have_css(".header-dropdown-toggle.current-user") end + it "redirects to the wizard after activating account" do + login_modal.open + login_modal.fill(username: "admin", password: "supersecurepassword") + login_modal.click_login + expect(page).to have_css(".not-activated-modal") + login_modal.click(".activation-controls button.resend") + + activation_link = wait_for_email_link(admin, :activation) + visit activation_link + + activate_account.click_activate_account + expect(page).to have_current_path("/wizard") + end + + it "shows error when when activation link is invalid" do + login_modal.open + login_modal.fill(username: "john", password: "supersecurepassword") + login_modal.click_login + expect(page).to have_css(".not-activated-modal") + + visit "/u/activate-account/invalid" + + activate_account.click_activate_account + expect(activate_account).to have_error + end + it "displays the right message when user's email has been marked as expired" do password = "myawesomepassword" user.update!(password:) diff --git a/spec/system/page_objects/pages/activate_account.rb b/spec/system/page_objects/pages/activate_account.rb new file mode 100644 index 00000000000..3adc95a3246 --- /dev/null +++ b/spec/system/page_objects/pages/activate_account.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class ActivateAccount < PageObjects::Pages::Base + def click_activate_account + find("#activate-account-button").click + end + + def click_continue + find(".perform-activation .continue-button").click + end + + def has_error? + has_css?("#simple-container .alert-error") + has_content?(I18n.t("js.user.activate_account.already_done")) + end + end + end +end diff --git a/spec/system/page_objects/pages/invite_form.rb b/spec/system/page_objects/pages/invite_form.rb new file mode 100644 index 00000000000..c514625ac61 --- /dev/null +++ b/spec/system/page_objects/pages/invite_form.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class InviteForm < PageObjects::Pages::Base + def open(key) + visit "/invites/#{key}" + end + + def fill_username(username) + find("#new-account-username").fill_in(with: username) + end + + def fill_password(password) + find("#new-account-password").fill_in(with: password) + end + + def has_valid_username? + find(".username-input").has_css?("#username-validation.good") + end + + def has_valid_password? + find(".password-input").has_css?("#password-validation.good") + end + + def has_valid_fields? + has_valid_username? + has_valid_password? + end + + def click_create_account + find(".invitation-cta__accept.btn-primary").click + end + + def has_successful_message? + has_css?(".invite-success") + end + end + end +end diff --git a/spec/system/signup_spec.rb b/spec/system/signup_spec.rb index 2746aff5f99..d6dc0dc0a68 100644 --- a/spec/system/signup_spec.rb +++ b/spec/system/signup_spec.rb @@ -3,11 +3,15 @@ shared_examples "signup scenarios" do let(:login_modal) { PageObjects::Modals::Login.new } let(:signup_modal) { PageObjects::Modals::Signup.new } + let(:invite_form) { PageObjects::Pages::InviteForm.new } + let(:activate_account) { PageObjects::Pages::ActivateAccount.new } + let(:invite) { Fabricate(:invite, email: "johndoe@example.com") } + let(:topic) { Fabricate(:topic, title: "Super cool topic") } context "when anyone can create an account" do - it "can signup" do - Jobs.run_immediately! + before { Jobs.run_immediately! } + it "can signup" do signup_modal.open signup_modal.fill_email("johndoe@example.com") signup_modal.fill_username("john") @@ -18,6 +22,53 @@ shared_examples "signup scenarios" do expect(page).to have_css(".account-created") end + it "can signup and activate account" do + signup_modal.open + signup_modal.fill_email("johndoe@example.com") + signup_modal.fill_username("john") + signup_modal.fill_password("supersecurepassword") + expect(signup_modal).to have_valid_fields + + signup_modal.click_create_account + expect(page).to have_css(".account-created") + + mail = ActionMailer::Base.deliveries.first + expect(mail.to).to contain_exactly("johndoe@example.com") + activation_link = mail.body.to_s[%r{/u/activate-account/\S+}] + + visit activation_link + + activate_account.click_activate_account + activate_account.click_continue + + expect(page).to have_current_path("/") + expect(page).to have_css(".header-dropdown-toggle.current-user") + end + + it "redirects to the topic the user was invited to after activating account" do + TopicInvite.create!(invite: invite, topic: topic) + + invite_form.open(invite.invite_key) + + invite_form.fill_username("john") + invite_form.fill_password("supersecurepassword") + + expect(invite_form).to have_valid_fields + + invite_form.click_create_account + expect(invite_form).to have_successful_message + + mail = ActionMailer::Base.deliveries.first + expect(mail.to).to contain_exactly("johndoe@example.com") + activation_link = mail.body.to_s[%r{/u/activate-account/\S+}] + + visit activation_link + + activate_account.click_activate_account + + expect(page).to have_current_path("/t/#{topic.slug}/#{topic.id}") + end + context "with invite code" do before { SiteSetting.invite_code = "cupcake" } @@ -124,6 +175,7 @@ shared_examples "signup scenarios" do user = User.find_by(username: "john") EmailToken.confirm(Fabricate(:email_token, user: user).token) + visit "/" login_modal.open login_modal.fill_username("john") login_modal.fill_password("supersecurepassword")