M-I-N / NimbleSurvey

0 stars 0 forks source link

[Question] Support pagination loading concern #13

Open minhnimble opened 2 years ago

minhnimble commented 2 years ago

Issue

As you can probably see from the Postman, the surveys list API should be capable of providing more data than what the application currently displays. Right now the data shown on the Surveys List screen is only fixed to 5 items and it loads only the first page. As a result, there is no way for the user to see all the data from API response with this hard-coded configuration:

https://github.com/M-I-N/NimbleSurvey/blob/c7575c9d61b58f84cd7124cd99f601633de2d22d/NimbleSurvey/API%2BServices/Service%2BAdapter.swift#L60-L90

Therefore, if you are requested to support the pagination loading to load all the surveys list data, can you share with me your detailed approach to achieve this requirement?

M-I-N commented 2 years ago

To achieve the paginated loading, first thing that we need is the separation of logic between the first page request and any other page request that is not the first page. We have to modify the SurveyItemService to incorporate this separation of concern. https://github.com/M-I-N/NimbleSurvey/blob/c7575c9d61b58f84cd7124cd99f601633de2d22d/NimbleSurvey/View/HomeScreenViewController.swift#L11-L13 Proposed approach is:

func loadFirstPageSurveyItems(completion: @escaping (Result<[SurveyItemViewModel], Error>) -> Void)
func loadNextPageSurveyItems(completion: @escaping (Result<[SurveyItemViewModel], Error>) -> Void)

Next thing is we find the end of the list, like here: https://github.com/M-I-N/NimbleSurvey/blob/c7575c9d61b58f84cd7124cd99f601633de2d22d/NimbleSurvey/View/HomeScreenViewController.swift#L82 And we execute and append the new items to the data source items:

service?.loadNextPageSurveyItems(completion: ...)

In the adapter layer, the actual implementation will take place. We will request for the next page from:

func loadNextPageSurveyItems(completion: @escaping (Result<[SurveyItemViewModel], Error>) -> Void) {
    // request for the next page items by adding +1 to the pageNumber property

    // on success case, assign the incremented page number to the property for next successive calls to succeed
}
M-I-N commented 2 years ago

Please have a look at #17

minhnimble commented 2 years ago

@M-I-N your implementation looks functional for handling the pagination logic. At the same time, it seems like only the main flow is handled and there is not a stopping condition for the load more logic. Another point I want to rise is that while the loading more API is called, there doesn't seem to be a way to let the user know that it is sending a request to the server. Therefore, the user can actually spam to load more multiple times I believe 🤔