Skip to content

[17.0][IMP] account_statement_import_sheet_file: add a hook function to filter certain rows from a sheet - #963

Open
luisDIXMIT wants to merge 1 commit into
OCA:17.0from
dixmit:17.0-imp-account_statement_import_sheet_file
Open

[17.0][IMP] account_statement_import_sheet_file: add a hook function to filter certain rows from a sheet#963
luisDIXMIT wants to merge 1 commit into
OCA:17.0from
dixmit:17.0-imp-account_statement_import_sheet_file

Conversation

@luisDIXMIT

Copy link
Copy Markdown

Forward port of #934

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @alexey-pelykh,
some modules you are maintaining are being modified, check this out!

@OCA-git-bot OCA-git-bot added mod:account_statement_import_sheet_file Module account_statement_import_sheet_file series:17.0 labels Jun 11, 2026

@BhaveshHeliconia BhaveshHeliconia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@alexey-pelykh alexey-pelykh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this — _skip_row(values, columns) is a clean extension point and I'm glad to have it in the module.

It follows the module's existing protected-hook convention (it sits right alongside _get_column_delimiter_character / _decode_column_delimiter_character on the mapping model), and defaulting to False makes it fully backwards-compatible: every existing mapping keeps its current behaviour, and a downstream module can override _skip_row to drop rows it doesn't want (subtotals, "pending" markers, footnotes, etc.).

Placement in _parse_rows is right, too — it runs after values = list(row) and before the skip_empty_lines short-circuit, with both values and columns in scope, so an override can filter on any raw row.

For whoever drives the merge: the red codecov/patch here is expected and not a real gap. The only uncovered line is the continue, which is unreachable until a subclass overrides _skip_row to return True, so the base module can't exercise it on its own. Fine to merge past.

Co-Reviewed-By: Claude Opus 4.8 noreply@anthropic.com

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants