widget generate_report_result should work regardless of whether the owner has a group

[d-m-u]

add safe operator to group rather than breaking on the last line if owner has no group

nothing preventing group from being nil, so rpt.build_create_results(:userid => userid_for_result, :report_source => WIDGET_REPORT_SOURCE, :timezone => timezone, :miq_group_id => group.id) is pretty fragile

this might inadvertently cause other reporting bugs, i really do not know

previously, anyone who hit this would have to manually assign the user to a group to fix and probably questioned why that is necessary to update a widget

_{"tomorrow is a work day btw"}

" MIQ(MiqWidget#generate_one_content_for_user) Widget: [Top Memory Consumers (weekly)] ID: [1000000000017] Failed for [User] [Managed Cloud] with error: [NoMethodError] [undefined method `id' for nil:NilClass]"

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

[d-m-u]

@miq-bot assign @kbrock
@miq-bot add_label bug
yes there is a related bz, no, i'm not going to link it until after i get feedback because this clearly doesn't fix anything

[NickLaMuro]

Heh, I was going to ask "why aren't you using string interpolation for this", until I realized that wouldn't work with a string translation, so this really is what you need to do. 👍

(also, clearly I don't deal with translated strings enough, because I am sure this would have been obvious to others)

[d-m-u]

evm.log:[----] E, [2020-07-24T04:01:10.422801 #125822:dbaf4c] ERROR -- : MIQ(MiqWidget#generate_one_content_for_user) Widget: [Top Memory Consumers (weekly)] ID: [redacted] Failed for [User] [also redacted] with error: [NoMethodError] [undefined method `id' for nil:NilClass]
evm.log:[----] E, [2020-07-24T04:01:10.423088 #125822:dbaf4c] ERROR -- : [NoMethodError]: undefined method `id' for nil:NilClass  Method:[block (2 levels) in <class:LogProxy>]
evm.log:[----] E, [2020-07-24T04:01:10.423269 #125822:dbaf4c] ERROR -- : /var/www/miq/vmdb/app/models/miq_widget.rb:321:in `generate_report_result'
/var/www/miq/vmdb/app/models/miq_widget.rb:284:in `generate_one_content_for_user'
/var/www/miq/vmdb/app/models/miq_widget/content_generator.rb:32:in `block in determine_content'
/var/www/miq/vmdb/app/models/miq_widget/content_generator.rb:32:in `collect'
/var/www/miq/vmdb/app/models/miq_widget/content_generator.rb:32:in `determine_content'
/var/www/miq/vmdb/app/models/miq_widget/content_generator.rb:8:in `generate'
/var/www/miq/vmdb/app/models/miq_widget.rb:223:in `generate_content'
/var/www/miq/vmdb/app/models/miq_queue.rb:455:in `block in dispatch_method'
/usr/share/ruby/timeout.rb:93:in `block in timeout'
/usr/share/ruby/timeout.rb:33:in `block in catch'
/usr/share/ruby/timeout.rb:33:in `catch'
/usr/share/ruby/timeout.rb:33:in `catch'
/usr/share/ruby/timeout.rb:108:in `timeout'
/var/www/miq/vmdb/app/models/miq_queue.rb:453:in `dispatch_method'
/var/www/miq/vmdb/app/models/miq_queue.rb:430:in `block in deliver'
/var/www/miq/vmdb/app/models/user.rb:275:in `with_user_group'
/var/www/miq/vmdb/app/models/miq_queue.rb:430:in `deliver'
/var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:104:in `deliver_queue_message'
/var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:137:in `deliver_message'
/var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:155:in `block in do_work'
/var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:149:in `loop'
/var/www/miq/vmdb/app/models/miq_queue_worker_base/runner.rb:149:in `do_work'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:329:in `block in do_work_loop'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:326:in `loop'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:326:in `do_work_loop'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:153:in `run'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:127:in `start'
/var/www/miq/vmdb/app/models/miq_worker/runner.rb:22:in `start_worker'
/var/www/miq/vmdb/app/models/miq_worker.rb:408:in `block in start_runner_via_fork'
/opt/rh/cfme-gemset/gems/nakayoshi_fork-0.0.4/lib/nakayoshi_fork.rb:23:in `fork'
/opt/rh/cfme-gemset/gems/nakayoshi_fork-0.0.4/lib/nakayoshi_fork.rb:23:in `fork'
/var/www/miq/vmdb/app/models/miq_worker.rb:406:in `start_runner_via_fork'
/var/www/miq/vmdb/app/models/miq_worker.rb:396:in `start_runner'
/var/www/miq/vmdb/app/models/miq_worker.rb:447:in `start'
/var/www/miq/vmdb/app/models/miq_worker.rb:277:in `start_worker'
/var/www/miq/vmdb/app/models/miq_worker.rb:154:in `block in sync_workers'
/var/www/miq/vmdb/app/models/miq_worker.rb:154:in `times'
/var/www/miq/vmdb/app/models/miq_worker.rb:154:in `sync_workers'
/var/www/miq/vmdb/app/models/miq_server/worker_management/monitor.rb:53:in `block in sync_workers'

[kbrock]

we store data for this report by user and by group. so all users have the ability to collect the results where possible rather than store data for each and every user

I'd prefer we punt rather than stop processing here

[kbrock]

ugh. thought we had gotten rid of the current_group gook

[kbrock]

lets not blow up here

I'd change the group.id to group&.id

sorry nick

[miq-bot]

Checked commit d-m-u@2417c37 with ruby 2.6.3, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. ⭐

[NickLaMuro]

sorry nick

@kbrock No need to apologize. Sounds like you have a better grasp and stronger opinion on this code, so I defer to your judgement.

---

@d-m-u @kbrock without looking at the code, (I lied, I looked at the code a bit) is this possibly something that needs to be addressed in the Group model, where we cascade deletes when a group is deleted? Not sure if this is even possible since it looks like the widget is associated to a report in some way, and not directly related to a widget.

¯\(°_o)/¯

[d-m-u]

Marked as work in progress/don't merge because I think @kbrock said that this might have unintended consequences for other users running reports. I'd really like to fix this anyway. I think despite the chance of unintended consequences we need this fix but that call is more up to @dmetzger57 and @tinaafitz at this point. Since I stole this from @yrudman (pto this week) perhaps his input would be helpful here as well. It's Friday night though so I'll get back to this on Monday.

[d-m-u]

hey @kbrock could I please get you to address the risk of this fix here? I thought we said that this might change what reports are displayed for other users but it'd be great to get some info from you. We ought to be able to merge to upstream and simply not backport, if it's too big of a change.