Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no pipelines error fix #1412

Merged
merged 2 commits into from
Jun 21, 2023

Conversation

Gkrumbach07
Copy link
Member

closes: #1379

Description

backend: blank config is merged for all calls to get dashboard config
frontend: updated no pipelines page to use useAccessReview and redirect to a search for the operator instead of direct link to it

How Has This Been Tested?

  • dont have pipelines installed
  • go to pipelines page as user with access to install operators (cluster admin)
  • press button when no pipelines page pops up
  • this should take you to a search for the pipelines operator
  • impersonate a regular user
  • go to pipelines page
  • see "Missing the pipelines operator" and no button

Test Impact

none

Request review criteria:

  • The commits have meaningful messages (squashes happen on merge by the bot).
  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work
  • The developer has added tests or explained why testing cannot be added (unit tests & storybook for related changes)

@Gkrumbach07
Copy link
Member Author

cc @andrewballantyne

backend/src/utils/resourceUtils.ts Outdated Show resolved Hide resolved
@@ -90,7 +90,7 @@ const fetchDashboardCR = async (fastify: KubeFastifyInstance): Promise<Dashboard
)
.then((res) => {
const dashboardCR = res?.body as DashboardConfig;
return [dashboardCR];
return [_.merge({}, blankDashboardCR, dashboardCR)]; // merge with blank CR to prevent any missing values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, I needed to edit this same file in another PR, it just bothers me that we are using both async/await and promises at the same time, it's kinda hard to read and it could lead to issues, I would say we either use one or another if you want I can rework my PR and you can piggyback into those changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. What pr are you talking about. I can switch my implementation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1407 This is the one, I'm adding changes and including that specific one

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne

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:

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

@openshift-merge-robot openshift-merge-robot merged commit a684b66 into opendatahub-io:main Jun 21, 2023
7 checks passed
@shalberd
Copy link
Contributor

shalberd commented Jun 21, 2023

@lucferbux @Gkrumbach07 very nice, thanks a bunch :-) working as intended now using the latest code base from you

Bildschirmfoto 2023-06-22 um 07 59 32

no more error message that pipelines operator is not installed.

Cool thing also about the new search link and only letting users see it if they can install CSVs

andrewballantyne pushed a commit that referenced this pull request Jun 22, 2023
* no pipelines error fix

* removed comments
@Gkrumbach07 Gkrumbach07 deleted the pipelines-bug branch August 10, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Install the Pipelines Operator message in GUI when in fact Red Hat Openshift Pipelines installed
5 participants