-
Notifications
You must be signed in to change notification settings - Fork 5
Session-wise student list #65
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
base: main
Are you sure you want to change the base?
Conversation
| try: | ||
| table = self.__db.Table("student_quiz_reports") | ||
| all_items = [] | ||
| last_evaluated_key = None |
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.
What is this variable for?
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.
Earlier I used a simple query approach, but that was returning incomplete list of students for many session id's that I tested with. I searched and figured out that this a pagination issue. So, I used the pagination approach as mentioned here: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Query.Pagination.html
app/requirements.txt
Outdated
| # urllib3==1.26.10 | ||
| # uvicorn==0.18.2 | ||
| # virtualenv==20.24.5 | ||
| annotated-types |
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.
Why this change? Let's not mess with requirements.txt
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.
Changing this. I had to add modify some dependencies for running on my local. Failed to remove them
app/routers/student_quiz_reports.py
Outdated
| {"request": request, "report_data": report_data}, | ||
| ) | ||
|
|
||
| @api_router.get("/session_students/{session_id}") |
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.
This should not be in student_quiz_reports.py . Move it to session_quiz_reports since it is a session level report.
Also the route should be /session_report, not session_students
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.
Noted. Changing this.
app/routers/student_quiz_reports.py
Outdated
| "user_id": item["user_id-section"].split("#")[0], | ||
| "marks_scored": item.get("marks_scored", 0), | ||
| "percentage": item.get("percentage", 0), | ||
| "rank": item.get("rank", None), |
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.
Add a report link here too.
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 have already added links in the html file. The student id links to the individual students reports.
| @@ -0,0 +1,129 @@ | |||
| {% extends "layout.html" %} | |||
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.
Rename to session_report.html
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.
Noted.
Summary of changes:
-Test using reports/session_students/{session_id}