Updated attendance table generation and web server error handling (gradebookServer.js)

[wildtayne]

This PR updates the attendance table generation in gradebookServer.js based on the M1 goals, and updates the default error handler based on (fixes #44).

• When generating the attendance table, a second query is now executed to get the attendance codes descriptions from the database.
• Each cell containing a code is given a tooltip using title= containing the matching description from the database.
• A left border is added to each column (using style=) that begins a new month. Currently the border is solid black, but feel free to provide opinions on how this should look.
• (#44) I've changed the default error handler for the server, it now just sends the text Internal server error instead of a stack trace.

[afig]

Changes look good and all work with the latest version of the client (in branch client-tasks2-M1).

As far as border colors, I think that a color that is darker than the white rows, but lighter than the dark rows would fit in more. From my testing, the color #e0e0e0 seems to look pretty good, and is part of the Materialize color pallet (grey lighten-2).

One small concern I have is whether it is best to return a 500 status code if there are no attendance records for a particular section. Would a 404 code (with the same response/message) be more appropriate? (This was not affected by this PR, but though I would mention it anyways.)

[wildtayne]

Thanks for the review. I think #e0e0e0 looks pretty good. It's a little light, but I think it fits in with the rest of the design. The next darker color (grey lighten-1) is definitely too dark.

I touched on your concern about the 500 error in issue (#52). I think the correct response is 204 (no content), and it will apply to all current rest calls except /login (which returns 401). I was thinking this change would be made after M1, since the current behavior is OK for the current state of the application.

I think that change will also require changes to the client, since we'll want to display something specific when a 204 is received.

[afig]

True, it is something that might be best left for after M1. Currently the client displays a different message, differentiating between the two errors based on the response/message.

One alternative to making the border darker is making it slightly wider. Using 2px instead of thin looks pretty good.

[wildtayne]

I agree, 2px looks good.

I just pushed a commit that adds a simple README for the web sever, and a package.json file for use with npm. package.json is a file that contains metadata npm uses to manage packages. While it has many uses, I'm using it to simplify installation and execution of the web server. I specified the required packages in the file, so going to the Gradebook/src/webapp folder and executing npm install will automatically install all dependencies. Also, I've set a start script, so executing npm start will start the web server. Since we can define what npm install and npm start do in package.json, the user can always execute those commands even if the install/startup procedure change.

I also put in metadata related to the license and authors. I wasn't sure exactly which authors to put, so I put everyone who has worked on the web client and server. package.json is supposed to be for a whole module, and to me it seems the client and server are both part of the same webapp module.

I'm also curious which version of node everyone is using. I just ran into an issue with a package which led me to realize that I was using a 4.x version, which is still the default shipped with Ubuntu. It looks like node maintains several versions in different levels of support, so I think we should ensure we are all using the same version. Looking at the support schedule here, I think 6.x or 8.x are the best choices. I upgraded to 6.x because there was a PPA available, but 8.x shouldn't be a problem either.

[afig]

The README and package.json file looks good, except for a misspelled name in the list of contributors in package.json.

Being able to just require running npm install and npm start will definitely be useful later. Is it possible to specify files in sub-directories? Right now it seems like a client directory is listed, but nothing is said about the contents of the directory.

I have been using version 6.11.0 of Node.js. I think we should opt for using 6.x, given that it has already been in LTS for a while.

[wildtayne]

I believe that listing a directory in the files section automatically includes the directory and all contained files. However, I don't think the files section will be used in our project at the moment, since we are assuming users are cloning the whole git repo anyway. I think the purpose of it is so that if you publish a module to npm's repo, the files in files will also be updated.

I agree that the 6.x branch of node is a good choice. Also, just a heads up, a code execution vulnerability exploit was found in pg, so everyone should make sure they update to one of the versions listed in the article.

I just pushed a commit updating package.json

[smurthys]

Awesome work @srrollo. Everything seems to work well (except for things we are consciously choosing to address after M1).

One suggestion: Avoid using SELECT * in queries, because such queries can return unnecessary data and can cause later code to fail when schema changes.