Add creation date to InProgressPullRequest#6019
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a CreationDate parameter to the InProgressPullRequest model to track when pull requests are created, addressing issue #6014. The change extends from the internal model through the API layer to the UI, with a temporary migration strategy to populate the creation date for existing tracked PRs.
Changes:
- Added CreationDate field to InProgressPullRequest model, API response models, and UI display
- Modified PullRequest model in DarcLib to include CreatedAt timestamp from GitHub and Azure DevOps
- Implemented a temporary workaround to populate CreationDate for existing PRs by fetching it from remote repositories
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ProductConstructionService/ProductConstructionService.DependencyFlow/Model/InProgressPullRequest.cs | Added CreationDate property to track when a PR was created |
| src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs | Added UpdatePullRequestCreationDateAsync method to set creation date when fetching PR info from remotes |
| src/ProductConstructionService/ProductConstructionService.BarViz/Pages/PullRequests.razor | Added "Created" column to display PR age using ToTimeAgo extension |
| src/ProductConstructionService/ProductConstructionService.Api/Api/v2020_02_20/Controllers/PullRequestController.cs | Added CreationDate to TrackedPullRequest record in API response |
| src/ProductConstructionService/Microsoft.DotNet.ProductConstructionService.Client/Generated/Models/TrackedPullRequest.cs | Added CreationDate property to client model |
| src/Microsoft.DotNet.Darc/DarcLib/IRemoteGitRepo.cs | Added CreatedAt property to PullRequest model |
| src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs | Populated CreatedAt from GitHub PR data |
| src/Microsoft.DotNet.Darc/DarcLib/AzureDevOpsClient.cs | Populated CreatedAt from Azure DevOps PR data |
|
|
||
| public bool BlockedFromFutureUpdates { get; set; } = false; | ||
|
|
||
| public DateTime CreationDate { get; set; } |
There was a problem hiding this comment.
The CreationDate property lacks initialization and should have a default value to handle existing tracked PRs that don't have this field set yet. Similar to LastUpdate and LastCheck properties (lines 45-47), this should be initialized to DateTime.UtcNow to ensure consistency and avoid issues with default DateTime values. Consider initializing it with a default value like: public DateTime CreationDate { get; set; } = DateTime.UtcNow;
| public DateTime CreationDate { get; set; } | |
| public DateTime CreationDate { get; set; } = DateTime.UtcNow; |
| return status != PullRequestStatus.Invalid; | ||
| } | ||
|
|
||
| private async Task UpdatePullRequestCreationDateAsync(InProgressPullRequest pr, DateTime creationDate) |
There was a problem hiding this comment.
There is extra whitespace between 'pr,' and 'DateTime' in the parameter list. This should be a single space for consistency with C# formatting standards.
| private async Task UpdatePullRequestCreationDateAsync(InProgressPullRequest pr, DateTime creationDate) | |
| private async Task UpdatePullRequestCreationDateAsync(InProgressPullRequest pr, DateTime creationDate) |
| private async Task UpdatePullRequestCreationDateAsync(InProgressPullRequest pr, DateTime creationDate) | ||
| { | ||
| _logger.LogInformation("Checking in-progress pull request {url}", pullRequestCheck.Url); | ||
| var pr = await GetPullRequestStatusAsync(pullRequestCheck, isCodeFlow, tryingToUpdate: false); | ||
| return pr.Status != PullRequestStatus.Invalid; | ||
| pr.CreationDate = creationDate; //todo this is a temporary solution to update existing PRs, remove it | ||
| await _pullRequestState.SetAsync(pr); | ||
| } |
There was a problem hiding this comment.
The UpdatePullRequestCreationDateAsync method unconditionally overwrites CreationDate on every PR check or update. While this serves as a migration path for existing PRs without a CreationDate, it's inefficient and conceptually incorrect to keep updating an immutable creation timestamp. The method should only update CreationDate if it's currently unset (default value). Consider changing line 282 to: if (pr.CreationDate == default) { pr.CreationDate = creationDate; } and updating the TODO comment to clarify that once all existing PRs have been migrated, this entire workaround method can be removed.
#6014
Tested on codeflow PRs, dependency PRs, github, and azdo