[11.0] website menu permission easy maintain #658

NL66278 · 2019-08-12T11:34:39Z

This consists of two parts.

a migration done by @gfcapalbo of the 10.0 module by @simahawk
an adaption to ensure just installing the module does not take away access rights for many existing users. Only after consciously setting groups on a menu well permissions change. To edit this easily, the editing of website menu's has been added to the website configuration menu.

The record-rule itself remains the same. Basically as also in the PR made for 8.0 by @hbrunn for this purpose.

hbrunn · 2019-08-13T16:06:10Z

looks good, and when you've changed the tests I think it will be green too. Let's then finally merge a version of this, the 8 and 10 version is hanging around like forever already

NL66278 · 2019-08-14T14:19:23Z

For some reason travis cannot setup database connection:

Creating instance:

createdb: could not connect to database template1: could not connect to server: No such file or directory

	Is the server running locally and accepting

	connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5432"?

Using previous openerp_template database.

hbrunn · 2019-08-14T14:22:33Z

you need to update this branch's .travis.yml. Best adapt to the current version in MQT - the crucial part here is to set a postgres version

rafaelbn · 2019-08-14T16:08:12Z

website_menu_permission/README.rst

+
+#. Go to Website ==> Configuration ==> Website menu's.
+#. Edit the menu that you want to limit to certain groups, and add
+   the names of the groups that should have access.


This explanation is confusing because:

In Website -> Configuration there are 2 different menu entries for the same model website.menu

You need developer mode in order to see "Website menu's" entry. The other entry "Menu" you can see without developer mode.

I think is enough one menu entry and not two.

I only have one menu entry for website menu's. And I am in developer mode. If you see a second option for website menu's can you check from what module that is installed?

that's runbot installing all modules from the repository. I see xmlid website_multi_theme.action_configure_menu_website

simahawk

@NL66278 @gfcapalbo thanks for taking care of this :)
Mostly ok, just some small things :)

simahawk · 2019-09-30T08:40:42Z

website_menu_permission/tests/test_menu_perm.py

+
+class TestMenuPerm(test_common.TransactionCase):
+
+    def setUp(self):


please use SavepointCase and move setUp to setUpClass and replace the env like this:

cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True, no_reset_password=True))

It's going to speed up tests a lot (actually we should use this approach for all tests 👨‍🏫 )

simahawk · 2019-09-30T08:41:18Z

website_menu_permission/security/record_rules.xml

+  <record id="website_menu_access" model="ir.rule">
+    <field name="name">website_menu_permission group access</field>
+    <field name="model_id" ref="model_website_menu"/>
+    <field name="domain_force">['|',('group_ids','in', [g.id for g in user.groups_id]), ('group_ids','=',False)]</field>


this one can use user.groups_id.ids instead of the loop

NL66278 · 2019-09-30T14:14:09Z

@simahawk Changes done!

simahawk

LG, can we squash?

OCA-git-bot · 2019-09-30T14:31:38Z

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). 🤖

1. Prevent just installing this module of making most menu's unavailable for logged in users; 2. Make menu's directly configurable from website settings, as not all menu's are linked to a page; 3. Use group_ids field in new website.menu form to edit groups. With this commit, just installing this module will not change anything in existing permissions. TODO: update tests and README.rst.

NL66278 · 2019-09-30T14:41:55Z

@simahawk I squashed my commits, so that leaves 2.

simahawk · 2019-10-01T07:12:18Z

@rafaelbn fine for you?
@NL66278 what's the plan for website_menu_by_user_status?
The original plan - from my POV - was to make it dependant on this one. Is this still the case?
There are a bunch of PRs that probably need some update or references https://github.com/OCA/website/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+status+

NL66278 · 2019-10-01T09:22:03Z

@simahawk I think the module website_menu_by_user_status does not add really anything, that cannot be done with website_menu_permission. But for some people it might be easier to just have the choice betwee users that are logged in or not. So I think the ..user_status module could be rebased om --.permission, doing away with any overlaps. But that can be done/proposed after the current PR has been merged.

simahawk · 2019-10-01T09:28:37Z

@NL66278 yeah, that was the original plan, also to keep backward compat: *_status module should rely on this one and just have a flag that sets the groups automatically.

simahawk · 2019-10-01T14:46:19Z

/ocabot merge

OCA-git-bot · 2019-10-01T14:46:30Z

This PR looks fantastic, let's merge it!
Prepared branch 11.0-ocabot-merge-pr-658-by-simahawk-bump-no, awaiting test results.

Signed-off-by simahawk

OCA-git-bot · 2019-10-02T00:48:59Z

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

hbrunn added this to the 11.0 milestone Aug 13, 2019

hbrunn added the needs review label Aug 13, 2019

NL66278 force-pushed the 11.0-website_menu_permission-easy-maintain branch from 89bc6fc to 7777985 Compare August 14, 2019 14:55

NL66278 changed the title ~~[WIP][11.0] website menu permission easy maintain~~ [11.0] website menu permission easy maintain Aug 14, 2019

NL66278 mentioned this pull request Aug 14, 2019

[FIX][11.0] website_menu_by_user_status travis warn #618

Closed

hbrunn approved these changes Aug 14, 2019

View reviewed changes

rafaelbn reviewed Aug 14, 2019

View reviewed changes

[ADD] [no task] website_menu_permission

61c74ce

NL66278 force-pushed the 11.0-website_menu_permission-easy-maintain branch from 7777985 to 5377b63 Compare August 29, 2019 10:37

simahawk requested changes Sep 30, 2019

View reviewed changes

simahawk approved these changes Sep 30, 2019

View reviewed changes

OCA-git-bot added approved ready to merge labels Sep 30, 2019

NL66278 force-pushed the 11.0-website_menu_permission-easy-maintain branch from a2f2cb8 to e4404f3 Compare September 30, 2019 14:40

OCA-git-bot added a commit that referenced this pull request Oct 1, 2019

Merge PR #658 into 11.0

3fa238b

Signed-off-by simahawk

OCA-git-bot merged commit e4404f3 into OCA:11.0 Oct 2, 2019

OCA-git-bot added the merged 🎉 label Oct 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[11.0] website menu permission easy maintain #658

[11.0] website menu permission easy maintain #658

NL66278 commented Aug 12, 2019 •

edited

hbrunn commented Aug 13, 2019

NL66278 commented Aug 14, 2019

hbrunn commented Aug 14, 2019

rafaelbn Aug 14, 2019

NL66278 Aug 14, 2019

hbrunn Aug 14, 2019

simahawk left a comment

simahawk Sep 30, 2019

simahawk Sep 30, 2019

NL66278 commented Sep 30, 2019

simahawk left a comment

OCA-git-bot commented Sep 30, 2019

NL66278 commented Sep 30, 2019

simahawk commented Oct 1, 2019

NL66278 commented Oct 1, 2019

simahawk commented Oct 1, 2019

simahawk commented Oct 1, 2019

OCA-git-bot commented Oct 1, 2019

OCA-git-bot commented Oct 2, 2019


		class TestMenuPerm(test_common.TransactionCase):

		def setUp(self):

[11.0] website menu permission easy maintain #658

[11.0] website menu permission easy maintain #658

Conversation

NL66278 commented Aug 12, 2019 • edited

hbrunn commented Aug 13, 2019

NL66278 commented Aug 14, 2019

hbrunn commented Aug 14, 2019

rafaelbn Aug 14, 2019

Choose a reason for hiding this comment

NL66278 Aug 14, 2019

Choose a reason for hiding this comment

hbrunn Aug 14, 2019

Choose a reason for hiding this comment

simahawk left a comment

Choose a reason for hiding this comment

simahawk Sep 30, 2019

Choose a reason for hiding this comment

simahawk Sep 30, 2019

Choose a reason for hiding this comment

NL66278 commented Sep 30, 2019

simahawk left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Sep 30, 2019

NL66278 commented Sep 30, 2019

simahawk commented Oct 1, 2019

NL66278 commented Oct 1, 2019

simahawk commented Oct 1, 2019

simahawk commented Oct 1, 2019

OCA-git-bot commented Oct 1, 2019

OCA-git-bot commented Oct 2, 2019

NL66278 commented Aug 12, 2019 •

edited