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

CSharp - Path Parameters should keep their name and descriptions #2978

Closed
pillowfication opened this issue Jul 21, 2023 · 3 comments · Fixed by #3071
Closed

CSharp - Path Parameters should keep their name and descriptions #2978

pillowfication opened this issue Jul 21, 2023 · 3 comments · Fixed by #3071
Assignees
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. WIP
Milestone

Comments

@pillowfication
Copy link

Using the OpenAPI 3.0.0 petstore sample

kiota generate -d "https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/examples/v3.0/petstore.json" -l CSharp

The part of interest is

"paths": {
  "/pets/{petId}": {
    "get": {
      "parameters": [
        {
          "name": "petId",
          "in": "path",
          "required": true,
          "description": "The id of the pet to retrieve",
          "schema": {
            "type": "string"
          }
        }
      ]
    }
  }
}

Current Behavior

The following block of code is created for the indexer

/// <summary>Gets an item from the ApiSdk.pets.item collection</summary>
public WithPetItemRequestBuilder this[string position] { get {
    var urlTplParams = new Dictionary<string, object>(PathParameters);
    if (!string.IsNullOrWhiteSpace(position)) urlTplParams.Add("petId", position);
    return new WithPetItemRequestBuilder(urlTplParams, RequestAdapter);
} }

current behavior

Expected Behavior

I would like for the position variable to be named the same as the path parameter petId. I would also like for path parameter's description to be kept and placed in a corresponding <param/> documentation.

/// <summary>Gets an item from the ApiSdk.pets.item collection</summary>
/// <param name="petId">The id of the pet to retrieve</param>
public WithPetItemRequestBuilder this[string petId] { get {
    var urlTplParams = new Dictionary<string, object>(PathParameters);
    if (!string.IsNullOrWhiteSpace(petId)) urlTplParams.Add("petId", petId);
    return new WithPetItemRequestBuilder(urlTplParams, RequestAdapter);
} }

expected behavior

@baywet
Copy link
Member

baywet commented Jul 21, 2023

Thanks for your interest in kiota and for reporting this.

This is something that could be easily fixed by adding a ParameterDescription property on the code indexer DOM element and populating that property.

return new CodeIndexer

public class CodeIndexer : CodeTerminal, IDocumentedElement, IDeprecableElement

Then we need to make sure we carry the information when we convert the indexer to a method in other languages

ArgumentNullException.ThrowIfNull(originalIndexer);

And lastly we need to make sure we write the information in dotnet (the other languages will rely on the code parameter information already)

conventions.WriteShortDescription(codeElement.Documentation.Description, writer);

Is this something you'd be willing to submit a pull request for?

@baywet baywet added enhancement New feature or request Csharp Pull requests that update .net code labels Jul 21, 2023
@baywet baywet added this to Kiota Jul 21, 2023
@github-project-automation github-project-automation bot moved this to Todo in Kiota Jul 21, 2023
@baywet baywet added this to the Kiota v1.6 milestone Jul 21, 2023
@sebastienlevert
Copy link
Contributor

@baywet Please review based on the 1.5 release improvements. Thanks!

@baywet baywet assigned baywet and unassigned pillowfication Aug 4, 2023
@baywet baywet added generator Issues or improvements relater to generation capabilities. and removed Csharp Pull requests that update .net code labels Aug 4, 2023
@baywet
Copy link
Member

baywet commented Aug 4, 2023

The 1.5 improvements didn't impact this issue much: We now support type specific indexer parameters (not just string anymore, but also int etc...). #2594
We can't change the parameter name, it'd be a breaking change even though it's unlikely anybody has a direct reference to it. I created #3070 to follow up on this during v2.
We should still carry on the description of the parameter for a better developer experience. I'll work on that for the next release.

@baywet baywet moved this from Todo to In Progress in Kiota Aug 4, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Kiota Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. WIP
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants