Code review comment for lp:~ivle-dev/ivle/exercise-ui

Revision history for this message
William Grant (wgrant) wrote :

- ivle.database has some XHTML in it! Please obliterate it.
- ExerciseDeleteView:
  + remove_exercise() should be a method of Exercise.
  + populate() confuses me.
- ExercisesRESTView:
  + get_permissions() doesn't make sense - why would the user be a member of the request's user's offering roles? Also, you don't have req at all there.
- ExerciseRESTView:
  + get_permissions(): same as ExercisesRESTView.
  + edit_exercise() returns {'result': 'moo'}.
  + delete_exercise() might not work, and duplicates ExerciseDeleteView.remove_exercise().
  + delete_suite()'s guts belong in TestSuite.
  + ExerciseRESTView.(add|edit)_var(): One casts argno to an int, the other doesn't. Why?

- The authorization mechanisms for Exercise editing need to be factored out. Probably into Exercise.get_permissions() itself.
- There are lots of NotFound('someobject')s.
- Somebody (probably me) also needs to fix the jQuery symlink (maybe make the installer do it?)

« Back to merge proposal