Commit 15e5277799f3f24222abb18a3629542b5d747367
Exists in
staging
and in
4 other branches
Merge branch 'allow_admin_change_homepage' into stable
Showing
6 changed files
with
140 additions
and
2 deletions
Show diff stats
@@ -0,0 +1,104 @@ | @@ -0,0 +1,104 @@ | ||
1 | +# Noosfero Development Policy | ||
2 | + | ||
3 | +## Developer Roles | ||
4 | + | ||
5 | +* *Developers* are everyone that is contributing code to Noosfero. | ||
6 | +* *Committers* are the people with direct commit access to the Noosfero source | ||
7 | + code. They are responsible for reviewing contributions from other developers | ||
8 | + and integrating them in the Noosfero code base. They are the members of the | ||
9 | + [Noosfero group on Gitlab](https://gitlab.com/groups/noosfero/members). | ||
10 | +* *Release managers* are the people that are managing the release of a new | ||
11 | + Noosfero version and/or the maintainance work of an existing Noosfero stable | ||
12 | + branch. See MAINTAINANCE.md for details on the maintaince policy. | ||
13 | + | ||
14 | +## Development process | ||
15 | + | ||
16 | +* Every new feature or non-trivial bugfix should be reviewed by at least one | ||
17 | + committer. This must be the case even if the original author is a committer. | ||
18 | + | ||
19 | + * In the case the original author is a committer, he/she should feel free to | ||
20 | + commit directly if after 1 week nobody has provided any kind of feedback. | ||
21 | + | ||
22 | +* Committers should feel free to push trivial (or urgent) changes directly. | ||
23 | + There are no strict rule on what makes a change trivial or urgent; committers | ||
24 | + are expected to exercise good judgement on a case by case basis. | ||
25 | + | ||
26 | + * Usually changes to the database are not trivial. | ||
27 | + | ||
28 | +* In the case of unsolvable conflict between commiters regarding any change to | ||
29 | + the code, the current release manager(s) will have the final say in the | ||
30 | + matter. | ||
31 | + | ||
32 | +* Release managers are responsible for stablishing a release schedule, and | ||
33 | + about deciding when and what to release. | ||
34 | + | ||
35 | + * Release managers should announce release schedules to the project mailing | ||
36 | + lists in advance. | ||
37 | + | ||
38 | + * The release schedule may include a period of feature freeze, during which | ||
39 | + no new features or any other changes that are not pre-approved by the | ||
40 | + release manager must be committed to the repository. | ||
41 | + | ||
42 | + * Committers must respect the release schedule and feature freezes. | ||
43 | + | ||
44 | +## Maintainance process | ||
45 | + | ||
46 | +### Not all feature releases will be maintained as a stable release | ||
47 | + | ||
48 | +We will be choosing specific release series to be maintained as stable | ||
49 | +releases. | ||
50 | + | ||
51 | +This means that a given release is not guaranteed to be maintained as a stable | ||
52 | +release, but does *not* mean it won't be. Any committer (or anyone, really) can | ||
53 | +decide to maintain a given release as stable and seek help from others to do | ||
54 | +so. | ||
55 | + | ||
56 | +### No merges from stable branches to master | ||
57 | + | ||
58 | +*All* changes must be submitted against the master branch first, and when | ||
59 | +applicable, backported to the desired stable releases. Exceptions to this rules | ||
60 | +are bug fixes that only apply to a given stable branch and not to master. | ||
61 | + | ||
62 | +In the past we had non-trivial changes accepted into stable releases while | ||
63 | +master was way ahead (e.g. during the rails3 migration period), that made the | ||
64 | +merge back into master very painful. By eliminating the need to do these | ||
65 | +merges, we save time for the people responsible for the release, and eliminate | ||
66 | +the possibility of human errors or oversights causing changes to be accepted | ||
67 | +into stable that will be a problem to merge back into master. | ||
68 | + | ||
69 | +By getting all fixes in master first, we improve the chances that a future | ||
70 | +release will not present regressions against bugs that should already be fixed, | ||
71 | +but the fixes got lost in a big, complicated merge (and those won't exist | ||
72 | +anymore, at least not from stable branches to master). | ||
73 | + | ||
74 | +After a fix gets into master, backporting changes into a stable release branch | ||
75 | +is the responsibility of whoever is maintaing that branch, and those interested | ||
76 | +in it. The stable branch release manager(s) are entitled the final say on any | ||
77 | +matters related to that branch. | ||
78 | + | ||
79 | +## Apendix A: how to become a committer | ||
80 | + | ||
81 | +Every developer that wants to be a committer should create [an issue on | ||
82 | +Gitlab](https://gitlab.com/noosfero/noosfero/issues) requesting to be added as | ||
83 | +a committer. This request must include information about the requestor's | ||
84 | +previous contributions to the project. | ||
85 | + | ||
86 | +If 2 or more commiters consider second the request, the requestor is accepted | ||
87 | +as new commiter and added to the Noosfero group. | ||
88 | + | ||
89 | +The existing committers are free to choose whatever criteria they want to | ||
90 | +second the request, but they must be sure that the new committer is a | ||
91 | +responsible developer and knows what she/he is doing. They must be aware that | ||
92 | +seconding these requests means seconding the actions of the new committer: if | ||
93 | +the new committer screw up, her/his seconds screwed up. | ||
94 | + | ||
95 | +## Apendix B: how to become a release manager | ||
96 | + | ||
97 | +A new release manager for the development version of Noosfero (i.e. the one | ||
98 | +that includes new features, a.k.a. the master branch) is apointed by the | ||
99 | +current release manager, and must be a committer first. | ||
100 | + | ||
101 | +Release managers for stable branches are self-appointed, i.e. whoever takes the | ||
102 | +work takes the role. In case of a conflict (e.g. 2+ different people want to do | ||
103 | +the work but can't agree on working together), the development release manager | ||
104 | +decides. |
app/controllers/my_profile/cms_controller.rb
@@ -174,6 +174,8 @@ class CmsController < MyProfileController | @@ -174,6 +174,8 @@ class CmsController < MyProfileController | ||
174 | 174 | ||
175 | post_only :set_home_page | 175 | post_only :set_home_page |
176 | def set_home_page | 176 | def set_home_page |
177 | + return render_access_denied unless user.can_change_homepage? | ||
178 | + | ||
177 | article = params[:id].nil? ? nil : profile.articles.find(params[:id]) | 179 | article = params[:id].nil? ? nil : profile.articles.find(params[:id]) |
178 | profile.update_attribute(:home_page, article) | 180 | profile.update_attribute(:home_page, article) |
179 | 181 |
app/models/person.rb
@@ -74,6 +74,10 @@ class Person < Profile | @@ -74,6 +74,10 @@ class Person < Profile | ||
74 | 74 | ||
75 | belongs_to :user, :dependent => :delete | 75 | belongs_to :user, :dependent => :delete |
76 | 76 | ||
77 | + def can_change_homepage? | ||
78 | + !environment.enabled?('cant_change_homepage') || is_admin? | ||
79 | + end | ||
80 | + | ||
77 | def can_control_scrap?(scrap) | 81 | def can_control_scrap?(scrap) |
78 | begin | 82 | begin |
79 | !self.scraps(scrap).nil? | 83 | !self.scraps(scrap).nil? |
app/views/cms/view.html.erb
@@ -2,7 +2,7 @@ | @@ -2,7 +2,7 @@ | ||
2 | <%= _('Content management') %> | 2 | <%= _('Content management') %> |
3 | </h1> | 3 | </h1> |
4 | 4 | ||
5 | -<% if !environment.enabled?('cant_change_homepage') && !remove_content_button(:home) %> | 5 | +<% if user.can_change_homepage? && !remove_content_button(:home) %> |
6 | <div class="cms-homepage"> | 6 | <div class="cms-homepage"> |
7 | <%= _('Profile homepage:') %> | 7 | <%= _('Profile homepage:') %> |
8 | <% if profile.home_page %> | 8 | <% if profile.home_page %> |
@@ -69,7 +69,7 @@ | @@ -69,7 +69,7 @@ | ||
69 | <%= expirable_button article, :edit, _('Edit'), {:action => 'edit', :id => article.id} if !remove_content_button(:edit) %> | 69 | <%= expirable_button article, :edit, _('Edit'), {:action => 'edit', :id => article.id} if !remove_content_button(:edit) %> |
70 | <%= button_without_text :eyes, _('Public view'), article.view_url %> | 70 | <%= button_without_text :eyes, _('Public view'), article.view_url %> |
71 | <%= display_spread_button(profile, article) unless article.folder? || remove_content_button(:spread)%> | 71 | <%= display_spread_button(profile, article) unless article.folder? || remove_content_button(:spread)%> |
72 | - <% if !environment.enabled?('cant_change_homepage') && !remove_content_button(:home) %> | 72 | + <% if user.can_change_homepage? && !remove_content_button(:home) %> |
73 | <% if profile.home_page != article %> | 73 | <% if profile.home_page != article %> |
74 | <%= expirable_button article, :home, _('Use as homepage'), { :action => 'set_home_page', :id => article.id }, :method => :post %> | 74 | <%= expirable_button article, :home, _('Use as homepage'), { :action => 'set_home_page', :id => article.id }, :method => :post %> |
75 | <% else %> | 75 | <% else %> |
test/functional/cms_controller_test.rb
@@ -101,12 +101,26 @@ class CmsControllerTest < ActionController::TestCase | @@ -101,12 +101,26 @@ class CmsControllerTest < ActionController::TestCase | ||
101 | assert_tag :tag => 'div', :content => /Profile homepage/, :attributes => { :class => "cms-homepage"} | 101 | assert_tag :tag => 'div', :content => /Profile homepage/, :attributes => { :class => "cms-homepage"} |
102 | end | 102 | end |
103 | 103 | ||
104 | + should 'display the profile homepage if logged user is an environment admin' do | ||
105 | + env = Environment.default; env.enable('cant_change_homepage'); env.save! | ||
106 | + env.add_admin(profile) | ||
107 | + get :index, :profile => profile.identifier | ||
108 | + assert_tag :tag => 'div', :content => /Profile homepage/, :attributes => { :class => "cms-homepage"} | ||
109 | + end | ||
110 | + | ||
104 | should 'not display the profile homepage if cannot change homepage' do | 111 | should 'not display the profile homepage if cannot change homepage' do |
105 | env = Environment.default; env.enable('cant_change_homepage') | 112 | env = Environment.default; env.enable('cant_change_homepage') |
106 | get :index, :profile => profile.identifier | 113 | get :index, :profile => profile.identifier |
107 | assert_no_tag :tag => 'div', :content => /Profile homepage/, :attributes => { :class => "cms-homepage"} | 114 | assert_no_tag :tag => 'div', :content => /Profile homepage/, :attributes => { :class => "cms-homepage"} |
108 | end | 115 | end |
109 | 116 | ||
117 | + should 'not allow profile homepage changes if cannot change homepage' do | ||
118 | + env = Environment.default; env.enable('cant_change_homepage') | ||
119 | + a = profile.articles.create!(:name => 'my new home page') | ||
120 | + post :set_home_page, :profile => profile.identifier, :id => a.id | ||
121 | + assert_response 403 | ||
122 | + end | ||
123 | + | ||
110 | should 'be able to set home page' do | 124 | should 'be able to set home page' do |
111 | a = profile.articles.build(:name => 'my new home page') | 125 | a = profile.articles.build(:name => 'my new home page') |
112 | a.save! | 126 | a.save! |
test/unit/person_test.rb
@@ -1470,4 +1470,18 @@ class PersonTest < ActiveSupport::TestCase | @@ -1470,4 +1470,18 @@ class PersonTest < ActiveSupport::TestCase | ||
1470 | person.reload | 1470 | person.reload |
1471 | end | 1471 | end |
1472 | end | 1472 | end |
1473 | + | ||
1474 | + should 'allow homepage change if user is an environment admin' do | ||
1475 | + person = create_user('person').person | ||
1476 | + person.environment.expects(:enabled?).with('cant_change_homepage').returns(true) | ||
1477 | + person.expects(:is_admin?).returns(true) | ||
1478 | + assert person.can_change_homepage? | ||
1479 | + end | ||
1480 | + | ||
1481 | + should 'allow homepage change if environment feature permit it' do | ||
1482 | + person = create_user('person').person | ||
1483 | + person.environment.expects(:enabled?).with('cant_change_homepage').returns(false) | ||
1484 | + assert person.can_change_homepage? | ||
1485 | + end | ||
1486 | + | ||
1473 | end | 1487 | end |