feat(portal): Display user display name in release modal #5294
Conversation
dosubot
bot
added
the
size:XS
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
WalkthroughThe changes in this pull request involve updates to the Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apollo-portal/src/main/resources/static/views/component/release-modal.html (1)
112-112: Consider adding a tooltip for long display names.Since user display names can be lengthy, consider adding a tooltip to show the full information on hover.
-<span class="block" ng-bind="config.item.dataChangeLastModifiedByDisplayName + '(' + config.item.dataChangeLastModifiedBy + ')'"></span> +<span class="block" ng-bind="config.item.dataChangeLastModifiedByDisplayName + '(' + config.item.dataChangeLastModifiedBy + ')'" title="{{config.item.dataChangeLastModifiedByDisplayName}} ({{config.item.dataChangeLastModifiedBy}})"></span>Also applies to: 166-166, 206-206
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apollo-portal/src/main/resources/static/views/component/release-modal.html(3 hunks)
🔇 Additional comments (2)
apollo-portal/src/main/resources/static/views/component/release-modal.html (2)
166-166: Consistent implementation across all table sections.
The user display format is consistently implemented across all three table sections (normal release, gray release, and update & publish), providing a uniform user experience.
Also applies to: 206-206
112-112: LGTM! User display format enhances readability.
The new format dataChangeLastModifiedByDisplayName + '(' + dataChangeLastModifiedBy + ')' improves user identification while maintaining the technical ID for reference.
Let's verify that these properties are consistently available:
✅ Verification successful
The user display format is consistently implemented across the codebase
The verification shows that the same user display pattern dataChangeLastModifiedByDisplayName + '(' + dataChangeLastModifiedBy + ')' is consistently used across all relevant HTML templates:
- namespace-panel-branch-tab.html (2 occurrences)
- namespace-panel-master-tab.html (3 occurrences)
- release-modal.html (3 occurrences)
This confirms that the implementation follows a uniform pattern throughout the application's UI components, maintaining a consistent user experience.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of display name properties
# Look for other instances where we might want to apply the same pattern
rg "dataChangeLastModifiedByDisplayName" --type html
Length of output: 2008
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apollo-portal/src/main/resources/static/views/component/release-modal.html (2)
112-112: LGTM! Consider adding a space before the parenthesisThe addition of the display name improves user identification. However, consider adding a space before the parenthesis for better readability.
-<span class="block" ng-bind="config.item.dataChangeLastModifiedByDisplayName + '(' + config.item.dataChangeLastModifiedBy + ')'"></span> +<span class="block" ng-bind="config.item.dataChangeLastModifiedByDisplayName + ' (' + config.item.dataChangeLastModifiedBy + ')'"></span>
112-112: Consider creating an AngularJS filter for user display formattingThe same user display format is repeated across multiple tables. Consider creating a reusable AngularJS filter to handle this formatting consistently.
Example implementation:
angular.module('apollo.portal') .filter('userDisplay', function() { return function(user) { if (!user.dataChangeLastModifiedByDisplayName) return user.dataChangeLastModifiedBy; return user.dataChangeLastModifiedByDisplayName + ' (' + user.dataChangeLastModifiedBy + ')'; }; });Then use it in the template:
-<span class="block" ng-bind="config.item.dataChangeLastModifiedByDisplayName + '(' + config.item.dataChangeLastModifiedBy + ')'"></span> +<span class="block" ng-bind="config.item | userDisplay"></span>Also applies to: 166-166, 206-206
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apollo-portal/src/main/resources/static/views/component/release-modal.html(3 hunks)
🔇 Additional comments (3)
apollo-portal/src/main/resources/static/views/component/release-modal.html (3)
166-166: Same formatting suggestion applies here
For consistency with the previous suggestion, consider adding a space before the parenthesis.
206-206: Same formatting suggestion applies here
For consistency with the previous suggestions, consider adding a space before the parenthesis.
112-112: Verify display name availability
The changes assume that dataChangeLastModifiedByDisplayName is always available. Let's verify this assumption.
Also applies to: 166-166, 206-206
✅ Verification successful
Let me gather more information about how the display name is populated.
Let me check one more aspect about the user info enrichment process since we found references to enricher adapters.
Based on the evidence gathered, I can now provide the final response:
Display names are properly handled through the enrichment process
The display names are automatically populated through a robust enrichment system:
- The
UserDisplayNameEnrichercomponent enriches DTOs with user display names - The
AdditionalUserInfoEnrichServiceImplservice orchestrates the enrichment process - The
BaseDtoUserInfoEnrichedAdapterensures display names are properly set
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any direct uses of dataChangeLastModifiedBy without corresponding display name
# Search for uses of dataChangeLastModifiedBy to ensure display name is always set
rg "dataChangeLastModifiedBy" --type java
# Search for display name setter/getter methods
ast-grep --pattern 'class $_ {
$$$
setDataChangeLastModifiedByDisplayName($$$) {
$$$
}
$$$
}'
Length of output: 2782
Script:
#!/bin/bash
# Search for where display names are set in the codebase
rg "setDataChangeLastModifiedByDisplayName" -B 5 -A 5
# Check if there's any controller or service that handles this
rg "dataChangeLastModifiedByDisplayName" --type java -g "*Controller.java" -g "*Service.java" -B 5 -A 5
Length of output: 4102
Script:
#!/bin/bash
# Check the enricher implementation to understand how user display names are populated
rg -l "UserInfoEnricher" --type java
rg "UserInfoEnricher" -B 5 -A 10 --type java
# Also check for any tests related to this functionality
rg "UserInfoEnricher" -g "*Test.java" -B 5 -A 10
Length of output: 13802
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apollo-portal/src/main/resources/static/views/component/release-modal.html (1)
206-206: LGTM! Consider extracting the user display format.The user information display format is consistent with other tables. Since this format is used in multiple places, consider extracting it into a reusable template or filter.
Example filter implementation:
app.filter('userDisplay', function() { return function(user) { return user.dataChangeLastModifiedByDisplayName + '(' + user.dataChangeLastModifiedBy + ')'; }; });Then use it in the template:
-<span class="block" ng-bind="config.item.dataChangeLastModifiedByDisplayName + '(' + config.item.dataChangeLastModifiedBy + ')'"></span> +<span class="block" ng-bind="config.item | userDisplay"></span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apollo-portal/src/main/resources/static/views/component/release-modal.html(3 hunks)
🔇 Additional comments (2)
apollo-portal/src/main/resources/static/views/component/release-modal.html (2)
166-166: LGTM! Consistent with normal release table.
The user information display format maintains consistency across different tables.
112-112: LGTM! Verify display name availability.
The user information display format looks good, combining the display name with ID for better identification.
Let's verify that the display name is always available:
✅ Verification successful
Based on the code analysis, I can now generate the final response. The code shows that dataChangeLastModifiedByDisplayName is properly managed through:
- The
BaseDTOclass which defines the field and its accessors - The
UserDisplayNameEnrichercomponent that populates display names - The
BaseDtoUserInfoEnrichedAdapterthat handles setting the display name - The
AdditionalUserInfoEnrichServiceImplthat orchestrates the enrichment process
LGTM! Display name is properly managed.
The display name field is part of the core data model and is automatically populated by the user info enrichment system. The changes to display the user's display name alongside their ID in the UI are safe and consistent with the backend implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for places where dataChangeLastModifiedByDisplayName is set
rg "dataChangeLastModifiedByDisplayName" --type js
Length of output: 50
Script:
#!/bin/bash
# Let's try searching in all file types since it might be in TypeScript or other frontend files
rg "dataChangeLastModifiedByDisplayName"
# Also search for related user display name patterns
rg "displayName" -A 2 -B 2
Length of output: 96502
Script:
#!/bin/bash
# Let's search for the related user data in Java files to understand how dataChangeLastModifiedByDisplayName is populated
rg "dataChangeLastModifiedByDisplayName" --type java -A 2 -B 2
# Also search for user display name related code in controllers
rg "setDataChangeLastModifiedByDisplayName" --type java -A 2 -B 2
Length of output: 3742
Script:
#!/bin/bash
# Let's check the UserInfoEnricher implementation to see how display names are populated
rg "UserInfoEnricher" --type java -A 5 -B 5
# Also check if there are any tests verifying this functionality
rg "UserInfoEnricher.*Test" --type java
Length of output: 9853
nobodyiam
force-pushed
the
fix/releaseModal
branch
from
8be9b56 to
964d09c
Compare
|
Thanks for the contribution. The layout feels a bit tight; could we adjust it slightly? For example, we could utilize the space around the left-side label.
|
What's the purpose of this PR
Display user display name in release-modal.html
Which issue(s) this PR fixes:
Fixes #
Brief changelog
feat(portal): Display user display name in release modal
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.CHANGESlog.Summary by CodeRabbit
New Features
Bug Fixes