-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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-11-07] [HOLD for payment 2024-10-30] [$125.00] Workspace -Previous invite message reappears after clearing the field and entering a character #49996
Comments
Triggered auto assignment to @adelekennedy ( |
@adelekennedy FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
We think that this bug might be related to #wave-collect - Release 1 |
ProposalPlease re-state the problem that we are trying to solve in this issue.Workspace -Previous invite message reappears after clearing the field and entering a character What is the root cause of that problem?The root cause of this problem stems from the interaction between the App/src/pages/workspace/WorkspaceInviteMessagePage.tsx Lines 192 to 193 in 2c07b39
What changes do you think we should make in order to solve the problem?We should either remove the What alternative solutions did you explore? (Optional) |
Edited by proposal-police: This proposal was edited at 2024-10-01 14:39:43 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Previous invite message reappears after clearing the field due to re-render What is the root cause of that problem?It is happening because we have App/src/pages/workspace/WorkspaceInviteMessagePage.tsx Lines 63 to 75 in d46f218
which is causing this App/src/pages/workspace/WorkspaceInviteMessagePage.tsx Lines 89 to 94 in d46f218
What changes do you think we should make in order to solve the problem?By removing
What alternative solutions did you explore? (Optional)None |
Edited by proposal-police: This proposal was edited at 2024-10-01 14:54:15 UTC. ProposalPlease 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
When we remove all the text and input new text,
and
What changes do you think we should make in order to solve the problem?We should remove // .src/pages/workspace/WorkspaceInviteMessagePage.tsx#L94
useEffect(() => {
- if (isEmptyObject(invitedEmailsToAccountIDsDraft) || !welcomeNote) {
+ if (isEmptyObject(invitedEmailsToAccountIDsDraft)) {
return;
}
setWelcomeNote(getDefaultWelcomeNote());
- }, [getDefaultWelcomeNote, invitedEmailsToAccountIDsDraft, welcomeNote]);
+ }, [invitedEmailsToAccountIDsDraft]); and We need to validate this input, if necessary, to avoid empty values ("") (optional) // .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-10-01.at.21.25.03.mov |
ProposalPlease re-state the problem that we are trying to solve in this issue.The default message shows briefly everytime we type in the ws invite message page. What is the root cause of that problem?We have this effect which updates the welcome note to the default every time the welcome note is updated. App/src/pages/workspace/WorkspaceInviteMessagePage.tsx Lines 89 to 94 in 7f0bdf0
This is added in #48660 because we migrated to useOnyx so when the component is mounted for the first time, App/src/pages/workspace/WorkspaceInviteMessagePage.tsx Lines 63 to 74 in 7f0bdf0
and is set as the input default value.
What changes do you think we should make in order to solve the problem?Remove this useEffect App/src/pages/workspace/WorkspaceInviteMessagePage.tsx Lines 89 to 94 in 7f0bdf0
and instead, to overcome the issue after migrating to useOnyx, we can re-trigger this effect when the App/src/pages/workspace/WorkspaceInviteMessagePage.tsx Lines 77 to 87 in 7f0bdf0
We can consider removing the defaultValue too
we can optionally returning null when isWorkspaceInviteMessageDraftLoading is true |
@adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
I can replicate this but it's so briefI think it's really a polish - updating price |
Job added to Upwork: https://www.upwork.com/jobs/~021842313701781491251 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 ( |
@abdulrahuman5196, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@abdulrahuman5196 a few proposals to review above! |
@bernhardoj 's proposal here #49996 (comment) looks good and works well. It addresses the root cause and solution where we don't need to update the defaultWelcomeNote whenever there is a welcomeNote change compared to other proposals. 🎀 👀 🎀 |
Triggered auto assignment to @dangrous, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@adelekennedy IMO, this could be lesser bug compared to other normal bugs, but reduced price of 4 times than normal bug seems little undervalued comparing to any normal bug. |
@abdulrahuman5196 just a reminder to check this follow-up PR |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.52-5 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-10-30. 🎊 For reference, here are some details about the assignees on this issue:
|
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:
|
Payment Summary
BugZero Checklist (@adelekennedy)
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.55-10 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-11-07. 🎊 For reference, here are some details about the assignees on this issue:
|
@abdulrahuman5196 @adelekennedy The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test Proposal Template
Regression Test ProposalPrecondition:Test:Do we agree 👍 or 👎 |
BugZero Checklist:
Source of bug:
Where bug was reported:
Who reported the bug:
Regression Test Proposal Template
Regression Test ProposalPrecondition:Test:
Do we agree 👍 or 👎 |
reviewing the issue I'm updating the price to $125 Payouts due:
|
Thanks! Requested in ND. |
$125 approved for @bernhardoj |
$125 approved for @abdulrahuman5196 |
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.42-0
Reproducible in staging?: Y
Reproducible in production?: Y
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:
Expected Result:
The previous invitation message will not appear after clearing the field and entering a character
Actual Result:
The previous invitation message reappears after clearing the field and entering a character
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6620980_1727778413019.ScreenRecording_10-01-2024_18-21-38_1.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @adelekennedyThe text was updated successfully, but these errors were encountered: