Browse Source

fix(aws-lambda): aws lambda cant finish installation (#30826)

This PR fixes a bug where the user can't finish installing the AWS Lambda integration
Stephen Cefali 3 years ago
parent
commit
20d815fb08

+ 5 - 4
src/sentry/integrations/aws_lambda/integration.py

@@ -18,7 +18,6 @@ from sentry.integrations import (
 from sentry.integrations.serverless import ServerlessMixin
 from sentry.models import OrganizationIntegration, Project, ProjectStatus
 from sentry.pipeline import PipelineView
-from sentry.utils import json
 from sentry.utils.compat import map
 from sentry.utils.sdk import capture_exception
 
@@ -316,9 +315,11 @@ class AwsLambdaCloudFormationPipelineView(PipelineView):
 class AwsLambdaListFunctionsPipelineView(PipelineView):
     def dispatch(self, request: Request, pipeline) -> Response:
         if request.method == "POST":
-            # accept form data or json data
-            # form data is needed for tests
-            data = request.POST or json.loads(request.body)
+            raw_data = request.POST
+            data = {}
+            for key, val in raw_data.items():
+                # form posts have string values for booleans and this form only sends booleans
+                data[key] = val == "true"
             pipeline.bind_state("enabled_lambdas", data)
             return pipeline.next_step()
 

+ 16 - 6
static/app/views/integrationPipeline/awsLambdaFunctionSelect.tsx

@@ -39,12 +39,11 @@ export default class AwsLambdaFunctionSelect extends Component<Props, State> {
     super(props);
     makeObservable(this, {allStatesToggled: computed});
   }
-
   state: State = {
     submitting: false,
   };
 
-  model = new FormModel({apiOptions: {baseUrl: window.location.origin}});
+  model = new FormModel();
 
   get initialData() {
     const {lambdaFunctions} = this.props;
@@ -72,8 +71,12 @@ export default class AwsLambdaFunctionSelect extends Component<Props, State> {
     return Object.values(this.model.getData()).every(val => val);
   }
 
+  get formFields() {
+    const data = this.model.getTransformedData();
+    return Object.entries(data).map(([name, value]) => ({name, value}));
+  }
+
   handleSubmit = () => {
-    this.model.saveForm();
     this.setState({submitting: true});
   };
 
@@ -155,10 +158,10 @@ export default class AwsLambdaFunctionSelect extends Component<Props, State> {
           {t('Decide which functions you would like to enable for Sentry monitoring')}
           <StyledForm
             initialData={this.initialData}
-            skipPreventDefault
             model={this.model}
             apiEndpoint="/extensions/aws_lambda/setup/"
             hideFooter
+            preventFormResetOnUnmount
           >
             <JsonForm renderHeader={() => FormHeader} forms={[formFields]} />
           </StyledForm>
@@ -178,9 +181,16 @@ export default class AwsLambdaFunctionSelect extends Component<Props, State> {
         <Observer>
           {() => (
             <FooterWithButtons
+              formProps={{
+                action: '/extensions/aws_lambda/setup/',
+                method: 'post',
+                onSubmit: this.handleSubmit,
+              }}
+              formFields={this.formFields}
               buttonText={t('Finish Setup')}
-              onClick={this.handleSubmit}
-              disabled={this.model.isError || this.model.isSaving}
+              disabled={
+                this.model.isError || this.model.isSaving || this.state.submitting
+              }
             />
           )}
         </Observer>

+ 20 - 5
static/app/views/integrationPipeline/components/footerWithButtons.tsx

@@ -4,13 +4,28 @@ import styled from '@emotion/styled';
 import Button from 'sentry/components/actions/button';
 import space from 'sentry/styles/space';
 
-type Props = {buttonText: string} & Partial<
-  Pick<React.ComponentProps<typeof Button>, 'disabled' | 'onClick' | 'href'>
->;
+type Props = {
+  buttonText: string;
+  formProps?: Omit<React.HTMLProps<HTMLFormElement>, 'as'>;
+  formFields?: Array<{name: string; value: any}>;
+} & Partial<Pick<React.ComponentProps<typeof Button>, 'disabled' | 'onClick' | 'href'>>;
 
-export default function FooterWithButtons({buttonText, ...rest}: Props) {
+export default function FooterWithButtons({
+  buttonText,
+  formFields,
+  formProps,
+  ...rest
+}: Props) {
+  /**
+   * We use a form post here to replicate what we do with standard HTML views for the integration pipeline.
+   * Since this is a form post, we need to pass a hidden replica of the form inputs
+   * so we can submit this form instead of the one collecting the user inputs.
+   */
   return (
-    <Footer>
+    <Footer data-test-id="aws-lambda-footer-form" {...formProps}>
+      {formFields?.map(field => {
+        return <input type="hidden" key={field.name} {...field} />;
+      })}
       <Button priority="primary" type="submit" size="xsmall" {...rest}>
         {buttonText}
       </Button>

+ 5 - 1
static/app/views/settings/components/forms/form.tsx

@@ -82,6 +82,10 @@ type Props = {
    */
   onSubmit?: OnSubmitCallback;
   onPreSubmit?: () => void;
+  /**
+   * Ensure the form model isn't reset when the form unmounts
+   */
+  preventFormResetOnUnmount?: boolean;
 } & Pick<FormOptions, 'onSubmitSuccess' | 'onSubmitError' | 'onFieldChange'>;
 
 export default class Form extends React.Component<Props> {
@@ -113,7 +117,7 @@ export default class Form extends React.Component<Props> {
   }
 
   componentWillUnmount() {
-    this.model.reset();
+    !this.props.preventFormResetOnUnmount && this.model.reset();
   }
 
   model: FormModel = this.props.model || new FormModel();

+ 1 - 1
static/app/views/settings/components/forms/jsonForm.tsx

@@ -44,7 +44,7 @@ class JsonForm extends React.Component<Props, State> {
   }
 
   UNSAFE_componentWillReceiveProps(nextProps: Props) {
-    if (this.props.location.hash !== nextProps.location.hash) {
+    if (nextProps.location && this.props.location.hash !== nextProps.location.hash) {
       const hash = nextProps.location.hash;
       this.scrollToHash(hash);
       this.setState({highlighted: hash});

+ 8 - 21
tests/js/spec/views/integrationPipeline/awsLambdaFunctionSelect.spec.jsx

@@ -1,36 +1,23 @@
-import {mountWithTheme} from 'sentry-test/enzyme';
+import {mountWithTheme, screen} from 'sentry-test/reactTestingLibrary';
 
-import {Client} from 'sentry/api';
 import AwsLambdaFunctionSelect from 'sentry/views/integrationPipeline/awsLambdaFunctionSelect';
 
 describe('AwsLambdaFunctionSelect', () => {
-  let wrapper;
-  let lambdaFunctions;
-  let mockRequest;
+  let lambdaFunctions, container;
   beforeEach(() => {
-    mockRequest = Client.addMockResponse({
-      url: '/extensions/aws_lambda/setup/',
-      body: {},
-    });
-
     lambdaFunctions = [
       {FunctionName: 'lambdaA', Runtime: 'nodejs12.x'},
       {FunctionName: 'lambdaB', Runtime: 'nodejs10.x'},
       {FunctionName: 'lambdaC', Runtime: 'nodejs10.x'},
     ];
-    wrapper = mountWithTheme(
+    ({container} = mountWithTheme(
       <AwsLambdaFunctionSelect lambdaFunctions={lambdaFunctions} />
-    );
+    ));
   });
   it('choose lambdas', () => {
-    wrapper.find('button[name="lambdaB"]').simulate('click');
-    wrapper.find('StyledButton[aria-label="Finish Setup"]').simulate('click');
-
-    expect(mockRequest).toHaveBeenCalledWith(
-      '/extensions/aws_lambda/setup/',
-      expect.objectContaining({
-        data: {lambdaA: true, lambdaB: false, lambdaC: true},
-      })
-    );
+    expect(container).toSnapshot();
+    expect(screen.getByLabelText('lambdaB')).toBeInTheDocument();
+    expect(screen.getByLabelText('Finish Setup')).toBeInTheDocument();
+    // TODO: add assertion for form post
   });
 });

+ 6 - 6
tests/sentry/integrations/aws_lambda/test_integration.py

@@ -180,7 +180,7 @@ class AwsLambdaIntegrationTest(IntegrationTestCase):
         # string instead of boolean
         resp = self.client.post(
             self.setup_path,
-            data={"lambdaB": True},
+            data={"lambdaB": "true", "lambdaA": "false"},
             format="json",
             HTTP_ACCEPT="application/json",
             headers={"Content-Type": "application/json", "Accept": "application/json"},
@@ -244,7 +244,7 @@ class AwsLambdaIntegrationTest(IntegrationTestCase):
 
         resp = self.client.post(
             self.setup_path,
-            data={"lambdaA": True},
+            data={"lambdaA": "true"},
             format="json",
             HTTP_ACCEPT="application/json",
             headers={"Content-Type": "application/json", "Accept": "application/json"},
@@ -320,7 +320,7 @@ class AwsLambdaIntegrationTest(IntegrationTestCase):
 
         resp = self.client.post(
             self.setup_path,
-            {"lambdaB": True},
+            {"lambdaB": "true"},
             format="json",
             HTTP_ACCEPT="application/json",
             headers={"Content-Type": "application/json", "Accept": "application/json"},
@@ -378,7 +378,7 @@ class AwsLambdaIntegrationTest(IntegrationTestCase):
 
         resp = self.client.post(
             self.setup_path,
-            {"lambdaB": True},
+            {"lambdaB": "true"},
             format="json",
             HTTP_ACCEPT="application/json",
             headers={"Content-Type": "application/json", "Accept": "application/json"},
@@ -437,7 +437,7 @@ class AwsLambdaIntegrationTest(IntegrationTestCase):
 
         resp = self.client.post(
             self.setup_path,
-            {"lambdaB": True},
+            {"lambdaB": "true"},
             format="json",
             HTTP_ACCEPT="application/json",
             headers={"Content-Type": "application/json", "Accept": "application/json"},
@@ -501,7 +501,7 @@ class AwsLambdaIntegrationTest(IntegrationTestCase):
 
         resp = self.client.post(
             self.setup_path,
-            {"lambdaB": True},
+            {"lambdaB": "true"},
             format="json",
             HTTP_ACCEPT="application/json",
             headers={"Content-Type": "application/json", "Accept": "application/json"},