Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding proper error message for Camera config page #7696

Closed
wants to merge 6 commits into from
Closed

Adding proper error message for Camera config page #7696

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b9ec5dd
changed notification for operate assets
hrit2773 Apr 24, 2024
9573351
Merge branch 'coronasafe:develop' into notification
hrit2773 Apr 24, 2024
36cbe3f
wording update
hrit2773 Apr 24, 2024
c93a96d
Wrong Middleware Hostname wording
hrit2773 Apr 24, 2024
7e4cc74
Merge branch 'develop' into notification
hrit2773 Apr 27, 2024
957ffbf
Merge branch 'develop' into notification
nihal467 May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/Redux/fireRequest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,15 @@ export const fireRequest = (
msg: error.response.data.detail,
});
} else {
Notification.Error({
msg: "Something went wrong...!",
});
if (key === "operateAsset") {
Notification.Error({
msg: "Wrong Middleware Hostname...!",
});
} else {
Comment on lines +161 to +165
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a hack. Also, I thought we moved to useQuery. Please check src/Utils/request/useQuery.ts. If the middleware API calls still use fireRequest, please consider refactoring it to useQuery.

cc: @rithviknishad

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@khavinshankar the middleware api call uses fire request.. of course I can change it to useQuery if @rithviknishad agrees to it

Copy link
Member

Choose a reason for hiding this comment

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

The existing usages of operateAsset was hard to switch to using useQuery/request as there were places where we were passing the redux's dispatch as params to the functions inside it.

However, switching to newer camera feed components would solve the issue as it is not using the deprecated operateAsset. (Eg: #7654)

The newer camera feed components handles the error messages as expected and wouldn't have this underlying issue in the first place.

@hrit2773 Do you want to switch to using the newer camera feed components in the asset config page? If yes, you can make a separate PR.

Notification.Error({
msg: "Something went wrong...!",
});
}
}
if (error.response.status === 429) {
return error.response;
Expand Down
Loading