-
-
Notifications
You must be signed in to change notification settings - Fork 326
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
[16.0][MIG] pingen: Migration to 16.0 #315
Conversation
68fe609
to
3e2c5f0
Compare
b8945a1
to
515fa7c
Compare
pingen/README.rst
Outdated
A second cron updates the informations of the documents from pingen.com, so we | ||
know which of them have been sent. |
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 now managed through webhooks calls from pingen, can you please update?
pingen/README.rst
Outdated
The authentication token is configured on the company's view. You can also | ||
tick a checkbox if the staging environment (https://stage-api.pingen.com) | ||
should be used. | ||
|
||
The setup of the 2 crons can be changed as well: | ||
|
||
* Run Pingen Document Push | ||
* Run Pingen Document Update |
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.
Can you please update this part as well? Explaining how to configure the webhooks and so on?
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.
@grindtildeath updated
3636851
to
3cc98c2
Compare
3cc98c2
to
836c7fa
Compare
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.
LG overall. Can you please squash commits?
staging=self.pingen_staging, | ||
) | ||
|
||
def _get_pingen_client(self): |
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.
isn't this redundant?
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.
i dont see why?
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.
because you return the same result of _pingen
?
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.
@ajaniszewska-dev
We could keep only _get_pingen_client
and remove the extra call to _pingen
. Not blocking though
b0e5817
to
945608f
Compare
945608f
to
6d2a3ef
Compare
@grindtildeath ping |
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.
Few more things to clean up
staging=self.pingen_staging, | ||
) | ||
|
||
def _get_pingen_client(self): |
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.
@ajaniszewska-dev
We could keep only _get_pingen_client
and remove the extra call to _pingen
. Not blocking though
pingen/models/pingen.py
Outdated
letter_id=document_uuid, | ||
) | ||
return response.json().get("data", {}).get("attributes") | ||
# return response.json()['item'] |
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.
🔥
pingen/models/pingen.py
Outdated
# if rjson.get('send'): | ||
# # confusing name but send_id is the posted id | ||
# posted_id = rjson['send'][0]['send_id'] | ||
# item = rjson['item'] |
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.
🔥
pingen/models/pingen.py
Outdated
# we cannot use the `files` param alongside | ||
# with the `datas`param when data is a | ||
# JSON-encoded data. We have to construct | ||
# the entire body and send it to `data` | ||
# https://github.com/kennethreitz/requests/issues/950 | ||
# formdata = { | ||
# 'file': (filename, filestream.read()), | ||
# } |
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.
🔥
pingen/models/pingen.py
Outdated
# file_upload = self._get_file_upload() | ||
|
||
# multipart, content_type = encode_multipart_formdata(formdata) |
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.
🔥
pingen/models/pingen.py
Outdated
"file_original_name": filename, | ||
"file_url": url, | ||
"file_url_signature": url_signature, | ||
# TODO Use parameters and mapping |
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 TODO still relevant?
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.
i'd like to know it too...probably not so ill remove together will all other commented code
pingen/models/ir_attachment.py
Outdated
def _prepare_pingen_document_vals(self): | ||
return { | ||
"attachment_id": self.id, | ||
# 'config': 'created from attachment' |
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.
🔥
pingen/models/pingen.py
Outdated
@@ -0,0 +1,317 @@ | |||
# Author: Guewen Baconnier | |||
# Copyright 2012-2017 Camptocamp SA |
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.
# Copyright 2012-2017 Camptocamp SA | |
# Copyright 2012-2023 Camptocamp SA |
I would do the same for all the other files
pingen/models/pingen_document.py
Outdated
# if post_id: | ||
# state = 'sendcenter' | ||
# elif infos['requirement_failure']: | ||
# state = 'pingen_error' | ||
# error = _('The document does not meet the Pingen requirements.') |
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.
🔥
pingen/data/pingen_data.xml
Outdated
|
||
<record forcecreate="True" id="ir_cron_update_pingen" model="ir.cron"> | ||
<field name="name">Run Pingen Document Update</field> | ||
<field eval="True" name="active" /> |
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.
Shouldn't this one be set to False as default since the webhooks must update the document?
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.
seems to make sense, updated
@pedrobaeza any chance to merge this? |
return "https://api-staging.v2.pingen.com" | ||
return "https://api.v2.pingen.com" |
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 urls with ".v2." will be discontinued in April 2024
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.
as there is card for that, we will followup in another PR, so lets try to merge this one first.
Please squash commit history for avoiding useless commits like "Remove commented code" or "fix pre-commit". You are also ignoring the commit message conventions: Please adjust them. |
Standard migration, incl. changes proposed in OCA#290 Add unit test.
94da297
to
0d86765
Compare
@pedrobaeza should be better. If there is still any wrong commit - please point what i missed. Thank you. |
/ocabot merge nobump |
Hey, thanks for contributing! Proceeding to merge this for you. |
Congratulations, your PR was merged at 671f36c. Thanks a lot for contributing to OCA. ❤️ |
includes changes from: https://github.com/OCA/report-print-send/pull/290/files