Page MenuHomePhabricator

[EPIC] Refactor form chooser
Open, Needs TriagePublic


And document the rules. We made it deterministic recently. It should probably stay that way, and any a/b testing should be controlled upstream and communicated via the ffname parameter.

Proposed (makes selection_weight more important):

  • if ffname is given, check if that form is valid and use it if so
  • if ffname is not valid or not present
    • filter form list to those valid for donor's payment method, country, currency, recurring preference, and when specified, gateway and payment submethod
    • group all valid forms by selection weight
    • going from highest to lowest selection weight bucket,
      • eliminate forms which match the donor's country and currency less specifically than the most specific match
      • break ties by total available submethods
        • if gateway uses mustache, querying for available submethods given the params
        • mustache form config can lose the submethod keys
        • if gateway uses RapidHTML, use the list in the form settings
        • still tied? choose the first one

TL;DR : ffname -> selection_weight -> specific match -> most submethods


Related Gerrit Patches:
mediawiki/extensions/DonationInterface : masterWIP: getAllValidForms uses isValidForm
mediawiki/extensions/DonationInterface : masterMake isValidForm self-contained

Event Timeline

Ejegg created this task.May 25 2016, 10:55 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptMay 25 2016, 10:55 PM
Ejegg updated the task description. (Show Details)May 25 2016, 10:57 PM
awight added a subscriber: awight.Jun 1 2016, 10:57 PM

"weight" is problematic because it's usually the reverse sense--higher weight sinks to the bottom.

I don't like the specificity comparisons, those will give us very unpredictable behavior.

This outline is missing "eliminate forms which don't support the donor's country or currency", and whether we do a fallback round if nothing matches.

Does not mention payment method, which is suspicious.

Ejegg updated the task description. (Show Details)Jun 1 2016, 11:00 PM
Ejegg added a comment.EditedJun 1 2016, 11:06 PM

I added an initial step explaining what the rest of the steps mean by 'valid' forms. Also, I'm fine with ditching the specificity test. @K4-713, can you think of reasons to keep it?

@awight, Which sounds better to you for suppressing unready forms?

  • rename/renumber selection weigh to 'priority' and traverse 1..2..3
  • use a 'stability' entry with 'production', 'beta', 'alpha'

I'm partial to 'priority' in case we want to promote for a non-stability reason like lower fees.

Ejegg added a subscriber: K4-713.Jun 1 2016, 11:11 PM

I like priority, but I think for suppressing automatic choice we should have a self-explanatory flag like 'isTestingOnly'

Change 292398 had a related patch set uploaded (by Ejegg):
Make isValidForm self-contained

If we are doing a refactor I'd love to get down to the fundamentals. Can we try using Flow to make a page that describes this particular piece of machinery? I started here:

DStrine renamed this task from Refactor form chooser to [EPIC] Refactor form chooser.Jul 5 2016, 8:30 PM
DStrine added a project: Epic.
DStrine moved this task from Triage to Q2 (Oct-Dec) 2019-20 on the Fundraising-Backlog board.

Change 292398 merged by jenkins-bot:
Make isValidForm self-contained

Change 338458 had a related patch set uploaded (by Ejegg):
WIP: getAllValidForms uses isValidForm

mmodell removed a subscriber: awight.Jun 22 2017, 9:39 PM