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.

Sign up for GitHub

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

[Fix]: New chat shows time zone for Unvalidated Accounts #39384

Merged
merged 5 commits into from
Apr 4, 2024
Merged

[Fix]: New chat shows time zone for Unvalidated Accounts #39384

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc4352a
Add unvalidated user check for timezone
allgandalf Apr 1, 2024
d49329f
remove null check as value can be undefined
allgandalf Apr 1, 2024
52ec52e
Merge branch 'Expensify:main' into issue38581
allgandalf Apr 4, 2024
c0a929e
Merge branch 'Expensify:main' into issue38581
allgandalf Apr 4, 2024
6b7e935
Update src/pages/ProfilePage.tsx
allgandalf Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/pages/ProfilePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,13 @@ function ProfilePage({personalDetails, personalDetailsMetadata, route, session,
const fallbackIcon = details?.fallbackIcon ?? '';
const login = details?.login ?? '';
const timezone = details?.timezone;
const reportRecipient = personalDetails?.[accountID];
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const isParticipantValidated = reportRecipient?.validated || false;
Copy link
Contributor

@aldo-expensify aldo-expensify Apr 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const isParticipantValidated = reportRecipient?.validated || false;
const isParticipantValidated = reportRecipient?.validated ?? false;

Why did we add // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing instead of using ?? as the lint error suggested to? the lint checks were added for a reason and are not meant to be disabled without a good reason. Is there a good reason in this case to disable the check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh no, you're right it is a boolean value, so if validated is undefined/null it will still become false, i assumed it to be a string for a second, commiting the suggestions right away

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing! @mollfpr @GandalfGwaihir always avoid disabling rules unless there is a good reason for it :)


// If we have a reportID param this means that we
// arrived here via the ParticipantsPage and should be allowed to navigate back to it
const shouldShowLocalTime = !ReportUtils.hasAutomatedExpensifyAccountIDs([accountID]) && !isEmptyObject(timezone);
const shouldShowLocalTime = !ReportUtils.hasAutomatedExpensifyAccountIDs([accountID]) && !isEmptyObject(timezone) && isParticipantValidated;
let pronouns = details?.pronouns ?? '';
if (pronouns?.startsWith(CONST.PRONOUNS.PREFIX)) {
const localeKey = pronouns.replace(CONST.PRONOUNS.PREFIX, '');
Expand Down
Loading