fix: Fix the database migration and prevent this error in the code go…#36
Merged
Conversation
Contributor
Руководство для ревьюераРефакторинг обработки ролей пользователя за счёт введения типизированного enum в доменной модели, замены базового столбца на обычную строку и одновременного обеспечения создания устаревшего типа PostgreSQL ENUM в начальной миграции Alembic для совместимости с существующими базами данных. Изменения на уровне файлов
Подсказки и командыВзаимодействие с Sourcery
Настройка работыПерейдите в свою панель управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's GuideRefactors the user role handling by introducing a typed enum in the domain model, changing the underlying column to a simple string while ensuring the legacy PostgreSQL ENUM type is created in the initial Alembic migration for compatibility with existing databases. 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.
Привет — я оставил несколько общих замечаний:
- Новый
UserRoleStrEnum определён, но столбец SQLAlchemy всё ещё имеет типMapped[str]сString(20)— имеет смысл маппить его напрямую как Enum (Mapped[UserRole]/Enum(UserRole, ...)) для более строгой типизации и согласованности между моделью и БД. - В миграции вы создаёте тип ENUM
user_role, но не удаляете его вdowngrade, из‑за чего в базе могут оставаться «осиротевшие» типы enum; стоит также удалить этот ENUM тип вdowngrade. - В миграции теперь явно создаётся ENUM
user_role, в то время как модель использует обычныйString(20)дляrole; будет хорошо выровнять типы столбцов, чтобы схема Alembic и модель ORM описывали один и тот же тип данных.
Подсказка для AI-агентов
Please address the comments from this code review:
## Overall Comments
- The new `UserRole` StrEnum is defined but the SQLAlchemy column is still typed as `Mapped[str]` with `String(20)`—consider mapping it directly as an Enum (`Mapped[UserRole]` / `Enum(UserRole, ...)`) for stronger typing and consistency between model and DB.
- In the migration you create the `user_role` ENUM type but never drop it in `downgrade`, which can leave orphan enum types in the database; consider dropping the ENUM type there as well.
- The migration now explicitly creates a `user_role` ENUM while the model uses a plain `String(20)` for `role`; it would be good to align the column types so that the Alembic schema and ORM model describe the same data type.Sourcery бесплатен для open source — если вам нравятся наши ревью, пожалуйста, расскажите о нас ✨
Original comment in English
Hey - I've left some high level feedback:
- The new
UserRoleStrEnum is defined but the SQLAlchemy column is still typed asMapped[str]withString(20)—consider mapping it directly as an Enum (Mapped[UserRole]/Enum(UserRole, ...)) for stronger typing and consistency between model and DB. - In the migration you create the
user_roleENUM type but never drop it indowngrade, which can leave orphan enum types in the database; consider dropping the ENUM type there as well. - The migration now explicitly creates a
user_roleENUM while the model uses a plainString(20)forrole; it would be good to align the column types so that the Alembic schema and ORM model describe the same data type.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `UserRole` StrEnum is defined but the SQLAlchemy column is still typed as `Mapped[str]` with `String(20)`—consider mapping it directly as an Enum (`Mapped[UserRole]` / `Enum(UserRole, ...)`) for stronger typing and consistency between model and DB.
- In the migration you create the `user_role` ENUM type but never drop it in `downgrade`, which can leave orphan enum types in the database; consider dropping the ENUM type there as well.
- The migration now explicitly creates a `user_role` ENUM while the model uses a plain `String(20)` for `role`; it would be good to align the column types so that the Alembic schema and ORM model describe the same data type.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Coverage Report for CI Build 26837916037Coverage increased (+0.3%) to 30.313%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions7 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
1 similar comment
Coverage Report for CI Build 26837916037Coverage increased (+0.3%) to 30.313%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions7 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
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.
…ing forward.
Summary by Sourcery
Изменение хранения роли пользователя и приведение миграции базы данных в соответствие с использованием enum
Новые возможности:
UserRoleдля представления ролей пользователей в доменной моделиИсправления ошибок:
user_roleдо создания таблицыusersУлучшения:
User.roleна использование простого строкового поля с значением по умолчанию на основе нового enumUserRoleвместо типаSQLAlchemy EnumOriginal summary in English
Summary by Sourcery
Adjust user role storage and align database migration with enum usage
New Features:
Bug Fixes:
Enhancements: