Skip to content

feat(server/fileuploads): use a presigned url to upload large files #4901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

iainsproat
Copy link
Contributor

@iainsproat iainsproat commented Jun 6, 2025

Description & motivation

The client can request a pre-signed URL via the GraphQL API.

Once the client has uploaded a file using the presigned url, this PR allows the client to register the completion of the upload via GraphQL API.

Changes:

  • graphql mutations for:

    • fileuploads
      • generateUploadUrl
      • startFileImport
  • environment variables:

    • FF_LARGE_FILE_IMPORTS_ENABLED
    • FILE_UPLOAD_URL_EXPIRY_MINUTES

To-do before merge:

Screenshots:

Validation of changes:

Checklist:

  • My pull request follows the guidelines in the Contributing guide?
  • My pull request does not duplicate any other open Pull Requests for the same update/change?
  • My commits are related to the pull request and do not amend unrelated code or documentation.
  • My code follows a similar style to existing code.
  • I have added appropriate tests.
  • I have updated or added relevant documentation.

References

Copy link

linear bot commented Jun 6, 2025

@@ -41,12 +41,9 @@ import {
import {
getBranchByIdFactory,
markCommitBranchUpdatedFactory,
getStreamBranchByNameFactory,
createBranchFactory
getStreamBranchByNameFactory
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the createStream and all the boilerplate in to a helper function so it can be shared amongst tests. Otherwise, no changes to this file.

Copy link

codecov bot commented Jun 9, 2025

Codecov Report

Attention: Patch coverage is 73.27935% with 66 lines in your changes missing coverage. Please review.

Project coverage is 69.64%. Comparing base (98632f4) to head (d8bbffe).

Files with missing lines Patch % Lines
packages/shared/src/blobs/fileTypes.ts 0.00% 32 Missing ⚠️
...modules/fileuploads/graph/resolvers/fileUploads.ts 84.61% 4 Missing and 4 partials ⚠️
...s/server/modules/fileuploads/services/presigned.ts 63.63% 8 Missing ⚠️
...er/modules/fileuploads/repositories/fileUploads.ts 40.00% 6 Missing ⚠️
...s/server/modules/blobstorage/services/presigned.ts 94.00% 1 Missing and 2 partials ⚠️
...ver/modules/fileuploads/services/resultListener.ts 0.00% 3 Missing ⚠️
packages/shared/src/authz/policies/index.ts 25.00% 3 Missing ⚠️
...s/server/modules/blobstorage/repositories/index.ts 87.50% 0 Missing and 1 partial ⚠️
packages/shared/src/blobs/index.ts 0.00% 1 Missing ⚠️
packages/shared/src/index.ts 0.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (73.27%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (69.64%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4901      +/-   ##
==========================================
- Coverage   70.54%   69.64%   -0.90%     
==========================================
  Files         587      602      +15     
  Lines       25300    25888     +588     
  Branches     3777     3816      +39     
==========================================
+ Hits        17847    18029     +182     
- Misses       6273     6700     +427     
+ Partials     1180     1159      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@iainsproat iainsproat marked this pull request as ready for review June 9, 2025 13:37
@iainsproat iainsproat requested review from fabis94 and danielgak June 9, 2025 13:39
@iainsproat
Copy link
Contributor Author

iainsproat commented Jun 10, 2025

Review comments

  • Support old gen file import
  • Use project.canPublish policy

@iainsproat iainsproat marked this pull request as draft June 10, 2025 12:32
@iainsproat iainsproat marked this pull request as ready for review June 10, 2025 15:03
@iainsproat iainsproat requested a review from gjedlicska June 10, 2025 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant