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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shaohuzhang1 merged 1 commit into from
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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 8 additions & 5 deletions apps/setting/serializers/team_serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ def is_valid(self, *, user_id=None):
os.path.join(PROJECT_DIR, "apps", "setting", 'sql', 'check_member_permission_target_exists.sql')),
[json.dumps(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) + ']')
raise AppApiException(500,
_('Non-existent application|knowledge base id[') + str(illegal_target_id_list) + ']')

def update_or_save(self, member_id: str):
team_member_permission_list = self.data.get("team_member_permission_list")
Expand Down Expand Up @@ -188,18 +189,20 @@ 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, use_user_id_list, team_id) for add_user_id in
user_id_list]
QuerySet(TeamMember).bulk_create(create_team_member_list) if len(create_team_member_list) > 0 else None
QuerySet(TeamMember).bulk_create(
[team_member for team_member in create_team_member_list if team_member is not None]) 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, use_user_id_list, user_id):
if use_user_id_list.__contains__(add_user_id):
if team_member_user_id_list.__contains__(add_user_id) or user_id == add_user_id:
raise AppApiException(500, _('The current members already exist in the team, do not add them again.'))
return None
else:
return TeamMember(team_id=self.data.get("team_id"), user_id=add_user_id)
else:
raise AppApiException(500, _('User does not exist'))
return None

def add_member(self, username_or_email: str, with_valid=True):
"""
Expand Down Expand Up @@ -318,4 +321,4 @@ def get_request_params_api():
in_=openapi.IN_PATH,
type=openapi.TYPE_STRING,
required=True,
description=_('member id')),]
description=_('member id')), ]
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Feb 25, 2025

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.