From b8ade54001d3480693016e52353b96a1bc79f9a2 Mon Sep 17 00:00:00 2001 From: Daniela Soares Feitosa Date: Thu, 27 Jan 2011 03:10:41 -0300 Subject: [PATCH] Procedure after last admin leaves organization --- app/controllers/my_profile/profile_members_controller.rb | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------- app/controllers/public/profile_controller.rb | 19 +++++++++++-------- app/models/environment_mailing.rb | 2 +- app/models/organization_mailing.rb | 2 +- app/models/person.rb | 20 ++++++++++++++++++-- app/models/profile.rb | 15 +++++++++++---- app/views/memberships/index.rhtml | 2 +- app/views/profile_members/_add_admins.rhtml | 30 ++++++++++++++++++++++++++++++ app/views/profile_members/_members_list.rhtml | 11 +++++++---- app/views/profile_members/add_admin.rhtml | 1 + app/views/profile_members/add_members.rhtml | 4 ++-- app/views/profile_members/find_users.rhtml | 6 +++--- app/views/profile_members/last_admin.rhtml | 27 +++++++++++++++++++++++++++ app/views/profile_members/remove_admin.rhtml | 1 + features/delete_profile.feature | 33 ++++++++++++--------------------- features/last_administrator_leaving.feature | 38 ++++++++++++++++++++++++++++++++++++++ features/register_enterprise.feature | 4 ++-- features/step_definitions/noosfero_steps.rb | 18 +++++++++++++++--- features/support/paths.rb | 6 ++++++ public/javascripts/add-and-join.js | 31 ++++++++++++++++++++----------- public/stylesheets/application.css | 11 ++++++++++- test/functional/content_viewer_controller_test.rb | 10 ++++++---- test/functional/memberships_controller_test.rb | 8 +++++--- test/functional/profile_controller_test.rb | 40 ++++++++++++++++++++++++++++++++++++---- test/functional/profile_members_controller_test.rb | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- test/unit/application_helper_test.rb | 13 +++++++++++-- test/unit/community_test.rb | 25 ++++++++++++++++++++++++- test/unit/enterprise_test.rb | 20 +++++++++++++++----- test/unit/person_test.rb | 39 ++++++++++++++++++++++++++++++++++----- test/unit/profile_test.rb | 38 +++++++++++++++++++++----------------- 30 files changed, 491 insertions(+), 134 deletions(-) create mode 100644 app/views/profile_members/_add_admins.rhtml create mode 120000 app/views/profile_members/add_admin.rhtml create mode 100644 app/views/profile_members/last_admin.rhtml create mode 120000 app/views/profile_members/remove_admin.rhtml create mode 100644 features/last_administrator_leaving.feature diff --git a/app/controllers/my_profile/profile_members_controller.rb b/app/controllers/my_profile/profile_members_controller.rb index e760c49..758987b 100644 --- a/app/controllers/my_profile/profile_members_controller.rb +++ b/app/controllers/my_profile/profile_members_controller.rb @@ -15,26 +15,29 @@ class ProfileMembersController < MyProfileController rescue ActiveRecord::RecordNotFound @person = nil end - if @person && @person.define_roles(@roles, profile) - session[:notice] = _('Roles successfuly updated') + if !params[:confirmation] && @person && @person.is_last_admin_leaving?(profile, @roles) + redirect_to :action => :last_admin, :roles => params[:roles], :person => @person else - session[:notice] = _('Couldn\'t change the roles') + if @person && @person.define_roles(@roles, profile) + session[:notice] = _('Roles successfuly updated') + else + session[:notice] = _('Couldn\'t change the roles') + end + if params[:confirmation] + redirect_to profile.url + else + redirect_to :action => :index + end end - redirect_to :action => :index end - def change_role - @roles = Profile::Roles.organization_member_roles(environment.id) - begin - @member = profile.members.find(params[:id]) - rescue ActiveRecord::RecordNotFound - @member = nil - end - if @member - @associations = @member.find_roles(@profile) - else - redirect_to :action => :index - end + def last_admin + @person = params[:person] + @roles = params[:roles] || [] + @members = profile.members.select {|member| !profile.admins.include?(member)} + @title = _('Current admins') + @collection = :profile_admins + @remove_action = {:action => 'remove_admin'} end def add_role @@ -59,14 +62,28 @@ class ProfileMembersController < MyProfileController render :layout => false end + def change_role + @roles = Profile::Roles.organization_member_roles(environment.id) + begin + @member = profile.members.find(params[:id]) + rescue ActiveRecord::RecordNotFound + @member = nil + end + if @member + @associations = @member.find_roles(@profile) + else + redirect_to :action => :index + end + end + def unassociate member = Person.find(params[:id]) associations = member.find_roles(profile) RoleAssignment.transaction do if associations.map(&:destroy) - session[:notice] = 'Member succefully unassociated' + session[:notice] = _('Member succesfully unassociated') else - session[:notice] = 'Failed to unassociate member' + session[:notice] = _('Failed to unassociate member') end end render :layout => false @@ -83,11 +100,39 @@ class ProfileMembersController < MyProfileController render :layout => false end + def add_admin + @title = _('Current admins') + @collection = :profile_admins + + if profile.community? + member = profile.members.find_by_identifier(params[:id]) + profile.add_admin(member) + end + render :layout => false + end + + def remove_admin + @title = _('Current admins') + @collection = :profile_admins + + if profile.community? + member = profile.members.find_by_identifier(params[:id]) + profile.remove_admin(member) + end + render :layout => false + end + def find_users if !params[:query] || params[:query].length <= 2 @users_found = [] - else - @users_found = Person.find_by_contents(params[:query] + '*') + elsif params[:scope] == 'all_users' + @users_found = Person.find_by_contents(params[:query] + '*').select {|user| !profile.members.include?(user)} + @button_alt = _('Add member') + @add_action = {:action => 'add_member'} + elsif params[:scope] == 'new_admins' + @users_found = Person.find_by_contents(params[:query] + '*').select {|user| profile.members.include?(user) && !profile.admins.include?(user)} + @button_alt = _('Add member') + @add_action = {:action => 'add_admin'} end render :layout => false end diff --git a/app/controllers/public/profile_controller.rb b/app/controllers/public/profile_controller.rb index 242bf89..461bebb 100644 --- a/app/controllers/public/profile_controller.rb +++ b/app/controllers/public/profile_controller.rb @@ -82,13 +82,13 @@ class ProfileController < PublicController def join if !user.memberships.include?(profile) profile.add_member(user) - if profile.closed? - render :text => _('%s administrator still needs to accept you as member.') % profile.name + if !profile.members.include?(user) + render :text => {:message => _('%s administrator still needs to accept you as member.') % profile.name}.to_json else - render :text => _('You just became a member of %s.') % profile.name + render :text => {:message => _('You just became a member of %s.') % profile.name}.to_json end else - render :text => _('You are already a member of %s.') % profile.name + render :text => {:message => _('You are already a member of %s.') % profile.name}.to_json end end @@ -110,11 +110,14 @@ class ProfileController < PublicController end def leave - if user.memberships.include?(profile) - profile.remove_member(current_user.person) - render :text => _('You just left %s.') % profile.name + if current_person.memberships.include?(profile) + if current_person.is_last_admin?(profile) + render :text => {:redirect_to => url_for({:controller => 'profile_members', :action => 'last_admin', :person => current_person.id})}.to_json + else + render :text => current_person.leave(profile, params[:reload]) + end else - render :text => _('You are already a member of %s.') % profile.name + render :text => {:message => _('You are not a member of %s.') % profile.name}.to_json end end diff --git a/app/models/environment_mailing.rb b/app/models/environment_mailing.rb index cf4502a..02f4e45 100644 --- a/app/models/environment_mailing.rb +++ b/app/models/environment_mailing.rb @@ -1,7 +1,7 @@ class EnvironmentMailing < Mailing def recipients(offset=0, limit=100) - source.people.all(:order => :id, :offset => offset, :limit => limit, :joins => "LEFT OUTER JOIN mailing_sents m ON (m.mailing_id = #{id} AND m.person_id = profiles.id)", :order => :id, :conditions => { "m.person_id" => nil }) + source.people.all(:order => :id, :offset => offset, :limit => limit, :joins => "LEFT OUTER JOIN mailing_sents m ON (m.mailing_id = #{id} AND m.person_id = profiles.id)", :conditions => { "m.person_id" => nil }) end def each_recipient diff --git a/app/models/organization_mailing.rb b/app/models/organization_mailing.rb index eafed6d..9cf9d86 100644 --- a/app/models/organization_mailing.rb +++ b/app/models/organization_mailing.rb @@ -5,7 +5,7 @@ class OrganizationMailing < Mailing end def recipients(offset=0, limit=100) - source.members.all(:order => :id, :offset => offset, :limit => limit, :joins => "LEFT OUTER JOIN mailing_sents m ON (m.mailing_id = #{id} AND m.person_id = profiles.id)", :order => :id, :conditions => { "m.person_id" => nil }) + source.members.all(:order => self.id, :offset => offset, :limit => limit, :joins => "LEFT OUTER JOIN mailing_sents m ON (m.mailing_id = #{id} AND m.person_id = profiles.id)", :conditions => { "m.person_id" => nil }) end def each_recipient diff --git a/app/models/person.rb b/app/models/person.rb index 9151321..9cb73c8 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -4,7 +4,7 @@ class Person < Profile acts_as_trackable :after_add => Proc.new {|p,t| notify_activity(t)} acts_as_accessor - named_scope :members_of, lambda { |resource| { :select => 'DISTINCT profiles.*', :joins => :role_assignments, :conditions => ['role_assignments.resource_type = ? AND role_assignments.resource_id = ?', resource.class.base_class.name, resource.id ] } } + named_scope :members_of, lambda { |resource| { :select => 'DISTINCT profiles.*', :include => :role_assignments, :group => 'profiles.id', :conditions => ['role_assignments.resource_type = ? AND role_assignments.resource_id = ?', resource.class.base_class.name, resource.id ] } } def memberships Profile.memberships_of(self) @@ -363,10 +363,26 @@ class Person < Profile generate_url(:profile => identifier, :controller => 'profile', :action => 'index', :anchor => 'profile-wall') end + def is_last_admin?(organization) + organization.admins == [self] + end + + def is_last_admin_leaving?(organization, roles) + is_last_admin?(organization) && roles.select {|role| role.key == "profile_admin"}.blank? + end + + def leave(profile, reload = false) + leave_hash = {:message => _('You just left %s.') % profile.name} + if reload + leave_hash.merge!({:reload => true}) + end + profile.remove_member(self) + leave_hash.to_json + end + protected def followed_by?(profile) self == profile || self.is_a_friend?(profile) end - end diff --git a/app/models/profile.rb b/app/models/profile.rb index 288dfcd..aeb8c18 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -424,8 +424,8 @@ class Profile < ActiveRecord::Base { :profile => identifier, :controller => 'profile_editor', :action => 'index' } end - def leave_url - { :profile => identifier, :controller => 'profile', :action => 'leave' } + def leave_url(reload = false) + { :profile => identifier, :controller => 'profile', :action => 'leave', :reload => reload } end def join_url @@ -563,13 +563,16 @@ private :generate_url, :url_options # Adds a person as member of this Profile. def add_member(person) if self.has_members? - if self.closed? + if self.closed? && members.count > 0 AddMember.create!(:person => person, :organization => self) unless self.already_request_membership?(person) else + if members.count == 0 + self.affiliate(person, Profile::Roles.admin(environment.id)) + end self.affiliate(person, Profile::Roles.member(environment.id)) end else - raise _("%s can't has members") % self.class.name + raise _("%s can't have members") % self.class.name end end @@ -582,6 +585,10 @@ private :generate_url, :url_options self.affiliate(person, Profile::Roles.admin(environment.id)) end + def remove_admin(person) + self.disaffiliate(person, Profile::Roles.admin(environment.id)) + end + def add_moderator(person) if self.has_members? self.affiliate(person, Profile::Roles.moderator(environment.id)) diff --git a/app/views/memberships/index.rhtml b/app/views/memberships/index.rhtml index a4b323d..887089f 100644 --- a/app/views/memberships/index.rhtml +++ b/app/views/memberships/index.rhtml @@ -23,7 +23,7 @@ <%= _('Created at: %s') % show_date(membership.created_at) unless membership.enterprise? %>
<% button_bar do %> <%= button 'menu-ctrl-panel', _('Control panel of this group'), membership.admin_url %> - <%= lightbox_button 'menu-logout', _('Leave'), membership.leave_url %> + <%= button 'menu-logout', _('Leave'), membership.leave_url(true), :class => 'leave-community' %> <% if (membership.community? && user.has_permission?(:destroy_profile, membership)) %> <%= button 'delete', _('Remove'), { :controller => 'profile_editor', :action => 'destroy_profile', :profile => membership.identifier } %> <% end %> diff --git a/app/views/profile_members/_add_admins.rhtml b/app/views/profile_members/_add_admins.rhtml new file mode 100644 index 0000000..c89eece --- /dev/null +++ b/app/views/profile_members/_add_admins.rhtml @@ -0,0 +1,30 @@ +

<%= _('Add admins to %s') % profile.name %>

+ +<% form_remote_tag :url => {:action => 'find_users', :profile => profile.identifier, :scope => 'new_admins'}, :update => 'users-list', :loading => '$("users-list").addClassName("loading")', :complete => '$("users-list").removeClassName("loading")' do %> + <%= text_field_tag('query', '', :autocomplete => 'off') %> + <%= submit_tag(_('Search')) %> +<% end %> + +<%= observe_field('query', :url => {:action => 'find_users', :profile => profile.identifier, :scope => 'new_admins'}, :update => 'users-list', :frequency => 1, :with => 'query', :condition => '$("query").value.length > 2', :loading => '$("users-list").addClassName("loading")', :complete => '$("users-list").removeClassName("loading")') %> +<%= observe_field('query', :frequency => 1, :condition => '$("query").value.length <= 2', :function => '$("users-list").update($("empty-query").innerHTML)') %> + +
+ <%= render :partial => 'find_users' %> +
+ + + +
+ <%= render :partial => 'members_list' %> +
+<%= drop_receiving_element('members-list', + :url => {:action => 'add_admin', :profile => profile.identifier, :leaving_admin => @person}, + :before => '$("tr-" + element.id).hide()', + :loading => '$("members-list").addClassName("loading")', + :update => 'members-list', + :success => '$("tr-" + element.id).hide(); $(element.id).show();', + :complete => '$("members-list").removeClassName("loading")') %> + +
diff --git a/app/views/profile_members/_members_list.rhtml b/app/views/profile_members/_members_list.rhtml index 52cc898..1de14e6 100644 --- a/app/views/profile_members/_members_list.rhtml +++ b/app/views/profile_members/_members_list.rhtml @@ -1,11 +1,15 @@ -

<%= _('Current Members') %>

+<% collection = @collection == :profile_admins ? profile.admins : profile.members %> +<% title = @title ? @title : "Current members" %> +<% remove_action = @remove_action ? @remove_action : {:action => 'unassociate'} %> + +

<%= _(title) %>

- <% profile.members.each do |m| %> + <% collection.each do |m| %> diff --git a/app/views/profile_members/add_admin.rhtml b/app/views/profile_members/add_admin.rhtml new file mode 120000 index 0000000..12e1b0b --- /dev/null +++ b/app/views/profile_members/add_admin.rhtml @@ -0,0 +1 @@ +_members_list.rhtml \ No newline at end of file diff --git a/app/views/profile_members/add_members.rhtml b/app/views/profile_members/add_members.rhtml index 4119e76..1f7c8c8 100644 --- a/app/views/profile_members/add_members.rhtml +++ b/app/views/profile_members/add_members.rhtml @@ -1,11 +1,11 @@

<%= _('Add members to %s') % profile.name %>

-<% form_remote_tag :url => {:action => 'find_users', :profile => profile.identifier}, :update => 'users-list', :loading => '$("users-list").addClassName("loading")', :complete => '$("users-list").removeClassName("loading")' do %> +<% form_remote_tag :url => {:action => 'find_users', :profile => profile.identifier, :scope => 'all_users'}, :update => 'users-list', :loading => '$("users-list").addClassName("loading")', :complete => '$("users-list").removeClassName("loading")' do %> <%= text_field_tag('query', '', :autocomplete => 'off') %> <%= submit_tag(_('Search')) %> <% end %> -<%= observe_field('query', :url => {:action => 'find_users', :profile => profile.identifier}, :update => 'users-list', :frequency => 1, :with => 'query', :condition => '$("query").value.length > 2', :loading => '$("users-list").addClassName("loading")', :complete => '$("users-list").removeClassName("loading")') %> +<%= observe_field('query', :url => {:action => 'find_users', :profile => profile.identifier, :scope => 'all_users'}, :update => 'users-list', :frequency => 1, :with => 'query', :condition => '$("query").value.length > 2', :loading => '$("users-list").addClassName("loading")', :complete => '$("users-list").removeClassName("loading")') %> <%= observe_field('query', :frequency => 1, :condition => '$("query").value.length <= 2', :function => '$("users-list").update($("empty-query").innerHTML)') %>
diff --git a/app/views/profile_members/find_users.rhtml b/app/views/profile_members/find_users.rhtml index 4ec8963..2584376 100644 --- a/app/views/profile_members/find_users.rhtml +++ b/app/views/profile_members/find_users.rhtml @@ -2,7 +2,7 @@
<%= _('Member') %> <%= _('Actions') %>
<%= link_to_profile m.short_name, m.identifier, :title => m.name %> @@ -16,8 +20,7 @@ :loading => '$("members-list").addClassName("loading")', :success => "$('tr-#{m.identifier}').show()", :complete => '$("members-list").removeClassName("loading")', - :confirm => _('Are you sure that you want to remove this member?'), - :url => {:action => 'unassociate', :id => m}) if m != user %> + :url => { :id => m }.merge(remove_action)) if m != user %>
<% @users_found.each do |user| %> - > +
<%= _('Name') %>
<%= image_tag('/images/grip-clue.png') %> @@ -14,10 +14,10 @@ <%= draggable_element(user.identifier, :revert => true) %>
- <%= button_to_remote_without_text(:add, _('Add member'), + <%= button_to_remote_without_text(:add, @button_alt, { :loading => '$("members-list").addClassName("loading")', :update => 'members-list', - :url => {:action => 'add_member', :profile => profile.identifier, :id => user.id}, + :url => {:id => user.id, :profile => profile.identifier}.merge(@add_action), :success => "$('tr-#{user.identifier}').hide()", :complete => '$("members-list").removeClassName("loading")'}) %> diff --git a/app/views/profile_members/last_admin.rhtml b/app/views/profile_members/last_admin.rhtml new file mode 100644 index 0000000..270225a --- /dev/null +++ b/app/views/profile_members/last_admin.rhtml @@ -0,0 +1,27 @@ +

<%= _('Last administrator leaving %s') % profile.name %>

+ +<% if profile.members.count > 1 %> +
+ <%= _('Since you are the last administrator, you must choose at least one member to administer this community.') % profile.name %> +
+ + <%= render :partial => 'add_admins' %> + + <% form_tag :action => 'update_roles', :roles => @roles, :person => @person, :confirmation => profile.admins.count > 1 do %> + <% button_bar do %> + <%= submit_button(:save, _("Leave administration and save"), :cancel => {:action => 'index'}) %> + <% end %> + <% end %> + +<% else %> + +
+ <%= _('Since you are the last administrator and there is no other member in this community, the next member to join this community will assume the administrator role.') % profile.name %> +
+ + <% form_tag :action => 'update_roles', :roles => @roles, :person => @person, :confirmation => true do %> + <% button_bar do %> + <%= submit_button(:ok, _("Ok, I want to leave"), :cancel => {:action => 'index'}) %> + <% end %> + <% end %> +<% end %> diff --git a/app/views/profile_members/remove_admin.rhtml b/app/views/profile_members/remove_admin.rhtml new file mode 120000 index 0000000..12e1b0b --- /dev/null +++ b/app/views/profile_members/remove_admin.rhtml @@ -0,0 +1 @@ +_members_list.rhtml \ No newline at end of file diff --git a/features/delete_profile.feature b/features/delete_profile.feature index a077e48..f443e2e 100644 --- a/features/delete_profile.feature +++ b/features/delete_profile.feature @@ -7,6 +7,11 @@ Feature: delete profile Given the following users | login | name | | joaosilva | Joao Silva | + | mariasilva | Maria Silva | + And the following community + | identifier | name | + | sample-community | Sample Community | + And "Maria Silva" is a member of "Sample Community" Scenario: deleting profile Given I am logged in as "joaosilva" @@ -20,10 +25,7 @@ Feature: delete profile Then I should see "There is no such page" Scenario: deleting other profile - Given the following users - | login | name | - | mariasilva | Maria Silva | - And I am logged in as "mariasilva" + Given I am logged in as "mariasilva" And I go to /myprofile/joaosilva/profile_editor/destroy_profile Then I should see "Access denied" @@ -37,20 +39,14 @@ Feature: delete profile Then I should be on Joao Silva's profile Scenario: community admin can see link to delete profile - Given the following community - | identifier | name | - | sample-community | Sample Community | - And "Joao Silva" is admin of "Sample Community" + Given "Joao Silva" is admin of "Sample Community" And I am logged in as "joaosilva" And I am on Sample Community's control panel When I follow "Community Info and settings" Then I should see "Delete profile" Scenario: community admin deletes the community - Given the following community - | identifier | name | - | sample-community | Sample Community | - And "Joao Silva" is admin of "Sample Community" + Given "Joao Silva" is admin of "Sample Community" And I am logged in as "joaosilva" And I am on Sample Community's control panel And I follow "Community Info and settings" @@ -62,10 +58,7 @@ Feature: delete profile Then I should see "There is no such page" Scenario: community regular member tries to delete the community - Given the following community - | identifier | name | - | sample-community | Sample Community | - And "Joao Silva" is a member of "Sample Community" + Given "Joao Silva" is a member of "Sample Community" And I am logged in as "joaosilva" And I go to /myprofile/sample-community/profile_editor/destroy_profile Then I should see "Access denied" @@ -96,19 +89,17 @@ Feature: delete profile Then I should see "There is no such page" Scenario: enterprise regular member tries to delete the enterprise - Given the following community + Given the following enterprise | identifier | name | | sample-enterprise | Sample Enterprise | + And "Maria Silva" is a member of "Sample Enterprise" And "Joao Silva" is a member of "Sample Enterprise" And I am logged in as "joaosilva" And I go to /myprofile/sample-enterprise/profile_editor/destroy_profile Then I should see "Access denied" Scenario: community regular member cannot see link to delete profile - Given the following community - | identifier | name | - | sample-community | Sample Community | - And "Joao Silva" is a member of "Sample Community" + Given "Joao Silva" is a member of "Sample Community" And I am logged in as "joaosilva" And I am on Sample Community's control panel When I follow "Community Info and settings" diff --git a/features/last_administrator_leaving.feature b/features/last_administrator_leaving.feature new file mode 100644 index 0000000..83debd1 --- /dev/null +++ b/features/last_administrator_leaving.feature @@ -0,0 +1,38 @@ +Feature: remove administrator role + As an organization administrator + I want to remove my administrator role + In order to stop administrating the organization + + Background: + Given the following users + | login | name | + | joaosilva | Joao Silva | + | mariasouza | Maria Souza | + And the following community + | name | identifier | + | Nice people | nice-people | + And "Joao Silva" is admin of "Nice people" + And I am logged in as "joaosilva" + + Scenario: the last administrator and member removes his administrator role and the next member to join becomes the new administrator + Given I am on Nice people's members management + And I follow "Edit" + And I uncheck "Profile Administrator" + When I press "Save changes" + Then I should see "Since you are the last administrator and there is no other member in this community" + And I press "Ok, I want to leave" + And I am logged in as "mariasouza" + When I go to Nice people's join page + Then "Maria Souza" should be admin of "Nice people" + + Scenario: the last administrator and member removes his administrator role and the next member to join becomes the new administrator even if the organization is closed. + Given the community "Nice people" is closed + And I am on Nice people's members management + And I follow "Edit" + And I uncheck "Profile Administrator" + When I press "Save changes" + Then I should see "Since you are the last administrator and there is no other member in this community" + And I press "Ok, I want to leave" + And I am logged in as "mariasouza" + When I go to Nice people's join page + Then "Maria Souza" should be admin of "Nice people" diff --git a/features/register_enterprise.feature b/features/register_enterprise.feature index f01aa40..0acaa05 100644 --- a/features/register_enterprise.feature +++ b/features/register_enterprise.feature @@ -93,7 +93,7 @@ Feature: register enterprise Then I should see "Enterprise registration completed" And I am logged in as admin And I go to the Control panel - When I follow "Tasks" + When I follow "Tasks" within ".control-panel" Then I should see "Joao Silva wants to create enterprise My Enterprise." And the first mail is to admin_user@example.com And I choose "Accept" @@ -120,7 +120,7 @@ Feature: register enterprise Then I should see "Enterprise registration completed" And I am logged in as admin And I go to the Control panel - When I follow "Tasks" + When I follow "Tasks" within ".control-panel" Then I should see "Joao Silva wants to create enterprise My Enterprise." And the first mail is to admin_user@example.com And I choose "Reject" diff --git a/features/step_definitions/noosfero_steps.rb b/features/step_definitions/noosfero_steps.rb index 1a51376..5c938d3 100644 --- a/features/step_definitions/noosfero_steps.rb +++ b/features/step_definitions/noosfero_steps.rb @@ -16,13 +16,13 @@ Given /^"(.+)" is (online|offline|busy) in chat$/ do |user, status| User.find_by_login(user).update_attributes(:chat_status => status, :chat_status_at => DateTime.now) end -Given /^the following (community|communities|enterprises?)$/ do |kind,table| +Given /^the following (community|communities|enterprises?|organizations?)$/ do |kind,table| klass = kind.singularize.camelize.constantize table.hashes.each do |row| owner = row.delete("owner") - community = klass.create!(row) + organization = klass.create!(row) if owner - community.add_admin(Profile[owner]) + organization.add_admin(Profile[owner]) end end end @@ -210,6 +210,12 @@ Given /^"(.+)" is admin of "(.+)"$/ do |person, organization| org.add_admin(user) end +Then /^"(.+)" should be admin of "(.+)"$/ do |person, organization| + org = Organization.find_by_name(organization) + user = Person.find_by_name(person) + org.admins.should include(user) +end + Given /^"([^\"]*)" has no articles$/ do |profile| (Profile[profile] || Profile.find_by_name(profile)).articles.delete_all end @@ -317,3 +323,9 @@ Given /^the following comments?$/ do |table| comment.save! end end + +Given /^the community "(.+)" is closed$/ do |community| + community = Community.find_by_name(community) + community.closed = true + community.save +end diff --git a/features/support/paths.rb b/features/support/paths.rb index 99e1eb2..8cbdcfe 100644 --- a/features/support/paths.rb +++ b/features/support/paths.rb @@ -36,6 +36,12 @@ module NavigationHelpers when /^the profile$/ '/profile/%s' % User.find_by_id(session[:user]).login + when /^(.*)'s join page/ + '/profile/%s/join' % Profile.find_by_name($1).identifier + + when /^(.*)'s leave page/ + '/profile/%s/leave' % Profile.find_by_name($1).identifier + when /^login page$/ '/account/login' diff --git a/public/javascripts/add-and-join.js b/public/javascripts/add-and-join.js index ddc4ee5..9055491 100644 --- a/public/javascripts/add-and-join.js +++ b/public/javascripts/add-and-join.js @@ -23,8 +23,8 @@ jQuery(function($) { }); clicked.css("cursor",""); $(".small-loading").remove(); - display_notice(data); - }); + display_notice(data.message); + }, "json"); return false; }) @@ -33,15 +33,24 @@ jQuery(function($) { url = clicked.attr("href"); loading_for_button(this); $.post(url, function(data){ - clicked.fadeOut(function(){ - clicked.css("display","none"); - clicked.parent().parent().find(".join-community").fadeIn(); - clicked.parent().parent().find(".join-community").css("display", ""); - }); - clicked.css("cursor",""); - $(".small-loading").remove(); - display_notice(data); - }); + if(data.redirect_to){ + document.location.href = data.redirect_to; + } + else if(data.reload){ + document.location.reload(true); + } + else{ + clicked.fadeOut(function(){ + clicked.css("display","none"); + clicked.parent().parent().find(".join-community").fadeIn(); + clicked.parent().parent().find(".join-community").css("display", ""); + }); + clicked.css("cursor",""); + $(".small-loading").remove(); + + display_notice(data.message); + } + }, "json"); return false; }) diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 4e0f8ae..7d30b85 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -1566,7 +1566,7 @@ a.comment-picture { a.button, a.button:visited, body.noosfero a.button, body.noosfero a.button:visited, input.button { - margin: 0px 2px; + margin: 0px 2px 0px 0px; background-repeat: no-repeat; background-position: 50% 50%; padding: 3px 0px 3px 20px; @@ -4094,6 +4094,15 @@ h1#agenda-title { width: 100%; } +#last-admin-message { + background-color: #CCC; + color: #000; + font-size: 14px; + padding: 20px 15px; + -moz-border-radius:10px; + -webkit-border-radius:10px; +} + /* ==> public/stylesheets/controller_search.css <== */ /* @import url(pagination.css); ALREADY INCLUDED ABOVE */ diff --git a/test/functional/content_viewer_controller_test.rb b/test/functional/content_viewer_controller_test.rb index bbba9f0..ccfa9a6 100644 --- a/test/functional/content_viewer_controller_test.rb +++ b/test/functional/content_viewer_controller_test.rb @@ -308,14 +308,16 @@ class ContentViewerControllerTest < Test::Unit::TestCase end should 'not show private content to members' do - community = Community.create!(:name => 'testcomm') - Folder.create!(:name => 'test', :profile => community, :published => false) - community.add_member(profile) + community = fast_create(Community) + admin = fast_create(Person) + community.add_member(admin) + folder = fast_create(Folder, :profile_id => community.id, :published => false) + community.add_member(profile) login_as(profile.identifier) @request.stubs(:ssl?).returns(true) - get :view_page, :profile => community.identifier, :page => [ 'test' ] + get :view_page, :profile => community.identifier, :page => [ folder.path ] assert_template 'access_denied.rhtml' end diff --git a/test/functional/memberships_controller_test.rb b/test/functional/memberships_controller_test.rb index 140ee01..8f990c2 100644 --- a/test/functional/memberships_controller_test.rb +++ b/test/functional/memberships_controller_test.rb @@ -95,11 +95,11 @@ class MembershipsControllerTest < Test::Unit::TestCase assert_no_tag :tag => 'li', :content => /Description:/ end - should 'show link to leave from community' do + should 'show link to leave from community with reload' do community = Community.create!(:name => 'my test community', :description => 'description test') community.add_member(profile) get :index, :profile => profile.identifier - assert_tag :tag => 'a', :attributes => { :href => "/profile/#{community.identifier}/leave" }, :content => 'Leave' + assert_tag :tag => 'a', :attributes => { :href => "/profile/#{community.identifier}/leave?reload=true" }, :content => 'Leave' end should 'current user is added as admin after create new community' do @@ -127,7 +127,9 @@ class MembershipsControllerTest < Test::Unit::TestCase end should 'not display destroy link to normal members' do - community = Community.create!(:name => 'A community to destroy') + community = fast_create(Community) + admin = fast_create(Person) + community.add_member(admin) person = Person['testuser'] community.add_member(person) diff --git a/test/functional/profile_controller_test.rb b/test/functional/profile_controller_test.rb index e287bbe..13ba103 100644 --- a/test/functional/profile_controller_test.rb +++ b/test/functional/profile_controller_test.rb @@ -383,14 +383,28 @@ class ProfileControllerTest < Test::Unit::TestCase assert profile.memberships.include?(community), 'profile should be actually added to the community' end - should 'create task when join to closed organization' do - community = Community.create!(:name => 'my test community', :closed => true) - login_as @profile.identifier + should 'create task when join to closed organization with members' do + community = fast_create(Community) + community.update_attribute(:closed, true) + admin = fast_create(Person) + community.add_member(admin) + + login_as profile.identifier assert_difference AddMember, :count do post :join, :profile => community.identifier end end + should 'not create task when join to closed and empty organization' do + community = fast_create(Community) + community.update_attribute(:closed, true) + + login_as profile.identifier + assert_no_difference AddMember, :count do + post :join, :profile => community.identifier + end + end + should 'require login to join community' do community = Community.create!(:name => 'my test community', :closed => true) get :join, :profile => community.identifier @@ -399,7 +413,10 @@ class ProfileControllerTest < Test::Unit::TestCase end should 'actually leave profile' do - community = Community.create!(:name => 'my test community') + community = fast_create(Community) + admin = fast_create(Person) + community.add_member(admin) + community.add_member(profile) assert_includes profile.memberships, community @@ -417,6 +434,21 @@ class ProfileControllerTest < Test::Unit::TestCase assert_redirected_to :controller => 'account', :action => 'login' end + should 'not leave if is last admin' do + community = fast_create(Community) + + community.add_admin(profile) + assert_includes profile.memberships, community + + login_as(profile.identifier) + post :leave, :profile => community.identifier + + profile.reload + assert_response :success + assert_match(/last_admin/, @response.body) + assert_includes profile.memberships, community + end + should 'store location before login when request join via get not logged' do community = Community.create!(:name => 'my test community') diff --git a/test/functional/profile_members_controller_test.rb b/test/functional/profile_members_controller_test.rb index 3a7b82e..dc85aae 100644 --- a/test/functional/profile_members_controller_test.rb +++ b/test/functional/profile_members_controller_test.rb @@ -236,22 +236,42 @@ class ProfileMembersControllerTest < Test::Unit::TestCase should 'find users' do ent = fast_create(Enterprise, :name => 'Test Ent', :identifier => 'test_ent') user = create_user_full('test_user').person - u = create_user_with_permission('ent_user', 'manage_memberships', ent) + person = create_user_with_permission('ent_user', 'manage_memberships', ent) login_as :ent_user - get :find_users, :profile => ent.identifier, :query => 'test*' + get :find_users, :profile => ent.identifier, :query => 'test*', :scope => 'all_users' assert_includes assigns(:users_found), user end - should 'not appear add button for member in add members page' do + should 'not display members when finding users in all_users scope' do ent = fast_create(Enterprise, :name => 'Test Ent', :identifier => 'test_ent') - p = create_user_with_permission('test_user', 'manage_memberships', ent) - login_as :test_user + user = create_user_full('test_user').person + + person = create_user_with_permission('ent_user', 'manage_memberships', ent) + login_as :ent_user + + get :find_users, :profile => ent.identifier, :query => '*user', :scope => 'all_users' + + assert_tag :tag => 'a', :content => /#{user.name}/ + assert_no_tag :tag => 'a', :content => /#{person.name}/ + end - get :find_users, :profile => ent.identifier, :query => 'test*' + should 'not display admins when finding users in new_admins scope' do + ent = fast_create(Enterprise, :name => 'Test Ent', :identifier => 'test_ent') + + person = create_user('admin_user').person + ent.add_admin(person) + + user = create_user_full('test_user').person + ent.add_member(user).finish - assert_tag :tag => 'tr', :attributes => {:id => 'tr-test_user', :style => 'display:none'} + login_as :admin_user + + get :find_users, :profile => ent.identifier, :query => '*user', :scope => 'new_admins' + + assert_tag :tag => 'a', :content => /#{user.name}/ + assert_no_tag :tag => 'a', :content => /#{person.name}/ end should 'return users with as a prefix' do @@ -259,10 +279,10 @@ class ProfileMembersControllerTest < Test::Unit::TestCase daniela = create_user_full('daniela').person ent = fast_create(Enterprise, :name => 'Test Ent', :identifier => 'test_ent') - p = create_user_with_permission('test_user', 'manage_memberships', ent) + person = create_user_with_permission('test_user', 'manage_memberships', ent) login_as :test_user - get :find_users, :profile => ent.identifier, :query => 'daniel' + get :find_users, :profile => ent.identifier, :query => 'daniel', :scope => 'all_users' assert_includes assigns(:users_found), daniel assert_includes assigns(:users_found), daniela @@ -307,4 +327,32 @@ class ProfileMembersControllerTest < Test::Unit::TestCase assert_equal Profile['profile_admin_user'], assigns(:mailing).person end + should 'set a community member as admin' do + community = fast_create(Community) + admin = create_user_with_permission('admin_user', 'manage_memberships', community) + member = create_user('test_member').person + community.add_member(member) + + assert_not_includes community.admins, member + + login_as :admin_user + get :add_admin, :profile => community.identifier, :id => member.identifier + + assert_includes community.admins, member + end + + should 'remove a community admin' do + community = fast_create(Community) + admin = create_user_with_permission('admin_user', 'manage_memberships', community) + member = create_user('test_member').person + community.add_admin(member) + + assert_includes community.admins, member + + login_as :admin_user + get :remove_admin, :profile => community.identifier, :id => member.identifier + + assert_not_includes community.admins, member + end + end diff --git a/test/unit/application_helper_test.rb b/test/unit/application_helper_test.rb index 42b76d9..2a768f5 100644 --- a/test/unit/application_helper_test.rb +++ b/test/unit/application_helper_test.rb @@ -96,11 +96,20 @@ class ApplicationHelperTest < Test::Unit::TestCase assert_equal 'black', role_color('none', Environment.default.id) end - should 'rolename for' do + should 'rolename for first organization member' do person = create_user('usertest').person community = fast_create(Community, :name => 'new community', :identifier => 'new-community', :environment_id => Environment.default.id) community.add_member(person) - assert_equal 'Profile Member', rolename_for(person, community) + assert_equal 'Profile Administrator', rolename_for(person, community) + end + + should 'rolename for a member' do + member1 = create_user('usertest1').person + member2 = create_user('usertest2').person + community = fast_create(Community, :name => 'new community', :identifier => 'new-community', :environment_id => Environment.default.id) + community.add_member(member1) + community.add_member(member2) + assert_equal 'Profile Member', rolename_for(member2, community) end should 'get theme from environment by default' do diff --git a/test/unit/community_test.rb b/test/unit/community_test.rb index 437a7f0..9588ca6 100644 --- a/test/unit/community_test.rb +++ b/test/unit/community_test.rb @@ -3,7 +3,7 @@ require File.dirname(__FILE__) + '/../test_helper' class CommunityTest < Test::Unit::TestCase def setup - @person = create_user('testuser').person + @person = fast_create(Person) end attr_reader :person @@ -183,10 +183,33 @@ class CommunityTest < Test::Unit::TestCase end end + should 'set as member without task if organization is closed and has no members' do + community = fast_create(Community) + community.closed = true + community.save + + assert_no_difference AddMember, :count do + community.add_member(person) + end + assert person.is_member_of?(community) + end + + should 'set as member without task if organization is not closed and has no members' do + community = fast_create(Community) + + assert_no_difference AddMember, :count do + community.add_member(person) + end + assert person.is_member_of?(community) + end + should 'not create new request membership if it already exists' do community = fast_create(Community) community.closed = true community.save + + community.add_member(fast_create(Person)) + assert_difference AddMember, :count do community.add_member(person) end diff --git a/test/unit/enterprise_test.rb b/test/unit/enterprise_test.rb index edca779..9d205fa 100644 --- a/test/unit/enterprise_test.rb +++ b/test/unit/enterprise_test.rb @@ -102,14 +102,24 @@ class EnterpriseTest < Test::Unit::TestCase assert_not_includes result, ent2 end + should 'allow to add new members if has no members' do + enterprise = fast_create(Enterprise) + + person = fast_create(Person) + enterprise.add_member(person) + + assert person.is_member_of?(enterprise) + end + should 'not allow to add new members' do - o = fast_create(Enterprise, :name => 'my test profile', :identifier => 'mytestprofile') - p = create_user('mytestuser').person + enterprise = fast_create(Enterprise) + member = fast_create(Person) + enterprise.add_member(member) - o.add_member(p) - o.reload + person = fast_create(Person) + enterprise.add_member(person) - assert_not_includes o.members, p + assert_equal false, person.is_member_of?(enterprise) end should 'allow to remove members' do diff --git a/test/unit/person_test.rb b/test/unit/person_test.rb index 5c64349..a798764 100644 --- a/test/unit/person_test.rb +++ b/test/unit/person_test.rb @@ -394,12 +394,16 @@ class PersonTest < Test::Unit::TestCase end should 'not allow simple member to view group pending tasks' do - c = fast_create(Community) - c.tasks << Task.new - p = create_user('user_without_tasks').person - c.add_member(p) + community = fast_create(Community) + member = fast_create(Person) + community.add_member(member) + + community.tasks << Task.new - assert_not_includes Person.with_pending_tasks, p + person = fast_create(Person) + community.add_member(person) + + assert_not_includes Person.with_pending_tasks, person end should 'person has organization pending tasks' do @@ -1116,4 +1120,29 @@ class PersonTest < Test::Unit::TestCase assert person.receives_scrap_notification? end + should 'check if person is the only admin' do + person = fast_create(Person) + organization = fast_create(Organization) + organization.add_admin(person) + + assert person.is_last_admin?(organization) + end + + should 'check if person is the last admin leaving the community' do + person = fast_create(Person) + organization = fast_create(Organization) + organization.add_admin(person) + + assert person.is_last_admin_leaving?(organization, []) + assert !person.is_last_admin_leaving?(organization, [Role.find_by_key('profile_admin')]) + end + + should 'return unique members of a community' do + person = fast_create(Person) + community = fast_create(Community) + community.add_member(person) + + assert_equal [person], Person.members_of(community) + assert_equal 1, Person.members_of(community).count + end end diff --git a/test/unit/profile_test.rb b/test/unit/profile_test.rb index 5e2947e..cb67532 100644 --- a/test/unit/profile_test.rb +++ b/test/unit/profile_test.rb @@ -1406,7 +1406,12 @@ class ProfileTest < Test::Unit::TestCase should 'provide URL to leave' do profile = build(Profile, :identifier => 'testprofile') - assert_equal({ :profile => 'testprofile', :controller => 'profile', :action => 'leave'}, profile.leave_url) + assert_equal({ :profile => 'testprofile', :controller => 'profile', :action => 'leave', :reload => false}, profile.leave_url) + end + + should 'provide URL to leave with reload' do + profile = build(Profile, :identifier => 'testprofile') + assert_equal({ :profile => 'testprofile', :controller => 'profile', :action => 'leave', :reload => true}, profile.leave_url(true)) end should 'provide URL to join' do @@ -1743,26 +1748,25 @@ class ProfileTest < Test::Unit::TestCase end should "return one member on label if the profile has one member" do - p = fast_create(Person) - c = fast_create(Community) - c.add_member(p) - assert_equal 1, c.members.count - assert_equal "one member", c.more_popular_label + person = fast_create(Person) + community = fast_create(Community) + community.add_member(person) + + assert_equal "one member", community.more_popular_label end should "return the number of members on label if the profile has more than one member" do - p1 = fast_create(Person) - p2 = fast_create(Person) - c = fast_create(Community) - c.add_member(p1) - c.add_member(p2) - assert_equal 2, c.members.count - assert_equal "2 members", c.more_popular_label + person1 = fast_create(Person) + person2 = fast_create(Person) + community = fast_create(Community) - p3 = fast_create(Person) - c.add_member(p3) - assert_equal 3, c.members.count - assert_equal "3 members", c.more_popular_label + community.add_member(person1) + community.add_member(person2) + assert_equal "2 members", community.more_popular_label + + person3 = fast_create(Person) + community.add_member(person3) + assert_equal "3 members", community.more_popular_label end should 'provide list of galleries' do -- libgit2 0.21.2