FIX: Add back the option to create invite without emailing (#29641)

Follow-up to a5497b74be

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.
This commit is contained in:
Osama Sayegh 2024-11-08 07:59:24 +03:00 committed by GitHub
parent 6c36af9f62
commit 4bc030f76f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 315 additions and 126 deletions

View File

@ -31,6 +31,7 @@ export default class CreateInvite extends Component {
@tracked saving = false; @tracked saving = false;
@tracked displayAdvancedOptions = false; @tracked displayAdvancedOptions = false;
@tracked isEmailInvite = emailValid(this.data.restrictTo);
@tracked flashText; @tracked flashText;
@tracked flashClass = "info"; @tracked flashClass = "info";
@ -40,6 +41,7 @@ export default class CreateInvite extends Component {
model = this.args.model; model = this.args.model;
invite = this.model.invite ?? Invite.create(); invite = this.model.invite ?? Invite.create();
sendEmail = false;
formApi; formApi;
constructor() { constructor() {
@ -103,11 +105,9 @@ export default class CreateInvite extends Component {
async save(data) { async save(data) {
let isLink = true; let isLink = true;
let isEmail = false;
if (data.emailOrDomain) { if (data.emailOrDomain) {
if (emailValid(data.emailOrDomain)) { if (this.isEmailInvite) {
isEmail = true;
isLink = false; isLink = false;
data.email = data.emailOrDomain; data.email = data.emailOrDomain;
} else if (hostnameValid(data.emailOrDomain)) { } else if (hostnameValid(data.emailOrDomain)) {
@ -120,14 +120,18 @@ export default class CreateInvite extends Component {
if (this.invite.email) { if (this.invite.email) {
data.email = data.custom_message = ""; data.email = data.custom_message = "";
} }
} else if (isEmail) { } else {
if (data.max_redemptions_allowed > 1) { if (data.max_redemptions_allowed > 1) {
data.max_redemptions_allowed = 1; data.max_redemptions_allowed = 1;
} }
data.send_email = true; if (this.sendEmail) {
if (data.topic_id) { data.send_email = true;
data.invite_to_topic = 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) { 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"; this.flashClass = "success";
} }
} catch (error) { } catch (error) {
@ -176,6 +188,20 @@ export default class CreateInvite extends Component {
return this.currentUser.staff && !this.siteSettings.must_approve_users; 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 @action
async onFormSubmit(data) { async onFormSubmit(data) {
const submitData = { const submitData = {
@ -199,6 +225,13 @@ export default class CreateInvite extends Component {
@action @action
saveInvite() { saveInvite() {
this.sendEmail = false;
this.formApi.submit();
}
@action
saveInviteAndSendEmail() {
this.sendEmail = true;
this.formApi.submit(); this.formApi.submit();
} }
@ -213,17 +246,9 @@ export default class CreateInvite extends Component {
this.displayAdvancedOptions = true; 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 @action
async createLink() { async createLink() {
this.sendEmail = false;
await this.save({ await this.save({
max_redemptions_allowed: this.defaultRedemptionsAllowed, max_redemptions_allowed: this.defaultRedemptionsAllowed,
expires_at: moment() expires_at: moment()
@ -254,6 +279,7 @@ export default class CreateInvite extends Component {
}} }}
@closeModal={{@closeModal}} @closeModal={{@closeModal}}
@hideFooter={{and this.simpleMode this.inviteCreated}} @hideFooter={{and this.simpleMode this.inviteCreated}}
@inline={{@inline}}
> >
<:belowHeader> <:belowHeader>
{{#if (or this.flashText @model.editing)}} {{#if (or this.flashText @model.editing)}}
@ -267,13 +293,8 @@ export default class CreateInvite extends Component {
> >
{{#if this.flashText}} {{#if this.flashText}}
{{htmlSafe this.flashText}} {{htmlSafe this.flashText}}
{{/if}} {{else}}
{{#if (and this.inviteCreated (notEq this.flashClass "error"))}} {{i18n "user.invited.invite.copy_link_and_share_it"}}
{{#if @model.editing}}
{{i18n "user.invited.invite.copy_link_and_share_it"}}
{{else}}
{{i18n "user.invited.invite.instructions"}}
{{/if}}
{{/if}} {{/if}}
</InviteModalAlert> </InviteModalAlert>
{{/if}} {{/if}}
@ -307,12 +328,13 @@ export default class CreateInvite extends Component {
@data={{this.data}} @data={{this.data}}
@onSubmit={{this.onFormSubmit}} @onSubmit={{this.onFormSubmit}}
@onRegisterApi={{this.registerApi}} @onRegisterApi={{this.registerApi}}
as |form transientData| as |form|
> >
<form.Field <form.Field
@name="restrictTo" @name="restrictTo"
@title={{i18n "user.invited.invite.restrict"}} @title={{i18n "user.invited.invite.restrict"}}
@format="large" @format="large"
@onSet={{this.handleRestrictToChange}}
as |field| as |field|
> >
<field.Input <field.Input
@ -322,7 +344,7 @@ export default class CreateInvite extends Component {
/> />
</form.Field> </form.Field>
{{#unless (emailValid transientData.restrictTo)}} {{#unless this.isEmailInvite}}
<form.Field <form.Field
@name="maxRedemptions" @name="maxRedemptions"
@title={{i18n "user.invited.invite.max_redemptions_allowed"}} @title={{i18n "user.invited.invite.max_redemptions_allowed"}}
@ -410,7 +432,7 @@ export default class CreateInvite extends Component {
</form.Field> </form.Field>
{{/if}} {{/if}}
{{#if (emailValid transientData.restrictTo)}} {{#if this.isEmailInvite}}
<form.Field <form.Field
@name="customMessage" @name="customMessage"
@title={{i18n "user.invited.invite.custom_message"}} @title={{i18n "user.invited.invite.custom_message"}}
@ -438,11 +460,27 @@ export default class CreateInvite extends Component {
/> />
{{else}} {{else}}
<DButton <DButton
@label="user.invited.invite.save_invite" @label={{if
this.inviteCreated
"user.invited.invite.update_invite"
"user.invited.invite.create_link"
}}
@action={{this.saveInvite}} @action={{this.saveInvite}}
@disabled={{this.saving}} @disabled={{this.saving}}
class="btn-primary save-invite" class="btn-primary save-invite"
/> />
{{#if this.isEmailInvite}}
<DButton
@label={{if
this.inviteCreated
"user.invited.invite.update_invite_and_send_email"
"user.invited.invite.create_link_and_send_email"
}}
@action={{this.saveInviteAndSendEmail}}
@disabled={{this.saving}}
class="btn-primary save-invite-and-send-email"
/>
{{/if}}
{{/if}} {{/if}}
<DButton <DButton
@label="user.invited.invite.cancel" @label="user.invited.invite.cancel"

View File

@ -15,6 +15,23 @@ class Field {
return this.element.dataset.controlType; return this.element.dataset.controlType;
} }
value() {
switch (this.controlType) {
case "input-text":
const input = this.element.querySelector("input");
return parseInt(input.value, 10);
}
}
options() {
if (this.controlType !== "select") {
throw new Error(`Unsupported control type: ${this.controlType}`);
}
return [...this.element.querySelectorAll("select option")].map((node) =>
node.getAttribute("value")
);
}
async fillIn(value) { async fillIn(value) {
let element; let element;
@ -131,15 +148,17 @@ class Form {
} }
field(name) { field(name) {
const field = new Field( const fieldElement = this.element.querySelector(`[data-name="${name}"]`);
this.element.querySelector(`[data-name="${name}"]`)
);
if (!field) { if (!fieldElement) {
throw new Error(`Field with name ${name} not found`); 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") { export default function form(selector = "form") {
@ -155,5 +174,9 @@ export default function form(selector = "form") {
field(name) { field(name) {
return helper.field(name); return helper.field(name);
}, },
hasField(name) {
return helper.hasField(name);
},
}; };
} }

View File

@ -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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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(<template>
<CreateInvite @inline={{true}} @model={{model}} />
</template>);
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"
);
});
});

View File

@ -1970,7 +1970,6 @@ en:
new_title: "Invite members" new_title: "Invite members"
edit_title: "Edit invite" edit_title: "Edit invite"
instructions: "Share this link to instantly grant access to this site:"
expires_in_time: "Expires in %{time}" expires_in_time: "Expires in %{time}"
expired_at_time: "Expired at %{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." 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: "Save and Send Email"
send_invite_email_instructions: "Restrict invite to email to send an invite 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" cancel: "Cancel"
create_link: "Create link" 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: bulk_invite:
none: "No invitations to display on this page." none: "No invitations to display on this page."

View File

@ -127,7 +127,8 @@ describe "Creating Invites", type: :system do
) )
end 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) another_group = Fabricate(:group)
user.update!(admin: true) user.update!(admin: true)
page.refresh page.refresh
@ -145,12 +146,14 @@ describe "Creating Invites", type: :system do
.field("customMessage") .field("customMessage")
.fill_in("Hello someone, this is a test invite") .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_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_link = create_invite_modal.invite_link_input.value
invite_key = invite_link.split("/").last
create_invite_modal.close 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( expect(user_invited_pending_page.latest_invite.expiry_date).to be_within(2.minutes).of(
Time.zone.now + 1.day, 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 end
it "adds the invite_expiry_days site setting to the list of options for the expiresAfterDays field" do it "is possible to create an email invite without sending an email to the invited address" do
options = Jobs.run_immediately!
create_invite_modal create_invite_modal.form.field("restrictTo").fill_in("invitedperson@email.org")
.form create_invite_modal.save_button.click
.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"])
SiteSetting.invite_expiry_days = 90 expect(create_invite_modal).to have_copy_button
page.refresh expect(create_invite_modal).to have_alert_message(
open_invite_modal I18n.t("js.user.invited.invite.invite_saved_without_sending_email"),
display_advanced_options )
options = invite_link = create_invite_modal.invite_link_input.value
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
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 create_invite_modal.close
expect(create_invite_modal.form.field("maxRedemptions").value).to eq("7")
SiteSetting.invite_link_max_redemptions_limit_users = 11 expect(user_invited_pending_page.invites_list.size).to eq(1)
page.refresh expect(user_invited_pending_page.latest_invite).to be_email_type("invitedperson@email.org")
open_invite_modal expect(ActionMailer::Base.deliveries).to eq([])
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")
end end
it "shows the inviteToGroups field for a normal user if they're owner on at least 1 group" do 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") expect(create_invite_modal.form).to have_field_with_name("inviteToGroups")
end 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 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.form.field("expiresAfterDays").select(1)
create_invite_modal.save_button.click 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 expire_date = Time.parse("#{date} #{time}:#{now.strftime("%S")}").utc
expect(expire_date).to be_within_one_minute_of(now + 1.day) expect(expire_date).to be_within_one_minute_of(now + 1.day)
end 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
end end

View File

@ -15,6 +15,10 @@ module PageObjects
within(modal) { find(".save-invite") } within(modal) { find(".save-invite") }
end end
def save_and_email_button
within(modal) { find(".save-invite-and-send-email") }
end
def copy_button def copy_button
within(modal) { find(".copy-button") } within(modal) { find(".copy-button") }
end end
@ -23,6 +27,10 @@ module PageObjects
within(modal) { has_css?(".copy-button") } within(modal) { has_css?(".copy-button") }
end end
def has_alert_message?(message)
within(modal) { has_css?("#modal-alert .invite-link", text: message) }
end
def has_invite_link_input? def has_invite_link_input?
within(modal) { has_css?("input.invite-link") } within(modal) { has_css?("input.invite-link") }
end end