-
Notifications
You must be signed in to change notification settings - Fork 36
Add SendMail capability #23
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
base: master
Are you sure you want to change the base?
Add SendMail capability #23
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @DavidArchibald,
thank you very much for your pull request and the work please review the comments I have added to the code lines, update it and then I'll merge the request. I guess you did test all of the functions, right?
@@ -1,3 +1,3 @@ | |||
module github.com/open-networks/go-msgraph | |||
module github.com/mypricehealth/go-msgraph |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change from a2b6df6 to module github.com/open-networks/go-msgraph
// See https://docs.microsoft.com/en-us/graph/api/message-reply | ||
type MessageReply struct { | ||
Comment string `json:"comment,omitempty"` // A comment to add. | ||
Message Message `json:"message,omitempty"` // Properties to be updated when repling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, plz replying
ReceivedDateTime time.Time `json:"receivedDateTime,omitempty"` | ||
ReplyTo []Recipient `json:"replyTo,omitempty"` | ||
Sender *Recipient `json:"sender,omitempty"` | ||
SentDateTime time.Time `json:"sentDateTime,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does SentDateTime
work and is correctly encoded upon loading? Did you test to get a message or only to send it?
// SendMail sends a message. | ||
// | ||
// See https://docs.microsoft.com/en-us/graph/api/user-sendmail | ||
func (g *GraphClient) SendMail(userID string, message Message, saveToSentItems bool) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, I saw that in the previous commit 9905cba you had the SendMail function in the User
struct. Please move it there again, and write a wrapper function here, something like:
func (g *GraphClient) SendMail(userID string, message Message, saveToSentItems bool) error {
user := User{ ID: userID, graphClient: g}
return user.SendMail(message, saveToSentItems)
}
Please also add a hint to the function comment / description that this is just a wrapper referencing to the User
struct
// | ||
// See https://docs.microsoft.com/en-us/graph/api/user-sendmail | ||
func (g *GraphClient) SendMail(userID string, message Message, saveToSentItems bool) error { | ||
if g == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually impossible here, because otherwise the function could not be called. However, if you move it to the User
struct, it makes sense again if you check for user.graphClient == nil
Adds the ability to send an email on the behalf of a user.
Some stylistic choices here were made in the intent to preserve the style of the source code. For instance a single file for a single struct.
Please note: I intend for this to be a contribution to be a path along to a finished version but likely not satisfactory for a full implementation. I hope it can help some but it's only one part of the mail capability.