fix(auth): Fix data type error when creating a user in the database#5
Merged
Conversation
Contributor
Руководство для ревьюера (свернуто для маленьких PR)Руководство для ревьюераИзменяет логику поиска пользователя в сервисе аутентификации так, чтобы Диаграмма последовательности для поиска пользователя в auth_service find_user_registrationsequenceDiagram
actor Client
participant AuthService
participant DB
participant User
Client->>AuthService: find_user_registration(user, db)
AuthService->>AuthService: build conditions with email and phone
alt telegram_id is not None
AuthService->>AuthService: telegram_id_str = str(user.telegram_id)
AuthService->>AuthService: add condition User.telegram_id == telegram_id_str
end
AuthService->>DB: execute(select(User).where(or(conditions)))
DB-->>AuthService: result
AuthService-->>Client: user or None
Диаграмма классов обработки telegram_id в auth_service и модели UserclassDiagram
class AuthService {
+find_user_registration(user, db)
}
class User {
+email
+phone
+telegram_id string
}
AuthService ..> User : queries
Изменения на уровне файлов
Подсказки и командыВзаимодействие с Sourcery
Настройка работы SourceryЗайдите в свою панель управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts user lookup logic in the auth service to consistently treat telegram_id as a string when querying the database, preventing data type mismatches during user creation. Sequence diagram for auth_service find_user_registration user lookupsequenceDiagram
actor Client
participant AuthService
participant DB
participant User
Client->>AuthService: find_user_registration(user, db)
AuthService->>AuthService: build conditions with email and phone
alt telegram_id is not None
AuthService->>AuthService: telegram_id_str = str(user.telegram_id)
AuthService->>AuthService: add condition User.telegram_id == telegram_id_str
end
AuthService->>DB: execute(select(User).where(or(conditions)))
DB-->>AuthService: result
AuthService-->>Client: user or None
Class diagram for auth_service and User model telegram_id handlingclassDiagram
class AuthService {
+find_user_registration(user, db)
}
class User {
+email
+phone
+telegram_id string
}
AuthService ..> User : queries
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Привет — я нашёл 1 проблему и оставил несколько общих комментариев:
- Если
telegram_idконцептуально является числом, но хранится в БД как строка, стоит рассмотреть нормализацию этого поля на уровне модели/ORM (например, через кастомный тип или преобразование в моделиUser), а не только в этом конкретном запросе, чтобы избежать подобных несоответствий в других местах. - Чтобы поведение оставалось единообразным, имеет смысл применять такую же нормализацию через
str()дляtelegram_idвезде, где оно записывается в базу или сравнивается в запросах, чтобы исправление не ограничивалось только этим одним маршрутом выполнения запроса.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If `telegram_id` is conceptually numeric but stored as a string in the DB, consider normalizing this at the model/ORM level (e.g., via a custom type or conversion in the `User` model) rather than only in this query to avoid similar mismatches elsewhere.
- To keep behavior consistent, you may want to apply the same `str()` normalization for `telegram_id` wherever it is written to or compared in the database, so this fix isn't limited just to this one query path.
## Individual Comments
### Comment 1
<location path="src/domain/services/auth_service.py" line_range="40" />
<code_context>
conditions.append(User.phone == user.phone)
if user.telegram_id is not None:
- conditions.append(User.telegram_id == user.telegram_id)
+ conditions.append(User.telegram_id == str(user.telegram_id))
result = await db.execute(select(User).where(
</code_context>
<issue_to_address>
**suggestion:** Consider normalizing `telegram_id` types earlier instead of casting at query time.
Casting `user.telegram_id` to `str` in the query can mask mismatches between your API schema and persistence layer. If `telegram_id` should be a string, align the types in the `User` model and `UserRegister` schema and handle normalization at the boundary (e.g., schema validators or a helper) so queries remain pure filters and future changes to `telegram_id` representation are less error‑prone.
Suggested implementation:
```python
conditions.append(User.email == user.email)
conditions.append(User.phone == user.phone)
# normalize telegram_id to string once at the service boundary
if user.telegram_id is not None and not isinstance(user.telegram_id, str):
user.telegram_id = str(user.telegram_id)
if user.telegram_id is not None:
conditions.append(User.telegram_id == user.telegram_id)
result = await db.execute(select(User).where(
or_(*conditions)
```
To fully align with the suggestion about schema/model consistency, you will likely also want to:
1. Ensure the `telegram_id` field in your `UserRegister` (or equivalent) schema is declared as a `str` and, if needed, add a validator to coerce incoming numeric values to strings.
2. Confirm the `telegram_id` column on the `User` model is a string type (e.g., `String`/`VARCHAR`) so the persistence layer matches the API contract.
These changes will move all type normalization to the boundaries (schema/model) and keep service/query code strictly typed and declarative.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- If
telegram_idis conceptually numeric but stored as a string in the DB, consider normalizing this at the model/ORM level (e.g., via a custom type or conversion in theUsermodel) rather than only in this query to avoid similar mismatches elsewhere. - To keep behavior consistent, you may want to apply the same
str()normalization fortelegram_idwherever it is written to or compared in the database, so this fix isn't limited just to this one query path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If `telegram_id` is conceptually numeric but stored as a string in the DB, consider normalizing this at the model/ORM level (e.g., via a custom type or conversion in the `User` model) rather than only in this query to avoid similar mismatches elsewhere.
- To keep behavior consistent, you may want to apply the same `str()` normalization for `telegram_id` wherever it is written to or compared in the database, so this fix isn't limited just to this one query path.
## Individual Comments
### Comment 1
<location path="src/domain/services/auth_service.py" line_range="40" />
<code_context>
conditions.append(User.phone == user.phone)
if user.telegram_id is not None:
- conditions.append(User.telegram_id == user.telegram_id)
+ conditions.append(User.telegram_id == str(user.telegram_id))
result = await db.execute(select(User).where(
</code_context>
<issue_to_address>
**suggestion:** Consider normalizing `telegram_id` types earlier instead of casting at query time.
Casting `user.telegram_id` to `str` in the query can mask mismatches between your API schema and persistence layer. If `telegram_id` should be a string, align the types in the `User` model and `UserRegister` schema and handle normalization at the boundary (e.g., schema validators or a helper) so queries remain pure filters and future changes to `telegram_id` representation are less error‑prone.
Suggested implementation:
```python
conditions.append(User.email == user.email)
conditions.append(User.phone == user.phone)
# normalize telegram_id to string once at the service boundary
if user.telegram_id is not None and not isinstance(user.telegram_id, str):
user.telegram_id = str(user.telegram_id)
if user.telegram_id is not None:
conditions.append(User.telegram_id == user.telegram_id)
result = await db.execute(select(User).where(
or_(*conditions)
```
To fully align with the suggestion about schema/model consistency, you will likely also want to:
1. Ensure the `telegram_id` field in your `UserRegister` (or equivalent) schema is declared as a `str` and, if needed, add a validator to coerce incoming numeric values to strings.
2. Confirm the `telegram_id` column on the `User` model is a string type (e.g., `String`/`VARCHAR`) so the persistence layer matches the API contract.
These changes will move all type normalization to the boundaries (schema/model) and keep service/query code strictly typed and declarative.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Bug Fixes:
telegram_idк строке перед выполнением запроса к базе данных.Original summary in English
Summary by Sourcery
Bug Fixes: