Closed lanitochka17 closed 4 months ago
melvin-bot[bot]
commented
5 months ago Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
lanitochka17
commented
5 months ago @sakluger FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
sakluger
commented
5 months ago FYI @lanitochka17 - GitHub was removing the \ from your comment because it's a Markdown formatting character in GH - it's an escape character used to ignore other markdown formatting (more details here).
I updated your comment to add that character back. 😄
melvin-bot[bot]
commented
5 months ago Job added to Upwork: https://www.upwork.com/jobs/~014a67ec2b09731f24
melvin-bot[bot]
commented
5 months ago Triggered auto assignment to Contributor-plus team member for initial proposal review - @eh2077 (External)
devguest07
commented
5 months ago System message about adding tag in Parent: Child format shows Parent\: Child
When displaying messages inside ReportActionItemFragment and TextCommentFragment, we do not clean the tag message escaping of colons (used to create multi-level tags, e.g., "Parent: Child") in the tag name received from the backend.
We need to clean up the message from the colons before displaying it. This can be done at multiple levels before being displayed. One way is by editing the fragment inside ReportActionItemFragment.
We can use if (actionName === 'POLICYCHANGELOG_ADD_TAG') to check if it's a tag message. If so, we can use the utility PolicyUtils.getCleanedTagName to return a clean tag message.
if (actionName === 'POLICYCHANGELOG_ADD_TAG') {
tagFragment = PolicyUtils.getCleanedTagName(fragment.html); // we can use fragment.text too
}If we have a new value, we can display it and pass it to the component TextCommentFragment.
truph01
commented
5 months ago
{
"actionName": "POLICYCHANGELOG_ADD_TAG",
"message": [
{
"html": "added the tag \"name\\: age\" to the list \"Tag\"",
"text": "added the tag \"name\\: age\" to the list \"Tag\""
}
]
}- And we display this message by using ```ReportActionItemMessage```:
https://github.com/Expensify/App/blob/8d11d0b185083a6a813daeb0fe7c0697a5627ef6/src/pages/home/report/ReportActionItem.tsx#L647
that will display a message base on raw ```reportAction.message``` data.
- So that leads to the bug.
### What changes do you think we should make in order to solve the problem?
- We need to add an additional else logic to this:
https://github.com/Expensify/App/blob/8d11d0b185083a6a813daeb0fe7c0697a5627ef6/src/pages/home/report/ReportActionItem.tsx#L638else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG){ children = <ReportActionItemBasicMessage message={PolicyUtils.getCleanedTagName(action.message?.[0]?.text ?? '')} />;
- Then, we add an additional else logic to this:
https://github.com/Expensify/App/blob/8d11d0b185083a6a813daeb0fe7c0697a5627ef6/src/libs/SidebarUtils.ts#L378else if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_TAG) { result.alternateText = PolicyUtils.getCleanedTagName(lastAction?.message?.[0]?.text ?? '');
to fix the LHN issue.
### What alternative solutions did you explore? (Optional)
- There are a few component where we have the same issue, for example, copy to clipboard action and we need to address them as well.
<!---
ATTN: Contributor+
You are the first line of defense in making sure every proposal has a clear and easily understood problem with a "root cause". Do not approve any proposals that lack a satisfying explanation to the first two prompts. It is CRITICALLY important that we understand the root cause at a minimum even if the solution doesn't directly address it. When we avoid this step we can end up solving the wrong problems entirely or just writing hacks and workarounds.
Instructions for how to review a proposal:
1. Address each contributor proposal one at a time and address each part of the question one at a time e.g. if a solution looks acceptable, but the stated problem is not clear then you should provide feedback and make suggestions to improve each prompt before moving on to the next. Avoid responding to all sections of a proposal at once. Move from one question to the next each time asking the contributor to "Please update your original proposal and tag me again when it's ready for review".
2. Limit excessive conversation and moderate issues to keep them on track. If someone is doing any of the following things please kindly and humbly course-correct them:
- Posting PRs.
- Posting large multi-line diffs (this is basically a PR).
- Skipping any of the required questions.
- Not using the proposal template at all.
- Suggesting that an existing issue is related to the current issue before a problem or root cause has been established.
- Excessively wordy explanations.
3. Choose the first proposal that has a reasonable answer to all the required questions.
-->
eh2077
commented
5 months ago Thank you for your proposals!
@devguest07 Your proposal is close to success. You dived into the code and found a line to fix the effect, though that's not the best way to fix it. No worries, as you seem new to the project. Just a small tip for you: try to find out if there are similar existing coding patterns in the codebase when you're going to make changes. I believe you will soon land on a successful proposal.
@truph01 's proposal looks good to me. Their solution fixes both report and LHN message. More detail changes can be discussed in PR.
🎀👀🎀 C+ reviewed
melvin-bot[bot]
commented
5 months ago Triggered auto assignment to @rlinoz, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
devguest07
commented
5 months ago Thank you @eh2077
rlinoz
commented
5 months ago Looks good, thanks!
melvin-bot[bot]
commented
5 months ago 📣 @eh2077 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
melvin-bot[bot]
commented
5 months ago 📣 @truph01 You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖
eh2077
commented
5 months ago @truph01 Kindly let us know when we can expect a PR to be ready for review, thanks!
truph01
commented
5 months ago @eh2077 I created PR https://github.com/Expensify/App/pull/43765. It can be reviewed now.
rlinoz
commented
4 months ago @sakluger this issue was fixed by some other PR and is not reproducible anymore, but we had a working PR and all, can you proceed with payment to @truph01 and @eh2077 ?
sakluger
commented
4 months ago Summarizing payment on this issue:
Contributor: @truph01 $250, payable via Upwork Contributor+: @eh2077 $250, paid via Upwork
rlinoz
commented
4 months ago Not sure why Melvin think this is overdue
sakluger
commented
4 months ago @truph01 could you please apply to the job on Upwork (https://www.upwork.com/jobs/~014a67ec2b09731f24) or share your Upwork profile link so that we can pay you for your work and close out the GH issue? Thanks!
sakluger
commented
4 months ago @truph01 could you please apply to the job on Upwork (https://www.upwork.com/jobs/~014a67ec2b09731f24) or share your Upwork profile link so that we can pay you for your work and close out the GH issue? Thanks!
rlinoz
commented
4 months ago @sakluger I actually messed up and got things wrong, the main issue is still reproducible, and we are fixing it in the PR, I am really sorry about that.
melvin-bot[bot]
commented
4 months ago The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.3-7 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2024-07-10. :confetti_ball:
For reference, here are some details about the assignees on this issue:
melvin-bot[bot]
commented
4 months ago 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:
sakluger
commented
4 months ago I sent an offer to @truph01 via Upwork: https://www.upwork.com/nx/wm/offer/103034403.
@eh2077 can you please complete the BZ checklist?
truph01
commented
4 months ago @sakluger I accepted that offer, TY
sakluger
commented
4 months ago Thanks, all paid!
@eh2077 we're just holding for the BZ checklist before we can close this issue.
sakluger
commented
4 months ago Bump @eh2077
eh2077
commented
4 months ago cc @sakluger
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-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team
Issue found when executing PR https://github.com/Expensify/App/pull/43392
Action Performed:
Expected Result:
System message about adding tag in Parent: Child format shows as Parent: Child, not
Parent\: ChildActual Result:
System message about adding tag in Parent: Child format shows
Parent\: ChildWorkaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @eh2077