Commit fdac9062c1d2ba9d42084c160b5c9ca1f34ece4b
1 parent
8dcf085c
Exists in
colab
and in
4 other branches
ReadingGroup#destroy with attributes memoization clean up and covered by unit tests
If the memoization was left it could lead to inconsistencies after destroying. Removed forgotten print. Signed off by: Heitor Reis <marcheing@gmail.com>
Showing
3 changed files
with
29 additions
and
2 deletions
Show diff stats
app/models/reading_group.rb
| @@ -25,6 +25,7 @@ class ReadingGroup < KalibroClient::Entities::Configurations::ReadingGroup | @@ -25,6 +25,7 @@ class ReadingGroup < KalibroClient::Entities::Configurations::ReadingGroup | ||
| 25 | 25 | ||
| 26 | def destroy | 26 | def destroy |
| 27 | attributes.destroy unless attributes.nil? | 27 | attributes.destroy unless attributes.nil? |
| 28 | + @attributes = nil | ||
| 28 | super | 29 | super |
| 29 | end | 30 | end |
| 30 | end | 31 | end |
features/step_definitions/reading_group_steps.rb
| @@ -61,6 +61,7 @@ Then(/^I should be in the Edit Reading Group page$/) do | @@ -61,6 +61,7 @@ Then(/^I should be in the Edit Reading Group page$/) do | ||
| 61 | end | 61 | end |
| 62 | 62 | ||
| 63 | Then(/^the Sample Reading Group should not be there$/) do | 63 | Then(/^the Sample Reading Group should not be there$/) do |
| 64 | + expects(@reading_group.attributes).to be_nil | ||
| 64 | expect { ReadingGroup.find(@reading_group.id) }.to raise_error | 65 | expect { ReadingGroup.find(@reading_group.id) }.to raise_error |
| 65 | end | 66 | end |
| 66 | 67 |
spec/models/reading_group_spec.rb
| @@ -45,6 +45,33 @@ describe ReadingGroup, :type => :model do | @@ -45,6 +45,33 @@ describe ReadingGroup, :type => :model do | ||
| 45 | expect(subject.readings).to include(reading) | 45 | expect(subject.readings).to include(reading) |
| 46 | end | 46 | end |
| 47 | end | 47 | end |
| 48 | + | ||
| 49 | + describe 'destroy' do | ||
| 50 | + context 'when attributes exist' do | ||
| 51 | + let!(:reading_group_attributes) { FactoryGirl.build(:reading_group_attributes) } | ||
| 52 | + let!(:reading_group) { reading_group_attributes.reading_group } | ||
| 53 | + | ||
| 54 | + it 'should be destroyed' do | ||
| 55 | + reading_group.expects(:attributes).twice.returns(reading_group_attributes) | ||
| 56 | + reading_group_attributes.expects(:destroy) | ||
| 57 | + KalibroClient::Entities::Configurations::ReadingGroup.any_instance.expects(:destroy).returns(reading_group) | ||
| 58 | + reading_group.destroy | ||
| 59 | + end | ||
| 60 | + | ||
| 61 | + it 'is expected to clean the attributes memoization' do | ||
| 62 | + # Call attributes once so it memoizes | ||
| 63 | + ReadingGroupAttributes.expects(:find_by).with(reading_group_id: reading_group.id).returns(reading_group_attributes) | ||
| 64 | + expect(reading_group.attributes).to eq(reading_group_attributes) | ||
| 65 | + | ||
| 66 | + # Destroying | ||
| 67 | + reading_group.destroy | ||
| 68 | + | ||
| 69 | + # The expectation call will try to find the attributes on the database which should be nil since it was destroyed | ||
| 70 | + ReadingGroupAttributes.expects(:find_by).with(reading_group_id: reading_group.id).returns(nil) | ||
| 71 | + expect(reading_group.attributes).to be_nil | ||
| 72 | + end | ||
| 73 | + end | ||
| 74 | + end | ||
| 48 | end | 75 | end |
| 49 | 76 | ||
| 50 | describe 'class methods' do | 77 | describe 'class methods' do |
| @@ -86,8 +113,6 @@ describe ReadingGroup, :type => :model do | @@ -86,8 +113,6 @@ describe ReadingGroup, :type => :model do | ||
| 86 | end | 113 | end |
| 87 | 114 | ||
| 88 | it 'should find all public and owned reading groups' do | 115 | it 'should find all public and owned reading groups' do |
| 89 | - p all_reading_groups | ||
| 90 | - | ||
| 91 | expect(ReadingGroup.public_or_owned_by_user(user)).to eq(owned_or_public_reading_groups) | 116 | expect(ReadingGroup.public_or_owned_by_user(user)).to eq(owned_or_public_reading_groups) |
| 92 | end | 117 | end |
| 93 | end | 118 | end |