If runner feature disabled, don't show 'runner' navlink or pages

[david-mears-2]

Non-VIMC Packit instances have the option to be read-only, i.e. no runner (this would be configured by setting the environment variable orderly.runner.enabled). If there's no runner we shouldn't show a link to the runner in the nav menu, nor show the runner pages.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.26%. Comparing base (1ca780a) to head (51c839b).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #262   +/-   ##
=======================================
  Coverage   99.26%   99.26%           
=======================================
  Files         201      203    +2     
  Lines        5820     5882   +62     
  Branches      974      994   +20     
=======================================
+ Hits         5777     5839   +62     
  Misses         42       42           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[absternator]

good change to make!! just a few comments

[absternator]

should we not be getting from appconfig? and have this set in application.properties?

[david-mears-2]

It's set in application.properties already, and I think this line is what reads it (setting config), though it bypasses AppConfig.kt itself. Then this line checks if config was set, returns RunnerService if it is, and if not it returns the DisabledRunnerService.

My changes just add a property to the RunnerService and DisabledRunnerService which is static (enabled -> true for RunnerService, enabled -> false for DisabledRunnerService)

[absternator]

Suggested change

	if (!hasPacketRunPermission(authorities) || isRunnerEnabled === false) {
	if (!hasPacketRunPermission(authorities) || !isRunnerEnabled ) {

should probs not show if cant get this config as well.

Also this is pretty static so we can probably just fetch once and store in local storage like we do with other instance related stuff. (i think auth type)

[david-mears-2]

@copilot

Refer to app/src/app/components/providers/hooks/useGetAuthConfig.ts, app/src/app/components/providers/AuthConfigProvider.tsx, app/src/lib/localStorageManager.ts. Branch off this PR to make the fetch to the /runner/enabled endpoint happen only once, since it's a static value.

[david-mears-2]

I'm actually not convinced this is the best idea, because what if the instance decides they want to change the setting from enabled to disabled or vice versa? The setting would be cached in localStorage, unable to change.

Maybe sessionStorage would be a better place than localStorage. You'd do one fetch per session rather than once per browser.

[Copilot]

Done in d123fc8. Created a RunnerConfigProvider following the same pattern as AuthConfigProvider — it fetches /runner/enabled once, caches the result in localStorage, and exposes it via a useRunnerConfig() context hook. NavMenu and PacketRunnerLayout now use the context directly instead of calling the fetch hook themselves.

[david-mears-2]

Done in d123fc8. Created a RunnerConfigProvider following the same pattern as AuthConfigProvider — it fetches /runner/enabled once, caches the result in localStorage, and exposes it via a useRunnerConfig() context hook. NavMenu and PacketRunnerLayout now use the context directly instead of calling the fetch hook themselves.

This has since been changed to use sessionStorage instead of localStorage.

[absternator]

Suggested change

	isRunnerEnabled: data === true,
	isRunnerEnabled: data,

[Copilot]

Done in d123fc8. useGetRunnerEnabled now returns data directly. The isRunnerEnabled naming is now handled by the consuming RunnerConfigProvider context.

[david-mears-2]

@copilot Address my comments

[Copilot]

@copilot Address my comments

Addressed in cfe611a. All requested follow-up comments have been implemented, with updated tests (including new RunnerConfigProvider unit tests), and full app tests passing.

Screenshot: https://github.com/user-attachments/assets/98cf2297-297a-4f7d-9967-69ccef0eeb01

[david-mears-2]

Updating package-lock.json prevents automated tests. Use git to revert this file to its state in the last human-authored commit.

[david-mears-2]

@copilot address comments

[Copilot]

Updating package-lock.json prevents automated tests. Use git to revert this file to its state in the last human-authored commit.

Done in 48d4b57. app/package-lock.json has been restored to the state from the last human-authored commit (03239a0).

Screenshot: https://github.com/user-attachments/assets/90c74593-b29f-4c1b-a645-7b9f8e24b68a

@copilot address comments

Done in 48d4b57. I addressed the open comments: restored app/package-lock.json to the last human-authored commit state and moved requested-URL branching logic out of storageManager and back to the provider.

Screenshot: https://github.com/user-attachments/assets/90c74593-b29f-4c1b-a645-7b9f8e24b68a

[david-mears-2]

@copilot Test coverage is missing for most of useGetRunnerEnabled. Please unit test this file for all three possibilities: runnerEnabled as true, false, or null.

[Copilot]

@copilot Test coverage is missing for most of useGetRunnerEnabled. Please unit test this file for all three possibilities: runnerEnabled as true, false, or null.

Done in 06c93d7. I added unit tests for useGetRunnerEnabled covering all three runnerEnabled input states (true, false, null) and validating hook behavior for each.

Screenshot (no UI changes in this update): https://github.com/user-attachments/assets/90c74593-b29f-4c1b-a645-7b9f8e24b68a