-
Notifications
You must be signed in to change notification settings - Fork 2
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
solomon/history-service #55
Conversation
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.
Thanks for setting up the CRUD operations for the history service, generally looks ok and I have left some suggestions.
apps/frontend/src/components/CollaborativeEditor/CollaborativeEditor.tsx
Outdated
Show resolved
Hide resolved
|
||
The server will be available at http://localhost:8082. | ||
|
||
## API Endpoints |
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.
I think we should also have the GET /userhistories/{username}
endpoint, which can be called on the profile page for the user to see their previous question attempts? What do you think?
Also to check my understanding on the history service, it is mainly used to store the user code submissions?
Implemented on the frontend:
POST /histories
: sent by the user when they submit their code for the first time.PUT /histories/{docRefId}
: sent by the user when they submit their code after the first time.
Not implemented on the frontend yet:
GET /histories/{docRefId}
: send by the user to check if a user has attempted a question before, so that a user can access their previous submissions on the question page.DELETE /histories/{docRefId}
: normally history should not be deleted, so is this used by an admin?
For the design of the user history, I have a suggestion:
- Instead of updating the user history directly from the client request, should we have the execution-service update the history-service instead? This means that to update the history, the code has to be submitted (and executed).
- This reduces the number of requests sent by the client
- This request can be asynchronous as we do not need to view the updated history immediately, and we can probably use a message queue for this.
- A trade-off is that potentially unnecessary information about the matching has to be sent along to the execution-service.
I think there are a few ways to expand on the history service:
- Store a user's matching history: the matching service can update another table in histories on the match details. on the user's profile page, they can view their past matchings
- Store a user's chat history
- Update the code changes even when not submitting: a savepoint every X minute/ upon user request to save or end/ continuously track changes
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.
Regarding the updates in routes
- Added 'GET /userhistories/{username}' and support on frontend
- Commented out 'DELETE /histories/{docRefId}', which could be assessed by admin if we would like that functionality in the future
- Added 'GET /histories/{docRefId}' support on frontend
Regarding execution-service updating the history-service
- Sounds good, will implement execution service with current design first, then modify to suggested design
Noted on the ways to expand the history service.
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.
Thanks @solomonng2001, I have approved, when you ready you can merge it.
GET /histories/{username}
=> Read user's matching/collaboration historyGET /histories/{docRefId}
=> Read historyPUT /histories/{docRefId}
=> CreateOrUpdate history