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.

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

Replace duplicated openReports #51093

Merged
merged 2 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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: 11 additions & 2 deletions src/libs/API/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,23 +208,32 @@ function paginate<TRequestType extends typeof CONST.API_REQUEST_TYPE.MAKE_REQUES
onyxData: OnyxData,
config: PaginationConfig,
): Promise<Response | void>;
function paginate<TRequestType extends typeof CONST.API_REQUEST_TYPE.READ | typeof CONST.API_REQUEST_TYPE.WRITE, TCommand extends CommandOfType<TRequestType>>(
function paginate<TRequestType extends typeof CONST.API_REQUEST_TYPE.READ, TCommand extends CommandOfType<TRequestType>>(
type: TRequestType,
command: TCommand,
apiCommandParameters: ApiRequestCommandParameters[TCommand],
onyxData: OnyxData,
config: PaginationConfig,
): void;
function paginate<TRequestType extends typeof CONST.API_REQUEST_TYPE.WRITE, TCommand extends CommandOfType<TRequestType>>(
type: TRequestType,
command: TCommand,
apiCommandParameters: ApiRequestCommandParameters[TCommand],
onyxData: OnyxData,
config: PaginationConfig,
conflictResolver?: RequestConflictResolver,
): void;
function paginate<TRequestType extends ApiRequestType, TCommand extends CommandOfType<TRequestType>>(
type: TRequestType,
command: TCommand,
apiCommandParameters: ApiRequestCommandParameters[TCommand],
onyxData: OnyxData,
config: PaginationConfig,
conflictResolver: RequestConflictResolver = {},
): Promise<Response | void> | void {
Log.info('[API] Called API.paginate', false, {command, ...apiCommandParameters});
const request: PaginatedRequest = {
...prepareRequest(command, type, apiCommandParameters, onyxData),
...prepareRequest(command, type, apiCommandParameters, onyxData, conflictResolver),
...config,
...{
isPaginated: true,
Expand Down
4 changes: 2 additions & 2 deletions src/libs/actions/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ function openApp() {
return getPolicyParamsForOpenOrReconnect().then((policyParams: PolicyParamsForOpenOrReconnect) => {
const params: OpenAppParams = {enablePriorityModeFilter: true, ...policyParams};
return API.write(WRITE_COMMANDS.OPEN_APP, params, getOnyxDataForOpenOrReconnect(true), {
checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, WRITE_COMMANDS.OPEN_APP),
checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, (request) => request.command === WRITE_COMMANDS.OPEN_APP),
});
});
}
Expand Down Expand Up @@ -287,7 +287,7 @@ function reconnectApp(updateIDFrom: OnyxEntry<number> = 0) {
}

API.write(WRITE_COMMANDS.RECONNECT_APP, params, getOnyxDataForOpenOrReconnect(), {
checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, WRITE_COMMANDS.RECONNECT_APP),
checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, (request) => request.command === WRITE_COMMANDS.RECONNECT_APP),
});
});
}
Expand Down
6 changes: 5 additions & 1 deletion src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject';
import * as CachedPDFPaths from './CachedPDFPaths';
import * as Modal from './Modal';
import navigateFromNotification from './navigateFromNotification';
import resolveDuplicationConflictAction from './RequestConflictUtils';
import * as Session from './Session';
import * as Welcome from './Welcome';
import * as OnboardingFlow from './Welcome/OnboardingFlow';
Expand Down Expand Up @@ -977,7 +978,10 @@ function openReport(
});
} else {
// eslint-disable-next-line rulesdir/no-multiple-api-calls
API.paginate(CONST.API_REQUEST_TYPE.WRITE, WRITE_COMMANDS.OPEN_REPORT, parameters, {optimisticData, successData, failureData}, paginationConfig);
API.paginate(CONST.API_REQUEST_TYPE.WRITE, WRITE_COMMANDS.OPEN_REPORT, parameters, {optimisticData, successData, failureData}, paginationConfig, {
checkAndFixConflictingRequest: (persistedRequests) =>
resolveDuplicationConflictAction(persistedRequests, (request) => request.command === WRITE_COMMANDS.OPEN_REPORT && request.data?.reportID === reportID),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also check if the previous request and the currentRequest if the emailList is same. Ref: #51651

});
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/libs/actions/RequestConflictUtils.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import type {WriteCommand} from '@libs/API/types';
import type OnyxRequest from '@src/types/onyx/Request';
import type {ConflictActionData} from '@src/types/onyx/Request';

type RequestMatcher = (request: OnyxRequest) => boolean;

/**
* Resolves duplication conflicts between persisted requests and a given command.
*
* This method checks if a specific command exists within a list of persisted requests.
* - If the command is not found, it suggests adding the command to the list, indicating a 'push' action.
* - If the command is found, it suggests updating the existing entry, indicating a 'replace' action at the found index.
*/
gedu marked this conversation as resolved.
Show resolved Hide resolved
function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], commandToFind: WriteCommand): ConflictActionData {
const index = persistedRequests.findIndex((request) => request.command === commandToFind);
function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], requestMatcher: RequestMatcher): ConflictActionData {
const index = persistedRequests.findIndex(requestMatcher);
if (index === -1) {
return {
conflictAction: {
Expand Down
51 changes: 51 additions & 0 deletions tests/actions/ReportTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {afterEach, beforeAll, beforeEach, describe, expect, it} from '@jest/glob
import {toZonedTime} from 'date-fns-tz';
import Onyx from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx';
import {WRITE_COMMANDS} from '@libs/API/types';
import CONST from '@src/CONST';
import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager';
import * as PersistedRequests from '@src/libs/actions/PersistedRequests';
Expand Down Expand Up @@ -757,4 +758,54 @@ describe('actions/Report', () => {
expect(reportActionReaction?.[EMOJI.name].users[TEST_USER_ACCOUNT_ID]).toBeUndefined();
});
});

it.only('should send only one OpenReport, replacing any extra ones with same reportIDs', async () => {
global.fetch = TestHelper.getGlobalFetchMock();

const REPORT_ID = '1';

await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true});
await waitForBatchedUpdates();

for (let i = 0; i < 5; i++) {
Report.openReport(REPORT_ID, undefined, ['test@user.com'], {
isOptimisticReport: true,
reportID: REPORT_ID,
});
}

expect(PersistedRequests.getAll().length).toBe(1);

await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
await waitForBatchedUpdates();

TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.OPEN_REPORT, 1);
});

it.only('should replace duplicate OpenReport commands with the same reportID', async () => {
global.fetch = TestHelper.getGlobalFetchMock();

const REPORT_ID = '1';

await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true});
await waitForBatchedUpdates();

for (let i = 0; i < 8; i++) {
let reportID = REPORT_ID;
if (i > 4) {
reportID = `${i}`;
}
Report.openReport(reportID, undefined, ['test@user.com'], {
isOptimisticReport: true,
reportID: REPORT_ID,
});
}

expect(PersistedRequests.getAll().length).toBe(4);

await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
await waitForBatchedUpdates();

TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.OPEN_REPORT, 4);
});
});
18 changes: 16 additions & 2 deletions tests/unit/RequestConflictUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ describe('RequestConflictUtils', () => {
it.each([['OpenApp'], ['ReconnectApp']])('resolveDuplicationConflictAction when %s do not exist in the queue should push %i', (command) => {
const persistedRequests = [{command: 'OpenReport'}, {command: 'AddComment'}, {command: 'CloseAccount'}];
const commandToFind = command as WriteCommand;
const result = resolveDuplicationConflictAction(persistedRequests, commandToFind);
const result = resolveDuplicationConflictAction(persistedRequests, (request) => request.command === commandToFind);
expect(result).toEqual({conflictAction: {type: 'push'}});
});

Expand All @@ -15,7 +15,21 @@ describe('RequestConflictUtils', () => {
])('resolveDuplicationConflictAction when %s exist in the queue should replace at index %i', (command, index) => {
const persistedRequests = [{command: 'OpenApp'}, {command: 'AddComment'}, {command: 'ReconnectApp'}];
const commandToFind = command as WriteCommand;
const result = resolveDuplicationConflictAction(persistedRequests, commandToFind);
const result = resolveDuplicationConflictAction(persistedRequests, (request) => request.command === commandToFind);
expect(result).toEqual({conflictAction: {type: 'replace', index}});
});

it('replaces the first OpenReport command with reportID 1 in case of duplication conflict', () => {
const persistedRequests = [
{command: 'OpenApp'},
{command: 'AddComment'},
{command: 'OpenReport', data: {reportID: 1}},
{command: 'OpenReport', data: {reportID: 2}},
{command: 'OpenReport', data: {reportID: 3}},
{command: 'ReconnectApp'},
];
const reportID = 1;
const result = resolveDuplicationConflictAction(persistedRequests, (request) => request.command === 'OpenReport' && request.data?.reportID === reportID);
expect(result).toEqual({conflictAction: {type: 'replace', index: 2}});
});
});
Loading