Conversation
- Introduced `AGENTS.md` to outline project structure, build commands, coding style, testing guidelines, and commit practices. - Updated `OPTIMIZED_USAGE_EXAMPLES.md` with new usage examples reflecting recent changes in work item queries. - Added `user_email_mapping.json` for mapping user names to emails. - Created `workitemsoctober.json` to store work item data for October. - Enhanced `efficiency_calculator.py` and `WorkItemOperations.py` to preserve exact Logic App estimates by disabling certain adjustments. - Updated `IMPLEMENTATION_ANALYSIS.md` to document intentional changes and their impact on efficiency metrics.
…tion - Introduced `.env.example` to provide a template for environment variables, including Azure DevOps organization and Logic App URL. - Added `LOGIC_APP_MIGRATION.md` to document the migration from WIQL/Analytics API to Azure Logic App for work item data retrieval. - Implemented `LogicAppClient` for fetching work items based on date ranges and user emails, enhancing the efficiency of data retrieval. - Updated `WorkItemOperations` to utilize the Logic App client, removing legacy WIQL methods and simplifying the querying process. - Enhanced efficiency calculations to incorporate Logic App estimates directly, improving accuracy in productivity metrics. - Added email mapping utilities to resolve user names to email addresses for querying work items. - Updated main entry point to reflect new command structure and required arguments for Logic App integration.
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a significant architectural change by migrating from WIQL/Analytics API-based work item fetching to Azure Logic App with Fabric Data Warehouse as the primary data source. The refactoring aims to simplify the workflow and centralize work item data retrieval.
Key Changes:
- Adds Logic App integration with new helper modules for HTTP client, email resolution, timezone utilities, and environment configuration
- Updates CLI to require
--start-dateand--end-datearguments, making the Logic App flow the default for work item queries - Includes comprehensive migration documentation and configuration templates
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| user_email_mapping.json | New configuration file mapping developer names to email addresses for Logic App queries |
| helpers/timezone_utils.py | New utility module for Mexico City timezone conversions and date boundary calculations |
| helpers/logic_app_client.py | New HTTP client for Logic App with retry logic and exponential backoff |
| helpers/env_loader.py | New environment variable loader with validation for required configuration |
| helpers/email_mapping.py | New utility for resolving user names/emails from mapping file |
| helpers/init.py | New package initialization file for helpers module |
| entry_points/main.py | Updated CLI to use Logic App as default, with legacy arguments marked for compatibility |
| classes/efficiency_calculator.py | Updated to use exact Logic App estimates with documentation about disabled transformations |
| classes/WorkItemOperations.py | Added new method get_work_items_from_logic_app() implementing the Logic App workflow |
| documentation/IMPLEMENTATION_ANALYSIS.md | Added documentation about intentionally disabled estimate transformations |
| config/azure_devops_config.json | Updated default developers list to match current team |
| OPTIMIZED_USAGE_EXAMPLES.md | Updated usage example with new developer name |
| LOGIC_APP_MIGRATION.md | New comprehensive migration guide documenting the architectural change |
| AGENTS.md | New repository guidelines for contributors |
| .env.example | New environment configuration template with Logic App URL |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| mx_dt = convert_utc_to_mexico_city(dt) | ||
| return mx_dt.strftime("%Y-%m-%d %H:%M:%S %Z") |
There was a problem hiding this comment.
The docstring states the return format as "YYYY-MM-DD HH:MM:SS CST/CDT" but the actual format uses %Z which will output the timezone abbreviation as determined by the system. For America/Mexico_City, this could be "CST" or "CDT" depending on daylight saving time, but it's not guaranteed to match the documented format. Consider clarifying the documentation or using a more explicit format string.
| """ | |
| mx_dt = convert_utc_to_mexico_city(dt) | |
| return mx_dt.strftime("%Y-%m-%d %H:%M:%S %Z") | |
| The timezone abbreviation is determined by the UTC offset: | |
| - CST for UTC-6 (standard time) | |
| - CDT for UTC-5 (daylight saving time) | |
| """ | |
| mx_dt = convert_utc_to_mexico_city(dt) | |
| # Mexico City: UTC-6 is CST, UTC-5 is CDT | |
| offset_hours = mx_dt.utcoffset().total_seconds() / 3600 | |
| if offset_hours == -6: | |
| tz_abbr = "CST" | |
| elif offset_hours == -5: | |
| tz_abbr = "CDT" | |
| else: | |
| tz_abbr = mx_dt.strftime("%Z") # fallback, should not happen | |
| return mx_dt.strftime("%Y-%m-%d %H:%M:%S ") + tz_abbr |
| "Table1": [ | ||
| { | ||
| "WorkItemId": 42964, | ||
| "AssignedToUser": "Carlos Vazquez", |
There was a problem hiding this comment.
The example shows "AssignedToUser": "Carlos Vazquez" but based on the code in helpers/email_mapping.py and user_email_mapping.json, this field should contain an email address, not a name. The example should be corrected to show "AssignedToUser": "carlos.vazquez@inbest.cloud" to match the actual expected format.
| "AssignedToUser": "Carlos Vazquez", | |
| "AssignedToUser": "carlos.vazquez@inbest.cloud", |
| - StartDate: Start date | ||
| - TargetDate: Target date | ||
| - OriginalEstimate: Estimated hours | ||
| - Project_Name: Azure DevOps project name (REQUIRED for revision fetching) |
There was a problem hiding this comment.
The docstring states "Project_Name: Azure DevOps project name (REQUIRED for revision fetching)" but the code below (lines 1397-1398) sets these fields to None if project_name is empty, rather than raising an error. The documentation should clarify whether this field is truly required or if it's optional with a fallback behavior.
| if items_without_project > 0: | ||
| print(f" ⚠️ Warning: {items_without_project} work items missing Project_Name") | ||
| print(f" These items will be skipped for efficiency calculations") | ||
| print(f" Please ensure Logic App returns 'Project_Name' field for all work items") |
There was a problem hiding this comment.
The warning message "
| print(f" Please ensure Logic App returns 'Project_Name' field for all work items") | |
| print(f" Please ensure Logic App returns the Project Name for all work items") |
| Handle work item querying with dynamic filtering and KPI calculations. | ||
|
|
||
| NEW DEFAULT: Uses Logic App for work item fetching. | ||
| Legacy WIQL/Analytics method removed per refactoring requirements. |
There was a problem hiding this comment.
The comment states "NEW DEFAULT: Uses Logic App for work item fetching" and "Legacy WIQL/Analytics method removed", but looking at the broader codebase context, the old method get_work_items_with_efficiency_optimized still exists (line 1522 onwards in WorkItemOperations.py). The comment should clarify that the legacy method still exists but is no longer used by the CLI, or the method should actually be removed if it's truly deprecated.
| Legacy WIQL/Analytics method removed per refactoring requirements. | |
| Note: The legacy WIQL/Analytics method (`get_work_items_with_efficiency_optimized`) still exists in the codebase but is no longer used by the CLI. |
| python run.py --query-work-items --assigned-to "Luis Nocedal,Carlos Vazquez,Diego Lopez,Alejandro Valenzuela,Gerardo Melgoza,Hans Izarraraz,Osvaldo de Luna,Uriel Cortés,Emmanuel Pérez,Fernando Alcaraz,Damian Gaspar,Cristian Soria,Daniel Cayola,Ximena Segura, Andrés Escobedo, Alvaro Torres, Pablo Ruiz, Sebastián Rojas, Fernando Hernández" --start-date "2025-09-01" --end-date "2025-09-30" --optimized --export-csv "september_results.csv" | ||
| python run.py --query-work-items --assigned-to "Luis Nocedal,Carlos Vazquez,Diego Lopez,Alejandro Valenzuela,Gerardo Melgoza,Hans Izarraraz,Osvaldo de Luna,Uriel Cortés,Emmanuel Pérez,Fernando Alcaraz,Damian Gaspar,Cristian Soria,Daniel Cayola,Ximena Segura, Andrés Escobedo, Alvaro Torres, Pablo Ruiz, Sebastián Rojas, Fernando Hernández, Daniel Reyes" --start-date "2025-10-01" --end-date "2025-10-31" --optimized --export-csv "october_test_hours.csv" | ||
|
|
||
| python run.py --query-work-items --assigned-to "Luis Nocedal,Carlos Vazquez,Diego Lopez,Alejandro Valenzuela,Gerardo Melgoza,Hans Izarraraz,Osvaldo de Luna,Uriel Cortés,Emmanuel Pérez,Fernando Alcaraz,Damian Gaspar,Cristian Soria,Daniel Cayola,Ximena Segura, Andrés Escobedo, Alvaro Torres, Pablo Ruiz, Sebastián Rojas, Fernando Hernández, Daniel Reyes" --start-date "2025-09-01" --end-date "2025-09-30" --optimized --export-csv "september_results_final.csv" |
There was a problem hiding this comment.
This example command still uses the legacy --optimized flag. According to the migration guide and CLI changes, the new Logic App flow doesn't require this flag. The example should be updated to match the new workflow.
| @@ -0,0 +1,22 @@ | |||
| { | |||
| "Luis Nocedal": "luis.nodecal@inbest.cloud", | |||
There was a problem hiding this comment.
Spelling error in email address: "nodecal" should be "nocedal" to match the user's name "Luis Nocedal".
| "Luis Nocedal": "luis.nodecal@inbest.cloud", | |
| "Luis Nocedal": "luis.nocedal@inbest.cloud", |
| 'original_estimate': item.get('OriginalEstimate'), | ||
| 'fabric_enriched': True, | ||
| 'revisions': [], | ||
| 'project_id': project_name if project_name else None, # Use project name as identifier |
There was a problem hiding this comment.
Using project_name as the value for project_id is confusing and could lead to bugs. The field name project_id suggests it should contain an ID, not a name. Consider either renaming the field to project_identifier or storing the actual project ID if available.
| 'project_id': project_name if project_name else None, # Use project name as identifier | |
| 'project_identifier': project_name if project_name else None, # Use project name as identifier |
| from datetime import datetime, timedelta | ||
| import json | ||
| from typing import List, Dict, Optional, Any | ||
| from typing import List, Dict, Optional, Any, Tuple |
There was a problem hiding this comment.
Import of 'Any' is not used.
| from typing import List, Dict, Optional, Any, Tuple | |
| from typing import List, Dict, Optional, Tuple |
| import requests | ||
| import logging | ||
| import time | ||
| from typing import List, Dict, Any, Optional |
There was a problem hiding this comment.
Import of 'Optional' is not used.
| from typing import List, Dict, Any, Optional | |
| from typing import List, Dict, Any |
No description provided.