Added PostHog #53

Closed
protuberanets wants to merge 1 commits from SEND004 into main
Collaborator
No description provided.
protuberanets added 1 commit 2025-12-09 18:04:14 +00:00
protuberanets requested review from tech 2025-12-09 18:04:14 +00:00
tech reviewed 2025-12-10 13:51:48 +00:00
tech left a comment
Owner

тест

тест
@@ -27,6 +27,7 @@ class AccountProvider extends ChangeNotifier {
Resource<Account?> get resource => _resource;
late LocaleProvider _localeProvider;
PendingLogin? _pendingLogin;
Future<void>? _restoreFuture;
Owner

не нужно наружу выставлять детали реализации. Это плохой дизайн, класс должен сам за себя отвечать, и вызов restore() не должен отличаться от вызова login(), соответственно наружу выставлять _restoreFuture категорически не надо.

не нужно наружу выставлять детали реализации. Это плохой дизайн, класс должен сам за себя отвечать, и вызов restore() не должен отличаться от вызова login(), соответственно наружу выставлять _restoreFuture категорически не надо.
@@ -222,2 +224,3 @@
}
}
Future<void> restoreIfPossible() {
Owner

а зачем это? ну есть же restore уже?

а зачем это? ну есть же restore уже?
Author
Collaborator

restoreIfPossible оборачивает restore(), сначала проверяя AuthorizationService.isAuthorizationStored(), чтобы вызывать восстановление только при наличии сохранённых данных, а затем кешируя текущий _restoreFuture, чтобы все параллельные вызовы ждали один и тот же процесс без дублирования работы.

restoreIfPossible оборачивает restore(), сначала проверяя AuthorizationService.isAuthorizationStored(), чтобы вызывать восстановление только при наличии сохранённых данных, а затем кешируя текущий _restoreFuture, чтобы все параллельные вызовы ждали один и тот же процесс без дублирования работы.
@@ -224,0 +227,4 @@
return _restoreFuture ??= () async {
final hasAuth = await AuthorizationService.isAuthorizationStored();
if (!hasAuth) return;
await restore();
Owner

тут говорил уже: не надо блокировать исполнение, возвращай Future, пусть его использует вызывающий код

тут говорил уже: не надо блокировать исполнение, возвращай Future, пусть его использует вызывающий код
@@ -110,3 +108,1 @@
// _status,
// _methods.keys.toSet(),
// );
await PosthogService.recipientAddCompleted(
Owner

не нужно блокировать отбрасывание статистики, ты результатом все равно не пользуешься

не нужно блокировать отбрасывание статистики, ты результатом все равно не пользуешься
@@ -28,0 +22,4 @@
WidgetsBinding.instance.addPostFrameCallback((_) {
final account = provider.account;
if (account != null) {
PosthogService.identify(account);
Owner

установкой текущего акаунта ведает провайдер, на уровне провайдера и надо отрбрасывать успешную идентификацию, НЕ на уровне виджета, цель которого состоит только в обеспечении того, что все защищенные страницы точно идут с загруженным акаунтом.

установкой текущего акаунта ведает провайдер, на уровне провайдера и надо отрбрасывать успешную идентификацию, НЕ на уровне виджета, цель которого состоит только в обеспечении того, что все защищенные страницы точно идут с загруженным акаунтом.
@@ -29,0 +40,4 @@
return const Center(child: CircularProgressIndicator());
}
if (provider.restoreFuture == null) {
Owner

почему просто restore() нельзя вызвать? зачем так?

почему просто restore() нельзя вызвать? зачем так?
@@ -29,0 +43,4 @@
if (provider.restoreFuture == null) {
WidgetsBinding.instance.addPostFrameCallback((_) {
provider.restoreIfPossible().catchError((error, stack) {
debugPrint('Account restore failed: $error');
Owner

вместо debugPrint в коде лучше использовать logger, у нас же есть в проекте легирование?

вместо debugPrint в коде лучше использовать logger, у нас же есть в проекте легирование?
@@ -43,10 +44,14 @@ class _LoginFormState extends State<LoginForm> {
password: _passwordController.text,
locale: context.read<LocaleProvider>().locale.languageCode,
);
await PosthogService.login(pending: outcome.isPending);
Owner

а зачем await тут? мы ж не используем результат

а зачем await тут? мы ж не используем результат
@@ -48,2 +50,4 @@
navigateAndReplace(context, Pages.sfactor);
} else {
if (outcome.account != null) {
await PosthogService.identify(outcome.account!);
Owner

именно вот поэтому идентификацию делать надо в провайдеры, когда устанавливаешь текущий аккаунт. Чтобы потом по коду не размазывать миллион идентификаций в зависимости от ....

именно вот поэтому идентификацию делать надо в провайдеры, когда устанавливаешь текущий аккаунт. Чтобы потом по коду не размазывать миллион идентификаций в зависимости от ....
@@ -0,0 +128,4 @@
if (properties != null) {
for (final entry in properties.entries) {
final value = entry.value;
if (value != null) filtered[entry.key] = value;
Owner

а почему null значения сфильтровываются?

а почему null значения сфильтровываются?
Author
Collaborator

filtered объявлен как Map<String, Object>. Перед фильтрацией просто все null сбрасываю, чтобы не слать их в PostHog и не ловить ошибки сериализации/платформенного канала

filtered объявлен как Map<String, Object>. Перед фильтрацией просто все null сбрасываю, чтобы не слать их в PostHog и не ловить ошибки сериализации/платформенного канала
@@ -35,6 +35,7 @@ dependencies:
sdk: flutter
pshared:
path: ../pshared
posthog_flutter: ^4.0.1
Owner

а почему четвертая версия, если уже 5.9 есть?

а почему четвертая версия, если уже 5.9 есть?
protuberanets closed this pull request 2025-12-16 15:51:22 +00:00

Pull request closed

Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: tech/sendico#53