lml / ost

OpenStax Tutor
Other
7 stars 8 forks source link

Feature/exercises #348

Closed navilan closed 10 years ago

navilan commented 10 years ago

WIP. Refactored and consolidated the previous pull requests.

navilan commented 10 years ago

@jpslav / @kjdav - I think this is fairly complete. I am not sure if the multiple cohorts are aggregated as I am still having issues in configuring the DB.

@jpslav - I am not too confident of the model code in the last commit. Can you please let me know if I have screwed up in a big way? :)

navilan commented 10 years ago

@jpslav / @kjdav - Thanks to Kim's dev db - I can see that cohorts don't work as expected yet. Will fix.

navilan commented 10 years ago

@jpslav / @kjdav - multiple cohort support is in there now. I am not too happy with the styles. Will work on polishing the styles and fixing any comments you have tomorrow.

navilan commented 10 years ago

Thanks so much @jpslav / @Dantemss. Great feedback. Will get these addressed now.

navilan commented 10 years ago

@Dantemss: The last commit has the new query. There is a harmless but nevertheless annoying issue where joins are repeated. Other than that seems to be a good solution. Thanks so much.

@jpslav : I added prev/next methods to acts as numberable - please let me know if it looks good.

@kjdav : Kim, at this point, I think that functionally the page is complete - whats remaining from my rather limited perspective is polish (styling, testing, optimization). If you can look through it one more time and let me know your comments, I can address them as a final pass before this gets merged.

navilan commented 10 years ago

@jpslav / @kjdav / @Dantemss - I think the PR is ready for final review / testing and merge.

Thanks so much for all your help. Much appreciated.

kjdav commented 10 years ago

this looks good from my perspective, both the changes to the branch and also the result when i merged into my local master. I checked that instructor/researcher permissions are correct for these pages, that the next/prev nav goes to the correct questions, and that students can see all the new info lakshmi added. looks great.

On Thu, May 22, 2014 at 4:15 PM, Lakshmi notifications@github.com wrote:

@jpslav https://github.com/jpslav / @kjdav https://github.com/kjdav / @Dantemss https://github.com/Dantemss - I think the PR is ready for final review / testing and merge.

Thanks so much for all your help. Much appreciated.

— Reply to this email directly or view it on GitHubhttps://github.com/lml/ost/pull/348#issuecomment-43938453 .

navilan commented 10 years ago

@kjdav - Awesome :) Excited about having my first commit to the project.

@jpslav - I am waiting for this to be merged before I start the rest of the issues. I am now in deathmatch mode ;)

jpslav commented 10 years ago

Thanks @lakshmivyas!