Skip to content

Fix database bugs #13

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.

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
merged 3 commits into from
Jul 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 24 additions & 2 deletions lib/src/config/app_dependencies.dart
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,21 @@ class AppDependencies {
}
return Headline.fromJson(json);
},
(h) => h.toJson(), // toJson already handles DateTime correctly
(headline) {
final json = headline.toJson();
// The database expects source_id and category_id, not nested objects.
// We extract the IDs and remove the original objects to match the
// schema.
if (headline.source != null) {
json['source_id'] = headline.source!.id;
}
if (headline.category != null) {
json['category_id'] = headline.category!.id;
}
json.remove('source');
json.remove('category');
return json;
},
Comment on lines +118 to +132

Choose a reason for hiding this comment

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

medium

This change introduces a duplication of logic. The logic to prepare Headline objects for the database now exists here in the repository configuration and also in lib/src/services/database_seeding_service.dart (lines 265-266).

This duplication can lead to maintenance issues, where a change in the database schema would require updates in multiple places. To improve maintainability, consider centralizing this database-specific serialization logic, such as by adding a toDbJson() method to the Headline data model.

);
categoryRepository = _createRepository(
connection,
Expand Down Expand Up @@ -147,7 +161,15 @@ class AppDependencies {
}
return Source.fromJson(json);
},
(s) => s.toJson(),
(source) {
final json = source.toJson();
// The database expects headquarters_country_id, not a nested object.
// We extract the ID and remove the original object to match the
// schema.
json['headquarters_country_id'] = source.headquarters?.id;
json.remove('headquarters');
return json;
},
Comment on lines +164 to +172

Choose a reason for hiding this comment

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

medium

This change introduces a duplication of logic. The logic to prepare Source objects for the database now exists here in the repository configuration and also in lib/src/services/database_seeding_service.dart (lines 232-233).

To improve maintainability, consider centralizing this database-specific serialization logic, such as by adding a toDbJson() method to the Source data model.

);
countryRepository = _createRepository(
connection,
Expand Down
19 changes: 5 additions & 14 deletions lib/src/services/database_seeding_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ class DatabaseSeedingService {
CREATE TABLE IF NOT EXISTS headlines (
id TEXT PRIMARY KEY,
title TEXT NOT NULL,
source_id TEXT NOT NULL,
category_id TEXT NOT NULL,
source_id TEXT REFERENCES sources(id),
category_id TEXT REFERENCES categories(id),
Comment on lines +109 to +110

Choose a reason for hiding this comment

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

high

Consider adding NOT NULL constraints to the source_id and category_id columns to enforce data integrity, since these columns are references to other tables.

            source_id TEXT REFERENCES sources(id) NOT NULL,
            category_id TEXT REFERENCES categories(id) NOT NULL,

image_url TEXT,
url TEXT,
published_at TIMESTAMPTZ,
Expand Down Expand Up @@ -260,19 +260,10 @@ class DatabaseSeedingService {
final headline = Headline.fromJson(data);
final params = headline.toJson();

// The `source_id` and `category_id` columns are NOT NULL. If a fixture
// is missing the nested source or category object, we cannot proceed.
if (headline.source == null || headline.category == null) {
_log.warning(
'Skipping headline fixture with missing source or category ID: '
'${headline.title}',
);
continue;
}

// Extract IDs from nested objects and remove the objects to match schema.
params['source_id'] = headline.source!.id;
params['category_id'] = headline.category!.id;
// These are now nullable to match the schema.
params['source_id'] = headline.source?.id;
params['category_id'] = headline.category?.id;
params.remove('source');
params.remove('category');

Expand Down
17 changes: 16 additions & 1 deletion routes/api/v1/data/index.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ import 'package:ht_shared/ht_shared.dart';

import '../../../_middleware.dart'; // Assuming RequestId is here

/// Converts a camelCase string to snake_case.
String _camelToSnake(String input) {
return input
.replaceAllMapped(
RegExp(r'(?<!^)(?=[A-Z])'),
(match) => '_${match.group(0)}',
)
.toLowerCase();
}

/// Handles requests for the /api/v1/data collection endpoint.
/// Dispatches requests to specific handlers based on the HTTP method.
Future<Response> onRequest(RequestContext context) async {
Expand Down Expand Up @@ -109,10 +119,15 @@ Future<Response> _handleGet(
final queryParams = context.request.uri.queryParameters;
final startAfterId = queryParams['startAfterId'];
final limitParam = queryParams['limit'];
final sortBy = queryParams['sortBy'];
final sortByParam = queryParams['sortBy'];

Choose a reason for hiding this comment

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

medium

Consider renaming sortByParam to sortBy to maintain consistency and avoid confusion, since it is immediately assigned to sortBy on line 129.

  final sortBy = queryParams['sortBy'];

final sortOrderRaw = queryParams['sortOrder']?.toLowerCase();
final limit = limitParam != null ? int.tryParse(limitParam) : null;

// Convert sortBy from camelCase to snake_case for the database query.
// This prevents errors where the client sends 'createdAt' and the database
// expects 'created_at'.
final sortBy = sortByParam != null ? _camelToSnake(sortByParam) : null;

SortOrder? sortOrder;
if (sortOrderRaw != null) {
if (sortOrderRaw == 'asc') {
Expand Down
Loading