Revert "Revert "FEATURE: Can create warnings for users via PM""

This reverts commit 1c7559380c.
This commit is contained in:
Robin Ward 2014-09-08 11:11:56 -04:00
parent 1c7559380c
commit 334e21a03a
37 changed files with 243 additions and 18 deletions

View file

@ -354,6 +354,10 @@
<div class='field'>{{i18n admin.user.posts_read_count}}</div>
<div class='value'>{{posts_read_count}}</div>
</div>
<div class='display-row'>
<div class='field'>{{i18n admin.user.warnings_received_count}}</div>
<div class='value'>{{warnings_received_count}}</div>
</div>
<div class='display-row'>
<div class='field'>{{i18n admin.user.flags_given_received_count}}</div>
<div class='value'>{{flags_given_count}} / {{flags_received_count}}</div>

View file

@ -1,6 +1,6 @@
export default Ember.Component.extend({
tagName: 'li',
classNameBindings: ['notification.read'],
classNameBindings: ['notification.read', 'notification.is_warning'],
_markRead: function(){
var self = this;

View file

@ -9,8 +9,8 @@
export default Ember.Component.extend({
classNames: ['topic-statuses'],
hasDisplayableStatus: Em.computed.or('topic.archived','topic.closed', 'topic.pinned', 'topic.unpinned', 'topic.invisible', 'topic.archetypeObject.notDefault'),
shouldRerender: Discourse.View.renderIfChanged('topic.archived','topic.closed', 'topic.pinned', 'topic.visible', 'topic.unpinned'),
hasDisplayableStatus: Em.computed.or('topic.archived','topic.closed', 'topic.pinned', 'topic.unpinned', 'topic.invisible', 'topic.archetypeObject.notDefault', 'topic.is_warning'),
shouldRerender: Discourse.View.renderIfChanged('topic.archived', 'topic.closed', 'topic.pinned', 'topic.visible', 'topic.unpinned', 'topic.is_warning'),
didInsertElement: function(){
var self = this;
@ -52,6 +52,7 @@ export default Ember.Component.extend({
// Allow a plugin to add a custom icon to a topic
this.trigger('addCustomIcon', buffer);
renderIconIf('topic.is_warning', 'envelope', 'warning');
renderIconIf('topic.closed', 'lock', 'locked');
renderIconIf('topic.archived', 'lock', 'archived');
renderIconIf('topic.pinned', 'thumb-tack', 'pinned', self.get("canAct") );

View file

@ -15,6 +15,16 @@ export default DiscourseController.extend({
this.set('similarTopics', []);
}.on('init'),
showWarning: function() {
var usernames = this.get('model.targetUsernames');
// We need exactly one user to issue a warning
if (Em.empty(usernames) || usernames.split(',').length !== 1) {
return false;
}
return this.get('model.creatingPrivateMessage');
}.property('model.creatingPrivateMessage', 'model.targetUsernames'),
actions: {
// Toggle the reply view
toggle: function() {
@ -103,6 +113,12 @@ export default DiscourseController.extend({
var composer = this.get('model'),
self = this;
// Clear the warning state if we're not showing the checkbox anymore
if (!this.get('showWarning')) {
this.set('model.isWarning', false);
}
if(composer.get('cantSubmitPost')) {
var now = Date.now();
this.setProperties({
@ -345,6 +361,7 @@ export default DiscourseController.extend({
this.set('model', composerModel);
composerModel.set('composeState', Discourse.Composer.OPEN);
composerModel.set('isWarning', false);
var composerMessages = this.get('controllers.composer-messages');
composerMessages.queryFor(composerModel);

View file

@ -10,6 +10,10 @@ export default DiscourseController.extend({
loginRequired: Em.computed.alias('controllers.application.loginRequired'),
canSignUp: Em.computed.alias('controllers.application.canSignUp'),
showPrivateMessageGlyph: function() {
return !this.get('topic.is_warning') && this.get('topic.isPrivateMessage');
}.property('topic.is_warning', 'topic.isPrivateMessage'),
showSignUpButton: function() {
return this.get('canSignUp') && !this.get('showExtraInfo');
}.property('canSignUp', 'showExtraInfo'),

View file

@ -149,6 +149,7 @@ Discourse.Post = Discourse.Model.extend({
var data = {
raw: this.get('raw'),
topic_id: this.get('topic_id'),
is_warning: this.get('is_warning'),
reply_to_post_number: this.get('reply_to_post_number'),
category: this.get('category'),
archetype: this.get('archetype'),

View file

@ -494,6 +494,7 @@ Discourse.Composer = Discourse.Model.extend({
title: this.get('title'),
category: this.get('categoryId'),
topic_id: this.get('topic.id'),
is_warning: this.get('isWarning'),
imageSizes: opts.imageSizes,
cooked: this.getCookedHtml(),
reply_count: 0,

View file

@ -35,6 +35,14 @@
placeholderKey="composer.users_placeholder"
tabindex="1"
usernames=model.targetUsernames}}
{{#if showWarning}}
<div class='add-warning'>
<label>
{{input type="checkbox" checked=model.isWarning}}
{{i18n "composer.add_warning"}}
</label>
</div>
{{/if}}
{{/if}}
<div class="title-input">

View file

@ -10,7 +10,7 @@
<a {{bind-attr class=":star topic.starred:starred"}} {{action toggleStar}} href='#' {{bind-attr title="topic.starTooltip"}}></a>
{{/if}}
<h1>
{{#if topic.isPrivateMessage}}
{{#if showPrivateMessageGlyph}}
<span class="private-message-glyph">{{icon envelope}}</span>
{{/if}}
{{#if topic.category.parentCategory}}

View file

@ -27,7 +27,9 @@
<button class='btn btn-small no-text' {{action cancelEditingTopic}}><i class='fa fa-times'></i></button>
{{else}}
<h1>
{{#unless is_warning}}
<span class="private-message-glyph"><i class='fa fa-envelope'></i></span>
{{/unless}}
{{#unless isPrivateMessage}}
{{#if category.parentCategory}}
{{bound-category-link category.parentCategory}}

View file

@ -80,6 +80,9 @@
{{#if number_of_suspensions}}
<div><span class="pill suspensions">{{number_of_suspensions}}</span>&nbsp;{{i18n user.staff_counters.suspensions}}</div>
{{/if}}
{{#if number_of_warnings}}
<div><span class="pill warnings-received">{{number_of_warnings}}</span>&nbsp;{{i18n user.staff_counters.warnings_received}}</div>
{{/if}}
</div>
{{bound-avatar model "huge"}}

View file

@ -6,6 +6,7 @@ export default Discourse.View.extend(AddCategoryClass, Discourse.Scrolling, {
userFiltersBinding: 'controller.userFilters',
classNameBindings: ['controller.multiSelect:multi-select',
'topic.archetype',
'topic.is_warning',
'topic.category.read_restricted:read_restricted',
'topic.deleted:deleted-topic',
'topic.categoryClass'],

View file

@ -208,6 +208,14 @@
overflow: hidden;
}
}
.is-warning {
i.fa-envelope-o {
&:before {
content: "\f0e0";
}
color: $danger;
}
}
.read {
background-color: $secondary;
}

View file

@ -3,6 +3,15 @@
// hack, this needs to be done cleaner
#private-message-users {
width: 400px;
float: left;
}
.add-warning {
width: 300px;
display: inline-block;
position: relative;
top: -30px;
margin-left: 20px;
}
.composer-popup-container {

View file

@ -125,6 +125,10 @@ body {
font-size: 15px;
}
}
i.fa-envelope {
color: $danger;
}
}
/* bootstrap carryover */

View file

@ -527,6 +527,7 @@ iframe {
.topic-statuses {
i { color: $header_primary; }
i.fa-envelope { color: $danger; }
.unpinned { color: $header_primary; }
}
.topic-link { color: $header_primary;

View file

@ -395,6 +395,9 @@
.flagged-posts {
background-color: #E49735;
}
.warnings-received {
background-color: #EC441B;
}
.deleted-posts {
background-color: #EC441B;
}

View file

@ -331,12 +331,21 @@ class PostsController < ApplicationController
permitted << :embed_url
end
params.require(:raw)
params.permit(*permitted).tap do |whitelisted|
result = params.permit(*permitted).tap do |whitelisted|
whitelisted[:image_sizes] = params[:image_sizes]
# TODO this does not feel right, we should name what meta_data is allowed
whitelisted[:meta_data] = params[:meta_data]
end
# Staff are allowed to pass `is_warning`
if current_user.staff?
params.permit(:is_warning)
result[:is_warning] = (params[:is_warning] == "true")
end
result
end
def too_late_to(action, post)

View file

@ -98,7 +98,7 @@ class Notification < ActiveRecord::Base
def self.recent_report(user, count = nil)
count ||= 10
notifications = user.notifications.recent(count).includes(:topic).to_a
notifications = user.notifications.recent(count).includes({:topic => :warning}).to_a
if notifications.present?
notifications += user.notifications

View file

@ -100,6 +100,7 @@ class Topic < ActiveRecord::Base
has_many :invites, through: :topic_invites, source: :invite
has_many :revisions, foreign_key: :topic_id, class_name: 'TopicRevision'
has_one :warning
# When we want to temporarily attach some data to a forum topic (usually before serialization)
attr_accessor :user_data

View file

@ -35,6 +35,7 @@ class User < ActiveRecord::Base
has_many :invites, dependent: :destroy
has_many :topic_links, dependent: :destroy
has_many :uploads
has_many :warnings
has_one :user_avatar, dependent: :destroy
has_one :facebook_user_info, dependent: :destroy
@ -393,6 +394,10 @@ class User < ActiveRecord::Base
PostAction.where(user_id: id, post_action_type_id: PostActionType.flag_types.values).count
end
def warnings_received_count
warnings.count
end
def flags_received_count
posts.includes(:post_actions).where('post_actions.post_action_type_id' => PostActionType.flag_types.values).count
end

5
app/models/warning.rb Normal file
View file

@ -0,0 +1,5 @@
class Warning < ActiveRecord::Base
belongs_to :user
belongs_to :topic
belongs_to :created_by, class_name: 'User'
end

View file

@ -17,7 +17,8 @@ class AdminDetailedUserSerializer < AdminUserSerializer
:can_be_deleted,
:suspend_reason,
:primary_group_id,
:badge_count
:badge_count,
:warnings_received_count
has_one :approved_by, serializer: BasicUserSerializer, embed: :objects
has_one :api_key, serializer: ApiKeySerializer, embed: :objects

View file

@ -19,6 +19,7 @@ class ListableTopicSerializer < BasicTopicSerializer
:visible,
:closed,
:archived,
:is_warning,
:notification_level
has_one :last_poster, serializer: BasicUserSerializer, embed: :objects
@ -37,6 +38,14 @@ class ListableTopicSerializer < BasicTopicSerializer
false
end
def is_warning
object.private_message? && object.warning.present?
end
def include_is_warning?
is_warning
end
def unseen
!seen
end

View file

@ -6,12 +6,21 @@ class NotificationSerializer < ApplicationSerializer
:post_number,
:topic_id,
:slug,
:data
:data,
:is_warning
def slug
Slug.for(object.topic.title) if object.topic.present?
end
def is_warning
object.topic.present? && object.topic.warning.present?
end
def include_is_warning?
is_warning
end
def data
object.data_hash
end

View file

@ -41,7 +41,9 @@ class TopicViewSerializer < ApplicationSerializer
:deleted_by,
:has_deleted,
:actions_summary,
:expandable_first_post
:expandable_first_post,
:is_warning
# Define a delegator for each attribute of the topic we want
attributes(*topic_attributes)
@ -109,6 +111,14 @@ class TopicViewSerializer < ApplicationSerializer
result
end
def is_warning
object.topic.private_message? && object.topic.warning.present?
end
def include_is_warning?
is_warning
end
def draft
object.draft
end

View file

@ -55,7 +55,8 @@ class UserSerializer < BasicUserSerializer
staff_attributes :number_of_deleted_posts,
:number_of_flagged_posts,
:number_of_flags_given,
:number_of_suspensions
:number_of_suspensions,
:number_of_warnings
private_attributes :email,
@ -193,6 +194,10 @@ class UserSerializer < BasicUserSerializer
.count
end
def number_of_warnings
object.warnings.count
end
def number_of_suspensions
UserHistory.for(object, :suspend_user).count
end

View file

@ -336,6 +336,7 @@ en:
flagged_posts: "flagged posts"
deleted_posts: "deleted posts"
suspensions: "suspensions"
warnings_received: "warnings received"
messages:
all: "All"
@ -624,6 +625,7 @@ en:
message: "Authenticating with GitHub (make sure pop up blockers are not enabled)"
composer:
add_warning: "This is an official warning."
posting_not_on_topic: "Which topic do you want to reply to?"
saving_draft_tip: "saving"
saved_draft_tip: "saved"
@ -1318,6 +1320,8 @@ en:
other: "%{count} clicks"
topic_statuses:
warning:
help: "This is an official warning."
locked:
help: "This topic is closed; it no longer accepts new replies"
archived:
@ -1898,6 +1902,7 @@ en:
topics_entered: Topics Viewed
flags_given_count: Flags Given
flags_received_count: Flags Received
warnings_received_count: Warnings Received
flags_given_received_count: 'Flags Given / Received'
approve: 'Approve'
approved_by: "approved by"

View file

@ -211,6 +211,9 @@ en:
models:
topic:
attributes:
base:
warning_requires_pm: "You can only attach warnings to private messages."
too_many_users: "You can only send warnings to one user at a time."
archetype:
cant_send_pm: "Sorry, you cannot send a private message to that user."
user:

View file

@ -0,0 +1,12 @@
class CreateWarnings < ActiveRecord::Migration
def change
create_table :warnings do |t|
t.references :topic, null: false
t.references :user, null: false
t.integer :created_by_id, null: false
t.timestamps
end
add_index :warnings, :user_id
add_index :warnings, :topic_id, unique: true
end
end

View file

@ -28,6 +28,7 @@ class PostCreator
# When creating a topic:
# title - New topic title
# archetype - Topic archetype
# is_warning - Is the topic a warning?
# category - Category to assign to topic
# target_usernames - comma delimited list of usernames for membership (private message)
# target_group_names - comma delimited list of groups for membership (private message)

View file

@ -10,6 +10,7 @@ class TopicCreator
@user = user
@guardian = guardian
@opts = opts
@added_users = []
end
def create
@ -18,6 +19,7 @@ class TopicCreator
setup_auto_close_time
process_private_message
save_topic
create_warning
watch_topic
@topic
@ -25,6 +27,27 @@ class TopicCreator
private
def create_warning
return unless @opts[:is_warning]
# We can only attach warnings to PMs
unless @topic.private_message?
@topic.errors.add(:base, :warning_requires_pm)
@errors = @topic.errors
raise ActiveRecord::Rollback.new
end
# Don't create it if there is more than one user
if @added_users.size != 1
@topic.errors.add(:base, :too_many_users)
@errors = @topic.errors
raise ActiveRecord::Rollback.new
end
# Create a warning record
Warning.create(topic: @topic, user: @added_users.first, created_by: @user)
end
def watch_topic
unless @opts[:auto_track] == false
@topic.notifier.watch_topic!(@topic.user_id)
@ -109,6 +132,7 @@ class TopicCreator
return unless usernames
User.where(username: usernames.split(',')).each do |user|
check_can_send_permission!(topic, user)
@added_users << user
topic.topic_allowed_users.build(user_id: user.id)
end
end

View file

@ -165,7 +165,7 @@ class TopicQuery
options.reverse_merge!(per_page: SiteSetting.topics_per_page)
# Start with a list of all topics
result = Topic.includes(:allowed_users)
result = Topic.includes(:allowed_users).includes(:warning)
.where("topics.id IN (SELECT topic_id FROM topic_allowed_users WHERE user_id = #{user.id.to_i})")
.joins("LEFT OUTER JOIN topic_users AS tu ON (topics.id = tu.topic_id AND tu.user_id = #{user.id.to_i})")
.order(TopicQuerySQL.order_nocategory_basic_bumped)

View file

@ -350,6 +350,9 @@ describe PostCreator do
end
it 'acts correctly' do
# It's not a warning
post.topic.warning.should be_blank
post.topic.archetype.should == Archetype.private_message
post.topic.topic_allowed_users.count.should == 3
@ -370,6 +373,43 @@ describe PostCreator do
end
end
context "warnings" do
let(:target_user1) { Fabricate(:coding_horror) }
let(:target_user2) { Fabricate(:moderator) }
let(:base_args) do
{ title: 'you need a warning buddy!',
raw: "you did something bad and I'm telling you about it!",
is_warning: true,
target_usernames: target_user1.username,
category: 1 }
end
it "works as expected" do
# Invalid archetype
creator = PostCreator.new(user, base_args)
creator.create
creator.errors.should be_present
# Too many users
creator = PostCreator.new(user, base_args.merge(archetype: Archetype.private_message,
target_usernames: [target_user1.username, target_user2.username].join(',')))
creator.create
creator.errors.should be_present
# Success
creator = PostCreator.new(user, base_args.merge(archetype: Archetype.private_message))
post = creator.create
creator.errors.should be_blank
topic = post.topic
topic.should be_present
topic.warning.should be_present
topic.warning.user.should == target_user1
topic.warning.created_by.should == user
target_user1.warnings.count.should == 1
end
end
context 'private message to group' do
let(:target_user1) { Fabricate(:coding_horror) }
let(:target_user2) { Fabricate(:moderator) }

View file

@ -384,6 +384,7 @@ describe PostsController do
describe 'when logged in' do
let!(:user) { log_in }
let(:moderator) { log_in(:moderator) }
let(:new_post) { Fabricate.build(:post, user: user) }
it "raises an exception without a raw parameter" do
@ -492,6 +493,24 @@ describe PostsController do
xhr :post, :create, {raw: 'hello', meta_data: {xyz: 'abc'}}
end
context "is_warning" do
it "doesn't pass `is_warning` through if you're not staff" do
PostCreator.expects(:new).with(user, Not(has_entries('is_warning' => true))).returns(post_creator)
xhr :post, :create, {raw: 'hello', archetype: 'private_message', is_warning: 'true'}
end
it "passes `is_warning` through if you're staff" do
PostCreator.expects(:new).with(moderator, has_entries('is_warning' => true)).returns(post_creator)
xhr :post, :create, {raw: 'hello', archetype: 'private_message', is_warning: 'true'}
end
it "passes `is_warning` as false through if you're staff" do
PostCreator.expects(:new).with(moderator, has_entries('is_warning' => false)).returns(post_creator)
xhr :post, :create, {raw: 'hello', archetype: 'private_message', is_warning: 'false'}
end
end
end
end