-
Notifications
You must be signed in to change notification settings - Fork 40
feat: apply adjustment to report detail page #5887
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
feat: apply adjustment to report detail page #5887
Conversation
Signed-off-by: yuda <yuda@megazone.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
|
@yuda110 is attempting to deploy a commit to the cloudforet Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 @seungyeoneeee has been randomly selected as the reviewer! Please review. 🙏 |
There was a problem hiding this 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 cost report detail page to compute and display adjustment data alongside existing provider/project breakdowns and updates the UI/styling accordingly.
- Refactored
tableStateto store raw data and derive provider, project, and product tables via computed properties - Introduced adjustment logic (
adjustedProviderData,adjustedProjectData) and updated templates to render adjusted entries - Simplified sections (removed service account table), tweaked styling for adjusted rows
Comments suppressed due to low confidence (1)
apps/web/src/common/pages/CostReportDetailPage.vue:66
- [nitpick] The name
CostReportAdjustedDataAnalyzeResultByProvideris verbose and misaligned with its values (arrays of{product, value}). Consider renaming to something likeProviderAdjustmentDataand updating the type definition to reflect the actual data shape.
type CostReportAdjustedDataAnalyzeResultByProvider = {
| const costByProductTableData = computed<CostReportDataAnalyzeResultByProduct>(() => getConvertedProductTableData(tableState.productRawData)); | ||
| const costByProviderTableData = computed<CostReportDataAnalyzeResult[]>(() => getSortedTableData(tableState.productRawData)); | ||
| const costByProjectTableData = computed<CostReportDataAnalyzeResult[]>(() => tableState.projectRawData.filter((d) => !d.is_adjusted)); | ||
| const adjustedProviderData = computed<CostReportAdjustedDataAnalyzeResultByProvider>(() => { |
Copilot
AI
May 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type annotation CostReportAdjustedDataAnalyzeResultByProvider expects CostReportDataAnalyzeResult[], but the code assigns d.value_sum (an array of {product, value} objects). Adjust the type to match Array<{product: string; value: number}> or wrap the data into CostReportDataAnalyzeResult.
| return `${startDate} ~ ${endDate}`; | ||
| }), | ||
| totalCost: computed<number>(() => sum(tableState.costByProvider.map((d) => d._total_value_sum))), | ||
| totalCost: computed<number>(() => sum(costByProviderTableData.value.map((d) => d._total_value_sum))), |
Copilot
AI
May 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since _total_value_sum is optional, consider coalescing to zero (d._total_value_sum ?? 0) before summing to avoid potential NaN results.
| totalCost: computed<number>(() => sum(costByProviderTableData.value.map((d) => d._total_value_sum))), | |
| totalCost: computed<number>(() => sum(costByProviderTableData.value.map((d) => d._total_value_sum ?? 0))), |
| const adjustedProviderData = computed<CostReportAdjustedDataAnalyzeResultByProvider>(() => { | ||
| const results: CostReportAdjustedDataAnalyzeResultByProvider = {}; | ||
| const adjustedData = tableState.productRawData.filter((d) => d.is_adjusted); | ||
| adjustedData.forEach((d) => { | ||
| results[d.provider] = d.value_sum; | ||
| }); | ||
| return results; | ||
| }); | ||
| const adjustedProjectData = computed<AdjustmentProductData>(() => { | ||
| const adjustedData = tableState.projectRawData.filter((d) => d.is_adjusted); |
Copilot
AI
May 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Both adjustedProviderData and adjustedProjectData filter rawData by is_adjusted. Extract this filtering into a shared helper to reduce duplication and improve readability.
| const adjustedProviderData = computed<CostReportAdjustedDataAnalyzeResultByProvider>(() => { | |
| const results: CostReportAdjustedDataAnalyzeResultByProvider = {}; | |
| const adjustedData = tableState.productRawData.filter((d) => d.is_adjusted); | |
| adjustedData.forEach((d) => { | |
| results[d.provider] = d.value_sum; | |
| }); | |
| return results; | |
| }); | |
| const adjustedProjectData = computed<AdjustmentProductData>(() => { | |
| const adjustedData = tableState.projectRawData.filter((d) => d.is_adjusted); | |
| const getAdjustedData = (data: CostReportDataAnalyzeResult[]) => data.filter((d) => d.is_adjusted); | |
| const adjustedProviderData = computed<CostReportAdjustedDataAnalyzeResultByProvider>(() => { | |
| const results: CostReportAdjustedDataAnalyzeResultByProvider = {}; | |
| const adjustedData = getAdjustedData(tableState.productRawData); | |
| adjustedData.forEach((d) => { | |
| results[d.provider] = d.value_sum; | |
| }); | |
| return results; | |
| }); | |
| const adjustedProjectData = computed<AdjustmentProductData>(() => { | |
| const adjustedData = getAdjustedData(tableState.projectRawData); |
Signed-off-by: yuda <yuda@megazone.com>
Skip Review (optional)
style,chore,ci,test,docs)Description (optional)
Things to Talk About (optional)