Skip to content

[18.0][FIX] account_invoice_en16931, l10n_fr_einvocing: Fixes a few errors#22

Open
remi-filament wants to merge 9 commits into
akretion:18.0from
lefilament:18.0-fix-en16931
Open

[18.0][FIX] account_invoice_en16931, l10n_fr_einvocing: Fixes a few errors#22
remi-filament wants to merge 9 commits into
akretion:18.0from
lefilament:18.0-fix-en16931

Conversation

@remi-filament

Copy link
Copy Markdown
Contributor

Hi @alexis-via I have fixed a few errors found while testing.
To ease analysis, I made a commit per fix but I could group them in a single commit if you prefer.

Fixes #4 #19 and #20

@ak-git-bot

Copy link
Copy Markdown

Hi @alexis-via,
some modules you are maintaining are being modified, check this out!

@ak-git-bot ak-git-bot added mod:l10n_fr_einvoicing Module l10n_fr_einvoicing series:18.0 mod:account_invoice_en16931 Module account_invoice_en16931 labels Jul 10, 2026

@bguillot bguillot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

beside small change LGTM
Thanks for the fixes !

@@ -513,8 +513,10 @@ def _fr_ctc_authorization_code_redirect(self):
siren = self.partner_id._get_siren(raise_if_none=True)
optional_uri_params = {"superpdp_company_number": siren}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can remove this line as it is set in if/else

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @bguillot I forgot it !

@remi-filament

Copy link
Copy Markdown
Contributor Author

Rebased
Also content of #10 is part of this PR

@alexis-via

Copy link
Copy Markdown
Member

Thank you very much for all those fixes. I integrated all of them in my last commit, except the change on the onboarding (I quoted your name in the commit). For some of them, I chose a different solution, in particular for the dependency on l10n_fr_business_document_import : I removed the call on _match_partner, which removed the need to depend on this module.
I hope my implementation of the fixes is ok for you.
I propose to close this PR, and open a new one if needed on a specific point.
Thanks again for your help on this testing/bugfix phase.

@remi-filament

Copy link
Copy Markdown
Contributor Author

Thanks @alexis-via I just checked, all my fixes have been implemented but one.
The one changing optional_uri_params on res_company for authorization_code with SuperPDP. SuperPDP refuses any connection if SIREN is not 1 or 2 on sandbox so it makes sense for me to force to use SIREN = 000000001 when using sandbox and SuperPDP so that Authorization flow can be properly tested.

See : e191b5e
If you prefer I create another PR with only this commit ?

@alexis-via

Copy link
Copy Markdown
Member

Yes, I didn't take this patch because I didn't understand it. So, what I understand now, is that you want to be able to test the SUPERPDP onbaording from a company where SIREN is not the SIREN of BurgerQueen nor Tricatel, but any SIREN, and the code would query with a SIREN different than the company's SIREN. Did I understand it correctly ?

@remi-filament

Copy link
Copy Markdown
Contributor Author

Not sure, my issue was that when trying to create a new company with authorization code, Super PDP replied that only SIREN 000000001 or SIREN 000000002 could be created on sandbox instance.

Now when trying to connect with Le Filament SIREN + scheme = sandbox, I get the following reply from SuperPDP API :

{'error': 'invalid_request', 'error_description': 'No company found with these superpdp_company_number_scheme and superpdp_company_number.', 'state': 'vDcyEKQDiAY45mBmrQCuqpOJyP5h3S'}

Maybe I am missing something on this authorization code flow ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:account_invoice_en16931 Module account_invoice_en16931 mod:l10n_fr_einvoicing Module l10n_fr_einvoicing series:18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[18.0] l10n_fr_einvoicing : reference to base.document.import not in dependencies

4 participants