Skip to content
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

Personal details - Error message displayed when user try to save Date of birth. #15107

Closed
2 of 6 tasks
kbecciv opened this issue Feb 13, 2023 · 8 comments
Closed
2 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff

Comments

@kbecciv
Copy link

kbecciv commented Feb 13, 2023

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. Launch App
  2. Navigate to Profile > Personal details
  3. Tap "date of birth" / add date
  4. Tap on Save

Expected Result:

Error message is not displayed when user try to save Date of birth.

Actual Result:

Error message displayed when user try to save Date of birth.

Workaround:

Unknown

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

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug5936394_RPReplay_Final1676321123.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b6b30a13e8f991fc
  • Upwork Job ID: 1625267351186849792
  • Last Price Increase: 2023-02-13
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 13, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Feb 13, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@marcaaron marcaaron self-assigned this Feb 13, 2023
@marcaaron marcaaron added the Internal Requires API changes or must be handled by Expensify staff label Feb 13, 2023
@melvin-bot melvin-bot bot unlocked this conversation Feb 13, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~01b6b30a13e8f991fc

@MelvinBot
Copy link

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

@marcaaron
Copy link
Contributor

Proposal

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

Our app must not produce unexpected errors for users or have platform inconsistencies like things working on native but not web.

What is the root cause of that problem?

We are checking to see if values.dob is empty using _.isEmpty() here:

if (_.isEmpty(values.dob)) {
errors.dob = this.props.translate('common.error.fieldRequired');
}

The dob value is a Date object and underscore will treat that as empty. This only seems to happen on native which suggests an issue with underscore.

2023-02-13_13-08-41

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

  • Stop using underscore for this check a simple check of !values.dob should be enough.
  • Make sure that we are actually passing a String around instead of the Date object

What alternative solutions did you explore? (Optional)

Fix it in underscore - I don't think it's worth fixing at that level. We don't have to use this method to check for the existence of a date.

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@mollfpr
Copy link
Contributor

mollfpr commented Feb 14, 2023

@marcaaron Proposal looks good to me.

Although we have the function to check if the date is valid and the required date field.

function isValidDate(date) {
if (!date) {
return false;
}
const pastDate = moment().subtract(1000, 'years');
const futureDate = moment().add(1000, 'years');
const testDate = moment(date);
return testDate.isValid() && testDate.isBetween(pastDate, futureDate);
}

/**
* Used to validate a value that is "required".
*
* @param {*} value
* @returns {Boolean}
*/
function isRequiredFulfilled(value) {
if (_.isString(value)) {
return !_.isEmpty(value.trim());
}
if (_.isDate(value)) {
return isValidDate(value);
}
if (_.isArray(value) || _.isObject(value)) {
return !_.isEmpty(value);
}
return Boolean(value);
}

@Prince-Mendiratta
Copy link
Contributor

Prince-Mendiratta commented Feb 14, 2023

Dupe #14936, this will be fixed by this PR - #15033, which is already in review. Here's the proposal in case anyone is interested in an explanation on the fix opted for.

cc @mollfpr @marcaaron

@mollfpr
Copy link
Contributor

mollfpr commented Feb 14, 2023

Thanks @Prince-Mendiratta! This definitely will be fixed with your PR.

@marcaaron Should we close this or wait until the PR is merged and try to reproduce it again?

@marcaaron marcaaron closed this as not planned Won't fix, can't repro, duplicate, stale Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

6 participants