Limit file extensions to be imported into a document #1520
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ONS` This should reduce the possibility that our users will attempt to import the wrong kind of file.
jordigh
force-pushed
the
jordigh/importable-types
branch
from
28191b0 to
9b17781
Compare
paulfitz
reviewed
Mar 17, 2025
app/client/lib/uploads.ts
Outdated
| @@ -31,6 +31,9 @@ export interface SelectFileOptions extends UploadOptions { | |||
| // e.g. [".jpg", ".png"] | |||
| } | |||
|
|
|||
| // This list coincides with the extensions defined in core/plugins/manifest.yml | |||
| export const IMPORTABLE_TABLE_EXTENSIONS = [".xlsx", ".json", ".csv", ".tsv", "dsv"]; | |||
There was a problem hiding this comment.
The relationship between IMPORTABLE_EXTENSIONS and IMPORTABLE_TABLE_EXTENSIONS is a bit confusing. The word TABLE suggests one of them might be for single table imports, but that isn't the case for .xlsx and .json
Maybe the distinction is more like IMPORTABLE_AS_DOC_EXTENSIONS and IMPORTABLE_WITHIN_DOC_EXTENSIONS?
There was a problem hiding this comment.
Okay, I've made that change.
…DOC` We're going to have two different kinds of importable extensions, so let's rename these to be more consistent with their purpose
What makes sense to import into a document isn't the same as what makes sense for importing as a whole document.
jordigh
force-pushed
the
jordigh/importable-types
branch
3 times, most recently
from
e1f5b27 to
0289e3d
Compare
fflorent
reviewed
Mar 18, 2025
fflorent
suggested changes
Mar 18, 2025
There was a problem hiding this comment.
Seems almost good to me!
There just seem to be a typo (as file specifiers using extensions should always start with dots).
paulfitz
approved these changes
Mar 18, 2025
jordigh
force-pushed
the
jordigh/importable-types
branch
from
c5e193d to
dd73feb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
Users have tried using the importer to add tables to an existing document, but without any indication of which file types are available for import, users may upload an incompatible file type and hit an error message.
Proposed solution
Limit the file picker to only the
table-levelimportable file types, analogous to the limits imposed on the document-level importable types.Original commit messages:
d917f5f uploads: add new constant, IMPORTABLE_TABLE_EXTENSIONS
What makes sense to import into a document isn't the same as what
makes sense for importing as a whole document.
28191b0 Importer: limit list of acceptable files to
IMPORTABLE_TABLE_EXTENSIONSThis should reduce the possibility that our users will attempt to
import the wrong kind of file.
Has this been tested?
We don't have tests for OS-level dialogues that the web browser could open, so instead I did some manual testing to ensure that the dialogue only allows uploading this restricted set of file types.
Screenshots / Screencasts
uploads.mp4