[HOLD for payment 2023-12-06] [$250] Workspace - Custom invitation message is still preserved after sending an invitation

izarutskaya commented 1 year ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Found when executing PR : https://github.com/Expensify/App/pull/30562

Version Number: v1.3.96-0 Reproducible in staging?: Y Reproducible in production?: N 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 Expensify/Expensify Issue URL: Issue reported by: Applause-Internal Team Slack conversation: @

Action Performed:

Navigate to staging.new.expensify.com.
Go to Settings > Workspaces > any workspace.
Go to Members > Invite.
Select any user and proceed to the next page.
Enter a custom invitation message.
Click invite.
Repeat Step 1-5 with another workspace.

Expected Result:

The invitation messages are saved per workspace basis. They are reset after signout out and sign back in but while user is signed in, we keep the welcome message saved per workspace

Actual Result:

The custom invitation message is preserved across policies

Workaround:

Unknown

Platforms:

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

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/115492554/71e3857f-198c-4f74-ae13-228a958e618b

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~017cd1e2909d9808bd
Upwork Job ID: 1721871527814250496
Last Price Increase: 2023-11-14
Automatic offers:

akinwale | Contributor | 27756684
DylanDylann | Contributor | 27785352

Issue Owner

Current Issue Owner: @mallenexpensify

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

OSBotify commented 1 year ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @mountiny (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

GItGudRatio commented 1 year ago

I'm available for a fix immediately.

Proposal

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

Custom invitation message is still preserved after sending an invitation

What is the root cause of that problem?

After sending the message, the savedWelcomeMessage onyx key is not cleared.

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

To fix this issue, we need to clear the onyx key in the sendInvitation method.

Onyx.set(`${ONYXKEYS.FORMS.WORKSPACE_INVITE_MESSAGE_FORM}Draft`, message);

What alternative solutions did you explore? (Optional)

Another logical issue in this implementation is that custom message is the same across all workspaces, even if we do not invite any user. We need to implement the Onyx Key using policy ID and then proceed accordingly.

Pujan92 commented 1 year ago

Seems like a regression from https://github.com/Expensify/App/issues/27902

allroundexperts commented 1 year ago

As per https://github.com/Expensify/App/issues/27902#issuecomment-1781394926, this is the expected behaviour.

mountiny commented 1 year ago

This seems to be intended behaviour now.

GItGudRatio commented 1 year ago

Just wondering about a feature request - What if I want to reset to the default welcome message? There is no way to do that with the current workflow I think

mountiny commented 1 year ago

We will discuss if we want to change anything about this flow or not.

melvin-bot[bot] commented 1 year ago

Upwork job price has been updated to $250

melvin-bot[bot] commented 1 year ago

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

mountiny commented 1 year ago

Based on the internal discussion this is not a bug but a new feature which is desired, however as such I am making this weekly and lower value issue as we dont have to focus on this so much https://expensify.slack.com/archives/C03U7DCU4/p1699364509319509

GItGudRatio commented 1 year ago

@mountiny Thank you for the update, can you please share the latest expected results?

mountiny commented 1 year ago

I have updated the OP, let me know if its not clear, thanks!

GItGudRatio commented 1 year ago

Proposal

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

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?

The simplest way to implement the change would be via these 2 changes:

https://github.com/Expensify/App/blob/f7128fec97e18d2252de1e59f0dd9a302fb276af/src/pages/workspace/WorkspaceInviteMessagePage.js#L256

key: ({route}) => `${ONYXKEYS.FORMS.WORKSPACE_INVITE_MESSAGE_FORM}_${route.params.policyID.toString()}Draft`,

and this: https://github.com/Expensify/App/blob/f7128fec97e18d2252de1e59f0dd9a302fb276af/src/pages/workspace/WorkspaceInviteMessagePage.js#L181

formID={`${ONYXKEYS.FORMS.WORKSPACE_INVITE_MESSAGE_FORM}_${this.props.route.params.policyID.toString()}`}

What alternative solutions did you explore? (Optional)

thesahindia commented 1 year ago

@GItGudRatio, please follow the proposal guidelines correctly. You

What is the root cause of that problem?

This is a feature request.

You have to add some info about the code that is responsible for the current behaviour.

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

The simplest way to implement the change would be via these 2 changes:

You need to use simple english to explain your solution. You have to describe what your solution is and how it will work.

DylanDylann commented 12 months ago

Proposal

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

Feature request: The invitation messages are saved per workspace basis. They are reset after signout out and sign back in but while user is signed in, we keep the welcome message saved per workspace

What is the root cause of that problem?

Currently, we are using workspaceInviteMessageFormDraft to store the invite message draft. Here is an example value:

{welcomeMessage: "You have been invited to ws! Download the Expensify mobile app at use.expensify.com/download to start tracking your expenses. "}

Once we edit the invite message, it will update the welcomeMessage. The workspaceInviteMessageFormDraft value is shared between the workspaces.

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

Generally, we need to store the draft invite message in workspace separately (Same as what we did with invitedEmailsToAccountIDsDraft ) https://github.com/Expensify/App/blob/82ee5775e9beecaf3163d13199ffd91d7b521527/src/pages/workspace/WorkspaceInviteMessagePage.js#L252-L254 Do it by these below steps:
1. Create a collection WORKSPACE_INVITE_MESSAGE_DRAFT
2. Create a set method for this collection
```
function setWorkspaceInviteMessageDraft(policyID, message) {
Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_INVITE_MESSAGE_DRAFT}${policyID}`, message)
}
```
3. Subscribe the ONYXKEYS.COLLECTION.WORKSPACE_INVITE_MESSAGE_DRAFT by updating: https://github.com/Expensify/App/blob/82ee5775e9beecaf3163d13199ffd91d7b521527/src/pages/workspace/WorkspaceInviteMessagePage.js#L255-L258 to:
```
workspaceInviteMessageDraft: {
    key: `${ONYXKEYS.COLLECTION.WORKSPACE_INVITE_MESSAGE_DRAFT}`,
},
```
4. Set shouldSaveDraft: false because we do not need to use the form draft feature anymore: https://github.com/Expensify/App/blob/82ee5775e9beecaf3163d13199ffd91d7b521527/src/pages/workspace/WorkspaceInviteMessagePage.js#L231
5. Get the initial invite message from workspaceInviteMessageDraft by updating: https://github.com/Expensify/App/blob/82ee5775e9beecaf3163d13199ffd91d7b521527/src/pages/workspace/WorkspaceInviteMessagePage.js#L79 to: welcomeNote: this.props.workspaceInviteMessageDraft || this.getDefaultWelcomeNote(),
6. Lastly, once the invite message change, update the onyx data. Do it by updating: https://github.com/Expensify/App/blob/82ee5775e9beecaf3163d13199ffd91d7b521527/src/pages/workspace/WorkspaceInviteMessagePage.js#L230 to:
```
onChangeText={(text) => {
                            Policy.setWorkspaceInviteMessageDraft(this.props.route.params.policyID, text);
                            this.setState({welcomeNote: text});
                        }}
```
  What alternative solutions did you explore? (Optional)
NA

melvin-bot[bot] commented 12 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

mallenexpensify commented 12 months ago

@thesahindia can you please review @DylanDylann 's proposal above?

melvin-bot[bot] commented 11 months ago

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

mountiny commented 11 months ago

Bumped for proposal review in Slack

thesahindia commented 11 months ago

Sorry for the delay. I missed the notifications and, unfortunately, I will need to unassign myself. I'm not in a good shape.

melvin-bot[bot] commented 11 months ago

📣 @akinwale 🎉 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 📖

akinwale commented 11 months ago

Taking over as C+ here.

akinwale commented 11 months ago

@DylanDylann's proposed solution follows an existing pattern of storing relevant data on a per-workspace basis using Onyx, so I recommend we move forward with the proposal.

🎀 👀 🎀 C+ reviewed.

melvin-bot[bot] commented 11 months ago

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

thienlnam commented 11 months ago

Looks like @mountiny is already assigned and has the context so will leave to him

melvin-bot[bot] commented 11 months ago

📣 @DylanDylann 🎉 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 📖

DylanDylann commented 11 months ago

@akinwale PR https://github.com/Expensify/App/pull/31770 is ready to review

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

https://github.com/Expensify/App/pull/31770

If no regressions arise, payment will be issued on 2023-12-06. :confetti_ball:

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:

@akinwale requires payment offer (Contributor)
@DylanDylann requires payment offer (Contributor)

melvin-bot[bot] commented 11 months ago

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:

[ ] [@akinwale] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
[ ] [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

mallenexpensify commented 11 months ago

Contributor: @DylanDylann paid $250 via Upwork Contributor+: @akinwale paid $250 via Upwork

@akinwale can you please propose the regression test steps to add for the new feature? Thx

melvin-bot[bot] commented 11 months ago

@akinwale, @mallenexpensify, @mountiny, @DylanDylann Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

mountiny commented 11 months ago

@akinwale bump please

akinwale commented 11 months ago

@akinwale bump please

Missed the initial notification. I'll get to it within an hour.

akinwale commented 11 months ago

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:

[x] [@akinwale] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.

Launch Expensify
Navigate to Settings > Workspaces and create at least two workspaces if there are no workspaces, or create an additional one if there is only one workspace created.
Select a workspace and navigate to Members > Invite.
Select any user and navigate to the next step.
Enter a custom invitation message and click invite.
Navigate back and repeat steps 3 through 6 for a different workspace.
Verify that the invitation message is different for the other workspace.
Repeat steps 3 through 8 for different workspaces
Verify that the custom invitation message entered for each corresponding workspace does not change.
Verify that two or more workspaces do not have the same custom invitation message (assuming all the custom invitation messages entered are different).

Do we agree 👍 or 👎?

mountiny commented 11 months ago

@akinwale I would update this to make it clear that the custom message should stay same on the particular workspace, ie its different across workspaces but remains unchanged on the same workspace

akinwale commented 11 months ago

@mountiny Updated.

mountiny commented 11 months ago

Thanks, @mallenexpensify all yours!

mallenexpensify commented 11 months ago

Thanbks @akinwale !

https://github.com/Expensify/Expensify/issues/348703

Expensify / App