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

[Advanced approvals] Update pendingAction/pendingFields when making an action offline (create/edit/remove) #47701

Closed
dylanexpensify opened this issue Aug 20, 2024 · 25 comments
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Monthly KSv2 Reviewing Has a PR in review Task

Comments

@dylanexpensify
Copy link
Contributor

dylanexpensify commented Aug 20, 2024

Coming from this project, we have this follow-up polish issue we need to complete where we update pendingAction and pendingFields when making an action offline. More details here.

Issue OwnerCurrent Issue Owner: @
@dylanexpensify
Copy link
Contributor Author

cc @blazejkustra

@blazejkustra
Copy link
Contributor

Hey! I’m Błażej Kustra from Software Mansion, an expert agency, and I’d like to work on this issue!

@dylanexpensify dylanexpensify added the External Added to denote the issue can be worked on by a contributor label Aug 20, 2024
Copy link

melvin-bot bot commented Aug 20, 2024

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 20, 2024
Copy link

melvin-bot bot commented Aug 20, 2024

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

@dylanexpensify
Copy link
Contributor Author

Hi @blazejkustra! It's all yours!

@blazejkustra
Copy link
Contributor

  1. Currently the whole section is greyed out when there are changes in any workflow (following offline pattern B), I reckon we want to grey out only these workflows that are affected?
Screen.Recording.2024-08-20.at.16.07.47.mov
  1. How should approval workflows section be displayed when the user is offline, should it be also greyed out?

image

Thoughts @tgolen @JmillsExpensify @DylanDylann?

@DylanDylann
Copy link
Contributor

@blazejkustra When users take actions, they could update submitTo field or forwardsTo field. IMO, If these actions is triggered offline we can add pendingFields in the employee object like this

Screenshot 2024-08-21 at 15 22 10
{
              .....
             pendingFields: {
                submitTo: "UPDATE"
                forwardTo: "UPDATE"
            }
}

And then when displaying the approval workflow, for each workflow

  • we will grey out "expenses member" if there is at least a member that has pendingFields.submitTo
  • we will grey out "first approver" if there is at least one member from the "expenses member" section that has pendingFields.submitTo
  • We will grey out next approvers if the previous approver has pendingFields.forwardTo

@blazejkustra What do you think about this approach?

@blazejkustra
Copy link
Contributor

Thank you for the idea of how this could be implemented, this is definietly a way to go 👌

I think it may be a little overcomplicated, and I'm still not sure how important is it to show exactly what people were changed offline vs grey out the whole workflow. Waiting for input from others 🙇

@JmillsExpensify
Copy link

Currently the whole section is greyed out when there are changes in any workflow (following offline pattern B), I reckon we want to grey out only these workflows that are affected?

I tend to agree with you, though I would say this is a low priority.

How should approval workflows section be displayed when the user is offline, should it be also greyed out?

Are you asking in general? If so, if a user is offline we don't need to grey the whole section out, we only use the offline patterns to give feedback when changes are made (including changes that can only be made online, so they are blocked offline).

@DylanDylann
Copy link
Contributor

@blazejkustra @JmillsExpensify What are we waiting for here?

@blazejkustra
Copy link
Contributor

@DylanDylann I'll get to this one tomorrow, sorry 🙇

@blazejkustra
Copy link
Contributor

@blazejkustra When users take actions, they could update submitTo field or forwardsTo field. IMO, If these actions is triggered offline we can add pendingFields in the employee object like this

And then when displaying the approval workflow, for each workflow

  • we will grey out "expenses member" if there is at least a member that has pendingFields.submitTo
  • we will grey out "first approver" if there is at least one member from the "expenses member" section that has pendingFields.submitTo
  • We will grey out next approvers if the previous approver has pendingFields.forwardTo

@blazejkustra What do you think about this approach?

I tried it @DylanDylann here: #48082 and have to say that it sounds better/easier on paper 😢 This approach doesn't work well when removing members - we should grey out "expenses from", but there is no field we can base the logic on. The other thing is that when updating a workflow all members and approvers are sent to the backend (not only people who were updated, but all people involved in the workflow). All in all, I think it creates some weird looking offline states where only part of the workflow is greyed out instead of the whole section:

image

Alternative approach is to grey out the whole workflow section in case any person was affected (added/updated/removed), I think this should be easier to code as well. Thoughts @DylanDylann?

@blazejkustra
Copy link
Contributor

blazejkustra commented Aug 27, 2024

@DylanDylann I came up with this draft PR, here is a video of how it works with these changes:

Screen.Recording.2024-08-27.at.15.34.27.mov

I'm open for your suggestions, but have in mind this issue has a low priority as Jason mentioned above.

@DylanDylann
Copy link
Contributor

@blazejkustra Let's bring this to Slack. We need to confirm the approach before moving forward

@blazejkustra
Copy link
Contributor

I moved the discussion to #wave-control channel here

@blazejkustra
Copy link
Contributor

@DylanDylann PR is up 👍

Copy link

melvin-bot bot commented Sep 2, 2024

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

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Sep 20, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

This issue has not been updated in over 15 days. @youssef-lr, @blazejkustra, @DylanDylann 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!

@DylanDylann
Copy link
Contributor

@Youssef-or Could you help process the payment here?

@dylanexpensify
Copy link
Contributor Author

Payment summary:

Contributor+: @DylanDylann $250 via Upwork

Please apply/request!

@DylanDylann
Copy link
Contributor

DylanDylann commented Oct 4, 2024

Please apply/request!

@dylanexpensify I don't see any offer for this issue

@dylanexpensify
Copy link
Contributor Author

Ah automation failed, lemme whip one up!

@DylanDylann
Copy link
Contributor

@dylanexpensify Have you got a chance to create the offer?
My profile on Upwork is https://www.upwork.com/freelancers/~010c6310ab55f3d17d in case you're looking for it.

Thanks 🙏

@DylanDylann
Copy link
Contributor

@dylanexpensify Bump on above comment

@dylanexpensify
Copy link
Contributor Author

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Monthly KSv2 Reviewing Has a PR in review Task
Projects
Status: Done
Development

No branches or pull requests

5 participants