[HOLD for payment 2024-10-07] Workspace - Invite message comes back after deleting it #49899

lanitochka17 · 2024-09-29T18:33:39Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: 9.0.41-2
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

Login to an account
Create a workspace
Go to the created workspace > Members
Click on invite member > Choose a user
On "add message" page try to delete the message

Expected Result:

Message is deleted

Actual Result:

The deleted message comes back

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6618028_1727488696018.Screen_Recording_2024-09-28_at_4.50.36_at_night.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @JmillsExpensify

melvin-bot · 2024-09-29T18:33:55Z

Triggered auto assignment to @dangrous (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions · 2024-09-29T18:33:55Z

👋 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.

Nodebrute · 2024-09-29T18:54:24Z

Edited by proposal-police: This proposal was edited at 2024-09-29 18:55:32 UTC.

Proposal

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

Invite message comes back after deleting it

What is the root cause of that problem?

We are setting setWelcomeNote(getDefaultWelcomeNote()) again here

App/src/pages/workspace/WorkspaceInviteMessagePage.tsx

Lines 89 to 94 in 3047c1b

    
           useEffect(() => { 
        
               if (isEmptyObject(invitedEmailsToAccountIDsDraft)) { 
        
                   return; 
        
               } 
        
               setWelcomeNote(getDefaultWelcomeNote()); 
        
           }, [getDefaultWelcomeNote, invitedEmailsToAccountIDsDraft]);

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

We can remove this code block

App/src/pages/workspace/WorkspaceInviteMessagePage.tsx

Lines 89 to 94 in 3047c1b

    
           useEffect(() => { 
        
               if (isEmptyObject(invitedEmailsToAccountIDsDraft)) { 
        
                   return; 
        
               } 
        
               setWelcomeNote(getDefaultWelcomeNote()); 
        
           }, [getDefaultWelcomeNote, invitedEmailsToAccountIDsDraft]);

We should test the code to make sure everything is working properly

What alternative solutions did you explore? (Optional)

Krishna2323 · 2024-09-29T19:16:35Z

Edited by proposal-police: This proposal was edited at 2024-09-29 19:17:45 UTC.

Proposal 

Offending PR: #48660

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

Workspace - Invite message comes back after deleting it

What is the root cause of that problem?

We have multiple useEffect hooks to set the welcome note when the component mounts or the members change.

App/src/pages/workspace/WorkspaceInviteMessagePage.tsx

Lines 77 to 94 in 3047c1b

    
           useEffect(() => { 
        
               if (!isEmptyObject(invitedEmailsToAccountIDsDraft)) { 
        
                   setWelcomeNote(getDefaultWelcomeNote()); 
        
                   return; 
        
               } 
        
               if (isEmptyObject(policy)) { 
        
                   return; 
        
               } 
        
               Navigation.goBack(ROUTES.WORKSPACE_INVITE.getRoute(route.params.policyID), true); 
        
               // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps 
        
           }, []); 
        
           useEffect(() => { 
        
               if (isEmptyObject(invitedEmailsToAccountIDsDraft)) { 
        
                   return; 
        
               } 
        
               setWelcomeNote(getDefaultWelcomeNote()); 
        
           }, [getDefaultWelcomeNote, invitedEmailsToAccountIDsDraft]);

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

We can remove the second useEffect hook here:

App/src/pages/workspace/WorkspaceInviteMessagePage.tsx

Lines 89 to 94 in 3047c1b

    
           useEffect(() => { 
        
               if (isEmptyObject(invitedEmailsToAccountIDsDraft)) { 
        
                   return; 
        
               } 
        
               setWelcomeNote(getDefaultWelcomeNote()); 
        
           }, [getDefaultWelcomeNote, invitedEmailsToAccountIDsDraft]);

And in WorkspaceInvitePage.tsx we need to clear the draft invite message when the component unmounts.

App/src/pages/workspace/WorkspaceInvitePage.tsx

Lines 78 to 83 in 3047c1b

    
           useEffect(() => { 
        
               return () => { 
        
                   Member.setWorkspaceInviteMembersDraft(route.params.policyID, {}); 
        
               }; 
        
               // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps 
        
           }, [route.params.policyID]);

TO:

    useEffect(() => {
        return () => {
            Member.setWorkspaceInviteMembersDraft(route.params.policyID, {});
            Policy.setWorkspaceInviteMessageDraft(route.params.policyID, null);
        };
        // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
    }, [route.params.policyID]);

If we want to clear the draft message when invite members list changes, we can clear the message draft here or here.

What alternative solutions did you explore? (Optional)

Result

Shahidullah-Muffakir · 2024-09-29T21:40:12Z

Proposal

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

when a user deletes the invite message in the workspace invite process, the message reappears unexpectedly.

What is the root cause of that problem?

The root cause of this problem is that the component was automatically resetting the welcome message to its default value, even when the user had intentionally deleted it

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

we should update the useEffect to add a check that skips resetting the invite message if it is already empty. This ensures that when the user deletes the message, it doesn't get reset to the default value.

Add!welcomeNotecheck here:

App/src/pages/workspace/WorkspaceInviteMessagePage.tsx

Lines 89 to 92 in 3047c1b

    
           useEffect(() => { 
        
               if (isEmptyObject(invitedEmailsToAccountIDsDraft)) { 
        
                   return; 
        
               }

to:

 useEffect(() => {
        if (isEmptyObject(invitedEmailsToAccountIDsDraft) || !welcomeNote) {
            return;
        }

Screen.20Recording.202024-09-30.20at.203.mp4

huult · 2024-09-30T02:26:40Z

Proposal

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

Invite message comes back after deleting it

What is the root cause of that problem?

Since we set getDefaultWelcomeNote as a dependency of this hook, the hook is triggered every time the input's onChange event occurs

App/src/pages/workspace/WorkspaceInviteMessagePage.tsx

Line 94 in 3634670

}, [getDefaultWelcomeNote, invitedEmailsToAccountIDsDraft]);

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

We should remove getDefaultWelcomeNote from the dependency array to avoid re-renders when the input changes.

// .src/pages/workspace/WorkspaceInviteMessagePage.tsx#L94
    useEffect(() => {
        if (isEmptyObject(invitedEmailsToAccountIDsDraft)) {
            return;
        }
        setWelcomeNote(getDefaultWelcomeNote());
-    }, [getDefaultWelcomeNote, invitedEmailsToAccountIDsDraft]);
+    }, [invitedEmailsToAccountIDsDraft]);

and We need to validate this input, if necessary, to avoid empty values ("")

// .src/pages/workspace/WorkspaceInviteMessagePage.tsx#L119
        if (isEmptyObject(invitedEmailsToAccountIDsDraft)) {
            errorFields.welcomeMessage = translate('workspace.inviteMessage.inviteNoMembersError');
-        }
+        } else if (isEmptyObject(welcomeNote)) {
+            errorFields.welcomeMessage = 'Please enter a welcome note.';
        }

POC

Screen.Recording.2024-09-30.at.09.24.15.mp4

Gonals · 2024-09-30T10:51:37Z

Seems to be a frontend issue, so removing Web-E blocker

dangrous · 2024-09-30T15:20:46Z

Yeah this was caused by PR #48660 and issue #34929 . We can just remove that new useEffect but I'm not sure what the point of it was originally, so pinged the PR authors. Thanks!

rojiphil · 2024-09-30T17:05:59Z

Yeah this was caused by PR #48660 and issue #34929 .

@dangrous Yeah. Fixing an eslint error (i.e. using useOnyx instead of withOnyx) during some last-minute changes caused this issue. I think we can consider this as a separate issue. Please assign me here as I have the context.

We can just remove that new useEffect but I'm not sure what the point of it was originally, so pinged the PR authors. Thanks!

We need the second useEffect to ensure workspaceInviteMessageDraft is populated from the Onyx. However, an edge case got left out i.e. when the user intentionally deletes the entire welcome note in which case we should not reset.
@Shahidullah-Muffakir proposal LGTM

dangrous · 2024-09-30T17:33:11Z

Yep checking for an empty welcome note makes sense to me. Can you raise that PR quick?

rojiphil · 2024-09-30T18:27:15Z

Yep checking for an empty welcome note makes sense to me. Can you raise that PR quick?

Sure. Working on the PR now. Will update in about an hour

rojiphil · 2024-09-30T19:40:40Z

@dangrous PR is ready for review.

dannymcclain · 2024-10-17T18:53:25Z

I think you've outlined it exactly how I was understanding it. 👍

rojiphil · 2024-10-17T18:59:06Z

are you up for tackling that adjustment, or should we open it up to contributors again?

@dangrous I think it’s best to open it up to contributors again to get the best proposal as this does not seem like a minor adjustment.

dangrous · 2024-10-18T13:55:38Z

Okay cool! I may actually just make a new issue for it, so we can close this one out / pay. It's more of a clarification on expected behavior than a regression or anything that should be handled here.

I'm also going to hold it on #49996 which is related and should make things easier.

dangrous · 2024-10-18T16:27:17Z

Made #51096!

rojiphil · 2024-10-18T18:00:43Z

Okay cool! I may actually just make a new issue for it, so we can close this one out / pay.

Yeah. Since another issue is created for the enhancement, I think we can pay as per this comment and close this one out. cc @JmillsExpensify

JmillsExpensify · 2024-10-21T11:44:55Z

Ok, so based on that comment, we'd have:

Contributor: $125 @rojiphil via Upwork
Contributor: $125 @Shahidullah-Muffakir via Upwork

JmillsExpensify · 2024-10-22T13:38:43Z

Offers sent to both via Upwork.

Shahidullah-Muffakir · 2024-10-22T14:38:25Z

Offers sent to both via Upwork.

Accepted, Thank you.

rojiphil · 2024-10-22T15:24:20Z

Offers sent to both via Upwork.

Accepted the offer. Thanks

JmillsExpensify · 2024-10-23T11:51:25Z

Both paid out. Thank you!

melvin-bot · 2024-10-23T21:53:52Z

Current assignee @JmillsExpensify is eligible for the Bug assigner, not assigning anyone new.

tgolen · 2024-10-23T21:54:49Z

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:

[@rojiphil] The PR that introduced the bug has been identified. Link to the PR:
[@rojiphil] 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:
[@rojiphil] 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:
[@rojiphil] Determine if we should create a regression test for this bug.
[@rojiphil] 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] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

rojiphil · 2024-10-24T16:01:09Z

[@rojiphil] The PR that introduced the bug has been identified. Link to the PR: Offending PR
[@rojiphil] 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: Added comment
[@rojiphil] 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: Not Required. Existing checklist is good enough to capture such issues.
[@rojiphil] Determine if we should create a regression test for this bug. : Yes. We can
[@rojiphil] 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.

Steps:

Go to Account Settings -> Workspaces
Select a workspace and go to Members->Invite new members
Select a member to invite and click on Next to go to the invite message view.
Delete the default welcome message text.
Verify that the message text remains empty and does not come back.

rojiphil · 2024-10-24T16:01:52Z

@tgolen BZ Checklist done

rojiphil · 2024-10-26T09:37:19Z

@tgolen The payment step is completed as mentioned here.
Can we close this issue if nothing else is pending? Thanks.

tgolen · 2024-10-28T14:58:35Z

Yep, we should be good here now. I'll close it out. Thanks!

tgolen · 2024-10-28T14:59:13Z

Actually, I was wrong. @JmillsExpensify this needs a regression test created and I think that's your part of the checklist.

JmillsExpensify · 2024-10-31T10:33:38Z

Test created and linked above, so closing this issue.

lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Sep 29, 2024

github-actions bot added Engineering Hourly KSv2 labels Sep 29, 2024

melvin-bot bot assigned dangrous Sep 29, 2024

github-actions bot mentioned this issue Sep 29, 2024

Deploy Checklist: New Expensify 2024-09-27 #49855

Closed

Gonals removed the DeployBlocker Indicates it should block deploying the API label Sep 30, 2024

dangrous mentioned this issue Sep 30, 2024

create announce room when 3 or more participants #48660

Merged

58 tasks

melvin-bot bot mentioned this issue Sep 30, 2024

[Hold #41442] [Guided Setup Stage 3][$500] Don't create default #announce rooms until a workspace has 3 or more members #34929

Closed

dangrous assigned rojiphil Sep 30, 2024

rojiphil mentioned this issue Sep 30, 2024

fix for empty welcome note #49955

Merged

58 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Sep 30, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Sep 30, 2024

melvin-bot bot changed the title ~~Workspace - Invite message comes back after deleting it~~ [HOLD for payment 2024-10-07] Workspace - Invite message comes back after deleting it Sep 30, 2024

dangrous mentioned this issue Oct 18, 2024

[HOLD #49996][$250] Update Default / Custom Workspace Invite Behavior #51096

Open

melvin-bot bot added the Overdue label Oct 21, 2024

melvin-bot bot removed the Overdue label Oct 21, 2024

JmillsExpensify closed this as completed Oct 23, 2024

tgolen added the Bug Something is broken. Auto assigns a BugZero manager. label Oct 23, 2024

tgolen reopened this Oct 23, 2024

tgolen closed this as completed Oct 28, 2024

tgolen reopened this Oct 28, 2024

melvin-bot bot added the Overdue label Oct 30, 2024

JmillsExpensify closed this as completed Oct 31, 2024

melvin-bot bot removed Overdue labels Oct 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-10-07] Workspace - Invite message comes back after deleting it #49899

[HOLD for payment 2024-10-07] Workspace - Invite message comes back after deleting it #49899

lanitochka17 commented Sep 29, 2024 •

edited by dangrous

Loading

melvin-bot bot commented Sep 29, 2024

github-actions bot commented Sep 29, 2024

Nodebrute commented Sep 29, 2024 •

edited by github-actions bot

Loading

Krishna2323 commented Sep 29, 2024 •

edited

Loading

Shahidullah-Muffakir commented Sep 29, 2024

huult commented Sep 30, 2024

Gonals commented Sep 30, 2024

dangrous commented Sep 30, 2024

rojiphil commented Sep 30, 2024

dangrous commented Sep 30, 2024

rojiphil commented Sep 30, 2024

rojiphil commented Sep 30, 2024

dannymcclain commented Oct 17, 2024

rojiphil commented Oct 17, 2024

dangrous commented Oct 18, 2024

dangrous commented Oct 18, 2024

rojiphil commented Oct 18, 2024

JmillsExpensify commented Oct 21, 2024

JmillsExpensify commented Oct 22, 2024

Shahidullah-Muffakir commented Oct 22, 2024

rojiphil commented Oct 22, 2024

JmillsExpensify commented Oct 23, 2024

melvin-bot bot commented Oct 23, 2024

tgolen commented Oct 23, 2024

rojiphil commented Oct 24, 2024

rojiphil commented Oct 24, 2024

rojiphil commented Oct 26, 2024

tgolen commented Oct 28, 2024

tgolen commented Oct 28, 2024

JmillsExpensify commented Oct 31, 2024

[HOLD for payment 2024-10-07] Workspace - Invite message comes back after deleting it #49899

[HOLD for payment 2024-10-07] Workspace - Invite message comes back after deleting it #49899

Comments

lanitochka17 commented Sep 29, 2024 • edited by dangrous Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Sep 29, 2024

github-actions bot commented Sep 29, 2024

Nodebrute commented Sep 29, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Krishna2323 commented Sep 29, 2024 • edited Loading

Proposal

Offending PR: #48660

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Result

Shahidullah-Muffakir commented Sep 29, 2024

Proposal

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

What is the root cause of that problem?

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

huult commented Sep 30, 2024

Proposal

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

What is the root cause of that problem?

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

Gonals commented Sep 30, 2024

dangrous commented Sep 30, 2024

rojiphil commented Sep 30, 2024

dangrous commented Sep 30, 2024

rojiphil commented Sep 30, 2024

rojiphil commented Sep 30, 2024

dannymcclain commented Oct 17, 2024

rojiphil commented Oct 17, 2024

dangrous commented Oct 18, 2024

dangrous commented Oct 18, 2024

rojiphil commented Oct 18, 2024

JmillsExpensify commented Oct 21, 2024

JmillsExpensify commented Oct 22, 2024

Shahidullah-Muffakir commented Oct 22, 2024

rojiphil commented Oct 22, 2024

JmillsExpensify commented Oct 23, 2024

melvin-bot bot commented Oct 23, 2024

tgolen commented Oct 23, 2024

rojiphil commented Oct 24, 2024

rojiphil commented Oct 24, 2024

rojiphil commented Oct 26, 2024

tgolen commented Oct 28, 2024

tgolen commented Oct 28, 2024

JmillsExpensify commented Oct 31, 2024

lanitochka17 commented Sep 29, 2024 •

edited by dangrous

Loading

Nodebrute commented Sep 29, 2024 •

edited by github-actions bot

Loading

Krishna2323 commented Sep 29, 2024 •

edited

Loading

Proposal 

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