From db2f5c1342e2f9c7e7e56707d04b37a69e592674 Mon Sep 17 00:00:00 2001 From: NaiJi Date: Fri, 7 Oct 2022 16:36:17 +0000 Subject: [PATCH 1/3] fix(job): Fix server settings toogles not creating upgrade server job --- .../cubit/client_jobs/client_jobs_cubit.dart | 41 +++++++++++-------- .../pages/server_details/server_settings.dart | 32 ++++----------- 2 files changed, 33 insertions(+), 40 deletions(-) diff --git a/lib/logic/cubit/client_jobs/client_jobs_cubit.dart b/lib/logic/cubit/client_jobs/client_jobs_cubit.dart index dacdcc29..d3330d7a 100644 --- a/lib/logic/cubit/client_jobs/client_jobs_cubit.dart +++ b/lib/logic/cubit/client_jobs/client_jobs_cubit.dart @@ -24,14 +24,10 @@ class JobsCubit extends Cubit { final ServicesCubit servicesCubit; void addJob(final ClientJob job) { - final List newJobsList = []; - if (state is JobsStateWithJobs) { - final JobsStateWithJobs jobsState = state as JobsStateWithJobs; - newJobsList.addAll(jobsState.clientJobList); - } - newJobsList.add(job); - getIt().showSnackBar('jobs.job_added'.tr()); - emit(JobsStateWithJobs(newJobsList)); + _updateJobsState([ + ...currentJobList, + ...[job] + ]); } void removeJob(final String id) { @@ -58,17 +54,28 @@ class JobsCubit extends Cubit { } } - void createShhJobIfNotExist(final CreateSSHKeyJob job) { - final List newJobsList = []; + List get currentJobList { + final List jobs = []; if (state is JobsStateWithJobs) { - newJobsList.addAll((state as JobsStateWithJobs).clientJobList); + jobs.addAll((state as JobsStateWithJobs).clientJobList); } - final bool isExistInJobList = - newJobsList.any((final el) => el is CreateSSHKeyJob); - if (!isExistInJobList) { - newJobsList.add(job); - getIt().showSnackBar('jobs.job_added'.tr()); - emit(JobsStateWithJobs(newJobsList)); + + return jobs; + } + + void _updateJobsState(final List newJobs) { + getIt().showSnackBar('jobs.job_added'.tr()); + emit(JobsStateWithJobs(newJobs)); + } + + void addUniqueJob(final J job) { + final List jobs = currentJobList; + final bool exists = jobs.any((final el) => el is J); + if (!exists) { + _updateJobsState([ + ...jobs, + ...[job] + ]); } } diff --git a/lib/ui/pages/server_details/server_settings.dart b/lib/ui/pages/server_details/server_settings.dart index f669dce0..ffeed77f 100644 --- a/lib/ui/pages/server_details/server_settings.dart +++ b/lib/ui/pages/server_details/server_settings.dart @@ -10,7 +10,6 @@ class _ServerSettings extends StatefulWidget { class _ServerSettingsState extends State<_ServerSettings> { bool? allowAutoUpgrade; bool? rebootAfterUpgrade; - bool? didSomethingChange; @override Widget build(final BuildContext context) { @@ -25,18 +24,14 @@ class _ServerSettingsState extends State<_ServerSettings> { rebootAfterUpgrade = serverDetailsState.autoUpgradeSettings.allowReboot; } - didSomethingChange ??= false; - return Column( children: [ SwitchListTile( value: allowAutoUpgrade ?? false, onChanged: (final switched) { - if (didSomethingChange == false) { - context.read().addJob( - RebuildServerJob(title: 'jobs.upgrade_server'.tr()), - ); - } + context.read().addUniqueJob( + RebuildServerJob(title: 'jobs.upgrade_server'.tr()), + ); context .read() .repository @@ -48,7 +43,6 @@ class _ServerSettingsState extends State<_ServerSettings> { ); setState(() { allowAutoUpgrade = switched; - didSomethingChange = true; }); }, title: Text('server.allow_autoupgrade'.tr()), @@ -60,11 +54,9 @@ class _ServerSettingsState extends State<_ServerSettings> { SwitchListTile( value: rebootAfterUpgrade ?? false, onChanged: (final switched) { - if (didSomethingChange == false) { - context.read().addJob( - RebuildServerJob(title: 'jobs.upgrade_server'.tr()), - ); - } + context.read().addUniqueJob( + RebuildServerJob(title: 'jobs.upgrade_server'.tr()), + ); context .read() .repository @@ -76,7 +68,6 @@ class _ServerSettingsState extends State<_ServerSettings> { ); setState(() { rebootAfterUpgrade = switched; - didSomethingChange = true; }); }, title: Text('server.reboot_after_upgrade'.tr()), @@ -91,14 +82,9 @@ class _ServerSettingsState extends State<_ServerSettings> { serverDetailsState.serverTimezone.toString(), ), onTap: () { - if (didSomethingChange == false) { - context.read().addJob( - RebuildServerJob(title: 'jobs.upgrade_server'.tr()), - ); - } - setState(() { - didSomethingChange = true; - }); + context.read().addUniqueJob( + RebuildServerJob(title: 'jobs.upgrade_server'.tr()), + ); Navigator.of(context).push( materialRoute( const SelectTimezone(), From 7bad11967a5c102fd7b558d598d9fa56cb9ad907 Mon Sep 17 00:00:00 2001 From: NaiJi Date: Fri, 7 Oct 2022 17:50:18 +0000 Subject: [PATCH 2/3] refactor(job): Implement polymorphic predicate for job accessibility Now every job type can impement canAddTo function to make JobsCubit know whether it can be applied or not --- .../cubit/client_jobs/client_jobs_cubit.dart | 41 +++------------ lib/logic/models/job.dart | 51 ++++++++++++------- .../pages/server_details/server_settings.dart | 6 +-- lib/ui/pages/services/service_page.dart | 2 +- lib/ui/pages/services/services.dart | 3 +- 5 files changed, 46 insertions(+), 57 deletions(-) diff --git a/lib/logic/cubit/client_jobs/client_jobs_cubit.dart b/lib/logic/cubit/client_jobs/client_jobs_cubit.dart index d3330d7a..e9378bed 100644 --- a/lib/logic/cubit/client_jobs/client_jobs_cubit.dart +++ b/lib/logic/cubit/client_jobs/client_jobs_cubit.dart @@ -24,10 +24,13 @@ class JobsCubit extends Cubit { final ServicesCubit servicesCubit; void addJob(final ClientJob job) { - _updateJobsState([ - ...currentJobList, - ...[job] - ]); + final jobs = currentJobList; + if (job.canAddTo(jobs)) { + _updateJobsState([ + ...jobs, + ...[job], + ]); + } } void removeJob(final String id) { @@ -35,25 +38,6 @@ class JobsCubit extends Cubit { emit(newState); } - void createOrRemoveServiceToggleJob(final ToggleJob job) { - final List newJobsList = []; - if (state is JobsStateWithJobs) { - newJobsList.addAll((state as JobsStateWithJobs).clientJobList); - } - final bool needToRemoveJob = newJobsList - .any((final el) => el is ServiceToggleJob && el.id == job.id); - if (needToRemoveJob) { - final ClientJob removingJob = newJobsList.firstWhere( - (final el) => el is ServiceToggleJob && el.id == job.id, - ); - removeJob(removingJob.id); - } else { - newJobsList.add(job); - getIt().showSnackBar('jobs.job_added'.tr()); - emit(JobsStateWithJobs(newJobsList)); - } - } - List get currentJobList { final List jobs = []; if (state is JobsStateWithJobs) { @@ -68,17 +52,6 @@ class JobsCubit extends Cubit { emit(JobsStateWithJobs(newJobs)); } - void addUniqueJob(final J job) { - final List jobs = currentJobList; - final bool exists = jobs.any((final el) => el is J); - if (!exists) { - _updateJobsState([ - ...jobs, - ...[job] - ]); - } - } - Future rebootServer() async { emit(JobsStateLoading()); final bool isSuccessful = await api.reboot(); diff --git a/lib/logic/models/job.dart b/lib/logic/models/job.dart index 13abdb72..cf55be9e 100644 --- a/lib/logic/models/job.dart +++ b/lib/logic/models/job.dart @@ -17,6 +17,7 @@ abstract class ClientJob extends Equatable { final String title; final String id; + bool canAddTo(final List jobs) => true; void execute(final JobsCubit cubit); @override @@ -25,10 +26,14 @@ abstract class ClientJob extends Equatable { class RebuildServerJob extends ClientJob { RebuildServerJob({ - required final super.title, - final super.id, + required super.title, + super.id, }); + @override + bool canAddTo(final List jobs) => + super.canAddTo(jobs) && !jobs.any((final job) => job is RebuildServerJob); + @override void execute(final JobsCubit cubit) async { await cubit.upgradeServer(); @@ -74,6 +79,12 @@ class DeleteUserJob extends ClientJob { final User user; + @override + bool canAddTo(final List jobs) => + super.canAddTo(jobs) && + !jobs.any( + (final job) => job is DeleteUserJob && job.user.login == user.login); + @override void execute(final JobsCubit cubit) async { await cubit.usersCubit.deleteUser(user); @@ -83,33 +94,31 @@ class DeleteUserJob extends ClientJob { List get props => [id, title, user]; } -abstract class ToggleJob extends ClientJob { - ToggleJob({ - required final this.service, - required final super.title, - }); - - final Service service; - - @override - List get props => [...super.props, service]; -} - -class ServiceToggleJob extends ToggleJob { +class ServiceToggleJob extends ClientJob { ServiceToggleJob({ - required super.service, + required this.service, required this.needToTurnOn, }) : super( title: '${needToTurnOn ? "jobs.service_turn_on".tr() : "jobs.service_turn_off".tr()} ${service.displayName}', ); + final bool needToTurnOn; + final Service service; + + @override + bool canAddTo(final List jobs) => + super.canAddTo(jobs) && + !jobs.any((final job) => + job is ServiceToggleJob && job.service.id == service.id); + @override void execute(final JobsCubit cubit) async { await cubit.api.switchService(service.id, needToTurnOn); } - final bool needToTurnOn; + @override + List get props => [...super.props, service]; } class CreateSSHKeyJob extends ClientJob { @@ -139,6 +148,14 @@ class DeleteSSHKeyJob extends ClientJob { final User user; final String publicKey; + @override + bool canAddTo(final List jobs) => + super.canAddTo(jobs) && + !jobs.any((final job) => + job is DeleteSSHKeyJob && + job.publicKey == publicKey && + job.user.login == user.login); + @override void execute(final JobsCubit cubit) async { await cubit.usersCubit.deleteSshKey(user, publicKey); diff --git a/lib/ui/pages/server_details/server_settings.dart b/lib/ui/pages/server_details/server_settings.dart index ffeed77f..a9facd5d 100644 --- a/lib/ui/pages/server_details/server_settings.dart +++ b/lib/ui/pages/server_details/server_settings.dart @@ -29,7 +29,7 @@ class _ServerSettingsState extends State<_ServerSettings> { SwitchListTile( value: allowAutoUpgrade ?? false, onChanged: (final switched) { - context.read().addUniqueJob( + context.read().addJob( RebuildServerJob(title: 'jobs.upgrade_server'.tr()), ); context @@ -54,7 +54,7 @@ class _ServerSettingsState extends State<_ServerSettings> { SwitchListTile( value: rebootAfterUpgrade ?? false, onChanged: (final switched) { - context.read().addUniqueJob( + context.read().addJob( RebuildServerJob(title: 'jobs.upgrade_server'.tr()), ); context @@ -82,7 +82,7 @@ class _ServerSettingsState extends State<_ServerSettings> { serverDetailsState.serverTimezone.toString(), ), onTap: () { - context.read().addUniqueJob( + context.read().addJob( RebuildServerJob(title: 'jobs.upgrade_server'.tr()), ); Navigator.of(context).push( diff --git a/lib/ui/pages/services/service_page.dart b/lib/ui/pages/services/service_page.dart index 6cd49f28..35741016 100644 --- a/lib/ui/pages/services/service_page.dart +++ b/lib/ui/pages/services/service_page.dart @@ -89,7 +89,7 @@ class _ServicePageState extends State { ListTile( iconColor: Theme.of(context).colorScheme.onBackground, onTap: () => { - context.read().createOrRemoveServiceToggleJob( + context.read().addJob( ServiceToggleJob( service: service, needToTurnOn: serviceDisabled, diff --git a/lib/ui/pages/services/services.dart b/lib/ui/pages/services/services.dart index 22fd3c91..89a7348a 100644 --- a/lib/ui/pages/services/services.dart +++ b/lib/ui/pages/services/services.dart @@ -157,8 +157,7 @@ class _Card extends StatelessWidget { return BrandSwitch( value: isActive, - onChanged: (final value) => - jobsCubit.createOrRemoveServiceToggleJob( + onChanged: (final value) => jobsCubit.addJob( ServiceToggleJob( service: service, needToTurnOn: value, From 0b5f8b6920532205ebf4d2a0361dd9d6b581e2e7 Mon Sep 17 00:00:00 2001 From: NaiJi Date: Sat, 8 Oct 2022 16:17:58 +0000 Subject: [PATCH 3/3] fix(job): Remove unnecessary cascade validation calls --- lib/logic/models/job.dart | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/logic/models/job.dart b/lib/logic/models/job.dart index cf55be9e..076e0d16 100644 --- a/lib/logic/models/job.dart +++ b/lib/logic/models/job.dart @@ -32,7 +32,7 @@ class RebuildServerJob extends ClientJob { @override bool canAddTo(final List jobs) => - super.canAddTo(jobs) && !jobs.any((final job) => job is RebuildServerJob); + !jobs.any((final job) => job is RebuildServerJob); @override void execute(final JobsCubit cubit) async { @@ -80,10 +80,9 @@ class DeleteUserJob extends ClientJob { final User user; @override - bool canAddTo(final List jobs) => - super.canAddTo(jobs) && - !jobs.any( - (final job) => job is DeleteUserJob && job.user.login == user.login); + bool canAddTo(final List jobs) => !jobs.any( + (final job) => job is DeleteUserJob && job.user.login == user.login, + ); @override void execute(final JobsCubit cubit) async { @@ -107,10 +106,9 @@ class ServiceToggleJob extends ClientJob { final Service service; @override - bool canAddTo(final List jobs) => - super.canAddTo(jobs) && - !jobs.any((final job) => - job is ServiceToggleJob && job.service.id == service.id); + bool canAddTo(final List jobs) => !jobs.any( + (final job) => job is ServiceToggleJob && job.service.id == service.id, + ); @override void execute(final JobsCubit cubit) async { @@ -149,12 +147,12 @@ class DeleteSSHKeyJob extends ClientJob { final String publicKey; @override - bool canAddTo(final List jobs) => - super.canAddTo(jobs) && - !jobs.any((final job) => - job is DeleteSSHKeyJob && - job.publicKey == publicKey && - job.user.login == user.login); + bool canAddTo(final List jobs) => !jobs.any( + (final job) => + job is DeleteSSHKeyJob && + job.publicKey == publicKey && + job.user.login == user.login, + ); @override void execute(final JobsCubit cubit) async {