Closed charlesconnell closed 10 years ago
@btbonval would you mind taking a quick peek at this too. You're code review is going to be better than mine. Overall it looks good and won't expose the new quiz functionality yet.
All the models need natural_key junk in them for database backups, but that can be done later since the backup stuff is a ways off and so many other things get priority.
The KeyWord URLs seem odd so I'm trying to figure out if there is a way to simplify that. It looks like KeyWord is defined uniquely by a note and a word. It seems like the URL should only need note slug and the word. Not sure why school slug or course slug is needed... and very confused why the word is not part of the URL. I'm definitely missing something there. https://github.com/FinalsClub/karmaworld/blob/quizzes/karmaworld/apps/quizzes/views.py#L60
Still looking through things...
Oh I see. keyword_edit lists all keywords for the given note. https://github.com/FinalsClub/karmaworld/blob/quizzes/karmaworld/apps/quizzes/views.py#L63
note_slug is uniquely identifying for any note in the database regardless of course or school. Course and school aren't needed in the URL. Just adds noise. Otherwise I get it.
Okay. Done reviewing.
I think the keyword_edit URL should be cleaned up before accepting the pull request.
I made a bunch of other comments but they're take-it-or-leave-it style.
Thanks Bryan.
On Mar 12, 2014, at 8:03 PM, Bryan Bonvallet notifications@github.com wrote:
Okay. Done reviewing.
I think the keyword_edit URL should be cleaned up before accepting the pull request.
I made a bunch of other comments but they're take-it-or-leave-it style.
— Reply to this email directly or view it on GitHub.
Keyword urls made simpler.
This merge should not have an impact on existing functionality or UI. Keyword and quiz pages are reachable, but links to them are not exposed. Note pages will now be at www.karmanotes.org/note/... This was something that I think (and Bryan has expressed as well) was inevitable -- it was just too ambiguous without something to differentiate note pages from everything else.