[16.0][IMP] base_group_backend: allow read on calendar.event and remove Dashboard menu for backend users

[clementmbr]

Hi @bealdav @petrus-v @FranzPoize @tansadio

I tried to use the base_group_backend module in a very basic Odoo 16 instance with few modules installed like sale.

It appeared that I had a non-blocking Error message displaying (and disappear instantly) everytime I load a page, because of some missing access on calendar.event and calendar.attendee models.
I also had a non-desired dashboard menu from the spreadsheet_dashboard module.

I don't know if these problem appeared because of some recent updates on Odoo 16 or just because of the sale module.

I know it's better to avoid unnecessary dependencies as much as possible, and as this module doesn't need the sale module to work, it could be theoretically better to make a glue module to solve these problems only "in the case of the sale module installed"...

But I though that as the modules calendar and spreadsheet_dashboard are so basic and used as a dependency in so many modules, it could be easier to add this fix directly on the base_group_backend without the use of a glue module.

Is it ok for you?

cc @sebastienbeau and #314 #205

[OCA-git-bot]

Hi @FranzPoize, @bealdav,
some modules you are maintaining are being modified, check this out!

[bealdav]

I suppose @petrus-v have a strong opinion here. On my side I'm shared between the 2 options.
If no other answer I vote for your suggestion to be pragmatic but I prefer let others decide here.

[petrus-v]

Same I feel it's a bit shame to add dependency to spreadsheet_dashboard as long sale can leave without spreadsheet_dashboard_sale but won't really oppose as long new access right are properly based on group backend_ui_users which is good to me. We can split it later if someone needs !

[pedrobaeza]

I don't think this module should depend on spreadsheet_dashboard. Any possible extra module adding a wildcard menu will have the same problem. You need to find another solution for not showing extra things in this group. Maybe an allowed list based system in the method that populates the menus.

[clementmbr]

@pedrobaeza thanks for your suggestion.
Thinking about it, it appeared to me that it could me more convenient to only avoid displaying root menus without security groups (for our backend UI users). Do you agree?

I made new commits following this idea. Please approve the changes if it's ok for you! :)

cc @petrus-v @bealdav

[pedrobaeza]

Well, that way I think that you will remove some useful ones. Have an allowed list for being able to customize it.

[clementmbr]

@pedrobaeza

Well, that way I think that you will remove some useful ones. Have an allowed list for being able to customize it.

What kind of list are you thinking about?

When you talked about it I first though of a list in a dedicated function like get_visible_root_menu() that would have to be override in the custom module in order to add the menus you want to make visible.

But, I thought it could be easier (and more intuitive) to just add the group of your Backend UI User to the menu you want to display (if there is any menu with no groups that you want to display, you just add your User's group to it).

I mean that it seemed more natural (and clear) to list my visible menus in the views.xml file of my custom module than overriding a function, but if you think of a better way of "listing" these visible menus, I'll be pleased to hear it.

Thanks!

[pedrobaeza]

OK, put that instructions to set a group on that menus in the README for them to appear, and it can be enough.

[clementmbr]

@pedrobaeza I added a line about this root menus configuration on the README, and seized the opportunity to re-write the whole README as it looked confusing for me when I first discovered the module.

I also removed the dependency on base_install_request and the RoadMap note as it is not necessary anymore.

Hope it's ok for you @petrus-v @bealdav @FranzPoize !

[bealdav]

LGTM

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[petrus-v]

Great doc improvement thanks !

[clementmbr]

@petrus-v @bealdav can we merge the PR now? :)

[bealdav]

Thanks a lot for this fix, improvement on doc
/ocabot merge minor

[OCA-git-bot]

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-326-by-bealdav-bump-minor, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 2caec0e. Thanks a lot for contributing to OCA. ❤️