search

BYU-ODH / hlrdesk

https://hlrdesk.hlrdev.byu.edu/
MIT License
0 stars 2 forks source link

Add message alert #352

Closed trevren11 closed 8 years ago

trevren11 commented 8 years ago

Added: message tests app module for messages new message notification database migrations made the code slightly easier on the eyes, but not much

trevren11 commented 8 years ago

I was using the .one() to try and figure out why some things were firing more than once so I kind of abused that. Also, before (and after I make some changes) it is merged, whoever merges either make a backup of the database or ask me to. I forgot to put that up top.

webnard commented 8 years ago

@trevren11 It looks good to me. I would ditch the .one calls, because I don't think they're doing anything, but other than that I would give the go ahead. What happens when you run npm install twice in a row? Do the migrations work as expected?

trevren11 commented 8 years ago

Running npm install works just fine in dev and production (well as far as me setting the variable DEV to false (also if you just invoke the script it works as expected), the function above will check to see if the column exists and if it does, it will not do anything, if not it will add it to the database. I was looking into how to run the migrations file twice but I don't really know how to test that on chai. Do you know a way to run a shell script from chai? I couldn't find a way.

aldahl commented 8 years ago

Looks good to me. Merging.