OP#59817 Remove activity menu for anonymous users with login required #17314
Conversation
top4ek
changed the title
| context "when login_required", with_settings: { login_required: true } do | ||
| it "redirects to login" do | ||
| expect(page).to have_current_path /login/ | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Please do not simply delete this test because the redirect should still work and be tested. I think you just have to set let(:open_menu) { false } for this context to work again.
There was a problem hiding this comment.
Not sure why :top_menu is forced before examples, but all other specs fail even if we don't open top menu there.
| @@ -215,6 +215,8 @@ def more_top_menu_items | |||
|
|
|||
| def module_top_menu_item_groups | |||
| items = more_top_menu_items | |||
| return items if items.empty? | |||
There was a problem hiding this comment.
Even though an empty array is an empty array in ruby, I would suggest not returning the empty items array here, but the item_groups array. You can do this in a way, that add & in line 223: unless items.first&.heading?. If you do it that way the reduce is operating again on an empty list or containing a single item.
Let me know if I should explain more in depth.
There was a problem hiding this comment.
Doing that gives us empty menu dropdown as unless items.first&.heading? adds to item_groups [{:title=>nil, :items=>[]}]
There was a problem hiding this comment.
As a variant?
def module_top_menu_item_groups
item_groups = []
items = more_top_menu_items
return item_groups if items.blank?
OP#59817
What are you trying to accomplish?
Remove top menu Activity item for anonymous users when login is required as it's leads to nowhere(same login page).
What approach did you choose and why?
Fresh installation shows only Activity item in top menu and leads to the same login page when login is required., so I removed that item when user is anonymous and login is required.
As we don't have any items in top menu in this case and menu is not rendered, I've removed spec for that.
Returning empty array when there's no items thus we don't need to group them(otherwise we have 500 with heading? access to nil)