[HOLD for payment 2023-08-10] [$1000] System messages for tasks are wrong/inconsistent with other report patterns

[kavimuru]

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. Create a task with
2. Assign whoever you’d like
3. Mark the task as done

Expected Result:

The system message should appear in all grey supporting text

Actual Result:

“Completed task” is oddly hyperlinked (why?) and the text fails to use support text, like all other system messages

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.43-2
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

Recording.1297.mp4

Expensify/Expensify Issue URL:
Issue reported by: @JmillsExpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689745091154569

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017ea86167d179c7a2
• Upwork Job ID: 1684032060275773440
• Last Price Increase: 2023-07-26

[DanutGavrus]

Proposal

Please re-state the problem that we are trying to solve in this issue.

System messages for tasks are wrong/inconsistent with other report patterns

What is the root cause of that problem?

In TaskAction, we use the chatItemMessageLink style even if the text is not a link:

App/src/components/ReportActionItem/TaskAction.js

Lines 58 to 63 in e87f2eb

	<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
	<Text>
	<Text style={styles.chatItemMessageLink}>{messageLinkText}</Text>
	<Text style={[styles.chatItemMessage]}>{` ${taskReportName}`}</Text>
	</Text>
	</View>

What changes do you think we should make in order to solve the problem?

We should replace it with another style which meets our desired style. If we want it to be gray, we should choose a style which has color: themeColors.textSupporting in it(such as mutedTextLabel or colorMuted or another existing one). If we want to apply other styling too and no existing one meets all our requirements, then we should create a new style for using it here. Decision depends on the output desired by the design team.

[melvin-bot]

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[neonbhai]

Proposal

Please re-state the problem that we are trying to solve in this issue.

System messages for tasks are wrong/inconsistent with other report patterns

What is the root cause of that problem?

The styles applied to the system message is styles.messageLinkText which is blue and looks inconsistent with other System messages

What changes do you think we should make in order to solve the problem?

• We should change the styles of messageLinkText applied here as, styles.colorMuted as this is used in other System messages and reinforces consistency in the app. Colormuted applies the style:
  

  App/src/styles/styles.js

  Lines 412 to 414 in 8dcad16

  	colorMuted: {
  	color: themeColors.textSupporting,
  	},

• The second style rendering taskReportName is white hence good as it Mirrors the task name

• I will also propose the capitalization of the first word in the task messages completed, canceled and repopened

  App/src/languages/en.js

  Lines 1317 to 1328 in 8dcad16

  	task: {
  	task: 'Task',
  	title: 'Title',
  	description: 'Description',
  	assignee: 'Assignee',
  	completed: 'Completed',
  	messages: {
  	completed: 'completed task',
  	canceled: 'canceled task',
  	reopened: 'reopened task',
  	error: 'You do not have the permission to do the requested action.',
  	},

  
  as they are System messages (in accordance to this issue)

• Also proposing to add a seperator (i.e. ':') between the message and the taskName the message points to.

Before

### Original Screenshot

Final Screenshot

Final Screenshot

What alternative solutions did you explore? (Optional)

xx

[melvin-bot]

@mallenexpensify Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017ea86167d179c7a2

[melvin-bot]

Current assignee @mallenexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[mallenexpensify]

Was able to reproduce, reckon it's a quick fix if we just want the text to be grey instead of blue (I didn't see a hyperlink).
Hi @ArekChr , first time seeing you as C+ on an issue for me. Can you please review the proposals above?

[JmillsExpensify]

Going to assign myself to this issue as well to ensure we land on the appropriate design patterns.

[ArekChr]

Agree, this issue is pretty straightforward, The first proposal LGTM.
🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[DanutGavrus]

@ArekChr @dangrous I'm OOO tomorrow & up in the mountains this weekend ⛰️, don't know if it's possible to postpone the assignment. In any case, I'll create the PR as soon as possible on Monday, thank you! 😁

[dangrous]

Hi! the proposal seems fine to me - however I'm checking with the folks who worked on tasks to see if the original idea was to have this link to something - in which case the bug wouldn't be the text styling but instead a missing link.

Stay tuned (this will probably give you the reprieve you need, @DanutGavrus - and either way it's 3 business days so you should probably be fine to raise it on Mon.)

[thienlnam]

Yeah this is a remnant of an old thing of tasks where we used to add the system message to the parent report. The link would take you to the task report but now since the system messages are in the task report it doesn't really make sense to keep them links anymore

[dangrous]

Okay great! Then we can move forward with @DanutGavrus's proposal. I'll assign you now, and that will give you through Tuesday to get that bonus, which should be fine!

[melvin-bot]

📣 @DanutGavrus 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 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• [23304] Update tasks status color. #23912

If no regressions arise, payment will be issued on 2023-08-10. 🎊

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.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @ArekChr does not require payment (Contractor)
• @DanutGavrus requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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:

• [@ArekChr] The PR that introduced the bug has been identified. Link to the PR:
• [@ArekChr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@ArekChr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@ArekChr] Determine if we should create a regression test for this bug.
• [@ArekChr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@JmillsExpensify / @mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[thienlnam]

I've been seeing this live - and for group / room chats, the double colon doesn't look that good. I'm wondering if we should remove the colon so it just says

test31@gmail.com: Created a task task title

Thoughts on this? Maybe it's not a big deal for these specific situations but wanted to point it out

[shawnborton]

Oh that would make sense to me, thoughts @JmillsExpensify ?

[dangrous]

I agree that it looks weird - but that's a different issue, right? this just changed the color, not the colon or anything

[shawnborton]

Fair, that can be fixed as a follow up.

[thienlnam]

Oh gotcha, yeah I guess this would be a seperate issue

[ArekChr]

• The PR that introduced the bug has been identified. Link to the PR: has not been identified.
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug: No regression test is needed for testing colors of text.

[dangrous]

Checklist looks good, this wasn't a regression.

@thienlnam are you able to make an issue for the colon thing? I can spin up a PR quick since it'll be a five second fix.

[thienlnam]

Nope not yet, though maybe it's fine because removing the colon makes it more confusing for non-group chats so I'm not really sure which we want to do

[dangrous]

Hrm, maybe we sync in Slack?

[dangrous]

Posted in Slack about that question, but either way won't affect the payment for this issue.

https://expensify.slack.com/archives/C01GTK53T8Q/p1691602405412179

[dangrous]

Got sidetracked a bit with that discussion about the follow up issue, but it looks like we're ready for payment on this one @mallenexpensify

[JmillsExpensify]

I like removing the double colon.

[JmillsExpensify]

Summarizing payouts:

• Issue reporter: N/A
• Contributor: @DanutGavrus $1,500 paid (incl. urgency bonus)
• Contributor+: @ArekChr $1,500 (does not require payment)

Upwork job is here: https://www.upwork.com/jobs/~017ea86167d179c7a2. @DanutGavrus can you please apply?

[DanutGavrus]

@JmillsExpensify It requires 16 connects, could you please send me an invitation? This is my Upwork profile. Thank you!

[JmillsExpensify]

Sure thing! I just sent you an offer.

[DanutGavrus]

Applied, thank you very much!

[melvin-bot]

@JmillsExpensify, @dangrous, @mallenexpensify, @ArekChr, @DanutGavrus Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@JmillsExpensify, @dangrous, @mallenexpensify, @ArekChr, @DanutGavrus Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

@DanutGavrus has been paid $1500 via Upwork, updated the post above to state.

• Contributor: @DanutGavrus $1,500 paid (incl. urgency bonus)