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.

Sign up for GitHub

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

Enhancement/wrobe0709/list unenrolled submissions #1128

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Enhancement/wrobe0709/list unenrolled submissions #1128

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2ce9e7c
Indicate is submission was made by a staff member in admin view
wrobe0709 Feb 26, 2017
2f2d4be
Merge remote-tracking branch 'upstream/master'
wrobe0709 Mar 5, 2017
ba5ea40
List students that have submissions but are unenrolled
wrobe0709 Mar 25, 2017
0cb91aa
Improve performance of finding un-enrolled submitters
wrobe0709 Mar 26, 2017
691be4a
Merge branch 'master' into enhancement/wrobe0709/list_unenrolled_subm…
colinschoen Dec 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions server/controllers/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ def assignment_stats(cid, aid):
return abort(401)

stats = Assignment.assignment_stats(assign.id)

submissions = [d for d in stats.pop('raw_data')]

pie_chart = pygal.Pie(half_pie=True, disable_xml_declaration=True,
Expand Down Expand Up @@ -941,7 +941,7 @@ def enrollment(cid):
students = current_course.get_students()
staff = current_course.get_staff()
lab_assistants = current_course.get_participants([LAB_ASSISTANT_ROLE])

return render_template('staff/course/enrollment/enrollment.html',
enrollments=students, staff=staff,
lab_assistants=lab_assistants,
Expand Down Expand Up @@ -1043,6 +1043,24 @@ def client(client_id):
return redirect(url_for(".clients"))
return render_template('staff/edit_client.html', client=client, form=form, courses=courses)

@admin.route("/course/<int:cid>/unenrolled", methods=['GET'])
@is_staff(course_arg='cid')
def unenrolled(cid):
courses, current_course = get_courses(cid)

submissions = set(b.submitter_id for b in (Backup.query.join(Backup.assignment).filter(Assignment.course_id == cid).all()))
enrollment = set(e.user_id for e in (Enrollment.query.filter(Enrollment.course_id == cid).all()))
Copy link
Member

Choose a reason for hiding this comment

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

These two lines are a bit too long. I would suggest putting the queries on their own lines

Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can use course.get_students here? It also does a joined load with Users - so you can just do e.user without the cost of another query

unenrolled_submitters = []

for s in submissions:
if s not in enrollment:
unenrolled_submitters.append(User.query.get(s))
Copy link
Member

Choose a reason for hiding this comment

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

See above - you should be able to do e.user if you use enrollment objects directly


return render_template('staff/course/enrollment/enrollment.unenrolled.html',
current_course=current_course,
unenrolled_submitters=unenrolled_submitters,
title="Submitter")

################
# Student View #
################
Expand Down
6 changes: 3 additions & 3 deletions server/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ def assignment_stats(assign_id, detailed=True):
'active_groups': active_groups,
'percent_groups_active': active_groups/(stats['groups'] or 1)
})

if detailed:
stats.update({
'raw_data': data
Expand Down Expand Up @@ -562,10 +562,10 @@ def course_submissions(self, include_empty=True):
current_db = db.engine.name
if current_db != 'mysql':
return self.course_submissions_slow(include_empty=include_empty)

# Can only run the fast query on MySQL
submissions = []

stats = self.mysql_course_submissions_query()
keys = stats.keys()
for r in stats:
Expand Down
6 changes: 6 additions & 0 deletions server/templates/staff/course/course.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ <h3 class="box-title">Quick Links</h3>
Enrollment
</a>
</li>
<li>
<a href="{{ url_for('.unenrolled', cid=current_course.id) }}">
<i class="fa fa-user-o"></i>
Un-Enrolled Submitters
</a>
</li>
<li>
<a href="{{ url_for('.list_extensions', cid=current_course.id) }}">
<i class="fa fa-calendar"></i>
Expand Down
71 changes: 71 additions & 0 deletions server/templates/staff/course/enrollment/enrollment.unenrolled.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
{% extends "staff/base.html" %}
{% import 'staff/_formhelpers.html' as forms %}

{% block title %} Un-Enrolled Submitters - {{ current_course.display_name_with_semester }}{% endblock %}

{% block main %}
<section class="content-header">
<h1>
{{ current_course.display_name_with_semester }} Un-Enrolled Submitters
<small>{{ current_course.offering }}</small>
</h1>
<ol class="breadcrumb">
<li><a href="{{ url_for(".course", cid=current_course.id) }}">
<i class="fa fa-university"></i> {{ current_course.offering }}
</a></li>
<li class="active"><a href="{{ url_for('.unenrolled', cid=current_course.id) }}">
<i class="fa fa-user-o"></i> Un-Enrolled Submitters</a>
</li>
</ol>
</section>
<section class="content">
{% include 'alerts.html' %}
<div class="row">
<div class="col-md-9 col-xs-12">
<div class="box">
<div class="box-header">
<h3 class="box-title"><span> {{ title }} </span></h3>
<div class="box-tools">
<div class="box-tools pull-right">
<button type="button" class="btn btn-box-tool" data-widget="collapse" data-toggle="tooltip" title="Collapse">
<i class="fa fa-minus"></i>
</button>
</div>
</div>
</div>

<!-- /.box-header -->
<div class="box-body table-responsive no-padding table-loading" id="{{ title }}-list">
<table class="table table-hover">
<thead>
<th class="sort" data-sort="email">User</th>
<th class="sort" data-sort="sid">SID</th>
</thead>

<tbody class="list">
{% for unenrolled_submitter in unenrolled_submitters %}
<tr>
<td class="email">
<a href="{{ url_for('.student_view', cid=current_course.id, email=unenrolled_submitter.email) }}">
{{ unenrolled_submitter.email }}
</a>
</td>
<td class="sid">{{ unenrolled_submitter.id }}</td>
Copy link
Member

Choose a reason for hiding this comment

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

The ID isn't particularly useful (it's our database ID) Maybe their name could go here instead?

</tr>
{% endfor %}
</tbody>
</table>
</div>
<!-- /.box-body -->
<div class="box-footer">
<div class="pull-left">
<h5 class="box-title"><span> Total: {{ unenrolled_submitters | length }} </span></h5>
</div>
</div>

</div>
</div>
</div>

</section>
{% endblock %}
6 changes: 6 additions & 0 deletions tests/test_web.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,12 @@ def test_admin_enrollment(self):
self.assertTrue(self.course.offering in self.driver.page_source)
self.assertTrue('Export Roster' in self.driver.page_source)

def test_admin_enrollment_unenrolled(self):
self._login(role="admin")
self.page_load(self.get_server_url() + "/admin/course/1/unenrolled")
self.assertIn('Un-Enrolled', self.driver.title)
self.assertTrue(self.course.offering in self.driver.page_source)

def test_admin_student_overview(self):
self._login(role="admin")
self.page_load(self.get_server_url() + "/admin/course/1/{}".format(self.user1.email))
Expand Down