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.

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

Jump to bottom

Remove error when all workspace is deleted #29781

Merged
merged 6 commits into from
Nov 3, 2023
Merged

Remove error when all workspace is deleted #29781

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d36a762
remove error when all workspace is deleted
dukenv0307 Oct 17, 2023
1dff6dd
using lodash instead of underscore
dukenv0307 Oct 20, 2023
165308b
Merge branch 'main' into fix/29235
dukenv0307 Oct 20, 2023
b8a97da
remove un-use variable
dukenv0307 Oct 24, 2023
9f606ed
merge main
dukenv0307 Nov 2, 2023
9b24175
fix lint
dukenv0307 Nov 2, 2023
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
87 changes: 57 additions & 30 deletions src/libs/actions/Policy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ Onyx.connect({
callback: (val) => (allPersonalDetails = val),
});

let reimbursementAccount;
Onyx.connect({
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
callback: (val) => (reimbursementAccount = val),
});

let allRecentlyUsedCategories = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_CATEGORIES,
Expand All @@ -81,6 +87,36 @@ function updateLastAccessedWorkspace(policyID) {
Onyx.set(ONYXKEYS.LAST_ACCESSED_WORKSPACE_POLICY_ID, policyID);
}

/**
* Check if the user has any active free policies (aka workspaces)
*
* @param {Array} policies
* @returns {Boolean}
*/
function hasActiveFreePolicy(policies) {
const adminFreePolicies = _.filter(policies, (policy) => policy && policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN);

if (adminFreePolicies.length === 0) {
return false;
}

if (_.some(adminFreePolicies, (policy) => !policy.pendingAction)) {
return true;
}

if (_.some(adminFreePolicies, (policy) => policy.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD)) {
return true;
}

if (_.some(adminFreePolicies, (policy) => policy.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)) {
return false;
}

// If there are no add or delete pending actions the only option left is an update
// pendingAction, in which case we should return true.
return true;
}

/**
* Delete the workspace
*
Expand All @@ -89,6 +125,10 @@ function updateLastAccessedWorkspace(policyID) {
* @param {String} policyName
*/
function deleteWorkspace(policyID, reports, policyName) {
const filteredPolicies = _.filter(allPolicies, (policy) => policy.id !== policyID);
Copy link
Contributor

Choose a reason for hiding this comment

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

please use lodash instead of underscore for the new code.

const hasActivePolicy = hasActiveFreePolicy(filteredPolicies);
Copy link
Contributor

Choose a reason for hiding this comment

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

If this var is only used 1 place, we can call it directly in the place it used to avoid headaches in naming

const oldReimbursementAccount = reimbursementAccount;
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to assign to a temporary var oldReimbursementAccount here?

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 think we need it to store the old value. The current value can be changed after the optimisticData is merged in Onyx.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, but we already built failureData below, so I think we don't need to cache the oldReimbursementAccount here. Could you help to verify it again? Don't hesitate to let me know if I'm wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right. Updated.


const optimisticData = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand Down Expand Up @@ -125,6 +165,18 @@ function deleteWorkspace(policyID, reports, policyName) {
value: optimisticReportActions,
};
}),

...(!hasActivePolicy
? [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
value: {
errors: null,
},
},
]
: []),
];

// Restore the old report stateNum and statusNum
Expand All @@ -139,6 +191,11 @@ function deleteWorkspace(policyID, reports, policyName) {
oldPolicyName,
},
})),
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.REIMBURSEMENT_ACCOUNT,
value: oldReimbursementAccount,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should restore the errors value only.

},
];

// We don't need success data since the push notification will update
Expand All @@ -162,36 +219,6 @@ function isAdminOfFreePolicy(policies) {
return _.some(policies, (policy) => policy && policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN);
}

/**
* Check if the user has any active free policies (aka workspaces)
*
* @param {Array} policies
* @returns {Boolean}
*/
function hasActiveFreePolicy(policies) {
const adminFreePolicies = _.filter(policies, (policy) => policy && policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN);

if (adminFreePolicies.length === 0) {
return false;
}

if (_.some(adminFreePolicies, (policy) => !policy.pendingAction)) {
return true;
}

if (_.some(adminFreePolicies, (policy) => policy.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD)) {
return true;
}

if (_.some(adminFreePolicies, (policy) => policy.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)) {
return false;
}

// If there are no add or delete pending actions the only option left is an update
// pendingAction, in which case we should return true.
return true;
}

/**
* Remove the passed members from the policy employeeList
*
Expand Down
Loading