Skip to content

Conversation

haydnli-shopify
Copy link
Contributor

@haydnli-shopify haydnli-shopify commented Jun 9, 2025

Summary

Implements atomic single and batch member management endpoints to eliminate race conditions when multiple automations add/remove project members simultaneously. The existing set_members endpoint required passing an authoritative list of all members, creating race conditions in concurrent scenarios.

This PR enhances the member management system to support both single-user and batch operations through the same endpoints, providing flexibility for different use cases while maintaining backwards compatibility.

Fixes #2684 (parent ticket #2742)

Changes Made

  • Enhanced AddProjectMemberRequest to accept single member or array of members
  • Enhanced RemoveProjectMemberRequest to accept single username or array of usernames
  • Implemented add_project_members() and remove_project_members() service functions supporting both single and batch operations
  • Enhanced POST /api/projects/{project_name}/add_member endpoint for single/batch operations
  • Enhanced POST /api/projects/{project_name}/remove_member endpoint for single/batch operations
  • Extended ProjectsAPIClient with backwards-compatible methods and new batch methods
  • Comprehensive permission controls (managers/admins only, global admin privileges) for all operations
  • Support for username or email lookup in both single and batch modes
  • Protection against removing all project admins
  • Atomic database operations eliminating read-modify-write patterns
  • Comprehensive test suite covering single, batch, and edge case scenarios

API Usage

Add a single member
POST /api/projects/my-project/add_member
Body:

{
  "members": {
    "username": "john@example.com",
    "project_role": "user"
  }
}

Add multiple members (batch)
POST /api/projects/my-project/add_member
Body:

{
  "members": [
    {"username": "john@example.com", "project_role": "user"},
    {"username": "jane_doe", "project_role": "manager"},
    {"username": "alice", "project_role": "user"}
  ]
}

Remove a single member
POST /api/projects/my-project/remove_member
Body:

{
  "usernames": "john_doe"
}

Remove multiple members (batch)
POST /api/projects/my-project/remove_member
Body:

{
  "usernames": ["john_doe", "jane@example.com", "alice"]
}

Update existing member role (via add_member)
POST /api/projects/my-project/add_member
Body:

{
  "members": {
    "username": "jane",
    "project_role": "admin"
  }
}

Race Condition Fix

Previously, concurrent automations had to read and rewrite the entire project member list using the set_members endpoint, which led to race conditions and unintended overwrites. This PR introduces atomic add_member and remove_member operations that support both single and batch operations, allowing changes to apply independently and safely—even when triggered simultaneously.

@peterschmidt85
Copy link
Contributor

Why not allow adding/removing a batch of users?

@haydnli-shopify haydnli-shopify force-pushed the PR2-add-single-member-endpoint branch from 5aa6f22 to 95d076d Compare June 9, 2025 19:55
@haydnli-shopify haydnli-shopify changed the title Add single member endpoints to fix race conditions (#2760) Add batch of members endpoints to fix race conditions (#2760) Jun 9, 2025
@peterschmidt85
Copy link
Contributor

peterschmidt85 commented Jun 10, 2025

One more thought:
In order to support Join or Leave functionality, can we allow add_members and remove_members when a user adds/removes themselves to a public project – without reqing project/global admin rights.

One this is done, we can also add UI via a separate PR.

@haydnli-shopify haydnli-shopify force-pushed the PR2-add-single-member-endpoint branch 8 times, most recently from e89a648 to 146cce7 Compare June 10, 2025 18:53
@haydnli-shopify haydnli-shopify requested a review from colinjc June 11, 2025 13:35
@haydnli-shopify haydnli-shopify force-pushed the PR2-add-single-member-endpoint branch 2 times, most recently from f00e467 to 5ca77f9 Compare June 12, 2025 14:29
@haydnli-shopify haydnli-shopify force-pushed the PR2-add-single-member-endpoint branch from 437899b to 806c095 Compare June 13, 2025 14:38
@haydnli-shopify haydnli-shopify marked this pull request as ready for review June 16, 2025 18:13
@haydnli-shopify
Copy link
Contributor Author

@r4victor Both ticket are ready for review
PR for backend APIs: #2779
PR for UI: #2795

@haydnli-shopify haydnli-shopify requested a review from r4victor June 18, 2025 14:04
@peterschmidt85
Copy link
Contributor

Is this PR a part of #2795? If so, should it be closed?

@haydnli-shopify
Copy link
Contributor Author

Is this PR a part of #2795? If so, should it be closed?

Yes it is part of #2795, in order to make the code review easier, we split the PR into two different one, once this is merged, the changes in this PR cannot be seen in the UI one

@haydnli-shopify haydnli-shopify force-pushed the PR2-add-single-member-endpoint branch from 948c5e4 to f15435f Compare June 23, 2025 15:19
@haydnli-shopify haydnli-shopify force-pushed the PR2-add-single-member-endpoint branch from f15435f to 2d5ce9e Compare June 23, 2025 15:49
@peterschmidt85
Copy link
Contributor

@r4victor please confirm if this is good to merge

@haydnli-shopify haydnli-shopify requested a review from r4victor June 25, 2025 14:25
@peterschmidt85
Copy link
Contributor

@haydnli-shopify Since we merged #2795, this PR can be closed, right?

@haydnli-shopify
Copy link
Contributor Author

@haydnli-shopify Since we merged #2795, this PR can be closed, right?

Yes, i will close it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Endpoint for adding single member to project
5 participants