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

Store state.accounts[].realm as a WHATWG URL object. #4235

Merged
merged 10 commits into from
Sep 22, 2020
Merged
6 changes: 3 additions & 3 deletions src/__tests__/lib/exampleData.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export const makeCrossRealmBot = (args: { name?: string } = {}): CrossRealmBot =
is_bot: true,
});

export const realm = 'https://zulip.example.org';
export const realm = new URL('https://zulip.example.org');

export const zulipVersion = new ZulipVersion('2.1.0-234-g7c3acf4');

Expand All @@ -133,7 +133,7 @@ export const makeAccount = (
args: {
user?: User,
email?: string,
realm?: string,
realm?: URL,
apiKey?: string,
zulipFeatureLevel?: number | null,
zulipVersion?: ZulipVersion | null,
Expand Down Expand Up @@ -475,7 +475,7 @@ export const action = deepFreeze({
realm_send_welcome_emails: true,
realm_show_digest_email: true,
realm_signup_notifications_stream_id: 3,
realm_uri: selfAccount.realm,
realm_uri: selfAccount.realm.toString(),
realm_video_chat_provider: 1,
realm_waiting_period_threshold: 3,
zulip_feature_level: 1,
Expand Down
2 changes: 1 addition & 1 deletion src/account-info/AccountDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const componentStyles = createStyleSheet({
const AVATAR_SIZE = 200;

type SelectorProps = {|
realm: string,
realm: URL,
userStatusText: string | void,
|};

Expand Down
4 changes: 2 additions & 2 deletions src/account/AccountItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const styles = createStyleSheet({
type Props = $ReadOnly<{|
index: number,
email: string,
realm: string,
realm: URL,
onSelect: (index: number) => void,
onRemove: (index: number) => void,
showDoneIcon: boolean,
Expand All @@ -58,7 +58,7 @@ export default class AccountItem extends PureComponent<Props> {
<View style={[styles.accountItem, showDoneIcon && styles.selectedAccountItem]}>
<View style={styles.details}>
<RawLabel style={styles.text} text={email} numberOfLines={1} />
<RawLabel style={styles.text} text={realm} numberOfLines={1} />
<RawLabel style={styles.text} text={realm.toString()} numberOfLines={1} />
</View>
{!showDoneIcon ? (
<IconTrash style={styles.icon} size={24} color="crimson" onPress={this.handleRemove} />
Expand Down
2 changes: 1 addition & 1 deletion src/account/AccountList.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export default class AccountList extends PureComponent<Props> {
<View>
<FlatList
data={accounts}
keyExtractor={item => `${item.email}${item.realm}`}
keyExtractor={item => `${item.email}${item.realm.toString()}`}
ItemSeparatorComponent={() => <ViewPlaceholder height={8} />}
renderItem={({ item, index }) => (
<AccountItem
Expand Down
12 changes: 6 additions & 6 deletions src/account/__tests__/accountsReducer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import * as eg from '../../__tests__/lib/exampleData';
describe('accountsReducer', () => {
describe('REALM_ADD', () => {
describe('on list of identities', () => {
const account1 = eg.makeAccount({ realm: 'https://realm.one.org', apiKey: '' });
const account2 = eg.makeAccount({ realm: 'https://realm.two.org', apiKey: '' });
const account1 = eg.makeAccount({ realm: new URL('https://realm.one.org'), apiKey: '' });
const account2 = eg.makeAccount({ realm: new URL('https://realm.two.org'), apiKey: '' });
const prevState = deepFreeze([account1, account2]);
const baseAction = deepFreeze({
type: REALM_ADD,
Expand All @@ -27,7 +27,7 @@ describe('accountsReducer', () => {
});

test('if no account with this realm exists, prepend new one, with empty email/apiKey', () => {
const newRealm = 'https://new.realm.org';
const newRealm = new URL('https://new.realm.org');
const action = deepFreeze({ ...baseAction, realm: newRealm });
expect(accountsReducer(prevState, action)).toEqual([
eg.makeAccount({ realm: newRealm, email: '', apiKey: '' }),
Expand Down Expand Up @@ -153,8 +153,8 @@ describe('accountsReducer', () => {
});

describe('LOGIN_SUCCESS', () => {
const account1 = eg.makeAccount({ email: '', realm: 'https://one.example.org' });
const account2 = eg.makeAccount({ realm: 'https://two.example.org' });
const account1 = eg.makeAccount({ email: '', realm: new URL('https://one.example.org') });
const account2 = eg.makeAccount({ realm: new URL('https://two.example.org') });

const prevState = deepFreeze([account1, account2]);

Expand All @@ -181,7 +181,7 @@ describe('accountsReducer', () => {
test('on login, if account does not exist, add as first item', () => {
const newAccount = eg.makeAccount({
email: '[email protected]',
realm: 'https://new.realm.org',
realm: new URL('https://new.realm.org'),
zulipVersion: null,
zulipFeatureLevel: null,
});
Expand Down
4 changes: 2 additions & 2 deletions src/account/accountActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const switchAccount = (index: number): Action => ({
});

export const realmAdd = (
realm: string,
realm: URL,
zulipFeatureLevel: number,
zulipVersion: ZulipVersion,
): Action => ({
Expand All @@ -30,7 +30,7 @@ export const removeAccount = (index: number): Action => ({
index,
});

export const loginSuccess = (realm: string, email: string, apiKey: string): Action => ({
export const loginSuccess = (realm: URL, email: string, apiKey: string): Action => ({
type: LOGIN_SUCCESS,
realm,
email,
Expand Down
3 changes: 2 additions & 1 deletion src/account/accountMisc.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export const identityOfAuth: Auth => Identity = identitySlice;
export const identityOfAccount: Account => Identity = identitySlice;

/** A string corresponding uniquely to an identity, for use in `Map`s. */
export const keyOfIdentity = ({ realm, email }: Identity): string => `${realm}\0${email}`;
export const keyOfIdentity = ({ realm, email }: Identity): string =>
`${realm.toString()}\0${email}`;

export const authOfAccount = (account: Account): Auth => {
const { realm, email, apiKey } = account;
Expand Down
7 changes: 5 additions & 2 deletions src/account/accountsReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import { NULL_ARRAY } from '../nullObjects';
const initialState = NULL_ARRAY;

const realmAdd = (state, action) => {
const accountIndex = state.findIndex(account => account.realm === action.realm);
const accountIndex = state.findIndex(
account => account.realm.toString() === action.realm.toString(),
);

if (accountIndex !== -1) {
const newAccount = {
Expand Down Expand Up @@ -60,7 +62,8 @@ const accountSwitch = (state, action) => {
const findAccount = (state: AccountsState, identity: Identity): number => {
const { realm, email } = identity;
return state.findIndex(
account => account.realm === realm && (!account.email || account.email === email),
account =>
account.realm.toString() === realm.toString() && (!account.email || account.email === email),
);
};

Expand Down
4 changes: 2 additions & 2 deletions src/actionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ type AccountSwitchAction = {|

type RealmAddAction = {|
type: typeof REALM_ADD,
realm: string,
realm: URL,
zulipFeatureLevel: number,
zulipVersion: ZulipVersion,
|};
Expand All @@ -154,7 +154,7 @@ type AccountRemoveAction = {|

type LoginSuccessAction = {|
type: typeof LOGIN_SUCCESS,
realm: string,
realm: URL,
email: string,
apiKey: string,
|};
Expand Down
2 changes: 1 addition & 1 deletion src/api/apiFetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const apiFetch = async (
auth: Auth,
route: string,
params: $Diff<$Exact<RequestOptions>, {| headers: mixed |}>,
) => fetchWithAuth(auth, `${auth.realm}/${apiVersion}/${route}`, params);
) => fetchWithAuth(auth, `${auth.realm.toString()}/${apiVersion}/${route}`, params);

export const apiCall = async (
auth: Auth,
Expand Down
2 changes: 1 addition & 1 deletion src/api/settings/getServerSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ export type ApiResponseServerSettings = {|
|};

/** See https://zulip.com/api/server-settings */
export default async (realm: string): Promise<ApiResponseServerSettings> =>
export default async (realm: URL): Promise<ApiResponseServerSettings> =>
apiGet({ apiKey: '', email: '', realm }, 'server_settings');
2 changes: 1 addition & 1 deletion src/api/transportTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* an `Auth`.
*/
export type Auth = {|
realm: string,
realm: URL,
apiKey: string,
email: string,
|};
Expand Down
16 changes: 16 additions & 0 deletions src/boot/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ const migrations: { [string]: (GlobalState) => GlobalState } = {
...state,
accounts: state.accounts.map(a => ({
...a,
// `a.realm` is a string until migration 15
// $FlowMigrationFudge
realm: a.realm.replace(/\/+$/, ''),
})),
}),
Expand Down Expand Up @@ -187,6 +189,16 @@ const migrations: { [string]: (GlobalState) => GlobalState } = {
})),
}),

// Convert Accounts[].realm from `string` to `URL`
'15': state => ({
...state,
accounts: state.accounts.map(a => ({
...a,
// $FlowMigrationFudge - `a.realm` will be a string here
realm: new URL(a.realm),
})),
}),

// TIP: When adding a migration, consider just using `dropCache`.
};

Expand Down Expand Up @@ -271,6 +283,8 @@ const SERIALIZED_TYPE_FIELD_NAME: '__serializedType__' = '__serializedType__';
const customReplacer = (key, value, defaultReplacer) => {
if (value instanceof ZulipVersion) {
return { data: value.raw(), [SERIALIZED_TYPE_FIELD_NAME]: 'ZulipVersion' };
} else if (value instanceof URL) {
return { data: value.toString(), [SERIALIZED_TYPE_FIELD_NAME]: 'URL' };
}
return defaultReplacer(key, value);
};
Expand All @@ -281,6 +295,8 @@ const customReviver = (key, value, defaultReviver) => {
switch (value[SERIALIZED_TYPE_FIELD_NAME]) {
case 'ZulipVersion':
return new ZulipVersion(data);
case 'URL':
return new URL(data);
default:
// Fall back to defaultReviver, below
}
Expand Down
2 changes: 1 addition & 1 deletion src/common/OwnAvatar.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type Props = $ReadOnly<{|
dispatch: Dispatch,
user: User,
size: number,
realm: string,
realm: URL,
|}>;

/**
Expand Down
3 changes: 1 addition & 2 deletions src/common/UserAvatarWithPresence.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type Props = $ReadOnly<{|
avatarUrl: ?string,
email: string,
size: number,
realm: string,
realm: URL,
shape: 'rounded' | 'square',
onPress?: () => void,
|}>;
Expand All @@ -43,7 +43,6 @@ class UserAvatarWithPresence extends PureComponent<Props> {
avatarUrl: '',
email: '',
size: 32,
realm: '',
shape: 'rounded',
};

Expand Down
2 changes: 1 addition & 1 deletion src/common/WebLink.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type Props = $ReadOnly<{|
dispatch: Dispatch,
label: string,
href: string,
realm: string,
realm: URL,
|}>;

/**
Expand Down
8 changes: 4 additions & 4 deletions src/emoji/__tests__/emojiSelectors-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('getActiveImageEmojiById', () => {
const state = {
accounts: [
{
realm: 'https://example.com',
realm: new URL('https://example.com'),
},
],
realm: {
Expand Down Expand Up @@ -56,7 +56,7 @@ describe('getActiveImageEmojiById', () => {
describe('getAllImageEmojiById', () => {
test('get realm emojis with absolute url', () => {
const state = {
accounts: [{ realm: 'https://example.com' }],
accounts: [{ realm: new URL('https://example.com') }],
realm: {
emoji: {
1: {
Expand Down Expand Up @@ -87,7 +87,7 @@ describe('getAllImageEmojiById', () => {
describe('getAllImageEmojiByCode', () => {
test('get realm emoji object with emoji names as the keys', () => {
const state = {
accounts: [{ realm: 'https://example.com' }],
accounts: [{ realm: new URL('https://example.com') }],
realm: {
emoji: {
1: {
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('getAllImageEmojiByCode', () => {
describe('getActiveImageEmojiByName', () => {
test('get realm emoji object with emoji names as the keys', () => {
const state = {
accounts: [{ realm: 'https://example.com' }],
accounts: [{ realm: new URL('https://example.com') }],
realm: {
emoji: {
1: {
Expand Down
10 changes: 6 additions & 4 deletions src/nav/__tests__/navReducer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ describe('navReducer', () => {
const action = deepFreeze({
type: REHYDRATE,
payload: {
accounts: [{ apiKey: '', realm: 'https://example.com' }],
accounts: [{ apiKey: '', realm: new URL('https://example.com') }],
users: [],
realm: {},
},
Expand All @@ -147,8 +147,8 @@ describe('navReducer', () => {
type: REHYDRATE,
payload: {
accounts: [
{ apiKey: '', realm: 'https://example.com', email: '[email protected]' },
{ apiKey: '', realm: 'https://example.com', email: '[email protected]' },
{ apiKey: '', realm: new URL('https://example.com'), email: '[email protected]' },
{ apiKey: '', realm: new URL('https://example.com'), email: '[email protected]' },
],
users: [],
realm: {},
Expand All @@ -165,7 +165,9 @@ describe('navReducer', () => {
const action = deepFreeze({
type: REHYDRATE,
payload: {
accounts: [{ apiKey: '', realm: 'https://example.com', email: '[email protected]' }],
accounts: [
{ apiKey: '', realm: new URL('https://example.com'), email: '[email protected]' },
],
users: [],
realm: {},
},
Expand Down
2 changes: 1 addition & 1 deletion src/nav/navActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const navigateToAccountDetails = (userId: number): NavigationAction =>
export const navigateToGroupDetails = (recipients: UserOrBot[]): NavigationAction =>
StackActions.push({ routeName: 'group-details', params: { recipients } });

export const navigateToRealmScreen = (realm?: string): NavigationAction =>
export const navigateToRealmScreen = (realm?: URL): NavigationAction =>
StackActions.push({ routeName: 'realm', params: { realm } });

export const navigateToLightbox = (src: string, message: Message): NavigationAction =>
Expand Down
4 changes: 2 additions & 2 deletions src/notification/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const getAccountFromNotificationData = (

const urlMatches = [];
identities.forEach((account, i) => {
if (account.realm === realm_uri) {
if (account.realm.toString() === realm_uri) {
urlMatches.push(i);
}
});
Expand All @@ -49,7 +49,7 @@ export const getAccountFromNotificationData = (
// just a race -- this notification was sent before the logout); or
// there's some confusion where the realm_uri we have is different from
// the one the server sends in notifications.
const knownUrls = identities.map(({ realm }) => realm);
const knownUrls = identities.map(({ realm }) => realm.toString());
logging.warn('notification realm_uri not found in accounts', {
realm_uri,
known_urls: knownUrls,
Expand Down
2 changes: 1 addition & 1 deletion src/settings/LegalScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getCurrentRealm } from '../selectors';

type Props = $ReadOnly<{|
dispatch: Dispatch,
realm: string,
realm: URL,
|}>;

class LegalScreen extends PureComponent<Props> {
Expand Down
Loading