From 002dd91a0577b524071f7a3022cf5776b993ccfe Mon Sep 17 00:00:00 2001
From: Ted Johansson <drenmi@gmail.com>
Date: Mon, 3 Apr 2023 13:56:41 +0800
Subject: [PATCH] FIX: Fix an issue where deselecting a filter tag would cause
 no results (#126)

In some cases, when having multiple tag filters selected and results in the list, deselecting one of the filters would cause no results. This is clearly incorrect behavior, as fewer filters should lead to more (or at least the same number of) results.

This happens when you have a list of selected filters, e.g. `foo|bar|baz`, and you deselect the "middle" one. This will result in the following filter: `foo||baz`, which causes the back-end to try and filter on empty string as well, and in turn leads to no results.

The order of the filter list depends on the order they were selected, which caused this to seem a bit erratic.

In the code, there's a regular expression that tries to remove consecutive `|` characters, but this is anchored to the beginning of the string, so it doesn't work for this case.

Instead of relying on a regular expression, this change splits the string into an array, filters out the deselected tag, and joins it back together into the filter string. This avoids the issues that regular expressions have.

The PR also includes unit tests for the three code paths of the filter selection.
---
 .../discourse/controllers/docs-index.js       |  5 +++-
 .../unit/controllers/docs-index-test.js       | 27 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/assets/javascripts/discourse/controllers/docs-index.js b/assets/javascripts/discourse/controllers/docs-index.js
index 877abd8..8a8b65e 100644
--- a/assets/javascripts/discourse/controllers/docs-index.js
+++ b/assets/javascripts/discourse/controllers/docs-index.js
@@ -273,7 +273,10 @@ export default Controller.extend({
   updateSelectedTags(tag) {
     let filter = this.filterTags;
     if (filter && filter.includes(tag.id)) {
-      filter = filter.replace(tag.id, "").replace(/^\|+|\|+$/g, "");
+      filter = filter
+        .split("|")
+        .filter((f) => f !== tag.id)
+        .join("|");
     } else if (filter) {
       filter = `${filter}|${tag.id}`;
     } else {
diff --git a/test/javascripts/unit/controllers/docs-index-test.js b/test/javascripts/unit/controllers/docs-index-test.js
index fc36b39..9f3471a 100644
--- a/test/javascripts/unit/controllers/docs-index-test.js
+++ b/test/javascripts/unit/controllers/docs-index-test.js
@@ -12,4 +12,31 @@ module("Unit | Controller | docs-index", function (hooks) {
     const controller = getOwner(this).lookup("controller:docs-index");
     assert.deepEqual(controller.docsCategories, ["bug", "feature", "meta"]);
   });
+
+  test("updateSelectedTags correctly removes tags", function (assert) {
+    const controller = getOwner(this).lookup("controller:docs-index");
+    controller.filterTags = "foo|bar|baz";
+
+    controller.updateSelectedTags({ id: "bar" });
+
+    assert.deepEqual(controller.filterTags, "foo|baz");
+  });
+
+  test("updateSelectedTags correctly appends tags to list", function (assert) {
+    const controller = getOwner(this).lookup("controller:docs-index");
+    controller.filterTags = "foo|bar";
+
+    controller.updateSelectedTags({ id: "baz" });
+
+    assert.deepEqual(controller.filterTags, "foo|bar|baz");
+  });
+
+  test("updateSelectedTags correctly appends tags to empty list", function (assert) {
+    const controller = getOwner(this).lookup("controller:docs-index");
+    controller.filterTags = null;
+
+    controller.updateSelectedTags({ id: "foo" });
+
+    assert.deepEqual(controller.filterTags, "foo");
+  });
 });