Various fixes found in front-end QA (#99)

* feat: Add ordering for review-user-assoc

* fix: Remove nonexistent order_by method

* fix: Use proper sa v2.0 bulk inserts in endpoint

* fix: Rename review user assoc attr in api

* fix: Fix missing ranker file for relevance order

* fix: Allow getting fulltext text content

* docs: Fix copy-paste error in docstring

* fix: Add docs params for export endpoints

* fix: Don't skip first row if not dicts in fileio

* fix: Get studies exports working again

but not streaming, womp

* feat: Allow jwt tokens by user email

this is used only in user invitation flows

* feat: Update new user invite email template

* feat: Allow inviting non-users to review

* fix: Fix last couple bugs in invite user flows

* fix: First row no longer duped in tabular write

* fix: Add user add option for admins only

plus a test

colandr/apis/auth.py

@@ -308,13 +308,17 @@ def put(self, args, token):
 
 
 @jwt.user_identity_loader
-def user_identity_loader(user: User):
+def user_identity_loader(user: t.Union[User, str]):
     """
     Callback function that takes the ``User`` passed in as the "identity"
     when creating JWTs and returns it in JSON serializable format,
     i.e. as the corresponding unique integer ``User.id`` .
     """
-    return user.id
+    if isinstance(user, User):
+        return user.id
+    elif isinstance(user, str):
+        Email()(user)  # validate as email
+        return user
 
 
 @jwt.user_lookup_loader
@@ -324,14 +328,25 @@ def user_lookup_callback(_jwt_header, jwt_data: dict) -> User:
     whenever a protected API route is accessed.
     """
     identity = jwt_data[current_app.config["JWT_IDENTITY_CLAIM"]]
-    user = db.session.get(User, identity)
+    if isinstance(identity, int):  # id
+        user = db.session.get(User, identity)
+    elif isinstance(identity, str):  # email
+        Email()(identity)  # validate as email
+        user = db.session.execute(
+            sa.select(User).filter_by(email=identity)
+        ).scalar_one_or_none()
+    else:
+        raise TypeError()
     assert user is not None  # type guard
     return user
 
 
 @jwt.additional_claims_loader
-def additional_claims_loader(user: User) -> dict:
-    return {"is_admin": user.is_admin}
+def additional_claims_loader(user: t.Union[User, str]) -> dict:
+    if isinstance(user, User):
+        return {"is_admin": user.is_admin}
+    else:
+        return {}
 
 
 @jwt.token_in_blocklist_loader
@@ -379,8 +394,16 @@ def get_user_from_token(token: str) -> t.Optional[User]:
     if it exists in the database; otherwise, return None.
     """
     jwt_data = jwtext.decode_token(token, allow_expired=False)
-    user_id = jwt_data[current_app.config["JWT_IDENTITY_CLAIM"]]
-    user = db.session.get(User, user_id)
+    identity = jwt_data[current_app.config["JWT_IDENTITY_CLAIM"]]
+    if isinstance(identity, int):
+        user = db.session.get(User, identity)
+    elif isinstance(identity, str):
+        Email()(identity)  # validate as email
+        user = db.session.execute(
+            sa.select(User).filter_by(email=identity)
+        ).scalar_one_or_none()
+    else:
+        raise TypeError()
     return user
 
 

colandr/apis/resources/citation_imports.py

@@ -227,10 +227,11 @@ def post(
             ]
 
         # insert studies, and get their primary keys _back_
-        stmt = sa.insert(Study).values(studies_to_insert).returning(Study.id)
-        with db.engine.connect() as conn:
-            study_ids = [result[0] for result in conn.execute(stmt)]
-
+        study_ids = list(
+            db.session.execute(
+                sa.insert(Study).returning(Study.id), studies_to_insert
+            ).scalars()
+        )
         # add study ids to citations as their primary keys
         # then bulk insert as mappings
         # this method is required because not all citations have all fields
@@ -242,14 +243,10 @@ def post(
         # the corresponding fulltexts, since bulk operations won't trigger
         # the fancy events defined in models.py
         if status == "included":
-            with db.engine.connect() as conn:
-                conn.execute(
-                    Fulltext.__table__.insert(),
-                    [
-                        {"id": study_id, "review_id": review_id}
-                        for study_id in study_ids
-                    ],
-                )
+            db.session.execute(
+                sa.insert(Fulltext),
+                [{"id": study_id, "review_id": review_id} for study_id in study_ids],
+            )
 
         # don't forget about a record of the import
         citations_import = Import(

colandr/apis/resources/citation_screenings.py

@@ -68,7 +68,7 @@ def get(self, id, fields):
             return not_found_error(f"<Citation(id={id})> not found")
         if (
             current_user.is_admin is False
-            and current_user.user_review_assoc.filter_by(
+            and current_user.review_user_assoc.filter_by(
                 review_id=citation.review_id
             ).one_or_none()
             is None
@@ -104,7 +104,7 @@ def delete(self, id):
             return not_found_error(f"<Citation(id={id})> not found")
         if (
             current_user.is_admin is False
-            and current_user.user_review_assoc.filter_by(
+            and current_user.review_user_assoc.filter_by(
                 review_id=citation.review_id
             ).one_or_none()
             is None
@@ -150,7 +150,7 @@ def post(self, args, id):
             return not_found_error(f"<Citation(id={id})> not found")
         if (
             current_user.is_admin is False
-            and current_user.user_review_assoc.filter_by(
+            and current_user.review_user_assoc.filter_by(
                 review_id=citation.review_id
             ).one_or_none()
             is None

colandr/apis/resources/citations.py

@@ -225,7 +225,7 @@ def post(self, args, review_id, source_type, source_name, source_url, status):
             return not_found_error(f"<Review(id={review_id})> not found")
         if (
             current_user.is_admin is False
-            and current_user.user_review_assoc.filter_by(
+            and current_user.review_user_assoc.filter_by(
                 review_id=review_id
             ).one_or_none()
             is None

colandr/apis/resources/data_extractions.py

@@ -52,10 +52,14 @@ def get(self, id):
         extracted_data = db.session.get(DataExtraction, id)
         if not extracted_data:
             return not_found_error(f"<DataExtraction(study_id={id})> not found")
+        # TODO: figure out if this is "better" approach
+        # if current_user.is_admin is False and not any(
+        #     review.id == extracted_data.review_id for review in current_user.reviews
+        # ):
         if (
             current_user.is_admin is False
-            and current_user.reviews.filter_by(
-                id=extracted_data.review_id
+            and current_user.review_user_assoc.filter_by(
+                review_id=extracted_data.review_id
             ).one_or_none()
             is None
         ):
@@ -103,7 +107,7 @@ def delete(self, id, labels):
         if not extracted_data:
             return not_found_error(f"<DataExtraction(study_id={id})> not found")
         if (
-            current_user.user_review_assoc.filter_by(
+            current_user.review_user_assoc.filter_by(
                 review_id=extracted_data.review_id
             ).one_or_none()
             is None
@@ -156,7 +160,7 @@ def put(self, args, id):
         if not extracted_data:
             return not_found_error(f"<DataExtraction(study_id={id})> not found")
         if (
-            current_user.user_review_assoc.filter_by(review_id=review_id).one_or_none()
+            current_user.review_user_assoc.filter_by(review_id=review_id).one_or_none()
             is None
         ):
             return forbidden_error(

colandr/apis/resources/deduplicate_studies.py

@@ -47,7 +47,7 @@ class DeduplicateStudiesResource(Resource):
     )
     @jwtext.jwt_required()
     def post(self, review_id):
-        """get all distinct tags assigned to studies"""
+        """manually trigger deduplication of studies for a given review"""
         current_user = jwtext.get_current_user()
         review = db.session.get(models.Review, review_id)
         if not review:

colandr/apis/resources/exports.py

@@ -24,6 +24,16 @@
 class ExportStudiesResource(Resource):
     @ns.doc(
         description="export studies data",
+        params={
+            "review_id": {
+                "in": "query",
+                "type": "integer",
+            },
+            "content_type": {
+                "in": "query",
+                "type": "string",
+            },
+        },
         responses={
             200: "successfully got studies data for specified review",
             403: "current app user forbidden to export studies data for specified review",
@@ -56,13 +66,6 @@ def get(self, review_id, content_type):
         ):
             return forbidden_error(f"{current_user} forbidden to get this review")
 
-        studies = db.session.execute(
-            sa.select(Study).filter_by(review_id=review_id).order_by(Study.id)
-        ).scalars()
-        data_extraction_form = db.session.execute(
-            sa.select(ReviewPlan.data_extraction_form).filter_by(id=review_id)
-        ).one_or_none()
-
         fieldnames = [
             "study_id",
             "study_tags",
@@ -85,6 +88,9 @@ def get(self, review_id, content_type):
             "fulltext_exclude_reasons",
         ]
         extraction_label_types: t.Optional[list[tuple[str, str]]]
+        data_extraction_form = db.session.execute(
+            sa.select(ReviewPlan.data_extraction_form).filter_by(id=review_id)
+        ).one_or_none()
         if data_extraction_form:
             extraction_label_types = [
                 (item["label"], item["field_type"]) for item in data_extraction_form[0]
@@ -93,7 +99,15 @@ def get(self, review_id, content_type):
         else:
             extraction_label_types = None
 
-        rows = (_study_to_row(study, extraction_label_types) for study in studies)
+        # TODO: make this query performant and fully streamable, even with lazy-loading
+        # see: https://docs.sqlalchemy.org/en/14/errors.html#parent-instance-x-is-not-bound-to-a-session-lazy-load-deferred-load-refresh-etc-operation-cannot-proceed
+        # see: https://docs.sqlalchemy.org/en/14/errors.html#object-cannot-be-converted-to-persistent-state-as-this-identity-map-is-no-longer-valid
+        studies = db.session.execute(
+            sa.select(Study).filter_by(review_id=review_id).order_by(Study.id),
+            execution_options={"prebuffer_rows": True},
+        ).scalars()
+        # rows = (_study_to_row(study, extraction_label_types) for study in studies)
+        rows = [_study_to_row(study, extraction_label_types) for study in studies]
         if content_type == "text/csv":
             export_data = fileio.tabular.write_stream(
                 fieldnames, rows, quoting=csv.QUOTE_NONNUMERIC
@@ -102,9 +116,13 @@ def get(self, review_id, content_type):
             raise NotImplementedError("only 'text/csv' content type is available")
 
         response = make_response(export_data, 200)
-        response.headers["Content-type"] = content_type
+        response.headers.update(
+            {
+                "Content-Type": content_type,
+                "Content-Disposition": "attachment; filename=colandr-review-studies.csv",
+            }
+        )
         current_app.logger.info("studies data exported for %s", review)
-
         return response
 
 
@@ -179,6 +197,12 @@ def _study_to_row(
 class ExportScreeningsResource(Resource):
     @ns.doc(
         description="export screenings data",
+        params={
+            "review_id": {
+                "in": "query",
+                "type": "integer",
+            },
+        },
         responses={
             200: "successfully got screenings data for specified review",
             403: "current app user forbidden to export screenings data for specified review",

colandr/apis/resources/fulltext_uploads.py

@@ -143,7 +143,7 @@ def post(self, id, uploaded_file):
             return not_found_error(f"<Fulltext(id={id})> not found")
         if (
             current_user.is_admin is False
-            and current_user.user_review_assoc.filter_by(
+            and current_user.review_user_assoc.filter_by(
                 review_id=fulltext.review_id
             ).one_or_none()
             is None
@@ -221,7 +221,7 @@ def delete(self, id):
             return not_found_error(f"<Fulltext(id={id})> not found")
         if (
             current_user.is_admin is False
-            and current_user.user_review_assoc.filter_by(
+            and current_user.review_user_assoc.filter_by(
                 review_id=fulltext.review_id
             ).one_or_none()
             is None

colandr/apis/resources/review_teams.py

@@ -11,7 +11,7 @@
 from ... import tasks
 from ...extensions import db
 from ...lib import constants
-from ...models import Review, User
+from ...models import Review, ReviewUserAssoc, User
 from .. import auth
 from ..errors import bad_request_error, forbidden_error, not_found_error
 from ..schemas import UserSchema
@@ -170,29 +170,33 @@ def put(self, id, action, user_id, user_email, user_role):
             elif user in review_users:
                 return forbidden_error(f"{user} is already on this review")
             else:
-                review_users.append(user)
+                review.review_user_assoc.append(ReviewUserAssoc(review, user))
         # user is being *invited*, so send an invitation email
         elif action == "invite":
-            if user is None:
-                return not_found_error("no user found with given id or email")
+            if user is not None:
+                identity = user
+                user_email = user.email
+                template_name = "emails/invite_user_to_review.html"
             else:
-                token = jwtext.create_access_token(identity=user)
-                confirm_url = flask.url_for(
-                    "review_teams_confirm_review_team_invite_resource",
-                    id=id,
-                    token=token,
-                    _external=True,
-                )
-                html = render_template(
-                    "emails/invite_user_to_review.html",
-                    url=confirm_url,
-                    inviter_email=current_user.email,
-                    review_name=review.name,
+                identity = user_email
+                template_name = "emails/invite_new_user_to_review.html"
+            token = jwtext.create_access_token(identity=identity)
+            confirm_url = flask.url_for(
+                "review_teams_confirm_review_team_invite_resource",
+                id=id,
+                token=token,
+                _external=True,
+            )
+            html = render_template(
+                template_name,
+                url=confirm_url,
+                inviter_email=current_user.email,
+                review_name=review.name,
+            )
+            if current_app.config["MAIL_SERVER"]:
+                tasks.send_email.apply_async(
+                    args=[[user_email], "Let's collaborate!", "", html]
                 )
-                if current_app.config["MAIL_SERVER"]:
-                    tasks.send_email.apply_async(
-                        args=[[user.email], "Let's collaborate!", "", html]
-                    )
         elif action in ("make_owner", "set_role"):
             if user is None:
                 return not_found_error("no user found with given id or email")
@@ -255,14 +259,13 @@ def get(self, id, token):
         review = db.session.get(Review, id)
         if not review:
             return not_found_error(f"<Review(id={id})> not found")
-        review_users = review.users
 
         user = auth.get_user_from_token(token)
         if user is None:
             return not_found_error(f"no user found for token='{token}'")
 
-        if user not in review_users:
-            review_users.append(user)
+        if user not in review.users:
+            db.session.add(ReviewUserAssoc(review, user))
         else:
             return forbidden_error(f"{user} is already on this review")
 

colandr/apis/resources/reviews.py

@@ -196,7 +196,7 @@ def get(self, fields, _review_ids):
                 f'non-admin {current_user} passed admin-only "_review_ids" param'
             )
         else:
-            reviews = current_user.reviews.order_by(Review.id).all()
+            reviews = current_user.reviews
         if fields and "id" not in fields:
             fields.append("id")
         return ReviewSchema(only=fields, many=True).dump(reviews)