From ab6bad17c914efbee944e2941ce862f47db302ff Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Tue, 2 Jun 2026 11:33:45 +0200 Subject: [PATCH] feat(hours): install-wide default rate + inline missing-rate CTA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hour-entry saves hard-failed with an English-only error when a customer had no rate, and the standalone hours page showed a disabled rate field that looked set. Add a global business_profile default_hourly_rate_minor (migration 113) as the last link in the rate chain (entry override → customer → install default), so saves succeed with the global rate. When no rate resolves anywhere, replace the save-time error with a read-only resolved-rate display + a CTA to set a customer or install-wide rate, disable Add-entry until a rate/override exists, and translate the backend HOURLY_RATE_REQUIRED toast (en+de). --- .../services/customerHoursService.test.js | 19 ++- .../core/113_add_default_hourly_rate.js | 35 +++++ backend/src/routes/adminBusinessProfile.js | 10 ++ .../src/services/businessProfileService.js | 15 ++ backend/src/services/customerHoursService.js | 57 ++++++-- .../src/components/admin/HoursSection.tsx | 133 ++++++++++++++---- frontend/src/i18n/locales/de.json | 20 ++- frontend/src/i18n/locales/en.json | 20 ++- .../settings/SettingsBusinessProfilePage.tsx | 25 ++++ .../src/services/businessProfile.service.ts | 5 + 10 files changed, 295 insertions(+), 44 deletions(-) create mode 100644 backend/migrations/core/113_add_default_hourly_rate.js diff --git a/backend/__tests__/services/customerHoursService.test.js b/backend/__tests__/services/customerHoursService.test.js index 23302408..4396a36a 100644 --- a/backend/__tests__/services/customerHoursService.test.js +++ b/backend/__tests__/services/customerHoursService.test.js @@ -58,13 +58,30 @@ describe('resolveEffectiveRate', () => { )).toBe(15000); }); - it('throws when both override and customer rate are unset', () => { + it('throws when override, customer rate, AND install default are all unset', () => { expect(() => resolveEffectiveRate( { hourly_rate_minor_override: null }, { hourly_rate_minor: null }, + null, )).toThrow(/No hourly rate/); }); + it('falls back to the install-wide default when override + customer rate are unset', () => { + expect(resolveEffectiveRate( + { hourly_rate_minor_override: null }, + { hourly_rate_minor: null }, + 12000, + )).toBe(12000); + }); + + it('customer rate wins over the install-wide default', () => { + expect(resolveEffectiveRate( + { hourly_rate_minor_override: null }, + { hourly_rate_minor: 15000 }, + 12000, + )).toBe(15000); + }); + it('treats override=0 as "explicitly zero" (not null)', () => { // Override === 0 is unusual but legal — pro bono blocks, internal // tracking. Must NOT fall through to the customer default. diff --git a/backend/migrations/core/113_add_default_hourly_rate.js b/backend/migrations/core/113_add_default_hourly_rate.js new file mode 100644 index 00000000..4b9a3c01 --- /dev/null +++ b/backend/migrations/core/113_add_default_hourly_rate.js @@ -0,0 +1,35 @@ +/** + * Migration: install-wide default hourly rate. + * + * Background: hour entries resolve a billing rate through + * entry.hourly_rate_minor_override → customer.hourly_rate_minor. + * When a customer had neither set, saving an entry hard-failed with + * HOURLY_RATE_REQUIRED — a confusing save-time error on the hours page. + * This adds an install-wide fallback so a single global rate covers + * every customer who hasn't been given an individual one. The chain + * becomes: + * entry override → customer rate → business_profile default → (CTA). + * + * Stored in minor units (matches customer_accounts.hourly_rate_minor). + * Nullable, default NULL: existing installs keep today's behaviour + * (no implicit rate) until the admin sets one — no surprise rate gets + * applied on upgrade (migration-preserve-existing-state guidance). + * + * Idempotent: guarded by hasColumn so a re-run is a no-op. + */ + +exports.up = async function(knex) { + if (!(await knex.schema.hasTable('business_profile'))) return; + if (await knex.schema.hasColumn('business_profile', 'default_hourly_rate_minor')) return; + await knex.schema.alterTable('business_profile', (table) => { + table.bigInteger('default_hourly_rate_minor'); + }); +}; + +exports.down = async function(knex) { + if (!(await knex.schema.hasTable('business_profile'))) return; + if (!(await knex.schema.hasColumn('business_profile', 'default_hourly_rate_minor'))) return; + await knex.schema.alterTable('business_profile', (table) => { + table.dropColumn('default_hourly_rate_minor'); + }); +}; diff --git a/backend/src/routes/adminBusinessProfile.js b/backend/src/routes/adminBusinessProfile.js index 5ddfd613..30a6ae9c 100644 --- a/backend/src/routes/adminBusinessProfile.js +++ b/backend/src/routes/adminBusinessProfile.js @@ -142,6 +142,10 @@ function transformProfile(p) { taxId: p.tax_id || '', vatLabel: p.vat_label || 'MwSt.', vatRateDefault: p.vat_rate_default == null ? null : Number(p.vat_rate_default), + // Install-wide fallback hourly rate (migration 113), minor units. + // null = no global default; the hours page then requires a per- + // customer or per-entry rate. + defaultHourlyRateMinor: p.default_hourly_rate_minor == null ? null : Number(p.default_hourly_rate_minor), defaultCurrency: p.default_currency || 'CHF', defaultLocale: p.default_locale || 'de', defaultQrFormat: p.default_qr_format || 'none', @@ -346,6 +350,11 @@ router.put( body('taxId').optional({ values: 'falsy' }).isString().isLength({ max: 64 }), body('vatLabel').optional({ values: 'falsy' }).isString().isLength({ max: 64 }), body('vatRateDefault').optional({ values: 'falsy' }).isFloat({ min: 0, max: 100 }), + // Migration 113 — install-wide default hourly rate, minor units. + // nullable so the admin can clear it; values: 'falsy' would drop a + // legitimate 0 (which we treat as "explicitly free"), so use the + // nullable form and let the service coerce. + body('defaultHourlyRateMinor').optional({ nullable: true }).isInt({ min: 0 }), body('defaultCurrency').optional({ values: 'falsy' }).isString().isLength({ min: 3, max: 3 }), body('defaultLocale').optional({ values: 'falsy' }).isString().isLength({ max: 8 }), body('defaultQrFormat').optional({ values: 'falsy' }).isIn(['swiss', 'epc', 'none']), @@ -393,6 +402,7 @@ router.put( taxId: 'tax_id', vatLabel: 'vat_label', vatRateDefault: 'vat_rate_default', + defaultHourlyRateMinor: 'default_hourly_rate_minor', defaultCurrency: 'default_currency', defaultLocale: 'default_locale', defaultQrFormat: 'default_qr_format', diff --git a/backend/src/services/businessProfileService.js b/backend/src/services/businessProfileService.js index 56e61e52..5f717bb6 100644 --- a/backend/src/services/businessProfileService.js +++ b/backend/src/services/businessProfileService.js @@ -41,6 +41,10 @@ const ALLOWED_PROFILE_FIELDS = [ 'tax_id', 'vat_label', 'vat_rate_default', + // Install-wide fallback hourly rate (migration 113), minor units. + // Last link in the hour-entry rate chain after the per-entry + // override and the per-customer default. + 'default_hourly_rate_minor', 'default_currency', 'default_locale', 'default_qr_format', @@ -161,6 +165,17 @@ function sanitiseProfilePayload(payload) { ? Math.max(24, Math.min(200, n)) : 56; } + // Install-wide default hourly rate (minor units). Empty / null clears + // it back to "no global default"; otherwise coerce to a non-negative + // integer so a stray decimal can't land sub-cent values in the column. + if (updates.default_hourly_rate_minor !== undefined) { + if (updates.default_hourly_rate_minor === null || updates.default_hourly_rate_minor === '') { + updates.default_hourly_rate_minor = null; + } else { + const n = parseInt(updates.default_hourly_rate_minor, 10); + updates.default_hourly_rate_minor = Number.isFinite(n) && n >= 0 ? n : null; + } + } return updates; } diff --git a/backend/src/services/customerHoursService.js b/backend/src/services/customerHoursService.js index d35065fa..6e17b864 100644 --- a/backend/src/services/customerHoursService.js +++ b/backend/src/services/customerHoursService.js @@ -25,6 +25,7 @@ const { db, logActivity } = require('../database/db'); const { formatBoolean } = require('../utils/dbCompat'); const { AppError } = require('../utils/errors'); +const { hasColumnCached } = require('../utils/schemaCache'); const logger = require('../utils/logger'); const invoiceService = require('./invoiceService'); @@ -50,24 +51,52 @@ function computeDurationMinutes(start, end) { } /** - * Resolve the rate this entry should bill at. Override on the entry - * wins; otherwise we fall back to the customer's default rate. If - * neither is set we throw — saves can't go through without a rate. + * Resolve the rate this entry should bill at. Resolution chain: + * 1. per-entry override + * 2. per-customer default rate + * 3. install-wide default rate (business_profile, migration 113) + * Only when all three are unset do we throw — the hours UI surfaces a + * "set a rate" CTA off the back of HOURLY_RATE_REQUIRED rather than a + * raw error. `installDefaultMinor` is loaded once per request by the + * caller (see getInstallDefaultRateMinor) and passed in so this stays + * a pure function. */ -function resolveEffectiveRate(entry, customer) { +function resolveEffectiveRate(entry, customer, installDefaultMinor = null) { if (entry.hourly_rate_minor_override != null) { return Number(entry.hourly_rate_minor_override); } if (customer.hourly_rate_minor != null) { return Number(customer.hourly_rate_minor); } + if (installDefaultMinor != null) { + return Number(installDefaultMinor); + } throw new AppError( - 'No hourly rate: set a per-entry override or a customer default.', + 'No hourly rate: set a per-entry override, a customer default, or an install-wide default rate.', 400, 'HOURLY_RATE_REQUIRED', ); } +/** + * Read the install-wide default hourly rate (minor units) off the + * singleton business_profile row. Returns null when unset OR when the + * column doesn't exist yet (pre-migration-113 install) — callers then + * fall through to the HOURLY_RATE_REQUIRED path. Accepts an optional + * transaction so it joins the caller's atomic unit. + */ +async function getInstallDefaultRateMinor(trx) { + const conn = trx || db; + if (!(await hasColumnCached('business_profile', 'default_hourly_rate_minor'))) { + return null; + } + const row = await conn('business_profile').where({ id: 1 }) + .first('default_hourly_rate_minor'); + return row && row.default_hourly_rate_minor != null + ? Number(row.default_hourly_rate_minor) + : null; +} + /** * Decide whether an entry is still editable. Pure function — callers * pass the loaded entry + (optionally) its current invoice row. @@ -181,9 +210,14 @@ async function createEntry(customerId, payload, adminId) { } const description = payload.description ? String(payload.description).slice(0, 1000) : null; + // Install-wide fallback rate (migration 113) — the last link in the + // resolution chain. Loaded once and reused for the pre-validate and + // the accumulator append below. + const installDefaultMinor = await getInstallDefaultRateMinor(); + // Pre-validate the rate resolves to something — fail before insert - // if neither override nor customer default is set. - resolveEffectiveRate({ hourly_rate_minor_override: override }, customer); + // if neither override, customer default, nor install default is set. + resolveEffectiveRate({ hourly_rate_minor_override: override }, customer, installDefaultMinor); return await db.transaction(async (trx) => { const row = { @@ -207,7 +241,7 @@ async function createEntry(customerId, payload, adminId) { // unbilled. Manual differs only in that its draft never auto-flushes. if (customer.billing_cadence === 'monthly' || customer.billing_cadence === 'manual') { const fullEntry = { ...row, id: entryId }; - const rate = resolveEffectiveRate(fullEntry, customer); + const rate = resolveEffectiveRate(fullEntry, customer, installDefaultMinor); const lineItem = buildLineItemFromEntry(fullEntry, rate); const { invoiceId, lineItemId } = await invoiceService.appendOneLineItemToMonthlyDraft( customer, lineItem, adminId, trx, @@ -288,7 +322,8 @@ async function updateEntry(entryId, payload, adminId) { // Recompute the linked line item if the entry is billed (on a // draft — the lock check above already proved it's mutable). if (entry.invoice_id && entry.invoice_line_item_id) { - const rate = resolveEffectiveRate(next, customer); + const installDefaultMinor = await getInstallDefaultRateMinor(trx); + const rate = resolveEffectiveRate(next, customer, installDefaultMinor); const newLineItem = buildLineItemFromEntry(next, rate); await trx('invoice_line_items').where({ id: entry.invoice_line_item_id }).update({ description: newLineItem.description, @@ -412,8 +447,9 @@ async function billUnbilledEntries(customerId, adminId) { throw new AppError('No unbilled entries to bill', 409, 'NO_UNBILLED'); } + const installDefaultMinor = await getInstallDefaultRateMinor(trx); const lineItems = unbilled.map((entry, idx) => { - const rate = resolveEffectiveRate(entry, customer); + const rate = resolveEffectiveRate(entry, customer, installDefaultMinor); const li = buildLineItemFromEntry(entry, rate); return { ...li, position: idx + 1 }; }); @@ -465,6 +501,7 @@ module.exports = { updateEntry, deleteEntry, billUnbilledEntries, + getInstallDefaultRateMinor, _internal: { computeDurationMinutes, resolveEffectiveRate, diff --git a/frontend/src/components/admin/HoursSection.tsx b/frontend/src/components/admin/HoursSection.tsx index 62536a2a..f9b86673 100644 --- a/frontend/src/components/admin/HoursSection.tsx +++ b/frontend/src/components/admin/HoursSection.tsx @@ -15,8 +15,9 @@ import React, { useMemo, useState } from 'react'; import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; import { useTranslation } from 'react-i18next'; +import { Link } from 'react-router-dom'; import { toast } from 'react-toastify'; -import { Clock } from 'lucide-react'; +import { Clock, AlertTriangle } from 'lucide-react'; import { Button, Card, LocalizedDateInput } from '../common'; import { DecimalInput } from '../common/DecimalInput'; import { parseLocaleDecimal, parseDuration } from '../../utils/parsers'; @@ -91,6 +92,22 @@ export const HoursSection: React.FC = ({ staleTime: 5 * 60 * 1000, }); const profileDefaultCurrency = profileSnapshot?.profile?.defaultCurrency || 'CHF'; + // Install-wide fallback rate (migration 113). Last link in the rate + // chain after the per-entry override and the per-customer default. + const installDefaultRateMinor = profileSnapshot?.profile?.defaultHourlyRateMinor ?? null; + // The rate that applies to a NEW entry when no per-entry override is + // typed: customer rate, else the install default. null = neither set, + // so a save would fail unless the admin enters an override. + const effectiveDefaultRateMinor = customerHourlyRateMinor ?? installDefaultRateMinor; + // True when there's genuinely no rate to bill at — drives the inline + // CTA + disables the save button. An override typed in the form lifts + // this (handled below where the button is rendered). + const noRateConfigured = effectiveDefaultRateMinor == null; + const overrideTyped = (() => { + if (!rateOverride.trim()) return false; + const n = parseLocaleDecimal(rateOverride); + return Number.isFinite(n) && n >= 0; + })(); const createMutation = useMutation({ mutationFn: () => customerAdminService.createHourEntry(customerId, { @@ -114,7 +131,17 @@ export const HoursSection: React.FC = ({ toast.success(t('customers.hours.toast.created', 'Entry logged')); }, onError: (err: any) => { - const msg = err?.response?.data?.error || err?.message || 'Failed to log entry'; + // The save-time "no rate" failure is translated here off the + // backend error code (the raw message is English-only). The inline + // guard below normally prevents this, but a race (rate cleared in + // another tab) can still surface it. + if (err?.response?.data?.code === 'HOURLY_RATE_REQUIRED') { + toast.error(t('customers.hours.error.noRate', + 'No hourly rate set for this customer. Enter a rate override, set a rate on the customer, or configure an install-wide default in Settings.')); + return; + } + const msg = err?.response?.data?.error || err?.message + || t('customers.hours.error.createFailed', 'Failed to log entry'); toast.error(msg); }, }); @@ -153,11 +180,11 @@ export const HoursSection: React.FC = ({ for (const e of entries) { if (e.status !== 'unbilled') continue; count += 1; - const rateMinor = e.hourlyRateMinorOverride ?? customerHourlyRateMinor ?? 0; + const rateMinor = e.hourlyRateMinorOverride ?? effectiveDefaultRateMinor ?? 0; minor += rateMinor * e.durationMinutes / 60; } return { unbilledCount: count, unbilledTotalMajor: minor / 100 }; - }, [entries, customerHourlyRateMinor]); + }, [entries, effectiveDefaultRateMinor]); const isMonthly = billingCadence === 'monthly'; // Local lockout check — mirrors customerHoursService.isEntryLocked @@ -184,33 +211,72 @@ export const HoursSection: React.FC = ({ 'Logged entries stay unbilled until you click "Create draft invoice" — a standalone draft invoice is generated with one line per entry, ready for you to review before sending.')}

- {/* Default rate — hidden in compact mode (history-only on the - customer detail page; admin edits the rate elsewhere). */} + {/* Rate summary — hidden in compact mode (history-only on the + customer detail page). When a caller wires onHourlyRateChange + the field is editable; otherwise (the standalone hours page) + we show the RESOLVED rate read-only so a disabled input can't + masquerade as an editable value, and surface a CTA when no rate + is configured anywhere along the chain. */} {!compact && (
- { - if (!onHourlyRateChange) return; - if (!Number.isFinite(n)) { - onHourlyRateChange(null); - return; - } - onHourlyRateChange(Math.max(0, Math.round(n * 100))); - }} - disabled={!onHourlyRateChange} - className="w-40 input" - placeholder="150.00" - /> -

- {t('customers.field.hourlyRateHint', - 'Major units (e.g. 150.00 for {{currency}} 150). Leave blank to require a per-entry override on every block.', - { currency: profileDefaultCurrency })} -

+ {onHourlyRateChange ? ( + <> + { + if (!Number.isFinite(n)) { onHourlyRateChange(null); return; } + onHourlyRateChange(Math.max(0, Math.round(n * 100))); + }} + className="w-40 input" + placeholder="150.00" + /> +

+ {t('customers.field.hourlyRateHint', + 'Major units (e.g. 150.00 for {{currency}} 150). Leave blank to require a per-entry override on every block.', + { currency: profileDefaultCurrency })} +

+ + ) : noRateConfigured ? ( +
+
+ +
+

+ {t('customers.hours.noRate.title', 'No hourly rate configured')} +

+

+ {t('customers.hours.noRate.body', + 'Logging needs a rate. Set one for this customer, type a per-entry override below, or configure an install-wide default.')} +

+
+ + {t('customers.hours.noRate.setForCustomer', 'Set a rate for this customer')} + + + {t('customers.hours.noRate.setInstallDefault', 'Set an install-wide default')} + +
+
+
+
+ ) : ( +

+ + {profileDefaultCurrency} {((effectiveDefaultRateMinor as number) / 100).toFixed(2)} + + + {customerHourlyRateMinor != null + ? t('customers.hours.rateSource.customer', 'from this customer') + : t('customers.hours.rateSource.installDefault', 'install-wide default')} + +

+ )}
)} @@ -270,8 +336,8 @@ export const HoursSection: React.FC = ({ inputMode="decimal" value={rateOverride} onChange={(e) => setRateOverride(e.target.value)} - placeholder={customerHourlyRateMinor != null - ? (customerHourlyRateMinor / 100).toFixed(2) + placeholder={effectiveDefaultRateMinor != null + ? (effectiveDefaultRateMinor / 100).toFixed(2) : '—'} className="input w-full" /> @@ -286,10 +352,15 @@ export const HoursSection: React.FC = ({ placeholder={t('customers.hours.form.notePlaceholder', 'What was worked on?') as string} /> -
+
+ {noRateConfigured && !overrideTyped && ( + + {t('customers.hours.form.needRate', 'Set a rate or enter an override to log time.')} + + )}