Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue25 #137

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added app/assets/images/discussion_settings.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion app/controllers/comment_thread_subscriptions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def destroy

respond_to do |format|
format.html do
if @comment_thread.commentable_type == 'Message'
if @comment_thread.commentable_type == 'Discussion'
redirect_to inbox_path
else
redirect_to(polymorphic_path([@commentable, :comments]))
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def new
@comment = Comment.new
@comment.comment_thread = @comment_thread
@comment.creator = present_user
if @comment_thread.commentable_type == 'Message'
if @comment_thread.commentable_type == 'Discussion'
@create_verb = 'Send'
@comment_name = 'Reply'
else
Expand All @@ -55,7 +55,7 @@ def create
@comment.comment_thread = @comment_thread
@comment.creator = present_user

if @comment_thread.commentable_type == 'Message'
if @comment_thread.commentable_type == 'Discussion'
@comment_notice = 'Reply sent.'
@hide_votes = true
else
Expand Down Expand Up @@ -94,7 +94,7 @@ def show
def edit
raise SecurityTransgression unless present_user.can_update?(@comment)

if @comment_thread.commentable_type == 'Message'
if @comment_thread.commentable_type == 'Discussion'
@comment_name = 'Reply'
else
@comment_name = 'Comment'
Expand All @@ -110,7 +110,7 @@ def edit
def update
raise SecurityTransgression unless present_user.can_update?(@comment)

if @comment_thread.commentable_type == 'Message'
if @comment_thread.commentable_type == 'Discussion'
@comment_notice = 'Reply updated.'
else
@comment_notice = 'Comment updated.'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,75 +1,75 @@
# Copyright 2011-2012 Rice University. Licensed under the Affero General Public
# License version 3 or later. See the COPYRIGHT file for details.

class MessagesController < ApplicationController
class DiscussionsController < ApplicationController

before_filter :include_jquery

before_filter :get_message, :only => [:add_recipient, :search_recipients, :leave]
before_filter :get_discussion, :only => [:add_recipient, :search_recipients, :leave]

# GET /messages/1
# GET /discussions/1
def show
@message = Message.find(params[:id])
@discussion = Discussion.find(params[:id])

raise SecurityTransgression unless present_user.can_read?(@message)
raise SecurityTransgression unless present_user.can_read?(@discussion)

@message.comment_thread.mark_as_read_for(present_user)
@discussion.comment_thread.mark_as_read_for(present_user)
present_user.reload

respond_to do |format|
format.html # show.html.erb
end
end

# GET /messages/new
# GET /discussions/new
def new
@message = Message.new
@discussion = Discussion.new

raise SecurityTransgression unless present_user.can_create?(@message)
raise SecurityTransgression unless present_user.can_create?(@discussion)

respond_to do |format|
if @message.save
@message.comment_thread.subscribe!(present_user)
format.html { redirect_to @message }
if @discussion.save
@discussion.comment_thread.subscribe!(present_user)
format.html { redirect_to @discussion }
else
format.html { redirect_to inbox_path }
end
end
end

# PUT /messages/1
# PUT /discussions/1
def update
@message = Message.find(params[:id])
@discussion = Discussion.find(params[:id])

raise SecurityTransgression unless present_user.can_create?(@message)
raise SecurityTransgression unless present_user.can_create?(@discussion)

@message.subject = params[:message][:subject]
@discussion.subject = params[:discussion][:subject]

@comment = Comment.new
@comment.message = params[:message][:body]
@comment.comment_thread = @message.comment_thread
@comment.message = params[:discussion][:body]
@comment.comment_thread = @discussion.comment_thread
@comment.creator = present_user

raise SecurityTransgression unless present_user.can_create?(@comment)

respond_to do |format|
if @message.save && @comment.save
@message.comment_thread.add_unread_except_for(present_user)
if @discussion.save && @comment.save
@discussion.comment_thread.add_unread_except_for(present_user)
flash[:notice] = 'Message was sent successfully.'
format.html { redirect_to @message }
format.html { redirect_to @discussion }
else
format.html { redirect_to @message }
format.html { redirect_to @discussion }
end
end
end

def leave
@message.comment_thread.unsubscribe!(present_user)
@discussion.comment_thread.unsubscribe!(present_user)
end

def new_recipient
@action_dialog_title = "Add a recipient"
@action_search_path = message_search_recipients_path(params[:message_id])
@action_search_path = discussion_search_recipients_path(params[:discussion_id])

respond_to do |format|
format.js { render :template => 'users/action_new' }
Expand All @@ -82,48 +82,48 @@ def search_recipients
@users = User.search(@selected_type, @text_query)

@users.reject! do |user|
@message.has_recipient?(user)
@discussion.has_recipient?(user)
end

@action_partial = 'messages/create_recipient_form'
@action_partial = 'discussions/create_recipient_form'

respond_to do |format|
format.js { render :template => 'users/action_search' }
end
end

# POST /messages/1/add_recipient
# POST /discussions/1/add_recipient
def add_recipient
raise SecurityTransgression unless present_user.can_update?(@message)
raise SecurityTransgression unless present_user.can_update?(@discussion)

@recipient = User.find_by_username(params[:username])

if @recipient.nil?
flash[:alert] = 'User ' + params[:username] + ' not found!'
respond_to do |format|
format.html { redirect_to @message }
format.html { redirect_to @discussion }
format.js { render :template => 'shared/display_flash' }
end
return
end

respond_to do |format|
if @message.comment_thread.subscribe!(@recipient)
@message.comment_thread.mark_as_unread_for(@recipient)
format.html { redirect_to @message }
if @discussion.comment_thread.subscribe!(@recipient)
@discussion.comment_thread.mark_as_unread_for(@recipient)
format.html { redirect_to @discussion }
format.js
else
flash[:alert] = @message.comment_thread.errors.values.to_sentence
format.html { redirect_to @message }
flash[:alert] = @discussion.comment_thread.errors.values.to_sentence
format.html { redirect_to @discussion }
format.js { render :template => 'shared/display_flash' }
end
end
end

protected

def get_message
@message = Message.find(params[:message_id])
def get_discussion
@discussion = Discussion.find(params[:discussion_id])
end

end
4 changes: 2 additions & 2 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ def commentable_name(comment_thread)
question_id_text(comment_thread.commentable.question)
when 'Project'
comment_thread.commentable.name
when 'Message'
"message: " + comment_thread.commentable.subject
when 'Discussion'
"discussion: " + comment_thread.commentable.subject
end
end

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
# Copyright 2011-2012 Rice University. Licensed under the Affero General Public
# License version 3 or later. See the COPYRIGHT file for details.

require 'test_helper'

class MessageTest < ActiveSupport::TestCase
# Replace this with your real tests.
test "the truth" do
assert true
end
module DiscussionsHelper
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I swear git is crazy sometimes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh
nvm.....

2 changes: 1 addition & 1 deletion app/mailers/subscription_notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def setup_variables(comment)
@commentable = @comment_thread.commentable.becomes(
Kernel.const_get(@comment_thread.commentable_type))
@active_subscribers = User.subscribers_for(@comment_thread).active_users
@is_message = @comment_thread.commentable_type == 'Message'
@is_message = @comment_thread.commentable_type == 'Discussion'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well just a minor thing you might wanna rename this to @is_discussion (and same on the method above)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooops, I seemed to have missed that

end

end
6 changes: 3 additions & 3 deletions app/models/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@ def can_be_created_by?(user)
end

def can_be_updated_by?(user)
!user.is_anonymous? && user == creator && (comment_thread.commentable_type != 'Message' || comment_thread.comments.last == self)
!user.is_anonymous? && user == creator && (comment_thread.commentable_type != 'Discussion' || comment_thread.comments.last == self)
end

def can_be_destroyed_by?(user)
!user.is_anonymous? && (user == creator || user.is_administrator?) &&
(comment_thread.commentable_type != 'Message' || comment_thread.comments.last == self)
(comment_thread.commentable_type != 'Discussion' || comment_thread.comments.last == self)
end

def can_be_voted_on_by?(user)
can_be_read_by?(user) && user != creator && comment_thread.commentable_type != 'Message'
can_be_read_by?(user) && user != creator && comment_thread.commentable_type != 'Discussion'
end

end
23 changes: 11 additions & 12 deletions app/models/comment_thread_subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,34 +10,33 @@ class CommentThreadSubscription < ActiveRecord::Base

validates_uniqueness_of :user_id, :scope => :comment_thread_id

scope :message_subscriptions, joins{comment_thread}.where{comment_thread.commentable_type == "Message"}
scope :discussion_subscriptions, joins{comment_thread}.where{comment_thread.commentable_type == "Discussion"}

def self.message_subscriptions_for(user)
where{user_id == user.id}.message_subscriptions
def self.discussion_subscriptions_for(user)
where{user_id == user.id}.discussion_subscriptions
end

def mark_all_as_read!
update_attribute(:unread_count, 0)
update_message_cache
update_discussion_cache
end

def mark_all_as_unread!
update_attribute(:unread_count, comment_thread.comments.count)
update_message_cache
update_discussion_cache
end

def add_unread!
update_attribute(:unread_count, unread_count + 1)
update_message_cache
update_discussion_cache
end

protected

def update_message_cache
return unless comment_thread.commentable_type == 'Message'
user.update_attribute(:unread_message_count,
Array.new(CommentThreadSubscription.message_subscriptions_for(user)).sum { |ms|
ms.unread_count })
def update_discussion_cache
return unless comment_thread.commentable_type == 'Discussion'
user.update_attribute(:unread_discussion_count,
Array.new(CommentThreadSubscription.discussion_subscriptions_for(user)).sum { |ms|
ms.unread_count })
end

end
8 changes: 4 additions & 4 deletions app/models/message.rb → app/models/discussion.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright 2011-2012 Rice University. Licensed under the Affero General Public
# License version 3 or later. See the COPYRIGHT file for details.

class Message < ActiveRecord::Base
class Discussion < ActiveRecord::Base

has_one :comment_thread, :as => :commentable, :dependent => :destroy
before_validation :build_comment_thread, :on => :create
Expand All @@ -12,8 +12,8 @@ class Message < ActiveRecord::Base

attr_accessible #none

def self.messages_for(user)
CommentThreadSubscription.message_subscriptions_for(user).collect { |cts| cts.comment_thread.commentable }
def self.discussions_for(user)
CommentThreadSubscription.discussion_subscriptions_for(user).collect { |cts| cts.comment_thread.commentable }
end

def subject
Expand Down Expand Up @@ -59,7 +59,7 @@ def can_be_joined_by?(user)

def subject_not_changed
return if !subject_changed? || comment_thread.comments.blank?
errors.add(:base, "You can't change a message's subject after it is sent.")
errors.add(:base, "You can't change a discussion's subject after it is sent.")
false
end

Expand Down
4 changes: 2 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ def can_join?(container_type, container_id)
return Question.find(container_id).can_be_joined_by?(self)
when 'project'
return Project.find(container_id).can_be_joined_by?(self)
when 'message'
return Message.find(container_id).can_be_joined_by?(self)
when 'discussion'
return Discussion.find(container_id).can_be_joined_by?(self)
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
subscription ||= false
%>
<% sub_string = subscription ? "Unsubscribe" : "Subscribe" %>
<% show_confirm = (subscription && commentable.comment_thread.commentable_type == 'Message') %>
<% show_confirm = (subscription && commentable.comment_thread.commentable_type == 'Discussion') %>
<%= link_to sub_string,
polymorphic_path([sub_string.downcase, commentable, :comments]),
:confirm => (show_confirm ? "Are you sure?\n" +
"Once you unsubscribe from this message, " +
"Once you unsubscribe from this discussion, " +
"you will not be able to see it anymore." : nil),
:remote => true %>
2 changes: 1 addition & 1 deletion app/views/comment_thread_subscriptions/destroy.js.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<%# Copyright 2011-2012 Rice University. Licensed under the Affero General Public
License version 3 or later. See the COPYRIGHT file for details. %>

<% if @comment_thread.commentable_type == 'Message' %>
<% if @comment_thread.commentable_type == 'Discussion' %>
window.location.replace("<%= escape_javascript(inbox_path) %>")
<% else %>
<%= display_flash %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/comments/create.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

<%= display_flash(false) %>

<% if @comment_thread.commentable_type == 'Message' %>
<% if @comment_thread.commentable_type == 'Discussion' %>
$(".edit_link").hide();
$(".delete_link").hide();
<% end %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

<%# Callers must supply a 'user' local variable %>

<%= form_tag(message_add_recipient_path(@message),
<%= form_tag(discussion_add_recipient_path(@discussion),
:method => :post,
:remote => true) do %>
<%= hidden_field_tag :username, user.username %>
<%= submit_tag "Add", :class => submit_classes, :onclick => please_wait_js %>
<% end %>
<% end %>
Loading