Skip to content

Conversation

@JaminenB
Copy link

More information in issue:

CHANGES:

  • ADD: if-statement to check if ADD_MEMBER attribute is true and if the user is not a member of the organization
    • Uses the org.add_or_update_membership function to add the user to the organization
  • CHANGE: if-statement to add a user to a GitHub team only if they are a member of the organization already

More information in issue: 
- 3rd-party-integrations#214 

CHANGES:
- ADD: if-statement to check if ADD_MEMBER attribute is `true` and if the user is not a member of the organization 
  - Uses the org.add_or_update_membership function to add the user to the organization
- CHANGE: if-statement to add a user to a GitHub team only if they are a member of the organization already
@JaminenB JaminenB requested a review from primetheus as a code owner August 29, 2025 21:26
@JaminenB JaminenB marked this pull request as draft August 29, 2025 21:26
@JaminenB
Copy link
Author

Pending further testing. Changed to draft.

@JaminenB JaminenB marked this pull request as ready for review August 29, 2025 21:41
@JaminenB
Copy link
Author

JaminenB commented Aug 29, 2025

My testing shows that users are now being added to the organization as intended whereas before they were not being added even if the ADD_MEMBER attribute was set to true. This confirms what I was thinking in #214 .

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request implements support for the ADD_MEMBER attribute to automatically add users to a GitHub organization before adding them to teams. The change addresses issue #214 by modifying the user synchronization logic to check organization membership and optionally add users to the organization when ADD_MEMBER is enabled.

Key changes:

  • Added logic to check if users are organization members and add them when ADD_MEMBER is true
  • Modified team membership addition to only occur after confirming organization membership
  • Added informational logging for organization membership operations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +215 to +223
if not org.is_member(user) and ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
else:
print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false")
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The else branch log message is misleading because it will be printed even when the user IS already a member of the organization. The message should differentiate between the two conditions: (1) user is already a member, or (2) ADD_MEMBER is false and the user is not a member. Consider splitting this into two separate conditions or revising the message to accurately reflect the actual state.

Suggested change
if not org.is_member(user) and ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
else:
print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false")
if org.is_member(user):
# User is already a member of the organization; no need to add them
print(f"User: {user} is already a member of the {org.login} organization")
else:
if ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
else:
# User is not a member, but automatic addition is disabled
print(
f"User: {user} is not a member of the {org.login} organization and ADD_MEMBER is false; not adding"
)

Copilot uses AI. Check for mistakes.
org.add_or_update_membership(user)
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The pass statement after the exception handler is redundant. The pass statement does nothing here and should be removed for cleaner code.

Copilot uses AI. Check for mistakes.
Comment on lines +214 to +226
# Add user to org if they are not already a member and the ADD_MEMBER attribute is "true"
if not org.is_member(user) and ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
else:
print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false")

# Add user to team if they are a member of the org
if org.is_member(user):
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

There's a potential race condition here. After adding a user to the organization at line 218, the code immediately checks org.is_member(user) at line 226. However, there's no guarantee that the membership will be immediately reflected in the org.is_member() check, especially if there are API delays or eventual consistency issues. If the membership hasn't propagated yet, the user won't be added to the team even though they were just added to the organization. Consider either: (1) assuming membership is successful after add_or_update_membership() completes without error, or (2) adding a retry mechanism with a small delay.

Suggested change
# Add user to org if they are not already a member and the ADD_MEMBER attribute is "true"
if not org.is_member(user) and ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
else:
print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false")
# Add user to team if they are a member of the org
if org.is_member(user):
# Determine if the user is already a member of the org
org_member = org.is_member(user)
# Add user to org if they are not already a member and the ADD_MEMBER attribute is "true"
if not org_member and ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
# Assume membership is successful if no exception was raised
org_member = True
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
else:
print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false")
# Add user to team if they are a member of the org (including those just added)
if org_member:

Copilot uses AI. Check for mistakes.
Comment on lines +214 to +226
# Add user to org if they are not already a member and the ADD_MEMBER attribute is "true"
if not org.is_member(user) and ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
else:
print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false")

# Add user to team if they are a member of the org
if org.is_member(user):
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The error handling catches all exceptions but only prints the error and continues execution. This means if adding a user to the organization fails, the code will still attempt to add them to the team at line 226, which will likely fail since they're not in the organization. Consider either: (1) tracking the success/failure state and skipping the team addition if org addition failed, or (2) re-raising the exception after logging to prevent further processing of this user.

Suggested change
# Add user to org if they are not already a member and the ADD_MEMBER attribute is "true"
if not org.is_member(user) and ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
pass
else:
print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false")
# Add user to team if they are a member of the org
if org.is_member(user):
org_membership_added = False
# Add user to org if they are not already a member and the ADD_MEMBER attribute is "true"
if not org.is_member(user) and ADD_MEMBER:
try:
print(f"User: {user} is not in the {org.login} organization. Attempting to add.")
org.add_or_update_membership(user)
org_membership_added = True
except Exception as e:
print(f"Error adding {user} to {org.login}: {e}")
else:
print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false")
# Add user to team if they are a member of the org or we just successfully added them
if org.is_member(user) or org_membership_added:

Copilot uses AI. Check for mistakes.
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.

2 participants