Update example app and demonstrate some other best practices #239
Conversation
|
Getting closer. It now creates an admin and then creates an app database using those admin credentials. |
rjcorwin
changed the title
|
|
|
@nolanlawson I may not get to the other items on my checklist so it may be good as is. I could squash the commits in this PR if you like. |
There was a problem hiding this comment.
Hi! Sorry for only getting to this now. It looks really nice! I gave two inline comments with suggestions, but if you disagree I'm fine with merging this as-is, at it is helpful like this too. I can squash the commits together when merging, but if you want to do so using a rebase that is fine as well.
|
|
||
| async function setup() { | ||
|
|
||
| // Set up the admin user. |
There was a problem hiding this comment.
app.couchConfig.set('admins', config.admin.username, config.admin.password, function (err, previousValue) {})
might be more robust. Less chance, although that chance is already small admittedly, of someone hijacking your database before the admin is set. Note that I did not run the code, so using it would require testing it.
There was a problem hiding this comment.
Less chance, although that chance is already small admittedly, of someone hijacking your database before the admin is set.
Lol, ya, I've thought about that but not overly worried. I didn't know about app.couchConfig.set. Cool stuff!
There was a problem hiding this comment.
app.couchConfig.set does not seem to work. It makes it crash with "TypeError: refreshUsersDBImpl is not a function". What is weird is that it still set up the config file properly...
There was a problem hiding this comment.
It seems what's happening in this case is that the users data base isn't ready yet, but setting up an admin will still cause it to try to update it.
There was a problem hiding this comment.
I've created an issue here #341
| console.log("We already have admins."); | ||
| } | ||
|
|
||
| // Set up the app database. |
There was a problem hiding this comment.
Similarly, if you store the PouchDB.defaults() intermediate result above, you could just do new PouchDB('todos') here. Again, it removes the HTTP roundtrip.
|
I'm in the middle of a crunch right no so I might not get to this for a couple of weeks but thank you so much for reviewing!! |
I was having some issues getting the example todo app running and @vinz243 had a good request in #224 that on application first start it set up some security.