Skip to content
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

Add Resolved/Unresolved Functionality to Topics #14

Merged
merged 11 commits into from
Feb 14, 2025

Conversation

Misterurias
Copy link
Contributor

@Misterurias Misterurias commented Feb 11, 2025

📝 Context

  • This PR introduces a resolved/unresolved feature for topics in NodeBB.
  • The purpose is to allow users to mark a topic as resolved when a question is answered and unresolved when it requires further discussion.
  • This improves thread management and makes it easier for users to find open/unanswered questions.

✅ Changes in the Codebase

  • Added API endpoint to set resolved/unresolved status
    • Modified: src/routes/api.js
    • Added: PUT topics/:tid/resolved route (end of file)
  • Added controller to handle resolved/unresolved status
    • Modified: src/controllers/topics.js
    • Added: setResolved() function to update the topic status (end of file)
  • Updated API logic to modify database
    • Modified: src/api/topics.js
    • Added: setResolved() function to flip resolved boolean status in Redis (end of file)
  • Updated Redis database schema to include resolved field
    • Modified: src/topics/create.js and public/openapi/components/schemas/TopicObject.yaml
    • Added resolved field (default is false) to topics data (beginning of file)
  • Modified: test/topics.js
    • Added Unit tests
    • Added Integration tests
    • Tests for:
      • Setting a topic as resolved
      • Unsetting a topic (marking it as unresolved)
      • Ensuring the database updates correctly

@Ginka3
Copy link
Contributor

Ginka3 commented Feb 13, 2025

Great work on adding the resolved/unresolved functionality! 🚀
Used the API endpoints and resolved states for marking topic as resolved branch already, no issues there. LGTM

@SubtoFelix
Copy link
Contributor

The PR description is very well-defined. There are proper API designs and database schema updates that are persistent, queryable, and optimized. Additionally, there are appropriate units and integration tests that cover edge cases and ensure code quality and correctness. Overall, every features look great!

@sophiazhuu
Copy link

Amazing work!! Just ran through the test suite and it looks great. Excellent code changes that are clear, uniform, and correct. Tests are thorough and successful! LGTM :)

@Misterurias Misterurias merged commit 51118dd into main Feb 14, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants