Browse Source

Print the payment panel and the title strip in the order their fields are in (#351)

Both blocks read the designer's field list into a Set and then printed from
their own fixed order: the payment panel walked the document's pairs, the
title strip a hardcoded list of cells. Switching a row off worked, because
that only asks whether an id is in the set. Dragging a row did nothing, and
the inspector showed the new order while the sheet kept the old one.

They now walk the field list itself, the way every panel already does. A pair
with no field of its own, which is how a workshop-defined line arrives,
follows the ones that have.
Bernt Christian Egeland 3 weeks ago
parent
commit
3f8e2c9c94

+ 112 - 0
src/__tests__/features/invoice-designer/field-order.test.ts

@@ -0,0 +1,112 @@
+/**
+ * The designer's field list is a running order, not just a set of switches.
+ * Two blocks used to read it as a set and print their rows in the order they
+ * happened to build them, so dragging a row in the inspector moved nothing on
+ * the sheet.
+ */
+import { describe, expect, it } from 'vitest'
+import { buildSampleData } from '@/features/invoice-designer/Components/sample'
+import { mergeWithDefaults } from '@/features/settings/Schema/invoiceLayoutSchema'
+import { buildDocumentSpec } from '@/features/invoice-designer/Spec/buildSpec'
+import { themeOf } from '@/features/invoice-designer/Components/designTheme'
+import type { InvoiceFieldConfig } from '@/features/settings/Schema/invoiceLayoutSchema'
+
+/* eslint-disable @typescript-eslint/no-explicit-any */
+const sample = () =>
+  buildSampleData(
+    { name: 'Shop', address: '', phone: '', email: '', logoUrl: null } as any,
+    [],
+    ((key: string) => key) as any,
+    {},
+    'invoice'
+  )
+
+/** The spec a layout prints, with one section's field list rewritten. */
+function specWithFields(sectionId: string, fields: InvoiceFieldConfig[]) {
+  const layout = mergeWithDefaults({})
+  layout.sections = layout.sections.map((s) => (s.id === sectionId ? { ...s, fields } : s))
+  return buildDocumentSpec(layout, themeOf({} as any, layout), sample())
+}
+
+function blockContent(spec: any, sectionId: string) {
+  return spec.blocks.find((b: any) => b.content?.id === sectionId)?.content
+}
+
+/** Ids of the rows inside a block, in the order they print. */
+function rowIds(node: any, skip: string): string[] {
+  const ids: string[] = []
+  const walk = (n: any) => {
+    if (!n || typeof n !== 'object') return
+    if (n.id && n.id !== skip) ids.push(n.id)
+    for (const child of n.children ?? []) walk(child.node ?? child)
+  }
+  walk(node)
+  return ids
+}
+
+/** Text of every string the block prints, in order. */
+function texts(node: any): string[] {
+  const out: string[] = []
+  const walk = (n: any) => {
+    if (!n || typeof n !== 'object') return
+    if (n.kind === 'text' && n.text) out.push(n.text)
+    for (const child of n.children ?? []) walk(child.node ?? child)
+  }
+  walk(node)
+  return out
+}
+
+const on = (ids: string[]): InvoiceFieldConfig[] => ids.map((id) => ({ id, visible: true }))
+
+describe('payment information order', () => {
+  const defaults = ['bank_account', 'org_number', 'payment_terms', 'due_date']
+
+  it('prints the rows in the order the field list is in', () => {
+    const spec = specWithFields('bank_account', on(defaults))
+    expect(rowIds(blockContent(spec, 'bank_account'), 'bank_account')).toEqual(
+      defaults.filter((id) => id !== 'bank_account')
+    )
+  })
+
+  it('follows a row dragged to the top', () => {
+    const moved = ['due_date', 'bank_account', 'org_number', 'payment_terms']
+    const spec = specWithFields('bank_account', on(moved))
+    const ids = rowIds(blockContent(spec, 'bank_account'), 'bank_account')
+    expect(ids[0]).toBe('due_date')
+    expect(ids).toEqual(moved.filter((id) => id !== 'bank_account'))
+  })
+
+  it('still leaves out a row that is switched off', () => {
+    const spec = specWithFields('bank_account', [
+      { id: 'due_date', visible: true },
+      { id: 'bank_account', visible: false },
+      { id: 'org_number', visible: true },
+      { id: 'payment_terms', visible: false },
+    ])
+    const printed = texts(blockContent(spec, 'bank_account'))
+    expect(printed.join(' ')).not.toContain('Bank Account')
+    expect(printed.join(' ')).not.toContain('Payment Terms')
+  })
+})
+
+describe('document title strip order', () => {
+  it('prints the cells in the order the field list is in', () => {
+    const spec = specWithFields(
+      'document_title',
+      on(['title', 'due_date', 'date', 'invoice_number'])
+    )
+    const printed = texts(blockContent(spec, 'document_title'))
+    expect(printed.indexOf('Due')).toBeLessThan(printed.indexOf('Date'))
+    expect(printed.indexOf('Date')).toBeLessThan(printed.indexOf('Invoice No.'))
+  })
+
+  it('keeps the old order when nobody has dragged anything', () => {
+    const spec = specWithFields(
+      'document_title',
+      on(['title', 'invoice_number', 'date', 'due_date'])
+    )
+    const printed = texts(blockContent(spec, 'document_title'))
+    expect(printed.indexOf('Invoice No.')).toBeLessThan(printed.indexOf('Date'))
+    expect(printed.indexOf('Date')).toBeLessThan(printed.indexOf('Due'))
+  })
+})

+ 38 - 17
src/features/invoice-designer/Spec/buildSpec.ts

@@ -800,21 +800,32 @@ function documentTitle(
   const size = look.fontSize ?? theme.fontSize
   const size = look.fontSize ?? theme.fontSize
   // What the strip is asked to say. A cell also needs something to say: a job
   // What the strip is asked to say. A cell also needs something to say: a job
   // with no customer number has no such cell, switch or no switch.
   // with no customer number has no such cell, switch or no switch.
-  const fields = new Set(sectionFields(section))
-  const cells = [
-    fields.has('invoice_number')
-      ? [label(data, 'invoiceNumberLabel', 'Invoice No.'), data.meta.number]
-      : null,
-    data.meta.customerNumber && fields.has('customer_number')
-      ? [label(data, 'customerNumberLabel', 'Customer No.'), data.meta.customerNumber]
-      : null,
-    fields.has('date') ? [label(data, 'dateLabel', 'Date'), data.meta.date] : null,
-    data.meta.due && fields.has('due_date')
-      ? [label(data, 'dueDateLabel', 'Due'), data.meta.due]
-      : null,
-  ].filter(Boolean) as [string, string][]
-
-  const showTitle = fields.has('title')
+  const fields = sectionFields(section)
+  // Each cell as the strip would print it, or nothing when the job has no
+  // such value. Walked in the field list's own order, so dragging a row in
+  // the designer moves the cell on the sheet.
+  const cellFor = (id: string): [string, string] | null => {
+    switch (id) {
+      case 'invoice_number':
+        return [label(data, 'invoiceNumberLabel', 'Invoice No.'), data.meta.number]
+      case 'customer_number':
+        return data.meta.customerNumber
+          ? [label(data, 'customerNumberLabel', 'Customer No.'), data.meta.customerNumber]
+          : null
+      case 'date':
+        return [label(data, 'dateLabel', 'Date'), data.meta.date]
+      case 'due_date':
+        return data.meta.due ? [label(data, 'dueDateLabel', 'Due'), data.meta.due] : null
+      default:
+        return null
+    }
+  }
+  const cells = fields.flatMap((id) => {
+    const cell = cellFor(id)
+    return cell ? [cell] : []
+  })
+
+  const showTitle = fields.includes('title')
   // Everything switched off is a strip with nothing to print, and an empty
   // Everything switched off is a strip with nothing to print, and an empty
   // block would still take its room and its rule on the sheet.
   // block would still take its room and its rule on the sheet.
   if (!showTitle && !cells.length) return null
   if (!showTitle && !cells.length) return null
@@ -1420,8 +1431,18 @@ function paymentBlock(
   theme: DocumentTheme,
   theme: DocumentTheme,
   data: DocumentData
   data: DocumentData
 ): Node | null {
 ): Node | null {
-  const fields = new Set(sectionFields(section))
-  const pairs = data.payment.filter((pair) => (pair.id ? fields.has(pair.id) : true))
+  // In the order the designer's field list is in, not the order the document
+  // happens to build its pairs: dragging a row up there has to move it here,
+  // the way it does in every panel.
+  const fields = sectionFields(section)
+  const byId = new Map(data.payment.filter((pair) => pair.id).map((pair) => [pair.id, pair]))
+  const pairs: PaymentPair[] = fields.flatMap((id) => {
+    const pair = byId.get(id)
+    return pair ? [pair] : []
+  })
+  // A pair with no id has no row of its own to drag, so it follows the ones
+  // that do rather than disappearing.
+  pairs.push(...data.payment.filter((pair) => !pair.id))
   // Workshop-defined fields assigned here join as label-over-value pairs.
   // Workshop-defined fields assigned here join as label-over-value pairs.
   // Their lines arrive worded as "Label: value", so split at the first colon.
   // Their lines arrive worded as "Label: value", so split at the first colon.
   for (const entry of customFieldEntries(section, data)) {
   for (const entry of customFieldEntries(section, data)) {