Skip to content
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.

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

chore: Remove mail.send helpers with on-behalf-of header #436

Merged
merged 4 commits into from Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -6,3 +6,4 @@ coverage.txt
sendgrid.env
.vscode
prism*
**/.idea/**/*
20 changes: 10 additions & 10 deletions USAGE.md
Expand Up @@ -6000,20 +6000,20 @@ if err != nil {
<a name="on-behalf-of"></a>
# On-Behalf of Subuser

The on-behalf-of header allows you to make calls for a particular subuser through the parent account; this can be useful for automating bulk updates or administering a subuser without changing authentication in your code.
## With Mail Helper Class
The `on-behalf-of` header allows you to make calls for a particular subuser through the parent account. This can be
useful for automating bulk updates or administering a subuser without changing authentication in your code.

```go

client := sendgrid.NewSendClientSubuser(os.Getenv("SENDGRID_API_KEY"), "SUBUSER_USERNAME")

```

## Without Mail Helper Class
## Note
The v3/mail/send endpoint does not support the `on-behalf-of` header. ([Source](
https://docs.sendgrid.com/api-reference/how-to-use-the-sendgrid-v3-api/on-behalf-of-subuser
))

```go

request := sendgrid.GetRequestSubuser(os.Getenv("SENDGRID_API_KEY"), "/v3/mail/send", "https://api.sendgrid.com", "SUBUSER_USERNAME")
request := sendgrid.GetRequestSubuser(
os.Getenv("SENDGRID_API_KEY"), "/v3/tracking_settings/subscription",
"https://api.sendgrid.com", "SUBUSER_USERNAME",
)

```

Expand Down
14 changes: 6 additions & 8 deletions sendgrid.go
@@ -1,6 +1,8 @@
package sendgrid

import (
"strings"

"github.com/sendgrid/rest"
)

Expand All @@ -21,6 +23,10 @@ func GetRequest(key, endpoint, host string) rest.Request {
// GetRequestSubuser like GetRequest but with On-Behalf of Subuser
// @return [Request] a default request object
func GetRequestSubuser(key, endpoint, host, subuser string) rest.Request {
if strings.Contains(endpoint, "v3/mail/send") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does using the on-behalf-of with this endpoint throw any error at all? I'm not too sure I like hardcoding this into the logic here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not throw an error, it just silently "fails" by ignoring the header and putting the email on the subuser who owns the API key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(My primary motivation for this change was so that there would be something to test to guard against someone adding this functionality back in)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am ok with getting rid of NewSendClientSubuser and updating the documentation since that's not really supported but I don't like adding this hardcoded panic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

// TODO: Breaking change: Return an error instead of panicking
panic("cannot use on-behalf-of for v3/mail/send")
}
return createSendGridRequest(sendGridOptions{key, endpoint, host, subuser})
}

Expand All @@ -47,11 +53,3 @@ func NewSendClient(key string) *Client {
request.Method = "POST"
return &Client{request}
}

// GetRequestSubuser like NewSendClient but with On-Behalf of Subuser
// @return [Client]
func NewSendClientSubuser(key, subuser string) *Client {
request := GetRequestSubuser(key, "/v3/mail/send", "", subuser)
request.Method = "POST"
return &Client{request}
}
19 changes: 16 additions & 3 deletions sendgrid_test.go
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/sendgrid/rest"
"github.com/sendgrid/sendgrid-go/helpers/mail"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestLicenseYear(t *testing.T) {
Expand Down Expand Up @@ -84,9 +85,21 @@ func TestGetRequestSubuser(t *testing.T) {
ShouldHaveHeaders(&request, t)
}

func TestNewSendClientSubuser(t *testing.T) {
client := NewSendClientSubuser("API_KEY", "subuserUsername")
ShouldHaveHeaders(&client.Request, t)
func TestGetRequestSubUser_ShouldRejectMailSend(t *testing.T) {
// https://docs.sendgrid.com/api-reference/how-to-use-the-sendgrid-v3-api/on-behalf-of-subuser
// > The on-behalf-of header does not work with the mail.send API.

assertPanicked := func() {
rec := recover()
err, ok := rec.(string)
require.True(t, ok, "Recover should have returned error")
assert.NotEqual(t, "", err, "Should have panicked for /v3/mail/send")
}
defer assertPanicked()

_ = GetRequestSubuser(
"any API key", "/v3/mail/send", "https://example.com", "subuserUsername",
)
}

func getRequest(endpoint string) rest.Request {
Expand Down