From a95303fcd81f304f9624e30076cf48fcd05432f3 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 21 Aug 2013 10:49:35 -0400 Subject: [PATCH] Log site customization changes. Use a modal to show staff action log details for site customizations. --- ...e_site_customization_details_controller.js | 25 ++++++++++++++++ .../admin/models/staff_action_log.js | 12 ++++++-- .../admin/routes/admin_logs_routes.js | 7 +++++ ...customization_change_details.js.handlebars | 14 +++++++++ ...e_customization_change_modal.js.handlebars | 29 +++++++++++++++++++ .../staff_action_logs_list_item.js.handlebars | 12 +++++--- .../change_site_customization_details_view.js | 12 ++++++++ app/assets/stylesheets/admin/admin_base.scss | 6 ++++ .../stylesheets/application/modal.css.scss | 7 +++++ .../admin/site_customizations_controller.rb | 9 ++++++ app/models/staff_action_log.rb | 10 ++++++- .../staff_action_log_serializer.rb | 16 ++++++++++ app/services/staff_action_logger.rb | 11 +++++++ config/locales/client.en.yml | 8 +++++ .../site_customizations_controller_spec.rb | 5 ++++ spec/services/staff_action_logger_spec.rb | 27 +++++++++++++++++ 16 files changed, 202 insertions(+), 8 deletions(-) create mode 100644 app/assets/javascripts/admin/controllers/change_site_customization_details_controller.js create mode 100644 app/assets/javascripts/admin/templates/logs/_site_customization_change_details.js.handlebars create mode 100644 app/assets/javascripts/admin/templates/logs/site_customization_change_modal.js.handlebars create mode 100644 app/assets/javascripts/admin/views/modals/change_site_customization_details_view.js diff --git a/app/assets/javascripts/admin/controllers/change_site_customization_details_controller.js b/app/assets/javascripts/admin/controllers/change_site_customization_details_controller.js new file mode 100644 index 00000000000..7fe0d014f08 --- /dev/null +++ b/app/assets/javascripts/admin/controllers/change_site_customization_details_controller.js @@ -0,0 +1,25 @@ +/** + The modal for viewing the details of a staff action log record. + + @class ChangeSiteCustomizationDetailsController + @extends Discourse.Controller + @namespace Discourse + @uses Discourse.ModalFunctionality + @module Discourse +**/ +Discourse.ChangeSiteCustomizationDetailsController = Discourse.ObjectController.extend(Discourse.ModalFunctionality, { + previousSelected: Ember.computed.equal('selectedTab', 'previous'), + newSelected: Ember.computed.equal('selectedTab', 'new'), + + onShow: function() { + this.selectNew(); + }, + + selectNew: function() { + this.set('selectedTab', 'new'); + }, + + selectPrevious: function() { + this.set('selectedTab', 'previous'); + } +}); diff --git a/app/assets/javascripts/admin/models/staff_action_log.js b/app/assets/javascripts/admin/models/staff_action_log.js index e52d9bf60e2..aa5a93ea6f4 100644 --- a/app/assets/javascripts/admin/models/staff_action_log.js +++ b/app/assets/javascripts/admin/models/staff_action_log.js @@ -17,8 +17,10 @@ Discourse.StaffActionLog = Discourse.Model.extend({ var formatted = ""; formatted += this.format('email', 'email'); formatted += this.format('admin.logs.staff_actions.ip_address', 'ip_address'); - formatted += this.format('admin.logs.staff_actions.new_value', 'new_value'); - formatted += this.format('admin.logs.staff_actions.previous_value', 'previous_value'); + if (!this.get('useModalForDetails')) { + formatted += this.format('admin.logs.staff_actions.new_value', 'new_value'); + formatted += this.format('admin.logs.staff_actions.previous_value', 'previous_value'); + } return formatted; }.property('ip_address', 'email'), @@ -28,7 +30,11 @@ Discourse.StaffActionLog = Discourse.Model.extend({ } else { return ''; } - } + }, + + useModalForDetails: function() { + return (this.get('action_name') === 'change_site_customization'); + }.property('action_name') }); Discourse.StaffActionLog.reopenClass({ diff --git a/app/assets/javascripts/admin/routes/admin_logs_routes.js b/app/assets/javascripts/admin/routes/admin_logs_routes.js index c14010109ed..94bb150297f 100644 --- a/app/assets/javascripts/admin/routes/admin_logs_routes.js +++ b/app/assets/javascripts/admin/routes/admin_logs_routes.js @@ -33,6 +33,13 @@ Discourse.AdminLogsStaffActionLogsRoute = Discourse.Route.extend({ return controller.show(); }, + events: { + showDetailsModal: function(logRecord) { + Discourse.Route.showModal(this, logRecord.action_name + '_details', logRecord); + this.controllerFor('modal').set('modalClass', 'tabbed-modal log-details-modal'); + } + }, + deactivate: function() { this._super(); diff --git a/app/assets/javascripts/admin/templates/logs/_site_customization_change_details.js.handlebars b/app/assets/javascripts/admin/templates/logs/_site_customization_change_details.js.handlebars new file mode 100644 index 00000000000..cb4c50fa73a --- /dev/null +++ b/app/assets/javascripts/admin/templates/logs/_site_customization_change_details.js.handlebars @@ -0,0 +1,14 @@ +
+ {{i18n admin.customize.css}}: ({{i18n character_count count=stylesheet.length}})
+ {{textarea value=stylesheet class="plain"}} +
+
+ {{i18n admin.customize.header}}: ({{i18n character_count count=header.length}})
+ {{textarea value=header class="plain"}} +
+
+ {{i18n admin.customize.enabled}}: {{enabled}} +
+
+ {{i18n admin.customize.override_default}}: {{override_default_style}} +
\ No newline at end of file diff --git a/app/assets/javascripts/admin/templates/logs/site_customization_change_modal.js.handlebars b/app/assets/javascripts/admin/templates/logs/site_customization_change_modal.js.handlebars new file mode 100644 index 00000000000..ca30597ed67 --- /dev/null +++ b/app/assets/javascripts/admin/templates/logs/site_customization_change_modal.js.handlebars @@ -0,0 +1,29 @@ +
+ +
+
+ {{#with new_value}} + {{partial "admin/templates/logs/site_customization_change_details"}} + {{/with}} +
+
+ {{#if previous_value}} + {{#with previous_value}} + {{partial "admin/templates/logs/site_customization_change_details"}} + {{/with}} + {{else}} + {{i18n admin.logs.staff_actions.no_previous}} + {{/if}} +
+
+
+ +
+
\ No newline at end of file diff --git a/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.js.handlebars b/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.js.handlebars index 3da2168aa0d..7296db2f08d 100644 --- a/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.js.handlebars +++ b/app/assets/javascripts/admin/templates/logs/staff_action_logs_list_item.js.handlebars @@ -17,11 +17,15 @@
{{unboundAgeWithTooltip created_at}}
{{{formattedDetails}}} - {{#if showFullDetails}} - {{i18n less}}
- {{details}} + {{#if useModalForDetails}} + {{i18n admin.logs.staff_actions.show}} {{else}} - {{i18n more}} + {{#if showFullDetails}} + {{i18n less}}
+ {{details}} + {{else}} + {{i18n more}} + {{/if}} {{/if}}
{{context}}
diff --git a/app/assets/javascripts/admin/views/modals/change_site_customization_details_view.js b/app/assets/javascripts/admin/views/modals/change_site_customization_details_view.js new file mode 100644 index 00000000000..1bafebf0d34 --- /dev/null +++ b/app/assets/javascripts/admin/views/modals/change_site_customization_details_view.js @@ -0,0 +1,12 @@ +/** + A modal view for details of a staff action log record in a modal. + + @class ChangeSiteCustomizationDetailsView + @extends Discourse.ModalBodyView + @namespace Discourse + @module Discourse +**/ +Discourse.ChangeSiteCustomizationDetailsView = Discourse.ModalBodyView.extend({ + templateName: 'admin/templates/logs/site_customization_change_modal', + title: I18n.t('admin.logs.staff_actions.modal_title') +}); diff --git a/app/assets/stylesheets/admin/admin_base.scss b/app/assets/stylesheets/admin/admin_base.scss index 6ff9b8acef4..384a4ee7472 100644 --- a/app/assets/stylesheets/admin/admin_base.scss +++ b/app/assets/stylesheets/admin/admin_base.scss @@ -806,3 +806,9 @@ table { border-top: solid 1px #ddd; } } + +.log-details-modal { + .modal-tab { + width: 95%; + } +} \ No newline at end of file diff --git a/app/assets/stylesheets/application/modal.css.scss b/app/assets/stylesheets/application/modal.css.scss index 7292b81d9ea..2e82fa84961 100644 --- a/app/assets/stylesheets/application/modal.css.scss +++ b/app/assets/stylesheets/application/modal.css.scss @@ -253,6 +253,13 @@ } } +.tabbed-modal { + .modal-body { + position: relative; + height: 350px; + } +} + .modal-tab { position: absolute; } diff --git a/app/controllers/admin/site_customizations_controller.rb b/app/controllers/admin/site_customizations_controller.rb index 046926a0b40..d55caa076f2 100644 --- a/app/controllers/admin/site_customizations_controller.rb +++ b/app/controllers/admin/site_customizations_controller.rb @@ -14,6 +14,7 @@ class Admin::SiteCustomizationsController < Admin::AdminController respond_to do |format| if @site_customization.save + log_site_customization_change(nil, site_customization_params) format.json { render json: @site_customization, status: :created} else format.json { render json: @site_customization.errors, status: :unprocessable_entity } @@ -23,11 +24,13 @@ class Admin::SiteCustomizationsController < Admin::AdminController def update @site_customization = SiteCustomization.find(params[:id]) + log_record = log_site_customization_change(@site_customization, site_customization_params) respond_to do |format| if @site_customization.update_attributes(site_customization_params) format.json { head :no_content } else + log_record.destroy if log_record format.json { render json: @site_customization.errors, status: :unprocessable_entity } end end @@ -37,6 +40,8 @@ class Admin::SiteCustomizationsController < Admin::AdminController @site_customization = SiteCustomization.find(params[:id]) @site_customization.destroy + # TODO: log this + respond_to do |format| format.json { head :no_content } end @@ -48,4 +53,8 @@ class Admin::SiteCustomizationsController < Admin::AdminController params.require(:site_customization).permit(:name, :stylesheet, :header, :position, :enabled, :key, :override_default_style, :stylesheet_baked) end + def log_site_customization_change(old_record, new_params) + StaffActionLogger.new(current_user).log_site_customization_change(old_record, new_params) + end + end diff --git a/app/models/staff_action_log.rb b/app/models/staff_action_log.rb index 1a9ff9beede..4bc3477a74e 100644 --- a/app/models/staff_action_log.rb +++ b/app/models/staff_action_log.rb @@ -9,7 +9,7 @@ class StaffActionLog < ActiveRecord::Base validates_presence_of :action def self.actions - @actions ||= Enum.new(:delete_user, :change_trust_level, :change_site_setting) + @actions ||= Enum.new(:delete_user, :change_trust_level, :change_site_setting, :change_site_customization) end def self.with_filters(filters) @@ -25,6 +25,14 @@ class StaffActionLog < ActiveRecord::Base query = query.where("subject = ?", filters[:subject]) if filters[:subject] query end + + def new_value_is_json? + action == StaffActionLog.actions[:change_site_customization] + end + + def previous_value_is_json? + new_value_is_json? + end end # == Schema Information diff --git a/app/serializers/staff_action_log_serializer.rb b/app/serializers/staff_action_log_serializer.rb index 35ec8e79a27..5451af8af65 100644 --- a/app/serializers/staff_action_log_serializer.rb +++ b/app/serializers/staff_action_log_serializer.rb @@ -15,4 +15,20 @@ class StaffActionLogSerializer < ApplicationSerializer def action_name StaffActionLog.actions.key(object.action).to_s end + + def new_value + if object.new_value + object.new_value_is_json? ? ::JSON.parse(object.new_value) : object.new_value + else + nil + end + end + + def previous_value + if object.previous_value + object.previous_value_is_json? ? ::JSON.parse(object.previous_value) : object.previous_value + else + nil + end + end end \ No newline at end of file diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index faf9f38436f..8ad76453b83 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -37,6 +37,17 @@ class StaffActionLogger })) end + def log_site_customization_change(old_record, site_customization_params, opts={}) + raise Discourse::InvalidParameters.new('site_customization_params is nil') unless site_customization_params + logged_attrs = ['stylesheet', 'header', 'position', 'enabled', 'key', 'override_default_style'] + StaffActionLog.create( params(opts).merge({ + action: StaffActionLog.actions[:change_site_customization], + subject: site_customization_params[:name], + previous_value: old_record ? old_record.attributes.slice(*logged_attrs).to_json : nil, + new_value: site_customization_params.slice(*(logged_attrs.map(&:to_sym))).to_json + })) + end + private def params(opts) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 1bbc2c8357c..d00f5318c10 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -100,6 +100,9 @@ en: read_more: 'read more' more: "More" less: "Less" + character_count: + one: "{{count}} character" + other: "{{count}} characters" in_n_seconds: one: "in 1 second" @@ -1196,10 +1199,15 @@ en: ip_address: "IP" previous_value: "Previous" new_value: "New" + diff: "Diff" + show: "Show" + modal_title: "Details" + no_previous: "There is no previous value." actions: delete_user: "delete user" change_trust_level: "change trust level" change_site_setting: "change site setting" + change_site_customization: "change site customization" screened_emails: title: "Screened Emails" description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed." diff --git a/spec/controllers/admin/site_customizations_controller_spec.rb b/spec/controllers/admin/site_customizations_controller_spec.rb index e0e58ee8099..76700ff3f14 100644 --- a/spec/controllers/admin/site_customizations_controller_spec.rb +++ b/spec/controllers/admin/site_customizations_controller_spec.rb @@ -34,6 +34,11 @@ describe Admin::SiteCustomizationsController do xhr :post, :create, site_customization: {name: 'my test name'} ::JSON.parse(response.body).should be_present end + + it 'logs the change' do + StaffActionLogger.any_instance.expects(:log_site_customization_change).once + xhr :post, :create, site_customization: {name: 'my test name'} + end end end diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb index d1af03c4a18..ca6a9ed5e06 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/spec/services/staff_action_logger_spec.rb @@ -73,4 +73,31 @@ describe StaffActionLogger do expect { logger.log_site_setting_change('title', 'Discourse', 'My Site') }.to change { StaffActionLog.count }.by(1) end end + + describe "log_site_customization_change" do + let(:valid_params) { {name: 'Cool Theme', stylesheet: "body {\n background-color: blue;\n}\n", header: "h1 {color: white;}"} } + + it "raises an error when params are invalid" do + expect { logger.log_site_customization_change(nil, nil) }.to raise_error(Discourse::InvalidParameters) + end + + it "logs new site customizations" do + log_record = logger.log_site_customization_change(nil, valid_params) + log_record.subject.should == valid_params[:name] + log_record.previous_value.should be_nil + log_record.new_value.should be_present + json = ::JSON.parse(log_record.new_value) + json['stylesheet'].should be_present + json['header'].should be_present + end + + it "logs updated site customizations" do + existing = SiteCustomization.new(name: 'Banana', stylesheet: "body {color: yellow;}", header: "h1 {color: brown;}") + log_record = logger.log_site_customization_change(existing, valid_params) + log_record.previous_value.should be_present + json = ::JSON.parse(log_record.previous_value) + json['stylesheet'].should == existing.stylesheet + json['header'].should == existing.header + end + end end