Skip to content

Commit

Permalink
redux: In REALM_ADD and LOGIN_SUCCESS, make realm be a URL object.
Browse files Browse the repository at this point in the history
As in the preceding handful of commits, stringify the realm exactly
where we need it as a string.

These should be the last sites in which it's not totally clear that
the realm is well-formed data.
  • Loading branch information
chrisbobbe committed Sep 21, 2020
1 parent 4207e2e commit 24c51e5
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/__tests__/lib/exampleData.js
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ export const action = deepFreeze({
},
login_success: {
type: LOGIN_SUCCESS,
realm: selfAccount.realm.toString(),
realm: selfAccount.realm,
email: selfAccount.email,
apiKey: selfAccount.apiKey,
},
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 @@ -28,7 +28,7 @@ describe('accountsReducer', () => {

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

test('if account with this realm exists, move to front of list', () => {
const action = deepFreeze({ ...baseAction, realm: account2.realm.toString() });
const action = deepFreeze({ ...baseAction, realm: account2.realm });
expect(accountsReducer(prevState, action)).toEqual([account2, account1]);
});
});
Expand All @@ -46,7 +46,7 @@ describe('accountsReducer', () => {
const existingAccountBase = eg.makeAccount({});
const baseAction = deepFreeze({
type: REALM_ADD,
realm: existingAccountBase.realm.toString(),
realm: existingAccountBase.realm,
zulipFeatureLevel: eg.zulipFeatureLevel,
zulipVersion: eg.zulipVersion,
});
Expand Down Expand Up @@ -168,7 +168,7 @@ describe('accountsReducer', () => {
type: LOGIN_SUCCESS,
apiKey: newAccount.apiKey,
email: newAccount.email,
realm: newAccount.realm.toString(),
realm: newAccount.realm,
});

const expectedState = [{ ...newAccount, zulipVersion: account1.zulipVersion }, account2];
Expand All @@ -190,7 +190,7 @@ describe('accountsReducer', () => {
type: LOGIN_SUCCESS,
apiKey: newAccount.apiKey,
email: newAccount.email,
realm: newAccount.realm.toString(),
realm: newAccount.realm,
});

const expectedState = [newAccount, account1, account2];
Expand All @@ -209,7 +209,7 @@ describe('accountsReducer', () => {
const action = deepFreeze({
type: LOGIN_SUCCESS,
apiKey: newAccount.apiKey,
realm: newAccount.realm.toString(),
realm: newAccount.realm,
email: newAccount.email,
});

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
17 changes: 6 additions & 11 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.toString() === action.realm);
const accountIndex = state.findIndex(
account => account.realm.toString() === action.realm.toString(),
);

if (accountIndex !== -1) {
const newAccount = {
Expand All @@ -29,7 +31,7 @@ const realmAdd = (state, action) => {

return [
{
realm: new URL(action.realm),
realm: action.realm,
apiKey: '',
email: '',
ackedPushToken: null,
Expand Down Expand Up @@ -67,17 +69,10 @@ const findAccount = (state: AccountsState, identity: Identity): number => {

const loginSuccess = (state, action) => {
const { realm, email, apiKey } = action;
const accountIndex = findAccount(state, { realm: new URL(realm), email });
const accountIndex = findAccount(state, { realm, email });
if (accountIndex === -1) {
return [
{
realm: new URL(realm),
email,
apiKey,
ackedPushToken: null,
zulipVersion: null,
zulipFeatureLevel: null,
},
{ realm, email, apiKey, ackedPushToken: null, zulipVersion: null, zulipFeatureLevel: null },
...state,
];
}
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/start/AuthScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class AuthScreen extends PureComponent<Props> {
const { dispatch, realm } = this.props;
const auth = webAuth.authFromCallbackUrl(event.url, otp, realm);
if (auth) {
dispatch(loginSuccess(auth.realm.toString(), auth.email, auth.apiKey));
dispatch(loginSuccess(auth.realm, auth.email, auth.apiKey));
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/start/DevAuthScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class DevAuthScreen extends PureComponent<Props, State> {

try {
const { api_key } = await api.devFetchApiKey(partialAuth, email);
this.props.dispatch(loginSuccess(partialAuth.realm.toString(), email, api_key));
this.props.dispatch(loginSuccess(partialAuth.realm, email, api_key));
this.setState({ progress: false });
} catch (err) {
this.setState({ progress: false, error: err.data && err.data.msg });
Expand Down
2 changes: 1 addition & 1 deletion src/start/PasswordAuthScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class PasswordAuthScreen extends PureComponent<Props, State> {
try {
const fetchedKey = await api.fetchApiKey(partialAuth, email, password);
this.setState({ progress: false });
dispatch(loginSuccess(partialAuth.realm.toString(), fetchedKey.email, fetchedKey.api_key));
dispatch(loginSuccess(partialAuth.realm, fetchedKey.email, fetchedKey.api_key));
} catch (err) {
this.setState({
progress: false,
Expand Down
2 changes: 1 addition & 1 deletion src/start/RealmScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class RealmScreen extends PureComponent<Props, State> {
const serverSettings: ApiResponseServerSettings = await api.getServerSettings(parsedRealm);
dispatch(
realmAdd(
realmInputValue,
parsedRealm,
serverSettings.zulip_feature_level ?? 0,
new ZulipVersion(serverSettings.zulip_version),
),
Expand Down

0 comments on commit 24c51e5

Please sign in to comment.