-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: show assignment information to students #22
Conversation
mindmap/mindmap.py
Outdated
StudentModule: A StudentModule object | ||
""" | ||
# pylint: disable=no-member | ||
student_module, created = StudentModule.objects.get_or_create( |
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 are we using StudentModule objects directly?
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.
edx-sga does something similar. Could we implement it in a better way?
@BryanttV I don't think is a good approach to load the StudentModule object directly as it's not a good practice. All those implementations are behind the XBlock abstractions. If your final goal is to store the state of the submission and show it to students, you can use some of the available scopes that will be best suited to your implementation. In the case none of the scopes is suitable for your purpose, try to compute the value your are searching for instead of storing it |
mindmap/mindmap.py
Outdated
module = self.get_student_module(data.get("module_id")) | ||
state = json.loads(module.state) | ||
state["submission_status"] = SubmissionStatus.SUBMITTED.value | ||
module.state = json.dumps(state) | ||
module.save() |
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.
Can we move this to a function? These lines are very similar the ones above
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.
Done
@Ian2012: I agree that there must be a better way of doing this, but we don't have much more time left to invest in this. So it's better to use what we know works and integrates well with what we already have. |
mindmap/public/js/src/mindmap.js
Outdated
const data = { | ||
student_id: student_id, | ||
module_id: module_id, | ||
}; | ||
console.log(data); |
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.
Please remove this
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.
Done
@@ -295,9 +298,11 @@ function MindMapXBlock(runtime, element, context) { | |||
.click(function () { |
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 would create a constant called xblockId
and would replace all these for xblockId
instead of having context.xblock_id
const xblockId = context.xblock_id;
const grade = $(element).find(`#grade-input_${xblockId}`).val();
const submission_id = $(element).find(`#submission-id-input_${xblockId}`).val();
const module_id = $(element).find(`#module-id-input_${xblockId`).val();
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.
Done
mindmap/public/js/src/mindmap.js
Outdated
const data = { | ||
grade: grade, | ||
submission_id: submission_id, | ||
module_id: module_id, | ||
}; | ||
console.log(data); |
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.
Remove this
mindmap/public/js/src/mindmap.js
Outdated
@@ -313,8 +318,10 @@ function MindMapXBlock(runtime, element, context) { | |||
.find(`#remove-grade-button_${context.xblock_id}`) | |||
.click(function () { | |||
const student_id = $(element).find(`#student-id-input_${context.xblock_id}`).val(); |
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.
Same as here
For state management, we evaluated different solutions among them:
The second option we tried to implement, but it still has some edge cases that need to be evaluated in detail. The third option is still just an idea, but it has a caveat, when deleting the status of a student using the staff support tool, the update of this field must be fixed. Finally, we decided to use the first option since it is already implemented and works correctly. |
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 is working as expected!
c7bce9c
to
e758703
Compare
Description
This PR adds:
submitted
andcompleted
status.How To Test
submitted
andcompleted
status.Screenshots
As Student
As Instructor