Skip to content

Conversation

@jinyoungmoonDEV
Copy link
Contributor

Category

  • New feature
  • Bug fix
  • Improvement
  • Refactor
  • etc

Description

  • fix get cost_report_data query in adjustment logic

Signed-off-by: jinyoungmoonDEV <moonjinyoung.dev@gmail.com>
Signed-off-by: jinyoungmoonDEV <moonjinyoung.dev@gmail.com>
@jinyoungmoonDEV jinyoungmoonDEV requested a review from Copilot June 5, 2025 06:15
@jinyoungmoonDEV jinyoungmoonDEV merged commit 7c1f049 into cloudforet-io:master Jun 5, 2025
3 checks passed
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 corrects the default status filter and aligns report data creation logic, while extending adjustment policy filtering to include service account scopes and cost report IDs.

  • Simplify and fix default 'DONE' status filter in list method
  • Update is_confirmed flag to use the 'DONE' status
  • Enhance adjustment policy applier with SERVICE_ACCOUNT scope and include cost_report_id in filters

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/spaceone/cost_analysis/service/cost_report_serivce.py Apply default DONE status filter using a concise any check
src/spaceone/cost_analysis/service/cost_report_data_service.py Update is_confirmed logic to reflect the DONE status
src/spaceone/cost_analysis/manager/cost_report/adjustment_policy_applier.py Add service_account_id handling, new SERVICE_ACCOUNT scope logic, and cost_report_id filter
Comments suppressed due to low confidence (3)

src/spaceone/cost_analysis/manager/cost_report/adjustment_policy_applier.py:209

  • The new SERVICE_ACCOUNT scope logic isn't covered by existing tests; add unit tests to verify policy applicability and the resulting query filters for service account scenarios.
elif scope == "SERVICE_ACCOUNT":

src/spaceone/cost_analysis/service/cost_report_serivce.py:206

  • The condition checks params.status before applying the default status filter, which inverts the original logic and may skip adding the 'DONE' filter when no status is provided. Remove the params.status check or change it to simply if not any(...) so the default 'DONE' filter is always applied when missing.
if params.status and not any(_filter.get("k") == "status" for _filter in query.get("filter", [])):

src/spaceone/cost_analysis/manager/cost_report/adjustment_policy_applier.py:199

  • If policy.scope is not provided or is outside the expected values, the method will return False and exclude valid policies. Add a default branch or validate scope earlier to preserve original behavior when no scope is set.
scope = policy.get("scope")

issue_date = cost_report_vo.issue_date
workspace_name = self._get_workspace_name(domain_id, workspace_id)
is_confirmed = True if cost_report_vo.status == "SUCCESS" else False
is_confirmed = True if cost_report_vo.status == "DONE" else False
Copy link

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

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

[nitpick] Hardcoding the status string 'DONE' can lead to typos and inconsistencies; consider using a constant or enum (e.g., CostReportStatus.DONE) for better maintainability and compile-time safety.

Suggested change
is_confirmed = True if cost_report_vo.status == "DONE" else False
is_confirmed = True if cost_report_vo.status == COST_REPORT_STATUS_DONE else False

Copilot uses AI. Check for mistakes.
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.

1 participant