-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: The role-based authorization option is temporarily hidden #3927
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
Conversation
--bug=1060787 --user=张展玮 【资源管理】偶先不展示按角色授权选项的现象 https://www.tapd.cn/62980211/s/1760688
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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 current code is mostly clean but can be optimized and improved in several ways:
-
Function Name: The name
getPermissionOptionsmore accurately reflects what the function does compared topermissionOptions. It's clearer that this function returns an array of permission options based on environment edition. -
Semicolon Placement: There are unnecessary semicolons after some lines (e.g.,
,) which should be removed for consistency. -
Redundant Comments: Some comments might be redundant since their purpose aligns with the preceding logic.
-
Line Continuation: Although not strictly necessary, it might improve readability to add line continuation characters (
\\) or use template literals where appropriate.
Here's a revised version of the code with these suggestions applied:
import { AuthorizationEnum } from '@/enums/system';
import { t } from '@/locales';
import { hasPermission } from '@/utils/permission';
import { EditionConst } from '@/utils/permission/data';
const permissionOptions = [
{
label: t('views.system.resourceAuthorization.setting.notAuthorized'),
value: 'NOT_AUTHORIZED',
},
// Other permissions...
];
function getPermissionOptions() {
if (hasPermission([EditionConst.IS_EE, EditionConst.IS_PE], 'OR')) {
return [...permissionOptions, {
label: t('views.system.resourceAuthorization.setting.role'),
value: AuthorizationEnum.ROLE,
desc: t('views.system.resourceAuthorization.setting.roleDesc'),
}];
}
return permissionOptions;
}
export { getPermissionOptions };Key Changes:
- Variable Naming: Changed
notCommunitytogetPermissionOptions. - Removed Unnecessary Semicolons.
- Improved Comments: Removed redundancies in comments.
- Enhanced Code Clarity: Improved code readability by separating concerns better and using modern JavaScript features like arrow functions and array spread syntax.
| }) | ||
| const permissionObj = ref<any>({ | ||
| APPLICATION: new ComplexPermission( | ||
| [RoleConst.ADMIN, RoleConst.WORKSPACE_MANAGE], |
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 code does not contain significant irregularities or critical issues. However, here are some optimizations and suggestions:
-
Refactor Computed Property: The
permissionOptionscomputed property could be more efficient if it directly calls the asynchronous function without wrapping it in acomputed. This allows you to callgetPermissionOptions()only when necessary. -
Type Assertion: Ensure that the type of
user.roleNameis explicitly defined or correctly inferred. For example, usingas RoleConst.Admincan help improve TypeScript's understanding of the expected input. -
Documentation Comments: Add comments explaining where relevant variables and functions are used or returned to enhance readability and maintainability.
Here's an improved version based on these suggestions:
@@ -142,8 +142,9 @@ import { isAppIcon, resetUrl } from '@/utils/common'
import { RoleConst, PermissionConst } from '@/utils/permission/data'
import { hasPermission } from '@/utils/permission/index'
import { ComplexPermission } from '@/utils/permission/type'
-import { permissionOptions } from '@/views/system/resource-authorization/constant'
+import { getPermissionOptions } from '@/views/system/resource-authorization/constant'
const store = useStore()
const { model, user } = storeToRefs(store) // Using Vue 3 composition API for refs
const route = useRoute()
const props = defineProps<{
getData?: () => void
}>()
const emit = defineEmits(['submitPermissions'])
// Explicitly declare user.roleName as RoleConst type (assuming it should be one of those)
const permissionOptions = computed(() => {
return getPermissionOptions() as Promise<ComplexPermission[]>; // Adjust return type accordingly
});
const permissionObj = ref<any>({
APPLICATION: new ComplexPermission(
[RoleConst.ADMIN, RoleConst.WORKSPACE_MANAGE],By making these changes, the code becomes more robust and easier to understand. Make sure to adjust any specific type assumptions based on your actual application logic and data structure.
fix: The role-based authorization option is temporarily hidden --bug=1060787 --user=张展玮 【资源管理】偶先不展示按角色授权选项的现象 https://www.tapd.cn/62980211/s/1760688