A favorite icon will let the authenticated user add the resource to his favorites #214
Conversation
|
please @allyoucanmap review and merge this. |
| @@ -243,6 +243,12 @@ export const getDocumentsByDocType = (docType = 'image', { | |||
| })); | |||
| }; | |||
|
|
|||
| export const setFavouriteResource = (pk, method) => { | |||
| // eslint-disable-next-line dot-notation | |||
| return axios[method](parseDevHostname(`${endpoints[RESOURCES]}/${pk}/favorite`)) | |||
There was a problem hiding this comment.
Why do we need to pass the method?
I would prefer to have to separated function if they are needed
There was a problem hiding this comment.
to have one function, instead of 2 similar function, in this case change only the CRUD method
There was a problem hiding this comment.
If the aim is to use a single function I propose a different approach and do not expose the method.
Now we have here:
setFavouriteResource(pk, 'post')// set favourite to truesetFavouriteResource(pk, 'delete')// set favourite to false
My proposal is to make this more explicit:
setFavouriteResource(pk, true)// set favourite to truesetFavouriteResource(pk, false)// set favourite to false
Another proposal is to avoid as much as possible eslint-disable. We should try to follow the eslint rules:
const setFavouriteResource = (pk, favourite) => {
const request = favourite ? axios.post : axios.get;
return request(parseDevHostname(`${endpoints[RESOURCES]}/${pk}/favorite`))
}| closePanel, | ||
| favourite, | ||
| onFavourite, | ||
| isLogged |
There was a problem hiding this comment.
I think we should use something more related to favourite instead of isLogged. isLogged is related to the user and the DetailsPanel component should not be aware of the user state
maybe enableFavourite?
| const state = store.getState(); | ||
| const pk = state?.gnresource?.data.pk; | ||
| const favourite = action.favourite; | ||
| const method = (favourite) ? 'post' : 'delete'; |
There was a problem hiding this comment.
I think we should have setFavouriteResource and deleteFavouriteResource
There was a problem hiding this comment.
probably is better removeFavouriteResource instead of delete
A favorite icon will let the authenticated user add the resource to his favorites
logged user
Registrazione.schermo.2021-05-27.alle.17.44.48.mov
unlogged user
Registrazione.schermo.2021-05-27.alle.17.46.03.mov