lorawan-stack icon indicating copy to clipboard operation
lorawan-stack copied to clipboard

Extend store logic for search results

Open kschiffer opened this issue 5 years ago • 8 comments

Summary

We need to change the way we handle search results in our redux store. Currently, we treat search request just like regular list requests.

Why do we need this?

  • The totalCount prop we're using will be affected by search requests, resulting in the UI showing wrong values for the total number of entities
  • Search requests will trigger a fetching toggle for the GET_*_LIST actions, resulting in some elements rendering a loading spinner when they don't actually have to

What is already there? What do you see now?

  • Current state of the store which includes the new pagination logic

What is missing? What do you want to see?

A distinction of LIST_* and SEARCH_* requests

Environment

Console @ v3.5.0

How do you propose to implement this?

Cannot think this through in detail right now, but maybe adding a filteredIds to the pagination store, which will hold the currently filtered entities.

Can you do this yourself and submit a Pull Request?

Yes but @bafonins feel free to take over

kschiffer avatar Jan 24 '20 12:01 kschiffer

  • regarding the issue with updating entity count: let's just have a new variable maxCount that stores the total count of entities unlike the totalCount that is used for pagination. I assume this variable would be only used in overview views because otherwise it is useful to display the number of search results. (haven't looked yet if it's possible to fetch the total number of devices only, but it should be independent of totalCount).

  • regarding the fetching issue, my suggestion is to create a new store SEARCH_* request that will not trigger the fetching when called.

pgalic96 avatar Jan 26 '20 22:01 pgalic96

Please take into account that sorting table entries also triggers the entity title section to reload. I think this shouldnt be the case

bafonins avatar Jan 28 '20 09:01 bafonins

I'd suggest the following:

  • Introduce new actions to retrieve the total number of entities per entity type (e.g. GET_TOTAL_APPLICATION_COUNT)
    • This will basically be a simple List call without filter parameters, from which we just extract the X-Total-Count-header
    • Store the result in a dedicated place in the store (I'd suggest e.g. applications/totalCount)
  • Use these actions to fetch the total number of entities for the entity title section
  • A distinction between LIST_* and SEARCH_* actions are then not needed (yet?) and we can avoid a larger store refactor

kschiffer avatar Feb 05 '20 01:02 kschiffer

Can we close this issue until (if) the distinction between list and search action becomes necessary again?

pgalic96 avatar Mar 25 '20 16:03 pgalic96

Let's move to Backlog then

johanstokking avatar Mar 25 '20 18:03 johanstokking

Has something changed here? Do we now need the distinction between LIST_* and SEARCH_* . I just finished implementing the new total counts actions, so I could already also include that. @kschiffer

ryaplots avatar Mar 08 '23 09:03 ryaplots

I'll need to revisit the exact root cause and issues this produces. Let's keep this in the backlog until then. What exactly are you working on with the total counts?

kschiffer avatar Mar 08 '23 14:03 kschiffer

I've introduced these this morning https://github.com/TheThingsNetwork/lorawan-stack/pull/6096:

* Introduce new actions to retrieve the total number of entities per entity type (e.g. `GET_TOTAL_APPLICATION_COUNT`)
  
  * This will basically be a simple `List` call without filter parameters, from which we just extract the `X-Total-Count`-header
  * Store the result in a dedicated place in the store (I'd suggest e.g. `applications/totalCount`)

* Use these actions to fetch the total number of entities for the entity title section

I can leave the branch and re-open the PR when this is relevant again.

ryaplots avatar Mar 08 '23 15:03 ryaplots