Skip to content

impersonate subuser#243

Merged
thinkingserious merged 3 commits intosendgrid:masterfrom
denwwer:impersonate-subuser
Oct 30, 2018
Merged

impersonate subuser#243
thinkingserious merged 3 commits intosendgrid:masterfrom
denwwer:impersonate-subuser

Conversation

@denwwer
Copy link
Copy Markdown
Contributor

@denwwer denwwer commented Nov 2, 2017 •
edited
Loading

Fixes #239

Checklist

  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the [Contribution Guide] and my PR follows them.
  • I updated my branch with the master branch.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation about the functionality in the appropriate .md file
  • I have added in line documentation to the code I modified

Short description of what this PR does:

  • HTTP header "On-Behalf-Of" for API request
  • updated tests
  • updated docs

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Nov 2, 2017
@SendGridDX
Copy link
Copy Markdown

SendGridDX commented Nov 2, 2017 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@thinkingserious
Copy link
Copy Markdown
Contributor

thinkingserious commented Nov 9, 2017

Thank you @denwwer,

I apologize for the delayed response. We had over 1,000 PRs submitted in October and we are currently working on this oldest to newest. We appreciate your patience.

With Best Regards,

Elmer

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Nov 11, 2017
dmowcomber
dmowcomber reviewed Jan 24, 2018
View reviewed changes
Comment thread sendgrid_test.go Outdated
func TestNewSendClientSubuser(t *testing.T) {
client := NewSendClientSubuser("API_KEY", "subuserUsername")

if client.Headers["On-Behalf-Of"] != "subuserUsername" {
Copy link
Copy Markdown
Contributor

@dmowcomber dmowcomber Jan 24, 2018

Choose a reason for hiding this comment

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

Could you also test that other expected headers are set?

Copy link
Copy Markdown
Contributor Author

@denwwer denwwer Jan 24, 2018

Choose a reason for hiding this comment

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

hi, first thanks that review process is started and yes I will add

@denwwer denwwer force-pushed the impersonate-subuser branch from 92d332e to 86729df Compare January 24, 2018 10:40
@denwwer
Copy link
Copy Markdown
Contributor Author

denwwer commented Jan 24, 2018

@dmowcomber failure from master after rebase:
--- FAIL: TestLicenseYear (0.00s)
sendgrid_test.go:130: License date range is not correct, it should be: Copyright (c) 2013-2018 SendGrid, Inc.
--- FAIL: TestRepoFiles (0.00s)
sendgrid_test.go:143: Repo file does not exist: .codeclimate.yml

@dmowcomber
Copy link
Copy Markdown
Contributor

dmowcomber commented Jan 24, 2018 •
edited
Loading

@denwwer sorry about that. I've created a PR to fix those tests #248

@dmowcomber
Copy link
Copy Markdown
Contributor

dmowcomber commented Jan 25, 2018

@denwwer #248 has been merged into master and tests are passing again. Could you pull master?

@denwwer denwwer force-pushed the impersonate-subuser branch from 86729df to 39a6c8b Compare January 25, 2018 16:25
@denwwer
Copy link
Copy Markdown
Contributor Author

denwwer commented Jan 25, 2018

@dmowcomber rebased, but test on CI for go1.8-1.9 is failed (same for #248), locally all passed

@dmowcomber
Copy link
Copy Markdown
Contributor

dmowcomber commented Jan 25, 2018

@denwwer that error was just fixed in another PR. Im approving your PR. Thanks for the contribution!

@dmowcomber
Copy link
Copy Markdown
Contributor

dmowcomber commented Jan 25, 2018

LGTM

@thinkingserious
Copy link
Copy Markdown
Contributor

thinkingserious commented Jan 26, 2018

Awesome! I have this on our backlog for merge now.

@thinkingserious thinkingserious added difficulty: medium fix is medium in difficulty type: twilio enhancement feature request on Twilio's roadmap status: ready for deploy code ready to be released in next deploy and removed status: code review request requesting a community code review or review from Twilio labels Mar 6, 2018
@sbfaulkner
Copy link
Copy Markdown

sbfaulkner commented Jun 7, 2018

do we have an update on merging this PR?

@thinkingserious
Copy link
Copy Markdown
Contributor

thinkingserious commented Jun 11, 2018

No update yet @sbfaulkner. I'm sorry I don't have a better answer at the moment. I have added your comment as a vote to it's priority on our backlog. Thanks!

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 23, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@b2166a6). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #243   +/-   ##
=========================================
  Coverage          ?   71.71%           
=========================================
  Files             ?        3           
  Lines             ?      456           
  Branches          ?        0           
=========================================
  Hits              ?      327           
  Misses            ?      125           
  Partials          ?        4
Impacted Files Coverage Δ
sendgrid.go 87.3% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2166a6...a24a53f. Read the comment docs.

@thinkingserious thinkingserious added status: code review request requesting a community code review or review from Twilio and removed status: ready for deploy code ready to be released in next deploy labels Oct 23, 2018
@robertacosta robertacosta added status: ready for deploy code ready to be released in next deploy and removed status: code review request requesting a community code review or review from Twilio labels Oct 30, 2018
@thinkingserious thinkingserious merged commit bd7db2d into sendgrid:master Oct 30, 2018
@thinkingserious
Copy link
Copy Markdown
Contributor

thinkingserious commented Oct 30, 2018

Hello @denwwer,

Thanks again for the PR!

We want to show our appreciation by sending you some swag. Could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX

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

Reviewers

@dmowcomber dmowcomber dmowcomber left review comments

@robertacosta robertacosta robertacosta approved these changes

Assignees

No one assigned

Labels

difficulty: medium fix is medium in difficulty status: ready for deploy code ready to be released in next deploy type: twilio enhancement feature request on Twilio's roadmap

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add the ability to impersonate a subuser

6 participants

@denwwer @SendGridDX @thinkingserious @dmowcomber @sbfaulkner @robertacosta