Course assistants #278
Course assistants #278
Conversation
| </tbody> | ||
| </table> | ||
|
|
||
| <%= link_to 'Add a new assistant', new_organization_course_assistant_path(@course.organization, @course), class: "btn btn-mini btn-warning" %> |
There was a problem hiding this comment.
this could also be a embedded form?
|
This is a step towards adding more controllable permissions; though always adding more and more relations doesn't really scale. However I consider this to be acceptable step towards goal of having more better control of permissions. |
|
Fixed assistant? methods, should be doing only one SQL query |
| @@ -121,7 +121,7 @@ def save_deadlines | |||
| end | |||
|
|
|||
| def enable | |||
| authorize! :teach, @organization | |||
| authorize! :teach, @organization # should assistants be able to enable/disable? | |||
There was a problem hiding this comment.
There is probably not enough time in this project but ideally we'd have more fine-grained abilities and allow organization admins to delegate them selectively.
There was a problem hiding this comment.
As I notice @jamox already said above..
I agree, OK to leave for now but should definitely refactor if we ever want to develop permissions further.
| </tr> | ||
| <% end %> | ||
| </tbody> | ||
| </table> |
|
LGTM except for the few comments |
…iew, fixed test names
|
|
||
| <ul> | ||
| <% @assistants.each do |assistant| %> | ||
| <li><%= link_to assistant.username, participant_path(assistant) %></li> |
There was a problem hiding this comment.
Note for myself: shouldn't this be login, not username...
There was a problem hiding this comment.
ok, so the login is the field, but we have a hax alias for it. nice
…-assistants Conflicts: app/models/ability.rb app/models/course.rb db/schema.rb
|
Should be able to auto-merge, and tests should pass if internet is on |
No description provided.