fix: minor fixes to display of public map data

[joaquimds]

No description provided.

[Copilot]

Pull request overview

This pull request aims to fix minor display issues with public map data by improving handling of empty/null values and implementing a full data replacement strategy during imports.

Key changes:

• Added null/undefined handling in name building to prevent "null" or "undefined" strings from appearing
• Introduced a new utility function to detect empty objects and filter them out during Airtable imports
• Changed import behavior to delete all existing records before re-importing data

Reviewed changes

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

Show a summary per file

File	Description
src/utils/objects.ts	Adds new isEmptyObject utility function to check if objects contain only falsy values
src/utils/dataRecord.ts	Fixes null/undefined handling in buildName to prevent literal "null"/"undefined" strings in display names
src/server/repositories/DataRecord.ts	Adds deleteByDataSourceId repository method to remove all records for a data source
src/server/jobs/importDataSource.ts	Modifies import flow to delete all existing records before importing new data
src/server/adaptors/airtable.ts	Uses isEmptyObject to filter out empty Airtable records during fetch operations

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The logic for checking if an object is empty is incorrect. Using Boolean(o[k]) will treat falsy values like 0, false, "", and null as "empty", causing objects with legitimate falsy values to be incorrectly identified as empty. For example, {count: 0} or {enabled: false} would return true (empty) when they should return false (not empty).

Consider checking if the value is null or undefined instead, or use a different approach like checking if all values are null/undefined, depending on the intended behavior.

Suggested change

	if (Boolean(o[k])) {
	if (o[k] !== null && o[k] !== undefined) {

[Copilot]

Deleting all records before starting the import could lead to data loss if the import fails (e.g., network error, API failure, or exception during processing). If the import fails after line 34, all existing data will be lost with no way to recover it.

Consider either:

1. Using a transaction-like approach where old records are only deleted after successful import
2. Implementing a soft-delete or versioning strategy
3. Moving the delete operation to after successful import completion

[Copilot]

The new isEmptyObject utility function lacks test coverage. The repository has test coverage for other utility functions (e.g., tests/unit/utils/text.test.ts), so this function should also have tests to verify its behavior, especially given its use in filtering data records.

[Copilot]

Pull request overview

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

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The isEmptyRecord function has incorrect logic. It currently returns true only if all fields are empty strings (""), but it will return false for records that have null, undefined, or any other falsy values. This means:

1. A record with fields set to null (which can happen based on line 326 in fetchPage) will not be considered empty
2. A record with numeric 0, boolean false, or other non-string falsy values will not be considered empty

Consider using a more comprehensive check that handles all falsy values or empty values appropriately.

[Copilot]

Deleting all data records before the import begins at line 34 creates a data loss risk. If the import fails after this point (e.g., due to adaptor errors, geocoding failures, or network issues), all existing data will have been deleted without being replaced. Consider implementing one of these safer alternatives:

1. Use a transaction with rollback capability
2. Delete records only after the new import completes successfully
3. Mark old records as stale and delete them only after confirming the new import succeeded
4. Use a two-phase approach where new records are imported first, then old ones are removed

Suggested change

await deleteByDataSourceId(dataSource.id);

[Copilot]

The newly added deleteByDataSourceId function lacks test coverage. Since there is a comprehensive test file for DataRecord repository at tests/unit/server/repositories/DataRecord.test.ts, and this function performs a critical operation (deleting all records for a data source), it should have dedicated test coverage to ensure it works correctly and safely.

[Copilot]

The newly added isEmptyRecord helper function lacks test coverage. Since there is a comprehensive test file for the AirtableAdaptor at tests/unit/server/adaptors/airtable.test.ts, and this function implements critical filtering logic that determines which records are imported, it should have dedicated test coverage to verify it correctly identifies empty records in various scenarios (all empty strings, mix of empty and non-empty, null values, etc.).