Commit 16bb89f46ee7d8ff136305b4aecd0133a8445f4b
Committed by
Diego Camarinha
1 parent
78614ea7
Exists in
colab
and in
4 other branches
Major fixes after ReadingGroup or KalibroConfiguration hidding
`public_or_owned_by_user` was using a syntax unsupported on all databases. Now it produces two queries to the database, but generated by ActiveRecord which takes care of compability. This was breaking KalibroConfiguration listing acceptance tests. Restored MetricConfiguration form for using `reading_group_options` helper. It was broken using a undefined method. This was breaking MetricConfiguration create and edit acceptance tests.
Showing
6 changed files
with
16 additions
and
25 deletions
Show diff stats
app/models/kalibro_configuration.rb
@@ -2,15 +2,12 @@ class KalibroConfiguration < KalibroClient::Entities::Configurations::KalibroCon | @@ -2,15 +2,12 @@ class KalibroConfiguration < KalibroClient::Entities::Configurations::KalibroCon | ||
2 | include KalibroRecord | 2 | include KalibroRecord |
3 | 3 | ||
4 | def self.public_or_owned_by_user(user=nil) | 4 | def self.public_or_owned_by_user(user=nil) |
5 | - query = if user | ||
6 | - KalibroConfigurationAttributes.where("user_id == ? OR public", user.id) | ||
7 | - else | ||
8 | - KalibroConfigurationAttributes.where(public: true) | ||
9 | - end | 5 | + kalibro_configuration_attributes = KalibroConfigurationAttributes.where(public: true) |
6 | + kalibro_configuration_attributes += KalibroConfigurationAttributes.where(user_id: user.id, public: false) if user | ||
10 | 7 | ||
11 | - query.map { |cfg_attr| | 8 | + kalibro_configuration_attributes.map { |kalibro_configuration_attribute| |
12 | begin | 9 | begin |
13 | - self.find(cfg_attr.kalibro_configuration_id) | 10 | + self.find(kalibro_configuration_attribute.kalibro_configuration_id) |
14 | rescue KalibroClient::Errors::RecordNotFound | 11 | rescue KalibroClient::Errors::RecordNotFound |
15 | nil | 12 | nil |
16 | end | 13 | end |
app/models/reading_group.rb
@@ -2,15 +2,12 @@ class ReadingGroup < KalibroClient::Entities::Configurations::ReadingGroup | @@ -2,15 +2,12 @@ class ReadingGroup < KalibroClient::Entities::Configurations::ReadingGroup | ||
2 | include KalibroRecord | 2 | include KalibroRecord |
3 | 3 | ||
4 | def self.public_or_owned_by_user(user=nil) | 4 | def self.public_or_owned_by_user(user=nil) |
5 | - query = if user | ||
6 | - ReadingGroupAttributes.where("user_id == ? OR public", user.id) | ||
7 | - else | ||
8 | - ReadingGroupAttributes.where(public: true) | ||
9 | - end | 5 | + reading_group_attributes = ReadingGroupAttributes.where(public: true) |
6 | + reading_group_attributes += ReadingGroupAttributes.where(user_id: user.id, public: false) if user | ||
10 | 7 | ||
11 | - query.map { |rg_attr| | 8 | + reading_group_attributes.map { |reading_group_attribute| |
12 | begin | 9 | begin |
13 | - self.find(rg_attr.reading_group_id) | 10 | + self.find(reading_group_attribute.reading_group_id) |
14 | rescue KalibroClient::Errors::RecordNotFound | 11 | rescue KalibroClient::Errors::RecordNotFound |
15 | nil | 12 | nil |
16 | end | 13 | end |
app/views/metric_configurations/_form.html.erb
@@ -34,7 +34,7 @@ | @@ -34,7 +34,7 @@ | ||
34 | <div class="form-row"> | 34 | <div class="form-row"> |
35 | <div class="field-container"> | 35 | <div class="field-container"> |
36 | <%= f.label :reading_group_id, class: 'control-label' %> | 36 | <%= f.label :reading_group_id, class: 'control-label' %> |
37 | - <%= f.select( :reading_group_id, reading_groups, {class: 'form-control'} ) %> | 37 | + <%= f.select( :reading_group_id, reading_group_options, {class: 'form-control'} ) %> |
38 | </div> | 38 | </div> |
39 | <div class="help-container"> | 39 | <div class="help-container"> |
40 | <p> | 40 | <p> |
features/step_definitions/kalibro_configuration_steps.rb
@@ -13,6 +13,7 @@ end | @@ -13,6 +13,7 @@ end | ||
13 | Given(/^I have a sample configuration$/) do | 13 | Given(/^I have a sample configuration$/) do |
14 | @kalibro_configuration = FactoryGirl.create(:kalibro_configuration) | 14 | @kalibro_configuration = FactoryGirl.create(:kalibro_configuration) |
15 | FactoryGirl.create(:kalibro_configuration_attributes, {id: nil, user_id: nil, kalibro_configuration_id: @kalibro_configuration.id}) | 15 | FactoryGirl.create(:kalibro_configuration_attributes, {id: nil, user_id: nil, kalibro_configuration_id: @kalibro_configuration.id}) |
16 | + | ||
16 | end | 17 | end |
17 | 18 | ||
18 | Given(/^I own a sample configuration$/) do | 19 | Given(/^I own a sample configuration$/) do |
spec/models/kalibro_configuration_spec.rb
@@ -34,13 +34,11 @@ describe KalibroConfiguration, :type => :model do | @@ -34,13 +34,11 @@ describe KalibroConfiguration, :type => :model do | ||
34 | kalibro_configurations.each do |kc| | 34 | kalibro_configurations.each do |kc| |
35 | KalibroConfiguration.stubs(:find).with(kc.id).returns(kc) | 35 | KalibroConfiguration.stubs(:find).with(kc.id).returns(kc) |
36 | end | 36 | end |
37 | + | ||
38 | + KalibroConfigurationAttributes.expects(:where).with(public: true).returns(public_attrs) | ||
37 | end | 39 | end |
38 | 40 | ||
39 | context 'when user is not provided' do | 41 | context 'when user is not provided' do |
40 | - before do | ||
41 | - KalibroConfigurationAttributes.expects(:where).with(public: true).returns(public_attrs) | ||
42 | - end | ||
43 | - | ||
44 | it 'should find all public reading groups' do | 42 | it 'should find all public reading groups' do |
45 | expect(KalibroConfiguration.public).to eq(public_kalibro_configurations) | 43 | expect(KalibroConfiguration.public).to eq(public_kalibro_configurations) |
46 | end | 44 | end |
@@ -48,7 +46,7 @@ describe KalibroConfiguration, :type => :model do | @@ -48,7 +46,7 @@ describe KalibroConfiguration, :type => :model do | ||
48 | 46 | ||
49 | context 'when user is provided' do | 47 | context 'when user is provided' do |
50 | before do | 48 | before do |
51 | - KalibroConfigurationAttributes.expects(:where).with(kind_of(String), one_user.id).returns(ones_or_public_attrs) | 49 | + KalibroConfigurationAttributes.expects(:where).with(user_id: one_user.id, public: false).returns([ones_private_attrs]) |
52 | end | 50 | end |
53 | 51 | ||
54 | it 'should find all public and owned reading groups' do | 52 | it 'should find all public and owned reading groups' do |
spec/models/reading_group_spec.rb
@@ -79,13 +79,11 @@ describe ReadingGroup, :type => :model do | @@ -79,13 +79,11 @@ describe ReadingGroup, :type => :model do | ||
79 | reading_groups.each do |rg| | 79 | reading_groups.each do |rg| |
80 | ReadingGroup.stubs(:find).with(rg.id).returns(rg) | 80 | ReadingGroup.stubs(:find).with(rg.id).returns(rg) |
81 | end | 81 | end |
82 | + | ||
83 | + ReadingGroupAttributes.expects(:where).with(public: true).returns(public_attrs) | ||
82 | end | 84 | end |
83 | 85 | ||
84 | context 'when user is not provided' do | 86 | context 'when user is not provided' do |
85 | - before do | ||
86 | - ReadingGroupAttributes.expects(:where).with(public: true).returns(public_attrs) | ||
87 | - end | ||
88 | - | ||
89 | it 'should find all public reading groups' do | 87 | it 'should find all public reading groups' do |
90 | expect(ReadingGroup.public).to eq(public_reading_groups) | 88 | expect(ReadingGroup.public).to eq(public_reading_groups) |
91 | end | 89 | end |
@@ -93,7 +91,7 @@ describe ReadingGroup, :type => :model do | @@ -93,7 +91,7 @@ describe ReadingGroup, :type => :model do | ||
93 | 91 | ||
94 | context 'when user is provided' do | 92 | context 'when user is provided' do |
95 | before do | 93 | before do |
96 | - ReadingGroupAttributes.expects(:where).with(kind_of(String), one_user.id).returns(ones_or_public_attrs) | 94 | + ReadingGroupAttributes.expects(:where).with(user_id: one_user.id, public: false).returns([ones_private_attrs]) |
97 | end | 95 | end |
98 | 96 | ||
99 | it 'should find all public and owned reading groups' do | 97 | it 'should find all public and owned reading groups' do |