Skip to content

[PB-6328/PB-6333]: feat/validate file size before upload#1957

Merged
xabg2 merged 17 commits into
masterfrom
feature/max-file-size-limit-per-tier
May 20, 2026
Merged

[PB-6328/PB-6333]: feat/validate file size before upload#1957
xabg2 merged 17 commits into
masterfrom
feature/max-file-size-limit-per-tier

Conversation

@xabg2
Copy link
Copy Markdown
Contributor

@xabg2 xabg2 commented May 14, 2026

Description

Implemented a pre-validation step before starting uploads to filter out files that exceed the maximum upload size allowed for the user’s current tier.

If a file is larger than the allowed limit, a dialog is displayed informing the user about:

  • their current upload size limit
  • why the upload cannot proceed
  • the next available tier required to upload larger files

This prevents invalid uploads from starting and provides clearer upgrade guidance to the user.

Related Issues

Related Pull Requests

Checklist

  • Changes have been tested locally.
  • Unit tests have been written or updated as necessary.
  • The code adheres to the repository's coding standards.
  • Relevant documentation has been added or updated.
  • No new warnings or errors have been introduced.
  • SonarCloud issues have been reviewed and addressed.
  • QA Passed

Testing Process

Additional Notes

This can be safely merged. If the server does not provide a maxFileSizeLimit, we allow the upload. The backend will check the file size limit if needed. So, this not affects to the current flow fn until we activate the limit in the backend.

@xabg2 xabg2 self-assigned this May 14, 2026
@xabg2 xabg2 added the enhancement New feature or request label May 14, 2026
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 14, 2026

Deploying drive-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1a1063b
Status: ✅  Deploy successful!
Preview URL: https://a956d774.drive-web.pages.dev
Branch Preview URL: https://feature-max-file-size-limit.drive-web.pages.dev

View logs

@xabg2 xabg2 changed the base branch from master to feature/max-file-size-limit-dialog May 18, 2026 19:31

return {
isAuthenticated: state.user.isAuthenticated,
maxUploadFileSize: state.fileVersions.limits?.maxUploadFileSize ?? undefined,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I know it looks a bit weird to get the max upload file size from fileVersions.limit. The issue is that this value actually represents the user's Drive limits based on their tier, but when it was originally introduced it was named fileVersions.limits instead of driveLimits or smth like that. We should eventually refactor this and rename it to something more generic, like limits, to better reflect its real purpose.

It will be done in another PR so we don't go out of scope.

Base automatically changed from feature/max-file-size-limit-dialog to master May 19, 2026 10:20
@xabg2 xabg2 marked this pull request as ready for review May 19, 2026 10:26
@xabg2 xabg2 requested review from a team, CandelR and larryrider as code owners May 19, 2026 10:26
}),
);

const openReachedPlanLimitDialog = () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we make it a common constant for both?

rootFolderItem ??= createdFolder;

if (!rootFolderData) {
rootFolderData = createdFolder;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we already did rootFolderItem ??= createdFolder; just a line before, this if won't help

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rootFolderItem and rootFolderData are different

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ops, you are right, it's another one

expect(uploadItemsParallelThunk).toHaveBeenCalledWith(expect.objectContaining({ files: [smallFile, bigFile] }));
});

test('When no size limit is configured, then all files are uploaded regardless of size', async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't we have some max limit for all to use instead of infinite?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I can limit it to 40GB as default (the max size we currently have right now in prod).

@xabg2 xabg2 requested a review from TamaraFinogina May 19, 2026 15:27
});

test('When no size limit is configured, then all files are uploaded regardless of size', async () => {
test('When no size limit is configured, then all files above the default size limit are uploaded', async () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe below and not above?

}

const allowedFilesToUpload = validateFileSize(dispatch, files, maxFileSize);
const allowedFilesToUpload = validateFileSize(dispatch, files, maxFileSize as number);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sonar says no need for 'as number'

@sonarqubecloud
Copy link
Copy Markdown

@xabg2 xabg2 requested a review from TamaraFinogina May 20, 2026 07:01
@xabg2 xabg2 merged commit e794b65 into master May 20, 2026
10 of 11 checks passed
@xabg2 xabg2 deleted the feature/max-file-size-limit-per-tier branch May 20, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants