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

Paged request, response enhancement #7720

Closed
wants to merge 11 commits into from

Conversation

ahmetfarukulu
Copy link
Member

I added some feature as below;

  • When client side send page request it have to calculate TakeSkip property that is unnecessary process
  • Page response must have additional info as PageCount(last page count),FirstRowOnPage

@maliming
Copy link
Member

hi @ahmetfarukulu

Can you share some use cases?

@maliming maliming added this to the 4.3-preview milestone Feb 15, 2021
@maliming maliming self-requested a review February 15, 2021 12:37
@ahmetfarukulu
Copy link
Member Author

Hello @maliming, I deleted my previous comment because i changed some basic types. There are 3 topics i want to talk about

  1. PageRequest may change a little, such as; By taking PageNumber instead of SkipCount this way easy to understand which page shown.
  2. PageResponse could be more informative with these properties; Last page count, index shown
  3. A more practical result can be returned to page requests ; we don't always need to write code for count items

Examples

  • For subjects 1 and 2

1

We give page number 3 rather than skip 50 and result said we shown page 3 and last page number is 20 response sample;

{
  "currentPage": 3,
  "pageCount": 20,
  "pageSize": 25,
  "firstRowOnPage": 51,
  "lastRowOnPage": 75,
  "totalCount": 500,
  "items": [
    {
      "code": "P-51",
      "name": "Product-51",
      "id": "64ce52d2-1331-efea-750f-39fac2fbe6da"
    },
    {
      "code": "P-52",
      "name": "Product-52",
      "id": "06a5a998-69e4-4533-c67e-39fac2fbe6df"
    },...
  ]
}
  • For subject 3

Without extension methods we have to write code for count items with conditions, order by sorting , mapping ...

    public async Task<IDetailedPagedResult<ProductDto>> GetListByPageAsync(GetProductListByPageDto input)
    {
        if (input.Sorting.IsNullOrWhiteSpace())
            input.Sorting = nameof(Product.Id);

        var queryable = await _productRepository.GetQueryableAsync();

        var query = queryable.WhereIf
                                (
                                    !input.Filter.IsNullOrWhiteSpace(),
                                    product => product.Code.Contains(input.Filter)
                                )
                                .OrderBy(input.Sorting)
                                .Page(input.Page, input.MaxResultCount);

        var products = await AsyncExecuter.ToListAsync(query);

        int totalCount = await AsyncExecuter.CountAsync(queryable.WhereIf
                                (
                                    !input.Filter.IsNullOrWhiteSpace(),
                                    product => product.Code.Contains(input.Filter)
                                ));

        return new DetailedPagedResultDto<ProductDto>(
            input.Page,
            (int)Math.Ceiling((double)totalCount / input.MaxResultCount),
            input.MaxResultCount,
            totalCount,
            ObjectMapper.Map<List<Product>, List<ProductDto>>(products)
        );
    }

With extension methods;

    public async Task<IDetailedPagedResult<ProductDto>> GetDynamicPagedResult(GetProductListByPageDto input)
    {
        if (input.Sorting.IsNullOrWhiteSpace())
            input.Sorting = nameof(Product.Id);

        var queryable = await _productRepository.GetQueryableAsync();

        var query = queryable.WhereIf
                    (
                        !input.Filter.IsNullOrWhiteSpace(),
                        product => product.Code.Contains(input.Filter)
                    );

        return await query.PageResultAsync<Product, ProductDto>(AsyncExecuter, ObjectMapper, input);
    }

@hikalkan
Copy link
Member

Hi @ahmetfarukulu

Thanks for your PR. I agree it simplifies some use cases and helps client side to get more info about paging. However, it is too specialized. We want to keep it simple in the framework side and don't want to introduce different DTOs for similar purposes. When we add these into framework, we have to support them to be compatible with all other systems.
I suggest you to keep this code in your own solution and use in your applications. So, I can not accept this PR, very sorry.

Thanks again for your contribution. Keep following and contributing to the ABP Framework :)

@hikalkan hikalkan closed this Feb 26, 2021
ahmetfarukulu added a commit to ahmetfarukulu/abp that referenced this pull request Feb 28, 2021
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 this pull request may close these issues.

None yet

3 participants