Frontend fix#97
Conversation
Reviewer's GuideImplements global client-side redirection on forbidden/conflict errors, standardizes backend group authorization responses to 403 Forbidden, aligns tests accordingly, and corrects hook dependencies in bill-splitting components. Sequence diagram for frontend error handling and redirection on 403/409 responsessequenceDiagram
participant User as actor User
participant Frontend as Frontend (React)
participant Backend as Backend (Spring)
User->>Frontend: Initiates API request
Frontend->>Backend: Sends request
Backend-->>Frontend: Responds with 403/409 error
Frontend->>Frontend: Axios interceptor detects 403/409
Frontend->>User: Redirects to /trips page
Class diagram for updated UserIsNotInGroupException and exception handlerclassDiagram
class UserIsNotInGroupException {
<<exception>>
+UserIsNotInGroupException(String message)
}
class FareShareResponseEntityExceptionHandler {
+userIsNotInGroupException(Exception ex, WebRequest request): ResponseEntity<ErrorDetails>
}
UserIsNotInGroupException <|-- RuntimeException
FareShareResponseEntityExceptionHandler ..> UserIsNotInGroupException : handles
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ssjablonski, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request refines the application's error handling by updating the backend to use the more semantically appropriate 403 Forbidden HTTP status code for scenarios where a user lacks necessary group permissions. Concurrently, it improves frontend robustness by correctly managing useEffect dependencies and introduces a global mechanism to gracefully handle and redirect users upon encountering specific authorization-related errors from the API.
Highlights
- Backend Error Handling Refinement: Corrected the HTTP status code for
UserIsNotInGroupExceptionfrom409 Conflictto403 Forbiddenacross the exception handler and the exception definition itself. This change aligns with REST best practices, as403 Forbiddenis semantically more appropriate for authorization failures where a user lacks the necessary permissions. - Frontend
useEffectDependency Fixes: Enhanced frontend stability by addingtripto the dependency arrays ofuseEffecthooks inpage.tsx(line 60) andExpensesDashboard.tsx(line 37). This ensures that data fetching functions are correctly re-executed when thetripobject changes, preventing stale data or infinite re-renders. - Global Frontend Error Redirection: Implemented a new global Axios response interceptor in
axiosInstance.tsto automatically redirect users to the/tripspage when the application receives403 Forbiddenor409 ConflictHTTP status codes from the backend. This improves user experience by gracefully handling unauthorized access or conflicting states.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @ssjablonski - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if ( | ||
| error.response && | ||
| (error.response.status === 403 || error.response.status === 409) | ||
| ) { | ||
| if (typeof window !== 'undefined') { | ||
| window.location.href = '/trips'; | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion (code-quality): Merge nested if conditions (merge-nested-ifs)
| if ( | |
| error.response && | |
| (error.response.status === 403 || error.response.status === 409) | |
| ) { | |
| if (typeof window !== 'undefined') { | |
| window.location.href = '/trips'; | |
| } | |
| } | |
| if (error.response && | |
| (error.response.status === 403 || error.response.status === 409) && typeof window !== 'undefined') { | |
| window.location.href = '/trips'; | |
| } | |
Explanation
Reading deeply nested conditional code is confusing, since you have to keep track of whichconditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two
if conditions can be combined usingand is an easy win.
Code Coverage
|
There was a problem hiding this comment.
Code Review
This pull request improves backend API semantics and frontend stability. The change from HTTP 409 to 403 for authorization errors is semantically correct. The fixes to useEffect dependencies prevent infinite loops. The review includes suggestions for the frontend code, specifically addressing the global Axios interceptor handling HTTP 409 errors and aligning the useEffect hooks with React's best practices.
| axiosInstance.interceptors.response.use( | ||
| (response) => response, | ||
| (error) => { | ||
| if ( | ||
| error.response && | ||
| (error.response.status === 403 || error.response.status === 409) | ||
| ) { | ||
| if (typeof window !== 'undefined') { | ||
| window.location.href = '/trips'; | ||
| } | ||
| } | ||
| return Promise.reject(error); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
The global response interceptor redirects users to /trips for 403 or 409 status codes. However, a 409 status code is used for various conflict scenarios where the user should see an inline error message on the current page. Remove the 409 status check from this interceptor.1
axiosInstance.interceptors.response.use(
(response) => response,
(error) => {
if (error.response && error.response.status === 403) {
if (typeof window !== 'undefined') {
window.location.href = '/trips';
}
}
return Promise.reject(error);
},
);Style Guide References
Footnotes
| useEffect(() => { | ||
| if (!trip || !trip.id) return; | ||
| fetchData(); | ||
| }); | ||
| }, [trip]); |
There was a problem hiding this comment.
To adhere to React best practices and prevent potential issues with stale closures, wrap the fetchData function in useCallback and include it in the dependency array.1
Style Guide References
Footnotes
| useEffect(() => { | ||
| fetchGroupExpenses(); | ||
| }, []); | ||
| }, [trip]); |
There was a problem hiding this comment.
To fully align with React best practices, wrap fetchGroupExpenses in a useCallback hook with trip as its dependency, and then add fetchGroupExpenses to the dependency array of this useEffect.1
const fetchGroupExpenses = useCallback(async () => {
// ... function body
}, [trip]);
useEffect(() => {
fetchGroupExpenses();
}, [fetchGroupExpenses]);Style Guide References
Footnotes
2 participants
Summary by Sourcery
Standardize “user not in group” errors to use Forbidden status, add frontend redirection on 403/409 responses, and fix React effect dependencies to avoid infinite updates.
Enhancements:
Tests: