Просмотр исходного кода

Fix per-job queue ETA showing for jobs that cannot start now

The scheduler only writes waiting_reason on the model-based assignment
path, so a job pinned to a specific printer sits behind a running print
with no marker at all. Every such job rendered an identical "starts now"
ETA that was wrong by the length of everything ahead of it.

Decide eligibility on the page instead: an item gets an ETA only when its
printer is idle and it is the item the scheduler would dispatch next,
following the same ordering the scheduler uses. Staged and future-
scheduled items do not block the item behind them, matching the
scheduler, and items conditional on a previous print are excluded.

The value also froze at first render, since react-query's structural
sharing keeps the queue reference stable and nothing re-rendered the row.
formatETA now accepts a base instant and the page drives it from a 30s
clock shared by every visible row.

Retire the borrowed printers.estimatedCompletion tooltip for a queue key
that says what the number means, translated into all 13 locales.
maziggy 1 месяц назад
Родитель
Сommit
30daed2756

+ 139 - 0
frontend/src/__tests__/pages/QueuePage.test.tsx

@@ -198,17 +198,156 @@ describe('QueuePage', () => {
     });
 
     it('shows one if-started-now ETA for an eligible pending item', async () => {
+      // Printer 1 is free: nothing is printing on it and nothing is queued ahead.
+      server.use(
+        http.get('/api/v1/queue/', () => {
+          return HttpResponse.json([mockQueueItems[0]]);
+        }),
+      );
+
+      render(<QueuePage />);
+
+      const name = await screen.findByText('Test Print 1');
+      const row = name.closest('.group');
+
+      expect(row).not.toBeNull();
+      const etaEl = within(row as HTMLElement).getAllByTestId('queue-item-eta');
+      expect(etaEl).toHaveLength(1);
+      // The tooltip must say what the number actually means, not reuse the
+      // printers-page "Estimated completion time" wording (which this is not).
+      expect(etaEl[0]).toHaveAttribute(
+        'title',
+        'Completion time if this job started now',
+      );
+    });
+
+    // The scheduler only writes waiting_reason on the model-based assignment
+    // path, so an item pinned to a specific printer carries no marker at all
+    // while it sits behind a running job. Without the printer-busy check every
+    // one of these quoted the same wrong "starts now" time.
+    it('does not show an ETA for an item pinned behind a running print', async () => {
       render(<QueuePage />);
 
+      // mockQueueItems[1] ("Active Print") is printing on printer 1, and
+      // "Test Print 1" is pending on the same printer with waiting_reason null.
       const name = await screen.findByText('Test Print 1');
       const row = name.closest('.group');
 
+      expect(row).not.toBeNull();
+      expect(
+        within(row as HTMLElement).queryByTestId('queue-item-eta'),
+      ).not.toBeInTheDocument();
+    });
+
+    it('shows the ETA only on the next item up when several share an idle printer', async () => {
+      server.use(
+        http.get('/api/v1/queue/', () => {
+          return HttpResponse.json([
+            { ...mockQueueItems[0], id: 10, position: 1, archive_name: 'First up' },
+            { ...mockQueueItems[0], id: 11, position: 2, archive_name: 'Second up' },
+            { ...mockQueueItems[0], id: 12, position: 3, archive_name: 'Third up' },
+          ]);
+        }),
+      );
+
+      render(<QueuePage />);
+
+      await screen.findByText('Third up');
+
+      const etaRowNames = screen
+        .queryAllByTestId('queue-item-eta')
+        .map((el) => el.closest('.group')?.textContent);
+
+      expect(etaRowNames).toHaveLength(1);
+      expect(etaRowNames[0]).toContain('First up');
+    });
+
+    it('shows an ETA for a staged item queued behind others on an idle printer', async () => {
+      // The scheduler skips manual-start items without claiming the printer, so
+      // a staged job is startable whenever its printer is free — queue order
+      // does not gate it.
+      server.use(
+        http.get('/api/v1/queue/', () => {
+          return HttpResponse.json([
+            { ...mockQueueItems[0], id: 20, position: 1, archive_name: 'Auto first' },
+            {
+              ...mockQueueItems[0],
+              id: 21,
+              position: 2,
+              archive_name: 'Staged second',
+              manual_start: true,
+            },
+          ]);
+        }),
+      );
+
+      render(<QueuePage />);
+
+      const name = await screen.findByText('Staged second');
+      const row = name.closest('.group');
+
       expect(row).not.toBeNull();
       expect(
         within(row as HTMLElement).getAllByTestId('queue-item-eta'),
       ).toHaveLength(1);
     });
 
+    it('does not show an ETA for an item conditional on a previous print', async () => {
+      server.use(
+        http.get('/api/v1/queue/', () => {
+          return HttpResponse.json([
+            {
+              ...mockQueueItems[0],
+              archive_name: 'Conditional Print',
+              require_previous_success: true,
+            },
+          ]);
+        }),
+      );
+
+      render(<QueuePage />);
+
+      const name = await screen.findByText('Conditional Print');
+      const row = name.closest('.group');
+
+      expect(row).not.toBeNull();
+      expect(
+        within(row as HTMLElement).queryByTestId('queue-item-eta'),
+      ).not.toBeInTheDocument();
+    });
+
+    it('advances the ETA as time passes', async () => {
+      vi.useFakeTimers({ shouldAdvanceTime: true });
+      vi.setSystemTime(new Date('2026-08-02T10:00:00Z'));
+
+      try {
+        server.use(
+          http.get('/api/v1/queue/', () => {
+            return HttpResponse.json([mockQueueItems[0]]);
+          }),
+        );
+
+        render(<QueuePage />);
+
+        const name = await screen.findByText('Test Print 1');
+        const row = name.closest('.group') as HTMLElement;
+        const before = within(row).getByTestId('queue-item-eta').textContent;
+
+        // The queue payload never changes, so react-query hands back the same
+        // object and nothing here re-renders on its own. Only the page's own
+        // clock can move this value.
+        await vi.advanceTimersByTimeAsync(45 * 60 * 1000);
+
+        await waitFor(() => {
+          expect(
+            within(row).getByTestId('queue-item-eta').textContent,
+          ).not.toBe(before);
+        });
+      } finally {
+        vi.useRealTimers();
+      }
+    });
+
     it('shows one if-started-now ETA for a staged item', async () => {
       server.use(
         http.get('/api/v1/queue/', () => {

+ 17 - 0
frontend/src/__tests__/utils/date.test.ts

@@ -339,6 +339,23 @@ describe('formatETA', () => {
     const result = formatETA(60 * 48); // 48 hours from now
     expect(result).not.toContain('Tomorrow');
   });
+
+  it('counts from baseTime when one is supplied', () => {
+    const base = new Date('2025-06-15T12:00:00Z').getTime();
+    // Same offset, two different starting instants: the results must differ by
+    // exactly the gap between those instants, not track the system clock.
+    const atNoon = formatETA(60, '24h', undefined, base);
+    const anHourLater = formatETA(60, '24h', undefined, base + 60 * 60 * 1000);
+
+    expect(atNoon).not.toBe(anHourLater);
+    expect(formatETA(60, '24h', undefined, base)).toBe(atNoon);
+    expect(formatETA(120, '24h', undefined, base)).toBe(anHourLater);
+  });
+
+  it('falls back to the system clock without baseTime', () => {
+    const explicit = formatETA(60, '24h', undefined, Date.now());
+    expect(formatETA(60, '24h')).toBe(explicit);
+  });
 });
 
 describe('formatDuration', () => {

+ 1 - 0
frontend/src/i18n/locales/de.ts

@@ -1293,6 +1293,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: 'Fertigstellungszeit, wenn dieser Auftrag jetzt starten würde',
       asap: 'Sofort',
       overdue: 'Überfällig',
       now: 'Jetzt',

+ 1 - 0
frontend/src/i18n/locales/en.ts

@@ -1308,6 +1308,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: 'Completion time if this job started now',
       asap: 'ASAP',
       overdue: 'Overdue',
       now: 'Now',

+ 1 - 0
frontend/src/i18n/locales/es.ts

@@ -1293,6 +1293,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: 'Hora de finalización si este trabajo comenzara ahora',
       asap: 'Lo antes posible',
       overdue: 'Atrasada',
       now: 'Ahora',

+ 1 - 0
frontend/src/i18n/locales/fr.ts

@@ -1293,6 +1293,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: 'Heure de fin si cette tâche démarrait maintenant',
       asap: 'Dès que possible',
       overdue: 'En retard',
       now: 'Maintenant',

+ 1 - 0
frontend/src/i18n/locales/it.ts

@@ -1293,6 +1293,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: 'Orario di completamento se questo lavoro iniziasse ora',
       asap: 'ASAP',
       overdue: 'Scaduto',
       now: 'Ora',

+ 1 - 0
frontend/src/i18n/locales/ja.ts

@@ -1292,6 +1292,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: 'このジョブを今開始した場合の完了予定時刻',
       asap: '即時',
       overdue: '期限超過',
       now: '今すぐ',

+ 1 - 0
frontend/src/i18n/locales/ko.ts

@@ -1223,6 +1223,7 @@ export default {
       description: '아카이브 페이지의 컨텍스트 메뉴에서 "예약" 옵션을 사용하거나 파일을 드래그 앤 드롭하여 시작하세요.'
     },
     time: {
+      etaIfStartedNow: '이 작업을 지금 시작할 경우의 완료 예정 시각',
       asap: '즉시',
       overdue: '기한 초과',
       now: '지금',

+ 1 - 0
frontend/src/i18n/locales/pt-BR.ts

@@ -1293,6 +1293,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: 'Horário de conclusão se este trabalho começasse agora',
       asap: 'ASAP',
       overdue: 'Atrasado',
       now: 'Agora',

+ 1 - 0
frontend/src/i18n/locales/ru.ts

@@ -1233,6 +1233,7 @@ export default {
       description: "Запланируйте печать на странице архива через пункт «Запланировать» в контекстном меню либо перетащите сюда файлы.",
     },
     time: {
+      etaIfStartedNow: "Время завершения, если запустить это задание сейчас",
       asap: "Как можно скорее",
       overdue: "Просрочено",
       now: "Сейчас",

+ 1 - 0
frontend/src/i18n/locales/tr.ts

@@ -1293,6 +1293,7 @@ export default {
     },
     // Zaman
     time: {
+      etaIfStartedNow: 'Bu iş şimdi başlatılırsa tamamlanma saati',
       asap: 'ASAP',
       overdue: 'Gecikmiş',
       now: 'Şimdi',

+ 1 - 0
frontend/src/i18n/locales/uk.ts

@@ -1308,6 +1308,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: "Час завершення, якщо запустити це завдання зараз",
       asap: "Якнайшвидше",
       overdue: "Прострочено",
       now: "Зараз",

+ 1 - 0
frontend/src/i18n/locales/zh-CN.ts

@@ -1293,6 +1293,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: '若此任务现在开始的预计完成时间',
       asap: '尽快',
       overdue: '已逾期',
       now: '现在',

+ 1 - 0
frontend/src/i18n/locales/zh-TW.ts

@@ -1293,6 +1293,7 @@ export default {
     },
     // Time
     time: {
+      etaIfStartedNow: '若此工作現在開始的預計完成時間',
       asap: '儘快',
       overdue: '已逾期',
       now: '現在',

+ 131 - 9
frontend/src/pages/QueuePage.tsx

@@ -356,6 +356,8 @@ function SortableQueueItem({
   hasPermission,
   canModify,
   printerState,
+  showEta = false,
+  etaNow,
   t,
 }: {
   item: PrintQueueItem;
@@ -377,6 +379,11 @@ function SortableQueueItem({
   hasPermission: (permission: Permission) => boolean;
   canModify: (resource: 'queue' | 'archives' | 'library', action: 'update' | 'delete' | 'reprint', createdById: number | null | undefined) => boolean;
   printerState?: string | null;
+  // Whether this item qualifies for an "if started now" ETA (#2740), and the
+  // instant to measure it from. Both are decided by the page so every row on
+  // screen quotes the same clock.
+  showEta?: boolean;
+  etaNow?: number;
   t: (key: string, options?: Record<string, unknown>) => string;
 }) {
   // Fetch printer status every 30 seconds while printing to monitor progress
@@ -428,15 +435,14 @@ function SortableQueueItem({
   const isPending = item.status === 'pending';
   const isHistory = ['completed', 'failed', 'skipped', 'cancelled'].includes(item.status);
 
-  // This is an "if started now" estimate, not a cumulative queue forecast.
-  // Do not show it for active, blocked, scheduled, or invalid queue items.
+  // This is an "if started now" estimate, not a cumulative queue forecast, so
+  // it is only shown for items the page determined could actually start now
+  // (see etaEligibleIds). etaNow is the caller's ticking clock — deriving the
+  // ETA from it rather than from Date.now() keeps this render deterministic and
+  // stops the value freezing at first paint.
   const queueItemEta =
-    isPending &&
-    !item.waiting_reason &&
-    !item.scheduled_time &&
-    item.print_time_seconds != null &&
-    item.print_time_seconds > 0
-      ? formatETA(item.print_time_seconds / 60, timeFormat, t)
+    isPending && showEta && item.print_time_seconds != null && item.print_time_seconds > 0
+      ? formatETA(item.print_time_seconds / 60, timeFormat, t, etaNow)
       : null;
 
   const isMobileSelectable = isPending && onToggleSelect;
@@ -618,7 +624,7 @@ function SortableQueueItem({
               <span
                 data-testid="queue-item-eta"
                 className="text-bambu-green font-medium"
-                title={t('printers.estimatedCompletion')}
+                title={t('queue.time.etaIfStartedNow')}
               >
                 ETA {queueItemEta}
               </span>
@@ -877,6 +883,10 @@ interface QueueRowRenderProps {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   canModify: (resource: any, action: any, createdById?: number | null) => boolean;
   t: (key: string, options?: Record<string, unknown>) => string;
+  // Items that qualify for an "if started now" ETA, and the shared clock it is
+  // measured from (#2740).
+  etaEligibleIds: Set<number>;
+  etaNow: number;
   aggregateForRows: (rows: QueueRow[]) => { count: number; time: number; weight: number };
   // Mobile tap-to-reorder (#2667). onMoveUp/onMoveDown move this whole row
   // (single item or batch) one step among its siblings; onMoveBlock is the
@@ -902,6 +912,8 @@ function QueueRowRender(props: QueueRowRenderProps) {
     hasPermission,
     canModify,
     t,
+    etaEligibleIds,
+    etaNow,
     onMoveUp,
     onMoveDown,
   } = props;
@@ -923,6 +935,8 @@ function QueueRowRender(props: QueueRowRenderProps) {
         onToggleSelect={() => handleToggleSelect(row.item.id)}
         hasPermission={hasPermission}
         canModify={canModify}
+        showEta={etaEligibleIds.has(row.item.id)}
+        etaNow={etaNow}
         t={t}
       />
     );
@@ -948,6 +962,8 @@ function SortableBatchRow({
   hasPermission,
   canModify,
   t,
+  etaEligibleIds,
+  etaNow,
   aggregateForRows,
   onMoveUp,
   onMoveDown,
@@ -1139,6 +1155,8 @@ function SortableBatchRow({
               onToggleSelect={() => handleToggleSelect(child.id)}
               hasPermission={hasPermission}
               canModify={canModify}
+              showEta={etaEligibleIds.has(child.id)}
+              etaNow={etaNow}
               t={t}
             />
           ))}
@@ -1779,6 +1797,106 @@ export function QueuePage() {
     return items;
   }, [queue, filterLocation, matchesLocationFilter]);
 
+  // Queue items eligible for an "if started now" ETA (#2740).
+  //
+  // The ETA answers "when would this finish if it began right now", so it may
+  // only appear on items that really could begin right now. Deriving that from
+  // waiting_reason alone is not enough: the scheduler only writes that field on
+  // the model-based assignment path (print_scheduler.py), so an item pinned to a
+  // specific printer sits behind a running job with waiting_reason still NULL.
+  //
+  // Computed from the unfiltered queue on purpose — hiding a printer behind the
+  // location filter must not make its printer look free.
+  const etaEligibleIds = useMemo(() => {
+    const eligible = new Set<number>();
+    if (!queue) return eligible;
+
+    const busyPrinters = new Set<number>();
+    queue.forEach(item => {
+      if (item.status === 'printing' && item.printer_id) busyPrinters.add(item.printer_id);
+    });
+
+    const isFutureScheduled = (item: PrintQueueItem): boolean => {
+      if (!item.scheduled_time) return false;
+      return (parseUTCDate(item.scheduled_time)?.getTime() ?? 0) > Date.now();
+    };
+
+    // Mirrors the scheduler's own ordering so "next up" here means the item the
+    // scheduler would actually dispatch next, not whatever the user sorted by.
+    const schedulerOrder = (a: PrintQueueItem, b: PrintQueueItem): number => {
+      if (settings?.queue_shortest_first) {
+        const aJumped = a.been_jumped ? 1 : 0;
+        const bJumped = b.been_jumped ? 1 : 0;
+        if (aJumped !== bJumped) return bJumped - aJumped;
+        const aTime = a.print_time_seconds ?? Infinity;
+        const bTime = b.print_time_seconds ?? Infinity;
+        if (aTime !== bTime) return aTime - bTime;
+      }
+      return a.position - b.position;
+    };
+
+    // Claimants for each printer, in the order the scheduler would take them.
+    // Staged and future-scheduled items are excluded: the scheduler skips both
+    // without marking the printer busy, so neither holds up the item behind it.
+    const contenders = new Map<number, PrintQueueItem[]>();
+    queue
+      .filter(
+        item =>
+          item.status === 'pending' &&
+          item.printer_id != null &&
+          !item.manual_start &&
+          !isFutureScheduled(item)
+      )
+      .sort(schedulerOrder)
+      .forEach(item => {
+        const list = contenders.get(item.printer_id!) ?? [];
+        list.push(item);
+        contenders.set(item.printer_id!, list);
+      });
+
+    queue.forEach(item => {
+      if (item.status !== 'pending') return;
+      // Blocked, scheduled for later, or no usable duration to add.
+      if (item.waiting_reason) return;
+      if (isFutureScheduled(item)) return;
+      if (item.print_time_seconds == null || item.print_time_seconds <= 0) return;
+      // Conditional on an earlier print's outcome, which the UI cannot see: the
+      // scheduler may skip it outright rather than ever running it.
+      if (item.require_previous_success) return;
+
+      // Model-based items have no printer yet; an empty waiting_reason is the
+      // scheduler saying it found one, so trust that.
+      if (item.printer_id == null) {
+        eligible.add(item.id);
+        return;
+      }
+
+      if (busyPrinters.has(item.printer_id)) return;
+      // Staged items wait on the user, not on the queue, so they are startable
+      // whenever their printer is free regardless of what is queued ahead.
+      if (item.manual_start) {
+        eligible.add(item.id);
+        return;
+      }
+      if (contenders.get(item.printer_id)?.[0]?.id === item.id) eligible.add(item.id);
+    });
+
+    return eligible;
+  }, [queue, settings?.queue_shortest_first]);
+
+  // The ETA is "now + duration", so it goes stale on its own. Nothing else
+  // re-renders these rows while the queue payload is unchanged (react-query's
+  // structural sharing keeps the reference stable), so drive it from a clock of
+  // our own. Only runs while an ETA is actually on screen.
+  const [etaNow, setEtaNow] = useState(() => Date.now());
+  const hasEtas = etaEligibleIds.size > 0;
+  useEffect(() => {
+    if (!hasEtas) return;
+    setEtaNow(Date.now());
+    const id = setInterval(() => setEtaNow(Date.now()), 30000);
+    return () => clearInterval(id);
+  }, [hasEtas]);
+
   // Get unique printer IDs from active items to fetch their statuses
   const activePrinterIds = useMemo(() => {
     const ids = new Set<number>();
@@ -2567,6 +2685,8 @@ export function QueuePage() {
                           hasPermission={hasPermission}
                           canModify={canModify}
                           t={t}
+                          etaEligibleIds={etaEligibleIds}
+                          etaNow={etaNow}
                           aggregateForRows={aggregateForRows}
                           {...rowMovers(groupedRows, idx)}
                           onMoveBlock={canReorderManually ? moveBlockRelativeTo : undefined}
@@ -2605,6 +2725,8 @@ export function QueuePage() {
                                   hasPermission={hasPermission}
                                   canModify={canModify}
                                   t={t}
+                                  etaEligibleIds={etaEligibleIds}
+                                  etaNow={etaNow}
                                   aggregateForRows={aggregateForRows}
                                   {...rowMovers(bucket.rows, idx)}
                                   onMoveBlock={canReorderManually ? moveBlockRelativeTo : undefined}

+ 8 - 2
frontend/src/utils/date.ts

@@ -319,14 +319,20 @@ export function formatTimeOnly(
  * @param remainingMinutes - Minutes until completion
  * @param timeFormat - Time format setting ('system', '12h', '24h')
  * @param t - Optional i18n translation function
+ * @param baseTime - Instant to count from, in epoch ms. Defaults to the current
+ *   clock. Callers that render an ETA for something not yet started must pass a
+ *   value that changes over time, or the string freezes at first render: it is
+ *   only recomputed when the component re-renders, which does not happen while
+ *   the underlying data is unchanged (#2740).
  * @returns Formatted ETA string (e.g., "3:45 PM", "Tomorrow 9:30 AM", "Wed 2:00 PM")
  */
 export function formatETA(
   remainingMinutes: number,
   timeFormat: TimeFormat = 'system',
-  t?: (key: string) => string
+  t?: (key: string) => string,
+  baseTime?: number
 ): string {
-  const now = new Date();
+  const now = baseTime != null ? new Date(baseTime) : new Date();
   const eta = new Date(now.getTime() + remainingMinutes * 60 * 1000);
 
   const today = new Date(now);

Разница между файлами не показана из-за своего большого размера
+ 0 - 0
static/assets/index-DPZgvI9N.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-CCCWDEkl.js"></script>
+    <script type="module" crossorigin src="/assets/index-DPZgvI9N.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-C_6BSgrK.css">
   </head>
   <body>

Некоторые файлы не были показаны из-за большого количества измененных файлов