Skip to content

Feature/redux architecture example#9

Open
agaskrobot wants to merge 18 commits intodevelopfrom
feature/redux-architecture-example
Open

Feature/redux architecture example#9
agaskrobot wants to merge 18 commits intodevelopfrom
feature/redux-architecture-example

Conversation

@agaskrobot
Copy link
Copy Markdown
Collaborator

No description provided.


it('should display a label and input elements with empty userName value', () => {
// Arrange
it('Set the email and name and trigger handleAdd "', () => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleAdd triggering is not being tested with the current test.

) as HTMLButtonElement;
buttonElement.click();
const spy = spyOn(myApi, 'getContactList');
expect(spy).toBeCalled();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is failing.


const labelElement = getByTestId('userName-label');
const inputElement = getByTestId('userName-input') as HTMLInputElement;
it('Change the email state and trigger handle Change "', async () => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleChange triggering is not being tested here with the current test.

return <h1>Something went wrong...</h1>;
}

const handleChange = (contact: ContactType) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When modifiying contacts (add, updated or delete), we are not asking for the updated contacts list in the current code, we are modifying the store directly by the action with the data of the component instead. Could it cause any issue with our data?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello could you specify your question??
Do you mean i should set first data in the state and after that call api and dispatch?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll put an example. Let's say that you are adding a new user, and the backend fails in storing it. With the current code you would have the new user in the local store, but not in backend.

The data that you are passing to the store is the data from the input and not the one from backend. This specific situation will cause to have different users in local and in the server.

This is only an example and could be more situations where we would end with incoherent data. The backend does not have to fail to be in this situation.

Could you change it and avoid this problem?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my current code when api fails i catch the error and i dont save anything in the store.
I think is imossible that i have inconsistent data because i dispach after getting good response.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot from 2020-04-13 16-14-28

here you can see when i wants to create new user and the api fails I catch the error and the store doesnt change.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you should concatenate actions and request the contact list after any change (update, delete or add). I am not saying that you should update the store with the response, you should request the full list.

Copy link
Copy Markdown
Collaborator Author

@agaskrobot agaskrobot Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you want me to do double request like for multi users??
If yes no problem i can do it but this is not gonna work in this api (the fake api which you suggest in the issue) because the get request returns always the same static list of users, so you are not gonna see any changes in the app.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but we would like to see how you implement the solution.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course! give a while!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello already done!

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.

2 participants