-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 2023-10-13] [$500] Chat - Strikethrough link isn't parsed correctly on link with URL that contains tilde #27329
Comments
Job added to Upwork: https://www.upwork.com/jobs/~0193c33b549a6d01ac |
Triggered auto assignment to @greg-schroeder ( |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to @sophiepintoraetz ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @robertKozik ( |
Hi @izarutskaya |
📣 @sahilbhosale63! 📣
|
ProposalPlease re-state the problem that we are trying to solve in this issueAfter sending a complex message with ~ ~, the text is not crossed out What is the root cause of that problem?The main problem is the regular expression that is used in our common library Which incorrectly processes links containing ~ For example: Match for As a result here we get incorrect result What changes do you think we should make in order to solve the problem?I'm not sure if it's possible to make changes to the common library, but it would be nice to make a few changes to the regular expression What alternative solutions did you explore? (Optional)Also we have a special function that allows us to apply strikethrough Like
Or we can add this condition in getParsedComment Lines 1771 to 1774 in 86d37d6
|
ProposalPlease re-state the problem that we are trying to solve in this issue.The strikethrough link does not parse correctly when the URL contains a tilde. What is the root cause of that problem?In expensify-common library, the link with URL that contains tilde is not handled correctly. What changes do you think we should make in order to solve the problem?If it is possible to modify the module, add a regular expression to the
Alternatively, we can add a regular expression to
Result: result.webm |
ProposalPlease re-state the problem that we are trying to solve in this issueThe current strikethrough markdown is not applied if url contains tilde. What is the root cause of that problem?Current regex patterns of strikethrough and bold rule are not covering this issue case. What changes do you think we should make in order to solve the problem?We need to fix regex pattern on strikethrough, also bold rules. As following:
and we can add test cases.
The solution does not make other test cases failed. |
awaiting proposal review |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
Sorry for keeping you waiting. Going to review the proposals now |
Hi all @Antasel @astrohunter62 @ZhenjaHorbach! Thank you all for your proposals. Yes, we can do the PR to Selected Proposals: #27329 (comment) |
@robertKozik can you make sure to use the proper formatting to assign an internal engineer to confirm the proposal selection? like the 🎀 👀 🎀 C+ reviewed! thing |
PR merged today! |
@Beamanator |
Not a stupid question! But yes, you can see in |
@Beamanator |
Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:
On to the next one 🚀 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.78-4 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 2023-10-13. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
This is ready for payment tomorrow unless any regressions pop up |
|
@Beamanator, @greg-schroeder, @robertKozik, @ZhenjaHorbach Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
I believe this is ready for payment 👍 |
Yes, sorry, I was sick on Friday |
Issue Participants: Issue reported by: @kerupuksambel Was this issue merged in time to be eligible for the speed bonus? No (context) Payment summary: Reporter: $50 |
Payments made, closing. |
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:
Expected Result:
The tilde should be parsed as styling character, and the strikethrough should be applied (like underscore and italicized link that behave like that, so this could be categorized as inconsistency issue as well)
Actual Result:
The tilde isn't parsed and the message isn't styled as expected
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: v1.3.69-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Strikethrough.Link.mp4
Recording.1519.mp4
Expensify/Expensify Issue URL:
Issue reported by: @kerupuksambel
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694320083877809
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: