From f2a8cbce7eb32d35c3f7538addaadd014d17d864 Mon Sep 17 00:00:00 2001
From: Daniel Uber <djuber@gmail.com>
Date: Wed, 19 Jan 2022 08:32:10 -0600
Subject: [PATCH] Remove "connect" feedback message special treatment (#16167)

* Remove special handling of "connect" feedback by name

The special casing was related to "connect" feedback having both a
reporter and an offender. Check for offender instead.

Additionally, there was special casing in the controller to rate-limit
connect feedback separately from other channels. Since connect doesn't
exist, we should not need this.

There's a small bit of functionality (when I post to feedback_messages, the
number of feedback messages increases) that was removed from the test
case, we can add that back (and "connect" type, and
offender_id attributes) since it looks like it might have been a
useful assertion.

* Add back feedback message controller creates feedback message case

This was removed in the last commit because it was in a "connect" chat
channel context, but the basic "should persist a record" test was
otherwise valid. Submit an abuse-report rather than a connect message
report.

* typo

feeedback, woops.
---
 .../feedback_messages_controller.rb           |  6 +-
 .../_feedback_message.html.erb                | 31 ++++----
 .../admin/feedback_messages/_style.html.erb   | 70 +++++++++----------
 spec/system/feedback_message_spec.rb          |  8 +--
 4 files changed, 51 insertions(+), 64 deletions(-)

diff --git a/app/controllers/feedback_messages_controller.rb b/app/controllers/feedback_messages_controller.rb
index f34869f6c..926f7d7b7 100644
--- a/app/controllers/feedback_messages_controller.rb
+++ b/app/controllers/feedback_messages_controller.rb
@@ -12,7 +12,7 @@ class FeedbackMessagesController < ApplicationController
     @feedback_message = FeedbackMessage.new(params)
 
     recaptcha_enabled = ReCaptcha::CheckEnabled.call(current_user)
-    if (!recaptcha_enabled || recaptcha_verified? || connect_feedback?) && !rate_limit? && @feedback_message.save
+    if (!recaptcha_enabled || recaptcha_verified?) && !rate_limit? && @feedback_message.save
       Slack::Messengers::Feedback.call(
         user: current_user,
         type: feedback_message_params[:feedback_type],
@@ -57,10 +57,6 @@ class FeedbackMessagesController < ApplicationController
     params["g-recaptcha-response"] && verify_recaptcha(recaptcha_params)
   end
 
-  def connect_feedback?
-    feedback_message_params[:feedback_type] == "connect"
-  end
-
   def feedback_message_params
     params.require(:feedback_message).permit(FEEDBACK_ALLOWED_PARAMS)
   end
diff --git a/app/views/admin/feedback_messages/_feedback_message.html.erb b/app/views/admin/feedback_messages/_feedback_message.html.erb
index 270311741..a5444a877 100644
--- a/app/views/admin/feedback_messages/_feedback_message.html.erb
+++ b/app/views/admin/feedback_messages/_feedback_message.html.erb
@@ -19,7 +19,7 @@
       <div class="row">
         <div class="col-9">
           <h5 class="fw-bold">
-          <% if feedback_message.feedback_type == "connect" %>
+          <% if feedback_message.offender %>
             Reporter and Affected:
           <% else %>
             Reporter:
@@ -34,7 +34,7 @@
             <% end %>
           </h5>
 
-          <% if feedback_message.feedback_type == "connect" %>
+          <% if feedback_message.offender %>
             <h5 class="fw-bold">
               Offender:
             </h5>
@@ -55,30 +55,27 @@
         <div class="col-3">
           <h3 class="report__tags">
             <span class="badge badge-warning float-right"><%= feedback_message.category.titleize %></span>
-            <% if feedback_message.feedback_type == "connect" %>
-            <span class="badge badge-warning float-right badge-connect"><%= feedback_message.feedback_type %></span>
-            <% end %>
           </h3>
         </div>
         <div class="col-12">
-          <% if feedback_message.feedback_type != "connect" %>
-          <h5 class="fw-bold">
-            Message:
-          </h5>
-          <p>
-            <% if feedback_message.message.blank? %>
-              <span class="font-italic">No message was left.</span>
-            <% else %>
-              <%= feedback_message.message %>
-            <% end %>
-          </p>
-          <% else %>
+          <% if feedback_message.offender %>
             <h5 class="fw-bold">
               Message from Offender:
             </h5>
             <div class="reported__message">
               <%= raw(feedback_message.message) %>
             </div>
+            <% else %>
+            <h5 class="fw-bold">
+              Message:
+            </h5>
+            <p>
+            <% if feedback_message.message.blank? %>
+              <span class="font-italic">No message was left.</span>
+            <% else %>
+              <%= feedback_message.message %>
+            <% end %>
+            </p>
           <% end %>
         </div>
       </div>
diff --git a/app/views/admin/feedback_messages/_style.html.erb b/app/views/admin/feedback_messages/_style.html.erb
index 873bde974..4a5dacf70 100644
--- a/app/views/admin/feedback_messages/_style.html.erb
+++ b/app/views/admin/feedback_messages/_style.html.erb
@@ -1,42 +1,36 @@
 <style>
-  .notefield {
-    width: 100%;
-    resize: none;
-    font-size: 18px;
-    height: 50px;
-    border-radius: 3px;
-    padding: 5px;
-    margin: 10px 0px;
-  }
+ .notefield {
+   width: 100%;
+   resize: none;
+   font-size: 18px;
+   height: 50px;
+   border-radius: 3px;
+   padding: 5px;
+   margin: 10px 0px;
+ }
 
-  .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;
-  }
-  .badge-connect {
-  background-color: #26d9ca;
-  margin-top: 5px;
-  text-transform: capitalize;
-}
-.reported__message {
-  border: 1px solid;
-  margin: 20px auto;
-  padding: 20px;
-  max-height: 300px;
-  overflow: scroll;
-}
-.report__tags{
-  display: grid;
-justify-items: end;
-}
+ .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;
+ }
+ .reported__message {
+   border: 1px solid;
+   margin: 20px auto;
+   padding: 20px;
+   max-height: 300px;
+   overflow: scroll;
+ }
+ .report__tags{
+   display: grid;
+   justify-items: end;
+ }
 </style>
diff --git a/spec/system/feedback_message_spec.rb b/spec/system/feedback_message_spec.rb
index 1204636c8..1c2fcacd0 100644
--- a/spec/system/feedback_message_spec.rb
+++ b/spec/system/feedback_message_spec.rb
@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe "Feedback report by chat channel messages", type: :system do
+RSpec.describe "Feedback report", type: :system do
   let(:user) { create(:user) }
   let(:message) { Faker::Lorem.paragraph }
   let(:url) { Faker::Lorem.sentence }
@@ -14,10 +14,10 @@ RSpec.describe "Feedback report by chat channel messages", type: :system do
       expect do
         post "/feedback_messages", params: {
           feedback_message: {
-            message: "Test Message",
-            feedback_type: "connect",
+            message: message,
+            feedback_type: "abuse-reports",
             category: "rude or vulgar",
-            offender_id: user.id
+            reported_url: url
           }
         }, as: :json
       end.to change(FeedbackMessage, :count).by(1)