-
Notifications
You must be signed in to change notification settings - Fork 301
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
Development
: Cleanup legacy and unused client code in the communication feature
#8842
Conversation
WalkthroughThe recent updates primarily focus on removing unused components and services in the Angular application. Specifically, several components were excluded from the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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 comments (8)
src/main/webapp/app/overview/course-conversations/dialogs/channels-overview-dialog/channels-overview-dialog.component.ts (2)
Line range hint
98-98
: Remove forbidden non-null assertions.Using non-null assertions (!) bypasses TypeScript's strict null checks, which can lead to runtime errors if not handled carefully. Consider using optional chaining or proper error handling to ensure robustness.
- this.course.id! + this.course?.idAlso applies to: 107-107, 134-134
Line range hint
118-126
: Change to an optional chain.To improve code safety and readability, replace the non-null assertion with an optional chain. This change prevents potential runtime errors due to null or undefined values.
- channels?.filter((channel) => channel.subType === this.channelSubType) ?? [] + channels?.filter((channel) => channel.subType === this.channelSubType)src/main/webapp/app/shared/metis/metis.service.ts (3)
Line range hint
64-64
: Avoid non-null assertions.The use of non-null assertions in multiple places in this service could lead to potential runtime errors if the assumptions about non-null values prove incorrect. Refactor these to use safer access patterns or add necessary checks.
- this.courseId = course.id!; + this.courseId = course?.id;Also applies to: 143-143, 178-178, 181-181, 203-203, 223-223, 234-234, 250-250, 270-270, 277-277, 293-293, 331-331, 348-348, 357-357, 374-374, 382-382
Line range hint
380-380
: Use strict equality checks.Replace the loose equality check (==) with strict equality (===) to avoid potential bugs due to type coercion.
- r.id == reaction.id + r.id === reaction.id
Line range hint
417-419
: Remove unnecessary else clause.The else clause is redundant and can be omitted to simplify the flow of control in the method.
- } else { - return; - }src/test/javascript/spec/service/metis/metis.service.spec.ts (3)
Line range hint
166-166
: Avoid non-null assertion.Using non-null assertions can lead to potential runtime errors. Consider using optional chaining or proper error handling.
- post.id! + post.id
Line range hint
315-315
: Replace non-null assertions with safer alternatives.Non-null assertions are used without checking for nullity, which might lead to errors. Use the optional chaining operator for safer access.
- answerPost.id! + answerPost.id - post.answers! + post.answersAlso applies to: 316-316
Line range hint
386-386
: Multiple non-null assertion issues.Several non-null assertions are used throughout the reaction service methods. Replace these with optional chaining to prevent potential runtime errors.
- reaction.id! + reaction.id - post.reactions! + post.reactions - metisPostInChannel.conversation!.id! + metisPostInChannel.conversation?.id - metisChannel.id + metisChannel.id - metisLectureChannelDTO.id + metisLectureChannelDTO.id - metisPostInChannel.content! + metisPostInChannel.contentAlso applies to: 399-399, 411-411, 424-424, 428-428, 429-429
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.
As far as I checked the code and testing locally nothing seems to have broken
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
319228d
# Conflicts: # src/main/webapp/app/overview/course-conversations/layout/conversation-selection-sidebar/conversation-sidebar-section/conversation-sidebar-section.component.ts
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.
Removal looks good to me
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.
Adaptions look good to me
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 locally, everything looks fine
Communication
: Cleanup legacy and unused code Development
: Cleanup legacy and unused client code in the communication feature
Client
Motivation and Context
This PR addresses the removal of unused and legacy code within the communications module, including old Sidebar files.
Steps for Testing
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.
Code Review
Summary by CodeRabbit
Refactor
ChannelsOverviewDialogComponent
by removing unnecessary service dependencies.MetisService
by removing unused methods and reordering existing ones.Tests
MetisService
test suite to remove references to outdated configurations and simplify tags update logic.