Skip to content

fix(voice-channel): improve channel deletion and state handling logic #212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1pride wants to merge 1 commit into from
Closed

fix(voice-channel): improve channel deletion and state handling logic #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ LARAVEL_CLOUDFLARE_ENABLED=false

BROADCAST_DRIVER=log
CACHE_DRIVER=file
CACHE_STORE=redis
FILESYSTEM_DISK=local
QUEUE_CONNECTION=sync
SESSION_DRIVER=file
Expand Down
94 changes: 83 additions & 11 deletions app-modules/bot-discord/src/Actions/VoiceChannel/DeleteVoiceChannelAction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,76 @@

use Discord\Discord;
use He4rt\BotDiscord\DTO\VoiceChannelDTO;
use stdClass;

final class DeleteVoiceChannelAction
{
public function execute(Discord $discord): void
{
$channels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys', []);
foreach ($channels as $index => $channel) {
/** @var VoiceChannelDTO $channel */
if ($channel->isEmpty() && $channel->isLongTermEmpty()) {
$this->delete($channel->guildId, $channel->channelId, $index, $discord);

$validChannels = [];
$hasInvalid = false;

foreach ($channels as $channel) {
$dto = $this->normalizeChannel($channel);

if (!$dto instanceof VoiceChannelDTO) {
$hasInvalid = true;

continue;
}

$validChannels[] = $dto;

if ($dto->isEmpty() && $dto->isLongTermEmpty()) {
$this->delete($dto->guildId, $dto->channelId, $discord);
}
}

if ($hasInvalid) {
cache()->tags(['voice_channels'])->put('active_voice_channels_keys', $this->dtosToArrays($validChannels));
}
}
Comment on lines 13 to +39
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Cache write can resurrect just-deleted channels.

When $hasInvalid is true and at least one channel passes the isEmpty() && isLongTermEmpty() check:

  1. delete() (line 32) reads the cache, filters out that channel, and writes back the filtered list.
  2. After the loop, line 37 unconditionally writes $validChannels — which was populated before the delete and still contains the deleted channel — overwriting step 1.

So channels that should have been removed will reappear in active_voice_channels_keys. The fix is to perform a single, post-loop write with the final desired state, and drop the cache mutation inside delete().

🐛 Proposed fix (single cache write, delete() only handles Discord) 
     public function execute(Discord $discord): void
     {
         $channels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys', []);

-        $validChannels = [];
-        $hasInvalid = false;
+        $remainingChannels = [];
+        $mutated = false;

         foreach ($channels as $channel) {
             $dto = $this->normalizeChannel($channel);

             if (!$dto instanceof VoiceChannelDTO) {
-                $hasInvalid = true;
-
+                $mutated = true;
                 continue;
             }

-            $validChannels[] = $dto;
-
             if ($dto->isEmpty() && $dto->isLongTermEmpty()) {
                 $this->delete($dto->guildId, $dto->channelId, $discord);
+                $mutated = true;
+                continue;
             }
+
+            $remainingChannels[] = $dto;
         }

-        if ($hasInvalid) {
-            cache()->tags(['voice_channels'])->put('active_voice_channels_keys', $this->dtosToArrays($validChannels));
+        if ($mutated) {
+            cache()->tags(['voice_channels'])->put('active_voice_channels_keys', $this->dtosToArrays($remainingChannels));
         }
     }

…and have delete() only remove the Discord channel (drop its own cache read/write).

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app-modules/bot-discord/src/Actions/VoiceChannel/DeleteVoiceChannelAction.php`
around lines 13 - 39, The loop in execute() can re-add channels deleted by
delete() because validChannels is populated before calling delete() and delete()
also mutates the cache; fix by removing cache read/write from
DeleteVoiceChannelAction::delete() (delete() should only remove the Discord
channel) and ensure execute() does a single cache write at the end using
cache()->tags(['voice_channels'])->put('active_voice_channels_keys',
$this->dtosToArrays($validChannels));; to avoid resurrecting just-deleted
channels either (A) move the $validChannels[] = $dto; so it happens only when
you do NOT call delete(), or (B) if you prefer current order, have delete()
return a boolean and skip adding to $validChannels when delete() removed
it—apply this change around normalizeChannel, VoiceChannelDTO checks, delete(),
and dtosToArrays usage so only one final cache write occurs.


/**
* @param array<VoiceChannelDTO> $dtos
* @return array<array<string, mixed>>
*/
private function dtosToArrays(array $dtos): array
{
return array_map(fn (VoiceChannelDTO $dto) => [
'guildId' => $dto->guildId,
'channelId' => $dto->channelId,
'ownerId' => $dto->ownerId,
'usersCount' => $dto->usersCount,
'users' => $dto->users,
'lastJoinedAt' => $dto->lastJoinedAt?->toIso8601String(),
], $dtos);
}

private function normalizeChannel(mixed $channel): ?VoiceChannelDTO
{
if ($channel instanceof VoiceChannelDTO) {
return $channel;
}

$data = null;

if (is_array($channel)) {
$data = $channel;
} elseif ($channel instanceof stdClass) {
$data = (array) $channel;
}

if ($data !== null && isset($data['guildId'], $data['channelId'], $data['ownerId'])) {
return VoiceChannelDTO::make($data);
}

return null;
}

private function delete(string $guildId, string $channelId, int $arrayIndex, Discord $discord): void
private function delete(string $guildId, string $channelId, Discord $discord): void
{
$guild = $discord->guilds->get('id', $guildId);

Expand All @@ -30,14 +85,31 @@ private function delete(string $guildId, string $channelId, int $arrayIndex, Dis

$channels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys', []);

if (isset($channels[$arrayIndex])) {
$filtered = array_values(array_filter($channels, function ($channel) use ($channelId): bool {
$dto = $this->normalizeChannel($channel);

unset($channels[$arrayIndex]);
return !$dto instanceof VoiceChannelDTO || $dto->channelId !== $channelId;
}));

$channels = array_filter($channels, fn (VoiceChannelDTO $channel) => $channel->channelId !== $channelId);
$channels = array_values($channels);
$filteredArrays = array_map(function ($channel): ?array {
$dto = $this->normalizeChannel($channel);

cache()->tags(['voice_channels'])->put('active_voice_channels_keys', $channels);
}
if (!$dto instanceof VoiceChannelDTO) {
return null;
}

return [
'guildId' => $dto->guildId,
'channelId' => $dto->channelId,
'ownerId' => $dto->ownerId,
'usersCount' => $dto->usersCount,
'users' => $dto->users,
'lastJoinedAt' => $dto->lastJoinedAt?->toIso8601String(),
];
}, $filtered);
$filteredArrays = array_filter($filteredArrays);
$filteredArrays = array_values($filteredArrays);

cache()->tags(['voice_channels'])->put('active_voice_channels_keys', $filteredArrays);
}
}
44 changes: 43 additions & 1 deletion app-modules/bot-discord/src/Actions/VoiceChannel/HandleStateChannelAction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,21 @@

namespace He4rt\BotDiscord\Actions\VoiceChannel;

use He4rt\BotDiscord\DTO\VoiceChannelDTO;

final class HandleStateChannelAction
{
public function execute(int|string $userId, ?string $channelId): void
{
$activeChannels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys', []);
$activeChannels = $this->loadChannels();
$oldChannelId = $this->getUserLastChannel($userId);

if ($this->isLeavingVoice($channelId, $oldChannelId)) {
resolve(LeftChannelAction::class)->execute(
activeChannels: $activeChannels,
user: $userId
);
$this->saveChannels($activeChannels);

$this->clearUserLastChannel($userId);

Expand All @@ -33,6 +36,7 @@ public function execute(int|string $userId, ?string $channelId): void
activeChannels: $activeChannels,
user: $userId
);
$this->saveChannels($activeChannels);

$this->setUserLastChannel($userId, $channelId);

Expand All @@ -45,6 +49,7 @@ public function execute(int|string $userId, ?string $channelId): void
activeChannels: $activeChannels,
user: $userId
);
$this->saveChannels($activeChannels);
$this->setUserLastChannel($userId, $channelId);

return;
Expand Down Expand Up @@ -94,4 +99,41 @@ private function isUpdatingInSameChannel(?string $newChannelId, ?string $oldChan
{
return !is_null($newChannelId) && $oldChannelId === $newChannelId;
}

/**
* @return array<VoiceChannelDTO>
*/
private function loadChannels(): array
{
$channels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys', []);

return array_map(function ($channel): ?VoiceChannelDTO {
if ($channel instanceof VoiceChannelDTO) {
return $channel;
}

if (is_array($channel) && isset($channel['guildId'], $channel['channelId'], $channel['ownerId'])) {
return VoiceChannelDTO::make($channel);
}

return null;
}, $channels);
}
Comment on lines +103 to +121
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

loadChannels() can return null entries and skips stdClass legacy data.

Two robustness gaps:

  1. array_map here returns null for any malformed entry, but the result is typed array<VoiceChannelDTO> and is fed straight into LeftChannelAction/JoiningChannelAction and saveChannels() — whose arrow function type-hints VoiceChannelDTO and will throw a TypeError on the first null.
  2. DeleteVoiceChannelAction::normalizeChannel() (lines 57–76) handles stdClass to recover from the exact serialization issue this PR fixes, but loadChannels() only handles array. A pre-existing cache populated by the old code path (still present until it expires) would silently drop those entries here.

Both can be addressed by reusing the same normalization and filtering nulls.

🛡️ Proposed fix 
     /**
      * `@return` array<VoiceChannelDTO>
      */
     private function loadChannels(): array
     {
         $channels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys', []);

-        return array_map(function ($channel): ?VoiceChannelDTO {
+        $mapped = array_map(function ($channel): ?VoiceChannelDTO {
             if ($channel instanceof VoiceChannelDTO) {
                 return $channel;
             }

-            if (is_array($channel) && isset($channel['guildId'], $channel['channelId'], $channel['ownerId'])) {
-                return VoiceChannelDTO::make($channel);
+            $data = match (true) {
+                is_array($channel) => $channel,
+                $channel instanceof \stdClass => (array) $channel,
+                default => null,
+            };
+
+            if ($data !== null && isset($data['guildId'], $data['channelId'], $data['ownerId'])) {
+                return VoiceChannelDTO::make($data);
             }

             return null;
         }, $channels);
+
+        return array_values(array_filter($mapped));
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* @return array<VoiceChannelDTO>
*/
private function loadChannels(): array
{
$channels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys', []);
return array_map(function ($channel): ?VoiceChannelDTO {
if ($channel instanceof VoiceChannelDTO) {
return $channel;
}
if (is_array($channel) && isset($channel['guildId'], $channel['channelId'], $channel['ownerId'])) {
return VoiceChannelDTO::make($channel);
}
return null;
}, $channels);
}
/**
* `@return` array<VoiceChannelDTO>
*/
private function loadChannels(): array
{
$channels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys', []);
$mapped = array_map(function ($channel): ?VoiceChannelDTO {
if ($channel instanceof VoiceChannelDTO) {
return $channel;
}
$data = match (true) {
is_array($channel) => $channel,
$channel instanceof \stdClass => (array) $channel,
default => null,
};
if ($data !== null && isset($data['guildId'], $data['channelId'], $data['ownerId'])) {
return VoiceChannelDTO::make($data);
}
return null;
}, $channels);
return array_values(array_filter($mapped));
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app-modules/bot-discord/src/Actions/VoiceChannel/HandleStateChannelAction.php`
around lines 103 - 121, loadChannels() currently maps entries to VoiceChannelDTO
but returns nulls for malformed entries and ignores legacy stdClass data,
causing TypeErrors downstream in LeftChannelAction/JoiningChannelAction and
saveChannels(); update loadChannels() to reuse
DeleteVoiceChannelAction::normalizeChannel() (or its normalization logic) to
accept arrays and stdClass, convert/normalize each cached entry, filter out any
null/invalid results, and return only concrete VoiceChannelDTO instances so
callers like LeftChannelAction, JoiningChannelAction, and saveChannels() always
receive an array<VoiceChannelDTO>.


/**
* @param array<VoiceChannelDTO> $channels
*/
private function saveChannels(array $channels): void
{
$arrays = array_map(fn (VoiceChannelDTO $dto) => [
'guildId' => $dto->guildId,
'channelId' => $dto->channelId,
'ownerId' => $dto->ownerId,
'usersCount' => $dto->usersCount,
'users' => $dto->users,
'lastJoinedAt' => $dto->lastJoinedAt?->toIso8601String(),
], $channels);

cache()->tags(['voice_channels'])->put('active_voice_channels_keys', $arrays);
}
}
12 changes: 11 additions & 1 deletion app-modules/bot-discord/src/DTO/VoiceChannelDTO.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace He4rt\BotDiscord\DTO;

use Carbon\CarbonInterface;
use Illuminate\Support\Facades\Date;

final class VoiceChannelDTO
{
Expand All @@ -23,13 +24,22 @@ public function __construct(
*/
public static function make(array $data): self
{
$lastJoinedAt = null;
if (filled($data['lastJoinedAt'])) {
if ($data['lastJoinedAt'] instanceof CarbonInterface) {
$lastJoinedAt = $data['lastJoinedAt'];
} elseif (is_string($data['lastJoinedAt'])) {
$lastJoinedAt = Date::parse($data['lastJoinedAt']);
}
}
Comment on lines +27 to +34
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Guard against undefined lastJoinedAt key (pipeline failure).

CI is failing with Undefined array key "lastJoinedAt" because filled($data['lastJoinedAt']) reads the key directly. Several callers (e.g., the test beforeEach that builds entries without lastJoinedAt, lines 14–27 of HandleStateChannelActionTest.php) omit it. Use null‑coalescing on the access.

🐛 Proposed fix 
-        $lastJoinedAt = null;
-        if (filled($data['lastJoinedAt'])) {
-            if ($data['lastJoinedAt'] instanceof CarbonInterface) {
-                $lastJoinedAt = $data['lastJoinedAt'];
-            } elseif (is_string($data['lastJoinedAt'])) {
-                $lastJoinedAt = Date::parse($data['lastJoinedAt']);
-            }
-        }
+        $lastJoinedAt = null;
+        $rawLastJoinedAt = $data['lastJoinedAt'] ?? null;
+        if (filled($rawLastJoinedAt)) {
+            if ($rawLastJoinedAt instanceof CarbonInterface) {
+                $lastJoinedAt = $rawLastJoinedAt;
+            } elseif (is_string($rawLastJoinedAt)) {
+                $lastJoinedAt = Date::parse($rawLastJoinedAt);
+            }
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$lastJoinedAt = null;
if (filled($data['lastJoinedAt'])) {
if ($data['lastJoinedAt'] instanceof CarbonInterface) {
$lastJoinedAt = $data['lastJoinedAt'];
} elseif (is_string($data['lastJoinedAt'])) {
$lastJoinedAt = Date::parse($data['lastJoinedAt']);
}
}
$lastJoinedAt = null;
$rawLastJoinedAt = $data['lastJoinedAt'] ?? null;
if (filled($rawLastJoinedAt)) {
if ($rawLastJoinedAt instanceof CarbonInterface) {
$lastJoinedAt = $rawLastJoinedAt;
} elseif (is_string($rawLastJoinedAt)) {
$lastJoinedAt = Date::parse($rawLastJoinedAt);
}
}
🧰 Tools
🪛 GitHub Actions: Continuous Integration

[error] 28-28: Undefined array key "lastJoinedAt" in VoiceChannelDTO::make().

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app-modules/bot-discord/src/DTO/VoiceChannelDTO.php` around lines 27 - 34,
The code reads $data['lastJoinedAt'] directly which throws when the key is
missing; change the access to use a null-coalesced local value (e.g., $value =
$data['lastJoinedAt'] ?? null) and then call filled($value) and the subsequent
checks against CarbonInterface and is_string on that local $value before
assigning $lastJoinedAt and parsing with Date::parse; update the block around
$lastJoinedAt so it never indexes $data directly without the ?? guard.


return new self(
guildId: $data['guildId'],
channelId: $data['channelId'],
ownerId: $data['ownerId'],
usersCount: $data['usersCount'],
users: $data['users'],
lastJoinedAt: $data['lastJoinedAt'] ?? null,
lastJoinedAt: $lastJoinedAt,
);
}
Comment on lines 25 to 44
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Consider centralizing DTO ↔ array conversion here.

The reverse mapping (DTO → array) is now duplicated in three places: DynamicVoiceCommand::dtoToArray, HandleStateChannelAction::saveChannels, and DeleteVoiceChannelAction::dtosToArrays/delete. Adding a toArray(): array method on VoiceChannelDTO (paired with the existing make()) keeps the serialization contract in one place and prevents drift in field names/formats (e.g., lastJoinedAt?->toIso8601String()).

♻️ Proposed addition 
     public function isLongTermEmpty(): bool
     {
         return abs(now()->diffInSeconds($this->lastJoinedAt)) >= 20;
     }
+
+    /**
+     * `@return` array<string, mixed>
+     */
+    public function toArray(): array
+    {
+        return [
+            'guildId' => $this->guildId,
+            'channelId' => $this->channelId,
+            'ownerId' => $this->ownerId,
+            'usersCount' => $this->usersCount,
+            'users' => $this->users,
+            'lastJoinedAt' => $this->lastJoinedAt?->toIso8601String(),
+        ];
+    }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static function make(array $data): self
{
$lastJoinedAt = null;
if (filled($data['lastJoinedAt'])) {
if ($data['lastJoinedAt'] instanceof CarbonInterface) {
$lastJoinedAt = $data['lastJoinedAt'];
} elseif (is_string($data['lastJoinedAt'])) {
$lastJoinedAt = Date::parse($data['lastJoinedAt']);
}
}
return new self(
guildId: $data['guildId'],
channelId: $data['channelId'],
ownerId: $data['ownerId'],
usersCount: $data['usersCount'],
users: $data['users'],
lastJoinedAt: $data['lastJoinedAt'] ?? null,
lastJoinedAt: $lastJoinedAt,
);
}
public static function make(array $data): self
{
$lastJoinedAt = null;
if (filled($data['lastJoinedAt'])) {
if ($data['lastJoinedAt'] instanceof CarbonInterface) {
$lastJoinedAt = $data['lastJoinedAt'];
} elseif (is_string($data['lastJoinedAt'])) {
$lastJoinedAt = Date::parse($data['lastJoinedAt']);
}
}
return new self(
guildId: $data['guildId'],
channelId: $data['channelId'],
ownerId: $data['ownerId'],
usersCount: $data['usersCount'],
users: $data['users'],
lastJoinedAt: $lastJoinedAt,
);
}
/**
* `@return` array<string, mixed>
*/
public function toArray(): array
{
return [
'guildId' => $this->guildId,
'channelId' => $this->channelId,
'ownerId' => $this->ownerId,
'usersCount' => $this->usersCount,
'users' => $this->users,
'lastJoinedAt' => $this->lastJoinedAt?->toIso8601String(),
];
}
🧰 Tools
🪛 GitHub Actions: Continuous Integration

[error] 28-28: Undefined array key "lastJoinedAt" in VoiceChannelDTO::make().

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app-modules/bot-discord/src/DTO/VoiceChannelDTO.php` around lines 25 - 44,
Add a toArray(): array method to VoiceChannelDTO to mirror make(), returning the
same keys (guildId, channelId, ownerId, usersCount, users, lastJoinedAt) and
serializing lastJoinedAt as an ISO8601 string or null (e.g.,
$this->lastJoinedAt?->toIso8601String()). Keep the field names and formats
identical to make() expectations, then replace the duplicated serialization
logic in DynamicVoiceCommand::dtoToArray,
HandleStateChannelAction::saveChannels, and
DeleteVoiceChannelAction::dtosToArrays/delete to call VoiceChannelDTO->toArray()
instead.


Expand Down
14 changes: 13 additions & 1 deletion app-modules/bot-discord/src/SlashCommands/DynamicVoiceCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
'users' => [],
'lastJoinedAt' => now(),
]);
$channels[] = $channelDto;
$channels[] = $this->dtoToArray($channelDto);

cache()->tags(['voice_channels'])->put('active_voice_channels_keys', $channels);

Expand Down Expand Up @@ -149,4 +149,16 @@
->content(sprintf('Sala Criada com sucesso !! <#%s>', $channel->id))
->reply($interaction, true);
}

private function dtoToArray(VoiceChannelDTO $dto): array

Check failure on line 153 in app-modules/bot-discord/src/SlashCommands/DynamicVoiceCommand.php

View workflow job for this annotation

GitHub Actions / Perform Phpstan Check / Run 

Method He4rt\BotDiscord\SlashCommands\DynamicVoiceCommand::dtoToArray() return type has no value type specified in iterable type array.
{
return [
'guildId' => $dto->guildId,
'channelId' => $dto->channelId,
'ownerId' => $dto->ownerId,
'usersCount' => $dto->usersCount,
'users' => $dto->users,
'lastJoinedAt' => $dto->lastJoinedAt?->toIso8601String(),
];
}
Comment on lines +153 to +163
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add @return value type and reuse a DTO helper.

PHPStan is failing on this method because the array shape is unspecified. Beyond that, this duplicates the same conversion present in HandleStateChannelAction::saveChannels() and DeleteVoiceChannelAction::dtosToArrays(). Adding VoiceChannelDTO::toArray() (see the DTO file comment) lets you delete this helper entirely.

🛠️ Minimal fix for PHPStan 
-    private function dtoToArray(VoiceChannelDTO $dto): array
+    /**
+     * `@return` array<string, mixed>
+     */
+    private function dtoToArray(VoiceChannelDTO $dto): array
     {
         return [
             'guildId' => $dto->guildId,
             'channelId' => $dto->channelId,
             'ownerId' => $dto->ownerId,
             'usersCount' => $dto->usersCount,
             'users' => $dto->users,
             'lastJoinedAt' => $dto->lastJoinedAt?->toIso8601String(),
         ];
     }

Preferred: replace the call at line 84 with $channels[] = $channelDto->toArray(); and remove dtoToArray() once VoiceChannelDTO::toArray() exists.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private function dtoToArray(VoiceChannelDTO $dto): array
{
return [
'guildId' => $dto->guildId,
'channelId' => $dto->channelId,
'ownerId' => $dto->ownerId,
'usersCount' => $dto->usersCount,
'users' => $dto->users,
'lastJoinedAt' => $dto->lastJoinedAt?->toIso8601String(),
];
}
/**
* `@return` array<string, mixed>
*/
private function dtoToArray(VoiceChannelDTO $dto): array
{
return [
'guildId' => $dto->guildId,
'channelId' => $dto->channelId,
'ownerId' => $dto->ownerId,
'usersCount' => $dto->usersCount,
'users' => $dto->users,
'lastJoinedAt' => $dto->lastJoinedAt?->toIso8601String(),
];
}
🧰 Tools
🪛 GitHub Check: Perform Phpstan Check / Run

[failure] 153-153:
Method He4rt\BotDiscord\SlashCommands\DynamicVoiceCommand::dtoToArray() return type has no value type specified in iterable type array.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app-modules/bot-discord/src/SlashCommands/DynamicVoiceCommand.php` around
lines 153 - 163, The dtoToArray method returns an untyped array shape causing
PHPStan failures and duplicates logic present in
HandleStateChannelAction::saveChannels and
DeleteVoiceChannelAction::dtosToArrays; replace usages of dtoToArray (e.g. where
channels are built at line ~84) with VoiceChannelDTO::toArray() and remove the
dtoToArray private method, or add a toArray() implementation on VoiceChannelDTO
and update callers to use VoiceChannelDTO::toArray() so array shapes are
centralized and PHPStan types are satisfied.

}
41 changes: 26 additions & 15 deletions app-modules/bot-discord/tests/Feature/Actions/VoiceChannel/HandleStateChannelActionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,24 @@
'users' => [],
]);

$activeChannels[] = $this->firstChannel;
$activeChannels[] = $this->secondChannel;
$activeChannels = [
[
'guildId' => 'guild-123-id',
'channelId' => $this->firstChannelId,
'ownerId' => $this->userId,
'usersCount' => 0,
'users' => [],
'lastJoinedAt' => null,
],
[
'guildId' => 'guild-123-id',
'channelId' => $this->secondChannelId,
'ownerId' => $this->userId,
'usersCount' => 0,
'users' => [],
'lastJoinedAt' => null,
],
];
cache()->tags(['voice_channels'])->put('active_voice_channels_keys', $activeChannels);

});
Expand All @@ -37,10 +53,8 @@

$cachedChannels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys');

/** @var VoiceChannelDTO $activeChannelFromCache */
$activeChannelFromCache = $cachedChannels[0];
$activeChannelFromCache = VoiceChannelDTO::make($cachedChannels[0]);
expect($cachedChannels)->not->toBeEmpty()
->and($activeChannelFromCache)->toBeInstanceOf(VoiceChannelDTO::class)
->and($activeChannelFromCache->guildId)->toBe($this->firstChannel->guildId)
->and($activeChannelFromCache->channelId)->toBe($this->firstChannel->channelId)
->and($activeChannelFromCache->usersCount)->toBe(1)
Expand All @@ -57,9 +71,8 @@

$cachedChannels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys');

$firstChannelFromCache = $cachedChannels[0];
$firstChannelFromCache = VoiceChannelDTO::make($cachedChannels[0]);
expect($cachedChannels)->not->toBeEmpty()
->and($firstChannelFromCache)->toBeInstanceOf(VoiceChannelDTO::class)
->and($firstChannelFromCache->guildId)->toBe($this->firstChannel->guildId)
->and($firstChannelFromCache->channelId)->toBe($this->firstChannel->channelId)
->and($firstChannelFromCache->usersCount)->toBe(1)
Expand All @@ -71,9 +84,8 @@
expect($userLastChannel)->not->toBeEmpty()
->and($userLastChannel)->toBe($this->firstChannel->channelId);

$secondChannelFromCache = $cachedChannels[1];
$secondChannelFromCache = VoiceChannelDTO::make($cachedChannels[1]);
expect($cachedChannels)->not->toBeEmpty()
->and($secondChannelFromCache)->toBeInstanceOf(VoiceChannelDTO::class)
->and($secondChannelFromCache->channelId)->toBe($this->secondChannel->channelId)
->and($secondChannelFromCache->usersCount)->toBe(0)
->and($secondChannelFromCache->lastJoinedAt)->toBeNull()
Expand All @@ -83,8 +95,9 @@

$action->execute($this->userId, $this->secondChannelId);

$cachedChannels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys');
$firstChannelFromCache = VoiceChannelDTO::make($cachedChannels[0]);
expect($cachedChannels)->not->toBeEmpty()
->and($firstChannelFromCache)->toBeInstanceOf(VoiceChannelDTO::class)
->and($firstChannelFromCache->guildId)->toBe($this->firstChannel->guildId)
->and($firstChannelFromCache->channelId)->toBe($this->firstChannel->channelId)
->and($firstChannelFromCache->usersCount)->toBe(0)
Expand All @@ -94,9 +107,9 @@
// user is now at second channel

$userLastChannel = cache()->tags(['voice_tracking'])->get('user_last_channel_'.$this->userId);
$secondChannelFromCache = VoiceChannelDTO::make($cachedChannels[1]);
expect($userLastChannel)->toBe($this->secondChannel->channelId)
->and($cachedChannels)->not->toBeEmpty()
->and($secondChannelFromCache)->toBeInstanceOf(VoiceChannelDTO::class)
->and($secondChannelFromCache->channelId)->toBe($this->secondChannel->channelId)
->and($secondChannelFromCache->usersCount)->toBe(1)
->and($secondChannelFromCache->lastJoinedAt)->not->toBeNull()
Expand All @@ -111,9 +124,8 @@

$cachedChannels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys');

$firstChannelFromCache = $cachedChannels[0];
$firstChannelFromCache = VoiceChannelDTO::make($cachedChannels[0]);
expect($cachedChannels)->not->toBeEmpty()
->and($firstChannelFromCache)->toBeInstanceOf(VoiceChannelDTO::class)
->and($firstChannelFromCache->guildId)->toBe($this->firstChannel->guildId)
->and($firstChannelFromCache->channelId)->toBe($this->firstChannel->channelId)
->and($firstChannelFromCache->usersCount)->toBe(1)
Expand All @@ -140,10 +152,9 @@
expect($userLastChannel)->toBeNull();

$cachedChannels = cache()->tags(['voice_channels'])->get('active_voice_channels_keys');
$firstChannelFromCache = $cachedChannels[0];
$firstChannelFromCache = VoiceChannelDTO::make($cachedChannels[0]);

expect($cachedChannels)->not->toBeEmpty()
->and($firstChannelFromCache)->toBeInstanceOf(VoiceChannelDTO::class)
->and($firstChannelFromCache->guildId)->toBe($this->firstChannel->guildId)
->and($firstChannelFromCache->channelId)->toBe($this->firstChannel->channelId)
->and($firstChannelFromCache->usersCount)->toBe(0)
Expand Down
Loading