Browse Source

fix(ui) Improve user errors for failed transfers (#26318)

Handle user errors when transferring a project. This should help reduce
volume on (but not fully fix) JAVASCRIPT-TG2.
Mark Story 3 years ago
parent
commit
0ab0f05e2f

+ 21 - 1
static/app/actionCreators/projects.tsx

@@ -180,7 +180,27 @@ export function transferProject(
         );
       },
       err => {
-        addErrorMessage(tct('Error transferring [project]', {project: project.slug}));
+        let message = '';
+        // Handle errors with known failures
+        if (err.status >= 400 && err.status < 500 && err.responseJSON) {
+          message = err.responseJSON?.detail;
+        }
+
+        if (message) {
+          addErrorMessage(
+            tct('Error transferring [project]. [message]', {
+              project: project.slug,
+              message,
+            })
+          );
+        } else {
+          addErrorMessage(
+            tct('Error transferring [project]', {
+              project: project.slug,
+            })
+          );
+        }
+
         throw err;
       }
     );

+ 8 - 3
static/app/views/settings/projectGeneralSettings.tsx

@@ -70,7 +70,7 @@ class ProjectGeneralSettings extends AsyncView<Props, State> {
     }, handleXhrErrorResponse('Unable to remove project'));
   };
 
-  handleTransferProject = () => {
+  handleTransferProject = async () => {
     const {orgId} = this.props.params;
     const project = this.state.data;
     if (!project) {
@@ -80,10 +80,15 @@ class ProjectGeneralSettings extends AsyncView<Props, State> {
       return;
     }
 
-    transferProject(this.api, orgId, project, this._form.email).then(() => {
+    try {
+      await transferProject(this.api, orgId, project, this._form.email);
       // Need to hard reload because lots of components do not listen to Projects Store
       window.location.assign('/');
-    }, handleXhrErrorResponse('Unable to transfer project'));
+    } catch (err) {
+      if (err.status >= 500) {
+        handleXhrErrorResponse('Unable to transfer project')(err);
+      }
+    }
   };
 
   isProjectAdmin = () => new Set(this.props.organization.access).has('project:admin');

+ 48 - 0
tests/js/spec/views/settings/projectGeneralSettings.spec.jsx

@@ -4,10 +4,13 @@ import {mountWithTheme} from 'sentry-test/enzyme';
 import {mountGlobalModal} from 'sentry-test/modal';
 import {selectByValue} from 'sentry-test/select-new';
 
+import {addErrorMessage, addSuccessMessage} from 'app/actionCreators/indicator';
 import ProjectsStore from 'app/stores/projectsStore';
 import ProjectContext from 'app/views/projects/projectContext';
 import ProjectGeneralSettings from 'app/views/settings/projectGeneralSettings';
 
+jest.mock('app/actionCreators/indicator');
+
 describe('projectGeneralSettings', function () {
   const org = TestStubs.Organization();
   const project = TestStubs.ProjectDetails();
@@ -59,6 +62,9 @@ describe('projectGeneralSettings', function () {
 
   afterEach(function () {
     window.location.assign.mockRestore();
+    MockApiClient.clearMockResponses();
+    addSuccessMessage.mockReset();
+    addErrorMessage.mockReset();
   });
 
   it('renders form fields', function () {
@@ -145,7 +151,10 @@ describe('projectGeneralSettings', function () {
       .find('input[name="email"]')
       .simulate('change', {target: {value: 'billy@sentry.io'}});
     modal.find('Modal Button[priority="danger"]').simulate('click');
+    await tick();
+    await modal.update();
 
+    expect(addSuccessMessage).toHaveBeenCalled();
     expect(deleteMock).toHaveBeenCalledWith(
       `/projects/${org.slug}/${project.slug}/transfer/`,
       expect.objectContaining({
@@ -157,6 +166,45 @@ describe('projectGeneralSettings', function () {
     );
   });
 
+  it('handles errors on transfer project', async function () {
+    const deleteMock = MockApiClient.addMockResponse({
+      url: `/projects/${org.slug}/${project.slug}/transfer/`,
+      method: 'POST',
+      statusCode: 400,
+      body: {detail: 'An organization owner could not be found'},
+    });
+
+    const wrapper = mountWithTheme(
+      <ProjectGeneralSettings params={{orgId: org.slug, projectId: project.slug}} />,
+      TestStubs.routerContext()
+    );
+
+    const removeBtn = wrapper.find('.ref-transfer-project').first();
+
+    expect(removeBtn.prop('children')).toBe('Transfer Project');
+
+    // Click button
+    removeBtn.simulate('click');
+
+    // Confirm Modal
+    const modal = await mountGlobalModal();
+    modal
+      .find('input[name="email"]')
+      .simulate('change', {target: {value: 'billy@sentry.io'}});
+    modal.find('Modal Button[priority="danger"]').simulate('click');
+    await tick();
+    await modal.update();
+
+    expect(deleteMock).toHaveBeenCalled();
+    expect(addSuccessMessage).not.toHaveBeenCalled();
+
+    expect(addErrorMessage).toHaveBeenCalled();
+    const content = mountWithTheme(addErrorMessage.mock.calls[0][0]);
+    expect(content.text()).toEqual(
+      expect.stringContaining('An organization owner could not be found')
+    );
+  });
+
   it('displays transfer/remove message for non-admins', function () {
     routerContext.context.organization.access = ['org:read'];
     const wrapper = mountWithTheme(