Add option for moderator approval for declarations and actions#1954
Draft
Add option for moderator approval for declarations and actions#1954
Conversation
Append localized approval system keys (approve, deny, pending actions, notifications) to four language files with proper translations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add MfApprovalRequest data model with ID, type, and reason - Add MfApprovalRequestService with in-memory ConcurrentHashMap storage - Add /f approve, /f deny, and /f pendingactions commands - Modify /f declarewar, /f ally, /f vassalize to check config and create pending approval requests when required - Add config options: warDeclarationRequiresApproval, allyDeclarationRequiresApproval, vassalizeDeclarationRequiresApproval - Add mf.approve permission node (default: op) - Add language keys to all 5 language files (en_US, en_GB, fr_FR, de_DE, pt_BR) - Update CONFIG.md and COMMANDS.md documentation Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com> Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/a413e2e4-ef4a-4d92-b66e-837051e6ded7
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com> Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/a413e2e4-ef4a-4d92-b66e-837051e6ded7
Copilot stopped work on behalf of
dmccoystephenson due to an error
March 22, 2026 20:40
Member
|
@copilot continue work |
Copilot stopped work on behalf of
dmccoystephenson due to an error
March 22, 2026 20:43
Member
|
@copilot continue work |
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com> Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/26f346bc-04f2-4d17-9b83-0e148b4d2597
Contributor
Author
All remaining tasks are now complete. In commit Summary of what's implemented:
Note: The CI build shows "action_required" because it needs first-run approval from repo maintainers. The build may also fail on dependency resolution since Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
[WIP] Add option for moderator approval for declarations and actions
Add option for moderator approval for declarations and actions
Mar 23, 2026
There was a problem hiding this comment.
Pull request overview
Adds an optional moderator approval workflow for faction declarations (war, alliance, vassalization) so that actions can be queued and only executed once a moderator approves/denies them.
Changes:
- Introduces an in-memory approval-request model + service (
approval/) and wires it into the plugin’s service container. - Adds moderator commands to approve/deny/list pending actions, and updates declare/ally/vassalize commands to optionally queue requests with an optional
-- <reason>. - Updates config/docs/plugin metadata and adds unit tests for the approval request service.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/kotlin/com/dansplugins/factionsystem/approval/MfApprovalRequest.kt | New data model for a pending approval request. |
| src/main/kotlin/com/dansplugins/factionsystem/approval/MfApprovalRequestId.kt | New request id wrapper + generator. |
| src/main/kotlin/com/dansplugins/factionsystem/approval/MfApprovalRequestService.kt | In-memory storage/service for pending requests. |
| src/main/kotlin/com/dansplugins/factionsystem/approval/MfApprovalRequestType.kt | Enum for request types (WAR/ALLY/VASSALIZE). |
| src/main/kotlin/com/dansplugins/factionsystem/MedievalFactions.kt | Instantiates and registers the approval request service. |
| src/main/kotlin/com/dansplugins/factionsystem/service/Services.kt | Extends the service container to include approvalRequestService. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/MfFactionCommand.kt | Registers new subcommands/aliases for approve/deny/pendingactions. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/approve/MfFactionApproveCommand.kt | New /f approve implementation to execute queued actions. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/deny/MfFactionDenyCommand.kt | New /f deny implementation to reject queued actions. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/pendingactions/MfFactionPendingActionsCommand.kt | New /f pendingactions command to list queued actions. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/declarewar/MfFactionDeclareWarCommand.kt | Adds optional approval gating + reason parsing for war declarations. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/ally/MfFactionAllyCommand.kt | Adds optional approval gating + reason parsing for alliance requests. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/vassalize/MfFactionVassalizeCommand.kt | Adds optional approval gating + reason parsing for vassalization requests. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/help/MfFactionHelpCommand.kt | Extends /f help output to include new moderator commands. |
| src/main/resources/config.yml | Adds 3 config toggles to require approval per action type. |
| src/main/resources/plugin.yml | Adds usage/help lines + mf.approve permission node. |
| src/main/resources/lang/lang_en_US.properties | Adds new language keys for approval workflow. |
| src/main/resources/lang/lang_en_GB.properties | Adds new language keys for approval workflow. |
| src/main/resources/lang/lang_de_DE.properties | Adds new language keys for approval workflow. |
| src/main/resources/lang/lang_fr_FR.properties | Adds new language keys for approval workflow. |
| src/main/resources/lang/lang_pt_BR.properties | Adds new language keys for approval workflow. |
| CONFIG.md | Documents the new config options. |
| COMMANDS.md | Documents new moderator commands and the -- <reason> syntax. |
| src/test/kotlin/com/dansplugins/factionsystem/approval/MfApprovalRequestServiceTest.kt | Adds unit tests for the approval request service. |
Comment on lines
+11
to
+12
| pendingRequests[request.id] = request | ||
| return request |
There was a problem hiding this comment.
addRequest/hasPendingRequest don’t provide an atomic way to prevent duplicate pending requests for the same (factionId, targetId, type). Since callers do a check-then-add, concurrent executions can still insert duplicates (and addRequest itself always overwrites by id). Consider keying the map by a composite key (factionId+targetId+type), or adding an addRequestIfAbsent(...)/getOrCreate(...) method that performs the check and insert atomically.
Suggested change
| pendingRequests[request.id] = request | |
| return request | |
| synchronized(this) { | |
| val existing = pendingRequests.values.firstOrNull { | |
| it.factionId == request.factionId && | |
| it.targetId == request.targetId && | |
| it.type == request.type | |
| } | |
| if (existing != null) { | |
| return existing | |
| } | |
| pendingRequests[request.id] = request | |
| return request | |
| } |
Comment on lines
+25
to
+41
| if (requests.isEmpty()) { | ||
| sender.sendMessage("$GRAY${plugin.language["CommandFactionPendingActionsNone"]}") | ||
| return@Runnable | ||
| } | ||
| val factionService = plugin.services.factionService | ||
| sender.sendMessage("$AQUA${plugin.language["CommandFactionPendingActionsHeader"]}") | ||
| for (request in requests) { | ||
| val faction = factionService.getFaction(request.factionId) | ||
| val target = factionService.getFaction(request.targetId) | ||
| val factionName = faction?.name ?: "Unknown" | ||
| val targetName = target?.name ?: "Unknown" | ||
| val reasonText = if (request.reason != null) " - ${request.reason}" else "" | ||
| sender.sendMessage( | ||
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, request.type.name.lowercase(), factionName, targetName]}$GRAY$reasonText" | ||
| ) | ||
| } | ||
| sender.sendMessage("$GRAY${plugin.language["CommandFactionPendingActionsFooter"]}") |
There was a problem hiding this comment.
This async task calls sender.sendMessage(...) multiple times. On Paper/Spigot, sending messages to players (and other Bukkit API calls) is expected on the main server thread and can trigger async-catcher errors. Consider building the output asynchronously, then scheduling a synchronous runTask to send the messages.
Suggested change
| if (requests.isEmpty()) { | |
| sender.sendMessage("$GRAY${plugin.language["CommandFactionPendingActionsNone"]}") | |
| return@Runnable | |
| } | |
| val factionService = plugin.services.factionService | |
| sender.sendMessage("$AQUA${plugin.language["CommandFactionPendingActionsHeader"]}") | |
| for (request in requests) { | |
| val faction = factionService.getFaction(request.factionId) | |
| val target = factionService.getFaction(request.targetId) | |
| val factionName = faction?.name ?: "Unknown" | |
| val targetName = target?.name ?: "Unknown" | |
| val reasonText = if (request.reason != null) " - ${request.reason}" else "" | |
| sender.sendMessage( | |
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, request.type.name.lowercase(), factionName, targetName]}$GRAY$reasonText" | |
| ) | |
| } | |
| sender.sendMessage("$GRAY${plugin.language["CommandFactionPendingActionsFooter"]}") | |
| val messages = mutableListOf<String>() | |
| if (requests.isEmpty()) { | |
| messages.add("$GRAY${plugin.language["CommandFactionPendingActionsNone"]}") | |
| } else { | |
| val factionService = plugin.services.factionService | |
| messages.add("$AQUA${plugin.language["CommandFactionPendingActionsHeader"]}") | |
| for (request in requests) { | |
| val faction = factionService.getFaction(request.factionId) | |
| val target = factionService.getFaction(request.targetId) | |
| val factionName = faction?.name ?: "Unknown" | |
| val targetName = target?.name ?: "Unknown" | |
| val reasonText = if (request.reason != null) " - ${request.reason}" else "" | |
| messages.add( | |
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, request.type.name.lowercase(), factionName, targetName]}$GRAY$reasonText" | |
| ) | |
| } | |
| messages.add("$GRAY${plugin.language["CommandFactionPendingActionsFooter"]}") | |
| } | |
| plugin.server.scheduler.runTask( | |
| plugin, | |
| Runnable { | |
| for (message in messages) { | |
| sender.sendMessage(message) | |
| } | |
| } | |
| ) |
Comment on lines
+36
to
+45
| val factionName = faction?.name ?: "Unknown" | ||
| val targetName = target?.name ?: "Unknown" | ||
| approvalService.removeRequest(requestId) | ||
| sender.sendMessage("$GREEN${plugin.language["CommandFactionDenySuccess", factionName, targetName, request.type.name.lowercase()]}") | ||
| plugin.server.scheduler.runTask( | ||
| plugin, | ||
| Runnable { | ||
| faction?.sendMessage( | ||
| plugin.language["FactionApprovalDeniedNotificationTitle"], | ||
| plugin.language["FactionApprovalDeniedNotificationBody", request.type.name.lowercase(), targetName] |
There was a problem hiding this comment.
Avoid hard-coded "Unknown" here; there are existing language keys like UnknownFaction/UnknownPlayer. Also, request.type.name.lowercase() is not localized—map the request type to a language key before inserting it into messages.
Suggested change
| val factionName = faction?.name ?: "Unknown" | |
| val targetName = target?.name ?: "Unknown" | |
| approvalService.removeRequest(requestId) | |
| sender.sendMessage("$GREEN${plugin.language["CommandFactionDenySuccess", factionName, targetName, request.type.name.lowercase()]}") | |
| plugin.server.scheduler.runTask( | |
| plugin, | |
| Runnable { | |
| faction?.sendMessage( | |
| plugin.language["FactionApprovalDeniedNotificationTitle"], | |
| plugin.language["FactionApprovalDeniedNotificationBody", request.type.name.lowercase(), targetName] | |
| val unknownFactionName = plugin.language["UnknownFaction"] | |
| val factionName = faction?.name ?: unknownFactionName | |
| val targetName = target?.name ?: unknownFactionName | |
| val requestTypeKey = "ApprovalRequestType.${request.type.name}" | |
| val requestType = plugin.language[requestTypeKey] | |
| approvalService.removeRequest(requestId) | |
| sender.sendMessage("$GREEN${plugin.language["CommandFactionDenySuccess", factionName, targetName, requestType]}") | |
| plugin.server.scheduler.runTask( | |
| plugin, | |
| Runnable { | |
| faction?.sendMessage( | |
| plugin.language["FactionApprovalDeniedNotificationTitle"], | |
| plugin.language["FactionApprovalDeniedNotificationBody", requestType, targetName] |
Comment on lines
+38
to
+45
| approvalService.removeRequest(requestId) | ||
| sender.sendMessage("$GREEN${plugin.language["CommandFactionDenySuccess", factionName, targetName, request.type.name.lowercase()]}") | ||
| plugin.server.scheduler.runTask( | ||
| plugin, | ||
| Runnable { | ||
| faction?.sendMessage( | ||
| plugin.language["FactionApprovalDeniedNotificationTitle"], | ||
| plugin.language["FactionApprovalDeniedNotificationBody", request.type.name.lowercase(), targetName] |
There was a problem hiding this comment.
The denial notification uses request.type.name.lowercase() in the user-facing body, which won’t be localized. Prefer passing a localized request/action label (from the language files) instead of the enum name.
Suggested change
| approvalService.removeRequest(requestId) | |
| sender.sendMessage("$GREEN${plugin.language["CommandFactionDenySuccess", factionName, targetName, request.type.name.lowercase()]}") | |
| plugin.server.scheduler.runTask( | |
| plugin, | |
| Runnable { | |
| faction?.sendMessage( | |
| plugin.language["FactionApprovalDeniedNotificationTitle"], | |
| plugin.language["FactionApprovalDeniedNotificationBody", request.type.name.lowercase(), targetName] | |
| val requestTypeLabel = plugin.language["ApprovalRequestType.${request.type.name.lowercase()}"] | |
| approvalService.removeRequest(requestId) | |
| sender.sendMessage("$GREEN${plugin.language["CommandFactionDenySuccess", factionName, targetName, requestTypeLabel]}") | |
| plugin.server.scheduler.runTask( | |
| plugin, | |
| Runnable { | |
| faction?.sendMessage( | |
| plugin.language["FactionApprovalDeniedNotificationTitle"], | |
| plugin.language["FactionApprovalDeniedNotificationBody", requestTypeLabel, targetName] |
| plugin.server.onlinePlayers | ||
| .filter { it.hasPermission("mf.approve") } | ||
| .forEach { moderator -> | ||
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, "war", target.name, request.id.value]}") |
There was a problem hiding this comment.
The action string passed into ApprovalRequestNotification is hard-coded ("war"). In non-English locales this will show up untranslated inside an otherwise localized message. Consider mapping the request type to a localized label before formatting the notification.
Suggested change
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, "war", target.name, request.id.value]}") | |
| val warActionLabel = plugin.language["ApprovalRequestTypeWar"] | |
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, warActionLabel, target.name, request.id.value]}") |
| plugin.server.onlinePlayers | ||
| .filter { it.hasPermission("mf.approve") } | ||
| .forEach { moderator -> | ||
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, "ally", target.name, request.id.value]}") |
There was a problem hiding this comment.
The action string passed into ApprovalRequestNotification is hard-coded ("ally"). In non-English locales this will show up untranslated inside an otherwise localized message. Consider mapping the request type to a localized label before formatting the notification.
Suggested change
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, "ally", target.name, request.id.value]}") | |
| val actionLabel = plugin.language["ApprovalRequestActionAlly"] | |
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, actionLabel, target.name, request.id.value]}") |
Comment on lines
+34
to
+38
| val factionName = faction?.name ?: "Unknown" | ||
| val targetName = target?.name ?: "Unknown" | ||
| val reasonText = if (request.reason != null) " - ${request.reason}" else "" | ||
| sender.sendMessage( | ||
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, request.type.name.lowercase(), factionName, targetName]}$GRAY$reasonText" |
There was a problem hiding this comment.
This command constructs user-facing fallback/formatting strings directly ("Unknown" and the " - " prefix for the reason). Please use existing language keys (e.g. UnknownFaction) and move the reason formatting into the lang string so the whole line is localizable.
Suggested change
| val factionName = faction?.name ?: "Unknown" | |
| val targetName = target?.name ?: "Unknown" | |
| val reasonText = if (request.reason != null) " - ${request.reason}" else "" | |
| sender.sendMessage( | |
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, request.type.name.lowercase(), factionName, targetName]}$GRAY$reasonText" | |
| val factionName = faction?.name ?: plugin.language["UnknownFaction"] | |
| val targetName = target?.name ?: plugin.language["UnknownFaction"] | |
| val reason = request.reason ?: "" | |
| sender.sendMessage( | |
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, request.type.name.lowercase(), factionName, targetName, reason]}$GRAY" |
Comment on lines
+37
to
+38
| sender.sendMessage( | ||
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, request.type.name.lowercase(), factionName, targetName]}$GRAY$reasonText" |
There was a problem hiding this comment.
request.type.name.lowercase() is not localized, so non-English locales will still see English enum values. Consider mapping MfApprovalRequestType to language keys (e.g. Ally/AtWar/Vassal or new approval-specific keys) before displaying it.
Suggested change
| sender.sendMessage( | |
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, request.type.name.lowercase(), factionName, targetName]}$GRAY$reasonText" | |
| val requestTypeLanguageKey = "ApprovalRequestType.${request.type.name}" | |
| val requestTypeLocalized = plugin.language[requestTypeLanguageKey] | |
| sender.sendMessage( | |
| "$YELLOW${plugin.language["CommandFactionPendingActionsEntry", request.id.value, requestTypeLocalized, factionName, targetName]}$GRAY$reasonText" |
Comment on lines
+112
to
+118
| faction.sendMessage( | ||
| plugin.language["FactionAllyRequestSentNotificationTitle", request.requesterId.toBukkitPlayer().name ?: "Unknown", target.name], | ||
| plugin.language["FactionAllyRequestSentNotificationBody", request.requesterId.toBukkitPlayer().name ?: "Unknown", target.name] | ||
| ) | ||
| target.sendMessage( | ||
| plugin.language["FactionAllyRequestReceivedNotificationTitle", request.requesterId.toBukkitPlayer().name ?: "Unknown", faction.name], | ||
| plugin.language["FactionAllyRequestReceivedNotificationBody", request.requesterId.toBukkitPlayer().name ?: "Unknown", faction.name] |
There was a problem hiding this comment.
The fallback player name "Unknown" is hard-coded and bypasses the language system. Prefer plugin.language["UnknownPlayer"] (and consider storing request.requesterId.toBukkitPlayer() / name once to avoid repeating the lookup).
Suggested change
| faction.sendMessage( | |
| plugin.language["FactionAllyRequestSentNotificationTitle", request.requesterId.toBukkitPlayer().name ?: "Unknown", target.name], | |
| plugin.language["FactionAllyRequestSentNotificationBody", request.requesterId.toBukkitPlayer().name ?: "Unknown", target.name] | |
| ) | |
| target.sendMessage( | |
| plugin.language["FactionAllyRequestReceivedNotificationTitle", request.requesterId.toBukkitPlayer().name ?: "Unknown", faction.name], | |
| plugin.language["FactionAllyRequestReceivedNotificationBody", request.requesterId.toBukkitPlayer().name ?: "Unknown", faction.name] | |
| val requesterName = request.requesterId.toBukkitPlayer().name ?: plugin.language["UnknownPlayer"] | |
| faction.sendMessage( | |
| plugin.language["FactionAllyRequestSentNotificationTitle", requesterName, target.name], | |
| plugin.language["FactionAllyRequestSentNotificationBody", requesterName, target.name] | |
| ) | |
| target.sendMessage( | |
| plugin.language["FactionAllyRequestReceivedNotificationTitle", requesterName, faction.name], | |
| plugin.language["FactionAllyRequestReceivedNotificationBody", requesterName, faction.name] |
Comment on lines
+113
to
+119
| plugin.server.scheduler.runTask( | ||
| plugin, | ||
| Runnable { | ||
| plugin.server.onlinePlayers | ||
| .filter { it.hasPermission("mf.approve") } | ||
| .forEach { moderator -> | ||
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, "vassalize", target.name, request.id.value]}") |
There was a problem hiding this comment.
The action string passed into ApprovalRequestNotification is hard-coded ("vassalize"). In non-English locales this will show up untranslated inside an otherwise localized message. Consider mapping the request type to a localized label before formatting the notification.
Suggested change
| plugin.server.scheduler.runTask( | |
| plugin, | |
| Runnable { | |
| plugin.server.onlinePlayers | |
| .filter { it.hasPermission("mf.approve") } | |
| .forEach { moderator -> | |
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, "vassalize", target.name, request.id.value]}") | |
| val actionLabel = plugin.language["ApprovalRequestActionVassalize"] | |
| plugin.server.scheduler.runTask( | |
| plugin, | |
| Runnable { | |
| plugin.server.onlinePlayers | |
| .filter { it.hasPermission("mf.approve") } | |
| .forEach { moderator -> | |
| moderator.sendMessage("${ChatColor.YELLOW}${plugin.language["ApprovalRequestNotification", faction.name, actionLabel, target.name, request.id.value]}") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a configurable moderator approval system for faction declarations (war, alliance, vassalization). When enabled via config, these actions require a moderator with the
mf.approvepermission to approve or deny them before they take effect. Players can optionally include a reason with their request.Changes Made
MfApprovalRequest,MfApprovalRequestId, andMfApprovalRequestTypeclasses in theapproval/packageMfApprovalRequestServiceusing in-memoryConcurrentHashMap, integrated intoServices.ktandMedievalFactions.kt/f approve [id]— Approves a pending action and executes it/f deny [id]— Denies a pending action and notifies the requesting faction/f pendingactions— Lists all pending actions with details/f declarewar,/f ally,/f vassalizenow check config and create pending approval requests when required, with optional reason support via-- reason textfactions.:warDeclarationRequiresApproval,allyDeclarationRequiresApproval,vassalizeDeclarationRequiresApproval(all defaultfalse)mf.approvepermission node (default: op, included inmf.admin)/f helpoutputOriginal prompt
📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.