From 6bce3004d905af443c81a11c4a8219947961dd77 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 13 Sep 2017 16:44:47 -0400 Subject: [PATCH] UX: Nicer selection of suspend duration --- .../modals/admin-suspend-user.js.es6 | 29 +++++++++---------- .../templates/modal/admin-suspend-user.hbs | 9 +++--- .../admin/templates/user-index.hbs | 8 +++-- .../components/combo-box.js.es6 | 2 +- .../components/auto-update-input.js.es6 | 5 ++++ .../discourse/helpers/format-date.js.es6 | 2 -- .../components/auto-update-input.hbs | 2 +- .../stylesheets/common/admin/suspend.scss | 2 +- app/controllers/admin/users_controller.rb | 2 +- .../admin_detailed_user_serializer.rb | 1 + config/locales/client.en.yml | 2 +- .../admin/users_controller_spec.rb | 9 +++--- spec/jobs/user_email_spec.rb | 15 +++++----- .../acceptance/admin-suspend-user-test.js.es6 | 6 ++-- 14 files changed, 51 insertions(+), 43 deletions(-) diff --git a/app/assets/javascripts/admin/controllers/modals/admin-suspend-user.js.es6 b/app/assets/javascripts/admin/controllers/modals/admin-suspend-user.js.es6 index 4c3f2a91990..8d93aea3c29 100644 --- a/app/assets/javascripts/admin/controllers/modals/admin-suspend-user.js.es6 +++ b/app/assets/javascripts/admin/controllers/modals/admin-suspend-user.js.es6 @@ -3,40 +3,37 @@ import computed from 'ember-addons/ember-computed-decorators'; import { popupAjaxError } from 'discourse/lib/ajax-error'; export default Ember.Controller.extend(ModalFunctionality, { - duration: null, + suspendUntil: null, reason: null, message: null, loading: false, onShow() { this.setProperties({ - duration: null, + suspendUntil: null, reason: null, message: null, loading: false }); }, - @computed('reason', 'loading') - submitDisabled(reason, loading) { - return (loading || !reason || reason.length < 1); + @computed('suspendUntil', 'reason', 'loading') + submitDisabled(suspendUntil, reason, loading) { + return (loading || Ember.isEmpty(suspendUntil) || !reason || reason.length < 1); }, actions: { suspend() { if (this.get('submitDisabled')) { return; } - let duration = parseInt(this.get('duration'), 10); - if (duration > 0) { - this.set('loading', true); - this.get('model').suspend({ - duration, - reason: this.get('reason'), - message: this.get('message') - }).then(() => { - this.send('closeModal'); - }).catch(popupAjaxError).finally(() => this.set('loading', false)); - } + this.set('loading', true); + this.get('model').suspend({ + suspend_until: this.get('suspendUntil'), + reason: this.get('reason'), + message: this.get('message') + }).then(() => { + this.send('closeModal'); + }).catch(popupAjaxError).finally(() => this.set('loading', false)); } } diff --git a/app/assets/javascripts/admin/templates/modal/admin-suspend-user.hbs b/app/assets/javascripts/admin/templates/modal/admin-suspend-user.hbs index 2810a0ed387..311782a68b1 100644 --- a/app/assets/javascripts/admin/templates/modal/admin-suspend-user.hbs +++ b/app/assets/javascripts/admin/templates/modal/admin-suspend-user.hbs @@ -1,9 +1,10 @@ {{#d-modal-body title="admin.user.suspend_modal_title"}} -
+
diff --git a/app/assets/javascripts/admin/templates/user-index.hbs b/app/assets/javascripts/admin/templates/user-index.hbs index 7b40e0469b4..0b42c80bcb2 100644 --- a/app/assets/javascripts/admin/templates/user-index.hbs +++ b/app/assets/javascripts/admin/templates/user-index.hbs @@ -298,7 +298,12 @@
{{i18n 'admin.user.suspended'}}
-
{{i18n-yes-no model.isSuspended}}
+
+ {{i18n-yes-no model.isSuspended}} + {{#if model.isSuspended}} + {{i18n "admin.user.suspended_until" until=model.suspendedTillDate}} + {{/if}} +
{{#if model.isSuspended}} {{d-button @@ -306,7 +311,6 @@ action=(action "unsuspend") icon="ban" label="admin.user.unsuspend"}} - {{suspendDuration}} {{i18n 'admin.user.suspended_explanation'}} {{else}} {{#if model.canSuspend}} diff --git a/app/assets/javascripts/discourse-common/components/combo-box.js.es6 b/app/assets/javascripts/discourse-common/components/combo-box.js.es6 index 0d8cd538b8a..9e8a97e66d1 100644 --- a/app/assets/javascripts/discourse-common/components/combo-box.js.es6 +++ b/app/assets/javascripts/discourse-common/components/combo-box.js.es6 @@ -124,7 +124,7 @@ export default Ember.Component.extend(bufferedRender({ if (val && val.length && castInteger) { val = parseInt(val, 10); } - this.set('value', val); + Ember.run(() => this.set('value', val)); }); Ember.run.scheduleOnce('afterRender', this, this._triggerChange); diff --git a/app/assets/javascripts/discourse/components/auto-update-input.js.es6 b/app/assets/javascripts/discourse/components/auto-update-input.js.es6 index be97128e141..900ecfa34e3 100644 --- a/app/assets/javascripts/discourse/components/auto-update-input.js.es6 +++ b/app/assets/javascripts/discourse/components/auto-update-input.js.es6 @@ -13,6 +13,7 @@ export default Ember.Component.extend({ time: null, isCustom: Ember.computed.equal('selection', PICK_DATE_AND_TIME), isBasedOnLastPost: Ember.computed.equal('selection', SET_BASED_ON_LAST_POST), + displayLabel: null, init() { this._super(); @@ -64,6 +65,10 @@ export default Ember.Component.extend({ } }, + didReceiveAttrs() { + this.set('displayLabel', I18n.t(this.get('label') || 'topic.topic_status_update.when')); + }, + @computed("statusType", "input", "isCustom", "date", "time", "willCloseImmediately", "categoryId") showTopicStatusInfo(statusType, input, isCustom, date, time, willCloseImmediately, categoryId) { if (!statusType || willCloseImmediately) return false; diff --git a/app/assets/javascripts/discourse/helpers/format-date.js.es6 b/app/assets/javascripts/discourse/helpers/format-date.js.es6 index 84602bebc82..046c5f93702 100644 --- a/app/assets/javascripts/discourse/helpers/format-date.js.es6 +++ b/app/assets/javascripts/discourse/helpers/format-date.js.es6 @@ -25,5 +25,3 @@ registerUnbound('format-date', function(val, params) { return new Handlebars.SafeString(autoUpdatingRelativeAge(date, {format: format, title: title, leaveAgo: leaveAgo})); } }); - - diff --git a/app/assets/javascripts/discourse/templates/components/auto-update-input.hbs b/app/assets/javascripts/discourse/templates/components/auto-update-input.hbs index 50f0a8ee258..9538d49a181 100644 --- a/app/assets/javascripts/discourse/templates/components/auto-update-input.hbs +++ b/app/assets/javascripts/discourse/templates/components/auto-update-input.hbs @@ -1,6 +1,6 @@
- + {{auto-update-input-selector valueAttribute="id" diff --git a/app/assets/stylesheets/common/admin/suspend.scss b/app/assets/stylesheets/common/admin/suspend.scss index ba0682cdf2e..efdfb68f791 100644 --- a/app/assets/stylesheets/common/admin/suspend.scss +++ b/app/assets/stylesheets/common/admin/suspend.scss @@ -1,6 +1,6 @@ .suspend-user-modal { - .duration-controls { + .until-controls { margin-bottom: 1em; } diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index d8b21aa0b61..889e54edc26 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -53,7 +53,7 @@ class Admin::UsersController < Admin::AdminController def suspend guardian.ensure_can_suspend!(@user) - @user.suspended_till = params[:duration].to_i.days.from_now + @user.suspended_till = params[:suspend_until] @user.suspended_at = DateTime.now message = params[:message] diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb index 60c9ea55f20..6df084c7274 100644 --- a/app/serializers/admin_detailed_user_serializer.rb +++ b/app/serializers/admin_detailed_user_serializer.rb @@ -17,6 +17,7 @@ class AdminDetailedUserSerializer < AdminUserSerializer :can_be_deleted, :can_be_anonymized, :suspend_reason, + :suspended_till, :primary_group_id, :badge_count, :warnings_received_count, diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6a13aeeba00..5ecba1d0525 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3264,7 +3264,6 @@ en: suspend_failed: "Something went wrong suspending this user {{error}}" unsuspend_failed: "Something went wrong unsuspending this user {{error}}" suspend_duration: "How long will the user be suspended for?" - suspend_duration_units: "(days)" suspend_reason_label: "Why are you suspending? This text will be visible to everyone on this user's profile page, and will be shown to the user when they try to log in. Keep it short." suspend_reason_hidden_label: "Why are you suspending? This text will be shown to the user when they try to log in. Keep it short." suspend_reason: "Reason" @@ -3272,6 +3271,7 @@ en: suspend_message: "Email Message" suspend_message_placeholder: "Optionally, provide more information about the suspension and it will be emailed to the user." suspended_by: "Suspended by" + suspended_until: "(until %{until})" delete_all_posts: "Delete all posts" # keys ending with _MF use message format, see https://meta.discourse.org/t/message-format-support-for-localization/7035 for details diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 8a714f29721..d072b1cbcec 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -128,7 +128,7 @@ describe Admin::UsersController do put( :suspend, user_id: user.id, - duration: 10, + suspend_until: 5.hours.from_now, reason: "because I said so", format: :json ) @@ -149,15 +149,14 @@ describe Admin::UsersController do :critical_user_email, has_entries( type: :account_suspended, - user_id: user.id, - message: "long reason" + user_id: user.id ) ) put( :suspend, user_id: user.id, - duration: 10, + suspend_until: 10.days.from_now, reason: "short reason", message: "long reason", format: :json @@ -168,10 +167,12 @@ describe Admin::UsersController do expect(log).to be_present expect(log.details).to match(/short reason/) expect(log.details).to match(/long reason/) + end it "also revoke any api keys" do User.any_instance.expects(:revoke_api_key) put :suspend, params: { user_id: evil_trout.id }, format: :json + expect(log.context).to match(/long reason/) end end diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb index b72e167582d..67876f14ad3 100644 --- a/spec/jobs/user_email_spec.rb +++ b/spec/jobs/user_email_spec.rb @@ -197,14 +197,13 @@ describe Jobs::UserEmail do it "doesn't send the email if the notification has been seen" do notification.update_column(:read, true) message, err = Jobs::UserEmail.new.message_for_email( - user, - post, - :user_mentioned, - notification, - notification.notification_type, - notification.data_hash, - nil, - nil) + user, + post, + :user_mentioned, + notification, + notification_type: notification.notification_type, + notification_data_hash: notification.data_hash + ) expect(message).to eq nil expect(err.skipped_reason).to match(/notification.*already/) diff --git a/test/javascripts/acceptance/admin-suspend-user-test.js.es6 b/test/javascripts/acceptance/admin-suspend-user-test.js.es6 index 0c2c62b0db5..11659f33a5a 100644 --- a/test/javascripts/acceptance/admin-suspend-user-test.js.es6 +++ b/test/javascripts/acceptance/admin-suspend-user-test.js.es6 @@ -43,12 +43,14 @@ QUnit.test("suspend, then unsuspend a user", assert => { andThen(() => { assert.equal(find('.perform-suspend[disabled]').length, 1, 'disabled by default'); + find('.suspend-until .combobox').select2('val', 'tomorrow'); + find('.suspend-until .combobox').trigger('change', 'tomorrow'); }); - fillIn('.suspend-duration', 12); + fillIn('.suspend-reason', "for breaking the rules"); fillIn('.suspend-message', "this is an email reason why"); andThen(() => { - assert.equal(find('.perform-suspend[disabled]').length, 0); + assert.equal(find('.perform-suspend[disabled]').length, 0, 'no longer disabled'); }); click('.perform-suspend'); andThen(() => {