microsoftgraph / msgraph-typescript-typings

Microsoft Graph TypeScript Type Definitions
https://graph.microsoft.com
Other
117 stars 33 forks source link

Bug: type of `FileAttachment.contentBytes` should be `string` instead of `number` #23

Open abdulkareemnalband opened 6 years ago

abdulkareemnalband commented 6 years ago

Bug

Type of FileAttachment.contentBytes should be string instead of number

in https://github.com/microsoftgraph/msgraph-typescript-typings/blob/33025e10483a74584e3aef03cdee9742047102a4/microsoft-graph.d.ts#L1920

AB#7041

muthurathinam commented 6 years ago

@abdulkareemnalband Thank you for reporting. will look into this and get back to you. 883193

darrelmiller commented 6 years ago

@muthurathinam This is declared as <Property Name="contentBytes" Type="Edm.Binary"/>. What do we normally use for mapping to Binary type? Can we use a byte array?

bamncan commented 4 years ago

Verifying that sending this property in as a base64 encoded string is what is accepted by the API.

let attachment: any = {
  '@odata.type': '#microsoft.graph.fileAttachment',
  contentBytes: Buffer.from('attachment').toString('base64'),
  contentType: 'text/plain',
  name: 'attachment.txt',
  size: 10
}

or

contentBytes: 'YXR0YWNobWVudA==',

Either will be accepted by the API when adding the attachment.

RobvH commented 2 years ago

This issue is still present and I've submitted a PR to correct the mistake. In the PR I link to the documentation and describe that it contains a contradiction that caused this error in the first place. In short, the type is not Edm.Binary in transmission, as the description accurately states, it is the "The base64-encoded contents of the file." Base64 encoding, by definition generates a string.

The impact of this bug is forcing test authors to cast through unknown to allow Typescript to ignore the contradiction between the incorrect type and actual API responses used as test fixtures; nullifying much of the value of the type in the first place.

Thanks for all your hard work and efforts on this and I hope this PR helps! ❤️

RobvH commented 2 years ago

I also raised this issue with the Docs project here: https://github.com/microsoftgraph/microsoft-graph-docs-contrib/issues/1972

RobvH commented 2 years ago

The fix was made upstream and the corrected types are in PR microsoftgraph/microsoft-graph-docs#328

❤️ Thanks to everyone who helped with this!