Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Allow editors to propose FAQ #9457Communication
Allow editors to propose FAQ #9457Changes from 5 commits
0544837
280e3f4
a7dbc99
2eb7316
2be8182
a1c3562
0ab7a91
47e51cc
edf208e
491e368
8045a7f
29add81
ebc28b2
a893024
3154064
87d73ec
db90e89
d8c99a2
2259241
0139625
6deb17c
d801ef8
330a677
1f6cc87
8720762
7beeea6
ba80f4e
059a0b5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
🧹 Nitpick (assertive)
Display user-friendly labels for FAQ states
Currently, the FAQ state is displayed using
faq.faqState
, which may show the raw enum value (e.g.,PROPOSED
,APPROVED
). To improve user experience, consider displaying a user-friendly label or translating the state.You can apply this change to use the Angular
translate
pipe with a translation key:Ensure that you have the corresponding entries in your translation files, such as:
Let me know if you need assistance in implementing this change or updating the translation files.
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.
🧹 Nitpick (assertive)
Consider allowing tutors to delete their own proposed FAQs
Currently, only instructors can delete FAQs. To enhance usability and allow tutors to manage their own content, consider permitting tutors to delete their own proposed FAQs before they are reviewed by an instructor.
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.
🛠️ Refactor suggestion
Refactor
rejectFaq
andacceptProposedFaq
to reduce code duplicationThe methods
rejectFaq
andacceptProposedFaq
have similar logic. To adhere to the DRY (Don't Repeat Yourself) principle, consider refactoring them into a single reusable method.You can create a generic method to handle FAQ state updates:
Then, refactor the original methods to utilize this new function:
This refactoring enhances maintainability and simplifies future updates.
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.
🧹 Nitpick (assertive)
LGTM! Consider adding error handling.
The new
findAllByCourseIdAndState
method is well-implemented and follows Angular best practices. It maintains consistency with other methods in the service, promotes code reuse, and adheres to type safety.Consider adding error handling to improve robustness:
This addition will log any errors and provide a user-friendly error message.