Disable remote execution tab if deployment is playground only

[sayomaki]

Description

Resolves #2224, allow disabling of the remote execution tab in playground configurable through the env file, under the newly added boolean value REACT_APP_ENABLE_REMOTE_EXECUTION.

The default value configured is FALSE (in the .env.example file)

The remote execution tab will only show when the REACT_APP_PLAYGROUND_ONLY is set to false (i.e. deployment with a backend), instead of the current behavior where it shows regardless of the deployment type.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Code quality improvements

How to test

Pull and run the changes locally, and by default the remote execution tab should not be accessible anymore in the playground.

Code is tested working on latest version of Chrome 123.

Checklist

• I have tested this code
• I have updated the documentation

[coveralls]

Pull Request Test Coverage Report for Build 7725779172

• 0 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.006%) to 37.184%

---

Totals
Change from base Build 7723051241:	0.006%
Covered Lines:	5722
Relevant Lines:	14452

---

💛 - Coveralls

[RichDom2185]

Hi, thanks for working on this. However, I don't think this is the correct approach.

I feel it makes more sense to just remove it entirely from the public, backendless deployment. Since remote execution involves the backend, I don't think it should be toggleable via an environment variable in the first place.

@chownces can I get your thoughts on this?

---

On a slightly related note, there's a login state check in the remote execution tab:

frontend/src/commons/sideContent/remoteExecution/SideContentRemoteExecution.tsx

Lines 137 to 143 in 199354c

	if (!isLoggedIn) {
	return (
	<Callout intent="danger">
	Please <NavLink to="/login">log in</NavLink> to execute on a remote device.
	</Callout>
	);
	}

In my view, this is a form of coupling as the component needs to be aware of the hardcoded /login route (which was the cause of the issue in the first place – since there's no such route in the public deployment).

Some refactoring would be needed to improve the separation of concerns between these two. Perhaps the login check can be done in the playground instead. I don't think a guard clause works well in the context of this component, but I'm also open to discussing more about this. What are your thoughts?

[sayomaki]

Since remote execution involves the backend,

In that case, do you think it is more appropriate to just check the USE_BACKEND flag instead? I think that will be a better approach as it ensures that the login route will be available, and not required for a separate check for the playground.

As far as I know, playground is only available after login in a backend-enabled deployment, otherwise it will redirect to the login page automatically.

If your concern is that the login check for the remote execution will no longer be applicable anymore due to the checks aforementioned, I can proceed to remove it as well.

[chownces]

Hi, thanks for working on this. However, I don't think this is the correct approach.

I feel it makes more sense to just remove it entirely from the public, backendless deployment. Since remote execution involves the backend, I don't think it should be toggleable via an environment variable in the first place.

@chownces can I get your thoughts on this?

Hey I agree on this, we shouldn't be using more and more variables when infact, the presence of this tab is solely dependent on the type of FE we are deploying (e.g. Playground-Only, SA with BE).

Another note, REACT_APP_USE_BACKEND is slightly misleading now that we have many more configurations in SA. It used to denote to use use the MockSagas (for local dev) or the actual BackendSagas that ping the BE API. REACT_APP_PLAYGROUND_ONLY might be the one you are looking for.

Perhaps, we should refactor the Constants.ts file too and clean up the naming. Nesting the constants object would also make it clearer which constants deal with which things (e.g. feature toggles, URLs, FE configuration type, workspace defaults, etc.)

[sayomaki]

Alright, I have updated the check to use the REACT_APP_PLAYGROUND_ONLY environment variable instead.

Perhaps, we should refactor the Constants.ts file too and clean up the naming. Nesting the constants object would also make it clearer which constants deal with which things (e.g. feature toggles, URLs, FE configuration type, workspace defaults, etc.)

This could be a good idea for another issue/PR, but I think is out of the scope for this PR. We will definitely still need to confirm on list of names to use, and work out the details of the refactor.

[RichDom2185]

LGTM, thanks; I'll wait for @chownces to give his review.

Do you think we need to remove the "please log in to execute on a remote device" banner? Since it won't be shown anymore.

[chownces]

LGTM. Will wait for the refactor in another PR.