fix: Application log permission error

[shaohuzhang1]

fix: Application log permission error

[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 snippet is mostly correct but can be improved for clarity and robustness:

Issues Identified:

1. Redundant compare=CompareConstants.AND:

   • The second @has_permissions decorator contains an unnecessary compare=CompareConstants.AND.

2. Lack of Consistency in Method Naming:

   • The method names should match the endpoint they are associated with (e.g., get) to maintain consistency.

Optimization Suggestions:

1. Parameter Validation:

   • Add input validation to ensure that application_id, current_page, and page_size are valid integers before processing.

2. Use a More Concise Permission Check Setup:

   • Combine similar permissions checks into a single instance or use context managers for better readability and efficiency.

Here's the revised version:

from rest_framework import APIView
from rest_framework.request import Request
from .models import ApplicationVersionApi, OperateAPI, Group, OperationalType, \
                       Permission, CompareConstants, has_permissions

class Page(APIView):
    @has_permissions(PermissionConstants.APPLICATION_READ)
    def get(self, request: Request, application_id: str, current_page: int, page_size: int):
        # Validate inputs
        if not application_id.isdigit() or not page_size.isdigit():
            return result.failure("Invalid parameters")

        try:
            response_data = ApplicationVersionSerializer.Query(
                filter_params=(
                    {'application_id': application_id},
                    *result.get_query_params(request.query_params),
                ),
                pagination_info={'limit': page_size, 'offset': (int(page_size) * (int(current_page) - 1))}
            )
        except Exception as e:
            return result.exception(e)

        return result.success(response_data)


class Operate(APIView):
    @has_permissions(PermissionConstants.APPLICATION_READ)
    def get(self, request: Request, application_id: str, workflow_version_id: str):
        # Validate inputs
        if not application_id.isdigit() or not workflow_version_id.isdigit():
            return result.failure("Invalid parameters")

        try:
            response_data = ApplicationVersionSerializer.Operate(
                filter_params={
                    'workflow_version_id': workflow_version_id
                }
            )
        except Exception as e:
            return result.exception(e)

        return result.success(response_data)

Key Improvements:

• Consistent Method Names: Ensures that each method name corresponds to its endpoint.
• Input Validation: Added basic validation to check if application_id and page_size are digits.
• Error Handling: Wrapped exception handling around operations to provide more meaningful error messages.
• Reduced Redundancy: Eliminated redundant compare=CompareConstants.AND.
• Readability: Improved variable naming and structure for better readability.