Fix view permission not preventing players from clicking an icon

plugin/src/main/java/me/filoghost/chestcommands/icon/IconPermission.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) filoghost and contributors
+ *
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+package me.filoghost.chestcommands.icon;
+
+import me.filoghost.commons.Strings;
+import org.bukkit.entity.Player;
+
+public class IconPermission {
+	
+	private final String permission;
+	private final boolean negated;
+
+	public IconPermission(String permission) {
+		if (permission != null) {
+			permission = permission.trim();
+		}
+		
+		if (Strings.isEmpty(permission)) {
+			this.permission = null;
+			negated = false;
+		} else {
+			if (permission.startsWith("-")) {
+				this.permission = permission.substring(1);
+				negated = true;
+			} else {
+				this.permission = permission;
+				negated = false;
+			}
+		}
+	}
+	
+	private boolean hasPermission(Player player) {
+		if (isEmpty()) {
+			return true;
+		}
+
+		if (negated) {
+			return !player.hasPermission(permission);
+		} else {
+			return player.hasPermission(permission);
+		}
+	}
+
+	public boolean isEmpty() {
+		return this.permission == null;
+	}
+
+	public static boolean hasPermission(Player player, IconPermission permission) {
+		return permission == null || permission.hasPermission(player);
+	}
+
+}

plugin/src/main/java/me/filoghost/chestcommands/icon/InternalConfigurableIcon.java

@@ -10,9 +10,9 @@ import me.filoghost.chestcommands.action.Action;
 import me.filoghost.chestcommands.action.OpenMenuAction;
 import me.filoghost.chestcommands.api.ClickResult;
 import me.filoghost.chestcommands.api.MenuInventory;
+import me.filoghost.chestcommands.config.Lang;
 import me.filoghost.chestcommands.icon.requirement.RequiredExpLevel;
 import me.filoghost.chestcommands.icon.requirement.RequiredMoney;
-import me.filoghost.chestcommands.icon.requirement.RequiredPermission;
 import me.filoghost.chestcommands.icon.requirement.Requirement;
 import me.filoghost.chestcommands.icon.requirement.item.RequiredItem;
 import me.filoghost.chestcommands.icon.requirement.item.RequiredItems;
@@ -27,14 +27,15 @@ import java.util.List;
 
 public class InternalConfigurableIcon extends BaseConfigurableIcon implements RefreshableIcon {
 
-	private RequiredPermission viewPermission;
+	private IconPermission viewPermission;
+	private IconPermission clickPermission;
+	private String noClickPermissionMessage;
 
-	private RequiredPermission clickPermission;
 	private RequiredMoney requiredMoney;
 	private RequiredExpLevel requiredExpLevel;
 	private RequiredItems requiredItems;
-	private ImmutableList<Action> clickActions;
 
+	private ImmutableList<Action> clickActions;
 	private ClickResult clickResult;
 
 	public InternalConfigurableIcon(Material material) {
@@ -44,7 +45,7 @@ public class InternalConfigurableIcon extends BaseConfigurableIcon implements Re
 	}
 
 	public boolean canViewIcon(Player player) {
-		return viewPermission == null || viewPermission.hasPermission(player);
+		return IconPermission.hasPermission(player, viewPermission);
 	}
 	
 	public boolean hasViewPermission() {
@@ -52,24 +53,15 @@ public class InternalConfigurableIcon extends BaseConfigurableIcon implements Re
 	}
 
 	public void setClickPermission(String permission) {
-		if (this.clickPermission == null) {
+		this.clickPermission = new IconPermission(permission);
-			this.clickPermission = new RequiredPermission();
-		}
-		this.clickPermission.setPermission(permission);
 	}
 	
-	public void setNoClickPermissionMessage(String clickNoPermissionMessage) {
+	public void setNoClickPermissionMessage(String noClickPermissionMessage) {
-		if (this.clickPermission == null) {
+		this.noClickPermissionMessage = noClickPermissionMessage;
-			this.clickPermission = new RequiredPermission();
-		}
-		this.clickPermission.setNoPermissionMessage(clickNoPermissionMessage);
 	}
 		
 	public void setViewPermission(String viewPermission) {
-		if (this.viewPermission == null) {
+		this.viewPermission = new IconPermission(viewPermission);
-			this.viewPermission = new RequiredPermission();
-		}
-		this.viewPermission.setPermission(viewPermission);
 	}
 
 	public void setRequiredMoney(double requiredMoney) {
@@ -123,8 +115,21 @@ public class InternalConfigurableIcon extends BaseConfigurableIcon implements Re
 
 	@Override
 	public ClickResult onClick(MenuInventory menuInventory, Player player) {
+		if (!IconPermission.hasPermission(player, viewPermission)) {
+			return ClickResult.KEEP_OPEN;
+		}
+
+		if (!IconPermission.hasPermission(player, clickPermission)) {
+			if (noClickPermissionMessage != null) {
+				player.sendMessage(noClickPermissionMessage);
+			} else {
+				player.sendMessage(Lang.default_no_icon_permission);
+			}
+			return clickResult;
+		}
+
 		// Check all the requirements
-		Requirement[] requirements = {clickPermission, requiredMoney, requiredExpLevel, requiredItems};
+		Requirement[] requirements = {requiredMoney, requiredExpLevel, requiredItems};
 		boolean hasAllRequirements = Requirement.hasAllCosts(player, requirements);
 		if (!hasAllRequirements) {
 			return clickResult;

plugin/src/main/java/me/filoghost/chestcommands/icon/requirement/RequiredPermission.java

@@ -1,78 +0,0 @@
-/*
- * Copyright (C) filoghost and contributors
- *
- * SPDX-License-Identifier: GPL-3.0-or-later
- */
-package me.filoghost.chestcommands.icon.requirement;
-
-import me.filoghost.chestcommands.config.Lang;
-import me.filoghost.commons.Strings;
-import org.bukkit.entity.Player;
-
-public class RequiredPermission implements Requirement {
-	
-	private String permission;
-	private boolean negated;
-	private String noPermissionMessage;
-
-	public void setPermission(String permission) {
-		if (permission != null) {
-			permission = permission.trim();
-		}
-		
-		if (Strings.isEmpty(permission)) {
-			this.permission = null;
-			negated = false;
-		} else {
-			if (permission.startsWith("-")) {
-				this.permission = permission.substring(1);
-				negated = true;
-			} else {
-				this.permission = permission;
-				negated = false;
-			}
-		}
-	}
-	
-	
-	public void setNoPermissionMessage(String noPermissionMessage) {
-		this.noPermissionMessage = noPermissionMessage;
-	}
-
-
-	@Override
-	public boolean hasCost(Player player) {
-		if (hasPermission(player)) {
-			return true;
-		} else {
-			if (noPermissionMessage != null) {
-				player.sendMessage(noPermissionMessage);
-			} else {
-				player.sendMessage(Lang.default_no_icon_permission);
-			}
-			return false;
-		}
-	}
-	
-	public boolean hasPermission(Player player) {
-		if (isEmpty()) {
-			return true;
-		}
-		
-		if (negated) {
-			return !player.hasPermission(permission);
-		} else {
-			return player.hasPermission(permission);
-		}
-	}
-	
-	@Override
-	public boolean takeCost(Player player) {
-		return true;
-	}
-
-	public boolean isEmpty() {
-		return this.permission == null;
-	}	
-
-}