SelfPrivacy
/
selfprivacy.org.app
10
7
Fork 5
Code Issues 75 Pull Requests 3 Projects 1 Releases 24 Wiki Activity

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
Conversation 1 Commits 36 Files Changed 116
inex commented 2024-01-26 16:46:59 +02:00
Owner

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.
👍 2
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 added 1 commit 2024-01-29 17:57:27 +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
lib/logic/bloc/services/services_bloc.dart
@ -0,0 +30,4 @@
);
}
on<ServicesListUpdate>(
misterfourtytwo commented 2024-01-31 07:43:56 +02:00

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.
👍 1
inex marked this conversation as resolved
misterfourtytwo reviewed 2024-01-31 07:57:01 +02:00
lib/ui/pages/services/services.dart Outdated
@ -52,7 +52,8 @@ class _ServicesPageState extends State<ServicesPage> {
)
: RefreshIndicator(
onRefresh: () async {
misterfourtytwo commented 2024-01-31 07:57:01 +02:00

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
lib/ui/pages/backups/backup_details.dart Outdated
@ -426,3 +417,3 @@
onTap: preventActions
? null
: () => {context.read<BackupsCubit>().forceUpdateBackups()},
: () => {
misterfourtytwo commented 2024-01-31 08:11:48 +02:00

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
NaiJi commented 2024-02-20 19:24:51 +02:00
Collaborator

image

![image](/attachments/0654c17f-718f-40d5-a449-52785af2b233)
image.png
87 KiB
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
#464 Add website and support links to the About page
inex added 1 commit 2024-02-21 04:00:50 +02:00
NaiJi reviewed 2024-02-22 00:57:00 +02:00
lib/logic/api_maps/graphql_maps/server_api/server_actions_api.dart
@ -59,0 +82,4 @@
),
message: result.parsedData!.system.runSystemUpgrade.message,
);
} else {
NaiJi commented 2024-02-22 00:57:00 +02:00
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?
inex commented 2024-02-23 16:25:37 +02:00
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
lib/logic/bloc/backups/backups_bloc.dart
@ -0,0 +147,4 @@
final Emitter<BackupsState> emit,
) async {
if (state is! BackupsUnititialized) {
return;
NaiJi commented 2024-02-22 01:00:55 +02:00
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?
inex commented 2024-02-23 16:26:53 +02:00
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
lib/logic/bloc/backups/backups_state.dart Outdated
@ -0,0 +131,4 @@
final List<Backup> list = _backupList;
list.sort((final a, final b) => b.time.compareTo(a.time));
return list;
} on UnsupportedError {
NaiJi commented 2024-02-22 01:04:44 +02:00
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
lib/logic/bloc/devices/devices_bloc.dart
@ -0,0 +37,4 @@
final DevicesListChanged event,
final Emitter<DevicesState> emit,
) async {
if (state is DevicesDeleting) {
NaiJi commented 2024-02-22 01:06:14 +02:00
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
lib/logic/bloc/devices/devices_bloc.dart
@ -0,0 +73,4 @@
emit(
DevicesLoaded(
devices: state.devices
.where((final d) => d.name != event.device.name)
NaiJi commented 2024-02-22 01:08:12 +02:00
Collaborator

Duplication with line 62?

Duplication with line 62?
inex commented 2024-02-23 16:37:05 +02:00
Poster
Owner

There are different states emited

There are different states emited
NaiJi reviewed 2024-02-22 01:13:34 +02:00
lib/ui/pages/backups/backup_details.dart Outdated
@ -450,3 +442,1 @@
onTap: preventActions
? null
: () => {context.read<BackupsCubit>().reuploadKey()},
// onTap: preventActions
NaiJi commented 2024-02-22 01:13:34 +02:00
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
lib/ui/components/jobs_content/jobs_content.dart
@ -73,0 +325,4 @@
? 0.0
: ((rebuildJob.progress ?? 0) < 1)
? null
: rebuildJob.progress! / 100.0,
NaiJi commented 2024-02-22 01:14:43 +02:00
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
inex commented 2024-02-23 16:45:25 +02:00
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
lib/ui/components/jobs_content/jobs_content.dart
@ -73,0 +223,4 @@
],
);
},
),
NaiJi commented 2024-02-22 01:14:59 +02:00
Collaborator

it's so over

it's so over
NaiJi reviewed 2024-02-22 01:17:14 +02:00
lib/logic/models/job.dart
@ -163,0 +371,4 @@
final String timezone;
@override
Future<(bool, String)> execute() async =>
NaiJi commented 2024-02-22 01:17:14 +02:00
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
lib/logic/models/hive/server_details.dart
@ -27,8 +27,9 @@ class ServerHostingDetails {
@HiveField(2)
final DateTime? startTime;
// TODO: Check if it is still needed
NaiJi commented 2024-02-22 01:19:18 +02:00
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.
Reviewers
No reviewers
NaiJi
Labels
Clear labels   
Blocked

Issue that can't be resolved right now due to some circumstances   
Bug

Discovered issue that affects regular operation   
Contributions welcome
   
Did not do

Issue is not queued for resolution due to low priority or introduced difficulties during the resolution   
Errata

Known issue with available workaround   
Feature request

Functionality/usability enchansement   
Fixed

Issue resolved in one of the application/backend releases   
How To

Question, regarding product operation   
Invalid
   
Needs design

A sketch or detailed description of the feature look is wanted   
No resolution

Issue is not queued for resolution due to architecture pecularities
  
Priority
Critical
   
Priority
High
   
Priority
Low
   
Priority
Medium
  
Providers
Digital Ocean
Issues related to Digital Ocean REST API   
Providers
Hetzner
Issues related to Hetzner REST API
  
Refactor
  
Severity
High
   
Severity
Low
   
Severity
Medium
  
Source
Community
Feature requests and bugs reported on our support chats   
Source
Core Team
   
Source
Stakeholders
Kirill, NLnet, and others
  
Translations
   
Under investigation

Issue details are currently being investigated
No Label
Blocked
Bug
Contributions welcome
Did not do
Errata
Feature request
Fixed
How To
Invalid
Needs design
No resolution
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Providers
Digital Ocean
Providers
Hetzner
Refactor
Severity
High
Severity
Low
Severity
Medium
Source
Community
Source
Core Team
Source
Stakeholders
Translations
Under investigation
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
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
#464 Add website and support links to the About page
SelfPrivacy/selfprivacy.org.app
Reference: SelfPrivacy/selfprivacy.org.app#440
There is no content yet.
Delete Branch "api-connection-refactor"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?