ODC-7399: Create getting started content in functions list page #13289

lokanandaprabhu · 2023-10-27T05:15:33Z

Story:
https://issues.redhat.com/browse/ODC-7399

Description:

Added getting started with functions section in functions list page
First section will have the samples to create serverless functions
Second section will have quick starts , if quick starts not provided then user will be show external link
Third section will have external link documentation
Create dropdown will have 2 options, import from git and samples
Add page will have 2 CTA to create serverless function
Topology add action will have 2 CTA to create serverless function

Demo:

Screen.Recording.2023-10-27.at.10.37.42.AM.mov

Screen.Recording.2023-10-27.at.10.42.50.AM.mov

Screen.Recording.2023-10-31.at.10.13.03.AM.mov

openshift-ci-robot · 2023-10-27T05:15:36Z

@lokanandaprabhu: This pull request references ODC-7399 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Story:
https://issues.redhat.com/browse/ODC-7399

Description:

Added getting started with functions section in functions list page

First section will have the samples to create serverless functions

Second section will have quick starts , if quick starts not provided then user will be show external link

Third section will have external link documentation

Create dropdown will have 2 options, import from git and samples

Add page will have 2 CTA to create serverless function

Topology add action will have 2 CTA to create serverless function

Demo:

Screen.Recording.2023-10-27.at.10.37.42.AM.mov

Screen.Recording.2023-10-27.at.10.42.50.AM.mov

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lokanandaprabhu · 2023-10-27T16:27:31Z

/retest

lokanandaprabhu · 2023-10-30T03:01:57Z

/retest

jerolimov

In general, fine! 👍

Some small code change requests.

Some UX topics that we should check in a call:

frontend/packages/console-shared/src/components/catalog/CatalogController.tsx

frontend/packages/dev-console/console-extensions.json

frontend/packages/knative-plugin/src/components/functions/GettingStartedCard.tsx

frontend/packages/knative-plugin/src/components/functions/GettingStartedSection.tsx

frontend/packages/knative-plugin/src/components/functions/SamplesGettingStartedCard.tsx

frontend/packages/knative-plugin/src/components/functions/GettingStartedSection.tsx

frontend/packages/knative-plugin/src/components/services/ServiceDetailsPage.tsx

openshift-ci-robot · 2023-10-31T04:46:30Z

@lokanandaprabhu: This pull request references ODC-7399 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Story:
https://issues.redhat.com/browse/ODC-7399

Description:

Added getting started with functions section in functions list page

First section will have the samples to create serverless functions

Second section will have quick starts , if quick starts not provided then user will be show external link

Third section will have external link documentation

Create dropdown will have 2 options, import from git and samples

Add page will have 2 CTA to create serverless function

Topology add action will have 2 CTA to create serverless function

Demo:

Screen.Recording.2023-10-27.at.10.37.42.AM.mov

Screen.Recording.2023-10-27.at.10.42.50.AM.mov

Screen.Recording.2023-10-31.at.10.13.03.AM.mov

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lokanandaprabhu · 2023-10-31T06:57:51Z

Hi @jerolimov ,

I have resolved the feedback comments. PTAL.

lokanandaprabhu · 2023-10-31T17:18:01Z

/retest

lokanandaprabhu · 2023-11-02T05:56:44Z

Propage labels from the epic:

/label docs-approved
/label px-approved

sanketpathak · 2023-11-03T15:41:43Z

Verified UI with admin and non-admin user

/label qe-approved

openshift-ci-robot · 2023-11-03T15:42:18Z

@lokanandaprabhu: This pull request references ODC-7399 which is a valid jira issue.

In response to this:

Story:
https://issues.redhat.com/browse/ODC-7399

Description:

Added getting started with functions section in functions list page

First section will have the samples to create serverless functions

Second section will have quick starts , if quick starts not provided then user will be show external link

Third section will have external link documentation

Create dropdown will have 2 options, import from git and samples

Add page will have 2 CTA to create serverless function

Topology add action will have 2 CTA to create serverless function

Demo:

Screen.Recording.2023-10-27.at.10.37.42.AM.mov

Screen.Recording.2023-10-27.at.10.42.50.AM.mov

Screen.Recording.2023-10-31.at.10.13.03.AM.mov

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Lucifergene · 2023-11-06T06:23:48Z

/assign @Lucifergene

Lucifergene

The header of the Expandable menu looks bold :

frontend/packages/knative-plugin/src/components/functions/GettingStartedSection.tsx

frontend/packages/knative-plugin/console-extensions.json

jerolimov

Small recommendations. Let me know if you have any question. Thanks for the great work 👍

jerolimov · 2023-11-06T14:31:18Z

frontend/packages/dev-console/src/components/import/SamplesCatalog.tsx

+  const sampleType = params.get('sampleType');
+  const labelFilter: Record<string, string> = {
+    label: 'sample-type',
+    value: sampleType,
+  };


In my previous comment, I mixed two options, a sampleType query parameter is one option. The Record is an option for a more generic solution where the filter is something like ?labelFilter=serverless=true

So this code currently mixes both, let us focus on one solution or the other. At the moment a sampleType is fine.

Suggested change

const sampleType = params.get('sampleType');

const labelFilter: Record<string, string> = {

label: 'sample-type',

value: sampleType,

};

const sampleType = params.get('sampleType');

We need both since typeLabel is translated string in other samples

jerolimov · 2023-11-06T14:35:11Z

frontend/packages/dev-console/src/components/import/SamplesCatalog.tsx

+              let filteredLists: CatalogItem[];
+              if (sampleType) {
+                filteredLists = service.items.filter((item) => {
+                  return (
+                    item?.typeLabel === labelFilter?.value ||
+                    item?.data?.metadata?.labels[labelFilter?.label] === labelFilter?.value
+                  );
+                });
+              } else {
+                filteredLists = service.items;
+              }
+              const catalogItems = { ...service, items: filteredLists };


We can talk about the alternative here, but for now I would recommend to focus on sampleType filter. A labelFilter might have more powers, but we shouldn't mix both.

Suggested change

let filteredLists: CatalogItem[];

if (sampleType) {

filteredLists = service.items.filter((item) => {

return (

item?.typeLabel === labelFilter?.value ||

item?.data?.metadata?.labels[labelFilter?.label] === labelFilter?.value

);

});

} else {

filteredLists = service.items;

}

const catalogItems = { ...service, items: filteredLists };

let filteredItems = service.items;

if (sampleType) {

filteredItems = filteredItems.filter((item) => item.typeLabel === sampleType);

}

const catalogItems = { ...service, items: filteredItems };

Maybe I miss something, but as long as the "all samples' link uses ...?sampleType=Serverless%20function we don't need more here.

Yes value is the value from the URL only, but typeLabel is the translated string in other samples. If we change the language, the type label will show in the respective language.

So if I look only for typeLabel, then if it is translated string, later we have to ask serverless team to add one metadata.label to indicate this is the sample wrt serverless functions

Now while creating the sample, if they add new label as sample-type : 'Serverless Function' then if typeLabel is translated string also we will not be having any issue.

frontend/packages/knative-plugin/console-extensions.json

jerolimov · 2023-11-06T14:41:49Z

frontend/packages/knative-plugin/src/components/functions/CreateActionDropdown.tsx

+  const items = menuActions
+    ? Object.keys(menuActions).reduce<Record<string, string>>((acc, key) => {
+        const menuAction: MenuAction = menuActions[key];
+        const label =
+          menuAction.label ||
+          (menuAction.model?.labelKey ? t(menuAction.model.labelKey) : menuAction.model?.label);
+        if (!label) return acc;
+
+        return {
+          ...acc,
+          [key]: label,
+        };
+      }, {})
+    : undefined;


The object above have well known actions with translated label and onSelection callbacks. So the code to use labelKey never runs. Also the if (!label) statement should never be true. It looks to me that the items object will be the same as menuActions, or?

menuActions is object with Key and Menu actions and the key is used while selecting the option from the dropdown. While opening the dropdown component also the shared component expecting key and label. I have removed code wrt labelKey.

frontend/packages/knative-plugin/src/components/functions/FunctionsList.tsx

frontend/packages/knative-plugin/src/components/functions/GettingStartedSection.tsx

jerolimov · 2023-11-06T14:50:34Z

frontend/packages/knative-plugin/src/components/functions/QuickStartGettingStartedCard.tsx

+        if (loaded && slicedQuickStarts.length === 0) {
+          links.push(
+            {
+              id: 'ide-extensions',
+              href:
+                'https://marketplace.visualstudio.com/items?itemName=redhat.vscode-openshift-connector',
+              title: t('knative-plugin~Create using IDE extension'),
+              external: true,
+            },
+            {
+              id: 'functions-tekton-pipelines',
+              title: t('knative-plugin~Building Functions on Cluster with Tekton Pipelines'),
+              href:
+                'https://github.com/knative/func/blob/main/docs/building-functions/on_cluster_build.md',
+              external: true,
+            },
+          );


Can we add one link if we found just QS, and both if there are 0?

It is discussed as either QS will be there or external links. One QS is there also, external link will be added to the 3rd section, which is documentation section.

lokanandaprabhu · 2023-11-07T04:16:06Z

/retest

lokanandaprabhu · 2023-11-07T14:17:22Z

/retest

lokanandaprabhu · 2023-11-08T09:48:19Z

/retest

Lucifergene · 2023-11-10T06:45:17Z

Looks good to me :)

lokanandaprabhu · 2023-11-10T11:19:54Z

/retest

lokanandaprabhu · 2023-11-10T11:24:56Z

/test e2e-gcp-console

Lucifergene · 2023-11-10T13:47:00Z

/retest

Lucifergene · 2023-11-10T13:47:09Z

/lgtm

jerolimov

/approve

openshift-ci · 2023-11-10T23:13:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerolimov, lokanandaprabhu, Lucifergene

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/packages/dev-console/OWNERS~~ [jerolimov]
~~frontend/packages/knative-plugin/OWNERS~~ [jerolimov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2023-11-11T05:52:52Z

/retest-required

Remaining retests: 0 against base HEAD 0a80f8f and 2 for PR HEAD 876798f in total

jerolimov · 2023-11-11T09:17:50Z

/retest

openshift-ci-robot · 2023-11-11T15:27:15Z

/retest-required

Remaining retests: 0 against base HEAD ccebf14 and 1 for PR HEAD 876798f in total

jerolimov · 2023-11-12T10:20:03Z

/retest

jerolimov · 2023-11-12T17:25:59Z

/retest

openshift-ci · 2023-11-12T21:22:41Z

@lokanandaprabhu: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 27, 2023

openshift-ci bot requested review from invincibleJai and vikram-raj October 27, 2023 05:15

openshift-ci bot added component/dev-console Related to dev-console component/knative Related to knative-plugin component/shared Related to console-shared kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated labels Oct 27, 2023

lokanandaprabhu force-pushed the feature/ODC-7399 branch from 462a631 to e01892e Compare October 27, 2023 09:29

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 27, 2023

lokanandaprabhu force-pushed the feature/ODC-7399 branch from e01892e to d2008ec Compare October 27, 2023 16:30

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 27, 2023

jerolimov suggested changes Oct 30, 2023

View reviewed changes

lokanandaprabhu force-pushed the feature/ODC-7399 branch 2 times, most recently from 7e7a224 to 1c48434 Compare October 31, 2023 06:56

lokanandaprabhu force-pushed the feature/ODC-7399 branch from 1c48434 to cd16eb6 Compare October 31, 2023 13:58

lokanandaprabhu force-pushed the feature/ODC-7399 branch 3 times, most recently from f891c64 to af8d987 Compare November 2, 2023 05:24

openshift-ci bot added docs-approved Signifies that Docs has signed off on this PR px-approved Signifies that Product Support has signed off on this PR labels Nov 2, 2023

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Nov 3, 2023

openshift-ci bot assigned Lucifergene Nov 6, 2023

Lucifergene suggested changes Nov 6, 2023

View reviewed changes

frontend/packages/knative-plugin/src/components/functions/GettingStartedSection.tsx Outdated Show resolved Hide resolved

frontend/packages/knative-plugin/console-extensions.json Show resolved Hide resolved

jerolimov suggested changes Nov 6, 2023

View reviewed changes

lokanandaprabhu force-pushed the feature/ODC-7399 branch 2 times, most recently from bee7efd to 3626392 Compare November 6, 2023 15:39

Added Getting started with Functions for functions list page

876798f

lokanandaprabhu force-pushed the feature/ODC-7399 branch from 3626392 to 876798f Compare November 8, 2023 05:27

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2023

jerolimov approved these changes Nov 10, 2023

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 10, 2023

jerolimov approved these changes Nov 10, 2023

View reviewed changes

openshift-merge-bot bot merged commit 65cdd68 into openshift:master Nov 12, 2023
6 checks passed

ODC-7399: Create getting started content in functions list page #13289

ODC-7399: Create getting started content in functions list page #13289

Conversation

lokanandaprabhu commented Oct 27, 2023 • edited

openshift-ci-robot commented Oct 27, 2023 • edited by openshift-ci bot

lokanandaprabhu commented Oct 27, 2023

lokanandaprabhu commented Oct 30, 2023

jerolimov left a comment • edited

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 31, 2023 • edited by openshift-ci bot

lokanandaprabhu commented Oct 31, 2023

lokanandaprabhu commented Oct 31, 2023

lokanandaprabhu commented Nov 2, 2023

sanketpathak commented Nov 3, 2023

openshift-ci-robot commented Nov 3, 2023 • edited by openshift-ci bot

Lucifergene commented Nov 6, 2023

Lucifergene left a comment • edited

Choose a reason for hiding this comment

jerolimov 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

Choose a reason for hiding this comment

lokanandaprabhu commented Nov 7, 2023

lokanandaprabhu commented Nov 7, 2023

lokanandaprabhu commented Nov 8, 2023

Lucifergene commented Nov 10, 2023

lokanandaprabhu commented Nov 10, 2023

lokanandaprabhu commented Nov 10, 2023

Lucifergene commented Nov 10, 2023

Lucifergene commented Nov 10, 2023

jerolimov left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Nov 10, 2023

openshift-ci-robot commented Nov 11, 2023

jerolimov commented Nov 11, 2023

openshift-ci-robot commented Nov 11, 2023

jerolimov commented Nov 12, 2023

jerolimov commented Nov 12, 2023

openshift-ci bot commented Nov 12, 2023

lokanandaprabhu commented Oct 27, 2023 •

edited

openshift-ci-robot commented Oct 27, 2023 •

edited by openshift-ci bot

jerolimov left a comment •

edited

openshift-ci-robot commented Oct 31, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Nov 3, 2023 •

edited by openshift-ci bot

Lucifergene left a comment •

edited