Skip to content

Comments

Linh - Fix: FAQ Tool — Unanswered Question Logging Issue#1718

Closed
linh2020 wants to merge 1 commit intodevelopmentfrom
linh-fix-faq-tool
Closed

Linh - Fix: FAQ Tool — Unanswered Question Logging Issue#1718
linh2020 wants to merge 1 commit intodevelopmentfrom
linh-fix-faq-tool

Conversation

@linh2020
Copy link
Contributor

@linh2020 linh2020 commented Sep 11, 2025

Description

image

Related PRS (if any):

To test this backend PR you need to checkout the #3118 PR.

Main changes explained:

  • Updated verifyToken to support both raw and Bearer tokens.
  • Improved duplicate detection: case-insensitive, trimmed comparison.
  • Logging now succeeds even if no Owner emails are found (email send is non-blocking).

How to test:

  1. Checkout this branch linh-fix-faq-tool.
  2. Run backend locally: npm install && npm run dev.
  3. Clear site data/cache
  4. Point frontend .env to local backend:
    REACT_APP_APIENDPOINT="http://localhost:4500/api"
  5. Log in as Dev Admin.
  6. Go to /faq, enter a new question → expect ✅ “Question logged successfully”.
  7. Try submitting the same question again → expect ❌ duplicate message.

Screenshots or videos of changes:

Fix.FAQ.Tool.Unanswered.Question.Logging.Issue.-.11.September.2025.mp4

Note:

Include the information the reviewers need to know.

@linh2020 linh2020 changed the title [WIP] Linh - Fix: FAQ Tool — Unanswered Question Logging Issue Linh - Fix: FAQ Tool — Unanswered Question Logging Issue Sep 12, 2025
@aditya2512
Copy link

This PR introduces technically sound enhancements to FAQ logging by broadening token support in verifyToken and refining duplicate question detection with case-insensitive, trimmed comparisons—a reliable approach that eliminates common UX pitfalls related to string inconsistencies. I appreciate making the email-sending step non-blocking; it ensures that logging remains robust even if no owner emails are present, and prevents user-facing errors tied solely to notification failures. These improvements strike a solid balance between usability and data integrity, and the step-by-step test instructions, along with video evidence, confirm the solution is well thought out. Before merging, just resolve the branch conflicts and confirm that the new authentication logic doesn’t introduce any regressions in protected endpoints.

@Anusha-Gali Anusha-Gali added the High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible label Feb 6, 2026
Copy link

@Anusha-Gali Anusha-Gali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Linh,

I have reviewed your PR locally and it works as per requirement in both the modes. However one small UI issue would be: The question is not that clearly visible in dark mode in http://localhost:5173/unanswered-faqs.
Image

Image Image Image Image Image

@linh2020 linh2020 closed this Feb 18, 2026
@linh2020
Copy link
Contributor Author

linh2020 commented Feb 18, 2026

Hi Linh,

I have reviewed your PR locally and it works as per requirement in both the modes. However one small UI issue would be: The question is not that clearly visible in dark mode in http://localhost:5173/unanswered-faqs. Image

Image Image Image Image Image

Hi Anusha,

Thank you for pointing that out.

The visibility issue in dark mode on /unanswered-faqs was caused by hardcoded light-theme text colors, which created low contrast against the dark background. The component styling has been updated to use theme-aware colors so the question text, timestamp, and card styles adapt correctly in both themes.

The fix has been pushed in a new PR: linh_fix_unanswered_faq_dark_mode_ui_1

Please pull and test in dark mode — the question text should now be clearly readable.
Thanks again for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants