Skip to content

fix: Fix Keyerror resulting from improperly normalized form data

stephen requested to merge correctly-normalize-form-data into main

In !135 (merged), logged data relating to donations was hydrated to ensure that anyone trawling through logs to make sense of a failed transaction would have better context - the goal was to give each log as many of transaction number, amount, status, and timestamp as made sense for what was being logged and when.

In civicrm.repository.handle_donation_form_data, we accept incoming form data, tweak it so it's easier for CiviCRM to ingest upon receipt, and hand it to civicrm.repository.queue_donation for later retrieval (to pair with a webhook-sent transaction confirmation).

However, between "pruning needless form data" and "usefully hydrating log entries," the existence of "custom_donation" on the list of items to prune seems to have implied that "donation_amount" would exist and remain unpruned - rather than the reality, which is that it is only occasionally on the form object at all (if a donor selects an amount to donate which is reflected in the list of pre-populated donation buttons).

This mistaken leap in logic led to queue_donation attempting to log donation_amount values for every transaction, even those without that variable set, and its absence led to a KeyError being thrown.

This commit resolves this bug in several ways:

  1. Rather than pruning custom_donation from the form data to be passed along to CiviCRM, donation_amount is now pruned if present, as when it is present it will only ever contain the same value as custom_donation.
  2. Since custom_donation is always present in the donation form data - we explicitly determine this during validation in forms.py - and since we now refrain from pruning it, we instead use it to populate logging during transactions.
  3. CiviCRM repository testing has been extended to cover handle_donation_form_data.

Merge request reports