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

Support "priority" and add 2 templates for InsERT-software-generated invoices #416

Merged
merged 3 commits into from
Feb 25, 2023

Conversation

rmilecki
Copy link
Collaborator

@rmilecki rmilecki commented Oct 1, 2022

templates: pl: add templates for InsERT's software issued invoices

InsERT is one of the most popular Polish accounting software company.
They have two very common softwares:
1. Subiekt nexo
2. Subiekt GT

Add 2 templates to parse invoices generated by above softwares. Those
templates are software-specific so they have priority set to 3.

This allows parsing a lot invoices issued in Poland.
Regex: support more grouping functions

In case of multiple matches we only had an option to return a sum of
numeric values. Add more functions.
Add "priority" support for templates

In case of multiple templates matching given invoice - choose the one
with the highest "priority" value. To provide proper support for
prioritizing AND existing templates (backward compatibility) the default
value 5 is assumed in case "priority" property is missing.

This feature can be used for writing more generic as well as more
specific templates. So far all templates were assumed to be
company-specific. With this change we can have:
1. Invoice-generating software specific templates
2. In-company varying templates

This feature may be very useful for:
1. Countries with just few very popular accounting software applications
2. Big companies with multiple departments adding some invoice details

@rmilecki rmilecki requested review from m3nu and bosd October 1, 2022 17:39
@bosd
Copy link
Collaborator

bosd commented Oct 3, 2022

Thanks for this very interesting PR!

Will need some time to test it.
I like the idea of creating generic templates, even selecting on the accounting software or library that generated it.
At some point it might be interesting to look / filter on the meta data passed in the invoicefile.

Just a thought..
Will the use of prioritys have a big impact on precessing time?

Either way, being able to parse multiple invoices and minimize the writing write specific templates is a big plus.

@rmilecki
Copy link
Collaborator Author

rmilecki commented Oct 3, 2022

I'm happy to get a positive respose to this. Take your time to review / test this.

Will the use of prioritys have a big impact on precessing time?

I think at loading templates takes the most time. This part doesn't get changed with this pull request.

With this change matches_input() gets called for every loaded template. Previously it was called until the first match. matches_input() is a simple function that just uses re.search(). So this pull request shouldn't affect perfroamnce much.


Since you mentioned perfromance I think we should check why loading templates takes so much time. Is that file access or parsing YAML? Maybe we could optimize / cache?

(Just a topic for another discussion)

@bosd bosd force-pushed the priority-and-insert-templates branch from b0f6ce1 to ab331d9 Compare October 23, 2022 19:05
Copy link
Collaborator

@bosd bosd left a comment

Choose a reason for hiding this comment

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

I'm about to give this one a 👍 as tests are all green.
Please attend to comments if possible.

@bosd bosd force-pushed the priority-and-insert-templates branch from ab331d9 to 6257ec7 Compare November 28, 2022 09:14
Copy link
Collaborator

@bosd bosd left a comment

Choose a reason for hiding this comment

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

"Fasttracking" this as getting the functionality is more of an priority then debating my detailed comments.

@rmilecki rmilecki mentioned this pull request Feb 6, 2023
@bosd bosd force-pushed the priority-and-insert-templates branch 2 times, most recently from cae8c67 to d99abc3 Compare February 14, 2023 09:22
@bosd
Copy link
Collaborator

bosd commented Feb 14, 2023

Made the same performance update as on the 'old code' in #470
Tests will pass after the flipkart template has been updated from that pr.

@bosd
Copy link
Collaborator

bosd commented Feb 14, 2023

btw, I run this trough the profiler. No noticable difference on the performance.

@bosd bosd force-pushed the priority-and-insert-templates branch from d99abc3 to 948a82f Compare February 25, 2023 17:03
Rafał Miłecki added 3 commits February 25, 2023 18:06
In case of multiple templates matching given invoice - choose the one
with the highest "priority" value. To provide proper support for
prioritizing AND existing templates (backward compatibility) the default
value 5 is assumed in case "priority" property is missing.

This feature can be used for writing more generic as well as more
specific templates. So far all templates were assumed to be
company-specific. With this change we can have:
1. Invoice-generating software specific templates
2. In-company varying templates

This feature may be very useful for:
1. Countries with just few very popular accounting software applications
2. Big companies with multiple departments adding some invoice details

Signed-off-by: Rafał Miłecki <[email protected]>
In case of multiple matches we only had an option to return a sum of
numeric values. Add more functions.

Signed-off-by: Rafał Miłecki <[email protected]>
InsERT is one of the most popular Polish accounting software company.
They have two very common softwares:
1. Subiekt nexo
2. Subiekt GT

Add 2 templates to parse invoices generated by above softwares. Those
templates are software-specific so they have priority set to 3.

This allows parsing a lot invoices issued in Poland.

Signed-off-by: Rafał Miłecki <[email protected]>
@bosd bosd force-pushed the priority-and-insert-templates branch from 948a82f to 7ffe2ac Compare February 25, 2023 17:06
@bosd
Copy link
Collaborator

bosd commented Feb 25, 2023

Just reverted the performance changes. As we are discussing them in #470.
This one is now independent from that one.
When tests are green, we can merge this one.

@bosd bosd merged commit 7bed841 into invoice-x:master Feb 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants