Reseting the date of incorporation crashes the app #14417

kavimuru · 2023-01-19T20:44:22Z

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

Action Performed:

1.Connect a bank account
2. Under company information, select incorporation date
3. Click on Reset option

Expected Result:

The date should be reset to today

Actual Result:

App crashes

Workaround:

unknown

Platforms:

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

iOS / native

Version Number: 1.2.56-0
Reproducible in staging?: Y
Reproducible in production?: Could not check
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:

Screen.Recording.2023-01-19.at.9.28.14.PM.mov

MRWN4995.1.MP4

Expensify/Expensify Issue URL:
Issue reported by: @esh-g
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1674144058692939

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~015f5204e26a9f2c93
Upwork Job ID: 1616224689148133376
Last Price Increase: 2023-01-20

marcochavezf · 2023-01-19T22:53:49Z

I reproduced the bug locally, seems we didn't implement an initial value for the DatePicker on iOS. Putting a PR shortly.

melvin-bot · 2023-01-20T00:02:50Z

Job added to Upwork: https://www.upwork.com/jobs/~015f5204e26a9f2c93

melvin-bot · 2023-01-20T00:02:56Z

Triggered auto assignment to Contributor Plus for review of internal employee PR - @aimane-chnaif (Internal)

marcochavezf · 2023-01-20T00:03:05Z

PR up.

marcochavezf · 2023-01-20T00:05:50Z

Oh noticed after creating the PR that @esh-g proposed a fix here. Not sure if we should compensate since the solution is straightforward.

mallenexpensify · 2023-01-20T00:55:11Z

Oh noticed after creating the PR that @esh-g proposed a fix #14270 (comment). Not sure if we should compensate since the solution is straightforward.

@marcochavezf , can you elaborate on this please? My understand is that there was a regression and @esh-g caught it, then we fixed it. If so, they're, at least, due compensation for finding the regression and reporting it. If they also provided the solution that we used, and you're wondering if compensation is due for that, we'd need to discuss more (cuz I'm unsure)

aimane-chnaif · 2023-01-20T01:13:36Z

I am assigned here but not in PR. It seems automation doesn't work correctly. Shall I review PR?

esh-g · 2023-01-20T03:02:04Z

Regression !!!!

I caught a regression from this PR: https://expensify.slack.com/archives/C049HHMV9SM/p1674144058692939
APP CRASHES on clicking RESET in date picker.

Solution

diff --git a/src/components/DatePicker/index.ios.js b/src/components/DatePicker/index.ios.js
index 6f37202cf..960fa798c 100644
--- a/src/components/DatePicker/index.ios.js
+++ b/src/components/DatePicker/index.ios.js
@@ -41,6 +41,7 @@ class DatePicker extends React.Component {
     showPicker(event) {
         // Opens the popover only after the keyboard is hidden to avoid a "blinking" effect where the keyboard was on iOS
         // See https://github.com/Expensify/App/issues/14084 for more context
+        this.initialValue = this.state.selectedDate;
         if (!this.props.isKeyboardShown) {
             this.setState({isPickerVisible: true});
             return;
@@ -50,7 +51,6 @@ class DatePicker extends React.Component {
             listener.remove();
         });
         Keyboard.dismiss();
-        this.initialValue = this.state.selectedDate;
         event.preventDefault();
     }

Reposting the comment from the PR to this issue

esh-g · 2023-01-20T03:05:54Z

Oh noticed after creating the PR that @esh-g proposed a fix #14270 (comment). Not sure if we should compensate since the solution is straightforward.

@marcochavezf , can you elaborate on this please? My understand is that there was a regression and @esh-g caught it, then we fixed it. If so, they're, at least, due compensation for finding the regression and reporting it. If they also provided the solution that we used, and you're wondering if compensation is due for that, we'd need to discuss more (cuz I'm unsure)

Yes, I thought this would work like reporting a regular bug as this PR was merged into staging....
And it took some work finding the PR it was linked to, at first I incorrectly commented on two PRs before finding the right one 😅
And I guess, my solution could be the proposal for this bug

aimane-chnaif · 2023-01-20T03:52:13Z

I am assigned here but not in PR. It seems automation doesn't work correctly. Shall I review PR?

Oh I noticed @thesahindia already started reviewing PR

marcochavezf · 2023-01-20T15:50:22Z

@marcochavezf , can you elaborate on this please? My understand is that there was a regression and @esh-g caught it, then we fixed it. If so, they're, at least, due compensation for finding the regression and reporting it.

I agree we should compensate for finding and reporting it.

If they also provided the solution that we used, and you're wondering if compensation is due for that, we'd need to discuss more (cuz I'm unsure)

Yeah, I saw the solution after I created the PR since it's a one-line change, so technically I didn't use the solution provided but it's the same solution 😄

mallenexpensify · 2023-01-20T19:10:44Z

Thanks @marcochavezf , i'll plan to compensate @esh-g $250 for reporting the regression when I pay out this issue.

@aimane-chnaif I think the issue for assignment got borked cuz the PR was created at the same time as you were assigned here. Let's consider it an edge case for now unless anyone disagrees

esh-g · 2023-01-21T16:09:54Z

The PR #14427 is merged, so is this issue now resolved?

mallenexpensify · 2023-01-23T16:46:17Z

@esh-g , per CONTRIBUTING.md,

Payment for your contributions will be made no less than 7 days after the pull request is deployed to production to allow for regression testing.

That's also when we consider it resolved.

esh-g · 2023-01-23T20:24:26Z

Yes, just wanted to confirm that it was on the 7-day period 👍

esh-g · 2023-02-01T04:02:59Z

Were there any regressions found in this time?

thesahindia · 2023-02-06T19:36:48Z

@mallenexpensify, this is ready for the payment.

mallenexpensify · 2023-02-07T18:35:19Z

Thanks @thesahindia , sorry for the delay @esh-g , the issue wasn't having the Overdue label added cuz of the Reviewing label. Can you please apply and comment here once you have?
https://www.upwork.com/jobs/~015f5204e26a9f2c93

It might say $1000 for the job, payment will be $250 for reporting.

thesahindia · 2023-02-07T18:42:06Z

Applied, thanks!

esh-g · 2023-02-07T19:13:59Z

@mallenexpensify I have applied under the name Tanya G.

mallenexpensify · 2023-02-08T01:44:27Z

@esh-g and @thesahindia both hired, can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~015f5204e26a9f2c93

esh-g · 2023-02-08T03:25:08Z

@mallenexpensify Accepted, successfully!

thesahindia · 2023-02-08T05:18:09Z

Accepted!

mallenexpensify · 2023-02-09T01:39:17Z

@esh-g paid $250 for reporting
@thesahindia paid $1000 for C+ PR review.

@thesahindia can you complete steps 1-3 below?

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:

The PR that introduced the bug has been identified. Link to the PR:
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:
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:
A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

Checking on this the last step here https://expensify.slack.com/archives/C01SKUP7QR0/p1675906686840469

Regression Test Buddy Check!
Context: Reseting the date of incorporation crashes the app #14417
Recommendation: Add step 5 and 6 under "Online behavior (Continue with bank account flow)" to https://expensify.testrail.io/index.php?/cases/view/1971143
5. Click reset
6. Verify The date is reset to today and that the app doesn't crash.
Please :yes: or :no: if you think this is the right path forward! Thank you!

thesahindia · 2023-02-09T16:47:06Z

The PR that introduced the bug has been identified. Link to the PR:

#14084

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:

#14084 (comment)

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:

https://expensify.slack.com/archives/C01GTK53T8Q/p1675961152450879

thesahindia · 2023-02-09T16:51:21Z

I am not sure what the first 4 steps are but if they look something like below then we are good

Go to company page
Tap on incorporation date
Set the date to today
Tap on incorporation date again
Click reset
Verify The date is reset to today and that the app doesn't crash.

MelvinBot · 2023-02-09T20:01:08Z

@marcochavezf @mallenexpensify this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

mallenexpensify · 2023-02-10T00:25:02Z

Thanks @thesahindia , we should be all set! Update TestRail GH is

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

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 19, 2023

melvin-bot bot locked and limited conversation to collaborators Jan 19, 2023

melvin-bot bot assigned mallenexpensify Jan 19, 2023

marcochavezf self-assigned this Jan 19, 2023

marcochavezf mentioned this issue Jan 20, 2023

Set initialValue right after date picker is shown on iOS #14427

Merged

54 tasks

melvin-bot bot added the Reviewing Has a PR in review label Jan 20, 2023

marcochavezf added the Internal Requires API changes or must be handled by Expensify staff label Jan 20, 2023

melvin-bot bot unlocked this conversation Jan 20, 2023

melvin-bot bot assigned aimane-chnaif Jan 20, 2023

mollfpr mentioned this issue Jan 20, 2023

fix/14084 Bank account : The bottom section of Personal Information blinks when opening and closing DatePicker #14270

Merged

53 tasks

aimane-chnaif removed their assignment Jan 20, 2023

mollfpr mentioned this issue Jan 25, 2023

[HOLD for payment 2023-01-25] [$1000] Bank account : The bottom section of Personal Information blinks when opening and closing DatePicker #14084

Closed

1 task

mallenexpensify removed the Reviewing Has a PR in review label Feb 7, 2023

melvin-bot bot added the Overdue label Feb 7, 2023

melvin-bot bot removed the Overdue label Feb 7, 2023

mallenexpensify closed this as completed Feb 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reseting the date of incorporation crashes the app #14417

Reseting the date of incorporation crashes the app #14417

kavimuru commented Jan 19, 2023 •

edited by melvin-bot bot

Loading

marcochavezf commented Jan 19, 2023

melvin-bot bot commented Jan 20, 2023

melvin-bot bot commented Jan 20, 2023

marcochavezf commented Jan 20, 2023

marcochavezf commented Jan 20, 2023

mallenexpensify commented Jan 20, 2023

aimane-chnaif commented Jan 20, 2023

esh-g commented Jan 20, 2023

esh-g commented Jan 20, 2023 •

edited

Loading

aimane-chnaif commented Jan 20, 2023

marcochavezf commented Jan 20, 2023

mallenexpensify commented Jan 20, 2023

esh-g commented Jan 21, 2023

mallenexpensify commented Jan 23, 2023

esh-g commented Jan 23, 2023

esh-g commented Feb 1, 2023

thesahindia commented Feb 6, 2023

mallenexpensify commented Feb 7, 2023

thesahindia commented Feb 7, 2023

esh-g commented Feb 7, 2023

mallenexpensify commented Feb 8, 2023

esh-g commented Feb 8, 2023

thesahindia commented Feb 8, 2023

mallenexpensify commented Feb 9, 2023

thesahindia commented Feb 9, 2023

thesahindia commented Feb 9, 2023

MelvinBot commented Feb 9, 2023

mallenexpensify commented Feb 10, 2023

Reseting the date of incorporation crashes the app #14417

Reseting the date of incorporation crashes the app #14417

Comments

kavimuru commented Jan 19, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

marcochavezf commented Jan 19, 2023

melvin-bot bot commented Jan 20, 2023

melvin-bot bot commented Jan 20, 2023

marcochavezf commented Jan 20, 2023

marcochavezf commented Jan 20, 2023

mallenexpensify commented Jan 20, 2023

aimane-chnaif commented Jan 20, 2023

esh-g commented Jan 20, 2023

Regression !!!!

esh-g commented Jan 20, 2023 • edited Loading

aimane-chnaif commented Jan 20, 2023

marcochavezf commented Jan 20, 2023

mallenexpensify commented Jan 20, 2023

esh-g commented Jan 21, 2023

mallenexpensify commented Jan 23, 2023

esh-g commented Jan 23, 2023

esh-g commented Feb 1, 2023

thesahindia commented Feb 6, 2023

mallenexpensify commented Feb 7, 2023

thesahindia commented Feb 7, 2023

esh-g commented Feb 7, 2023

mallenexpensify commented Feb 8, 2023

esh-g commented Feb 8, 2023

thesahindia commented Feb 8, 2023

mallenexpensify commented Feb 9, 2023

thesahindia commented Feb 9, 2023

thesahindia commented Feb 9, 2023

MelvinBot commented Feb 9, 2023

mallenexpensify commented Feb 10, 2023

kavimuru commented Jan 19, 2023 •

edited by melvin-bot bot

Loading

esh-g commented Jan 20, 2023 •

edited

Loading