Skip to content

feat: improve trusted account auto sync configuration and validation#6142

Merged
seungyeoneeee merged 4 commits into
cloudforet-io:trusted-account-auto-syncfrom
seungyeoneeee:feature/trusted-account-async-api
Nov 12, 2025
Merged

feat: improve trusted account auto sync configuration and validation#6142
seungyeoneeee merged 4 commits into
cloudforet-io:trusted-account-auto-syncfrom
seungyeoneeee:feature/trusted-account-async-api

Conversation

@seungyeoneeee

Copy link
Copy Markdown
Contributor

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)

Enhanced Trusted Account auto sync configuration with workspace/project group mapping options and comprehensive real-time validation including name duplication check, workspace selection, and custom depth validation.


워크스페이스/프로젝트 그룹 매핑 옵션과 이름 중복 검사, 워크스페이스 선택, 커스텀 뎁스 검증을 포함한 포괄적인 실시간 검증으로 Trusted Account 자동 동기화 설정 개선

Things to Talk About (optional)

seungyeoneeee and others added 3 commits November 11, 2025 14:33
- Add duplicate name validation in ServiceAccountBaseInformationForm
  - Check against both Service Account and Trusted Account names
  - Support UPDATE mode to allow keeping the original name
  - Use parallel API calls for better performance
- Improve form validation initialization
  - Change initial validation states from true to false in store
  - Add immediate: true to validation watchers for real-time updates
  - Fix validation state synchronization between forms and store

Signed-off-by: 이승연 <sylee1274@mz.co.kr>
- Add validation logic in ServiceAccountAutoSyncMappingMethod
  - Validate workspace selection for SINGLE workspace mapping
  - Validate custom depth value for CUSTOM depth mapping
  - Emit validation state changes to parent component
- Fix event naming: change from update:isValid to update:is-valid
  - Vue automatically converts camelCase to kebab-case for events
- Add workspace dropdown prop synchronization
- Update sync_options fields in create/update APIs
  - Add workspace_mapping_type and custom_depth fields
  - Replace skip_project_group with project_group_mapping_type

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: 이승연 <sylee1274@mz.co.kr>
Signed-off-by: 이승연 <sylee1274@mz.co.kr>
@vercel

vercel Bot commented Nov 11, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
console Ignored Ignored Preview Nov 12, 2025 1:26am

@vercel

vercel Bot commented Nov 11, 2025

Copy link
Copy Markdown

@seungyeoneeee is attempting to deploy a commit to the cloudforet Team on Vercel.

A member of the Team first needs to authorize it.

Copilot AI left a comment

Copy link
Copy Markdown

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 enhances the Trusted Account auto sync configuration by implementing workspace/project group mapping options with comprehensive real-time validation. The changes transition form validation from optimistic (starting as valid) to pessimistic (starting as invalid and validating immediately), add name duplication checks across both service accounts and trusted accounts, and introduce detailed workspace mapping configuration with custom depth validation.

  • Form validation initialization changed from true to false with immediate: true watchers for proper initial validation
  • Added comprehensive name duplication checking that includes both service account and trusted account names
  • Implemented workspace and project group mapping configuration with real-time validation
  • Replaced deprecated skip_project_group with structured mapping types (workspace_mapping_type, project_group_mapping_type, custom_depth)

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
apps/web/src/services/asset-inventory/stores/service-account-page-store.ts Updated form validation defaults to false and removed skip_project_group initialization from origin data
apps/web/src/services/asset-inventory/pages/ServiceAccountAddPage.vue Updated sync_options to use new mapping types instead of skip_project_group
apps/web/src/services/asset-inventory/components/WorkspaceDropdown.vue Added watcher to sync internal state with prop changes
apps/web/src/services/asset-inventory/components/ServiceAccountCredentialsForm.vue Added immediate validation watcher
apps/web/src/services/asset-inventory/components/ServiceAccountBaseInformationForm.vue Enhanced validation logic, added trusted account name checking, added immediate validation watcher
apps/web/src/services/asset-inventory/components/ServiceAccountAutoSyncMappingMethod.vue Implemented workspace/project group mapping validation with multiple watchers and emit validation state
apps/web/src/services/asset-inventory/components/ServiceAccountAutoSyncForm.vue Integrated mapping method validation into auto sync form
apps/web/src/services/asset-inventory/components/ServiceAccountAutoSync.vue Updated sync_options to use new mapping types
.gitignore Added Claude-related files to ignore list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

skip_project_group: serviceAccountPageFormState.skipProjectGroup,
project_group_mapping_type: serviceAccountPageFormState.projectGroupMappingType,
single_workspace_id: serviceAccountPageFormState.selectedSingleWorkspace ?? undefined,
custom_depth: serviceAccountPageFormState.customDepth ?? 1,

Copilot AI Nov 11, 2025

Copy link

Choose a reason for hiding this comment

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

The custom_depth field is sent with a default value of 1 when it's null or undefined (?? 1). However, this may not be appropriate for all workspace mapping types. When workspace_mapping_type is not CUSTOM_DEPTH_GROUPS, sending a custom_depth value could be misleading or incorrect.

Consider only including custom_depth in the payload when the workspace mapping type is CUSTOM_DEPTH_GROUPS, or document why a default of 1 is always appropriate.

Suggested change
custom_depth: serviceAccountPageFormState.customDepth ?? 1,
...(serviceAccountPageFormState.workspaceMappingType === 'CUSTOM_DEPTH_GROUPS'
? { custom_depth: serviceAccountPageFormState.customDepth ?? 1 }
: {}),

Copilot uses AI. Check for mistakes.
Comment on lines +138 to +141
formState.isBaseInformationFormValid = false;
formState.isCredentialFormValid = false;
formState.credential = {};
formState.isAutoSyncFormValid = false;

Copilot AI Nov 11, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Lines 139-140 were added to reset credential form state, but line 139 sets isCredentialFormValid to false when resetting. However, this reset function (actions.reset()) is called when navigating away or starting a new form. Setting validation to false during reset is inconsistent with the pattern where false means "needs validation" rather than "explicitly invalid."

This is consistent with the new pattern (lines 95, 98, 101) but creates a potential issue where the form appears invalid before any user interaction. Verify this is the intended behavior for the reset flow.

Suggested change
formState.isBaseInformationFormValid = false;
formState.isCredentialFormValid = false;
formState.credential = {};
formState.isAutoSyncFormValid = false;
formState.isBaseInformationFormValid = undefined;
formState.isCredentialFormValid = undefined;
formState.credential = {};
formState.isAutoSyncFormValid = undefined;

Copilot uses AI. Check for mistakes.
@@ -152,7 +154,6 @@ export const useServiceAccountPageStore = defineStore('page-service-account', ()
formState.isAutoSyncEnabled = state.originServiceAccountItem?.schedule?.state === 'ENABLED';
formState.scheduleHours = state.originServiceAccountItem?.schedule?.hours ?? [];
formState.selectedSingleWorkspace = state.originServiceAccountItem?.sync_options?.single_workspace_id ?? '';

Copilot AI Nov 11, 2025

Copy link

Choose a reason for hiding this comment

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

The line that previously set formState.skipProjectGroup from state.originServiceAccountItem?.sync_options?.skip_project_group was removed (line 156), but the skipProjectGroup field is still being used elsewhere in the store (e.g., lines 195, 201) and is still part of the FormState interface. This creates an inconsistency where the field is no longer initialized from origin data but is still being reset and managed elsewhere.

Consider updating all references to skipProjectGroup consistently, either removing it entirely or ensuring it's properly initialized.

Suggested change
formState.selectedSingleWorkspace = state.originServiceAccountItem?.sync_options?.single_workspace_id ?? '';
formState.selectedSingleWorkspace = state.originServiceAccountItem?.sync_options?.single_workspace_id ?? '';
formState.skipProjectGroup = state.originServiceAccountItem?.sync_options?.skip_project_group ?? false;

Copilot uses AI. Check for mistakes.
skip_project_group: serviceAccountPageFormState.skipProjectGroup,
project_group_mapping_type: serviceAccountPageFormState.projectGroupMappingType,
single_workspace_id: serviceAccountPageFormState.selectedSingleWorkspace ?? undefined,
custom_depth: serviceAccountPageFormState.customDepth ?? 1,

Copilot AI Nov 11, 2025

Copy link

Choose a reason for hiding this comment

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

The custom_depth field is sent with a default value of 1 when it's null or undefined (?? 1). However, this may not be appropriate for all workspace mapping types. When workspace_mapping_type is not CUSTOM_DEPTH_GROUPS, sending a custom_depth value could be misleading or incorrect.

Consider only including custom_depth in the payload when the workspace mapping type is CUSTOM_DEPTH_GROUPS, or document why a default of 1 is always appropriate.

Suggested change
custom_depth: serviceAccountPageFormState.customDepth ?? 1,
...(serviceAccountPageFormState.workspaceMappingType === 'CUSTOM_DEPTH_GROUPS'
? { custom_depth: serviceAccountPageFormState.customDepth ?? 1 }
: {}),

Copilot uses AI. Check for mistakes.
Signed-off-by: 이승연 <sylee1274@mz.co.kr>
@seungyeoneeee seungyeoneeee merged commit 4dac530 into cloudforet-io:trusted-account-auto-sync Nov 12, 2025
3 of 4 checks passed
@seungyeoneeee seungyeoneeee deleted the feature/trusted-account-async-api branch November 12, 2025 01:27
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.

2 participants