[16.0][MIG] report_qweb_encrypt: Migration to 16.0 #779

payen000 · 2023-07-24T17:28:23Z

No description provided.

payen000 · 2023-07-24T18:48:17Z

Hello @luisg123v, could you review this, please?

luisg123v

There is only one commit, you're not bringing commit history.

Please follow these instructions to bring commit history.

This is important, because otherwise:

It's hard for reviewer to review the actuall migration changes
Authorship for all existing changes is lost

luisg123v · 2023-08-02T02:28:38Z

report_qweb_encrypt/__manifest__.py

+    "assets": {
+        "web.assets_backend": [
+            "report_qweb_encrypt/static/src/js/report/action_manager_report.esm.js",
+            "report_qweb_encrypt/static/src/js/report/encrypt_dialog.xml",


You're placing a XML file under the js directory.

I think what you should do instead is removing the js directory and place both files inside report, because, starting from OWL structure, files are not placed according to their languages but according to their purpose.

I think this makes sense and is consistent with other modules' structures, I've corrected this, thanks!

luisg123v · 2023-08-02T02:34:02Z

report_qweb_encrypt/i18n/es.po

+msgstr ""
+"Project-Id-Version: Odoo Server 14.0\n"
+"Report-Msgid-Bugs-To: \n"
+"Last-Translator: Automatically generated\n"


Don't touch translation files.

You may regenerate pot file if you would like to. Both pot and po will be regenerated by the bot anyway.

Fixed, thanks!

luisg123v · 2023-08-02T02:35:56Z

report_qweb_encrypt/models/ir_actions_report.py

+    def _get_pdf_password(self, res_ids):
+        encrypt_password = False
+        if self.encrypt == "manual":
+            encrypt_password = self._context.get("encrypt_password", False)


You're removing comments above this line

I originally deleted them because I believed they were no longer relevant, but they still are, so I've added the back, thanks

luisg123v · 2023-08-02T02:37:36Z

report_qweb_encrypt/static/src/js/report/action_manager_report.esm.js

+    let url = `/report/${type}/${action.report_name}`;
+    const actionContext = action.context || {};
+    const userContext = Object.assign(env.services.user.context, {
+        encrypt_password: action.context.encrypt_password,


did you mean encrypted?

No, encrypt_password is the context key that points to the password that is fetched in the backend to encrypt the pdf - here:

reporting-engine/report_qweb_encrypt/models/ir_actions_report.py

Line 49 in e953692

encrypt_password = self._context.get("encrypt_password", False)

so the name was intentional.

luisg123v · 2023-08-02T02:48:23Z

/ocabot migration report_qweb_encrypt

OCA-git-bot · 2023-08-02T05:57:17Z

Sorry @luisg123v you are not allowed to mark the addon tobe migrated.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

moylop260 · 2023-08-02T15:43:22Z

/ocabot migration report_qweb_encrypt

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: reporting-engine-15.0/reporting-engine-15.0-report_qweb_encrypt Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-15-0/reporting-engine-15-0-report_qweb_encrypt/

payen000 · 2023-08-03T16:54:55Z

@luisg123v I've updated this PR, could you review again, please?

luisg123v

I noticed you brought commit history, you just missed one commit: 556e60b

luisg123v · 2023-08-10T15:59:45Z

report_qweb_encrypt/models/ir_actions_report.py

+            except Exception as e:
+                raise ValidationError(
+                    _("Python code used for encryption password is invalid.\n%s")
+                    % self.encrypt_password


Previous version was the correct one, you should use the lazy syntax:

_("...%s..", param)

Instead of:

_("...%s..") % param

Done but in the incorrect commit, it's placed in the pre-commit commit

I figured this was something "pre-commit-ish", so I placed it there, but I've moved it to the other commit, thanks!

luisg123v · 2023-08-10T16:12:24Z

requirements.txt

@@ -1,4 +1,5 @@
 # generated from manifests external_dependencies
 lxml
+PyPDF2


Please remove this dependency from manifest, this is already considered by Odoo's requirements here.

Please take into account this requirements file is autogenerated from module's external dependencies. So, once you remove it from there, it should also be removed from here.

Instead of adding it into the pre-commit commit to then removing it into the migration commit, just don't include it.

That makes sense, I've updated the code, thanks!

Currently translated at 100.0% (11 of 11 strings) Translation: reporting-engine-15.0/reporting-engine-15.0-report_qweb_encrypt Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-15-0/reporting-engine-15-0-report_qweb_encrypt/es/

payen000 · 2023-08-10T17:11:24Z

@luisg123v I've added the missing commit and applied the rest of your suggestions, please let me know if there's anything else missing, thanks!

luisg123v

LGTM 👍

@moylop260 could you review and merge, please?

moylop260

👍

moylop260 · 2023-08-10T22:16:47Z

/ocabot merge nobump

OCA-git-bot · 2023-08-10T22:17:30Z

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-779-by-moylop260-bump-nobump, awaiting test results.

OCA-git-bot · 2023-08-10T22:24:25Z

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

payen000 force-pushed the 16.0-report_qweb_encrypt-mig-payen branch 6 times, most recently from 29f9feb to 7ef5792 Compare July 24, 2023 18:18

payen000 marked this pull request as ready for review July 24, 2023 18:29

payen000 changed the title ~~[MIG] report_qweb_encrypt: Migration to 16.0~~ [16.0][MIG] report_qweb_encrypt: Migration to 16.0 Aug 1, 2023

luisg123v suggested changes Aug 2, 2023

View reviewed changes

OCA-git-bot added this to the 16.0 milestone Aug 2, 2023

OCA-git-bot mentioned this pull request May 10, 2023

Migration to version 16.0 #653

Open

15 tasks

etobella and others added 15 commits August 3, 2023 08:44

[ADD] report_qweb_encrypt

c966229

[ENH] Add option to auto encrypt password based on python syntax

fa771c9

[UPD] Update report_qweb_encrypt.pot

b35d2b8

[UPD] README.rst

0bcdacb

[IMP] : black, isort, prettier

fc38b43

[14.0][MIG] report_qweb_encrypt

575e639

[UPD] Update report_qweb_encrypt.pot

e0a8a8f

[UPD] README.rst

e76616e

Added translation using Weblate (Spanish)

b1ff525

[FIX] report_qweb_encrypt: conflict with bank statement

8a0ed07

report_qweb_encrypt 14.0.1.0.1

004594d

[FIX] report_qweb_encrypt: the case without res_ids

04dac84

[FIX] report_download controller method signature in two modules

d1d625b

[FIX] readme sample

ba5f197

report_qweb_encrypt 14.0.1.1.0

aaafe99

oca-ci and others added 3 commits August 3, 2023 08:44

[UPD] Update report_qweb_encrypt.pot

7687b11

[UPD] README.rst

642a6b0

payen000 marked this pull request as draft August 3, 2023 16:28

payen000 force-pushed the 16.0-report_qweb_encrypt-mig-payen branch 2 times, most recently from ca8b7bd to e953692 Compare August 3, 2023 16:35

payen000 marked this pull request as ready for review August 3, 2023 16:42

payen000 changed the title ~~[16.0][MIG] report_qweb_encrypt: Migration to 16.0~~ [16.0][MIG] report_qweb_encrypt: Migration to 16.0 task#71917 Aug 3, 2023

payen000 changed the title ~~[16.0][MIG] report_qweb_encrypt: Migration to 16.0 task#71917~~ [16.0][MIG] report_qweb_encrypt: Migration to 16.0 Aug 3, 2023

luisg123v suggested changes Aug 10, 2023

View reviewed changes

payen000 force-pushed the 16.0-report_qweb_encrypt-mig-payen branch from e953692 to 381add4 Compare August 10, 2023 17:05

payen000 force-pushed the 16.0-report_qweb_encrypt-mig-payen branch from 381add4 to 842b0af Compare August 10, 2023 17:48

payen000 added 2 commits August 10, 2023 10:52

[IMP] report_qweb_encrypt: pre-commit run

484b258

[MIG] report_qweb_encrypt: Migration to 16.0

065973e

payen000 force-pushed the 16.0-report_qweb_encrypt-mig-payen branch from 842b0af to 065973e Compare August 10, 2023 17:52

luisg123v approved these changes Aug 10, 2023

View reviewed changes

moylop260 approved these changes Aug 10, 2023

View reviewed changes

OCA-git-bot added the approved label Aug 10, 2023

OCA-git-bot added the bot is merging ⏳ label Aug 10, 2023

OCA-git-bot merged commit 59698bf into OCA:16.0 Aug 10, 2023
6 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Aug 10, 2023

moylop260 deleted the 16.0-report_qweb_encrypt-mig-payen branch August 10, 2023 22:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][MIG] report_qweb_encrypt: Migration to 16.0 #779

[16.0][MIG] report_qweb_encrypt: Migration to 16.0 #779

payen000 commented Jul 24, 2023

payen000 commented Jul 24, 2023

luisg123v left a comment

luisg123v Aug 2, 2023

payen000 Aug 3, 2023

luisg123v Aug 2, 2023

payen000 Aug 3, 2023

luisg123v Aug 2, 2023

payen000 Aug 3, 2023 •

edited

Loading

luisg123v Aug 2, 2023

payen000 Aug 3, 2023

luisg123v commented Aug 2, 2023

OCA-git-bot commented Aug 2, 2023

moylop260 commented Aug 2, 2023

payen000 commented Aug 3, 2023

luisg123v left a comment

luisg123v Aug 10, 2023

payen000 Aug 10, 2023

luisg123v Aug 10, 2023

payen000 Aug 10, 2023

luisg123v Aug 10, 2023

payen000 Aug 10, 2023

luisg123v Aug 10, 2023

payen000 Aug 10, 2023

payen000 commented Aug 10, 2023

luisg123v left a comment

moylop260 left a comment

moylop260 commented Aug 10, 2023

OCA-git-bot commented Aug 10, 2023

OCA-git-bot commented Aug 10, 2023

[16.0][MIG] report_qweb_encrypt: Migration to 16.0 #779

[16.0][MIG] report_qweb_encrypt: Migration to 16.0 #779

Conversation

payen000 commented Jul 24, 2023

payen000 commented Jul 24, 2023

luisg123v left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

payen000 Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luisg123v commented Aug 2, 2023

OCA-git-bot commented Aug 2, 2023

moylop260 commented Aug 2, 2023

payen000 commented Aug 3, 2023

luisg123v left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

payen000 commented Aug 10, 2023

luisg123v left a comment

Choose a reason for hiding this comment

moylop260 left a comment

Choose a reason for hiding this comment

moylop260 commented Aug 10, 2023

OCA-git-bot commented Aug 10, 2023

OCA-git-bot commented Aug 10, 2023

payen000 Aug 3, 2023 •

edited

Loading