From 057737c75948c927671135f03db78f471acfa942 Mon Sep 17 00:00:00 2001 From: Heitor Reis Date: Fri, 6 Nov 2015 17:44:49 -0200 Subject: [PATCH] Fix ProjectAttributes association with Project --- app/controllers/projects_controller.rb | 2 +- app/models/project.rb | 22 +++++++++++++++++----- app/models/project_attributes.rb | 7 ++++++- db/migrate/20151106182639_change_project_attributes_public_default.rb | 6 ++++++ db/schema.rb | 22 +++++++++++----------- features/project/deletion.feature | 1 - features/project/edition.feature | 10 ++-------- features/project/listing.feature | 1 - features/project/show.feature | 5 ----- features/repository/create.feature | 1 - features/step_definitions/project_steps.rb | 6 ++---- spec/controllers/projects_controller_spec.rb | 6 ++---- spec/factories/project_attributes.rb | 15 ++++++++++++--- spec/models/project_attributes_spec.rb | 21 ++++++++++++++++++--- spec/models/project_spec.rb | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 15 files changed, 170 insertions(+), 57 deletions(-) create mode 100644 db/migrate/20151106182639_change_project_attributes_public_default.rb diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 09789fd..cb27876 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -13,7 +13,7 @@ class ProjectsController < ApplicationController # GET /projects # GET /projects.json def index - @projects = Project.all.select { |project| !project.attributes.hidden } + @projects = Project.public_or_owned_by_user(current_user) end # POST /projects diff --git a/app/models/project.rb b/app/models/project.rb index c6e9263..586a9a0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -3,18 +3,30 @@ class Project < KalibroClient::Entities::Processor::Project attr_writer :attributes + def self.public_or_owned_by_user(user = nil) + project_attributes = ProjectAttributes.where(public: true) + project_attributes += ProjectAttributes.where(user_id: user.id, public: false) if user + + project_attributes.map do |attribute| + begin + self.find(attribute.project_id) + rescue KalibroClient::Errors::RecordNotFound + nil + end + end.compact + end + def self.latest(count = 1) - all.sort { |a,b| b.id <=> a.id }.select { |project| !project.attributes.hidden}.first(count) + all.sort { |a, b| b.id <=> a.id }.select { |project| project.attributes.public }.first(count) end def attributes - @project_attributes ||= ProjectAttributes.find_by_project_id(self.id) - @project_attributes.nil? ? ProjectAttributes.new : @project_attributes + @attributes ||= ProjectAttributes.find_by_project_id(@id) end def destroy - self.attributes.destroy if self.attributes - @project_attributes = nil + self.attributes.destroy unless self.attributes.nil? + @attributes = nil super end end diff --git a/app/models/project_attributes.rb b/app/models/project_attributes.rb index bccff97..647c2ca 100644 --- a/app/models/project_attributes.rb +++ b/app/models/project_attributes.rb @@ -4,6 +4,11 @@ class ProjectAttributes < ActiveRecord::Base validates :user, presence: true def project - Project.find(self.project_id) + @project ||= Project.find(project_id) + end + + def project=(project) + @project = project + self.project_id = project.id end end diff --git a/db/migrate/20151106182639_change_project_attributes_public_default.rb b/db/migrate/20151106182639_change_project_attributes_public_default.rb new file mode 100644 index 0000000..76fe74b --- /dev/null +++ b/db/migrate/20151106182639_change_project_attributes_public_default.rb @@ -0,0 +1,6 @@ +class ChangeProjectAttributesPublicDefault < ActiveRecord::Migration + def change + rename_column :project_attributes, :hidden, :public + change_column_default :project_attributes, :public, true + end +end diff --git a/db/schema.rb b/db/schema.rb index d871019..433ef20 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150616164352) do +ActiveRecord::Schema.define(version: 20151106182639) do create_table "kalibro_configuration_attributes", force: :cascade do |t| t.integer "user_id" @@ -25,9 +25,9 @@ ActiveRecord::Schema.define(version: 20150616164352) do t.integer "project_id" t.string "image_url" t.integer "user_id" - t.boolean "hidden", default: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.boolean "public", default: true + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end create_table "reading_group_attributes", force: :cascade do |t| @@ -48,19 +48,19 @@ ActiveRecord::Schema.define(version: 20150616164352) do add_index "repository_attributes", ["user_id"], name: "index_repository_attributes_on_user_id" create_table "users", force: :cascade do |t| - t.string "name", limit: 255, default: "", null: false - t.string "email", limit: 255, default: "", null: false + t.string "name", default: "", null: false + t.string "email", default: "", null: false t.datetime "created_at" t.datetime "updated_at" - t.string "encrypted_password", limit: 255, default: "", null: false - t.string "reset_password_token", limit: 255 + t.string "encrypted_password", default: "", null: false + t.string "reset_password_token" t.datetime "reset_password_sent_at" t.datetime "remember_created_at" - t.integer "sign_in_count", default: 0 + t.integer "sign_in_count", default: 0 t.datetime "current_sign_in_at" t.datetime "last_sign_in_at" - t.string "current_sign_in_ip", limit: 255 - t.string "last_sign_in_ip", limit: 255 + t.string "current_sign_in_ip" + t.string "last_sign_in_ip" end add_index "users", ["email"], name: "index_users_on_email", unique: true diff --git a/features/project/deletion.feature b/features/project/deletion.feature index 1fb7c4f..238fe9b 100644 --- a/features/project/deletion.feature +++ b/features/project/deletion.feature @@ -22,7 +22,6 @@ Feature: Project Deletion Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes And I am at the Sample Project page When I click the Destroy Project link Then I should be in the All Projects page diff --git a/features/project/edition.feature b/features/project/edition.feature index 525229a..65c819c 100644 --- a/features/project/edition.feature +++ b/features/project/edition.feature @@ -8,13 +8,12 @@ Feature: Project Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes And I am at the All Projects page When I click the Edit link Then I should be in the Edit Project page @kalibro_processor_restart - Scenario: Should not show edit links from projects that doesn't belongs to me + Scenario: Should not show edit links from projects that doesn't belong to me Given I am a regular user And I am signed in And I have a sample project @@ -22,7 +21,7 @@ Feature: Project Then I should not see Edit within table @kalibro_processor_restart - Scenario: Should not render the edit page if the project doesn't belongs to the current user + Scenario: Should not render the edit page if the project doesn't belong to the current user Given I am a regular user And I am signed in And I have a sample project @@ -35,7 +34,6 @@ Feature: Project Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes And I am at the All Projects page When I click the Edit link Then The field "project[name]" should be filled with the sample project "name" @@ -46,7 +44,6 @@ Feature: Project Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes And I am at the sample project edit page And I fill the Name field with "Kalibro" And I fill the Description field with "Web Service to collect metrics" @@ -61,7 +58,6 @@ Feature: Project And I have a project named "Qt-Calculator" And I own a project named "Kalibro" And I am at the sample project edit page - And I have sample project_attributes And I fill the Name field with "Qt-Calculator" When I press the Save button Then I should see "Name has already been taken" @@ -71,7 +67,6 @@ Feature: Project Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes And I am at the sample project edit page And I fill the Description field with "Web Service to collect metrics" When I press the Save button @@ -82,7 +77,6 @@ Feature: Project Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes And I am at the sample project edit page And I fill the Name field with " " When I press the Save button diff --git a/features/project/listing.feature b/features/project/listing.feature index fec09b9..35bd7ab 100644 --- a/features/project/listing.feature +++ b/features/project/listing.feature @@ -25,7 +25,6 @@ Feature: Project listing Given I am a regular user And I am signed in And I have a sample project - And I have sample project_attributes And I am at the All Projects page When I click the Show link Then the sample project should be there diff --git a/features/project/show.feature b/features/project/show.feature index fcda17c..601e1b4 100644 --- a/features/project/show.feature +++ b/features/project/show.feature @@ -7,7 +7,6 @@ Feature: Show Project Scenario: Should not show the create repository link to user that doesn't own the project Given I am a regular user And I have a sample project - And I have sample project_attributes And I have a sample configuration with native metrics And I have a sample repository within the sample project When I am at the Sample Project page @@ -20,7 +19,6 @@ Scenario: Should show the create repository link the project owner Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes When I am at the Sample Project page Then I should see "New Repository" @@ -29,7 +27,6 @@ Scenario: Should not show the independent repositories for a project Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes And I have a sample configuration And I have a sample repository When I am at the Sample Project page @@ -39,7 +36,6 @@ Scenario: Should not show the independent repositories for a project Scenario: Considering the project has no repositories Given I am a regular user And I have a sample project - And I have sample project_attributes When I am at the Sample Project page Then I should see "There are no Repositories yet!" @@ -56,6 +52,5 @@ Scenario: Considering the project has repositories Scenario: Checking project contents Given I am a regular user And I have a sample project - And I have sample project_attributes When I am at the Sample Project page Then the sample project should be there diff --git a/features/repository/create.feature b/features/repository/create.feature index 66a33e5..c0e386f 100644 --- a/features/repository/create.feature +++ b/features/repository/create.feature @@ -8,7 +8,6 @@ Scenario: repository creation associated with a project Given I am a regular user And I am signed in And I own a sample project - And I have sample project_attributes And I have a sample configuration with native metrics And I am at the New Repository page And I fill the Name field with "Kalibro" diff --git a/features/step_definitions/project_steps.rb b/features/step_definitions/project_steps.rb index 32cf9db..59ff406 100644 --- a/features/step_definitions/project_steps.rb +++ b/features/step_definitions/project_steps.rb @@ -6,14 +6,12 @@ end Given(/^I have a sample project$/) do @project = FactoryGirl.create(:project) -end - -Given(/^I have sample project_attributes$/) do - @project_attributes = FactoryGirl.create(:project_attributes, {id: nil, user_id: @user.id}) + @project.attributes = FactoryGirl.create(:project_attributes, project: @project) end Given(/^I have a project named "(.*?)"$/) do |name| @project = FactoryGirl.create(:project, {name: name}) + @project.attributes = FactoryGirl.create(:project_attributes, project: @project) end Given(/^I own a sample project$/) do diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 4b72aab..aa8df6e 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -148,12 +148,10 @@ describe ProjectsController, :type => :controller do end describe 'index' do - let(:project_attributes) { FactoryGirl.build(:project_attributes) } + subject { FactoryGirl.build(:project_with_id) } before :each do - @subject = FactoryGirl.build(:project_with_id) - Project.expects(:all).returns([@subject]) - @subject.expects(:attributes).returns(project_attributes) + Project.expects(:public_or_owned_by_user).returns([subject]) get :index end diff --git a/spec/factories/project_attributes.rb b/spec/factories/project_attributes.rb index befae79..1e62c56 100644 --- a/spec/factories/project_attributes.rb +++ b/spec/factories/project_attributes.rb @@ -1,12 +1,21 @@ FactoryGirl.define do factory :project_attributes, :class => 'ProjectAttributes' do - project_id 1 image_url '' - user_id 1 - hidden false + self.public true + association :project, :with_id, strategy: :build + association :user, strategy: :build trait :with_image do image_url '#' end + + trait :private do + self.public false + end + + trait :bare do + project_id nil + user_id nil + end end end diff --git a/spec/models/project_attributes_spec.rb b/spec/models/project_attributes_spec.rb index fb7564e..da271d2 100644 --- a/spec/models/project_attributes_spec.rb +++ b/spec/models/project_attributes_spec.rb @@ -11,10 +11,9 @@ RSpec.describe ProjectAttributes, type: :model do end describe 'methods' do + let(:project) { FactoryGirl.build(:project_with_id) } describe 'project' do - subject { FactoryGirl.build(:project_attributes) } - let(:project) {FactoryGirl.build(:project_with_id)} - + subject { FactoryGirl.build(:project_attributes, :bare, project_id: project.id) } before :each do Project.expects(:find).with(subject.project_id).returns(project) end @@ -23,5 +22,21 @@ RSpec.describe ProjectAttributes, type: :model do expect(subject.project).to eq(project) end end + + describe 'project=' do + subject { FactoryGirl.build(:project_attributes, :bare) } + + before do + subject.project = project + end + + it 'is expected to set the project' do + expect(subject.project).to eq project + end + + it 'is expected to set the project_id' do + expect(subject.project_id).to eq project.id + end + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9b12493..4c5b201 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -28,33 +28,47 @@ describe Project, :type => :model do describe 'attributes' do subject { FactoryGirl.build(:project_with_id) } - let!(:project_attributes) { FactoryGirl.build(:project_attributes) } + context 'when there are attributes' do + let!(:project_attributes) { FactoryGirl.build(:project_attributes) } - before :each do - ProjectAttributes.expects(:find_by_project_id).returns(project_attributes) + before :each do + ProjectAttributes.expects(:find_by_project_id).returns(project_attributes) + end + + it 'is expected to return the project attributes' do + expect(subject.attributes).to eq(project_attributes) + end end - it 'is expected to return the project attributes' do - expect(subject.attributes).to eq(project_attributes) + context 'when there are no attributes' do + before :each do + ProjectAttributes.expects(:find_by_project_id).returns(nil) + end + + it 'is expected to return the project attributes' do + expect(subject.attributes).to be_nil + end end end describe 'destroy' do + let!(:project) { FactoryGirl.build(:project) } + context 'when attributes exist' do - let!(:project) { FactoryGirl.build(:project) } let!(:project_attributes) { FactoryGirl.build(:project_attributes, project_id: project.id) } + before :each do + KalibroClient::Entities::Processor::Project.any_instance.expects(:destroy).returns(project) + end - it 'should be destroyed' do + it 'is expected to be destroyed' do project.expects(:attributes).twice.returns(project_attributes) project_attributes.expects(:destroy) - KalibroClient::Entities::Processor::Project.any_instance.expects(:destroy).returns(project) project.destroy end it 'is expected to clean the attributes memoization' do # Call attributes once so it memoizes ProjectAttributes.expects(:find_by).with(project_id: project.id).returns(project_attributes) - KalibroClient::Entities::Processor::Project.any_instance.expects(:destroy).returns(project) expect(project.attributes).to eq(project_attributes) # Destroying @@ -65,6 +79,76 @@ describe Project, :type => :model do expect(project.attributes).to_not eq(project_attributes) end end + + context 'when attributes is nil' do + before do + project.expects(:attributes).returns(nil) + KalibroClient::Entities::Processor::Project.any_instance.expects(:destroy).returns(project) + end + + it 'is expected to not try to destroy the attributes' do + project.destroy + end + end + end + end + + describe 'class methods' do + describe 'public_or_owned_by_user' do + let!(:user) { FactoryGirl.build(:user, :with_id) } + + let!(:owned_private_attrs) { FactoryGirl.build(:project_attributes, :private, user_id: user.id) } + let!(:owned_public_attrs) { FactoryGirl.build(:project_attributes, user_id: user.id) } + let!(:not_owned_private_attrs) { FactoryGirl.build(:project_attributes, :private, user_id: user.id + 1) } + let!(:not_owned_public_attrs) { FactoryGirl.build(:project_attributes, user_id: user.id + 1) } + + let!(:public_attrs) { [owned_public_attrs, not_owned_public_attrs] } + let(:public_projects) { public_attrs.map(&:project) } + + let!(:owned_or_public_attrs) { public_attrs + [owned_private_attrs] } + let!(:owned_or_public_projects) { owned_or_public_attrs.map(&:project) } + + let(:all_projects) { owned_or_public_projects + [not_owned_private_attrs.project] } + + context 'when projects exist' do + before :each do + all_projects.each do |project| + described_class.stubs(:find).with(project.id).returns(project) + end + + ProjectAttributes.expects(:where).with(public: true).returns(public_attrs) + end + + context 'when user is not provided' do + it 'is expected to find all public reading groups' do + expect(described_class.public_or_owned_by_user).to eq(public_projects) + end + end + + context 'when user is provided' do + before do + ProjectAttributes.expects(:where).with(user_id: user.id, public: false).returns([owned_private_attrs]) + end + + it 'is expected to find all public and owned reading groups' do + expect(described_class.public_or_owned_by_user(user)).to eq(owned_or_public_projects) + end + end + end + + context 'when no reading groups exist' do + before :each do + all_projects.each do |project| + described_class.stubs(:find).with(project.id).raises(KalibroClient::Errors::RecordNotFound) + end + + ProjectAttributes.expects(:where).with(public: true).returns(public_attrs) + end + + it 'is expected to be empty' do + expect(described_class.public_or_owned_by_user).to be_empty + end + end end end end -- libgit2 0.21.2