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

feat(app): Support extensible entity tabs #1173

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

gashcrumb
Copy link
Member

@gashcrumb gashcrumb commented Apr 11, 2024

Description

This commit adds the ability to customize and extend the default set of tabs available for catalog entity items. The default set of tabs is configured as part of the static frontend app configuration, overrides can be applied using the regular backstage config file via the dynamicPlugins.entityTabs configuration setting. This commit also fixes a couple rendering warnings that can occur when configuring the same plugin for the same view multiple times.

Which issue(s) does this PR fix

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

image

With this change a new tab could be added like:

dynamicPlugins:
  frontend:
    some-plugin-id:
      entityTabs:
        - path: /quarkus:                                                # catalog sub-route
          title: Quarkus                                       # tab title
          mountPoint: entity.page.quarkus    #tab mount point
      mountPoints:
        - mountPoint: entity.page.quarkus

An existing tab can be renamed too, for example:

dynamicPlugins:
  frontend:
    some-plugin-id:
      entityTabs:
        - path: /                                                    # catalog sub-route
          title: General                                         # tab title, this renames "Overview" to "General"
          mountPoint: entity.page.overview   # tab mount point, this can be customized too

Visibility of a tab is determined by the availability of items for the configured mount point and the type of catalog entity being displayed. If multiple plugins try and configure the same route, a warning will be raised to the browser console and only the first entry will be used.

Here's a better screenshot with some other plugins enabled:

image

Which uses this configuration:

dynamicPlugins:
  frontend:
    my-custom-tabs:   # an arbitrary map item can even be used to configure and add tabs, no plugin installation needed
      entityTabs:
        - path: /
          title: General
          mountPoint: entity.page.overview
        - path: /topology
          title: Service Diagram
          mountPoint: entity.page.topology
        - path: /kubernetes
          title: My Cluster
          mountPoint: entity.page.kubernetes
    janus-idp.backstage-plugin-simple-test-components:
      entityTabs:
        - path: /some-new-route
          title: A New Tab
          mountPoint: entity.page.new-mountpoint
      mountPoints:
        - mountPoint: entity.page.new-mountpoint/cards
          importName: SimpleTestComponentsPage
          config:
            layout:
              gridColumn: "1 / -1"
              gridRow: "1 / 5"
          props:
            text: Look here is a new tab for a catalog item

@gashcrumb gashcrumb requested a review from a team as a code owner April 11, 2024 10:42
@openshift-ci openshift-ci bot requested review from nickboldt and tumido April 11, 2024 10:42
@gashcrumb
Copy link
Member Author

gashcrumb commented Apr 11, 2024

I would argue this Sonarcloud result 😄

Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-1173!

@gashcrumb
Copy link
Member Author

Something to investigate:

image

@gashcrumb
Copy link
Member Author

That issue should be sorted now. We'll want to have SonarCloud ignore the baseFrontendConfig.ts file for duplicate code structures.

Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-1173!

@gashcrumb
Copy link
Member Author

This could do with some more testing on openshift, let's keep this as a draft for now.

Copy link
Member

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

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

I' not a big fan of having the entityTabs field directly at the dynamicPlugins level, because it is still a frontend thing.
I probably would also prefer letting the contribution to a new tab be placed inside a frontend plugin config, like this:

dynamicPlugins:
  frontend:
    some-plugin-id:
      entityTabs:
        /quarkus:                                                # catalog sub-route
          title: Quarkus                                       # tab title
          mountPoint: entity.page.quarkus    #tab mount point
      mountPoints:
        - mountPoint: entity.page.quarkus

@gashcrumb
Copy link
Member Author

I' not a big fan of having the entityTabs field directly at the dynamicPlugins level, because it is still a frontend thing.

It's not my favorite solution either to be honest but given that frontend is a dictionary I didn't want to add it there.

I probably would also prefer letting the contribution to a new tab be placed inside a frontend plugin config, like this:

Great suggestion! I think I can make this work, I might revise the name choice but I think this is a better direction. Thanks!

@gashcrumb gashcrumb force-pushed the RHIDP-1881 branch 2 times, most recently from 37c1859 to 744bcbe Compare April 16, 2024 18:31
@gashcrumb gashcrumb marked this pull request as ready for review April 16, 2024 18:31
@openshift-ci openshift-ci bot requested a review from davidfestal April 16, 2024 18:31
@gashcrumb gashcrumb force-pushed the RHIDP-1881 branch 2 times, most recently from e501581 to fb8f754 Compare April 16, 2024 18:59
Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-1173!

Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-1173!

@davidfestal
Copy link
Member

davidfestal commented Apr 30, 2024

@gashcrumb About the SonarCould duplication report related to this block: https://github.com/janus-idp/backstage-showcase/pull/1173/files#diff-dc23b2c0e4792453496e74f891819cf0cf5172dd3d6b426bca6b9f85aef4d7e5R67-R128, would it make sense to extract this defaultTabs constant to its own file (or a file dedicated to such default config constants).
This would allow adding a SonarCloud exception for this file only with the following content in a .sonarcloud.properties file:

sonar.cpd.exclusions=packages/app/src/components/catalog/EntityPage/<your new file>

I'm not saying it should be done now in this PR, but at least it might be something we want to think about to avoid bypassing the SonarCloud analysis in such cases and having to force-merge.

We can also add the corresponding setting directl in the SonarCloud project settings, but I don't have the right permissions, which would be much simpler obviously.

cc @kadel Any idea who can do that ?

@gashcrumb
Copy link
Member Author

@gashcrumb About the SonarCould duplication report related to this block: #1173 (files), would it make sense to extract this defaultTabs constant to its own file (or a file dedicated to such default config constants). This would allow adding a SonarCloud exception for this file only with the following content in a .sonarcloud.properties file:

sonar.cpd.exclusions=packages/app/src/components/catalog/EntityPage/<your new file>

I'm not saying it should be done now in this PR, but at least it might be something we want to think about to avoid bypassing the SonarCloud analysis in such cases and having to force-merge.

We can also add the corresponding setting directl in the SonarCloud project settings, but I don't have the right permissions, which would be much simpler obviously.

cc @kadel Any idea who can do that ?

I can do that in this PR. I think probably this is the way to go too, as then we've some pointer of these duplicate lines somewhere.

Also likely #1202 will also need a similar approach to handle the mapping that needs to be done to match up the API response to the config.

Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-1173!

@davidfestal
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Apr 30, 2024
This commit adds the ability to customize and extend the default set of
tabs available for catalog entity items.  The default set of tabs is
hard-coded in the entity page but can be reconfigured and extended per
plugin using the `entityTabs` property.  If multiple plugins target the
same entity route, only the first one will be used and a warning will be
raised.

Signed-off-by: Stan Lewis <gashcrumb@gmail.com>
@davidfestal
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Apr 30, 2024
@openshift-ci openshift-ci bot removed the lgtm label Apr 30, 2024
@davidfestal
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Apr 30, 2024
Copy link

openshift-ci bot commented Apr 30, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidfestal

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

Copy link

sonarcloud bot commented Apr 30, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-1173!

@davidfestal
Copy link
Member

/restest

@davidfestal
Copy link
Member

/retest

@openshift-merge-bot openshift-merge-bot bot merged commit e998d9f into janus-idp:main Apr 30, 2024
8 checks passed
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.

Allow adding a new Entity tab through dynamic plugin configuration
4 participants