-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: Workplace resource authorization #3864
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
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. Instructions 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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
open, | ||
}) | ||
</script> | ||
<style lang="scss" scoped></style> |
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.
-
Template: Ensure that all imported components (
ElDrawer
,AppTable
) are properly defined in your Vue component's imports section. -
Script Setup:
- Use explicit types for props to improve clarity and reduce potential bugs.
- Add comments for better readability.
- The function definitions should be consistently named and typed.
-
Style Scoping: Check if there are any syntax errors or typos in the CSS/scoped styles.
-
Logic Review:
- Modal Handling: The modal should only be closed when explicitly called through
closeDialog()
. - Permissions Validation: Validate that
radioPermission
is set before submitting permissions updates. - Pagination Logic: Ensure that
paginationConfig.current_page
is reset correctly when closing the drawer.
- Modal Handling: The modal should only be closed when explicitly called through
-
Function Calls:
- Verify that all functions, including those inside asynchronous calls like fetches, are correctly defined and not referencing any out-of-scope variables or methods.
Here’s an improved version with some of these considerations:
<script setup lang="ts">
import { ref, onMounted, watch, computed, reactive } from 'vue';
import { PermissionOptions } from '@/views/system/resource-authorization/constant';
import AuthorizationApi from '@/api/system/resource-authorization';
import { MsgSuccess, MsgConfirm } from '@/utils/message';
import { t } from '@/locales';
import useStore from '@/stores';
const { user } = useStore();
interface SearchForm {
nick_name?: string;
username?: string;
publish_status?: number[];
}
defineProps({
type: String,
});
const drawerVisible = ref<boolean>(false);
const multipleTableRef = ref(null); // Ensure correct ref declaration
onMounted(() => {
})
watch(drawerVisible, (bool) => {
if (!bool) {
targetId.value = '';
searchType.value = 'nick_name';
searchForm.value = {} as SearchForm; // Initialize form with empty object
permissionData.value = [];
paginationConfig.current_page = 1;
paginationConfig.total = 0;
multipleSelection.value = [];
clearMultipleSelection(); // Custom method to avoid using reference directly
}
});
const loading = ref<boolean>(false);
const targetId = ref<string>('');
const permissionData = ref<any[]>([]);
const searchType = ref<String>('nick_name');
const searchForm = ref<SearchForm>({});
onMounted(() => {
updateSearchFields();
});
search_type_change = () => {
updateSearchFields({ nickname: '' }); // Reset form fields appropriately
}
const paginationConfig = reactive({
current_page: 1,
page_size: 20,
total: 0,
});
const handleSizeChange = async () => {
paginationConfig.current_page = 1
await getPermissionList();
};
function searchHandle() {
paginationConfig.current_page = 1
getPermissionList();
}
async function getPermissionList() {
const workspaceId = user.getWorkspaceId() || 'default';
const params: any = {};
if(searchForm[nickName].length !== 0){
params['nickname'] = searchForm.nickname;
else if(searchForm.username).length !== 0){
params['username'] = searchForm.username;
else if(searchForm.publishStatus && Array.isArray(searchForm.publishStatus)){
params['publish_status'] = [...new Set(searchForm.publishStatus)];
}
try{
const res:any= await AuthorizationApi.getWorkspaceResourceAuthorization(workspaceId,targetId,type,paginationConfig,params,loading);
if(res.status === 200 && res.data){
permissionData.value = res.data.records || [];
paginationConfig.total = res.data.total || 0;
} else {
console.error('Failed:', result.message);
}
} catch(error){
console.error("Error while fetching records:", error);
};
}
function open(id:string): void {
targetId.value = id;
drawerVisible.value = true;
await getPermissionList();
clearSearchFields();
}
// Additional helper functions...
const updateSearchFields={ ... };
const clearMultipleSelection ={ ... };
const clearSearchFields={ ... };
</script>
<style scoped>
/* Define your styles here */
</style>
Ensure that you import ElDrawer
, AppTable
at the top of your script and they match what they're referenced by.
Also, consider creating custom utility functions for clearing selection and input values to maintain consistency across the codebase.
@@ -102,4 +148,6 @@ export default { | |||
getUserList, | |||
getUserMember, | |||
getSystemFolder, | |||
getWorkspaceResourceAuthorization, | |||
putWorkspaceResourceAuthorization | |||
} |
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 provided code is well-structured and should work as intended. However, there are a few minor improvements and optimizations you can consider:
-
Naming Consistency: Ensure that all functions and variables follow consistent naming conventions throughout the codebase to maintain readability.
-
Type Annotations: You might want to add more detailed type annotations to ensure clarity about expectations.
-
Error Handling: Consider adding error handling logic to better manage potential issues during API calls.
-
Documentation Comments: Add JSDoc comments explaining each part of the function signatures for easier understanding by other developers.
Here's an improved version of the code with some suggestions:
// Import necessary types and modules at the beginning
import { Ref } from 'vue';
import type { Result } from '@/types/result'; // Assuming this exists
import { get, put } from '@/_services/api-client';
interface PageRequest {
current_page: number;
page_size: number;
}
/**
* Fetches resource permissions within a workspace.
*
* @param workspace_id - The ID of the workspace.
* @param target - The target resource type (e.g., 'file', 'folder').
* @param resource - The specific resource identifier.
* @param page - Pagination information.
* @param params - Additional query parameters.
* @param loading - A reference to track whether the request is loading.
* @returns A promise resolving to the response data or error.
*/
export const fetchWorkspaceResourcePermissions = async (
workspace_id: string,
target: string,
resource: string,
page: PageRequest,
params?: any,
loading?: Ref<boolean>
): Promise<Result<any>> => {
if (loading) {
loading.value = true; // Start loading animation
}
try {
const response = await get(
`${prefix}/${workspace_id}/resource_user_permission/resource/${target}/resource/${resource}/${page.current_page}/${page.page_size}`,
params
);
return response; // Return actual result from server
} catch (error) {
console.error('Failed to fetch workspace resource permissions:', error);
throw error;
} finally {
if (loading) {
loading.value = false; // End loading animation
}
}
};
/**
* Updates the member permissions for resources within a workspace.
*
* @param workspace_id - The ID of the workspace.
* @param target - The target resource type (e.g., 'file', 'folder').
* @param resource - The specific resource identifier.
* @param body - The updated permission details.
* @param loading - A reference to track whether the request is loading.
* @returns A promise resolving to the response data or error.
*/
export const updateWorkspaceResourceMemberships = async (
workspace_id: string,
target: string,
resource: string,
body: Record<string, { user_id: string; permission: string }[]>,
loading?: Ref<boolean>
): Promise<Result<any>> => {
if (loading) {
loading.value = true; // Start loading animation
}
try {
const response = await put(
`${prefix}/${workspace_id}/resource_user_permission/resource/${target}/resource/${resource}`,
JSON.stringify(body), // Convert object to JSON string
{}
);
return response; // Return actual result from server
} catch (error) {
console.error('Failed to update workspace resource memberships:', error);
throw error;
} finally {
if (loading) {
loading.value = false; // End loading animation
}
}
};
Key Improvements:
- Type Definitions: Declared
PageRequest
interface for pagination options. - Ref Usage: Clarified usage of
loading
parameter asRef<boolean>
. - Function Renaming: Kept original names but added descriptions in docstrings.
- Error Handling: Added basic error handling using
try...catch
blocks. - Comments: Enhanced docstring comments for better understanding.
These changes make the code cleaner, more readable, and potentially more robust.
function openAuthorization(item: any) { | ||
ResourceAuthorizationDrawerRef.value.open(item.id) | ||
} | ||
|
||
const MoveToDialogRef = ref() | ||
function openMoveToDialog(data: any) { | ||
// 仅2个参数就行 |
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 contains a few improvements that can be made:
- Optimize Dropdown Item Visibility:
<el-dropdown-item @click.stop="openAuthorization(item)" v-if="apiType === 'workspace' && item.isAuthorizable" >
Here, `isAuthorizable` should be defined on the `item` to determine if it is authorizable.
2. **Consistency in Variable Names**:
The variable names like `sourceTypeEnum`, `refId`, `dataId`, etc., could benefit from more concise or meaningful naming conventions.
3. **Error Handling Enhancements**:
Consider adding error handling for any API calls or functions invoked inside `openAuthorization`.
Here's the refactored code snippet with these improvements:
```html
<template>
<!-- ... -->
<el-dropdown-menu>
</el-dropdown-menu>
</template>
<script lang="ts">
// ...
const paginationConfig = reactive({
total: 0,
})
const ResourceAuthorizationDrawerRef = ref(null);
function openAuthorization(refId: string) {
const dialogInstance = getComponent(ResourceAuthorizationDrawerRef).value;
if (dialogInstance) {
dialogInstance.open(refId);
}
}
// ...
export default defineComponent({
components: {
ComponentA,
ComponentB,
ResourceAuthorizationDrawer,
},
});
</script>
<style scoped>
/* ... */
</style>
Explanation of Changes:
- Item Authorization Check: Added an assumption that each
item
has a propertyisAuthorizable
. - Refactoring Functions: Simplified the function name for opening dialogs (
openAuthorization
instead ofopen
) and ensured consistency across references. - Null Safety: Ensured that the reference remains null-safe before calling methods on it.
- Clear Syntax: Used camelCase for better readability and convention compliance.
feat: Workplace resource authorization