Extension:DonationInterface/Refactor 2015

Defining Refactor

• Refactoring something should not change any features - there's no UI changes at all to the user, only to the back end
• We should write complete tests that pass as is, in places that should more or less surround the pieces we want to refactor. Then, we write the new pieces, and know we didn't fail completely if the tests still pass. Or, you know... get a really solid false sense of security about the whole deal.
• Fixing bugs as a direct by-product of the refactor is OK, but not as a goal for the refactor
• Documentation is a requirement to mark a refactored item as finished

Candidates for Refactoring

UI/Forms

• CSS - it's all over the place. Much of this must be unused, and should be removed.
• JS - same thing
• Reuse as many template pieces between gateways / countries / whatever else we're currently duplicating on, as possible
  • Handle the excessive amount of template file duplication without sacrificing flexibility, reusing as universally as possible
• Remove php form cruft - Much of the extension doesn't take RapidHTML for granted, and we basically do now.
  • Replace template engine
• Universal form (may not be possible)
• Friendly error messages for situations we can tell the donors about (not a refactor concern. This is new work)
• Form validation is currently taking place in GatewayPage.php, in the validateForm function. It says all over the thing that it should be somewhere else. task T86249
• Worldpay forms are using a grid system. Would be nice to take the other forms in this direction too.

GatewayPage child classes

• Remove all control logic from the GatewayPage view classes, and move to a new UI controller class, or back into the adapter class. Requires us to think about routing.
• Communication between the adapter and UI controllers should be made simpler and standardized. Define the possibilities: show a form (possibly with an iframe and other dynamic content), handle form submission, redirect to a gateway-hosted page, or redirect to our Thank You or failure pages.

DonationData class

• There is some evidence currently in the code, relating to a dream we had once, in the way we're currently trying (and failing) to handle order_id. This should be rolled in to the main strategy:
  • Each piece of data that DonationData keeps, should be an instance of a new data item class, not a raw array.
    • The new class should keep track of all places that the data item can currently be pulled from ($_GET, $_POST, $_SESSION, passed to other class via constructor denoting some kind of batch mode, whatever else)
    • DonationData should keep general rules about what sources should win over other data sources, for each individual data item
    • DonationData should try to call back to specific gateway adapter classes for rule overrides
• Not entirely sure where to put this one, but it's possible that an instantiated DonationData object shouldn't be a member of the gateway at all, if we have a good mechanism for callbacks.

Gateway adapter classes

• Refactor Transaction Structure
  • Put transaction structure data nodes somewhere that they can easily be added/ignored
  • Transformations become first class entities, or at least easy (and clear) to compose.
  • Consider moving each transaction to its own controller class.
• Improve readability and reduce complexity
• Examine child classes for things to move to parent, and vice versa
• Kill PFP and all its stupid references, right in its face.
  • This also involves moving all the resources that live in pfp, or are *called* pfp-something, or Payflow-something, or any variation on those things, to the common area and renaming them everywhere.
• Separate declarative and code elements of adapters, see https://gerrit.wikimedia.org/r/#/c/65003/
• Remove logging and logging-related functionality from adapter classes
  • Create a logger class that will do all the things we currently do, but won't require that the current adapter object is fully constructed before the logging will work without erroring out.
• Clean up session handling
• Refactor the way we store and handle payment method and submethod... violates REFACTOR ONLY rule. AW: I think this is worth revisiting: encapsulate as PaymentMethod objects, existing storage can still use (method, submethod) but also introduce a new, single unique name for each (sub)method. Most important in my perspective is to stop making a distinction between the two levels of payment method, simplifying adapter code. task T86256
• Polish antifraud rules
  • antifraud cruft removal
• Transaction Status buckets: The map of their status to our buckets
• Move XML and NVP building into helper classes, but we need a way to override some of that behavior from adapters.
• Look at all the places where unexpected things were overridden in (particularly) Adyen and Amazon, and try to build these things in to the parent class where it makes sense to do so. (lots of TODO complaining in these classes)
• Look at all the gateways and how they use the functions defined in the gateway parent's interface, and if they're being properly used. (I know they're not: getResponseErrors is for instance being used to return a response code (error or not), and the error switching is happening in processResponse for at least one gateway.

ContributionTracking

• Not working on Donate wiki
• Make it not a SPOF
• UTM information handling
• Later step: would be nice to record each event rather than rollup
  • UTM source in its current form is a joke. All the dot-separated bits of information should be their own columns, so we have a hope of querying any of that information in a reasonable way.

Misc

• Remove all heavy lifting from class constructors <--helps with unit testing A LOT
• Vagrant role for working on payments.wikimedia.org. The remaining glue here is to modify the MultiWiki module to handle a second mediawiki-core source checkout, under another branch.
  • https://phabricator.wikimedia.org/T78739
• LocalSettings. Is a mess. See what we can do about that.
  • Admin UI.
  • Status page for all the stuff, lockouts.
• Once the UI is decoupled
  • take UI out of the DI extension entirely.
  • Then, beef up the standaloneness of DI payments code.
  • Merge into SmashPig.
• Make i18n strings sane.
  • Lots of unused stuff and copies.
  • Actually sort into interface, gateway, etc... or pull out into more obviously universal place
    • Definitely the latter if possible. There seems to be a lot of stuff duplicated between gateways.
  • Some messages are being used on donate.wikimedia.org. Pcoombe will help with finding/fixing stuff here.
• UNIT TESTS - there's probably already a bunch of useless mess in there.
  • Anything hard to test, should probably be refactored in the actual code
• activemq_stomp.php
  • All the things that "normalize" for our queues should probably start using smashpig message formats
  • reduce code duplication in this file and allow for more flexibility, as opposed to the current bizarre rigidity
  • And while we're here: Please note how frequently stomp-related functions come up in totally other files. Pretty good indication that we did something wrong.
• Load dependencies using Composer. This allows us to pull in libraries.

Preliminary meeting : Additional etherpad notes, December 15 2014

Approach

• Commits should be as contained as possible so that we can roll them back individually as we go if there is a problem
• Does it make sense to have a difference between the old and new so that we can toggle between them?
  • Pros
    • If a new goal is set by not-fr-tech, we can hopefully meet that quickly
    • Having a staging vs. dev environment
  • Cons
    • Can't build a house on 2 foundations
    • Toggling between the 2 is hard/tricky/fiddly
    • If we want to really use the new thing, we need other people to use the new thing, too.
  • Don't change queue message format or queues in the refactor

Goals

Near-term

• Achieve maintainable and modern UI.
• Make it easy for other WMF engineers and volunteers to help with code.

Long-term

• Our payments stuff is used and maintained by other orgs.

Steps

• SmashPig
  • Push DonationInterface payments code into SmashPig.
  • Push sundry audit and listener components into SmashPig.
  • SmashPig should use standard libraries like Composer autoload and Monolog.