From 9aa44b8769fa28aada6b0cc28f1820a2634cac04 Mon Sep 17 00:00:00 2001 From: Wiktor Sieprawski Date: Wed, 11 Oct 2023 15:40:15 +0200 Subject: [PATCH] Fix/mobile init redirection (#1956) * fix: move extra responsibility away from redirection saga * chore: display errors during on-the-go-events * test: correct rntl tests * fix: navigation methods * test: duplicated username warning * fix: lint --- packages/mobile/src/RootNavigation.ts | 7 ++ .../PossibleImpersonationAttack.component.tsx | 2 +- .../PossibleImpersonationAttack.test.tsx | 2 +- .../ChannelList/ChannelList.screen.tsx | 29 +++++++- .../UsernameTaken/UsernameTaken.screen.tsx | 12 ++-- .../showNotification.saga.test.ts | 6 +- .../navigation/navigation.master.saga.ts | 2 + .../store/navigation/navigation.selectors.ts | 2 +- .../src/store/navigation/navigation.slice.ts | 11 +-- .../src/store/navigation/pop/pop.saga.ts | 8 +++ .../redirection/redirection.saga.ts | 28 +------- .../src/tests/duplicateUsername.test.tsx | 68 +++++++++++++++++++ .../possibleImpersonationAttack.test.tsx | 32 +++------ .../src/sagas/identity/identity.selectors.ts | 6 +- 14 files changed, 144 insertions(+), 71 deletions(-) create mode 100644 packages/mobile/src/store/navigation/pop/pop.saga.ts create mode 100644 packages/mobile/src/tests/duplicateUsername.test.tsx diff --git a/packages/mobile/src/RootNavigation.ts b/packages/mobile/src/RootNavigation.ts index e0a512d014..e9069aaf2c 100644 --- a/packages/mobile/src/RootNavigation.ts +++ b/packages/mobile/src/RootNavigation.ts @@ -17,3 +17,10 @@ export const replaceScreen = >(screen: Sc navigationRef.dispatch(StackActions.replace(screen, params)) } } + +export const pop = (): void => { + if (navigationRef.isReady()) { + // @ts-ignore + navigationRef.dispatch(StackActions.pop()) + } +} diff --git a/packages/mobile/src/components/PossibleImpersonationAttack/PossibleImpersonationAttack.component.tsx b/packages/mobile/src/components/PossibleImpersonationAttack/PossibleImpersonationAttack.component.tsx index 883a41fbbb..5899f6b117 100644 --- a/packages/mobile/src/components/PossibleImpersonationAttack/PossibleImpersonationAttack.component.tsx +++ b/packages/mobile/src/components/PossibleImpersonationAttack/PossibleImpersonationAttack.component.tsx @@ -40,7 +40,7 @@ const PossibleImpersonationAttackComponent: React.FC diff --git a/packages/mobile/src/components/PossibleImpersonationAttack/PossibleImpersonationAttack.test.tsx b/packages/mobile/src/components/PossibleImpersonationAttack/PossibleImpersonationAttack.test.tsx index 002fca33a6..088afdc2e7 100644 --- a/packages/mobile/src/components/PossibleImpersonationAttack/PossibleImpersonationAttack.test.tsx +++ b/packages/mobile/src/components/PossibleImpersonationAttack/PossibleImpersonationAttack.test.tsx @@ -14,13 +14,13 @@ describe('PossibleImpersonationAttack component', () => { expect(toJSON()).toMatchInlineSnapshot(` { const dispatch = useDispatch() + const usernameTaken = useSelector(identity.selectors.usernameTaken) + const duplicateCerts = useSelector(users.selectors.duplicateCerts) + + useEffect(() => { + if (usernameTaken) { + dispatch( + navigationActions.navigation({ + screen: ScreenNames.UsernameTakenScreen, + }) + ) + } + + if (duplicateCerts) { + dispatch( + navigationActions.navigation({ + screen: ScreenNames.PossibleImpersonationAttackScreen, + }) + ) + } + }, [dispatch, usernameTaken, duplicateCerts]) + const redirect = useCallback( (id: string) => { dispatch( @@ -34,10 +55,12 @@ export const ChannelListScreen: FC = () => { ) const community = useSelector(communities.selectors.currentCommunity) + const channelsStatusSorted = useSelector(publicChannels.selectors.channelsStatusSorted) const tiles = channelsStatusSorted.map(status => { const newestMessage = status.newestMessage + const message = newestMessage?.message || '...' const date = newestMessage?.createdAt ? formatMessageDisplayDate(newestMessage.createdAt) : undefined diff --git a/packages/mobile/src/screens/UsernameTaken/UsernameTaken.screen.tsx b/packages/mobile/src/screens/UsernameTaken/UsernameTaken.screen.tsx index 9e2b3dcaed..46e0549983 100644 --- a/packages/mobile/src/screens/UsernameTaken/UsernameTaken.screen.tsx +++ b/packages/mobile/src/screens/UsernameTaken/UsernameTaken.screen.tsx @@ -9,17 +9,17 @@ import { navigationActions } from '../../store/navigation/navigation.slice' const UsernameTakenScreen: React.FC = () => { const dispatch = useDispatch() + const currentIdentity = useSelector(identity.selectors.currentIdentity) + const usernameRegistered = currentIdentity?.userCertificate != null - const error = useSelector(errors.selectors.registrarErrors) + const registeredUsers = useSelector(users.selectors.certificatesMapping) + const error = useSelector(errors.selectors.registrarErrors) + const handleBackButton = useCallback(() => { - dispatch( - navigationActions.replaceScreen({ - screen: ScreenNames.ChannelListScreen, - }) - ) + dispatch(navigationActions.pop()) }, [dispatch]) const handleAction = (nickname: string) => { diff --git a/packages/mobile/src/store/nativeServices/showNotification/showNotification.saga.test.ts b/packages/mobile/src/store/nativeServices/showNotification/showNotification.saga.test.ts index 8ede98660d..9265012dd4 100644 --- a/packages/mobile/src/store/nativeServices/showNotification/showNotification.saga.test.ts +++ b/packages/mobile/src/store/nativeServices/showNotification/showNotification.saga.test.ts @@ -129,7 +129,7 @@ describe('showNotificationSaga', () => { }, [StoreKeys.Navigation]: { ...new NavigationState(), - currentScreen: ScreenNames.ChannelScreen, + backStack: [ScreenNames.ChannelScreen], }, } ) @@ -174,7 +174,7 @@ describe('showNotificationSaga', () => { }, [StoreKeys.Navigation]: { ...new NavigationState(), - currentScreen: ScreenNames.ChannelScreen, + backStack: [ScreenNames.ChannelScreen], }, } ) @@ -213,7 +213,7 @@ describe('showNotificationSaga', () => { }, [StoreKeys.Navigation]: { ...new NavigationState(), - currentScreen: ScreenNames.ChannelScreen, + backStack: [ScreenNames.ChannelScreen], }, } ) diff --git a/packages/mobile/src/store/navigation/navigation.master.saga.ts b/packages/mobile/src/store/navigation/navigation.master.saga.ts index 9867a9e006..6735316601 100644 --- a/packages/mobile/src/store/navigation/navigation.master.saga.ts +++ b/packages/mobile/src/store/navigation/navigation.master.saga.ts @@ -3,11 +3,13 @@ import { navigationActions } from './navigation.slice' import { redirectionSaga } from './redirection/redirection.saga' import { navigationSaga } from './navigation/navigation.saga' import { replaceScreenSaga } from './replaceScreen/replaceScreen.saga' +import { popSaga } from './pop/pop.saga' export function* navigationMasterSaga(): Generator { yield all([ takeEvery(navigationActions.redirection.type, redirectionSaga), takeEvery(navigationActions.navigation.type, navigationSaga), takeEvery(navigationActions.replaceScreen.type, replaceScreenSaga), + takeEvery(navigationActions.pop.type, popSaga), ]) } diff --git a/packages/mobile/src/store/navigation/navigation.selectors.ts b/packages/mobile/src/store/navigation/navigation.selectors.ts index b180fb2d58..4553b47f44 100644 --- a/packages/mobile/src/store/navigation/navigation.selectors.ts +++ b/packages/mobile/src/store/navigation/navigation.selectors.ts @@ -5,7 +5,7 @@ import { CreatedSelectors, StoreState } from '../store.types' const navigationSlice: CreatedSelectors[StoreKeys.Navigation] = (state: StoreState) => state[StoreKeys.Navigation] -export const currentScreen = createSelector(navigationSlice, reducerState => reducerState.currentScreen) +export const currentScreen = createSelector(navigationSlice, reducerState => reducerState.backStack.slice(-1)[0]) export const contextMenuVisibility = (menu: MenuName) => createSelector(navigationSlice, reducerState => { diff --git a/packages/mobile/src/store/navigation/navigation.slice.ts b/packages/mobile/src/store/navigation/navigation.slice.ts index 6a1d1a1b43..fd9b45717a 100644 --- a/packages/mobile/src/store/navigation/navigation.slice.ts +++ b/packages/mobile/src/store/navigation/navigation.slice.ts @@ -4,7 +4,7 @@ import { ScreenNames } from '../../const/ScreenNames.enum' import { MenuName } from '../../const/MenuNames.enum' export class NavigationState { - public currentScreen: ScreenNames = ScreenNames.SplashScreen + public backStack: ScreenNames[] = [ScreenNames.SplashScreen] public confirmationBox: ConfirmationBox = { open: false, args: {}, @@ -53,16 +53,19 @@ export const navigationSlice = createSlice({ redirection: state => state, navigation: (state, action: PayloadAction) => { const { screen } = action.payload - state.currentScreen = screen + state.backStack.push(screen) }, // Replace screen overrides last screen in backstack replaceScreen: (state, action: PayloadAction) => { const { screen } = action.payload - state.currentScreen = screen + state.backStack.pop() + state.backStack.push(screen) + }, + pop: state => { + state.backStack.pop() }, setPendingNavigation: (state, action: PayloadAction) => { const { screen } = action.payload - state.currentScreen = screen state.pendingNavigation = screen }, clearPendingNavigation: state => { diff --git a/packages/mobile/src/store/navigation/pop/pop.saga.ts b/packages/mobile/src/store/navigation/pop/pop.saga.ts new file mode 100644 index 0000000000..b7dc6fa416 --- /dev/null +++ b/packages/mobile/src/store/navigation/pop/pop.saga.ts @@ -0,0 +1,8 @@ +import { PayloadAction } from '@reduxjs/toolkit' +import { call } from 'typed-redux-saga' +import { navigationActions } from '../navigation.slice' +import { pop } from '../../../RootNavigation' + +export function* popSaga(_action: PayloadAction['payload']>): Generator { + yield* call(pop) +} diff --git a/packages/mobile/src/store/navigation/redirection/redirection.saga.ts b/packages/mobile/src/store/navigation/redirection/redirection.saga.ts index f05f04593d..a2f60b5519 100644 --- a/packages/mobile/src/store/navigation/redirection/redirection.saga.ts +++ b/packages/mobile/src/store/navigation/redirection/redirection.saga.ts @@ -3,7 +3,7 @@ import { initSelectors } from '../../init/init.selectors' import { navigationSelectors } from '../navigation.selectors' import { navigationActions } from '../navigation.slice' import { ScreenNames } from '../../../const/ScreenNames.enum' -import { identity, publicChannels, users } from '@quiet/state-manager' +import { identity } from '@quiet/state-manager' import { initActions } from '../../init/init.slice' export function* redirectionSaga(): Generator { @@ -25,34 +25,10 @@ export function* redirectionSaga(): Generator { return } - const isUsernameTaken = yield* select(identity.selectors.usernameTaken) - - if (isUsernameTaken) { - yield* put( - navigationActions.replaceScreen({ - screen: ScreenNames.UsernameTakenScreen, - }) - ) - } - - const duplicateCerts = yield* select(users.selectors.duplicateCerts) - - if (duplicateCerts) { - yield* put( - navigationActions.replaceScreen({ - screen: ScreenNames.PossibleImpersonationAttackScreen, - }) - ) - return - } - // If user belongs to a community, let him directly into the app - // Check while QA session! - const currentChannelDisplayableMessages = yield* select(publicChannels.selectors.currentChannelMessagesMergedBySender) - const areMessagesLoaded = Object.values(currentChannelDisplayableMessages).length > 0 const communityMembership = yield* select(identity.selectors.communityMembership) - if (communityMembership && areMessagesLoaded) { + if (communityMembership) { yield* put( navigationActions.replaceScreen({ screen: ScreenNames.ChannelListScreen, diff --git a/packages/mobile/src/tests/duplicateUsername.test.tsx b/packages/mobile/src/tests/duplicateUsername.test.tsx new file mode 100644 index 0000000000..769b3ebcfc --- /dev/null +++ b/packages/mobile/src/tests/duplicateUsername.test.tsx @@ -0,0 +1,68 @@ +import React from 'react' +import '@testing-library/jest-native/extend-expect' +import MockedSocket from 'socket.io-mock' +import { ioMock } from '../setupTests' +import { prepareStore } from './utils/prepareStore' +import { renderComponent } from './utils/renderComponent' +import { FactoryGirl } from 'factory-girl' +import { getFactory, communities, identity, users } from '@quiet/state-manager' +import { ScreenNames } from '../const/ScreenNames.enum' +import { initActions } from '../store/init/init.slice' +import { ChannelListScreen } from '../screens/ChannelList/ChannelList.screen' +import { navigationSelectors } from '../store/navigation/navigation.selectors' +import { navigationActions } from '../store/navigation/navigation.slice' + +describe('Duplicate username warning', () => { + let socket: MockedSocket + + let factory: FactoryGirl + + beforeEach(async () => { + socket = new MockedSocket() + ioMock.mockImplementation(() => socket) + }) + + it("Display prompt for username change if it's already taken", async () => { + const { store, root } = await prepareStore({}, socket) + + store.dispatch(initActions.setStoreReady()) + + factory = await getFactory(store) + + const community = await factory.create['payload']>( + 'Community' + ) + + const alice = ( + await factory.build('Identity', { + id: community.id, + nickname: 'alice', + }) + ).payload + + store.dispatch( + users.actions.storeUserCertificate({ + certificate: alice.userCertificate || 'certificate_alice', + }) + ) + + await factory.create['payload']>('Identity', { + id: community.id, + nickname: 'alice', + userCertificate: null, + }) + + store.dispatch(navigationActions.navigation({ screen: ScreenNames.ChannelListScreen })) + + renderComponent(, store) + + // Confirm there's duplication of usernames + const usernameTaken = identity.selectors.usernameTaken(store.getState()) + expect(usernameTaken).toBe(true) + + const currentScreen = navigationSelectors.currentScreen(store.getState()) + expect(currentScreen).toBe(ScreenNames.UsernameTakenScreen) + + root?.cancel() + }) +}) diff --git a/packages/mobile/src/tests/possibleImpersonationAttack.test.tsx b/packages/mobile/src/tests/possibleImpersonationAttack.test.tsx index d427c06030..80e8b4963e 100644 --- a/packages/mobile/src/tests/possibleImpersonationAttack.test.tsx +++ b/packages/mobile/src/tests/possibleImpersonationAttack.test.tsx @@ -1,17 +1,15 @@ import React from 'react' import '@testing-library/jest-native/extend-expect' -import { act } from '@testing-library/react-native' import MockedSocket from 'socket.io-mock' import { ioMock } from '../setupTests' import { prepareStore } from './utils/prepareStore' import { renderComponent } from './utils/renderComponent' import { FactoryGirl } from 'factory-girl' import { getFactory, communities, identity, users } from '@quiet/state-manager' -import { navigationActions } from '../store/navigation/navigation.slice' -import { PossibleImpersonationAttackScreen } from '../screens/PossibleImpersonationAttack/PossibleImpersonationAttack.screen' import { ScreenNames } from '../const/ScreenNames.enum' -import { navigationSelectors } from '../store/navigation/navigation.selectors' +import { ChannelListScreen } from '../screens/ChannelList/ChannelList.screen' import { initActions } from '../store/init/init.slice' +import { navigationSelectors } from '../store/navigation/navigation.selectors' describe('Possible Impersonation Attack', () => { let socket: MockedSocket @@ -23,9 +21,11 @@ describe('Possible Impersonation Attack', () => { ioMock.mockImplementation(() => socket) }) - it('Open modal when certifcates are duplicated', async () => { + it('Display warning when certifcates are duplicated', async () => { const { store, root } = await prepareStore({}, socket) + store.dispatch(initActions.setStoreReady()) + factory = await getFactory(store) const community = await factory.create['payload']>( @@ -37,17 +37,6 @@ describe('Possible Impersonation Attack', () => { nickname: 'alice', }) - const route: { key: string; name: ScreenNames.PossibleImpersonationAttackScreen; path?: string | undefined } = { - key: '', - name: ScreenNames.PossibleImpersonationAttackScreen, - } - renderComponent( - <> - - , - store - ) - const cert1 = 'MIIDeDCCAx6gAwIBAgIGAYr6Jw3hMAoGCCqGSM49BAMCMA8xDTALBgNVBAMTBGZyZmQwHhcNMjMxMDA0MTAwNjE4WhcNMzAwMTMxMjMwMDAwWjBJMUcwRQYDVQQDEz5tenhydWhyNWJzdGt3dmp3eWJnZ2Y2M2Jma2dreWw0aWs0bG5lanN1YnFlaG9td3Vpbm43d3JxZC5vbmlvbjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABGcKhXLUNjkS9+xd0hYfJBOA7bXB5LwZojhzgBQps3SW/CSR6ABiAuirdP0x/byxTXSkZY23lBkvc5CqMjWe3lWjggIqMIICJjAJBgNVHRMEAjAAMAsGA1UdDwQEAwIAgDAdBgNVHSUEFjAUBggrBgEFBQcDAgYIKwYBBQUHAwEwggFHBgkqhkiG9w0BCQwEggE4BIIBNAXhwXxmLy7Gg5uonlWXiqRUimGLj2cPbAoK9DnKHkcohqdLvEzyz6rM7KBewO068fag0d/PR0uh37Oyb7d/JAbBjhJmf8wOl2HfLTThPEEH8isy3bxHXx4Ir5prVVk1zx8UiXtPAu6gK41FY5Oin6SpV07MBewqQGcbCovcbBSwkp6EXmLXPOGgmpFlQf5CNGIs3YqPD+Ll1vn8Lq5QIGCa210Pq/T65mrPsXVAw2vJO6DFRIAGrAF5VxDS8G2dSwnDnje+bD2NO8qlfwFdO3bkDeheOqZXCSxlPA6q1bY34qYR2zrwSiQCjRiCQjifRCmF2Jg4ojzLGUL0pKdvi+8fDQXollmazh5boJWN9GRy+1sDLTk01cW2kF7esew5PlDi8kX0v2hY+XsR5eQga1j3MkXkMBgGCisGAQQBg4wbAgEEChMIdXNlcm5hbWUwPQYJKwYBAgEPAwEBBDATLlFtUHF6THFheFk1UmI4VlpjM1VuZmlXZXRxVDZKWkp6SnRjWVFXNmhXTENvRXIwSQYDVR0RBEIwQII+bXp4cnVocjVic3Rrd3Zqd3liZ2dmNjNiZmtna3lsNGlrNGxuZWpzdWJxZWhvbXd1aW5uN3dycWQub25pb24wCgYIKoZIzj0EAwIDSAAwRQIhAJx4YX8KtOA4WGAWzW0M7FvuoblNOb370521GsfuHfbMAiBEYQ4l074oUEF2DTVK1agJlhMR5USRxav5xEpx2ujMeA==' @@ -66,17 +55,14 @@ describe('Possible Impersonation Attack', () => { }) ) - await act(async () => {}) - - store.dispatch(navigationActions.redirection()) - - await act(async () => {}) + renderComponent(, store) const duplicateCerts = users.selectors.duplicateCerts(store.getState()) - const currentScreen = navigationSelectors.currentScreen(store.getState()) + expect(duplicateCerts).toBe(true) + const currentScreen = navigationSelectors.currentScreen(store.getState()) expect(currentScreen).toBe(ScreenNames.PossibleImpersonationAttackScreen) - expect(duplicateCerts).toBeTruthy() + root?.cancel() }) }) diff --git a/packages/state-manager/src/sagas/identity/identity.selectors.ts b/packages/state-manager/src/sagas/identity/identity.selectors.ts index 9004adb288..8e9b48cc98 100644 --- a/packages/state-manager/src/sagas/identity/identity.selectors.ts +++ b/packages/state-manager/src/sagas/identity/identity.selectors.ts @@ -2,7 +2,7 @@ import { StoreKeys } from '../store.keys' import { createSelector } from '@reduxjs/toolkit' import { identityAdapter } from './identity.adapter' import { type CreatedSelectors, type StoreState } from '../store.types' -import { communitiesSelectors, selectCommunities } from '../communities/communities.selectors' +import { communitiesSelectors, selectCommunities, currentCommunity } from '../communities/communities.selectors' import { certificatesMapping } from '../users/users.selectors' const identitySlice: CreatedSelectors[StoreKeys.Identity] = (state: StoreState) => state[StoreKeys.Identity] @@ -22,8 +22,8 @@ export const currentIdentity = createSelector( } ) -export const communityMembership = createSelector(currentIdentity, identity => { - return Boolean(identity?.userCsr) +export const communityMembership = createSelector(currentIdentity, currentCommunity, (identity, community) => { + return Boolean(identity?.userCsr && community?.name) }) export const hasCertificate = createSelector(currentIdentity, identity => {