Skip to content

Update index.js#119

Closed
gabrielsroka wants to merge 2 commits intopostmanlabs:developfrom
gabrielsroka:patch-3
Closed

Update index.js#119
gabrielsroka wants to merge 2 commits intopostmanlabs:developfrom
gabrielsroka:patch-3

Conversation

@gabrielsroka
Copy link
Contributor

Invoke-RestMethod converts the response from JSON to a PowerShell native object.

From https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-restmethod

If the request returns JSON strings, Invoke-RestMethod returns a PSObject that represents the strings

why are we calling ConvertTo-Json to convert it back to JSON? it's more useful to leave it as is. if you want just the raw text, use Invoke-WebRequest instead

`Invoke-RestMethod` converts the response from JSON to a PowerShell native object.

From https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-restmethod
> If the request returns JSON strings, `Invoke-RestMethod` returns a PSObject that represents the strings

why are we calling `ConvertTo-Json` to convert it back to JSON? it's more useful to leave it as is. if you want just the raw text, use `Invoke-WebRequest` instead
@abhijitkane
Copy link
Member

@shreys7 Can you take a look at this?

@umeshp7
Copy link
Contributor

umeshp7 commented Nov 25, 2019

@shreys7 any updates on this?

@shreys7
Copy link
Member

shreys7 commented Nov 26, 2019

@shreys7 any updates on this?

We can remove the ConvertTo-Json if we wish. As initially it was added to the snippet for tests, now that we have only unit tests and the concept of footer snippet while testing, we can remove this. @umeshp7

@shreys7
Copy link
Member

shreys7 commented Nov 26, 2019

@gabrielsroka Thanks for the fix. Although can you just delete the | ConvertTo-Json part. As we still want to have the printing of native PSObject code to be included in the generated snippet. Also, see if you can update the unit tests accordingly.

@gabrielsroka
Copy link
Contributor Author

@shreys7
i added back the rest of the code and just removed the ConvertTo-Json part.

i'm not sure where the unit test is (and how to update it...). any clues?

@shreys7
Copy link
Member

shreys7 commented Dec 18, 2019

@gabrielsroka There are unit tests for checking the snippet generation for PowerShell here:
https://github.com/postmanlabs/postman-code-generators/blob/develop/codegens/powershell-restmethod/test/unit/convert.test.js

It tests for the snippet generated against some hard coded strings. Here, you need to remove the ConvertTo-JSON part to correctly test the snippets.

@gabrielsroka
Copy link
Contributor Author

see #145

(i couldn't figure out how to add the new file to this PR)

@umeshp7
Copy link
Contributor

umeshp7 commented Dec 27, 2019

Closing, as new PR created.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants