Skip to content

Conversation

@JorianWoltjer
Copy link
Contributor

I was a bit confused when I noticed the discord_username parameter in the Discord provider-config.yml file was not actually used when sending the webhook message. It is saved in the Options struct here:

type Options struct {
	ID                      string `yaml:"id,omitempty"`
	DiscordWebHookURL       string `yaml:"discord_webhook_url,omitempty"`
	DiscordWebHookUsername  string `yaml:"discord_username,omitempty"`
	DiscordWebHookAvatarURL string `yaml:"discord_avatar,omitempty"`
	DiscordFormat           string `yaml:"discord_format,omitempty"`
}

But this DiscordWebHookUsername is not actually used further on in the code. This commit simply adds an &username= parameter to the shoutrrr discord URL that is created and makes sure to escape it to allow special characters in the username.

url := fmt.Sprintf("discord://%s@%s?splitlines=no&username=%s", webhookToken, webhookID, 
			url.QueryEscape(pr.DiscordWebHookUsername))
		
sendErr := shoutrrr.Send(url, msg)

Note: This is my first time writing anything in go, but it's a pretty simple change, so it's probably fine

Example

image

cyicz123 and others added 3 commits November 3, 2022 23:42
change example `notify -version` to `notify -verbose`. It's a small typo.
Fix a typo mistake in README.md
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@JorianWoltjer
Copy link
Contributor Author

For completeness, we can also add the AvatarURL and other options from shoutrrr, but that would require updating some examples of the provider-config.yml file

@ehsandeep ehsandeep changed the base branch from master to dev February 26, 2023 10:02
Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

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

@ehsandeep
Copy link
Member

Thank you @JorianWoltjer for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants