-
Notifications
You must be signed in to change notification settings - Fork 297
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
Communication
: Add profile picture to sidebar element and conversation header
#9719
Communication
: Add profile picture to sidebar element and conversation header
#9719
Conversation
WalkthroughThe changes in this pull request include the introduction of a new function Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
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.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (9)
src/main/webapp/app/entities/metis/conversation/one-to-one-chat.model.ts (1)
19-24
: Add JSDoc documentation for better maintainability.While the function implementation is correct and follows TypeScript best practices, adding JSDoc documentation would improve maintainability and provide better IDE support.
Consider adding documentation like this:
+/** + * Safely converts a ConversationDTO to OneToOneChatDTO if possible. + * @param conversation - The conversation to convert + * @returns The conversation as OneToOneChatDTO if it's a one-to-one chat, undefined otherwise + */ export function getAsOneToOneChatDTO(conversation: ConversationDTO | undefined): OneToOneChatDTO | undefined {src/main/webapp/app/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.ts (1)
Line range hint
18-24
: Consider adding ARIA label for the group iconSince you're replacing the group chat icon component with a Font Awesome icon, ensure that the template includes proper ARIA labels for accessibility.
Example usage in the template:
// In the template where the icon is used: <fa-icon [icon]="faPeopleGroup" size="xs" [attr.aria-label]="'artemis.conversation.groupChat' | translate" />src/main/webapp/app/overview/course-conversations/layout/conversation-header/conversation-header.component.html (1)
19-32
: Enhance accessibility and maintainability of the profile picture component.While the implementation aligns with the PR objectives, consider the following improvements:
- Add accessibility attributes for screen readers
- Extract hardcoded values to component constants
- Handle profile picture loading errors
Apply these improvements:
@if (getAsOneToOneChat(activeConversation) && otherUser) { <jhi-profile-picture [imageSizeInRem]="'2'" [fontSizeInRem]="'0.9'" [imageId]="'sidebar-profile-picture'" [defaultPictureId]="'sidebar-default-profile-picture'" [isGray]="false" [authorId]="otherUser.id" [authorName]="otherUser.name" class="me-2" [imageUrl]="otherUser.imageUrl" [isEditable]="false" + [attr.aria-label]="'Profile picture of ' + otherUser.name" + (error)="handleProfilePictureError($event)" > </jhi-profile-picture> }Consider moving these values to the component:
// conversation-header.component.ts readonly PROFILE_PICTURE_CONFIG = { imageSize: '2', fontSize: '0.9', imageId: 'sidebar-profile-picture', defaultPictureId: 'sidebar-default-profile-picture' } as const;src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.html (1)
57-70
: LGTM! Consider adding accessibility attributes.The profile picture implementation looks good and aligns with the PR objectives. However, consider these improvements:
Add an aria-label for better accessibility:
@if (otherUser) { <jhi-profile-picture [imageSizeInRem]="'1.1'" [fontSizeInRem]="'0.5'" [imageId]="'sidebar-profile-picture'" [defaultPictureId]="'sidebar-default-profile-picture'" [isGray]="false" [authorId]="otherUser.id" [authorName]="otherUser.name" [imageUrl]="otherUser.imageUrl" [isEditable]="false" + [attr.aria-label]="'Profile picture of ' + otherUser.name" > </jhi-profile-picture>
Consider making the image size configurable via component input to allow for different sizes in different contexts:
// In the component class @Input() profilePictureSizeRem = 1.1; @Input() profilePictureFontSizeRem = 0.5;Then in the template:
<jhi-profile-picture - [imageSizeInRem]="'1.1'" - [fontSizeInRem]="'0.5'" + [imageSizeInRem]="profilePictureSizeRem" + [fontSizeInRem]="profilePictureFontSizeRem"src/main/webapp/app/overview/course-conversations/layout/conversation-header/conversation-header.component.ts (1)
Line range hint
98-113
: Consider enhancing modal subscription cleanup.While the component generally handles memory leaks well, the modal subscriptions in
openAddUsersDialog
andopenConversationDetailDialog
could benefit from additional cleanup.Consider storing and cleaning up modal references:
+ private modalRef?: NgbModalRef; openAddUsersDialog(event: MouseEvent) { event.stopPropagation(); - const modalRef: NgbModalRef = this.modalService.open(ConversationAddUsersDialogComponent, defaultFirstLayerDialogOptions); + this.modalRef = this.modalService.open(ConversationAddUsersDialogComponent, defaultFirstLayerDialogOptions); - modalRef.componentInstance.course = this.course; + this.modalRef.componentInstance.course = this.course; // ... rest of the method } ngOnDestroy() { + if (this.modalRef) { + this.modalRef.close(); + } this.ngUnsubscribe.next(); this.ngUnsubscribe.complete(); }Also applies to: 117-134
src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-add-users-dialog/conversation-add-users-dialog.component.spec.ts (1)
44-44
: LGTM! Consider organizing test declarations for better maintainability.The removal of GroupChatIconComponent aligns with the PR changes. The test configuration is correct and follows the coding guidelines.
Consider extracting the declarations into a constant for better maintainability:
const TEST_DECLARATIONS = [ ConversationAddUsersDialogComponent, ConversationAddUsersFormStubComponent, MockPipe(ArtemisTranslatePipe), MockComponent(ChannelIconComponent), ]; TestBed.configureTestingModule({ declarations: TEST_DECLARATIONS, providers: [/*...*/], });src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.spec.ts (1)
Line range hint
94-154
: Add test coverage for new UI features.While the existing test cases are comprehensive, consider adding tests for the new UI features:
- Verify profile picture rendering in direct chats
- Validate group icon display
- Test conversation header profile picture visibility
Example test structure:
it('should show profile picture for direct chats', () => { const activeConversation = generateOneToOneChatDTO({}); initializeDialog(component, fixture, { course, activeConversation }); fixture.detectChanges(); const profilePicture = fixture.debugElement.query(By.css('.profile-picture')); expect(profilePicture).toBeTruthy(); }); it('should show group icon for group chats', () => { const activeConversation = generateExampleGroupChatDTO({}); initializeDialog(component, fixture, { course, activeConversation }); fixture.detectChanges(); const groupIcon = fixture.debugElement.query(By.css('fa-icon')); expect(groupIcon).toBeTruthy(); expect(groupIcon.componentInstance.icon).toBe('faPeopleGroup'); });src/main/webapp/app/shared/sidebar/sidebar.component.ts (1)
Line range hint
23-29
: Add JSDoc comments for improved documentation.Consider adding JSDoc comments to document the purpose of the new output and input properties, especially since they're part of the public API.
Example:
/** Emits when a direct chat is initiated */ onDirectChatPressed = output<void>(); /** Emits when a group chat is initiated */ onGroupChatPressed = output<void>(); /** Controls whether the component is used in communication context */ inCommunication = input<boolean>(false);src/test/javascript/spec/component/overview/course-conversations/layout/conversation-header/conversation-header.component.spec.ts (1)
30-30
: Consider using path aliases for cleaner imports.The import statement uses a long relative path with multiple parent directory references. Consider configuring TypeScript path aliases in
tsconfig.json
to make imports more maintainable.Example configuration:
// tsconfig.json { "compilerOptions": { "paths": { + "@shared/*": ["app/shared/*"] } } }
Then update the import:
-import { ProfilePictureComponent } from '../../../../../../../../main/webapp/app/shared/profile-picture/profile-picture.component'; +import { ProfilePictureComponent } from '@shared/profile-picture/profile-picture.component';
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (17)
src/main/webapp/app/entities/metis/conversation/one-to-one-chat.model.ts
(1 hunks)src/main/webapp/app/overview/course-conversations/course-conversations.module.ts
(0 hunks)src/main/webapp/app/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.html
(1 hunks)src/main/webapp/app/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.ts
(2 hunks)src/main/webapp/app/overview/course-conversations/layout/conversation-header/conversation-header.component.html
(1 hunks)src/main/webapp/app/overview/course-conversations/layout/conversation-header/conversation-header.component.ts
(6 hunks)src/main/webapp/app/overview/course-conversations/other/group-chat-icon/group-chat-icon.component.html
(0 hunks)src/main/webapp/app/overview/course-conversations/other/group-chat-icon/group-chat-icon.component.ts
(0 hunks)src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.html
(1 hunks)src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts
(3 hunks)src/main/webapp/app/shared/sidebar/sidebar.component.html
(1 hunks)src/main/webapp/app/shared/sidebar/sidebar.component.ts
(2 hunks)src/main/webapp/app/shared/sidebar/sidebar.module.ts
(2 hunks)src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-add-users-dialog/conversation-add-users-dialog.component.spec.ts
(1 hunks)src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.spec.ts
(2 hunks)src/test/javascript/spec/component/overview/course-conversations/layout/conversation-header/conversation-header.component.spec.ts
(3 hunks)src/test/javascript/spec/component/overview/course-conversations/other/group-chat-icon/group-chat-icon.component.spec.ts
(0 hunks)
💤 Files with no reviewable changes (4)
- src/main/webapp/app/overview/course-conversations/course-conversations.module.ts
- src/main/webapp/app/overview/course-conversations/other/group-chat-icon/group-chat-icon.component.html
- src/main/webapp/app/overview/course-conversations/other/group-chat-icon/group-chat-icon.component.ts
- src/test/javascript/spec/component/overview/course-conversations/other/group-chat-icon/group-chat-icon.component.spec.ts
✅ Files skipped from review due to trivial changes (1)
- src/main/webapp/app/shared/sidebar/sidebar.component.html
🧰 Additional context used
📓 Path-based instructions (12)
src/main/webapp/app/entities/metis/conversation/one-to-one-chat.model.ts (1)
src/main/webapp/app/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.html (1)
Pattern src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.ts (1)
src/main/webapp/app/overview/course-conversations/layout/conversation-header/conversation-header.component.html (1)
Pattern src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/overview/course-conversations/layout/conversation-header/conversation-header.component.ts (1)
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.html (1)
Pattern src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts (1)
src/main/webapp/app/shared/sidebar/sidebar.component.ts (1)
src/main/webapp/app/shared/sidebar/sidebar.module.ts (1)
src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-add-users-dialog/conversation-add-users-dialog.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/overview/course-conversations/layout/conversation-header/conversation-header.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
🔇 Additional comments (18)
src/main/webapp/app/entities/metis/conversation/one-to-one-chat.model.ts (1)
19-24
: LGTM! Well-structured type conversion utility.
The function is well-implemented and serves its purpose in supporting the UI differentiation between chat types. It:
- Handles undefined cases safely
- Reuses the existing type guard
- Follows TypeScript best practices
- Aligns with the PR objective of improving chat type distinction
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts (2)
3-4
: LGTM: Imports are appropriate and follow guidelines.
The new imports support the chat type differentiation requirements while adhering to the Angular style guide.
24-24
: LGTM: Appropriate lifecycle hook usage.
The initialization of user data in ngOnInit follows Angular best practices.
src/main/webapp/app/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.ts (2)
10-10
: LGTM: Import statement follows Angular style guide
The Font Awesome icon import is correctly placed and aligns with the PR objective of improving visual distinction between chat types.
28-28
: LGTM: Property declaration follows best practices
The faPeopleGroup
property is correctly declared as readonly and follows Angular naming conventions.
src/main/webapp/app/shared/sidebar/sidebar.module.ts (1)
19-19
: LGTM! Import statement follows Angular guidelines.
The import statement follows proper conventions with absolute paths and PascalCase naming.
src/main/webapp/app/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.html (2)
Line range hint 1-93
: Well-structured template with proper Angular syntax usage.
The template demonstrates excellent practices:
- Consistent usage of new Angular syntax (@if, @switch)
- Clear separation between different conversation types
- Proper initialization check
- Well-organized sections for header, tabs, and content
9-10
: Verify icon size consistency across different chat types.
The Font Awesome icon replacement looks good and aligns with the PR's objective of improving visual distinction. However, let's ensure the size="xs"
matches the size of other icons (channel icon) in the header for visual consistency.
✅ Verification successful
Icon size is consistent with other similar use cases
The size="xs"
attribute for the group chat icon is consistent with other icons used in similar contexts within the conversation components. Specifically:
- The same
size="xs"
is used for icons in the conversation member row component - The group chat icon in the conversation header also uses
size="xs"
- Larger sizes are only used for special cases like loading spinners (
3x
) and empty state icons (5x
)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other conversation icons use consistent size attributes
# Expected: All icon size attributes should match for visual consistency
# Search for icon size attributes in conversation-related components
rg -g '*.html' 'size="[^"]*".*(?:fa-icon|channel-icon)'
Length of output: 287
Script:
#!/bin/bash
# Let's search more specifically in the conversation-related components
rg -g '*conversation*.html' '<fa-icon' -A 1 -B 1
# Also check for channel-icon usage
rg -g '*conversation*.html' '<channel-icon' -A 1 -B 1
Length of output: 13365
src/main/webapp/app/overview/course-conversations/layout/conversation-header/conversation-header.component.html (1)
16-18
: LGTM! Verify icon consistency across the application.
The group chat icon implementation using Font Awesome is appropriate and follows the new Angular syntax guidelines.
✅ Verification successful
Icon usage is consistent across the application ✓
The faPeopleGroup
icon is consistently used across the application for group chat representation:
- Conversation header component for active conversations
- Conversation detail dialog for group chat details
- Sidebar component for group chat creation
- Sidebar card items for group chat entries
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify consistent usage of group chat icon across the application
rg -l "faPeopleGroup" --type ts
Length of output: 406
Script:
#!/bin/bash
# Check the actual usage of faPeopleGroup icon in the components
rg "faPeopleGroup" -A 2 -B 2 --type ts
Length of output: 6080
Script:
#!/bin/bash
# Check the HTML templates for consistent usage of the group chat icon
rg "faPeopleGroup" --type html -A 2 -B 2
Length of output: 2507
src/main/webapp/app/overview/course-conversations/layout/conversation-header/conversation-header.component.ts (3)
2-2
: LGTM: Import statements are well-organized.
The new imports support the feature requirements for differentiating chat types and handling user profile information.
Also applies to: 21-22
44-44
: LGTM: Properties are well-defined and properly typed.
The new properties follow Angular style guidelines and maintain type safety:
otherUser
for storing the other participant's information in one-to-one chatsreadonly faPeopleGroup
for the group chat icon
Also applies to: 50-50
94-94
: LGTM: Subscription handling is correct.
The getOtherUser()
call is properly integrated into the existing subscription, and memory leak prevention is maintained through the use of takeUntil
.
src/test/javascript/spec/component/overview/course-conversations/dialogs/conversation-detail-dialog/conversation-detail-dialog.component.spec.ts (2)
4-4
: LGTM: Import changes align with icon replacement.
The addition of FontAwesomeModule import and removal of GroupChatIconComponent are consistent with the PR's objective to update the icon system.
86-86
: LGTM: TestBed configuration properly updated.
The TestBed configuration correctly includes FontAwesomeModule and follows Angular testing best practices.
src/main/webapp/app/shared/sidebar/sidebar.component.ts (3)
2-2
: LGTM: Icon change aligns with UI enhancement goals.
The replacement of faUsers
with faPeopleGroup
provides better visual distinction for group chats, consistent with the PR objectives.
60-60
: LGTM: Icon property follows Angular guidelines.
The faPeopleGroup
readonly property is properly initialized and follows naming conventions.
Line range hint 33-36
: Verify backward compatibility of input/output changes.
The changes to use modern Angular syntax (output<void>()
and required inputs) improve type safety but may affect existing code.
Also applies to: 45-45, 52-52
✅ Verification successful
Based on the search results, I can now generate a final response as I have found all the component usages and can verify the impact of the changes.
Input/output changes are safe and properly handled in all usages.
The component usages show that:
- All instances provide the required inputs properly
- The new
channelTypeIcon
property is only used in the communication context where it's explicitly provided - The
itemSelected
boolean is correctly used across different contexts (exercises, lectures, exams, etc.) - The
courseId
input is consistently provided in all parent components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing usages of the component to ensure they provide required inputs
ast-grep --pattern 'jhi-sidebar $$$'
Length of output: 162
Script:
#!/bin/bash
# Search for component usages with ripgrep for better encoding handling
rg -l "<jhi-sidebar" --type html
# Then get content of any found files
for file in $(rg -l "<jhi-sidebar" --type html); do
echo "=== $file ==="
cat "$file"
done
Length of output: 21576
src/test/javascript/spec/component/overview/course-conversations/layout/conversation-header/conversation-header.component.spec.ts (1)
54-54
: LGTM!
The test module setup correctly uses NgMocks to mock the ProfilePictureComponent, following the coding guidelines.
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts
Show resolved
Hide resolved
...pp/overview/course-conversations/layout/conversation-header/conversation-header.component.ts
Show resolved
Hide resolved
...erview/course-conversations/layout/conversation-header/conversation-header.component.spec.ts
Show resolved
Hide resolved
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts (1)
40-50
: Improve component architecture and method organization.The current implementation has several architectural concerns:
- The method is marked as private in the code but implemented as public
- It violates Single Responsibility Principle by handling both user extraction and icon assignment
- The icon assignment could be more reactive
Consider splitting the responsibilities:
- extractMessageUser(): void { + private extractMessageUser(): void { if (this.sidebarItem.type === 'oneToOneChat' && (this.sidebarItem.conversation as OneToOneChatDTO)?.members) { this.otherUser = (this.sidebarItem.conversation as OneToOneChatDTO).members!.find((user) => !user.isRequestingUser); } else { this.otherUser = null; } + } + get sidebarIcon(): any { if (this.sidebarItem.type === 'groupChat') { - this.sidebarItem.icon = this.faPeopleGroup; + return this.faPeopleGroup; } - } + return this.sidebarItem.icon; + }Then update the template to use:
[icon]="sidebarIcon"src/test/javascript/spec/component/shared/sidebar/sidebar-card-item.component.spec.ts (3)
26-38
: Consider using TypeScript interface for mock data.While the mock data is well-structured, consider defining a proper interface instead of using
any
type for better type safety and maintainability.interface SidebarItemMock { title: string; id: string; size: SidebarCardSize; difficulty: DifficultyLevel; type: string; conversation: OneToOneChatDTO; }
69-74
: Improve group chat icon test robustness.The test could be more robust with a proper setup and specific assertions.
it('should set group icon for group chats in extractMessageUser', () => { - component.sidebarItem.type = 'groupChat'; - component.sidebarItem.icon = undefined; + component.sidebarItem = { + ...sidebarItemMock, + type: 'groupChat', + }; component.extractMessageUser(); - expect(component.sidebarItem.icon).toBe(faPeopleGroup); + expect(component.sidebarItem.icon).toEqual(faPeopleGroup); });
76-79
: Enhance one-to-one chat test reliability.The test could be more reliable with explicit member selection and specific assertions.
it('should set otherUser for one-to-one chat in extractMessageUser', () => { + const expectedOtherUser = sidebarItemMock.conversation.members.find(m => !m.isRequestingUser); component.extractMessageUser(); - expect(component.otherUser).toEqual(sidebarItemMock.conversation.members[1]); + expect(component.otherUser).toEqual(expectedOtherUser); + expect(component.otherUser.name).toBe('User1'); + expect(component.otherUser.isRequestingUser).toBeFalse(); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts
(3 hunks)src/test/javascript/spec/component/shared/sidebar/sidebar-card-item.component.spec.ts
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts (1)
src/test/javascript/spec/component/shared/sidebar/sidebar-card-item.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
🔇 Additional comments (4)
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts (1)
3-4
: LGTM: Clean imports and icon declaration.
The imports are well-organized and the FontAwesome icon is correctly declared as readonly.
Also applies to: 18-18
src/test/javascript/spec/component/shared/sidebar/sidebar-card-item.component.spec.ts (3)
6-10
: LGTM! Imports follow best practices.
The new imports are properly organized and the use of MockComponent
from ng-mocks aligns with our testing guidelines.
19-21
: LGTM! Proper component mocking setup.
The TestBed configuration correctly uses MockComponent
for ProfilePictureComponent
as per our testing guidelines.
49-51
: LGTM! Proper test assertion.
The test uses specific assertion (toContain
) as per our testing guidelines.
src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.ts
Show resolved
Hide resolved
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.
Tested on TS1 and works as expected. Nice change!
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.
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.
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.
Tested on TS1, works as described.
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.
Code LGTM. Tested on TS1, looks great 👍
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.
Checklist
General
Client
Motivation and Context
Description
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
updated conversation header
updated sidebar elements
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores