-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Don't use report for isArchived check #52805
base: main
Are you sure you want to change the base?
Conversation
Have to fix the jest unit tests, I think we can do this by calling Onyx.set with the |
Update: The main problem here is the Jest unit tests. For example, Sidebar test is current failing. But running
does change the text in the sidebar to include Screen.Recording.2024-12-27.at.1.08.46.AM.mov |
Hi @allgandalf, could you please take a look at this comment when you have a chance. I've been working on this PR on the side but looking to prioritize some other backend tasks so wanted to get some frontend expertise :) This PR should set the foundation for using the reportNameValuePairs collection in the frontend. More context about that is in the linked issue Screen.Recording.2024-12-27.at.1.56.42.AM.mov |
I will take a look and update you |
Looking into a bit more, running Also, the archived chat does not float to the bottom of the LHN even when the chat isn't pinned, so looks like there's places where we're not listening to |
I will comment on the above breifly, About the failing es lint, i think for this PR, lets ignore this workflow, our PR is already touching many parts. what do you think ? the failing es lint is because we don’t removr default assertione like policyId ?? -1 |
Yup, lets not worry about the failing lint for now, we can revisit whether or not we want to fix those once the jest tests are passing. The diff is pretty big as you said. |
Hi @allgandalf! Have you had a chance to look at this? |
Hi @allgandalf, did you get a chance to look at this today? Looking to merge in the next couple of days |
Oops, sorry this slipped my radar, I will surely prioritize this one today |
Working on updates here |
SidebarTest.ts import {screen} from '@testing-library/react-native';
import Onyx from 'react-native-onyx';
import DateUtils from '@libs/DateUtils';
import CONST from '@src/CONST';
import * as Localize from '@src/libs/Localize';
import ONYXKEYS from '@src/ONYXKEYS';
import type {ReportCollectionDataSet} from '@src/types/onyx/Report';
import type {ReportActionsCollectionDataSet} from '@src/types/onyx/ReportAction';
import type {ReportNameValuePairsCollectionDataSet} from '@src/types/onyx/ReportNameValuePairs';
import * as LHNTestUtils from '../utils/LHNTestUtils';
import * as TestHelper from '../utils/TestHelper';
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatchedUpdates';
// Be sure to include the mocked Permissions and Expensicons libraries or else the beta tests won't work
jest.mock('@src/libs/Permissions');
jest.mock('@src/hooks/useActiveWorkspaceFromNavigationState');
jest.mock('@src/hooks/useIsCurrentRouteHome');
jest.mock('@src/components/Icon/Expensicons');
const TEST_USER_ACCOUNT_ID = 1;
const TEST_USER_LOGIN = '[email protected]';
describe('Sidebar', () => {
beforeAll(() =>
Onyx.init({
keys: ONYXKEYS,
safeEvictionKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS],
}),
);
beforeEach(() => {
// Wrap Onyx each onyx action with waitForBatchedUpdates
wrapOnyxWithWaitForBatchedUpdates(Onyx);
// Initialize the network key for OfflineWithFeedback
return TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN).then(() => Onyx.merge(ONYXKEYS.NETWORK, {isOffline: false}));
});
// Clear out Onyx after each test so that each test starts with a clean slate
afterEach(() => {
Onyx.clear();
});
describe('archived chats', () => {
it('renders the archive reason as the preview message of the chat', () => {
const report = {
...LHNTestUtils.getFakeReport([1, 2], 3, true),
chatType: CONST.REPORT.CHAT_TYPE.POLICY_ROOM,
lastMessageText: 'test',
};
const action = {
...LHNTestUtils.getFakeReportAction('[email protected]', 3),
actionName: 'CLOSED',
originalMessage: {
reason: CONST.REPORT.ARCHIVE_REASON.DEFAULT,
},
};
const reportNameValuePairs = {
// eslint-disable-next-line @typescript-eslint/naming-convention
private_isArchived: DateUtils.getDBTime(),
};
// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS];
return (
waitForBatchedUpdates()
.then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0'))
// When Onyx is updated with the data and the sidebar re-renders
.then(() => {
const reportCollection: ReportCollectionDataSet = {
[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]: report,
};
const reportAction: ReportActionsCollectionDataSet = {
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`]: {[action.reportActionID]: action},
} as ReportActionsCollectionDataSet;
const reportNameValuePairsCollection: ReportNameValuePairsCollectionDataSet = {
[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.reportID}`]: reportNameValuePairs,
};
return Onyx.multiSet({
[ONYXKEYS.BETAS]: betas,
[ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.GSD,
[ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails,
[ONYXKEYS.IS_LOADING_APP]: false,
...reportNameValuePairsCollection,
...reportCollection,
...reportAction,
});
})
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNames = screen.queryAllByLabelText(hintText);
expect(displayNames.at(0)).toHaveTextContent('Report (archived)');
const hintMessagePreviewText = Localize.translateLocal('accessibilityHints.lastChatMessagePreview');
const messagePreviewTexts = screen.queryAllByLabelText(hintMessagePreviewText);
expect(messagePreviewTexts.at(0)).toHaveTextContent('This chat room has been archived.');
})
);
});
it('renders the policy deleted archive reason as the preview message of the chat', () => {
const report = {
...LHNTestUtils.getFakeReport([1, 2], 3, true),
policyName: 'Vikings Policy',
chatType: CONST.REPORT.CHAT_TYPE.POLICY_ROOM,
statusNum: CONST.REPORT.STATUS_NUM.CLOSED,
stateNum: CONST.REPORT.STATE_NUM.APPROVED,
// eslint-disable-next-line @typescript-eslint/naming-convention
private_isArchived: DateUtils.getDBTime(),
lastMessageText: 'test',
};
const action = {
...LHNTestUtils.getFakeReportAction('[email protected]', 3),
actionName: 'CLOSED',
originalMessage: {
policyName: 'Vikings Policy',
reason: 'policyDeleted',
},
};
const reportNameValuePairs = {
// eslint-disable-next-line @typescript-eslint/naming-convention
private_isArchived: DateUtils.getDBTime(),
};
// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS];
return (
waitForBatchedUpdates()
.then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0'))
// When Onyx is updated with the data and the sidebar re-renders
.then(() => {
const reportCollection: ReportCollectionDataSet = {
[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]: report,
};
const reportAction: ReportActionsCollectionDataSet = {
[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`]: {[action.reportActionID]: action},
} as ReportActionsCollectionDataSet;
const reportNameValuePairsCollection: ReportNameValuePairsCollectionDataSet = {
[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.reportID}`]: reportNameValuePairs,
};
return Onyx.multiSet({
[ONYXKEYS.BETAS]: betas,
[ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.GSD,
[ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails,
[ONYXKEYS.IS_LOADING_APP]: false,
...reportNameValuePairsCollection,
...reportCollection,
...reportAction,
});
})
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
const displayNames = screen.queryAllByLabelText(hintText);
expect(displayNames.at(0)).toHaveTextContent('Report (archived)');
const hintMessagePreviewText = Localize.translateLocal('accessibilityHints.lastChatMessagePreview');
const messagePreviewTexts = screen.queryAllByLabelText(hintMessagePreviewText);
expect(messagePreviewTexts.at(0)).toHaveTextContent('This chat is no longer active because Vikings Policy is no longer an active workspace.');
})
);
});
});
});
|
we have some conflicts here |
Explanation of Change
Fixed Issues
$ #54590
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop