Repository: pajlads/DinkPlugin (java)

Issue:
Reopen discussion on rsn whitelist option
I would once again like to request a rsn whitelist feature.

I am the original requester of this feature (https://github.com/pajlads/DinkPlugin/issues/111), and even though a whitelist suited my use case at the time. Over the last year multiple friends have expressed disappointment of not being able to configure a whitelist. For some friends this was a deal breaker and they chose to stick to other (older/abanonded) plugins even they prefer the features of Dink. Therefore I now believe the user should be able to choose between a whitelist and an ignorelist.

I understand it could be confusing for some users and therefore propose the following:

A menu called 'Filter mode' that allows the user to choose between 'Allow all RSNs' and 'Ignore all RSNs', this option would sit above the current input box for RSNs. This way of doing is is not unheard of in runelite plugins, for example the [Clan Spam Filter](https://github.com/andreasx23/GuildSpamFilter) plugin has a similar toggle:

![JLGyomp](https://github.com/pajlads/DinkPlugin/assets/9064744/06636921-70c2-47da-a081-027eda777544)

The current 'Ignored RSN's' input box would also be renamed to a more apt 'RSN list' or 'List of RSNs' input box, as well as the tooltip.
In addition the default should still be set to 'Ignore all RSNs' so that the plugin doesn't break if a user fails to spot the configuration option.

Happy to go back on forth on this if needed, it really is a feature me and my friend group would love to see.

Thanks!


Pull request description:
feat: add allowlist filter mode for rsn advanced config
Closes #305

Uses @Felanbird's suggestion of a single input box and allow/deny toggle in https://github.com/pajlads/DinkPlugin/pull/114#issuecomment-1375997995 to avoid complexity of the initial approach of having a separate box for the allowlist and denylist.

Still, we discourage use of the allow filter mode since users can get confused when a new account does not fire notifications (because they have not added it to the input box).

![config](https://github.com/pajlads/DinkPlugin/assets/8106344/781e739d-7c65-4d80-8e92-134ee7886acf)



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 301bc59..837e90d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## Unreleased
 
+- Minor: Add allowlist mode for Filtered RSNs under the Advanced category. Use this setting with caution. (#306)
 - Minor: Send kill count notifications for Penance Queen kills. (#304)
 - Minor: Include popup notification widget in collection log screenshots. (#309)
 - Minor: Support new thread creation for Discord forum channels. (#302)
diff --git a/README.md b/README.md
index ff17c57..aa52c05 100644
--- a/README.md
+++ b/README.md
@@ -55,7 +55,7 @@ Some notifiers require in-game settings to be configured to send chat messages u
 - Screenshots can be individually configured for each notifier
 - Screenshots are compressed if needed to comply with Discord limits
 - The chat box (and private messages above chat) can be hidden from screenshots
-- The plugin can skip notifications if the current player name is on the user-configured RSN ignore list
+- The plugin can skip notifications for player names that do not comply with the user-configured RSN filter list
 - Users can choose whether their webhook messages are sent in Discord's rich embed format or a traditional format
 - The player name in Discord rich embeds can be linked to various tracking services (from HiScores to Wise Old Man)
 - Discord rich embed footers can be customized with user-specified text and image url
@@ -94,7 +94,7 @@ For example: `::dinkexport pet` or `::dinkexport collectionlog`.
 With the output of the above command (`::dinkexport`) copied to your clipboard, you can merge these settings with your own via the `::dinkimport` chat command.
 
 This import can replace all of your notifier settings.
-However, webhook URL lists and ignored RSNs will be combined, rather than outright replaced.
+However, webhook URL lists and filtered RSNs will be combined, rather than outright replaced.
 If you would like all settings overwritten rather than merged during import, simply press the `Reset` button at the bottom of the plugin settings panel to clear out all settings (including URLs) before running `::dinkimport`.
 
 After an import, if the dink plugin settings panel was open, simply close and open it for the updated configuration to be reflected in the user interface.
diff --git a/src/main/java/dinkplugin/DinkPluginConfig.java b/src/main/java/dinkplugin/DinkPluginConfig.java
index 3b6eccd..80ce835 100644
--- a/src/main/java/dinkplugin/DinkPluginConfig.java
+++ b/src/main/java/dinkplugin/DinkPluginConfig.java
@@ -3,6 +3,7 @@ package dinkplugin;
 import dinkplugin.domain.AchievementDiary;
 import dinkplugin.domain.ClueTier;
 import dinkplugin.domain.CombatAchievementTier;
+import dinkplugin.domain.FilterMode;
 import dinkplugin.domain.PlayerLookupService;
 import net.runelite.client.config.Config;
 import net.runelite.client.config.ConfigGroup;
@@ -241,21 +242,34 @@ public interface DinkPluginConfig extends Config {
     }
 
     @ConfigItem(
-        keyName = "ignoredNames",
-        name = "Ignored RSNs",
-        description = "Prevent notifications from the following player names (One name per line)",
+        keyName = "ignoredNames", // historical name, preserved for backwards compatibility
+        name = "Filtered RSNs",
+        description = "Restrict what player names can trigger notifications (One name per line)<br/>" +
+            "This acts as an allowlist or denylist based on the 'RSN Filter Mode' setting below.",
         position = 1007,
         section = advancedSection
     )
-    default String ignoredNames() {
+    default String filteredNames() {
         return "";
     }
 
+    @ConfigItem(
+        keyName = "nameFilterMode",
+        name = "RSN Filter Mode",
+        description = "Allow Mode: Only allow notifications for RSNs on the list above (discouraged).<br/>" +
+            "Deny Mode: Prevent notifications from RSNs on the list above (default/recommended).",
+        position = 1008,
+        section = advancedSection
+    )
+    default FilterMode nameFilterMode() {
+        return FilterMode.DENY;
+    }
+
     @ConfigItem(
         keyName = "playerLookupService",
         name = "Player Lookup Service",
         description = "The service used to lookup a players account, to make their name clickable in Discord embeds",
-        position = 1008,
+        position = 1009,
         section = advancedSection
     )
     default PlayerLookupService playerLookupService() {
@@ -267,7 +281,7 @@ public interface DinkPluginConfig extends Config {
         name = "Hide Chat in Images",
         description = "Whether to hide the chat box and private messages when capturing screenshots.<br/>" +
             "Note: visually you may notice the chat box momentarily flicker as it is hidden for the screenshot.",
-        position = 1009,
+        position = 1010,
         section = advancedSection
     )
     default boolean screenshotHideChat() {
@@ -278,7 +292,7 @@ public interface DinkPluginConfig extends Config {
         keyName = "sendDiscordUser",
         name = "Send Discord Profile",
         description = "Whether to send your discord user information to the webhook server via metadata",
-        position = 1010,
+        position = 1011,
         section = advancedSection
     )
     default boolean sendDiscordUser() {
@@ -289,7 +303,7 @@ public interface DinkPluginConfig extends Config {
         keyName = "sendClanName",
         name = "Send Clan Name",
         description = "Whether to send your clan information to the webhook server via metadata",
-        position = 1011,
+        position = 1012,
         section = advancedSection
     )
     default boolean sendClanName() {
@@ -300,7 +314,7 @@ public interface DinkPluginConfig extends Config {
         keyName = "sendGroupIronClanName",
         name = "Send GIM Clan Name",
         description = "Whether to send your group ironman clan information to the webhook server via metadata",
-        position = 1012,
+        position = 1013,
         section = advancedSection
     )
     default boolean sendGroupIronClanName() {
diff --git a/src/main/java/dinkplugin/SettingsManager.java b/src/main/java/dinkplugin/SettingsManager.java
index 138df58..e187d76 100644
--- a/src/main/java/dinkplugin/SettingsManager.java
+++ b/src/main/java/dinkplugin/SettingsManager.java
@@ -3,6 +3,7 @@ package dinkplugin;
 import com.google.common.collect.ImmutableSet;
 import com.google.gson.Gson;
 import com.google.gson.reflect.TypeToken;
+import dinkplugin.domain.FilterMode;
 import dinkplugin.notifiers.CollectionNotifier;
 import dinkplugin.notifiers.CombatTaskNotifier;
 import dinkplugin.notifiers.KillCountNotifier;
@@ -69,9 +70,9 @@ public class SettingsManager {
     private Collection<String> webhookConfigKeys;
 
     /**
-     * User-specified RSNs that should not trigger webhook notifications.
+     * User-specified RSNs that should or should not (depending on filter mode) trigger webhook notifications.
      */
-    private final Collection<String> ignoredNames = new HashSet<>();
+    private final Collection<String> filteredNames = new HashSet<>();
 
     private final AtomicBoolean justLoggedIn = new AtomicBoolean();
 
@@ -83,19 +84,21 @@ public class SettingsManager {
     private final ConfigManager configManager;
 
     /**
-     * Check whether a username is not on the configured ignore list.
+     * Check whether a username complies with the configured RSN filter list.
      *
      * @param name the local player's name
      * @return whether notifications for this player should be sent
      */
     @Synchronized
     public boolean isNamePermitted(String name) {
-        return name != null && !ignoredNames.contains(name.toLowerCase());
+        if (name == null) return false;
+        if (filteredNames.isEmpty()) return true;
+        return (config.nameFilterMode() == FilterMode.ALLOW) == filteredNames.contains(name.toLowerCase());
     }
 
     @VisibleForTesting
     public void init() {
-        setIgnoredNames(config.ignoredNames());
+        setFilteredNames(config.filteredNames());
         configManager.getConfigDescriptor(config).getItems().forEach(item -> {
             String key = item.key();
             configValueTypes.put(key, item.getType());
@@ -158,7 +161,7 @@ public class SettingsManager {
         String value = event.getNewValue();
 
         if ("ignoredNames".equals(key)) {
-            setIgnoredNames(value);
+            setFilteredNames(value);
             return;
         }
 
@@ -273,12 +276,12 @@ public class SettingsManager {
     }
 
     @Synchronized
-    private void setIgnoredNames(String configValue) {
-        ignoredNames.clear();
+    private void setFilteredNames(String configValue) {
+        filteredNames.clear();
         readDelimited(configValue)
             .map(String::toLowerCase)
-            .forEach(ignoredNames::add);
-        log.debug("Updated RSN Deny List to: {}", ignoredNames);
+            .forEach(filteredNames::add);
+        log.debug("Updated RSN Filter List to: {}", filteredNames);
 
         // clear any outdated notifier state
         plugin.resetNotifiers();
diff --git a/src/main/java/dinkplugin/domain/FilterMode.java b/src/main/java/dinkplugin/domain/FilterMode.java
new file mode 100644
index 0000000..a2da704
--- /dev/null
+++ b/src/main/java/dinkplugin/domain/FilterMode.java
@@ -0,0 +1,16 @@
+package dinkplugin.domain;
+
+import lombok.RequiredArgsConstructor;
+
+@RequiredArgsConstructor
+public enum FilterMode {
+    ALLOW("Exclusively Allow"),
+    DENY("Selectively Deny");
+
+    private final String displayName;
+
+    @Override
+    public String toString() {
+        return this.displayName;
+    }
+}


Test patch:
diff --git a/src/test/java/dinkplugin/notifiers/MockedNotifierTest.java b/src/test/java/dinkplugin/notifiers/MockedNotifierTest.java
index 324ffe8..4664753 100644
--- a/src/test/java/dinkplugin/notifiers/MockedNotifierTest.java
+++ b/src/test/java/dinkplugin/notifiers/MockedNotifierTest.java
@@ -7,6 +7,7 @@ import dinkplugin.DinkPluginConfig;
 import dinkplugin.MockedTestBase;
 import dinkplugin.SettingsManager;
 import dinkplugin.domain.AccountType;
+import dinkplugin.domain.FilterMode;
 import dinkplugin.domain.PlayerLookupService;
 import dinkplugin.message.DiscordMessageHandler;
 import dinkplugin.message.templating.Template;
@@ -129,6 +130,7 @@ abstract class MockedNotifierTest extends MockedTestBase {
         when(config.embedFooterIcon()).thenReturn("https://github.com/pajlads/DinkPlugin/raw/master/icon.png");
         when(config.playerLookupService()).thenReturn(PlayerLookupService.OSRS_HISCORE);
         when(config.threadNameTemplate()).thenReturn("[%TYPE%] %MESSAGE%");
+        when(config.nameFilterMode()).thenReturn(FilterMode.DENY);
     }
 
     protected void mockItem(int id, int price, String name) {
diff --git a/src/test/java/dinkplugin/notifiers/PetNotifierTest.java b/src/test/java/dinkplugin/notifiers/PetNotifierTest.java
index 0e8b3f2..726d76b 100644
--- a/src/test/java/dinkplugin/notifiers/PetNotifierTest.java
+++ b/src/test/java/dinkplugin/notifiers/PetNotifierTest.java
@@ -1,6 +1,7 @@
 package dinkplugin.notifiers;
 
 import com.google.inject.testing.fieldbinder.Bind;
+import dinkplugin.domain.FilterMode;
 import dinkplugin.message.NotificationBody;
 import dinkplugin.message.NotificationType;
 import dinkplugin.notifiers.data.PetNotificationData;
@@ -312,7 +313,7 @@ class PetNotifierTest extends MockedNotifierTest {
     @Test
     void testNotifyIrrelevantNameIgnore() {
         // ignore notifications for an irrelevant player
-        when(config.ignoredNames()).thenReturn("xqc");
+        when(config.filteredNames()).thenReturn("xqc");
         settingsManager.init();
 
         // send fake message
@@ -331,10 +332,47 @@ class PetNotifierTest extends MockedNotifierTest {
         );
     }
 
+    @Test
+    void testNotifyNameAllowList() {
+        // only allow notifications for "dank dank"
+        when(config.nameFilterMode()).thenReturn(FilterMode.ALLOW);
+        when(config.filteredNames()).thenReturn(PLAYER_NAME);
+        settingsManager.init();
+
+        // send fake message
+        notifier.onChatMessage("You feel something weird sneaking into your backpack.");
+        IntStream.rangeClosed(0, MAX_TICKS_WAIT).forEach(i -> notifier.onTick());
+
+        // verify handled
+        verify(messageHandler).createMessage(
+            PRIMARY_WEBHOOK_URL,
+            false,
+            NotificationBody.builder()
+                .extra(new PetNotificationData(null, null, false))
+                .text(buildTemplate(PLAYER_NAME + " got a pet"))
+                .type(NotificationType.PET)
+                .build());
+    }
+
+    @Test
+    void testIgnoreNameAllowList() {
+        // only allow notifications for a different player
+        when(config.nameFilterMode()).thenReturn(FilterMode.ALLOW);
+        when(config.filteredNames()).thenReturn("xqc");
+        settingsManager.init();
+
+        // send fake message
+        notifier.onChatMessage("You feel something weird sneaking into your backpack.");
+        IntStream.rangeClosed(0, MAX_TICKS_WAIT).forEach(i -> notifier.onTick());
+
+        // ensure no notification occurred
+        verify(messageHandler, never()).createMessage(any(), anyBoolean(), any());
+    }
+
     @Test
     void testIgnoredPlayerName() {
         // ignore notifications for our player name
-        when(config.ignoredNames()).thenReturn(PLAYER_NAME);
+        when(config.filteredNames()).thenReturn(PLAYER_NAME);
         settingsManager.init();
 
         // send fake message
diff --git a/src/test/java/dinkplugin/notifiers/SpeedrunNotifierTest.java b/src/test/java/dinkplugin/notifiers/SpeedrunNotifierTest.java
index b32c286..015f112 100644
--- a/src/test/java/dinkplugin/notifiers/SpeedrunNotifierTest.java
+++ b/src/test/java/dinkplugin/notifiers/SpeedrunNotifierTest.java
@@ -114,7 +114,7 @@ class SpeedrunNotifierTest extends MockedNotifierTest {
     @Test
     void testNotifyDenyList() {
         // configure irrelevant deny list
-        when(config.ignoredNames()).thenReturn("pajlads");
+        when(config.filteredNames()).thenReturn("pajlads");
         settingsManager.init();
 
         // mock widget
@@ -141,7 +141,7 @@ class SpeedrunNotifierTest extends MockedNotifierTest {
     @Test
     void testIgnoreDenyList() {
         // configure deny list
-        when(config.ignoredNames()).thenReturn(PLAYER_NAME);
+        when(config.filteredNames()).thenReturn(PLAYER_NAME);
         settingsManager.init();
 
         // mock widget