Skip to content
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

fix: uploading progress percentage #763

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

KristinAoki
Copy link
Member

JIRA Ticket: TNL-11308

When chunking the uploaded file was introduced, the upload progress percentage began to loop for each chunks upload. The looping of percentages is confusing to users as it seems like the upload is stuck in an infinite loop and they do not know how much the file has successfully uploaded. This PR fixes the progress bar so it updates after each chunk is uploaded.

Test

  1. Export the current content of your course
  2. Open the network tab in DevTools
  3. Import the recently exported file
  4. Progress percentage in Uploading step should only reach 100% one time even when the file is chunked multiple times (confirm number of chunks in network tab)

Copy link

codecov bot commented Dec 21, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (782fadd) 89.56% compared to head (a8bcddb) 89.56%.
Report is 1 commits behind head on master.

Files Patch % Lines
src/import-page/data/thunks.js 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #763   +/-   ##
=======================================
  Coverage   89.56%   89.56%           
=======================================
  Files         495      495           
  Lines        8069     8072    +3     
  Branches     1697     1698    +1     
=======================================
+ Hits         7227     7230    +3     
  Misses        815      815           
  Partials       27       27           

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

Copy link
Member

@jesperhodge jesperhodge left a comment

Choose a reason for hiding this comment

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

I checked this manually and this works. However, if this was bothersome enough that people actually filed a bug for it, then it also warrants us writing a test to ensure that we don't break it in the future.

Let's just add a test to the specific thing that was changed here until the codecov/patch pipeline passes. I'll approve the PR though.

Generally the api.js file seems barely test covered, and we should extend the test coverage here quite a bit anyway. Since it's mostly adding tests to existing code, I would suggest that we create a followup issue for this.

@KristinAoki KristinAoki merged commit 52b75e0 into master Jan 4, 2024
5 of 6 checks passed
@KristinAoki KristinAoki deleted the KristinAoki/fix-import-percentage branch January 4, 2024 14:36
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.

2 participants