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 2023-09-29] [$1000] Show different UI for a cancelled task #19504

Closed
thienlnam opened this issue May 23, 2023 · 43 comments
Closed
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2

Comments

@thienlnam
Copy link
Contributor

thienlnam commented May 23, 2023

Right now, when a task is cancelled nothing about the task preview changes

Screenshot 2023-05-23 at 4 18 48 PM

Updates:

The task report is no longer archived. We will adjust the UI if a task parentReportID is deleted.
image

TODO:

  • Update the taskPreview to show [Deleted task]
  • Update the copy from Cancel task to Delete Task
  • Show the task detailed view in a 'thread' view
  • Update the header to also show [Deleted task]
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fbfabca6cddecf7f
  • Upwork Job ID: 1665781150641840128
  • Last Price Increase: 2023-06-05
@thienlnam thienlnam added the Weekly KSv2 label May 23, 2023
@melvin-bot melvin-bot bot added the Overdue label Jun 5, 2023
@thienlnam thienlnam changed the title [HOLD] Show different UI for a cancelled task Show different UI for a cancelled task Jun 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

@shawnborton
Copy link
Contributor

Can you start a Slack discussion for this? I think the strikethrough might not be a good idea because that's an offline pattern to indicate that something would be deleted

cc @JmillsExpensify @trjExpensify

Also, I'm curious why a task can even be cancelled in the first place? Why not just delete it?

@melvin-bot melvin-bot bot removed the Overdue label Jun 5, 2023
@thienlnam
Copy link
Contributor Author

Started thread here: https://expensify.slack.com/archives/C04QEB4MJEQ/p1685984349292879

Seems like what we need to do here is:
Update the taskPreview to show [deleted task]

@thienlnam thienlnam removed the Design label Jun 5, 2023
@thienlnam thienlnam self-assigned this Jun 5, 2023
@thienlnam thienlnam added the External Added to denote the issue can be worked on by a contributor label Jun 5, 2023
@melvin-bot melvin-bot bot changed the title Show different UI for a cancelled task [$1000] Show different UI for a cancelled task Jun 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01fbfabca6cddecf7f

@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

Triggered auto assignment to @greg-schroeder (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

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

@Vishrut19
Copy link
Contributor

how to create a task?

@sethraj14
Copy link

sethraj14 commented Jun 5, 2023

Proposal

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

Show different UI for a cancelled task

What is the root cause of that problem?

We use this to show cancel task -
canceled: 'Canceled task'

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

Change the UI for cancelled task to this

canceled: '[Deleted task]',

Screenshot 2023-06-06 at 12 00 47 AM

What alternative solutions did you explore? (Optional)

If we want to hide the task title, we can hide the title when task is deleted -

 {props.actionName !== CONST.REPORT.ACTIONS.TYPE.TASKCANCELED  && <Text style={[styles.chatItemMessage]}>{` ${taskReportName}`}</Text>}

Screenshot 2023-06-06 at 12 06 43 AM

@kushu7
Copy link
Contributor

kushu7 commented Jun 5, 2023

Proposal

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

Show different UI for a cancelled task

What is the root cause of that problem?

Not a bug, its enchantment.

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

For achieving this result. we need to refactor TaskPreview.js
We need to add childReportID prop to it. required to get current state of task.
will get report using withOnyx.

childTaskReport:{
    key:({childReportID})=> `${ONYXKEYS.COLLECTION.REPORT}${childReportID}`,
    }

then we can update component to check if task is cancelled or not.

const isTaskCancelled = props.childTaskReport.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && props.childTaskReport.statusNum === CONST.REPORT.STATUS.CLOSED
    if(isTaskCancelled) {
        return  <Text>{props.translate('task.messages.deleted')}</Text>
    }
    // other code
    return (
        ...
    )

and we need to pass childReportID prop to TaskPreview from ReportActionItem.

children = (
<TaskPreview
taskReportID={props.action.originalMessage.taskReportID.toString()}
action={props.action}
isHovered={hovered}
/>
);

we can update this to:

 <TaskPreview
    taskReportID={props.action.originalMessage.taskReportID.toString()}
    childReportID={props.action.childReportID.toString()}
    action={props.action}
    isHovered={hovered}
/>
Video
sScreen.Recording.2023-06-06.at.12.45.12.AM.mov

What alternative solutions did you explore? (Optional)

None

@aidear3
Copy link

aidear3 commented Jun 5, 2023

Contributor details
Your Expensify account email: javsfdev323@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01b7fc496875b2b3dd

@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

⚠️ Invalid email. Please make sure to create an Expensify account with this email first here.

@spiderman128
Copy link

Contributor details
Your Expensify account email: talent111dev@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0199bfb95677b75486

@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@greg-schroeder
Copy link
Contributor

Hey @thienlnam is this issue complete? It seems the PR was deployed to Prod... and also seems like maybe it was 100% Internal?

@thienlnam
Copy link
Contributor Author

The issue requires BE and FE changes - I'm currently working on the App (FE) changes in #24137

Mostly internal, but we'll still need to pay C+ for reviewing the App PR when it is ready

@greg-schroeder
Copy link
Contributor

Gotcha 👍

@greg-schroeder
Copy link
Contributor

Draft PR continues to be worked on

@thienlnam
Copy link
Contributor Author

Heads down in wave 5 this week so haven't been able to get around to this

@greg-schroeder
Copy link
Contributor

Sounds like Jack will get to this soon!

@greg-schroeder
Copy link
Contributor

Sorry - can you help me understand where we're at on this one @thienlnam?

@thienlnam
Copy link
Contributor Author

App PR in review!

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Sep 22, 2023
@melvin-bot melvin-bot bot changed the title [$1000] Show different UI for a cancelled task [HOLD for payment 2023-09-29] [$1000] Show different UI for a cancelled task Sep 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Sep 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 22, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.72-11 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-09-29. 🎊

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:

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

@greg-schroeder
Copy link
Contributor

This is internal and there's no issue reporter. So only @mollfpr requires payment. Looks like that'll be $1,000 - you can make a manual request!

@greg-schroeder
Copy link
Contributor

I don't think we need the traditional C+ checklist for this, but please let me know if you disagree. Closing this out as payment will be handled via NewDot.

@JmillsExpensify
Copy link

$1,000 payment approved for @mollfpr based on comment above.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Jan 10, 2024
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 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests