-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature publish assignment #47
base: main
Are you sure you want to change the base?
Feature publish assignment #47
Conversation
bb9b4ca
to
551ca6a
Compare
# Conflicts: # app.py # Conflicts: # app.py # assignment.py # db.py # templates/assignment.html
…ers only # Conflicts: # app.py
# Conflicts: # assignment.py
551ca6a
to
bafc49f
Compare
…the home page according to user type
Can you rebase ? |
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.
Changes are needed regarding the way draft and published assignments work.
Also use conditional fetching from DB and computing.
db.py
Outdated
publication_status = db.Column( | ||
Enum(*PUBLICATION_STATUS, name='publication_status_enum'), | ||
nullable=False, | ||
default='Draft' | ||
) |
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 don't understand why have a publication status inside the year table.
The assignements we consider up to date are the ones in the assignement_published
table. When a draft is published all the assignements that were in the draft tables should be copied to the published table. So what is the use of publication_status
?
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 had a talk with Anthony. We aim to only keep the two assignment tables and remove the publication_status
from the Year table. That way one can make changes on a published assignment and save it as draft, later publishing it.
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.
We just need to handle the case where we want the assignment to be visible only for teachers.
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.
How do we manage the case of publication for teachers only?
app.py
Outdated
researcher = db.session.query(Researcher).filter_by(user_id=user.id).first() | ||
teacher = db.session.query(Teacher).filter_by(user_id=user.id).first() if user.is_teacher else None |
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.
You can also handle these with conditionals. If the user is a teacher no need to check if they are a researcher. That way we have one less call to the database.
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.
We hadn't discussed the fact that it was possible to be a teacher and a researcher at the same time?
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'll ask @anthony. But even if that is the case, we better do these calls in conditions. We could use the session to check the roles the user has.
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.
If it's OK with @anthonygego, I can store this in the session
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.
We still have a problem differentiating between a global assignment and an assignment for teachers only.
app.py
Outdated
researcher_current_course = [] | ||
researcher_evaluations = [] | ||
|
||
if teacher: |
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.
if teacher: | |
if user.is_teacher : | |
... | |
else : | |
researcher = db.session.query(Researcher).filter_by(user_id=user.id).first() | |
... |
No need to go fetch the teacher
in the database because you don't use it later. And you can check if the user is a researcher here in the condition as suggested in the comment 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.
I'm going to have to add an if condition in the else if the user is neither a teacher nor a researcher
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.
That is a possibility. You can use an if/elif/else checking on the session flags for example
Assignments can be published for everyone or just for teachers
Display assignments in the researcher/teacher view