Role based code cleanup

[btbonval]

Until #264 is pursued, it might be a good idea to compress as much of the role based logic as possible into one place.

For example, we could make partial templates which render the following:

plots2/app/views/users/profile.html.erb

Lines 6 to 17 in fe2ba03

	<% if @user.user.nil? && current_user && current_user.role == "admin" %>
	<a href="/admin/migrate/<%= @user.uid %>" class="btn btn-block"><i class="icon-bolt"></i> Migrate to new site</a>
	<% end %>
	<% if @user.user && current_user && current_user.role == "admin" && @user.user.role != "admin" %>
	<a href="/admin/promote/admin/<%= @user.user.id %>" class="btn btn-block"><i class="icon-certificate"></i> Make admin</a>
	<% end %>
	<% if @user.user && current_user && current_user.role == "admin" && @user.role == "moderator" %>
	<a href="/admin/demote/basic/<%= @user.user.id %>" class="btn btn-block"><i class="icon-ban-circle"></i> Revert to basic user</a>
	<% end %>
	<% if @user.user && current_user && (current_user.role == "admin" || current_user.role == "moderator") && @user.role == "basic" %>
	<a href="/admin/promote/moderator/<%= @user.user.id %>" class="btn btn-block"><i class="icon-certificate"></i> Make moderator</a>
	<% end %>

All of the if/then logic would move into the partial, and that highlighted section would include the partial. That allows us to change how the template logic works later in one place. If we have a few partials, they can all go in the same directory under maybe templates/roles/.

Another copy/paste logic can be seen here:

plots2/app/controllers/admin_controller.rb

Lines 6 to 13 in ad7fa8d

	if current_user && current_user.role == "admin"
	@user = User.find params[:id]
	@user.role = 'admin'
	@user.save({})
	flash[:notice] = "User '<a href='/profile/"+@user.username+"'>"+@user.username+"</a>' is now an admin."
	else
	flash[:error] = "Only admins can promote other users to admins."
	end

If this were Python, I'd write a decorator which checks user credentials and returns an error if they are not met. Since this is Ruby, I don't know a good way. Here's what I'd do in my naivety: write a function which takes the user and required role in; if there is a failed match, it would set the flash error and return False. Then you could copy paste if not has_permission(user, 'admin') return or something more Ruby-like.

The above is a little ugly, but such a function could probably be rewritten for RBAC later without needing to make wide sweeping code changes (though this change itself would be such a wide-sweeping code change).

[btbonval]

I just noticed that the highlighted section of the partial has two different ways to check if a user is defined. if @user.user.nil? && current_user and if @user.user && current_user. This already shows one of the problems with not being DRY.

If we write a has_permission function, it should contain the check to see if @user is a proper user, however that is decided. Then we can call the has_permission function directly from the partial template. That'd make the code a little more sane as well.

[btbonval]

Alright it turns out the differences are intentional. Misread that one is looking for no, the user does not exist and one is looking for yes, the user does exist. At any rate, same logic still applies for DRY to prevent the possibility of copy/paste mistakes :)

[jywarren]

We should consolidate using this code:

plots2/app/helpers/application_helper.rb

Line 3 in 69ff570

def logged_in_as(roles)

[grvsachdeva]

@jywarren please update the issue. Thanks!

[jywarren]

I'll open a new issue! I think we can build out our model code to make this simpler, without yet planning out a full multi-role system. If we go there, then we might merge this into profile tags!