Fix comment moderation's UI bugs (#19554)

.github/workflows/ci-cd.yml

@@ -369,6 +369,8 @@ jobs:
       - run: yarn cypress install
       - name: cypress
         env:
+          CYPRESS_RAILS_HOST: localhost
+          CYPRESS_RAILS_PORT: 3000
           KNAPSACK_PRO_CI_NODE_TOTAL: ${{ matrix.ci_node_total }}
           KNAPSACK_PRO_CI_NODE_INDEX: ${{ matrix.ci_node_index }}
           KNAPSACK_PRO_TEST_SUITE_TOKEN_CYPRESS: ${{ secrets.KNAPSACK_PRO_TEST_SUITE_TOKEN_CYPRESS }}

app/javascript/packs/commentModPage.js

@@ -1,20 +1,32 @@
 import { updateExperienceLevel } from '../actionsPanel/actionsPanel';
 
-function applyReactedClass(category) {
-  const upVote = document.querySelector("[data-category='thumbsup']");
-  const downVote = document.querySelector("[data-category='thumbsdown']");
-  const vomitVote = document.querySelector("[data-category='vomit']");
+/**
+ * A thumbsup reaction on a comment/article will invalidate a previous thumbsdown
+ * or vomit reaction (they will be deleted on the server by the reaction handler)
+ * and vice versa. This function updates the UI to match.
+ * @param {HTMLButtonElement} clickedBtn The reaction button that was clicked
+ */
+function toggleContradictoryReactions(clickedBtn) {
+  const contentActions = document.querySelector('#content-mod-actions');
 
-  if (category === 'thumbsup') {
-    downVote.classList.remove('reacted');
-    vomitVote.classList.remove('reacted');
-  } else {
-    upVote.classList.remove('reacted');
+  if (clickedBtn.parentElement === contentActions) {
+    const upVote = contentActions.querySelector("[data-category='thumbsup']");
+    const downVote = contentActions.querySelector(
+      "[data-category='thumbsdown']",
+    );
+    const vomitVote = contentActions.querySelector("[data-category='vomit']");
+
+    if (clickedBtn.dataset.category === 'thumbsup') {
+      downVote.classList.remove('reacted');
+      vomitVote.classList.remove('reacted');
+    } else {
+      upVote.classList.remove('reacted');
+    }
   }
 }
 
-async function updateMainReactions(reactableType, category, reactableId) {
-  const clickedBtn = document.querySelector(`[data-category="${category}"]`);
+async function updateMainReactions(clickedBtn) {
+  const { reactableType, category, reactableId } = clickedBtn.dataset;
   try {
     const response = await fetch('/reactions', {
       method: 'POST',
@@ -34,6 +46,7 @@ async function updateMainReactions(reactableType, category, reactableId) {
     const outcome = await response.json();
 
     if (outcome.result === 'create') {
+      toggleContradictoryReactions(clickedBtn);
       clickedBtn.classList.add('reacted');
     } else if (outcome.result === 'destroy') {
       clickedBtn.classList.remove('reacted');
@@ -66,20 +79,17 @@ Array.from(document.getElementsByClassName('level-rating-button')).forEach(
 document
   .querySelectorAll('.reaction-button, .reaction-vomit-button')
   .forEach((btn) => {
-    btn.addEventListener('click', () => {
-      applyReactedClass(btn.dataset.category);
-      updateMainReactions(
-        btn.dataset.reactableType,
-        btn.dataset.category,
-        btn.dataset.reactableId,
-      );
+    btn.addEventListener('click', async () => {
+      await updateMainReactions(btn);
     });
   });
 
 const form = document.getElementsByClassName('button_to')[0];
-form.addEventListener('submit', (e) => {
-  e.preventDefault();
-  if (confirm('Are you SURE you want to delete this comment?')) {
-    form.submit();
-  }
-});
+if (form) {
+  form.addEventListener('submit', (e) => {
+    e.preventDefault();
+    if (confirm('Are you SURE you want to delete this comment?')) {
+      form.submit();
+    }
+  });
+}

app/services/reaction_handler.rb

@@ -37,14 +37,14 @@ class ReactionHandler
   delegate :rate_limiter, to: :current_user
 
   def create
-    destroy_contradictory_mod_reactions if reactable_type == "Article"
+    destroy_contradictory_mod_reactions if %w[Article Comment].include?(reactable_type)
     return noop_result if existing_reaction
 
     create_new_reaction
   end
 
   def toggle
-    destroy_contradictory_mod_reactions if reactable_type == "Article"
+    destroy_contradictory_mod_reactions if %w[Article Comment].include?(reactable_type)
     return handle_existing_reaction if existing_reaction
 
     create_new_reaction

app/views/moderations/mod.html.erb

@@ -33,7 +33,7 @@
     <hr />
   <% end %>
 
-  <div style="display: flex; justify-content: space-around;">
+  <div id="content-mod-actions" class="flex justify-around">
   <button class="reaction-button <%= Reaction.cached_any_reactions_for?(@moderatable, current_user, "thumbsup") ? "reacted" : "" %>"
           data-reactable-id="<%= @moderatable.id %>"
           data-category="thumbsup"
@@ -82,7 +82,7 @@
   </p>
   <div class="tag-mod-form" style="display: flex; flex-direction: column; align-items: center;">
     <h2><%= t("views.moderations.actions.abusive.subtitle_html", user: link_to(@moderatable.user.username, @moderatable.user.path)) %></h2>
-    <button class="reaction-button <%= Reaction.cached_any_reactions_for?(@moderatable, current_user, "vomit") ? "reacted" : "" %>"
+    <button class="reaction-button <%= Reaction.cached_any_reactions_for?(@moderatable.user, current_user, "vomit") ? "reacted" : "" %>"
             data-reactable-id="<%= @moderatable.user.id %>"
             data-category="vomit"
             data-reactable-type="<%= @moderatable.user.class.name %>">

bin/e2e

@@ -1,4 +1,12 @@
 #!/bin/bash
+
+# Setting the port that the Rails app launches on for a stable app domain in permalinks
+# (otherwise, Cypress chooses a random port each run)
+# Choosing 30300 here to avoid conflicts with local dev on :3000
+export CYPRESS_RAILS_HOST="localhost"
+export CYPRESS_RAILS_PORT="30300"
+export APP_DOMAIN="localhost:30300"
+
 printf "Doing a quick bundle check to make sure gems are all up to date.\n\n"
 bundle check # ensure gems are up to date
 
@@ -34,5 +42,5 @@ if [ "$1" = "--creator-onboarding-seed" ]; then
    echo "Running E2E tests with creator onboarding seed data"
    CYPRESS_RAILS_CYPRESS_OPTS="--config-file cypress.dev.config.js --e2e" RAILS_ENV=test E2E=true CREATOR_ONBOARDING_SEED_DATA=1 E2E_FOLDER=creatorOnboardingFlows bundle exec rake cypress:open
 else
-   CYPRESS_RAILS_CYPRESS_OPTS="--config-file cypress.dev.config.js --e2e" RAILS_ENV=test E2E=true bundle exec rake cypress:open 
+   CYPRESS_RAILS_CYPRESS_OPTS="--config-file cypress.dev.config.js --e2e" RAILS_ENV=test E2E=true bundle exec rake cypress:open
 fi

cypress/e2e/seededFlows/articleFlows/commentModerationTools.spec.js

@@ -0,0 +1,135 @@
+describe('Moderation Tools for Comments', () => {
+  const navigateToCommentFlagPage = () => {
+    return cy.get('#comments').within(() => {
+      cy.findByRole('button', { name: 'Toggle dropdown menu' }).click();
+      cy.findByRole('link', { name: 'Moderate' }).click();
+    });
+  };
+
+  const findButton = (labelText, { as }) => {
+    cy.get('.reaction-button')
+      .filter(`:contains("${labelText}")`)
+      .as(as)
+      .should('not.have.class', 'reacted');
+  };
+
+  const clickButton = (buttonAlias) => {
+    cy.get(buttonAlias).click();
+    cy.wait('@flagRequest');
+  };
+
+  beforeEach(() => {
+    cy.testSetup();
+
+    cy.fixture('users/questionableUser.json').as('questionableUser');
+  });
+
+  context('when the user is only a trusted user', () => {
+    beforeEach(() => {
+      cy.fixture('users/trustedUser.json').as('trustedUser');
+
+      cy.get('@trustedUser').then((trustedUser) => {
+        cy.loginAndVisit(trustedUser, '/series_user/series-test-article-slug');
+      });
+    });
+
+    it('flags and unflags only the comment when clicked', () => {
+      navigateToCommentFlagPage().then(() => {
+        cy.get('@questionableUser').then(({ username }) => {
+          cy.intercept('POST', '/reactions').as('flagRequest');
+
+          findButton('Flag to Admins', { as: 'contentFlag' });
+          findButton(`Flag ${username}`, { as: 'userFlag' });
+
+          clickButton('@contentFlag');
+
+          cy.get('@contentFlag').should('have.class', 'reacted');
+          cy.get('@userFlag').should('not.have.class', 'reacted');
+
+          clickButton('@contentFlag');
+
+          cy.get('@contentFlag').should('not.have.class', 'reacted');
+          cy.get('@userFlag').should('not.have.class', 'reacted');
+        });
+      });
+    });
+
+    it('flags and unflags only the user when clicked', () => {
+      navigateToCommentFlagPage().then(() => {
+        cy.get('@questionableUser').then(({ username }) => {
+          cy.intercept('POST', '/reactions').as('flagRequest');
+
+          findButton('Flag to Admins', { as: 'contentFlag' });
+          findButton(`Flag ${username}`, { as: 'userFlag' });
+
+          clickButton('@userFlag');
+
+          cy.get('@contentFlag').should('not.have.class', 'reacted');
+          cy.get('@userFlag').should('have.class', 'reacted');
+
+          clickButton('@userFlag');
+
+          cy.get('@contentFlag').should('not.have.class', 'reacted');
+          cy.get('@userFlag').should('not.have.class', 'reacted');
+        });
+      });
+    });
+
+    it('visually toggles contradictory mod reactions off', () => {
+      navigateToCommentFlagPage().then(() => {
+        cy.intercept('POST', '/reactions').as('flagRequest');
+
+        findButton('High Quality', { as: 'thumbsUp' });
+        findButton('Low Quality', { as: 'thumbsDown' });
+        findButton('Flag to Admins', { as: 'vomit' });
+
+        clickButton('@thumbsUp');
+
+        cy.get('@thumbsUp').should('have.class', 'reacted');
+        cy.get('@thumbsDown').should('not.have.class', 'reacted');
+        cy.get('@vomit').should('not.have.class', 'reacted');
+
+        clickButton('@thumbsDown');
+
+        cy.get('@thumbsUp').should('not.have.class', 'reacted');
+        cy.get('@thumbsDown').should('have.class', 'reacted');
+        cy.get('@vomit').should('not.have.class', 'reacted');
+
+        clickButton('@vomit');
+
+        cy.get('@thumbsUp').should('not.have.class', 'reacted');
+        cy.get('@thumbsDown').should('have.class', 'reacted');
+        cy.get('@vomit').should('have.class', 'reacted');
+
+        clickButton('@thumbsUp');
+
+        cy.get('@thumbsUp').should('have.class', 'reacted');
+        cy.get('@thumbsDown').should('not.have.class', 'reacted');
+        cy.get('@vomit').should('not.have.class', 'reacted');
+      });
+    });
+  });
+
+  context('when the user is an admin', () => {
+    beforeEach(() => {
+      cy.fixture('users/adminUser.json').as('admin');
+
+      cy.get('@admin').then((admin) => {
+        cy.loginAndVisit(admin, '/series_user/series-test-article-slug');
+      });
+    });
+
+    it('also permits deleting comment with confirmation', () => {
+      cy.findByText('Comment deleted').should('not.exist');
+
+      navigateToCommentFlagPage().then(() => {
+        cy.on('window:confirm', () => true);
+
+        cy.findByRole('button', { name: /Delete Comment/ }).click();
+
+        cy.url().should('include', '/series_user/series-test-article-slug');
+        cy.findByText('Comment deleted').should('exist');
+      });
+    });
+  });
+});

cypress/fixtures/users/questionableUser.json

@@ -0,0 +1,5 @@
+{
+  "username": "questionable_user",
+  "email": "questionable-user@forem.local",
+  "password": "password"
+}

spec/services/reaction_handler_spec.rb

@@ -122,6 +122,25 @@ RSpec.describe ReactionHandler, type: :service do
         expect(reaction_handler.reaction.user.last_reacted_at).to eq Time.current
       end
     end
+
+    context "when the reactable is a comment" do
+      let(:user) { moderator }
+      let(:comment) { create(:comment) }
+      let!(:contradictory_mod) { comment.reactions.create! user: moderator, category: "thumbsup" }
+      let(:params) do
+        {
+          reactable_id: comment.id,
+          reactable_type: comment.class.polymorphic_name,
+          category: "vomit"
+        }
+      end
+
+      it "also destroys contradictory mod reactions" do
+        expect(result).to be_success
+        expect(result.action).to eq("create")
+        expect(Reaction.ids).not_to include(contradictory_mod.id)
+      end
+    end
   end
 
   describe "#toggle" do
@@ -219,5 +238,24 @@ RSpec.describe ReactionHandler, type: :service do
         expect(Reaction.ids).not_to include(contradictory_mod.id)
       end
     end
+
+    context "when the reactable is a comment" do
+      let(:user) { moderator }
+      let(:comment) { create(:comment) }
+      let!(:contradictory_mod) { comment.reactions.create! user: moderator, category: "thumbsup" }
+      let(:params) do
+        {
+          reactable_id: comment.id,
+          reactable_type: comment.class.polymorphic_name,
+          category: "vomit"
+        }
+      end
+
+      it "also destroys contradictory mod reactions" do
+        expect(result).to be_success
+        expect(result.action).to eq("create")
+        expect(Reaction.ids).not_to include(contradictory_mod.id)
+      end
+    end
   end
 end

spec/support/seeds/seeds_e2e.rb

@@ -764,6 +764,48 @@ end
 
 ##############################################################################
 
+seeder.create_if_doesnt_exist(User, "email", "questionable-user@forem.local") do
+  User.create!(
+    name: "Questionable User",
+    email: "questionable-user@forem.local",
+    username: "questionable_user",
+    profile_image: Rails.root.join("app/assets/images/#{rand(1..40)}.png").open,
+    confirmed_at: Time.current,
+    registered_at: Time.current,
+    password: "password",
+    password_confirmation: "password",
+    saw_onboarding: true,
+    checked_code_of_conduct: true,
+    checked_terms_and_conditions: true,
+  )
+end
+
+questionable_user = User.find_by(email: "questionable-user@forem.local")
+
+##############################################################################
+
+seeder.create_if_doesnt_exist(Article, "title", "Questionable article") do
+  markdown = <<~MARKDOWN
+    ---
+    title:  Questionable article
+    published: true
+    cover_image: #{Faker::Company.logo}
+    ---
+    #{Faker::Hipster.paragraph(sentence_count: 2)}
+    #{Faker::Markdown.random}
+    #{Faker::Hipster.paragraph(sentence_count: 2)}
+  MARKDOWN
+  Article.create(
+    body_markdown: markdown,
+    featured: false,
+    show_comments: true,
+    slug: "questionable-test-article-slug",
+    user_id: questionable_user.id,
+  )
+end
+
+##############################################################################
+
 seeder.create_if_doesnt_exist(Article, "title", "Series test article") do
   markdown = <<~MARKDOWN
     ---
@@ -776,13 +818,22 @@ seeder.create_if_doesnt_exist(Article, "title", "Series test article") do
     #{Faker::Markdown.random}
     #{Faker::Hipster.paragraph(sentence_count: 2)}
   MARKDOWN
-  Article.create(
+  article = Article.create(
     body_markdown: markdown,
     featured: true,
     show_comments: true,
     slug: "series-test-article-slug",
     user_id: User.find_by(email: "series-user@forem.local").id,
   )
+
+  comment_attributes = {
+    body_markdown: Faker::Hipster.paragraph(sentence_count: 1),
+    user_id: questionable_user.id,
+    commentable_id: article.id,
+    commentable_type: "Article"
+  }
+
+  Comment.create!(comment_attributes)
 end
 
 ##############################################################################