feat: Folder authorization can optionally include sub-resources

[shaohuzhang1]

feat: Folder authorization can optionally include sub-resources

[f2c-ci-robot]

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.

Details

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.

[f2c-ci-robot]

[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.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

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.

[shaohuzhang1]

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.

[shaohuzhang1]

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.