Ensure we have current user information before deciding how to handle the "flag user" button (#13279)

* Make test fail again

Minimal reproduction via `rspec
spec/system/user/trusted_user_flags_user_spec.rb --order=random
--seed=9374` which runs in this order:

- when signed in as a trusted user
- when not logged in
- when signed in as the non-trusted user
- when signed in as the user

Because "not logged in" immediately precedes "non-trusted user" in this
order, the browser store cache is cleared and there is no user. Since
there's no user, the flag is not removed.

* Wait for current user promise before processing current user

* Extract button callback registration to function

This addresses a code climate concern (function exceeded 50 lines) by
extracting the button behavior to a function of (button, id, name),
and calls that within the exported initFlag function.

* Prefer request to fetch

Addresses feedback to use @utilities/http's request method in  place
of fetch (which automatically adds the needed csrf headers)

* Reorder imports

Satisfies code climate report that imports are out of order

* Add honeybadger notify to error handling

Do more than just notify that something went wrong. Notify honeybadger
on failure to flag/unflag a user.

* Remove temp variable

This makes the notify code look more like the suggestion

* Reduce function arglist

Since the user id and name are properties of the flagButton's dataset,
we can efficiently extract them from the flagButton.

Only pull user id from dataset to check if current user = profile
user, and extract id and name from dataset after passing the
flagButton.

* reorder imports

Not sure how I managed to reverse this in 18aeb675b but here we go again

* Test button behavior

The original tests only asserted that the link to reactions was
present and labeled correctly. Add additional check that we can use
the button and that the label toggling occurs (this adds a request to
the test case, but adds a test for user facing behavior).

* Tame eslint check

I was getting conflicting feedback on import ordering from code
climate and eslint. Since telling eslint to ignore its rules was
immediately clear to me (there's an example on the line before this)
that's the direction I headed, but I can revisit if it matters

https://github.com/forem/forem/pull/13279#issuecomment-814411401
captures the conflict (code climate wants @utilities/http first,
eslint wants ../chat/util first, one or the other fails regardless of
the ordering.

* Use multiple rules in one ignore comment

https://eslint.org/docs/user-guide/configuring/rules#disabling-rules
supports multiple warnings separated by commas

* Remove stray comment

* Move documentation comment to the code it describes

* Replace invalid name

I had copied from the suggested code snippet the
userData.profileUserID name, but userData in this context is a global
function, and `profileUserId` (capitalization) is the bound variable
in this context.

Fix it before we throw an error trying to report an error (ironically,
before the window alert telling the user an error occurred, I think
this would have been visible only in console).

* Actually call the remove button function

app/javascript/profileDropdown/flagButton.js

@@ -1,32 +1,12 @@
 /* global userData */
-/* eslint-disable no-alert */
-/**
- * Adds a flag button visible only to trusted users on profile pages.
- * @function initFlag
- * @returns {(void|undefined)} This function has no useable return value.
- */
-export function initFlag() {
-  const flagButton = document.getElementById(
-    'user-profile-dropdownmenu-flag-button',
-  );
-
-  if (!flagButton) {
-    // button not always present when this is called
-    return;
-  }
-
-  const user = userData();
-  if (!user) {
-    return;
-  }
+/* eslint-disable no-alert, import/order */
+import { request } from '@utilities/http';
+import { getUserDataAndCsrfToken } from '../chat/util';
 
+function addFlagUserBehavior(flagButton) {
   const { profileUserId, profileUserName } = flagButton.dataset;
-  let isUserFlagged = flagButton.dataset.isUserFlagged === 'true';
-  const trustedOrAdmin = user.trusted || user.admin;
 
-  if (!trustedOrAdmin || user.id === parseInt(profileUserId, 10)) {
-    flagButton.remove();
-  }
+  let isUserFlagged = flagButton.dataset.isUserFlagged === 'true';
 
   function flag() {
     const confirmFlag = window.confirm(
@@ -36,17 +16,13 @@ export function initFlag() {
     );
 
     if (confirmFlag) {
-      fetch('/reactions', {
+      request('/reactions', {
         method: 'POST',
-        headers: {
-          'X-CSRF-Token': window.csrfToken,
-          'Content-Type': 'application/json',
-        },
-        body: JSON.stringify({
+        body: {
           reactable_type: 'User',
           category: 'vomit',
           reactable_id: profileUserId,
-        }),
+        },
       })
         .then((response) => response.json())
         .then((response) => {
@@ -58,10 +34,49 @@ export function initFlag() {
             flagButton.innerHTML = `Flag ${profileUserName}`;
           }
         })
-        .catch((e) => window.alert(`Something went wrong: ${e}`));
+        .catch((e) => {
+          Honeybadger.notify(
+            isUserFlagged ? 'Unable to unflag user' : 'Unable to flag user',
+            profileUserId,
+          );
+          window.alert(`Something went wrong: ${e}`);
+        });
     }
   }
 
   flagButton.addEventListener('click', flag);
 }
+
+/**
+ * Adds a flag button visible only to trusted users on profile pages.
+ * @function initFlag
+ * @returns {(void|undefined)} This function has no useable return value.
+ */
+
+export function initFlag() {
+  const flagButton = document.getElementById(
+    'user-profile-dropdownmenu-flag-button',
+  );
+
+  if (!flagButton) {
+    // button not always present when this is called
+    return;
+  }
+
+  getUserDataAndCsrfToken().then(() => {
+    const user = userData();
+    if (!user) {
+      flagButton.remove();
+      return;
+    }
+
+    const trustedOrAdmin = user.trusted || user.admin;
+    const { profileUserId } = flagButton.dataset;
+
+    if (!trustedOrAdmin || user.id === parseInt(profileUserId, 10)) {
+      flagButton.remove();
+    }
+    addFlagUserBehavior(flagButton);
+  });
+}
 /* eslint-enable no-alert */

spec/system/user/trusted_user_flags_user_spec.rb

@@ -2,6 +2,7 @@ require "rails_helper"
 
 RSpec.describe "Flagging users from profile pages", type: :system, js: true do
   let(:user) { create :user }
+  let(:unflag_text) { "Unflag @#{user.username}" }
   let(:flag_text) { "Flag @#{user.username}" }
 
   context "when not logged in" do
@@ -16,12 +17,7 @@ RSpec.describe "Flagging users from profile pages", type: :system, js: true do
     it "does not show the flag button" do
       sign_in create(:user)
 
-      # TODO: Fix me! Loading the page twice here ensures that the dropdown menu
-      # does not include the "Flag @username" option. This is a band-aid solution
-      # in order to get this spec to consistently pass and unblock builds.
-      2.times do
-        visit user_profile_path(user.username)
-      end
+      visit user_profile_path(user.username)
 
       click_button(id: "user-profile-dropdown")
       expect(page).not_to have_link(flag_text)
@@ -43,7 +39,11 @@ RSpec.describe "Flagging users from profile pages", type: :system, js: true do
 
       visit user_profile_path(user.username)
       click_button(id: "user-profile-dropdown")
-      expect(page).to have_link(flag_text)
+
+      accept_confirm do
+        click_link(id: "user-profile-dropdownmenu-flag-button")
+      end
+      expect(page).to have_link(unflag_text)
     end
   end
 end