uwblueprint / somo

Text messaging survey app for Somo
http://www.thesomoproject.org
5 stars 1 forks source link

Survey models #40

Closed katrinacrisostomo closed 7 years ago

katrinacrisostomo commented 7 years ago

not wip anymore, ready for code review

Database schema

chrisjluc commented 7 years ago

@dinahshi we fixed the tests. By looking at the schema.rb and making sure the associations line up in the model. Kind of a mess of changes. We then changed the column ids because it was complaining about not having question_id vs questions_id etc.

dinahshi commented 7 years ago

Noticing some commits that only affect schema.rb and some with migrations and no changes to schema.rb. This is bad because it messes with our mental model of the db vs what it actually looks like which could lead to having to drop the db to reset. Every migration should be paired with a schema.rb update and the schema doc should not be changed except through a migration.

chrisjluc commented 7 years ago

Here is the migration commit that reflects changes in the schema commit. We deleted the tables and ran rake db:migrate and it produced the same schema.rb

dinahshi commented 7 years ago

I dropped my local db, recreated, and ran migrations and had inconsistencies with schema.rb - we should try to let the framework handle this for us rather than manually checking. Also rather than editing existing migrations, we should create new migrations. Not that big of a deal in this case since the migrations haven't been deployed but good habit to get into.

chrisjluc commented 7 years ago

@dinahshi and I clarified some confusions within the database models and our mental models.

So we don't want question_orders to be associated directly with a question. Instead, a question should have many response choices which point to the question_order, which in turn points to the next question. This allows for simplicity by following one branch of execution no matter what.

Therefore, execution is done by looking at the response choices and from there looking at the associated question order.

Previously question points to question_order and if it's not conditional we use this direct relationship, but if it's a conditional question we look at the response choice and go to the question order from there.

chrisjluc commented 7 years ago

I think we pluralized things when they should have been singular and vice versa. Which caused some column names to be named wrong in the tables.

i.e. the framework was looking for question_id but it didn't exist because the column was named questions_id in the table. That's why we changed the migration and schema and we thought it created the same schema.rb.

We did something wrong there if the schema.rb is different

katrinacrisostomo commented 7 years ago

OKAY LGTM 👍 @dinahshi, @chrisjluc ?

chrisjluc commented 7 years ago

LGTM