WIP

metecho/api/model_mixins.py

@@ -1,5 +1,6 @@
 import re
 from collections import namedtuple
+from typing import Optional
 
 from asgiref.sync import async_to_sync
 from django.db import models
@@ -113,7 +114,12 @@ def notify_error(self, error, *, type_=None, originating_user_id, message=None):
         )
 
     def notify_scratch_org_error(
-        self, *, error, type_, originating_user_id, message=None
+        self,
+        *,
+        error: Exception,
+        type_: str,
+        originating_user_id: str,
+        message: Optional[dict] = None,
     ):
         """
         This is only used in the ScratchOrg model currently, but it

metecho/api/push.py

@@ -64,10 +64,14 @@
         SCRATCH_ORG_RECREATE
 """
 from copy import deepcopy
+from typing import Optional, TYPE_CHECKING
 
 from channels.layers import get_channel_layer
 from django.utils.translation import gettext_lazy as _
 
+if TYPE_CHECKING:
+    from metecho.api.models import ScratchOrg
+
 from ..consumer_utils import get_set_message_semaphore
 from .constants import CHANNELS_GROUP_NAME, LIST
 
@@ -105,13 +109,18 @@ async def report_error(user):
 
 
 async def report_scratch_org_error(
-    instance, *, error, type_, originating_user_id, message=None
+    instance: "ScratchOrg",
+    *,
+    error: Exception,
+    type_: str,
+    originating_user_id: str,
+    message: Optional[dict] = None
 ):
     # Unwrap the error in the case that there's only one,
     # which is the most common case, per this discussion:
     # https://github.com/SFDO-Tooling/Metecho/pull/149#discussion_r327308563
     try:
-        prepared_message = error.content
+        prepared_message = error.content  # type: ignore
         if isinstance(prepared_message, list) and len(prepared_message) == 1:
             prepared_message = prepared_message[0]
         if isinstance(prepared_message, dict):

metecho/api/serializers.py

@@ -1,4 +1,4 @@
-from typing import Optional, OrderedDict
+from typing import Literal, Optional, OrderedDict
 
 from allauth.socialaccount.models import SocialAccount
 from django.contrib.auth import get_user_model
@@ -26,6 +26,7 @@
     SiteProfile,
     Task,
     TaskReviewStatus,
+    User as UserModel,
 )
 from .sf_run_flow import is_org_good
 from .validators import CaseInsensitiveUniqueTogetherValidator, UnattachedIssueValidator
@@ -895,11 +896,16 @@ def update(self, task, data):
         return task
 
     def _handle_reassign(
-        self, type_, instance, validated_data, user, originating_user_id
+        self,
+        type_: Literal["dev"] | Literal["qa"],
+        instance: Task,
+        validated_data,
+        user: UserModel,
+        originating_user_id: str,
     ):
         epic = instance.epic
-        new_assignee = validated_data.get(f"assigned_{type_}")
-        existing_assignee = getattr(instance, f"assigned_{type_}")
+        new_assignee: Optional[GitHubUser] = validated_data.get(f"assigned_{type_}")
+        existing_assignee: Optional[GitHubUser] = getattr(instance, f"assigned_{type_}")
         assigned_user_has_changed = new_assignee != existing_assignee
         has_assigned_user = bool(new_assignee)
         org_type = {"dev": ScratchOrgType.DEV, "qa": ScratchOrgType.QA}[type_]
@@ -912,48 +918,37 @@ def _handle_reassign(
             if validated_data.get(f"should_alert_{type_}"):
                 self.try_send_assignment_emails(instance, type_, validated_data, user)
 
-            reassigned_org = False
-            # We want to consider soft-deleted orgs, too:
-            orgs = [
-                *instance.orgs.active().filter(org_type=org_type),
-                *instance.orgs.inactive().filter(org_type=org_type),
-            ]
-            for org in orgs:
-                new_user = self._valid_reassign(
-                    type_, org, validated_data[f"assigned_{type_}"]
+            candidate_org = instance.orgs.active().filter(org_type=org_type).first()
+            if candidate_org:
+                new_user = self.get_matching_assigned_user(
+                    type_, {f"assigned_{type_}": new_assignee}
                 )
-                valid_commit = org.latest_commit == (
-                    instance.commits[0] if instance.commits else instance.origin_sha
-                )
-                org_still_exists = is_org_good(org)
+                org_still_exists = is_org_good(candidate_org)
+
+                # See TaskViewSet.can_reassign() in views.py for details on this logic.
+                # Here, we also validate that the org hasn't been deleted behind the scenes.
                 if (
                     org_still_exists
                     and new_user
-                    and valid_commit
-                    and not reassigned_org
+                    and candidate_org.owner_sf_username == new_user.sf_username
                 ):
-                    org.queue_reassign(
+                    candidate_org.queue_reassign(
                         new_user=new_user, originating_user_id=originating_user_id
                     )
-                    reassigned_org = True
-                elif org.deleted_at is None:
-                    org.delete(
-                        originating_user_id=originating_user_id, preserve_sf_org=True
+                else:
+                    # We don't particularly want to leak scratch orgs.
+                    # If we have an org but cannot transfer it, raise an exception.
+                    raise serializers.ValidationError(
+                        _(
+                            "Unable to transfer scratch org. It may be deleted or owned by a different Dev Hub."
+                        )
                     )
         elif not has_assigned_user:
             for org in [*instance.orgs.active().filter(org_type=org_type)]:
                 org.delete(
                     originating_user_id=originating_user_id, preserve_sf_org=True
                 )
 
-    def _valid_reassign(self, type_, org, new_assignee):
-        new_user = self.get_matching_assigned_user(
-            type_, {f"assigned_{type_}": new_assignee}
-        )
-        if new_user and org.owner_sf_username == new_user.sf_username:
-            return new_user
-        return None
-
     def try_send_assignment_emails(self, instance, type_, validated_data, user):
         assigned_user = self.get_matching_assigned_user(type_, validated_data)
         if assigned_user:
@@ -972,8 +967,10 @@ def try_send_assignment_emails(self, instance, type_, validated_data, user):
             )
             assigned_user.notify(subject, body)
 
-    def get_matching_assigned_user(self, type_, validated_data):
-        gh_user = validated_data.get(f"assigned_{type_}")
+    def get_matching_assigned_user(
+        self, type_: Literal["dev"] | Literal["qa"], validated_data
+    ) -> Optional[UserModel]:
+        gh_user: GitHubUser = validated_data.get(f"assigned_{type_}")
         sa = SocialAccount.objects.filter(
             provider="github", uid=getattr(gh_user, "id", "")
         ).first()

metecho/api/sf_run_flow.py

@@ -104,7 +104,7 @@ def is_org_good(org):
     org_name = org.org_config_name
     try:
         org_config = OrgConfig(config, org_name)
-        org_config.refresh_oauth_token(None)
+        org_config.refresh_oauth_token(None, is_sandbox=True)
         return "access_token" in org_config.config
     except Exception:
         return False

metecho/api/views.py

@@ -513,19 +513,31 @@ def can_reassign(self, request, pk=None):
             "user",
             None,
         )
-        valid_commit = org and org.latest_commit == (
-            task.commits[0] if task.commits else task.origin_sha
-        )
-        return Response(
-            {
-                "can_reassign": bool(
-                    new_user
-                    and org
-                    and org.owner_sf_username == new_user.sf_username
-                    and valid_commit
+        # An org can be reassigned if:
+        # - We have a user to assign it to (not just a GitHubUser)
+        # - The org exists
+        # - The org is owned by the same Dev Hub user as the Dev Hub user of the target user
+        issues = []
+        if not new_user:
+            issues.append(
+                str(
+                    _(
+                        "Scratch orgs must be owned by a user who has logged in to Metecho."
+                    )
                 )
-            }
-        )
+            )
+        if not org:
+            issues.append(str(_("No scratch org of the specified type was found.")))
+        if new_user and org.owner_sf_username != new_user.sf_username:
+            issues.append(
+                str(
+                    _(
+                        "The new owner has a different Dev Hub. Scratch orgs cannot be transferred across Dev Hubs."
+                    )
+                )
+            )
+
+        return Response({"can_reassign": len(issues) == 0, "issues": issues})
 
     @extend_schema(request=TaskAssigneeSerializer)
     @action(detail=True, methods=["POST", "PUT"])

src/js/components/orgs/taskOrgCards.tsx

@@ -13,6 +13,7 @@ import { deleteObject, updateObject } from '@/js/store/actions';
 import { refetchOrg } from '@/js/store/orgs/actions';
 import { Org, OrgsByParent } from '@/js/store/orgs/reducer';
 import { Task } from '@/js/store/tasks/reducer';
+import { addToast } from '@/js/store/toasts/actions';
 import { GitHubUser, User } from '@/js/store/user/reducer';
 import { selectUserState } from '@/js/store/user/selectors';
 import apiFetch from '@/js/utils/api';
@@ -100,23 +101,24 @@ const TaskOrgCards = ({
     [dispatch],
   );
 
-  const checkIfTaskCanBeReassigned = async (assignee: number) => {
-    const { can_reassign } = await apiFetch({
-      url: window.api_urls.task_can_reassign(task.id),
-      dispatch,
-      opts: {
-        method: 'POST',
-        body: JSON.stringify({
-          role: 'assigned_dev',
-          gh_uid: assignee,
-        }),
-        headers: {
-          'Content-Type': 'application/json',
+  const checkIfTaskCanBeReassigned = useCallback(
+    (assignee: number): Promise<{ can_reassign: boolean; issues: string[] }> =>
+      apiFetch({
+        url: window.api_urls.task_can_reassign(task.id),
+        dispatch,
+        opts: {
+          method: 'POST',
+          body: JSON.stringify({
+            role: 'assigned_dev',
+            gh_uid: assignee,
+          }),
+          headers: {
+            'Content-Type': 'application/json',
+          },
         },
-      },
-    });
-    return can_reassign;
-  };
+      }),
+    [dispatch, task.id],
+  );
 
   const deleteOrg = useCallback(
     (org: Org) => {
@@ -190,28 +192,47 @@ const TaskOrgCards = ({
     }
   };
 
-  const handleAssignUser = async ({
-    type,
-    assignee,
-    shouldAlertAssignee,
-  }: AssignedUserTracker) => {
-    const org = orgs[type];
-    /* istanbul ignore else */
-    if (org && type === ORG_TYPES.DEV) {
-      let canReassign = false;
-      if (assignee) {
-        canReassign = await checkIfTaskCanBeReassigned(assignee);
-      }
-      if (canReassign) {
+  const handleAssignUser = useCallback(
+    async ({ type, assignee, shouldAlertAssignee }: AssignedUserTracker) => {
+      const org = orgs[type];
+      /* istanbul ignore else */
+      if (org && type === ORG_TYPES.DEV) {
+        if (assignee) {
+          const { can_reassign, issues } = await checkIfTaskCanBeReassigned(
+            assignee,
+          );
+          if (can_reassign) {
+            assignUser({ type, assignee, shouldAlertAssignee });
+          } else {
+            // eslint-disable-next-line @typescript-eslint/no-unused-vars
+            dispatch(
+              addToast({
+                heading: t('Cannot transfer scratch org'),
+                details: t(
+                  'The current scratch org cannot be transferred to the selected GitHub user. Remove the scratch org before transferring this task or correct the following issues: {{issueDescription}}',
+                  { issueDescription: issues.join('\n') },
+                ),
+                variant: 'error',
+              }),
+            );
+          }
+        } else {
+          checkForOrgChanges(org as Org);
+          setIsWaitingToRemoveUser({ type, assignee, shouldAlertAssignee });
+        }
+      } else if (type !== ORG_TYPES.PLAYGROUND) {
         assignUser({ type, assignee, shouldAlertAssignee });
-      } else {
-        checkForOrgChanges(org as Org);
-        setIsWaitingToRemoveUser({ type, assignee, shouldAlertAssignee });
       }
-    } else if (type !== ORG_TYPES.PLAYGROUND) {
-      assignUser({ type, assignee, shouldAlertAssignee });
-    }
-  };
+    },
+    [
+      assignUser,
+      checkForOrgChanges,
+      checkIfTaskCanBeReassigned,
+      dispatch,
+      orgs,
+      t,
+    ],
+  );
 
   let handleCreate: (...args: any[]) => void = openConnectModal;
   const userIsConnected =