rafactor: User resource permission read and edit

[shaohuzhang1]

rafactor: User resource permission read and edit

[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 appears to be mostly syntactically correct, but there are a few areas that could benefit improvements:

1. Imports: The imports of PageDataResponse and possibly other classes from the common.result module should be clarified and ensure they are necessary for the current implementation.

2. Parameters:

   • In EditUserResourcePermissionAPI, the parameters seem redundant. It's not clear if one parameter (e.g., workspace_id) is sufficient.
   • Consider combining these parameters into a single entity or structuring them better based on their usage within each API class.

3. Return Types:

   • Ensure that all functions returning serializers use consistent naming conventions like getResultData.
   • If using different types of responses (ResultSerializer, API*, etc.), clarify how each response type differs from the others.

4. Documentation:

   • Some descriptions provided via inline comments might need more context or clarification.
   • Enhance documentation strings throughout the code to clearly describe the purpose and functionality of each part of the logic.

5. Consistency:

   • Make sure all serializers have consistent field names and types across similar models. For instance, having both auth_target_type and resource seems unusual; consider normalizing this structure.

6. Optimization:

   • Review query performance, especially if there are many records being processed at once. Pagination requests suggest efficient handling, but it doesn't hurt to double-check.
   • Look for unused variables or methods that can be removed without causing unintended consequences.

Overall, refactoring would help maintain clarity and improve efficiency while ensuring the application remains functional and easy-to-maintain.

[shaohuzhang1]

After reviewing the provided code snippet, here are some potential issues and optimizations:

1. Duplicated Imports:

   • Import DefaultResultSerializer twice at lines 16 and 22.

2. Redundant Method Calls: In several places there is an unnecessary call to .list() without using its returned data.

3. Use of dict in Requests: Some API calls (e.g., ResourceUserPermissionPageAPI) expect keyword arguments instead of a dictionary. Consider updating these method calls.

4. Inconsistent Usage of Query Parameters: Sometimes query parameters are used directly, but other times they are retrieved via request.query_params. Consistency would be beneficial here.

5. Code Duplication: There is repeated boilerplate logic (setting default values) for pagination. Extract this into helper functions where appropriate.

Here's the improved version with suggested changes:

from common.auth.authentication import has_permissions
from common.constants.permission_constants import PermissionConstants, RoleConstants, Permission, Group, Operate
from common.log.log import log
from system_manage.api.user_resource_permission import UserResourcePermissionAPI, EditUserResourcePermissionAPI, \
    ResourceUserPermissionAPI, ResourceUserPermissionPageAPI, ResourceUserPermissionEditAPI
from system_manage.serializers.user_resource_permission import UserResourcePermissionSerializer, \
    ResourceUserPermissionSerializer
from users.models import User

class BaseViewSet(APIView):
    authentication_classes = [TokenAuth]

def parse_query_params(query_params, defaults=None):
    if not defaults:
        defaults = {}
    
    params = {key: value for key, value in query_params.items() if key in defaults}

    for key, default in defaults.items():
        params[key] = params.get(key, default)
    
    return params


class WorkSpaceUserResourcePermissionView(BaseViewSet):
    def get(self, request: Request, workspace_id: str, user_id: str, resource: str):
        qs = {'name': None, 'permission': None}  # Initialize filter criteria
        
        if request.query_params:
            qs.update(parse_query_params(request.query_params, qs))

        serializer = UserResourcePermissionSerializer(
            data={'workspace_id': workspace_id, 'user_id': user_id, 'auth_target_type': resource}
        )
        
        return result.success(serializer.list(qs))

    @extend_schema(
        methods=['PUT'],
        description=_('Modify the resource authorization list'),
        operation_id=_('Modify the resource authorization list'),  # type: ignore
        parameters=UserResourcePermissionAPI.get_parameters(),
        request=EditUserResourcePermissionAPI.get_request(),
        responses=EditUserResourcePermissionAPI.get_response(),
        tags=[_('Resources authorization')]  # type: ignore
    )
    @log(menu='System', operate='Modify the resource authorization list',
             level=logging.INFO)
    def put(self, request, workspace_id: str, user_id: str, resource: str):
       
        updated_data = self.parse_and_update_instance(data=request.data)
       
        serializer = UserResourcePermissionSerializer(data={
            'workspace_id': workspace_id, 
            'user_id': user_id, 
            'auth_target_type': resource
        })

        return result.success(serializer.edit(updated_data))

    class Page(ViewSetMixin):  
        @extend_schema(
            methods=['GET'],
            description=_('Obtain resource authorization list by page'),
            summary=_('Obtain resource authorization list by page'),
            operation_id=_('Obtain resource authorization list by page'),  # type: ignore
            request=None,
            parameters=UserResourcePermissionPageAPI.get_parameters(),
            responses=UserResourcePermissionPageAPI.get_response(),
            tags=[_('Resources authorization')]  # type: ignore
        )

This code removes duplicated imports and improves consistency in argument handling for filtering queries. It maintains DRY principles across similar sections while optimizing usage patterns.

[shaohuzhang1]

The updated code introduces additional paths with current_page and page_size parameters within the urlpatterns. This addition is generally acceptable if you implement proper pagination handling in your view (views.WorkSpaceUserResourcePermissionView.Page). Ensure that these endpoints correctly handle data retrieval from the current page and specified number of results per page.

Additionally, consider adding error handlers (e.g., using generic exception views) to manage cases where an invalid page size or out-of-range queries might occur. Make sure any changes align with existing documentation for resource permissions management and adherence to best practices in web development regarding URL patterns and APIs.