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

Replace PIL renderer with Reportlab renderer #105

Merged
merged 14 commits into from
Apr 15, 2015

Conversation

nbessi
Copy link
Contributor

@nbessi nbessi commented Feb 24, 2015

It allows to have lighter, faster, vectorial payment slip.

This should fix most of current performance and OCR issues

@coveralls
Copy link

Coverage Status

Coverage increased (+0.46%) to 71.84% when pulling a3a74e4 on nbessi:report_lab_renderer into a8eb85e on OCA:8.0.

@PCatinean
Copy link
Contributor

👍 Works nicely even with standard Ubuntu python-reportlab packages

@nbessi nbessi changed the title Replace PIL rendered with Reportlab rendered Replace PIL renderer with Reportlab renderer Feb 25, 2015
@nbessi
Copy link
Contributor Author

nbessi commented Feb 27, 2015

Maybe we should add a migration step that reset vertical and horizontal delta

@PCatinean
Copy link
Contributor

That's a good idea, since quite a few people might be migrating

@yvaucher
Copy link
Member

👍 Would be great to add migration step to convert those mm in inches.

@fclementic2c
Copy link
Member

👍

@@ -23,22 +23,22 @@ msgstr "Données BVR"

#. module: l10n_ch_payment_slip
#: field:res.company,bvr_delta_horz:0
msgid "BVR Horz. Delta (px)"
msgid "BVR Horz. Delta (inch)"
msgstr "Décalage horizontal global en pixels"
Copy link
Member

Choose a reason for hiding this comment

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

The translation still says "en pixels", should be "en pouces" (same comment for the other translations in the file)

@lsimonin
Copy link

Do you have any idea when the pull request will be validated?

@nbessi
Copy link
Contributor Author

nbessi commented Mar 11, 2015

Branche rebased and french translation corrected

@nbessi
Copy link
Contributor Author

nbessi commented Mar 11, 2015

@lsimonin I guess it can be merged now except, if a reviewer insists to have a migration script in this PR

@lsimonin
Copy link

@nbessi Thank's for you response. I waiting for the merge.

@mdietrichc2c
Copy link

👍

@lsimonin
Copy link

Can you make a patch or merge this PR? i would'like to test this.

@nbessi
Copy link
Contributor Author

nbessi commented Mar 17, 2015

@lsimonin you can try directly without waiting it to be merged ??

@fclementic2c
Copy link
Member

@lsimonin Thanks for you review. Did you notice that you have several options you can select:

  • on the bank form allowing you to print or not the bank, the account number, the partner adress
    like where you can dicide to display or not the background, adresses... ?
    image
  • on the company form (tab BVR data) : here you can insert the background if you like
    image

Frederic

@lsimonin
Copy link

@fclementic2c Thank you response

On the tab bvr data i see this. but for example i send 80% of invoices by email and some times by paper. the aim being to avoid the user to edit the configuration
Also when sending invoice by email, it is not possible to join 2 report. (impossible to attach the invoice and bvr)

@nbessi
Copy link
Contributor Author

nbessi commented Mar 18, 2015

@lsimonin to merge report we use https://github.com/OCA/reporting-engine/tree/7.0/base_report_assembler but it seems that it is not ported yet in version 8.0.

For your other concerns you should open an enchancement issue. They are out of the scope of this PR.

@nbessi
Copy link
Contributor Author

nbessi commented Mar 18, 2015

@lsimonin I will add the missing translation

@yvaucher
Copy link
Member

Just add need fixing so we don't forget the missing translation

@rdeheele
Copy link

👍

@lsimonin
Copy link

I had a return of the validation of the BVR by Postfinance.
It miss the "Reference number" of the BVR in the "Récépissé"
Please fix this.

@lsimonin
Copy link

Can you please increase the space between "Payment slip to invoice ..." and the BVR

Please see this.
paymentslip

while waiting community tools or banking addons to be repared
in order to avoid printing on preprinted marks
@nbessi
Copy link
Contributor Author

nbessi commented Mar 24, 2015

@lsimonin Fixed

@lsimonin
Copy link

When I move the address, reference number at the top does not move at the same time in the "Récépissé"

@lsimonin
Copy link

See the problem with my last message. See the red line.
The first 2 character are in the margin and are not printed.
bvr

@lsimonin
Copy link

lsimonin commented Apr 2, 2015

the bvr was validated by Postfinance

@nbessi
Copy link
Contributor Author

nbessi commented Apr 3, 2015

@lsimonin That's a good new I will fix your last comment ASAP

@nbessi
Copy link
Contributor Author

nbessi commented Apr 15, 2015

Positioning fixed

@yvaucher
Copy link
Member

Ok let's merge as it was validated by PostFinance

yvaucher added a commit that referenced this pull request Apr 15, 2015
Replace PIL renderer with Reportlab renderer
@yvaucher yvaucher merged commit a03f6a8 into OCA:8.0 Apr 15, 2015
yvaucher added a commit to yvaucher/l10n-switzerland that referenced this pull request Nov 14, 2017
Replace PIL renderer with Reportlab renderer
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.

None yet

9 participants