Skip to content

232 upload file to folder#233

Merged
max-zilla merged 16 commits into
developfrom
232-upload-file-to-folder
Sep 13, 2021
Merged

232 upload file to folder#233
max-zilla merged 16 commits into
developfrom
232-upload-file-to-folder

Conversation

@tcnichol
Copy link
Copy Markdown
Contributor

This pull request enabled users to upload files to a folder in a dataset through the api. Previously this option was not available.

The route api.Files.uploadToDataset now has a parameter folder_id: Option[String].

Swagger documentation for that route has also been updated.

@tcnichol tcnichol requested review from lmarini and max-zilla May 21, 2021 20:09
Comment thread app/api/Files.scala Outdated
Comment thread app/api/Files.scala Outdated
case 0 => BadRequest("No files uploaded")
case 1 => Ok(Json.obj("id" -> uploadedFiles.head.id))
case _ => Ok(Json.obj("ids" -> uploadedFiles.toList))
folder_id match {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can simplify (not tested)

val folder = folder_id.flatmap(x => folders.get(x))
val uploadedFiles = FileUtils.uploadFilesMultipart(request, Some(dataset), folder, showPreviews = showPreviews, originalZipFile = originalZipFile, flagsFromPrevious = flagsFromPrevious, runExtractors = extract, apiKey = request.apiKey) 
uploadedFiles.length match {
    case 0 => BadRequest("No files uploaded")
    case 1 => Ok(Json.obj("id" -> uploadedFiles.head.id))
    case _ => Ok(Json.obj("ids" -> uploadedFiles.toList))
}

This will use flatmap (if there is a value, return an option) to get the folder and then just call the next function with ether None, or Some(folder).

Copy link
Copy Markdown
Contributor Author

@tcnichol tcnichol May 24, 2021

Choose a reason for hiding this comment

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

I am using the flatmap.

This works if folder_id is a UUID, and it works if folder_id is None, however, it breaks if folder_id is a string but not a UUID.

I know that there is a way to use conditionals inside of a flatMap, so I am trying to figure that out. Unless you know a quick fix offhand.

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 am now using a conditional that seems to handle all cases without breaking.

Regrettably it kept giving me an error if I tried to put an Option[UUID] in the api route and didn't send it, but this seems to match what is done in a few other cases with optional uuid parameters.

Comment thread app/api/Files.scala Outdated
Copy link
Copy Markdown
Member

@lmarini lmarini left a comment

Choose a reason for hiding this comment

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

(This PR is not urgent, it can go in the next release) There is one issue mentioned below. Can you also please updated the changelog? Thank you!

Comment thread app/api/Files.scala Outdated
datasets.get(dataset_id) match {
case Some(dataset) => {
val uploadedFiles = FileUtils.uploadFilesMultipart(request, Some(dataset), showPreviews = showPreviews, originalZipFile = originalZipFile, flagsFromPrevious = flagsFromPrevious, runExtractors = extract, apiKey = request.apiKey)
val current_folder = if (UUID.isValid(folder_id.get)){
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This throws an error if folder == None

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 believe I have fixed this. Also updated changelog

@tcnichol tcnichol linked an issue Jul 11, 2021 that may be closed by this pull request
@max-zilla max-zilla merged commit 3b85ce9 into develop Sep 13, 2021
@max-zilla max-zilla deleted the 232-upload-file-to-folder branch September 13, 2021 14:24
@lmarini lmarini mentioned this pull request Oct 5, 2021
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.

Not possible to upload a file directly into a folder

4 participants