Closed mho22 closed 1 month ago
This won't actually fix the issue. The error in the original issue states that the username must be between 1 and 80 characters:
[2024-07-09 14:48:58] local.INFO: {"username": ["Must be between 1 and 80 in length.", "Username cannot be \"\""]}
Checking for null doesn't solve that.
I suggest that instead of changing the signature to allow nulls, don't add the username
key to the array in the toArray
method.
As per the Discord documentation, the username
field is optional. So doing something like
public function toArray()
{
$data = [
'avatar_url' => $this->avatarUrl,
'embeds' => [
[
'title' => $this->title,
'url' => $this->url,
'type' => 'rich',
'description' => $this->description,
'fields' => $this->fields,
'color' => hexdec((string) $this->color),
'footer' => [
'text' => $this->footer ?? '',
],
'timestamp' => $this->timestamp ?? now(),
],
],
];
if (!empty($this->username)) {
$data['username'] = $this->username;
}
return $data;
}
Will address this point properly. When the username
field is not passed, the default name in the webhook is used instead.
The same applies #1818, which I believe could just be backported.
@RVxLab You're right, this doesn't fix the issue the way you want. I initially suggested this PR because if you don't specify username
[ or avatar_url
] in the config file, an exception occurs. After reading your issue, I thought this PR might address your problem by not requiring username and using Laravel Backup
as the default.
I've considered this further. I was torn between making Laravel Backup
the default value for username
and making username
optional in toArray()
.
Let's wait for input from a maintainer before modifying the PRs with your suggestion.
You're right, this doesn't fix the issue the way you want.
This isn't about what I want or don't want. Changing the username from string
to ?string
and then checking for null
doesn't solve the issue that was being reported.
If the username
property is an empty string and passed to Discord's API like that, the call throws a validation error, which contradicts the config option's comment.
Based on this and Discord's documentation stating that it's an optional field (analogous to nullable|between:1,80
in Laravel's validation. Making the default value null
and only checking for that doesn't solve the issue.
@RVxLab I found this PR #1634 that addresses the same issue as mine. It suggests replacing an empty string with Laravel Backup, ensuring that a username is always returned. This is likely the behavior they want to maintain. We should wait for a response from @freekmurze.
The wrong comment in the config file should probably be modified.
Fixed by #1634
Based on issue #1815,
username
parameter infrom
method should be nullable inDiscordMessage.php
on line13
:src/Notifications/Channels/Discord/DiscordMessage.php