katrinahoffert/eatsafe

An app for finding safe places to eat

eatsafe's Issues

Consider alternative format for backlog

The current format is getting unweildy. Difficult to keep track of what's done, rearranging order, etc. Comments are crowded.

Consider if dedicated software or a different format would be beneficial. The other group used an excel document.

Not clear if this is worthwhile.

Automate retrieval of CSV files from source website

Should investigate if there's some pattern to the URL that can be followed. If unsuccessful, look into browser scripting, eg, with Selenium.

Function of this script or program is to download a CSV file for every location on the source website into a single folder.

Consider how to name said files (if necessary).

Stakeholder Change: Health region link

For the respective health region, we could add a link to there website, and/or have the region's phone number on our site if users want to complain directly to the health region.

Ideas are welcome

Have server compile changes automatically

Would require creating a chron job that checks for updates (see here). If updates have occurred, restart the server (SBT will automatically recompile).

However, this will fail if the build ever breaks. We could simply ignore this (don't break the master, yo!).

Typeaheads submit to old ID after deleting text

Deleting text should invalidate the ID and it shouldn't be possible to submit when there's no city/location chosen.

Views: Reverse Routing

• When we need links in templates, we’d want to use reverse routing. In some cases, we’ll need to use reverse JavaScript routing (to generate routes client side). Reverse routing means that we take a function and get the URL for it. This provides safety that a change to a URL won’t cause a runtime bug. Setting up the reverse routing is a [3]

CSV parser should be converted to Scala and built with existing build tools

Also, improve documentation of this.

Model: Model Tests

When given bad data, the functions within the model, getLocationsByCity, getViolations, getInspections
are not passing back failures.
for some of these functions it might be okay, for example: an inspection might not have violations, so it passes back an empty set (which is good) but if an inspection id is passed in that doesnt exist, a failure should be passed back, same with non-existent cities and inspections

The getLocationyId seems to be working properly, passing back failures when its bad data, if that is any help

Database: Inspections should store dates in a specific date type

Storing as a string makes it possible for invalid dates to be introduced. As well, a date type will allow for easier access to information about the date. For example, we could detail in a human readable fashion how long ago a date was (eg, 2014-07-01 was about "five months ago").

JodeTime is usually the preferred date library for use with Scala.

Database: Changes to schema

So, I'm assuming that database/371CreateTables(2).sql is what's creating the current schema? Should probably be renamed.

At either rate, I can think of a few places to improve on.

1. These domains are everything I hated about Jim Carter's class. We don't need domains for these. Further, there's no need to be using VARCHAR as the type. It creates unnecessary limitations. Instead, we should use the TEXT type. TEXT in Postgres is like String in Scala. We wouldn't use Array(100) to store strings in Scala! That's the C approach. TEXT is also more space efficient, since it stores only what we need. Finally, Postgres treats VARCHAR of over 255 (or so) as TEXT, anyway.

   So we should abolish all the domains. None of them add anything. Most tools won't even show the domains, but will instead show the real type, so these don't even add type safety. They just make the schema more complicated. Finally, no real world database that I've seen uses domains like this (take a look at the databases for MediaWiki or your favourite forum software). As far as I can tell, this was just a Jim Carter thing (and I've yet to meet someone who likes Dr Carter's teaching methods).

2. We should be consistent with our case. We current have attributes like ID, location_restaurant_name, and Reinspection_Priority. Fortunately, Postgres doesn't retain case, so the case in the schema doesn't really matter, but I think it's a good idea to be consistent all the same.

   What's more important is the case we use to format the file, in general. I see that we're using an "uppercase keywords, lowercase everything else" approach, and I support that. However, we should be consistent with it. For example, types like INT should also be capitalized, then.

   Where acronyms are concerned, I think we should treat them as common words. So instead of RHA, we'd just use rha (camel case would be Rha). This simplifies the issue of whether certain words are actually acronyms or not.

3. What's the begin; on line 1 for? As far as I can tell, it doesn't do anything.

4. I think we should simplify our attribute names to not repeat the table name. For example, in location, we shouldn't have an attribute location_address, but instead simply have address. This will reduce unnecessary verbosity. If we ever have complicated SQL queries (currently all of our queries are quite simple), we can use aliases to make it clear what tables are being referred to (the standard approach for when there's two tables with columns of the same name).

   The exception is the foreign keys. They should make it clear what table is being referenced. For example, if we have an ID in inspection that refers to the location ID, then this should be named something like location_id.

   Note that some attributes cannot be simplified because they'd conflict with an SQL keyword. For example, we can't change inspection_date to date because date is a reserved keyword (list here).

5. I think that the foreign keys shouldn't be cascading changes. Instead, set them to NO ACTION. This facilitates error checking. There's currently no reason to change any of the foreign keys. All the IDs we currently have should never change. Doing so is probably a bug. Using NO ACTION will serve to prevent accidental change.

6. We should remove the rating, category_description, and categories tables. They're not currently being used and it's not clear if they ever will be (categories are a stretch goal and storing ratings in the database is a performance thing, which we should not be worrying about any time soon). In particular, we don't want to be maintaining code that we don't currently need. Especially since requirements may change in the future, which could render the existing tables useless.

7. Table names should be singular. violations and violation_types should be changed.

8. Inspections should have a serial ID. The script that creates violations for each inspection (ie, the script that creates the SQL data) would have to link these up, so this is a larger change (the rest of the changes in this list are super easy to make). The main reason for this is because the existing approach of referring to a violation by location ID and date is insufficient. While it seems unlikely, it's entirely possible that there would be more than one inspection on the same day. A unique ID per inspection will get around this.

9. Violations shouldn't have a date. This is already in the inspection. Violations is a simple table mapping violation types to an inspection (so there's just two attributes, both foreign keys). It's currently needed because of point 8 and can be removed once point 8 is resolved.

These issues should be resolved early in the project, as they will have ramifications on existing code (in the model).

CSV parser needs to be tested

Wait until #31 is completed.

Then this tool needs unit and system tests.

General error page

The miscellanious errors can be (and currently are) treated as 500 errors (internal server error, implying an error entirely on our end that we cannot preempt or handle).

As you know, the current reporting is ugly -- it's plain text. Make a prettier page that:

1. Mentions error 500. Nerds will know what that means.
2. Mentions that error 500 means an internal server error and that it's our fault.
3. Mentions how to contact us (print out an email address from the configuration file. For now, this can be a placeholder email (but being configurable from a central location is highly useful).
4. Print out the stacktrace (which is obtained from the exception). This is so users have a message to report.

Views: Template that contains the pages

• Would create the head, load in all the needed scripts, etc.
• Would provide arguments for the body (everything between the header and footer) and additional scripts (optional, would be used, eg, to provide per-page JS).
• Mike has a pattern for this.
• Suggested format: mainBody(additionalScripts: Html = Html(""))(body: Html)
• Create common CSS [1]

Views: View for displaying a location [4]

• Takes in a Location and creates a page displaying information about it. Start by just displaying the sheer basics (no rating and no inspection data).
• Suggested format: displayLocation(location: Location)

VM: Finish Setting up the VM

see title

Controllers: Controller to select a location [3]

• Takes in the city name in the URL (will be hardcoded into controller) and gets a list of Location objects from the model.
• Would then pass the list of Locations to a view. Until that’s written, just display the list.
• We need special handling if there’s no locations for a city. This should be an error condition because this should never happen (since the list of cities is generated from the list of locations and thus every city in the list must have at least one location). Let’s not worry about such pages for now and just display a plain text error if this happens.
• Suggested format: findLocation(city: String)

Separate error page for when the list of locations in a city was empty

Use a guard on pattern matching to do this.

If we successfully get a list of locations, but it's empty, this isn't really an error, but we cannot consider it a success, either. In this case, we should have a custom error page that explains that there's no locations in that city and suggests that the city may have been mispelled or that there's a bug in the software.

Stakeholder Change: In view recent issues

In view

on most "recent issues"

• not clear to expand
• downward arrow to show it can expand

Database: Fix DB data generator (CSV parser) to ensure that generated IDs are correct

As discussed in chat.

Controllers: Create routing for location controller functions

Work on controllers/LocationController.scala

Both controller tasks must create the routing command and the controller function. I suggest that both be in a file controllers/LocationController.scala. Is a single object with two methods.
Both controllers need to handle the possibility of the model failing (since the model functions return Try objects).

Research if PingDom would be useful

For testing latency in resource load times.

Note: would require Play Framework in production mode and requires VPS be setup.

Display Google map location for location addresses

Make the addresses (on the "display location" page) link to a page that shows the address as a simple, embedded Google Maps instance. The embedded instance would just have the heading and the map.

1. Create controller to show the location on a map. Something like MapsController.showAddress(address: String, city: String). This would simply delegate the parameters to a view (with the same parameters).
2. Add a line to the routing. I suggest treating the address and city as GET parameters. To do so, simply have the controller called in the routing file require arguments. The Play Framework will then expect to get such arguments in the form of GET parameters. For example, if the URL is /maps/address and the associated controller is MapsController.showAddress(address: String, city: String), then the Play Framework will expect a full URL along the lines of /maps/address?address=123+Fake+St&city=Saskatoon.
3. Create view (something like showAddress(address: String, city: String)) to display the map as previously described.
4. Adapt the existing "display location" view to create the link. Use the reverse routing (see documentation) to generate the URL.

NOTE: This is lower priority than your other assigned issues. Do it last, if time allows.

Controllers: Controller to show a location [3]

• Takes in a location ID, gets that Location from the model, and passes it to a view (see above point).
• Needs to handle case of invalid ID (see above).
• Suggested format: showLocation(locationId: Int)

Typeahead for location needs to be formatted

See the mockup.

Names should be most prominent, with addresses being secondary data.

The styling must allow for mobile compatibility. It's currently being cutoff!

Stakeholder Change: About page

About page at the bottom of view

• explain where we get the data + link to the old site
• about our group
• class project for 371
• who we are
• etc

View: TypeaHead

use a typeahead instead of a selector

General: Style guide

We really need to come up with a style guide. We need to maintain consistency in all our code. This is complicated by the fact that we have several languages: SQL, Scala, HTML, CSS, and JS.

I won't waste time repeating any of the things that the prof would discuss in class (like "use good variable names").

Here's my thoughts, based on past experience:

1. Scala practically needs 2 space indentation. The language uses closures heavily and thus we usually have a lot of indentation going on. For consistency, we should use this everywhere. It's very useful in HTML for the same reasons (with 4 space indentation, it's very easy for the middle of an HTML page to be half spaces). The reason to use spaces instead of tabs is for consistency with most existing Scala code (it's pretty much the Scala convention), but is also useful because web browsers think tabs should be 8 spaces wide.

2. We should use "K & R" braces. That is, the braces start on the first line, after whatever necessary identifiers. This is sometimes necessary in Scala, since there's no semicolons. It's very helpful for JS, since JS loves its nested functions. Example:

   $("#collapsableSection").hide(function() {
     if(foo) {
       // Do something
     }
     else {
       for(var i = 0; i < list.length; i++) {
         // More something
       }
     }
   })

3. As the above example shows, I prefer not to have a space between keywords like if and the parenthesis. But I do prefer to have a space before opening braces as well as between binary operators (so it's width = perimeter / shape.numSides and not width=perimeter/shape.numSides).

4. Nested functions start on the same line, but have their closing brace on a new line (which continues as normal. This was illustrated above, where the nested function starts on the same line as the hide call and the closing brace is followed by the parenthesis that makes up the end of the hide call.

5. 120 character line length. We don't need our code to be viewable in a tiny little console and we'd want to minimize wrapping lines. But if many of you are using small screens, a 100 character limit can also work (but that's as low as we should go).

6. Wrapped lines are indented twice (ie, for 2 space indentation, wrapped lines are indented 4 spaces). Example:

   // Pretend we're wrapping at 60 chars
   def someLongFunction(someInt: Int, someChar: Char,
       somethingElse: ThingElse, somebody: Body) {
     // Do something
   }

7. Don't wrap HTML. In my experience, indentation is really important for HTML, so that we can follow nesting. wrapping just confuses that. On the topic of indentation in HTML, not every element needs the contents to be indented. Just use whatever looks best. Example:

   <div class="modal-content">
     <div class="modal-header">
       <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
       <h4 class="modal-title">Modal title</h4>
     </div>
     <div class="modal-body">
       <p>One fine body&hellip;</p>
     </div>
     <div class="modal-footer">
       <button type="button" class="btn btn-default" data-dismiss="modal">Close</button>
       <button type="button" class="btn btn-primary">Save changes</button>
     </div>
   </div>

   This is an actual example from the Bootstrap documentation, by the way. As you can see, they also use 2 space indentation. Note how tags like the h4 button are all one line.

   In particular, note that sometimes it will be necessary to have tags on the same line to prevent unwanted spaces from appearing in the output.

8. Scala methods should never throw exceptions. Return a Try instead. Note that all database access can throw an exception, so must be wrapped in a Try.

9. Otherwise, do whatever the hell looks best. You're not morons who need to be micromanaged. Just try and style things in a fashion that is readable and somewhat consistent with existing code. For example, in the Application.scala file, I've implemented the JavaScript routing as:

   def javascriptRoutes = Action { implicit request =>
     import routes.javascript._
     Ok(
       Routes.javascriptRouter("jsRoutes")(
         LocationController.findLocation,
         LocationController.showLocation
       )
     ).as("text/javascript")
   }

   Note I usually have arguments on the same line as a function call, constructor etc (eg, foo(1, "abc")), but here I put the content indented inside the Ok, using the parenthesis akin to how braces are usually used. This is because the arguments are quite long here and makes it clear that the actual listed part is an argument to Routes.javascriptRouter("jsRoutes") and not to Ok.

10. Document all CSS rules (by the way, the main CSS file is in public/stylesheets/main.css). CSS is rarely self documenting and it's difficult to tell where CSS rules are being used (much less their intent), so document that shit.

11. Minimize in-line CSS (that is, CSS applied with the HTML style attribute). The style tag is useful for simple, single element styles, but anything else should go in the CSS file.

12. For SQL, use ALL CAPS for keywords and lowercase for everything else. Example:

    CREATE TABLE inspection(
      id SERIAL NOT NULL,
      location_id INT NOT NULL,
      inspection_date DATE NOT NULL,
      inspection_type TEXT NOT NULL,
      reinspection_priority TEXT NOT NULL,
      PRIMARY KEY (id),
      FOREIGN KEY (location_id)
        REFERENCES location
        ON UPDATE NO ACTION
        ON DELETE NO ACTION
    );

13. When naming things in Scala, JS, CSS, etc, use camelcase. Treat acronyms like regular words. Examples: foo, isEmpty, xmlParser, httpJsonRequest. Class names, however, start with a capital letter (eg,Foo,XmlParser`, etc).

14. The exception is SQL, since Postgres doesn't preserve case. We'll use snake case (underscores) there. Examples: is_empty, xml_parser, etc.

15. Never use null in Scala. Use Option[SomeType] instead.

16. In JS, use double quotes for strings. This is mostly for consistency, although one reason to use them is because single quotes within our strings is far more common than double quotes (so less escaping is required). Also, JSON requires the use of double quotes, which is another argument for consistency.

View for selecting location

1. Takes in a list of Locations and uses them to create a typeahead from which we can select a location.
   Locations are parsed into minimalistic JSON for the typeahead. You must do this creation of JSON. Create a separate utility function that converts a Seq[Location] into a Seq[JsonObject] (see here). This JSON array can then be embedded into the view for the typeahead to access (as a normal JS variable). This JSON should be minimalistic. Only store the data we need for the typeahead (location name, address, and ID).
2. Typeahead should display each object with the name of the location. Below the location name will be the address, in smaller, lighter type.
3. Typeahead’s updater function is executed when we select an item. Here, we would set a hidden input (which is what’s actually submitted when we press the submit button). The value of this hidden input is the ID of the location, obtained from the JSON.
4. Clicking the submit button will change the page according to the ID in the hidden input. Use JS reverse routing. See documentation in controllers/Application.scala.
5. Use JavaScript to preventing submitting when there’s no selected location. You should handle the case of the typeahead being cleared.

Use "use strict" in JS

Stubbing and Specifications

Anyone who plans to write new methods or controllers, or pages, please take a hour soon (like in the next day or two) to write a stub method and specification, it would really help the test team be able to work.

Model: Create model package

One file per class.
Create Violation case class [2]
Suggested format: Violation(id, name, description, severity)

Create Inspection case class [2]
Assume that we always will use Inspection/Violation inside a Location. As a result, Inspections don’t have to store things like the location ID.
Suggested format: Inspection(date, inspectedBy, type, priority, violations)

Create Location case class [2]
Suggested format: Location(name, address, postalCode, city, inspections)

Add method to Location that gets a Location from the database based on location ID [8]
Must get the lists of inspections and violations for each inspection and add to the Location.
Suggested format: getLocationById(id: Int): Try[Location]

Add method to Location that gets a list of Location objects, one for each location in a city. [2]
The fact that there’s commonality between this and the previous method suggests that we should have a helper function that converts a location row into a Location object).
Once we have that commonality done, the rest is easy -- just filter by the city (filter at the SQL level)
Suggested format: getLocationsByCity(city: String): Try[Seq[Location]]

Inspections and Violations should share the existing connection

Currently we never need to get inspections and violations on their own. Only with the locations. So we should pass the connection instance that we use to get the location into the methods that get the inspections and violations.

This is because creating new connections is expensive and unnecessary here. This also removes a point of failure, since creating an exception can fail.

View: Character escapes/ Multi-Word Cities

The view breaks if you select any city with a multi-word name i.e "Belle Plaine"
If you take a look at the HTML source of the page
`

Belle Plaine ` but this should instead be something like ` Belle Plaine ` where the value of the option is in quotes or in some way encompasses multi-words

NOTE You do not necessarily need to spend a bunch of time fixing this, but keep this problem in mind when making the typeahead.

Restaurant spelled wrong in the database as "restaruant"

Minor fix, but I don't want change your script, as I might break something else. =P

Create "select city" page

This page is very similar to the "select location" page, but selects a city by name instead. The purpose is to refine the selection of location. In fact, the select location page is already taking in a city name. So the typeahead will be much simpler. It can directly set the text input and submit that. Since the "select location" page wasn't finished last iteration, you may wish to do this first, due to the simpler typeahead. There's more to do here, though.

1. Implement Location.listCities: Try[Seq[String]]. As you can probably tell, this should get a list of city names. See the other model functions for how to use SQL properly.
2. Implement LocationController.selectCity. Super straightforward, see the existing LocationController.findLocation for how this would work.
3. Create a route to that controller. Make it the index (ie, the / route). Note that the / route has already been defined, so you're actually redefining it. Remove the controller that the / route is currently pointing at. It's no longer necessary and we don't want unused code in our codebase.
4. Implement the view. Choose an appropriate name. This should submit a request to the findLocation controller. Use reverse routing (as previously mentioned for the "select location" page).

Database: Schema is missing violation type name

Violations have a name, but it's not in the schema.

Database: Program the Database (.sql file)

TODO: Program the SQL Database to be setup once I get the VM Set-up.

Database: Create .csv files for new Database

Create new .csv files to be added to the new Database.
Parsed from http://orii.health.gov.sk.ca/rhaReport.aspx?RHA=8

Database: Minor changes/Documentation

• Someone is going to have to document the process of getting locations into the database (basically the whole DB setup)
• There's a ton of files in the database folder that are very vague in intent. These should be documented somewhere.
• For example, what are the tab delimited files like location for? [DONE]
• Some files should perhaps be renamed. For example, why is there a "371CreateTables(2).sql"? [DONE]
• There's some SQL scripts that I'm not sure of their purpose (and this needs to be documented). Eg, 371Insert.sql.
• Now that we've actually used the CSV files, we should perhaps document what the obscure column names all refer to.

Testing: Milestone 1

• Integration tests, Mocks, etc for Milestone one

Documentation: Fill in time work on tasks

Hey everyone, I started up a activity log to record the time worked on a certain task. I'll fill most of the tasks from gitHub. If you have other things that you have done fill them in please

Database: Adapt database

Database parser needs to be in repo. [1] Done
Need to make sure that there’s no un-escaped characters in the database. [2] Done
Text in database needs to be changed to Title Case (first character of every non-common word capitalized, rest lowercase). [2] Done
Parser should output SQL files and not directly insert to database. [4]

Stakeholder Change: Current re-inspection priority

"Current re-inspection priority"

• currently is confusing to users
• maybe get rid of it
• We could just use ratings

Views: Template for displaying an inspection as an accordion [4]

• Use Bootstrap’s accordions.
• Needs to handle case of there not being any inspections (pretty sure that this shouldn’t happen, but we should prepare for the possibility, since the database and model allow for it).
• Suggested format: displayInspection(inspection: Inspection)
• Template for creating the past inspections section of the display locations page (which is collapsed and has an accordion for all older inspections). [5]
  --Needs to handle the case of there not being any older inspections.
  --Suggested format: displayPastInspections(location: Location)

need help on getter functions---Duan

In the branch "duan_need_help", I added the getLocation(), getInspections() and getViolations() functions which will execute SQL query and parser data to create new objects.

But I have some issues when I try to create a inspection with a list of violations (as well as a location with a list of inspections), I think it might be something to do with the try-match. could someone review it for me?

VM: VM credentials

I will update this when I receive the credentials

Create test hooks for database

This can be done by creating a custom type that stores the name of the database to use (recommended name: ActiveDatabase). The Play Framework's connections can specify the DB name, which we would get from this custom type.

Then every function that works on the database takes in an ActiveDatabase implicitly. This is why it must be a custom type: implicit parameters look for an implicit variable of a given type and use the first found.

Create a "globals" page (suggested name: Globals.scala) that is a package object and have it contain an instance of ActiveDatabase with the non-testing database (declare it as implicit and lazy). This must only be done for controllers and will only work for directly accessing the model. So for integration tests, you have to use the normal database. No way around that.

In the non-testing pages, we include this (ie, with import Globals._). Testing pages will declare their own implicit ActiveDatabase (or make their own globals file).

Setup VM

Once I get access, I'll setup the VM. :)

Typeaheads are converting some stuff to lowercase

Shouldn't be doing that.

Create ratings formula and function

1. Come up with a formula for ratings. See the forum board post by Mike for a start. You will have to decide the exact weights, and should implement the weights in such a way that it's easy to change them. Don't worry about their exact values, as you can come back and tweak them later.
2. Implement rating: String in Location, which returns a letter grade (eg, A+, B-, etc) according to the previously mentioned formula. Return "N/A" if a rating cannot be calculated (ie, there's somehow no inspections).
3. Modify the display location page to call this function where there's currently a hard coded "N/A". The page should check the rating an apply a different CSS style to each letter (so "A+", "A", and "A-" all would have the same CSS rule applied to them). This is for the purpose of applying colors (and possibly other styles?). Choose appropriate colors (green = A, red = F, etc).
4. Create a simple, static view describing the rating formula. Be sure to mention the weights (should use the values directly, so that there's only one place to change the weights) and shortfalls of the ratings in general (eg, the fact that our violations are binary pass-fail values and don't tell us the severity of the violation). Also mention a general disclaimer that the values don't necessarily represent the current situation at the location (may be outdated).
5. Add a link to open that page next to the rating (see the original mockup).