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

(15) refactor: network settings actions #360

Merged
merged 23 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
5b16276
chore: remove legacy comment
alexruzenhack Oct 23, 2023
300e4b4
refactor: rename action to NETWORKSETTINGS_UPDATE_REQUEST
alexruzenhack Oct 23, 2023
d14be82
feat: add action NETWORKSETTINGS_UPDATE_STATE
alexruzenhack Oct 23, 2023
7ffd6eb
feat: add action NETWORKSETTINGS_PERSIST_STORE
alexruzenhack Oct 23, 2023
461f1d4
feat: add action NETWORKSETTINGS_UPDATE_WAITING
alexruzenhack Oct 23, 2023
e938aea
refactor: action NETWORKSETTINGS_UPDATE_SUCCESS
alexruzenhack Oct 23, 2023
46de5af
refactor: action NETWORKSETTINGS_UPDATE_INVALID
alexruzenhack Oct 23, 2023
03ba63f
refactor: action NETWORKSETTINGS_UPDATE_FAILURE
alexruzenhack Oct 23, 2023
61d88a5
chore: add docstring to action NETWORKSETTINGS_UPDATE_READY
alexruzenhack Oct 24, 2023
0352e19
refactor: NetworkStatusBar
alexruzenhack Oct 24, 2023
c70ac97
refactor: saga effect persistNetworkSettings
alexruzenhack Oct 24, 2023
02acec2
lint: resolve all the lint issues
alexruzenhack Oct 24, 2023
67fa735
review: apply msg suggestions
alexruzenhack Oct 30, 2023
1be1450
refactor: inline retrieve of network settings
alexruzenhack Oct 31, 2023
0f97f75
fix: typos
alexruzenhack Oct 31, 2023
e0ad2a1
refactor: replace to helper's functions
alexruzenhack Oct 31, 2023
393eb4b
feat: add fallback for the ready status
alexruzenhack Oct 31, 2023
185670b
chore: wrap text in localization mechanism
alexruzenhack Oct 31, 2023
51609ad
feat: ignore wallet service URLs for mainnet check on network status bar
alexruzenhack Oct 31, 2023
bbb8f5d
feat: add full check of custom network against mainnet pre-settings
alexruzenhack Nov 1, 2023
f31c263
chore: add comments explaining the two checks on notMainnet
alexruzenhack Nov 1, 2023
3825b2e
chore: add comment to explain set READY fallback behavior
alexruzenhack Nov 1, 2023
d994fdb
chore: update docstrings of action methods
alexruzenhack Nov 1, 2023
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
46 changes: 23 additions & 23 deletions __tests__/sagas/networkSettings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { all, effectTypes, fork } from 'redux-saga/effects';
import createSagaMiddleware, { END, runSaga } from 'redux-saga';
import { applyMiddleware, createStore } from 'redux';
import { reducer } from '../../src/reducers/reducer';
import { networkSettingsUpdate, networkSettingsUpdateSuccess, reloadWalletRequested, types } from '../../src/actions';
import { networkSettingsUpdateRequest, networkSettingsUpdateSuccess, reloadWalletRequested, types } from '../../src/actions';
import AsyncStorage from '@react-native-async-storage/async-storage';
import { networkSettingsKeyMap } from '../../src/constants';
import { STORE } from '../../src/store';
Expand Down Expand Up @@ -46,63 +46,63 @@ describe('updateNetworkSettings', () => {
const task = middleware.run(defaultSaga);

Promise.resolve()
.then(() => store.dispatch(networkSettingsUpdate(null)))
.then(() => store.dispatch(networkSettingsUpdate(undefined)))
.then(() => store.dispatch(networkSettingsUpdate({})))
.then(() => store.dispatch(networkSettingsUpdate({ explorerUrl: undefined })))
.then(() => store.dispatch(networkSettingsUpdate({ explorerUrl: null })))
.then(() => store.dispatch(networkSettingsUpdate({ explorerUrl: '' })))
.then(() => store.dispatch(networkSettingsUpdate({ explorerUrl: 1 })))
.then(() => store.dispatch(networkSettingsUpdate({ explorerUrl: 'invalid.url.com' })))
.then(() => store.dispatch(networkSettingsUpdateRequest(null)))
.then(() => store.dispatch(networkSettingsUpdateRequest(undefined)))
.then(() => store.dispatch(networkSettingsUpdateRequest({})))
.then(() => store.dispatch(networkSettingsUpdateRequest({ explorerUrl: undefined })))
.then(() => store.dispatch(networkSettingsUpdateRequest({ explorerUrl: null })))
.then(() => store.dispatch(networkSettingsUpdateRequest({ explorerUrl: '' })))
.then(() => store.dispatch(networkSettingsUpdateRequest({ explorerUrl: 1 })))
.then(() => store.dispatch(networkSettingsUpdateRequest({ explorerUrl: 'invalid.url.com' })))
// explorerUrl is valid, however it must have at least nodeUrl
.then(() => store.dispatch(networkSettingsUpdate({ explorerUrl: 'http://localhost:8081/' })))
.then(() => store.dispatch(networkSettingsUpdateRequest({ explorerUrl: 'http://localhost:8081/' })))
// explorerUrl is valid, but explorerServiceUrl is empty
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: '',
})))
// explorerUrl is valid, but explorerServiceUrl is invalid
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'invalid.url.com',
})))
// explorer urls are valid, but nodeUrl is empty
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: '',
})))
// explorer urls are valid, but nodeUrl is invalid
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'invalid.url.com'
})))
// explorer and node urls are valid, but waletServiceUrl is invalid
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'http://localhost:3000/',
walletServiceUrl: 'invalid.url.com'
})))
// explorer, node, and wallet service urls are valid, but walletServiceWsUrl is empty
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'http://localhost:3000/',
walletServiceUrl: 'http://localhost:8083/',
walletServiceWsUrl: ''
})))
// explorer, node, and wallet service urls are valid, but walletServiceWsUrl is invalid
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'http://localhost:3000/',
walletServiceUrl: 'http://localhost:8083/',
walletServiceWsUrl: 'invalid.url.com'
})))
// all urls are valid, except nodeUrl
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'invalid.url.com',
Expand Down Expand Up @@ -171,36 +171,36 @@ describe('updateNetworkSettings', () => {

Promise.resolve()
// calls getFullnodeNetwork
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'http://localhost:3000/',
})))
// calls getWalletServiceNetwork
// it will fail because is lacking the walletServiceWsUrl
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'http://localhost:3000/',
walletServiceUrl: 'http://localhost:8080/'
})))
// calls getWalletServiceNetwork
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'http://localhost:3000/',
walletServiceUrl: 'http://localhost:8080/',
walletServiceWsUrl: 'ws://ws.localhost:4040/'
})))
// calls getFullnodeNetwork
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'http://localhost:3000/',
})))
// calls getFullnodeNetwork
// here the getFullnodeNetwork rejects throwing an error
.then(() => store.dispatch(networkSettingsUpdate({
.then(() => store.dispatch(networkSettingsUpdateRequest({
explorerUrl: 'http://localhost:8081/',
explorerServiceUrl: 'http://localhost:8082/',
nodeUrl: 'http://localhost:3000/',
Expand Down
69 changes: 57 additions & 12 deletions src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,22 @@ export const types = {
// NOTE: These actions follows a taxonomy that should be applied
// to all other actions.
// See: https://github.com/HathorNetwork/hathor-wallet-mobile/issues/334
NETWORKSETTINGS_UPDATE: 'NETWORK_SETTINGS_UPDATE',
/* It delivers the user's network settings input from the form. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/* It delivers the user's network settings input from the form. */
/* It initiates an update of the network settings based on user input from a form. */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved: d994fdb

NETWORKSETTINGS_UPDATE_REQUEST: 'NETWORK_SETTINGS_UPDATE_REQUEST',
/* It updates the redux state */
NETWORKSETTINGS_UPDATE_STATE: 'NETWORKSETTINGS_UPDATE_STATE',
/* It persists the complete structure of network settings in the app storage and updates the redux store. */
NETWORKSETTINGS_PERSIST_STORE: 'NETWORKSETTINGS_PERSIST_STORE',
/* It indicates the persistence is complete and the wallet will be reloaded. */
NETWORKSETTINGS_UPDATE_WAITING: 'NETWORKSETTINGS_UPDATE_WAITING',
/* It indicates the update is complete after wallet reloads. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/* It indicates the update is complete after wallet reloads. */
/* It indicates the update is complete after the wallet reloads. */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved: d994fdb

NETWORKSETTINGS_UPDATE_SUCCESS: 'NETWORK_SETTINGS_UPDATE_SUCCESS',
NETWORKSETTINGS_UPDATE_READY: 'NETWORK_SETTINGS_UPDATE_READY',
/* It indicates the update request has invalid inputs. */
NETWORKSETTINGS_UPDATE_INVALID: 'NETWORKSETTINGS_UPDATE_INVALID',
/* It indicates the update request has failed. */
NETWORKSETTINGS_UPDATE_FAILURE: 'NETWORK_SETTINGS_UPDATE_FAILURE',
NETWORKSETTINGS_UPDATE_ERRORS: 'NETWORK_SETTINGS_UPDATE_ERRORS',
/* It updates the redux state of network settings status */
NETWORKSETTINGS_UPDATE_READY: 'NETWORK_SETTINGS_UPDATE_READY',
};

export const featureToggleInitialized = () => ({
Expand Down Expand Up @@ -861,13 +872,13 @@ export const setWCConnectionFailed = (failed) => ({
* walletServiceWsUrl?: string
* }} customNetworkRequest Request input
*/
export const networkSettingsUpdate = (customNetworkRequest) => ({
type: types.NETWORKSETTINGS_UPDATE,
export const networkSettingsUpdateRequest = (customNetworkRequest) => ({
type: types.NETWORKSETTINGS_UPDATE_REQUEST,
payload: customNetworkRequest,
});

/**
* Emits the custom network settings to be stored and persisted.
* Emits the custom network settings to update the redux store.
* @param {{
* stage: string,
* network: string,
Expand All @@ -878,22 +889,56 @@ export const networkSettingsUpdate = (customNetworkRequest) => ({
* walletServiceWsUrl?: string
* }} customNetwork Settings to persist
*/
export const networkSettingsUpdateSuccess = (customNetwork) => ({
type: types.NETWORKSETTINGS_UPDATE_SUCCESS,
export const networkSettingsUpdateState = (customNetwork) => ({
type: types.NETWORKSETTINGS_UPDATE_STATE,
payload: customNetwork,
});

/**
* Emits the custom network settings to persist in the app storage and update the redux store.
* @param {{
* stage: string,
* network: string,
* nodeUrl: string,
* explorerUrl: string,
* explorerServiceUrl: string,
* walletServiceUrl?: string
* walletServiceWsUrl?: string
* }} customNetwork Settings to persist
*/
export const networkSettingsPersistStore = (customNetwork) => ({
type: types.NETWORKSETTINGS_PERSIST_STORE,
payload: customNetwork,
});

/**
* Emits the waiting signal after persist the custom network.
* It means the wallet will reload.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Emits the waiting signal after persist the custom network.
* It means the wallet will reload.
* Action indicating that the network settings update process is in a waiting state.
* This is used after persisting custom network configurations, resulting in a wallet reload.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved: d994fdb

*/
export const networkSettingsUpdateWaiting = () => ({
type: types.NETWORKSETTINGS_UPDATE_WAITING,
});

/**
* Emits the success signal after wallet reloads.
* It servers as hook for the frontend to provide feedback for the user.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Emits the success signal after wallet reloads.
* It servers as hook for the frontend to provide feedback for the user.
* Action indicating that the network settings update was successful
* This serves as a hook for the frontend to provide feedback to the user.

Copy link
Contributor

Choose a reason for hiding this comment

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

The idea here (and in the previous comments) is that the action by itself does not emit anything, somewhere else is dispatching this action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved: d994fdb

*/
export const networkSettingsUpdateSuccess = () => ({
type: types.NETWORKSETTINGS_UPDATE_SUCCESS,
});

/**
* Emits the failure signal for custom network settings request.
* It means the request couldn't be processed due to internal error.
* It serves as hook for the frontend to provide feedback for the user.
*/
export const networkSettingsUpdateFailure = () => ({
type: types.NETWORKSETTINGS_UPDATE_FAILURE,
});

/**
* Emits errors signal for custom network settings form representing
* invalid inputs.
* Emits invalid signal for custom network settings request inputs.
* It means the form should present the invalid message on the corresponding inputs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Emits invalid signal for custom network settings request inputs.
* It means the form should present the invalid message on the corresponding inputs.
* Action indicating an invalid state for the custom network settings request inputs.
* Intended to trigger the presentation of invalid messages on the corresponding form inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved: d994fdb

* @param {{
* message: string,
* nodeUrl: string,
Expand All @@ -903,8 +948,8 @@ export const networkSettingsUpdateFailure = () => ({
* walletServiceWsUrl?: string
* }} errors The validation errors from custom network settings form
*/
export const networkSettingsUpdateErrors = (errors) => ({
type: types.NETWORKSETTINGS_UPDATE_ERRORS,
export const networkSettingsUpdateInvalid = (errors) => ({
type: types.NETWORKSETTINGS_UPDATE_INVALID,
payload: errors,
});

Expand Down
42 changes: 31 additions & 11 deletions src/components/NetworkSettings/NetworkStatusBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,46 @@
* LICENSE file in the root directory of this source tree.
*/
import { useSelector } from 'react-redux';
import { eq } from 'lodash';
import { isEqual } from 'lodash';
import { t } from 'ttag';
import { AlertUI } from '../../styles/themes';
import { ToplineBar } from '../ToplineBar';
import { PRE_SETTINGS_MAINNET } from '../../constants';

const customNetworkText = t`Custom network`;

export const NetworkStatusBar = () => {
const networkSettings = useSelector((state) => state.networkSettings);
if (eq(networkSettings, PRE_SETTINGS_MAINNET)) {
return null;
function notMainnet(networkSettings) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this method deserves a comment explaining why you split the validation in two.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added: f31c263

if (networkSettings.walletServiceUrl) {
return !isEqual(networkSettings, PRE_SETTINGS_MAINNET);
}

const style = {
backgroundColor: AlertUI.primaryColor,
color: AlertUI.dark40Color,
const currNetwork = {
stage: networkSettings.stage,
network: networkSettings.network,
nodeUrl: networkSettings.nodeUrl,
explorerUrl: networkSettings.explorerUrl,
explorerServiceUrl: networkSettings.explorerServiceUrl,
};
const mainnet = {
stage: PRE_SETTINGS_MAINNET.stage,
network: PRE_SETTINGS_MAINNET.network,
nodeUrl: PRE_SETTINGS_MAINNET.nodeUrl,
explorerUrl: PRE_SETTINGS_MAINNET.explorerUrl,
explorerServiceUrl: PRE_SETTINGS_MAINNET.explorerServiceUrl,
};
const text = `${customNetworkText}: ${networkSettings.network}`;
return (
<ToplineBar style={style} text={text} />
return !isEqual(currNetwork, mainnet);
}

const style = {
backgroundColor: AlertUI.primaryColor,
color: AlertUI.dark40Color,
};

export const NetworkStatusBar = () => {
const getStatusText = (networkSettings) => `${customNetworkText}: ${networkSettings.network}`;
const networkSettings = useSelector((state) => state.networkSettings);

return notMainnet(networkSettings) && (
<ToplineBar style={style} text={getStatusText(networkSettings)} />
);
};
2 changes: 2 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ export const NETWORKSETTINGS_STATUS = {
READY: 'ready',
FAILED: 'failed',
LOADING: 'loading',
WAITING: 'waiting',
SUCCESSFUL: 'successful',
};

/**
Expand Down
Loading
Loading