Webtools:Mozparty

From MozillaWiki
Revision as of 21:42, 8 October 2006 by Reed (talk | contribs) (Fixes from Ryan on the last two known issues)
Jump to navigation Jump to search

Code

Notes About Issues

  • This was a good first-run -- this is meant as constructive feedback, and we hope this helps. We appreciate the time you spent on this.  :)
  • Before pushing this into production, I suggest fixing all remaining issues below. I don't see any "eh, it's okay" issues -- they are all pretty core problems since the app is centered around users and invites and those two actions have either extra or unknown steps without good UI flow.
  • I don't think any of the issues below are show stoppers - lets get this staged and get some more people playing around with it. Our timeline is shrinking fast.--User:clouserw

New Issues

  • Migration plans from current party tool need to be worked out
    • All users currently stored in flat-files for every country
  • README with steps to test (verify that things are working) -- there isn't a solid way to test all of this -- so even after it's set up, what are the criteria to test it by to make sure it works end-to-end?


Wil's Points

  • /config/database.php shouldn't be in the repository. IT prefers a .default or -dist file. (Same goes for config/bootstrap.php if you expect IT to change values in it)
  • There needs to be a README with
    • description of project
    • any special server requirements
    • steps to install
  • Line 288 of controllers/user_controller.php has a hardcoded link in it that isn't going to work.
  • Going to /user/activate/<random characters> inserts new rows in the database
  • When updating a profile, if the passwords don't match the error just says "There was an error in your submission..." but doesn't say what it was -- There is still a problem here. There is a JS alert, then it'll just change it without any error now. - fixed (as well as the other validation issues below). There was an invalidate() missing.
  • When updating a profile, if input is "<'name" it is saved/returned as "<'name". Each time the profile is updated, the entities are re-encoded again.
  • When I created the party I did not choose "Invite only," however, when I go to the "Invite a guest" option it says "Since your party is invite only..."
  • /register/ is a 404, but is linked to in the code (line 333 in controllers/party_controller.php - might be elsewhere too) -- haven't seen this. An invite code might be null and cause this in the controllers, but I didn't produce that situation
  • After updating a profile, the user is logged out
  • When updating a profile, if a password is entered into the first box but not the second (or the second doesn't match), the password overwrites the saved password in the database in plain text.
  • When updating a profile, if a password is entered into the second box but not the first, the form continues submission, no updates are made, no errors are shown
  • going to /party/view/<number> and clicking "Count me in!" will create rows in the database for parties that don't exist
  • When viewing a party that is invitation only, there is no way for someone to tell that they have to be invited. -- This is still an issue -- a user really has no way of seeing "parties I've been invited to".
  • After the invite is sent out, user goes to register page, but then has no where to go. The original invite is lost, somehow. Also, if you send an invite out to an existing user, why shouldn't have to re-register, and they should also retain the invite context (be added to the party when they do log in).
  • Session method should be 'database', not 'php'. If this were deployed on a cluster we'd want to avoid using /tmp for storing session data. See line: 78
78     define('CAKE_SESSION_SAVE', 'php');
  • Line 46 of users_controller.php redirects to "user" which no longer exists - that should be "users"
  • If you go to /parties/invite/$number you get a page that offers to invite guests with a URL or an email address. The URL gives a 404, it should be s/users/parties/