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

Attendance of students by AJAX call #113

Merged
merged 8 commits into from
Feb 28, 2021

Conversation

sagarpandyansit
Copy link
Contributor

Issue that resolved:
#82

Description of change:
I have changed the traditional way of submitting the form for attendance. I have made a function which triggers asynchronous request to change in the database for the attendance.

@sagarpandyansit
Copy link
Contributor Author

@garg3133, could you help me to solve this linting error?

# path('attendance/view/', views.view_attendance, name='view_attendance'),
path('profile/update/<int:pk>/', views.update_profile, name='update_profile'), # Not needed, update in profile/ only

path('update_from_sheets/', views.update_from_sheets, name='update_from_sheets'),

# AJAX calls
path('ajax/ajax_attendance_fetch_students/', views.ajax_attendance_fetch_students, name='ajax_attendance_fetch_students'),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't repeat ajax in the endpoint. ajax/fetch_students/ and ajax/mark_attendance should work fine. Replace / with _ for view name and URL namespace.

def ajax_student_attendance(request):
stu_id = request.GET['std_id']
stu_att = StudentAttendance.objects.get(student__id=stu_id, cal_date=today_date)
stu_att.present = not stu_att.present
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't just toggle the attendance, be sure whether you want to mark the student present or absent. Bring in one more key-value pair for this in GET request. (Send True if the checkbox is checked and False if it is unchecked).

},
dataType: 'json',
success: function (data) {

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a success toast here showing that the attendance was marked successfully. You may refer b4715eb to see how I implemented toast in your previous PR.

Also, add the error parameter to in this AJAX and toggle the checkbox to it's original state if an error occurs along with showing a toast displaying the failure message.

@garg3133
Copy link
Owner

garg3133 commented Feb 26, 2021

@sagarpandyansit Leave the linting errors for now, just improve the code as suggested in the review above.

We'll handle the linting errors separately as the whole codebase needs to be lint properly.

@sagarpandyansit
Copy link
Contributor Author

sagarpandyansit commented Feb 26, 2021

@garg3133, Okay. There is a problem. as soon as we mark the attendance, the database is going to update but, what about the HTML page. It remains the same. for example stu_att.present is false initially. We mark the attendance of the student hence the database changed but the variable in the HTML page remains the same means stu_att.present is false.

@garg3133
Copy link
Owner

@sagarpandyansit The stu_att.present variable in HTML is only used at the page load time so it should not create any problem. Is there any issue it is creating?

@sagarpandyansit
Copy link
Contributor Author

@garg3133, no issue with the creation. I am saying that the value of it is not changing as per the database. If it was false, it remains false until you reload the page. That is what I am saying.

@sagarpandyansit
Copy link
Contributor Author

It is not synchronous with the AJAX call. I made AJAX call for marking the attendance of the student. It works and made stu_att.present variable as true in the database but, in the HTML page the stu_att.present remains 'false`.

@garg3133
Copy link
Owner

Yeah, that's not a problem. These variables are only used at the reload time for rendering the HTML. Once the HTML is rendered completely, these variables are of no use on the page (unless you are using them in JavaScript). The value of there variables won't cause any problem on the page and will automatically get updated once the page reloads.

@sagarpandyansit
Copy link
Contributor Author

sagarpandyansit commented Feb 26, 2021

No. It will cause a problem. What if you click on that button again. Again you have to check whether stu_att.present is false or true, as you have denied that toggle variable solution. Now, we get the wrong information of that variable and that will cause the problem. Let me push the updated code for your reference.

@garg3133
Copy link
Owner

garg3133 commented Feb 26, 2021

No, you should not check the value of stu_att.present in JavaScript but you should check the current state of that checkbox (you can pass the id of that checkbox as well, as an argument to the function). If the current state of the checkbox is checked, instruct the backend to mark the student as present, otherwise mark the student as absent.

Also, if the AJAX fails for some reason, toggle the checkbox back to its original state (but make your it doesn't get stuck in a loop).
This might help: https://forum.sencha.com/forum/showthread.php?254484-How-to-set-checkbox-value-without-triggering-change-event&p=1043407&viewfull=1#post1043407

https://stackoverflow.com/a/5962581/9890886

@sagarpandyansit
Copy link
Contributor Author

sagarpandyansit commented Feb 27, 2021

@garg3133, kindly review the AJAX call now. It's working fine without suspending the event. I will remove the submit button and make changes accordingly.

Copy link
Owner

@garg3133 garg3133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working great @sagarpandyansit, thanks for working on this.

Below are the few changes required. Plus, you can now remove the "Submit" button as you said above.

@@ -173,14 +172,22 @@ def ajax_attendance(request):
return JsonResponse(data)


def ajax_mark_attendance(request):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the two decorators above this function. (Refer to the other ajax_fetch_students function above).

<input type="checkbox" name="attended" value="{{ stu_att.student.id }}"
class="custom-control-input"
id="s{{ stu_att.id }}"
onchange="attendance({{ stu_att.student.id }}, {{ stu_att.id }})"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass the arguments as strings here. Also, pass the id of the checkbox instead of stu_att.id.

Try this: onchange="attendance('{{ stu_att.student.id }}', 's{{ stu_att.id }}')"

stu_att_table += '<tr><td>' + name + '</td><td>' + school_class + '</td> \
<script>
function attendance(student_id, checkboxID) {
let student = document.getElementById('s' + checkboxID);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable name student doesn't seem correct here. You should use a more descriptive name like att_checkbox instead.

dataType: 'json',
success: function (data, status) {
$('#alert-toast-div > div').html(
`<div class="toast" id="alert_toast" role="alert" aria-live="assertive" aria-atomic="true" data-delay="3500">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the data-delay to 500. As this toast is going to appear again and again, we don't need to show it for 3.5 sec.

$('#alert-toast-div > div').html(
`<div class="toast" id="alert_toast" role="alert" aria-live="assertive" aria-atomic="true" data-delay="3500">
<div class="toast-body p-2 px-3">
<strong class="mr-auto">Attendance marked successfully.</strong>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the text here to "Attendence updated".

$("#alert_toast").addClass("bg-success");
},
error: function (data, status) {
is_present ? student.checked = false : student.checked = true;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment before this line: "// Un-toggle the checkbox".

@garg3133 garg3133 linked an issue Feb 27, 2021 that may be closed by this pull request
@garg3133
Copy link
Owner

@sagarpandyansit Any updates on this? The program is going to end in about 4 hrs and I don't want you to miss your points on this PR.

@sagarpandyansit
Copy link
Contributor Author

sagarpandyansit commented Feb 28, 2021

@garg3133, I have viva tomorrow though I tried. I don't know why but, I can't schedule a class of students for today as well. They don't appear in the list for attendance. Please check, maybe it's a bug.
You may accept it with the SWoC label and merge it later. It's completely okay if you don't want to do this. I will contribute after SWoC also because I like the concept of this project and want to contribute whatever I can.

@garg3133
Copy link
Owner

garg3133 commented Feb 28, 2021

As I had already made most of the requested changes on my local system while reviewing this PR, I'm merging this PR with those after applying those changes in 8a90fb5. I've opened a new issue #118 for the remaining changes and I'm assigning it to you.

Also, I really appreciate your willingness to contribute to this project even after SWOC and all the best for your viva 👍

@garg3133 garg3133 merged commit 1385651 into garg3133:master Feb 28, 2021
@sagarpandyansit
Copy link
Contributor Author

Thanks @garg3133. I will start working on that issue after the viva phase.

@garg3133 garg3133 mentioned this pull request Mar 7, 2021
2 tasks
@sagarpandyansit sagarpandyansit deleted the attendance branch March 10, 2021 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Take student attendance using AJAX
2 participants