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 2024-03-18] [$500] Add edit system messages for tasks #36196

Closed
thienlnam opened this issue Feb 9, 2024 · 25 comments
Closed
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@thienlnam
Copy link
Contributor

thienlnam commented Feb 9, 2024

When you edit a task, there is there is a TASKEDITED reportAction that is added to the report. Right now we are hiding it in the front-end, but the necessary changes have been made in the BE to turn this into a visible system message.

What we'll need to have done is:

  • Stop hiding the TASKEDITED reportActions
  • Ensure that once we make these visible, we also update the visibleReportActions to ensure this is counted
  • To handle the offline case, we'll need to optimistically create the message in the for the TASKEDITED reportAction. These optimistic messages will be:

When you update the assignee

  • If you remove the assignee, the message should say "removed the assignee"
  • If you update the assignee, the message should say "assigned to assignee" <- the assignee should be a mention

When you update the description

  • If you remove the description, the message should say "removed the description"
  • If you update the description, the message should say "updated the description to [new_description]"

When you update the title, the message should say "updated the task title to [new_task_title]"

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0177e8b2882a2299a2
  • Upwork Job ID: 1755757782524846080
  • Last Price Increase: 2024-02-09
  • Automatic offers:
    • paultsimura | Contributor | 0
@thienlnam thienlnam added External Added to denote the issue can be worked on by a contributor Daily KSv2 NewFeature Something to build that is a new item. labels Feb 9, 2024
@thienlnam thienlnam self-assigned this Feb 9, 2024
Copy link

melvin-bot bot commented Feb 9, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0177e8b2882a2299a2

@melvin-bot melvin-bot bot changed the title Add edit system messages for tasks [$500] Add edit system messages for tasks Feb 9, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 9, 2024
Copy link

melvin-bot bot commented Feb 9, 2024

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

Copy link

melvin-bot bot commented Feb 9, 2024

@melvin-bot melvin-bot bot added Weekly KSv2 Daily KSv2 and removed Daily KSv2 Weekly KSv2 labels Feb 9, 2024
@paultsimura
Copy link
Contributor

paultsimura commented Feb 9, 2024

Proposal

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

Need to show the TASKEDITED report actions and create the correct ones optimistically.

What is the root cause of that problem?

This is a feature request.

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

First of all, we need to stop hiding this type of actions by removing this piece of code:

if (reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.TASKEDITED) {
return false;
}

Then, we should modify this method (we may consider splitting it into 2 functions: one for fields modification, and another – for the assignees):

App/src/libs/ReportUtils.ts

Lines 3372 to 3402 in 749a395

function buildOptimisticEditedTaskReportAction(emailEditingTask: string): OptimisticEditedTaskReportAction {
return {
reportActionID: NumberUtils.rand64(),
actionName: CONST.REPORT.ACTIONS.TYPE.TASKEDITED,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
actorAccountID: currentUserAccountID,
message: [
{
type: CONST.REPORT.MESSAGE.TYPE.TEXT,
style: 'strong',
text: emailEditingTask,
},
{
type: CONST.REPORT.MESSAGE.TYPE.TEXT,
style: 'normal',
text: ' edited this task',
},
],
person: [
{
type: CONST.REPORT.MESSAGE.TYPE.TEXT,
style: 'strong',
text: allPersonalDetails?.[currentUserAccountID ?? '']?.displayName ?? currentUserEmail,
},
],
automatic: false,
avatar: allPersonalDetails?.[currentUserAccountID ?? '']?.avatar ?? UserUtils.getDefaultAvatarURL(currentUserAccountID),
created: DateUtils.getDBTime(),
shouldShow: false,
};
}

We need to make it more robust and build the report actions with different content depending on the action that was performed – as described in the issue – this action basically should reflect the report action that's coming in the EditTask API response from the Server, so it's a rather trivial task (we are currently implementing a similar thing in #35826 for rooms renaming).

The task is edited in these 2 places:

function editTaskAssignee(report: OnyxTypes.Report, ownerAccountID: number, assigneeEmail: string, assigneeAccountID = 0, assigneeChatReport: OnyxEntry<OnyxTypes.Report> = null) {

function editTask(report: OnyxTypes.Report, {title, description}: OnyxTypes.Task) {

Based on these functions and the data passed to them, we can determine each of the required flows: description/title being updated/removed, or assignee changed/removed. And build each optimistic report action according to the flow it represents.

What alternative solutions did you explore? (Optional)

Copy link

melvin-bot bot commented Feb 12, 2024

@eVoloshchak, @thienlnam, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@thienlnam
Copy link
Contributor Author

cc @eVoloshchak Could you please review the proposal above?

@eVoloshchak
Copy link
Contributor

This is a feature request, and it's a pretty straightforward one, @paultsimura's proposal looks good to me!

🎀👀🎀 C+ reviewed!

Copy link

melvin-bot bot commented Feb 14, 2024

Current assignee @thienlnam is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 14, 2024
Copy link

melvin-bot bot commented Feb 14, 2024

📣 @paultsimura 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

@quinthar quinthar moved this to FUTURE in [#whatsnext] #vip-vsb Feb 25, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 26, 2024
@MitchExpensify
Copy link
Contributor

Not overdue, waiting on some PRs to hit production

Same

@melvin-bot melvin-bot bot added Overdue Reviewing Has a PR in review Weekly KSv2 and removed Overdue Daily KSv2 labels Feb 26, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Mar 6, 2024
Copy link

melvin-bot bot commented Mar 8, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

Copy link

melvin-bot bot commented Mar 8, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@MitchExpensify
Copy link
Contributor

Same: #36768

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Mar 11, 2024
@melvin-bot melvin-bot bot changed the title [$500] Add edit system messages for tasks [HOLD for payment 2024-03-18] [$500] Add edit system messages for tasks Mar 11, 2024
Copy link

melvin-bot bot commented Mar 11, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Mar 11, 2024
Copy link

melvin-bot bot commented Mar 11, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.49-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 2024-03-18. 🎊

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

Copy link

melvin-bot bot commented Mar 11, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

@MitchExpensify
Copy link
Contributor

MitchExpensify commented Mar 11, 2024

Reminder set to pay!

Payment summary:

C+: $500 @eVoloshchak (NewDot)
C: $500 @paultsimura (Upwork)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Mar 17, 2024
@eVoloshchak
Copy link
Contributor

Regression Test Proposal

  1. Create a task, click on it to open the task details
  2. Go offline
  3. Add a description
  4. Verify the updated the description to {value} report action is added
  5. Modify the title
  6. Verify another updated the task title to {value} report action is added
  7. Remove the description
  8. Verify the removed the description report action is added
  9. Assign somebody
  10. Verify the assigned to @user report action is added
  11. Go back online
  12. Refresh the page
  13. Verify all the report actions remain

Do we agree 👍 or 👎

@MitchExpensify
Copy link
Contributor

Paid and contract ended. BZ step issue created

@github-project-automation github-project-automation bot moved this from FUTURE to CRITICAL in [#whatsnext] #vip-vsb Mar 20, 2024
@JmillsExpensify
Copy link

$500 approved for @eVoloshchak based on summary.

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 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
No open projects
Status: CRITICAL
Development

No branches or pull requests

5 participants