FIX: makes tests more reliable (#6)

FIX: makes tests more reliable (#6)

diff --git a/assets/javascripts/discourse/initializers/init-code-review.js.es6 b/assets/javascripts/discourse/initializers/init-code-review.js.es6
index 22a62dc..8ef5f7e 100644
--- a/assets/javascripts/discourse/initializers/init-code-review.js.es6
+++ b/assets/javascripts/discourse/initializers/init-code-review.js.es6
@@ -37,23 +37,20 @@ function initialize(api) {
     }.property("authProviders")
   });
 
-  function allowUser() {
-    const currentUser = api.getCurrentUser();
+  function allowUser(currentUser) {
     if (!currentUser) {
       return false;
     }
+
     return currentUser.get("staff");
   }
 
-  function allowApprove(topic) {
-    const currentUser = api.getCurrentUser();
+  function allowApprove(currentUser, topic, siteSettings) {
     if (!currentUser) {
       return false;
     }
 
-    const siteSettings = api.container.lookup("site-settings:main");
     const allowSelfApprove = siteSettings.code_review_allow_self_approval;
-
     const approvedTag = siteSettings.code_review_approved_tag;
     const pendingTag = siteSettings.code_review_pending_tag;
     const followupTag = siteSettings.code_review_followup_tag;
@@ -67,9 +64,7 @@ function initialize(api) {
     );
   }
 
-  function allowFollowup(topic) {
-    const siteSettings = api.container.lookup("site-settings:main");
-
+  function allowFollowup(topic, siteSettings) {
     const approvedTag = siteSettings.code_review_approved_tag;
     const pendingTag = siteSettings.code_review_pending_tag;
     const followupTag = siteSettings.code_review_followup_tag;
@@ -93,7 +88,7 @@ function initialize(api) {
     classNames: ["approve"],
     dependentKeys: ["topic.tags"],
     displayed() {
-      return allowUser() && allowApprove(this.get("topic"));
+      return allowUser(this.currentUser) && allowApprove(this.currentUser, this.get("topic"), this.siteSettings);
     }
   });
 
@@ -108,7 +103,7 @@ function initialize(api) {
     classNames: ["followup"],
     dependentKeys: ["topic.tags"],
     displayed() {
-      return allowUser() && allowFollowup(this.get("topic"));
+      return allowUser(this.currentUser) && allowFollowup(this.get("topic"), this.siteSettings);
     }
   });
 }
diff --git a/test/javascripts/acceptance/self-approve-desktop-test.js.es6 b/test/javascripts/acceptance/self-approve-desktop-test.js.es6
new file mode 100644
index 0000000..fa8346e
--- /dev/null
+++ b/test/javascripts/acceptance/self-approve-desktop-test.js.es6
@@ -0,0 +1,33 @@
+import { replaceCurrentUser, acceptance } from "helpers/qunit-helpers";
+import Fixtures from "fixtures/topic";
+
+acceptance("review desktop", {
+  loggedIn: true,
+  settings: {
+    code_review_approved_tag: "approved",
+    code_review_pending_tag: "pending",
+    code_review_followup_tag: "followup"
+  }
+});
+
+QUnit.test("shows approve button by default", async assert => {
+  const json = $.extend(true, {}, Fixtures["/t/280/1.json"]);
+
+  json.tags = ["pending"];
+
+  server.get("/t/281.json", () => {
+    return [200, { "Content-Type": "application/json" }, json];
+  });
+
+  await visit("/t/internationalization-localization/281");
+
+  assert.ok(exists("#topic-footer-button-approve"));
+});
+
+QUnit.test("hides approve button if user is self", async assert => {
+  replaceCurrentUser({ id: 1 });
+
+  await visit("/t/this-is-a-test-topic/9/1");
+
+  assert.ok(!exists("#topic-footer-button-approve"));
+});
diff --git a/test/javascripts/acceptance/self-approve-mobile-test.js.es6 b/test/javascripts/acceptance/self-approve-mobile-test.js.es6
new file mode 100644
index 0000000..debe539
--- /dev/null
+++ b/test/javascripts/acceptance/self-approve-mobile-test.js.es6
@@ -0,0 +1,40 @@
+import { replaceCurrentUser, acceptance } from "helpers/qunit-helpers";
+import Fixtures from "fixtures/topic";
+
+acceptance("review mobile", {
+  loggedIn: true,
+  mobileView: true,
+  settings: {
+    code_review_approved_tag: "approved",
+    code_review_pending_tag: "pending",
+    code_review_followup_tag: "followup"
+  }
+});
+
+QUnit.test("shows approve button by default", async assert => {
+  const json = $.extend(true, {}, Fixtures["/t/280/1.json"]);
+
+  json.tags = ["pending"];
+
+  server.get("/t/281.json", () => {
+    return [200, { "Content-Type": "application/json" }, json];
+  });
+
+  await visit("/t/internationalization-localization/281");
+
+  const menu = selectKit(".topic-footer-mobile-dropdown");
+  await menu.expand();
+
+  assert.ok(menu.rowByValue("approve").exists());
+});
+
+QUnit.test("hides approve button if user is self", async assert => {
+  replaceCurrentUser({ id: 1 });
+
+  await visit("/t/this-is-a-test-topic/9/1");
+
+  const menu = selectKit(".topic-footer-mobile-dropdown");
+  await menu.expand();
+
+  assert.notOk(menu.rowByValue("approve").exists());
+});
diff --git a/test/javascripts/acceptance/self-approve-test-desktop.js.es6 b/test/javascripts/acceptance/self-approve-test-desktop.js.es6
deleted file mode 100644
index bd7f0a5..0000000
--- a/test/javascripts/acceptance/self-approve-test-desktop.js.es6
+++ /dev/null
@@ -1,34 +0,0 @@
-import { replaceCurrentUser, acceptance } from "helpers/qunit-helpers";
-import Fixtures from "fixtures/topic";
-
-acceptance("review desktop", {
-  loggedIn: true,
-  mobileView: false,
-  settings: {
-    code_review_approved_tag: "approved",
-    code_review_pending_tag: "pending",
-    code_review_followup_tag: "followup"
-  }
-});
-
-QUnit.test("shows approve button by default", async assert => {
-  const json = $.extend(true, {}, Fixtures["/t/280/1.json"]);
-
-  json.tags = ["pending"];
-
-  server.get("/t/281.json", () => {
-    return [200, { "Content-Type": "application/json" }, json];
-  });
-
-  await visit("/t/internationalization-localization/281");
-
-  assert.ok(count("#topic-footer-button-approve") > 0);
-});
-
-QUnit.test("hides approve button if user is self", async assert => {
-  replaceCurrentUser({ id: 1 });
-
-  await visit("/t/this-is-a-test-topic/9/1");
-
-  assert.ok(count("#topic-footer-button-approve") === 0);
-});
diff --git a/test/javascripts/acceptance/self-approve-test-mobile.js.es6 b/test/javascripts/acceptance/self-approve-test-mobile.js.es6
deleted file mode 100644
index e88c559..0000000
--- a/test/javascripts/acceptance/self-approve-test-mobile.js.es6
+++ /dev/null
@@ -1,49 +0,0 @@
-import { replaceCurrentUser, acceptance } from "helpers/qunit-helpers";
-import { clearCallbacks } from "select-kit/mixins/plugin-api";
-import Fixtures from "fixtures/topic";
-
-acceptance("review mobile", {
-  loggedIn: true,
-  mobileView: true,
-  settings: {
-    code_review_approved_tag: "approved",
-    code_review_pending_tag: "pending",
-    code_review_followup_tag: "followup"
-  },
-  afterEach() {
-    clearCallbacks();
-  },
-  beforeEach() {
-    // we need to clean this up in core
-    // plugin api keeps being re-initialized
-    clearCallbacks();
-  }
-});
-
-QUnit.test("shows approve button by default", async assert => {
-  const json = $.extend(true, {}, Fixtures["/t/280/1.json"]);
-
-  json.tags = ["pending"];
-
-  server.get("/t/281.json", () => {
-    return [200, { "Content-Type": "application/json" }, json];
-  });
-
-  await visit("/t/internationalization-localization/281");
-
-  const menu = selectKit(".topic-footer-mobile-dropdown");
-  await menu.expand();
-
-  assert.ok(menu.rowByValue("approve").exists());
-});
-
-QUnit.test("hides approve button if user is self", async assert => {
-  replaceCurrentUser({ id: 1 });
-
-  await visit("/t/this-is-a-test-topic/9/1");
-
-  const menu = selectKit(".topic-footer-mobile-dropdown");
-  await menu.expand();
-
-  assert.ok(!menu.rowByValue("approve").exists());
-});

GitHub sha: 44cebb78

What about?

function alloweUser(currentUser) {
  return currentUser && currentUser.get("staff");
}
1 Like

agreed.

Done in minor refactoring and prettification · discourse/discourse-code-review@91a278f · GitHub, I also wanted to run prettier on the file so I did it in this commit.

1 Like