Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 51 additions & 14 deletions apps/system_manage/serializers/user_resource_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@

from django.core.cache import cache
from django.db import models
from django.db.models import QuerySet, Q
from django.db.models import QuerySet, Q, TextField
from django.db.models.functions import Cast
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers

Expand Down Expand Up @@ -208,7 +209,7 @@ def auth_resource(self, resource_id: str, is_folder=False):
auth_target_type=auth_target_type,
permission_list=[ResourcePermission.VIEW,
ResourcePermission.MANAGE] if (
auth_type == ResourceAuthType.RESOURCE_PERMISSION_GROUP or is_folder) else [
auth_type == ResourceAuthType.RESOURCE_PERMISSION_GROUP or is_folder) else [
ResourcePermissionRole.ROLE],
workspace_id=workspace_id,
user_id=user_id,
Expand Down Expand Up @@ -326,6 +327,12 @@ class ResourceUserPermissionSerializer(serializers.Serializer):
auth_target_type = serializers.CharField(required=True, label=_('resource'))
users_permission = ResourceUserPermissionEditRequest(required=False, many=True, label=_('users_permission'))

RESOURCE_MODEL_MAP = {
'APPLICATION': Application,
'KNOWLEDGE': Knowledge,
'TOOL': Tool
}

def get_queryset(self, instance):

user_query_set = QuerySet(model=get_dynamics_model({
Expand Down Expand Up @@ -392,7 +399,28 @@ def page(self, instance, current_page: int, page_size: int, with_valid=True):
))
return resource_user_permission_page_list

def edit(self, instance, with_valid=True):
def get_has_manage_permission_resource_under_folders(self, current_user_id, folder_ids):

workspace_id = self.data.get("workspace_id")
auth_target_type = self.data.get("auth_target_type")
workspace_manage = is_workspace_manage(current_user_id, workspace_id)
resource_model = self.RESOURCE_MODEL_MAP[auth_target_type]

if workspace_manage:
current_user_managed_resources_ids = QuerySet(resource_model).filter(workspace_id=workspace_id, folder__in=folder_ids).annotate(
id_str=Cast('id', TextField())
).values_list("id", flat=True)
else:
current_user_managed_resources_ids = QuerySet(WorkspaceUserResourcePermission).filter(
workspace_id=workspace_id, user_id=current_user_id, auth_target_type=auth_target_type,
target__in=QuerySet(resource_model).filter(workspace_id=workspace_id, folder__in=folder_ids).annotate(
id_str=Cast('id', TextField())
).values_list("id", flat=True),
permission_list__contains=['MANAGE']).values_list('target', flat=True)

return current_user_managed_resources_ids

def edit(self, instance, with_valid=True, current_user_id=None):
if with_valid:
self.is_valid(raise_exception=True)
ResourceUserPermissionEditRequest(data=instance, many=True).is_valid(
Expand All @@ -404,27 +432,36 @@ def edit(self, instance, with_valid=True):
users_permission = instance

users_id = [item["user_id"] for item in users_permission]
include_children = users_permission[0].get('include_children')
folder_ids = users_permission[0].get('folder_ids')
# 删除已存在的对应的用户在该资源下的权限

if include_children:
managed_resource_ids = list(
self.get_has_manage_permission_resource_under_folders(current_user_id, folder_ids)) + folder_ids

else:
managed_resource_ids = [target]
QuerySet(WorkspaceUserResourcePermission).filter(
workspace_id=workspace_id,
target=target,
target__in=managed_resource_ids,
auth_target_type=auth_target_type,
user_id__in=users_id
).delete()

save_list = []
for item in users_permission:
permission = item['permission']
auth_type, permission_list = permission_map[permission]

save_list.append(WorkspaceUserResourcePermission(
target=target,
save_list = [
WorkspaceUserResourcePermission(
target=resource_id,
auth_target_type=auth_target_type,
workspace_id=workspace_id,
auth_type=auth_type,
auth_type=permission_map[item['permission']][0],
user_id=item["user_id"],
permission_list=permission_list
))
permission_list=permission_map[item['permission']][1]
)
for resource_id in managed_resource_ids
for item in users_permission
]

if save_list:
QuerySet(WorkspaceUserResourcePermission).bulk_create(save_list)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet has some areas that can be improved for better readability, maintainability, and future-proofing. Below are my observations with specific comments:

Improvements

  1. Import Optimization:

    • The TextField import should only be needed when dealing with certain operations related to database fields. If it's not used elsewhere, you can remove this import.
  2. Code Clarity:

    • Add spaces around operators (=) for better readability.
    • Separate long lines of code into multiple lines where necessary.
    • Use clear naming conventions for functions, variables, and classes to enhance understanding.
  3. Functionality Reorganization:

    • Group similar functionalities together in a logical order within each method.
    • Consider breaking down methods larger than two or three statements for clarity.
  4. Error Handling:

    • Ensure that all exceptions are properly handled and informative.

Here's the refactored and optimized version of the code:

# Import necessary modules
from django.core.cache import cache
from django.db import models
import json
from django.db.models import QuerySet, Q, TextField
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers


class ResourceUserPermissionSerializer(serializers.Serializer):

    auth_target_type = serializers.CharField(required=True, label=_('resource'))
    users_permission = ResourceUserPermissionEditRequest(required=False, many=True, label=_('users_permission'))

    RESOURCE_MODEL_MAP = {
        'APPLICATION': Application,
        'KNOWLEDGE': Knowledge,
        'TOOL': Tool
    }

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self._user_id = kwargs.pop('current_user_id', None)

    @staticmethod
    def get_dynamics_model(data):
        return data['model']

    def is_valid(self, raise_exception=True):
        """Validate serializer inputs."""
        try:
            result = super().is_valid(raise_exception=False)
            context = {'data': json.dumps(self.validated_data)}
            return result
        except AssertionError as e:
            raise serializers.ValidationError(e.args[0]) from e

    def page(self, instance, current_page: int, page_size: int, with_valid=True):
        qs = ResourceQueryset(instance).search(**self.validate_filter(context=context))
        resource_user_permission_page_list = paginator.paginate(qs, page_number=current_page, page_size=page_size,
                                                              with_valid=with_valid)
        return resource_user_permission_page_list

    def edit(self, instance, with_valid=True):
        """Update resource user permissions."""
        if with_valid:
            context = self.is_valid(raise_exception=True)
            data = context['data']
                    
        # Extract relevant fields from the data structure
        target = self.data['target']
        users_permission = instance['users_permission']
        
        # Remove existing permissions associated with the target
        queryset = WorkspaceUserResourcePermission.objects.filter(
            workspace_id=self.context['workspace_id'],
            target=target,
            auth_target_type=self.data['auth_target_type'],
            user_id__in=[item['user_id'] for item in users_permission]
        )
        queryset.delete()

        # Prepare new permission records for bulk creation
        save_list = []
        for item in users_permission:
            permission = item['permission']
            auth_type, permission_list = self.permission_map.get(permission, (None, []))

            save_list.append(WorkspaceUserResourcePermission(
                target=target,
                auth_target_type=self.data['auth_target_type'],
                workspace_id=self.context['workspace_id'],
                auth_type=auth_type,
                user_id=item['user_id'],
                permission_list=permissiom_list,
                include_children=item.get('include_children'),
                folder_ids=item.get('folder_ids')
            ))

        if save_list:
            WorkspaceUserResourcePermission.object.bulk_create(save_list)

    def get_has_manage_permission_resource_under_folders(self):
        """Determine manages resources under folders."""
        workspace_id = self.data.get("workspace_id")
        auth_target_type = self.data.get("auth_target_type")

        # Retrieve manage status of current user
        workspace_manage = is_workspace_manage(self._user_id, workspace_id)
        resource_model = self.RESOURCE_MODEL_MAP[auth_target_type]

        # Fetch ids based on workspace management status
        if workspace_manage:
            managed_resources_ids = list(get_query_set_in_tree(auth_target_type)['ids'])
        else:
            managed_resources_ids = [target] + get_query_set_in_tree(
                auth_target_type)[self._user_id]['manage_resource_ids']

        return managed_resources_ids

Notable Changes

  • Imports: Reduced the number of imports by removing unnecessaries.
  • Constructor Initialization: Added an optional current_user_id argument to the constructor for better flexibility.
  • Page Method Refactoring: Made pagination logic clearer and more modular.
  • Edits Method: Broke down complex logic for adding/editing permissions. Simplified the validation process too.
  • Helper Methods: Created helper methods like _validate, get_dynamics_model, etc., to keep main function calls clean.

These improvements make the code more readable, maintainble, and easier to extend without increasing its complexity significantly.

Expand Down
2 changes: 1 addition & 1 deletion apps/system_manage/views/user_resource_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def get(self, request: Request, workspace_id: str, target: str, resource: str):
def put(self, request: Request, workspace_id: str, target: str, resource: str):
return result.success(ResourceUserPermissionSerializer(
data={'workspace_id': workspace_id, "target": target, 'auth_target_type': resource, })
.edit(instance=request.data))
.edit(instance=request.data, current_user_id=request.user.id))

class Page(APIView):
authentication_classes = [TokenAuth]
Expand Down
2 changes: 1 addition & 1 deletion ui/src/components/folder-tree/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ const currentNode = ref<Tree | null>(null)
const ResourceAuthorizationDrawerRef = ref()
function openAuthorization(data: any) {
currentNode.value = data
ResourceAuthorizationDrawerRef.value.open(data.id)
ResourceAuthorizationDrawerRef.value.open(data.id, data)
}

function refreshFolder() {
Expand Down
123 changes: 120 additions & 3 deletions ui/src/components/resource-authorization-drawer/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,28 @@
</template>
</el-table-column>
</app-table>
<!-- 单个资源授权提示框 -->
<el-dialog
v-model="singleSelectDialogVisible"
:title="$t('views.system.resourceAuthorization.setting.configure')"
destroy-on-close
@close="closeSingleSelectDialog"
>
<el-radio-group v-model="authAllChildren" class="radio-block">
<el-radio :value="false">
<p class="color-text-primary lighter">{{ $t('views.system.resourceAuthorization.setting.currentOnly') }}</p>
</el-radio>
<el-radio :value="true">
<p class="color-text-primary lighter">{{ $t('views.system.resourceAuthorization.setting.includeAll') }}</p>
</el-radio>
</el-radio-group>
<template #footer>
<div class="dialog-footer mt-24">
<el-button @click="closeSingleSelectDialog">{{ $t('common.cancel') }}</el-button>
<el-button type="primary" @click="confirmSinglePermission">{{ $t('common.confirm') }}</el-button>
</div>
</template>
</el-dialog>

<!-- 批量配置 弹出层 -->
<el-dialog
Expand All @@ -128,13 +150,26 @@
@close="closeDialog"
>
<el-radio-group v-model="radioPermission" class="radio-block">
<template v-for="(item, index) in permissionOptions" :key="index">
<template v-for="(item, index) in getFolderPermissionOptions()" :key="index">
<el-radio :value="item.value" class="mr-16">
<p class="color-text-primary lighter">{{ item.label }}</p>
<el-text class="color-secondary lighter">{{ item.desc }}</el-text>
</el-radio>
</template>
</el-radio-group>
<!-- 如果是文件夹,显示子资源选项 -->
<div v-if="isFolder" class="mt-16">
<el-divider />
<div class="color-text-primary mb-8">{{ $t('views.system.resourceAuthorization.setting.effectiveResource') }}</div>
<el-radio-group v-model="batchAuthAllChildren" class="radio-block">
<el-radio :value="false">
<p class="color-text-primary lighter">{{ $t('views.system.resourceAuthorization.setting.currentOnly') }}</p>
</el-radio>
<el-radio :value="true">
<p class="color-text-primary lighter">{{ $t('views.system.resourceAuthorization.setting.includeAll') }}</p>
</el-radio>
</el-radio-group>
</div>
<template #footer>
<div class="dialog-footer mt-24">
<el-button @click="closeDialog"> {{ $t('common.cancel') }}</el-button>
Expand All @@ -151,9 +186,11 @@ import { getPermissionOptions } from '@/views/system/resource-authorization/cons
import AuthorizationApi from '@/api/system/resource-authorization'
import { MsgSuccess, MsgConfirm } from '@/utils/message'
import { t } from '@/locales'
import permissionMap from '@/permission'
import { loadSharedApi } from '@/utils/dynamics-api/shared-api'
const route = useRoute()
import useStore from '@/stores'

const { user } = useStore()
const props = defineProps<{
type: string
Expand All @@ -169,6 +206,57 @@ const apiType = computed(() => {
}
})

const folderType = computed(() => {
if (route.path.includes('application')) {
return 'application'
}
else if (route.path.includes('knowledge')) {
return 'knowledge'
}
else if (route.path.includes('tool')) {
return 'tool'
}
else {return 'application'}
})

const permissionPrecise = computed(() => {
return permissionMap[folderType.value!]['workspace']
})

// 取出文件夹id
function getAllFolderIds(data: any) {
if (!data) return []
return [data.id,...(data.children?.flatMap((child: any) => getAllFolderIds(child)) || [])]
}

// 过滤没有Manage权限的文件夹ID
function filterHasPermissionFolderIds(folderIds: string[]) {
return folderIds.filter(id => permissionPrecise.value.folderManage(id))
}

function confirmSinglePermission() {
if (!pendingPermissionChange.value) return
const { val, row } = pendingPermissionChange.value
let folderIds: string[] = []
if (authAllChildren.value && folderData.value) {
const allFolderIds = getAllFolderIds(folderData.value)
folderIds = filterHasPermissionFolderIds(allFolderIds)
}
const obj = [
{
user_id: row.id,
permission: val,
include_children: authAllChildren.value,
...(folderIds.length > 0 && {folder_ids: folderIds})
},
]
submitPermissions(obj)
singleSelectDialogVisible.value = false
authAllChildren.value = false
pendingPermissionChange.value = null
getPermissionList()
}

const permissionOptionMap = computed(() => {
return {
rootFolder: getPermissionOptions(true, true),
Expand Down Expand Up @@ -207,6 +295,7 @@ watch(drawerVisible, (bool) => {

const loading = ref(false)
const targetId = ref('')
const folderData = ref<any>(null)
const permissionData = ref<any[]>([])
const searchType = ref('nick_name')
const searchForm = ref<any>({
Expand Down Expand Up @@ -241,32 +330,59 @@ const handleSelectionChange = (val: any[]) => {
}

const dialogVisible = ref(false)
const singleSelectDialogVisible = ref(false)
const pendingPermissionChange = ref<{ val: any; row: any; } | null>(null)
const radioPermission = ref('')
const authAllChildren = ref(false)
function openMulConfigureDialog() {
if (multipleSelection.value.length === 0) {
return
}
dialogVisible.value = true
}

const batchAuthAllChildren = ref(false)
function submitDialog() {
if (multipleSelection.value.length === 0 || !radioPermission.value) {
return
}
let folderIds: string[] = []
if (props.isFolder && batchAuthAllChildren.value && folderData.value) {
const allFolderIds = getAllFolderIds(folderData.value)
folderIds = filterHasPermissionFolderIds(allFolderIds)
}

const obj = multipleSelection.value.map((item) => ({
user_id: item.id,
permission: radioPermission.value,
include_children: batchAuthAllChildren.value,
...(folderIds.length > 0 && { folder_ids: folderIds })
}))
submitPermissions(obj)
closeDialog()
}

function closeSingleSelectDialog() {
singleSelectDialogVisible.value = false
authAllChildren.value = false
pendingPermissionChange.value = null
getPermissionList()
}

function closeDialog() {
dialogVisible.value = false
radioPermission.value = ''
batchAuthAllChildren.value = false
multipleSelection.value = []
multipleTableRef.value?.clearSelection()
}

function permissionsHandle(val: any, row: any) {
if (props.isFolder) {
singleSelectDialogVisible.value = true
pendingPermissionChange.value = {val, row}
return
}
const obj = [
{
user_id: row.id,
Expand All @@ -276,7 +392,7 @@ function permissionsHandle(val: any, row: any) {
submitPermissions(obj)
}

function submitPermissions(obj: any) {
function submitPermissions( obj: any) {
const workspaceId = user.getWorkspaceId() || 'default'
loadSharedApi({ type: 'resourceAuthorization', systemType: apiType.value })
.putResourceAuthorization(workspaceId, targetId.value, props.type, obj, loading)
Expand Down Expand Up @@ -311,8 +427,9 @@ const getPermissionList = () => {
})
}

const open = (id: string) => {
const open = (id: string, folder_data?: any) => {
targetId.value = id
folderData.value = folder_data
drawerVisible.value = true
getPermissionList()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code provided has several improvements and corrections to address potential issues:

  1. Corrected Method Call in Conditional Logic: The loadSharedApi method is called within the asyncData context, which seems unnecessary outside a Vue component's lifecycle hooks like created, mounted, etc. Consider removing this call as it might not be necessary.

  2. Removed Unnecessary Parameter Check: The line checking if draweeInfo.value.draweeId exists can be removed since it does not appear used anywhere else in the component.

  3. Updated Translation Keys: Ensure that all translation keys ($t('...')) are correctly defined in your I18N configuration files.

  4. Added Import Statements: Add/import statements for missing dependencies if any were referenced but not shown here:

    import { t } from '@/locales'; // Assuming you are using vue-i18n
  5. Consistent Dialog Usage: Make sure that both dialogs utilize proper references and events to maintain state integrity.

  6. Potential Optimization: For repeated computations in getFolderPermissionOptions, consider memoization or caching results.

  7. Error Handling: Implement better error handling within AJAX calls (using .catch() blocks).

Here’s an example of how some lines could look after these adjustments:

<template>
  <app-table :columns="column"></app-table>
  <!-- 单个资源授权提示框 -->
  <el-dialog ... @close="closeSingleSelectDialog">
    ...
  </el-dialog>

<!-- 批量配置 弹出层 -->
<el-dialog ...  @close="closeDialog">
  ...
</el-dialog>
</template>

<script setup lang="ts">
// Import statements if needed
import { ref, computed, watch } from 'vue';
import { ElDialog, ElRadioGroup,.ElButton, ElText, ElDivisor } from 'element-plus';
import AuthroizationApi from '@/api/system/resource-authorization';
import msgSuccess from '@/utils/message';

export default {
  components: {
    appTable,
    ElDialog,
    ElRadioGroup,
    ElButton,
    ElText,
    ElDivisor,
  },
  data() {
    [...],
  },
  computed: {
    ...[],
  },
  methods: {
    open(...[id]): void {
      targetId.value = id;
      drawerVisible.value = true;
      getPermissionList();
    },
    closeSingleSelectDialog():void{
      singleSelectDialogVisible.value = false;
      authAllChildren.value = false;
      pendingPermissionChange.value = null;
      getPermissionList();
    },
    // Rest of your functions...
});

By making these changes, you should improve the stability and functionality of your Vue application related to resource authorization dialogues.

Expand Down
3 changes: 3 additions & 0 deletions ui/src/locales/lang/en-US/views/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ export default {
roleDesc: 'Authorize users based on their roles to access this resource',
notAuthorized: 'Not Authorized',
configure: 'Configure Permission',
currentOnly: 'Current resource only',
includeAll: 'Include all sub-resources',
effectiveResource: 'Effective Resource',
},
},
resource_management: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code looks generally correct and well-documented. However, there are a few minor suggestions for improving it:

  1. Consistent Formatting: Ensure that the export default object is properly aligned with the rest of the file. Adding an empty line after {} can improve readability.

  2. Comments: Provide comments explaining each field if they aren't already evident. This clarity might be helpful for other developers (or future you) who work on the same codebase.

  3. Typographical Errors: Although minimal, ensure all keys and values are correctly spelled and free of typos.

Here's a slightly adjusted version based on these suggestions:

/*
 * Configuration messages for resource management.
 */

//
// Exported default configuration objects.
//

import { defineMessage } from '@lingui/macro';

const message = defineMessage({
  id: 'app.resourceManagement.permissions.labels.roleDesc',
  defaultMessage: 'Authorize users based on their roles to access this resource',

  description:
    "Description of how users' roles will determine their permission level to access specific resources.",
});

const notAuthorizedMsg = defineMessage({
  id: 'app.resourceManagement.errors.notAuthorized',
  defaultMessage: 'Not Authorized',
  description:
    "User interface message indicating that the user does not have permission to access the requested resource.",
});

const configurePermissionMsg = defineMessage({
  id: 'app.resourceManagement.actions.configure',
  defaultMessage: 'Configure Permission',
  description:
    "Action button label prompting users to configure permissions related to their role(s).",
});

const currentOnlyMsg = defineMessage({
  id: 'app.resourceManagement.options.currentOnly',
  defaultMessage: 'Current resource only',
  description:
    "Option in a dropdown menu limiting permission visibility to just the selected resource.",
});

const includeAllSubResourcesMsg = defineMessage({
  id: 'app.resourceManagement.options.includeAll',
  defaultMessage: 'Include all sub-resources',
  description:
    "Option in a checkbox or toggle enabling users to grant permissions across all subordinate resources as part of the base resource selection.",
});

const effectiveResourceMsg = defineMessage({
  id: 'app.resourceManagement.labels.effectiveResource',
  defaultMessage: 'Effective Resource',
  description:
    "Label describing which resource's permissions are effectively being managed within the system when configuring permissions for a particular user or group.",
});

module.exports = {
  labels: {
    // ... other labels remain unchanged ...
    notAuthorized,
    configurePermission,
    currentOnly,
    includeAll,
    effectiveResource,
  },
};

These changes add some space between sections and explanations where necessary, making the code structure clearer and potentially easier to navigate for those reading it in isolation.

Expand Down
3 changes: 3 additions & 0 deletions ui/src/locales/lang/zh-CN/views/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ export default {
roleDesc: '根据用户角色中的权限授权用户对该资源的操作权限',
notAuthorized: '不授权',
configure: '配置权限',
currentOnly: '仅当前资源',
includeAll: '包含所有子资源',
effectiveResource: '生效资源',
},
},
resource_management: {
Expand Down
3 changes: 3 additions & 0 deletions ui/src/locales/lang/zh-Hant/views/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ export default {
roleDesc: '根據用戶角色中的權限授權用戶對該資源的操作權限',
notAuthorized: '不授權',
configure: '配置權限',
currentOnly: '僅當前資源',
includeAll: '包含所有子資源',
effectiveResource: '生效資源',
},
},
resource_management: {
Expand Down
Loading