FEATURE: User fields required for existing users - Part 2 (#27172)

We want to allow admins to make new required fields apply to existing users. In order for this to work we need to have a way to make those users fill up the fields on their next page load. This is very similar to how adding a 2FA requirement post-fact works. Users will be redirected to a page where they can fill up the remaining required fields, and until they do that they won't be able to do anything else.
This commit is contained in:
Ted Johansson 2024-06-25 19:32:18 +08:00 committed by GitHub
parent 867b3822f3
commit d63f1826fe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
51 changed files with 661 additions and 209 deletions

View File

@ -34,17 +34,36 @@
<label class="optional"> <label class="optional">
<RadioButton <RadioButton
@value="optional" @value="optional"
@name="optional" @name="requirement"
@selection={{this.buffered.requirement}} @selection={{this.buffered.requirement}}
@onChange={{action "changeRequirementType"}}
/> />
<span>{{i18n "admin.user_fields.requirement.optional.title"}}</span> <span>{{i18n "admin.user_fields.requirement.optional.title"}}</span>
</label> </label>
<label class="for_all_users">
<RadioButton
@value="for_all_users"
@name="requirement"
@selection={{this.buffered.requirement}}
@onChange={{action "changeRequirementType"}}
/>
<div class="label-text">
<span>{{i18n
"admin.user_fields.requirement.for_all_users.title"
}}</span>
<div class="description">{{i18n
"admin.user_fields.requirement.for_all_users.description"
}}</div>
</div>
</label>
<label class="on_signup"> <label class="on_signup">
<RadioButton <RadioButton
@value="on_signup" @value="on_signup"
@name="on_signup" @name="requirement"
@selection={{this.buffered.requirement}} @selection={{this.buffered.requirement}}
@onChange={{action "changeRequirementType"}}
/> />
<div class="label-text"> <div class="label-text">
<span>{{i18n "admin.user_fields.requirement.on_signup.title"}}</span> <span>{{i18n "admin.user_fields.requirement.on_signup.title"}}</span>
@ -57,7 +76,11 @@
<AdminFormRow @label="admin.user_fields.preferences"> <AdminFormRow @label="admin.user_fields.preferences">
<label> <label>
<Input @type="checkbox" @checked={{this.buffered.editable}} /> <Input
@type="checkbox"
@checked={{this.buffered.editable}}
disabled={{this.editableDisabled}}
/>
<span>{{i18n "admin.user_fields.editable.title"}}</span> <span>{{i18n "admin.user_fields.editable.title"}}</span>
</label> </label>

View File

@ -3,6 +3,7 @@ import { action } from "@ember/object";
import { schedule } from "@ember/runloop"; import { schedule } from "@ember/runloop";
import { service } from "@ember/service"; import { service } from "@ember/service";
import { isEmpty } from "@ember/utils"; import { isEmpty } from "@ember/utils";
import { Promise } from "rsvp";
import { popupAjaxError } from "discourse/lib/ajax-error"; import { popupAjaxError } from "discourse/lib/ajax-error";
import { i18n, propertyEqual } from "discourse/lib/computed"; import { i18n, propertyEqual } from "discourse/lib/computed";
import { bufferedProperty } from "discourse/mixins/buffered-content"; import { bufferedProperty } from "discourse/mixins/buffered-content";
@ -12,6 +13,7 @@ import UserField from "admin/models/user-field";
export default Component.extend(bufferedProperty("userField"), { export default Component.extend(bufferedProperty("userField"), {
adminCustomUserFields: service(), adminCustomUserFields: service(),
dialog: service(),
tagName: "", tagName: "",
isEditing: false, isEditing: false,
@ -64,8 +66,29 @@ export default Component.extend(bufferedProperty("userField"), {
return ret.join(", "); return ret.join(", ");
}, },
@discourseComputed("buffered.requirement")
editableDisabled(requirement) {
return requirement === "for_all_users";
},
@action @action
save() { changeRequirementType(requirement) {
this.buffered.set("requirement", requirement);
this.buffered.set("editable", requirement === "for_all_users");
},
async _confirmChanges() {
return new Promise((resolve) => {
this.dialog.yesNoConfirm({
message: I18n.t("admin.user_fields.requirement.confirmation"),
didCancel: () => resolve(false),
didConfirm: () => resolve(true),
});
});
},
@action
async save() {
const attrs = this.buffered.getProperties( const attrs = this.buffered.getProperties(
"name", "name",
"description", "description",
@ -79,6 +102,16 @@ export default Component.extend(bufferedProperty("userField"), {
...this.adminCustomUserFields.additionalProperties ...this.adminCustomUserFields.additionalProperties
); );
let confirm = true;
if (attrs.requirement === "for_all_users") {
confirm = await this._confirmChanges();
}
if (!confirm) {
return;
}
return this.userField return this.userField
.save(attrs) .save(attrs)
.then(() => { .then(() => {

View File

@ -69,7 +69,7 @@
<LinkTo @route="preferences" @model={{this.currentUser}}> <LinkTo @route="preferences" @model={{this.currentUser}}>
{{d-icon "cog"}} {{d-icon "cog"}}
<span class="item-label"> <span class="item-label">
{{i18n "user.preferences"}} {{i18n "user.preferences.title"}}
</span> </span>
</LinkTo> </LinkTo>
</li> </li>

View File

@ -74,7 +74,7 @@
class="user-nav__preferences" class="user-nav__preferences"
> >
{{d-icon "cog"}} {{d-icon "cog"}}
<span>{{i18n "user.preferences"}}</span> <span>{{i18n "user.preferences.title"}}</span>
</DNavigationItem> </DNavigationItem>
{{/if}} {{/if}}
{{#if (and @isMobileView @isStaff)}} {{#if (and @isMobileView @isStaff)}}

View File

@ -42,6 +42,13 @@ export default Controller.extend({
return; return;
} }
if (this.showEnforcedRequiredFieldsNotice) {
return this._missingRequiredFields(
this.site.user_fields,
this.model.user_fields
);
}
// Staff can edit fields that are not `editable` // Staff can edit fields that are not `editable`
if (!this.currentUser.staff) { if (!this.currentUser.staff) {
siteUserFields = siteUserFields.filterBy("editable", true); siteUserFields = siteUserFields.filterBy("editable", true);
@ -53,6 +60,11 @@ export default Controller.extend({
}); });
}, },
@discourseComputed("currentUser.needs_required_fields_check")
showEnforcedRequiredFieldsNotice(needsRequiredFieldsCheck) {
return needsRequiredFieldsCheck;
},
@discourseComputed("model.user_option.default_calendar") @discourseComputed("model.user_option.default_calendar")
canChangeDefaultCalendar(defaultCalendar) { canChangeDefaultCalendar(defaultCalendar) {
return defaultCalendar !== "none_selected"; return defaultCalendar !== "none_selected";
@ -81,6 +93,16 @@ export default Controller.extend({
document.querySelector(".feature-topic-on-profile-btn")?.focus(); document.querySelector(".feature-topic-on-profile-btn")?.focus();
}, },
_missingRequiredFields(siteFields, userFields) {
return siteFields
.filter(
(siteField) =>
siteField.requirement === "for_all_users" &&
isEmpty(userFields[siteField.id])
)
.map((field) => EmberObject.create({ field, value: "" }));
},
actions: { actions: {
clearFeaturedTopicFromProfile() { clearFeaturedTopicFromProfile() {
this.dialog.yesNoConfirm({ this.dialog.yesNoConfirm({
@ -132,6 +154,7 @@ export default Controller.extend({
.then(() => { .then(() => {
model.set("bio_cooked"); model.set("bio_cooked");
this.set("saved", true); this.set("saved", true);
this.currentUser.set("needs_required_fields_check", false);
}) })
.catch(popupAjaxError); .catch(popupAjaxError);
}) })

View File

@ -1,7 +1,26 @@
import { action } from "@ember/object";
import { service } from "@ember/service";
import RestrictedUserRoute from "discourse/routes/restricted-user"; import RestrictedUserRoute from "discourse/routes/restricted-user";
export default class PreferencesProfile extends RestrictedUserRoute { export default class PreferencesProfile extends RestrictedUserRoute {
@service currentUser;
setupController(controller, model) { setupController(controller, model) {
controller.set("model", model); controller.set("model", model);
} }
@action
willTransition(transition) {
super.willTransition(...arguments);
if (
this.currentUser?.needs_required_fields_check &&
!transition?.to.name.startsWith("admin")
) {
transition.abort();
return false;
}
return true;
}
} }

View File

@ -13,7 +13,7 @@ export default class Preferences extends RestrictedUserRoute {
let controller = this.controllerFor(this.router.currentRouteName); let controller = this.controllerFor(this.router.currentRouteName);
let subpageTitle = controller?.subpageTitle; let subpageTitle = controller?.subpageTitle;
return subpageTitle return subpageTitle
? `${subpageTitle} - ${I18n.t("user.preferences")}` ? `${subpageTitle} - ${I18n.t("user.preferences.title")}`
: I18n.t("user.preferences"); : I18n.t("user.preferences.title");
} }
} }

View File

@ -1,58 +1,66 @@
{{#if this.canChangeBio}} {{#if this.showEnforcedRequiredFieldsNotice}}
<div class="control-group pref-bio" data-setting-name="user-bio"> <div class="alert alert-error">{{i18n
<label class="control-label">{{i18n "user.bio"}}</label> "user.preferences.profile.enforced_required_fields"
<div class="controls bio-composer input-xxlarge"> }}</div>
<DEditor @value={{this.model.bio_raw}} />
</div>
</div>
{{/if}} {{/if}}
<div class="control-group pref-timezone" data-setting-name="user-timezone"> {{#unless this.showEnforcedRequiredFieldsNotice}}
<label class="control-label">{{i18n "user.timezone"}}</label> {{#if this.canChangeBio}}
<TimezoneInput <div class="control-group pref-bio" data-setting-name="user-bio">
@value={{this.model.user_option.timezone}} <label class="control-label">{{i18n "user.bio"}}</label>
@onChange={{fn (mut this.model.user_option.timezone)}} <div class="controls bio-composer input-xxlarge">
class="input-xxlarge" <DEditor @value={{this.model.bio_raw}} />
/> </div>
<DButton
@icon="globe"
@label="user.use_current_timezone"
@action={{action "useCurrentTimezone"}}
class="btn-default"
/>
</div>
{{#if this.model.can_change_location}}
<div class="control-group pref-location" data-setting-name="user-location">
<label class="control-label" for="edit-location">{{i18n
"user.location"
}}</label>
<div class="controls">
<Input
@type="text"
@value={{this.model.location}}
class="input-xxlarge"
id="edit-location"
/>
</div> </div>
</div> {{/if}}
{{/if}}
{{#if this.model.can_change_website}} <div class="control-group pref-timezone" data-setting-name="user-timezone">
<div class="control-group pref-website" data-setting-name="user-website"> <label class="control-label">{{i18n "user.timezone"}}</label>
<label class="control-label" for="edit-website">{{i18n <TimezoneInput
"user.website" @value={{this.model.user_option.timezone}}
}}</label> @onChange={{fn (mut this.model.user_option.timezone)}}
<div class="controls"> class="input-xxlarge"
<Input />
@type="text" <DButton
@value={{this.model.website}} @icon="globe"
class="input-xxlarge" @label="user.use_current_timezone"
id="edit-website" @action={{action "useCurrentTimezone"}}
/> class="btn-default"
</div> />
</div> </div>
{{/if}}
{{#if this.model.can_change_location}}
<div class="control-group pref-location" data-setting-name="user-location">
<label class="control-label" for="edit-location">{{i18n
"user.location"
}}</label>
<div class="controls">
<Input
@type="text"
@value={{this.model.location}}
class="input-xxlarge"
id="edit-location"
/>
</div>
</div>
{{/if}}
{{#if this.model.can_change_website}}
<div class="control-group pref-website" data-setting-name="user-website">
<label class="control-label" for="edit-website">{{i18n
"user.website"
}}</label>
<div class="controls">
<Input
@type="text"
@value={{this.model.website}}
class="input-xxlarge"
id="edit-website"
/>
</div>
</div>
{{/if}}
{{/unless}}
{{#each this.userFields as |uf|}} {{#each this.userFields as |uf|}}
<div class="control-group" data-setting-name="user-user-fields"> <div class="control-group" data-setting-name="user-user-fields">
@ -61,128 +69,133 @@
{{/each}} {{/each}}
<div class="clearfix"></div> <div class="clearfix"></div>
{{#if this.siteSettings.allow_profile_backgrounds}} {{#unless this.showEnforcedRequiredFieldsNotice}}
{{#if this.canUploadProfileHeader}} {{#if this.siteSettings.allow_profile_backgrounds}}
<div {{#if this.canUploadProfileHeader}}
class="control-group pref-profile-bg" <div
data-setting-name="user-profile-bg" class="control-group pref-profile-bg"
> data-setting-name="user-profile-bg"
<label class="control-label">{{i18n >
"user.change_profile_background.title" <label class="control-label">{{i18n
}}</label> "user.change_profile_background.title"
<div class="controls"> }}</label>
<UppyImageUploader <div class="controls">
@imageUrl={{this.model.profile_background_upload_url}} <UppyImageUploader
@type="profile_background" @imageUrl={{this.model.profile_background_upload_url}}
@id="profile-background-uploader" @type="profile_background"
/> @id="profile-background-uploader"
/>
</div>
<div class="instructions">
{{i18n "user.change_profile_background.instructions"}}
</div>
</div> </div>
<div class="instructions">
{{i18n "user.change_profile_background.instructions"}}
</div>
</div>
{{/if}}
{{#if this.canUploadUserCardBackground}}
<div class="control-group pref-profile-bg" data-setting-name="user-card-bg">
<label class="control-label">{{i18n
"user.change_card_background.title"
}}</label>
<div class="controls">
<UppyImageUploader
@imageUrl={{this.model.card_background_upload_url}}
@type="card_background"
@id="profile-card-background-uploader"
/>
</div>
<div class="instructions">
{{i18n "user.change_card_background.instructions"}}
</div>
</div>
{{/if}}
{{/if}}
{{#if this.siteSettings.allow_featured_topic_on_user_profiles}}
<div class="control-group" data-setting-name="user-featured-topic">
<label class="control-label">{{i18n "user.featured_topic"}}</label>
{{#if this.model.featured_topic}}
<label class="featured-topic-link">
<LinkTo
@route="topic"
@models={{array
this.model.featured_topic.slug
this.model.featured_topic.id
}}
>
{{replace-emoji (html-safe this.model.featured_topic.fancy_title)}}
</LinkTo>
</label>
{{/if}} {{/if}}
{{#if this.canUploadUserCardBackground}}
<div
class="control-group pref-profile-bg"
data-setting-name="user-card-bg"
>
<label class="control-label">{{i18n
"user.change_card_background.title"
}}</label>
<div class="controls">
<UppyImageUploader
@imageUrl={{this.model.card_background_upload_url}}
@type="card_background"
@id="profile-card-background-uploader"
/>
</div>
<div class="instructions">
{{i18n "user.change_card_background.instructions"}}
</div>
</div>
{{/if}}
{{/if}}
<div> {{#if this.siteSettings.allow_featured_topic_on_user_profiles}}
<DButton <div class="control-group" data-setting-name="user-featured-topic">
@action={{this.showFeaturedTopicModal}} <label class="control-label">{{i18n "user.featured_topic"}}</label>
@label="user.feature_topic_on_profile.open_search"
class="btn-default feature-topic-on-profile-btn"
/>
{{#if this.model.featured_topic}} {{#if this.model.featured_topic}}
<DButton <label class="featured-topic-link">
@action={{action "clearFeaturedTopicFromProfile"}} <LinkTo
@label="user.feature_topic_on_profile.clear.title" @route="topic"
class="btn-danger clear-feature-topic-on-profile-btn" @models={{array
/> this.model.featured_topic.slug
this.model.featured_topic.id
}}
>
{{replace-emoji (html-safe this.model.featured_topic.fancy_title)}}
</LinkTo>
</label>
{{/if}} {{/if}}
<div>
<DButton
@action={{this.showFeaturedTopicModal}}
@label="user.feature_topic_on_profile.open_search"
class="btn-default feature-topic-on-profile-btn"
/>
{{#if this.model.featured_topic}}
<DButton
@action={{action "clearFeaturedTopicFromProfile"}}
@label="user.feature_topic_on_profile.clear.title"
class="btn-danger clear-feature-topic-on-profile-btn"
/>
{{/if}}
</div>
<div class="instructions">
{{i18n "user.change_featured_topic.instructions"}}
</div>
</div> </div>
<div class="instructions"> {{/if}}
{{i18n "user.change_featured_topic.instructions"}}
{{#if this.canChangeDefaultCalendar}}
<div class="control-group" data-setting-name="user-default-calendar">
<label class="control-label">{{i18n
"download_calendar.default_calendar"
}}</label>
<div>
<ComboBox
@valueProperty="value"
@content={{this.calendarOptions}}
@value={{this.model.user_option.default_calendar}}
@id="user-default-calendar"
@onChange={{fn (mut this.model.user_option.default_calendar)}}
/>
</div>
<div class="instructions">
{{i18n "download_calendar.default_calendar_instruction"}}
</div>
</div> </div>
</div> {{/if}}
{{/if}}
{{#if this.canChangeDefaultCalendar}} <span>
<div class="control-group" data-setting-name="user-default-calendar"> <PluginOutlet
<label class="control-label">{{i18n @name="user-preferences-profile"
"download_calendar.default_calendar" @connectorTagName="div"
}}</label> @outletArgs={{hash model=this.model save=(action "save")}}
<div> />
<ComboBox </span>
@valueProperty="value"
@content={{this.calendarOptions}}
@value={{this.model.user_option.default_calendar}}
@id="user-default-calendar"
@onChange={{fn (mut this.model.user_option.default_calendar)}}
/>
</div>
<div class="instructions">
{{i18n "download_calendar.default_calendar_instruction"}}
</div>
</div>
{{/if}}
<span> <span>
<PluginOutlet <PluginOutlet
@name="user-preferences-profile" @name="user-custom-preferences"
@connectorTagName="div" @connectorTagName="div"
@outletArgs={{hash model=this.model save=(action "save")}} @outletArgs={{hash model=this.model}}
/> />
</span> </span>
<span> <br />
<PluginOutlet
@name="user-custom-preferences"
@connectorTagName="div"
@outletArgs={{hash model=this.model}}
/>
</span>
<br /> <span>
<PluginOutlet
<span> @name="user-custom-controls"
<PluginOutlet @connectorTagName="div"
@name="user-custom-controls" @outletArgs={{hash model=this.model}}
@connectorTagName="div" />
@outletArgs={{hash model=this.model}} </span>
/> {{/unless}}
</span>
<SaveControls <SaveControls
@model={{this.model}} @model={{this.model}}

View File

@ -529,7 +529,7 @@ acceptance("User menu", function (needs) {
); );
assert.strictEqual( assert.strictEqual(
preferencesLink.textContent.trim(), preferencesLink.textContent.trim(),
I18n.t("user.preferences"), I18n.t("user.preferences.title"),
"preferences link has the right label" "preferences link has the right label"
); );
assert.ok( assert.ok(

View File

@ -41,6 +41,7 @@ class ApplicationController < ActionController::Base
before_action :authorize_mini_profiler before_action :authorize_mini_profiler
before_action :redirect_to_login_if_required before_action :redirect_to_login_if_required
before_action :block_if_requires_login before_action :block_if_requires_login
before_action :redirect_to_profile_if_required
before_action :preload_json before_action :preload_json
before_action :check_xhr before_action :check_xhr
after_action :add_readonly_header after_action :add_readonly_header
@ -907,6 +908,34 @@ class ApplicationController < ActionController::Base
(!SiteSetting.enforce_second_factor_on_external_auth && secure_session["oauth"] == "true") (!SiteSetting.enforce_second_factor_on_external_auth && secure_session["oauth"] == "true")
end end
def redirect_to_profile_if_required
return if request.format.json?
return if !current_user
return if !current_user.needs_required_fields_check?
if current_user.populated_required_custom_fields?
current_user.bump_required_fields_version
return
end
redirect_path = path("/u/#{current_user.encoded_username}/preferences/profile")
second_factor_path = path("/u/#{current_user.encoded_username}/preferences/second-factor")
allowed_paths = [redirect_path, second_factor_path, path("/admin")]
if allowed_paths.none? { |p| request.fullpath.start_with?(p) }
rate_limiter = RateLimiter.new(current_user, "redirect_to_required_fields_log", 1, 24.hours)
if rate_limiter.performed!(raise_error: false)
UserHistory.create!(
action: UserHistory.actions[:redirected_to_required_fields],
acting_user_id: current_user.id,
)
end
redirect_to path(redirect_path)
nil
end
end
def build_not_found_page(opts = {}) def build_not_found_page(opts = {})
if SiteSetting.bootstrap_error_pages? if SiteSetting.bootstrap_error_pages?
preload_json preload_json

View File

@ -3,7 +3,10 @@
class EmailController < ApplicationController class EmailController < ApplicationController
layout "no_ember" layout "no_ember"
skip_before_action :check_xhr, :preload_json, :redirect_to_login_if_required skip_before_action :check_xhr,
:preload_json,
:redirect_to_login_if_required,
:redirect_to_profile_if_required
def unsubscribe def unsubscribe
key = UnsubscribeKey.includes(:user).find_by(key: params[:key]) key = UnsubscribeKey.includes(:user).find_by(key: params[:key])

View File

@ -6,6 +6,7 @@ class ExtraLocalesController < ApplicationController
skip_before_action :check_xhr, skip_before_action :check_xhr,
:preload_json, :preload_json,
:redirect_to_login_if_required, :redirect_to_login_if_required,
:redirect_to_profile_if_required,
:verify_authenticity_token :verify_authenticity_token
OVERRIDES_BUNDLE ||= "overrides" OVERRIDES_BUNDLE ||= "overrides"

View File

@ -1,7 +1,10 @@
# frozen_string_literal: true # frozen_string_literal: true
class FinishInstallationController < ApplicationController class FinishInstallationController < ApplicationController
skip_before_action :check_xhr, :preload_json, :redirect_to_login_if_required skip_before_action :check_xhr,
:preload_json,
:redirect_to_login_if_required,
:redirect_to_profile_if_required
layout "finish_installation" layout "finish_installation"
before_action :ensure_no_admins, except: %w[confirm_email resend_email] before_action :ensure_no_admins, except: %w[confirm_email resend_email]

View File

@ -3,6 +3,7 @@
class HighlightJsController < ApplicationController class HighlightJsController < ApplicationController
skip_before_action :preload_json, skip_before_action :preload_json,
:redirect_to_login_if_required, :redirect_to_login_if_required,
:redirect_to_profile_if_required,
:check_xhr, :check_xhr,
:verify_authenticity_token, :verify_authenticity_token,
only: [:show] only: [:show]

View File

@ -16,6 +16,7 @@ class InvitesController < ApplicationController
skip_before_action :check_xhr, except: [:perform_accept_invitation] skip_before_action :check_xhr, except: [:perform_accept_invitation]
skip_before_action :preload_json, except: [:show] skip_before_action :preload_json, except: [:show]
skip_before_action :redirect_to_login_if_required skip_before_action :redirect_to_login_if_required
skip_before_action :redirect_to_profile_if_required
before_action :ensure_invites_allowed, only: %i[show perform_accept_invitation] before_action :ensure_invites_allowed, only: %i[show perform_accept_invitation]
before_action :ensure_new_registrations_allowed, only: %i[show perform_accept_invitation] before_action :ensure_new_registrations_allowed, only: %i[show perform_accept_invitation]

View File

@ -2,7 +2,10 @@
class MetadataController < ApplicationController class MetadataController < ApplicationController
layout false layout false
skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required skip_before_action :preload_json,
:check_xhr,
:redirect_to_login_if_required,
:redirect_to_profile_if_required
def manifest def manifest
expires_in 1.minutes expires_in 1.minutes

View File

@ -2,7 +2,10 @@
class OfflineController < ApplicationController class OfflineController < ApplicationController
layout false layout false
skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required skip_before_action :preload_json,
:check_xhr,
:redirect_to_login_if_required,
:redirect_to_profile_if_required
def index def index
render :offline, content_type: "text/html" render :offline, content_type: "text/html"

View File

@ -3,6 +3,7 @@
class PageviewController < ApplicationController class PageviewController < ApplicationController
skip_before_action :check_xhr, skip_before_action :check_xhr,
:redirect_to_login_if_required, :redirect_to_login_if_required,
:redirect_to_profile_if_required,
:preload_json, :preload_json,
:verify_authenticity_token :verify_authenticity_token

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
class PresenceController < ApplicationController class PresenceController < ApplicationController
skip_before_action :check_xhr skip_before_action :check_xhr, :redirect_to_profile_if_required
before_action :ensure_logged_in, only: [:update] before_action :ensure_logged_in, only: [:update]
before_action :skip_persist_session before_action :skip_persist_session

View File

@ -4,7 +4,7 @@ class PublishedPagesController < ApplicationController
skip_before_action :preload_json skip_before_action :preload_json
skip_before_action :check_xhr, :verify_authenticity_token, only: [:show] skip_before_action :check_xhr, :verify_authenticity_token, only: [:show]
before_action :ensure_publish_enabled before_action :ensure_publish_enabled
before_action :redirect_to_login_if_required, except: [:show] before_action :redirect_to_login_if_required, :redirect_to_profile_if_required, except: [:show]
def show def show
params.require(:slug) params.require(:slug)

View File

@ -1,7 +1,12 @@
# frozen_string_literal: true # frozen_string_literal: true
class QunitController < ApplicationController class QunitController < ApplicationController
skip_before_action *%i[check_xhr preload_json redirect_to_login_if_required] skip_before_action *%i[
check_xhr
preload_json
redirect_to_login_if_required
redirect_to_profile_if_required
]
layout false layout false
def theme def theme

View File

@ -2,7 +2,10 @@
class RobotsTxtController < ApplicationController class RobotsTxtController < ApplicationController
layout false layout false
skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required skip_before_action :preload_json,
:check_xhr,
:redirect_to_login_if_required,
:redirect_to_profile_if_required
OVERRIDDEN_HEADER = "# This robots.txt file has been customized at /admin/customize/robots\n" OVERRIDDEN_HEADER = "# This robots.txt file has been customized at /admin/customize/robots\n"

View File

@ -5,6 +5,7 @@ class SessionController < ApplicationController
only: %i[create forgot_password passkey_challenge passkey_login] only: %i[create forgot_password passkey_challenge passkey_login]
before_action :rate_limit_login, only: %i[create email_login] before_action :rate_limit_login, only: %i[create email_login]
skip_before_action :redirect_to_login_if_required skip_before_action :redirect_to_login_if_required
skip_before_action :redirect_to_profile_if_required
skip_before_action :preload_json, skip_before_action :preload_json,
:check_xhr, :check_xhr,
only: %i[sso sso_login sso_provider destroy one_time_password] only: %i[sso sso_login sso_provider destroy one_time_password]

View File

@ -3,7 +3,9 @@
class SiteController < ApplicationController class SiteController < ApplicationController
layout false layout false
skip_before_action :preload_json, :check_xhr skip_before_action :preload_json, :check_xhr
skip_before_action :redirect_to_login_if_required, only: %w[basic_info statistics] skip_before_action :redirect_to_login_if_required,
:redirect_to_profile_if_required,
only: %w[basic_info statistics]
def site def site
render json: Site.json_for(guardian) render json: Site.json_for(guardian)

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
class StaticController < ApplicationController class StaticController < ApplicationController
skip_before_action :check_xhr, :redirect_to_login_if_required skip_before_action :check_xhr, :redirect_to_login_if_required, :redirect_to_profile_if_required
skip_before_action :verify_authenticity_token, skip_before_action :verify_authenticity_token,
only: %i[cdn_asset enter favicon service_worker_asset] only: %i[cdn_asset enter favicon service_worker_asset]
skip_before_action :preload_json, only: %i[cdn_asset enter favicon service_worker_asset] skip_before_action :preload_json, only: %i[cdn_asset enter favicon service_worker_asset]

View File

@ -3,6 +3,7 @@
class StylesheetsController < ApplicationController class StylesheetsController < ApplicationController
skip_before_action :preload_json, skip_before_action :preload_json,
:redirect_to_login_if_required, :redirect_to_login_if_required,
:redirect_to_profile_if_required,
:check_xhr, :check_xhr,
:verify_authenticity_token, :verify_authenticity_token,
only: %i[show show_source_map color_scheme] only: %i[show show_source_map color_scheme]

View File

@ -3,6 +3,7 @@
class SvgSpriteController < ApplicationController class SvgSpriteController < ApplicationController
skip_before_action :preload_json, skip_before_action :preload_json,
:redirect_to_login_if_required, :redirect_to_login_if_required,
:redirect_to_profile_if_required,
:check_xhr, :check_xhr,
:verify_authenticity_token, :verify_authenticity_token,
only: %i[show search svg_icon] only: %i[show search svg_icon]

View File

@ -8,6 +8,7 @@ class ThemeJavascriptsController < ApplicationController
:handle_theme, :handle_theme,
:preload_json, :preload_json,
:redirect_to_login_if_required, :redirect_to_login_if_required,
:redirect_to_profile_if_required,
:verify_authenticity_token, :verify_authenticity_token,
only: %i[show show_map show_tests], only: %i[show show_map show_tests],
) )

View File

@ -11,6 +11,7 @@ class UploadsController < ApplicationController
skip_before_action :preload_json, skip_before_action :preload_json,
:check_xhr, :check_xhr,
:redirect_to_login_if_required, :redirect_to_login_if_required,
:redirect_to_profile_if_required,
only: %i[show show_short _show_secure_deprecated show_secure] only: %i[show show_short _show_secure_deprecated show_secure]
protect_from_forgery except: :show protect_from_forgery except: :show

View File

@ -4,7 +4,9 @@ class UserApiKeysController < ApplicationController
layout "no_ember" layout "no_ember"
requires_login only: %i[create create_otp revoke undo_revoke] requires_login only: %i[create create_otp revoke undo_revoke]
skip_before_action :redirect_to_login_if_required, only: %i[new otp] skip_before_action :redirect_to_login_if_required,
:redirect_to_profile_if_required,
only: %i[new otp]
skip_before_action :check_xhr, :preload_json skip_before_action :check_xhr, :preload_json
AUTH_API_VERSION ||= 4 AUTH_API_VERSION ||= 4

View File

@ -3,6 +3,7 @@
class UserAvatarsController < ApplicationController class UserAvatarsController < ApplicationController
skip_before_action :preload_json, skip_before_action :preload_json,
:redirect_to_login_if_required, :redirect_to_login_if_required,
:redirect_to_profile_if_required,
:check_xhr, :check_xhr,
:verify_authenticity_token, :verify_authenticity_token,
only: %i[show show_letter show_proxy_letter] only: %i[show show_letter show_proxy_letter]

View File

@ -2,7 +2,7 @@
# frozen_string_literal: true # frozen_string_literal: true
class Users::OmniauthCallbacksController < ApplicationController class Users::OmniauthCallbacksController < ApplicationController
skip_before_action :redirect_to_login_if_required skip_before_action :redirect_to_login_if_required, :redirect_to_profile_if_required
layout "no_ember" layout "no_ember"

View File

@ -86,6 +86,7 @@ class UsersController < ApplicationController
# once that happens you can't log in with social # once that happens you can't log in with social
skip_before_action :verify_authenticity_token, only: [:create] skip_before_action :verify_authenticity_token, only: [:create]
skip_before_action :redirect_to_login_if_required, skip_before_action :redirect_to_login_if_required,
:redirect_to_profile_if_required,
only: %i[ only: %i[
check_username check_username
check_email check_email
@ -102,6 +103,7 @@ class UsersController < ApplicationController
admin_login admin_login
confirm_admin confirm_admin
] ]
skip_before_action :redirect_to_profile_if_required, only: %i[show staff_info update]
after_action :add_noindex_header, only: %i[show my_redirect] after_action :add_noindex_header, only: %i[show my_redirect]

View File

@ -6,6 +6,7 @@ class UsersEmailController < ApplicationController
skip_before_action :check_xhr, only: %i[show_confirm_old_email show_confirm_new_email] skip_before_action :check_xhr, only: %i[show_confirm_old_email show_confirm_new_email]
skip_before_action :redirect_to_login_if_required, skip_before_action :redirect_to_login_if_required,
:redirect_to_profile_if_required,
only: %i[ only: %i[
show_confirm_old_email show_confirm_old_email
show_confirm_new_email show_confirm_new_email

View File

@ -1843,6 +1843,21 @@ class User < ActiveRecord::Base
end end
end end
def populated_required_custom_fields?
UserField
.required
.pluck(:id)
.all? { |field_id| custom_fields["#{User::USER_FIELD_PREFIX}#{field_id}"].present? }
end
def needs_required_fields_check?
(required_fields_version || 0) < UserRequiredFieldsVersion.current
end
def bump_required_fields_version
update(required_fields_version: UserRequiredFieldsVersion.current)
end
protected protected
def badge_grant def badge_grant
@ -2225,6 +2240,7 @@ end
# flair_group_id :integer # flair_group_id :integer
# last_seen_reviewable_id :integer # last_seen_reviewable_id :integer
# password_algorithm :string(64) # password_algorithm :string(64)
# required_fields_version :integer
# #
# Indexes # Indexes
# #

View File

@ -15,9 +15,12 @@ class UserField < ActiveRecord::Base
accepts_nested_attributes_for :user_field_options accepts_nested_attributes_for :user_field_options
before_save :sanitize_description before_save :sanitize_description
after_create :update_required_fields_version
after_update :update_required_fields_version, if: -> { saved_change_to_requirement? }
after_save :queue_index_search after_save :queue_index_search
scope :public_fields, -> { where(show_on_profile: true).or(where(show_on_user_card: true)) } scope :public_fields, -> { where(show_on_profile: true).or(where(show_on_user_card: true)) }
scope :required, -> { not_optional }
enum :requirement, { optional: 0, for_all_users: 1, on_signup: 2 }.freeze enum :requirement, { optional: 0, for_all_users: 1, on_signup: 2 }.freeze
enum :field_type_enum, { text: 0, confirm: 1, dropdown: 2, multiselect: 3 }.freeze enum :field_type_enum, { text: 0, confirm: 1, dropdown: 2, multiselect: 3 }.freeze
@ -37,6 +40,13 @@ class UserField < ActiveRecord::Base
private private
def update_required_fields_version
return if !for_all_users?
UserRequiredFieldsVersion.create
Discourse.request_refresh!
end
def sanitize_description def sanitize_description
if description_changed? if description_changed?
self.description = sanitize_field(self.description, additional_attributes: ["target"]) self.description = sanitize_field(self.description, additional_attributes: ["target"])

View File

@ -144,6 +144,8 @@ class UserHistory < ActiveRecord::Base
create_watched_word_group: 105, create_watched_word_group: 105,
update_watched_word_group: 106, update_watched_word_group: 106,
delete_watched_word_group: 107, delete_watched_word_group: 107,
redirected_to_required_fields: 108,
filled_in_required_fields: 109,
) )
end end

View File

@ -0,0 +1,14 @@
# frozen_string_literal: true
class UserRequiredFieldsVersion < ActiveRecord::Base
def self.current = maximum(:id) || 0
end
# == Schema Information
#
# Table name: user_required_fields_versions
#
# id :bigint not null, primary key
# created_at :datetime not null
# updated_at :datetime not null
#

View File

@ -58,6 +58,7 @@ class CurrentUserSerializer < BasicUserSerializer
:associated_account_ids, :associated_account_ids,
:top_category_ids, :top_category_ids,
:groups, :groups,
:needs_required_fields_check?,
:second_factor_enabled, :second_factor_enabled,
:ignored_users, :ignored_users,
:featured_topic, :featured_topic,

View File

@ -266,6 +266,13 @@ class UserUpdater
end end
end end
DiscourseEvent.trigger(:user_updated, user) DiscourseEvent.trigger(:user_updated, user)
if attributes[:custom_fields].present? && user.needs_required_fields_check?
UserHistory.create!(
action: UserHistory.actions[:filled_in_required_fields],
acting_user_id: user.id,
)
end
end end
saved saved

View File

@ -1197,7 +1197,10 @@ en:
activity_stream: "Activity" activity_stream: "Activity"
read: "Read" read: "Read"
read_help: "Recently read topics" read_help: "Recently read topics"
preferences: "Preferences" preferences:
title: "Preferences"
profile:
enforced_required_fields: "You are required to provide additional information before continuing to use this site."
feature_topic_on_profile: feature_topic_on_profile:
open_search: "Select a New Topic" open_search: "Select a New Topic"
title: "Select a Topic" title: "Select a Topic"
@ -6669,9 +6672,13 @@ en:
title: "Field Requirement" title: "Field Requirement"
optional: optional:
title: "Optional" title: "Optional"
for_all_users:
title: "For all users"
description: "When new users sign up, they must fill out this field. When existing users return to the site and this is a new required field for them, they will also be prompted to fill it out. To re-prompt all users, delete this custom field and re-create it."
on_signup: on_signup:
title: "On signup" title: "On signup"
description: "When new users sign up, they must fill out this field. Existing users are unaffected." description: "When new users sign up, they must fill out this field. Existing users are unaffected."
confirmation: "This will prompt existing users to fill in this field and will not allow them to do anything else on your site until the field is filled. Proceed?"
editable: editable:
title: "Editable after signup" title: "Editable after signup"
enabled: "editable" enabled: "editable"

View File

@ -2931,7 +2931,7 @@ en:
email_too_long: "The email you provided is too long. Mailbox names must be no more than 254 characters, and domain names must be no more than 253 characters." email_too_long: "The email you provided is too long. Mailbox names must be no more than 254 characters, and domain names must be no more than 253 characters."
wrong_invite_code: "The invite code you entered was incorrect." wrong_invite_code: "The invite code you entered was incorrect."
reserved_username: "That username is not allowed." reserved_username: "That username is not allowed."
missing_user_field: "You have not completed all the user fields" missing_user_field: "You have not completed all the required user fields"
auth_complete: "Authentication is complete." auth_complete: "Authentication is complete."
click_to_continue: "Click here to continue." click_to_continue: "Click here to continue."
already_logged_in: "Sorry! This invitation is intended for new users, who do not already have an existing account." already_logged_in: "Sorry! This invitation is intended for new users, who do not already have an existing account."

View File

@ -0,0 +1,11 @@
# frozen_string_literal: true
class CreateUserRequiredFieldsVersion < ActiveRecord::Migration[7.0]
def change
create_table :user_required_fields_versions do |t|
t.timestamps null: false
end
add_column :users, :required_fields_version, :integer
end
end

View File

@ -8,7 +8,9 @@ module Chat
WEBHOOK_MESSAGES_PER_MINUTE_LIMIT = 10 WEBHOOK_MESSAGES_PER_MINUTE_LIMIT = 10
skip_before_action :verify_authenticity_token, :redirect_to_login_if_required skip_before_action :verify_authenticity_token,
:redirect_to_login_if_required,
:redirect_to_profile_if_required
before_action :validate_payload before_action :validate_payload

View File

@ -3552,4 +3552,45 @@ RSpec.describe User do
expect(user.new_personal_messages_notifications_count).to eq(1) expect(user.new_personal_messages_notifications_count).to eq(1)
end end
end end
describe "#populated_required_fields?" do
let!(:required_field) { Fabricate(:user_field, name: "hairstyle") }
let!(:optional_field) { Fabricate(:user_field, name: "haircolor", requirement: "optional") }
context "when all required fields are populated" do
before { user.set_user_field(required_field.id, "bald") }
it { expect(user.populated_required_custom_fields?).to eq(true) }
end
context "when some required fields are missing values" do
it { expect(user.populated_required_custom_fields?).to eq(false) }
end
end
describe "#needs_required_fields_check?" do
let!(:version) { UserRequiredFieldsVersion.create! }
context "when version number is up to date" do
before { user.update(required_fields_version: version.id) }
it { expect(user.needs_required_fields_check?).to eq(false) }
end
context "when version number is out of date" do
before { user.update(required_fields_version: version.id - 1) }
it { expect(user.needs_required_fields_check?).to eq(true) }
end
end
describe "#bump_required_fields_version" do
let!(:version) { UserRequiredFieldsVersion.create! }
it do
expect { user.bump_required_fields_version }.to change { user.required_fields_version }.to(
version.id,
)
end
end
end end

View File

@ -256,6 +256,45 @@ RSpec.describe ApplicationController do
end end
end end
describe "#redirect_to_profile_if_required" do
fab!(:user)
before { sign_in(user) }
context "when the user is missing required custom fields" do
before do
Fabricate(:user_field, requirement: "for_all_users")
UserRequiredFieldsVersion.create!
end
it "redirects the user to the profile preferences" do
get "/hot"
expect(response).to redirect_to("/u/#{user.username}/preferences/profile")
end
it "only logs user history once per day" do
expect do
RateLimiter.enable
get "/hot"
get "/hot"
end.to change { UserHistory.count }.by(1)
end
end
context "when the user has filled up all required custom fields" do
before do
Fabricate(:user_field, requirement: "for_all_users")
UserRequiredFieldsVersion.create!
user.bump_required_fields_version
end
it "redirects the user to the profile preferences" do
get "/hot"
expect(response).not_to redirect_to("/u/#{user.username}/preferences/profile")
end
end
end
describe "invalid request params" do describe "invalid request params" do
before do before do
@old_logger = Rails.logger @old_logger = Rails.logger

View File

@ -602,40 +602,61 @@ RSpec.describe UserUpdater do
end end
end end
it "logs the action" do context "when updating the name" do
user = Fabricate(:user, name: "Billy Bob") it "logs the action" do
user = Fabricate(:user, name: "Billy Bob")
expect do UserUpdater.new(user, user).update(name: "Jim Tom") end.to change { expect do UserUpdater.new(user, user).update(name: "Jim Tom") end.to change {
UserHistory.count UserHistory.count
}.by(1) }.by(1)
expect(UserHistory.last.action).to eq(UserHistory.actions[:change_name]) expect(UserHistory.last.action).to eq(UserHistory.actions[:change_name])
expect do UserUpdater.new(user, user).update(name: "JiM TOm") end.to_not change { expect do UserUpdater.new(user, user).update(name: "JiM TOm") end.to_not change {
UserHistory.count UserHistory.count
} }
expect do UserUpdater.new(user, user).update(bio_raw: "foo bar") end.to_not change { expect do UserUpdater.new(user, user).update(bio_raw: "foo bar") end.to_not change {
UserHistory.count UserHistory.count
} }
user_without_name = Fabricate(:user, name: nil) user_without_name = Fabricate(:user, name: nil)
expect do expect do
UserUpdater.new(user_without_name, user_without_name).update(bio_raw: "foo bar") UserUpdater.new(user_without_name, user_without_name).update(bio_raw: "foo bar")
end.to_not change { UserHistory.count } end.to_not change { UserHistory.count }
expect do expect do
UserUpdater.new(user_without_name, user_without_name).update(name: "Jim Tom") UserUpdater.new(user_without_name, user_without_name).update(name: "Jim Tom")
end.to change { UserHistory.count }.by(1) end.to change { UserHistory.count }.by(1)
expect(UserHistory.last.action).to eq(UserHistory.actions[:change_name]) expect(UserHistory.last.action).to eq(UserHistory.actions[:change_name])
expect do UserUpdater.new(user, user).update(name: "") end.to change { UserHistory.count }.by( expect do UserUpdater.new(user, user).update(name: "") end.to change {
1, UserHistory.count
) }.by(1)
expect(UserHistory.last.action).to eq(UserHistory.actions[:change_name]) expect(UserHistory.last.action).to eq(UserHistory.actions[:change_name])
end
end
context "when updating required fields" do
it "logs the action" do
user = Fabricate(:user)
Fabricate(:user_field, name: "favorite_pokemon", requirement: "for_all_users")
UserRequiredFieldsVersion.create!
expect do
UserUpdater.new(user, user).update(custom_fields: { "favorite_pokemon" => "Mudkip" })
end.to change { UserHistory.count }.by(1)
user.bump_required_fields_version
expect do
UserUpdater.new(user, user).update(custom_fields: { "favorite_pokemon" => "Mudkip" })
end.not_to change { UserHistory.count }
end
end end
it "clears the homepage_id when the special 'custom' id is chosen" do it "clears the homepage_id when the special 'custom' id is chosen" do

View File

@ -25,4 +25,38 @@ describe "Admin User Fields", type: :system, js: true do
expect(user_fields_page).to have_text(/Description can't be blank/) expect(user_fields_page).to have_text(/Description can't be blank/)
end end
it "makes sure new required fields are editable after signup" do
user_fields_page.visit
page.find(".user-fields .btn-primary").click
form = page.find(".user-field")
editable_label = I18n.t("admin_js.admin.user_fields.editable.title")
user_fields_page.choose_requirement("for_all_users")
expect(form).to have_field(editable_label, checked: true, disabled: true)
user_fields_page.choose_requirement("optional")
expect(form).to have_field(editable_label, checked: false, disabled: false)
end
it "requires confirmation when applying required fields retroactively" do
user_fields_page.visit
page.find(".user-fields .btn-primary").click
form = page.find(".user-field")
form.find(".user-field-name").fill_in(with: "Favourite Pokémon")
form.find(".user-field-desc").fill_in(with: "Hint: It's Mudkip")
user_fields_page.choose_requirement("for_all_users")
form.find(".btn-primary").click
expect(page).to have_text(I18n.t("admin_js.admin.user_fields.requirement.confirmation"))
end
end end

View File

@ -8,6 +8,12 @@ module PageObjects
self self
end end
def choose_requirement(requirement)
form = page.find(".user-field")
form.choose(I18n.t("admin_js.admin.user_fields.requirement.#{requirement}.title"))
end
def add_field(name: nil, description: nil, requirement: nil, preferences: []) def add_field(name: nil, description: nil, requirement: nil, preferences: [])
page.find(".user-fields .btn-primary").click page.find(".user-fields .btn-primary").click

View File

@ -0,0 +1,12 @@
# frozen_string_literal: true
module PageObjects
module Pages
class UserPreferencesProfile < PageObjects::Pages::Base
def visit(user)
page.visit("/u/#{user.username}/preferences/profile")
self
end
end
end
end

View File

@ -0,0 +1,52 @@
# frozen_string_literal: true
describe "User preferences | Profile", type: :system do
fab!(:user) { Fabricate(:user, active: true) }
let(:user_preferences_profile_page) { PageObjects::Pages::UserPreferencesProfile.new }
let(:user_preferences_page) { PageObjects::Pages::UserPreferences.new }
before { sign_in(user) }
describe "enforcing required fields" do
before do
UserRequiredFieldsVersion.create!
UserField.create!(
field_type: "text",
name: "Favourite Pokemon",
description: "Hint: It's Mudkip.",
requirement: :for_all_users,
editable: true,
)
end
it "redirects to the profile page to fill up required fields" do
visit("/")
expect(page).to have_current_path("/u/bruce0/preferences/profile")
expect(page).to have_selector(
".alert-error",
text: I18n.t("js.user.preferences.profile.enforced_required_fields"),
)
end
it "disables client-side routing while missing required fields" do
user_preferences_profile_page.visit(user)
find("#site-logo").click
expect(page).to have_current_path("/u/bruce0/preferences/profile")
end
it "allows user to fill up required fields" do
user_preferences_profile_page.visit(user)
find(".user-field-favourite-pokemon input").fill_in(with: "Mudkip")
find(".save-button .btn-primary").click
visit("/")
expect(page).to have_current_path("/")
end
end
end