Skip to content

Conversation

@kurotych
Copy link
Member

@kurotych kurotych commented Jul 11, 2025

asserted_location_changed_at = refreshed_at if a new (arrived) radio has location.

Add PostMigrateMobileTracker command to fill in missing values caused by a bug

Ex:
RUST_LOG=debug .mobile-config -c ./mobile-config.toml post-migrate-mobile-tracker

@kurotych kurotych changed the title Fix TrackedMobileRadio logic Fix TrackedMobileRadio asserted_location_changed_at Jul 11, 2025
@kurotych kurotych requested a review from Copilot July 11, 2025 15:03
Copy link

Copilot AI left a comment

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 fixes the initialization and backfill of asserted_location_changed_at for tracked mobiles and adds a one-shot command to migrate missing values.

  • Change add_db_record to accept Option<i64> for location and populate num_location_asserts.
  • Update TrackedMobileRadio::new to set asserted_location_changed_at when a location is present.
  • Implement post_migrate_mobile_tracker_locations and wire PostMigrateMobileTracker into the CLI.
  • Add new tests covering location updates for radios.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
mobile_config/tests/mobile_radio_tracker.rs Update test function names, adapt signature to Option<i64>, add tests for location changes
mobile_config/tests/gateway_service.rs Update gateway tests to pass Some(asset_hex_idx) to add_db_record
mobile_config/tests/common/mod.rs Change add_db_record and add_mobile_hotspot_infos to use Option<i64> and include num_location_asserts
mobile_config/src/mobile_radio_tracker.rs Set initial asserted_location_changed_at, remove old CSV logic, add post_migrate_mobile_tracker_locations
mobile_config/src/main.rs Replace MigrateMobileTracker with PostMigrateMobileTracker command in CLI
Comments suppressed due to low confidence (3)

mobile_config/src/mobile_radio_tracker.rs:301

  • Add a doc comment (///) above this function to describe its purpose, parameters, and high-level behavior, which will help future maintainers understand the migration steps.
pub async fn post_migrate_mobile_tracker_locations(

mobile_config/src/main.rs:48

  • Introduce an integration or end-to-end test for the PostMigrateMobileTracker command to verify that it correctly backfills asserted_location_changed_at under various scenarios.
            Cmd::PostMigrateMobileTracker => {

mobile_config/tests/mobile_radio_tracker.rs:10

  • [nitpick] The test name mt_handle_entity_duplicates is abbreviated; consider renaming it to mobile_tracker_handle_entity_duplicates for clarity and consistency with other descriptive test names.
async fn mt_handle_entity_duplicates(pool: PgPool) {

);

for (k, _v) in tracked_radios {
if let Some(mobile_info) = mobile_infos.iter().find(|v| v.entity_key == k) {
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

To avoid O(n²) lookups, build a HashMap from mobile_infos keyed by entity_key before this loop so you can do O(1) lookups instead of .iter().find() on each iteration.

Suggested change
if let Some(mobile_info) = mobile_infos.iter().find(|v| v.entity_key == k) {
if let Some(mobile_info) = mobile_infos_map.get(&k) {

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@kurotych kurotych Jul 11, 2025

Choose a reason for hiding this comment

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

A small number of elements (<200) is expected

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option is swap around the iteration.

for mobile_radio in mobile_radios {
    if tracked_radios.contains_key(&mobile_radio.entity_key) {
      // ... Do the work
    }
}

@kurotych kurotych marked this pull request as ready for review July 11, 2025 15:06
@kurotych kurotych merged commit 4d59731 into main Jul 15, 2025
55 checks passed
@kurotych kurotych deleted the fix-mobile-tracker-bug branch July 15, 2025 11:37
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.

4 participants