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

fixes the "Room chat LHN preview displays "removed 0 user" after editing room description " #44300

Merged
merged 3 commits into from
Jun 25, 2024
Merged

fixes the "Room chat LHN preview displays "removed 0 user" after editing room description " #44300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4552845
fixes the issue
ishpaul777 Jun 24, 2024
28492c4
fixes empty string
ishpaul777 Jun 24, 2024
a018b79
fixes translation for room description
ishpaul777 Jun 24, 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
3 changes: 3 additions & 0 deletions src/languages/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3363,4 +3363,7 @@ export default {
additionalInfoTitle: 'What software are you moving to and why?',
additionalInfoInputLabel: 'Your response',
},
roomChangeLog: {
updateRoomDescription: 'set the room description to:',
},
} satisfies TranslationBase;
3 changes: 3 additions & 0 deletions src/languages/es.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3867,4 +3867,7 @@ export default {
additionalInfoTitle: '¿A qué software está migrando y por qué?',
additionalInfoInputLabel: 'Tu respuesta',
},
roomChangeLog: {
updateRoomDescription: 'establece la descripción de la sala a:',
},
} satisfies EnglishTranslation;
5 changes: 5 additions & 0 deletions src/libs/SidebarUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,11 @@ function getOptionData({
: ` ${Localize.translate(preferredLocale, 'workspace.invite.from')}`;
result.alternateText += `${preposition} ${roomName}`;
}
if (lastActionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.UPDATE_ROOM_DESCRIPTION) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We missed the case if the room description is markdown/html/emojis, this overlook caused #44448.

Never thought that i would have to comment on the PR that i myself reviewed 😆 . I agree that this should had been noticed during testing but it was missed, i will be more careful moving forward

result.alternateText = `${lastActorDisplayName} ${Localize.translate(preferredLocale, 'roomChangeLog.updateRoomDescription')} ${
lastActionOriginalMessage?.description
}`.trim();
}
Comment on lines +356 to +359
Copy link
Contributor

Choose a reason for hiding this comment

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

We always display set the room description to: here and it caused this bug .
More detail in this comment: #45522 (comment)

Copy link
Contributor

@allgandalf allgandalf Aug 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Opps, I was perhaps not in my best mind while reviewing this PR 😓 , thanks for pointing that out @hungvu193 🙌

Edit:

ahh now that i look at the original issue, i recall that this was the last deploy blocker and needed to be reviewed quickly hence the hurry to deploy caused the quality of the PR to come done, honest mistake 😅

} else if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.LEAVE_POLICY) {
result.alternateText = Localize.translateLocal('workspace.invite.leftWorkspace');
} else if (lastAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW && lastActorDisplayName && lastMessageTextFromReport) {
Expand Down
5 changes: 4 additions & 1 deletion src/pages/home/report/ReportActionItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type * as OnyxTypes from '@src/types/onyx';
import type {Errors} from '@src/types/onyx/OnyxCommon';
import type {JoinWorkspaceResolution} from '@src/types/onyx/OriginalMessage';
import type {JoinWorkspaceResolution, OriginalMessageChangeLog} from '@src/types/onyx/OriginalMessage';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import {RestrictedReadOnlyContextMenuActions} from './ContextMenu/ContextMenuActions';
import MiniReportActionContextMenu from './ContextMenu/MiniReportActionContextMenu';
Expand Down Expand Up @@ -623,6 +623,9 @@ function ReportActionItem({
children = <ReportActionItemBasicMessage message={translate('iou.unheldExpense')} />;
} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.MERGED_WITH_CASH_TRANSACTION) {
children = <ReportActionItemBasicMessage message={translate('systemMessage.mergedWithCashTransaction')} />;
} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.UPDATE_ROOM_DESCRIPTION) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ishpaul777 check out all the is$ACTION_TYPEAction utility functions in ReportActionUtils. You can use type predicates to narrow the type of a report action, and thus avoid having to do any type casting.

const message = `${translate('roomChangeLog.updateRoomDescription')} ${(originalMessage as OriginalMessageChangeLog)?.description}`;
children = <ReportActionItemBasicMessage message={message} />;
} else if (ReportActionsUtils.isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.DISMISSED_VIOLATION)) {
children = <ReportActionItemBasicMessage message={ReportActionsUtils.getDismissedViolationMessageText(ReportActionsUtils.getOriginalMessage(action))} />;
} else {
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/OriginalMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,9 @@ type OriginalMessageChangeLog = {
/** Name of the chat room */
roomName?: string;

/** Description of the chat room */
description?: string;

/** ID of the report */
reportID?: number;
};
Expand Down
Loading