pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
909 stars 317 forks source link

Log the entire API response for `teams app publish` #4010

Closed milanholemans closed 1 year ago

milanholemans commented 1 year ago

I noticed that the command teams app publish only logs the ID of the published app on success. https://github.com/pnp/cli-microsoft365/blob/da9cd2cc6424ee8d9f9e2d8fe8be8a84bd4661c5/src/m365/teams/commands/app/app-publish.ts#L75-L79

I'm questioning why this is done. The original response of the API looks like:

{
  "id": "e3e29acb-8c79-412b-b746-e6c39ff4cd22",
  "externalId": "b5561ec9-8cab-4aa3-8aa2-d8d7172e4311",
  "name": "Test App",
  "version": "1.0.0",
  "distributionMethod": "organization"
}
  1. Shouldn't we log the entire response instead of only the ID?
  2. We should also add a responseType: 'json' to the request options. Right now the response is not formatted as json but as a JSON string. Therefore the command cannot find res.id and won't log anything at all.
  3. Update the command output in the docs
milanholemans commented 1 year ago

@pnp/cli-for-microsoft-365-maintainers do you have an idea why we do this? Should this be updated?

garrytrinder commented 1 year ago

I don't see why not and makes sense to return all the information we have to avoid round trips to the API get the same information.

Adam-it commented 1 year ago

I also agree. My feeling is CLI should usually output what is coming from the API response and if possible extend it with additional data but rather not trim it.

milanholemans commented 1 year ago

Ok thx for the feedback, opening it up!

nicodecleyre commented 1 year ago

Ok thx for the feedback, opening it up!

Pick me!

milanholemans commented 1 year ago

All yours!

nicodecleyre commented 1 year ago

For completeness, it seems that the actual data returned from https://graph.microsoft.com/v1.0/appCatalogs/teamsApps is a little less then the one in the specs:

{ id: "e3e29acb-8c79-412b-b746-e6c39ff4cd22", externalId: "b5561ec9-8cab-4aa3-8aa2-d8d7172e4311", displayName: "Test App", distributionMethod: "organization" }

so no version parameter is returned and name is actual displayName

milanholemans commented 1 year ago

I blame the Graph docs 😁