From 4bc030f76f0c032474d11b1791df7ef8d225e272 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Fri, 8 Nov 2024 07:59:24 +0300 Subject: [PATCH] FIX: Add back the option to create invite without emailing (#29641) Follow-up to https://github.com/discourse/discourse/commit/a5497b74be76c9250c59ed12d516f9e8bbbca5ef In the linked commit, as part of simplifying the invite modal, we removed the option to skip sending an email when creating an invite restricted to a specific address. This has caused confusion about whether an email will be sent by Discourse or not, so we're adding back the option to create a restricted invite without emailing. Internal topic: t/134023/48. --- .../app/components/modal/create-invite.gjs | 94 ++++++--- .../tests/helpers/form-kit-helper.js | 33 +++- .../components/create-invite-test.gjs | 186 ++++++++++++++++++ config/locales/client.en.yml | 8 +- spec/system/create_invite_spec.rb | 112 +++-------- .../page_objects/modals/create_invite.rb | 8 + 6 files changed, 315 insertions(+), 126 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/integration/components/create-invite-test.gjs diff --git a/app/assets/javascripts/discourse/app/components/modal/create-invite.gjs b/app/assets/javascripts/discourse/app/components/modal/create-invite.gjs index 62baabbfbaa..e9b6454f26f 100644 --- a/app/assets/javascripts/discourse/app/components/modal/create-invite.gjs +++ b/app/assets/javascripts/discourse/app/components/modal/create-invite.gjs @@ -31,6 +31,7 @@ export default class CreateInvite extends Component { @tracked saving = false; @tracked displayAdvancedOptions = false; + @tracked isEmailInvite = emailValid(this.data.restrictTo); @tracked flashText; @tracked flashClass = "info"; @@ -40,6 +41,7 @@ export default class CreateInvite extends Component { model = this.args.model; invite = this.model.invite ?? Invite.create(); + sendEmail = false; formApi; constructor() { @@ -103,11 +105,9 @@ export default class CreateInvite extends Component { async save(data) { let isLink = true; - let isEmail = false; if (data.emailOrDomain) { - if (emailValid(data.emailOrDomain)) { - isEmail = true; + if (this.isEmailInvite) { isLink = false; data.email = data.emailOrDomain; } else if (hostnameValid(data.emailOrDomain)) { @@ -120,14 +120,18 @@ export default class CreateInvite extends Component { if (this.invite.email) { data.email = data.custom_message = ""; } - } else if (isEmail) { + } else { if (data.max_redemptions_allowed > 1) { data.max_redemptions_allowed = 1; } - data.send_email = true; - if (data.topic_id) { - data.invite_to_topic = true; + if (this.sendEmail) { + data.send_email = true; + if (data.topic_id) { + data.invite_to_topic = true; + } + } else { + data.skip_email = true; } } @@ -140,7 +144,15 @@ export default class CreateInvite extends Component { } if (!this.simpleMode) { - this.flashText = sanitize(I18n.t("user.invited.invite.invite_saved")); + if (this.sendEmail) { + this.flashText = sanitize( + I18n.t("user.invited.invite.invite_saved_with_sending_email") + ); + } else { + this.flashText = sanitize( + I18n.t("user.invited.invite.invite_saved_without_sending_email") + ); + } this.flashClass = "success"; } } catch (error) { @@ -176,6 +188,20 @@ export default class CreateInvite extends Component { return this.currentUser.staff && !this.siteSettings.must_approve_users; } + get simpleMode() { + return !this.args.model.editing && !this.displayAdvancedOptions; + } + + get inviteCreated() { + return !!this.invite.get("id"); + } + + @action + handleRestrictToChange(value, { set }) { + set("restrictTo", value); + this.isEmailInvite = emailValid(value); + } + @action async onFormSubmit(data) { const submitData = { @@ -199,6 +225,13 @@ export default class CreateInvite extends Component { @action saveInvite() { + this.sendEmail = false; + this.formApi.submit(); + } + + @action + saveInviteAndSendEmail() { + this.sendEmail = true; this.formApi.submit(); } @@ -213,17 +246,9 @@ export default class CreateInvite extends Component { this.displayAdvancedOptions = true; } - get simpleMode() { - return !this.args.model.editing && !this.displayAdvancedOptions; - } - - get inviteCreated() { - // use .get to track the id - return !!this.invite.get("id"); - } - @action async createLink() { + this.sendEmail = false; await this.save({ max_redemptions_allowed: this.defaultRedemptionsAllowed, expires_at: moment() @@ -254,6 +279,7 @@ export default class CreateInvite extends Component { }} @closeModal={{@closeModal}} @hideFooter={{and this.simpleMode this.inviteCreated}} + @inline={{@inline}} > <:belowHeader> {{#if (or this.flashText @model.editing)}} @@ -267,13 +293,8 @@ export default class CreateInvite extends Component { > {{#if this.flashText}} {{htmlSafe this.flashText}} - {{/if}} - {{#if (and this.inviteCreated (notEq this.flashClass "error"))}} - {{#if @model.editing}} - {{i18n "user.invited.invite.copy_link_and_share_it"}} - {{else}} - {{i18n "user.invited.invite.instructions"}} - {{/if}} + {{else}} + {{i18n "user.invited.invite.copy_link_and_share_it"}} {{/if}} {{/if}} @@ -307,12 +328,13 @@ export default class CreateInvite extends Component { @data={{this.data}} @onSubmit={{this.onFormSubmit}} @onRegisterApi={{this.registerApi}} - as |form transientData| + as |form| > - {{#unless (emailValid transientData.restrictTo)}} + {{#unless this.isEmailInvite}} {{/if}} - {{#if (emailValid transientData.restrictTo)}} + {{#if this.isEmailInvite}} {{else}} + {{#if this.isEmailInvite}} + + {{/if}} {{/if}} + node.getAttribute("value") + ); + } + async fillIn(value) { let element; @@ -131,15 +148,17 @@ class Form { } field(name) { - const field = new Field( - this.element.querySelector(`[data-name="${name}"]`) - ); + const fieldElement = this.element.querySelector(`[data-name="${name}"]`); - if (!field) { + if (!fieldElement) { throw new Error(`Field with name ${name} not found`); } - return field; + return new Field(fieldElement); + } + + hasField(name) { + return !!this.element.querySelector(`[data-name="${name}"]`); } } export default function form(selector = "form") { @@ -155,5 +174,9 @@ export default function form(selector = "form") { field(name) { return helper.field(name); }, + + hasField(name) { + return helper.hasField(name); + }, }; } diff --git a/app/assets/javascripts/discourse/tests/integration/components/create-invite-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/create-invite-test.gjs new file mode 100644 index 00000000000..b5bdadc6cd8 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/create-invite-test.gjs @@ -0,0 +1,186 @@ +import { click, render } from "@ember/test-helpers"; +import { module, test } from "qunit"; +import CreateInvite from "discourse/components/modal/create-invite"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import formKit from "discourse/tests/helpers/form-kit-helper"; + +module("Integration | Component | CreateInvite", function (hooks) { + setupRenderingTest(hooks); + + test("typing an email address in the restrictTo field", async function (assert) { + const model = {}; + + await render(); + + await click(".edit-link-options"); + + assert.false( + formKit().hasField("customMessage"), + "customMessage field is not shown before typing an email address in the restrictTo field" + ); + assert.true( + formKit().hasField("maxRedemptions"), + "maxRedemptions field is shown before typing an email address in the restrictTo field" + ); + assert + .dom(".save-invite-and-send-email") + .doesNotExist( + "'Create invite and send email' button is not shown before typting an email address in the restrictTo field" + ); + assert + .dom(".save-invite") + .exists( + "'Create invite' button is shown before typting an email address in the restrictTo field" + ); + + await formKit().field("restrictTo").fillIn("discourse@example.com"); + + assert.true( + formKit().hasField("customMessage"), + "customMessage field is shown after typing an email address in the restrictTo field" + ); + assert.false( + formKit().hasField("maxRedemptions"), + "maxRedemptions field is not shown after typing an email address in the restrictTo field" + ); + assert + .dom(".save-invite-and-send-email") + .exists( + "'Create invite and send email' button is shown after typting an email address in the restrictTo field" + ); + assert + .dom(".save-invite") + .exists( + "'Create invite' button is shown after typting an email address in the restrictTo field" + ); + }); + + test("the inviteToTopic field", async function (assert) { + const model = {}; + this.currentUser.admin = true; + this.siteSettings.must_approve_users = true; + + await render(); + await click(".edit-link-options"); + + assert.false( + formKit().hasField("inviteToTopic"), + "inviteToTopic field is not shown to admins if must_approve_users is true" + ); + + this.siteSettings.must_approve_users = false; + await render(); + await click(".edit-link-options"); + + assert.true( + formKit().hasField("inviteToTopic"), + "inviteToTopic field is shown to admins if must_approve_users is false" + ); + + this.currentUser.set("admin", false); + this.currentUser.set("moderator", false); + await render(); + await click(".edit-link-options"); + + assert.false( + formKit().hasField("inviteToTopic"), + "inviteToTopic field is not shown to regular users" + ); + }); + + test("the maxRedemptions field for non-staff users", async function (assert) { + const model = {}; + this.siteSettings.invite_link_max_redemptions_limit_users = 11; + + await render(); + await click(".edit-link-options"); + + assert.strictEqual( + formKit().field("maxRedemptions").value(), + 10, + "uses 10 as the default value if invite_link_max_redemptions_limit_users is larger than 10" + ); + + this.siteSettings.invite_link_max_redemptions_limit_users = 9; + + await render(); + await click(".edit-link-options"); + + assert.strictEqual( + formKit().field("maxRedemptions").value(), + 9, + "uses invite_link_max_redemptions_limit_users as the default value if it's smaller than 10" + ); + }); + + test("the maxRedemptions field for staff users", async function (assert) { + const model = {}; + this.currentUser.set("moderator", true); + this.siteSettings.invite_link_max_redemptions_limit = 111; + + await render(); + await click(".edit-link-options"); + + assert.strictEqual( + formKit().field("maxRedemptions").value(), + 100, + "uses 100 as the default value if invite_link_max_redemptions_limit is larger than 100" + ); + + this.siteSettings.invite_link_max_redemptions_limit = 98; + + await render(); + await click(".edit-link-options"); + + assert.strictEqual( + formKit().field("maxRedemptions").value(), + 98, + "uses invite_link_max_redemptions_limit as the default value if it's smaller than 10" + ); + }); + + test("the expiresAfterDays field", async function (assert) { + const model = {}; + this.siteSettings.invite_expiry_days = 3; + + await render(); + await click(".edit-link-options"); + + assert.deepEqual( + formKit().field("expiresAfterDays").options(), + ["1", "3", "7", "30", "90", "999999"], + "the value of invite_expiry_days is added to the dropdown" + ); + + this.siteSettings.invite_expiry_days = 90; + + await render(); + await click(".edit-link-options"); + + assert.deepEqual( + formKit().field("expiresAfterDays").options(), + ["1", "7", "30", "90", "999999"], + "the value of invite_expiry_days is not added to the dropdown if it's already one of the options" + ); + }); +}); diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 34e7fde9e39..db90f6b253d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1970,7 +1970,6 @@ en: new_title: "Invite members" edit_title: "Edit invite" - instructions: "Share this link to instantly grant access to this site:" expires_in_time: "Expires in %{time}" expired_at_time: "Expired at %{time}" create_link_to_invite: "Create a link that can be shared to instantly grant access to this site." @@ -2006,11 +2005,14 @@ en: send_invite_email: "Save and Send Email" send_invite_email_instructions: "Restrict invite to email to send an invite email" - save_invite: "Save" + update_invite: "Update" + update_invite_and_send_email: "Update and send email" cancel: "Cancel" create_link: "Create link" + create_link_and_send_email: "Create link and send email" - invite_saved: "Invite saved." + invite_saved_without_sending_email: "Invite saved. Copy the link below and share it to instantly grant access to this site." + invite_saved_with_sending_email: "Invite email has been sent. You can also copy the link below and share it to instantly grant access to this site." bulk_invite: none: "No invitations to display on this page." diff --git a/spec/system/create_invite_spec.rb b/spec/system/create_invite_spec.rb index 06fe2138ace..2839de17843 100644 --- a/spec/system/create_invite_spec.rb +++ b/spec/system/create_invite_spec.rb @@ -127,7 +127,8 @@ describe "Creating Invites", type: :system do ) end - it "is possible to create an email invite" do + it "is possible to create an email invite and send email to the invited address" do + Jobs.run_immediately! another_group = Fabricate(:group) user.update!(admin: true) page.refresh @@ -145,12 +146,14 @@ describe "Creating Invites", type: :system do .field("customMessage") .fill_in("Hello someone, this is a test invite") - create_invite_modal.save_button.click + create_invite_modal.save_and_email_button.click expect(create_invite_modal).to have_copy_button + expect(create_invite_modal).to have_alert_message( + I18n.t("js.user.invited.invite.invite_saved_with_sending_email"), + ) invite_link = create_invite_modal.invite_link_input.value - invite_key = invite_link.split("/").last create_invite_modal.close @@ -163,58 +166,28 @@ describe "Creating Invites", type: :system do expect(user_invited_pending_page.latest_invite.expiry_date).to be_within(2.minutes).of( Time.zone.now + 1.day, ) + sent_email = ActionMailer::Base.deliveries.first + expect(sent_email.to).to contain_exactly("someone@discourse.org") + expect(sent_email.parts[0].body.raw_source).to include(invite_link) end - it "adds the invite_expiry_days site setting to the list of options for the expiresAfterDays field" do - options = - create_invite_modal - .form - .field("expiresAfterDays") - .component - .all(".form-kit__control-option") - .map(&:text) - expect(options).to eq(["1 day", "3 days", "7 days", "30 days", "90 days", "Never"]) + it "is possible to create an email invite without sending an email to the invited address" do + Jobs.run_immediately! + create_invite_modal.form.field("restrictTo").fill_in("invitedperson@email.org") + create_invite_modal.save_button.click - SiteSetting.invite_expiry_days = 90 - page.refresh - open_invite_modal - display_advanced_options + expect(create_invite_modal).to have_copy_button + expect(create_invite_modal).to have_alert_message( + I18n.t("js.user.invited.invite.invite_saved_without_sending_email"), + ) - options = - create_invite_modal - .form - .field("expiresAfterDays") - .component - .all(".form-kit__control-option") - .map(&:text) - expect(options).to eq(["1 day", "7 days", "30 days", "90 days", "Never"]) - end + invite_link = create_invite_modal.invite_link_input.value - it "uses the invite_link_max_redemptions_limit_users setting as the default value for the maxRedemptions field if the setting is lower than 10" do - expect(create_invite_modal.form.field("maxRedemptions").value).to eq("7") + create_invite_modal.close - SiteSetting.invite_link_max_redemptions_limit_users = 11 - page.refresh - open_invite_modal - display_advanced_options - - expect(create_invite_modal.form.field("maxRedemptions").value).to eq("10") - end - - it "uses the invite_link_max_redemptions_limit setting as the default value for the maxRedemptions field for staff users if the setting is lower than 100" do - user.update!(admin: true) - page.refresh - open_invite_modal - display_advanced_options - - expect(create_invite_modal.form.field("maxRedemptions").value).to eq("63") - - SiteSetting.invite_link_max_redemptions_limit = 108 - page.refresh - open_invite_modal - display_advanced_options - - expect(create_invite_modal.form.field("maxRedemptions").value).to eq("100") + expect(user_invited_pending_page.invites_list.size).to eq(1) + expect(user_invited_pending_page.latest_invite).to be_email_type("invitedperson@email.org") + expect(ActionMailer::Base.deliveries).to eq([]) end it "shows the inviteToGroups field for a normal user if they're owner on at least 1 group" do @@ -237,35 +210,6 @@ describe "Creating Invites", type: :system do expect(create_invite_modal.form).to have_field_with_name("inviteToGroups") end - it "doesn't show the inviteToTopic field to normal users" do - SiteSetting.must_approve_users = false - page.refresh - open_invite_modal - display_advanced_options - - expect(create_invite_modal.form).to have_no_field_with_name("inviteToTopic") - end - - it "shows the inviteToTopic field to admins if the must_approve_users setting is false" do - user.update!(admin: true) - SiteSetting.must_approve_users = false - page.refresh - open_invite_modal - display_advanced_options - - expect(create_invite_modal.form).to have_field_with_name("inviteToTopic") - end - - it "doesn't show the inviteToTopic field to admins if the must_approve_users setting is true" do - user.update!(admin: true) - SiteSetting.must_approve_users = true - page.refresh - open_invite_modal - display_advanced_options - - expect(create_invite_modal.form).to have_no_field_with_name("inviteToTopic") - end - it "replaces the expiresAfterDays field with expiresAt with date and time controls after creating the invite" do create_invite_modal.form.field("expiresAfterDays").select(1) create_invite_modal.save_button.click @@ -281,17 +225,5 @@ describe "Creating Invites", type: :system do expire_date = Time.parse("#{date} #{time}:#{now.strftime("%S")}").utc expect(expire_date).to be_within_one_minute_of(now + 1.day) end - - context "when an email is given to the restrictTo field" do - it "shows the customMessage field and hides the maxRedemptions field" do - expect(create_invite_modal.form).to have_no_field_with_name("customMessage") - expect(create_invite_modal.form).to have_field_with_name("maxRedemptions") - - create_invite_modal.form.field("restrictTo").fill_in("discourse@cdck.org") - - expect(create_invite_modal.form).to have_field_with_name("customMessage") - expect(create_invite_modal.form).to have_no_field_with_name("maxRedemptions") - end - end end end diff --git a/spec/system/page_objects/modals/create_invite.rb b/spec/system/page_objects/modals/create_invite.rb index b560bdb2571..e4e9eee7280 100644 --- a/spec/system/page_objects/modals/create_invite.rb +++ b/spec/system/page_objects/modals/create_invite.rb @@ -15,6 +15,10 @@ module PageObjects within(modal) { find(".save-invite") } end + def save_and_email_button + within(modal) { find(".save-invite-and-send-email") } + end + def copy_button within(modal) { find(".copy-button") } end @@ -23,6 +27,10 @@ module PageObjects within(modal) { has_css?(".copy-button") } end + def has_alert_message?(message) + within(modal) { has_css?("#modal-alert .invite-link", text: message) } + end + def has_invite_link_input? within(modal) { has_css?("input.invite-link") } end