Versent / redux-crud

A set of standard actions and reducers for Redux CRUD Applications
MIT License
623 stars 54 forks source link

Option to clear existing records on fetchSuccess #58

Closed pinkynrg closed 5 years ago

pinkynrg commented 7 years ago

So first of all thanks for this library, is has been very useful so far.

Now I'm at the point where I need to filter records from my list. I see that the fetchSuccess will merge the records with the old ones and not replace the entire collection as I used to do with React (before starting using Redux).

So if I have:

1) Item one 2) Item two 3) Item twenty-two

and I type "two", the api will return record 2 and 3 and the reducer will merge the 2 records with the existing ones (which are already there).

I created a reducer that keeps track of the last fetched record ids, for now I might search those record ids in the map and use those to print my updated list but I was wondering if there is a better way to do it or this is the way to do it.

hwaterke commented 7 years ago

I'm not sure to understand what you are trying to do.

What do you mean by "I type 'two'"? Could you elaborate?

pinkynrg commented 7 years ago

Typing on the keyboard the word "two". Sorry i was not clear enough.

What I'm trying to say in that both the reducer map and the reducer list are both collecting records instead of just replacing then every time with the new payload passed by the API to fetchSuccess(payload).

So going back to the example above.

When no filter applied (GET api.getSomething.json) I get:

[{id: 1, title: "Hello world!", article_id: 1},
{id: 2, title: "Redux is solid", article_id: 1},
{id: 3, title: "The world is yours!", article_id: 1}]

After the filter (GET api.getSomething.json?match="world") I get:

[{id: 1, title: "Hello world!", article_id: 1},
{id: 3, title: "The world is yours!", article_id: 1}]

Here I would expect to get back both map and list reducer have 2 items in it, 1 and 3, instead I see that item 2 is still there.

What would be the best way to actually get my current filtered list? The one with only 1 and 3? What would you suggest?

sporto commented 7 years ago

One idea I can think of is to have a reducer that removes all records. You would dispatch that first and then fetchSuccess.

Also, maybe this is a common use case enough that warrants a PR to add a parameter to fetchSuccess to clear existing data.

sporto commented 7 years ago

So a future enhacement could be to add an option to this action e.g.

fetchSuccess: function(users, data) {
    return {
      clearExisting: true,
      data:    data,
      records: users,
      type:    'USERS_FETCH_SUCCESS',
    };
  },
pinkynrg commented 7 years ago

That would make total sense.

hwaterke commented 7 years ago

This is indeed a pretty common use case and I think it would make sense to have it included in the library.

To have this functionality with the library as it is now, I have patched the reducersFor like so in my project:

export function reducersFor(resource: string) {
  const baseReducer = reduxCrud.Map.reducersFor(resource);
  const actionTypes = reduxCrud.actionTypesFor(resource);

  return function(state: {}, action: Action) {
    if (action.type === actionTypes.fetchSuccess) {
      if (action.data && action.data.replace === true) {
        // If replace is true, we want to replace the existing state instead of merging
        return baseReducer(undefined, action);
      }
    }
    return baseReducer(state, action);
  };
}

@sporto would you welcome a PR to have this implemented in the library?

sporto commented 7 years ago

Yes a PR would be very welcome. Otherwise it won't get done. Thanks.

hwaterke commented 7 years ago

PR submitted https://github.com/Versent/redux-crud/pull/60 Let me know what you think!

pinkynrg commented 5 years ago

Closing