-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[Due for payment 2025-03-21] [$250] Expense - Saving multiline description with mark down with no edit triggers system message #43403
Comments
Triggered auto assignment to @kadiealexander ( |
We think this issue might be related to the #collect project. |
ProposalPlease re-state the problem that we are trying to solve in this issue.Expense - Saving multiline description with mark down with no edit triggers system message What is the root cause of that problem?The backend returns App/src/pages/iou/request/step/IOURequestStepDescription.tsx Lines 115 to 121 in 2929a91
What changes do you think we should make in order to solve the problem?To make the const currentDescription =
isEditingSplitBill && !lodashIsEmpty(splitDraftTransaction)
? splitDraftTransaction?.comment.comment ?? ''
: parser.htmlToMarkdown(parser.replace(transaction?.comment.comment ?? '')).trim(); We should also check for similar bug in other similar components. What alternative solutions did you explore? (Optional)We can replace |
ProposalPlease re-state the problem that we are trying to solve in this issue.System message for description edit shows up when no edit is made to the description. What is the root cause of that problem?The description that is returned by back-end includes What changes do you think we should make in order to solve the problem?
We need back-end change to accept the description of money request can be stored as htm.
What alternative solutions did you explore? (Optional) |
Job added to Upwork: https://www.upwork.com/jobs/~018d19307f19622d14 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @mkhutornyi ( |
reviewing proposals |
If this issue doesn't happen on other multiline fields like task description, room description, we should apply same logic to be consistent. |
@mkhutornyi is anything wrong with the current proposals? |
@kadiealexander, @mkhutornyi Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@dominictb thanks for the proposal.
Is backend already doing this on task description and room description? |
@mkhutornyi Yes. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@dominictb's proposal looks good to me. |
Triggered auto assignment to @MariaHCD, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@MariaHCD @kadiealexander @mkhutornyi this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
@MariaHCD, @kadiealexander, @mkhutornyi Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@dominictb sorry this has been a while, can you add description of the issue you are seeing now? |
@mountiny We all agreed to update We need to do the same for |
@mountiny How's the PR going? |
@dominictb the PR was merged and deployed, can you test now? |
@mountiny I just tested all the expense flows and found 1 problem. Same reason as #43403 (comment). We need to make all the expense creation APIs to accept HTML
Correct me if I missed any. |
In addition to those you have mentioned above, we have (not all might be used in the App right now):
Can you make sure to update the code there accordingly too? |
Backedn Pr is up |
@mountiny Notify us when they are done. |
@dominictb deployed |
This issue has not been updated in over 15 days. @mountiny, @kadiealexander, @mkhutornyi, @dominictb eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
PR was merged. |
@dominictb Please be on a look out for any deploy blockers related to this as it was a large change |
Deployed to production 2025-03-12. Payment due 2025-03-19. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.12-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 2025-03-21. 🎊 For reference, here are some details about the assignees on this issue:
|
@dominictb @kadiealexander @dominictb The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
@kadiealexander I took over as C+ on this issue for the PR review. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 1.4.81-1
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team
Action Performed:
Example:
Expected Result:
No system message for description edit will show up because no edit is made to the description.
Actual Result:
System message for description edit shows up when no edit is made to the description.
When repeating Step 6 and 7, the transaction thread turns blank with the error "There was an error loading more messages."
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Bug6508340_1718036856778.bandicam_2024-06-11_00-22-35-147.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @kadiealexanderThe text was updated successfully, but these errors were encountered: