Skip to content

Sl/feat/db optimization #423

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.

Sign up for GitHub

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

Merged
ViktorSvertoka merged 3 commits into from
Mar 28, 2026
+24 −4
Merged

Sl/feat/db optimization #423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9545943
perf(sessions): move heartbeat from DB writes to Redis sorted set
LesiaUKR Mar 22, 2026
6a76a87
Merge branch 'develop' into sl/feat/db-optimization
LesiaUKR Mar 28, 2026
6b4dfec
fix(db): require explicit APP_ENV and add runtime env diagnostics
LesiaUKR Mar 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions frontend/app/[locale]/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export default async function LocaleLayout({
title: string;
}> = [];


const enableAdmin =
(
process.env.ENABLE_ADMIN_API ??
Expand Down
19 changes: 17 additions & 2 deletions frontend/db/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,29 @@ dotenv.config();
type AppDatabase = PgDatabase<PgQueryResultHKT, typeof schema>;

const APP_ENV = process.env.APP_ENV?.trim().toLowerCase();

if (!APP_ENV) {
throw new Error(
'[db] APP_ENV is required. Set APP_ENV=local in .env for development, or APP_ENV=develop/production for deployment'
);
}
Comment on lines +16 to +20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) Locate test setup files that set APP_ENV/SHOP_* in hooks:"
rg -n -C2 "beforeAll\\(|process\\.env\\.(APP_ENV|SHOP_STRICT_LOCAL_DB|SHOP_REQUIRED_DATABASE_URL_LOCAL)" frontend/lib/tests

echo
echo "2) Locate db module imports in test-related files:"
rg -n -C2 "from ['\"][^'\"]*db/index['\"]|from ['\"]@/db/index['\"]|from ['\"][^'\"]*db['\"]" frontend --iglob "*test*.ts" --iglob "*setup*.ts"

echo
echo "Expected: any file importing db before APP_ENV is set indicates this failure mode is real."

Repository: DevLoversTeam/devlovers.net

Length of output: 50383

🏁 Script executed:

# Check the actual db/index.ts to confirm import-time behavior
head -30 frontend/db/index.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 1002

🏁 Script executed:

# Check if there's a vitest config and how it sets up env
fd -name "vitest.config.*" -o -name "vite.config.*" frontend && head -50 vitest.config.ts 2>/dev/null || head -50 vite.config.ts 2>/dev/null

Repository: DevLoversTeam/devlovers.net

Length of output: 301

🏁 Script executed:

# Check if setup.ts is registered as a global setup in vitest config
rg -n "setupFiles|globalSetup" frontend --type ts --type js | head -20

Repository: DevLoversTeam/devlovers.net

Length of output: 240

🏁 Script executed:

# Check what vitest.setup.ts does
cat frontend/vitest.setup.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 113

🏁 Script executed:

# Check if APP_ENV is set in vitest configs or via any env command
rg -A5 -B5 "APP_ENV" frontend/vitest.config.ts frontend/vitest.shop.config.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Search for where APP_ENV might be defaulted in test environment
rg "APP_ENV.*=" frontend --type ts | grep -i "test\|env\|setup" | head -20

Repository: DevLoversTeam/devlovers.net

Length of output: 1786

🏁 Script executed:

# Check if .env file exists and has APP_ENV
ls -la frontend/.env* 2>/dev/null || echo "No .env files found"

Repository: DevLoversTeam/devlovers.net

Length of output: 141

🏁 Script executed:

# Check package.json test scripts to see if APP_ENV is set when running tests
rg "test.*:" frontend/package.json -A2

Repository: DevLoversTeam/devlovers.net

Length of output: 1017

🏁 Script executed:

# Check if there's a .env.test or similar
find frontend -name ".env*" -type f

Repository: DevLoversTeam/devlovers.net

Length of output: 92

🏁 Script executed:

# Check one of the test files that imports db to see the exact import sequence
head -50 frontend/lib/tests/shop/admin-orders-query-filters.test.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 1414

🏁 Script executed:

# Check GitHub Actions or CI config for test environment setup
find . -name "*.yml" -path "*/.github/*" -type f | head -5

Repository: DevLoversTeam/devlovers.net

Length of output: 219

🏁 Script executed:

# Check if there's a .env.test or test-specific dotenv loading
cat frontend/.env.example | head -20

Repository: DevLoversTeam/devlovers.net

Length of output: 607

🏁 Script executed:

# Verify: does a test file that imports db at top level actually work in practice?
# Search for any CI/test configuration that might set APP_ENV globally
rg "APP_ENV" .github -A2 -B2 2>/dev/null || echo "No .github config found"

Repository: DevLoversTeam/devlovers.net

Length of output: 94

🏁 Script executed:

# Check if there's a setup that pre-loads environment before tests
cat frontend/vitest.config.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 903

🏁 Script executed:

# Check the full content of lib/tests/shop/setup.ts to see if APP_ENV is set at module level
cat frontend/lib/tests/shop/setup.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 1393

🏁 Script executed:

# Also check vitest.shop.config.ts in full
cat frontend/vitest.shop.config.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 603

🏁 Script executed:

# Check if .env is in .gitignore
cat frontend/.gitignore | grep -i "\.env"

Repository: DevLoversTeam/devlovers.net

Length of output: 100

🏁 Script executed:

# Look for any mechanism that might pre-populate APP_ENV before test execution
rg "process\.env\.APP_ENV\s*=" frontend --type ts | grep -v "test\.ts" | head -10

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Check if there's a script or config that sets APP_ENV before running tests
rg "APP_ENV.*local" . --type ts --type js --type json | grep -E "(test|setup|config)" | head -15

Repository: DevLoversTeam/devlovers.net

Length of output: 946

🏁 Script executed:

# Search for any .env loading in the test setup itself
rg "dotenv|process\.env\[" frontend/vitest.setup.ts frontend/lib/tests/shop/setup.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Check if vitest or the test runner sets APP_ENV somewhere before file execution
rg "APP_ENV" frontend/vitest* frontend/playwright.config.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 124

🏁 Script executed:

# Search for any environment configuration that might set APP_ENV globally in tests
find frontend -name "*.config.*" -type f | xargs grep -l "APP_ENV" 2>/dev/null

Repository: DevLoversTeam/devlovers.net

Length of output: 100

🏁 Script executed:

# Verify: does the setup file actually set APP_ENV at module level?
grep -n "process\.env\.APP_ENV\s*=" frontend/lib/tests/shop/setup.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Check if any test actually runs and passes - look for test run output or logs
find . -name "test-results*" -o -name "*coverage*" -o -name ".nyc*" 2>/dev/null | head -5

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Key check: does the setup.ts file set APP_ENV at module level before beforeAll?
head -40 frontend/lib/tests/shop/setup.ts | grep -n "process.env.APP_ENV"

Repository: DevLoversTeam/devlovers.net

Length of output: 139

🏁 Script executed:

# Check the order: is APP_ENV set anywhere at module level in setup.ts?
awk '/^import/,/^beforeAll/' frontend/lib/tests/shop/setup.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 1139

🏁 Script executed:

# Verify the exact problem: do tests actually run without APP_ENV being pre-set?
# Check if there's a .env file with APP_ENV set that would be loaded before tests
cat frontend/.env 2>/dev/null | head -20 || echo ".env file not present or empty"

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Critical: check if setup.ts itself is imported as a module before other tests
# and whether setupFiles in vitest execute BEFORE module imports
# Look at how vitest loads setupFiles relative to test file imports
rg "setupFiles" frontend/vitest.shop.config.ts -A10 -B5

Repository: DevLoversTeam/devlovers.net

Length of output: 252

🏁 Script executed:

# Check if there's an environment file that's loaded via NODE_OPTIONS or similar
env | grep -i app_env || echo "APP_ENV not in current environment"

Repository: DevLoversTeam/devlovers.net

Length of output: 105

🏁 Script executed:

# Check the actual import order: does setup.ts get imported before test files try to import db?
# Look for any documented flow or CI configuration
rg "vitest\|npm.*test" .github/workflows -A5 2>/dev/null || echo "No CI workflow found"

Repository: DevLoversTeam/devlovers.net

Length of output: 91

🏁 Script executed:

# Final verification: search for any place where APP_ENV might be set before test module loading
rg "process\.env\.APP_ENV\s*=" frontend -B5 -A2 | grep -E "^[^:]+:(import|const|let|var|process)" -A2 | head -20

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Final critical check: In Vitest, does setupFiles execute BEFORE test file imports?
# Let me verify the actual behavior by checking if there's documentation or examples
# of how setupFiles interact with test file module imports

# Check if there's any code that actually sets APP_ENV at module level in setupFiles
cat frontend/vitest.setup.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 113

🏁 Script executed:

# Verify: are there any test files that DON'T import db, to confirm imports happen at module load?
rg "^import.*from.*@/db" frontend/lib/tests/shop/*.test.ts | wc -l

Repository: DevLoversTeam/devlovers.net

Length of output: 74

🏁 Script executed:

# Count total shop test files
fd ".*\.test\.ts$" frontend/lib/tests/shop | wc -l

Repository: DevLoversTeam/devlovers.net

Length of output: 74

🏁 Script executed:

# Confirm: does setup.ts set APP_ENV at MODULE level (outside beforeAll/beforeEach)?
grep -n "process\.env\.APP_ENV\s*=" frontend/lib/tests/shop/setup.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Check if there's a different setup mechanism that sets APP_ENV before module imports
rg "process\.env\.APP_ENV\s*=" frontend --type ts -B10 | grep -E "^--$|^frontend" | head -30

Repository: DevLoversTeam/devlovers.net

Length of output: 2713

🏁 Script executed:

# Verify the execution order: confirm setupFiles run before imports in Vitest
# by checking if there's any runtime evidence or docs
rg "setupFiles" frontend/vitest.shop.config.ts -B2 -A15

Repository: DevLoversTeam/devlovers.net

Length of output: 212

🏁 Script executed:

# Check if setup.ts ever sets APP_ENV at module level (not in hooks)
cat frontend/lib/tests/shop/setup.ts | head -8

Repository: DevLoversTeam/devlovers.net

Length of output: 360

🏁 Script executed:

# Confirm: does setup.ts SET APP_ENV anywhere, or only validate it?
rg "process\.env\.APP_ENV\s*=" frontend/lib/tests/shop/setup.ts

Repository: DevLoversTeam/devlovers.net

Length of output: 53

🏁 Script executed:

# Check if maybe APP_ENV is supposed to be set in vitest.setup.ts but isn't
cat frontend/vitest.setup.ts | grep -i "app_env" || echo "No APP_ENV in vitest.setup.ts"

Repository: DevLoversTeam/devlovers.net

Length of output: 100

🏁 Script executed:

# Search for any environment setup that might happen at test init time
rg "APP_ENV.*local" frontend --type ts -B3 | grep -E "^(frontend/vitest|frontend/lib/tests/shop/setup)" -A3

Repository: DevLoversTeam/devlovers.net

Length of output: 468

🏁 Script executed:

# Critical question: In Vitest, when a file is listed in setupFiles, 
# does it execute at module level before test file imports?
# Check if there's any code in setup.ts that runs at module level (outside functions/hooks)

awk '/^(import|const|let|var|function|class)/ && !/^(function|class|async function) (assertStrictShopLocalDb|assertNotProductionDb)/ {print NR": "$0}' frontend/lib/tests/shop/setup.ts | head -20

Repository: DevLoversTeam/devlovers.net

Length of output: 228

🏁 Script executed:

# Check what actually happens at module load in setup.ts
sed -n '1,40p' frontend/lib/tests/shop/setup.ts | cat -n

Repository: DevLoversTeam/devlovers.net

Length of output: 1533

🏁 Script executed:

# Final verification: confirm APP_ENV is NEVER set at module level in the setup chain
rg "process\.env\.APP_ENV\s*=" frontend/vitest.setup.ts frontend/lib/tests/shop/setup.ts frontend/.env* 2>/dev/null

Repository: DevLoversTeam/devlovers.net

Length of output: 53

Import-time APP_ENV hard-fail breaks test bootstrap for shop tests.

frontend/db/index.ts:16-20 throws during module evaluation if APP_ENV is undefined. Shop test files (149 of 163) import db at the top level, triggering this check before beforeAll hooks in lib/tests/shop/setup.ts:38-42 can set required environment variables. Since Vitest executes setupFiles before importing test modules, the validation hooks never run.

Consider deferring the APP_ENV check to lazy initialization (e.g., on first db operation) or setting APP_ENV in vitest.setup.ts at module level before any test files are loaded.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@frontend/db/index.ts` around lines 16 - 20, The module currently throws
during import when APP_ENV is missing (the top-level check in
frontend/db/index.ts), which breaks test bootstrap; change the validation so it
runs lazily instead of at import time: remove the immediate throw and move the
APP_ENV presence check into the DB initialization path (e.g., inside the
function(s) that create or return the DB client such as getDbClient / initDb),
or provide an exported ensureAppEnv() called by those init functions;
alternatively add a module-level Vitest setup that defines APP_ENV before tests
load (vitest.setup.ts). Ensure all references to APP_ENV in frontend/db/index.ts
use the lazy-check path so importing the module does not throw.


const IS_LOCAL_ENV = APP_ENV === 'local';

Comment on lines 14 to +23
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate APP_ENV against an allowlist, not only presence.

A typoed value still passes and is treated as non-local. That undermines the fail-fast safety goal.

✅ Suggested fix 
 const APP_ENV = process.env.APP_ENV?.trim().toLowerCase();
+const ALLOWED_APP_ENVS = new Set(['local', 'develop', 'production']);

-if (!APP_ENV) {
+if (!APP_ENV || !ALLOWED_APP_ENVS.has(APP_ENV)) {
   throw new Error(
-    '[db] APP_ENV is required. Set APP_ENV=local in .env for development, or APP_ENV=develop/production for deployment'
+    '[db] APP_ENV must be one of: local, develop, production'
   );
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const APP_ENV = process.env.APP_ENV?.trim().toLowerCase();
if (!APP_ENV) {
throw new Error(
'[db] APP_ENV is required. Set APP_ENV=local in .env for development, or APP_ENV=develop/production for deployment'
);
}
const IS_LOCAL_ENV = APP_ENV === 'local';
const APP_ENV = process.env.APP_ENV?.trim().toLowerCase();
const ALLOWED_APP_ENVS = new Set(['local', 'develop', 'production']);
if (!APP_ENV || !ALLOWED_APP_ENVS.has(APP_ENV)) {
throw new Error(
'[db] APP_ENV must be one of: local, develop, production'
);
}
const IS_LOCAL_ENV = APP_ENV === 'local';
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@frontend/db/index.ts` around lines 14 - 23, APP_ENV is only checked for
presence so typos become silent non-local environments; replace the simple
presence check with an allowlist validation: define an array of allowed values
(e.g., 'local', 'develop', 'production'), normalize process.env.APP_ENV into
APP_ENV, and if APP_ENV is missing or not included in that allowlist throw an
Error; keep IS_LOCAL_ENV = APP_ENV === 'local' and reference APP_ENV and
IS_LOCAL_ENV when updating the code paths that depend on environment.

if (process.env.NODE_ENV !== 'test') {
console.log('[db] runtime env check', {
has_DATABASE_URL: Boolean(process.env.DATABASE_URL?.trim()),
has_DATABASE_URL_LOCAL: Boolean(process.env.DATABASE_URL_LOCAL?.trim()),
});
}

const STRICT_LOCAL_DB_GUARD = process.env.SHOP_STRICT_LOCAL_DB === '1';
const REQUIRED_LOCAL_DB_URL = process.env.SHOP_REQUIRED_DATABASE_URL_LOCAL;

if (STRICT_LOCAL_DB_GUARD) {
if (!IS_LOCAL_ENV) {
throw new Error(
`[db] SHOP_STRICT_LOCAL_DB=1 requires APP_ENV=local (got "${APP_ENV ?? 'undefined'}")`
`[db] SHOP_STRICT_LOCAL_DB=1 requires APP_ENV=local (got "${APP_ENV}")`
);
}

Expand Down Expand Up @@ -68,7 +83,7 @@ if (IS_LOCAL_ENV) {

if (!url) {
throw new Error(
`[db] APP_ENV=${APP_ENV ?? 'undefined'} requires DATABASE_URL to be set`
`[db] APP_ENV=${APP_ENV} requires DATABASE_URL to be set`
);
}

Expand Down
8 changes: 6 additions & 2 deletions frontend/db/queries/categories/admin-categories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ export async function getAdminCategoryList(): Promise<AdminCategoryItem[]> {
title: categoryTranslations.title,
})
.from(categories)
.innerJoin(
.leftJoin(
categoryTranslations,
sql`${categoryTranslations.categoryId} = ${categories.id} AND ${categoryTranslations.locale} = ${ADMIN_LOCALE}`
)
.orderBy(categories.displayOrder);

return rows;
return rows.map(row => ({
id: row.id,
slug: row.slug,
title: row.title ?? row.slug,
}));
Comment on lines +29 to +33
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use a non-empty fallback for category title.

Line 32 uses ??, so empty-string translations still render as blank labels in the admin dropdown. Prefer a trimmed non-empty fallback.

💡 Suggested fix 
   return rows.map(row => ({
     id: row.id,
     slug: row.slug,
-    title: row.title ?? row.slug,
+    title: row.title?.trim() || row.slug,
   }));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return rows.map(row => ({
id: row.id,
slug: row.slug,
title: row.title ?? row.slug,
}));
return rows.map(row => ({
id: row.id,
slug: row.slug,
title: row.title?.trim() || row.slug,
}));
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@frontend/db/queries/categories/admin-categories.ts` around lines 29 - 33, The
mapping currently uses row.title ?? row.slug which treats an empty string as a
valid title; change the logic in the rows.map callback to trim row.title and use
row.slug when the trimmed title is empty (e.g., compute const title =
row.title?.trim(); then set title: title && title.length ? title : row.slug) so
empty-string translations fall back to the slug while preserving non-empty
titles; keep id and slug fields unchanged.

}

export async function getMaxQuizDisplayOrder(
Expand Down
Loading