Skip to content
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

Guid uri template path/query parameter values should be normalized to string #1113

Closed
baywet opened this issue Mar 19, 2024 · 6 comments · Fixed by #1226
Closed

Guid uri template path/query parameter values should be normalized to string #1113

baywet opened this issue Mar 19, 2024 · 6 comments · Fixed by #1226
Assignees
Labels
bug Something isn't working goodfirstissue Standard GitHub label used for easy to resolve issues targeting beginner contributors
Milestone

Comments

@baywet
Copy link
Member

baywet commented Mar 19, 2024

the std uri template library does not handle guids, and wont because it's not a standard API across languages and they don't want to bring on additional dependencies.
We should normalize the value to string (no curlies) before we pass it to the uri template engine.
Originally reported:
std-uritemplate/std-uritemplate#140
microsoft/kiota#4335

Example implementation https://github.com/microsoft/kiota-abstractions-dotnet/blob/5a93d126e9ce30ae332e65f30df7b941e7d7c265/src/RequestInformation.cs#L123

@illunix
Copy link
Contributor

illunix commented Mar 19, 2024

Honestly I can't find file in kiota typescript in witch I should do this changes

@illunix
Copy link
Contributor

illunix commented Mar 19, 2024

Can you help me @baywet ?

@illunix
Copy link
Contributor

illunix commented Mar 19, 2024

Take a look @baywet
#1116

@baywet
Copy link
Member Author

baywet commented Mar 19, 2024

If we look at the dotnet implementation we can see that the URI getter calls into an additional method that's missing in typescript and which is responsible for handling enum query/path parameters as other scalar but non trivial types (guid, etc...)

This is effectively what we need to replicate int typescript

You can then add unit tests here to prevent any future regression.

Thank you for starting the pull request already!

@illunix
Copy link
Contributor

illunix commented Mar 19, 2024

It seems hard for me and I'm not sure if I will be able to complete this but I'll try

@illunix
Copy link
Contributor

illunix commented Mar 19, 2024

Also I don't understand this part with replication @baywet

@andrueastman andrueastman added this to the Kiota v1.14 milestone Apr 12, 2024
@andrueastman andrueastman moved this from Todo 📃 to In Review 💭 in Kiota Apr 18, 2024
@baywet baywet modified the milestones: Kiota v1.14, Kiota v1.15 May 2, 2024
@andrueastman andrueastman modified the milestones: Kiota v1.15, Kiota v1.16 Jun 6, 2024
@fey101 fey101 added goodfirstissue Standard GitHub label used for easy to resolve issues targeting beginner contributors and removed Standard GitHub label labels Jun 6, 2024
@github-project-automation github-project-automation bot moved this from In Review 💭 to Done ✔️ in Kiota Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working goodfirstissue Standard GitHub label used for easy to resolve issues targeting beginner contributors
Projects
Archived in project
6 participants