Include asset locations in GetBuild Api calls - #6532
Conversation
There was a problem hiding this comment.
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
GetBuildqueries in PCS API versioned controllers toThenInclude(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. |
| .Include(b => b.BuildChannels) | ||
| .ThenInclude(bc => bc.Channel) | ||
| .Include(b => b.Assets) | ||
| .ThenInclude(a => a.Locations) | ||
| .FirstOrDefaultAsync(); |
| .Include(b => b.BuildChannels) | ||
| .ThenInclude(bc => bc.Channel) | ||
| .Include(b => b.Assets) | ||
| .ThenInclude(a => a.Locations) | ||
| .FirstOrDefaultAsync(); |
There was a problem hiding this comment.
Also we should check why we have QuerySplittingBehavior set
| .Include(b => b.BuildChannels) | ||
| .ThenInclude(bc => bc.Channel) | ||
| .Include(b => b.Assets) | ||
| .ThenInclude(a => a.Locations) | ||
| .FirstOrDefaultAsync(); |
| .Include(b => b.Assets) | ||
| .ThenInclude(a => a.Locations) |
| .Include(b => b.Assets) | ||
| .ThenInclude(a => a.Locations) |
This reverts commit d2c160e.
…Api/v2019_01_16/Controllers/BuildsController.cs Co-authored-by: Přemek Vysoký <premek.vysoky@microsoft.com>
premun
left a comment
There was a problem hiding this comment.
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 |
#6534
#6227