-
Notifications
You must be signed in to change notification settings - Fork 40
refactor: resolve conflict and match MFA codes with vue query #6132
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
refactor: resolve conflict and match MFA codes with vue query #6132
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
🎉 @yuda110 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 multi-factor authentication (MFA) system to use Vue Query and resolves conflicts around MFA code matching. The main changes include replacing traditional API calls with Vue Query patterns, updating MFA modal components, and adding new functionality for MFA enforcement in user management.
- Replaced manual API calls with Vue Query mutations and queries for MFA operations
- Refactored MFA modal components to use a more modular approach with separate enable/disable modals
- Added MFA enforcement functionality in user management with bulk operations support
Reviewed Changes
Copilot reviewed 72 out of 73 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
packages/mirinae/src/controls/select-card/PSelectCard.vue | Added readonly prop and styling to support disabled selection cards |
packages/language-pack/*.json | Added new translation keys for MFA enforcement and user management features |
packages/core-lib/src/space-connector/ | Removed service configuration features and added token removal methods |
apps/web/src/store/user/ | Updated user store types to support required actions and MFA enforcement |
apps/web/src/services/my-page/ | Refactored MFA components to use Vue Query and modular modal approach |
apps/web/src/services/iam/ | Added bulk MFA setting functionality with new modal components |
apps/web/src/services/auth/ | Added MFA setup page for enforced authentication flow |
apps/web/src/common/components/mfa/ | Created reusable MFA components with Vue Query integration |
apps/web/src/api-clients/identity/ | Updated API schemas to support MFA enforcement parameters |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
passwordFormState.invalidText = ''; | ||
passwordFormState.passwordCheckModalVisible = false; | ||
showSuccessMessage(i18n.t('COMMON.PROFILE.SUCCESS_PASSWORD_CHECK')); | ||
showSuccessMessage(i18n.t('COMMON.PROFILE.SUCCESS_PASSWORD_CHECK'), ''); |
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 showSuccessMessage function is being called with an empty string as the second argument. This pattern is inconsistent and the empty string parameter should either be removed or properly documented if it serves a specific purpose.
showSuccessMessage(i18n.t('COMMON.PROFILE.SUCCESS_PASSWORD_CHECK'), ''); | |
showSuccessMessage(i18n.t('COMMON.PROFILE.SUCCESS_PASSWORD_CHECK')); |
Copilot uses AI. Check for mistakes.
passwordFormState.invalidText = ''; | ||
passwordFormState.passwordCheckModalVisible = false; | ||
showSuccessMessage(i18n.t('COMMON.PROFILE.SUCCESS_PASSWORD_CHECK')); | ||
showSuccessMessage(i18n.t('COMMON.PROFILE.SUCCESS_PASSWORD_CHECK'), ''); |
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.
Same issue as above - inconsistent use of showSuccessMessage with empty string parameter.
showSuccessMessage(i18n.t('COMMON.PROFILE.SUCCESS_PASSWORD_CHECK'), ''); | |
showSuccessMessage(i18n.t('COMMON.PROFILE.SUCCESS_PASSWORD_CHECK')); |
Copilot uses AI. Check for mistakes.
/* API */ | ||
const { mutateAsync: confirmMfa, isPending: isConfirmingMfa } = useUserProfileConfirmMfaMutation({ | ||
onSuccess: (data: UserModel) => { | ||
showSuccessMessage(i18n.t('COMMON.MFA_MODAL.ALT_S_ENABLED'), ''); |
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.
Inconsistent use of showSuccessMessage with empty string parameter.
showSuccessMessage(i18n.t('COMMON.MFA_MODAL.ALT_S_ENABLED'), ''); | |
showSuccessMessage(i18n.t('COMMON.MFA_MODAL.ALT_S_ENABLED')); |
Copilot uses AI. Check for mistakes.
</template> | ||
<template #col-tags-format="{value}"> | ||
<template v-if="value && !!Object.keys(value).length"> | ||
<template v-if="value && typeof value === 'object' && !!Object.keys(value).length"> |
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 check 'typeof value === 'object'' should also exclude null since typeof null === 'object' in JavaScript. Consider using 'value && typeof value === 'object' && value !== null' or a more specific check.
<template v-if="value && typeof value === 'object' && !!Object.keys(value).length"> | |
<template v-if="value && typeof value === 'object' && value !== null && !!Object.keys(value).length"> |
Copilot uses AI. Check for mistakes.
</template> | ||
<template #col-tags-format="{value}"> | ||
<template v-if="value !== undefined && Object.keys(value).length > 0"> | ||
<template v-if="value && typeof value === 'object' && Object.keys(value).length > 0"> |
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.
Same issue as above - the type check should exclude null since typeof null === 'object' in JavaScript.
<template v-if="value && typeof value === 'object' && Object.keys(value).length > 0"> | |
<template v-if="value && typeof value === 'object' && value !== null && Object.keys(value).length > 0"> |
Copilot uses AI. Check for mistakes.
if (!oneNumberValidator(value)) return i18n.t('IDENTITY.USER.FORM.ONE_NUMBER_INVALID'); | ||
const passwordRegex = /^(?=.*[a-z])(?=.*[A-Z])(?=.*\d).{8,}$/; | ||
if (!passwordRegex.test(value)) { |
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 password validation regex is duplicated in multiple files (UserManagementAddPassword.vue and UserManagementFormPasswordForm.vue). Consider extracting this to a shared constant or utility function to maintain consistency and avoid duplication.
if (!passwordRegex.test(value)) { | |
if (!PASSWORD_REGEX.test(value)) { |
Copilot uses AI. Check for mistakes.
}, { | ||
password(value: string) { | ||
if (value === '') return ''; | ||
const passwordRegex = /^(?=.*[a-z])(?=.*[A-Z])(?=.*\d).{8,}$/; |
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 password validation regex is duplicated from UserManagementFormPasswordForm.vue. Consider extracting to a shared utility.
const passwordRegex = /^(?=.*[a-z])(?=.*[A-Z])(?=.*\d).{8,}$/; |
Copilot uses AI. Check for mistakes.
Signed-off-by: 이승연 <[email protected]>
5299a2a
into
cloudforet-io:feature-query-integration
Skip Review (optional)
style
,chore
,ci
,test
,docs
)Description (optional)
Things to Talk About (optional)