HOLY GAWD PEOPLE..
- Don't write all your new code on the production machine!
- Use a VCS!
- Document your functions using DocBlocks!
- Document your API (at least LIST the endpoints)... I have to decipher the meaning from your code, and that isn't really clear
- Use a single identifier as the primary ID in the API for a resource (ie don't return a list of group ids for listing groups and then expect the group name as the identifying parameter for modifying a group)
- Use a consistent model for representing the same data model in different endpoints!
- Don't concatenate fields in the API results unless there is a good reason to (e.g first name and last name)
- Don't expose the underlying database logic. This is leaky abstraction, especially when you do it inconsistently!!
- Use HTTP codes to indicate request/response status! Don't add that information to the response body as a boolean field.
- Even if you have to use a boolean field in the response, true = 1 and false = 0, not vice versa.
- Don't rely on the client to implement business logic like validation!! That is going to lead to extremely complicated client code, and lead to errors and data inconsistencies.
- There is absolutely no reason not to use HTTPS. We're passing potentially sensitive information here.
- Don't require multiple HTTP calls for authorization when auth/access can be done in one step! Atomic things should be atomic.
- The user login field should have one name and one name only. We call it
unixaccount, and possibly others all over the place.
- Put some thought into model attribute names! 'approved_comments' should be 'approver_comments', for example!
- Consistency in naming is important: Don't use 'requestor' in some places and 'requester' in others
- Limit the amount of potential entropy wherever possible. Don't allow fields to have any value if only a few values make sense!
And definitely don't leave it up the clients to decide what those rules are!!!!
- The server should handle data integrity, not the client! Eg. if a group request is fulfilled and it should change two
tables in the database, don't make the clients make two separate calls; that is a recipe for data integrity violations!
- A timestamp should be an actual timestamp down to hours/min/secs in ISO8601 format. A date is a 'date', not a timestamp!
- Don't use completely different nouns in the system for the same object
- If you expect me to send you the ID for a record, there should be at least one endpoint in the API where I can actually get it.
- Do not mix data and logic. Hard-coding IDs in the code of mutable/deletable records from the database is a recipe for trouble.
- The Request Signature algorithm should include a hash of the data being sent! Not just the timestamp and a secret, otherwise it is not secure at all.
- Use relative paths for file includes! Or better yet, use an autoloader!
In the code
- Don't Repeat Yourself! Holy moly.. 80% of the code is repeated overhead stuff, like getting a database connection
- Don't manually create JSON strings by concatenating JSON syntax! Use arrays + json_encode function!
- Configuration values should not be stored in the code! They should be stored in environment variables or a config file
- Don't leave stray variables and other crud lying around in the codebase
- Comments anywhere? Nope!