Skip to content

Conversation

@RamyHakam
Copy link
Owner

No description provided.

@RamyHakam RamyHakam requested a review from Copilot September 19, 2025 23:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where the migration command was forcing the use of getId() instead of the configured tenant identifier field. The key changes update the system to properly use the configured identifier value throughout the migration process.

  • Updates the system to use getIdentifierValue() method instead of hardcoded getId() for tenant identification
  • Changes parameter types from int to mixed to support various identifier types
  • Adds deprecation warning for the dbId argument in the migration command

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Services/TenantDbConfigurationInterface.php Adds getIdentifierValue() method to interface for retrieving configured identifier
src/Config/TenantConnectionConfigDTO.php Changes identifier parameter type from ?int to mixed
src/Adapter/Doctrine/DoctrineTenantDatabaseManager.php Updates method signatures to use mixed identifiers and switches to getIdentifierValue()
src/Command/MigrateCommand.php Adds single database migration logic with deprecation warning
tests/Unit/EventListener/DbSwitchEventListenerTest.php Implements getIdentifierValue() method in test class

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@RamyHakam RamyHakam self-assigned this Sep 20, 2025
@RamyHakam RamyHakam added the bug-fix PR to fix an issue label Sep 20, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@RamyHakam RamyHakam merged commit c03114d into master Sep 20, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR to fix an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant