mirror of
https://github.com/elisiariocouto/leggen.git
synced 2025-12-13 11:22:21 +00:00
Implement display_name field with migration and API support
Co-authored-by: elisiariocouto <818914+elisiariocouto@users.noreply.github.com>
This commit is contained in:
committed by
Elisiário Couto
parent
3352e110b8
commit
33a7ad5ad2
@@ -81,8 +81,8 @@ export default function AccountsOverview() {
|
|||||||
const queryClient = useQueryClient();
|
const queryClient = useQueryClient();
|
||||||
|
|
||||||
const updateAccountMutation = useMutation({
|
const updateAccountMutation = useMutation({
|
||||||
mutationFn: ({ id, name }: { id: string; name: string }) =>
|
mutationFn: ({ id, display_name }: { id: string; display_name: string }) =>
|
||||||
apiClient.updateAccount(id, { name }),
|
apiClient.updateAccount(id, { display_name }),
|
||||||
onSuccess: () => {
|
onSuccess: () => {
|
||||||
queryClient.invalidateQueries({ queryKey: ["accounts"] });
|
queryClient.invalidateQueries({ queryKey: ["accounts"] });
|
||||||
setEditingAccountId(null);
|
setEditingAccountId(null);
|
||||||
@@ -95,14 +95,15 @@ export default function AccountsOverview() {
|
|||||||
|
|
||||||
const handleEditStart = (account: Account) => {
|
const handleEditStart = (account: Account) => {
|
||||||
setEditingAccountId(account.id);
|
setEditingAccountId(account.id);
|
||||||
setEditingName(account.name || "");
|
// Use display_name if available, otherwise fall back to name
|
||||||
|
setEditingName(account.display_name || account.name || "");
|
||||||
};
|
};
|
||||||
|
|
||||||
const handleEditSave = () => {
|
const handleEditSave = () => {
|
||||||
if (editingAccountId && editingName.trim()) {
|
if (editingAccountId && editingName.trim()) {
|
||||||
updateAccountMutation.mutate({
|
updateAccountMutation.mutate({
|
||||||
id: editingAccountId,
|
id: editingAccountId,
|
||||||
name: editingName.trim(),
|
display_name: editingName.trim(),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@@ -267,7 +268,7 @@ export default function AccountsOverview() {
|
|||||||
setEditingName(e.target.value)
|
setEditingName(e.target.value)
|
||||||
}
|
}
|
||||||
className="flex-1 px-3 py-1 text-base sm:text-lg font-medium border border-input rounded-md bg-background focus:outline-none focus:ring-2 focus:ring-ring focus:border-ring"
|
className="flex-1 px-3 py-1 text-base sm:text-lg font-medium border border-input rounded-md bg-background focus:outline-none focus:ring-2 focus:ring-ring focus:border-ring"
|
||||||
placeholder="Account name"
|
placeholder="Custom account name"
|
||||||
name="search"
|
name="search"
|
||||||
autoComplete="off"
|
autoComplete="off"
|
||||||
onKeyDown={(e) => {
|
onKeyDown={(e) => {
|
||||||
@@ -303,7 +304,7 @@ export default function AccountsOverview() {
|
|||||||
<div>
|
<div>
|
||||||
<div className="flex items-center space-x-2 min-w-0">
|
<div className="flex items-center space-x-2 min-w-0">
|
||||||
<h4 className="text-base sm:text-lg font-medium text-foreground truncate">
|
<h4 className="text-base sm:text-lg font-medium text-foreground truncate">
|
||||||
{account.name || "Unnamed Account"}
|
{account.display_name || account.name || "Unnamed Account"}
|
||||||
</h4>
|
</h4>
|
||||||
<button
|
<button
|
||||||
onClick={() => handleEditStart(account)}
|
onClick={() => handleEditStart(account)}
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ export function AccountCombobox({
|
|||||||
);
|
);
|
||||||
|
|
||||||
const formatAccountName = (account: Account) => {
|
const formatAccountName = (account: Account) => {
|
||||||
const displayName = account.name || "Unnamed Account";
|
const displayName = account.display_name || account.name || "Unnamed Account";
|
||||||
return `${displayName} (${account.institution_id})`;
|
return `${displayName} (${account.institution_id})`;
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -89,7 +89,7 @@ export function AccountCombobox({
|
|||||||
{accounts.map((account) => (
|
{accounts.map((account) => (
|
||||||
<CommandItem
|
<CommandItem
|
||||||
key={account.id}
|
key={account.id}
|
||||||
value={`${account.name} ${account.institution_id}`}
|
value={`${account.display_name || account.name} ${account.institution_id}`}
|
||||||
onSelect={() => {
|
onSelect={() => {
|
||||||
onAccountChange(account.id);
|
onAccountChange(account.id);
|
||||||
setOpen(false);
|
setOpen(false);
|
||||||
@@ -105,7 +105,7 @@ export function AccountCombobox({
|
|||||||
/>
|
/>
|
||||||
<div className="flex flex-col">
|
<div className="flex flex-col">
|
||||||
<span className="font-medium">
|
<span className="font-medium">
|
||||||
{account.name || "Unnamed Account"}
|
{account.display_name || account.name || "Unnamed Account"}
|
||||||
</span>
|
</span>
|
||||||
<span className="text-xs text-gray-500">
|
<span className="text-xs text-gray-500">
|
||||||
{account.institution_id}
|
{account.institution_id}
|
||||||
|
|||||||
@@ -41,8 +41,8 @@ export const apiClient = {
|
|||||||
updateAccount: async (
|
updateAccount: async (
|
||||||
id: string,
|
id: string,
|
||||||
updates: AccountUpdate,
|
updates: AccountUpdate,
|
||||||
): Promise<{ id: string; name?: string }> => {
|
): Promise<{ id: string; display_name?: string }> => {
|
||||||
const response = await api.put<ApiResponse<{ id: string; name?: string }>>(
|
const response = await api.put<ApiResponse<{ id: string; display_name?: string }>>(
|
||||||
`/accounts/${id}`,
|
`/accounts/${id}`,
|
||||||
updates,
|
updates,
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ export interface Account {
|
|||||||
status: string;
|
status: string;
|
||||||
iban?: string;
|
iban?: string;
|
||||||
name?: string;
|
name?: string;
|
||||||
|
display_name?: string;
|
||||||
currency?: string;
|
currency?: string;
|
||||||
created: string;
|
created: string;
|
||||||
last_accessed?: string;
|
last_accessed?: string;
|
||||||
@@ -18,7 +19,7 @@ export interface Account {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export interface AccountUpdate {
|
export interface AccountUpdate {
|
||||||
name?: string;
|
display_name?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface RawTransactionData {
|
export interface RawTransactionData {
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ class AccountDetails(BaseModel):
|
|||||||
status: str
|
status: str
|
||||||
iban: Optional[str] = None
|
iban: Optional[str] = None
|
||||||
name: Optional[str] = None
|
name: Optional[str] = None
|
||||||
|
display_name: Optional[str] = None
|
||||||
currency: Optional[str] = None
|
currency: Optional[str] = None
|
||||||
created: datetime
|
created: datetime
|
||||||
last_accessed: Optional[datetime] = None
|
last_accessed: Optional[datetime] = None
|
||||||
@@ -36,7 +37,7 @@ class AccountDetails(BaseModel):
|
|||||||
class AccountUpdate(BaseModel):
|
class AccountUpdate(BaseModel):
|
||||||
"""Account update model"""
|
"""Account update model"""
|
||||||
|
|
||||||
name: Optional[str] = None
|
display_name: Optional[str] = None
|
||||||
|
|
||||||
class Config:
|
class Config:
|
||||||
json_encoders = {datetime: lambda v: v.isoformat() if v else None}
|
json_encoders = {datetime: lambda v: v.isoformat() if v else None}
|
||||||
|
|||||||
@@ -53,6 +53,7 @@ async def get_all_accounts() -> APIResponse:
|
|||||||
status=db_account["status"],
|
status=db_account["status"],
|
||||||
iban=db_account.get("iban"),
|
iban=db_account.get("iban"),
|
||||||
name=db_account.get("name"),
|
name=db_account.get("name"),
|
||||||
|
display_name=db_account.get("display_name"),
|
||||||
currency=db_account.get("currency"),
|
currency=db_account.get("currency"),
|
||||||
created=db_account["created"],
|
created=db_account["created"],
|
||||||
last_accessed=db_account.get("last_accessed"),
|
last_accessed=db_account.get("last_accessed"),
|
||||||
@@ -112,6 +113,7 @@ async def get_account_details(account_id: str) -> APIResponse:
|
|||||||
status=db_account["status"],
|
status=db_account["status"],
|
||||||
iban=db_account.get("iban"),
|
iban=db_account.get("iban"),
|
||||||
name=db_account.get("name"),
|
name=db_account.get("name"),
|
||||||
|
display_name=db_account.get("display_name"),
|
||||||
currency=db_account.get("currency"),
|
currency=db_account.get("currency"),
|
||||||
created=db_account["created"],
|
created=db_account["created"],
|
||||||
last_accessed=db_account.get("last_accessed"),
|
last_accessed=db_account.get("last_accessed"),
|
||||||
@@ -324,7 +326,7 @@ async def get_account_transactions(
|
|||||||
async def update_account_details(
|
async def update_account_details(
|
||||||
account_id: str, update_data: AccountUpdate
|
account_id: str, update_data: AccountUpdate
|
||||||
) -> APIResponse:
|
) -> APIResponse:
|
||||||
"""Update account details (currently only name)"""
|
"""Update account details (currently only display_name)"""
|
||||||
try:
|
try:
|
||||||
# Get current account details
|
# Get current account details
|
||||||
current_account = await database_service.get_account_details_from_db(account_id)
|
current_account = await database_service.get_account_details_from_db(account_id)
|
||||||
@@ -336,16 +338,16 @@ async def update_account_details(
|
|||||||
|
|
||||||
# Prepare updated account data
|
# Prepare updated account data
|
||||||
updated_account_data = current_account.copy()
|
updated_account_data = current_account.copy()
|
||||||
if update_data.name is not None:
|
if update_data.display_name is not None:
|
||||||
updated_account_data["name"] = update_data.name
|
updated_account_data["display_name"] = update_data.display_name
|
||||||
|
|
||||||
# Persist updated account details
|
# Persist updated account details
|
||||||
await database_service.persist_account_details(updated_account_data)
|
await database_service.persist_account_details(updated_account_data)
|
||||||
|
|
||||||
return APIResponse(
|
return APIResponse(
|
||||||
success=True,
|
success=True,
|
||||||
data={"id": account_id, "name": update_data.name},
|
data={"id": account_id, "display_name": update_data.display_name},
|
||||||
message=f"Account {account_id} name updated successfully",
|
message=f"Account {account_id} display name updated successfully",
|
||||||
)
|
)
|
||||||
|
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
|
|||||||
@@ -215,6 +215,7 @@ class DatabaseService:
|
|||||||
await self._migrate_balance_timestamps_if_needed()
|
await self._migrate_balance_timestamps_if_needed()
|
||||||
await self._migrate_null_transaction_ids_if_needed()
|
await self._migrate_null_transaction_ids_if_needed()
|
||||||
await self._migrate_to_composite_key_if_needed()
|
await self._migrate_to_composite_key_if_needed()
|
||||||
|
await self._migrate_add_display_name_if_needed()
|
||||||
|
|
||||||
async def _migrate_balance_timestamps_if_needed(self):
|
async def _migrate_balance_timestamps_if_needed(self):
|
||||||
"""Check and migrate balance timestamps if needed"""
|
"""Check and migrate balance timestamps if needed"""
|
||||||
@@ -632,6 +633,79 @@ class DatabaseService:
|
|||||||
logger.error(f"Composite key migration failed: {e}")
|
logger.error(f"Composite key migration failed: {e}")
|
||||||
raise
|
raise
|
||||||
|
|
||||||
|
async def _migrate_add_display_name_if_needed(self):
|
||||||
|
"""Check and add display_name column to accounts table if needed"""
|
||||||
|
try:
|
||||||
|
if await self._check_display_name_migration_needed():
|
||||||
|
logger.info("Display name column migration needed, starting...")
|
||||||
|
await self._migrate_add_display_name()
|
||||||
|
logger.info("Display name column migration completed")
|
||||||
|
else:
|
||||||
|
logger.info("Display name column already exists")
|
||||||
|
except Exception as e:
|
||||||
|
logger.error(f"Display name column migration failed: {e}")
|
||||||
|
raise
|
||||||
|
|
||||||
|
async def _check_display_name_migration_needed(self) -> bool:
|
||||||
|
"""Check if display_name column needs to be added to accounts table"""
|
||||||
|
db_path = path_manager.get_database_path()
|
||||||
|
if not db_path.exists():
|
||||||
|
return False
|
||||||
|
|
||||||
|
try:
|
||||||
|
conn = sqlite3.connect(str(db_path))
|
||||||
|
cursor = conn.cursor()
|
||||||
|
|
||||||
|
# Check if accounts table exists
|
||||||
|
cursor.execute(
|
||||||
|
"SELECT name FROM sqlite_master WHERE type='table' AND name='accounts'"
|
||||||
|
)
|
||||||
|
if not cursor.fetchone():
|
||||||
|
conn.close()
|
||||||
|
return False
|
||||||
|
|
||||||
|
# Check if display_name column exists
|
||||||
|
cursor.execute("PRAGMA table_info(accounts)")
|
||||||
|
columns = cursor.fetchall()
|
||||||
|
|
||||||
|
# Check if display_name column exists
|
||||||
|
has_display_name = any(col[1] == "display_name" for col in columns)
|
||||||
|
|
||||||
|
conn.close()
|
||||||
|
return not has_display_name
|
||||||
|
|
||||||
|
except Exception as e:
|
||||||
|
logger.error(f"Failed to check display_name migration status: {e}")
|
||||||
|
return False
|
||||||
|
|
||||||
|
async def _migrate_add_display_name(self):
|
||||||
|
"""Add display_name column to accounts table"""
|
||||||
|
db_path = path_manager.get_database_path()
|
||||||
|
if not db_path.exists():
|
||||||
|
logger.warning("Database file not found, skipping migration")
|
||||||
|
return
|
||||||
|
|
||||||
|
try:
|
||||||
|
conn = sqlite3.connect(str(db_path))
|
||||||
|
cursor = conn.cursor()
|
||||||
|
|
||||||
|
logger.info("Adding display_name column to accounts table...")
|
||||||
|
|
||||||
|
# Add the display_name column
|
||||||
|
cursor.execute("""
|
||||||
|
ALTER TABLE accounts
|
||||||
|
ADD COLUMN display_name TEXT
|
||||||
|
""")
|
||||||
|
|
||||||
|
conn.commit()
|
||||||
|
conn.close()
|
||||||
|
|
||||||
|
logger.info("Display name column migration completed successfully")
|
||||||
|
|
||||||
|
except Exception as e:
|
||||||
|
logger.error(f"Display name column migration failed: {e}")
|
||||||
|
raise
|
||||||
|
|
||||||
def _unix_to_datetime_string(self, unix_timestamp: float) -> str:
|
def _unix_to_datetime_string(self, unix_timestamp: float) -> str:
|
||||||
"""Convert Unix timestamp to datetime string"""
|
"""Convert Unix timestamp to datetime string"""
|
||||||
dt = datetime.fromtimestamp(unix_timestamp)
|
dt = datetime.fromtimestamp(unix_timestamp)
|
||||||
@@ -1045,7 +1119,8 @@ class DatabaseService:
|
|||||||
currency TEXT,
|
currency TEXT,
|
||||||
created DATETIME,
|
created DATETIME,
|
||||||
last_accessed DATETIME,
|
last_accessed DATETIME,
|
||||||
last_updated DATETIME
|
last_updated DATETIME,
|
||||||
|
display_name TEXT
|
||||||
)"""
|
)"""
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -1060,6 +1135,16 @@ class DatabaseService:
|
|||||||
)
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
# First, check if account exists and preserve display_name
|
||||||
|
cursor.execute(
|
||||||
|
"SELECT display_name FROM accounts WHERE id = ?", (account_data["id"],)
|
||||||
|
)
|
||||||
|
existing_row = cursor.fetchone()
|
||||||
|
existing_display_name = existing_row[0] if existing_row else None
|
||||||
|
|
||||||
|
# Use existing display_name if not provided in account_data
|
||||||
|
display_name = account_data.get("display_name", existing_display_name)
|
||||||
|
|
||||||
# Insert or replace account data
|
# Insert or replace account data
|
||||||
cursor.execute(
|
cursor.execute(
|
||||||
"""INSERT OR REPLACE INTO accounts (
|
"""INSERT OR REPLACE INTO accounts (
|
||||||
@@ -1071,8 +1156,9 @@ class DatabaseService:
|
|||||||
currency,
|
currency,
|
||||||
created,
|
created,
|
||||||
last_accessed,
|
last_accessed,
|
||||||
last_updated
|
last_updated,
|
||||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
display_name
|
||||||
|
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||||
(
|
(
|
||||||
account_data["id"],
|
account_data["id"],
|
||||||
account_data["institution_id"],
|
account_data["institution_id"],
|
||||||
@@ -1083,6 +1169,7 @@ class DatabaseService:
|
|||||||
account_data["created"],
|
account_data["created"],
|
||||||
account_data.get("last_accessed"),
|
account_data.get("last_accessed"),
|
||||||
account_data.get("last_updated", account_data["created"]),
|
account_data.get("last_updated", account_data["created"]),
|
||||||
|
display_name,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
conn.commit()
|
conn.commit()
|
||||||
|
|||||||
@@ -106,7 +106,8 @@ class SampleDataGenerator:
|
|||||||
currency TEXT,
|
currency TEXT,
|
||||||
created DATETIME,
|
created DATETIME,
|
||||||
last_accessed DATETIME,
|
last_accessed DATETIME,
|
||||||
last_updated DATETIME
|
last_updated DATETIME,
|
||||||
|
display_name TEXT
|
||||||
)
|
)
|
||||||
""")
|
""")
|
||||||
|
|
||||||
@@ -373,8 +374,8 @@ class SampleDataGenerator:
|
|||||||
cursor.execute(
|
cursor.execute(
|
||||||
"""
|
"""
|
||||||
INSERT OR REPLACE INTO accounts
|
INSERT OR REPLACE INTO accounts
|
||||||
(id, institution_id, status, iban, name, currency, created, last_accessed, last_updated)
|
(id, institution_id, status, iban, name, currency, created, last_accessed, last_updated, display_name)
|
||||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)
|
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||||
""",
|
""",
|
||||||
(
|
(
|
||||||
account["id"],
|
account["id"],
|
||||||
@@ -386,6 +387,7 @@ class SampleDataGenerator:
|
|||||||
account["created"],
|
account["created"],
|
||||||
account["last_accessed"],
|
account["last_accessed"],
|
||||||
account["last_updated"],
|
account["last_updated"],
|
||||||
|
None, # display_name is initially None for sample data
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -24,6 +24,8 @@ class TestAccountsAPI:
|
|||||||
"institution_id": "REVOLUT_REVOLT21",
|
"institution_id": "REVOLUT_REVOLT21",
|
||||||
"status": "READY",
|
"status": "READY",
|
||||||
"iban": "LT313250081177977789",
|
"iban": "LT313250081177977789",
|
||||||
|
"name": "Personal Account",
|
||||||
|
"display_name": None,
|
||||||
"created": "2024-02-13T23:56:00Z",
|
"created": "2024-02-13T23:56:00Z",
|
||||||
"last_accessed": "2025-09-01T09:30:00Z",
|
"last_accessed": "2025-09-01T09:30:00Z",
|
||||||
}
|
}
|
||||||
@@ -80,6 +82,8 @@ class TestAccountsAPI:
|
|||||||
"institution_id": "REVOLUT_REVOLT21",
|
"institution_id": "REVOLUT_REVOLT21",
|
||||||
"status": "READY",
|
"status": "READY",
|
||||||
"iban": "LT313250081177977789",
|
"iban": "LT313250081177977789",
|
||||||
|
"name": "Personal Account",
|
||||||
|
"display_name": None,
|
||||||
"created": "2024-02-13T23:56:00Z",
|
"created": "2024-02-13T23:56:00Z",
|
||||||
"last_accessed": "2025-09-01T09:30:00Z",
|
"last_accessed": "2025-09-01T09:30:00Z",
|
||||||
}
|
}
|
||||||
@@ -283,3 +287,58 @@ class TestAccountsAPI:
|
|||||||
response = api_client.get("/api/v1/accounts/nonexistent")
|
response = api_client.get("/api/v1/accounts/nonexistent")
|
||||||
|
|
||||||
assert response.status_code == 404
|
assert response.status_code == 404
|
||||||
|
|
||||||
|
def test_update_account_display_name_success(
|
||||||
|
self, api_client, mock_config, mock_auth_token, mock_db_path
|
||||||
|
):
|
||||||
|
"""Test successful update of account display name."""
|
||||||
|
mock_account = {
|
||||||
|
"id": "test-account-123",
|
||||||
|
"institution_id": "REVOLUT_REVOLT21",
|
||||||
|
"status": "READY",
|
||||||
|
"iban": "LT313250081177977789",
|
||||||
|
"name": "Personal Account",
|
||||||
|
"display_name": None,
|
||||||
|
"created": "2024-02-13T23:56:00Z",
|
||||||
|
"last_accessed": "2025-09-01T09:30:00Z",
|
||||||
|
}
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("leggen.utils.config.config", mock_config),
|
||||||
|
patch(
|
||||||
|
"leggen.api.routes.accounts.database_service.get_account_details_from_db",
|
||||||
|
return_value=mock_account,
|
||||||
|
),
|
||||||
|
patch(
|
||||||
|
"leggen.api.routes.accounts.database_service.persist_account_details",
|
||||||
|
return_value=None,
|
||||||
|
),
|
||||||
|
):
|
||||||
|
response = api_client.put(
|
||||||
|
"/api/v1/accounts/test-account-123",
|
||||||
|
json={"display_name": "My Custom Account Name"},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
data = response.json()
|
||||||
|
assert data["success"] is True
|
||||||
|
assert data["data"]["id"] == "test-account-123"
|
||||||
|
assert data["data"]["display_name"] == "My Custom Account Name"
|
||||||
|
|
||||||
|
def test_update_account_not_found(
|
||||||
|
self, api_client, mock_config, mock_auth_token, mock_db_path
|
||||||
|
):
|
||||||
|
"""Test updating non-existent account."""
|
||||||
|
with (
|
||||||
|
patch("leggen.utils.config.config", mock_config),
|
||||||
|
patch(
|
||||||
|
"leggen.api.routes.accounts.database_service.get_account_details_from_db",
|
||||||
|
return_value=None,
|
||||||
|
),
|
||||||
|
):
|
||||||
|
response = api_client.put(
|
||||||
|
"/api/v1/accounts/nonexistent",
|
||||||
|
json={"display_name": "New Name"},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 404
|
||||||
|
|||||||
Reference in New Issue
Block a user