apache / apisix-dashboard

Dashboard for Apache APISIX
https://apisix.apache.org/
Apache License 2.0
996 stars 521 forks source link

bug: The route created by admin API is missing fields in the dashboard display #717

Closed idbeta closed 3 years ago

idbeta commented 3 years ago

The route created by admin API is missing fields in the dashboard display. This is an example provided by APISIX

$ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
 "uri": "/",
 "plugins": {
     "jwt-auth": {}
 },
 "upstream": {
     "type": "roundrobin",
     "nodes": {
         "139.217.190.60:80": 1
     }
 }
}'

after create the route, the dashboard display like this: image

I think there should be default values of Name, Path, UpdateAt when the route was created like that.

membphis commented 3 years ago

@ShiningRush do you have any idea about this issue?

membphis commented 3 years ago

the Admin API user will create a route with any of those fields, the frontend needs to show them in the dashboard correctly.

For example, create the route with field uri, and we need to show it at frontend:

$ curl http://127.0.0.1:9080/apisix/admin/routes/1 -X PUT -i -d '
{
    "uri": "/index.html",
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "127.0.0.1:80": 1
        }
    }
}'
ShiningRush commented 3 years ago
membphis commented 3 years ago
  • I think that in the future, we only need to support fields of arrays(uris, hosts, remote_addrs would be better). We can convert single-valued fields such as uri into arrays when manager-api list resources.

manager-api can implement this feature after 2.0RC3, need more time(manager-api and admin API will update to this process).

2.0 version, we should keep manager-api and admin-api same.

juzhiyuan commented 3 years ago

FYI: All fields without value would fallback to None on the frontend.

juzhiyuan commented 3 years ago

And we will convert all host to hosts before sending requests because the variable in arrays should be more uniform when there have both host and hosts.

juzhiyuan commented 3 years ago

The table component supports columnEmptyText, we could use it to set default content when that field doesn't have a value, but it will check if that field is returned from the data source.

According to the following snapshot, every record doesn't have the same structure, so I have to add a checker function for every field on the frontend, they are too many so it's not a good idea IMO.

image

Summary: Leave those fields' value as empty currently. cc @membphis @idbeta

ShiningRush commented 3 years ago

manager-api can do convert in list action, no need do that in web. web just care array fields such as uris, hosts etc.

juzhiyuan commented 3 years ago

manager-api can do convert in list action, no need do that in web. web just care array fields such as uris, hosts etc.

There have 2 opinions:

I agree with @ShiningRush's opinion is because that would be better and the web will reduce many tricks.

I agree with @membphis's opinion is because the manager-api will have the same params with admin API in this way.

Because I don't think the manager-api will add those logics before publishing the RC3, so I just took some time and adding the convertor on the frontend.

membphis commented 3 years ago

web just care array fields such as uris, hosts etc.

we can do this later, 2.1 or 2.2.