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

[CP Staging] Fix: No notification when closing account #28800

Merged
merged 5 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
4 changes: 4 additions & 0 deletions src/libs/Middleware/HandleUnusedOptimisticID.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe
const responseOnyxData = response?.onyxData ?? [];
responseOnyxData.forEach((onyxData) => {
const key = onyxData.key;
if (!key) {
return;
}

if (!key.startsWith(ONYXKEYS.COLLECTION.REPORT)) {
Comment on lines +13 to 17
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI optional chaining is allowed in TS:

Suggested change
if (!key) {
return;
}
if (!key.startsWith(ONYXKEYS.COLLECTION.REPORT)) {
if (!key?.startsWith(ONYXKEYS.COLLECTION.REPORT)) {

return;
}
Expand Down
86 changes: 44 additions & 42 deletions src/libs/Middleware/SaveResponseInOnyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,50 +14,52 @@ const requestsToIgnoreLastUpdateID = ['OpenApp', 'ReconnectApp', 'GetMissingOnyx
* @returns {Promise}
*/
function SaveResponseInOnyx(requestResponse, request) {
return requestResponse.then((response) => {
// Make sure we have response data (i.e. response isn't a promise being passed down to us by a failed retry request and response undefined)
if (!response) {
return;
}
const onyxUpdates = response.onyxData;

// Sometimes we call requests that are successfull but they don't have any response or any success/failure data to set. Let's return early since
// we don't need to store anything here.
if (!onyxUpdates && !request.successData && !request.failureData) {
return Promise.resolve(response);
}

// If there is an OnyxUpdate for using memory only keys, enable them
_.find(onyxUpdates, ({key, value}) => {
if (key !== ONYXKEYS.IS_USING_MEMORY_ONLY_KEYS || !value) {
return false;
return requestResponse
Copy link
Contributor

@abdulrahuman5196 abdulrahuman5196 Oct 4, 2023

Choose a reason for hiding this comment

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

@tienifr Why are having this much diff here? I know we are only adding catch to this?

Can we remove the unwanted diff? Any specific reason to have?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's prettier diff. Maybe because I added the catch phrase thus everything was shift down 1 line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've already compared line by line and verified that they're the same. You can check it yourself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you. I also verified. No changes.

.then((response) => {
// Make sure we have response data (i.e. response isn't a promise being passed down to us by a failed retry request and response undefined)
if (!response) {
return;
}
const onyxUpdates = response.onyxData;

MemoryOnlyKeys.enable();
return true;
});

const responseToApply = {
type: CONST.ONYX_UPDATE_TYPES.HTTPS,
lastUpdateID: Number(response.lastUpdateID || 0),
previousUpdateID: Number(response.previousUpdateID || 0),
request,
response,
};

if (_.includes(requestsToIgnoreLastUpdateID, request.command) || !OnyxUpdates.doesClientNeedToBeUpdated(Number(response.previousUpdateID || 0))) {
return OnyxUpdates.apply(responseToApply);
}

// Save the update IDs to Onyx so they can be used to fetch incremental updates if the client gets out of sync from the server
OnyxUpdates.saveUpdateInformation(responseToApply);

// Ensure the queue is paused while the client resolves the gap in onyx updates so that updates are guaranteed to happen in a specific order.
return Promise.resolve({
...response,
shouldPauseQueue: true,
});
});
// Sometimes we call requests that are successfull but they don't have any response or any success/failure data to set. Let's return early since
// we don't need to store anything here.
if (!onyxUpdates && !request.successData && !request.failureData) {
return Promise.resolve(response);
}

// If there is an OnyxUpdate for using memory only keys, enable them
_.find(onyxUpdates, ({key, value}) => {
if (key !== ONYXKEYS.IS_USING_MEMORY_ONLY_KEYS || !value) {
return false;
}

MemoryOnlyKeys.enable();
return true;
});

const responseToApply = {
type: CONST.ONYX_UPDATE_TYPES.HTTPS,
lastUpdateID: Number(response.lastUpdateID || 0),
previousUpdateID: Number(response.previousUpdateID || 0),
request,
response,
};

if (_.includes(requestsToIgnoreLastUpdateID, request.command) || !OnyxUpdates.doesClientNeedToBeUpdated(Number(response.previousUpdateID || 0))) {
return OnyxUpdates.apply(responseToApply);
}

// Save the update IDs to Onyx so they can be used to fetch incremental updates if the client gets out of sync from the server
OnyxUpdates.saveUpdateInformation(responseToApply);

// Ensure the queue is paused while the client resolves the gap in onyx updates so that updates are guaranteed to happen in a specific order.
return Promise.resolve({
...response,
shouldPauseQueue: true,
});
})
.catch(console.error);
Copy link
Contributor

Choose a reason for hiding this comment

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

Kindly add small meaningful tags with error, like Got exception while saving response in onyx @tienifr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

}

export default SaveResponseInOnyx;
Loading