Skip to content
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][17.0] document_page_approval #498

Closed
wants to merge 63 commits into from

Conversation

EdgarRetes
Copy link

cloutierjo and others added 30 commits October 2, 2024 10:39
(../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)
[ADD] Translation file + french
[FIX] employee_id field does not exist
[FIX] user_email field deprecated
[IMP] Split long lines
[ADD] Images
fix act_draft and minor ui fix

remove unused variables

change order of xml data files
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.
* 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
* Rename classes and files to match model names, and removed workflow engine.
* Some fields should be readonly if not in draft.
* Allow Knowledge / Manager to edit previous history names and summary
* Improve ux by adding user_has_drafts to document pages.
* Changes because of implementation of active field on document_page
* Add ir.rules for document.page.history
* Allow to quickly cancel&draft from 'to approve' state
* FIX BUG: am_i_approver was being run as sudo(), hence always giving the user Approving rights.
* FIX hooks
  Because of default='draft' on the state field, state IS NULL wasn't selecting anything.
  Left the condition in case this is changed in the future.
 Add the the group eployee to the rule "Change Request Global" to allow to the user with the rule "Change Request Approver" to create pages
[UPD] Update document_page_approval.pot
To pass test is necesary set approval required to False in demo data
[UPD] Update document_page_approval.pot

[UPD] README.rst
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/
Currently translated at 100.0% (61 of 61 strings)

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/it/
Currently translated at 100.0% (61 of 61 strings)

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/it/
OCA-git-bot and others added 14 commits October 2, 2024 10:39
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/
Currently translated at 100.0% (61 of 61 strings)

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/it/
Currently translated at 100.0% (61 of 61 strings)

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/es/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/
Currently translated at 100.0% (60 of 60 strings)

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/pt/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/
Currently translated at 100.0% (62 of 62 strings)

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/it/
Currently translated at 100.0% (62 of 62 strings)

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/es/
Currently translated at 100.0% (62 of 62 strings)

Translation: knowledge-16.0/knowledge-16.0-document_page_approval
Translate-URL: https://translation.odoo-community.org/projects/knowledge-16-0/knowledge-16-0-document_page_approval/sv/
@max3903 max3903 added this to the 17.0 milestone Oct 8, 2024
@max3903 max3903 self-assigned this Oct 8, 2024
@max3903
Copy link
Member

max3903 commented Oct 8, 2024

/ocabot migration document_page_approval

@OCA-git-bot OCA-git-bot mentioned this pull request Oct 8, 2024
9 tasks
@max3903
Copy link
Member

max3903 commented Oct 8, 2024

@EdgarRetes I get an error message when editing a controlled category:
AttributeError: 'document.page.history' object has no attribute 'message_post_with_template'

@max3903
Copy link
Member

max3903 commented Oct 8, 2024

/ocabot migration document_page_approval

Copy link
Member

@victoralmau victoralmau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review:

  • The document_page changes must be correct and not just comment code.
  • Can you update the example images (they are quite old)?
  • All changes must be in the Migration to 17.0 commit
  • Can you squahs the administrative commits?

Comment on lines 138 to 139
action = self.env.ref("document_page_approval.action_change_requests")
action = action.sudo().read()[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
action = self.env.ref("document_page_approval.action_change_requests")
action = action.sudo().read()[0]
action = self.env["ir.actions.act_window"]._for_xml_id(
"document_page_approval.action_change_requests"
)

@@ -0,0 +1 @@
id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This empty file makes no sense, I think the best thing to do is to delete it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this file is necessary for the tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not necessary and does nothing, remove it.

type="object"
string="Send to Review"
class="oe_highlight"
invisible="not is_approval_required or not am_i_owner or state not in 'draft'"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
invisible="not is_approval_required or not am_i_owner or state not in 'draft'"
invisible="not is_approval_required or not am_i_owner or state != 'draft'"

name="action_draft"
type="object"
string="Back to draft"
invisible="(not am_i_owner or not am_i_approver) and (state not in 'cancelled')"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
invisible="(not am_i_owner or not am_i_approver) and (state not in 'cancelled')"
invisible="(not am_i_owner or not am_i_approver) and (state != 'cancelled')"

name="action_draft"
type="object"
string="Back to draft"
invisible="(not am_i_owner or not am_i_approver) and (state not in 'to approve')"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
invisible="(not am_i_owner or not am_i_approver) and (state not in 'to approve')"
invisible="(not am_i_owner or not am_i_approver) and (state != 'to approve')"

@EdgarRetes EdgarRetes force-pushed the 17.0-mig-document_page_approval branch from bcf2c25 to 416b8e4 Compare November 6, 2024 20:37
@EdgarRetes EdgarRetes force-pushed the 17.0-mig-document_page_approval branch from 416b8e4 to 81725ef Compare November 6, 2024 20:52
@@ -66,11 +66,5 @@ def _get_diff(self, v1, v2):
context=True,
)

# TODO: Replace for _compute_display_name
def name_get(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the name_get() method, but do NOT remove the _compute_display_name() method. That's what is stated in the migration guidelines https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-17.0

@@ -0,0 +1 @@
id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not necessary and does nothing, remove it.

@victoralmau
Copy link
Member

Superseded by #509

@max3903 max3903 closed this Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.