Skip to content

fix: reduced unnecessary API calls on workspace features #61169

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
merged 4 commits into from
May 12, 2025
Merged

fix: reduced unnecessary API calls on workspace features #61169

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3141a0f
fix: reduced unnecessary API calls on workspace features
martasudol Apr 30, 2025
05d36d6
ESLint fixes
martasudol Apr 30, 2025
27a9ee4
Merge branch 'main' into reduce-unnecessary-api-calls-on-workspace-fe…
martasudol May 5, 2025
ae0075e
Merge branch 'main' into reduce-unnecessary-api-calls-on-workspace-fe…
martasudol May 12, 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
13 changes: 8 additions & 5 deletions src/pages/workspace/WorkspaceMembersPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,17 @@ function WorkspaceMembersPage({personalDetails, route, policy, currentUserPerson
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [selectedEmployees, policy?.owner, session?.accountID]);

// useFocus would make getWorkspaceMembers get called twice on fresh login because policyEmployee is a dependency of getWorkspaceMembers.
// Fetch members only once when the component mounts, like in Categories page
useEffect(() => {
if (!isFocused) {
setSelectedEmployees([]);
getWorkspaceMembers();
}, [getWorkspaceMembers]);

// Clear selected employees when losing focus
useEffect(() => {
if (isFocused) {
return;
}
getWorkspaceMembers();
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
setSelectedEmployees([]);
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a main merge conflict. @martasudol Can you please raise a quick PR and remove this line

Copy link
Contributor

Choose a reason for hiding this comment

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

@s77rt was this handled? @martasudol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry folks, I don't follow. What's the conflict? I verified and in main it works correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

The setSelectedEmployees[] call should be removed. It has been removed in another PR but this one added it back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, thanks. Will handle it in the moment!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s77rt #62463

}, [isFocused]);

useEffect(() => {
Expand Down
6 changes: 4 additions & 2 deletions src/pages/workspace/reportFields/WorkspaceReportFieldsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {useFocusEffect, useIsFocused} from '@react-navigation/native';
import {useIsFocused} from '@react-navigation/native';
import {Str} from 'expensify-common';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import {ActivityIndicator, View} from 'react-native';
Expand Down Expand Up @@ -101,7 +101,9 @@ function WorkspaceReportFieldsPage({

const hasVisibleReportField = Object.values(filteredPolicyFieldList).some((reportField) => reportField.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || isOffline);

useFocusEffect(fetchReportFields);
useEffect(() => {
fetchReportFields();
}, [fetchReportFields]);

useEffect(() => {
if (isFocused) {
Expand Down