-
Notifications
You must be signed in to change notification settings - Fork 525
fix(entity-feedback): Add link to feedback notifications #6611
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
fix(entity-feedback): Add link to feedback notifications #6611
Conversation
Changed Packages
|
|
Thanks for the contribution! |
Add link field to notification payload so users can navigate directly
to the entity's feedback page when they receive a notification about
new feedback on their owned entities.
Notifications now include a clickable link in the format:
/catalog/{namespace}/{kind}/{name}/feedback
Signed-off-by: David <[email protected]>
Signed-off-by: David <[email protected]>
074a3aa to
92b7cd7
Compare
Signed-off-by: David <[email protected]>
| // Construct entity URL from entityRef (format: "kind:namespace/name") | ||
| const [kind, namespaceName] = req.params.entityRef.split(':'); | ||
| const [namespace, name] = namespaceName.split('/'); | ||
| const entityUrl = `/catalog/${namespace}/${kind}/${name}/feedback`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for suggesting this change however it is not guaranteed that this is always the entity URL - consumers are free to attach the catalog plugin and "feedback" tab to any custom route name of their choosing even though this is the default. Therefore we probably need a more configurable way to construct this url that is consistent with how the frontend plugin is setup
| const [kind, namespaceName] = req.params.entityRef.split(':'); | ||
| const [namespace, name] = namespaceName.split('/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI you can use parseEntityRef to parse entity refs
Address review feedback: - Use parseEntityRef utility instead of manual string parsing - Make notification link URL configurable via entityFeedback.feedbackUrlPattern - Default pattern: /catalog/:namespace/:kind/:name - Allows customization for non-standard routes Users can now configure custom URL patterns in app-config.yaml: ```yaml entityFeedback: feedbackUrlPattern: '/catalog/:namespace/:kind/:name/feedback' ``` This addresses the concern that the /feedback route suffix may be customized in different Backstage installations. Signed-off-by: David <[email protected]>
|
Thanks for the review! I've addressed both comments: 1. Using 2. Configurable URL pattern: ✅ const feedbackUrlPattern =
config.getOptionalString('entityFeedback.feedbackUrlPattern') ||
'/catalog/:namespace/:kind/:name';
const entityUrl = feedbackUrlPattern
.replace(':kind', kind)
.replace(':namespace', namespace)
.replace(':name', name);Default behavior: Links to Custom configuration example: entityFeedback:
feedbackUrlPattern: '/catalog/:namespace/:kind/:name/feedback'This allows users to configure the exact pattern that matches their frontend routing setup. Tests have been updated to reflect the new default pattern. |
| // Allow customization of the feedback URL pattern via config | ||
| // Default: /catalog/:namespace/:kind/:name | ||
| const feedbackUrlPattern = | ||
| config.getOptionalString('entityFeedback.feedbackUrlPattern') || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also define and document the new config - see https://backstage.io/docs/conf/defining for more info and the playlist plugin for an example: https://github.com/backstage/community-plugins/blob/main/workspaces/playlist/plugins/playlist/config.d.ts, https://github.com/backstage/community-plugins/blob/main/workspaces/playlist/plugins/playlist/package.json#L85 see comment below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually another option which may be more robust is to send this info from the frontend plugin since the routing can be derived there automatically:
if you can emulate what's done here to derive the route of the entity: https://github.com/backstage/backstage/blob/fea3e3972daf42fc75c107568a693eeb14a4d672/plugins/catalog-react/src/components/EntityRefLink/EntityRefLink.tsx#L84-L88, then pass it along in the request to the backend here:
Line 132 in 4d58c3f
| await feedbackApi.recordResponse(stringifyEntityRef(entity), { |
The benefit of the above is that it will always consistently construct the appropriate entity url (including custom ones) without any additional configuration from the consumer
…cations Changes the implementation to derive entity URLs from the frontend routing configuration instead of using backend configuration. This ensures notification links always match the actual routes configured in the app. **Frontend changes:** - Use entityRouteRef and entityRouteParams (same as EntityRefLink) to derive URL - Pass entity URL in feedback request **Backend changes:** - Accept link from request instead of constructing it - Remove config-based URL pattern logic - Add database migration for link field - Update tests to include link field This approach is more robust as it uses a single source of truth (frontend router) and requires no additional configuration. Signed-off-by: David <[email protected]>
Signed-off-by: David <[email protected]>
Signed-off-by: David <[email protected]>
Signed-off-by: David <[email protected]>
Signed-off-by: David <[email protected]>
Signed-off-by: David <[email protected]>
kuangp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks !
Summary
This PR fixes a UX issue in the entity-feedback-backend plugin where notifications about new feedback don't include a clickable link to navigate to the entity.
Changes
linkfield to notification payload in/responses/:entityRef(*)endpoint/catalog/{namespace}/{kind}/{name}/feedbackkind:namespace/name) to construct proper Backstage entity URLMotivation
When entity owners receive notifications about new feedback on their entities, the notification currently shows the entity name and comments but doesn't provide a way to navigate to the feedback. This PR adds a clickable link that takes users directly to the entity's feedback page.
Implementation
Testing
Tested in a production Backstage instance with entity-feedback plugin enabled:
The notification link successfully takes users to the correct entity feedback page in the format
/catalog/{namespace}/{kind}/{name}/feedback.