[Hold Date picker refactor PR#:21792] Selecting a valid date updates only the date and not the month

[kavimuru]

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:

1. Go to Settings > Profile > Personal Details
2. Go to Date of birth
3. Click on the year picker
4. Select any year before 2018
5. Select any future month e.g. July if today's date is in June
6. Select any date e.g July 4th
7. Click on the year picker again
8. Select 2018
9. Observe the error message appear as the DOB is less than 5 years old
10. Click on any valid date e.g. 25th June 2018 if today's date is 26th June 2023
11. Observe the day changes in the date field but not the month (thus the error doesn't clear).

Expected Result:

The selected month is updated and the error message is cleared upon entering a valid date.

Actual Result:

The selected month is not updated and the error message persists

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.29-6
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

Screen.Recording.2023-06-13.at.12.38.31.AM.mov

dob.mp4

Expensify/Expensify Issue URL:
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686600703269489

View all open jobs on GitHub

[melvin-bot]

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

[melvin-bot]

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

[esh-g]

Proposal

Please re-state the problem

The error message doesn't go away even after selecting a valid date in the date picker.

What is the root cause of that problem?

The root cause is that when we update the year, the following state gets updated:

App/src/components/CalendarPicker/index.js

Lines 70 to 78 in ffd9672

	this.setState(
	(prev) => {
	const newMomentDate = moment(prev.currentDateView).set('year', newSelectedYear);
	return {
	selectedYear: newSelectedYear,
	currentDateView: this.clampDate(newMomentDate.toDate()),
	};
	},

When the year changes, the selected date and month remain the same and only the year changes because the changes are not clamped. Whereas, on updating the calenderDateView (which is what selects the displayed month), we are clamping that so it doesn't get set to an invalid month while the actual date selection does get set to the invalid one.

What changes do you think we should make to solve this problem?

Here is a possible list of alternatives:

Option 1:

Since the actual date selection is not being clamped, we can also forgo clamping the currentDateView so the selected month only gets displayed, which will respect the user's choice as well.

return {
    selectedYear: newSelectedYear,
    currentDateView: newMomentDate.toDate() // remove this.clampDate()
};

Result

Screen.Recording.2023-06-19.at.1.25.56.PM.mov

Option 2:

We can update the currentDateView to show the month to which the date is being clamped to with the following code:

return {
    selectedYear: newSelectedYear,
    currentDateView: this.clampDate(newMomentDate.toDate()),
    selectedMonth: this.getNumberStringWithLeadingZero(newMomentDate.get('month') + 1), // Add this
};

Just like we do here:

App/src/components/CalendarPicker/index.js

Line 44 in ffd9672

selectedMonth: this.getNumberStringWithLeadingZero(currentSelection.get('month') + 1),

Result

Screen.Recording.2023-06-19.at.1.27.42.PM.mov

Option 3:

We can update the selected date as well an month to be clamped so if the date exceeds the allowable, it will automatically be set to the maximum date allowed.

return {
    selectedYear: newSelectedYear,
    currentDateView: this.clampDate(newMomentDate.toDate()),
    selectedMonth: this.getNumberStringWithLeadingZero(newMomentDate.get('month') + 1),
    selectedDay: this.getNumberStringWithLeadingZero(newMomentDate.get('date')),
};

Just like we do here:

App/src/components/CalendarPicker/index.js

Lines 44 to 45 in ffd9672

	selectedMonth: this.getNumberStringWithLeadingZero(currentSelection.get('month') + 1),
	selectedDay: this.getNumberStringWithLeadingZero(currentSelection.get('date')),

Result

Screen.Recording.2023-06-19.at.1.43.04.PM.mov

PS I would recommend going with 1, but all options are equally viable.

What other alternatives did you explore?

Already broken down the solution into possible options.

[trjExpensify]

I can repro this, but before moving it on, do I recall you mentioning a refactor on this component @JmillsExpensify? Is this something to do with it?

[trjExpensify]

Ah okay, found it: #20630

Hey @ArekChr! I see you're working on a refactor to the date picker component. I don't suppose it solves this bug does it?

CC: @mountiny

[JmillsExpensify]

Hmm yeah oddly enough we are refactoring the year implementation in particular, not sure if this is related or not though.

[melvin-bot]

@trjExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[trjExpensify]

Reached out to @ArekChr directly. If you want steps to test against your PR's branch, here they are:

1. Go to Settings > Profile > Personal Details
2. Go to Date of birth
3. Click on the year picker
4. Select any year before 2018
5. Select any future month e.g. July if today's date is in June
6. Select any date e.g July 4th
7. Click on the year picker again
8. Select 2018
9. Observe the error message appear as the DOB is less than 5 years old
10. Click on any valid date e.g. 25th June 2018 if today's date is 26th June 2023
11. Observe the day changes in the date field but not the month (thus the error doesn't clear).

Either way, we're actively refactoring this code, so this should wait until that PR has merged to avoid conflicts. I've popped it on hold for that PR.

[ArekChr]

Hey, @trjExpensify, this test scenario works in my PR. I can add it to reproduce steps too. But I think it is not needed as now the year picked doesn't change the value from the input.

[trjExpensify]

Excellent! Okay, so I'll retest after that PR has deployed and then we'll be able to close this one out (and pay the bug report).

[trjExpensify]

Awaiting the linked PR before I retest this one. 👍

[trjExpensify]

PR we're held on is in review. Dropping to weekly though for the hold.

[trjExpensify]

PR hit prod at the end of last week. Re-tested and this is fixed. @adeel0202 I've sent you an offer for the bug report, after which we can close this out. 👍

[adeel0202]

Accepted the offer, thanks.

[trjExpensify]

Paid!