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.

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

fix: clear anonymous user records after login #48144

Merged
merged 5 commits into from
Sep 5, 2024
Merged
Changes from all commits
Commits
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
25 changes: 24 additions & 1 deletion src/libs/actions/Session/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ import type {HybridAppRoute, Route} from '@src/ROUTES';
import ROUTES from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import type Credentials from '@src/types/onyx/Credentials';
import type {AutoAuthState} from '@src/types/onyx/Session';
import type Session from '@src/types/onyx/Session';
import type {AutoAuthState} from '@src/types/onyx/Session';
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB - was this just a prettier / style thing?

import clearCache from './clearCache';
import updateSessionAuthTokens from './updateSessionAuthTokens';

Expand Down Expand Up @@ -418,6 +418,21 @@ function beginSignIn(email: string) {
API.read(READ_COMMANDS.BEGIN_SIGNIN, params, {optimisticData, successData, failureData});
}

/**
* Create Onyx update to clean up anonymous user data
*/
function buildOnyxDataToCleanUpAnonymousUser() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need this if we have the anonymous session (and therefore the accountID) just before logging in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before this fix, there's a potential case where user:

  • Access the public room -> create anonymous record 1
  • Login then logout
  • Access the public room again -> create anonymous record 2

-> It's better to clear both records, no?

Also, I think it's a more holistic approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see your point! Just thinking though, don't we clear all Onyx data on logging out, so all those hypothetical extra anonymous records would get cleared on next log-out anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jjcoffee hmm 🤔 , I'll give it a thought and let you know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

const data: Record<string, null> = {};
if (session.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS && session.accountID) {
data[session.accountID] = null;
}
return {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
value: data,
onyxMethod: Onyx.METHOD.MERGE,
};
}

/**
* Creates an account for the new user and signs them into the application with the newly created account.
*
Expand All @@ -434,6 +449,8 @@ function signUpUser() {
},
];

const onyxOperationToCleanUpAnonymousUser = buildOnyxDataToCleanUpAnonymousUser();

const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand All @@ -442,6 +459,7 @@ function signUpUser() {
isLoading: false,
},
},
onyxOperationToCleanUpAnonymousUser,
];

const failureData: OnyxUpdate[] = [
Expand Down Expand Up @@ -517,6 +535,8 @@ function signIn(validateCode: string, twoFactorAuthCode?: string) {
},
];

const onyxOperationToCleanUpAnonymousUser = buildOnyxDataToCleanUpAnonymousUser();

const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand All @@ -533,6 +553,7 @@ function signIn(validateCode: string, twoFactorAuthCode?: string) {
validateCode,
},
},
onyxOperationToCleanUpAnonymousUser,
];

const failureData: OnyxUpdate[] = [
Expand Down Expand Up @@ -567,6 +588,7 @@ function signInWithValidateCode(accountID: number, code: string, twoFactorAuthCo
// If this is called from the 2fa step, get the validateCode directly from onyx
// instead of the one passed from the component state because the state is changing when this method is called.
const validateCode = twoFactorAuthCode ? credentials.validateCode : code;
const onyxOperationToCleanUpAnonymousUser = buildOnyxDataToCleanUpAnonymousUser();

const optimisticData: OnyxUpdate[] = [
{
Expand Down Expand Up @@ -607,6 +629,7 @@ function signInWithValidateCode(accountID: number, code: string, twoFactorAuthCo
key: ONYXKEYS.SESSION,
value: {autoAuthState: CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN},
},
onyxOperationToCleanUpAnonymousUser,
];

const failureData: OnyxUpdate[] = [
Expand Down
Loading