web
web copied to clipboard
Make web_responsive compatible with Enterprise Edition
The web_responsive module per se does work alongside web_enterprise, so I removed the "excludes" from the manifest.
I also created a new module, web_responsive_enterprise, that enables the searchable Apps Menu on the Enterprise Edition. I tried to be as little intrusive as possible. I only added a method named _getInitialOpenState in the original AppsMenu so as to keep the original behaviors in each edition: in the Community Edition, the initial app (whatever it is) is displayed at startup, while in the Enterprise Edition, the Apps Menu is displayed at startup.
Hi @Yajo, @SplashS, @Tardo, some modules you are maintaining are being modified, check this out!
Not sure what to do about the remaining failed checks:
- the pre-commit checks complain about missing files in the
setupdirectories, but other modules do not have it either. - the tests abd builds check fail because the
web_enterprisemodule is missing
the pre-commit checks complain about missing files in the setup directories, but other modules do not have it either.
you can run locally pre-commit run -a and it should fix all the errors.
the tests abd builds check fail because the web_enterprise module is missing
That's quite normal. You are proposing a module that depends on web_enterprise module, that is not available. I think you have to remove this module from your PR and maintain it separately. (AFAIK, there is no possiblity to run tests with EE module, and runboat will not work neither).
regards.
Thanks Sylvain, I just ran the pre-commit hooks locally and the check now passes.
Regarding the dependency on the web_enterprise module, I understand that the checks will not pass. If it is OCA policy not to host modules that depend on the Enterprise Edition, then I'll remove it from the PR and host it elsewhere as you suggest. However the main point of the PR was to make the web_responsive module work well with EE, so without the new web_responsive_enterprise module, the contribution to the web_responsive module is rather minor. I guess I could just add a pointer to where I host the other module. Maybe the logic I added in the https://github.com/OCA/web/pull/2652/commits/fbef752547a76df40b119008c685385360b602c0 commit could be brought to the web_responsive module though. What it does is display the Apps menu instead of the default app if there is no action specified in the URL, which typically occurs when one opens the "home" URL of the Odoo instance. What do you think? It would actually make the code a bit more concise.
That logic was there, but removed, as it's problematic sometimes. If you put it, it should be an opt-in option. About the reference to the other module outside OCA and depending on enterprise, they can't be done here by the rules.
Anyway, note that most of us are pro-community and open source.
Hi Pedro, sure, I can update the PR so as to make this behavior optional. Actually I had already started and I stashed those changes to upload a simpler version of the PR, so it shouldn't be long. What I wasn't able to do however was to force the page to reload when the value of the config field (in the user's preferences) is changed, like it does when the language changes for instance. It seems that this is implemented for some reason in the hr module; I tried to replicate what I found there, but it didn't work. Any idea how to achieve this?
I can't say, sorry, but it's not a big deal. Just let a manual refresh to happen.
If you want send a reload from server to client: https://github.com/odoo/odoo/blob/fb42f0a705c83f263714f13e7d7bc3f46ac2d6b3/odoo/addons/base/models/ir_module.py#L608
If you want simple reload on client side: window.location.reload() (pass true to bypass firefox cache)
Hi @Yajo, @SplashS, @Tardo, some modules you are maintaining are being modified, check this out!
Okay I think the PR is decent now:
- I removed the
web_responsive_enterprisemodule, so that there is no dependency on Enterprise Edition in this repo - I made the initial display state of the Apps Menu configurable in the user profile
Not sure what to do about the code coverage checks, is it a strict requirement that they pass? If so, I'll need help I think.
@Tardo wrt the reload, what I was looking for is a concise way to reload the client when a certain field on the res.users model is changed. Returning an action with 'tag': 'reload' would work for instance with a button click or something requiring user interaction (as far as I know), which I do not want. The reload on language switch is implemented in the hr module for some reason: https://github.com/odoo/odoo/blob/71bd4f1378f56b333830cbc5aa11c1c55dcefcc3/addons/hr/static/src/views/profile_form_view.js#L12
I tried to do something similar but it didn't work.
Hi. There are conflicts. Could you rebase and solve them ? Thanks !