Skip to content

Commit d8ef004

Browse files
cfsmp3claude
andcommitted
refactor: Code quality improvements and testing
Quick Wins: - Fix typo: orignalList -> originalList - Add logger package for proper logging - Replace all print statements with logger calls - Fix silent catch blocks with proper error logging Error Handling: - Add comprehensive HTTP error handling in updater_bloc - Add response status validation and null checks - Add proper try-catch with logging throughout Refactoring: - Refactor triple-nested BlocBuilders in start_stop_button using context.select - Extract button logic into separate _handlePress method - Clean up process_tile.dart catch blocks Accessibility: - Add Tooltip to start/stop button - Add semantic labels to icons - Add Tooltip and Semantics to add files button - Add const constructors where applicable Testing: - Add comprehensive tests for DashboardBloc (5 tests) - Add comprehensive tests for ProcessBloc (4 tests) - Total: 14 passing tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 50e5f3a commit d8ef004

14 files changed

Lines changed: 447 additions & 235 deletions

File tree

lib/bloc/process_bloc/process_bloc.dart

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'package:file_selector/file_selector.dart';
33
import 'package:flutter_bloc/flutter_bloc.dart';
44

55
import 'package:ccxgui/repositories/ccextractor.dart';
6+
import 'package:ccxgui/utils/logger.dart';
67

78
part 'process_event.dart';
89
part 'process_state.dart';
@@ -12,7 +13,7 @@ class ProcessBloc extends Bloc<ProcessEvent, ProcessState> {
1213

1314
ProcessBloc()
1415
: super(ProcessState(
15-
orignalList: [],
16+
originalList: [],
1617
queue: [],
1718
processed: [],
1819
log: [],
@@ -44,7 +45,7 @@ class ProcessBloc extends Bloc<ProcessEvent, ProcessState> {
4445
if (!state.started || state.current != null || state.queue.isEmpty) {
4546
if (state.queue.isEmpty) {
4647
emit(state.copyWith(
47-
queue: state.orignalList,
48+
queue: state.originalList,
4849
processed: state.processed,
4950
started: false,
5051
progress: '0',
@@ -102,7 +103,7 @@ class ProcessBloc extends Bloc<ProcessEvent, ProcessState> {
102103
void _extractFilesInSplitMode(Emitter<ProcessState> emit) {
103104
_extractor
104105
.extractFilesInSplitMode(
105-
state.orignalList,
106+
state.originalList,
106107
listenProgress: (progress) => add(ProcessFileExtractorProgress(progress)),
107108
listenOutput: (line) => add(ProcessFileExtractorOutput(line)),
108109
listenVideoDetails: (videoDetails) =>
@@ -121,7 +122,7 @@ class ProcessBloc extends Bloc<ProcessEvent, ProcessState> {
121122
void _onStartAllProcess(StartAllProcess event, Emitter<ProcessState> emit) {
122123
emit(state.copyWith(
123124
current: state.current,
124-
processed: state.queue == state.orignalList ? [] : state.processed,
125+
processed: state.queue == state.originalList ? [] : state.processed,
125126
started: true,
126127
));
127128
_extractNext(emit);
@@ -136,10 +137,12 @@ class ProcessBloc extends Bloc<ProcessEvent, ProcessState> {
136137
void _onStopAllProcess(StopAllProcess event, Emitter<ProcessState> emit) {
137138
try {
138139
_extractor.cancelRun();
139-
} catch (_) {}
140+
} catch (e) {
141+
logger.w('Failed to cancel extractor run on stop all', error: e);
142+
}
140143
emit(state.copyWith(
141144
current: null,
142-
queue: state.orignalList,
145+
queue: state.originalList,
143146
processed: [],
144147
progress: '0',
145148
started: false,
@@ -149,25 +152,29 @@ class ProcessBloc extends Bloc<ProcessEvent, ProcessState> {
149152
void _onProcessKill(ProcessKill event, Emitter<ProcessState> emit) {
150153
try {
151154
_extractor.cancelRun();
152-
} catch (_) {}
155+
} catch (e) {
156+
logger.w('Failed to cancel extractor run on process kill', error: e);
157+
}
153158
emit(state.copyWith(
154159
current: state.current,
155-
orignalList:
156-
state.orignalList.where((element) => element != event.file).toList(),
160+
originalList:
161+
state.originalList.where((element) => element != event.file).toList(),
157162
queue: state.queue.where((element) => element != event.file).toList(),
158163
));
159164
}
160165

161166
void _onProcessRemoveAll(ProcessRemoveAll event, Emitter<ProcessState> emit) {
162167
try {
163168
_extractor.cancelRun();
164-
} catch (_) {}
169+
} catch (e) {
170+
logger.w('Failed to cancel extractor run on remove all', error: e);
171+
}
165172
emit(state.copyWith(
166173
current: null,
167174
progress: '0',
168175
processed: [],
169176
queue: [],
170-
orignalList: [],
177+
originalList: [],
171178
started: false,
172179
exitCode: null,
173180
log: [],
@@ -221,7 +228,7 @@ class ProcessBloc extends Bloc<ProcessEvent, ProcessState> {
221228
ProcessFilesSubmitted event, Emitter<ProcessState> emit) {
222229
emit(state.copyWith(
223230
current: state.current,
224-
orignalList: List.from(state.orignalList)..addAll(event.files),
231+
originalList: List.from(state.originalList)..addAll(event.files),
225232
processed: state.processed,
226233
queue: state.started || state.processed.isEmpty
227234
? state.queue.followedBy(event.files).toList()
@@ -233,8 +240,8 @@ class ProcessBloc extends Bloc<ProcessEvent, ProcessState> {
233240
ProcessFileRemoved event, Emitter<ProcessState> emit) {
234241
emit(state.copyWith(
235242
current: state.current,
236-
orignalList:
237-
state.orignalList.where((element) => element != event.file).toList(),
243+
originalList:
244+
state.originalList.where((element) => element != event.file).toList(),
238245
queue: state.queue.where((element) => element != event.file).toList(),
239246
));
240247
}

lib/bloc/process_bloc/process_state.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
part of 'process_bloc.dart';
22

33
class ProcessState extends Equatable {
4-
final List<XFile> orignalList;
4+
final List<XFile> originalList;
55
final List<XFile> processed;
66
final List<XFile> queue;
77
final List<String> log;
@@ -12,7 +12,7 @@ class ProcessState extends Equatable {
1212
final String? version;
1313
final int? exitCode;
1414
const ProcessState({
15-
required this.orignalList,
15+
required this.originalList,
1616
required this.queue,
1717
required this.processed,
1818
required this.videoDetails,
@@ -25,7 +25,7 @@ class ProcessState extends Equatable {
2525
});
2626

2727
ProcessState copyWith({
28-
List<XFile>? orignalList,
28+
List<XFile>? originalList,
2929
List<XFile>? processed,
3030
List<XFile>? queue,
3131
List<String>? log,
@@ -37,7 +37,7 @@ class ProcessState extends Equatable {
3737
int? exitCode,
3838
}) =>
3939
ProcessState(
40-
orignalList: orignalList ?? this.orignalList,
40+
originalList: originalList ?? this.originalList,
4141
queue: queue ?? this.queue,
4242
processed: processed ?? this.processed,
4343
log: log ?? this.log,
@@ -57,7 +57,7 @@ class ProcessState extends Equatable {
5757
started,
5858
progress,
5959
videoDetails,
60-
orignalList,
60+
originalList,
6161
version,
6262
exitCode,
6363
];

lib/bloc/updater_bloc/updater_bloc.dart

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import 'package:equatable/equatable.dart';
55
import 'package:http/http.dart' as http;
66
import 'package:url_launcher/url_launcher.dart';
77

8+
import 'package:ccxgui/utils/logger.dart';
9+
810
part 'updater_event.dart';
911
part 'updater_state.dart';
1012

@@ -19,32 +21,75 @@ class UpdaterBloc extends Bloc<UpdaterEvent, UpdaterState> {
1921

2022
Future<void> _onCheckForUpdates(
2123
CheckForUpdates event, Emitter<UpdaterState> emit) async {
22-
var url = Uri.parse(apiUrl);
23-
String changelog = '';
24-
int currentVersionIndex = 0;
25-
http.Response response = await http.get(url);
26-
var data = jsonDecode(response.body);
27-
for (var i = 0; i < data.length; i++) {
28-
if (event.currentVersion == data[i]['tag_name'].toString().substring(1)) {
29-
currentVersionIndex = i;
24+
try {
25+
var url = Uri.parse(apiUrl);
26+
String changelog = '';
27+
int currentVersionIndex = 0;
28+
http.Response response = await http.get(url);
29+
30+
if (response.statusCode != 200) {
31+
logger.e('Failed to check for updates: HTTP ${response.statusCode}');
32+
return;
33+
}
34+
35+
var data = jsonDecode(response.body);
36+
37+
if (data is! List || data.isEmpty) {
38+
logger.e('Invalid response format from GitHub API');
39+
return;
3040
}
41+
42+
for (var i = 0; i < data.length; i++) {
43+
final tagName = data[i]['tag_name']?.toString();
44+
if (tagName != null &&
45+
tagName.length > 1 &&
46+
event.currentVersion == tagName.substring(1)) {
47+
currentVersionIndex = i;
48+
}
49+
}
50+
for (var i = 0; i < currentVersionIndex; i++) {
51+
changelog += '\n${data[i]['body'] ?? ''}';
52+
}
53+
54+
final firstRelease = data[0];
55+
final tagName = firstRelease['tag_name']?.toString();
56+
if (tagName == null || tagName.length < 2) {
57+
logger.e('Invalid tag name in release data');
58+
return;
59+
}
60+
61+
String latestVersion = tagName.substring(1);
62+
63+
final assets = firstRelease['assets'];
64+
if (assets is! List || assets.isEmpty) {
65+
logger.w('No download assets found in latest release');
66+
return;
67+
}
68+
69+
String downloadURL = assets[0]['browser_download_url']?.toString() ?? '';
70+
71+
bool updateAvailable =
72+
_compareVersions(latestVersion, event.currentVersion);
73+
emit(state.copyWith(
74+
currentVersion: event.currentVersion,
75+
latestVersion: latestVersion,
76+
updateAvailable: updateAvailable,
77+
downloadURL: downloadURL,
78+
changelog: changelog,
79+
));
80+
} catch (e, stackTrace) {
81+
logger.e('Error checking for updates', error: e, stackTrace: stackTrace);
3182
}
32-
for (var i = 0; i < currentVersionIndex; i++) {
33-
changelog += '\n${data[i]['body']}';
83+
}
84+
85+
/// Compare version strings. Returns true if latest > current.
86+
bool _compareVersions(String latest, String current) {
87+
try {
88+
return double.parse(latest) > double.parse(current);
89+
} catch (_) {
90+
// If parsing fails, do string comparison
91+
return latest.compareTo(current) > 0;
3492
}
35-
String latestVersion = data[0]['tag_name'].toString().substring(1);
36-
String downloadURL =
37-
data[0]['assets'][0]['browser_download_url'].toString();
38-
39-
bool updateAvailable =
40-
double.parse(latestVersion) > double.parse(event.currentVersion);
41-
emit(state.copyWith(
42-
currentVersion: event.currentVersion,
43-
latestVersion: latestVersion,
44-
updateAvailable: updateAvailable,
45-
downloadURL: downloadURL,
46-
changelog: changelog,
47-
));
4893
}
4994

5095
Future<void> _onDownloadUpdate(

lib/bloc_observer.dart

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,23 @@
1-
// Logs events and states during transition, needs some improvement to reduce spam
2-
// @override
3-
// String toString() => 'Event { prop: prop }';
4-
51
import 'package:bloc/bloc.dart';
62

3+
import 'package:ccxgui/utils/logger.dart';
4+
75
class SimpleBlocObserver extends BlocObserver {
86
@override
97
void onCreate(BlocBase bloc) {
108
super.onCreate(bloc);
11-
print('onCreate -- ${bloc.runtimeType}');
9+
logger.d('onCreate -- ${bloc.runtimeType}');
1210
}
1311

14-
// @override
15-
// void onEvent(Bloc bloc, Object? event) {
16-
// super.onEvent(bloc, event);
17-
18-
// if ({ProcessFileExtractorOutput, ProcessFileExtractorProgress}
19-
// .contains(event)) print('onEvent -- ${bloc.runtimeType}, $event');
20-
// }
21-
22-
// @override
23-
// void onTransition(Bloc bloc, Transition transition) {
24-
// // CurrentSettingsState and ProcessState have a lot of spam
25-
// if ({transition.nextState, transition.nextState}
26-
// .contains(CurrentSettingsState) ||
27-
// {transition.nextState, transition.nextState}.contains(ProcessState)) {
28-
// print(transition);
29-
// }
30-
// super.onTransition(bloc, transition);
31-
// }
32-
3312
@override
3413
void onError(BlocBase bloc, Object error, StackTrace stackTrace) {
35-
print('${bloc.runtimeType} $error $stackTrace');
14+
logger.e('${bloc.runtimeType}', error: error, stackTrace: stackTrace);
3615
super.onError(bloc, error, stackTrace);
3716
}
3817

3918
@override
4019
void onClose(BlocBase bloc) {
4120
super.onClose(bloc);
42-
print('onClose -- ${bloc.runtimeType}');
21+
logger.d('onClose -- ${bloc.runtimeType}');
4322
}
4423
}

lib/repositories/ccextractor.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import 'package:file_selector/file_selector.dart';
55

66
import 'package:ccxgui/models/settings_model.dart';
77
import 'package:ccxgui/repositories/settings_repository.dart';
8+
import 'package:ccxgui/utils/logger.dart';
89

910
class CCExtractor {
1011
late Process process;
@@ -194,7 +195,7 @@ class CCExtractor {
194195
ccxStdOut = versionMatch.group(1) ?? '0';
195196
}
196197
} catch (e) {
197-
print('Error getting CCExtractor version: $e');
198+
logger.e('Failed to get CCExtractor version', error: e);
198199
}
199200
return ccxStdOut;
200201
}

lib/repositories/settings_repository.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import 'package:path_provider/path_provider.dart';
55

66
import 'package:ccxgui/models/settings_model.dart';
77
import 'package:ccxgui/utils/constants.dart';
8+
import 'package:ccxgui/utils/logger.dart';
89

910
class SettingsRepository {
1011
Future<String> get _localPath async {
@@ -40,7 +41,7 @@ class SettingsRepository {
4041
}
4142
return true;
4243
} catch (e) {
43-
print('Rewriting config.json file. Error $e');
44+
logger.w('Rewriting config.json file', error: e);
4445
await file.writeAsString(jsonEncode(SettingsModel().toJson()));
4546
return false;
4647
}
@@ -94,7 +95,7 @@ class SettingsRepository {
9495
settings = SettingsModel.fromJson(jsonDecode(contents));
9596
}
9697
} catch (e) {
97-
print('GetSettings Error $e');
98+
logger.e('Failed to get settings', error: e);
9899
}
99100
return settings;
100101
}
@@ -109,7 +110,7 @@ class SettingsRepository {
109110
final file = await _localFile;
110111
await file.writeAsString(jsonEncode(settingsModel.toJson()));
111112
} catch (e) {
112-
print('Error saving settings $e');
113+
logger.e('Failed to save settings', error: e);
113114
}
114115
}
115116
}

0 commit comments

Comments
 (0)