Commit 48cb8c1a1c7a99baf6214aa72bfb20dabd0ef41d
Exists in
colab
and in
4 other branches
Merge pull request #291 from mezuro/bug_busters
Bug busters
Showing
18 changed files
with
182 additions
and
55 deletions
Show diff stats
app/controllers/projects_controller.rb
... | ... | @@ -13,7 +13,7 @@ class ProjectsController < ApplicationController |
13 | 13 | # GET /projects |
14 | 14 | # GET /projects.json |
15 | 15 | def index |
16 | - @projects = Project.all.select { |project| !project.attributes.hidden } | |
16 | + @projects = Project.public_or_owned_by_user(current_user) | |
17 | 17 | end |
18 | 18 | |
19 | 19 | # POST /projects | ... | ... |
app/models/project.rb
... | ... | @@ -3,18 +3,30 @@ class Project < KalibroClient::Entities::Processor::Project |
3 | 3 | |
4 | 4 | attr_writer :attributes |
5 | 5 | |
6 | + def self.public_or_owned_by_user(user = nil) | |
7 | + project_attributes = ProjectAttributes.where(public: true) | |
8 | + project_attributes += ProjectAttributes.where(user_id: user.id, public: false) if user | |
9 | + | |
10 | + project_attributes.map do |attribute| | |
11 | + begin | |
12 | + self.find(attribute.project_id) | |
13 | + rescue KalibroClient::Errors::RecordNotFound | |
14 | + nil | |
15 | + end | |
16 | + end.compact | |
17 | + end | |
18 | + | |
6 | 19 | def self.latest(count = 1) |
7 | - all.sort { |a,b| b.id <=> a.id }.select { |project| !project.attributes.hidden}.first(count) | |
20 | + all.sort { |a, b| b.id <=> a.id }.select { |project| project.attributes.public }.first(count) | |
8 | 21 | end |
9 | 22 | |
10 | 23 | def attributes |
11 | - @project_attributes ||= ProjectAttributes.find_by_project_id(self.id) | |
12 | - @project_attributes.nil? ? ProjectAttributes.new : @project_attributes | |
24 | + @attributes ||= ProjectAttributes.find_by_project_id(@id) | |
13 | 25 | end |
14 | 26 | |
15 | 27 | def destroy |
16 | - self.attributes.destroy if self.attributes | |
17 | - @project_attributes = nil | |
28 | + self.attributes.destroy unless self.attributes.nil? | |
29 | + @attributes = nil | |
18 | 30 | super |
19 | 31 | end |
20 | 32 | end | ... | ... |
app/models/project_attributes.rb
... | ... | @@ -4,6 +4,11 @@ class ProjectAttributes < ActiveRecord::Base |
4 | 4 | validates :user, presence: true |
5 | 5 | |
6 | 6 | def project |
7 | - Project.find(self.project_id) | |
7 | + @project ||= Project.find(project_id) | |
8 | + end | |
9 | + | |
10 | + def project=(project) | |
11 | + @project = project | |
12 | + self.project_id = project.id | |
8 | 13 | end |
9 | 14 | end | ... | ... |
app/views/modules/_module_tree.html.erb
... | ... | @@ -10,7 +10,8 @@ |
10 | 10 | <strong><%= KalibroModule.human_attribute_name('grade') %>:</strong> |
11 | 11 | <%= format_grade(@root_module_result.grade) %> |
12 | 12 | </p> |
13 | -<% unless @root_module_result.parent_id.nil? %> | |
13 | + | |
14 | +<% unless @root_module_result.parent_id == 0 %> | |
14 | 15 | <p id="parent"><i class="icon-arrow-up"></i><%= link_to '../', '#parent', onClick: "Module.Tree.load('#{escape_javascript(image_tag 'loader.gif')} #{escape_javascript(t('loading_data'))}', #{@root_module_result.parent_id});" %></p> |
15 | 16 | <% end %> |
16 | 17 | ... | ... |
config/deploy.rb
db/migrate/20151106182639_change_project_attributes_public_default.rb
0 → 100644
... | ... | @@ -0,0 +1,15 @@ |
1 | +class ChangeProjectAttributesPublicDefault < ActiveRecord::Migration | |
2 | + def up | |
3 | + rename_column :project_attributes, :hidden, :public | |
4 | + change_column_default :project_attributes, :public, true | |
5 | + | |
6 | + ProjectAttributes.all.update_all('public = NOT public') | |
7 | + end | |
8 | + | |
9 | + def down | |
10 | + rename_column :project_attributes, :public, :hidden | |
11 | + change_column_default :project_attributes, :hidden, false | |
12 | + | |
13 | + ProjectAttributes.all.update_all('hidden = NOT hidden') | |
14 | + end | |
15 | +end | ... | ... |
db/schema.rb
... | ... | @@ -11,7 +11,10 @@ |
11 | 11 | # |
12 | 12 | # It's strongly recommended that you check this file into your version control system. |
13 | 13 | |
14 | -ActiveRecord::Schema.define(version: 20150616164352) do | |
14 | +ActiveRecord::Schema.define(version: 20151106182639) do | |
15 | + | |
16 | + # These are extensions that must be enabled in order to support this database | |
17 | + enable_extension "plpgsql" | |
15 | 18 | |
16 | 19 | create_table "kalibro_configuration_attributes", force: :cascade do |t| |
17 | 20 | t.integer "user_id" |
... | ... | @@ -25,9 +28,9 @@ ActiveRecord::Schema.define(version: 20150616164352) do |
25 | 28 | t.integer "project_id" |
26 | 29 | t.string "image_url" |
27 | 30 | t.integer "user_id" |
28 | - t.boolean "hidden", default: false | |
29 | - t.datetime "created_at", null: false | |
30 | - t.datetime "updated_at", null: false | |
31 | + t.boolean "public", default: true | |
32 | + t.datetime "created_at", null: false | |
33 | + t.datetime "updated_at", null: false | |
31 | 34 | end |
32 | 35 | |
33 | 36 | create_table "reading_group_attributes", force: :cascade do |t| |
... | ... | @@ -45,7 +48,7 @@ ActiveRecord::Schema.define(version: 20150616164352) do |
45 | 48 | t.datetime "updated_at", null: false |
46 | 49 | end |
47 | 50 | |
48 | - add_index "repository_attributes", ["user_id"], name: "index_repository_attributes_on_user_id" | |
51 | + add_index "repository_attributes", ["user_id"], name: "index_repository_attributes_on_user_id", using: :btree | |
49 | 52 | |
50 | 53 | create_table "users", force: :cascade do |t| |
51 | 54 | t.string "name", limit: 255, default: "", null: false |
... | ... | @@ -63,7 +66,7 @@ ActiveRecord::Schema.define(version: 20150616164352) do |
63 | 66 | t.string "last_sign_in_ip", limit: 255 |
64 | 67 | end |
65 | 68 | |
66 | - add_index "users", ["email"], name: "index_users_on_email", unique: true | |
67 | - add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true | |
69 | + add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree | |
70 | + add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree | |
68 | 71 | |
69 | 72 | end | ... | ... |
features/project/deletion.feature
... | ... | @@ -22,7 +22,6 @@ Feature: Project Deletion |
22 | 22 | Given I am a regular user |
23 | 23 | And I am signed in |
24 | 24 | And I own a sample project |
25 | - And I have sample project_attributes | |
26 | 25 | And I am at the Sample Project page |
27 | 26 | When I click the Destroy Project link |
28 | 27 | Then I should be in the All Projects page | ... | ... |
features/project/edition.feature
... | ... | @@ -8,13 +8,12 @@ Feature: Project |
8 | 8 | Given I am a regular user |
9 | 9 | And I am signed in |
10 | 10 | And I own a sample project |
11 | - And I have sample project_attributes | |
12 | 11 | And I am at the All Projects page |
13 | 12 | When I click the Edit link |
14 | 13 | Then I should be in the Edit Project page |
15 | 14 | |
16 | 15 | @kalibro_processor_restart |
17 | - Scenario: Should not show edit links from projects that doesn't belongs to me | |
16 | + Scenario: Should not show edit links from projects that doesn't belong to me | |
18 | 17 | Given I am a regular user |
19 | 18 | And I am signed in |
20 | 19 | And I have a sample project |
... | ... | @@ -22,7 +21,7 @@ Feature: Project |
22 | 21 | Then I should not see Edit within table |
23 | 22 | |
24 | 23 | @kalibro_processor_restart |
25 | - Scenario: Should not render the edit page if the project doesn't belongs to the current user | |
24 | + Scenario: Should not render the edit page if the project doesn't belong to the current user | |
26 | 25 | Given I am a regular user |
27 | 26 | And I am signed in |
28 | 27 | And I have a sample project |
... | ... | @@ -35,7 +34,6 @@ Feature: Project |
35 | 34 | Given I am a regular user |
36 | 35 | And I am signed in |
37 | 36 | And I own a sample project |
38 | - And I have sample project_attributes | |
39 | 37 | And I am at the All Projects page |
40 | 38 | When I click the Edit link |
41 | 39 | Then The field "project[name]" should be filled with the sample project "name" |
... | ... | @@ -46,7 +44,6 @@ Feature: Project |
46 | 44 | Given I am a regular user |
47 | 45 | And I am signed in |
48 | 46 | And I own a sample project |
49 | - And I have sample project_attributes | |
50 | 47 | And I am at the sample project edit page |
51 | 48 | And I fill the Name field with "Kalibro" |
52 | 49 | And I fill the Description field with "Web Service to collect metrics" |
... | ... | @@ -61,7 +58,6 @@ Feature: Project |
61 | 58 | And I have a project named "Qt-Calculator" |
62 | 59 | And I own a project named "Kalibro" |
63 | 60 | And I am at the sample project edit page |
64 | - And I have sample project_attributes | |
65 | 61 | And I fill the Name field with "Qt-Calculator" |
66 | 62 | When I press the Save button |
67 | 63 | Then I should see "Name has already been taken" |
... | ... | @@ -71,7 +67,6 @@ Feature: Project |
71 | 67 | Given I am a regular user |
72 | 68 | And I am signed in |
73 | 69 | And I own a sample project |
74 | - And I have sample project_attributes | |
75 | 70 | And I am at the sample project edit page |
76 | 71 | And I fill the Description field with "Web Service to collect metrics" |
77 | 72 | When I press the Save button |
... | ... | @@ -82,7 +77,6 @@ Feature: Project |
82 | 77 | Given I am a regular user |
83 | 78 | And I am signed in |
84 | 79 | And I own a sample project |
85 | - And I have sample project_attributes | |
86 | 80 | And I am at the sample project edit page |
87 | 81 | And I fill the Name field with " " |
88 | 82 | When I press the Save button | ... | ... |
features/project/listing.feature
... | ... | @@ -25,7 +25,6 @@ Feature: Project listing |
25 | 25 | Given I am a regular user |
26 | 26 | And I am signed in |
27 | 27 | And I have a sample project |
28 | - And I have sample project_attributes | |
29 | 28 | And I am at the All Projects page |
30 | 29 | When I click the Show link |
31 | 30 | Then the sample project should be there | ... | ... |
features/project/show.feature
... | ... | @@ -7,7 +7,6 @@ Feature: Show Project |
7 | 7 | Scenario: Should not show the create repository link to user that doesn't own the project |
8 | 8 | Given I am a regular user |
9 | 9 | And I have a sample project |
10 | - And I have sample project_attributes | |
11 | 10 | And I have a sample configuration with native metrics |
12 | 11 | And I have a sample repository within the sample project |
13 | 12 | When I am at the Sample Project page |
... | ... | @@ -20,7 +19,6 @@ Scenario: Should show the create repository link the project owner |
20 | 19 | Given I am a regular user |
21 | 20 | And I am signed in |
22 | 21 | And I own a sample project |
23 | - And I have sample project_attributes | |
24 | 22 | When I am at the Sample Project page |
25 | 23 | Then I should see "New Repository" |
26 | 24 | |
... | ... | @@ -29,7 +27,6 @@ Scenario: Should not show the independent repositories for a project |
29 | 27 | Given I am a regular user |
30 | 28 | And I am signed in |
31 | 29 | And I own a sample project |
32 | - And I have sample project_attributes | |
33 | 30 | And I have a sample configuration |
34 | 31 | And I have a sample repository |
35 | 32 | When I am at the Sample Project page |
... | ... | @@ -39,7 +36,6 @@ Scenario: Should not show the independent repositories for a project |
39 | 36 | Scenario: Considering the project has no repositories |
40 | 37 | Given I am a regular user |
41 | 38 | And I have a sample project |
42 | - And I have sample project_attributes | |
43 | 39 | When I am at the Sample Project page |
44 | 40 | Then I should see "There are no Repositories yet!" |
45 | 41 | |
... | ... | @@ -56,6 +52,5 @@ Scenario: Considering the project has repositories |
56 | 52 | Scenario: Checking project contents |
57 | 53 | Given I am a regular user |
58 | 54 | And I have a sample project |
59 | - And I have sample project_attributes | |
60 | 55 | When I am at the Sample Project page |
61 | 56 | Then the sample project should be there | ... | ... |
features/repository/create.feature
... | ... | @@ -8,7 +8,6 @@ Scenario: repository creation associated with a project |
8 | 8 | Given I am a regular user |
9 | 9 | And I am signed in |
10 | 10 | And I own a sample project |
11 | - And I have sample project_attributes | |
12 | 11 | And I have a sample configuration with native metrics |
13 | 12 | And I am at the New Repository page |
14 | 13 | And I fill the Name field with "Kalibro" | ... | ... |
features/repository/show/modules_tree.feature
... | ... | @@ -17,6 +17,7 @@ Feature: Repository modules tree |
17 | 17 | When I visit the repository show page |
18 | 18 | And I click the "Modules Tree" h3 |
19 | 19 | Then I should see the given module result |
20 | + And I should not see "../" | |
20 | 21 | |
21 | 22 | @kalibro_configuration_restart @kalibro_processor_restart @javascript |
22 | 23 | Scenario: Should show children of root when the process has been finished | ... | ... |
features/step_definitions/project_steps.rb
... | ... | @@ -6,14 +6,12 @@ end |
6 | 6 | |
7 | 7 | Given(/^I have a sample project$/) do |
8 | 8 | @project = FactoryGirl.create(:project) |
9 | -end | |
10 | - | |
11 | -Given(/^I have sample project_attributes$/) do | |
12 | - @project_attributes = FactoryGirl.create(:project_attributes, {id: nil, user_id: @user.id}) | |
9 | + @project.attributes = FactoryGirl.create(:project_attributes, project: @project) | |
13 | 10 | end |
14 | 11 | |
15 | 12 | Given(/^I have a project named "(.*?)"$/) do |name| |
16 | 13 | @project = FactoryGirl.create(:project, {name: name}) |
14 | + @project.attributes = FactoryGirl.create(:project_attributes, project: @project) | |
17 | 15 | end |
18 | 16 | |
19 | 17 | Given(/^I own a sample project$/) do | ... | ... |
spec/controllers/projects_controller_spec.rb
... | ... | @@ -148,12 +148,10 @@ describe ProjectsController, :type => :controller do |
148 | 148 | end |
149 | 149 | |
150 | 150 | describe 'index' do |
151 | - let(:project_attributes) { FactoryGirl.build(:project_attributes) } | |
151 | + subject { FactoryGirl.build(:project_with_id) } | |
152 | 152 | |
153 | 153 | before :each do |
154 | - @subject = FactoryGirl.build(:project_with_id) | |
155 | - Project.expects(:all).returns([@subject]) | |
156 | - @subject.expects(:attributes).returns(project_attributes) | |
154 | + Project.expects(:public_or_owned_by_user).returns([subject]) | |
157 | 155 | get :index |
158 | 156 | end |
159 | 157 | ... | ... |
spec/factories/project_attributes.rb
1 | 1 | FactoryGirl.define do |
2 | 2 | factory :project_attributes, :class => 'ProjectAttributes' do |
3 | - project_id 1 | |
4 | 3 | image_url '' |
5 | - user_id 1 | |
6 | - hidden false | |
4 | + self.public true | |
5 | + association :project, :with_id, strategy: :build | |
6 | + association :user, strategy: :build | |
7 | 7 | |
8 | 8 | trait :with_image do |
9 | 9 | image_url '#' |
10 | 10 | end |
11 | + | |
12 | + trait :private do | |
13 | + self.public false | |
14 | + end | |
15 | + | |
16 | + trait :bare do | |
17 | + project_id nil | |
18 | + user_id nil | |
19 | + end | |
11 | 20 | end |
12 | 21 | end | ... | ... |
spec/models/project_attributes_spec.rb
... | ... | @@ -11,10 +11,9 @@ RSpec.describe ProjectAttributes, type: :model do |
11 | 11 | end |
12 | 12 | |
13 | 13 | describe 'methods' do |
14 | + let(:project) { FactoryGirl.build(:project_with_id) } | |
14 | 15 | describe 'project' do |
15 | - subject { FactoryGirl.build(:project_attributes) } | |
16 | - let(:project) {FactoryGirl.build(:project_with_id)} | |
17 | - | |
16 | + subject { FactoryGirl.build(:project_attributes, :bare, project_id: project.id) } | |
18 | 17 | before :each do |
19 | 18 | Project.expects(:find).with(subject.project_id).returns(project) |
20 | 19 | end |
... | ... | @@ -23,5 +22,21 @@ RSpec.describe ProjectAttributes, type: :model do |
23 | 22 | expect(subject.project).to eq(project) |
24 | 23 | end |
25 | 24 | end |
25 | + | |
26 | + describe 'project=' do | |
27 | + subject { FactoryGirl.build(:project_attributes, :bare) } | |
28 | + | |
29 | + before do | |
30 | + subject.project = project | |
31 | + end | |
32 | + | |
33 | + it 'is expected to set the project' do | |
34 | + expect(subject.project).to eq project | |
35 | + end | |
36 | + | |
37 | + it 'is expected to set the project_id' do | |
38 | + expect(subject.project_id).to eq project.id | |
39 | + end | |
40 | + end | |
26 | 41 | end |
27 | 42 | end | ... | ... |
spec/models/project_spec.rb
... | ... | @@ -28,33 +28,47 @@ describe Project, :type => :model do |
28 | 28 | describe 'attributes' do |
29 | 29 | subject { FactoryGirl.build(:project_with_id) } |
30 | 30 | |
31 | - let!(:project_attributes) { FactoryGirl.build(:project_attributes) } | |
31 | + context 'when there are attributes' do | |
32 | + let!(:project_attributes) { FactoryGirl.build(:project_attributes) } | |
32 | 33 | |
33 | - before :each do | |
34 | - ProjectAttributes.expects(:find_by_project_id).returns(project_attributes) | |
34 | + before :each do | |
35 | + ProjectAttributes.expects(:find_by_project_id).returns(project_attributes) | |
36 | + end | |
37 | + | |
38 | + it 'is expected to return the project attributes' do | |
39 | + expect(subject.attributes).to eq(project_attributes) | |
40 | + end | |
35 | 41 | end |
36 | 42 | |
37 | - it 'is expected to return the project attributes' do | |
38 | - expect(subject.attributes).to eq(project_attributes) | |
43 | + context 'when there are no attributes' do | |
44 | + before :each do | |
45 | + ProjectAttributes.expects(:find_by_project_id).returns(nil) | |
46 | + end | |
47 | + | |
48 | + it 'is expected to return the project attributes' do | |
49 | + expect(subject.attributes).to be_nil | |
50 | + end | |
39 | 51 | end |
40 | 52 | end |
41 | 53 | |
42 | 54 | describe 'destroy' do |
55 | + let!(:project) { FactoryGirl.build(:project) } | |
56 | + | |
43 | 57 | context 'when attributes exist' do |
44 | - let!(:project) { FactoryGirl.build(:project) } | |
45 | 58 | let!(:project_attributes) { FactoryGirl.build(:project_attributes, project_id: project.id) } |
59 | + before :each do | |
60 | + KalibroClient::Entities::Processor::Project.any_instance.expects(:destroy).returns(project) | |
61 | + end | |
46 | 62 | |
47 | - it 'should be destroyed' do | |
63 | + it 'is expected to be destroyed' do | |
48 | 64 | project.expects(:attributes).twice.returns(project_attributes) |
49 | 65 | project_attributes.expects(:destroy) |
50 | - KalibroClient::Entities::Processor::Project.any_instance.expects(:destroy).returns(project) | |
51 | 66 | project.destroy |
52 | 67 | end |
53 | 68 | |
54 | 69 | it 'is expected to clean the attributes memoization' do |
55 | 70 | # Call attributes once so it memoizes |
56 | 71 | ProjectAttributes.expects(:find_by).with(project_id: project.id).returns(project_attributes) |
57 | - KalibroClient::Entities::Processor::Project.any_instance.expects(:destroy).returns(project) | |
58 | 72 | expect(project.attributes).to eq(project_attributes) |
59 | 73 | |
60 | 74 | # Destroying |
... | ... | @@ -65,6 +79,76 @@ describe Project, :type => :model do |
65 | 79 | expect(project.attributes).to_not eq(project_attributes) |
66 | 80 | end |
67 | 81 | end |
82 | + | |
83 | + context 'when attributes is nil' do | |
84 | + before do | |
85 | + project.expects(:attributes).returns(nil) | |
86 | + KalibroClient::Entities::Processor::Project.any_instance.expects(:destroy).returns(project) | |
87 | + end | |
88 | + | |
89 | + it 'is expected to not try to destroy the attributes' do | |
90 | + project.destroy | |
91 | + end | |
92 | + end | |
93 | + end | |
94 | + end | |
95 | + | |
96 | + describe 'class methods' do | |
97 | + describe 'public_or_owned_by_user' do | |
98 | + let!(:user) { FactoryGirl.build(:user, :with_id) } | |
99 | + | |
100 | + let!(:owned_private_attrs) { FactoryGirl.build(:project_attributes, :private, user_id: user.id) } | |
101 | + let!(:owned_public_attrs) { FactoryGirl.build(:project_attributes, user_id: user.id) } | |
102 | + let!(:not_owned_private_attrs) { FactoryGirl.build(:project_attributes, :private, user_id: user.id + 1) } | |
103 | + let!(:not_owned_public_attrs) { FactoryGirl.build(:project_attributes, user_id: user.id + 1) } | |
104 | + | |
105 | + let!(:public_attrs) { [owned_public_attrs, not_owned_public_attrs] } | |
106 | + let(:public_projects) { public_attrs.map(&:project) } | |
107 | + | |
108 | + let!(:owned_or_public_attrs) { public_attrs + [owned_private_attrs] } | |
109 | + let!(:owned_or_public_projects) { owned_or_public_attrs.map(&:project) } | |
110 | + | |
111 | + let(:all_projects) { owned_or_public_projects + [not_owned_private_attrs.project] } | |
112 | + | |
113 | + context 'when projects exist' do | |
114 | + before :each do | |
115 | + all_projects.each do |project| | |
116 | + described_class.stubs(:find).with(project.id).returns(project) | |
117 | + end | |
118 | + | |
119 | + ProjectAttributes.expects(:where).with(public: true).returns(public_attrs) | |
120 | + end | |
121 | + | |
122 | + context 'when user is not provided' do | |
123 | + it 'is expected to find all public reading groups' do | |
124 | + expect(described_class.public_or_owned_by_user).to eq(public_projects) | |
125 | + end | |
126 | + end | |
127 | + | |
128 | + context 'when user is provided' do | |
129 | + before do | |
130 | + ProjectAttributes.expects(:where).with(user_id: user.id, public: false).returns([owned_private_attrs]) | |
131 | + end | |
132 | + | |
133 | + it 'is expected to find all public and owned reading groups' do | |
134 | + expect(described_class.public_or_owned_by_user(user)).to eq(owned_or_public_projects) | |
135 | + end | |
136 | + end | |
137 | + end | |
138 | + | |
139 | + context 'when no reading groups exist' do | |
140 | + before :each do | |
141 | + all_projects.each do |project| | |
142 | + described_class.stubs(:find).with(project.id).raises(KalibroClient::Errors::RecordNotFound) | |
143 | + end | |
144 | + | |
145 | + ProjectAttributes.expects(:where).with(public: true).returns(public_attrs) | |
146 | + end | |
147 | + | |
148 | + it 'is expected to be empty' do | |
149 | + expect(described_class.public_or_owned_by_user).to be_empty | |
150 | + end | |
151 | + end | |
68 | 152 | end |
69 | 153 | end |
70 | 154 | end | ... | ... |