Payment & Collection methods should remain visible in the inline order list.

Why is the payment method removed here? This is important to have access to as admin.

See other comments: Payment & collection methods should remain part of the Order and be visible in the admin.

This is just a dictionary lookup.

This is a conceptual change that I was not aware of, and that I also don't agree with. A customer should be able to have different subscriptions connected to different payment methods, and know exactly which payment method is going to be used for which subscription.

This for-loop creates a transaction for every active payment method, so instead of charging once for a subscription it charges every payment method. This is unacceptable. Even for a simulated tick.

Also, since when does order.generate_transaction() actually charge a customer? And since when do we charge a customer in a simulated tick? The comment is probably misleading.

Same comment as above, I don't agree with simply iterating over the payment methods until one works.

What is the reason that these signals don't receive the transaction any more? The transaction is the thing that failed, and thus contains important information.

Not supporting manual Braintree payments was a deliberate design choice for the Development Fund. I'm assuming that this change was deliberate as well, to allow migrating existing Cloud subscribers.

To support both Cloud and DevFund in Looper, the collection methods for each gateway should be configurable via settings.py, and not hard-coded in Looper.

By removing SharedFields and adding some fields back, the Order no longer has a payment_method. This is not good, as we should be able to look at previous orders and see which payment method they were using. There was a good reason the code was keeping track of things like that, so please restore it. The concept is that an Order basically copies the information from Subscription, so that the Subscription can change without influencing any existing Orders. The code was structured to make this very easy to see & extend. Now not only is that structure gone, but there is also no comment in the Order or Subscription classes that say anything about fields being copied or that they should remain in sync between the two classes.

Motivate this choice, as right now it just won't happen (as there is no clear benefit described).

PEP 257: Multi-line docstrings consist of a summary line just like a one-line docstring, followed by a blank line, followed by a more elaborate description.

This can still return IpAddress(''), or any string that is not a valid IP address. Just use str as it was used before, as currently this type annotation hides the fact that any string can be returned.

I see the static typing advantage of using Optional[IpAddress], but as the function can still return any string, including empty strings and strings that don't represent any IP address, I strongly disagree with this approach. This now invites errors like in D7244 (where it is assumed that the value returned from this function is either None and not an empty string).