[HOLD for payment 2022-08-08] [HOLD for payment 2022-08-01] [$500] At #admins and #announce rooms, users own timezone displayed - reported by @Puneet-here

[mvtglobally]

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. Create a new workspace
2. Search announce and admins room of that workspace
3. Check the timezone above the text composer

Expected Result:

It should not show users their own timezone

Actual Result:

It's showing the user their own timezone

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.69-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @Puneet-here
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1651012796912239

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[Christinadobrzyn]

I don't see any dupe GHs about this subject and it sounds like we've confirmed it's a new bug in this convo. So sending to eng to see if this can be fixed externally!

[melvin-bot]

Triggered auto assignment to @thienlnam (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[Puneet-here]

Proposal

We have canShowReportRecipientLocalTime function which returns a bool and based on it we show the timezone.

Depending on the expected behaviour I have two proposals.

First case : If we don't wanna show the timezone for rooms we can just use !isChatRoom(report) below

App/src/libs/ReportUtils.js

Lines 314 to 317 in db27a3d

	&& !hasMultipleParticipants
	&& reportRecipient
	&& reportRecipientTimezone
	&& reportRecipientTimezone.selected;

Second case: If we wanna show timezone when another person is in the room except you.
For this case we can pass myPersonalDetails as an argument when calling canShowReportRecipientLocalTime and we can use myPersonalDetails.login to filter out our own email and we can just check if reportParticipants.length ===1 instead of !hasMultipleParticipants at line 314

App/src/libs/ReportUtils.js

Lines 308 to 314 in db27a3d

	function canShowReportRecipientLocalTime(personalDetails, report) {
	const reportParticipants = lodashGet(report, 'participants', []);
	const hasMultipleParticipants = reportParticipants.length > 1;
	const reportRecipient = personalDetails[reportParticipants[0]];
	const reportRecipientTimezone = lodashGet(reportRecipient, 'timezone', CONST.DEFAULT_TIME_ZONE);
	return !hasExpensifyEmails(reportParticipants)
	&& !hasMultipleParticipants

[melvin-bot]

Triggered auto assignment to @stephanieelliott (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[thienlnam]

Yeah, good point @Puneet-here it depends on what we expect would happen here. Since a workspace is basically a group chat, I imagine it should function in the same way in that we don't show the timezone.

However, it doesn't seem like your proposal would work. The workspace rooms are considered a default room and would return true for isChatRoom(report)

[Puneet-here]

If we use isChatRoom like below it will work -

function canShowReportRecipientLocalTime(personalDetails, report) {
    const reportParticipants = lodashGet(report, 'participants', []);
    const hasMultipleParticipants = reportParticipants.length > 1;
    const reportRecipient = personalDetails[reportParticipants[0]];
    const reportRecipientTimezone = lodashGet(reportRecipient, 'timezone', CONST.DEFAULT_TIME_ZONE);
    return !hasExpensifyEmails(reportParticipants)
         && !hasMultipleParticipants
+        && !isChatRoom(report)
         && reportRecipient
         && reportRecipientTimezone
         && reportRecipientTimezone.selected;
}

[thienlnam]

The type of room in the issue is an POLICY_ANNOUNCE room. This is considered a default room and will pass the check

App/src/libs/ReportUtils.js

Lines 116 to 128 in 3e52b9a

	/**
	* Whether the provided report is a default room
	* @param {Object} report
	* @param {String} report.chatType
	* @returns {Boolean}
	*/
	function isDefaultRoom(report) {
	return _.contains([
	CONST.REPORT.CHAT_TYPE.POLICY_ADMINS,
	CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE,
	CONST.REPORT.CHAT_TYPE.DOMAIN_ALL,
	], lodashGet(report, ['chatType'], ''));
	}

Which returns true in isChatRoom

App/src/libs/ReportUtils.js

Lines 165 to 168 in 3e52b9a

	*/
	function isChatRoom(report) {
	return isUserCreatedPolicyRoom(report) || isDefaultRoom(report);
	}

This probably happens when there is a single person in the workspace

[Puneet-here]

Yes and that's why if we use !isChatRoom(report) (notice the exclamation mark) it will return false for rooms.

[thienlnam]

We don't want to disable it for all rooms.. just in group rooms

[Puneet-here]

Oh I thought we wanna disable it for user created rooms too, if we only wanna disable it for default rooms we can use !isDefaultRoom(report) instead of !isChatRoom(report).

This problem will stay in user created chat rooms so if we wanna show the timezone when a second member is inside the room we can also use my second approach together with !isDefaultRoom(report) check

Second case: If we wanna show timezone when another person is in the room except you.
For this case we can pass myPersonalDetails as an argument when calling canShowReportRecipientLocalTime and we can use myPersonalDetails.login to filter out our own email and we can just check if reportParticipants.length ===1 instead of !hasMultipleParticipants at line 314

App/src/libs/ReportUtils.js

Lines 308 to 314 in db27a3d

	function canShowReportRecipientLocalTime(personalDetails, report) {
	const reportParticipants = lodashGet(report, 'participants', []);
	const hasMultipleParticipants = reportParticipants.length > 1;
	const reportRecipient = personalDetails[reportParticipants[0]];
	const reportRecipientTimezone = lodashGet(reportRecipient, 'timezone', CONST.DEFAULT_TIME_ZONE);
	return !hasExpensifyEmails(reportParticipants)
	&& !hasMultipleParticipants

[Puneet-here]

Just for the clarification:

1. We want to disable it for announce/admins/domain rooms (which all are group rooms)
2. We wanna show timezone in user created rooms if there's an another person except you.

Question:- Isn't user created room also a group room because all the workspace members will be in user created rooms?

[stephanieelliott]

Posted this to Upwork: https://www.upwork.com/jobs/~01e673ea3f72bd1dd4

[Puneet-here]

@stephanieelliott, can you add the exported label.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (Exported)

[melvin-bot]

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

[stephanieelliott]

Reassigning this one as I am going OOO, thanks @trjExpensify!

[trjExpensify]

Sure thing! Putting the reviewing label on.

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.85-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: timezone shown for our own email #9642
• fix: typo in personaldetails prop #9966

If no regressions arise, payment will be issued on 2022-08-01. 🎊

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.86-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Use currentUserPersonalDetails key #9987

If no regressions arise, payment will be issued on 2022-08-08. 🎊

[trjExpensify]

👋 @Puneet-here you still need to apply to the job here before I can settle up on this one. @mananjadhav with the two deploys above, can you confirm there was a regression here for C+ payment due?

[Puneet-here]

Applied.
Thank you.

[trjExpensify]

Cool, offer sent to you @Puneet-here. @mananjadhav still waiting on confirmation of this:

@mananjadhav with the two deploys above, can you confirm there was a regression here for C+ payment due?

[trjExpensify]

Cool, settled up with @Puneet-here.

[Puneet-here]

Eep sorry @trjExpensify, I didn't notice it first but the total compensation was $750. $500 for the fix and $250 for the reporting.

[Puneet-here]

@mananjadhav with the two deploys above, can you confirm there was a regression here for C+ payment due?

Just sharing a note. I think we can't call it a regression, the PR was working and was approved but #9560 got merged just before the PR for this issue and #9560 changed an onyx key which we were using at our PR, that's why we were getting an error.

[trjExpensify]

I didn't notice it first but the total compensation was $750. $500 for the fix and $250 for the reporting.

Ahh, got it. Sent an offer for an additional $250.

I think we can't call it a regression, the PR was working and was approved but #9560 got merged just before the PR for this issue and #9560 changed an onyx key which we were using at our PR, that's why we were getting an error.

Okay, helpful context. It was a timing issue. That makes sense. Thanks! @mananjadhav, I've sent you an offer now as well.

[trjExpensify]

Alright, both settled up. Closing.