Chore/update migration#34
Merged
Merged
Conversation
…d fix date fields by adding the timezone=True property to store time zone information. Fix and add missing relationships
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…d also fix the issue of multiple bans for one user simultaneously, while preserving the ban history.
…e for consistency
* delete cloudflared.deb" * feat(models): Start implementing the role system, add a ban table, and fix date fields by adding the timezone=True property to store time zone information. Fix and add missing relationships * chore(.gitignore): remove tracking of different caches * refactor(tg_link): rename MagicTokens to MagicToken Co-authored-by: Cursor <cursoragent@cursor.com> * fix(bot): send login-link URL on start when telegram is not linked Co-authored-by: Cursor <cursoragent@cursor.com> * test(auth): add JWT unit tests Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(models): Improve the readability of field checks in the bans table * feat(models): Add validation of the revoked_at field at the database level * feat(models): Add checks for the expires_at and revoked_at fields, and also fix the issue of multiple bans for one user simultaneously, while preserving the ban history. * refactor(models): Rename MagicTokens to MagicToken across the codebase for consistency * fix(modedls): Add foreigkey to the appointments table relationship * fix(tasks): Fix database issues with the background task for clearing magic tokens --------- Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…18) * feat(ci): Add Docker image building and pushing to the ghcr registry * feat(docker-compose): Add watchtower to query the registry and download dcoker images from there. * feat(ci): Add a commit hash tag to Docker images
* fix: Remove the get_db file from infrastructure and remove logging of all SQL queries to reduce the IO load. * chore(jwt): Remove dprecated definition of the current time
Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
…al network (#22) Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
* fix(auth): Fix a vulnerability in the process of linking a Telegram bot to an account by adding X-Bot-Secret to the header * chore(auth): Improve logging of the create_telegram_magic_link function * fix(auth): Fix X-Bot-Secret authentication. Eliminates timig attack. * chore(requirements): update requrements.txt list * fix(bot): Add sending of the X-Bot-Secret header from the Telegram bot * fix(config): Remove os.environ, which causes an error in tests * Update comment src/presentation/api/v1/auth/tg_link.py Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> * docs(services): Fix logging and doc string errors --------- Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
* feat(docker): Isolate services by separating docker networks * feat(docker): Add a system user app. Remove buffering and .pyc, __pycache__ files. * feat(docker): Remove root privileges. Add cap_drop to disable root privileges. Enable write access to the /tmp folder. * feat(logger): Remove logging to a file * feat(docker): Write a user creation command in the api container and transfer the user change to the bot container * docs: Correct syntax errors --------- Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
* feat: Add a health endpoint to check if the API is online * feat(docker): Add cleaning of outdated Docker images * fix: Change your HealthCheck internet connection to avoid being blocked by RateLimiting --------- Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
#29) * feat: Bring the architecture to a clean archeticture. In the structure, repeat the project structure for easier navigation. Separate the models into different files. * fix(models): Fix circular import * fix(models): Fix circular import * fix(models): correct error texts in models --------- Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
* fix: Delete alembic.ini * chore: add alembic.ini into .gitignore * chore: update alembic scheme * fix: Remove the backup key from config.py * fix: Fix the SECRET_KEY test environment variable * fix(auth): Fixing a missing SECRET_KEY error in .env * fix(tests): Fix E401 [*] Multiple imports on one line * feat(config): Add a check for the absence of DATABSE_URL in .env * Add a database engine to lifespan's fastapi * refactor: Move session factory to lifespan --------- Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
Contributor
Руководство для ревьюера (свёрнуто для небольших PR)Руководство для ревьюераКорректирует исходную миграцию Alembic, чтобы упростить условие уникальности для активного бана и корректно определить enum Диаграмма сущностей для обновлённого индекса банов и enum user_roleerDiagram
Users {
int id PK
enum user_role "default: user"
}
Bans {
int id PK
int user_id FK
timestamp revoked_at "uq_active_ban: user_id WHERE revoked_at IS NULL"
}
Users ||--o{ Bans : has_bans
Изменения по файлам
Подсказки и командыВзаимодействие с Sourcery
Настройка работыЗайдите в свою панель управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the initial Alembic migration to simplify the active ban uniqueness condition and correctly define the user_role enum with a default value, while cleaning up unused service column alterations and Alembic boilerplate comments. Entity relationship diagram for updated bans index and user_role enumerDiagram
Users {
int id PK
enum user_role "default: user"
}
Bans {
int id PK
int user_id FK
timestamp revoked_at "uq_active_ban: user_id WHERE revoked_at IS NULL"
}
Users ||--o{ Bans : has_bans
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Coverage Report for CI Build 26832245115Coverage remained the same at 30.025%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Привет — я нашёл 1 проблему и оставил несколько общих замечаний:
- В миграции остался лишний комментарий
# hi; пожалуйста, уберите его, чтобы миграция оставалась аккуратной и сфокусированной. - Так как это начальная миграция, перепроверьте, что добавление именованного enum’а
user_roleсserver_default='user'согласовано с названием Enum и значением по умолчанию в модели SQLAlchemy, чтобы будущие миграции не генерировали лишние отличия.
Подсказка для AI-агентов
Please address the comments from this code review:
## Overall Comments
- There is a stray `# hi` comment left in the migration; please remove it to keep the migration clean and focused.
- Since this is an initial migration, double-check that introducing the named enum `user_role` with `server_default='user'` aligns with the SQLAlchemy model’s Enum name and default, so future migrations don’t generate unnecessary diffs.
## Individual Comments
### Comment 1
<location path="alembic/versions/4972fe91500d_v0_0_1.py" line_range="51" />
<code_context>
- op.add_column('users', sa.Column('user_role', sa.Enum('user', 'admin', 'moderator'), nullable=False))
+
+# hi
+ op.add_column('users', sa.Column('user_role', sa.Enum('user', 'admin', 'moderator', name='user_role'), nullable=False, server_default='user'))
op.alter_column('users', 'email',
existing_type=sa.VARCHAR(length=50),
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The named ENUM type `user_role` is created on upgrade but never dropped on downgrade.
Because `name='user_role'` is specified, Alembic/SQLAlchemy creates a concrete ENUM type in the DB. The downgrade currently only drops the column, leaving the ENUM type behind, which can conflict with future migrations that reuse `user_role` with different values.
Please also drop the ENUM type in `downgrade` (for example, `op.execute("DROP TYPE user_role")` after dropping the column, or the dialect-appropriate equivalent).
Suggested implementation:
```python
def downgrade() -> None:
# Drop the user_role column before dropping the ENUM type
op.drop_column('users', 'user_role')
# Drop the underlying ENUM type created in upgrade()
op.execute("DROP TYPE user_role")
```
If `downgrade()` already has other operations in this file, you should merge the `op.drop_column('users', 'user_role')` and `op.execute("DROP TYPE user_role")` calls into the existing body rather than replacing it entirely. They should appear in the correct reverse order relative to the `upgrade()` steps (i.e., after reverting indexes/constraints that depend on `user_role`, but before any operations that assume the type no longer exists).
</issue_to_address>Sourcery бесплатен для open source — если вам нравятся наши обзоры, пожалуйста, расскажите о нас ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- There is a stray
# hicomment left in the migration; please remove it to keep the migration clean and focused. - Since this is an initial migration, double-check that introducing the named enum
user_rolewithserver_default='user'aligns with the SQLAlchemy model’s Enum name and default, so future migrations don’t generate unnecessary diffs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There is a stray `# hi` comment left in the migration; please remove it to keep the migration clean and focused.
- Since this is an initial migration, double-check that introducing the named enum `user_role` with `server_default='user'` aligns with the SQLAlchemy model’s Enum name and default, so future migrations don’t generate unnecessary diffs.
## Individual Comments
### Comment 1
<location path="alembic/versions/4972fe91500d_v0_0_1.py" line_range="51" />
<code_context>
- op.add_column('users', sa.Column('user_role', sa.Enum('user', 'admin', 'moderator'), nullable=False))
+
+# hi
+ op.add_column('users', sa.Column('user_role', sa.Enum('user', 'admin', 'moderator', name='user_role'), nullable=False, server_default='user'))
op.alter_column('users', 'email',
existing_type=sa.VARCHAR(length=50),
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The named ENUM type `user_role` is created on upgrade but never dropped on downgrade.
Because `name='user_role'` is specified, Alembic/SQLAlchemy creates a concrete ENUM type in the DB. The downgrade currently only drops the column, leaving the ENUM type behind, which can conflict with future migrations that reuse `user_role` with different values.
Please also drop the ENUM type in `downgrade` (for example, `op.execute("DROP TYPE user_role")` after dropping the column, or the dialect-appropriate equivalent).
Suggested implementation:
```python
def downgrade() -> None:
# Drop the user_role column before dropping the ENUM type
op.drop_column('users', 'user_role')
# Drop the underlying ENUM type created in upgrade()
op.execute("DROP TYPE user_role")
```
If `downgrade()` already has other operations in this file, you should merge the `op.drop_column('users', 'user_role')` and `op.execute("DROP TYPE user_role")` calls into the existing body rather than replacing it entirely. They should appear in the correct reverse order relative to the `upgrade()` steps (i.e., after reverting indexes/constraints that depend on `user_role`, but before any operations that assume the type no longer exists).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| op.add_column('users', sa.Column('user_role', sa.Enum('user', 'admin', 'moderator'), nullable=False)) | ||
|
|
||
| # hi | ||
| op.add_column('users', sa.Column('user_role', sa.Enum('user', 'admin', 'moderator', name='user_role'), nullable=False, server_default='user')) |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Именованный тип ENUM user_role создаётся при upgrade, но не удаляется при downgrade.
Поскольку указано name='user_role', Alembic/SQLAlchemy создаёт конкретный тип ENUM в базе данных. Текущая реализация downgrade удаляет только столбец, оставляя тип ENUM, что может привести к конфликтам с будущими миграциями, которые повторно используют user_role с другим набором значений.
Пожалуйста, также удалите тип ENUM в downgrade (например, с помощью op.execute("DROP TYPE user_role") после удаления столбца, либо эквивалента для используемого диалекта).
Предлагаемая реализация:
def downgrade() -> None:
# Drop the user_role column before dropping the ENUM type
op.drop_column('users', 'user_role')
# Drop the underlying ENUM type created in upgrade()
op.execute("DROP TYPE user_role")Если в downgrade() уже есть другие операции в этом файле, вам следует добавить вызовы op.drop_column('users', 'user_role') и op.execute("DROP TYPE user_role") в существующее тело функции, а не заменять его целиком. Эти операции должны идти в корректном обратном порядке по отношению к шагам upgrade() (то есть после отката индексов/ограничений, зависящих от user_role, но до любых операций, которые предполагают, что этот тип больше не существует).
Original comment in English
suggestion (bug_risk): The named ENUM type user_role is created on upgrade but never dropped on downgrade.
Because name='user_role' is specified, Alembic/SQLAlchemy creates a concrete ENUM type in the DB. The downgrade currently only drops the column, leaving the ENUM type behind, which can conflict with future migrations that reuse user_role with different values.
Please also drop the ENUM type in downgrade (for example, op.execute("DROP TYPE user_role") after dropping the column, or the dialect-appropriate equivalent).
Suggested implementation:
def downgrade() -> None:
# Drop the user_role column before dropping the ENUM type
op.drop_column('users', 'user_role')
# Drop the underlying ENUM type created in upgrade()
op.execute("DROP TYPE user_role")If downgrade() already has other operations in this file, you should merge the op.drop_column('users', 'user_role') and op.execute("DROP TYPE user_role") calls into the existing body rather than replacing it entirely. They should appear in the correct reverse order relative to the upgrade() steps (i.e., after reverting indexes/constraints that depend on user_role, but before any operations that assume the type no longer exists).
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
2 participants
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
Адаптировать начальную миграцию базы данных, чтобы схема банов и пользовательских ролей соответствовала текущей модели приложения.
Исправления ошибок:
user_roleв начальной миграции и обеспечить установку роли пользователя по умолчанию.Улучшения:
Original summary in English
Summary by Sourcery
Adjust initial database migration to align bans and user role schema with current application model.
Bug Fixes:
Enhancements:
Новые возможности:
Исправления ошибок:
user_roleв начальной миграции и обеспечена установка роли по умолчанию.Улучшения:
AsyncIOSchedulerв lifespan FastAPI с дополнительным заданием для очистки просроченных банов.SessionLocalв инфраструктурном слое вместо состояния приложения.Original summary in English
Summary by Sourcery
Адаптировать начальную миграцию базы данных, чтобы схема банов и пользовательских ролей соответствовала текущей модели приложения.
Исправления ошибок:
user_roleв начальной миграции и обеспечить установку роли пользователя по умолчанию.Улучшения:
Original summary in English
Summary by Sourcery
Adjust initial database migration to align bans and user role schema with current application model.
Bug Fixes:
Enhancements: