Skip to content

fix(FR-2777): reset upload status when a new cycle starts after the previous one ended#7160

Open
ironAiken2 wants to merge 1 commit intomainfrom
fix/FR-2777-reset-upload-status-on-new-cycle
Open

fix(FR-2777): reset upload status when a new cycle starts after the previous one ended#7160
ironAiken2 wants to merge 1 commit intomainfrom
fix/FR-2777-reset-upload-status-on-new-cycle

Conversation

@ironAiken2
Copy link
Copy Markdown
Contributor

@ironAiken2 ironAiken2 commented Apr 30, 2026

Resolves #7159 (FR-2777)

Summary

When a file upload fails, the failed file remained in the upload progress counter and the next upload's notification reused the same key, so leftover state from the previous cycle polluted the new cycle's display (e.g., a single new file showed 1 / 3 because previously-failed files were still being counted, and the previous cycle's resolved/rejected notification was overwritten by the new progress notification).

Two changes:

  • Reset on new cycle start. In the uploadRequests effect, detect "previous cycle ended" via _.isEmpty(prev[vFolderId]?.pendingFiles). When ended, reset completedFiles, failedFiles, completedBytes, and totalExpectedSize before adding the new batch. Removed the partial reset that previously ran on the resolved-notification branch so all reset logic lives in one place.
  • Per-cycle notification key. Added cycleKey to UploadStatusInfo. A fresh upload:${vFolderId}:${Date.now()} is generated when a new cycle starts; the same key is reused while the cycle is in progress. Progress, throttled progress updates, resolved, and rejected notifications all use this cycleKey. The previous cycle's result notification (resolved auto-closes after 3s; rejected stays until dismissed) is no longer overwritten by the next cycle's progress notification.

Mid-cycle adds still accumulate (e.g., 2 / 32 / 4) and mid-cycle failures still keep the failed file in the denominator until the cycle ends, matching current behavior.

Test plan

  • Upload a file that fails. Confirm the failure notification appears and stays visible.
  • Without dismissing the failure notification, start a brand-new upload of a single file. Confirm a separate progress notification appears showing 0 / 1 (not 0 / 2), and the previous failure notification is still visible alongside it.
  • During an in-progress upload of multiple files, drop additional files in. Confirm the denominator increases (e.g., 2 / 32 / 4) within the same notification instead of opening a new one.
  • Mix success and failure within one cycle. Confirm progress accumulates within the cycle, the cycle's final notification shows the failure list, and the next new cycle starts as a clean separate notification.
  • After a fully-successful upload, start another upload before the resolved toast auto-closes. Confirm the new upload appears as a separate progress notification.

Copy link
Copy Markdown
Contributor Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added the size:M 30~100 LoC label Apr 30, 2026
@ironAiken2 ironAiken2 marked this pull request as ready for review April 30, 2026 02:52
Copilot AI review requested due to automatic review settings April 30, 2026 02:52
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2026

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
8.67% (-0% 🔻)
1859/21431
🔴 Branches
7.85% (-0.01% 🔻)
1187/15119
🔴 Functions
5.13% (+0.02% 🔼)
297/5785
🔴 Lines
8.41% (-0% 🔻)
1750/20799

Test suite run success

865 tests passing in 40 suites.

Report generated by 🧪jest coverage report action from 9551bb1

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes upload progress counter leakage across upload cycles in the WebUI’s upload notification manager by resetting accumulated per-folder upload status at the start of a new upload cycle (after the previous one fully ended), instead of partially resetting on success.

Changes:

  • Detect “previous cycle ended” when the prior pendingFiles list is empty and reset accumulated counters (completedFiles, failedFiles, completedBytes, totalExpectedSize) before adding the new batch.
  • Recompute progress percent based on reset/accumulated totals as appropriate.
  • Remove the prior “resolved-branch-only” partial reset so reset logic is centralized at cycle start.
Comments suppressed due to low confidence (1)

react/src/components/FileUploadManager.tsx:211

  • upsertNotification() merges with the existing notification state (via _.merge in useBAINotification), so if the previous cycle ended in failure the old extraDescription (failed file list) will persist when you start a new pending upload unless you explicitly clear it. This can leave stale failed filenames visible during the next upload cycle.

Consider explicitly setting extraDescription: null (and description: null/undefined if applicable) in this “start upload cycle” notification update so the UI is guaranteed to start clean.

        upsertNotification({
          key: 'upload:' + vFolderId,
          open: true,
          message: (
            <span>
              {t('explorer.VFolder')}:&nbsp;
              <BAILink
                style={{
                  fontWeight: 'normal',
                }}
                to={generateFolderPath(vFolderId)}
                onClick={() => {
                  closeNotification('upload:' + vFolderId);
                }}
              >{`${vFolderName}`}</BAILink>
            </span>
          ),
          backgroundTask: {
            status: 'pending',
            percent: currPct,
            onChange: {
              pending: t('explorer.ProcessingUpload'),
            },
          },
          duration: 0,
        });

Comment thread react/src/components/FileUploadManager.tsx
@ironAiken2 ironAiken2 force-pushed the fix/FR-2777-reset-upload-status-on-new-cycle branch from 36a8fd4 to bd724e4 Compare April 30, 2026 04:39
@ironAiken2 ironAiken2 force-pushed the fix/FR-2777-reset-upload-status-on-new-cycle branch from bd724e4 to 9551bb1 Compare April 30, 2026 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reset upload status when a new upload cycle starts after the previous one ended

2 participants