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

return file extension in file_upload list and retrieve views #1019

Merged
merged 1 commit into from
Sep 14, 2022
Merged

return file extension in file_upload list and retrieve views #1019

merged 1 commit into from
Sep 14, 2022

Conversation

sainak
Copy link
Member

@sainak sainak commented Sep 9, 2022 •
edited
Loading

Proposed Changes

return file extension in file_upload list and retrieve views

@coronasafe/code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

@sainak sainak requested a review from a team as a code owner September 9, 2022 17:57
@sainak sainak changed the title return internal name in file list to make determining the file extens… return internal name in file list to make determining the file extension easier Sep 9, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1019 (04affa5) into master (1c2b1ac) will increase coverage by 0.00%.
The diff coverage is 50.00%.

@@           Coverage Diff           @@
##           master    #1019   +/-   ##
=======================================
  Coverage   53.80%   53.80%           
=======================================
  Files         180      180           
  Lines        8916     8917    +1     
  Branches     1506     1521   +15     
=======================================
+ Hits         4797     4798    +1     
  Misses       4070     4070           
  Partials       49       49
Impacted Files Coverage Δ
care/facility/api/serializers/file_upload.py 45.65% <50.00%> (ø)
care/users/api/viewsets/change_password.py 62.96% <0.00%> (+1.42%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

vigneshhari
vigneshhari requested changes Sep 10, 2022
View reviewed changes
Copy link
Member

@vigneshhari vigneshhari left a comment

Choose a reason for hiding this comment

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

Let's not give the internal name out, our goal here was to break the relationship between the file name and the patient in the unlikely case our buckets are attacked.
Let's serialize only the extension.

@sainak
Copy link
Member Author

sainak commented Sep 10, 2022

Let's not give the internal name out, our goal here was to break the relationship between the file name and the patient in the unlikely case our buckets are attacked. Let's serialize only the extension.

yeah I thought returning the extension would be a better solution
do you have any suggestions for implementing that?
I have 2 implementations in mind

  • making the query return the extension
  • make a model method to return extension or file type

@vigneshhari
Copy link
Member

the model method sounds good enough. make sure you handle the edge cases.

@sainak sainak force-pushed the sainak/patch/file-upload-list branch from 04affa5 to bcaff00 Compare September 12, 2022 12:17
@sainak sainak changed the title return internal name in file list to make determining the file extension easier return file extension in file_upload list and retrieve views Sep 12, 2022
@sonarcloud
Copy link

sonarcloud bot commented Sep 12, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@patelaryan7751 patelaryan7751 mentioned this pull request Sep 12, 2022
Closed
6 tasks
@vigneshhari vigneshhari merged commit 6e7827f into master Sep 14, 2022
vigneshhari pushed a commit that referenced this pull request Sep 15, 2022
@sainak @vigneshhari
return file extension in file_upload list and retrieve views (#1019)
a3d752b 
return file extension from file upload views
@sainak sainak deleted the sainak/patch/file-upload-list branch September 22, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vigneshhari vigneshhari vigneshhari approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sainak @codecov-commenter @vigneshhari