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.

Sign up for GitHub

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

Jump to bottom

3338 fix bad file uploads #3564

Merged
merged 8 commits into from
Dec 17, 2020
Merged

3338 fix bad file uploads #3564

merged 8 commits into from
Dec 17, 2020

Conversation

entantoencuanto
Copy link
Member

@entantoencuanto entantoencuanto commented Dec 14, 2020 •
edited
Loading

Closes #3338
Closes #3339
Closes #3526

✌️ What does this PR do?

  • Prevents exceptions when a file is incorrectly uploaded in the multipart-form version and a string is sent in the data-file attribute instead of an uploaded file and makes the application to return an error message.
  • Prevents exceptions when the csv_separator is sent in the request as an empty string validating the attribute length in the form
  • Validates the presence of database configuration for module and prevents creation of datasets if not present. Also changes the fallback of query used in execute_write_query_from_file_using_stdin to send an error message instead of a blank array.
  • Adds some tests.

🔍 How should this be manually tested?

Use a valid curl request but remove the @ before the file path and send the csv_separator attribute empty

:shipit: Does this PR changes any configuration file?

No

(Changes in these files might need to update the role in Ansible)

📖 Does this PR require updating the documentation?

No

@codecov
Copy link

codecov bot commented Dec 14, 2020 •
edited
Loading

Codecov Report

Merging #3564 (c103c2b) into master (902b5a4) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3564      +/-   ##
==========================================
+ Coverage   84.36%   84.38%   +0.02%     
==========================================
  Files         791      791              
  Lines       21135    21145      +10     
==========================================
+ Hits        17830    17843      +13     
+ Misses       3305     3302       -3
Impacted Files Coverage Δ
app/forms/gobierto_data/dataset_form.rb 96.22% <100.00%> (+0.30%) ⬆️
app/models/gobierto_data/connection.rb 97.46% <100.00%> (+0.06%) ⬆️
app/helpers/gobierto_budgets/application_helper.rb 65.62% <0.00%> (+2.08%) ⬆️
...ierto_budgets/budgets_execution/metric_box_cell.rb 100.00% <0.00%> (+3.44%) ⬆️

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 902b5a4...c103c2b. Read the comment docs.

@entantoencuanto
Validate length of csv_separator in datasets upload
54a006a
@entantoencuanto
Add some datasets create/update API controller tests
03b4ab3 
* Uploading datasets with wrong data_file
* Uploading datasets with blank csv_separator
@entantoencuanto entantoencuanto marked this pull request as ready for review December 14, 2020 17:28
@ferblape ferblape merged commit 6675f2f into master Dec 17, 2020
@ferblape ferblape deleted the 3338-fix_bad_file_uploads branch December 17, 2020 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ferblape ferblape ferblape approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@entantoencuanto @ferblape