Skip to content

Include asset locations in GetBuild Api calls - #6532

Merged
dkurepa merged 12 commits into
dotnet:mainfrom
dkurepa:dkurepa/BuildApiFix
Aug 3, 2026
Merged

Include asset locations in GetBuild Api calls#6532
dkurepa merged 12 commits into
dotnet:mainfrom
dkurepa:dkurepa/BuildApiFix

Conversation

@dkurepa

@dkurepa dkurepa commented Jul 30, 2026

Copy link
Copy Markdown
Member

Copilot AI review requested due to automatic review settings July 30, 2026 15:44
premun
premun previously approved these changes Jul 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Build retrieval paths to ensure asset location data is eagerly loaded when fetching a single build, so API consumers receive complete asset metadata (including locations) from GetBuild calls.

Changes:

  • Updated GetBuild queries in PCS API versioned controllers to ThenInclude(a => a.Locations) after including assets.
  • Aligned the BAR SQL client build query to include asset locations with a clearer lambda parameter name.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/ProductConstructionService/ProductConstructionService.Api/Api/v2020_02_20/Controllers/BuildsController.cs Eager-loads Asset.Locations in GetBuild query for the 2020-02-20 API.
src/ProductConstructionService/ProductConstructionService.Api/Api/v2019_01_16/Controllers/BuildsController.cs Eager-loads Asset.Locations in GetBuild query for the 2019-01-16 API.
src/ProductConstructionService/ProductConstructionService.Api/Api/v2018_07_16/Controllers/BuildsController.cs Eager-loads Asset.Locations in GetBuild query for the 2018-07-16 API.
src/Maestro/Maestro.DataProviders/SqlBarClient.cs Keeps asset-location include in BAR build query; updates lambda parameter naming for clarity.

Comment on lines 95 to 99
.Include(b => b.BuildChannels)
.ThenInclude(bc => bc.Channel)
.Include(b => b.Assets)
.ThenInclude(a => a.Locations)
.FirstOrDefaultAsync();
Comment on lines 87 to 91
.Include(b => b.BuildChannels)
.ThenInclude(bc => bc.Channel)
.Include(b => b.Assets)
.ThenInclude(a => a.Locations)
.FirstOrDefaultAsync();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dkurepa this sounds possibly important

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also we should check why we have QuerySplittingBehavior set

Comment on lines 151 to 155
.Include(b => b.BuildChannels)
.ThenInclude(bc => bc.Channel)
.Include(b => b.Assets)
.ThenInclude(a => a.Locations)
.FirstOrDefaultAsync();
Comment on lines +89 to +90
.Include(b => b.Assets)
.ThenInclude(a => a.Locations)
Comment on lines +97 to +98
.Include(b => b.Assets)
.ThenInclude(a => a.Locations)
…Api/v2019_01_16/Controllers/BuildsController.cs

Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>
premun
premun previously approved these changes Jul 31, 2026
@dkurepa
dkurepa enabled auto-merge (squash) July 31, 2026 12:28
@dkurepa
dkurepa requested a review from premun August 3, 2026 08:39
premun
premun previously approved these changes Aug 3, 2026

@premun premun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any danger of some places now not having the assets when they had them before?

@dkurepa

dkurepa commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Any danger of some places now not having the assets when they had them before?

I don't think so, I went through it carefully, first without the default value, so I had it set explicitly everywhere, except the tests

@dkurepa
dkurepa merged commit 0b52ebd into dotnet:main Aug 3, 2026
8 of 11 checks passed
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.

3 participants