Skip to content

fix: ensure order is preserved in Promise.all of adjustments #5922

Merged
yuda110 merged 2 commits intomasterfrom
hotfix-cost-report
Jun 4, 2025
Merged

fix: ensure order is preserved in Promise.all of adjustments #5922
yuda110 merged 2 commits intomasterfrom
hotfix-cost-report

Conversation

@yuda110
Copy link
Member

@yuda110 yuda110 commented Jun 4, 2025

Skip Review (optional)

  • Minor changes that don't affect the functionality (e.g. style, chore, ci, test, docs)
  • Previously reviewed in feature branch, further review is not mandatory
  • Self-merge allowed for solo developers or urgent changes

Description (optional)

Things to Talk About (optional)

Signed-off-by: yuda <yuda@megazone.com>
@vercel
Copy link

vercel bot commented Jun 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cost-report ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 4, 2025 2:43am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
console ⬜️ Ignored (Inspect) Visit Preview Jun 4, 2025 2:43am
web-storybook ⬜️ Ignored (Inspect) Visit Preview Jun 4, 2025 2:43am

@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2025

🎉 @skdud4659 has been randomly selected as the reviewer! Please review. 🙏

@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2025

✅ There are no commits in this PR that require review.

@github-actions github-actions bot requested a review from skdud4659 June 4, 2025 02:36
@yuda110 yuda110 removed the request for review from skdud4659 June 4, 2025 02:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the processing of adjustment policies and adjustments to enforce sequential execution, thereby ensuring order preservation.

  • Sequentially processes policy updates/creations using a promise chain
  • Processes adjustments in order via a nested promise chain

Comment on lines +225 to +245
await formPolicies.value.reduce(async (promise, policy, idx) => {
await promise;
if (policy.id.startsWith('rap-')) {
return updateAdjustmentPolicy(policy, idx);
}
return createAdjustmentPolicy(policy, idx);
});
await Promise.all(policyPromises);
}, Promise.resolve());

// CUD Adjustment
await deleteAdjustment(deletedPolicyIds);
const adjustmentPromises = formPolicies.value.flatMap(async (policy) => {
await formPolicies.value.reduce(async (promise, policy) => {
await promise;
const adjustments = formAdjustments.value.filter((adjustment) => adjustment.policyId === policy.id);
return adjustments.map(async (adjustment, idx) => {
return adjustments.reduce(async (adjPromise, adjustment, idx) => {
await adjPromise;
if (adjustment.id.startsWith('ra-')) {
return updateAdjustment(adjustment, idx);
}
return createAdjustment(adjustment, idx);
});
});
const resolvedAdjustments = await Promise.all(adjustmentPromises);
await Promise.all(resolvedAdjustments.flat());
}, Promise.resolve());
}, Promise.resolve());
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Using the reduce method for chaining promises can be less intuitive; consider using an explicit for-of loop for sequential processing to improve readability and maintainability.

Copilot uses AI. Check for mistakes.
Comment on lines +225 to +244
await formPolicies.value.reduce(async (promise, policy, idx) => {
await promise;
if (policy.id.startsWith('rap-')) {
return updateAdjustmentPolicy(policy, idx);
}
return createAdjustmentPolicy(policy, idx);
});
await Promise.all(policyPromises);
}, Promise.resolve());

// CUD Adjustment
await deleteAdjustment(deletedPolicyIds);
const adjustmentPromises = formPolicies.value.flatMap(async (policy) => {
await formPolicies.value.reduce(async (promise, policy) => {
await promise;
const adjustments = formAdjustments.value.filter((adjustment) => adjustment.policyId === policy.id);
return adjustments.map(async (adjustment, idx) => {
return adjustments.reduce(async (adjPromise, adjustment, idx) => {
await adjPromise;
if (adjustment.id.startsWith('ra-')) {
return updateAdjustment(adjustment, idx);
}
return createAdjustment(adjustment, idx);
});
});
const resolvedAdjustments = await Promise.all(adjustmentPromises);
await Promise.all(resolvedAdjustments.flat());
}, Promise.resolve());
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Using reduce to iterate over adjustments for sequential promise resolution may obscure the intent; a for-of loop might provide clearer control flow, making the code easier to understand and maintain.

Copilot uses AI. Check for mistakes.
Signed-off-by: yuda <yuda@megazone.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jun 4, 2025

✅ There are no commits in this PR that require review.

Copy link
Member

@skdud4659 skdud4659 left a comment

Choose a reason for hiding this comment

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

lgtm!

@yuda110 yuda110 merged commit ca063a4 into master Jun 4, 2025
13 checks passed
@yuda110 yuda110 deleted the hotfix-cost-report branch June 4, 2025 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants