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.

Sign up for GitHub

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

Widgets for itless environment #716

Merged
merged 5 commits into from
Nov 12, 2024
Merged

Widgets for itless environment #716

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
20a28d2
feat: Add a widget model specifically for FR env
SteveHNH Oct 25, 2024
4f2ffbd
feat: Add FR based widget mappings via unleash
SteveHNH Oct 25, 2024
ad8ece9
fix: use an existing feature flag
SteveHNH Oct 30, 2024
5b46228
refactor: rework filter widgets
SteveHNH Nov 8, 2024
3ddf227
fix: delete widget from mapping when hidden
SteveHNH Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion rest/routes/dashboardTemplate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/RedHatInsights/chrome-service-backend/rest/models"
"github.com/RedHatInsights/chrome-service-backend/rest/service"
"github.com/RedHatInsights/chrome-service-backend/rest/util"
"github.com/RedHatInsights/chrome-service-backend/rest/featureflags"
"github.com/go-chi/chi/v5"
"github.com/sirupsen/logrus"
"gorm.io/gorm"
Expand Down Expand Up @@ -280,6 +281,15 @@ func GetWidgetMappings(w http.ResponseWriter, r *http.Request) {
handleDashboardResponse[models.WidgetModuleFederationMapping](resp, err, w)
}

func GetWidgetMappingsFR(w http.ResponseWriter, r *http.Request) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@SteveHNH I wanted to list out a few long term considerations we will need in the future (I know you and I have discussed several of these offline).

  1. We currently only support one template type (currently "landingPage"). See: https://github.com/RedHatInsights/chrome-service-backend/blob/main/docs/dashboard-layouts.md#base-layouts (backend) and https://github.com/RedHatInsights/widget-layout/blob/master/src/api/dashboard-templates.ts#L13 (frontend).
  2. We plan to migrate this backend logic to its own service (outside of chrome service) and allow for additional "types" in the future. For example if another UI - such as the insights UI - would want to utilize our common widgetized layout service but render a different template and widgets than our landing page.
  3. As @Hyperkid123 pointed out - we also will be migrating the individual widget configurations into the applications that surface them (via frontend config and the frontend operator).
  4. Finally - we need to consider the ordering of widgets by default (when a user first loads the template type and it is automatically forked). I do not believe it is a requirement today - but would we ever want to have a different base layout for the landing page in itless than we do in commercial?

Getting back to this PR though - we have a few options to unblock us in the short term:

  1. Fork the widget-mapping itself for itless (this PR). This would pose minimal risk to our commercial flow - however will have duplication between commercial and itless widget-mappings (for those widgets that appear in both lists). We would also need to remember to add any new widgets that itless would be interested in to both lists.
  2. Add a new metadata/config field inside of each widget struct to indicate if it should appear in the itless environment (more self-service model). This would prevent the duplicated widget-mapping (as we would just filter for that metadata field when running inside of itless). However this model doesn't scale well when it comes to adding support for multiple envs, layouts, etc. and could result in widgets appearing in itless unexpectedly if the field is copy/pasted. This also defeats the purpose of having the widget-mapping (whitelist of widgets that can be rendered).

That being said - I think I prefer option 1 (as you have here). My only concern is we are not updating the base layout for itless as well. See https://github.com/RedHatInsights/chrome-service-backend/blob/main/rest/service/dashboardTemplate.go#L450-L507. This means each time the "landingPage" layout if forked when the user first loads the page in itless - we will have references in the template to widgets that do not exist in the itless widget mapping (such as "ansible"). I believe these widgets will simply be ignored by the UI as they do not appear in the mapping - however we should confirm that in our itless environment and confirm customizing the layout works as expected (even with these invalid references).

Copy link
Contributor

@Hyperkid123 Hyperkid123 Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@florkbr I'd like to provide a 3rd option. Which is using feature flags to enable/disable items in an environment. rather than adding a flag saying this is for itless. Why?

Well, we want feature flags anyway, it will be easier to maintain without introducing tech debt, and it will have the same effect of "messed up initial layout" anyway.

Chrome service is hooked up to unleash already. We can do some "inverted" feature flags like "widget.supportCase.hidden". This way we can remove the widget from a layout when it gets "forked" or when the struct is created from the base layout yaml

Copy link
Contributor

Choose a reason for hiding this comment

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

@Hyperkid123 that seems like a reasonable approach to me - I think I was deep in the itless headspace with the other 2 options - I like that solution more. @SteveHNH thoughts on using @Hyperkid123's suggestion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me! I understand the idea, but I don't fully understand how to make that happen. This is also the first time I've messed with this code, so I just need to dive in and get a handle on it. Thanks for the feedback!

var err error
resp := util.EntityResponse[models.WidgetModuleFederationMapping]{
Data: service.WidgetMappingFR,
}

handleDashboardResponse[models.WidgetModuleFederationMapping](resp, err, w)
}

func ResetDashboardTemplate(w http.ResponseWriter, r *http.Request) {
user := r.Context().Value(util.USER_CTX_KEY).(models.UserIdentity)
userID := user.ID
Expand Down Expand Up @@ -323,5 +333,9 @@ func MakeDashboardTemplateRoutes(sub chi.Router) {
sub.Get("/base-template", GetBaseDashboardTemplates)
sub.Get("/base-template/fork", ForkBaseTemplate)

sub.Get("/widget-mapping", GetWidgetMappings)
if featureflags.IsEnabled("platform.chrome.itless") {
sub.Get("/widget-mapping", GetWidgetMappingsFR)
} else {
sub.Get("/widget-mapping", GetWidgetMappings)
}
}
60 changes: 60 additions & 0 deletions rest/service/dashboardTemplate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,66 @@ var (
},
},
}
WidgetMappingFR models.WidgetModuleFederationMapping = models.WidgetModuleFederationMapping{
models.Rhel: models.ModuleFederationMetadata{
Scope: "landing",
Module: "./RhelWidget",
Defaults: models.BaseWidgetDimensions.InitDimensions(models.BaseWidgetDimensions{}, 1, 4, 10, 1),
Config: models.WidgetConfiguration{
Icon: models.RhelIcon,
Title: "Red Hat Enterprise Linux",
},
},
models.OpenShift: models.ModuleFederationMetadata{
Scope: "landing",
Module: "./OpenShiftWidget",
Defaults: models.BaseWidgetDimensions.InitDimensions(models.BaseWidgetDimensions{}, 1, 4, 10, 1),
Config: models.WidgetConfiguration{
Icon: models.OpenShiftIcon,
Title: "Red Hat OpenShift",
},
},
models.RecentlyVisited: models.ModuleFederationMetadata{
Scope: "landing",
Module: "./RecentlyVisited",
Defaults: models.BaseWidgetDimensions.InitDimensions(models.BaseWidgetDimensions{}, 1, 7, 10, 1),
Config: models.WidgetConfiguration{
Icon: models.HistoryIcon,
Title: "Recently visited",
},
},
models.FavoriteServices: models.ModuleFederationMetadata{
Scope: "chrome",
Module: "./DashboardFavorites",
Defaults: models.BaseWidgetDimensions.InitDimensions(models.BaseWidgetDimensions{}, 1, 6, 10, 1),
Config: models.WidgetConfiguration{
HeaderLink: models.WidgetHeaderLink{
Title: "View all services",
Href: "/allservices",
},
Icon: models.StarIcon,
Title: "My favorite services",
},
},
models.NotificationsEvents: models.ModuleFederationMetadata{
Scope: "notifications",
Module: "./DashboardWidget",
Defaults: models.BaseWidgetDimensions.InitDimensions(models.BaseWidgetDimensions{}, 1, 3, 10, 1),
Config: models.WidgetConfiguration{
HeaderLink: models.WidgetHeaderLink{
Title: "View event log",
Href: "/settings/notifications/eventlog",
},
Icon: models.BellIcon,
Title: "Events",
Permissions: []models.WidgetPermission{
models.WidgetPermission{
Method: models.OrgAdmin,
},
},
},
},
}
)

func ForkBaseTemplate(userId uint, dashboard models.AvailableTemplates) (models.DashboardTemplate, error) {
Expand Down
Loading