-
Notifications
You must be signed in to change notification settings - Fork 3k
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-07-14] Web- A Green 'Yes, Continues' button is shown when closing an account. #22257
Comments
Triggered auto assignment to @dylanexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
This was missed accidentally during the refactoring of the concerned component. Refactor PR was on hold for a long time and this change was done during that period. It got overlooked during the merge conflict resolution. I have created a PR to solve it. |
@dylanexpensify you either assign an engineer here or wait for the PR to be merged. No C+ will be required here because I was C+ on original PR. This was the original PR #19328 which caused this issue. To speed up the things, I am creating the PR being C+ on it. |
Ah nice, thanks for the context @parasharrajat! How long do you think it will take for the PR to be merged? This will help inform if I assign an engineer here! |
As soon as an engineer is assigned to merge it. PR is #22251 |
ProposalPlease re-state the problem that we are trying to solve in this issue.Green "Yes,Continue" button instead of red button when closing an account What is the root cause of that problem?App/src/pages/settings/Security/CloseAccountPage.js Lines 111 to 120 in 4f9f428
There is no danger props here. This is the root cause What changes do you think we should make in order to solve the problem?Add This works as expected. What alternative solutions did you explore? (Optional) |
this is intentional! |
Is the green button intentional? Did you confirm this @dylanexpensify? |
It seems the discussion happened in #20130 and landed on red color for consistency. |
not sure this should have been closed, I merged the PR. sorry for missing this while doing the refactor. |
Oh wow, sorry, yes agree - this shouldn't have been - my fault! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.37-7 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-07-14. 🎊 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.
As a reminder, here are the bonuses/penalties that should be applied for any External 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:
|
Reviewing today! |
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:
|
@parasharrajat any payments needed here? |
@dylanexpensify There will be reporting bonus payment. I am not sure about the payment for work done by me. If you think a basic payout($250) should be done for efforts then yes. I jumped in quickly to prevent duplicate issues from popping up. Here we fixed an issue caused during refactoring PR done by internal Engineers where I was C+. |
@daveSeife mind applying here? @parasharrajat we're discussing rewards for preventing duplicates, but currently nothing in stone! TY though! LMK if you think this should require payment though |
Hi, I understand your problem and I know how to solve this problem.
Thanks. Roman |
📣 @avalonprod! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@dylanexpensify I have applied. Thank You! |
@dylanexpensify I think Yes. Does $250 look good to you? |
Sounds good! |
@daveSeife sent offer, @parasharrajat mind applying? |
@dylanexpensify I will request this later. |
Requested payment 250. |
payment sorted, closing out |
Reviewed details for @parasharrajat. This is accurate based on the agreement from @dylanexpensify above, as the Business Reviewer. This is approved for payment in NewDot. |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Expected Result:
A Red 'Yes, Continues' button is shown
Actual Result:
A Green 'Yes, Continues' button is shown
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.35-5
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation
Test45_Closinggreenbutton-1.mp4
Expensify/Expensify Issue URL:
Issue reported by: @daveSeife
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688188374632229
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: