Skip to content

RG-T41 Bug fixes #525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ucswift merged 1 commit into from
Sep 24, 2026
Merged

RG-T41 Bug fixes #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Core/Resgrid.Search/LuceneIndexHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,10 @@ private void StartBackgroundPullIfDue()
catch (Exception ex)
{
// Only one background pull runs at a time, so the counter has a single writer. Error, not Fatal: the reader
// keeps serving its last local revision and the next attempt backs off.
// keeps serving its last local revision and the next attempt backs off. The backoff runs from the failure,
// not from the attempt: a request that hangs until the client timeout would otherwise have used it all up.
var failures = ++_consecutivePullFailures;
_lastPullAttemptUtc = DateTime.UtcNow;
Logging.LogError(ex, $"Search index '{IndexName}' pull from the object store failed ({failures} in a row); next attempt in {PullInterval().TotalSeconds:0}s at the earliest.");
}
});
Expand Down
7 changes: 5 additions & 2 deletions Core/Resgrid.Services/Records/RecordsOccupancyService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,11 @@ async Task<string> AddressOf(Contact c)
list.Add(new SourceCandidate { Kind = RmsOccupancyCrosswalkSourceKind.Contact, SourceId = contact.ContactId, ContactId = contact.ContactId, DisplayName = contact.Name,
NormalizedAddress = address, Latitude = point.HasValue ? (decimal?)(decimal)point.Value.Latitude : null, Longitude = point.HasValue ? (decimal?)(decimal)point.Value.Longitude : null });
}
// Pois carry no DepartmentId column; a department owns its POIs through their POI type.
var poiTypes = (await _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId)) ?? Enumerable.Empty<PoiType>();
// Pois carry no DepartmentId column; a department owns its POIs through their POI type. The repository logs and
// returns null on failure (an empty query is an empty list), and treating that as "no POIs" would stamp an
// inventory that never saw them as complete.
var poiTypes = (await _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId))

@Resgrid-Bot Resgrid-Bot Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules high

External repository failure in _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId) can leave the IReadOnlyList<PoiType> poiTypes assignment without department context or return null. Catch Exception, map null to InvalidOperationException, log the failure with _logger.LogError and departmentId, and rethrow the original exception.

Kody rule violation: Add try-catch blocks for external calls

IReadOnlyList<PoiType> poiTypes;
try
{
    poiTypes = (await _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId)) ?? throw new InvalidOperationException("The department's points of interest could not be loaded.");
}
catch (Exception ex)
{
    _logger.LogError(ex, "Failed to load POI types for department {DepartmentId}", departmentId);
    throw;
}
Prompt for LLM 

File Core/Resgrid.Services/Records/RecordsOccupancyService.cs:

Line 498:

External repository failure in `_poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId)` can leave the `IReadOnlyList<PoiType> poiTypes` assignment without department context or return null. Catch `Exception`, map null to `InvalidOperationException`, log the failure with `_logger.LogError` and `departmentId`, and rethrow the original exception.

Suggested Code:

IReadOnlyList<PoiType> poiTypes;
try
{
    poiTypes = (await _poiTypes.GetPoiTypesByDepartmentIdAsync(departmentId)) ?? throw new InvalidOperationException("The department's points of interest could not be loaded.");
}
catch (Exception ex)
{
    _logger.LogError(ex, "Failed to load POI types for department {DepartmentId}", departmentId);
    throw;
}

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

?? throw new InvalidOperationException("The department's points of interest could not be loaded, so the inventory was not recorded. Try again.");
foreach (var poi in poiTypes.Where(t => t?.Pois != null).SelectMany(t => t.Pois).Where(p => p != null))
{
list.Add(new SourceCandidate { Kind = RmsOccupancyCrosswalkSourceKind.Poi, SourceId = poi.PoiId.ToString(), DisplayName = poi.Name, NormalizedAddress = AddressNormalizer.Normalize(poi.Address),
Expand Down
18 changes: 18 additions & 0 deletions Tests/Resgrid.Tests/Rms/RecordsOccupancyServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,24 @@ public async Task Inventory_groups_sources_by_address_and_proximity_and_suggests
_h.Crosswalks.Rows.Should().HaveCount(3);
}

[Test]
public async Task A_failed_poi_lookup_fails_the_inventory_instead_of_recording_it_without_pois()
{
SeedContactsWorld();
// PoiTypesRepository logs and returns null when its query fails; a department with no POI types gets an empty list.
_h.PoiTypes.Setup(p => p.GetPoiTypesByDepartmentIdAsync(Dept)).ReturnsAsync((IEnumerable<PoiType>)null);

Func<Task> inventory = () => _h.OccupancyService.InventoryCandidatesAsync(Dept, Admin);
await inventory.Should().ThrowAsync<InvalidOperationException>();
_h.Crosswalks.Rows.Should().BeEmpty();
var status = await _h.OccupancyService.GetReconciliationStatusAsync(Dept);

@Resgrid-Bot Resgrid-Bot Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kody code-review Kody Rules high

Unhandled task rejection from _h.OccupancyService.GetReconciliationStatusAsync(Dept) can fail the test without useful context at Tests/Resgrid.Tests/Rms/RecordsOccupancyServiceTests.cs:151, corresponding to Core/Resgrid.Services/Records/RecordsOccupancyService.cs:498. Catch Exception and use Assert.Fail to report the reconciliation-status retrieval error.

Kody rule violation: Handle async operations with proper error handling

ReconciliationStatus status;
try
{
    status = await _h.OccupancyService.GetReconciliationStatusAsync(Dept);
}
catch (Exception ex)
{
    Assert.Fail($"Failed to retrieve reconciliation status: {ex.Message}");
    return;
}
Prompt for LLM 

File Tests/Resgrid.Tests/Rms/RecordsOccupancyServiceTests.cs:

Line 146:

Unhandled task rejection from `_h.OccupancyService.GetReconciliationStatusAsync(Dept)` can fail the test without useful context at `Tests/Resgrid.Tests/Rms/RecordsOccupancyServiceTests.cs:151`, corresponding to `Core/Resgrid.Services/Records/RecordsOccupancyService.cs:498`. Catch `Exception` and use `Assert.Fail` to report the reconciliation-status retrieval error.

Suggested Code:

ReconciliationStatus status;
try
{
    status = await _h.OccupancyService.GetReconciliationStatusAsync(Dept);
}
catch (Exception ex)
{
    Assert.Fail($"Failed to retrieve reconciliation status: {ex.Message}");
    return;
}

Talk to Kody by mentioning @kody

Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.

​

​

status.InventoriedOn.Should().BeNull("an inventory that never saw the POIs is not recorded as run");
status.State.Should().Be(RmsOccupancyOwnershipState.ContactsOwned);

_h.PoiTypes.Setup(p => p.GetPoiTypesByDepartmentIdAsync(Dept)).ReturnsAsync(new List<PoiType>());
(await _h.OccupancyService.InventoryCandidatesAsync(Dept, Admin)).SourcesScanned.Should().Be(2, "no POI types is a successful, empty lookup");
}

[Test]
public async Task Linking_a_preplan_candidate_creates_an_occupancy_with_provenance_hazards_and_a_site_link_then_the_switch_becomes_possible()
{
Expand Down
Loading