Communication: Fix open a new direct chat immediately after forwarding a message to a new user
#11623
Conversation
… reload sidebar after forwarding a message
github-actions
bot
added
client
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
…r forwarding a message
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
Senan04
changed the title
Senan04
changed the title
Communication: Fix open a new direct chat immediately after forwarding a message to a new user
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
WalkthroughA new sidebar reload mechanism is implemented in CourseSidebarService with an EventEmitter. When message forwarding completes in PostingReactionsBar, reloadSidebar() is called, triggering an event that CourseConversations component listens to, refreshing sidebar data via prepareSidebarData(). Changes
Sequence DiagramsequenceDiagram
actor User
participant PBComp as PostingReactionsBar
participant Service as CourseSidebarService
participant CCComp as CourseConversations
User->>PBComp: Forward message
PBComp->>PBComp: createForwardedMessages()
PBComp->>Service: reloadSidebar()
Service->>Service: reloadSidebar$.emit()
Service-->>CCComp: reloadSidebar$ event
CCComp->>CCComp: prepareSidebarData()
CCComp-->>User: Sidebar updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (4)
src/main/webapp/app/communication/shared/course-conversations/course-conversations.component.ts (1)
165-165: Sidebar reload subscription is correct; optional consistency refactorSubscribing to
courseSidebarService.reloadSidebar$and callingprepareSidebarData()(with proper unsubscribe inngOnDestroy) cleanly wires the new reload behavior into the component and fulfills the bugfix objective.If you want to align with the rest of the component’s Rx usage, you could alternatively expose
reloadSidebar$as anObservable<void>and subscribe viapipe(takeUntil(this.ngUnsubscribe))instead of maintaining a separateSubscriptionfield. Not required for this PR.Also applies to: 258-260, 383-383, 504-513
src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.ts (1)
34-35: Sidebar reload hook after forwarding is correct; minor optimization possibleInjecting
CourseSidebarServiceand callingreloadSidebar()in thecreateForwardedMessagescompletion handler is a good way to ensure the new DM appears immediately in the sidebar.Note that when a post is forwarded to multiple conversations,
forwardPostwill trigger a full sidebar reload once per conversation. If this ever becomes a hot path, you could wrap the forwards in a single aggregation (e.g.forkJoinor a counter) and callreloadSidebar()only once after all forwards complete.Also applies to: 151-151, 586-592
src/main/webapp/app/communication/shared/course-conversations/course-conversations.component.spec.ts (1)
363-370: Reload integration test is valid; could be tightened if desiredSpying on
prepareSidebarDataand asserting it is called aftercourseSidebarService.reloadSidebar()verifies the new subscription works as intended.If you ever want more precision, you could call
fixture.detectChanges()first and thenprepareSidebarDataSpy.mockClear()before triggeringreloadSidebar()to assert only the reload emission, but the current test is already acceptable.src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.ts (1)
47-47: Sidebar reload behavior after forwarding is well-covered in testsMocking
CourseSidebarServiceviaMockProvider, spying onreloadSidebar, and asserting it is called whenforwardPostcompletes gives good coverage of the new behavior.Given
createForwardedMessagesSpyis stubbed withof(null), you could dropfakeAsync/tick()in the last test, but it’s not required for correctness.Also applies to: 55-57, 94-95, 102-104, 730-742
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.ts(3 hunks)src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.ts(3 hunks)src/main/webapp/app/communication/shared/course-conversations/course-conversations.component.spec.ts(1 hunks)src/main/webapp/app/communication/shared/course-conversations/course-conversations.component.ts(3 hunks)src/main/webapp/app/core/course/overview/services/course-sidebar.service.spec.ts(1 hunks)src/main/webapp/app/core/course/overview/services/course-sidebar.service.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/core/course/overview/services/course-sidebar.service.tssrc/main/webapp/app/communication/shared/course-conversations/course-conversations.component.tssrc/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.tssrc/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.tssrc/main/webapp/app/communication/shared/course-conversations/course-conversations.component.spec.tssrc/main/webapp/app/core/course/overview/services/course-sidebar.service.spec.ts
🧠 Learnings (7)
📚 Learning: 2025-09-01T10:20:40.706Z
Learnt from: Michael-Breu-UIbk
Repo: ls1intum/Artemis PR: 10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:132-149
Timestamp: 2025-09-01T10:20:40.706Z
Learning: In the Artemis codebase, Angular component test files for ProgrammingExerciseDetailComponent follow a pattern where the component is imported but not explicitly declared in TestBed.configureTestingModule(), yet TestBed.createComponent() still works successfully. This pattern is consistently used across test files like programming-exercise-detail.component.spec.ts and programming-exercise-detail.component.with-sharing.spec.ts.
Applied to files:
src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.tssrc/main/webapp/app/communication/shared/course-conversations/course-conversations.component.spec.ts
📚 Learning: 2024-10-10T11:42:23.069Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9443
File: src/test/javascript/spec/component/hestia/git-diff-report/git-diff-modal.component.spec.ts:55-60
Timestamp: 2024-10-10T11:42:23.069Z
Learning: In `git-diff-report-modal.component.spec.ts`, using `fakeAsync` and `tick` does not work for handling asynchronous operations in the tests; alternative methods are needed.
Applied to files:
src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.tssrc/main/webapp/app/communication/shared/course-conversations/course-conversations.component.spec.ts
📚 Learning: 2024-10-20T22:00:52.335Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9505
File: src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts:179-181
Timestamp: 2024-10-20T22:00:52.335Z
Learning: In `src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts`, `ResizeObserver` is mocked within the `beforeEach` block.
Applied to files:
src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.tssrc/main/webapp/app/communication/shared/course-conversations/course-conversations.component.spec.tssrc/main/webapp/app/core/course/overview/services/course-sidebar.service.spec.ts
📚 Learning: 2024-10-13T12:03:02.430Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9463
File: src/test/javascript/spec/component/shared/monaco-editor/monaco-editor.service.spec.ts:50-55
Timestamp: 2024-10-13T12:03:02.430Z
Learning: In `src/test/javascript/spec/component/shared/monaco-editor/monaco-editor.service.spec.ts`, when a function is called multiple times in a test, use `toHaveBeenCalledTimes` and `toHaveBeenNthCalledWith` assertions instead of `toHaveBeenCalledExactlyOnceWith`.
Applied to files:
src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.tssrc/main/webapp/app/core/course/overview/services/course-sidebar.service.spec.ts
📚 Learning: 2025-08-21T17:30:20.530Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/drag-and-drop-question/drag-and-drop-question.component.spec.ts:34-34
Timestamp: 2025-08-21T17:30:20.530Z
Learning: FitTextDirective in src/main/webapp/app/quiz/shared/fit-text/fit-text.directive.ts is a standalone directive marked with standalone: true, so it should be imported in TestBed imports array, not declarations array.
Applied to files:
src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.ts
📚 Learning: 2025-05-31T23:25:34.020Z
Learnt from: eylulnc
Repo: ls1intum/Artemis PR: 10944
File: src/main/webapp/app/communication/message/message-reply-inline-input/message-reply-inline-input.component.spec.ts:177-190
Timestamp: 2025-05-31T23:25:34.020Z
Learning: In MessageReplyInlineInputComponent tests, the manual call to component['loadDraft']() after ngOnInit() is necessary because ngOnInit() starts an async loadCurrentUser() operation that only calls loadDraft() after the account identity is resolved. The manual call ensures the draft loading logic is tested directly, bypassing async timing dependencies.
Applied to files:
src/main/webapp/app/communication/posting-reactions-bar/posting-reactions-bar.component.spec.tssrc/main/webapp/app/communication/shared/course-conversations/course-conversations.component.spec.ts
📚 Learning: 2025-09-01T13:47:02.624Z
Learnt from: Michael-Breu-UIbk
Repo: ls1intum/Artemis PR: 10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:167-169
Timestamp: 2025-09-01T13:47:02.624Z
Learning: In Jest tests, prefer using jest.spyOn() over direct method assignment for mocking service methods, as spies are automatically restored by Jest's cleanup mechanisms (jest.restoreAllMocks()), while direct assignment bypasses this system and can lead to test pollution where mocked methods affect subsequent tests.
Applied to files:
src/main/webapp/app/core/course/overview/services/course-sidebar.service.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/main/webapp/app/core/course/overview/services/course-sidebar.service.ts (1)
10-11: Reload event API fits existing sidebar service pattern
reloadSidebar$andreloadSidebar()mirror the existing close/open/toggle event emitters, keeping the API uniform and easy to consume from components and tests.Also applies to: 24-26
src/main/webapp/app/core/course/overview/services/course-sidebar.service.spec.ts (1)
35-39: Tests adequately cover the new reloadSidebar eventThe added specs correctly assert that
reloadSidebar()emits onreloadSidebar$and that subscribers receive the event, in line with the existing close/open/toggle tests and Jest spying practices.Also applies to: 45-46, 50-50, 55-56, 60-60
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
toukhi
left a comment
toukhi
left a comment
There was a problem hiding this comment.
Tested on TS4 and works as described
Checklist
General
Client
Motivation and Context
This change is required because when a user forwards a message to another user with whom they have no prior direct message conversation, the newly created DM chat is not immediately displayed in the left-hand navigation sidebar. This resulted in a poor user experience, forcing users to manually refresh the page or course to see the new conversation tab.
Fixes #11513
Description
I have added a Method
reloadSidebartoCourseSidebarServicethat gets called byforwardPostinPostingReactionsbarComponentwhen the forwarded Message was succesfully created by themetisService.CourseConversationsComponentgets notified via its event subscription so the data gets refreshed and the sidebar updated accordingly.Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.