Code review comment for lp:~sinzui/launchpad/location-bug-262193

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

This is my branch to only permit users to edit their own location.

    lp:~sinzui/launchpad/location-bug-262193
    Diff size: 460
    Launchpad bug: https://bugs.launchpad.net/bugs/262193
    Test command: ./bin/test -vvt "personlocation"
    Pre-implementation: my conscience
    Target release: 3.1.11

= Only permit users to edit their own location. =

After much debate about allowing users to set someone else's location, it
is clear from the usage statistics that the feature is rarely used.
Removing the feature will not have a significant impact on Launchpad.

== Rules ==

    * Restrict the permissions to edit a location to the owner or admin:
      launchpad.Edit
    * Remove the launchpad.Editlocation permission.
      \o/ one less exception permission in security.py
    * Remove any UI that suggests that a user can edit someone else's
      location. Most, maybe all the UI elements suggesting that a person
      can set another person's location was removed in 3.0.
    * Remove the notification email set when some edits another person's
      location.

== QA ==

    * On staging, verify that the page does not prompt you to set someone's
      location.
    * Verify that you cannot access the user's +editlocation page.

== Lint ==

Linting changed files:
  lib/canonical/launchpad/permissions.zcml
  lib/canonical/launchpad/security.py
  lib/lp/registry/configure.zcml
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/person.py
  lib/lp/registry/doc/personlocation.txt
  lib/lp/registry/model/person.py
  lib/lp/registry/stories/location/personlocation-edit.txt
  lib/lp/registry/stories/location/personlocation.txt

== Test ==

    * lib/lp/registry/doc/personlocation.txt
      * Revised the test to show that a user's location is only editable
        by himself or an admin.
      * Removed all tests that showed that a user could set someone else's
        location if the location was unset.
    * lib/lp/registry/stories/location/personlocation-edit.txt
      * Removed the tests that demonstrated a user could set another user's
        location if the location was unset.
    * lib/lp/registry/stories/location/personlocation.txt
      * Removed the test that implied a user could set another user's
        location.
      * Revised to the test to show that any user can see another user's
        location if it is public (the previous test implied the user had
        to be logged in).

== Implementation ==

    * lib/canonical/launchpad/emailtemplates/person-location-modified.txt
      * Deleted because it is not possible to set another user's location.

    * lib/canonical/launchpad/permissions.zcml
    * lib/canonical/launchpad/security.py
      * Removed launchpad.EditLocation.

    * lib/lp/registry/configure.zcml
    * lib/lp/registry/browser/configure.zcml
      * Replaced launchpad.EditLocation with launchpad.Edit.

    * lib/lp/registry/browser/person.py
      * Replaced launchpad.EditLocation with launchpad.Edit.
      * Since there is only one permission to access +editlocation, the
        field_names never change.

    * lib/lp/registry/model/person.py
      * Removed the notification about a user setting another user's location.

« Back to merge proposal