diff --git a/config.py b/config.py index 25a554c..809270d 100644 --- a/config.py +++ b/config.py @@ -3,17 +3,13 @@ ENV = os.getenv('OH_QUEUE_ENV', 'dev') -if ENV in ('dev', 'staging'): - DEBUG = True -elif ENV == 'prod': - DEBUG = False +DEBUG = ENV in ('dev', 'staging') +SECRET_KEY = os.getenv('SECRET_KEY') +SQLALCHEMY_DATABASE_URI = os.getenv('DATABASE_URL').replace('mysql://', 'mysql+pymysql://') if ENV == 'dev': - SECRET_KEY = 'dev' - SQLALCHEMY_DATABASE_URI = 'sqlite:///' + os.path.join(basedir, 'app.db') -else: - SECRET_KEY = os.getenv('SECRET_KEY') - SQLALCHEMY_DATABASE_URI = os.getenv('DATABASE_URL').replace('mysql://', 'mysql+pymysql://') + SECRET_KEY = SECRET_KEY or 'dev' + SQLALCHEMY_DATABASE_URI = SQLALCHEMY_DATABASE_URI or 'sqlite:///' + os.path.join(basedir, 'app.db') SQLALCHEMY_TRACK_MODIFICATIONS = False DATABASE_CONNECT_OPTIONS = {} diff --git a/manage.py b/manage.py index c874bcb..4ebad70 100755 --- a/manage.py +++ b/manage.py @@ -1,16 +1,32 @@ #!/usr/bin/env python3 import datetime +import functools import random +import sys +from flask_migrate import Migrate, MigrateCommand from flask_script import Manager import names from oh_queue import app, socketio from oh_queue.models import db, Ticket, User, TicketStatus +migrate = Migrate(app, db) + manager = Manager(app) +manager.add_command('db', MigrateCommand) + +def not_in_production(f): + @functools.wraps(f) + def wrapper(*args, **kwargs): + if app.config.get('ENV') == 'prod': + print('this commend should not be run in production. Aborting') + sys.exit(1) + return f(*args, **kwargs) + return wrapper @manager.command +@not_in_production def seed(): print('Seeding...') for i in range(20): @@ -41,6 +57,7 @@ def seed(): @manager.command +@not_in_production def resetdb(): print('Dropping tables...') db.drop_all(app=app) @@ -49,11 +66,9 @@ def resetdb(): seed() @manager.command +@not_in_production def server(): socketio.run(app) if __name__ == '__main__': - if app.config.get('ENV') == 'prod': - print('manage.py should not be run in production. Aborting') - sys.exit(1) manager.run() diff --git a/migrations/README.md b/migrations/README.md new file mode 100755 index 0000000..11460d1 --- /dev/null +++ b/migrations/README.md @@ -0,0 +1,106 @@ +# Generating Migrations + +To generate migrations, you'll need to be running MySQL locally. + +## Installing MySQL + +### OSX + +``` +brew install mysql +``` + +Reccomended SQL Client for local exploration: [Sequel Pro](https://sequelpro.com/) + +## Configuring MySQL +Start MySQL and connect +``` +mysql.server start +mysql -u root +``` +The password for root is blank by default. Just hit enter and you will enter the mysql console. +See the [MySQL docs](http://dev.mysql.com/doc/mysql-getting-started/en/) for more info. + +Run the following commands to create a user and a table for OK + +``` +create database ohdevel; +CREATE USER 'ohdev'@'localhost'; +GRANT ALL PRIVILEGES ON ohdevel.* TO 'ohdev'@'localhost'; +``` + +## Running a migration + +If the `master` branch is the current state of the production branch - start by checking out the master branch. + +`git checkout master` + +Make sure OK is using MySQL and not sqlite by setting the `DATABASE_URL` +environment variable. For Bash, +``` +export DATABASE_URL=mysql://ohdev:@127.0.0.1:3306/ohdevel?charset=utf8mb4 +``` + +``` +# Rebuild the database +./manage.py resetdb +./manage.py db upgrade # Make sure this does not crash. If it does: see Common Errors below +``` + +Now you can checkout the branch with changes: + +`git checkout feature-branch` + +``` +./manage.py db migrate -m "Added new feature" +``` +That should produce the output along the lines of +``` +INFO [alembic.runtime.migration] Context impl MySQLImpl. +INFO [alembic.runtime.migration] Will assume non-transactional DDL. +INFO [alembic.autogenerate.compare] Detected added column 'assignment.published_scores' + Generating /dev/ok/server/migrations/versions/6ce2cf5c4534_publish_scores_for_assignments.py ... done +``` + +## Inspecting the output + +Check the file contents before running the migrations to make sure the changes make sense. There should not be unncessary changes. It's ok to manually make changes to the script (or manually write a migration file) if needed. + +## Deploying the migration. +Make sure the deploy the app. After it's running, run +``` +dokku run officehours-web ./manage.py db upgrade +``` +to upgrade. + +## Common errors + +If you are getting long tracebacks that end in somethng like this +``` +raise errorclass(errno, errorvalue) +sqlalchemy.exc.ProgrammingError: (pymysql.err.ProgrammingError) (1146, "Table 'ok-dev.client' doesn't exist") [SQL: 'ALTER TABLE client ADD COLUMN created DATETIME NOT NULL DEFAULT now()'] +``` +or +``` + raise util.CommandError("Target database is not up to date.") +alembic.util.exc.CommandError: Target database is not up to date. +``` + +It's likely that Alembic is trying to rerun migrations (or has not run all of the migrations in the folder) + +If you are _sure_ that the DB is up to date - find the ID most recent migration run (the most recent commit in them migrations folder should reveal the file most recently created) and then run + +``` +./manage.py db stamp f7f27412d13f +``` + +This tells alembic that the DB is at revision `f7f27412d13f`. Now running `./manage.py db upgrade` should result in this output + +``` +$ ./manage.py db upgrade +INFO [alembic.runtime.migration] Context impl MySQLImpl. +INFO [alembic.runtime.migration] Will assume non-transactional DDL. +$ +``` + +If that doesn't resolve it - make sure there a not unrun migrations in the branch. diff --git a/migrations/alembic.ini b/migrations/alembic.ini new file mode 100644 index 0000000..f8ed480 --- /dev/null +++ b/migrations/alembic.ini @@ -0,0 +1,45 @@ +# A generic, single database configuration. + +[alembic] +# template used to generate migration files +# file_template = %%(rev)s_%%(slug)s + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + + +# Logging configuration +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/migrations/env.py b/migrations/env.py new file mode 100755 index 0000000..4593816 --- /dev/null +++ b/migrations/env.py @@ -0,0 +1,87 @@ +from __future__ import with_statement +from alembic import context +from sqlalchemy import engine_from_config, pool +from logging.config import fileConfig +import logging + +# this is the Alembic Config object, which provides +# access to the values within the .ini file in use. +config = context.config + +# Interpret the config file for Python logging. +# This line sets up loggers basically. +fileConfig(config.config_file_name) +logger = logging.getLogger('alembic.env') + +# add your model's MetaData object here +# for 'autogenerate' support +# from myapp import mymodel +# target_metadata = mymodel.Base.metadata +from flask import current_app +config.set_main_option('sqlalchemy.url', + current_app.config.get('SQLALCHEMY_DATABASE_URI')) +target_metadata = current_app.extensions['migrate'].db.metadata + +# other values from the config, defined by the needs of env.py, +# can be acquired: +# my_important_option = config.get_main_option("my_important_option") +# ... etc. + + +def run_migrations_offline(): + """Run migrations in 'offline' mode. + + This configures the context with just a URL + and not an Engine, though an Engine is acceptable + here as well. By skipping the Engine creation + we don't even need a DBAPI to be available. + + Calls to context.execute() here emit the given string to the + script output. + + """ + url = config.get_main_option("sqlalchemy.url") + context.configure(url=url) + + with context.begin_transaction(): + context.run_migrations() + + +def run_migrations_online(): + """Run migrations in 'online' mode. + + In this scenario we need to create an Engine + and associate a connection with the context. + + """ + + # this callback is used to prevent an auto-migration from being generated + # when there are no changes to the schema + # reference: http://alembic.readthedocs.org/en/latest/cookbook.html + def process_revision_directives(context, revision, directives): + if getattr(config.cmd_opts, 'autogenerate', False): + script = directives[0] + if script.upgrade_ops.is_empty(): + directives[:] = [] + logger.info('No changes in schema detected.') + + engine = engine_from_config(config.get_section(config.config_ini_section), + prefix='sqlalchemy.', + poolclass=pool.NullPool) + + connection = engine.connect() + context.configure(connection=connection, + target_metadata=target_metadata, + process_revision_directives=process_revision_directives, + **current_app.extensions['migrate'].configure_args) + + try: + with context.begin_transaction(): + context.run_migrations() + finally: + connection.close() + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/migrations/script.py.mako b/migrations/script.py.mako new file mode 100755 index 0000000..3f8d9ed --- /dev/null +++ b/migrations/script.py.mako @@ -0,0 +1,23 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision} +Create Date: ${create_date} + +""" + +# revision identifiers, used by Alembic. +revision = ${repr(up_revision)} +down_revision = ${repr(down_revision)} + +from alembic import op +import sqlalchemy as sa +from oh_queue.models import * +${imports if imports else ""} + +def upgrade(): + ${upgrades if upgrades else "pass"} + + +def downgrade(): + ${downgrades if downgrades else "pass"} diff --git a/migrations/versions/826d994e7e11_multiple_user_roles.py b/migrations/versions/826d994e7e11_multiple_user_roles.py new file mode 100644 index 0000000..4e40fa9 --- /dev/null +++ b/migrations/versions/826d994e7e11_multiple_user_roles.py @@ -0,0 +1,31 @@ +"""Multiple user roles + +Revision ID: 826d994e7e11 +Revises: None +Create Date: 2016-10-26 14:55:36.981449 + +""" + +# revision identifiers, used by Alembic. +revision = '826d994e7e11' +down_revision = None + +from alembic import op +import sqlalchemy as sa +from oh_queue.models import * +from sqlalchemy.dialects import mysql + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.add_column('user', sa.Column('role', EnumType(Role), default=Role.student, nullable=False)) + op.create_index(op.f('ix_user_role'), 'user', ['role'], unique=False) + op.drop_column('user', 'is_staff') + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.add_column('user', sa.Column('is_staff', mysql.TINYINT(display_width=1), autoincrement=False, nullable=True)) + op.drop_index(op.f('ix_user_role'), table_name='user') + op.drop_column('user', 'role') + ### end Alembic commands ### diff --git a/oh_queue/auth.py b/oh_queue/auth.py index 14e9881..8188831 100644 --- a/oh_queue/auth.py +++ b/oh_queue/auth.py @@ -4,7 +4,7 @@ from werkzeug import security -from oh_queue.models import db, User +from oh_queue.models import db, User, Role auth = Blueprint('auth', __name__) auth.config = {} @@ -20,7 +20,7 @@ def record_params(setup_state): consumer_key=app.config.get('OK_KEY'), consumer_secret=app.config.get('OK_SECRET'), request_token_params={ - 'scope': 'email', + 'scope': 'all', 'state': lambda: security.gen_salt(10) }, base_url=server_url + '/api/v3/', @@ -52,14 +52,14 @@ def authorize_user(user): # TODO validate after_login URL return redirect(after_login) -def user_from_email(name, email, is_staff): +def user_from_email(name, email, role): """Get a User with the given email, or create one.""" user = User.query.filter_by(email=email).one_or_none() if not user: - user = User(name=name, email=email, is_staff=is_staff) + user = User(name=name, email=email, role=role) else: user.name = name - user.is_staff = is_staff + user.role = role db.session.add(user) db.session.commit() return user @@ -92,12 +92,21 @@ def authorized(): if ', ' in name: last, first = name.split(', ') name = first + ' ' + last - is_staff = False + role = Role.student offering = auth.course_offering for p in info['participations']: - if p['course']['offering'] == offering and p['role'] != 'student': - is_staff = True - user = user_from_email(name, email, is_staff) + if p['course']['offering'] == offering: + role_str = p['role'] + if role_str == 'instructor': + role = Role.instructor + elif role_str == 'staff': + role = Role.ta + elif role_str == 'grader': + role = Role.grader + elif role_str == 'lab assistant': + role = Role.lab_assistant + break + user = user_from_email(name, email, role) return authorize_user(user) @auth.route('/logout/') @@ -119,7 +128,8 @@ def testing_authorized(): abort(404) form = request.form is_staff = form.get('is_staff') == 'on' - user = user_from_email(form['name'], form['email'], is_staff) + role = Role.staff if is_staff else Role.student + user = user_from_email(form['name'], form['email'], role) return authorize_user(user) def init_app(app): diff --git a/oh_queue/models.py b/oh_queue/models.py index ce1e77e..6cb1d77 100644 --- a/oh_queue/models.py +++ b/oh_queue/models.py @@ -23,13 +23,15 @@ def process_result_value(self, name, dialect): def python_type(self): return self.enum_class +Role = enum.Enum('Role', 'instructor ta grader lab_assistant student') + class User(db.Model, UserMixin): __tablename__ = 'user' id = db.Column(db.Integer, primary_key=True) created = db.Column(db.DateTime, default=db.func.now()) email = db.Column(db.String(255), nullable=False, index=True) name = db.Column(db.String(255), nullable=False) - is_staff = db.Column(db.Boolean, default=False) + role = db.Column(EnumType(Role), default=Role.student, nullable=False, index=True) @property def short_name(self): @@ -38,6 +40,15 @@ def short_name(self): return first_name.rsplit('@')[0] return first_name + @property + def is_staff(self): + return self.role is Role.instructor or self.role is Role.ta \ + or self.role is Role.grader + + @property + def is_helper(self): + return self.is_staff or self.role is Role.lab_assistant + TicketStatus = enum.Enum('TicketStatus', 'pending assigned resolved deleted') class Ticket(db.Model): diff --git a/oh_queue/static/js/components/base.js b/oh_queue/static/js/components/base.js index f2a0c84..da0eabd 100644 --- a/oh_queue/static/js/components/base.js +++ b/oh_queue/static/js/components/base.js @@ -1,7 +1,7 @@ let Base = ({state, children}) => { let myTicket = getMyTicket(state); - if (isStaff(state) || myTicket) { + if (isHelper(state) || myTicket) { requestNotificationPermission(); } diff --git a/oh_queue/static/js/components/queue.js b/oh_queue/static/js/components/queue.js index 3825407..41cf258 100644 --- a/oh_queue/static/js/components/queue.js +++ b/oh_queue/static/js/components/queue.js @@ -6,11 +6,11 @@ let Queue = ({state}) => { ); return (
- {!isStaff(state) && } + {!isHelper(state) && }
- {isStaff(state) && + {isHelper(state) &&
Click on a ticket to view the student's name diff --git a/oh_queue/static/js/components/ticket.js b/oh_queue/static/js/components/ticket.js index dc03a7e..62ff13b 100644 --- a/oh_queue/static/js/components/ticket.js +++ b/oh_queue/static/js/components/ticket.js @@ -20,7 +20,7 @@ let TicketLink = ({state, ticket, myTicket, children}) => {
); - } else if (isStaff(state)) { // staff + } else if (isHelper(state)) { // staff return (
diff --git a/oh_queue/static/js/components/ticket_buttons.js b/oh_queue/static/js/components/ticket_buttons.js index 5c926c0..ef1da4e 100644 --- a/oh_queue/static/js/components/ticket_buttons.js +++ b/oh_queue/static/js/components/ticket_buttons.js @@ -42,7 +42,7 @@ class TicketButtons extends React.Component { render() { let {state, ticket} = this.props; - let staff = isStaff(state); + let helper = isHelper(state); function makeButton(text, style, action) { return ( @@ -58,13 +58,13 @@ class TicketButtons extends React.Component { if (ticket.status === 'pending') { bottomButtons.push(makeButton('Delete', 'danger', this.delete)); - if (staff) { + if (helper) { topButtons.push(makeButton('Help', 'primary', this.assign)); } } if (ticket.status === 'assigned') { bottomButtons.push(makeButton('Resolve', 'default', this.resolve)); - if (staff) { + if (helper) { if (ticket.helper.id === state.currentUser.id) { topButtons.push(makeButton('Resolve and Next', 'primary', this.resolveAndNext)); bottomButtons.push(makeButton('Requeue', 'default', this.unassign)); @@ -74,7 +74,7 @@ class TicketButtons extends React.Component { } } } - if (staff && (ticket.status === 'resolved' || ticket.status === 'deleted')) { + if (helper && (ticket.status === 'resolved' || ticket.status === 'deleted')) { topButtons.push(makeButton('Next Ticket', 'default', this.next)); } diff --git a/oh_queue/static/js/components/ticket_view.js b/oh_queue/static/js/components/ticket_view.js index 0a22160..7cf2ccd 100644 --- a/oh_queue/static/js/components/ticket_view.js +++ b/oh_queue/static/js/components/ticket_view.js @@ -21,7 +21,7 @@ class TicketView extends React.Component { } } - if (!isStaff(state) && !ticketIsMine(state, ticket)) { + if (!isHelper(state) && !ticketIsMine(state, ticket)) { return ; } @@ -31,7 +31,7 @@ class TicketView extends React.Component {

- { (ticket.status === 'pending' && isStaff(state)) ? 'Help to View Name' : ticket.user.name } + { (ticket.status === 'pending' && isHelper(state)) ? 'Help to View Name' : ticket.user.name } { ticketDisplayTime(ticket) } in { ticket.location }

{ ticketStatus(state, ticket) }

diff --git a/oh_queue/static/js/state.js b/oh_queue/static/js/state.js index 4b6d428..ca75131 100644 --- a/oh_queue/static/js/state.js +++ b/oh_queue/static/js/state.js @@ -9,7 +9,9 @@ type User = { id: number, email: string, name: string, + role: string, isStaff: boolean, + isHelper: boolean, }; type Ticket = { @@ -79,6 +81,10 @@ function ticketStatus(state: State, ticket: Ticket): string { } } +function isHelper(state: State): boolean { + return state.currentUser != null && state.currentUser.isHelper; +} + function isStaff(state: State): boolean { return state.currentUser != null && state.currentUser.isStaff; } diff --git a/oh_queue/views.py b/oh_queue/views.py index cbf2a63..7892646 100644 --- a/oh_queue/views.py +++ b/oh_queue/views.py @@ -14,7 +14,9 @@ def user_json(user): 'id': user.id, 'email': user.email, 'name': user.name, + 'role': user.role.name, 'isStaff': user.is_staff, + 'isHelper': user.is_helper } def ticket_json(ticket): @@ -74,10 +76,10 @@ def wrapper(*args, **kwds): return f(*args, **kwds) return wrapper -def is_staff(f): +def is_helper(f): @functools.wraps(f) def wrapper(*args, **kwds): - if not (current_user.is_authenticated and current_user.is_staff): + if not (current_user.is_authenticated and current_user.is_helper): return socket_unauthorized() return f(*args, **kwds) return wrapper @@ -150,7 +152,7 @@ def get_next_ticket(): return socket_redirect() @socketio.on('next') -@is_staff +@is_helper def next_ticket(ticket_id): return get_next_ticket() @@ -158,7 +160,7 @@ def next_ticket(ticket_id): @logged_in def delete(ticket_id): ticket = Ticket.query.get(ticket_id) - if not (current_user.is_staff or ticket.user.id == current_user.id): + if not (current_user.is_helper or ticket.user.id == current_user.id): return socket_unauthorized() ticket.status = TicketStatus.deleted db.session.commit() @@ -180,7 +182,7 @@ def resolve(ticket_id): return get_next_ticket() @socketio.on('assign') -@is_staff +@is_helper def assign(ticket_id): ticket = Ticket.query.get(ticket_id) ticket.status = TicketStatus.assigned @@ -190,7 +192,7 @@ def assign(ticket_id): emit_event(ticket, TicketEventType.assign) @socketio.on('unassign') -@is_staff +@is_helper def unassign(ticket_id): ticket = Ticket.query.get(ticket_id) ticket.status = TicketStatus.pending @@ -200,7 +202,7 @@ def unassign(ticket_id): emit_event(ticket, TicketEventType.unassign) @socketio.on('load_ticket') -@is_staff +@is_helper def load_ticket(ticket_id): ticket = Ticket.query.get(ticket_id) if ticket: diff --git a/requirements.txt b/requirements.txt index 346d708..f163e19 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,7 @@ click==6.6 Flask==0.11.1 Flask-Login==0.3.2 +Flask-Migrate==2.0.0 Flask-OAuthlib==0.9.3 Flask-Script==2.0.5 Flask-SocketIO==2.6.2