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

Render request headers #1427

Closed
mar-gil opened this issue Nov 16, 2022 · 15 comments · Fixed by #1978 · May be fixed by #1534
Closed

Render request headers #1427

mar-gil opened this issue Nov 16, 2022 · 15 comments · Fixed by #1978 · May be fixed by #1534
Assignees
Labels
👨‍🎨 Status: UX Review Requires review from design team 🚀 Type: New Feature Something new

Comments

@mar-gil
Copy link

mar-gil commented Nov 16, 2022

In the endpoint description, we would need to render the request headers contained in Frontend APIs RAML files.

get:
  is:
    - right-header
    - frontastic-locale
  headers:
    Frontastic-Path:
      displayName: Frontastic Path
      type: string
      description: Path
      required: true
      pattern: ^/.*$
  description: Returns the structure and data of the published page displayed from commercetools Frontend.
  responses:
    200:
      description: Page folder information, page structure, and data source data.
      body:
        application/json:
          type: PageResource
    404:
      description: Page folder not found.
      body:
        application/json:
          type: Error
          example: !include ../examples/PageFolderError.raml
          
@FFawzy FFawzy added the 🚀 Status: Groomed All is known to start development label Nov 25, 2022
@FFawzy
Copy link
Contributor

FFawzy commented Nov 25, 2022

are the headers the same in every endpoint?
one way to look at it is that if they are the same across all the endpoints maybe we want to explain it once in a separate page?
or is it necessary in every page ?
WDYT?

this is an example of a custom header in the CoCo responses as well but we defined it once in a page

@AgaMillward AgaMillward added the 🚀 Type: New Feature Something new label Nov 25, 2022
@mar-gil
Copy link
Author

mar-gil commented Nov 25, 2022

are the headers the same in every endpoint? one way to look at it is that if they are the same across all the endpoints maybe we want to explain it once in a separate page? or is it necessary in every page ? WDYT?

this is an example of a custom header in the CoCo responses as well but we defined it once in a page

@FFawzy sorry for the late reply. I must have missed the notification.
Nope, the headers are not always the same. Furthermore, some are standard headers while some are "frontend-custom" headers. Therefore, I think that having a shared separate page would not be ideal.

@gabriele-ct gabriele-ct added 🚀 Status: Groomed All is known to start development and removed 🚀 Status: Groomed All is known to start development labels Nov 28, 2022
@gabriele-ct gabriele-ct removed the 🚀 Status: Groomed All is known to start development label Dec 12, 2022
@gabriele-ct
Copy link
Contributor

@timonrey First step is to make and analysis of the ticket and the expected outcome is a tech proposal on how to implement the feature (check if rmfgen is supporting this feature)

@timonrey
Copy link
Contributor

I tried out the raml file that is provided in the description with the custom header to see if rmf-codegen supports this data structure, but it looks like the headers are not being generated.

@jenschude maybe you can double-check, but we will probably need an update from the rmf-codegen to support this use-case. Any thoughts from your side are welcome as always ;)

@mar-gil
Copy link
Author

mar-gil commented Jan 20, 2023

I tried out the raml file provided in the description with the custom header to see if rmf-codegen supports this data structure, but it looks like the headers are not being generated.

@jenschude maybe you can double-check, but we will probably need an update from the rmf-codegen to support this use-case. Any thoughts from your side are welcome as always ;)

@timonrey (cc @jenschude) what I did was moving the first headers "in the method" like this:

get:
  is:
    - right-header
    - frontastic-locale
  headers:
    Frontastic-Path:
      displayName: Frontastic Path
      type: string
      description: Path
      required: true
      pattern: ^/.*$
  description: |

I forgot to update the issue description, now it's updated. They are rendered in the cURL examples now, but we would also need to render them in the endpoint description. For the moment, they're handwritten.
image

@jenschude
Copy link
Collaborator

Update to at least version 13.23. of the codegen. Then the headers are in the resource files and the curl examples

@stmeissner
Copy link

stmeissner commented Nov 17, 2023

Why is this issue blocked? If you need more input to groom it, please reach out to me 🙂

@Anshuman71
Copy link

@timonrey can we help clarify anything to unblock this issue?

@timonrey
Copy link
Contributor

timonrey commented Jan 2, 2024

If I remember correctly, there was still a problem with the data structure rmf-codegen can work with. But maybe that's not the case anymore. I will unblock it and revisit the issue.

@timonrey
Copy link
Contributor

Update: The rmf codegen is generating the required fields correctly so we can move forward with the implementation as soon as agreed on the design. @zbalek

@timonrey timonrey added the 👨‍🎨 Status: UX Review Requires review from design team label Apr 15, 2024
@timonrey
Copy link
Contributor

The docs-team agreed on the visual implementation:
https://commercetools.atlassian.net/wiki/spaces/DOCS/pages/699793472/Docs+Team+Weekly+2024-04-17

@Anshuman71
Copy link

Hi @timonrey, we need to do some work on Frontend API docs before this feature goes live on the published docs. Do you have a timeline for the release of this change? So we can prepare.

@timonrey
Copy link
Contributor

@Anshuman71 We can release it whenever you feel ready. If it's not too much work you could also adjust the frontend API in the release preview PR in the docs repo.

@Anshuman71
Copy link

Hi @timonrey, thanks. Can you link the preview PR here? I'll start the work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👨‍🎨 Status: UX Review Requires review from design team 🚀 Type: New Feature Something new
Projects
None yet
8 participants