[9.x] username parameter in from method should be nullable

[mho22]

Based on issue #1815, username parameter in from method should be nullable in DiscordMessage.php on line 13 :

src/Notifications/Channels/Discord/DiscordMessage.php

 protected string $username = 'Laravel Backup';
 
 ...
 public function from(string $username, string $avatarUrl = null): self
 {
         $this->username = $username;
...

[RVxLab]

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.

[mho22]

@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.

[RVxLab]

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.

[mho22]

@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.

[freekmurze]

Fixed by #1634