Fix email notification not sending

AtlassianPS / JiraPS

PowerShell module to interact with Atlassian JIRA

https://AtlassianPS.org/module/JiraPS

MIT License

322 stars 131 forks source link

Fix email notification not sending #384

Closed johnheusinger closed 4 years ago

johnheusinger commented 4 years ago

Description

Changed 'notify' to 'notification' in the request body to align with API documentation

Motivation and Context

Closes issue #382

Types of changes

[X] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

[X] My code follows the code style of this project.
[ ] I have added Pester Tests that describe what my changes should do.
[ ] I have updated the documentation accordingly.

skuzzles commented 4 years ago

Commenting from my phone here. But you must also change the notify param to String and value "true"

FROM [Boolean] $Notify = $true

TO [String] $Notify = "true"

johnheusinger commented 4 years ago

@skuzzles, can you explain why it should be a string?

I agree that there might be a better way to do this, particularly with @replicaJunction's idea to identify default behavior and use a switch parameter to change it. But, adjusting the data type would be a functionality change, potentially breaking existing automation. The fix I submitted has been tested to resolve the issue and keeps this in the 'bug fix' category.

lipkau commented 4 years ago

@skuzzles it should definitely not be a string, as the user can't type anything he wants in there. if a string true is needed, the following would be a way to implement it:

if ($Notify) {"true"} else {"false"}

but not to allow the user to type any string

@johnheusinger Have you tested this manually? The tests in the module can't test if an email was sent or not

johnheusinger commented 4 years ago

@lipkau Yup, tested it manually

skuzzles commented 4 years ago

https://docs.atlassian.com/software/jira/docs/api/REST/8.5.0/?_ga=2.157337087.49623261.1571848884-351338371.1565634928#api/2/user-createUser

Atlassian documentation states the notification property is a string. "true" is default and "false" is accepted alternative. The reason @johnheusinger suggestion works is because it is defaulting to true, since the property is now successfully changed from notify to notification. If you've tested it manually, try testing it with -Notify false to see if you do not receive a notification.

lipkau commented 4 years ago

@johnheusinger : can you please test it with -Notify:$false?

johnheusinger commented 4 years ago

@lipkau I tested the following cases:

Without -Notify param - Sends email -Notify:$true - Sends email -Notify:$false - Does not send email