-
Notifications
You must be signed in to change notification settings - Fork 984
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
Add 'Test Networks Enabled' option in settings #17879
Conversation
05abe42
to
6bf782f
Compare
@@ -242,6 +242,7 @@ | |||
|
|||
(defn- internal-discover-view | |||
[params] | |||
(rf/dispatch [:fetch-contract-communities]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dispatch is not related to the Test Networks Enabled option.
Just added back the curated communities fetch event dispatch that got removed in a previous PR.
Jenkins BuildsClick to see older builds (48)
|
@@ -56,6 +57,10 @@ | |||
{:profile.settings/webview-debug-changed value} | |||
(profile-update :webview-debug (boolean value) {}))) | |||
|
|||
(rf/reg-event-fx :profile.settings/change-test-networks-enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to add a simple test here? 🙏
@briansztamfater, @vkjr, @OmarBasem I believe we can use this for wallet test-net being enabled. Also @ajayesivan - I believe there is some corresponding UI for this where there is an orange banner at the top of the application. Should we add this here or in a follow up? |
@@ -56,6 +57,10 @@ | |||
{:profile.settings/webview-debug-changed value} | |||
(profile-update :webview-debug (boolean value) {}))) | |||
|
|||
(rf/reg-event-fx :profile.settings/change-test-networks-enabled | |||
(fn [_ [value]] | |||
{:fx [[:dispatch [:profile.settings/profile-update :test-networks-enabled? (boolean value) {}]]]})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see much of a need for this new event if it's just re-dispatching and passing value
converted to a boolean.
We can create a more useful event, like :profile.settings/toggle-test-networks
and have it read from the app db itself. So the view wouldn't even need to pass value
, since that's already in the global state.
Or just consume :profile.settings/profile-update
directly in the view.
modified src/status_im/ui/screens/advanced_settings/views.cljs
@@ -101,8 +101,7 @@
:accessibility-label :test-networks-enabled
:container-margin-bottom 8
:on-press
- #(re-frame/dispatch
- [:profile.settings/change-test-networks-enabled (not test-networks-enabled?)])
+ #(re-frame/dispatch [:profile.settings/toggle-test-networks])
:accessory :switch
:active test-networks-enabled?}
{:size :small
modified src/status_im2/contexts/profile/settings/events.cljs
@@ -61,6 +61,11 @@
(fn [_ [value]]
{:fx [[:dispatch [:profile.settings/profile-update :test-networks-enabled? (boolean value) {}]]]}))
+(rf/reg-event-fx :profile.settings/toggle-test-networks
+ (fn [{:keys [db]}]
+ (let [value (get-in db [:profile/profile :test-networks-enabled?])]
+ {:fx [[:dispatch [:profile.settings/profile-update :test-networks-enabled? (not value) {}]]]})))
@ajayesivan, why don't we make logout automatic here? |
443ae8b
to
29d99f0
Compare
b62f6b6
to
4e29983
Compare
81a9fc1
to
f5ec2ac
Compare
@J-Son89 On desktop they are not logging out. I mentioned it as a test note to confirm the toggle is working properly. |
okay that's fine for now - Desktop has setup it up that the login initialisation calls are repeated (at least for wallet) so that the tokens etc are on testnet. It might be good for us even to prompt/alert the user that they should logout/login again to avoid confusion among ourselves |
f5ec2ac
to
bb5c080
Compare
bb5c080
to
a26358c
Compare
@ajayesivan Is this an issue or a temporary behavior until the wallet setting screen, as mentioned by @J-Son89 in this comment, gets implemented? |
ISSUE 1: ETH assets unavailable when test mode activated with mainnet or deactivated with goerli network enabled This pr just adds the testnet network toggle that will make that change in Status-Go. There are a few minor adjustments we then need to make in Wallet once we have this pr merged so we will handle that in a separate issue/pr which someone from the wallet team can take 👍 |
@ajayesivan, you mentioned in this comment that 'This option is required to test curated communities in status.test fleet.' when I visit https://community-dapp-git-review-status-im-web.vercel.app/votes testnet (Optimism Goerli) to view the 'status.test' fleet communities, I don't see any community listed there. However, one community is fetched in the Status app in the Curated Communities section when the 'test mode' is activated. I'm a bit confused about what kind of community is being fetched into our Status app? Actual result:No communities are shown in the https://community-dapp-git-review-status-im-web.vercel.app/votes but one community is fetched in Status app in discovery page Expected result:If no communities are shown in https://community-dapp-git-review-status-im-web.vercel.app/votes , then no communities are fetched to Curated communities page |
Hi @VolodLytvynenko, Concerning curated communities, to retrieve test network communities, we must enable both this toggle and switch to the |
a26358c
to
c36f316
Compare
hi @ajayesivan . Thank you for the clarification. No additional issues from my side. PR is ready to be merged |
fixes: #17826
Add Test Networks Enabled toggle in Advanced Settings
Test Notes
Note: This option is required to test curated communities in
status.test
fleet. This PR doesn't fix the issue logged here: #17852, additional changes are required to fully fix that.