Skip to content

fix: When adding members in bulk, existing members will be automatically passed in, do not affect the joining of other members #2351#2397

Merged
shaohuzhang1 merged 1 commit intomainfrom
pr@main@fix_add_member
Feb 25, 2025
Merged

fix: When adding members in bulk, existing members will be automatically passed in, do not affect the joining of other members #2351#2397
shaohuzhang1 merged 1 commit intomainfrom
pr@main@fix_add_member

Conversation

@shaohuzhang1
Copy link
Contributor

fix: When adding members in bulk, existing members will be automatically passed in, do not affect the joining of other members #2351

…lly passed in, do not affect the joining of other members #2351
@shaohuzhang1 shaohuzhang1 merged commit e1f0f39 into main Feb 25, 2025
4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@main@fix_add_member branch February 25, 2025 08:54
type=openapi.TYPE_STRING,
required=True,
description=_('member id')),]
description=_('member id')), ]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are no obvious syntax errors or logic problems in the provided code. However, here are some minor improvements and suggestions:

  1. Line Continuation: The import statements should be wrapped correctly. In Python, it's best practice to align imports under one header.

  2. String Formatting: For consistency, you might consider using f-strings (formatted string literals) instead of concatenation for string formatting. This can make the code more readable.

  3. Variable Naming: Ensure variable names are descriptive. While illegal_target_id_list is clear enough, use_user_id_list could be renamed to something like existing_team_member_ids.

  4. Empty List Check: When checking if a list is empty, using if []: is redundant because an empty list evaluates to false when used as a condition.

Here's a revised version of the code:

def is_valid(self, *, user_id=None):
    sql_template_path = os.path.join(PROJECT_DIR, "apps", "setting", 'sql', 'check_member_permission_target_exists.sql')
    permission_list = json.dumps(self.data["permission_list"])
    
    try:
        illegal_target_id_list = execute_sql_query(sql_template_path, [permission_list, user_id, user_id])
        
        if illegal_target_id_list is not None and len(illegal_target_id_list) > 0:
            raise AppApiException(500, _('Non-existent application|knowledge base id[') + str(illegal_target_id_list) + ']')
    except Exception as e:
        # Handle unexpected exceptions
        logging.error(f"An error occurred during validation: {e}")

def update_or_save(self, member_id: str):
    team_member_permission_list = self.data.get("team_member_permission_list")
    # ... rest

def batch_add_member(self, user_id_list: List[str], with_valid=True):
    create_team_member_list = [
        self.to_member_model(add_user_id, team_member_user_id_list, existing_team_member_ids, team_id)
        for add_user_id in user_id_list
    ]
    
    QuerySet(TeamMember).bulk_create(
        [tm for tm in create_team_member_list if tm is not None]  # Assuming TeamMember() returns None on failure
    ) if len(create_team_member_list) > 0 else None
    
    return TeamMemberSerializer(data={'team_id': self.data.get('team_id')}).list_member()

def to_member_model(self, add_user_id, team_member_user_id_list, existing_team_member_ids, team_id):
    if add_user_id in existing_team_member_ids:
        if add_user_id in team_member_user_id_list or add_user_id == username_or_email:
           raise AppApiException(500, _('The current members already exist in the team, do not add them again.'))
        else:
            return TeamMember(team_id=team_id, user_id=add_user_id)
    else:
        raise AppApiException(500, _('User does not exist'))

def add_member(self, username_or_email: str, with_valid=True):
    # ... rest

    @app_api_router.post('/api/user/batch-add-member/', responses={HTTP_200_OK: ResponseSchema})
    async def handle_batch_add_members(request: Request):
        params = request.json()
        try:
            results = await app_api_logic.user_logic.batch_add_member(params['email_addresses'])
            return {"results": results}
        except Exception as e:
            logging.error(f"Failed to process batch add members request: {e}")
            return ErrorResponseModel(code=-1, message=str(e))

# Update route docstring
@app_api_router.get("/api/member-detail/", response_description="Get Member Detail",
                  operation_id='Get Member Detail',
                  summary="Retrieve detailed information about a project member.",
                  tags=["Members"],
                  parameters=[OpenAPIParameter(name='member_id', location=current_app.config.CONTEXT_PATH,
                                          in_=openapi.IN_QUERY,
                                          type=openapi.TYPE_STRING,
                                          required=True)],
                  responses={
                      HTTP_200_OK: {
                          "model": MemberDetailResponse,
                          "description": "Detailed details about a project member."
                      },
                      HTTP_404: {
                          "model": ErrorMessage,
                          "desription": "Error retrieving the member detail"
                      }
                  })
async def handler_get_member_detail():
    """
    Retrieve detailed information about a project member.
    ---
    requestBody:
        type: object
        properties:
            member_id:
                type: string
                example: "abc123"
                description: "Unique identifier for the member."
    responses:
        '200':
            description: Returns detailed information about the specified member.
            content:
                application/json:
                    schema:
                        $ref: '#/components/schemas/MemberDetailResponse'
        '404':
            description: The requested member was not found.
            content:
                application/json:
                    schema:
                        $ref: '#/components/schemas/ErrorMessage'
    """

    # ... rest

Points Made:

  • Code Structure: Ensured that different sections of the function are organized logically.
  • Consistency: Consistent use of indentation, spacing, and line breaks.
  • Logging: Added basic exception handling for better debugging.
  • Documentation: Updated API routes with proper comments and descriptions.

This will help improve the readability and maintainability of the codebase.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Feb 25, 2025

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/test-infra repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Feb 25, 2025

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant