Code Review - Githubissues

Hello,

I'll be reviewing your code & project today. I downloaded your repository with a hash of "4fb309276125d2a567932755bd1da6b0f6b00cca". No doubt GitHub shows this as "4fb3092". Now that I've given the hash, I don't think it's of any use to also tell the date that it was downloaded but since it was mandated, it's 2018-05-07 at 15:51:00.

First off, I'd like to begin by saying that this is looking great already. You're using common Bootstrap components to achieve the necessary goals. I especially like the intro page. When I first landed on it, I had to check that I had opened the right link.

I'm using Visual Studio Code to review your code and test it out. I created a virtual environment and installed all the requirements from the TXT file.

From the requirements, I see that you're using Argon2 password hashing algorithm. Any reason to skip bcrypt that was suggested by the course? But not storing plaintext passwords in the database is the correct way to go, and even algorithms like SHA-2 are still suitable (if you provide the salt yourself).

Your commit messages are short but to the point. I understood what is going on simple from those.

You might be interested to know that Flask has released version 1. I'd suggest you take at look at Dependabot on GitHub. It automatically goes through your dependencies and if it finds some that are out of date, it creates a pull request automatically. Also works for Java with Maven ("pom.xml").

I like your user stories page. I took a long and it seems you're using non-breaking spaces. Yes it's a good choice considering Markdown. Bonus points for indicating which functionality is in working order and which isn't yet.

The database schema is good and actually really easy to understand. I hope you saved the data that is used by the app to generate that image, so that you can easily make changes to it. I keep such files in a folder called "private" which is also in my gitignore -file.

I'm also using a functionality to manually reset the database. It's great for testing. Perhaps you'd like to add sample data also? This way, on reset, you could choose if to add sample data to the database or not. Sample data like accounts and tasks and tags for them.

Okay, I'm now running your app locally on a virtual environment. Everything's working fine on Windows 10 with Python 3.6.5 (the latest as of now).

The feature wrappers are nice. But you're still setting the width of the images using inline CSS. But I guess this was just a quick way to do it, and the final version will have it properly in the CSS file.

I didn't see the name of the project at the root of your site ("index.html"). I think it would be good to be somewhere, like as a logo or something. The same goes for the login and register pages. It's good for the user to know (and remember) on what site they're registering/logging-in for.

After the account creation, you could automatically sign the user in. But one of the reasons for not doing that if there's something like an email confirmation via a sent message.

I'm using tags myself on my project (a dating Website). I managed to add a tag with the same name twice. This could and maybe should be prevented. If you use a unique database constraint, and then add a check, that would prevent duplicate, identical tags.

Speaking of tags, deleting them can be done by accident since the system won't prompt for a confirmation on their deletion. The delete buttons could also be red, or be red on hover.

Locally, I wasn't able to create tasks for some reason. But online (on Heroku) it worked on my test account.

Moving the tasks up and down doesn't seem to work either. When a task is either at the top (or the bottom) already, consider dimming (disabling) the option to move it further into the direction it cannot go anymore.

Same goes for moving tasks between time periods; if one is already at today, then the choice to move to to today can be hidden altogether in my opinion.

I see you're also using PyCharm. Students can get the Professional/Ultimate version for free, if you'd like to give that a shot.

In the documentation that is already somewhat written its folder, I'd like to read about how the session system of your app works. There's some interesting code there.

Also, now you probably need to log back in each time the Flask server is reset, unless USE_SESSION_FOR_NEXT keeps the login data between runs. If that isn't the case, consider changing the SECRET_KEY to something static when running this app locally. This way, on each restart, you don't need to log back in. On Heroku, you can use their own environment variables to control the SECRET_KEY.

From your code, I can see that CSRF protection is not active. It's really easy to activate, just change the meta tag to true, and add this into each form:

{{ form.hidden_tag() }}

It'll add the CSRF tag into the form as a hidden element, which is then verified when the form's validity is checked.

Perhaps put your JS files into their own folder, and CSS files into theirs. I also have a 'static' folder, but I created a 'classes' folder to house all the Python packages. This way, 'application' folder contains folders 'static', 'classes' and 'templates', where 'static' also contains 'js', 'css' and 'img' folders.

Your code is very clear and structured well. I noticed some AJAX in there as well (which was evident from the test run of the app as well). I highly recommend the full-stack Web development course at our school. There these things are taught more in-depth.

Finally, I'll test how this looks like on mobile devices. And just as I suspected, it's working great.

Clearly a lot of labor has gone into this project, and perhaps even learning new things such as about Bootstrap has taken place.

Keep up the good work and best of luck in the project!

doc97 / tman

Code Review #2