Code review comment for lp:~jamestait/canonical-identity-provider/add-ax-fetch

- missing commit message
- instead of *_EMAIL_VERIFIED I'd use *_ACCOUNT_VERIFIED (as we should care about the status of the account, not the email; if the account is verified, the returned email is also verified).
- l. 96-99: I'd extract this into a helper method for readability
- l. 101-105: ditto
- l. 114-135: since these values (except email_verified) are the same for AX or SREG, why not have a common helper function to get them (DRY)?
- l. 462-463: you can use {% elif ... %} here I believe?
- l. 516: why not get the entry again and confirm it has the fields you just posted?
- l. 531: this could match any checkbox in the form? why not rather add a specific test to verify email is listed as an ax field?
- l. 613-614: you could also use assertIn, assertNotIn
- l. 818-819,870-894,930-954,988-1024,1065-1086: we now tend to prefer using pyquery for asserting against the DOM
- l. 1166: can we call _get_openid_request with named params? it's hard to understand what each param means otherwise
- l. 1250: why not just do: if ax_form.data: ?
- l. 1250-1253: or maybe add a helper to the form to get the values without having to know that data is a dict?

Despite some desired changes, I must say I'm impressed with the clarity of the work done. It was very easy to understand the changes and follow the new logic.

Well done!