refactor_accounts #17

Merged
arthur.wambst merged 6 commits from refactor_accounts into dev 2026-07-23 12:42:41 +02:00
No description provided.
Author
Owner

1er truc a faire : fix les serialisation avec des DTO, car actuellement ca fait des loops infinie

1er truc a faire : fix les serialisation avec des DTO, car actuellement ca fait des loops infinie
arthur.wambst changed title from refactor_accounts to WIP: refactor_accounts 2026-07-08 20:21:06 +02:00
@ -0,0 +14,4 @@
@Column(nullable = false) var iban: String = ""
@OneToMany(mappedBy = "bankAccount", cascade = [CascadeType.ALL], orphanRemoval = true)
var transactions: MutableList<Transaction> = ArrayList()
Owner
var transactions: MutableList<Transaction> = mutableListOf()
```kotlin var transactions: MutableList<Transaction> = mutableListOf() ```
mateo.ziegler marked this conversation as resolved
arthur.wambst force-pushed refactor_accounts from 56de0d97d3 to 8c8ab4ee86 2026-07-08 21:47:10 +02:00 Compare
@ -0,0 +8,4 @@
@Table(name = "bank_accounts")
class BankAccount {
@Id @GeneratedValue(strategy = GenerationType.IDENTITY) var id: Long? = null
@Column(nullable = false, length = 100) var name: String = "New Bank Account"

why not string vide ?

why not string vide ?
mateo.ziegler marked this conversation as resolved
@ -0,0 +14,4 @@
@Column(nullable = false) var iban: String = ""
@OneToMany(mappedBy = "bankAccount", cascade = [CascadeType.ALL], orphanRemoval = true)
var transactions: MutableList<Transaction> = ArrayList()

mutableListOf()

mutableListOf()
mateo.ziegler marked this conversation as resolved
@ -0,0 +8,4 @@
@Table(name = "sub_accounts")
class SubAccount {
@Id @GeneratedValue(strategy = GenerationType.IDENTITY) var id: Long? = null
@Column(nullable = false, length = 100) var name: String = "New Sub Bank Account"

meme chose que pour BankAccount.kt

meme chose que pour BankAccount.kt
mateo.ziegler marked this conversation as resolved
@ -0,0 +12,4 @@
@ApplicationScoped
class BankAccountRepository : PanacheRepository<BankAccount> {
fun getBankAccount(accountId: Long): Optional<BankAccount> {

why optional et pas BankAccount?

why optional et pas `BankAccount?`
Author
Owner

j prefere, les optionnal rende le code + lisible puis ca rend les erreurs + propres que "null ptr", sans que ca perde specialement de perf
apres j suis pas non plus pret a defendre les optionnal a mort hein, ca a juste l'air + propre

j prefere, les optionnal rende le code + lisible puis ca rend les erreurs + propres que "null ptr", sans que ca perde specialement de perf apres j suis pas non plus pret a defendre les optionnal a mort hein, ca a juste l'air + propre
Author
Owner

en fait mb vaut juste enelver le optionnal/null, vaut juste mettre le type de retour et on throw une erreur dans la fonction qui appelle, mais oui faut pas se trainer d optionnal / de null

en fait mb vaut juste enelver le optionnal/null, vaut juste mettre le type de retour et on throw une erreur dans la fonction qui appelle, mais oui faut pas se trainer d optionnal / de null
arthur.wambst marked this conversation as resolved
@ -0,0 +20,4 @@
// TODO : Check for null and/or duplicates
account.transactions.add(transaction)
persist(account)
return true

sartek le return True, return rien ou check les erreurs

sartek le return True, return rien ou check les erreurs
Author
Owner

oui vaut mieux pas return

oui vaut mieux pas return
mateo.ziegler marked this conversation as resolved
@ -0,0 +19,4 @@
fun createAccount(request: NewBankAccountRequest): BankAccount {
// TODO list :
// - Check for null and/or duplicates
// - Validate iban

cf. la rfc qui concerne les ibans (ou ISO jsp)

cf. la rfc qui concerne les ibans (ou ISO jsp)
Author
Owner

la rfc pour les iban ?

la rfc pour les iban ?
arthur.wambst marked this conversation as resolved
@ -0,0 +21,4 @@
// - Check for null and/or duplicates
// - Validate iban
val newAccount = BankAccount().apply { name = request.name; iban = request.iban };

tu va pas link les subaccount ?

tu va pas link les subaccount ?
Author
Owner

si ?
pas sur de comprendre la question

si ? pas sur de comprendre la question
arthur.wambst marked this conversation as resolved
@ -0,0 +28,4 @@
}
fun getAllAccounts(): List<BankAccount> {
return bankAccountRepository.findAll().list<BankAccount>()

move to le repo sah

move to le repo sah
Author
Owner

oui mais faut penser a garder la fonction exposee dans service car on en a besoin dans l api

oui mais faut penser a garder la fonction exposee dans service car on en a besoin dans l api
arthur.wambst marked this conversation as resolved
@ -0,0 +32,4 @@
}
fun getAccount(accountId: Long): BankAccount {
val account : Optional<BankAccount> = bankAccountRepository.getBankAccount(accountId)

getAccount et getBackAccount faut se mettre d'accord sur le naming, encore une fois why les optional

getAccount et getBackAccount faut se mettre d'accord sur le naming, encore une fois why les optional
Author
Owner

go sur getBankAccount

go sur `getBankAccount`
mateo.ziegler marked this conversation as resolved
@ -0,0 +40,4 @@
}
fun getAllSubAccounts(accountId: Long): List<SubAccount> {
return getAccount(accountId).subAccounts

maybe move to repo ?

maybe move to repo ?
Author
Owner

ba non on en a besoin dans rest

ba non on en a besoin dans rest
arthur.wambst marked this conversation as resolved
@ -0,0 +48,4 @@
if (account.isEmpty) {
throw Error("Account with id $accountId does not exist");
}
return account.get().transactions.map { transaction -> transaction.amount }.sumOf { i -> i }

vaut pas mieux stocker la valeur plutot que sum toutes les transa ?

vaut pas mieux stocker la valeur plutot que sum toutes les transa ?
Author
Owner

apres avoir pas mal bidouille avec c est assez foireux, vaut mieux sum quand un appel est fait sur un /account/sum/ de l api puis cache le result tant que y a pas de nouvelles transactions (a voir comment ca marche plus tard)

apres avoir pas mal bidouille avec c est assez foireux, vaut mieux sum quand un appel est fait sur un `/account/sum/` de l api puis cache le result tant que y a pas de nouvelles transactions (a voir comment ca marche plus tard)
arthur.wambst marked this conversation as resolved
@ -0,0 +56,4 @@
// TODO : Check for null and/or duplicates
account.subAccounts.add(subAccount)
persist(account)
return true

return true sans check d'erreur sympa

return true sans check d'erreur sympa
mateo.ziegler marked this conversation as resolved
@ -0,0 +64,4 @@
// TODO : Check for null and/or duplicates
account.transactions.add(transaction)
persist(account)
return true

pareil

pareil
mateo.ziegler marked this conversation as resolved
@ -0,0 +17,4 @@
) {
@Transactional
fun createAccount(request: NewSubAccountRequest): BankAccountResponse {

c'est le meme code a peu de chose pres que pour les parent, tu peux pas mutualisé ?

c'est le meme code a peu de chose pres que pour les parent, tu peux pas mutualisé ?
Author
Owner

nop, c est vraiment la v0 des account donc ils ont pas bcp de diffs pour le moment mais y ne aura d autres tres bientot, puis le but de la nouvelle "archi" avec 2 types de comptes, c est d avoir 2 objects bien disctinct en separes autremenet que par un booleen

nop, c est vraiment la v0 des account donc ils ont pas bcp de diffs pour le moment mais y ne aura d autres tres bientot, puis le but de la nouvelle "archi" avec 2 types de comptes, c est d avoir 2 objects bien disctinct en separes autremenet que par un booleen
arthur.wambst marked this conversation as resolved
@ -0,0 +16,4 @@
import jakarta.ws.rs.core.Response
import java.util.Optional
@Path("/api/bank")

/api/bank et /api/sub ???? relou un peu

/api/bank et /api/sub ???? relou un peu

c'est pas le prime de la naming convention

c'est pas le prime de la naming convention
mateo.ziegler marked this conversation as resolved
@ -0,0 +37,4 @@
@GET
@RolesAllowed("\${roles-admin}") // Todo: que les admin d un compte peuvent y acceder
@Path("/account/")
fun getAllBankAccounts(

ca inclut les sub ou pas ?

ca inclut les sub ou pas ?

nan c'est bon j'ai eu ma reponse

nan c'est bon j'ai eu ma reponse
arthur.wambst marked this conversation as resolved
@ -0,0 +10,4 @@
val id: Long,
val name: String,
val createdAt: LocalDateTime,
val updatedAt: LocalDateTime,

ca sert a rien de renvoyer createdAt, updatedAt et allowedRols ?

ca sert a rien de renvoyer createdAt, updatedAt et allowedRols ?
mateo.ziegler marked this conversation as resolved
@ -0,0 +24,4 @@
createdAt = createdAt,
updatedAt = updatedAt,
iban = iban,
transactionsId = transactions.map { it.id!! }.toMutableList(),

sur un get bank account, ya pas besoin d'inclure les transactions, imo c'est mieux si ca va sur un get /transaction/

sur un get bank account, ya pas besoin d'inclure les transactions, imo c'est mieux si ca va sur un get /transaction/
mateo.ziegler marked this conversation as resolved
@ -0,0 +12,4 @@
val updatedAt: LocalDateTime,
val transactionsId: MutableList<Long>,
val bankAccountId: Long,
val allowedGroups: Set<String>

same remarque as au dessus

same remarque as au dessus
mateo.ziegler marked this conversation as resolved
arthur.wambst changed title from WIP: refactor_accounts to refactor_accounts 2026-07-23 12:41:58 +02:00
arthur.wambst deleted branch refactor_accounts 2026-07-23 12:42:42 +02:00
arthur.wambst referenced this pull request from a commit 2026-07-23 12:42:43 +02:00
Sign in to join this conversation.
No description provided.