Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not pass nil to the assignment mixin #14713

Merged
merged 1 commit into from
Apr 10, 2017
Merged

Conversation

isimluk
Copy link
Member

@isimluk isimluk commented Apr 10, 2017

BZ

https://bugzilla.redhat.com/show_bug.cgi?id=1440917

Addressing

 [NoMethodError]: undefined method `base_model' for NilClass:Class  Method:[rescue in _async_generate_table]
app/models/mixins/assignment_mixin.rb:146:in `block in get_assigned_for_target'
app/models/mixins/assignment_mixin.rb:146:in `collect'
app/models/mixins/assignment_mixin.rb:146:in `get_assigned_for_target'
app/models/chargeback/rates_cache.rb:13:in `rates'
app/models/chargeback/rates_cache.rb:7:in `get'
app/models/chargeback.rb:9:in `block in build_results_for_report_chargeback'
app/models/chargeback/consumption_history.rb:31:in `block (2 levels) in for_report'

Introduced in 31fac86

Lesson learned: when you see two lists with [].compact above your edit, it is usually good idea to consider using .compact too.

If we merged #14090 we wouldn't see this at customer. 😑 ❗️

Addressing: [NoMethodError]: undefined method `base_model' for NilClass:Class  Method:[rescue in _async_generate_table]
app/models/mixins/assignment_mixin.rb:146:in `block in get_assigned_for_target'
app/models/mixins/assignment_mixin.rb:146:in `collect'
app/models/mixins/assignment_mixin.rb:146:in `get_assigned_for_target'
app/models/chargeback/rates_cache.rb:13:in `rates'
app/models/chargeback/rates_cache.rb:7:in `get'
app/models/chargeback.rb:9:in `block in build_results_for_report_chargeback'
app/models/chargeback/consumption_history.rb:31:in `block (2 levels) in for_report'

Introduced in 31fac86

Lesson learned: when you see two lists with [].compact, it is usually
good idea to use compact too.

If we merged ManageIQ#14090 we wouldn't
see this at customer.
@miq-bot
Copy link
Member

miq-bot commented Apr 10, 2017

Checked commit isimluk@36222c1 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. ⭐

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good, thanks!

@gtanzillo gtanzillo added this to the Sprint 58 Ending Apr 10, 2017 milestone Apr 10, 2017
@gtanzillo gtanzillo merged commit 17e9829 into ManageIQ:master Apr 10, 2017
@isimluk isimluk deleted the bz-tbd branch April 11, 2017 07:52
simaishi pushed a commit that referenced this pull request Apr 11, 2017
Do not pass nil to the assignment mixin
(cherry picked from commit 17e9829)

https://bugzilla.redhat.com/show_bug.cgi?id=1441199
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit 7b145d073db2b540dbece9b4064323f6e8e1ad11
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Mon Apr 10 15:47:21 2017 -0400

    Merge pull request #14713 from isimluk/bz-tbd
    
    Do not pass nil to the assignment mixin
    (cherry picked from commit 17e9829704c2d19dec2787f5c7d11128bfea40b8)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1441199

simaishi pushed a commit that referenced this pull request Apr 11, 2017
Do not pass nil to the assignment mixin
(cherry picked from commit 17e9829)

https://bugzilla.redhat.com/show_bug.cgi?id=1441198
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit bdeaaf101a321e0cee174262b886d08494e79db4
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Mon Apr 10 15:47:21 2017 -0400

    Merge pull request #14713 from isimluk/bz-tbd
    
    Do not pass nil to the assignment mixin
    (cherry picked from commit 17e9829704c2d19dec2787f5c7d11128bfea40b8)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1441198

agrare pushed a commit to agrare/manageiq that referenced this pull request Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants