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

Define success response type #38

Closed
1 of 3 tasks
ymaheshwari1 opened this issue Jan 4, 2023 · 6 comments · May be fixed by #47
Closed
1 of 3 tasks

Define success response type #38

ymaheshwari1 opened this issue Jan 4, 2023 · 6 comments · May be fixed by #47

Comments

@ymaheshwari1
Copy link
Contributor

What is the motivation for adding/enhancing this feature?

Currently, for product specific methods the return type is any | Response but using any is not a recommended way so we need to define a type for ProductResponse

What are the acceptance criteria?

  • Update return type for product specific methods

Can you complete this feature request by yourself?

  • YES
  • NO

Additional information

Approach:

Define a separate type in Product type named ProductResponse that contain two properties.

interface ProductResponse {
  products: Product[],
  total: number
} | {
  product: Product
}
@ymaheshwari1
Copy link
Contributor Author

ymaheshwari1 commented Jan 5, 2023

Another pattern to define product response is:

interface ProductResponse {
 [string]?: number
} & ({
  product: Product
} | {
  products: Product[]
})

Also, we can even think of defining the interface without using union patten and simply define it as:

interface ProductResponse {
  product: Product | Product[],
 [string]?: number
}

But in this way the property name will no more self-explanatory, also it might be difficult to handle the response on application side.

@adityasharma7
Copy link
Contributor

We could define a base response interface and extend it to specific responses like Product, Shipment etc

https://www.typescriptlang.org/docs/handbook/2/objects.html#extending-types

Create a basic POCs for these cases then we will conclude accordingly

@ymaheshwari1
Copy link
Contributor Author

ymaheshwari1 commented Jan 9, 2023

Concluded that we will use the pattern of generics.

interface SuccessResponse<Type> {
  list: Type[];
  total: number;
  groups?: number
}

@ymaheshwari1 ymaheshwari1 moved this from 📅 Planned to 👀 In review in Digital Experience Platform Jan 9, 2023
@ymaheshwari1
Copy link
Contributor Author

ymaheshwari1 commented Jan 12, 2023

Explored around how success response are returned using rest api from ecoms

In initial exploration found that only the original object/array is returned without any other extra property like total, size, count etc

  // For getting a list of products/orders
  {
    products: [{}, {}]
  }
  
  // For getting a single order/product
  {
    product: {}
  }

In another exploration found that there is no specific key used while returning the response

  // Can be accessible using response.data
  // Getting a list of products/orders
  [{}, {}]
  
  // Get a single product/order
  {}

In one more exploration found that, in case of retriving a single product just return a single object and for retriving a list, multiple other fields with items array is returned

  // Get a single product/order
  {}
  
  // Retriving a list of products/orders
  {
    startIndex: 0,
    pageCount: 0,
    totalCount: 0,
    items: [{}, {}]
  }

One more pattern is identified in which for every api endpoint a separate response type is defined which extends another base response type.
For example when accessing api to get a single product, the response is ProductResponse extends BaseResponse, when accessing api to get a list of products, the response type is ProductListResponse extends OtherBaseResponse`.

@ymaheshwari1 ymaheshwari1 moved this from 👀 In review to 🏗 In progress in Digital Experience Platform Jan 12, 2023
@ymaheshwari1 ymaheshwari1 changed the title Define product response type Define success response type Jan 12, 2023
@ymaheshwari1
Copy link
Contributor Author

ymaheshwari1 commented Jan 12, 2023

Discussed on the above points, and concluded that in case of fetching a specific record like a product/order/shipment using it's id, we will define the return type same as the request resource(product/ shipment / order), and in case when requesting a list of records for a resource we will return an array, total (optional), and groups(optional).

For the case of requesting multiple records:

interface ListResponse<T> {
  list: Array<T>;
  total?: number; // number of records on the server for the specific resource
  groups?: number; // number of groups found when fetching the resource by grouping on a field
}

@ymaheshwari1
Copy link
Contributor Author

ymaheshwari1 commented Jan 12, 2023

Explored around the property/variable naming convention used to hold the total and groups value.

In case of solr, ngroups contain the count of the total groups, and matches contain the count of records.

In case of open search, https://opensearch.org/docs/latest/opensearch/bucket-agg/ is similar to grouped logic, and in open search case we get doc_count specifying the number of document in each bucket.

// Response in open search in case of buckets
"aggregations" : {
  "response_codes" : {
    "doc_count_error_upper_bound" : 0,
    "sum_other_doc_count" : 0,
    "buckets" : [
      {
        "key" : "200",
        "doc_count" : 12832
      },
      {
        "key" : "404",
        "doc_count" : 801
      },
      {
        "key" : "503",
        "doc_count" : 441
      }
    ]
  }
 }
}

@ymaheshwari1 ymaheshwari1 moved this from 🏗 In progress to ✋Hold in Digital Experience Platform Jan 17, 2023
@dt2patel dt2patel moved this from ✋Hold to 📋 Backlog in Digital Experience Platform Feb 1, 2023
@Dhiraj1405 Dhiraj1405 moved this to 📋 Backlog in Digital Experience Platform Aug 2, 2023
@dt2patel dt2patel closed this as completed Mar 1, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Digital Experience Platform Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants