-
Notifications
You must be signed in to change notification settings - Fork 3
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 N5 and fully implement question integration to collab service #53
Conversation
Mahidharah
commented
Nov 6, 2024
- Implement genAI prompting on the backend
- Fix question service functions to return mongodbID instead of question title
- Update model to include mongoose Object as a parameter
- Update routing to collab to be neater
- Update collabUI to fetch question from question service before displaying question details
…tails from the backend correctly
Frontend URL: |
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.
Let's get this merged! Just in time for the demo.
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.
Fantastic implementation
@@ -907,6 +907,15 @@ asynckit@^0.4.0: | |||
resolved "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz" | |||
integrity sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q== | |||
|
|||
axios@^1.7.7: |
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.
Good idea to use axios v1.7.7 here
@@ -52,7 +52,7 @@ const App: React.FC = () => { | |||
<Route path="/collab" element={<CollaborationServiceView topic={''} difficulty={''} sessionId={''} />} /> | |||
|
|||
{/* Public Route for Collaboration Service Integrated View */} | |||
<Route path="/collabFull/:topic/:difficulty/:questionId/:sessionId" element={<CollaborationServiceIntegratedView />} /> |
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.
Good choice, it wasn't feasible to have the difficulty and topic in the url
}; | ||
|
||
export const getQuestionById = async (id: string): Promise<Question> => { |
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.
Great idea to return a Question object asynchronously instead of just the string
|
||
|
||
const CollaborationServiceIntegratedView: React.FC = () => { | ||
const { topic, difficulty, questionId, sessionId } = useParams<{ topic: string; difficulty: string; questionId: string; sessionId: string; }>(); |
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.
LGTM
@@ -41,12 +78,12 @@ const CollaborationServiceIntegratedView: React.FC = () => { | |||
}; | |||
|
|||
useEffect(() => { | |||
console.log(`Session ID: ${sessionId}, Topic: ${topic}, Difficulty: ${difficulty}`); | |||
console.log(`Session ID: ${sessionId}, Topics: ${topics}, Difficulty: ${difficulty}`); |
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.
Nice catch
<h2>Collaboration Session</h2> | ||
<p>Topic: {topic} | Difficulty: {difficulty} | Session: {sessionId}</p> | ||
<p>Question: {questionId}</p> | ||
<h3>Collaboration Session</h3> |
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.
Good idea to change the Collab Session title to h3, h2 was a bit too big anyway
@@ -75,7 +75,8 @@ const MatchingServiceMainView: React.FC = () => { | |||
console.log(data.message); | |||
setMatchFound(true); // Set match status to true | |||
stopProgressBar(); | |||
setStatusMessage(`Match found for ${topic} and ${difficulty}!`); | |||
//setStatusMessage(`Match found for ${topic} and ${difficulty}!`); |
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.
Just a nit, might want to remove this. But LGTM otherwise