Compare commits
2 Commits
011d84c3c1
...
de7afbcbe9
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
de7afbcbe9 | ||
|
|
728b3ec172 |
82
.agents/skills/naming-conventions/SKILL.md
Normal file
82
.agents/skills/naming-conventions/SKILL.md
Normal file
@@ -0,0 +1,82 @@
|
||||
---
|
||||
name: naming-conventions
|
||||
description: "Use when: improving variable, function, module, or API names; refactoring code for clarity; choosing consistent naming in the dashback backend."
|
||||
---
|
||||
|
||||
# Naming Conventions — dashback
|
||||
|
||||
Apply these conventions when writing or refactoring code in this project.
|
||||
|
||||
## Core rules
|
||||
|
||||
- Prefer clear domain names over vague generic names.
|
||||
- Name functions as actions, not nouns.
|
||||
- Name variables and parameters by what they represent, not by their implementation detail.
|
||||
- Prefer explicit names over abbreviations unless the abbreviation is standard in the project.
|
||||
- Keep names consistent across the same subsystem.
|
||||
|
||||
## Function naming
|
||||
|
||||
- Use verb-based names for routines that do work: `registerRoutes`, `buildApiApp`, `listenOnPort`, `resolveUser`, `validateInput`.
|
||||
- Use `register`, `build`, `create`, `resolve`, `configure`, `respond`, `validate`, and `render` when the function performs an action.
|
||||
- Avoid vague names like `load`, `walk`, `handle`, `do`, `process`, `thing`, and `data` when a more specific name is available.
|
||||
- When a function builds a response object, prefer names like `respondWith...`, `build...Response`, or `create...Payload`.
|
||||
|
||||
## Variable naming
|
||||
|
||||
- Prefer descriptive nouns or noun phrases: `serverStartedAt`, `startupPromise`, `localPrivateHostPattern`.
|
||||
- Use boolean names that read like a question when possible: `isAllowedCorsOrigin`, `hasSelectedCompany`, `isDocsRequest`.
|
||||
- Use `statusCode` instead of `status` when a numeric HTTP status is involved.
|
||||
- Use `origin` and `request`/`req` precisely; avoid ambiguous names like `body` when `payload` or `userData` is clearer.
|
||||
|
||||
## Module naming
|
||||
|
||||
- Prefer names that describe the module’s responsibility, not incidental implementation details.
|
||||
- For API helper modules, names should communicate the response shape or purpose: `meJson` → `respondWithAuthenticatedUser` or `buildCurrentUserResponse`.
|
||||
- Use consistent file-level naming patterns across the same feature area.
|
||||
|
||||
## File naming
|
||||
|
||||
- Keep each route responsibility in its own file under `src/api/routes`.
|
||||
- Use one route module per resource or workflow, not multiple unrelated endpoints in a single file.
|
||||
- Prefer a single clear file name per route group, for example `auth/me.js`, `company/list.js`, `article/create.js`, or `journal/entries.js`.
|
||||
- Match the file name to the route’s primary resource or action so the file’s purpose is obvious from the directory tree.
|
||||
- Avoid generic names like `index.js`, `utils.js`, or `helpers.js` for route modules unless the file is truly the entry point for a module group.
|
||||
- Keep nested route directories aligned with the URL hierarchy when it improves clarity, e.g. `auth/me.js` for `/auth/me`.
|
||||
|
||||
## API and HTTP naming
|
||||
|
||||
- Prefer `registerRoutes` over `loadRoutes` when the function actively registers route modules on the app.
|
||||
- Prefer `buildApiApp` over `createApi` when the function constructs the Express app object.
|
||||
- Prefer `isAllowedCorsOrigin` over `isAllowedOrigin` for clarity around the specific policy being enforced.
|
||||
- When a helper adds Google client info to a response, include the domain concept in the name, e.g. `respondWithAuthenticatedUser`.
|
||||
|
||||
## Examples from this codebase
|
||||
|
||||
Good:
|
||||
- `registerRoutes`
|
||||
- `buildApiApp`
|
||||
- `listenOnPort`
|
||||
- `startupPromise`
|
||||
- `serverStartedAt`
|
||||
- `respondWithAuthenticatedUser`
|
||||
- `isAllowedCorsOrigin`
|
||||
- `LOCAL_PRIVATE_HOST_PATTERN`
|
||||
|
||||
Avoid:
|
||||
- `loadRoutes` when the function actually registers route modules
|
||||
- `walk` in a route auto-loader unless it is explicitly about recursive traversal and the context is clear
|
||||
- `starting` and `startedAt` when `startupPromise` and `serverStartedAt` are more specific
|
||||
- `meJson` when the function is really a response builder for authenticated-user payloads
|
||||
- generic names like `data`, `thing`, `value`, `handle`, `doWork`
|
||||
|
||||
## Review checklist
|
||||
|
||||
Before finalizing a rename, ask:
|
||||
- Does the name describe the actual responsibility?
|
||||
- Does it read clearly in the local code context?
|
||||
- Is it consistent with neighboring names?
|
||||
- Is there any ambiguity about the action, domain, or response shape?
|
||||
- Is it specific enough that a teammate can infer the purpose without reading the implementation?
|
||||
|
||||
Use these rules as the default naming standard for backend refactors and new code in the project.
|
||||
@@ -22,16 +22,16 @@ const apiLog = createLogger('api');
|
||||
const __filename = fileURLToPath(import.meta.url);
|
||||
const __dirname = path.dirname(__filename);
|
||||
|
||||
async function loadRoutes(app, basePath = '') {
|
||||
async function registerRoutes(app, basePath = '') {
|
||||
const routesDir = path.join(__dirname, 'routes');
|
||||
|
||||
async function walk(dir) {
|
||||
async function registerRouteFilesRecursively(dir) {
|
||||
for (const file of fs.readdirSync(dir)) {
|
||||
const fullPath = path.join(dir, file);
|
||||
const stat = fs.statSync(fullPath);
|
||||
|
||||
if (stat.isDirectory()) {
|
||||
await walk(fullPath);
|
||||
await registerRouteFilesRecursively(fullPath);
|
||||
} else if (file.endsWith('.js')) {
|
||||
const routeModule = await import(pathToFileURL(fullPath).href);
|
||||
if (routeModule.register) {
|
||||
@@ -41,17 +41,17 @@ async function loadRoutes(app, basePath = '') {
|
||||
}
|
||||
}
|
||||
|
||||
await walk(routesDir);
|
||||
await registerRouteFilesRecursively(routesDir);
|
||||
}
|
||||
|
||||
let server;
|
||||
let httpsServer;
|
||||
let starting;
|
||||
let startedAt;
|
||||
let startupPromise;
|
||||
let serverStartedAt;
|
||||
|
||||
const PRIVATE_HOST_REGEX = /^(127\.0\.0\.1|localhost|192\.168\.\d{1,3}\.\d{1,3}|10\.\d{1,3}\.\d{1,3}\.\d{1,3}|172\.(1[6-9]|2\d|3[01])\.\d{1,3}\.\d{1,3})$/;
|
||||
const LOCAL_PRIVATE_HOST_PATTERN = /^(127\.0\.0\.1|localhost|192\.168\.\d{1,3}\.\d{1,3}|10\.\d{1,3}\.\d{1,3}\.\d{1,3}|172\.(1[6-9]|2\d|3[01])\.\d{1,3}\.\d{1,3})$/;
|
||||
|
||||
function isAllowedOrigin(origin) {
|
||||
function isAllowedCorsOrigin(origin) {
|
||||
if (!origin) return false;
|
||||
if (env.corsOrigins.includes(origin)) return true;
|
||||
try {
|
||||
@@ -59,7 +59,7 @@ function isAllowedOrigin(origin) {
|
||||
if (u.hostname === 'quixpos.com' || u.hostname.endsWith('.quixpos.com')) {
|
||||
return true;
|
||||
}
|
||||
if (PRIVATE_HOST_REGEX.test(u.hostname)) {
|
||||
if (LOCAL_PRIVATE_HOST_PATTERN.test(u.hostname)) {
|
||||
return true;
|
||||
}
|
||||
} catch {
|
||||
@@ -101,7 +101,7 @@ function corsMiddleware(req, res, next) {
|
||||
return next();
|
||||
}
|
||||
|
||||
if (origin && isAllowedOrigin(origin)) {
|
||||
if (origin && isAllowedCorsOrigin(origin)) {
|
||||
res.setHeader('Access-Control-Allow-Origin', origin);
|
||||
res.setHeader('Vary', 'Origin');
|
||||
res.setHeader('Access-Control-Allow-Credentials', 'true');
|
||||
@@ -120,7 +120,7 @@ function corsMiddleware(req, res, next) {
|
||||
next();
|
||||
}
|
||||
|
||||
async function createApi() {
|
||||
async function buildApiApp() {
|
||||
const app = express();
|
||||
|
||||
app.use((req, res, next) => {
|
||||
@@ -136,14 +136,14 @@ async function createApi() {
|
||||
app.use(loggerMiddleware);
|
||||
app.use(authMiddleware);
|
||||
|
||||
if (startedAt) {
|
||||
if (serverStartedAt) {
|
||||
app.use((req, res, next) => {
|
||||
res.setHeader('X-Server-Started-At', startedAt);
|
||||
res.setHeader('X-Server-Started-At', serverStartedAt);
|
||||
next();
|
||||
});
|
||||
}
|
||||
|
||||
await loadRoutes(app, '/api');
|
||||
await registerRoutes(app, '/api');
|
||||
app.use('/v1', v1Router);
|
||||
app.use('/api/v1', v1Router);
|
||||
|
||||
@@ -166,7 +166,7 @@ async function createApi() {
|
||||
return app;
|
||||
}
|
||||
|
||||
function listenServer(instance, port, protocol) {
|
||||
function listenOnPort(instance, port, protocol) {
|
||||
return new Promise((resolve, reject) => {
|
||||
instance.once('listening', () => {
|
||||
resolve();
|
||||
@@ -196,18 +196,18 @@ export async function startApi() {
|
||||
if (server || httpsServer) {
|
||||
return server;
|
||||
}
|
||||
if (starting) {
|
||||
return starting;
|
||||
if (startupPromise) {
|
||||
return startupPromise;
|
||||
}
|
||||
|
||||
starting = (async () => {
|
||||
startupPromise = (async () => {
|
||||
await ensureConnection();
|
||||
|
||||
startedAt = new Date().toISOString();
|
||||
const app = await createApi();
|
||||
serverStartedAt = new Date().toISOString();
|
||||
const app = await buildApiApp();
|
||||
|
||||
const httpInstance = http.createServer(app);
|
||||
await listenServer(httpInstance, env.port, 'HTTP');
|
||||
await listenOnPort(httpInstance, env.port, 'HTTP');
|
||||
server = httpInstance;
|
||||
apiLog.info(`API listening on port ${env.port} (HTTP)`);
|
||||
|
||||
@@ -215,7 +215,7 @@ export async function startApi() {
|
||||
try {
|
||||
const tls = await getOrCreateCert();
|
||||
const httpsInstance = https.createServer(tls, app);
|
||||
await listenServer(httpsInstance, env.httpsPort, 'HTTPS');
|
||||
await listenOnPort(httpsInstance, env.httpsPort, 'HTTPS');
|
||||
httpsServer = httpsInstance;
|
||||
apiLog.info(`API listening on port ${env.httpsPort} (HTTPS self-signed)`);
|
||||
} catch (err) {
|
||||
@@ -227,9 +227,9 @@ export async function startApi() {
|
||||
})();
|
||||
|
||||
try {
|
||||
return await starting;
|
||||
return await startupPromise;
|
||||
} finally {
|
||||
starting = undefined;
|
||||
startupPromise = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import jwt from 'jsonwebtoken';
|
||||
import env from '../../config/env.js';
|
||||
import * as db from '../../db/googleAuth.js';
|
||||
import { meJson } from '../auth/meJson.js';
|
||||
import { respondWithAuthenticatedUser } from '../auth/meJson.js';
|
||||
|
||||
const userCache = new Map();
|
||||
const USER_CACHE_TTL = 10 * 60 * 1000;
|
||||
@@ -39,9 +39,9 @@ export function clearCachedUser(authId) {
|
||||
userCache.delete(authId);
|
||||
}
|
||||
|
||||
function authMeJson(res, req, status, body) {
|
||||
function respondWithAuthPayload(res, req, status, body) {
|
||||
if (req.path === '/api/auth/me') {
|
||||
return meJson(res, status, body);
|
||||
return respondWithAuthenticatedUser(res, status, body);
|
||||
}
|
||||
return res.status(status).json(body);
|
||||
}
|
||||
@@ -71,7 +71,7 @@ export const authMiddleware = async (req, res, next) => {
|
||||
const isDocs = req.path.startsWith('/docs') || req.path.startsWith('/api/docs');
|
||||
if (!token) {
|
||||
if (isDocs) return next();
|
||||
return authMeJson(res, req, 401, { error: 'Unauthorized' });
|
||||
return respondWithAuthPayload(res, req, 401, { error: 'Unauthorized' });
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -80,7 +80,7 @@ export const authMiddleware = async (req, res, next) => {
|
||||
if (!Number.isInteger(authId) || authId <= 0) {
|
||||
if (req.cookies.token) clearTokenCookie(res);
|
||||
if (isDocs) return next();
|
||||
return authMeJson(res, req, 401, { error: 'Invalid token' });
|
||||
return respondWithAuthPayload(res, req, 401, { error: 'Invalid token' });
|
||||
}
|
||||
|
||||
let user = getCachedUser(authId);
|
||||
@@ -97,7 +97,7 @@ export const authMiddleware = async (req, res, next) => {
|
||||
clearCachedUser(authId);
|
||||
if (req.cookies.token) clearTokenCookie(res);
|
||||
if (isDocs) return next();
|
||||
return authMeJson(res, req, 403, { error: 'User is no longer authorized' });
|
||||
return respondWithAuthPayload(res, req, 403, { error: 'User is no longer authorized' });
|
||||
}
|
||||
|
||||
req.user = {
|
||||
@@ -119,7 +119,7 @@ export const authMiddleware = async (req, res, next) => {
|
||||
} catch {
|
||||
if (req.cookies.token) clearTokenCookie(res);
|
||||
if (isDocs) return next();
|
||||
return authMeJson(res, req, 401, { error: 'Invalid token' });
|
||||
return respondWithAuthPayload(res, req, 401, { error: 'Invalid token' });
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { meJson } from '../../auth/meJson.js';
|
||||
import { respondWithAuthenticatedUser } from '../../auth/meJson.js';
|
||||
import * as db from '../../../db/googleAuth.js';
|
||||
import { toPictureBuffer } from '../../../util/pictureBinary.js';
|
||||
|
||||
@@ -49,7 +49,7 @@ export function register(app, basePath = '') {
|
||||
try {
|
||||
const row = await db.getById(req.user.id);
|
||||
if (!row) {
|
||||
return meJson(res, 403, { error: 'User is no longer authorized' });
|
||||
return respondWithAuthenticatedUser(res, 403, { error: 'User is no longer authorized' });
|
||||
}
|
||||
|
||||
const pictureBinary = toPictureBuffer(row.pictureBinary);
|
||||
@@ -59,10 +59,10 @@ export function register(app, basePath = '') {
|
||||
selectedCompanyId: row.selectedCompanyId,
|
||||
...(pictureBinary && { picture: pictureBinary.toString('base64') }),
|
||||
};
|
||||
meJson(res, 200, { user });
|
||||
respondWithAuthenticatedUser(res, 200, { user });
|
||||
} catch (error) {
|
||||
console.error('Get current user failed:', error);
|
||||
meJson(res, 500, { error: 'Failed to load user' });
|
||||
respondWithAuthenticatedUser(res, 500, { error: 'Failed to load user' });
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user