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

[feature] Get /services/:service implementation #49

Closed
4 tasks
benoitblanc opened this issue Mar 13, 2023 · 5 comments · Fixed by #50
Closed
4 tasks

[feature] Get /services/:service implementation #49

benoitblanc opened this issue Mar 13, 2023 · 5 comments · Fixed by #50
Assignees

Comments

@benoitblanc
Copy link
Contributor

Create GET /services/:service like it is specified inside https://github.com/IGNF/road2/blob/develop/documentation/apis/administration/1.0.0/api.yaml.

  • Code it (see next paragrah)
  • Update doc (ex. functionalities, changelog)
  • Update tests (new classes if there are; rtest for this route)
  • Run tests inside docker (rtest, ctest, utest, itest)

Code

  • Add the route inside the router of admin/1.0.0 (no need of an interface)
  • Create a function getServiceConfiguration() inside administrator.js to handle the request and to get the information. Everything is in this._configuration of the administrator instance (this._configuration is the content of road2.json) :
    • Id is in the config of the admin (aka road2.json)
    • the rest is the content of the config file of the service (aka service.json)
  • Refactor function getServicesConfigurations() to use the new getServiceConfiguration() to get configuration for each service
@lgrd
Copy link
Collaborator

lgrd commented Mar 13, 2023

You will have to update the api.yaml because there is no id in the response of this route. But it could be interesting to have the same object for this route and the /services route. What do you think ?

@benoitblanc
Copy link
Contributor Author

@lgrd There is no id in serviceConfiguration response in api.yml for now, I have to add it to have the correct response for /services route. Also, I started to implement this new route with the same object so /services/:service will also have an id.

Does it look good for you ?

@lgrd
Copy link
Collaborator

lgrd commented Mar 13, 2023

Oh ! I got the point...

Now, you have this in the yaml :

        200:
          description: "successful operation"
          content:
            application/json:
              schema:
                type: "array"
                items:
                  type: "object"
                  allOf:
                    - type: "object"
                      properties:
                        id:
                          type: "string"
                    - $ref: "#/components/schemas/serviceConfiguration"

This is because, when you POST or PATCH a service, you will not give the id inside the body. It is only inside the URL parameter.

That's why I add it only in the response of /services. So, to be in line with the other /service/{serviceId}, let us keep the api.yaml like it is.

So, in your code section, you can only read the content of service.json and return it. You don't have to add the id.

Is that ok for you ?

@benoitblanc
Copy link
Contributor Author

Sorry I did not see that id was added outside serviceConfiguration in /services. So yes I will update my branch to only return serviceConfiguration without id.

@lgrd
Copy link
Collaborator

lgrd commented Mar 13, 2023

thanks ! I will test your code tomorrow but it looks good ! ;)

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 a pull request may close this issue.

2 participants