Code review

[stadibo]

Cloned on 12.12.2018 at 18:05.

Use cases:

• Each use case worked well a this stage in the project

Installation:

• Installation guide does not quite work for MacOS/Linux. Otherwise solid instructions.

$ python -m venv venv
$ source venv/Scripts/activate

No such commands on unix. Optional commands for unix systems:

$ python3 -m venv venv
$ source venv/bin/activate

Database file already exist in the github repo and is used if downloaded. Should probably be removed from github and user may follow instructions in the installation guide.

UI:

• The time for "completion" and "reserved" might be a bit cleaner without the time in milliseconds added to the end. For dates in python:

theDateObject.strftime("%Y-%m-%d - %H:%M%S")

• Since the individual rows in the listing on the “Thesis list” page are a bit tall, the amount of theses shown on the page could be reduced, maybe by two, so that there is no need to scroll every time you want to click to the next page if you’re just browsing. Or one could make the “search” a bit smaller or less padded to get more space.

UI paging:

• When searching with only one field in the search it implies that there are matching listings, but does not show them.
• How it was produced:
  • When the listing of all theses is open and you move to another page to view other theses, then if you search using only one criteria, it will find theses matching that search but will not show them. It happens no matter which type of criteria you search by.
  • I think this is because the pagination still applies somehow when showing the searched for results.

Login:

• A minimum password length of two seems a bit too short for security.

User list:

• The table stretches past the page if not shown in full window. Could be fixed by making the table scrollable like the theses listing.

Account:

• When it comes to checking access rights to the route “account_update” it could be using “@login_required(role="ADMIN")” instead of the condition check at the beginning of the method.

General notes:

• Good comments improve readability
• Variable names could be kind of descriptive, not just single letters (e.g. in department/views.py), keep it consistent
• Routes that should only be accessed by admin should probably be protected with @login_required(role="ADMIN") (e.g. /account)
• Code quality is fairly good and readable on its own
• Organisation into modules is reasonable and well thought out

Good luck for the final review!