-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/new account #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/new account #10
Conversation
…t and enhance migration script generation
…ieval, and transaction handling
… enhance account management
…iled messages and failure reasons
…streamline default account assignment
There was a problem hiding this 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 implements a comprehensive account system for the transaction API, refactoring the codebase from a user-based to an account-based transaction model. It introduces multi-account support per user, adds database migrations for the new account structure, and reorganizes networking packets.
Key changes:
- Introduces account-based transactions replacing user-based transactions
- Adds multi-account support with default account functionality
- Implements database migrations to support the new account structure
Reviewed Changes
Copilot reviewed 91 out of 93 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| surf-transaction-api/src/main/kotlin/dev/slne/surf/transaction/api/account/*.kt | New account API interfaces, data classes, and serialization components |
| surf-transaction-api/src/main/kotlin/dev/slne/surf/transaction/api/user/TransactionUser.kt | Refactored user interface to work with accounts |
| surf-transaction-server/src/main/kotlin/dev/slne/surf/transaction/server/account/*.kt | Server-side account management implementation |
| surf-transaction-server/src/main/resources/db/migration/*.sql | Database migrations for account tables and foreign key constraints |
| surf-transaction-core/.../netty/packets/ | Reorganized and updated network packets for account-based operations |
| surf-transaction-paper/src/main/kotlin/.../commands/account/*.kt | New account management commands for Paper plugin |
| * @return The [AccountEntity] if found, or null if not found. | ||
| */ | ||
| suspend fun fetchByAccountName(name: String): AccountEntity? = | ||
| AccountEntity.find { AccountTable.name like name }.singleOrNull() |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using 'like' operator for exact name matching is incorrect and could lead to unintended matches. Use 'eq' operator instead for exact string comparison.
| AccountEntity.find { AccountTable.name like name }.singleOrNull() | |
| AccountEntity.find { AccountTable.name eq name }.singleOrNull() |
| ): BigDecimal { | ||
| val account = accountRepository.fetchByAccountId(accountId) | ||
|
|
||
| return TransactionTable |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function returns before executing the query. The return statement should be after the query execution block, not before it.
| this.accountId = UUID.randomUUID() | ||
| this.name = name | ||
| this.defaultAccount = defaultAccount | ||
| }.toApi() |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an account with the same name already exists, this logic sets it as default without clearing the previous default account, potentially creating multiple default accounts.
| }.toApi() | |
| val accountByName = fetchByAccountName(name) | |
| if (accountByName != null) { | |
| if (defaultAccount) { | |
| clearDefaultAccountForOwner(owner, exceptAccountId = accountByName.accountId) | |
| accountByName.defaultAccount = true | |
| } | |
| return accountByName.toApi() | |
| } | |
| val newAccount = AccountEntity.new { | |
| this.owner = owner.uuid | |
| this.accountId = UUID.randomUUID() | |
| this.name = name | |
| this.defaultAccount = defaultAccount | |
| } | |
| if (defaultAccount) { | |
| clearDefaultAccountForOwner(owner, exceptAccountId = newAccount.accountId) | |
| } | |
| return newAccount.toApi() |
| identifier = identifier, | ||
| senderUuid = sender, | ||
| receiverUuid = receiver, | ||
| initiator = initiator?.toOfflineCloudPlayer(), |
Copilot
AI
Aug 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The property 'initiator' is already of type UUID?, but toOfflineCloudPlayer() is being called on it, which should be called on the UUID value directly.
No description provided.