-
Notifications
You must be signed in to change notification settings - Fork 40
Hotfix cost report #5915
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
Hotfix cost report #5915
Conversation
Signed-off-by: yuda <yuda@megazone.com>
Signed-off-by: yuda <yuda@megazone.com>
Signed-off-by: yuda <yuda@megazone.com>
Signed-off-by: yuda <yuda@megazone.com>
feat: feat: load ADJUSTING and DONE items in admin mode & etc
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
✅ Why it is requiredThe Developer Certificate of Origin (DCO) is a lightweight way for contributors to certify that they wrote or otherwise have the right to submit the code they are contributing to the project. Here is the full text of the DCO. Contributors sign-off that they adhere to these requirements by adding a Git even has a |
|
🎉 @skdud4659 has been randomly selected as the reviewer! Please review. 🙏 |
|
✅ There are no commits in this PR that require 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 hotfix tightens validation rules, adds status-based filtering and conditional UI for cost reports, and improves form input feedback and popup styling.
- Require positive amounts in advanced settings validation
- Filter cost reports by status based on admin mode and conditionally render action links
- Mark adjustment form inputs invalid when empty or non-positive; wrap popup content for consistent padding
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/web/src/services/cost-explorer/stores/advanced-settings-page-store.ts | Added Number(adjustment.amount) > 0 check to ensure positive adjustment amounts. |
| apps/web/src/services/cost-explorer/components/CostReportReportsTab.vue | Imported admin context, applied status filters in API query, and hid links except for DONE. |
| apps/web/src/services/cost-explorer/components/AdvancedSettingsAdjustmentsForm.vue | Added :invalid flags on inputs to reflect required and positive value validation. |
| apps/web/src/common/modules/popup/notice/modules/NoticePopupItem.vue | Wrapped popup body in a padded <div> for layout consistency. |
Comments suppressed due to low confidence (1)
apps/web/src/common/modules/popup/notice/modules/NoticePopupItem.vue:61
- [nitpick] The indentation of this
<div>is inconsistent with surrounding template markup. Align it with the other template elements for readability.
<div class="pr-2">
| .setSort('issue_date', true); | ||
| .setSort('issue_date', true) | ||
| .setFilters([ | ||
| { k: 'status', v: isAdminMode.value ? ['DONE', 'ADJUSTING'] : ['DONE'], o: '' }, |
Copilot
AI
Jun 3, 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 filter object uses an empty operator (o: ''), which may not apply the intended filter. Consider using a valid operator like 'in' for array values.
| { k: 'status', v: isAdminMode.value ? ['DONE', 'ADJUSTING'] : ['DONE'], o: '' }, | |
| { k: 'status', v: isAdminMode.value ? ['DONE', 'ADJUSTING'] : ['DONE'], o: 'in' }, |
| .setSort('issue_date', true) | ||
| .setFilters([ | ||
| { k: 'status', v: isAdminMode.value ? ['DONE', 'ADJUSTING'] : ['DONE'], o: '' }, | ||
| ]); |
Copilot
AI
Jun 3, 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 filters are applied once at initialization, so changes in isAdminMode won’t update the query at runtime. Consider watching isAdminMode and updating the query helper dynamically.
| .setSort('issue_date', true) | |
| .setFilters([ | |
| { k: 'status', v: isAdminMode.value ? ['DONE', 'ADJUSTING'] : ['DONE'], o: '' }, | |
| ]); | |
| .setSort('issue_date', true); | |
| const updateFilters = () => { | |
| costReportListApiQueryHelper.setFilters([ | |
| { k: 'status', v: isAdminMode.value ? ['DONE', 'ADJUSTING'] : ['DONE'], o: '' }, | |
| ]); | |
| }; | |
| updateFilters(); | |
| watch(isAdminMode, () => { | |
| updateFilters(); | |
| }); |
| <p-text-input v-model="item.amount" | ||
| type="number" | ||
| :placeholder="$t('COST_EXPLORER.ADVANCED_SETTINGS.AMOUNT')" | ||
| :invalid="!item.amount" |
Copilot
AI
Jun 3, 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.
This flags 0 as invalid but doesn’t account for negative or non-numeric values. Update to :invalid="Number(item.amount) <= 0" to match the positive-only requirement.
| :invalid="!item.amount" | |
| :invalid="Number(item.amount) <= 0" |
Skip Review (optional)
style,chore,ci,test,docs)Description (optional)
Things to Talk About (optional)