Code review comment for lp:~sinzui/launchpad/override-blacklist-0

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Abel, Stuart.

Thank you for the review. I am going to update the UI of the forms to clarify when a team can be set as the admin of an regexp. I think I can update the name change forms to add a notice when the name was changed and it matches a blacklisted expression (call the isBlacklistedName() method without the user).

There are two kinds of blacklisted names: Those for because the application reserves the name, and those that are reserved by organisations. No one can ever be permitted to override an application restriction. The LOSA want this feature because they fear they will do that using SQL. They want a way to override organisational names...

...Stuart, I cannot accept your rewrite of name_blacklist_match because it breaks tests. It allows admins to override application blacklisted names. The SQL is hardcoded to give ~admins the power, but the intent is to assign a team to do the act. A rule without an admin team can never be overridden. Very few rules will permit someone to override them, I know of four rules:
  private: ~commercial-admins
  canonical: ~admin
  launchpad: ~admin
  ubuntuone: ~admin

We may ~registry instead of ~admin, or assign the names to their respective teams (~launchpad and ~canonical). This approach allows organisations to buy namespaces and create names without complaining to Launchpad staff.

I think the admin_select_plan wrongly uses ~admin instead of the team in the nameblacklisttable. I will attempt to revise the SQL to do the right thing.

...
        # admins is a celebrity and its id is immutable.
        admins_id = plpy.execute(
            "SELECT id FROM Person WHERE name='admins'")[0]["id"]

        SD["admin_select_plan"] = plpy.prepare("""
            SELECT TRUE FROM TeamParticipation
            WHERE
                TeamParticipation.team = %d
                AND TeamParticipation.person = $1
            LIMIT 1
            """ % admins_id, ["integer"])
...

    # Names are never blacklisted for admins.
    if user_id is not None and plpy.execute(
        SD["admin_select_plan"], [user_id]).nrows() > 0:
        return None

« Back to merge proposal