diff --git a/app/controllers/concerns/ownership_authentication.rb b/app/controllers/concerns/ownership_authentication.rb index 458047e..9c8f897 100644 --- a/app/controllers/concerns/ownership_authentication.rb +++ b/app/controllers/concerns/ownership_authentication.rb @@ -52,7 +52,7 @@ module OwnershipAuthentication private def check_project_ownership(id) - if current_user.project_ownerships.find_by_project_id(id).nil? + if current_user.project_attributes.find_by_project_id(id).nil? respond_to do |format| format.html { redirect_to projects_url, notice: "You're not allowed to do this operation" } format.json { head :no_content } diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 7453aff..644e704 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -9,7 +9,6 @@ class ProjectsController < ApplicationController # GET /projects/new def new @project = Project.new - @project_image = ProjectImage.new end # GET /projects @@ -25,7 +24,7 @@ class ProjectsController < ApplicationController @project = Project.new(project_params) respond_to do |format| create_and_redir(format) - ProjectImage.create(url: image_url, project_id: @project.id ) + @project.attributes.update(image_url: image_url) unless @project.attributes.nil? end end @@ -45,7 +44,7 @@ class ProjectsController < ApplicationController def update set_project image_url = project_params.delete(:image_url) - if @project.update(project_params) && @project_image.update(url: image_url) + if @project.update(project_params) && @project.attributes.update(image_url: image_url) redirect_to(project_path(@project.id)) else render "edit" @@ -56,7 +55,7 @@ class ProjectsController < ApplicationController # DELETE /project/1.json def destroy set_project - current_user.project_ownerships.find_by_project_id(@project.id).destroy + current_user.project_attributes.find_by_project_id(@project.id).destroy @project.destroy respond_to do |format| format.html { redirect_to projects_url } @@ -68,7 +67,6 @@ class ProjectsController < ApplicationController # Use callbacks to share common setup or constraints between actions. def set_project @project = find_resource(Project, params[:id].to_i) - @project_image = ProjectImage.find_by_project_id(@project.id) if @project.is_a?(Project) end # Never trust parameters from the scary internet, only allow the white list through. @@ -80,7 +78,7 @@ class ProjectsController < ApplicationController # Extracted code from create action def create_and_redir(format) if @project.save - current_user.project_ownerships.create project_id: @project.id + current_user.project_attributes.create(project_id: @project.id) format.html { redirect_to project_path(@project.id), notice: 'Project was successfully created.' } format.json { render action: 'show', status: :created, location: @project } else diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 52542e9..90aa575 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -1,5 +1,5 @@ module ProjectsHelper def project_owner? project_id - user_signed_in? && !current_user.project_ownerships.find_by_project_id(project_id).nil? + user_signed_in? && !current_user.project_attributes.find_by_project_id(project_id).nil? end end \ No newline at end of file diff --git a/app/models/project.rb b/app/models/project.rb index b78dea2..a63f2f5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1,6 +1,10 @@ class Project < KalibroClient::Entities::Processor::Project include KalibroRecord def self.latest(count = 1) - all.sort { |a,b| b.id <=> a.id }.first(count) + all.sort { |a,b| b.id <=> a.id }.select { |project| !project.attributes.hidden}.first(count) + end + + def attributes + ProjectAttributes.find_by_project_id(self.id) end end diff --git a/app/models/project_attributes.rb b/app/models/project_attributes.rb new file mode 100644 index 0000000..4a8dfec --- /dev/null +++ b/app/models/project_attributes.rb @@ -0,0 +1,8 @@ +class ProjectAttributes < ActiveRecord::Base + belongs_to :user + belongs_to :project + + def project + Project.find(self.project_id) + end +end diff --git a/app/models/project_image.rb b/app/models/project_image.rb index 9929099..91db734 100644 --- a/app/models/project_image.rb +++ b/app/models/project_image.rb @@ -1,3 +1,4 @@ +#FIXME: remove this after the migration has been done and modify the migration accordingly class ProjectImage < ActiveRecord::Base belongs_to :project end diff --git a/app/models/project_ownership.rb b/app/models/project_ownership.rb index 74357c0..d1ef993 100644 --- a/app/models/project_ownership.rb +++ b/app/models/project_ownership.rb @@ -1,3 +1,4 @@ +#FIXME: remove this after the migration has been done and modify the migration accordingly class ProjectOwnership < ActiveRecord::Base belongs_to :user validates :project_id, presence: true diff --git a/app/models/user.rb b/app/models/user.rb index 57ad0eb..f50d981 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -8,12 +8,12 @@ class User < ActiveRecord::Base validates :email, presence: true validates :email, uniqueness: true - has_many :project_ownerships + has_many :project_attributes, class_name: 'ProjectAttributes' has_many :reading_group_ownerships has_many :kalibro_configuration_ownerships # Alert: when adding new parameters to this model, they should also be added to registrations_controller def projects - project_ownerships.map { |project_ownership| project_ownership.project } + project_attributes.map { |project_attr| project_attr.project } end end diff --git a/app/views/projects/_form.html.erb b/app/views/projects/_form.html.erb index 54ce46c..6885d36 100644 --- a/app/views/projects/_form.html.erb +++ b/app/views/projects/_form.html.erb @@ -17,7 +17,7 @@
<%= f.label "Image url", class: 'control-label' %>
- <%= f.text_field :image_url, class: 'text-area', value: @project_image.nil? ? '#' : @project_image.url %> + <%= f.text_field :image_url, class: 'text-area', value: @project.attributes.image_url.nil? ? '#' : @project.attributes.image_url %>
diff --git a/db/migrate/20150225170704_create_project_attributes.rb b/db/migrate/20150225170704_create_project_attributes.rb new file mode 100644 index 0000000..1562fb2 --- /dev/null +++ b/db/migrate/20150225170704_create_project_attributes.rb @@ -0,0 +1,56 @@ +class CreateProjectAttributes < ActiveRecord::Migration + def up + create_table :project_attributes do |t| + t.integer :project_id + t.string :image_url + t.integer :user_id + t.boolean :hidden, default: false + + t.timestamps null: false + end + + ProjectOwnership.all.each do |project_ownership| + project_image = ProjectImage.find_by_project_id(project_ownership.project_id) + image_url = project_image.nil? ? "": project_image.url + + begin + # We want to hides projects prior this date since they probably have a invalid configuration + if Project.find(project_ownership.project_id).updated_at < DateTime.parse("Mon, 23 Feb 2015") + hidden = true + else + hidden = false + end + + ProjectAttributes.create(user_id: project_ownership.user_id, project_id: project_ownership.project_id, image_url: image_url, hidden: hidden) + rescue KalibroClient::Errors::RecordNotFound + puts "Could not find project with id #{project_ownership.project_id} owned by user with #{project_ownership.user_id} and image url #{image_url}" + end + end + + drop_table :project_ownerships + drop_table :project_images + end + + def down + create_table :project_ownerships do |t| + t.integer :user_id + t.integer :project_id + + t.timestamps + end + + create_table :project_images do |t| + t.belongs_to :project + t.string :url + + t.timestamps + end + + ProjectAttributes.all.each do |project_attribute| + ProjectOwnership.create(user_id: project_attribute.user_id, project_id: project_attribute.project_id) + ProjectImage.create(url: project_attribute.image_url, project_id: project_attribute.project_id) + end + + drop_table :project_attributes + end +end diff --git a/db/schema.rb b/db/schema.rb index 032d100..d2fe7bb 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: 20141211114023) do +ActiveRecord::Schema.define(version: 20150225170704) do create_table "kalibro_configuration_ownerships", force: :cascade do |t| t.integer "user_id" @@ -20,18 +20,13 @@ ActiveRecord::Schema.define(version: 20141211114023) do t.datetime "updated_at" end - create_table "project_images", force: :cascade do |t| + create_table "project_attributes", force: :cascade do |t| t.integer "project_id" - t.string "url", limit: 255 - t.datetime "created_at" - t.datetime "updated_at" - end - - create_table "project_ownerships", force: :cascade do |t| + t.string "image_url" t.integer "user_id" - t.integer "project_id" - t.datetime "created_at" - t.datetime "updated_at" + t.boolean "hidden", default: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end create_table "reading_group_ownerships", force: :cascade do |t| diff --git a/spec/controllers/concerns/ownership_authentication_spec.rb b/spec/controllers/concerns/ownership_authentication_spec.rb index 6874542..110c8c6 100644 --- a/spec/controllers/concerns/ownership_authentication_spec.rb +++ b/spec/controllers/concerns/ownership_authentication_spec.rb @@ -111,12 +111,12 @@ describe OwnershipAuthentication, type: :controller do end context 'when the user owns the Repository' do - let!(:project_ownership) { FactoryGirl.build(:project_ownership, {user_id: current_user.id, project_id: project.id}) } + let!(:project_attributes) { FactoryGirl.build(:project_attributes, {user_id: current_user.id, project_id: project.id}) } before do - project_ownerships = Object.new - project_ownerships.expects(:find_by_project_id).with(project.id).returns(project_ownership) - current_user.expects(:project_ownerships).returns(project_ownerships) + project_attrs = Object.new + project_attrs.expects(:find_by_project_id).with(project.id).returns(project_attributes) + current_user.expects(:project_attributes).returns(project_attrs) end it 'should return true' do @@ -126,9 +126,9 @@ describe OwnershipAuthentication, type: :controller do context 'when the user does not own the Repository' do before do - project_ownerships = Object.new - project_ownerships.expects(:find_by_project_id).with(project.id).returns(nil) - current_user.expects(:project_ownerships).returns(project_ownerships) + project_attrs = Object.new + project_attrs.expects(:find_by_project_id).with(project.id).returns(nil) + current_user.expects(:project_attributes).returns(project_attrs) end it 'should respond' do # FIXME: this is not the best test, but it it's the closest we can do I think diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 9474f3b..c041628 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -91,21 +91,20 @@ describe ProjectsController, :type => :controller do context 'with a User logged in' do before do sign_in FactoryGirl.create(:user) - @ownership = FactoryGirl.build(:project_ownership) - @ownerships = [] - + @project_attributes = FactoryGirl.build(:project_attributes) + @attributes = [] end context 'when the user owns the project' do before :each do - @ownership.expects(:destroy) + @project_attributes.expects(:destroy) @subject.expects(:destroy) #Those two mocks looks the same but they are necessary since params[:id] is a String and @project.id is an Integer :( - @ownerships.expects(:find_by_project_id).with("#{@subject.id}").returns(@ownership) - @ownerships.expects(:find_by_project_id).with(@subject.id).returns(@ownership) + @attributes.expects(:find_by_project_id).with("#{@subject.id}").returns(@project_attributes) + @attributes.expects(:find_by_project_id).with(@subject.id).returns(@project_attributes) - User.any_instance.expects(:project_ownerships).at_least_once.returns(@ownerships) + User.any_instance.expects(:project_attributes).at_least_once.returns(@attributes) subject.expects(:find_resource).with(Project, @subject.id).returns(@subject) delete :destroy, :id => @subject.id @@ -120,8 +119,8 @@ describe ProjectsController, :type => :controller do context "when the user doesn't own the project" do before :each do - @ownerships.expects(:find_by_project_id).with("#{@subject.id}").returns(nil) - User.any_instance.expects(:project_ownerships).at_least_once.returns(@ownerships) + @attributes.expects(:find_by_project_id).with("#{@subject.id}").returns(nil) + User.any_instance.expects(:project_attributes).at_least_once.returns(@attributes) delete :destroy, :id => @subject.id end @@ -152,15 +151,14 @@ describe ProjectsController, :type => :controller do describe 'edit' do before do @subject = FactoryGirl.build(:project_with_id) - @project_image = FactoryGirl.create(:project_image) end context 'with an User logged in' do before do @user = FactoryGirl.create(:user) - @ownership = FactoryGirl.build(:project_ownership) - @ownerships = [] - User.any_instance.expects(:project_ownerships).at_least_once.returns(@ownerships) + @attribute = FactoryGirl.build(:project_attributes) + @attributes = [] + User.any_instance.expects(:project_attributes).at_least_once.returns(@attributes) sign_in @user end @@ -168,8 +166,7 @@ describe ProjectsController, :type => :controller do context 'when the user owns the project' do before :each do subject.expects(:find_resource).with(Project, @subject.id).returns(@subject) - @ownerships.expects(:find_by_project_id).with("#{@subject.id}").returns(@ownership) - ProjectImage.expects(:find_by_project_id).with(@subject.id).returns(@project_image) + @attributes.expects(:find_by_project_id).with("#{@subject.id}").returns(@attribute) get :edit, :id => @subject.id end @@ -184,7 +181,7 @@ describe ProjectsController, :type => :controller do context 'when the user does not own the project' do before do @subject = FactoryGirl.build(:another_project) - @ownerships.expects(:find_by_project_id).with("#{@subject.id}").returns(nil) + @attributes.expects(:find_by_project_id).with("#{@subject.id}").returns(nil) get :edit, :id => @subject.id end @@ -205,7 +202,6 @@ describe ProjectsController, :type => :controller do describe 'update' do before do - @project_image = FactoryGirl.build(:project_image) @subject = FactoryGirl.build(:project_with_id) @subject_params = @subject.to_hash end @@ -217,17 +213,18 @@ describe ProjectsController, :type => :controller do context 'when user owns the project' do before do - @ownership = FactoryGirl.build(:project_ownership) - @ownerships = [] - @ownerships.expects(:find_by_project_id).with("#{@subject.id}").returns(@ownership) - User.any_instance.expects(:project_ownerships).at_least_once.returns(@ownerships) - ProjectImage.expects(:find_by_project_id).with(@subject.id).returns(@project_image) + @project_attributes = FactoryGirl.build(:project_attributes) + @attributes = [] + @attributes.expects(:find_by_project_id).with("#{@subject.id}").returns(@project_attributes) + User.any_instance.expects(:project_attributes).at_least_once.returns(@attributes) end context 'with valid fields' do before :each do subject.expects(:find_resource).with(Project, @subject.id).returns(@subject) Project.any_instance.expects(:update).with(@subject_params).returns(true) + @project_attributes.expects(:update).with(image_url: @subject_params[:image_url]).returns(true) + @subject.expects(:attributes).returns(@project_attributes) end context 'rendering the show' do diff --git a/spec/factories/project_attributes.rb b/spec/factories/project_attributes.rb new file mode 100644 index 0000000..10da652 --- /dev/null +++ b/spec/factories/project_attributes.rb @@ -0,0 +1,8 @@ +FactoryGirl.define do + factory :project_attributes, :class => 'ProjectAttributes' do + project_id 1 + image_url "MyString" + user_id 1 + hidden false + end +end diff --git a/spec/factories/project_images.rb b/spec/factories/project_images.rb deleted file mode 100644 index be2a1e4..0000000 --- a/spec/factories/project_images.rb +++ /dev/null @@ -1,12 +0,0 @@ -# Read about factories at https://github.com/thoughtbot/factory_girl - -FactoryGirl.define do - factory :project_image do - project_id 1 - url "logo.png" - end - - factory :project_no_image, class: ProjectImage do - url nil - end -end diff --git a/spec/factories/project_ownerships.rb b/spec/factories/project_ownerships.rb deleted file mode 100644 index 666eb68..0000000 --- a/spec/factories/project_ownerships.rb +++ /dev/null @@ -1,8 +0,0 @@ -# Read about factories at https://github.com/thoughtbot/factory_girl - -FactoryGirl.define do - factory :project_ownership do - user_id 1 - project_id 1 - end -end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index c0425e0..7d6c58c 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -19,10 +19,10 @@ describe ProjectsHelper, :type => :helper do helper.expects(:user_signed_in?).returns(true) helper.expects(:current_user).returns(FactoryGirl.build(:user)) - @ownerships = [] - @ownerships.expects(:find_by_project_id).with(@subject.id).returns(nil) + @attributes = [] + @attributes.expects(:find_by_project_id).with(@subject.id).returns(nil) - User.any_instance.expects(:project_ownerships).returns(@ownerships) + User.any_instance.expects(:project_attributes).returns(@attributes) end it { expect(helper.project_owner?(@subject.id)).to be_falsey } @@ -33,10 +33,10 @@ describe ProjectsHelper, :type => :helper do helper.expects(:user_signed_in?).returns(true) helper.expects(:current_user).returns(FactoryGirl.build(:user)) - @ownership = FactoryGirl.build(:project_ownership) - @ownerships = [] - @ownerships.expects(:find_by_project_id).with(@subject.id).returns(@ownership) - User.any_instance.expects(:project_ownerships).returns(@ownerships) + @project_attributes = FactoryGirl.build(:project_attributes) + @attributes = [] + @attributes.expects(:find_by_project_id).with(@subject.id).returns(@project_attributes) + User.any_instance.expects(:project_attributes).returns(@attributes) end it { expect(helper.project_owner?(@subject.id)).to be_truthy } diff --git a/spec/models/project_attributes_spec.rb b/spec/models/project_attributes_spec.rb new file mode 100644 index 0000000..3511b4e --- /dev/null +++ b/spec/models/project_attributes_spec.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +RSpec.describe ProjectAttributes, type: :model do + describe 'associations' do + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:project) } + end + + describe 'methods' do + describe 'project' do + subject { FactoryGirl.build(:project_attributes) } + let(:project) {FactoryGirl.build(:project_with_id)} + + before :each do + Project.expects(:find).with(subject.project_id).returns(project) + end + + it 'should return the project' do + expect(subject.project).to eq(project) + end + end + end +end diff --git a/spec/models/project_image_spec.rb b/spec/models/project_image_spec.rb deleted file mode 100644 index fa7cda4..0000000 --- a/spec/models/project_image_spec.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'rails_helper' - -RSpec.describe ProjectImage, :type => :model do - describe 'associations' do - it { is_expected.to belong_to(:project) } - end -end diff --git a/spec/models/project_ownership_spec.rb b/spec/models/project_ownership_spec.rb deleted file mode 100644 index 42dfe39..0000000 --- a/spec/models/project_ownership_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'rails_helper' - -describe ProjectOwnership, :type => :model do - describe 'associations' do - it { is_expected.to belong_to(:user) } - end - - describe 'methods' do - describe 'project' do - subject {FactoryGirl.build(:project_ownership)} - let(:project) {FactoryGirl.build(:project_with_id)} - - before :each do - Project.expects(:find).with(subject.project_id).returns(project) - end - - it 'should return the project' do - expect(subject.project).to eq(project) - end - end - end -end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b5a3128..677e4be 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3,22 +3,40 @@ require 'rails_helper' describe Project, :type => :model do describe 'methods' do describe 'latest' do + let!(:project) { FactoryGirl.build(:project_with_id, id: 1) } + let!(:another_project) { FactoryGirl.build(:another_project, id: 2) } + let!(:project_attributes) { FactoryGirl.build(:project_attributes) } + before :each do - @qt = FactoryGirl.build(:project_with_id) - @kalibro = FactoryGirl.build(:another_project) + project.expects(:attributes).returns(project_attributes) + another_project.expects(:attributes).returns(project_attributes) - Project.expects(:all).returns([@qt, @kalibro]) + Project.expects(:all).returns([project, another_project]) end it 'should return the two projects ordered' do - expect(Project.latest(2)).to eq([@kalibro, @qt]) + expect(Project.latest(2)).to eq([another_project, project]) end context 'when no parameter is passed' do it 'should return just the most recent project' do - expect(Project.latest).to eq([@kalibro]) + expect(Project.latest).to eq([another_project]) end end end + + describe 'attributes' do + subject { FactoryGirl.build(:project_with_id) } + + let!(:project_attributes) { FactoryGirl.build(: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 end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index eb967a6..c7526ed 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -10,7 +10,7 @@ describe User, :type => :model do end describe 'associations' do - it { is_expected.to have_many(:project_ownerships) } + it { is_expected.to have_many(:project_attributes) } it { is_expected.to have_many(:reading_group_ownerships) } it { is_expected.to have_many(:kalibro_configuration_ownerships) } end @@ -19,11 +19,11 @@ describe User, :type => :model do describe 'projects' do subject { FactoryGirl.build(:user) } let(:project) {FactoryGirl.build(:project_with_id)} - let(:project_ownership) {FactoryGirl.build(:project_ownership)} + let(:project_attributes) {FactoryGirl.build(:project_attributes)} before :each do - project_ownership.expects(:project).returns(project) - subject.expects(:project_ownerships).returns([project_ownership]) + project_attributes.expects(:project).returns(project) + subject.expects(:project_attributes).returns([project_attributes]) end it 'should return a list of projects owned by the user' do -- libgit2 0.21.2