Skip to content

feat: Join public courses #328

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

Merged
merged 11 commits into from
Oct 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 19 additions & 6 deletions lms/lmsdb/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,21 +278,34 @@ def _add_exercise_course_id_and_number_columns_constraint() -> bool:
Exercise = models.Exercise
migrator = db_config.get_migrator_instance()
with db_config.database.transaction():
course_not_exists = _add_not_null_column(Exercise, Exercise.course)
number_not_exists = _add_not_null_column(Exercise, Exercise.number)
if course_not_exists and number_not_exists:
_add_not_null_column(Exercise, Exercise.course)
_add_not_null_column(Exercise, Exercise.number)
try:
migrate(
migrator.add_index('exercise', ('course_id', 'number'), True),
)
except OperationalError as e:
if 'already exists' in str(e):
log.info(f'index exercise already exists: {e}')
else:
raise
db_config.database.commit()


def _add_user_course_constaint() -> bool:
migrator = db_config.get_migrator_instance()
with db_config.database.transaction():
migrate(
migrator.add_index('usercourse', ('user_id', 'course_id'), True),
)
try:
migrate(
migrator.add_index(
'usercourse', ('user_id', 'course_id'), True,
),
)
except OperationalError as e:
if 'already exists' in str(e):
log.info(f'index usercourse already exists: {e}')
else:
raise
db_config.database.commit()


Expand Down
10 changes: 9 additions & 1 deletion lms/lmsdb/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,14 @@ def fetch(cls, user: 'User') -> Iterable['Course']:
.order_by(Course.name.desc())
)

@classmethod
def public_courses(cls):
return cls.select().where(cls.is_public)

@classmethod
def public_course_exists(cls):
return cls.public_courses().exists()

def __str__(self):
return f'{self.name}: {self.date} - {self.end_date}'

Expand Down Expand Up @@ -279,7 +287,7 @@ def is_user_registered(cls, user_id: int, course_id: int) -> bool:
@post_save(sender=UserCourse)
def on_save_user_course(model_class, instance, created):
"""Changes user's last course viewed."""
if instance.user.last_course_viewed is None:
if instance.user.last_course_viewed is None or instance.course.is_public:
instance.user.last_course_viewed = instance.course
instance.user.save()

Expand Down
34 changes: 24 additions & 10 deletions lms/lmsweb/translations/he/LC_MESSAGES/messages.po
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ msgid ""
msgstr ""
"Project-Id-Version: 1.0\n"
"Report-Msgid-Bugs-To: EMAIL@ADDRESS\n"
"POT-Creation-Date: 2021-10-03 22:01+0300\n"
"POT-Creation-Date: 2021-10-06 15:07+0300\n"
"PO-Revision-Date: 2021-09-29 11:30+0300\n"
"Last-Translator: Or Ronai\n"
"Language: he\n"
Expand All @@ -18,7 +18,7 @@ msgstr ""
"Content-Transfer-Encoding: 8bit\n"
"Generated-By: Babel 2.9.1\n"

#: lmsdb/models.py:879
#: lmsdb/models.py:864
msgid "Fatal error"
msgstr "כישלון חמור"

Expand Down Expand Up @@ -116,14 +116,19 @@ msgstr "%(checker)s הגיב לך על תרגיל \"%(subject)s\"."
msgid "Your solution for the \"%(subject)s\" exercise has been checked."
msgstr "הפתרון שלך לתרגיל \"%(subject)s\" נבדק."

#: models/users.py:28
#: models/users.py:29
msgid "Invalid username or password"
msgstr "שם המשתמש או הסיסמה שהוזנו לא תקינים"

#: models/users.py:31
#: models/users.py:32
msgid "You have to confirm your registration with the link sent to your email"
msgstr "עליך לאשר את מייל האימות"

#: models/users.py:50
#, python-format
msgid "You are already registered to %(course_name)s course."
msgstr "אתה כבר רשום לקורס %(course_name)s."

#: templates/banned.html:8 templates/login.html:7
#: templates/recover-password.html:8 templates/reset-password.html:8
#: templates/signup.html:8
Expand Down Expand Up @@ -247,6 +252,11 @@ msgstr "בדוק תרגילים"
msgid "Logout"
msgstr "התנתקות"

#: templates/public-courses.html:6
#, fuzzy
msgid "Public Courses List"
msgstr "רשימת קורסים פתוחים"

#: templates/recover-password.html:9 templates/recover-password.html:17
#: templates/reset-password.html:9
msgid "Reset Password"
Expand Down Expand Up @@ -293,7 +303,7 @@ msgstr "חמ\"ל תרגילים"
msgid "Name"
msgstr "שם"

#: templates/status.html:13 templates/user.html:44
#: templates/status.html:13 templates/user.html:46
msgid "Checked"
msgstr "נבדק/ו"

Expand Down Expand Up @@ -341,27 +351,31 @@ msgstr "פרטי משתמש"
msgid "Actions"
msgstr "פעולות"

#: templates/user.html:24
#: templates/user.html:21
msgid "Join Courses"
msgstr "הירשם לקורסים"

#: templates/user.html:27
msgid "Exercises Submitted"
msgstr "תרגילים שהוגשו"

#: templates/user.html:29
msgid "Course name"
msgstr "שם קורס"

#: templates/user.html:30
#: templates/user.html:33
msgid "Exercise name"
msgstr "שם תרגיל"

#: templates/user.html:31
#: templates/user.html:34
msgid "Submission status"
msgstr "מצב הגשה"

#: templates/user.html:32
#: templates/user.html:35
msgid "Submission"
msgstr "הגשה"

#: templates/user.html:33
#: templates/user.html:36
msgid "Checker"
msgstr "בודק"

Expand Down
32 changes: 30 additions & 2 deletions lms/lmsweb/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@
PERMISSIVE_CORS, get_next_url, login_manager,
)
from lms.models import (
comments, notes, notifications, share_link, solutions, upload,
comments, notes, notifications, share_link, solutions, upload, users,
)
from lms.models.errors import (
FileSizeError, ForbiddenPermission, LmsError,
AlreadyExists, FileSizeError, ForbiddenPermission, LmsError,
UnauthorizedError, UploadError, fail,
)
from lms.models.users import SERIALIZER, auth, retrieve_salt
Expand Down Expand Up @@ -505,9 +505,37 @@ def user(user_id):
user=target_user,
is_manager=is_manager,
notes_options=Note.get_note_options(),
public_course_exists=Course.public_course_exists(),
)


@webapp.route('/course')
@login_required
def public_courses():
return render_template(
'public-courses.html',
courses=Course.public_courses(),
)


@webapp.route('/course/join/<int:course_id>')
@login_required
def join_public_course(course_id: int):
course = Course.get_or_none(course_id)
if course is None:
return fail(404, 'There is no such course.')
if not course.is_public:
return fail(403, "You aren't allowed to do this method.")

try:
users.join_public_course(course, current_user)
except AlreadyExists as e:
error_message, status_code = e.args
return fail(status_code, error_message)

return redirect(url_for('exercises_page'))


@webapp.route('/send/<int:course_id>', methods=['GET'])
@login_required
def send_(course_id: int):
Expand Down
2 changes: 1 addition & 1 deletion lms/models/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class LmsError(Exception):
pass


class AlreadyExists(LmsError):
class AlreadyExists(LmsError): # Usually a 409 HTTP Error
pass


Expand Down
18 changes: 16 additions & 2 deletions lms/models/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
from flask_babel import gettext as _ # type: ignore
from itsdangerous import URLSafeTimedSerializer

from lms.lmsdb.models import User
from lms.lmsdb.models import Course, User, UserCourse
from lms.lmsweb import config
from lms.models.errors import (
ForbiddenPermission, UnauthorizedError, UnhashedPasswordError,
AlreadyExists, ForbiddenPermission, UnauthorizedError,
UnhashedPasswordError,
)


Expand Down Expand Up @@ -38,3 +39,16 @@ def auth(username: str, password: str) -> User:

def generate_user_token(user: User) -> str:
return SERIALIZER.dumps(user.mail_address, salt=retrieve_salt(user))


def join_public_course(course: Course, user: User) -> None:
__, created = UserCourse.get_or_create(**{
UserCourse.user.name: user, UserCourse.course.name: course,
})
if not created:
raise AlreadyExists(
_(
'You are already registered to %(course_name)s course.',
course_name=course.name,
), 409,
)
11 changes: 7 additions & 4 deletions lms/static/my.css
Original file line number Diff line number Diff line change
Expand Up @@ -835,16 +835,18 @@ code .grader-add .fa {
}

/* User's page */
#user {
#user, #public-courses {
text-align: right;
}

#user h1 {
#user h1,
#public-courses h1 {
margin: 5vh 0;
text-align: center;
}

#user .body {
#user .body,
#public-courses .body {
width: 80vw;
margin: auto;
}
Expand All @@ -853,7 +855,8 @@ code .grader-add .fa {
width: 80vw;
}

#user .user-actions {
#user .user-actions,
#public-courses .public-courses-links {
margin-bottom: 5em;
}

Expand Down
19 changes: 19 additions & 0 deletions lms/templates/public-courses.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{% extends 'base.html' %}

{% block page_content %}
<div id="page-public-courses">
<div id="public-courses" class="{{ direction }}">
<h1>{{ _('Public Courses List') }}</h1>
<div class="body">
<div class="public-courses-links {{ direction }}">
<ul>
{% for course in courses %}
<li><a href="{{ url_for('join_public_course', course_id=course.id) }}" role="button">{{ course.name | e }}</a></li>
{% endfor %}
</ul>
</div>
</div>
</div>
</div>

{% endblock %}
3 changes: 3 additions & 0 deletions lms/templates/user.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ <h2>{{ _('Actions') }}:</h2>
<div id="change-password-user">
<ul>
<li><a href="{{ url_for('change_password') }}" role="button">{{ _('Change Password') }}</a></li>
{% if public_course_exists %}
<li><a href="{{ url_for('public_courses') }}" role="button">{{ _('Join Courses') }}</a></li>
{% endif %}
</ul>
</div>
</div>
Expand Down
23 changes: 22 additions & 1 deletion tests/test_registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from flask.testing import FlaskClient

from lms.lmsweb.config import CONFIRMATION_TIME
from lms.lmsdb.models import User
from lms.lmsdb.models import Course, User
from lms.models.users import generate_user_token
from tests import conftest

Expand Down Expand Up @@ -146,3 +146,24 @@ def test_registartion_closed(client: FlaskClient, captured_templates):
template, _ = captured_templates[-1]
assert template.name == 'login.html'
assert '/signup' not in response.get_data(as_text=True)

@staticmethod
def test_register_public_course(
student_user: User, course: Course, captured_templates,
):
client = conftest.get_logged_user(username=student_user.username)
not_public_course_response = client.get(f'/course/join/{course.id}')
assert not_public_course_response.status_code == 403

unknown_course_response = client.get('/course/join/123456')
assert unknown_course_response.status_code == 404

course.is_public = True
course.save()
course = Course.get_by_id(course.id)
client.get(f'/course/join/{course.id}')
template, _ = captured_templates[-1]
assert template.name == 'exercises.html'

already_registered_response = client.get(f'/course/join/{course.id}')
assert already_registered_response.status_code == 409