diff --git a/containers/Dockerfile b/containers/Dockerfile index 473949ec..646eddfc 100644 --- a/containers/Dockerfile +++ b/containers/Dockerfile @@ -1,4 +1,4 @@ -FROM tigrlab/datman:0.1 as prod +FROM tigrlab/datman:0.2.1 as prod # NOTE: When building this container, do it from # inside the dashboard folder and build with @@ -12,6 +12,10 @@ COPY containers/entrypoint.sh /entrypoint.sh # that rebuilds are faster. COPY ./requirements.txt / +# Install requirements for pip installing uwsgi +RUN apt update && \ + apt install -y --no-install-recommends gcc python3.11-dev + RUN pip install --upgrade pip && \ pip install -r /requirements.txt diff --git a/containers/dashboard.env b/containers/dashboard.env index 59b5f7c3..31d55ea2 100644 --- a/containers/dashboard.env +++ b/containers/dashboard.env @@ -23,8 +23,7 @@ POSTGRES_SRVR=dash_postgres POSTGRES_USER=dashboard POSTGRES_DATABASE=dashboard # POSTGRES_TEST_DATABASE=test_dashboard - - +# POSTGRES_PORT=5432 # TIMEZONE=-240 @@ -58,6 +57,11 @@ POSTGRES_DATABASE=dashboard # DASH_LOG_DIR= +# Menu Configuration +# ------------------ +# DASH_MENU_CONFIG= + + # Debugging settings # ------------------ # FLASK_ENV= diff --git a/containers/prod/docker-compose.yml b/containers/prod/docker-compose.yml index de51956c..37c3a0cf 100644 --- a/containers/prod/docker-compose.yml +++ b/containers/prod/docker-compose.yml @@ -2,10 +2,7 @@ version: '3' services: app: container_name: dashboard - build: - context: ../../ - dockerfile: containers/Dockerfile - target: prod + image: tigrlab/dashboard:0.2.1 env_file: ../dashboard.env ports: - 5000:5000 diff --git a/dashboard/__init__.py b/dashboard/__init__.py index e0329fac..d0b31a2d 100644 --- a/dashboard/__init__.py +++ b/dashboard/__init__.py @@ -173,7 +173,7 @@ def create_app(config=None): load_blueprints(app) - if app.debug and app.env == 'development': + if app.debug and app.config['ENV'] == 'development': # Never run this on a production server! setup_devel_ext(app) diff --git a/dashboard/blueprints/main/views.py b/dashboard/blueprints/main/views.py index 597d99b7..7669614c 100644 --- a/dashboard/blueprints/main/views.py +++ b/dashboard/blueprints/main/views.py @@ -124,7 +124,7 @@ def study(study_id=None, active_tab=None): display_metrics = current_app.config['DISPLAY_METRICS'] # get the study object from the database - study = Study.query.get(study_id) + study = db.session.get(Study, study_id) # this is used to update the readme text file form = StudyOverviewForm() @@ -416,7 +416,7 @@ def analysis(analysis_id=None): if not analysis_id: analyses = Analysis.query.all() else: - analyses = Analysis.query.get(analysis_id) + analyses = db.session.get(Analysis, analysis_id) for analysis in analyses: # format the user objects for display on page diff --git a/dashboard/blueprints/redcap/monitors.py b/dashboard/blueprints/redcap/monitors.py index bb8664d3..560279ac 100644 --- a/dashboard/blueprints/redcap/monitors.py +++ b/dashboard/blueprints/redcap/monitors.py @@ -8,7 +8,7 @@ from .emails import missing_session_data from dashboard.monitors import add_monitor, get_emails -from dashboard.models import Session, User +from dashboard.models import Session, User, db from dashboard.exceptions import MonitorException from dashboard.queue import submit_job @@ -78,7 +78,7 @@ def check_scans(name, num, recipients=None): :obj:`dashboard.exceptions.MonitorException`: If a matching session can't be found. """ - session = Session.query.get((name, num)) + session = db.session.get(Session, (name, num)) if not session: raise MonitorException("Monitored session {}_{:02d} is no " "longer in database. Cannot verify whether " @@ -151,7 +151,7 @@ def monitor_scan_download(session, end_time=None): def download_session(name, num, end_time): - session = Session.query.get((name, num)) + session = db.session.get(Session, (name, num)) if not session: raise MonitorException( "Monitored session {}_{} is no longer in database, aborting " diff --git a/dashboard/blueprints/redcap/utils.py b/dashboard/blueprints/redcap/utils.py index b95cde66..036c9359 100644 --- a/dashboard/blueprints/redcap/utils.py +++ b/dashboard/blueprints/redcap/utils.py @@ -8,7 +8,7 @@ import redcap as REDCAP from .monitors import monitor_scan_import, monitor_scan_download -from dashboard.models import Session, Timepoint, RedcapRecord, RedcapConfig +from dashboard.models import Session, Timepoint, RedcapRecord, RedcapConfig, db from dashboard.queries import get_studies from dashboard.exceptions import RedcapException import datman.scanid @@ -20,7 +20,7 @@ def get_redcap_record(record_id, fail_url=None): if not fail_url: fail_url = url_for('main.index') - record = RedcapRecord.query.get(record_id) + record = db.session.get(RedcapRecord, record_id) if record is None: logger.error("Tried and failed to retrieve RedcapRecord with " @@ -149,7 +149,7 @@ def set_session(name): name = ident.get_full_subjectid_with_timepoint() num = datman.scanid.get_session_num(ident) - session = Session.query.get((name, num)) + session = db.session.get(Session, (name, num)) if not session: timepoint = get_timepoint(ident) session = timepoint.add_session(num) @@ -166,7 +166,8 @@ def find_study(ident): def get_timepoint(ident): - timepoint = Timepoint.query.get(ident.get_full_subjectid_with_timepoint()) + timepoint = db.session.get( + Timepoint, ident.get_full_subjectid_with_timepoint()) if not timepoint: study = find_study(ident)[0] timepoint = study.add_timepoint(ident) diff --git a/dashboard/blueprints/timepoints/forms.py b/dashboard/blueprints/timepoints/forms.py index b3fc36a1..c7fe3099 100644 --- a/dashboard/blueprints/timepoints/forms.py +++ b/dashboard/blueprints/timepoints/forms.py @@ -1,5 +1,5 @@ from flask_wtf import FlaskForm -from wtforms import SubmitField, TextAreaField, TextField, BooleanField +from wtforms import SubmitField, TextAreaField, StringField, BooleanField from wtforms.validators import DataRequired @@ -43,7 +43,7 @@ class TimepointCommentsForm(FlaskForm): class NewIssueForm(FlaskForm): - title = TextField( + title = StringField( "Title: ", validators=[DataRequired()], render_kw={'required': True}) diff --git a/dashboard/blueprints/timepoints/utils.py b/dashboard/blueprints/timepoints/utils.py index c630c724..458ffede 100644 --- a/dashboard/blueprints/timepoints/utils.py +++ b/dashboard/blueprints/timepoints/utils.py @@ -1,8 +1,9 @@ import logging from github import Github + from flask import current_app, flash -from ...models import Study +from ...models import Study, db logger = logging.getLogger(__name__) @@ -22,7 +23,7 @@ def search_issues(token, timepoint): def handle_issue(token, issue_form, study_id, timepoint): title = clean_issue_title(issue_form.title.data, timepoint) - study = Study.query.get(study_id) + study = db.session.get(Study, study_id) staff_member = study.choose_staff_contact() if staff_member: diff --git a/dashboard/blueprints/users/forms.py b/dashboard/blueprints/users/forms.py index dca7453c..6c429cd8 100644 --- a/dashboard/blueprints/users/forms.py +++ b/dashboard/blueprints/users/forms.py @@ -1,22 +1,21 @@ from flask_wtf import FlaskForm -from wtforms import (SubmitField, HiddenField, TextField, FormField, - BooleanField, FieldList, RadioField, SelectMultipleField) -from wtforms.fields.html5 import EmailField, TelField -from wtforms.compat import iteritems +from wtforms import (SubmitField, HiddenField, StringField, FormField, + BooleanField, FieldList, RadioField, SelectMultipleField, + EmailField, TelField) from wtforms.validators import DataRequired class UserForm(FlaskForm): id = HiddenField() - first_name = TextField('First Name: ', + first_name = StringField('First Name: ', validators=[DataRequired()], render_kw={ 'required': True, 'maxlength': '64', 'placeholder': 'Jane' }) - last_name = TextField('Last Name: ', + last_name = StringField('Last Name: ', validators=[DataRequired()], render_kw={ 'required': True, @@ -34,7 +33,7 @@ class UserForm(FlaskForm): validators=[DataRequired()], choices=[('github', 'GitHub')], default='github') - account = TextField('Username: ', + account = StringField('Username: ', validators=[DataRequired()], render_kw={ 'required': @@ -44,12 +43,12 @@ class UserForm(FlaskForm): 'placeholder': 'Username used on account ' + 'provider\'s site' }) - position = TextField('Position: ', + position = StringField('Position: ', render_kw={ 'maxlength': '64', 'placeholder': 'Job title or position' }) - institution = TextField('Institution: ', + institution = StringField('Institution: ', render_kw={ 'maxlength': '128', @@ -61,7 +60,7 @@ class UserForm(FlaskForm): 'maxlength': '20', 'placeholder': '555-555-5555' }) - ext = TextField('Extension: ', + ext = StringField('Extension: ', render_kw={ 'maxlength': '10', 'placeholder': 'XXXXXXXXXX' @@ -71,7 +70,7 @@ class UserForm(FlaskForm): 'maxlength': '20', 'placeholder': '555-555-5555' }) - alt_ext = TextField('Alt. Extension: ', + alt_ext = StringField('Alt. Extension: ', render_kw={ 'maxlength': '10', 'placeholder': 'XXXXXXXXXX' @@ -136,7 +135,7 @@ def process(self, formdata=None, obj=None, data=None, **kwargs): # Temporarily, this can simply be merged with kwargs. kwargs = dict(data, **kwargs) - for name, field, in iteritems(self._fields): + for name, field, in self._fields.items(): if obj is not None and hasattr(obj, name): # This if statement is the only change made to the original # code for BaseForm.process() - Dawn @@ -158,7 +157,7 @@ def populate_obj(self, obj): default with the 'studies' field treated as a special case to account for the fact that it is a mapped collection """ - for name, field in iteritems(self._fields): + for name, field in self._fields.items(): if name == 'studies': for study_form in self.studies.entries: if study_form.site_id.data == '': diff --git a/dashboard/blueprints/users/views.py b/dashboard/blueprints/users/views.py index fd82ce88..53700755 100644 --- a/dashboard/blueprints/users/views.py +++ b/dashboard/blueprints/users/views.py @@ -6,13 +6,13 @@ from . import user_bp from .utils import get_user_form, parse_enabled_sites from .forms import UserForm -from ...models import User, AccountRequest +from ...models import User, AccountRequest, db from ...utils import report_form_errors, dashboard_admin_required @lm.user_loader def load_user(uid): - return User.query.get(int(uid)) + return db.session.get(User, int(uid)) @user_bp.before_app_request @@ -58,7 +58,7 @@ def user(user_id=None): return redirect(url_for('users.user')) if user_id: - user = User.query.get(user_id) + user = db.session.get(User, user_id) else: user = current_user @@ -74,7 +74,7 @@ def user(user_id=None): flash("You are not authorized to update other users' settings.") return redirect(url_for('users.user')) - updated_user = User.query.get(submitted_id) + updated_user = db.session.get(User, submitted_id) if form.update_access.data: # Give user access to a new study or site @@ -123,7 +123,7 @@ def manage_users(user_id=None, approve=False): # URL gets parsed into unicode approve = False - user_request = AccountRequest.query.get(user_id) + user_request = db.session.get(AccountRequest, user_id) if not approve: try: user_request.reject() diff --git a/dashboard/forms.py b/dashboard/forms.py index 6a537bfe..add11044 100644 --- a/dashboard/forms.py +++ b/dashboard/forms.py @@ -8,7 +8,7 @@ from flask_wtf import FlaskForm from wtforms import (SelectMultipleField, HiddenField, TextAreaField, - TextField) + StringField) from wtforms.validators import DataRequired @@ -49,6 +49,6 @@ class AnalysisForm(FlaskForm): This feature has not yet been fully implemented. """ - name = TextField('Brief name', validators=[DataRequired()]) + name = StringField('Brief name', validators=[DataRequired()]) description = TextAreaField('Description', validators=[DataRequired()]) software = TextAreaField('Software') diff --git a/dashboard/models/models.py b/dashboard/models/models.py index 6197c8c4..ca9e85c7 100644 --- a/dashboard/models/models.py +++ b/dashboard/models/models.py @@ -8,7 +8,7 @@ from flask import current_app from flask_login import UserMixin, AnonymousUserMixin -from sqlalchemy import and_, or_, exists, func +from sqlalchemy import and_, or_, exists, func, select from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.orm import deferred, backref from sqlalchemy.schema import UniqueConstraint, ForeignKeyConstraint @@ -138,6 +138,7 @@ class User(PermissionMixin, UserMixin, TableMixin, db.Model): analysis_comments = db.relationship('AnalysisComment') sessions_reviewed = db.relationship('Session') pending_approval = db.relationship('AccountRequest', + back_populates='user', uselist=False, cascade="all, delete") @@ -480,7 +481,8 @@ class AccountRequest(TableMixin, db.Model): db.Integer, db.ForeignKey("users.id", ondelete='CASCADE'), primary_key=True) - user = db.relationship('User', uselist=False) + user = db.relationship( + 'User', back_populates="pending_approval", uselist=False) def __init__(self, user_id): self.user_id = user_id @@ -704,7 +706,7 @@ def update_site(self, site_id, redcap=None, notes=None, code=None, """ if isinstance(site_id, Site): site_id = Site.id - elif not Site.query.get(site_id): + elif not db.session.get(Site, site_id): if not create: raise InvalidDataException("Site {} does not exist.".format( site_id)) @@ -779,13 +781,14 @@ def update_scantype(self, site_id, scantype, num=None, pha_num=None, site_id, self.id)) if not isinstance(scantype, Scantype): - found = Scantype.query.get(scantype) + found = db.session.get(Scantype, scantype) if not found: raise InvalidDataException("Undefined scan type " "{}.".format(scantype)) scantype = found - expected = ExpectedScan.query.get((self.id, site_id, scantype.tag)) + expected = db.session.get(ExpectedScan, + (self.id, site_id, scantype.tag)) if expected: if num: expected.count = num @@ -824,10 +827,12 @@ def num_timepoints(self, type=''): return len(timepoints) def outstanding_issues(self): - # use from_self to get a tuple of Session.name, Session.num like from - # the other queries - new_sessions = self.get_new_sessions().from_self( - Session.name, Session.num).all() + # Get a tuple of Session.name, Session.num like from + # the other two queries + new_sessions_q = select(Session.name, Session.num).where( + Session.name.in_(self.get_new_sessions())) + new_sessions = db.session.execute(new_sessions_q).all() + missing_redcap = self.get_missing_redcap() missing_scans = self.get_missing_scans() @@ -906,8 +911,8 @@ def get_missing_redcap(self): that are expecting a redcap survey but dont yet have one. """ uses_redcap = self.get_sessions_using_redcap() - sessions = uses_redcap.filter(SessionRedcap.name == None).from_self( - Session.name, Session.num) + sessions = uses_redcap.filter(SessionRedcap.name == None) \ + .with_entities(Session.name, Session.num) return sessions.all() def get_missing_scans(self): @@ -922,7 +927,7 @@ def get_missing_scans(self): and_(Scan.timepoint == Session.name, Scan.repeat == Session.num))).filter(~exists().where( and_(Session.name == EmptySession.name, Session.num == - EmptySession.num))).from_self( + EmptySession.num))).with_entities( Session.name, Session.num) return sessions.all() @@ -1049,7 +1054,7 @@ def get_pipelines(self, scope=None): return self._pipelines def add_pipeline(self, pipeline_key, view, name, scope): - record = StudyPipeline.query.get((self.id, pipeline_key)) + record = db.session.get(StudyPipeline, (self.id, pipeline_key)) if not record: record = StudyPipeline(study_id=self.id, pipeline_id=pipeline_key) record.view = view @@ -1246,7 +1251,7 @@ def missing_scans(self): return any(sess.missing_scans() for sess in self.sessions.values()) def dismiss_redcap_error(self, session_num): - existing = SessionRedcap.query.get((self.name, session_num)) + existing = db.session.get(SessionRedcap, (self.name, session_num)) if existing: return session_redcap = SessionRedcap(self.name, session_num) @@ -2200,7 +2205,7 @@ def completed_value(self, user_value): def get_config(config_id=None, project=None, instrument=None, url=None, version=None, create=False): if config_id: - found = RedcapConfig.query.get(config_id) + found = db.session.get(RedcapConfig, config_id) cfg = [found] if found else [] elif project and url and instrument: cfg = RedcapConfig.query \ @@ -2387,6 +2392,7 @@ class StudySite(TableMixin, db.Model): primaryjoin='and_(StudySite.study_id==StudyUser.study_id,' 'or_(StudySite.site_id==StudyUser.site_id,' 'StudyUser.site_id==None))', + viewonly=True, cascade='all, delete') site = db.relationship('Site', back_populates='studies') study = db.relationship('Study', back_populates='sites') @@ -2394,7 +2400,8 @@ class StudySite(TableMixin, db.Model): back_populates='study_site', cascade='all, delete', lazy='joined') - expected_scans = db.relationship('ExpectedScan', cascade="all, delete") + expected_scans = db.relationship( + 'ExpectedScan', overlaps="scantypes", cascade="all, delete") __table_args__ = (UniqueConstraint(study_id, site_id), ) diff --git a/dashboard/monitors.py b/dashboard/monitors.py index 05415067..bd38378b 100644 --- a/dashboard/monitors.py +++ b/dashboard/monitors.py @@ -23,7 +23,7 @@ from datetime import datetime, timedelta from dashboard import scheduler -from .models import Session +from .models import db, Session from .emails import missing_redcap_email from .exceptions import MonitorException, SchedulerException @@ -131,7 +131,7 @@ def monitor_redcap_import(name, num, users=None, study=None): :obj:`dashboard.exceptions.SchedulerException`: If the job cannot be added to the server """ - session = Session.query.get((name, num)) + session = db.session.get(Session, (name, num)) if not session.timepoint.expects_redcap() or session.redcap_record: return @@ -173,7 +173,7 @@ def check_redcap(name, num, recipients=None): :obj:`dashboard.exceptions.MonitorException`: If a matching session can't be found. """ - session = Session.query.get((name, num)) + session = db.session.get(Session, (name, num)) if not session: raise MonitorException("Monitored session {}_{:02d} is no longer in " "database. Cannot verify whether redcap record " diff --git a/dashboard/queries.py b/dashboard/queries.py index 139431d3..50238102 100644 --- a/dashboard/queries.py +++ b/dashboard/queries.py @@ -90,7 +90,7 @@ def get_session(name, num): """ Used by datman. Return a specific session or None """ - return Session.query.get((name, num)) + return db.session.get(Session, (name, num)) def get_timepoint(name, bids_ses=None, study=None): @@ -98,7 +98,7 @@ def get_timepoint(name, bids_ses=None, study=None): Used by datman. Return one timepoint or None """ if not bids_ses: - return Timepoint.query.get(name) + return db.session.get(Timepoint, name) query = Timepoint.query.filter(Timepoint.bids_name == name)\ .filter(Timepoint.bids_session == bids_ses) @@ -263,7 +263,7 @@ def get_scantypes(tag_id=None, create=False): if not tag_id: return Scantype.query.all() - found = Scantype.query.get(tag_id) + found = db.session.get(Scantype, tag_id) if found: return [found] @@ -327,9 +327,9 @@ def get_scan_qc(approved=True, blacklisted=True, flagged=True, by scan name. Defaults to False. Returns: - list(tuple): A list of tuples of the format - (scan name, status, comment), where 'status' is a boolean value - that represents whether the scan was approved or + list(dict): A list of tuples of the format + {name: str, approved: bool, comment: str}, where 'status' is a + boolean value that represents whether the scan was approved or flagged/blacklisted. """ @@ -424,8 +424,12 @@ def get_list(input_var): # Restrict output values to only needed columns query = query.with_entities(Scan.name, ScanChecklist.approved, ScanChecklist.comment) + output = [ + {'name': item[0], 'approved': item[1], 'comment': item[2]} + for item in query.all() + ] - return query.all() + return output def query_metric_values_byid(**kwargs): diff --git a/dashboard/utils.py b/dashboard/utils.py index 89dc46ef..49089e04 100644 --- a/dashboard/utils.py +++ b/dashboard/utils.py @@ -10,7 +10,7 @@ from flask import flash, url_for, request, redirect from werkzeug.routing import RequestRedirect -from .models import Timepoint, Scan +from .models import Timepoint, Scan, db logger = logging.getLogger(__name__) @@ -39,7 +39,7 @@ def report_form_errors(form): def get_timepoint(study_id, timepoint_id, current_user): - timepoint = Timepoint.query.get(timepoint_id) + timepoint = db.session.get(Timepoint, timepoint_id) if timepoint is None: flash("Timepoint {} does not exist".format(timepoint_id)) @@ -67,7 +67,7 @@ def get_scan(scan_id, study_id, current_user, fail_url=None): if not fail_url: fail_url = url_for('main.index') - scan = Scan.query.get(scan_id) + scan = db.session.get(Scan, scan_id) if scan is None: logger.error("User {} attempted to retrieve scan with ID {}. " diff --git a/docs/configuration.rst b/docs/configuration.rst index 0952d6fa..10c87c79 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -305,14 +305,15 @@ Optional * **FLASK_ENV** * Description: Tells Flask what type of environment it is running within. - `See here for more info `_ + Switching to 'development' mode can enable extensions to help with testing + and development. Do not use 'development' mode on a production server. * Accepted values: ``'production'`` or ``'development'`` * Default value: ``'production'`` * **FLASK_DEBUG** * Description: Tells Flask and its plugins to run in debug mode. Setting 'FLASK_ENV' to development mode automatically turns on FLASK_DEBUG. - `See here for more info `_ + `See here for more info `_ * Accepted values: ``True`` (if it should run in debug mode) or ``False`` * Default value: ``False`` * **LOGIN_DISABLED** diff --git a/docs/developer.rst b/docs/developer.rst index 1b634786..0e5ac6ce 100644 --- a/docs/developer.rst +++ b/docs/developer.rst @@ -2,34 +2,34 @@ Developer Resources ------------------- -This page contains additional information that may be useful to anyone +This page contains additional information that may be useful to anyone hoping to work on the dashboard's code. -The app uses a `postgreSQL `_ database and the -front end is programmed in python using the `Flask `_ -framework. If you're trying to get familiar with Flask we recommend +The app uses a `postgreSQL `_ database and the +front end is programmed in python using the `Flask `_ +framework. If you're trying to get familiar with Flask we recommend `this tutorial. `_ We also use `SQLAlchemy `_ to manage the database models. -Our production web server uses `uWSGI `_ +Our production web server uses `uWSGI `_ behind an `NGINX `_ server. Authentication is handled -using `OAuth. `_ +using `OAuth. `_ Dashboard Structure ------------------- -``dashboard/models/models.py`` defines the *Object Relational Mapping (ORM)* -used by SQLAlchemy to map from the relational database to the python objects +``dashboard/models/models.py`` defines the *Object Relational Mapping (ORM)* +used by SQLAlchemy to map from the relational database to the python objects used in the code. All of the 'views.py' scripts define the *entry points* (i.e. valid URLs for the app). These have been organized into blueprints (``dashboard/blueprints``) that group related functionality together. -If a blueprint requires any HTML, it is stored inside a 'templates' folder -nested within the blueprint. All other HTML templates are in +If a blueprint requires any HTML, it is stored inside a 'templates' folder +nested within the blueprint. All other HTML templates are in ``dashboard/templates/``. Templates with `_snip.html` or in a 'snips' subfolder are used as reusable pieces embedded in other pages. @@ -43,26 +43,26 @@ To keep the database migrations accurate you should: the changes with a simple ``flask db upgrade`` #. Verify that the script generated by ``flask db migrate`` is accurate. Flask-migrate uses Alembic, which is not capable of reliably detecting - all changes to the schema so manual intervention is sometimes needed. + all changes to the schema so manual intervention is sometimes needed. `See here `_ for detailed info on what to look out for. - + Jinja Template Tips ------------------- -Flask's HTML templates use `Jinja `_ -to render pages. The most important thing to be aware of is that while you can -break up your pages into smaller, more readable, chunks by saving HTML in +Flask's HTML templates use `Jinja `_ +to render pages. The most important thing to be aware of is that while you can +break up your pages into smaller, more readable, chunks by saving HTML in another file and then importing it with something like this: .. code-block:: jinja {% include 'my_other_file.html' %} -performance-wise, this is not always a good idea. Each and every time the -'include' statement is read while the page is constructed, the included -file has to be read from the filesystem. File reads are (relatively) slow and -if the include is inside of a loop with a large number of iterations, you can +performance-wise, this is not always a good idea. Each and every time the +'include' statement is read while the page is constructed, the included +file has to be read from the filesystem. File reads are (relatively) slow and +if the include is inside of a loop with a large number of iterations, you can easily add extra seconds of load time for a minimal boost in HTML readability. Some tips to get the most out of Jinja without adding too much overhead: @@ -73,9 +73,9 @@ Some tips to get the most out of Jinja without adding too much overhead: snippet, it's better to keep the 'if' in your original file, so you don't need to open the snippet just to discover the 'if' statement failed -Also note that if you're organizing your HTML snippets in a nested folder, you -always need to give the full path from the root of the template directory to -the file you want to include. If you get an error about a missing template, +Also note that if you're organizing your HTML snippets in a nested folder, you +always need to give the full path from the root of the template directory to +the file you want to include. If you get an error about a missing template, make sure you quoted the name of the file to be included. .. code-block:: jinja @@ -87,20 +87,20 @@ make sure you quoted the name of the file to be included. # Bad {% include my_snippet.html %} # Missing quotes on file name {% include 'incidental_findings.html' %} # This file won't be found without the full path - + SQLAlchemy Tips --------------- -SQLAlchemy is awesome and very powerful BUT sometimes it makes really naive -queries. If you try to work with objects from ``dashboard.models`` like they're -normal python objects, you can very easily end up generating thousands of queries +SQLAlchemy is awesome and very powerful BUT sometimes it makes really naive +queries. If you try to work with objects from ``dashboard.models`` like they're +normal python objects, you can very easily end up generating thousands of queries without realizing it. For instance, if you tried doing something like this: .. code-block:: python - from dashboard.models import Site + from dashboard.models import Site, db - cmh = Site.query.get('CMH') + cmh = db.session.get(Site, 'CMH') for timepoint in cmh.timepoints: # Do some stuff with the timepoint record here diff --git a/migrations/versions/4a842feae63a_.py b/migrations/versions/4a842feae63a_.py index e1550f3e..573be0d3 100644 --- a/migrations/versions/4a842feae63a_.py +++ b/migrations/versions/4a842feae63a_.py @@ -7,6 +7,7 @@ """ from alembic import op import sqlalchemy as sa +from sqlalchemy import text # revision identifiers, used by Alembic. @@ -55,9 +56,11 @@ def upgrade(): # Transfer existing config to new table conn = op.get_bind() records = conn.execute( - 'select project_id, instrument, url, redcap_version ' - ' from redcap_records ' - ' group by project_id, instrument, url, redcap_version;' + text( + 'select project_id, instrument, url, redcap_version ' + ' from redcap_records ' + ' group by project_id, instrument, url, redcap_version;' + ) ) op.bulk_insert( redcap_config, diff --git a/migrations/versions/77bce5fefcf4_.py b/migrations/versions/77bce5fefcf4_.py index 681d0ef5..fed6720c 100644 --- a/migrations/versions/77bce5fefcf4_.py +++ b/migrations/versions/77bce5fefcf4_.py @@ -7,6 +7,7 @@ """ from alembic import op import sqlalchemy as sa +from sqlalchemy import text # revision identifiers, used by Alembic. @@ -68,11 +69,11 @@ def upgrade(): ) conn = op.get_bind() - records = conn.execute( + records = conn.execute(text( 'select study_sites.study, study_sites.site, study_scantypes.scantype ' ' from study_sites, study_scantypes ' ' where study_sites.study = study_scantypes.study;' - ) + )) op.bulk_insert( expected_scans, [{'study': record[0], diff --git a/migrations/versions/b265c18f529c_.py b/migrations/versions/b265c18f529c_.py index eb3c6f30..58f88516 100644 --- a/migrations/versions/b265c18f529c_.py +++ b/migrations/versions/b265c18f529c_.py @@ -6,6 +6,7 @@ """ from alembic import op +from sqlalchemy import text # revision identifiers, used by Alembic. revision = 'b265c18f529c' @@ -16,10 +17,10 @@ def upgrade(): conn = op.get_bind() - conn.execute( + conn.execute(text( "INSERT INTO users(id, first_name, last_name) values " "(-1, 'Guest', 'Guest')" - ) + )) def downgrade(): diff --git a/requirements.txt b/requirements.txt index a5ee7303..1c72729f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,59 +1,20 @@ -APScheduler==3.6.3 -bcrypt==3.1.7 -blinker==1.4 -certifi==2022.12.7 -cffi==1.13.2 -chardet==3.0.4 -Click==7.0 -cryptography==3.3.2 -decorator==4.4.1 -Deprecated==1.2.7 +APScheduler==3.10.4 docopt==0.6.2 -enum34==1.1.6 -Flask==1.1.1 -Flask-APScheduler==1.11.0 -Flask-Login==0.4.1 +Flask==3.0.2 +Flask-APScheduler==1.13.1 +Flask-Login==0.6.3 Flask-Mail==0.9.1 -Flask-Migrate==2.5.2 -Flask-SQLAlchemy==2.4.1 -Flask-WTF==0.14.2 -future==0.18.2 -futures==3.1.1 -idna==2.8 -ipaddress==1.0.23 -itsdangerous==1.1.0 -Jinja2==2.11.3 -lxml==4.9.1 -MarkupSafe==1.1.1 -mock==3.0.5 -nose==1.3.7 -numpy==1.22.0 -paramiko==2.10.1 -pbr==5.4.4 -psycopg2-binary==2.8.5 -pyasn1==0.4.8 -PyCap -pycodestyle==2.5.0 -pycparser==2.19 -pydantic==2.6.1 -PyGithub==1.44.1 -PyJWT==2.4.0 -PyNaCl==1.3.0 -python-dateutil==2.8.1 -pytz==2019.3 -PyYAML==5.4 +Flask-Migrate==4.0.7 +Flask-SQLAlchemy==3.1.1 +Flask-WTF==1.2.1 +psycopg2-binary==2.9.9 +PyCap==2.6.0 +pydantic==2.6.4 +PyGithub==2.2.0 +PyYAML==6.0.1 rauth==0.7.3 -requests>=2.22.0 -semantic-version==2.8.3 -six==1.13.0 -SQLAlchemy==1.3.12 -sqlalchemy-migrate==0.13.0 -sqlparse==0.3.0 -Tempita==0.5.2 -tzlocal==2.0.0 -urllib3>=1.25.8 -uWSGI==2.0.18 -Werkzeug==0.16.0 -wrapt==1.11.2 -WTForms==2.2.1 -xnat==0.3.28 \ No newline at end of file +SQLAlchemy==2.0.28 +uwsgi==2.0.24 +Werkzeug==3.0.1 +WTForms==3.1.2 +xnat==0.5.3 \ No newline at end of file diff --git a/tests/blueprints/qc_search/test_views.py b/tests/blueprints/qc_search/test_views.py index 719292d7..fe61fe84 100644 --- a/tests/blueprints/qc_search/test_views.py +++ b/tests/blueprints/qc_search/test_views.py @@ -10,19 +10,19 @@ class TestGetTags: def test_tags_are_not_duplicated(self): - user = models.User.query.get(1) + user = models.db.session.get(models.User, 1) tags = views.get_tags(user) expected = self.get_tags_from_db() assert sorted(tags) == sorted(expected) def test_tags_are_sorted(self): - user = models.User.query.get(1) + user = models.db.session.get(models.User, 1) tags = views.get_tags(user) expected = self.get_tags_from_db() assert tags == sorted(expected) def test_admin_user_has_access_to_all_tags(self): - admin = models.User.query.get(2) + admin = models.db.session.get(models.User, 2) tags = views.get_tags(admin) expected = tests.utils.query_db( "SELECT DISTINCT scantypes.tag" diff --git a/tests/test_datman_utils.py b/tests/test_datman_utils.py index 2fdb5da0..df130e91 100644 --- a/tests/test_datman_utils.py +++ b/tests/test_datman_utils.py @@ -4,6 +4,7 @@ from json import JSONDecodeError import datman +from datman.config import config import dashboard import dashboard.datman_utils as dm_utils @@ -24,7 +25,7 @@ def test_returns_empty_dict_when_no_qc_dir_defined( def get_path(key): raise datman.exceptions.UndefinedSetting - mock_conf = Mock(spec=datman.config.config) + mock_conf = Mock(spec=config) mock_conf.get_path = get_path mock_config.return_value = mock_conf diff --git a/tests/test_models.py b/tests/test_models.py index acd1691e..a3002aba 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -10,7 +10,7 @@ class TestUser: def test_get_sites_returns_all_sites_avail_for_user(self): - user = models.User.query.get(1) + user = models.db.session.get(models.User, 1) result = user.get_sites() expected = self.get_result( "SELECT DISTINCT ss.site" @@ -23,40 +23,40 @@ def test_get_sites_returns_all_sites_avail_for_user(self): assert sorted(result) == sorted(expected) def test_get_sites_doesnt_duplicate_site_names(self): - user = models.User.query.get(1) + user = models.db.session.get(models.User, 1) result = user.get_sites() expected = list(set(result)) assert sorted(result) == sorted(expected) def test_get_sites_returns_list_of_strings_for_reg_user(self): - user = models.User.query.get(1) + user = models.db.session.get(models.User, 1) result = user.get_sites() assert all(isinstance(item, str) for item in result) def test_get_sites_returns_list_of_strings_for_admin_user(self): - admin = models.User.query.get(2) + admin = models.db.session.get(models.User, 2) assert admin.dashboard_admin is True result = admin.get_sites() assert all(isinstance(item, str) for item in result) def test_get_sites_sorts_results_for_user(self): - user = models.User.query.get(1) + user = models.db.session.get(models.User, 1) assert user.dashboard_admin is False result = user.get_sites() assert result == sorted(result) def test_get_sites_sorts_results_for_admin(self): - user = models.User.query.get(2) + user = models.db.session.get(models.User, 2) assert user.dashboard_admin is True result = user.get_sites() assert result == sorted(result) def test_get_studies_sorts_results_for_user(self): - user = models.User.query.get(1) + user = models.db.session.get(models.User, 1) assert user.dashboard_admin is False result = [item.id for item in user.get_studies()] @@ -69,7 +69,7 @@ def test_get_studies_sorts_results_for_user(self): assert result == expected def test_get_studies_sorts_results_for_admins(self): - admin = models.User.query.get(2) + admin = models.db.session.get(models.User, 2) assert admin.dashboard_admin is True result = [item.id for item in admin.get_studies()] diff --git a/tests/test_parse_config.py b/tests/test_parse_config.py index a25bb0ee..aba9bd09 100644 --- a/tests/test_parse_config.py +++ b/tests/test_parse_config.py @@ -52,9 +52,11 @@ def test_existing_tag_updated_when_config_settings_differ( dash_db.session.add(t1) dash_db.session.commit() - assert dashboard.models.Scantype.query.get("T1").qc_type == "func" + assert dashboard.models.db.session.get( + dashboard.models.Scantype, "T1").qc_type == "func" pc.update_tags(config) - assert dashboard.models.Scantype.query.get("T1").qc_type == "anat" + assert dashboard.models.db.session.get( + dashboard.models.Scantype, "T1").qc_type == "anat" def test_deletes_tag_record_if_not_defined_in_config_file( self, dash_db, config): @@ -356,7 +358,8 @@ def test_study_is_created_in_database_if_doesnt_already_exist( self, config, dash_db): assert dashboard.models.Study.query.all() == [] pc.update_study("SPINS", config, skip_delete=True) - assert dashboard.models.Study.query.get("SPINS") is not None + assert dashboard.models.db.session.get( + dashboard.models.Study, "SPINS") is not None @patch("bin.parse_config.update_site") def test_updating_study_calls_update_site_for_configured_sites( @@ -410,16 +413,19 @@ def test_creates_or_updates_each_study_found_in_config( assert len(config.get_key("Projects")) > 0 pc.update_studies(config) for study in config.get_key("Projects"): - assert dashboard.models.Study.query.get(study) is not None + assert dashboard.models.db.session.get( + dashboard.models.Study, study) is not None def test_deletes_studies_not_defined_in_config(self, config, dash_db): extra_study = dashboard.models.Study("STUDY5") dash_db.session.add(extra_study) dash_db.session.commit() - assert dashboard.models.Study.query.get("STUDY5") is not None + assert dashboard.models.db.session.get( + dashboard.models.Study, "STUDY5") is not None pc.update_studies(config, delete_all=True) - assert dashboard.models.Study.query.get("STUDY5") is None + assert dashboard.models.db.session.get( + dashboard.models.Study, "STUDY5") is None @pytest.fixture def config(self): diff --git a/tests/test_queries.py b/tests/test_queries.py index b90a8267..151b19df 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -344,7 +344,7 @@ def test_limits_records_based_on_user_access_rights_when_user_id_given( self.assert_result_matches_expected(result, expected) # An extra check, to confirm no UTO records are found for this user for item in result: - assert "_UTO_" not in item[0] + assert "_UTO_" not in item['name'] def test_returns_empty_list_when_user_access_rights_prevents_access(self): result = dashboard.queries.get_scan_qc(study=["STUDY3"], user_id=2) @@ -376,9 +376,18 @@ def test_sorts_output_by_scan_when_flag_given(self): " ORDER BY s.name;" ) - result_names = [item[0] for item in result] + result_names = [item['name'] for item in result] assert result_names == expected + def test_returns_list_of_dicts(self): + result = dashboard.queries.get_scan_qc() + for item in result: + assert isinstance(item, dict) + for item in result: + assert 'name' in item + assert 'comment' in item + assert 'approved' in item + def get_records(self, sql_query): return [item[0] for item in query_db(sql_query)] @@ -387,7 +396,7 @@ def assert_result_matches_expected(self, result, expected): assert len(result) == len(expected) # Ensure every entry expected is found in the actual result - scan_list = [item[0] for item in result] + scan_list = [item['name'] for item in result] for scan in expected: assert scan in scan_list diff --git a/tests/test_redcap_bp_utils.py b/tests/test_redcap_bp_utils.py index 94bee8ed..dfa226c1 100644 --- a/tests/test_redcap_bp_utils.py +++ b/tests/test_redcap_bp_utils.py @@ -203,7 +203,8 @@ def test_exception_raised_if_record_not_found_on_server( def test_adds_redcap_record_to_session( self, mock_http, mock_monitor, det, records): mock_http.return_value = self.mock_redcap_export() - session = dashboard.models.Session.query.get((self.session[:-3], 1)) + session = dashboard.models.db.session.get( + dashboard.models.Session, (self.session[:-3], 1)) assert session.redcap_record is None rc_utils.create_from_request(det) @@ -216,13 +217,16 @@ def test_adds_session_if_doesnt_exist( mock_http.return_value = self.mock_redcap_export() timepoint = self.session[:-3] - session = dashboard.models.Session.query.get((timepoint, 1)) + session = dashboard.models.db.session.get( + dashboard.models.Session, (timepoint, 1)) if session: session.delete() - assert dashboard.models.Session.query.get((timepoint, 1)) is None + assert dashboard.models.db.session.get( + dashboard.models.Session, (timepoint, 1)) is None rc_utils.create_from_request(det) - session = dashboard.models.Session.query.get((timepoint, 1)) + session = dashboard.models.db.session.get( + dashboard.models.Session, (timepoint, 1)) assert session is not None def test_updates_redcap_record_if_updates_on_server( @@ -241,7 +245,8 @@ def test_updates_redcap_record_if_updates_on_server( # Resend the data entry trigger rc_utils.create_from_request(det) - session = dashboard.models.Session.query.get((self.session[:-3], 1)) + session = dashboard.models.db.session.get( + dashboard.models.Session, (self.session[:-3], 1)) assert session is not None assert session.redcap_record.record.comment == new_comment @@ -250,7 +255,8 @@ def test_sets_event_id_of_redcap_record( mock_http.return_value = self.mock_redcap_export() rc_utils.create_from_request(det) - session = dashboard.models.Session.query.get((self.session[:-3], 1)) + session = dashboard.models.db.session.get( + dashboard.models.Session, (self.session[:-3], 1)) assert session.redcap_record is not None db_record = session.redcap_record.record @@ -261,14 +267,15 @@ def test_doesnt_fail_if_config_event_ids_not_set( mock_http.return_value = self.mock_redcap_export() # Delete the event_ids from config - rc = dashboard.models.RedcapConfig.query.get(1) + rc = dashboard.models.db.session.get(dashboard.models.RedcapConfig, 1) rc.event_ids = None dashboard.models.db.session.add(rc) dashboard.models.db.session.commit() assert rc.event_ids is None rc_utils.create_from_request(det) - record = dashboard.models.RedcapRecord.query.get(1) + record = dashboard.models.db.session.get( + dashboard.models.RedcapRecord, 1) assert record is not None def test_det_with_different_version_causes_redcap_config_version_update( @@ -292,23 +299,26 @@ def test_calls_monitor_scan_import_for_session( mock_http.return_value = self.mock_redcap_export() rc_utils.create_from_request(det) - session = dashboard.models.Session.query.get((self.session[:-3], 1)) - assert mock_monitor.called_once_with(session) + session = dashboard.models.db.session.get( + dashboard.models.Session, (self.session[:-3], 1)) + mock_monitor.assert_called_once_with(session) @patch("dashboard.blueprints.redcap.utils.monitor_scan_download") def test_calls_monitor_scan_download_if_download_script_is_set( self, mock_download, mock_http, mock_scan_monitor, det, records): mock_http.return_value = self.mock_redcap_export() - study_site = dashboard.models.StudySite.query.get(("STUDY", "ABC")) + study_site = dashboard.models.db.session.get( + dashboard.models.StudySite, ("STUDY", "ABC")) study_site.download_script = "/some/path/post_download.sh" dashboard.models.db.session.add(study_site) dashboard.models.db.session.commit() rc_utils.create_from_request(det) - session = dashboard.models.Session.query.get((self.session[:-3], 1)) - assert mock_download.called_once_with(session) + session = dashboard.models.db.session.get( + dashboard.models.Session, (self.session[:-3], 1)) + mock_download.assert_called_once_with(session) def mock_redcap_export(self, records=None): def export_records(id_list): diff --git a/tests/utils.py b/tests/utils.py index 843498af..aadad688 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -41,7 +41,7 @@ def add_studies(study_conf): study.update_site(site, create=True) for tag in study_conf[study_id][site]: - if not models.Scantype.query.get(tag): + if not models.db.session.get(models.Scantype, tag): models.db.session.add(models.Scantype(tag)) study.update_scantype(site, tag, create=True) @@ -95,7 +95,8 @@ def query_db(sql_query): list: A list of tuples containing the result. """ try: - records = models.db.session.execute(sql_query).fetchall() + records = models.db.session.execute( + sqlalchemy.text(sql_query)).fetchall() except sqlalchemy.exc.ProgrammingError: models.db.session.rollback() raise