refactor: Introduce the API connection repository #440

Merged
inex merged 36 commits from api-connection-refactor into master 2024-02-23 16:49:39 +02:00

Major refactor to remove connections between cubits.

Also:
Closes #277
Closes #166
Closes #254

DO NOT MERGE until API supports system rebuild tracking (SelfPrivacy/selfprivacy-rest-api#91) There is a fallback available now. PR is too big now, and there seems to be no regressions.

Major refactor to remove connections between cubits. Also: Closes #277 Closes #166 Closes #254 ~~**DO NOT MERGE until API supports system rebuild tracking** (https://git.selfprivacy.org/SelfPrivacy/selfprivacy-rest-api/issues/91)~~ There is a fallback available now. PR is too big now, and there seems to be no regressions.
inex added 4 commits 2024-01-26 16:47:00 +02:00
inex added 1 commit 2024-01-29 15:54:14 +02:00
inex force-pushed api-connection-refactor from 4b735483bf to 831a0e95eb 2024-01-29 17:58:52 +02:00 Compare
inex added 1 commit 2024-01-29 18:14:16 +02:00
inex added 1 commit 2024-01-29 18:45:53 +02:00
misterfourtytwo reviewed 2024-01-31 07:43:56 +02:00
@ -0,0 +30,4 @@
);
}
on<ServicesListUpdate>(

imho handler registration should preceed event subscription. not sure how it works for bloc default state now, but it could easily drop event from 26 line, because handler was not yet registered.

imho handler registration should preceed event subscription. not sure how it works for bloc default state now, but it could easily drop event from 26 line, because handler was not yet registered.
inex marked this conversation as resolved
misterfourtytwo reviewed 2024-01-31 07:57:01 +02:00
@ -52,7 +52,8 @@ class _ServicesPageState extends State<ServicesPage> {
)
: RefreshIndicator(
onRefresh: () async {

could just write:

onRefresh: context.read<ServicesBloc>().awaitReload,
could just write: ```dart onRefresh: context.read<ServicesBloc>().awaitReload, ```
inex marked this conversation as resolved
misterfourtytwo reviewed 2024-01-31 08:11:48 +02:00
@ -426,3 +417,3 @@
onTap: preventActions
? null
: () => {context.read<BackupsCubit>().forceUpdateBackups()},
: () => {

should probably fix this project-wide, callbacks should not return sets of values.

() {} // function
() => expr; // function

() => 5; // function returning integer value
() => {} // function returning set or map

callbacks could still work as intended, BUT this could become a habit and introduce real tricky bugs.

should probably fix this project-wide, callbacks should not return sets of values. ```dart () {} // function () => expr; // function () => 5; // function returning integer value () => {} // function returning set or map ``` callbacks could still work as intended, BUT this could become a habit and introduce real tricky bugs.
inex marked this conversation as resolved
inex added 4 commits 2024-01-31 12:57:17 +02:00
31c6a18918 Merge remote-tracking branch 'origin/directives_ordering' into api-connection-refactor
# Conflicts:
#	lib/config/bloc_config.dart
#	lib/logic/cubit/app_config_dependent/authentication_dependend_cubit.dart
#	lib/logic/cubit/backups/backups_cubit.dart
#	lib/logic/cubit/dns_records/dns_records_cubit.dart
#	lib/logic/cubit/providers/providers_cubit.dart
#	lib/logic/models/service.dart
#	lib/ui/pages/backups/backup_details.dart
#	lib/ui/pages/backups/change_period_modal.dart
#	lib/ui/pages/backups/change_rotation_quotas_modal.dart
#	lib/ui/pages/backups/copy_encryption_key_modal.dart
#	lib/ui/pages/more/more.dart
#	lib/ui/pages/server_storage/binds_migration/migration_process_page.dart
#	lib/ui/pages/server_storage/server_storage.dart
#	lib/ui/pages/server_storage/storage_card.dart
inex added 3 commits 2024-01-31 13:14:57 +02:00
inex added 2 commits 2024-01-31 14:17:32 +02:00
inex added 2 commits 2024-01-31 16:06:54 +02:00
inex added 1 commit 2024-02-01 16:30:11 +02:00
inex added 1 commit 2024-02-06 17:21:25 +02:00
inex added 2 commits 2024-02-08 17:08:43 +02:00
inex changed title from WIP: refactior: Introduce the API connection repository to WIP: refactor: Introduce the API connection repository 2024-02-08 17:09:22 +02:00
inex added 1 commit 2024-02-09 13:07:09 +02:00
inex added 1 commit 2024-02-09 15:54:07 +02:00
inex added 1 commit 2024-02-09 17:01:09 +02:00
inex added 1 commit 2024-02-13 11:41:11 +02:00
inex added 1 commit 2024-02-15 15:30:01 +02:00
inex added 1 commit 2024-02-20 18:33:29 +02:00
16094a3257 refactor: Rework ClientJobs cubit so it doesn't depend on other cubits
Also implemented tracking of the jobs and rebuild status
inex added 2 commits 2024-02-20 19:09:19 +02:00
inex requested review from NaiJi 2024-02-20 19:11:21 +02:00
inex added 1 commit 2024-02-20 19:15:26 +02:00
inex changed title from WIP: refactor: Introduce the API connection repository to refactor: Introduce the API connection repository 2024-02-20 19:15:48 +02:00
Collaborator

image

![image](/attachments/0654c17f-718f-40d5-a449-52785af2b233)
inex added 1 commit 2024-02-20 22:17:47 +02:00
inex added 1 commit 2024-02-20 22:34:48 +02:00
inex added 1 commit 2024-02-20 23:45:37 +02:00
inex added a new dependency 2024-02-21 00:52:36 +02:00
inex added 1 commit 2024-02-21 04:00:50 +02:00
NaiJi reviewed 2024-02-22 00:57:00 +02:00
@ -59,0 +82,4 @@
),
message: result.parsedData!.system.runSystemUpgrade.message,
);
} else {
Collaborator

What the hell 😨 , looks scary. is this how it's supposed to be now for all GraphQL requests?

What the hell 😨 , looks scary. is this how it's supposed to be now for all GraphQL requests?
Poster
Owner

I think there can be a better approach, but in general, yeah, every request must return a GenericResult or something like that.

I think there can be a better approach, but in general, yeah, every request must return a GenericResult or something like that.
NaiJi reviewed 2024-02-22 01:00:55 +02:00
@ -0,0 +147,4 @@
final Emitter<BackupsState> emit,
) async {
if (state is! BackupsUnititialized) {
return;
Collaborator

This is quite extreme, is this impossible to trigger? Do we need a notification for it?

This is quite extreme, is this impossible to trigger? Do we need a notification for it?
Poster
Owner

Better safe than sorry. Events now trigger these functions, and not the direct calls.

Better safe than sorry. Events now trigger these functions, and not the direct calls.
NaiJi reviewed 2024-02-22 01:04:44 +02:00
@ -0,0 +131,4 @@
final List<Backup> list = _backupList;
list.sort((final a, final b) => b.time.compareTo(a.time));
return list;
} on UnsupportedError {
Collaborator

Why specifically UnsupportedError ? We don't care about other possible errors in this block?

Why specifically `UnsupportedError` ? We don't care about other possible errors in this block?
NaiJi reviewed 2024-02-22 01:06:14 +02:00
@ -0,0 +37,4 @@
final DevicesListChanged event,
final Emitter<DevicesState> emit,
) async {
if (state is DevicesDeleting) {
Collaborator

This really scares me. I suspect it's in all other blocs like this... what do you think?

This really scares me. I suspect it's in all other blocs like this... what do you think?
NaiJi reviewed 2024-02-22 01:08:12 +02:00
@ -0,0 +73,4 @@
emit(
DevicesLoaded(
devices: state.devices
.where((final d) => d.name != event.device.name)
Collaborator

Duplication with line 62?

Duplication with line 62?
Poster
Owner

There are different states emited

There are different states emited
NaiJi reviewed 2024-02-22 01:13:34 +02:00
@ -450,3 +442,1 @@
onTap: preventActions
? null
: () => {context.read<BackupsCubit>().reuploadKey()},
// onTap: preventActions
Collaborator

Why? Probs you wanted to just

onTap: () => {context.read<BackupsCubit>().reuploadKey()}

?

Why? Probs you wanted to just ``` onTap: () => {context.read<BackupsCubit>().reuploadKey()} ``` ?
NaiJi reviewed 2024-02-22 01:14:43 +02:00
@ -73,0 +325,4 @@
? 0.0
: ((rebuildJob.progress ?? 0) < 1)
? null
: rebuildJob.progress! / 100.0,
Collaborator

I can already see how we create tickets to refactor all that

I can already see how we create tickets to refactor all that
Poster
Owner

Welp...

Server API does not return the progress value yet, but might return later.

When we pass null, it does the cute loading animation.

Welp... Server API does not return the progress value yet, but might return later. When we pass `null`, it does the cute loading animation.
NaiJi reviewed 2024-02-22 01:14:59 +02:00
@ -73,0 +223,4 @@
],
);
},
),
Collaborator

it's so over

it's so over
NaiJi reviewed 2024-02-22 01:17:14 +02:00
@ -163,0 +371,4 @@
final String timezone;
@override
Future<(bool, String)> execute() async =>
Collaborator

Can't we use GenericResult everywhere? Is it overkill?

Can't we use `GenericResult` everywhere? Is it overkill?
NaiJi reviewed 2024-02-22 01:19:18 +02:00
@ -27,8 +27,9 @@ class ServerHostingDetails {
@HiveField(2)
final DateTime? startTime;
// TODO: Check if it is still needed
Collaborator

I am 99% sure it isn't 🙏 will do some cleaning evenetually

I am 99% sure it isn't 🙏 will do some cleaning evenetually
NaiJi approved these changes 2024-02-22 01:21:27 +02:00
inex added 1 commit 2024-02-23 16:49:14 +02:00
inex merged commit 0d12b1d2d7 into master 2024-02-23 16:49:39 +02:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Blocks
Reference: SelfPrivacy/selfprivacy.org.app#440
There is no content yet.