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.

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

[HOLD for payment 2022-09-15] [$250] Chat - Error message is received when attaching files with extension in caps #10694

Closed
kavimuru opened this issue Aug 30, 2022 · 15 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented Aug 30, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Access staging.new.expensify.com
  2. Sign into a valid account
  3. Open any chat
  4. Attempt to add any attachment through "+" or "Drag & Drop" with the type in CAPS (For example: .PDF or .PNG)

Expected Result:

User expects the attachment to be added with no issues

Actual Result:

The user receives an error message if the attachment type is in CAPS

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • Desktop App

Version Number: 1.1.94-4
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Bug5710801_PNG_leads_to_error.mp4

Upwork URL https://www.upwork.com/jobs/~01d1500d5c8671f017
Issue reported by: Applause internal team
Slack conversation:

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented Aug 30, 2022

Triggered auto assignment to @marcaaron (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@theTrozen77
Copy link

PROPOSAL

I was able to reproduce the issue and fix it.
It is a simple fix. Just convert the extension string to lowercase using .toLowerCase() method when the file extension is separated from the complete file string.
const fileExtension = splitFileName.pop().toLowerCase();

Thankyou

@varshamb
Copy link
Contributor

Proposal

Previous proposal modifies the extension, but I think we should only convert to lowercase during validation.

if (!_.contains(CONST.API_ATTACHMENT_VALIDATIONS.ALLOWED_EXTENSIONS, fileExtension)) {

@marcaaron marcaaron added the External Added to denote the issue can be worked on by a contributor label Aug 30, 2022
@marcaaron marcaaron removed their assignment Aug 30, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 30, 2022

Triggered auto assignment to @trjExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@trjExpensify
Copy link
Contributor

Woof, nice catch. Job is on Upwork here: https://www.upwork.com/jobs/~01d1500d5c8671f017

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Aug 30, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 30, 2022

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 30, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 30, 2022

Triggered auto assignment to @stitesExpensify (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot changed the title Chat - Error message is received when attaching files with extension in caps [$250] Chat - Error message is received when attaching files with extension in caps Aug 30, 2022
@parasharrajat
Copy link
Member

parasharrajat commented Aug 30, 2022

Minor difference(extension case does not matter much) but I agree, let's just fix the issue.

@varshamb's proposal looks good to me.

cc: @stitesExpensify

🎀 👀 🎀 C+ reviewed

@theTrozen77
Copy link

Hi. Who should be applying to this since my solution is also relevant?

@stitesExpensify
Copy link
Contributor

Hi @theTrozen77, We're going to go with @varshamb's proposal here (and they will be implementing it). While your proposal came first and they are similar, I think that @varshamb's is more accurate in this case which is typically how we judge the "best" solution.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 31, 2022
@melvin-bot
Copy link

melvin-bot bot commented Aug 31, 2022

📣 @varshamb You have been assigned to this job by @stitesExpensify!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Sep 8, 2022
@melvin-bot melvin-bot bot added the Awaiting Payment Auto-added when associated PR is deployed to production label Sep 8, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.97-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2022-09-15. 🎊

@melvin-bot melvin-bot bot changed the title [$250] Chat - Error message is received when attaching files with extension in caps [HOLD for payment 2022-09-15] [$250] Chat - Error message is received when attaching files with extension in caps Sep 8, 2022
@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Sep 14, 2022
@trjExpensify
Copy link
Contributor

Not overdue, payment due tomorrow.

@varshamb @parasharrajat - can you both accept the offer please?

@melvin-bot melvin-bot bot removed the Overdue label Sep 14, 2022
@varshamb
Copy link
Contributor

@trjExpensify Accepted.

@trjExpensify
Copy link
Contributor

Settled up with you both. Thanks! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

7 participants