From 22c9714919bee841f152c12613900c76ce78944f Mon Sep 17 00:00:00 2001 From: Andy Zhao Date: Tue, 4 Sep 2018 17:08:41 -0400 Subject: [PATCH] V2 of Reports System (#441) * Fix weird breakpoint top-nav issue * Fix error handling when submitting blank note * Change status closed to invalid * Add ability to sort between status * WIP * Add working views for new report page * Add mailer WIP * Add MVP of new reports system * Move collapse to the right & add show page link * Fix buttons for index page * Change collapse to collapse/expand * Remove email functionality from new report * Update copy of report abuse responses * WIP of showing emails in reports * Fix notes for user_role_service.rb * Remove unused scripts from internal.html.erb * Update tests for reports dashboard * Add missing migration file * Add finishing touches to reports dashboard * Remove deprecated methods * Update view with better timestamps * Update and write new tests * Add .codeclimate.yml * Remove commented out editor thing * Undo commented out code for recaptcha method * Undo comment out code for development * Use new comment path instead of old comment path --- .codeclimate.yml | 0 app/controllers/admin/users_controller.rb | 2 +- app/controllers/comments_controller.rb | 2 +- .../feedback_messages_controller.rb | 5 - .../internal/feedback_messages_controller.rb | 56 ++- app/helpers/feedback_messages_helper.rb | 52 +++ app/mailers/notify_mailer.rb | 8 + app/models/email_message.rb | 5 + app/models/feedback_message.rb | 25 +- app/models/note.rb | 3 +- app/models/notification.rb | 2 +- app/models/user.rb | 1 + app/services/user_role_service.rb | 10 +- app/views/feedback_messages/index.html.erb | 3 + app/views/feedback_messages/show.html.erb | 30 -- .../_feedback_message.html.erb | 393 +++++++++++++++--- .../internal/feedback_messages/index.html.erb | 24 +- .../internal/feedback_messages/show.html.erb | 10 + app/views/layouts/internal.html.erb | 10 +- ...feedback_message_resolution_email.html.erb | 3 + ...feedback_message_resolution_email.text.erb | 1 + .../notify_mailer/new_report_email.html.erb | 6 - .../notify_mailer/new_report_email.text.erb | 4 - config/routes.rb | 8 +- ...709_update_feedback_message_note_system.rb | 6 + ...204032_add_feedback_message_id_to_email.rb | 5 + ...1849_remove_slug_from_feedback_messages.rb | 5 + db/schema.rb | 6 +- spec/mailers/notify_mailer_spec.rb | 22 +- .../mailers/previews/notify_mailer_preview.rb | 21 + spec/models/feedback_message_spec.rb | 4 - .../internal_feedback_messages_spec.rb | 97 +++++ .../internal_feedback_messages_update_spec.rb | 42 -- spec/services/user_role_service_spec.rb | 40 +- 34 files changed, 692 insertions(+), 219 deletions(-) create mode 100644 .codeclimate.yml create mode 100644 app/helpers/feedback_messages_helper.rb delete mode 100644 app/views/feedback_messages/show.html.erb create mode 100644 app/views/internal/feedback_messages/show.html.erb create mode 100644 app/views/mailers/notify_mailer/feedback_message_resolution_email.html.erb create mode 100644 app/views/mailers/notify_mailer/feedback_message_resolution_email.text.erb delete mode 100644 app/views/mailers/notify_mailer/new_report_email.html.erb delete mode 100644 app/views/mailers/notify_mailer/new_report_email.text.erb create mode 100644 db/migrate/20180713180709_update_feedback_message_note_system.rb create mode 100644 db/migrate/20180821204032_add_feedback_message_id_to_email.rb create mode 100644 db/migrate/20180824191849_remove_slug_from_feedback_messages.rb create mode 100644 spec/requests/internal_feedback_messages_spec.rb delete mode 100644 spec/requests/internal_feedback_messages_update_spec.rb diff --git a/.codeclimate.yml b/.codeclimate.yml new file mode 100644 index 000000000..e69de29bb diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index c57a88f43..4a8240e81 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -2,7 +2,7 @@ module Admin class UsersController < Admin::ApplicationController def update user = User.find(params[:id]) - UserRoleService.new(user).check_for_roles(params[:user]) + UserRoleService.new(user, current_user.id).check_for_roles(params[:user]) if user.errors.messages.blank? && user.update(user_params) flash[:notice] = "User successfully updated" redirect_to "/admin/users/#{params[:id]}" diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index d1fa99c8b..904e103bb 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -102,7 +102,7 @@ class CommentsController < ApplicationController authorize @comment if @comment.update(permitted_attributes(@comment).merge(edited_at: Time.zone.now)) Mention.create_all(@comment) - redirect_to "#{@comment.commentable.path}/comments/#{@comment.id_code_generated}", notice: "Comment was successfully updated." + redirect_to @comment.path, notice: "Comment was successfully updated." else @commentable = @comment.commentable render :edit diff --git a/app/controllers/feedback_messages_controller.rb b/app/controllers/feedback_messages_controller.rb index fcc7dfaf3..426dcea89 100644 --- a/app/controllers/feedback_messages_controller.rb +++ b/app/controllers/feedback_messages_controller.rb @@ -9,7 +9,6 @@ class FeedbackMessagesController < ApplicationController ) if recaptcha_verified? && @feedback_message.save send_slack_message - # NotifyMailer.new_report_email(@feedback_message).deliver if @feedback_message.reporter_id? redirect_to "/feedback_messages" elsif feedback_message_params[:feedback_type] == "bug-reports" flash[:notice] = "Make sure the forms are filled 🤖 " @@ -21,10 +20,6 @@ class FeedbackMessagesController < ApplicationController end end - def show - @feedback_message = FeedbackMessage.find_by(slug: params[:slug]) - end - private def recaptcha_verified? diff --git a/app/controllers/internal/feedback_messages_controller.rb b/app/controllers/internal/feedback_messages_controller.rb index 693b1ac5b..bad0dcc30 100644 --- a/app/controllers/internal/feedback_messages_controller.rb +++ b/app/controllers/internal/feedback_messages_controller.rb @@ -3,26 +3,50 @@ class Internal::FeedbackMessagesController < Internal::ApplicationController def index @feedback_type = params[:state] || "abuse-reports" + @status = params[:status] || "Open" @feedback_messages = FeedbackMessage. - where(feedback_type: @feedback_type). + where(feedback_type: @feedback_type, status: @status). order("created_at DESC"). page(params[:page] || 1).per(25) end - def update - @feedback_message = FeedbackMessage.find(params[:id]) - @feedback_message.status = feedback_message_params[:status] - @feedback_message.reviewer_id = feedback_message_params[:reviewer_id] - note = @feedback_message.find_or_create_note(@feedback_message.feedback_type) - note.content = feedback_message_params[:note][:content] - if @feedback_message.save! && note.save! - @feedback_message.touch(:last_reviewed_at) - flash[:success] = "Report ##{@feedback_message.id} saved. Remember to send emails!" - redirect_to "/internal/reports?state=#{@feedback_message.feedback_type}" + def save_status + feedback_message = FeedbackMessage.find(params[:id]) + if feedback_message.update(status: params[:status]) + render json: { outcome: "Success" } else - @feedback_messages = FeedbackMessage.where(feedback_type: @feedback_type) - flash[:error] = @feedback_message.errors.full_messages - render "index.html.erb", state: @feedback_message.feedback_type + render json: { outcome: feedback_message.errors.full_messages } + end + end + + def show + @feedback_message = FeedbackMessage.find_by(id: params[:id]) + end + + def send_email + if NotifyMailer.feedback_message_resolution_email(params).deliver + render json: { outcome: "Success" } + else + render json: { outcome: "Failure" } + end + end + + def create_note + note = Note.new( + noteable_id: params["noteable_id"], + noteable_type: params["noteable_type"], + author_id: params["author_id"], + content: params["content"], + reason: params["reason"], + ) + if note.save + render json: { + outcome: "Success", + content: params["content"], + author_name: note.author.name, + } + else + render json: { outcome: note.errors.full_messages } end end @@ -30,8 +54,8 @@ class Internal::FeedbackMessagesController < Internal::ApplicationController def feedback_message_params params[:feedback_message].permit( - :status, :reviewer_id, - note: %i[content reason] + :id, :status, :reviewer_id, + note: %i[content reason noteable_id noteable_type author_id] ) end end diff --git a/app/helpers/feedback_messages_helper.rb b/app/helpers/feedback_messages_helper.rb new file mode 100644 index 000000000..cf5d25afe --- /dev/null +++ b/app/helpers/feedback_messages_helper.rb @@ -0,0 +1,52 @@ +module FeedbackMessagesHelper + def offender_email_details + body = <<~HEREDOC + Hi [*USERNAME*], + + All dev.to members are expected to help foster a welcoming environment for the community and abide by our terms and conditions of use. It's been brought to our attention that you may have violated our code of conduct. If this behavior continues, we will need to ban your posting privileges on dev.to. + + If you think there's been a mistake, please reply to this email and we'll sort it out. + + Thanks, + dev.to team + HEREDOC + { + subject: "dev.to Status Update", + body: body, + }.freeze + end + + def reporter_email_details + body = <<~HEREDOC + Hi [*USERNAME*], + + We wanted to say thank you for flagging a [*comment/post*] that was in violation of the dev.to code of conduct and terms of service. Your action has helped us continue our work of fostering an open and welcoming community. + + We've also removed the offending posts and reached out to the offender(s). + + Thanks again for being a great part of the community. + + PBJ + HEREDOC + { + subject: "dev.to Status Update", + body: body, + }.freeze + end + + def victim_email_details + body = <<~HEREDOC + Hi [*USERNAME*], + + We noticed some comments (made by others) on your [*post/comment*] that violated the dev.to code of conduct. We want you to know that we have zero tolerance for such behavior, and have removed the offending posts and reached out to the offender(s). + + Thanks for being awesome and please don't hesitate to email us with any questions. We welcome all feedback and ideas as we continue our work of fostering an open and welcoming community. + + PBJ + HEREDOC + { + subject: "Courtesy Notice from dev.to", + body: body, + }.freeze + end +end diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index e0310a785..7dccb1de9 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -49,6 +49,14 @@ class NotifyMailer < ApplicationMailer mail(to: @user.email, subject: "You just got a badge") end + def feedback_message_resolution_email(params) + @user = User.find_by(email: params[:email_to]) + @email_body = params[:email_body] + track utm_campaign: params[:email_type] + track extra: { feedback_message_id: params[:feedback_message_id] } + mail(to: params[:email_to], subject: params[:email_subject]) + end + def new_report_email(report) @feedback_message = report @user = report.reporter diff --git a/app/models/email_message.rb b/app/models/email_message.rb index dd7f737b3..09c455632 100644 --- a/app/models/email_message.rb +++ b/app/models/email_message.rb @@ -2,4 +2,9 @@ class EmailMessage < Ahoy::Message # So far this is mostly used to be compatible with administrate gem, # which doesn't seem to play nicely with namespaces. But there could be other # reasons to define behavior here, similar to how we use the Tag model. + def body_html_content + doctype_index = content.index("") + 6 + content[doctype_index..closing_html_index] + end end diff --git a/app/models/feedback_message.rb b/app/models/feedback_message.rb index 32222314a..127f03594 100644 --- a/app/models/feedback_message.rb +++ b/app/models/feedback_message.rb @@ -3,7 +3,7 @@ class FeedbackMessage < ApplicationRecord belongs_to :reviewer, foreign_key: "reviewer_id", class_name: "User", optional: true belongs_to :reporter, foreign_key: "reporter_id", class_name: "User", optional: true belongs_to :victim, foreign_key: "victim_id", class_name: "User", optional: true - has_one :note, as: :noteable, dependent: :destroy + has_many :notes, as: :noteable, dependent: :destroy validates_presence_of :feedback_type, :message validates_presence_of :reported_url, :category, if: :abuse_report? @@ -11,31 +11,20 @@ class FeedbackMessage < ApplicationRecord inclusion: { in: ["spam", "other", "rude or vulgar", "harassment", "bug"], } - - before_validation :generate_slug + validates :status, + inclusion: { + in: ["Open", "Invalid", "Resolved"], + } def abuse_report? feedback_type == "abuse-reports" end - def generate_slug - self.slug = SecureRandom.hex(10) unless slug? - end - def capitalize_status self.status = status.capitalize unless status.blank? end - def to_eastern_strftime(time) - return if time.nil? - time.in_time_zone("America/New_York").strftime("%A, %b %d %Y - %I:%M %p %Z") - end - - def path - "/reports/#{slug}" - end - - def find_or_create_note(reason) - note || Note.new(reason: reason, noteable_id: id, noteable_type: "FeedbackMessage") + def email_messages + EmailMessage.where(feedback_message_id: id) end end diff --git a/app/models/note.rb b/app/models/note.rb index c2135b9bf..c6288e091 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -1,6 +1,5 @@ class Note < ApplicationRecord belongs_to :noteable, polymorphic: true + belongs_to :author, class_name: "User" validates :reason, :content, presence: true - validates :noteable_id, uniqueness: - { scope: :reason, message: "limited to one note per noteable per reason" } end diff --git a/app/models/notification.rb b/app/models/notification.rb index ce5422242..ecfa4abf0 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -51,7 +51,7 @@ class Notification < ApplicationRecord if notifiable.class.name == "Broadcast" || action == "Moderation" User.find(ApplicationConfig["DEVTO_USER_ID"]) else - notifiable.user + notifiable&.user end end diff --git a/app/models/user.rb b/app/models/user.rb index 0aa4237d0..6d4fa65df 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -23,6 +23,7 @@ class User < ApplicationRecord has_many :mentions, dependent: :destroy has_many :messages has_many :notes, as: :noteable + has_many :authored_notes, as: :author, class_name: "Note" has_many :notifications, dependent: :destroy has_many :reactions, dependent: :destroy has_many :tweets, dependent: :destroy diff --git a/app/services/user_role_service.rb b/app/services/user_role_service.rb index 58ca9b48e..80b9f1fa0 100644 --- a/app/services/user_role_service.rb +++ b/app/services/user_role_service.rb @@ -1,6 +1,7 @@ class UserRoleService - def initialize(user) + def initialize(user, current_user_id) @user = user + @current_user_id = current_user_id end def check_for_roles(params) @@ -34,7 +35,11 @@ class UserRoleService def new_roles?(params) params[:trusted] == "1" ? @user.add_role(:trusted) : @user.remove_role(:trusted) - params[:analytics] == "1" ? @user.add_role(:analytics_beta_tester) : @user.remove_role(:analytics_beta_tester) + if params[:analytics] == "1" + @user.add_role(:analytics_beta_tester) + else + @user.remove_role(:analytics_beta_tester) + end if params[:scholar] == "1" @user.add_role(:workshop_pass) @user.update(workshop_expiration: params[:workshop_expiration]) @@ -70,6 +75,7 @@ class UserRoleService note = Note.find_by(noteable_id: @user.id, noteable_type: "User", reason: reason) if note.nil? Note.create( + author_id: @current_user_id, noteable_id: @user.id, noteable_type: "User", reason: reason, diff --git a/app/views/feedback_messages/index.html.erb b/app/views/feedback_messages/index.html.erb index 3b3296550..80d8f8bfe 100644 --- a/app/views/feedback_messages/index.html.erb +++ b/app/views/feedback_messages/index.html.erb @@ -2,5 +2,8 @@

Thank you for your report.

Return to home page

+

+ Questions? Send an email to yo@dev.to +

diff --git a/app/views/feedback_messages/show.html.erb b/app/views/feedback_messages/show.html.erb deleted file mode 100644 index ce28c3898..000000000 --- a/app/views/feedback_messages/show.html.erb +++ /dev/null @@ -1,30 +0,0 @@ - -
-

Thank you for your report. You can use this link to check for any updates on your report:

-

https://dev.to<%= @feedback_message.path %>

-

Note that the link above is public, but secret.

-
-

Current Status: <%= @feedback_message.status %>

- <% if @feedback_message.feedback_type == "abuse-reports" %> -

- Reported URL: <%= link_to @feedback_message.reported_url, @feedback_message.reported_url, target: "_blank" %> -
- (Opens in a new tab) -

-
-

Category: <%= @feedback_message.category %>

- <% end %> -

Message:

-

- <% if @feedback_message.message.blank? %> - No message was left. - <% else %> - <%= @feedback_message.message %> - <% end %> -

-
-
-

- Questions? Send an email to yo@dev.to -

-
diff --git a/app/views/internal/feedback_messages/_feedback_message.html.erb b/app/views/internal/feedback_messages/_feedback_message.html.erb index 80b94aaa3..7d6597293 100644 --- a/app/views/internal/feedback_messages/_feedback_message.html.erb +++ b/app/views/internal/feedback_messages/_feedback_message.html.erb @@ -3,58 +3,355 @@ width: 100%; resize: none; font-size: 18px; - height: 100px; + height: 50px; border-radius: 3px; + padding: 5px; + margin: 10px 0px; } .blankmessage{ font-style: italic; } +.note-content{ + border: 1px dashed black; + border-radius: 5px; + width: 100%; + padding: 5px; + padding-top: 25px; + word-wrap: break-word; +} +.email__alert{ + display: none; + margin: 0; +} +.panel-right-elements{ + float: right; +} +.email__container{ + border: 1px solid gray; + margin: 10px; + padding-left: 50px; + padding-top: 5px; +} +.to__subject{ + margin: 5px; + font-size: 20px; + color: black; + text-align: center; +} +.time__badge{ + float: right; + margin-top: 7px; + margin-right: 7px; +} +.single__note{ + width: 60%; +} +
+
+ +
+
+

+ Reporter: + <% if feedback_message.reporter_id? %> + <%= feedback_message.reporter.name %> + @<%= feedback_message.reporter.username %> + <% else %> + Anonymous + <% end %> +

+

+ Reported URL (new tab): +
+ <%= feedback_message.reported_url %> +

+

Category: <%= feedback_message.category %>

+

+ Message: +

+

+ <% if feedback_message.message.blank? %> + No message was left. + <% else %> + <%= feedback_message.message %> + <% end %> +

+

+ Status: <%= f.select :status, ['Open', 'Invalid', 'Resolved'], {}, id: "status__#{feedback_message.id}" %> + +

+
+

Previous Emails:

+
+ <% EmailMessage.where(feedback_message_id: feedback_message.id).each do |email| %> + + <% end %> +
+

Email Form:

+ -

- Report #<%= feedback_message.id %> - Submitted on: <%= feedback_message.to_eastern_strftime(feedback_message.created_at) %> -
- Reporter: - <% if feedback_message.reporter_id? %> - <%= feedback_message.reporter.name %> - @<%= feedback_message.reporter.username %> - -- - Email: <%= feedback_message.reporter.email %> - <% else %> - Anonymous - <% end %> -

-

- Reported URL (new tab): -
- <%= feedback_message.reported_url %> -

-

Category: <%= feedback_message.category %>

-

- Message: -

-

- <% if feedback_message.message.blank? %> - No message was left. - <% else %> - <%= feedback_message.message %> - <% end %> -

-
-

- Status: <%= f.select :status, ['Open', 'Closed', 'Resolved'] %> -

-

Note:

- <%= f.fields_for :note do |note| %> - <%= note.hidden_field :reason, value: feedback_message.feedback_type %> - <%= note.text_area :content, value: feedback_message.note&.content, placeholder: "Leave some notes about the status and context of this report.", class: "notefield" %> - <% end %> - <%= f.hidden_field :reviewer_id, value: current_user.id %> - <% if feedback_message.reviewer_id %> -

Last reviewed by: <%= feedback_message.reviewer.name %>

-

Last reviewed on: <%= feedback_message.to_eastern_strftime(feedback_message.last_reviewed_at) %>

- <% else %> -

Last reviewed by: No one yet

- <% end %> -
- +
+
+

+ Send to: +
+ <%= email_field_tag :reporter_email_to, feedback_message.reporter&.email, class: "form-control", id: "reporter__emailto__#{feedback_message.id}", required: true %> +

+

Subject:

+ <%= text_field_tag :reporter_email_subject, reporter_email_details[:subject], class: "form-control", id: "reporter__subject__#{feedback_message.id}" %> +

Body:

+ <%= text_area_tag (:reporter_email_body), reporter_email_details[:body], class: "form-control", style: "height: 300px;", id: "reporter__body__#{feedback_message.id}" %> +
+
+

+ Send to: +
+ <%= email_field_tag :offender_email_to, feedback_message.offender&.email, class: "form-control", id: "offender__emailto__#{feedback_message.id}", required: true %> +

+

Subject:

+ <%= text_field_tag :offender_email_subject, offender_email_details[:subject], class: "form-control", id: "offender__subject__#{feedback_message.id}" %> +

Body:

+ <%= text_area_tag (:offender_email_body), offender_email_details[:body], class: "form-control", style: "height: 300px;", id: "offender__body__#{feedback_message.id}" %> +
+
+

+ Send to: +
+ <%= email_field_tag :victim_email_to, feedback_message.victim&.email, class: "form-control", id: "victim__emailto__#{feedback_message.id}", required: true %> +

+

Subject:

+ <%= text_field_tag :victim_email_subject, victim_email_details[:subject], class: "form-control", id: "victim__subject__#{feedback_message.id}" %> +

Body:

+ <%= text_area_tag (:victim_email_body), victim_email_details[:body], class: "form-control", style: "height: 300px;", id: "victim__body__#{feedback_message.id}" %> +
+
+ +
+ + + +

Notes:

+
+ <% feedback_message.notes&.order("created_at").each do |note| %> +
+ + <%= time_ago_in_words note.created_at %> ago + +

+ <%= note.author.name %>: <%= note.content %> +

+
+ <% end %> +
+ + + + + +
+ + +
+
+
+
+ + diff --git a/app/views/internal/feedback_messages/index.html.erb b/app/views/internal/feedback_messages/index.html.erb index 981ebb5a6..a7e13e58b 100644 --- a/app/views/internal/feedback_messages/index.html.erb +++ b/app/views/internal/feedback_messages/index.html.erb @@ -2,6 +2,14 @@ .top-nav{ line-height: 1.4em; } + @media (max-width:991px) and (min-width:768px) { + .top-nav{ + margin-top: 100px; + } + .alert{ + margin-top: 100px; + } + } .top-nav a { display: inline-block; padding: 10px; @@ -12,8 +20,12 @@

- ">Abuse Reports | - ">Bug Reports + ">Abuse Reports | + ">Bug Reports +
+ ">Open/Unresolved | + ">Resolved | + ">Invalid

<%= @feedback_type.titleize %>

@@ -21,11 +33,9 @@ <%= paginate @feedback_messages %> <% @feedback_messages.each do |feedback_message| %> -
- <%= form_for [:internal, feedback_message] do |f| %> - <%= render "feedback_message", f: f, feedback_message: feedback_message %> - <% end %> -
+ <%= form_for [:internal, feedback_message] do |f| %> + <%= render "feedback_message", f: f, feedback_message: feedback_message %> + <% end %> <% end %>

diff --git a/app/views/internal/feedback_messages/show.html.erb b/app/views/internal/feedback_messages/show.html.erb new file mode 100644 index 000000000..d186cbc4e --- /dev/null +++ b/app/views/internal/feedback_messages/show.html.erb @@ -0,0 +1,10 @@ +
+
+
+
+
+
+ +<%= form_for [:internal, @feedback_message] do |f| %> + <%= render "feedback_message", f: f, feedback_message: @feedback_message %> +<% end %> \ No newline at end of file diff --git a/app/views/layouts/internal.html.erb b/app/views/layouts/internal.html.erb index 11c2e9639..f20271f37 100644 --- a/app/views/layouts/internal.html.erb +++ b/app/views/layouts/internal.html.erb @@ -8,6 +8,7 @@ + <%= csrf_meta_tags %> <%= favicon_link_tag %> @@ -112,14 +113,5 @@ - - - - - - - - diff --git a/app/views/mailers/notify_mailer/feedback_message_resolution_email.html.erb b/app/views/mailers/notify_mailer/feedback_message_resolution_email.html.erb new file mode 100644 index 000000000..7530aa9d4 --- /dev/null +++ b/app/views/mailers/notify_mailer/feedback_message_resolution_email.html.erb @@ -0,0 +1,3 @@ +

+ <%= @email_body %> +

\ No newline at end of file diff --git a/app/views/mailers/notify_mailer/feedback_message_resolution_email.text.erb b/app/views/mailers/notify_mailer/feedback_message_resolution_email.text.erb new file mode 100644 index 000000000..bf1c04f85 --- /dev/null +++ b/app/views/mailers/notify_mailer/feedback_message_resolution_email.text.erb @@ -0,0 +1 @@ +<%= @email_body %> \ No newline at end of file diff --git a/app/views/mailers/notify_mailer/new_report_email.html.erb b/app/views/mailers/notify_mailer/new_report_email.html.erb deleted file mode 100644 index 0e59288a1..000000000 --- a/app/views/mailers/notify_mailer/new_report_email.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -Thank you for your report. You can check the status of your report here: -
-https://dev.to<%= @feedback_message.path %> -
-
- If you have any questions, feel free to reply to this email, or send an email to yo@dev.to. \ No newline at end of file diff --git a/app/views/mailers/notify_mailer/new_report_email.text.erb b/app/views/mailers/notify_mailer/new_report_email.text.erb deleted file mode 100644 index 377f86665..000000000 --- a/app/views/mailers/notify_mailer/new_report_email.text.erb +++ /dev/null @@ -1,4 +0,0 @@ -Thank you for your report. You can check the status of your report here: -https://dev.to<%= @feedback_message.path %> - -If you have any questions, feel free to reply to this email, or send an email to yo@dev.to. \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index ce97a17fc..b9788b43d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -36,8 +36,12 @@ Rails.application.routes.draw do end resources :members, only: [:index] resources :events - resources :feedback_messages, only: [:update] - resources :reports, only: %i[index update], controller: "feedback_messages" + resources :feedback_messages, only: [:update, :show] + resources :reports, only: %i[index update show], controller: "feedback_messages" do + post "send_email", to: :send_email, on: :collection + post "create_note", to: :create_note, on: :collection + post "save_status", to: :save_status, on: :collection + end mount Flipflop::Engine => "/features" end diff --git a/db/migrate/20180713180709_update_feedback_message_note_system.rb b/db/migrate/20180713180709_update_feedback_message_note_system.rb new file mode 100644 index 000000000..659f7bb1b --- /dev/null +++ b/db/migrate/20180713180709_update_feedback_message_note_system.rb @@ -0,0 +1,6 @@ +class UpdateFeedbackMessageNoteSystem < ActiveRecord::Migration[5.1] + def change + remove_column :feedback_messages, :reviewer_id + add_column :notes, :author_id, :integer + end +end diff --git a/db/migrate/20180821204032_add_feedback_message_id_to_email.rb b/db/migrate/20180821204032_add_feedback_message_id_to_email.rb new file mode 100644 index 000000000..a5e73dc0b --- /dev/null +++ b/db/migrate/20180821204032_add_feedback_message_id_to_email.rb @@ -0,0 +1,5 @@ +class AddFeedbackMessageIdToEmail < ActiveRecord::Migration[5.1] + def change + add_column :ahoy_messages, :feedback_message_id, :integer + end +end diff --git a/db/migrate/20180824191849_remove_slug_from_feedback_messages.rb b/db/migrate/20180824191849_remove_slug_from_feedback_messages.rb new file mode 100644 index 000000000..6f4f9e505 --- /dev/null +++ b/db/migrate/20180824191849_remove_slug_from_feedback_messages.rb @@ -0,0 +1,5 @@ +class RemoveSlugFromFeedbackMessages < ActiveRecord::Migration[5.1] + def change + remove_column :feedback_messages, :slug + end +end diff --git a/db/schema.rb b/db/schema.rb index 626b504da..080cfc96e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. @@ -17,6 +19,7 @@ ActiveRecord::Schema.define(version: 20180826174411) do create_table "ahoy_messages", id: :serial, force: :cascade do |t| t.datetime "clicked_at" t.text "content" + t.integer "feedback_message_id" t.string "mailer" t.datetime "opened_at" t.datetime "sent_at" @@ -286,8 +289,6 @@ ActiveRecord::Schema.define(version: 20180826174411) do t.string "reported_url" t.boolean "reporter_email_sent?", default: false t.integer "reporter_id" - t.integer "reviewer_id" - t.string "slug" t.string "status", default: "Open" t.datetime "updated_at" t.boolean "victim_email_sent?", default: false @@ -387,6 +388,7 @@ ActiveRecord::Schema.define(version: 20180826174411) do end create_table "notes", id: :serial, force: :cascade do |t| + t.integer "author_id" t.text "content" t.datetime "created_at", null: false t.integer "noteable_id" diff --git a/spec/mailers/notify_mailer_spec.rb b/spec/mailers/notify_mailer_spec.rb index 4a60a211c..d8aa3f01b 100644 --- a/spec/mailers/notify_mailer_spec.rb +++ b/spec/mailers/notify_mailer_spec.rb @@ -72,17 +72,29 @@ RSpec.describe NotifyMailer, type: :mailer do end end - describe "#new_report_email" do + describe "#new_feedback_message_resolution_email" do + def params(user_email, feedback_message_id) + { + email_to: user_email, + email_subject: "dev.to Status Update", + email_body: "You've violated our code of conduct", + email_type: "Reporter", + feedback_message_id: feedback_message_id, + } + end + it "renders proper subject" do feedback_message = create(:feedback_message, :abuse_report, reporter_id: user.id) - new_report_email = described_class.new_report_email(feedback_message) - expect(new_report_email.subject).to eq("Thank you for your report") + feedback_message_resolution_email = described_class. + feedback_message_resolution_email(params(user.email, feedback_message.id)) + expect(feedback_message_resolution_email.subject).to eq("dev.to Status Update") end it "renders proper receiver" do feedback_message = create(:feedback_message, :abuse_report, reporter_id: user.id) - new_report_email = described_class.new_report_email(feedback_message) - expect(new_report_email.to).to eq([user.email]) + feedback_message_resolution_email = described_class. + feedback_message_resolution_email(params(user.email, feedback_message.id)) + expect(feedback_message_resolution_email.to).to eq([user.email]) end end end diff --git a/spec/mailers/previews/notify_mailer_preview.rb b/spec/mailers/previews/notify_mailer_preview.rb index f37d8ca9c..e6354d35e 100644 --- a/spec/mailers/previews/notify_mailer_preview.rb +++ b/spec/mailers/previews/notify_mailer_preview.rb @@ -28,6 +28,27 @@ class NotifyMailerPreview < ActionMailer::Preview NotifyMailer.new_report_email(FeedbackMessage.first) end + def feedback_message_resolution_email + # change email_body text when you need to see a different version + email_body = <<~HEREDOC + Hi [*USERNAME*], + + We wanted to say thank you for flagging a [comment/post] that was in violation of the dev.to code of conduct and terms of service. Your action has helped us continue our work of fostering an open and welcoming community. + + We've also removed the offending posts and reached out to the offender(s). + + Thanks again for being a great part of the community. + + PBJ + HEREDOC + params = { + email_to: "andy@dev.to", + email_subject: "Courtesy notice from dev.to", + email_body: email_body, + } + NotifyMailer.feedback_message_resolution_email(params) + end + def new_message_email NotifyMailer.new_message_email(Message.last) end diff --git a/spec/models/feedback_message_spec.rb b/spec/models/feedback_message_spec.rb index 819e39662..b98cbde96 100644 --- a/spec/models/feedback_message_spec.rb +++ b/spec/models/feedback_message_spec.rb @@ -42,8 +42,4 @@ RSpec.describe FeedbackMessage, type: :model do in_array(["spam", "other", "rude or vulgar", "harassment", "bug"]) end end - - it "always generates a slug" do - expect(abuse_report.slug).not_to be_nil - end end diff --git a/spec/requests/internal_feedback_messages_spec.rb b/spec/requests/internal_feedback_messages_spec.rb new file mode 100644 index 000000000..73c425d25 --- /dev/null +++ b/spec/requests/internal_feedback_messages_spec.rb @@ -0,0 +1,97 @@ +require "rails_helper" + +RSpec.describe "/internal/reports", type: :request do + let(:feedback_message) { create(:feedback_message, :abuse_report) } + let(:feedback_message2) { create(:feedback_message, :abuse_report) } + let(:feedback_message3) { create(:feedback_message, :abuse_report) } + let(:user) { create(:user) } + let(:admin) { create(:user, :super_admin) } + + describe "GET " + + describe "POST /save_status" do + context "when a valid request is made" do + before do + login_as admin + valid_save_status_params = { status: "Resolved" } + valid_save_status_params[:id] = feedback_message.id + post "/internal/reports/save_status", params: + valid_save_status_params + end + + it "returns a JSON with an outcome key and Success value" do + expect(JSON.parse(response.body)).to eq("outcome" => "Success") + end + + it "updates the status of the feedback message" do + expect(FeedbackMessage.last.status).to eq("Resolved") + end + end + end + + describe "POST /send_email" do + context "when a valid request is made" do + valid_send_email_params = { + "feedback_message_id" => 1, + "email_subject" => "Thank you for your report", + "email_body" => "Thanks for your report and being an awesome member!", + "email_type" => "reporter", + } + + email_message_attributes = { + "feedback_message_id" => 1, + "subject" => "Thank you for your report", + "utm_campaign" => "reporter", + } + + before do + feedback_message + login_as admin + valid_send_email_params["email_to"] = user.email + post "/internal/reports/send_email", params: + valid_send_email_params + end + + it "returns a JSON with an outcome key and Success value" do + expect(JSON.parse(response.body)).to eq("outcome" => "Success") + end + + it "creates a new email message with the same params" do + email_message_attributes["to"] = user.email + expect(EmailMessage.last.attributes).to include(email_message_attributes) + end + end + end + + describe "POST /internal/reports/create_note" do + context "when a valid request is made" do + note_params = { + "content" => "test note", + "reason" => "abuse-reports", + "noteable_type" => "FeedbackMessage", + } + + json_response = { + outcome: "Success", + content: "test note", + } + + before do + feedback_message + login_as admin + note_params["noteable_id"] = feedback_message.id + note_params["author_id"] = admin.id + post "/internal/reports/create_note", params: note_params + end + + it "renders the proper JSON response" do + json_response["author_name"] = admin.name + expect(response.body).to eq(json_response.to_json) + end + + it "creates a note with the correct params" do + expect(Note.last.attributes).to include(note_params) + end + end + end +end diff --git a/spec/requests/internal_feedback_messages_update_spec.rb b/spec/requests/internal_feedback_messages_update_spec.rb deleted file mode 100644 index 098384823..000000000 --- a/spec/requests/internal_feedback_messages_update_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -require "rails_helper" - -RSpec.describe "/internal/feedback_messages", type: :request do - describe "PUTS /internal/feedback_messages" do - context "with a valid request" do - let(:feedback_message) { create(:feedback_message, :abuse_report) } - let(:admin) { create(:user, :super_admin) } - - valid_abuse_report_params = { - feedback_message: { - id: 1, - status: "Resolved", - note: { - content: "this is valid", - reason: "abuse-reports", - }, - }, - } - - before do - login_as(admin) - valid_abuse_report_params[:feedback_message][:reviewer_id] = admin.id - patch "/internal/feedback_messages/#{feedback_message.id}", params: - valid_abuse_report_params - end - - it "adds a note to a report" do - expect(FeedbackMessage.last.note.content).to eq( - valid_abuse_report_params[:feedback_message][:note][:content], - ) - end - - it "adds the current user as the reviewer" do - expect(FeedbackMessage.find_by(reviewer_id: admin.id)).not_to eq(nil) - end - - it "updates the last_reviewed_at timestamp" do - expect(FeedbackMessage.last.last_reviewed_at).not_to eq(nil) - end - end - end -end diff --git a/spec/services/user_role_service_spec.rb b/spec/services/user_role_service_spec.rb index 893db5f09..a628b1193 100644 --- a/spec/services/user_role_service_spec.rb +++ b/spec/services/user_role_service_spec.rb @@ -2,44 +2,49 @@ require "rails_helper" RSpec.describe UserRoleService do let(:user) { create(:user) } + let(:admin) { create(:user, :super_admin) } let(:banned_user) { create(:user, :banned) } describe "#check_for_roles" do it "unbans a user if they were previously banned" do - described_class.new(banned_user).check_for_roles(banned: "0") + described_class.new(banned_user, admin.id).check_for_roles(banned: "0") expect(banned_user.has_role?(:banned)).to eq(false) end it "bans a user when there is a reason included" do - described_class.new(user).check_for_roles(banned: "1", reason_for_ban: "some reason") + described_class.new(user, admin.id). + check_for_roles(banned: "1", reason_for_ban: "some reason") expect(user.has_role?(:banned)).to eq(true) end it "gives an error if there was no reason for ban included" do - described_class.new(user).check_for_roles(banned: "1") + described_class.new(user, admin.id).check_for_roles(banned: "1") expect(user.errors.messages[:reason_for_ban].first). to eq("can't be blank if banned is checked") end it "gives an error if there was a reason for ban but banned was not checked" do - described_class.new(user).check_for_roles(banned: "0", reason_for_ban: "some reason") + described_class.new(user, admin.id). + check_for_roles(banned: "0", reason_for_ban: "some reason") expect(user.errors.messages[:banned].first). to eq("was not checked but had the reason filled out") end it "warns a user when there is a reason included" do - described_class.new(user).check_for_roles(warned: "1", reason_for_warning: "some_reason") + described_class.new(user, admin.id). + check_for_roles(warned: "1", reason_for_warning: "some_reason") expect(user.has_role?(:warned)).to eq(true) end it "gives an error if there was no reason for warning included" do - described_class.new(user).check_for_roles(warned: "1") + described_class.new(user, admin.id).check_for_roles(warned: "1") expect(user.errors.messages[:reason_for_warning].first). to eq("can't be blank if warned is checked") end it "gives an error if there was a reason for warning but warned was not checked" do - described_class.new(user).check_for_roles(warned: "0", reason_for_warning: "some reason") + described_class.new(user, admin.id). + check_for_roles(warned: "0", reason_for_warning: "some reason") expect(user.errors.messages[:warned].first). to eq("was not checked but had the reason filled out") end @@ -47,39 +52,46 @@ RSpec.describe UserRoleService do describe "#new_roles?" do it "adds the trusted role to a user with valid params" do - described_class.new(user).send(:new_roles?, trusted: "1") + described_class.new(user, admin.id).send(:new_roles?, trusted: "1") expect(user.has_role?(:trusted)).to eq(true) end it "adds the analytics role to a user with valid params" do - described_class.new(user).send(:new_roles?, analytics: "1") + described_class.new(user, admin.id).send(:new_roles?, analytics: "1") expect(user.has_role?(:analytics_beta_tester)).to eq(true) end it "adds the scholar role to a user with valid params" do - described_class.new(user).send(:new_roles?, scholar: "1") + described_class.new(user, admin.id).send(:new_roles?, scholar: "1") expect(user.has_role?(:workshop_pass)).to eq(true) end it "adds workshop_expiration date with valid params" do expiration_date = Time.now + 1.year - described_class.new(user). + described_class.new(user, admin.id). send(:new_roles?, scholar: "1", workshop_expiration: expiration_date) expect(user.workshop_expiration).to eq(expiration_date) end it "doesn't add a workshop_expiration date if scholar is not checked" do expiration_date = Time.now + 1.year - described_class.new(user). + described_class.new(user, admin.id). send(:new_roles?, scholar: "0", workshop_expiration: expiration_date) expect(user.workshop_expiration).to eq(nil) end end describe "#create_or_update_note" do + note_params = { + reason: "banned", + content: "some reason", + noteable_type: "User", + } + it "updates a user's previous note" do - user.notes.create(reason: "banned", content: "some reason") - described_class.new(user).send(:create_or_update_note, "banned", "a more specific reason") + user.notes.create(note_params.merge(noteable_id: user.id, author_id: user.id)) + described_class.new(user, admin.id). + send(:create_or_update_note, "banned", "a more specific reason") expect(user.notes.count).to eq(1) end end