Skip to content

Fix fileupload defaults not applied #7086

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

Merged

Conversation

mtrezza
Copy link
Member

@mtrezza mtrezza commented Dec 19, 2020

New Pull Request Checklist

Issue Description

Default values of fileUpload are not applied if fileUpload option is not set explicitly in the Parse Server config option.

Related issue: closes #7085

Approach

Added default definition for fileUpload group parameter.

TODOs before merging

  • Add test cases

@codecov
Copy link

codecov bot commented Dec 19, 2020

Codecov Report

Merging #7086 (977e16a) into master (41a052c) will increase coverage by 0.23%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7086      +/-   ##
==========================================
+ Coverage   93.66%   93.89%   +0.23%     
==========================================
  Files         169      169              
  Lines       12499    12501       +2     
==========================================
+ Hits        11707    11738      +31     
+ Misses        792      763      -29     
Impacted Files Coverage Δ
src/Options/index.js 100.00% <ø> (ø)
src/Config.js 90.96% <83.33%> (-0.50%) ⬇️
src/Options/Definitions.js 100.00% <100.00%> (ø)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.84% <0.00%> (-0.68%) ⬇️
src/Controllers/DatabaseController.js 95.76% <0.00%> (+0.14%) ⬆️
src/RestWrite.js 93.84% <0.00%> (+0.16%) ⬆️
src/Options/parsers.js 100.00% <0.00%> (+93.93%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41a052c...977e16a. Read the comment docs.

@mtrezza mtrezza requested a review from davimacedo December 19, 2020 17:07
@Moumouls
Copy link
Member

@mtrezza seems to have many change, did you run prettier and lint fix ?

@mtrezza mtrezza requested review from dplewis and davimacedo and removed request for davimacedo and dplewis December 21, 2020 13:00
@mtrezza
Copy link
Member Author

mtrezza commented Dec 29, 2020

@parse-community/server This PR should be reviewed ASAP, it fixes a bug in the current main branch that prevents file upload if default settings are not set.

Copy link
Member

@Moumouls Moumouls left a comment

Choose a reason for hiding this comment

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

A lot of changes on spec/ParseFile, does prettier is ok on it ?

Comment on lines +178 to +182
fileUpload: {
env: 'PARSE_SERVER_FILE_UPLOAD_OPTIONS',
help: 'Options for file uploads',
action: parsers.objectParser,
default: {},
Copy link
Member

Choose a reason for hiding this comment

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

Any way to remove the default empty object ? An empty object can be translated to undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why do you want to remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Moumouls Do you want to elaborate your concern? Otherwise I’ll merge.

@mtrezza
Copy link
Member Author

mtrezza commented Jan 11, 2021

@dplewis @davimacedo can you please approve this, so we can merge this? The current master has a bug that does not allow file upload. I am afraid this may be forgotten before a new Parse Server release, so I'd rather see this merged ASAP.

@dplewis
Copy link
Member

dplewis commented Jan 11, 2021

Looks like the Parse.File.spec.js has a lot of changes. @Moumouls Also mentioned this.

@mtrezza
Copy link
Member Author

mtrezza commented Jan 11, 2021

Just some describe grouping, hence large diff, see 977e16a

@dplewis
Copy link
Member

dplewis commented Jan 11, 2021

LGTM!

@mtrezza mtrezza merged commit e08618e into parse-community:master Jan 11, 2021
rsouzas pushed a commit to back4app/parse-server that referenced this pull request Jan 29, 2021
* added fileUpload definition default value

* added undefined and null as invalid

* removed explicit default value reference

* improved test grouping in describes
dplewis pushed a commit that referenced this pull request Feb 21, 2021
* added fileUpload definition default value

* added undefined and null as invalid

* removed explicit default value reference

* improved test grouping in describes
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 1, 2021
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File upload fails with "Cannot read property 'enableForAnonymousUser' of undefined"
4 participants