Add support to sync cost centers with teams #20
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…upport - Add teams-based cost center assignment mode (alternative to PRU-based mode) - Support both organization-level and enterprise-level teams - Implement auto mode (one cost center per team) and manual mode (explicit mappings) - Add required scope parameter: 'organization' or 'enterprise' - Implement single cost center constraint per user (last team wins for multi-team users) - Add comprehensive documentation (README, TEAMS_INTEGRATION, TEAMS_QUICKSTART) - Add new GitHub API methods for teams and memberships - Fix enterprise teams API to handle direct user objects (not wrapped in membership objects) - Document that incremental mode is NOT supported for teams mode
- Add get_cost_center_members() API to fetch current cost center membership - Add remove_users_from_cost_center() API to remove users from cost centers - Add teams.remove_orphaned_users config option (default: false) - Implement orphaned user detection in sync_team_assignments() - Show orphaned users in plan mode - Remove orphaned users in apply mode when configured - Works with both organization and enterprise team scopes - Add configuration display in main.py output
- Remove attempt to fetch cost center members in plan mode (IDs don't exist yet) - Show clear message that orphaned detection is enabled and will run in apply mode - Remove unused _show_orphaned_users_plan method - Simplify plan mode messaging
- Parse improved API 409 response to extract existing cost center UUID - Avoid unnecessary GET request to list all cost centers when name conflict occurs - Fall back to name search only if UUID extraction fails - Add regex pattern to match 'existing cost center UUID: <uuid>' format - Improves performance by reducing API calls
- Add count of cost centers being checked - Add debug logging showing current vs expected members per cost center - Helps troubleshoot orphaned user detection issues
- Always detect orphaned users regardless of remove_orphaned_users setting - Only remove orphaned users when remove_orphaned_users is true - When removal is disabled, log warning that users will remain in cost centers - Add 'remove' parameter to _remove_orphaned_users method - Improve summary messages to indicate whether removal was performed or skipped
- Check current cost center members before adding users - Only POST users who are not already in the cost center - Avoids unnecessary API calls and potential issues with re-adding existing users - Return success for users already assigned (no action needed) - Improves efficiency and reduces API rate limit usage
- Change enterprise from 'avocado-corp' to empty string (use env var) - Change organizations from 'cost-center-automation' to placeholder 'your-org-name' - Ensures config.yaml doesn't contain specific test values
This was an internal development/explanation document and not needed in the final PR
This was an internal implementation summary and not needed in the final PR
- Make teams mode a first-class experience (not a bolt-on)
* Update main.py help text and docstring to equally highlight both modes
* Add comprehensive examples in argparse showing both PRU and Teams modes
* Improve flag descriptions to be mode-agnostic or clearly specify mode
- Implement standardized bracket notation naming
* Remove configurable template system from teams mode
* Use fixed naming: [enterprise team] {team-name} and [org team] {org-name}/{team-name}
* Update config examples to document naming conventions
- Simplify and streamline documentation
* Reduce README from 650 to 346 lines (47% reduction)
* Add collapsible Quick Start sections for both modes
* Move detailed Teams docs to dedicated files
* Focus on getting users started quickly
* Make both modes equally prominent
- Clean up configuration files
* Set teams.enabled default to false
* Update config.example.yaml with bracket notation docs
* Set remove_orphaned_users default to true
* Remove obsolete template configuration
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive GitHub Teams Integration for cost center automation, adding an alternative "Teams-based mode" alongside the existing PRU-based assignments. The integration supports both organization and enterprise team scopes with automatic/manual assignment modes and includes orphaned user detection.
Key changes:
- Added teams-based cost center assignment with organization/enterprise scope support
- Implemented orphaned user detection and optional removal functionality
- Enhanced API with new team management and cost center membership methods
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/teams_cost_center_manager.py | New teams integration manager with auto/manual modes and multi-org support |
| src/github_api.py | Enhanced with team listing, member fetching, and cost center management APIs |
| src/config_manager.py | Added teams configuration properties loading |
| main.py | Integrated teams mode with validation and execution flow |
| config/config.yaml | Added comprehensive teams configuration section |
| config/config.example.yaml | Added teams configuration examples and documentation |
| TEAMS_QUICKSTART.md | Complete quickstart guide for teams integration setup |
| TEAMS_INTEGRATION.md | Comprehensive teams mode reference documentation |
| README.md | Updated with teams mode overview and usage examples |
| ORPHANED_USERS_FEATURE.md | Detailed documentation for orphaned user detection feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+33
to
+34
| self.members_cache: Dict[str, List[str]] = {} # "org/team_slug" or "team_slug" -> list of usernames | ||
| self.cost_center_cache: Dict[str, str] = {} # "org/team_slug" or "team_slug" -> cost_center_id |
There was a problem hiding this comment.
The cache key format comments are inconsistent with the actual implementation. For enterprise scope, the cache keys are just team_slug (no org prefix), but the comments suggest org/team_slug format for all caches.
Suggested change
| self.members_cache: Dict[str, List[str]] = {} # "org/team_slug" or "team_slug" -> list of usernames | |
| self.cost_center_cache: Dict[str, str] = {} # "org/team_slug" or "team_slug" -> cost_center_id | |
| self.members_cache: Dict[str, List[str]] = {} # org scope: "org/team_slug", enterprise scope: "team_slug" -> list of usernames | |
| self.cost_center_cache: Dict[str, str] = {} # org scope: "org/team_slug", enterprise scope: "team_slug" -> cost_center_id |
Copilot uses AI. Check for mistakes.
| # Enterprise name is required. Prefer setting via environment var GITHUB_ENTERPRISE. | ||
| # Leave as placeholder or blank if supplying via environment. | ||
| enterprise: "REPLACE_WITH_ENTERPRISE_SLUG" | ||
| enterprise: "" |
There was a problem hiding this comment.
Setting enterprise to an empty string could cause issues if not properly validated. Consider using null/~ or adding validation to ensure it's set via environment variable.
Suggested change
| enterprise: "" | |
| enterprise: null |
Copilot uses AI. Check for mistakes.
Comment on lines
+16
to
18
| prus_exception_users: [] | ||
| # - "example.user1" | ||
| # - "example.user2" |
There was a problem hiding this comment.
The prus_exception_users field has been changed from commented examples to an empty list, which removes helpful inline documentation for users setting up the configuration.
Suggested change
| prus_exception_users: [] | |
| # - "example.user1" | |
| # - "example.user2" | |
| # Example: | |
| # - "example.user1" | |
| # - "example.user2" | |
| prus_exception_users: [] |
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Teams Integration and Assignment Logic
src/teams_cost_center_manager.py) supporting both auto and manual modes for assigning users to cost centers based on GitHub team membership. Multi-organization support and customizable cost center naming are included.Orphaned User Detection and Removal
get_cost_center_members,remove_users_from_cost_center) and updated configuration options (remove_orphaned_users) to support this feature.Documentation and Configuration Updates
README.md,TEAMS_INTEGRATION.md,TEAMS_QUICKSTART.md,IMPLEMENTATION_SUMMARY.md,config/config.yaml,config/config.example.yaml) to reflect the new teams mode, assignment logic, orphaned user handling, and best practices. [1] [2] [3]Testing and Logging Improvements
Migration and Best Practices