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: LSDV-4907: Import - validate filetypes on the FE #4515

Merged
merged 10 commits into from
Jul 18, 2023

Conversation

jombooth
Copy link
Contributor

@jombooth jombooth commented Jul 14, 2023

PR fulfills these requirements

  • Commit message(s) and PR title follows the format [fix|feat|ci|chore|doc]: TICKET-ID: Short description of change made ex. fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errors
  • Tests for the changes have been added/updated (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • Best efforts were made to ensure docs/code are concise and coherent (checked for spelling/grammatical errors, commented out code, debug logs etc.)
  • Self-reviewed and ran all changes on a local instance (for bug fixes/features)

Change has impacts in these area(s)

(check all that apply)

  • Product design
  • Backend (Database)
  • Backend (API)
  • Frontend

Describe the reason for change

https://labelstudio.aha.io/develop/comments/7247239546298336794

We have filetype validation on the backend, but we want it on the frontend also to show more helpful error messages to users, and avoid server calls that will 4xx. We could check filetypes in greater depth on the FE but since we also have backend validation I feel that's not necessary and this lightweight approach will suffice.

Also remove .aiff and .au because per @bmartel these are not supported by the backend

What does this fix?

(if this is a bug fix)

What is the new behavior?

(if this is a breaking or feature change)

Attempt to add multiple files, one of which is the wrong type. None will be uploaded and we'll show an error for the first invalid file.
image
Files with valid extensions upload as before, and the error state is cleared as expected.
image

What is the current behavior?

(if this is a breaking or feature change)

What libraries were added/updated?

(list all with version changes)

Does this change affect performance?

(if so describe the impacts positive or negative)

Does this change affect security?

(if so describe the impacts positive or negative)

What alternative approaches were there?

(briefly list any if applicable)

What feature flags were used to cover this change?

(briefly list any if applicable)

Does this PR introduce a breaking change?

(check only one)

  • Yes, and covered entirely by feature flag(s)
  • Yes, and covered partially by feature flag(s)
  • No
  • Not sure (briefly explain the situation below)

What level of testing was included in the change?

(check all that apply)

  • e2e
  • integration
  • unit

Which logical domain(s) does this change affect?

The Import component in the FE.

@netlify
Copy link

netlify bot commented Jul 14, 2023

Deploy Preview for label-studio-docs-new-theme canceled.

Name Link
🔨 Latest commit f499fbf
🔍 Latest deploy log https://app.netlify.com/sites/label-studio-docs-new-theme/deploys/64b61190c37de0000843e29f

@netlify
Copy link

netlify bot commented Jul 14, 2023

Deploy Preview for heartex-docs canceled.

Name Link
🔨 Latest commit f499fbf
🔍 Latest deploy log https://app.netlify.com/sites/heartex-docs/deploys/64b61190b89f920008befd8e

@github-actions github-actions bot added the fix label Jul 14, 2023
@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (0ff4bd9) 75.65% compared to head (f499fbf) 75.65%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4515   +/-   ##
========================================
  Coverage    75.65%   75.65%           
========================================
  Files          156      156           
  Lines        12299    12299           
========================================
  Hits          9305     9305           
  Misses        2994     2994           

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jombooth jombooth marked this pull request as ready for review July 18, 2023 03:55
@jombooth jombooth enabled auto-merge (squash) July 18, 2023 15:36
@bmartel bmartel marked this pull request as draft July 18, 2023 15:44
auto-merge was automatically disabled July 18, 2023 15:44

Pull request was converted to draft

@bmartel bmartel marked this pull request as ready for review July 18, 2023 15:45
@jombooth jombooth merged commit 9a111b3 into develop Jul 18, 2023
40 checks passed
@jombooth jombooth deleted the fb-LSDV-4907/frontend-checks-file-extensions branch July 18, 2023 17:33
shayantabatabaee pushed a commit to shayantabatabaee/label-studio that referenced this pull request Sep 19, 2023
* fix: LSDV-4907: Import - validate filetypes on the FE

* ci: Build frontend

Workflow run: https://github.com/heartexlabs/label-studio/actions/runs/5549397332

* use semicolons

* ci: Build frontend

Workflow run: https://github.com/heartexlabs/label-studio/actions/runs/5549447635

* remove aiff and au since those are not supported anymore, FE+BE

* ci: Build frontend

Workflow run: https://github.com/heartexlabs/label-studio/actions/runs/5582119630

* add jpEg as on the BE, add note about keeping compatibility

* actually add jpEg

* ci: Build frontend

Workflow run: https://github.com/heartexlabs/label-studio/actions/runs/5583232992

---------

Co-authored-by: robot-ci-heartex <robot-ci-heartex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants