New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MIG] document_page: Migration to version 10.0 #120
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only small nits
<field name="name">Create Menu</field> | ||
<field name="model">document.page.create.menu</field> | ||
<field name="arch" type="xml"> | ||
<form string="Create Menu" version="7.0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the string
and version
attributes can go away I think
<field name="name">Show Difference</field> | ||
<field name="model">wizard.document.page.history.show_diff</field> | ||
<field name="arch" type="xml"> | ||
<form string="Difference" version="7.0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove string
and version
attribute
@@ -43,7 +32,5 @@ | |||
'demo': [ | |||
'demo/document_page.xml' | |||
], | |||
'installable': False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's recommended to keep the 'installable': True
flag here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
who recommends that? I usually ask people to remove keys when set to the default value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a special case due to the OCA migrate_branch script that was not covered in the regexp expressions, so it could lead to broken manifest files, but I think @StefanRijnhart fixed it when he adapted the script for v10, so it's no more an issue (but needs confirmation by his part).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the script now detects if the key is not present and adds it in that case. So removing it is harmless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, keeping the install key makes it much easier to use grep to list installable/non installable modules.
But that is just a personal preference and I won't pick on this then.
@@ -43,7 +32,5 @@ | |||
'demo': [ | |||
'demo/document_page.xml' | |||
], | |||
'installable': False, | |||
'auto_install': False, | |||
'css': ['static/src/css/document_page.css'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a valid manifest key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, not since 7.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please replace this by an XML extending the web client assets to add the intended CSS file.
readonly=True) | ||
|
||
write_uid = fields.Many2one( | ||
'res.users', | ||
"Last Contributor", | ||
select=True, | ||
index=True, | ||
readonly=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open question: do we really need to index any or all these these fields?
</tree> | ||
</field> | ||
</record> | ||
<menuitem name="Pages" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the top menu ("App") name.
Should we rather name it "Documents" or "Knowledge" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@max3903 what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dreispt I am fine with Knowledge
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dreispt I double-checked and this is not the top menu name. It's inheriting from knowledge.menu_document
Thanks for your review everyone. I will fix the module today or tomorrow. |
339baef
to
b558936
Compare
I believe everything is fixed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment inline, but nothing major. Code + functional 👍
Looks like there's a warning in Runbot though:
2017-01-19 20:42:19,767 211 WARNING openerp_test odoo.addons.base.ir.ir_model: Creating the ir.model.data group_document_user in module base instead of knowledge.
Curious - is there a reason this security is being created in a module that we don't own?
@@ -40,7 +40,7 @@ Bug Tracker | |||
|
|||
Bugs are tracked on `GitHub Issues <https://github.com/OCA/Knowledge/issues>`_. | |||
In case of trouble, please check there if your issue has already been reported. | |||
If you spotted it first, help us smashing it by providing a detailed and welcomed feedback `here <https://github.com/OCA/knowledge/issues/new?body=module:%20document_page%0Aversion:%209.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_. | |||
If you spotted it first, help us smashing it by providing a detailed and welcomed feedback `here <https://github.com/OCA/knowledge/issues/new?body=module:%20document_page%0Aversion:%2010.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update to new readme template
@lasley Good catch! Thanks. All fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on runbot. Thanks!
@hbrunn Can you update your review here? Thank you! |
Holger's concerns were fixed. Merging. |
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [UPD] README.rst * Translated using Weblate (Portuguese (Brazil)) Currently translated at 93.9% (31 of 33 strings) Translation: knowledge-11.0/knowledge-11.0-knowledge Translate-URL: https://translation.odoo-community.org/projects/knowledge-11-0/knowledge-11-0-knowledge/pt_BR/ * [ADD] document_page from odoo/7.0 * use new API * [FIX+IMP] document_page: * Add dependency to knowledge * Adding hack in document_page_history to allow calling method that don't exists * document_page_history don't have _sql attribute * Filter more fields * Slovene translations added * [FIX+IMP] document_page: * Changed history widget to html. * Improved views and added followers to pages. * Updated document_page pot and es translations. * document_page:^Cissing dependency. * Translations sync with templates * [MIG] document_page: Migration to 9.0 * [MIG] document_page: Migration to version 10.0 (OCA#120) * [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user * [IMP][10.0][document_page] Change Requests and workflow improvements on documents (OCA#155) * OCA Transbot updated translations from Transifex * [MIG] document_page: Migration to 11.0 * [MIG] document_page: Migration to 11.0 continue migration * [UPD] Update document_page.pot * create document_page_approval module (../7.0pageapproval/ rev 1) fix status bar in page approval (../7.0pageapproval/ rev 2) change approval workflow in page history and started to add showing approved page in normal page (../7.0pageapproval/ rev 3) show last approved content and aproved by (../7.0pageapproval/ rev 4) Fix bug on page creation (../7.0pageapproval/ rev 5) add status in history tree view (../7.0pageapproval/ rev 6) add a need approval field and hide the workflow when no approval needed (../7.0pageapproval/ rev 7) add access validation to approval (../7.0pageapproval/ rev 8) give rigth to Page aprover group on the page_history model and hide the button for unallowed group thought the security isn't enforced a the orm level (../7.0pageapproval/ rev 9) made invisible the prover group when not needed (../7.0pageapproval/ rev 10) add email to aprovers group's members (../7.0pageapproval/ rev 11) * [IMP] Documentation [ADD] Translation file + french [FIX] employee_id field does not exist [FIX] user_email field deprecated * [IMP] Split long lines * [ADD] Images * Fix pep8 * Set document_page_approval as unported * Slovene translation added * [IMP] - Changed string from email template. * [FIX] - document-page-approval: Fixes OCA#60 url not valid when website module is installed. * [UPD] prefix versions with 8.0 * [MIG] Make modules uninstallable * OCA Transbot updated translations from Transifex * [FIX] remove en.po that was erroneously created by transbot * [MIG] document_page_approval * [IMP][10.0][document_page_approval] Change Requests and workflow improvements on documents (OCA#155) * [FIX] View * remove obsolete .pot files [ci skip] * start porting fix act_draft and minor ui fix remove unused variables change order of xml data files * Comment workflow that cause the error It should work but further errors could help understand what's going Commented other necessary part Add back workflow Add dependency to knowledge * [IMP] - Improved views to match document_page improvements. [IMP] - Added notification to followers when a new version is approved. * [ADD] - Updated document_page_approval pot and added es translations. [IMP] - document_page_approval: Updated spanish translation. * Translations sync with templates Translations and templates sync Updated languages * document_page_approval migration from Odoo 8.0 to Odoo 9.0 Lint error corrected test improved lint error corrected in test file readme corrected test improved test of document page history workflow added CI error corrected dreipst comment Migration to new api improved in model's file and data tag remove in xml file code totally migrate to odoo 9.0 api * test bug fixed test error fixed Lint error improved * [MIG] Make modules uninstallable [MIG] Rename manifest files * [ADD] setup.py * [MIG] document_page_approval: Migration to 11.0 * [UPD] Update document_page_approval.pot
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
* [MIG] Migration to version 10.0 * [FIX] Comments and CI errors * [FIX] ValueError: External ID not found in the system: base.menu_base_partner * [FIX] ValueError: Wrong value for ir.actions.act_window.target: 'inlineview' * [FIX] Based on @lasley comments * [FIX] External ID not found in the system: base.group_document_user
Depends on #118