Type the segment date offset as an interval, not year and month spinners
The Date offset on a baseline or reference segment was a pair of type="number" inputs, years and months, both clamped at min 0. So no characters could be typed, days could not be expressed at all, and a segment could only ever be shifted forward in whole months -- while the stored value is a Postgres interval that happily accepts "4 months", "1 year" or "-90 days". parseOffset only read year and month, so anything else would not have survived a round trip through the edit form either. One text field now, with suggestions, matching what clone already offers. parseInterval parses it far enough to draw the timeline preview; Postgres stays the authority, and assertInterval -- now shared by the baseline, reference and clone routes -- rejects what it will not accept, with a message naming the field rather than a parse error from inside a CTE. Timeline takes months and days separately rather than years and months, because the two are not interchangeable: a month shift lands on the same day of another month, a day shift can cross a month boundary. It adds months then days, as Postgres does, and its label handles negatives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8f96fa3f7f
commit
ee4a60475e
@ -48,6 +48,24 @@ module.exports = function(pool) {
|
||||
: [{ slices, where: buildWhereAny(slices, ctx.filterCols) }];
|
||||
}
|
||||
|
||||
// The offset is interpolated into the statement as an interval literal, so a
|
||||
// typo would surface as a Postgres parse error from the middle of a CTE. Ask
|
||||
// Postgres to parse it alone first, where the failure is cheap and can name the
|
||||
// field it came from. Negative intervals are valid and useful -- '-90 days'
|
||||
// pulls a plan back a quarter -- so this checks validity, not sign.
|
||||
async function assertInterval(value, res) {
|
||||
try {
|
||||
await pool.query(`SELECT $1::interval`, [value]);
|
||||
return true;
|
||||
} catch {
|
||||
res.status(400).json({
|
||||
error: `"${value}" is not a valid interval. Try something like `
|
||||
+ `"4 months", "1 year", "-90 days" or "0 days".`
|
||||
});
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// A slice is only meaningful if at least one of its keys is a filterable
|
||||
// column. buildWhere silently drops unknown keys, so {"typo": "x"} would
|
||||
// otherwise reduce to TRUE and apply the operation to the whole version.
|
||||
@ -405,6 +423,7 @@ module.exports = function(pool) {
|
||||
const { where_clause, date_offset, note, filters, raw_where } = req.body;
|
||||
const pf_user = sessionUser(req);
|
||||
const dateOffset = date_offset || '0 days';
|
||||
if (!await assertInterval(dateOffset, res)) return;
|
||||
const filterClause = (raw_where || where_clause || '').trim() || 'TRUE';
|
||||
try {
|
||||
const ctx = await getContext(parseInt(req.params.id), 'baseline');
|
||||
@ -441,6 +460,7 @@ module.exports = function(pool) {
|
||||
const { where_clause, date_offset, note, filters, raw_where } = req.body;
|
||||
const pf_user = sessionUser(req);
|
||||
const dateOffset = date_offset || '0 days';
|
||||
if (!await assertInterval(dateOffset, res)) return;
|
||||
const filterClause = (raw_where || where_clause || '').trim() || 'TRUE';
|
||||
|
||||
const client = await pool.connect();
|
||||
@ -734,20 +754,7 @@ module.exports = function(pool) {
|
||||
const scaleFactor = (scale != null) ? parseFloat(scale) : 1.0;
|
||||
const dateOffset = (date_offset || '0 days').trim() || '0 days';
|
||||
|
||||
// The offset is interpolated into the SQL as an interval literal, so a
|
||||
// typo would surface as a Postgres parse error mid-statement. Ask
|
||||
// Postgres to parse it on its own first, where the failure is cheap and
|
||||
// can be reported against the field the user typed it into. Negative
|
||||
// intervals are fine and useful -- '-90 days' pulls a plan back a
|
||||
// quarter -- so this checks validity, not sign.
|
||||
try {
|
||||
await pool.query(`SELECT $1::interval`, [dateOffset]);
|
||||
} catch {
|
||||
return res.status(400).json({
|
||||
error: `"${dateOffset}" is not a valid interval. Try something like `
|
||||
+ `"12 months", "-90 days" or "0 days".`
|
||||
});
|
||||
}
|
||||
if (!await assertInterval(dateOffset, res)) return;
|
||||
|
||||
// exclude_iters deliberately does not apply here. It exists to stop
|
||||
// operations *modifying* reference rows: scale would attribute forecast
|
||||
|
||||
@ -32,11 +32,16 @@ function roundRect(ctx, x, y, w, h, r, fill, stroke) {
|
||||
if (stroke) ctx.stroke()
|
||||
}
|
||||
|
||||
export default function Timeline({ dateFrom, dateTo, offsetYr, offsetMo, type = 'baseline' }) {
|
||||
export default function Timeline({ dateFrom, dateTo, offsetMonths = 0, offsetDays = 0, type = 'baseline' }) {
|
||||
const canvasRef = useRef(null)
|
||||
|
||||
const offsetMoTotal = (offsetYr || 0) * 12 + (offsetMo || 0)
|
||||
const twoBands = type === 'baseline' && offsetMoTotal > 0
|
||||
// Months and days are kept apart because they are not interchangeable: a month
|
||||
// shift lands on the same day of a different month, a day shift can cross a
|
||||
// month boundary. The preview adds months first, then days, as Postgres does.
|
||||
const offsetMoTotal = offsetMonths || 0
|
||||
const offsetDayTotal = offsetDays || 0
|
||||
const shifted = offsetMoTotal !== 0 || offsetDayTotal !== 0
|
||||
const twoBands = type === 'baseline' && shifted
|
||||
const canvasH = twoBands ? 90 : 52
|
||||
|
||||
useEffect(() => {
|
||||
@ -61,8 +66,9 @@ export default function Timeline({ dateFrom, dateTo, offsetYr, offsetMo, type =
|
||||
const srcEnd = parseDate(dateTo)
|
||||
if (!srcStart || !srcEnd || isNaN(srcStart) || isNaN(srcEnd)) return
|
||||
|
||||
const projStart = addMonths(srcStart, offsetMoTotal)
|
||||
const projEnd = addMonths(srcEnd, offsetMoTotal)
|
||||
const addDays = (d, n) => { const x = new Date(d); x.setDate(x.getDate() + n); return x }
|
||||
const projStart = addDays(addMonths(srcStart, offsetMoTotal), offsetDayTotal)
|
||||
const projEnd = addDays(addMonths(srcEnd, offsetMoTotal), offsetDayTotal)
|
||||
|
||||
const winStart = addMonths(srcStart, -1)
|
||||
const winEnd = addMonths(twoBands ? projEnd : srcEnd, 1)
|
||||
@ -147,7 +153,14 @@ export default function Timeline({ dateFrom, dateTo, offsetYr, offsetMo, type =
|
||||
ctx.lineTo(px1 - 4, arrowY + 4)
|
||||
ctx.closePath()
|
||||
ctx.fill()
|
||||
const offsetLabel = '+' + (offsetYr ? offsetYr + 'yr ' : '') + (offsetMo ? offsetMo + 'mo' : '')
|
||||
const yrs = Math.trunc(offsetMoTotal / 12)
|
||||
const mos = offsetMoTotal % 12
|
||||
const sign = (offsetMoTotal + offsetDayTotal) < 0 ? '' : '+'
|
||||
const offsetLabel = sign + [
|
||||
yrs ? `${yrs}yr` : '',
|
||||
mos ? `${mos}mo` : '',
|
||||
offsetDayTotal ? `${offsetDayTotal}d` : '',
|
||||
].filter(Boolean).join(' ')
|
||||
ctx.fillStyle = '#64748b'
|
||||
ctx.font = '9px system-ui'
|
||||
ctx.textAlign = 'center'
|
||||
@ -156,7 +169,7 @@ export default function Timeline({ dateFrom, dateTo, offsetYr, offsetMo, type =
|
||||
}
|
||||
raf = requestAnimationFrame(draw)
|
||||
return () => cancelAnimationFrame(raf)
|
||||
}, [dateFrom, dateTo, offsetYr, offsetMo, type, twoBands, canvasH])
|
||||
}, [dateFrom, dateTo, offsetMoTotal, offsetDayTotal, type, twoBands, canvasH])
|
||||
|
||||
return <canvas ref={canvasRef} height={canvasH} style={{ width: '100%', display: 'block' }} />
|
||||
}
|
||||
|
||||
@ -48,11 +48,28 @@ function getDateRange(groups) {
|
||||
return null
|
||||
}
|
||||
|
||||
function parseOffset(offsetStr) {
|
||||
if (!offsetStr || offsetStr === '0 days') return { yr: 0, mo: 0 }
|
||||
const yr = parseInt(offsetStr.match(/(\d+)\s+year/)?.[1] || 0)
|
||||
const mo = parseInt(offsetStr.match(/(\d+)\s+month/)?.[1] || 0)
|
||||
return { yr, mo }
|
||||
// The offset is stored and sent as a Postgres interval, so it is typed as one --
|
||||
// "4 months", "1 year", "-90 days". This only parses it far enough to draw the
|
||||
// timeline preview; Postgres remains the authority on what is valid, and the
|
||||
// server rejects anything it will not accept.
|
||||
//
|
||||
// It replaced a pair of year/month number spinners, which could not express days
|
||||
// at all and were clamped at zero, so a segment could only ever be shifted
|
||||
// forward in whole months.
|
||||
export function parseInterval(str) {
|
||||
let months = 0, days = 0
|
||||
if (!str) return { months, days }
|
||||
const re = /([+-]?\d+(?:\.\d+)?)\s*(years?|yrs?|y|months?|mons?|mo|weeks?|wks?|w|days?|d)\b/gi
|
||||
for (const [, n, unit] of str.matchAll(re)) {
|
||||
const v = parseFloat(n)
|
||||
const u = unit.toLowerCase()
|
||||
if (u.startsWith('y')) months += v * 12
|
||||
else if (u.startsWith('mo') || u === 'mons' || u === 'mon') months += v
|
||||
else if (u.startsWith('w')) days += v * 7
|
||||
else if (u.startsWith('d')) days += v
|
||||
else months += v // 'm' alone reads as months here
|
||||
}
|
||||
return { months, days }
|
||||
}
|
||||
|
||||
function emptyCondition(cols) {
|
||||
@ -84,8 +101,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
|
||||
const [filters, setFilters] = useState([]) // [[cond,...], [cond,...]]
|
||||
const [useRaw, setUseRaw] = useState(false)
|
||||
const [rawSql, setRawSql] = useState('')
|
||||
const [offsetYr, setOffsetYr] = useState(0)
|
||||
const [offsetMo, setOffsetMo] = useState(0)
|
||||
const [offset, setOffset] = useState('0 days')
|
||||
const [segNote, setSegNote] = useState('')
|
||||
const [submitting, setSubmitting] = useState(false)
|
||||
const [editingLogId, setEditingLogId] = useState(null)
|
||||
@ -170,7 +186,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
|
||||
const clause = useRaw ? rawSql.trim() : buildFilterClause(filters)
|
||||
if (!clause) { flash(useRaw ? 'Enter a WHERE clause' : 'Add at least one filter', 'error'); return }
|
||||
const isRef = segType === 'reference'
|
||||
const offsetStr = [offsetYr > 0 ? `${offsetYr} year` : '', offsetMo > 0 ? `${offsetMo} month` : ''].filter(Boolean).join(' ') || '0 days'
|
||||
const offsetStr = offset.trim() || '0 days'
|
||||
const endpoint = isRef ? 'reference' : 'baseline'
|
||||
const body = {
|
||||
where_clause: clause,
|
||||
@ -212,9 +228,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
|
||||
setSegType(entry.operation)
|
||||
setSegNote(entry.note || '')
|
||||
setDescription('')
|
||||
const off = parseOffset(params.date_offset)
|
||||
setOffsetYr(off.yr)
|
||||
setOffsetMo(off.mo)
|
||||
setOffset(params.date_offset || '0 days')
|
||||
const groups = normalizeFilters(params.filters)
|
||||
if (groups) {
|
||||
setUseRaw(false)
|
||||
@ -478,8 +492,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
|
||||
rawSql={rawSql} setRawSql={setRawSql}
|
||||
description={description} setDescription={setDescription}
|
||||
segNote={segNote} setSegNote={setSegNote}
|
||||
offsetYr={offsetYr} setOffsetYr={setOffsetYr}
|
||||
offsetMo={offsetMo} setOffsetMo={setOffsetMo}
|
||||
offset={offset} setOffset={setOffset}
|
||||
filterCols={filterCols}
|
||||
onSubmit={loadSegment}
|
||||
submitting={submitting}
|
||||
@ -498,7 +511,6 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
|
||||
// derive view-mode props for a saved segment
|
||||
function segmentValuesFor(entry, filterCols) {
|
||||
const params = entry.params || {}
|
||||
const off = parseOffset(params.date_offset)
|
||||
const groups = normalizeFilters(params.filters)
|
||||
return {
|
||||
segType: entry.operation === 'reference' ? 'reference' : 'baseline',
|
||||
@ -507,8 +519,7 @@ function segmentValuesFor(entry, filterCols) {
|
||||
rawSql: params.where_clause || '',
|
||||
description: '',
|
||||
segNote: entry.note || '',
|
||||
offsetYr: off.yr,
|
||||
offsetMo: off.mo,
|
||||
offset: params.date_offset || '0 days',
|
||||
}
|
||||
}
|
||||
|
||||
@ -520,8 +531,7 @@ function SegmentForm({
|
||||
rawSql, setRawSql,
|
||||
description, setDescription,
|
||||
segNote, setSegNote,
|
||||
offsetYr, setOffsetYr,
|
||||
offsetMo, setOffsetMo,
|
||||
offset, setOffset,
|
||||
filterCols,
|
||||
onSubmit,
|
||||
submitting,
|
||||
@ -698,10 +708,19 @@ function SegmentForm({
|
||||
<div className="flex items-center gap-3">
|
||||
<label className="text-xs text-gray-500 w-28 shrink-0">Date offset</label>
|
||||
<div className="flex items-center gap-2">
|
||||
<input disabled={disabled} type="number" value={offsetYr} min={0} onChange={e => setOffsetYr(parseInt(e.target.value) || 0)} className={`${baseInp} text-sm w-16 text-center`} />
|
||||
<span className="text-xs text-gray-500">yr</span>
|
||||
<input disabled={disabled} type="number" value={offsetMo} min={0} max={11} onChange={e => setOffsetMo(parseInt(e.target.value) || 0)} className={`${baseInp} text-sm w-16 text-center`} />
|
||||
<span className="text-xs text-gray-500">mo</span>
|
||||
<input disabled={disabled} value={offset} list="pf-offset-options"
|
||||
onChange={e => setOffset(e.target.value)}
|
||||
placeholder="0 days"
|
||||
className={`${baseInp} text-sm w-32`} />
|
||||
<datalist id="pf-offset-options">
|
||||
<option value="12 months" />
|
||||
<option value="1 year" />
|
||||
<option value="4 months" />
|
||||
<option value="-90 days" />
|
||||
<option value="-12 months" />
|
||||
<option value="0 days" />
|
||||
</datalist>
|
||||
<span className="text-xs text-gray-400">any Postgres interval</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@ -712,8 +731,8 @@ function SegmentForm({
|
||||
<Timeline
|
||||
dateFrom={dateRange.from}
|
||||
dateTo={dateRange.to}
|
||||
offsetYr={offsetYr}
|
||||
offsetMo={offsetMo}
|
||||
offsetMonths={parseInterval(offset).months}
|
||||
offsetDays={parseInterval(offset).days}
|
||||
type={segType}
|
||||
/>
|
||||
</div>
|
||||
|
||||
Loading…
Reference in New Issue
Block a user