Skip to content

Refactor sync the auth feature with the backend and client update #23

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
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
29 changes: 13 additions & 16 deletions lib/app/bloc/app_bloc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,43 +45,40 @@ class AppBloc extends Bloc<AppEvent, AppState> {
AppUserChanged event,
Emitter<AppState> emit,
) async {
// Determine the AppStatus based on the user object and its role
final user = event.user;
final AppStatus status;

switch (event.user?.role) {
case null:
status = AppStatus.unauthenticated;
case UserRole.standardUser:
status = AppStatus.authenticated;
// ignore: no_default_cases
default: // Fallback for any other roles not explicitly handled
status = AppStatus
.unauthenticated; // Treat other roles as unauthenticated for dashboard
if (user != null &&
(user.roles.contains(UserRoles.admin) ||
user.roles.contains(UserRoles.publisher))) {
status = AppStatus.authenticated;
} else {
status = AppStatus.unauthenticated;
}

// Emit user and status update
emit(state.copyWith(status: status, user: event.user));
emit(state.copyWith(status: status, user: user));

// If user is authenticated, load their app settings
if (event.user != null) {
if (status == AppStatus.authenticated && user != null) {
try {
final userAppSettings = await _userAppSettingsRepository.read(
id: event.user!.id,
id: user.id,
);
emit(state.copyWith(userAppSettings: userAppSettings));
} on NotFoundException {
// If settings not found, create default ones
final defaultSettings = UserAppSettings(id: event.user!.id);
final defaultSettings = UserAppSettings(id: user.id);
await _userAppSettingsRepository.create(item: defaultSettings);
emit(state.copyWith(userAppSettings: defaultSettings));
} on HtHttpException catch (e) {
// Handle HTTP exceptions during settings load
print('Error loading user app settings: ${e.message}');
emit(state.copyWith()); // Clear settings on error
emit(state.copyWith(clearUserAppSettings: true));
} catch (e) {
// Handle any other unexpected errors
print('Unexpected error loading user app settings: $e');
emit(state.copyWith()); // Clear settings on error
emit(state.copyWith(clearUserAppSettings: true));
Comment on lines 76 to +81

Choose a reason for hiding this comment

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

medium

Consider using a dedicated logging package instead of print() for more robust error handling. A logging package allows for log levels, structured formatting, and routing logs to different destinations, which is beneficial for debugging and monitoring in production environments.

}
} else {
// If user is unauthenticated, clear app settings
Expand Down
52 changes: 29 additions & 23 deletions lib/app_configuration/view/app_configuration_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ class _AppConfigurationPageState extends State<AppConfigurationPage> {
),
children: [
_UserPreferenceLimitsForm(
userRole: UserRole.guestUser,
userRole: UserRoles.guestUser,
appConfig: appConfig,
onConfigChanged: (newConfig) {
context.read<AppConfigurationBloc>().add(
Expand All @@ -303,7 +303,7 @@ class _AppConfigurationPageState extends State<AppConfigurationPage> {
),
children: [
_UserPreferenceLimitsForm(
userRole: UserRole.standardUser,
userRole: UserRoles.standardUser,
appConfig: appConfig,
onConfigChanged: (newConfig) {
context.read<AppConfigurationBloc>().add(
Expand All @@ -323,7 +323,7 @@ class _AppConfigurationPageState extends State<AppConfigurationPage> {
),
children: [
_UserPreferenceLimitsForm(
userRole: UserRole.premiumUser,
userRole: UserRoles.premiumUser,
appConfig: appConfig,
onConfigChanged: (newConfig) {
context.read<AppConfigurationBloc>().add(
Expand Down Expand Up @@ -359,7 +359,7 @@ class _AppConfigurationPageState extends State<AppConfigurationPage> {
),
children: [
_AdConfigForm(
userRole: UserRole.guestUser,
userRole: UserRoles.guestUser,
appConfig: appConfig,
onConfigChanged: (newConfig) {
context.read<AppConfigurationBloc>().add(
Expand All @@ -379,7 +379,7 @@ class _AppConfigurationPageState extends State<AppConfigurationPage> {
),
children: [
_AdConfigForm(
userRole: UserRole.standardUser,
userRole: UserRoles.standardUser,
appConfig: appConfig,
onConfigChanged: (newConfig) {
context.read<AppConfigurationBloc>().add(
Expand All @@ -399,7 +399,7 @@ class _AppConfigurationPageState extends State<AppConfigurationPage> {
),
children: [
_AdConfigForm(
userRole: UserRole.premiumUser,
userRole: UserRoles.premiumUser,
appConfig: appConfig,
onConfigChanged: (newConfig) {
context.read<AppConfigurationBloc>().add(
Expand Down Expand Up @@ -438,7 +438,7 @@ class _AppConfigurationPageState extends State<AppConfigurationPage> {
),
children: [
_AccountActionConfigForm(
userRole: UserRole.guestUser,
userRole: UserRoles.guestUser,
appConfig: appConfig,
onConfigChanged: (newConfig) {
context.read<AppConfigurationBloc>().add(
Expand All @@ -458,7 +458,7 @@ class _AppConfigurationPageState extends State<AppConfigurationPage> {
),
children: [
_AccountActionConfigForm(
userRole: UserRole.standardUser,
userRole: UserRoles.standardUser,
appConfig: appConfig,
onConfigChanged: (newConfig) {
context.read<AppConfigurationBloc>().add(
Expand Down Expand Up @@ -779,7 +779,7 @@ class _UserPreferenceLimitsForm extends StatefulWidget {
required this.buildIntField,
});

final UserRole userRole;
final String userRole;
final AppConfig appConfig;
final ValueChanged<AppConfig> onConfigChanged;
final Widget Function(
Expand Down Expand Up @@ -936,7 +936,7 @@ class _UserPreferenceLimitsFormState extends State<_UserPreferenceLimitsForm> {
final userPreferenceLimits = widget.appConfig.userPreferenceLimits;

switch (widget.userRole) {
case UserRole.guestUser:
case UserRoles.guestUser:
return Column(
children: [
widget.buildIntField(
Expand Down Expand Up @@ -975,7 +975,7 @@ class _UserPreferenceLimitsFormState extends State<_UserPreferenceLimitsForm> {
),
],
);
case UserRole.standardUser:
case UserRoles.standardUser:
return Column(
children: [
widget.buildIntField(
Expand Down Expand Up @@ -1015,7 +1015,7 @@ class _UserPreferenceLimitsFormState extends State<_UserPreferenceLimitsForm> {
),
],
);
case UserRole.premiumUser:
case UserRoles.premiumUser:
return Column(
children: [
widget.buildIntField(
Expand Down Expand Up @@ -1055,10 +1055,12 @@ class _UserPreferenceLimitsFormState extends State<_UserPreferenceLimitsForm> {
),
],
);
case UserRole.admin:
case UserRoles.admin:
// Admin role might not have specific limits here, or could be
// a separate configuration. For now, return empty.
return const SizedBox.shrink();
default:
return const SizedBox.shrink();
Comment on lines +1062 to +1063

Choose a reason for hiding this comment

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

medium

To prevent silent failures with unhandled user roles, add an assert to the default case. This will notify developers of unhandled roles during development.

      default:
        assert(false, 'Unhandled user role in _UserPreferenceLimitsForm: ${widget.userRole}');
        return const SizedBox.shrink();

}
}
}
Expand All @@ -1071,7 +1073,7 @@ class _AdConfigForm extends StatefulWidget {
required this.buildIntField,
});

final UserRole userRole;
final String userRole;
final AppConfig appConfig;
final ValueChanged<AppConfig> onConfigChanged;
final Widget Function(
Expand Down Expand Up @@ -1271,7 +1273,7 @@ class _AdConfigFormState extends State<_AdConfigForm> {
final adConfig = widget.appConfig.adConfig;

switch (widget.userRole) {
case UserRole.guestUser:
case UserRoles.guestUser:
return Column(
children: [
widget.buildIntField(
Expand Down Expand Up @@ -1329,7 +1331,7 @@ class _AdConfigFormState extends State<_AdConfigForm> {
),
],
);
case UserRole.standardUser:
case UserRoles.standardUser:
return Column(
children: [
widget.buildIntField(
Expand Down Expand Up @@ -1391,7 +1393,7 @@ class _AdConfigFormState extends State<_AdConfigForm> {
),
],
);
case UserRole.premiumUser:
case UserRoles.premiumUser:
return Column(
children: [
widget.buildIntField(
Expand Down Expand Up @@ -1450,7 +1452,9 @@ class _AdConfigFormState extends State<_AdConfigForm> {
),
],
);
case UserRole.admin:
case UserRoles.admin:
return const SizedBox.shrink();
default:
return const SizedBox.shrink();
Comment on lines +1457 to 1458

Choose a reason for hiding this comment

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

medium

Add an assert to the default case to flag unhandled roles during development. This will improve code robustness and developer experience.

      default:
        assert(false, 'Unhandled user role in _AdConfigForm: ${widget.userRole}');
        return const SizedBox.shrink();

}
}
Expand All @@ -1464,7 +1468,7 @@ class _AccountActionConfigForm extends StatefulWidget {
required this.buildIntField,
});

final UserRole userRole;
final String userRole;
final AppConfig appConfig;
final ValueChanged<AppConfig> onConfigChanged;
final Widget Function(
Expand Down Expand Up @@ -1553,7 +1557,7 @@ class _AccountActionConfigFormState extends State<_AccountActionConfigForm> {
final accountActionConfig = widget.appConfig.accountActionConfig;

switch (widget.userRole) {
case UserRole.guestUser:
case UserRoles.guestUser:
return Column(
children: [
widget.buildIntField(
Expand All @@ -1576,7 +1580,7 @@ class _AccountActionConfigFormState extends State<_AccountActionConfigForm> {
),
],
);
case UserRole.standardUser:
case UserRoles.standardUser:
return Column(
children: [
widget.buildIntField(
Expand All @@ -1599,8 +1603,10 @@ class _AccountActionConfigFormState extends State<_AccountActionConfigForm> {
),
],
);
case UserRole.premiumUser:
case UserRole.admin:
case UserRoles.premiumUser:
case UserRoles.admin:
return const SizedBox.shrink();
default:
return const SizedBox.shrink();
Comment on lines +1609 to 1610

Choose a reason for hiding this comment

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

medium

Add an assert to the default case to help developers catch configuration omissions for new user roles early during development.

      default:
        assert(false, 'Unhandled user role in _AccountActionConfigForm: ${widget.userRole}');
        return const SizedBox.shrink();

}
}
Expand Down
20 changes: 18 additions & 2 deletions lib/authentication/bloc/authentication_bloc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ import 'package:ht_auth_repository/ht_auth_repository.dart';
import 'package:ht_shared/ht_shared.dart'
show
AuthenticationException,
ForbiddenException,
HtHttpException,
InvalidInputException,
NetworkException,
NotFoundException,
OperationFailedException,
ServerException,
UnauthorizedException,
User;

part 'authentication_event.dart';
Expand Down Expand Up @@ -65,10 +68,17 @@ class AuthenticationBloc
}
emit(AuthenticationRequestCodeLoading());
try {
await _authenticationRepository.requestSignInCode(event.email);
await _authenticationRepository.requestSignInCode(
event.email,
isDashboardLogin: true,
);
emit(AuthenticationCodeSentSuccess(email: event.email));
} on InvalidInputException catch (e) {
emit(AuthenticationFailure('Invalid input: ${e.message}'));
} on UnauthorizedException catch (e) {
emit(AuthenticationFailure(e.message));
} on ForbiddenException catch (e) {
emit(AuthenticationFailure(e.message));
} on NetworkException catch (_) {
emit(const AuthenticationFailure('Network error occurred.'));
} on ServerException catch (e) {
Expand All @@ -95,13 +105,19 @@ class AuthenticationBloc
) async {
emit(AuthenticationLoading());
try {
await _authenticationRepository.verifySignInCode(event.email, event.code);
await _authenticationRepository.verifySignInCode(
event.email,
event.code,
isDashboardLogin: true,
);
// On success, the _AuthenticationUserChanged listener will handle
// emitting AuthenticationAuthenticated.
} on InvalidInputException catch (e) {
emit(AuthenticationFailure(e.message));
} on AuthenticationException catch (e) {
emit(AuthenticationFailure(e.message));
} on NotFoundException catch (e) {
emit(AuthenticationFailure(e.message));
} on NetworkException catch (_) {
emit(const AuthenticationFailure('Network error occurred.'));
} on ServerException catch (e) {
Expand Down
12 changes: 6 additions & 6 deletions pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ packages:
description:
path: "."
ref: HEAD
resolved-ref: fd31ce8e255c27e1fcc17e849c41f9c8511a6d87
resolved-ref: f89241dfd482d2a72b1168f979597a34b1004df5
url: "https://github.com/headlines-toolkit/ht-auth-api.git"
source: git
version: "0.0.0"
Expand All @@ -214,7 +214,7 @@ packages:
description:
path: "."
ref: HEAD
resolved-ref: f9c3b44b79fc19dfd9b9a7e0d1e21e60f4885617
resolved-ref: a003eb493db4fc134db419a721ee2fda0b598032
url: "https://github.com/headlines-toolkit/ht-auth-client.git"
source: git
version: "0.0.0"
Expand All @@ -223,7 +223,7 @@ packages:
description:
path: "."
ref: HEAD
resolved-ref: "3a8dc5ff81c59805fa59996517eb0fdf136a0b67"
resolved-ref: "721a028b926a5a8af2b5176de039cd6394a21724"
url: "https://github.com/headlines-toolkit/ht-auth-inmemory"
source: git
version: "0.0.0"
Expand All @@ -232,7 +232,7 @@ packages:
description:
path: "."
ref: HEAD
resolved-ref: "596ba311cdbbdf61a216f60dac0218fab9e234d9"
resolved-ref: b7de5cc86d432b17710c83a1bf8de105bb4fa00d
url: "https://github.com/headlines-toolkit/ht-auth-repository.git"
source: git
version: "0.0.0"
Expand Down Expand Up @@ -286,7 +286,7 @@ packages:
description:
path: "."
ref: HEAD
resolved-ref: e2860560d21c1cf43f0e65f28c9ba722823254f2
resolved-ref: "2378d6698df1cdeb7c5a17470f94fb8a5a99ca01"
url: "https://github.com/headlines-toolkit/ht-kv-storage-service.git"
source: git
version: "0.0.0"
Expand All @@ -304,7 +304,7 @@ packages:
description:
path: "."
ref: HEAD
resolved-ref: b3a339a2957b35a2bb7baf249e89ef9ca296eb3e
resolved-ref: "30aff4d0e2661ff79f2b84070af5f7982d88ba66"
url: "https://github.com/headlines-toolkit/ht-shared.git"
source: git
version: "0.0.0"
Expand Down
Loading