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

Jump to bottom

Show Room name in Desktop notification title if message is come from a chat room #19053

Merged
merged 4 commits into from
May 18, 2023
Merged

Show Room name in Desktop notification title if message is come from a chat room #19053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e4762e
Show roomName in notif title if message come from a chat room
hoangzinh May 16, 2023
709b63d
Build web notification title & body if message is come from chatroom
hoangzinh May 17, 2023
dc63f9b
Refactor code
hoangzinh May 17, 2023
33e560c
Add empty lines to make code look better
hoangzinh May 17, 2023
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
22 changes: 19 additions & 3 deletions src/libs/Notification/LocalNotification/BrowserNotifications.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import _ from 'underscore';
import focusApp from './focusApp';
import * as AppUpdate from '../../actions/AppUpdate';
import EXPENSIFY_ICON_URL from '../../../../assets/images/expensify-logo-round-clearspace.png';
import * as ReportUtils from '../../ReportUtils';

const DEFAULT_DELAY = 4000;

Expand Down Expand Up @@ -92,20 +93,35 @@ export default {
* Create a report comment notification
*
* @param {Object} params
* @param {Object} params.report
* @param {Object} params.reportAction
* @param {Function} params.onClick
* @param {Boolean} usesIcon true if notification uses right circular icon
*/
pushReportCommentNotification({reportAction, onClick}, usesIcon = false) {
pushReportCommentNotification({report, reportAction, onClick}, usesIcon = false) {
let title;
let body;

const isChatRoom = ReportUtils.isChatRoom(report);

const {person, message} = reportAction;
const plainTextPerson = _.map(person, (f) => f.text).join();

// Specifically target the comment part of the message
const plainTextMessage = (_.find(message, (f) => f.type === 'COMMENT') || {}).text;

if (isChatRoom) {
const roomName = _.get(report, 'displayName', '');
Copy link
Contributor

@techievivek techievivek Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Does this need to be reportName? Because I can't see the displayName property. I was working on this PR #28200 (comment) and realized that the notifications for chat rooms might be broken for web and desktop?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a regression from #21406 #21406 (comment). This should be changed to:

const roomName = ReportUtils.getReportName(report);

But if reportName prop already exists we can use it directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, great, thanks for the context. I will update it to use getReportName.

title = roomName;
body = `${plainTextPerson}: ${plainTextMessage}`;
} else {
title = plainTextPerson;
body = plainTextMessage;
}

push({
title: plainTextPerson,
body: plainTextMessage,
title,
body,
delay: 0,
onClick,
icon: usesIcon ? EXPENSIFY_ICON_URL : '',
Expand Down
4 changes: 2 additions & 2 deletions src/libs/Notification/LocalNotification/index.desktop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import BrowserNotifications from './BrowserNotifications';

function showCommentNotification({reportAction, onClick}) {
BrowserNotifications.pushReportCommentNotification({reportAction, onClick});
function showCommentNotification({report, reportAction, onClick}) {
BrowserNotifications.pushReportCommentNotification({report, reportAction, onClick});
}

function showUpdateAvailableNotification() {
Expand Down
4 changes: 2 additions & 2 deletions src/libs/Notification/LocalNotification/index.website.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import BrowserNotifications from './BrowserNotifications';

function showCommentNotification({reportAction, onClick}) {
BrowserNotifications.pushReportCommentNotification({reportAction, onClick}, true);
function showCommentNotification({report, reportAction, onClick}) {
BrowserNotifications.pushReportCommentNotification({report, reportAction, onClick}, true);
}

function showUpdateAvailableNotification() {
Expand Down
1 change: 1 addition & 0 deletions src/libs/actions/Report.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,7 @@ function showReportActionNotification(reportID, action) {

Log.info('[LocalNotification] Creating notification');
LocalNotification.showCommentNotification({
report: allReports[reportID],
reportAction: action,
onClick: () => {
// Navigate to this report onClick
Expand Down