Fix for issue #3180

[gets0ul]

#3180

Error message has been already shown by alert popup.
Error page will be only shown for error code 500 (as hard-coded for the CoderBot).
https://github.com/appirio-tech/connect-app/blob/b2022930027fa7e0e97cd1a42e0373ed0b70cdb3/src/routes/metadata/containers/ProjectTemplatesContainer.jsx#L117

[maxceem]

@gets0ul I cannot test it properly as I cannot trigger error with code 500 though it looks for me that the main issue is not resolved.

If during create/update/delete we would get error with code 500 and after we switch to another page, we would still see the error page as was described in the issue #3180

I guess there are two ways to go:

1. If we don't need this error inside Redux Store for create/update/remove actions, we can just remove this error property and don't set it when error happens when we create/update/remove objects. As you've mentioned before we already showing the error message. So we just have to verify we really don't use this error in these cases elsewhere.
2. If we still need these error in redux after create/update/remove actions, then we should reset it when we change page. So even if some error happens during create/update/remove it shouldn't break other functionality when we are switching page.

Let me know what you think.

[gets0ul]

Yes the error is is not needed and can be removed from the store as it is not used anywhere.
The GridView component which is this error ends up to, ignores it completely.

I cannot test it properly as I cannot trigger error with code 500 though it looks for me that the main issue is not resolved.

You don't need to trigger the 500. I mean, main issue is whenever error occurs when dealing with metadata management, we can't go back to the any page as error page will be shown (because of that error in the redux)

And my fix already solves that. Using the example project, with phsah_manager account, error is shown by popup, and we are now able to switch to other page as required by the issue spec.

If during create/update/delete we would get error with code 500 and after we switch to another page, we would still see the error page as was described in the issue #3180

Yes that's why I don't remove the error from the store for 500 error:
error: action.payload.response.data.result.status >= 500

[maxceem]

And my fix already solves that. Using the example project, with phsah_manager account, error is shown by popup, and we are now able to switch to other page as required by the issue spec.

Right, it work because now error which we get has code 403. But imagine if we update some object and there is error on server with code 500. In such case if we switch the page after, we would see error page. But we shouldn't see error page after we switch page.

For example, for testing I can update your code to catch errors from code 400:

error: action.payload.response.data.result.status >= 400

Now if I try to save any object I will get an error, and if I switch the page, I would still see error page. But I shouldn't see error page, because this error happens before and shouldn't break other pages.

[gets0ul]

we should reset it when we change page.

So we will be doing it instead. Right?

[maxceem]

So we will be doing it instead. Right?

We should either reset it when we change page.
Or if we don't use this error anywhere, which happens during create/update/delete, we just can remove saving it to the Redux Store in these actions. But we have to make sure, that we don't use it. So if we don't set it at all, we don't have to reset it after.

[gets0ul]

It is not needed.

Lets take Project Template metadata management as example.
https://github.com/appirio-tech/connect-app/blob/23ad36e35de1e8ff389dc5e83f83ebfda434153d/src/routes/metadata/containers/ProjectTemplatesContainer.jsx#L83

The error is used when creating the ProjectTemplatesGridView, which will delegate it again to GridView component:
https://github.com/appirio-tech/connect-app/blob/23ad36e35de1e8ff389dc5e83f83ebfda434153d/src/routes/metadata/components/ProjectTemplatesGridView.jsx#L156

The GridView ignores this passed error prop completely. So in the end, it is not needed at all.

[maxceem]

Thanks for details @gets0ul.

So I guess we can just delete lines with error from the actions, right?

As we don't use these errors, we can skip setting them, and error alerts would be still shown, while we would avoid the issue #3180

[gets0ul]

Yes, though we would want it to be set to false as template grid view marks the prop as required.

https://github.com/appirio-tech/connect-app/blob/23ad36e35de1e8ff389dc5e83f83ebfda434153d/src/routes/metadata/components/ProjectTemplatesGridView.jsx#L166

I will submit new commit.
Thanks.

[maxceem]

Yes, though we would want it to be set to false as template grid view marks the prop as required.

Hmm, I'm not sure why we should set it to false. It has default value false, so if we don't update its value in these actions, it would just stay false:
https://github.com/appirio-tech/connect-app/pull/3321/files#diff-14bb32ff0dd23f86057cd28d9b083888R77
Let me know if I miss somethings.

[gets0ul]

Yeah, I checked again. We do not need to as it has default false value in initial state. I will update the PR.
Thanks.

[maxceem]

Thanks for exploring this issue @gets0ul.

Works good without keeping the error in Redux Store.