add conditional to show reports menu tab

[ccarruitero]

This fix to show menu in admin even when user is not logged.

[aitbw]

Hey @ccarruitero, thanks a lot for this PR! Can you elaborate a little bit on why this patch is needed?

Also, can you have a look at the specs? Seems like one of them is failing

[jtapia]

@ccarruitero thanks for this PR, this change would remove the non-admin user roles ability to see the reports, I think this is not necessary because as non-admin user rol you be able to see the orders, so the reports related to orders as well, what do you think? @aitbw

[ccarruitero]

@jtapia until I know all admin tabs are hidden for users that can't admin the resource. https://github.com/solidusio/solidus/blob/master/backend/app/models/spree/backend_configuration.rb#L94

@aitbw looking into the spec failing, I found that you are using a custom authorization in that spec.
Not sure, why is necessary all that context?

Isn't a critical change anyway. You can close if prefer

[jtapia]

@ccarruitero basically is the same behavior like here https://github.com/solidusio/solidus/blob/e6f648f8034443600428be26ffe611833a3e50b2/backend/spec/features/admin/homepage_spec.rb#L68

you can fix the broken spec changing the to have_link to not_to have_link here https://github.com/solidusio-contrib/solidus_reports/blob/master/spec/features/admin/homepage_spec.rb#L33 because of the condition https://github.com/solidusio-contrib/solidus_reports/pull/4/files#diff-77000fc48df08c22aa7f9c6fc6de1d49R8

[ccarruitero]

@jtapia I just did the change that you proposed.

Aside, I was thinking that an alternative fix could be add the permission into the custom authorization block, but is not working. Do you think I missing something?

[kennyadsl]

Restarting Travis, now it should be all green. It was failing for an issue in 2.8 that has been fixed recently.

[spaghetticode]

@ccarruitero looks good to me, thank you 👍

[cedum]

Can we merge this one?

[spaghetticode]

@ccarruitero can you please rebase with master? After that, we can merge the PR... thank you!

[ccarruitero]

@spaghetticode I just updated the branch