Skip to content

Frontend fix #97

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

Merged
ssjablonski merged 5 commits into from
Jun 24, 2025
Merged

Frontend fix #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f7b4283
Poprawienie useEffect by zniwelować pętlę requestow
ssjablonski Jun 24, 2025
2170ada
Merge branch 'main' into frontend-fix
ssjablonski Jun 24, 2025
0e10250
Dodano interceptor przekierowujący na strone glowna przy requescie z …
ssjablonski Jun 24, 2025
4177fb9
Usuniecie console.log
ssjablonski Jun 24, 2025
ee458ae
change status code when user is not in group from conflict to forbidden
szymongrabski Jun 24, 2025
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
2 changes: 1 addition & 1 deletion ...d/src/main/java/share/fare/backend/exception/FareShareResponseEntityExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public final ResponseEntity<ErrorDetails> userAlreadyInGroupException(Exception
@ExceptionHandler(UserIsNotInGroupException.class)
public final ResponseEntity<ErrorDetails> userIsNotInGroupException(Exception ex, WebRequest request) {
ErrorDetails errorDetails = new ErrorDetails(LocalDateTime.now(), ex.getMessage(), request.getDescription(false), ex.getStackTrace().length);
return new ResponseEntity<>(errorDetails, HttpStatus.CONFLICT);
return new ResponseEntity<>(errorDetails, HttpStatus.FORBIDDEN);
}

@ExceptionHandler(UserIsNotGroupOwner.class)
Expand Down
2 changes: 1 addition & 1 deletion backend/src/main/java/share/fare/backend/exception/UserIsNotInGroupException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ResponseStatus;

@ResponseStatus(HttpStatus.CONFLICT)
@ResponseStatus(HttpStatus.FORBIDDEN)
public class UserIsNotInGroupException extends RuntimeException {
public UserIsNotInGroupException(String message) {
super(message);
Expand Down
2 changes: 1 addition & 1 deletion backend/src/test/java/share/fare/backend/controller/GroupChatControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void getRecentMessagesUserNotInGroupTest() throws Exception {
.param("size", "20")
.with(user(testSecondUser))
)
.andExpect(status().isConflict())
.andExpect(status().isForbidden())
.andExpect(jsonPath("$.message").value("User " + testSecondUser.getId() + " is not in group " + testGroup.getName()));
}
}
2 changes: 1 addition & 1 deletion backend/src/test/java/share/fare/backend/controller/GroupInvitationControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ void sendGroupInvitationSenderNotInGroupTest() throws Exception {
.principal(() -> String.valueOf(testUser.getId()))
.param("receiverId", String.valueOf(testSecondUser.getId()))
.param("groupId", String.valueOf(testGroup.getId())))
.andExpect(status().isConflict())
.andExpect(status().isForbidden())
.andExpect(jsonPath("$.message").value("Sender does not belong to group."));
}

Expand Down
2 changes: 1 addition & 1 deletion backend/src/test/java/share/fare/backend/controller/GroupMembershipControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ void removeNonMemberTest() throws Exception {
mockMvc.perform(delete(URI + "/{userId}", testGroup.getId(), newUser.getId())
.with(user(testUser))
.principal(() -> String.valueOf(testUser.getId())))
.andExpect(status().isConflict());
.andExpect(status().isForbidden());
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion backend/src/test/java/share/fare/backend/controller/VoteControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void addVoteNotGroupMemberTest() throws Exception {
.content(objectMapper.writeValueAsString(voteRequest))
.with(user(anotherUser))
.principal(() -> String.valueOf(anotherUser.getId())))
.andExpect(status().isConflict())
.andExpect(status().isForbidden())
.andExpect(jsonPath("$.message").value("User with ID: " + anotherUser.getId() +" is not a member of the group"));
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/app/(root)/trips/[id]/bill-splitting/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export default function GroupExpensesPage() {
useEffect(() => {
if (!trip || !trip.id) return;
fetchData();
});
}, [trip]);
Comment on lines 57 to +60

@gemini-code-assist gemini-code-assist Bot Jun 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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

  1. Using useCallback ensures that the function instance is stable across renders unless its dependencies change, preventing unnecessary re-renders and potential issues with stale closures. (link) ↩


const fetchData = async () => {
try {
Expand Down
2 changes: 1 addition & 1 deletion frontend/components/bill-splitting/ExpensesDashboard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export default function ExpansesDashboard({ trip }: { trip: Group }) {

useEffect(() => {
fetchGroupExpenses();
}, []);
}, [trip]);
Comment on lines 35 to +37

@gemini-code-assist gemini-code-assist Bot Jun 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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

  1. Using useCallback ensures that the function instance is stable across renders unless its dependencies change, preventing unnecessary re-renders and potential issues with stale closures. (link) ↩


if (error) {
return (
Expand Down
15 changes: 15 additions & 0 deletions frontend/lib/axiosInstance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,19 @@ axiosInstance.interceptors.request.use(
},
);

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';
}
}
Comment on lines +26 to +33

@sourcery-ai sourcery-ai Bot Jun 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (code-quality): Merge nested if conditions (merge-nested-ifs)

Suggested change
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';
}


ExplanationReading deeply nested conditional code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two if conditions can be combined using
and is an easy win.

return Promise.reject(error);
},
);
Comment on lines +23 to +36

@gemini-code-assist gemini-code-assist Bot Jun 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

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

  1. 409 status code should be handled locally, not globally. (link) ↩


export default axiosInstance;
Loading