DB2 #4
DB2 #4
Conversation
|
|
||
| func (db *EventDB) DeleteEvent(ctx context.Context, ID int) error { | ||
| _, err := db.pool.Exec(ctx, deleteEventQuery, ID) | ||
| return err |
There was a problem hiding this comment.
На удаление, добавление и обновление сущностей в базе нужна обработка ошибок. Чтобы кидать 500 только в случае, когда действительно база упала
| return port | ||
| conf.Port = port | ||
|
|
||
| redisConfig, err := sessionRepository.GetRedisConfig() |
There was a problem hiding this comment.
Странно выходит, что конфиг импортирует модули репозитория. По идее должно быть наоборот
internal/service/events/event.go
Outdated
| event, err := s.EventDB.AddEvent(ctx, event) | ||
| if err != nil { | ||
| if path != "" { | ||
| s.ImageDB.DeleteImage(ctx, path) |
There was a problem hiding this comment.
Надо залогировать ошибку. Я бы для этого прокинул логгер через контекст
| r.HandleFunc("/logout", authHandler.Logout).Methods(http.MethodPost) | ||
| r.HandleFunc("/session", authHandler.CheckSession).Methods(http.MethodGet) | ||
|
|
||
| r.HandleFunc("/profile/{id}", authHandler.Profile).Methods(http.MethodGet) |
There was a problem hiding this comment.
Забыли условие на число поставить
| subscriber_id INT NOT NULL, | ||
| subscribed_id INT NOT NULL, |
There was a problem hiding this comment.
Я бы задумался о переименовании этих полей, как будто легко запутаться
| VALUES ($1, $2, $3, $4, $5, $6, $7, $8) | ||
| RETURNING id` | ||
|
|
||
| func (db *EventDB) AddEvent(ctx context.Context, event models.Event) (models.Event, error) { |
There was a problem hiding this comment.
Имхо лучше Create, чем Add, мне кажется интуитивнее (но это имхо, можете оставить)
| LEFT JOIN tag ON tag.id = event_tag.tag_id | ||
| LEFT JOIN media_url ON event.id = media_url.event_id | ||
| WHERE | ||
| ($1::TEXT IS NULL OR event.title ILIKE '%' || $1 || '%' OR event.description ILIKE '%' || $1 || '%') |
There was a problem hiding this comment.
ILIKE нормально не работает с индексами, почитайте доку постгреса. Лучше делать lower() с обычным LIKE
| var pgErr *pgconn.PgError | ||
| if errors.As(err, &pgErr) { | ||
| return models.ErrForeignKeyViolation | ||
| } |
There was a problem hiding this comment.
Надо проверять на код конкретной ошибки постгреса
| @@ -14,6 +14,9 @@ func CORSMiddleware(next http.Handler) http.Handler { | |||
| "http://37.139.40.252", | |||
| "http://37.139.40.252:8080", | |||
| "37.139.40.252", | |||
| "127.0.0.1:8080", | |||
There was a problem hiding this comment.
Голые айпи я бы отсюда убрал, как я понимаю, origin всегда включает в себя протокол
| uploadPath = "./static/images" | ||
| defaultPage = 0 | ||
| defaultLimit = 30 | ||
| maxUploadSize = 1 * 1024 * 1024 // 1Mb |
There was a problem hiding this comment.
1 МБ может быть слишком маленьким значением. Также стоит задуматься над тем, чтобы выводить на фронте максимальный размер вложений (лучше перед загрузкой картинки, но нормальную ошибку с указанием макс размера тоже возвращать надо)
| logger: logger, | ||
| } | ||
| }, | ||
| wantCode: http.StatusInternalServerError, |
There was a problem hiding this comment.
Мне кажется, лучше оперировать терминами expected/actual, чем want
| recorder := httptest.NewRecorder() | ||
| tt.setupFunc(ctrl).GetUpcomingEvents(recorder, tt.req) | ||
|
|
||
| assert.Equal(t, tt.wantCode, recorder.Code) |
There was a problem hiding this comment.
Кода недостаточно, нужно проверять и ответ. Либо парсить его в структуру и проверять на равенство с ожидаемой, либо JSONEqual
| mockSetup: func(m pgxmock.PgxConnIface) { | ||
| m.ExpectExec(`DELETE FROM event WHERE id=\$1`). | ||
| WithArgs(3). | ||
| WillReturnError(fmt.Errorf("database error")) |
There was a problem hiding this comment.
Когда появится нормальная обработка кодов ошибок постгреса, надо будет возвращать здесь ошибки pgx. Также, где уместно, добавить проверки на sql.ErrNoRows
No description provided.