Tested TaskReview edge cases for Prolific

mephisto/README.md

@@ -79,7 +79,7 @@ A simple project with HTML-based UI task template [simple_static_task](../exampl
       --build \
       --publish 3001:3000 \
       --rm mephisto_dc \
-      cd /mephisto/examples/simple_static_task && python ./static_test_script.py
+      python /mephisto/examples/simple_static_task/static_test_script.py
   ```
 - Browser page (for the first task unit): [http://localhost:3001/?worker_id=x&assignment_id=1](http://localhost:3001/?worker_id=x&assignment_id=1)
 - Browser page should display an image, instruction, select and file inputs, and a submit button.
@@ -97,7 +97,7 @@ A simple project with React-based UI task template [static_react_task](../exampl
       --build \
       --publish 3001:3000 \
       --rm mephisto_dc \
-      cd /mephisto/examples/static_react_task && python ./run_task.py
+      python /mephisto/examples/static_react_task/run_task.py
   ```
 - Browser page (for the first task unit): [http://localhost:3001/?worker_id=x&assignment_id=1](http://localhost:3001/?worker_id=x&assignment_id=1).
 - Browser page should display an instruction line and two buttons (green and red).
@@ -118,7 +118,7 @@ A more complex example featuring worker-generated dynamic input: [mnist](../exam
       apt install curl && \
       pip install grafana torch pillow numpy && \
       mephisto metrics install && \
-      cd /mephisto/examples/remote_procedure/mnist && python ./run_task.py
+      python /mephisto/examples/remote_procedure/mnist/run_task.py
   ```
 - Browser page (for the first task unit): [http://localhost:3001/?worker_id=x&assignment_id=1](http://localhost:3001/?worker_id=x&assignment_id=1).
 - Browser page should display instructions and a layout with 3 rectangle fields for drawing numbers with a mouse, each field having inputs at the bottom.
@@ -248,8 +248,7 @@ or simply embed that command into your docker-compose entrypoint script.
         --build \
         --rm mephisto_dc \
         rm -rf /mephisto/tmp && \
-        cd /mephisto/examples/simple_static_task && \
-        HYDRA_FULL_ERROR=1 python ./static_test_prolific_script.py
+        HYDRA_FULL_ERROR=1 python /mephisto/examples/simple_static_task/static_test_prolific_script.py
     ```
 
   This TaskRun script will spin up an EC2 server, upload your React Task App to it, and create a Study on Prolific. Now all eligible workers will see your Task Units (with links poiting to EC2 server) on Prolific, and can complete it.

mephisto/abstractions/providers/prolific/api/exceptions.py

@@ -17,6 +17,9 @@ class ProlificException(Exception):
     def __init__(self, message: Optional[str] = None):
         self.message = message or self.default_message
 
+    def __str__(self) -> str:
+        return self.message
+
 
 class ProlificAPIKeyError(ProlificException):
     default_message = "API key is missing."

mephisto/abstractions/providers/prolific/prolific_agent.py

@@ -116,10 +116,11 @@ def approve_work(
         client = self._get_client()
         prolific_study_id = self.unit.get_prolific_study_id()
         worker_id = self.worker.get_prolific_participant_id()
+
+        datastore_unit = self.datastore.get_unit(self.unit_id)
         prolific_utils.approve_work(
             client,
-            study_id=prolific_study_id,
-            worker_id=worker_id,
+            submission_id=datastore_unit['prolific_submission_id'],
         )
 
         logger.debug(
@@ -148,10 +149,11 @@ def soft_reject_work(self, review_note: Optional[str] = None) -> None:
         client = self._get_client()
         prolific_study_id = self.unit.get_prolific_study_id()
         worker_id = self.worker.get_prolific_participant_id()
+
+        datastore_unit = self.datastore.get_unit(self.unit_id)
         prolific_utils.approve_work(
             client,
-            study_id=prolific_study_id,
-            worker_id=worker_id,
+            submission_id=datastore_unit['prolific_submission_id'],
         )
 
         logger.debug(
@@ -171,15 +173,15 @@ def reject_work(self, review_note: Optional[str] = None) -> None:
         client = self._get_client()
         prolific_study_id = self.unit.get_prolific_study_id()
         worker_id = self.worker.get_prolific_participant_id()
+        datastore_unit = self.datastore.get_unit(self.unit_id)
 
         # [Depends on Prolific] remove this suppression of exception when Prolific fixes their API
         from .api.exceptions import ProlificException
 
         try:
             prolific_utils.reject_work(
                 client,
-                study_id=prolific_study_id,
-                worker_id=worker_id,
+                submission_id=datastore_unit['prolific_submission_id'],
             )
         except ProlificException:
             logger.info(

mephisto/abstractions/providers/prolific/prolific_unit.py

@@ -28,10 +28,11 @@
 from mephisto.utils.logger_core import get_logger
 
 SUBMISSION_STATUS_TO_ASSIGNMENT_STATE_MAP = {
+    # Note that both "Accepted" and "Soft-rejected" Mephisto statuses
+    # result in "Approved" status on Prolific, so we can't match it here as a simple mapping
     SubmissionStatus.RESERVED: AssignmentState.CREATED,
     SubmissionStatus.TIMED_OUT: AssignmentState.EXPIRED,
     SubmissionStatus.AWAITING_REVIEW: AssignmentState.COMPLETED,
-    SubmissionStatus.APPROVED: AssignmentState.COMPLETED,
     SubmissionStatus.RETURNED: AssignmentState.COMPLETED,
     SubmissionStatus.REJECTED: AssignmentState.REJECTED,
 }
@@ -136,9 +137,14 @@ def get_status(self) -> str:
         if agent is None:
             if self.db_status in AssignmentState.completed():
                 logger.warning(f"Agent for completed unit {self} is None")
-
             return self.db_status
 
+        agent_status = agent.get_status()
+        if agent_status == AgentState.STATUS_EXPIRED:
+            # TODO: should we handle EXPIRED agent status somewhere earlier?
+            self.set_db_status(AssignmentState.EXPIRED)
+            return AssignmentState.EXPIRED
+
         # Get API client
         requester: "ProlificRequester" = self.get_requester()
         client = self._get_client(requester.requester_name)
@@ -193,12 +199,19 @@ def get_status(self) -> str:
             elif prolific_submission.status == SubmissionStatus.PROCESSING:
                 # This is just Prolific's transient status to move Submission between 2 statuses
                 pass
+            elif prolific_submission.status == SubmissionStatus.APPROVED:
+                if agent_status == AgentState.STATUS_APPROVED:
+                    external_status = AssignmentState.ACCEPTED
+                elif agent_status == AgentState.STATUS_SOFT_REJECTED:
+                    external_status = AssignmentState.SOFT_REJECTED
+                else:
+                    raise Exception(f"Unexpected Agent status `{agent_status}`")
             else:
                 external_status = SUBMISSION_STATUS_TO_ASSIGNMENT_STATE_MAP.get(
                     prolific_submission.status,
                 )
                 if not external_status:
-                    raise Exception(f"Unexpected Submission status {prolific_submission.status}")
+                    raise Exception(f"Unexpected Submission status `{prolific_submission.status}`")
 
         if external_status != local_status:
             self.set_db_status(external_status)

mephisto/abstractions/providers/prolific/prolific_utils.py

@@ -28,7 +28,6 @@
 from .api.base_api_resource import CREDENTIALS_CONFIG_PATH
 from .api.client import ProlificClient
 from .api.data_models import BonusPayments
-from .api.data_models import ListSubmission
 from .api.data_models import Participant
 from .api.data_models import ParticipantGroup
 from .api.data_models import Project
@@ -69,12 +68,14 @@ def get_authenticated_client(profile_name: str) -> ProlificClient:
 
 def setup_credentials(
     profile_name: str,
-    register_args: Optional[ProlificRequesterArgs],
+    register_args: ProlificRequesterArgs,
 ) -> bool:
-    if register_args is None:
+    # Get API key from task asrgs, then from env, then from user prompt
+    api_key = register_args.api_key
+    if not api_key:
+        api_key = os.environ.get("PROLIFIC_API_KEY", None)
+    if not api_key:
         api_key = input(f"Provide api key for {profile_name}: ")
-    else:
-        api_key = register_args.api_key
 
     if not os.path.exists(os.path.expanduser(CREDENTIALS_CONFIG_DIR)):
         os.mkdir(os.path.expanduser(CREDENTIALS_CONFIG_DIR))
@@ -738,26 +739,6 @@ def calculate_pay_amount(
     return total_cost
 
 
-# --- Submissions ---
-def _find_submission(
-    client: ProlificClient,
-    study_id: str,
-    worker_id: str,
-) -> Optional[ListSubmission]:
-    """Find a Submission by Study and Worker"""
-    try:
-        submissions: List[ListSubmission] = client.Submissions.list(study_id=study_id)
-    except (ProlificException, ValidationError):
-        logger.exception(f'Could not receive submissions for study "{study_id}"')
-        raise
-
-    for submission in submissions:
-        if submission.participant_id == worker_id:
-            return submission
-
-    return None
-
-
 def get_submission(client: ProlificClient, submission_id: str) -> Submission:
     try:
         submission: Submission = client.Submissions.retrieve(id=submission_id)
@@ -769,13 +750,12 @@ def get_submission(client: ProlificClient, submission_id: str) -> Submission:
 
 def approve_work(
     client: ProlificClient,
-    study_id: str,
-    worker_id: str,
+    submission_id: str,
 ) -> Union[Submission, None]:
-    submission: ListSubmission = _find_submission(client, study_id, worker_id)
+    submission: Submission = get_submission(client, submission_id)
 
     if not submission:
-        logger.warning(f'No submission found for study "{study_id}" and participant "{worker_id}"')
+        logger.warning(f'No submission found for id "{submission_id}"')
         return None
 
     # TODO (#1008): Handle other statuses later (when Submission was reviewed in Prolific UI)
@@ -784,27 +764,24 @@ def approve_work(
             submission: Submission = client.Submissions.approve(submission.id)
             return submission
         except (ProlificException, ValidationError):
-            logger.exception(
-                f'Could not approve submission for study "{study_id}" and participant "{worker_id}"'
-            )
+            logger.exception(f'Could not approve submission for id "{submission.id}"')
             raise
     else:
         logger.warning(
-            f'Cannot approve submission "{submission.id}" with status "{submission.status}"'
+            f'Cannot approve submission with id "{submission.id}" and status "{submission.status}"'
         )
 
     return None
 
 
 def reject_work(
     client: ProlificClient,
-    study_id: str,
-    worker_id: str,
+    submission_id: str,
 ) -> Union[Submission, None]:
-    submission: ListSubmission = _find_submission(client, study_id, worker_id)
+    submission: Submission = get_submission(client, submission_id)
 
     if not submission:
-        logger.warning(f'No submission found for study "{study_id}" and participant "{worker_id}"')
+        logger.warning(f'No submission found for id "{submission_id}"')
         return None
 
     # TODO (#1008): Handle other statuses later (when Submission was reviewed in Prolific UI)
@@ -813,13 +790,11 @@ def reject_work(
             submission: Submission = client.Submissions.reject(submission.id)
             return submission
         except (ProlificException, ValidationError):
-            logger.exception(
-                f'Could not reject submission for study "{study_id}" and participant "{worker_id}"'
-            )
+            logger.exception(f'Could not reject submission for id "{submission.id}"')
             raise
     else:
         logger.warning(
-            f'Cannot reject submission "{submission.id}" with status "{submission.status}"'
+            f'Cannot reject submission with id "{submission.id}" and status "{submission.status}"'
         )
 
     return None

mephisto/client/review_app/server/api/views/qualifications_view.py

@@ -24,9 +24,6 @@ def _find_qualifications_by_ids(
     with db.table_access_condition:
         conn = db._get_connection()
 
-        if debug:
-            conn.set_trace_callback(print)
-
         c = conn.cursor()
 
         qualifications_string = ",".join([f"{s}" for s in qualification_ids])
@@ -44,9 +41,6 @@ def _find_qualifications_by_ids(
         )
         rows = c.fetchall()
 
-        if debug:
-            conn.set_trace_callback(None)
-
         return [
             Qualification(db, str(r["qualification_id"]), row=r, _used_new_call=True) for r in rows
         ]

mephisto/client/review_app/server/api/views/stats_view.py

@@ -68,7 +68,6 @@ def _find_unit_reviews(
 
     with db.table_access_condition:
         conn = db._get_connection()
-        conn.set_trace_callback(print)
         c = conn.cursor()
         c.execute(
             f"""
@@ -130,7 +129,6 @@ def _find_units_for_worker(
 
     with db.table_access_condition:
         conn = db._get_connection()
-        conn.set_trace_callback(print)
         c = conn.cursor()
         c.execute(
             f"""

mephisto/client/review_app/server/api/views/tasks_view.py

@@ -19,9 +19,6 @@ def _find_tasks(db, debug: bool = False) -> List[StringIDRow]:
     with db.table_access_condition:
         conn = db._get_connection()
 
-        if debug:
-            conn.set_trace_callback(print)
-
         c = conn.cursor()
         c.execute(
             """
@@ -30,9 +27,6 @@ def _find_tasks(db, debug: bool = False) -> List[StringIDRow]:
         )
         rows = c.fetchall()
 
-        if debug:
-            conn.set_trace_callback(None)
-
         return rows
 
 

mephisto/client/review_app/server/db_queries.py

@@ -20,9 +20,6 @@ def find_units(
     with db.table_access_condition:
         conn = db._get_connection()
 
-        if debug:
-            conn.set_trace_callback(print)
-
         params = []
 
         task_query = "task_id = ?" if task_id else ""
@@ -46,7 +43,4 @@ def find_units(
         )
         rows = c.fetchall()
 
-        if debug:
-            conn.set_trace_callback(None)
-
         return rows