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

Removed is_teacher and is_admin, changed to role enum #127

Merged
merged 10 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 1 addition & 2 deletions backend/db_construct.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
CREATE TABLE users (
uid VARCHAR(255),
is_teacher BOOLEAN,
is_admin BOOLEAN,
role ENUM('student', 'teacher', 'admin') NOT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

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

PRIMARY KEY(uid)
);

Expand Down
36 changes: 12 additions & 24 deletions backend/project/endpoints/index/OpenAPI_Object.json
Original file line number Diff line number Diff line change
Expand Up @@ -1374,14 +1374,11 @@
"uid": {
"type": "string"
},
"is_teacher": {
"type": "boolean"
},
"is_admin": {
"type": "boolean"
"role": {
"type": "enum"
}
},
"required": ["uid", "is_teacher", "is_admin"]
"required": ["uid", "role"]
}
}
}
Expand All @@ -1399,14 +1396,11 @@
"uid": {
"type": "string"
},
"is_teacher": {
"type": "boolean"
},
"is_admin": {
"type": "boolean"
"role": {
"type": "enum"
}
},
"required": ["uid", "is_teacher", "is_admin"]
"required": ["uid", "role"]
}
}
}
Expand Down Expand Up @@ -1451,14 +1445,11 @@
"uid": {
"type": "string"
},
"is_teacher": {
"type": "boolean"
},
"is_admin": {
"type": "boolean"
"role": {
"type": "enum"
}
},
"required": ["uid", "is_teacher", "is_admin"]
"required": ["uid", "role"]
}
}
}
Expand Down Expand Up @@ -1487,14 +1478,11 @@
"schema": {
"type": "object",
"properties": {
"is_teacher": {
"type": "boolean"
},
"is_admin": {
"type": "boolean"
"role": {
"type": "role"
}
},
"required": ["is_teacher", "is_admin"]
"required": ["role"]
}
}
}
Expand Down
31 changes: 11 additions & 20 deletions backend/project/endpoints/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,10 @@ def get(self):
"""
try:
query = userModel.query
is_teacher = request.args.get('is_teacher')
is_admin = request.args.get('is_admin')
role = request.args.get("role")

if is_teacher is not None:
query = query.filter(userModel.is_teacher == (is_teacher.lower() == 'true'))

if is_admin is not None:
query = query.filter(userModel.is_admin == (is_admin.lower() == 'true'))
if role is not None:
query = query.filter(userModel.role == role.lower())

users = query.all()

Expand All @@ -54,25 +50,23 @@ def post(self):
It should create a new user and return a success message.
"""
uid = request.json.get('uid')
is_teacher = request.json.get('is_teacher')
is_admin = request.json.get('is_admin')
role = request.args.get("role")

if is_teacher is None or is_admin is None or uid is None:
if role is None or uid is None:
return {
"message": "Invalid request data!",
"correct_format": {
"uid": "User ID (string)",
"is_teacher": "Teacher status (boolean)",
"is_admin": "Admin status (boolean)"
"role": "User role (string)"
},"url": f"{API_URL}/users"
}, 400
try:
user = db.session.get(userModel, uid)
if user is not None:
# bad request, error code could be 409 but is rarely used
return {"message": f"User {uid} already exists"}, 400
# Code to create a new user in the database using the uid, is_teacher, and is_admin
new_user = userModel(uid=uid, is_teacher=is_teacher, is_admin=is_admin)
# Code to create a new user in the database using the uid and role
new_user = userModel(uid=uid, role=role)
db.session.add(new_user)
db.session.commit()
return jsonify({"message": "User created successfully!",
Expand Down Expand Up @@ -114,17 +108,14 @@ def patch(self, user_id):
dict: A dictionary containing the message indicating the success
or failure of the update.
"""
is_teacher = request.json.get('is_teacher')
is_admin = request.json.get('is_admin')
role = request.args.get("role")
try:
user = db.session.get(userModel, user_id)
if user is None:
return {"message": "User not found!","url": f"{API_URL}/users"}, 404

if is_teacher is not None:
user.is_teacher = is_teacher
if is_admin is not None:
user.is_admin = is_admin
if role is not None:
user.role = role

# Save the changes to the database
db.session.commit()
Expand Down
15 changes: 10 additions & 5 deletions backend/project/models/user.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
"""User model"""

from enum import Enum
from dataclasses import dataclass
from sqlalchemy import Boolean, Column, String
from sqlalchemy import Column, String, Enum as EnumField
from project.db_in import db

class Role(Enum):
"""This class defines the roles of a user"""
STUDENT = 0
TEACHER = 1
ADMIN = 2

@dataclass
class User(db.Model):
"""This class defines the users table,
a user has a uid,
is_teacher and is_admin booleans because a user
a user has a uid and a role, a user
can be either a student,admin or teacher"""

__tablename__ = "users"
uid: str = Column(String(255), primary_key=True)
is_teacher: bool = Column(Boolean)
is_admin: bool = Column(Boolean)
role: Role = Column(EnumField(Role))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't be nullable

8 changes: 4 additions & 4 deletions backend/project/utils/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ def return_authenticated_user_id():

if user:
return auth_user_id
is_teacher = False
role = 'student'
if user_info["jobTitle"] != None:
is_teacher = True
role = 'teacher'
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the enum for this instead


# add user if not yet in database
try:
new_user = User(uid=auth_user_id, is_teacher=is_teacher, is_admin=False)
new_user = User(uid=auth_user_id, role=role)
db.session.add(new_user)
db.session.commit()
except SQLAlchemyError:
Expand All @@ -89,7 +89,7 @@ def is_teacher(auth_user_id):
"url": f"{API_URL}/users"}, 500
if not user: # should realistically never happen
abort(500, "A database error occured")
if user.is_teacher:
if user.role == 'teacher':
return True
return False

Expand Down
8 changes: 4 additions & 4 deletions backend/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ def db_session():
def users():
"""Return a list of users to populate the database"""
return [
User(uid="brinkmann", is_admin=True, is_teacher=True),
User(uid="laermans", is_admin=True, is_teacher=True),
User(uid="student01", is_admin=False, is_teacher=False),
User(uid="student02", is_admin=False, is_teacher=False)
User(uid="brinkmann", role='admin'),
User(uid="laermans", role='admin'),
User(uid="student01", role='student'),
User(uid="student02", role='student')
]

def courses():
Expand Down
16 changes: 8 additions & 8 deletions backend/tests/endpoints/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def valid_user():
"""
return {
"uid": "w_student",
"is_teacher": False
"role": 'student'
}

@pytest.fixture
Expand All @@ -73,10 +73,10 @@ def valid_user_entries(session):
Returns a list of users that are in the database
"""
users = [
User(uid="del", is_admin=False, is_teacher=True),
User(uid="pat", is_admin=False, is_teacher=True),
User(uid="u_get", is_admin=False, is_teacher=True),
User(uid="query_user", is_admin=True, is_teacher=False)]
User(uid="del", role='teacher'),
User(uid="pat", role='teacher'),
User(uid="u_get", role='teacher'),
User(uid="query_user", role='admin')]

session.add_all(users)
session.commit()
Expand Down Expand Up @@ -127,7 +127,7 @@ def app():
@pytest.fixture
def course_teacher_ad():
"""A user that's a teacher for testing"""
ad_teacher = User(uid="Gunnar", is_teacher=True, is_admin=True)
ad_teacher = User(uid="Gunnar", role='teacher')
return ad_teacher

@pytest.fixture
Expand Down Expand Up @@ -177,7 +177,7 @@ def client(app):
@pytest.fixture
def valid_teacher_entry(session):
"""A valid teacher for testing that's already in the db"""
teacher = User(uid="Bart", is_teacher=True)
teacher = User(uid="Bart", role='teacher')
session.add(teacher)
session.commit()
return teacher
Expand All @@ -204,7 +204,7 @@ def valid_course_entry(session, valid_course):
def valid_students_entries(session):
"""Valid students for testing that are already in the db"""
students = [
User(uid=f"student_sel2_{i}", is_teacher=False)
User(uid=f"student_sel2_{i}", role='student')
for i in range(3)
]
session.add_all(students)
Expand Down
31 changes: 18 additions & 13 deletions backend/tests/endpoints/user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ def user_db_session():
db.metadata.create_all(engine)
session = Session()
session.add_all(
[User(uid="del", is_admin=False, is_teacher=True),
User(uid="pat", is_admin=False, is_teacher=True),
User(uid="u_get", is_admin=False, is_teacher=True),
User(uid="query_user", is_admin=True, is_teacher=False)
[User(uid="del", role='teacher'),
User(uid="pat", role='teacher'),
User(uid="u_get", role='teacher'),
User(uid="query_user", role='admin')
]
)
session.commit()
Expand Down Expand Up @@ -117,37 +117,42 @@ def test_get_one_user_wrong_authentication(self, client, valid_user_entry):
def test_patch_user(self, client, valid_user_entry):
"""Test updating a user."""

new_is_teacher = not valid_user_entry.is_teacher
if valid_user_entry.role == 'teacher':
new_role = 'admin'
if valid_user_entry.role == 'admin':
new_role = 'student'
else:
new_role = 'teacher'

response = client.patch(f"/users/{valid_user_entry.uid}", json={
'is_teacher': new_is_teacher,
'is_admin': not valid_user_entry.is_admin
'role': new_role
})
assert response.status_code == 403 # Patching a user is never necessary and thus not allowed

def test_patch_non_existent(self, client):
"""Test updating a non-existent user."""
response = client.patch("/users/-20", json={
'is_teacher': False,
'is_admin': True
'role': 'teacher'
})
assert response.status_code == 403 # Patching is not allowed

def test_patch_non_json(self, client, valid_user_entry):
"""Test sending a non-JSON patch request."""
valid_user_form = asdict(valid_user_entry)
valid_user_form["is_teacher"] = not valid_user_form["is_teacher"]
if valid_user_form["role"] == 'teacher':
valid_user_form["role"] = 'student'
else:
valid_user_form["role"] = 'teacher'
response = client.patch(f"/users/{valid_user_form['uid']}", data=valid_user_form)
assert response.status_code == 403 # Patching is not allowed

def test_get_users_with_query(self, client, valid_user_entries):
"""Test getting users with a query."""
# Send a GET request with query parameters, this is a nonsense entry but good for testing
response = client.get("/users?is_admin=true&is_teacher=false", headers={"Authorization":"teacher1"})
response = client.get("/users?role=admin", headers={"Authorization":"teacher1"})
assert response.status_code == 200

# Check that the response contains only the user that matches the query
users = response.json["data"]
for user in users:
assert user["is_admin"] is True
assert user["is_teacher"] is False
assert user["role"] == 'admin'
4 changes: 2 additions & 2 deletions backend/tests/models/user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class TestUserModel:

def test_create_user(self, session: Session):
"""Test if a user can be created"""
user = User(uid="user01", is_teacher=False, is_admin=False)
user = User(uid="user01", role='student')
session.add(user)
session.commit()
assert session.get(User, "user01") is not None
Expand All @@ -21,7 +21,7 @@ def test_query_user(self, session: Session):
assert session.query(User).count() == 4
teacher = session.query(User).filter_by(uid="brinkmann").first()
assert teacher is not None
assert teacher.is_teacher
assert teacher.role == 'teacher'

def test_update_user(self, session: Session):
"""Test if a user can be updated"""
Expand Down
Loading