-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: fetch and parse full gmail message #5160
Changes from 3 commits
dc83466
c2ec260
77aec94
d4831b0
7a4446d
bc30639
5632a66
daa91ca
fb34418
40d2a6b
e492f7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,6 @@ | ||
export type GmailMessageParsedResponse = { | ||
id: string; | ||
threadId: string; | ||
labelIds: string[]; | ||
snippet: string; | ||
sizeEstimate: number; | ||
raw: string; | ||
historyId: string; | ||
internalDate: string; | ||
import { gmail_v1 } from 'googleapis'; | ||
|
||
export type GmailMessageParsedResponse = gmail_v1.Schema$Message & { | ||
error?: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a big fan of having errors mixed with the message. Also, it seems that the new type is having all fields as optionals. Can we infer a type from gmail_v1.Schema$Message with all fields we need required? Do we need to have errors here or can we add it as an additional parameter of functions when needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure, it was there before, I assume perhaps because of the way how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aren't you adding the errors to this type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I tried to have a look at this once again and even though im still not completely able to understand what We can of course make the type nicer by making it a literal like
I mean, this is the best type we can get, or is it not? It comes straight from Google that clearly says some properties might are optional. When testing, those we need were always defined, but isnt it better to be safe than sorry? Im doing a runtime assertion using:
This would throw early (message wouldnt be parsed) if some of them are actually missing. Safer approach actually rather than letting it throw further down the line with "cant read property X of undefined" or saving invalid data into the DB. Let me know what you think about these two topics :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer the assert pattern that you are using I've just remove the errors from your type and it looks good |
||
code: number; | ||
message: string; | ||
|
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.
@rostaklein why are we removing these participants?