(15) refactor: network settings actions #360

alexruzenhack · 2023-10-24T03:32:51Z

This PR aims to improve the Network Settings Actions by decomposing it in actions that conveys a better semantic.

Acceptance Criteria

rename action to NETWORKSETTINGS_UPDATE_REQUEST
add action NETWORKSETTINGS_UPDATE_STATE
add action NETWORKSETTINGS_PERSIST_STORE
add action NETWORKSETTINGS_UPDATE_WAITING
refactor action NETWORKSETTINGS_UPDATE_SUCCESS
refactor action NETWORKSETTINGS_UPDATE_INVALID
refactor action NETWORKSETTINGS_UPDATE_FAILURE
add docstring to action NETWORKSETTINGS_UPDATE_READY
refactor NetworkStatusBar
refactor saga effect persistNetworkSettings
resolve all the lint issues

Warning

The following video has a legacy behavior when showing the network status bar alert for mainnet settings when removing only the wallet service URLs fields. This bar should show up only if the required fields diverges from the what is set to be the mainnet. You find the video with the correct behavior after this one.

ds-action-refactoring-clip.mp4

Correct behavior regarding network status bar:

ds-action-refactoring-rv1-clip.mp4

Closes: #358

Security Checklist

Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

- This action replaces the NETWORK_UPDATE_ERRORS

- Improve readability

- change network settings status to `WAITING` just after persist the custom network settings - throw a fatal exception message to the user if the wallet is not found - stop the wallet and clean its storage and addresses - add a reducer for RELOAD_WALLET_REQUESTED action to clean the tokens history and balance on redux store

src/screens/NetworkSettings/CustomNetworkSettingsScreen.js

src/screens/NetworkSettings/helper.js

src/sagas/helpers.js

src/screens/NetworkSettings/CustomNetworkSettingsScreen.js

src/sagas/helpers.js

src/components/NetworkSettings/NetworkStatusBar.js

src/actions.js

src/reducers/reducer.js

src/screens/NetworkSettings/CustomNetworkSettingsScreen.js

src/sagas/networkSettings.js

- The full check will happen when the custom network contain a valid URL for wallet-service. This behavior is to avoid a bypass on the network status bar when the wallet-service is given. This is important because the wallet-service has precedence over fullnode.

pedroferreira1 · 2023-11-01T03:08:09Z

src/components/NetworkSettings/NetworkStatusBar.js

-  const networkSettings = useSelector((state) => state.networkSettings);
-  if (eq(networkSettings, PRE_SETTINGS_MAINNET)) {
-    return null;
+function notMainnet(networkSettings) {


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

Added: f31c263

pedroferreira1 · 2023-11-01T03:08:44Z

src/sagas/networkSettings.js

+  if (status === NETWORKSETTINGS_STATUS.WAITING) {
+    yield put(networkSettingsUpdateSuccess());
+  } else {
+    yield put(networkSettingsUpdateReady());


I believe this deserves a comment explaining these state changes and why they happen here

Added: 3825b2e

andreabadesso · 2023-11-01T02:57:30Z

src/actions.js

@@ -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. */


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. */

Resolved: d994fdb

andreabadesso · 2023-11-01T03:00:10Z

src/actions.js

+  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. */


Suggested change

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

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

Resolved: d994fdb

andreabadesso · 2023-11-01T03:04:34Z

src/actions.js

+ * Emits the waiting signal after persist the custom network.
+ * It means the wallet will reload.


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.

Resolved: d994fdb

andreabadesso · 2023-11-01T03:06:53Z

src/actions.js

+ * Emits the success signal after wallet reloads.
+ * It servers as hook for the frontend to provide feedback for the user.


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.

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

Resolved: d994fdb

andreabadesso · 2023-11-01T03:09:49Z

src/actions.js

+ * Emits invalid signal for custom network settings request inputs.
+ * It means the form should present the invalid message on the corresponding inputs.


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.

Resolved: d994fdb

alexruzenhack self-assigned this Oct 24, 2023

alexruzenhack requested a review from andreabadesso October 24, 2023 03:33

alexruzenhack marked this pull request as ready for review October 24, 2023 18:43

alexruzenhack requested a review from pedroferreira1 as a code owner October 24, 2023 18:43

alexruzenhack changed the title ~~refactor: network settings actions~~ (15) refactor: network settings actions Oct 24, 2023

alexruzenhack force-pushed the feat/ds-alert-ui branch from 05d28ca to 9e2c5c0 Compare October 25, 2023 23:15

alexruzenhack force-pushed the refactor/ds-actions branch from 6e85529 to 9f954f2 Compare October 25, 2023 23:41

Base automatically changed from feat/ds-alert-ui to dev October 27, 2023 13:07

alexruzenhack added 11 commits October 27, 2023 14:11

chore: remove legacy comment

5b16276

refactor: rename action to NETWORKSETTINGS_UPDATE_REQUEST

300e4b4

feat: add action NETWORKSETTINGS_UPDATE_STATE

d14be82

feat: add action NETWORKSETTINGS_PERSIST_STORE

7ffd6eb

feat: add action NETWORKSETTINGS_UPDATE_WAITING

461f1d4

refactor: action NETWORKSETTINGS_UPDATE_SUCCESS

e938aea

refactor: action NETWORKSETTINGS_UPDATE_INVALID

46de5af

- This action replaces the NETWORK_UPDATE_ERRORS

refactor: action NETWORKSETTINGS_UPDATE_FAILURE

03ba63f

chore: add docstring to action NETWORKSETTINGS_UPDATE_READY

61d88a5

refactor: NetworkStatusBar

0352e19

- Improve readability

alexruzenhack force-pushed the refactor/ds-actions branch from 9f954f2 to 4558d11 Compare October 27, 2023 13:12

lint: resolve all the lint issues

02acec2

alexruzenhack force-pushed the refactor/ds-actions branch from 4558d11 to 02acec2 Compare October 27, 2023 13:13

andreabadesso approved these changes Oct 27, 2023

View reviewed changes

src/screens/NetworkSettings/CustomNetworkSettingsScreen.js Outdated Show resolved Hide resolved

src/screens/NetworkSettings/helper.js Outdated Show resolved Hide resolved

review: apply msg suggestions

67fa735

alexruzenhack requested a review from andreabadesso October 30, 2023 15:51

andreabadesso requested changes Oct 31, 2023

View reviewed changes

src/sagas/helpers.js Outdated Show resolved Hide resolved

src/screens/NetworkSettings/CustomNetworkSettingsScreen.js Outdated Show resolved Hide resolved

pedroferreira1 reviewed Oct 31, 2023

View reviewed changes

src/screens/NetworkSettings/CustomNetworkSettingsScreen.js Outdated Show resolved Hide resolved

src/sagas/networkSettings.js Outdated Show resolved Hide resolved

src/sagas/networkSettings.js Show resolved Hide resolved

src/sagas/networkSettings.js Show resolved Hide resolved

alexruzenhack added 2 commits October 31, 2023 20:23

refactor: inline retrieve of network settings

1be1450

fix: typos

0f97f75

alexruzenhack added 3 commits October 31, 2023 20:26

refactor: replace to helper's functions

e0ad2a1

feat: add fallback for the ready status

393eb4b

chore: wrap text in localization mechanism

185670b

alexruzenhack requested review from pedroferreira1 and andreabadesso October 31, 2023 20:47

alexruzenhack added 2 commits October 31, 2023 23:15

feat: ignore wallet service URLs for mainnet check on network status bar

51609ad

pedroferreira1 approved these changes Nov 1, 2023

View reviewed changes

andreabadesso requested changes Nov 1, 2023

View reviewed changes

alexruzenhack added 3 commits November 1, 2023 13:28

chore: add comments explaining the two checks on notMainnet

f31c263

chore: add comment to explain set READY fallback behavior

3825b2e

chore: update docstrings of action methods

d994fdb

andreabadesso approved these changes Nov 1, 2023

View reviewed changes

alexruzenhack merged commit a620076 into dev Nov 1, 2023
2 checks passed

alexruzenhack deleted the refactor/ds-actions branch November 1, 2023 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(15) refactor: network settings actions #360

(15) refactor: network settings actions #360

alexruzenhack commented Oct 24, 2023 •

edited

Loading

pedroferreira1 Nov 1, 2023

alexruzenhack Nov 1, 2023

pedroferreira1 Nov 1, 2023

alexruzenhack Nov 1, 2023

andreabadesso Nov 1, 2023

alexruzenhack Nov 1, 2023

andreabadesso Nov 1, 2023

alexruzenhack Nov 1, 2023

andreabadesso Nov 1, 2023

alexruzenhack Nov 1, 2023

andreabadesso Nov 1, 2023

andreabadesso Nov 1, 2023

alexruzenhack Nov 1, 2023

andreabadesso Nov 1, 2023

alexruzenhack Nov 1, 2023

	/* 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. */

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

		* Emits the waiting signal after persist the custom network.
		* It means the wallet will reload.

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

		* Emits invalid signal for custom network settings request inputs.
		* It means the form should present the invalid message on the corresponding inputs.

(15) refactor: network settings actions #360

(15) refactor: network settings actions #360

Conversation

alexruzenhack commented Oct 24, 2023 • edited Loading

Acceptance Criteria

Security Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexruzenhack commented Oct 24, 2023 •

edited

Loading