Code review comment for lp:~michael.nelson/launchpad/db-598464-get-or-create-from-identity

Revision history for this message
Abel Deuring (adeuring) wrote :

(12:04:05) adeuring: noodles775: overall, your code looks good. Just one question: SHouldn't the "with MasterDBPolicy():" be moved from processPositiveAssrtion() to getOrCreateByOpenidIdentifier()?
(12:06:19) noodles775: adeuring: yes, you're right.

And a minor nitpick:

> @@ -1754,6 +1761,33 @@
> on the displayname or other arguments.
> """
>
> + def getOrCreateByOpenIDIdentifier(openid_identifier, email,
> + full_name, creation_rationale, comment):
> + """Get or create a person for a given OpenID identifier.
> +
> + This is used when users login. We get the account with the given
> + OpenID identifier (creating one if it doesn't already exist) and
> + act according to the account's state:
> + - If the account is suspended, we stop and raise an error.
> + - If the account is deactivated, we reactivate it and proceed;
> + - If the account is active, we just proceed.
> +
> + If there is no existing Launchpad person for the account, we
> + create it.
> +
> + :param openid_identifier: representing the authenticated user.
> + :param email_address: the email address of the user.
> + :param full_name: the full name of the user.
> + :param creataion_rationale: When an account or person needs to
> + be created, this indicates why it was created.

s/creataion_rationale/creation_rationale/

review: Approve (code)

« Back to merge proposal