Fix action form's .onSuccess behaviour

The `handleSuccess` function was mutating the response object (data) to
add a `petitionForm` field with the values of the action form's
elements. There are (at least) a couple of reasons why this was wrong:

1. It's bad practice to mutate a function parameter as we don't know
where else it's going to be consumed, and how. It adds unnecessary
complexity to the codebase and you're introducing side effects that may
make debugging difficult in the future (case in point: now).

2. Not all action forms are petition forms. This side effect can cause
issues in other types of forms, and also places limits on how we can
extend action form's functionality.

Furthermore, the reason why that code was added was simply to pass the
data to the petition form's callback, so that it, in turn, could
dispatch an action to the redux store so that we can "log in" a member.
That was the only place where that extra field was being used.

A better approach
-----------------

This commit refactors the code to dispatch the action from the
`handleSuccess` function in action_form.js, and make sure that the data
is correctly serialised when we dispatch. In the reducer, we only pick
the form values that are relevant to the fundraiser form[1]. Since both
forms are different and may contain different required / optional
fields, we only pick the relevant fields for our form, and check which
ones are still outstanding. This is a cleaner way to copy data from one
form to another, avoiding corrupted data.

Notes
-----

1. Our codebase is still not "correct" in the sense that, if a page were
to have _two_ donation forms (for example a normal fundraiser and an
after-donate monthly upsell), the same reducer would kick in. We still
need to refactor our fundraiser code, and separate forms from
fundraisers.

app/javascript/member-facing/backbone/action_form.js

@@ -20,6 +20,7 @@ import {
   toggleModal,
 } from '../../state/consent';
 import { resetMember } from '../../state/member/reducer';
+import { actionFormUpdated } from '../../state/fundraiser/actions';
 
 const ActionForm = Backbone.View.extend({
   el: 'form.action-form',
@@ -336,28 +337,20 @@ const ActionForm = Backbone.View.extend({
 
   handleSuccess(e, data) {
     // FIXME: we should return consistently from the backend
-    // FIXME: we should not rely on mutating function arguments of unkown type
     ee.emit('action:submitted_success');
-    if (typeof data === 'object') data.petitionForm = this.formValues();
     Backbone.trigger('form:submitted', e, data);
+    this.store.dispatch(actionFormUpdated(this.formData()));
   },
 
-  formValues() {
-    const values = {};
-    this.$(
-      'input[type=text], input[type=email], input[type=tel], textarea, select'
-    ).each((i, input) => {
-      values[input.name] = this.$(input).val();
-    });
-
-    this.$('input[type=checkbox]').each((i, input) => {
-      values[input.name] = input.checked ? '1' : '0';
-    });
-
-    this.$('input[type=radio]:checked').each((i, input) => {
-      values[input.name] = this.$(input).val();
-    });
-    return values;
+  formData() {
+    return _.reduce(
+      $(this.el).serializeArray(),
+      (reducedData, arrayItem) => ({
+        ...reducedData,
+        [arrayItem.name]: arrayItem.value,
+      }),
+      {}
+    );
   },
 
   handleFailure(e, data) {

app/javascript/state/fundraiser/actions.js

@@ -1,7 +1,7 @@
 // @flow
 import ee from '../../shared/pub_sub';
 
-import type { FundraiserAction, PaymentType } from './types';
+import type { FundraiserAction, PaymentType, ActionFormField } from './types';
 
 export function changeAmount(payload: ?number): FundraiserAction {
   ee.emit('fundraiser:change_amount', payload);
@@ -38,3 +38,7 @@ export function setStoreInVault(payload: boolean = false): FundraiserAction {
 export function setPaymentType(payload: PaymentType): FundraiserAction {
   return { type: 'set_payment_type', payload };
 }
+
+export function actionFormUpdated(data: ActionFormField): FundraiserAction {
+  return { type: '@@chmp:action_form:updated', payload: data };
+}

app/javascript/state/fundraiser/reducer.js

@@ -62,18 +62,6 @@ export default (state: State = initialState, action: Action): State => {
       return { ...state, ...initialData };
     case 'search_string_overrides':
       return searchStringOverrides(state, action.payload);
-    case 'login_member':
-      const formValues = action.payload.formValues || {};
-      const outstandingFields = state.fields
-        .map(field => field.name)
-        .filter(fieldName => !keys(formValues).includes(fieldName));
-
-      return {
-        ...state,
-        form: formValues,
-        formValues,
-        outstandingFields,
-      };
     case 'reset_member':
       return {
         ...state,
@@ -141,6 +129,19 @@ export default (state: State = initialState, action: Action): State => {
           currency: state.currency,
         }),
       };
+    // Update our form with data from another form
+    // E.g. petition was signed, so we can re-use the data from that form in
+    // this form.
+    case '@@chmp:action_form:updated':
+      const relevantFields = state.fields.map(field => field.name);
+      const formValues = pick(action.payload, relevantFields);
+      const outstandingFields = relevantFields.filter(key => !formValues[key]);
+      return {
+        ...state,
+        form: formValues,
+        formValues,
+        outstandingFields,
+      };
     default:
       return state;
   }

app/javascript/state/fundraiser/types.js

@@ -13,6 +13,8 @@ export type FeaturedAmountState = {
   preselectAmount: boolean,
 };
 
+export type ActionFormField = { [key: string]: string };
+
 export type Fundraiser = {
   currency: string,
   currentPaymentType: PaymentType,
@@ -40,6 +42,7 @@ export type Fundraiser = {
 
 export type FundraiserAction =
   | { type: 'initialize_fundraiser', payload: FundraiserInitializationOptions }
+  | { type: '@@chmp:action_form:updated', payload: ActionFormField }
   | { type: 'change_amount', payload: ?number }
   | { type: 'change_currency', payload: string }
   | { type: 'change_step', payload: number }