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
Fix replace media #7787
Fix replace media #7787
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7787 +/- ##
==========================================
+ Coverage 27.16% 27.20% +0.03%
==========================================
Files 1163 1164 +1
Lines 15518 15519 +1
Branches 2410 2412 +2
==========================================
+ Hits 4216 4222 +6
+ Misses 9534 9528 -6
- Partials 1768 1769 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modifications look good to me :) I have a question/remark regarding a related code though
|
||
const createFileToDownloadName = ({ file, fileInfo: { name } }) => { | ||
const ext = get(file, 'ext', ''); | ||
|
||
return `${name.replace(ext, '')}${ext}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case the extension is also present in the name (ex: my.txtfile.txt
), I think this line would not do what expected.
What about:
return name.endsWith(ext)
? name
: `${name}${ext}`;
Signed-off-by: soupette <cyril.lpz@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
Signed-off-by: soupette cyril.lpz@gmail.com
Description of what you did:
This PR fixes #7776