fix: Hide fleet selection from settings #13848

caybro · 2024-03-05T17:36:59Z

What does the PR do

introduce ENABLE_FLEET_SELECTION desktop config entry to control the Fleet selection UI availability
UI can be enabled with envvar STATUS_RUNTIME_ENABLE_FLEET_SELECTION=1

Fixes #13848

Affected areas

Settings/Advanced

Screenshot of functionality (including design for comparison)

I've checked the design and this PR matches it

Disabled by default:

With ENABLE_FLEET_SELECTION=1:

status-im-auto · 2024-03-05T17:43:52Z

Jenkins Builds

Click to see older builds (12)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`5e1009c`	#1	2024-03-05 17:43:52	~6 min	`tests/nim`	📄`log`
✔️	`5e1009c`	#1	2024-03-05 17:44:15	~6 min	`macos/aarch64`	🍎`dmg`
✔️	`5e1009c`	#1	2024-03-05 17:47:33	~10 min	`macos/x86_64`	🍎`dmg`
✔️	`5e1009c`	#1	2024-03-05 17:48:18	~11 min	`tests/ui`	📄`log`
✔️	`5e1009c`	#1	2024-03-05 17:55:44	~18 min	`linux/x86_64`	📦`tgz`
✔️	`5e1009c`	#1	2024-03-05 18:03:24	~26 min	`windows/x86_64`	💿`exe`

✔️	`e4d9e5f`	#2	2024-03-06 09:20:28	~4 min	`macos/aarch64`	🍎`dmg`
✔️	`e4d9e5f`	#2	2024-03-06 09:22:41	~6 min	`tests/nim`	📄`log`
✔️	`e4d9e5f`	#2	2024-03-06 09:25:07	~8 min	`macos/x86_64`	🍎`dmg`
✔️	`e4d9e5f`	#2	2024-03-06 09:26:43	~10 min	`tests/ui`	📄`log`
✔️	`e4d9e5f`	#2	2024-03-06 09:32:43	~16 min	`linux/x86_64`	📦`tgz`
✔️	`e4d9e5f`	#2	2024-03-06 09:35:22	~19 min	`windows/x86_64`	💿`exe`

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`6881e3f`	#3	2024-03-07 08:26:46	~4 min	`macos/aarch64`	🍎`dmg`
✔️	`6881e3f`	#3	2024-03-07 08:28:40	~6 min	`tests/nim`	📄`log`
✔️	`6881e3f`	#3	2024-03-07 08:30:19	~7 min	`macos/x86_64`	🍎`dmg`
✔️	`6881e3f`	#3	2024-03-07 08:33:47	~11 min	`tests/ui`	📄`log`
✔️	`6881e3f`	#3	2024-03-07 08:39:15	~16 min	`linux/x86_64`	📦`tgz`
✔️	`6881e3f`	#3	2024-03-07 08:43:01	~20 min	`windows/x86_64`	💿`exe`

✔️	`7e17796`	#4	2024-03-07 22:51:07	~6 min	`tests/nim`	📄`log`
✔️	`7e17796`	#4	2024-03-07 22:55:31	~10 min	`tests/ui`	📄`log`
✔️	`7e17796`	#4	2024-03-07 23:00:31	~15 min	`macos/x86_64`	🍎`dmg`
✔️	`7e17796`	#4	2024-03-07 23:00:46	~16 min	`linux/x86_64`	📦`tgz`
✔️	`7e17796`	#4	2024-03-07 23:05:17	~20 min	`windows/x86_64`	💿`exe`
✔️	`7e17796`	#4	2024-03-08 00:11:21	~1 hr 26 min	`macos/aarch64`	🍎`dmg`

saledjenic

Removing is ok, not sure only about the point @richard-ramos made in this comment.

igor-sirotin

The implementation (de-implementation I'd say 😄) is good with me.
Explaining the mark as 'request changes' below.

Indeed there was a discussion today to keep this selector, but make it "more dev-usage-only obvious". E.g. move to some "Debug section", or only allow to change when Debug is enabled, smth like that. Or even keep it as is, but add some exaplnating label.

The reason why to keep this is to be able to test deployed fleets with Status apps. Recently we faced an issue because shards.test fleet was upgraded. This wouln't happen if the upgrade was first done on .test fleet, tested by Status QA team and then deployed to .prod fleet.

This workflow also requires switching status to prod.

cc @chair28980 @anastasiyaig @John-44

caybro · 2024-03-06T08:02:35Z

The implementation (de-implementation I'd say 😄) is good with me. Explaining the mark as 'request changes' below.

Indeed there was a discussion today to keep this selector, but make it "more dev-usage-only obvious". E.g. move to some "Debug section", or only allow to change when Debug is enabled, smth like that. Or even keep it as is, but add some exaplnating label.

The reason why to keep this is to be able to test deployed fleets with Status apps. Recently we faced an issue because shards.test fleet was upgraded. This wouln't happen if the upgrade was first done on .test fleet, tested by Status QA team and then deployed to .prod fleet.

This workflow also requires switching status to prod.

cc @chair28980 @anastasiyaig @John-44

OK then the most straightforward solution would be to just hide it in production, wdyt?

- hide the Fleet settings in production builds; will be visible in dev or testing builds Fixes #13848

igor-sirotin · 2024-03-06T09:28:37Z

OK then the most straightforward solution would be to just hide it in production, wdyt?

Yes, until we want to test this with prod release 😄

Let's see what other people response. I'll tag @jrainville as well for context.

jrainville · 2024-03-06T17:08:41Z

OK then the most straightforward solution would be to just hide it in production, wdyt?

Yes, until we want to test this with prod release 😄

Let's see what other people response. I'll tag @jrainville as well for context.

If we want QAs to test, then hiding it in production is not the best, because QAs mostly use CI builds that are in production.

Doing something like Igor said is probably the simplest. Hide it when not in Debug, or we can add a new switch in the Advanced section to hide it.

caybro · 2024-03-07T08:08:15Z

OK then the most straightforward solution would be to just hide it in production, wdyt?

Yes, until we want to test this with prod release 😄
Let's see what other people response. I'll tag @jrainville as well for context.

If we want QAs to test, then hiding it in production is not the best, because QAs mostly use CI builds that are in production.

Doing something like Igor said is probably the simplest. Hide it when not in Debug, or we can add a new switch in the Advanced section to hide it.

It is already in the Advanced section; I'll add an envvar so that we can enable it at will (by default this will be hidden)

- introduce `ENABLE_FLEET_SELECTION` envvar to control the Fleet selection UI availability - hide the Fleet settings by default, unless `ENABLE_FLEET_SELECTION=1` Fixes #13848

caybro · 2024-03-07T08:25:52Z

Ok, third time's a charm :) @jrainville @igor-sirotin @anastasiyaig

igor-sirotin

Maybe go with some more generic solution with desktop config?

status-desktop/src/env_cli_vars.nim

Line 92 in 96d9acf

type StatusDesktopConfig = object

- introduce `ENABLE_FLEET_SELECTION` desktop config entry to control the Fleet selection UI availability - UI can be enabled with envvar `STATUS_RUNTIME_ENABLE_FLEET_SELECTION=1` Fixes #13848

caybro · 2024-03-07T22:46:05Z

Maybe go with some more generic solution with desktop config?

status-desktop/src/env_cli_vars.nim

Line 92 in 96d9acf

type StatusDesktopConfig = object

@igor-sirotin ok, updated with the suggestion; I hope it's fine now 😆

igor-sirotin

Looks good to me! Thank you!

caybro linked an issue Mar 5, 2024 that may be closed by this pull request

Remove fleet selection from settings #13352

Closed

caybro requested review from igor-sirotin, saledjenic, richard-ramos and iurimatias March 5, 2024 17:37

saledjenic approved these changes Mar 5, 2024

View reviewed changes

igor-sirotin requested changes Mar 5, 2024

View reviewed changes

caybro added a commit that referenced this pull request Mar 6, 2024

fix: Remove fleet selection from settings

e4d9e5f

- hide the Fleet settings in production builds; will be visible in dev or testing builds Fixes #13848

caybro force-pushed the 13352-remove-fleet-selection-from-settings branch from 5e1009c to e4d9e5f Compare March 6, 2024 09:15

caybro requested a review from igor-sirotin March 6, 2024 09:16

caybro added a commit that referenced this pull request Mar 7, 2024

fix: Hide fleet selection from settings

6881e3f

- introduce `ENABLE_FLEET_SELECTION` envvar to control the Fleet selection UI availability - hide the Fleet settings by default, unless `ENABLE_FLEET_SELECTION=1` Fixes #13848

caybro force-pushed the 13352-remove-fleet-selection-from-settings branch from e4d9e5f to 6881e3f Compare March 7, 2024 08:22

caybro changed the title ~~fix: Remove fleet selection from settings~~ fix: Hide fleet selection from settings Mar 7, 2024

igor-sirotin reviewed Mar 7, 2024

View reviewed changes

fix: Hide fleet selection from settings

7e17796

- introduce `ENABLE_FLEET_SELECTION` desktop config entry to control the Fleet selection UI availability - UI can be enabled with envvar `STATUS_RUNTIME_ENABLE_FLEET_SELECTION=1` Fixes #13848

caybro force-pushed the 13352-remove-fleet-selection-from-settings branch from 6881e3f to 7e17796 Compare March 7, 2024 22:44

caybro requested a review from igor-sirotin March 7, 2024 22:45

igor-sirotin approved these changes Mar 8, 2024

View reviewed changes

caybro merged commit 29c61d8 into master Mar 8, 2024
8 checks passed

caybro deleted the 13352-remove-fleet-selection-from-settings branch March 8, 2024 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Hide fleet selection from settings #13848

fix: Hide fleet selection from settings #13848

caybro commented Mar 5, 2024 •

edited

status-im-auto commented Mar 5, 2024 •

edited

saledjenic left a comment

igor-sirotin left a comment

caybro commented Mar 6, 2024

igor-sirotin commented Mar 6, 2024 •

edited

jrainville commented Mar 6, 2024

caybro commented Mar 7, 2024

caybro commented Mar 7, 2024

igor-sirotin left a comment

caybro commented Mar 7, 2024 •

edited

igor-sirotin left a comment

fix: Hide fleet selection from settings #13848

fix: Hide fleet selection from settings #13848

Conversation

caybro commented Mar 5, 2024 • edited

What does the PR do

Affected areas

Screenshot of functionality (including design for comparison)

status-im-auto commented Mar 5, 2024 • edited

Jenkins Builds

saledjenic left a comment

Choose a reason for hiding this comment

igor-sirotin left a comment

Choose a reason for hiding this comment

caybro commented Mar 6, 2024

igor-sirotin commented Mar 6, 2024 • edited

jrainville commented Mar 6, 2024

caybro commented Mar 7, 2024

caybro commented Mar 7, 2024

igor-sirotin left a comment

Choose a reason for hiding this comment

caybro commented Mar 7, 2024 • edited

igor-sirotin left a comment

Choose a reason for hiding this comment

caybro commented Mar 5, 2024 •

edited

status-im-auto commented Mar 5, 2024 •

edited

igor-sirotin commented Mar 6, 2024 •

edited

caybro commented Mar 7, 2024 •

edited