feat: set resource class for a session launcher #3196

andre-code · 2024-06-10T09:24:49Z

Description

PR to add a feature for selecting a resource class in a session launcher and prompt users to choose a resource class if they lack access to the default one.

Type of Change

Feat
it requires a new version of renku-data-services

Related Tickets & Documents

Related Issue(s): [ShapeUp] Set resource pool/class for a session launcher 2.0 renku#3694
Closes: [ShapeUp] Set resource pool/class for a session launcher 2.0 #3221

Screenshots

UI Accessibility Checklist

For UI changes, ensure the following:

Use Semantic HTML
Keyboard Navigation, issue created for bug not fixed here: [A11Y] Fix keyboard navigation for adding a new launcher #3222
Color contrast
Lighthouse screenshot
Screen Reader Compatibility Not included
Resizing and Zooming

Testing

Cases to test:

Select a default resource class when add/update a global session environment launcher
Select a default resource class when add/update a custom session environment launcher
Find out which resource class is assigned to a session launcher.
Display a modal to select a resource class when launching a session and not having access to the default session launcher resource class.
Be able to select a resource class other than the default when launching a session.

Added/Updated Unit Tests?

Yes
No

Added/Updated E2E Tests?

Yes
No

Renku Acceptance Tests

Does this involve adding or modifying acceptance tests in Renku Repository? No

Storybook

Not included new stories because the components may change due to ongoing app redesigns.

/deploy

RenkuBot · 2024-06-10T09:25:43Z

You can access the deployment of this PR at https://renku-ci-ui-3196.dev.renku.ch

leafty

Some code comments:

leafty · 2024-06-26T10:47:45Z

client/src/features/sessionsV2/StartSessionButton.tsx

+  const onClickCustomLaunch = useCallback(() => {
+    const customStartUrl = generatePath(
+      ABSOLUTE_ROUTES.v2.projects.show.sessions.startCustom,
+      {
+        launcherId,
+        namespace,
+        slug,
+      }
+    );
+    navigate(customStartUrl);
+  }, [launcherId, namespace, slug, navigate]);


Why is this not a link component? This breaks navigation for keyboard users.

leafty · 2024-06-26T10:48:52Z

client/src/routing/routes.constants.ts

+          startCustom:
+            "/v2/projects/:namespace/:slug/sessions/:launcherId/start?custom",


This is not needed. Add a constant for the query param you can re-use (e.g. in client/src/features/sessionsV2/SessionStartPage.tsx).

leafty · 2024-06-26T10:58:00Z

client/src/features/sessionsV2/SessionStartPage.tsx

+      launcher,
+      project,
+    });
+  const { isPendingResourceClass, setResourceClass } = useSessionResourceClass({


Why is this not part of the useSessionLauncherState hook? It seems more natural to host all state in there.

leafty · 2024-06-26T10:58:27Z

client/src/features/sessionsV2/SessionStartPage.tsx

+  const cancelLaunchSession = useCallback(() => {
+    const url = generatePath(ABSOLUTE_ROUTES.v2.projects.show.root, {
+      namespace: project.namespace,
+      slug: project.slug,
+    });
+    navigate(url);
+  }, [navigate, project]);


Why is this not just a Link component?

leafty · 2024-06-26T11:00:00Z

client/src/features/session/components/options/SessionClassOption.tsx

 import { toHumanDuration } from "../../../../utils/helpers/DurationUtils";
+import { FetchingResourcePools } from "../../../sessionsV2/components/SessionModals/ResourceClassWarning.tsx";


Suggested change

import { FetchingResourcePools } from "../../../sessionsV2/components/SessionModals/ResourceClassWarning.tsx";

import { FetchingResourcePools } from "../../../sessionsV2/components/SessionModals/ResourceClassWarning";

leafty · 2024-06-26T11:02:41Z

client/src/features/sessionsV2/SessionStartPage.tsx

-    resourcePools,
-    startSessionOptionsV2,
-  } = useSessionLauncherState({
+  const hasCustomQuery = new URLSearchParams(location.search).has("custom");


"custom" should probably be defined as a constant. Also, this should use a hook to get the query params:

Suggested change

const hasCustomQuery = new URLSearchParams(location.search).has("custom");

const [searchParams] = useSearchParams(); // from "react-router-dom-v5-compat"

const hasCustomQuery = searchParams.has("custom");

leafty · 2024-06-26T11:05:53Z

client/src/features/sessionsV2/components/SessionModals/ModifyResourcesLauncher.tsx

Note: this form doesn't seem to let users unset the resource class.

leafty

Non-blocking: it seems that the project page uses different icons to indicate "edit".

Only one icon should be used for all these cases.

Also the edit button doesn't feel like it's a button at all, there should be some hover and activation styles to communicate to the user that it is an interactive element in the page.

andre-code · 2024-06-27T10:41:55Z

Thanks @leafty for your suggestions. You can find them implemented in the latest commit. Regarding the icon, I share your opinion, but I will create a separate PR for those changes.

leafty · 2024-06-27T14:17:04Z

client/src/features/session/components/options/SessionClassOption.tsx

  components,
 } from "react-select";
+import { useGetNotebooksVersionQuery } from "../../../versions/versions.api.ts";


Suggested change

import { useGetNotebooksVersionQuery } from "../../../versions/versions.api.ts";

import { useGetNotebooksVersionQuery } from "../../../versions/versions.api";

RenkuBot · 2024-07-01T07:37:44Z

Tearing down the temporary RenkuLab deplyoment for this PR.

andre-code changed the title ~~add modal to select resource class~~ feat: add modal to select resource class Jun 10, 2024

andre-code had a problem deploying to renku-ci-ui-3196 June 10, 2024 09:25 — with GitHub Actions Error

andre-code force-pushed the andrea/resource-pool-when-launch-session branch from e2f4046 to 15e547b Compare June 10, 2024 09:29

andre-code had a problem deploying to renku-ci-ui-3196 June 10, 2024 09:29 — with GitHub Actions Error

andre-code had a problem deploying to renku-ci-ui-3196 June 10, 2024 09:36 — with GitHub Actions Failure

andre-code changed the title ~~feat: add modal to select resource class~~ WIP feat: add modal to select resource class Jun 10, 2024

andre-code had a problem deploying to renku-ci-ui-3196 June 10, 2024 19:38 — with GitHub Actions Error

andre-code had a problem deploying to renku-ci-ui-3196 June 10, 2024 19:39 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 10, 2024 20:26 — with GitHub Actions Failure

andre-code temporarily deployed to renku-ci-ui-3196 June 11, 2024 06:41 — with GitHub Actions Inactive

andre-code force-pushed the andrea/resource-pool-when-launch-session branch 2 times, most recently from 5516411 to e030808 Compare June 14, 2024 11:45

andre-code temporarily deployed to renku-ci-ui-3196 June 14, 2024 11:46 — with GitHub Actions Inactive

andre-code had a problem deploying to renku-ci-ui-3196 June 14, 2024 13:04 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 14, 2024 13:28 — with GitHub Actions Error

andre-code had a problem deploying to renku-ci-ui-3196 June 14, 2024 14:00 — with GitHub Actions Failure

andre-code temporarily deployed to renku-ci-ui-3196 June 14, 2024 14:34 — with GitHub Actions Inactive

andre-code had a problem deploying to renku-ci-ui-3196 June 14, 2024 14:58 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 14, 2024 17:18 — with GitHub Actions Failure

andre-code force-pushed the andrea/resource-pool-when-launch-session branch from e030808 to 3b4097a Compare June 14, 2024 18:04

andre-code had a problem deploying to renku-ci-ui-3196 June 14, 2024 18:04 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 14, 2024 18:41 — with GitHub Actions Failure

andre-code temporarily deployed to renku-ci-ui-3196 June 17, 2024 09:37 — with GitHub Actions Inactive

andre-code had a problem deploying to renku-ci-ui-3196 June 17, 2024 11:26 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 17, 2024 12:40 — with GitHub Actions Failure

andre-code force-pushed the andrea/resource-pool-when-launch-session branch from 94a8b76 to 004c5d1 Compare June 18, 2024 13:36

andre-code had a problem deploying to renku-ci-ui-3196 June 18, 2024 13:36 — with GitHub Actions Error

andre-code force-pushed the andrea/resource-pool-when-launch-session branch from 004c5d1 to 9c95aa6 Compare June 18, 2024 13:38

fix accessibility issues in session view

79155a0

andre-code had a problem deploying to renku-ci-ui-3196 June 25, 2024 14:44 — with GitHub Actions Error

andre-code had a problem deploying to renku-ci-ui-3196 June 25, 2024 15:07 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 25, 2024 18:34 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 25, 2024 19:16 — with GitHub Actions Failure

andre-code temporarily deployed to renku-ci-ui-3196 June 25, 2024 19:46 — with GitHub Actions Inactive

andre-code changed the title ~~WIP feat: add modal to select resource class~~ feat: set resource class for a session launcher Jun 25, 2024

andre-code temporarily deployed to renku-ci-ui-3196 June 25, 2024 20:18 — with GitHub Actions Inactive

andre-code marked this pull request as ready for review June 25, 2024 20:18

andre-code requested a review from a team as a code owner June 25, 2024 20:18

andre-code self-assigned this Jun 25, 2024

andre-code requested review from leafty and lorenzo-cavazzi June 25, 2024 20:18

leafty reviewed Jun 26, 2024

View reviewed changes

leafty reviewed Jun 27, 2024

View reviewed changes

apply code suggestions

5423345

andre-code had a problem deploying to renku-ci-ui-3196 June 27, 2024 10:40 — with GitHub Actions Failure

andre-code requested review from leafty and removed request for lorenzo-cavazzi June 27, 2024 10:53

andre-code temporarily deployed to renku-ci-ui-3196 June 27, 2024 11:36 — with GitHub Actions Inactive

leafty approved these changes Jun 27, 2024

View reviewed changes

andre-code had a problem deploying to renku-ci-ui-3196 June 27, 2024 14:31 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 27, 2024 19:11 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 27, 2024 20:15 — with GitHub Actions Failure

andre-code had a problem deploying to renku-ci-ui-3196 June 28, 2024 08:19 — with GitHub Actions Failure

andre-code deployed to renku-ci-ui-3196 June 28, 2024 08:45 — with GitHub Actions View deployment

andre-code merged commit a79512a into main Jul 1, 2024
31 checks passed

andre-code deleted the andrea/resource-pool-when-launch-session branch July 1, 2024 07:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: set resource class for a session launcher #3196

feat: set resource class for a session launcher #3196

andre-code commented Jun 10, 2024 •

edited

Loading

RenkuBot commented Jun 10, 2024

leafty left a comment

leafty Jun 26, 2024

leafty Jun 26, 2024

leafty Jun 26, 2024

leafty Jun 26, 2024

leafty Jun 26, 2024

leafty Jun 26, 2024

leafty Jun 26, 2024

leafty left a comment

andre-code commented Jun 27, 2024

leafty Jun 27, 2024

RenkuBot commented Jul 1, 2024

		startCustom:
		"/v2/projects/:namespace/:slug/sessions/:launcherId/start?custom",

		import { toHumanDuration } from "../../../../utils/helpers/DurationUtils";
		import { FetchingResourcePools } from "../../../sessionsV2/components/SessionModals/ResourceClassWarning.tsx";

	const hasCustomQuery = new URLSearchParams(location.search).has("custom");
	const [searchParams] = useSearchParams(); // from "react-router-dom-v5-compat"
	const hasCustomQuery = searchParams.has("custom");

	import { useGetNotebooksVersionQuery } from "../../../versions/versions.api.ts";
	import { useGetNotebooksVersionQuery } from "../../../versions/versions.api";

feat: set resource class for a session launcher #3196

feat: set resource class for a session launcher #3196

Conversation

andre-code commented Jun 10, 2024 • edited Loading

Description

Type of Change

Related Tickets & Documents

Screenshots

UI Accessibility Checklist

Testing

Added/Updated Unit Tests?

Added/Updated E2E Tests?

Renku Acceptance Tests

Storybook

RenkuBot commented Jun 10, 2024

leafty left a 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

leafty left a comment

Choose a reason for hiding this comment

andre-code commented Jun 27, 2024

Choose a reason for hiding this comment

RenkuBot commented Jul 1, 2024

andre-code commented Jun 10, 2024 •

edited

Loading