671374da9a
- setWiFi() now throws detailed errors instead of returning false - Shows specific error messages: "WiFi credentials rejected", timeout etc. - Added logging throughout BLE WiFi configuration flow - Fixed WiFi network deduplication (keeps strongest signal) - Ignore "Operation cancelled" error (normal cleanup behavior) - BatchSetupProgress shows actual error in hint field 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
265 lines
11 KiB
Markdown
265 lines
11 KiB
Markdown
# Code Review Report — WellNuo Push Notifications System
|
||
|
||
**Date:** 2026-01-26
|
||
**Reviewers:** Claude Code
|
||
**Workers:** @worker1 (Backend), @worker2 (Mobile App)
|
||
**Status:** Partial Completion
|
||
|
||
---
|
||
|
||
## Executive Summary
|
||
|
||
Параллельное выполнение задач завершено с **частичным успехом**. Backend часть выполнена хорошо, Mobile App часть выполнена частично — отсутствует ключевой компонент `expo-notifications`.
|
||
|
||
**Оценка качества: 6/10**
|
||
|
||
---
|
||
|
||
## Task Completion Status
|
||
|
||
### @worker1 — Backend Tasks
|
||
|
||
| Task | Status | Comments |
|
||
|------|--------|----------|
|
||
| **TASK-1:** Улучшить sendPushNotifications с проверкой настроек | ✅ Done | Отличная реализация в `notifications.js` |
|
||
| **TASK-2:** notification_history таблица и логирование | ✅ Done | Миграция + сервис созданы |
|
||
| **TASK-3:** API для получения истории алертов | ✅ Done | `/api/notification-settings/history` работает |
|
||
| **TASK-4:** Деплой backend изменений | ⚠️ Not Verified | Нужен ручной деплой на сервер |
|
||
|
||
### @worker2 — Mobile App Tasks
|
||
|
||
| Task | Status | Comments |
|
||
|------|--------|----------|
|
||
| **TASK-5:** Установить expo-notifications | ❌ Not Done | Пакет НЕ установлен в package.json |
|
||
| **TASK-6:** Создать сервис pushNotifications.ts | ❌ Not Done | Файл отсутствует |
|
||
| **TASK-7:** Интеграция при логине | ❌ Not Done | Зависит от TASK-5/6 |
|
||
| **TASK-8:** Обработка входящих push уведомлений | ❌ Not Done | Зависит от TASK-5/6 |
|
||
| **TASK-9:** UI настроек уведомлений | ✅ Done | `notifications.tsx` создан и работает |
|
||
|
||
---
|
||
|
||
## Code Quality Analysis
|
||
|
||
### Backend Code (`backend/src/services/notifications.js`)
|
||
|
||
**Strengths:**
|
||
1. ✅ Хорошая структура — модульный подход с отдельными функциями
|
||
2. ✅ Проверка notification settings перед отправкой
|
||
3. ✅ Quiet hours поддержка с учётом overnight периодов (22:00-07:00)
|
||
4. ✅ Emergency alerts обходят quiet hours — правильно
|
||
5. ✅ Batch отправка (chunks по 100) — согласно рекомендациям Expo
|
||
6. ✅ Логирование в notification_history для всех статусов (sent/skipped/failed)
|
||
7. ✅ Валидация Expo Push Token формата
|
||
|
||
**Potential Issues:**
|
||
1. ⚠️ `isInQuietHours` использует `timezone = 'UTC'` по умолчанию, но пользователь может быть в другом timezone. Timezone не передаётся из settings.
|
||
2. ⚠️ `getUserPushTokens` фильтрует по `is_active = true`, но нигде не видно логики деактивации токенов при ошибках.
|
||
|
||
### Backend Code (`backend/src/routes/notification-settings.js`)
|
||
|
||
**Strengths:**
|
||
1. ✅ Правильный маппинг snake_case (DB) → camelCase (API)
|
||
2. ✅ Upsert для settings — работает корректно
|
||
3. ✅ Pagination с лимитом (max 100) в history endpoint
|
||
4. ✅ Фильтрация по type/status в history
|
||
|
||
**Potential Issues:**
|
||
1. ⚠️ Нет input validation — `quietStart`/`quietEnd` могут быть любыми строками
|
||
|
||
### Backend Code (`backend/src/services/mqtt.js`)
|
||
|
||
**Issues Found:**
|
||
1. ❌ **НЕ использует новый notifications.js сервис!** Функция `sendPushNotifications` в mqtt.js — старая версия, которая НЕ проверяет notification_settings
|
||
2. ❌ Дублирование кода — есть две версии `sendPushNotifications`:
|
||
- `mqtt.js:216` — старая, без проверки настроек
|
||
- `notifications.js:294` — новая, с полной логикой
|
||
|
||
### Migration (`010_create_notification_history.sql`)
|
||
|
||
**Strengths:**
|
||
1. ✅ Все необходимые поля (user_id, type, status, skip_reason, etc.)
|
||
2. ✅ Индексы для частых запросов
|
||
3. ✅ CHECK constraint на status
|
||
4. ✅ Комментарии на колонки
|
||
|
||
### Mobile App (`types/index.ts`)
|
||
|
||
**Strengths:**
|
||
1. ✅ Типы `NotificationSettings`, `NotificationHistoryItem`, `NotificationHistoryResponse` корректны
|
||
2. ✅ Соответствуют API response формату
|
||
|
||
### Mobile App (`services/api.ts`)
|
||
|
||
**Strengths:**
|
||
1. ✅ `getNotificationSettings()` — корректная реализация
|
||
2. ✅ `updateNotificationSettings()` — работает
|
||
3. ✅ `getNotificationHistory()` — с фильтрами и pagination
|
||
|
||
### Mobile App (`app/(tabs)/profile/notifications.tsx`)
|
||
|
||
**Strengths:**
|
||
1. ✅ Хороший UI с Switch компонентами
|
||
2. ✅ Загрузка настроек с сервера
|
||
3. ✅ Сохранение через API
|
||
4. ✅ Quiet hours toggle с отображением времени
|
||
|
||
**Issues:**
|
||
1. ⚠️ Time picker для quiet hours — заглушка "Coming Soon"
|
||
2. ⚠️ "Send Test Notification" — только Alert, не отправляет реальный push
|
||
|
||
---
|
||
|
||
## Security Analysis
|
||
|
||
### Positive:
|
||
1. ✅ JWT authentication на всех endpoints
|
||
2. ✅ Expo Push Token validation
|
||
3. ✅ Admin-only endpoints в mqtt.js
|
||
|
||
### Concerns:
|
||
1. ⚠️ `quietStart`/`quietEnd` не валидируются как HH:MM формат
|
||
2. ⚠️ MQTT credentials хардкодены в коде (хоть и через env)
|
||
|
||
---
|
||
|
||
## Critical Missing Components
|
||
|
||
### 1. expo-notifications Package
|
||
```bash
|
||
# Ожидалось в package.json:
|
||
"expo-notifications": "~0.31.0" # НЕ НАЙДЕНО!
|
||
```
|
||
|
||
### 2. pushNotifications.ts Service
|
||
Файл `services/pushNotifications.ts` **не создан**. Должен содержать:
|
||
- `registerForPushNotificationsAsync()`
|
||
- `registerTokenOnServer(token)`
|
||
- `unregisterToken()`
|
||
|
||
### 3. MQTT → Notifications Integration
|
||
`mqtt.js` НЕ интегрирован с новым `notifications.js`:
|
||
|
||
```javascript
|
||
// mqtt.js:140 — ТЕКУЩИЙ КОД (неправильно):
|
||
await sendPushNotifications(alert); // Локальная функция без проверки settings
|
||
|
||
// ДОЛЖНО БЫТЬ:
|
||
const { sendPushNotifications } = require('./notifications');
|
||
await sendPushNotifications({
|
||
userIds: users.map(u => u.user_id),
|
||
title: `Alert: ${beneficiaryName}`,
|
||
body: alert.body,
|
||
type: 'emergency', // или определять по содержимому
|
||
beneficiaryId: alert.beneficiaryId,
|
||
});
|
||
```
|
||
|
||
---
|
||
|
||
## Build & TypeScript Check
|
||
|
||
```
|
||
✅ TypeScript: No errors in main WellNuo project
|
||
❌ npm run build: Script not defined (expected — Expo project)
|
||
```
|
||
|
||
---
|
||
|
||
## Recommendations
|
||
|
||
### Critical (Must Fix):
|
||
|
||
1. **Установить expo-notifications:**
|
||
```bash
|
||
npx expo install expo-notifications
|
||
```
|
||
|
||
2. **Создать services/pushNotifications.ts:**
|
||
- Реализовать регистрацию push токена
|
||
- Интегрировать в login flow
|
||
|
||
3. **Интегрировать mqtt.js с notifications.js:**
|
||
- Заменить локальную `sendPushNotifications` на импорт из `notifications.js`
|
||
- Определять тип алерта (emergency/activity/low_battery) по содержимому
|
||
|
||
### Important:
|
||
|
||
4. **Добавить input validation для quiet hours:**
|
||
```javascript
|
||
const timeRegex = /^([01]\d|2[0-3]):([0-5]\d)$/;
|
||
if (quietStart && !timeRegex.test(quietStart)) {
|
||
return res.status(400).json({ error: 'Invalid quietStart format' });
|
||
}
|
||
```
|
||
|
||
5. **Добавить timezone в notification_settings таблицу:**
|
||
- Quiet hours должны работать в timezone пользователя
|
||
|
||
6. **Деплой backend на сервер:**
|
||
```bash
|
||
rsync -avz backend/ root@91.98.205.156:/var/www/wellnuo-api/
|
||
ssh root@91.98.205.156 "cd /var/www/wellnuo-api && npm install && pm2 restart wellnuo-api"
|
||
```
|
||
|
||
### Nice to Have:
|
||
|
||
7. Реализовать time picker для quiet hours (вместо заглушки)
|
||
8. Добавить логику деактивации push токенов при ошибках доставки
|
||
|
||
---
|
||
|
||
## Test Checklist
|
||
|
||
| Test | Status |
|
||
|------|--------|
|
||
| Push токен регистрируется при логине | ❌ Не реализовано |
|
||
| MQTT алерты сохраняются в mqtt_alerts | ✅ Работает |
|
||
| Push отправляется с учётом настроек | ⚠️ Код есть, но не интегрирован |
|
||
| Notification history записывается | ✅ Работает |
|
||
| UI настроек работает | ✅ Работает |
|
||
| TypeScript без ошибок | ✅ Проходит |
|
||
|
||
---
|
||
|
||
## Files Changed
|
||
|
||
| File | Worker | Status | Quality |
|
||
|------|--------|--------|---------|
|
||
| `backend/migrations/010_create_notification_history.sql` | @worker1 | ✅ Created | Good |
|
||
| `backend/src/routes/notification-settings.js` | @worker1 | ✅ Modified | Good |
|
||
| `backend/src/services/notifications.js` | @worker1 | ✅ Created | Excellent |
|
||
| `services/api.ts` | @worker2 | ✅ Modified | Good |
|
||
| `types/index.ts` | @worker2 | ✅ Modified | Good |
|
||
| `app/(tabs)/profile/notifications.tsx` | @worker2 | ✅ Exists | Good |
|
||
| `services/pushNotifications.ts` | @worker2 | ❌ Missing | — |
|
||
|
||
---
|
||
|
||
## Final Score
|
||
|
||
| Category | Score | Max | Notes |
|
||
|----------|-------|-----|-------|
|
||
| Task Completion | 5 | 10 | 5/9 tasks done |
|
||
| Code Quality | 8 | 10 | Clean, well-structured |
|
||
| TypeScript | 10 | 10 | No errors |
|
||
| Security | 8 | 10 | Good auth, minor validation gaps |
|
||
| Integration | 4 | 10 | mqtt.js not connected to notifications.js |
|
||
|
||
**Total: 6/10**
|
||
|
||
---
|
||
|
||
## Conclusion
|
||
|
||
Backend часть (@worker1) выполнена качественно — создан полноценный notification service с проверкой настроек, quiet hours, и логированием истории.
|
||
|
||
Mobile App часть (@worker2) выполнена частично — UI настроек работает, но ключевой функционал регистрации push токенов отсутствует.
|
||
|
||
**Главная проблема:** `mqtt.js` использует старую локальную версию `sendPushNotifications` вместо нового сервиса `notifications.js`. Это означает, что настройки уведомлений НЕ применяются к реальным MQTT алертам.
|
||
|
||
Для достижения минимального балла 8/10 необходимо:
|
||
1. Установить expo-notifications
|
||
2. Создать pushNotifications.ts сервис
|
||
3. Интегрировать mqtt.js с notifications.js
|
||
4. Задеплоить backend на сервер
|