fix: make propms run on Frappe and ERPNext version 16 - #75
Merged
aakvatech merged 21 commits intoSep 3, 2026
Conversation
chore: align ruff config and add erpnext frappe dependency for version-16
Install hooks at three stages. pre-commit checks staged files only so commits stay fast, commit-msg runs commitlint against the conventional commit rules, and pre-push re-enters pre-commit over the whole repository so a push cannot carry unchecked commits. Anchor the exclude regex. The old "node_modules|.git" pattern was unanchored, so ".git" also matched ".github" and no workflow YAML was ever validated, and it hid COMMIT_EDITMSG from the commit-msg stage. Add the frappe-semgrep-rules hook as language: python with semgrep in additional_dependencies, so it installs its own semgrep and clones the rules into the repo on first run instead of depending on a path that only exists on one machine. Ignore the clone target. Add no-commit-to-branch for main, master, production, and the version branches. Working branches such as version-15-hotfix stay open. Own the ruff config in this app. line-length moves from 120 to 110, target stays py310, and the missing [tool.ruff.format] section is added with double quotes and tab indentation. E101 and W191 join the ignore list because W is selected and the app indents with tabs. An explicit [tool.ruff.lint] section stops ruff resolving upward to a config outside the app. Add the dev extra and scripts/setup-git-hooks.sh so a fresh clone can install the hooks in one command.
Remove ci.yml and ci-tests.yml. Both build a full bench on the runner (bench init, get-app, new-site, install-app, migrate) with MariaDB and two Redis services. Frappe tests run locally on the bench terminal, not in GitHub Actions, so these only added five to eight minutes per pull request and went red for upstream breakage unrelated to the diff. Remove release.yml and .releaserc.json. Tagging, releasing, and promotion belong to tag-and-promote-from-pr-label.yml. Running semantic-release alongside it tags a separate release commit, after which tag-and-promote finds the tag on a different commit and fails. Add pre-commit.yml so pre-commit runs over all files on every pull request, and semantic-commits.yml so commitlint re-checks the whole PR commit range. Both gates are server side and hold for contributors who never installed the local hooks. Rescope linter.yml to a full semgrep scan plus pip-audit. The blocking step now uses r/python.lang.security at ERROR severity, with a non-blocking WARNING pass for information. The duplicate pre-commit step is gone, since pre-commit.yml covers it over all files.
Use frappe.db.get_single_value for the four reads of Property Management Settings in issue_hook. That doctype is a single, and frappe.db.get_value is not type safe against a single. The rest of the file already reads singles this way. Initialise invoice_item in leaseInvoiceAutoCreate. The flush branch reads the previous iteration's row, which the row_num guard makes safe at runtime, but the name had no binding at the top of the loop and ruff reported it undefined. Drop three dead locals: foreign_currency in both rent invoice reports, which nothing reads after the branch assigns it, and name_in_json in create_property_setter. Replace explicit positional format indices and rename an unused loop variable.
Apply ruff at line-length 110 with tab indentation and double quotes across the app, together with the safe lint autofixes and the whitespace and end-of-file hooks. No behaviour changes.
ci: adopt the standard pre-commit, ruff, and semgrep toolchain
Without the [tool.ruff.lint] header ruff read select and ignore as formatter keys and refused to parse pyproject.toml, so both ruff hooks exited 2 in CI. Widen the bench dependency range to cover v15 and v16 now that this branch carries the toolchain for both.
fix(ci): restore the tool.ruff.lint section and widen frappe deps
# Conflicts: # .gitignore # propms/__init__.py # propms/hooks.py # propms/issue_hook.py # propms/lease_invoice.py # propms/pos.js # propms/property_management_solution/doctype/lease/lease.py # propms/property_management_solution/doctype/property/property.py # pyproject.toml
av-dev2
force-pushed
the
fix/v16-compat
branch
from
September 3, 2026 12:48
f132568 to
57baa67
Compare
aakvatech
merged commit Sep 3, 2026
cad3a01
into
Aakvatech-Limited:version-16-hotfix
4 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes propms run on Frappe 16 / ERPNext 16. Every change here was reproduced as
a failing check on a version-16 bench (
propms-test.localhost, Frappe 16.31,ERPNext 16.30, Python 3.14) before it was fixed.
What version 16 broke
Issuevalidate hookCannot commit database transaction from document hooksfrappe.db.commit()calls; the row writes already belong to the caller's transactionFilter fields have invalid backtick notation: ifnull(\parent_property`, '')`["parent_property", "is", "not set"]Unknown column 'jc.resolution_date'sla_resolution_datePOSCart is not defined— the page script targeted the ERPNext 11 offline POSerpnext.PointOfSale.ItemCart/.Controller, loaded throughfrappe.require("point-of-sale.bundle.js")page_jsposis not a page any morepoint-of-saleis registered/appwas rerouted to/desk/desk/lease/<name>Custom Error Logpropms/pos.jsis deleted. It was 2116 lines of the ERPNext 11 offline POScontroller (
erpnext.taxes_and_totals.extend({...}),cur_pos, a Jinja{% include %}), unreferenced byhooks.pyor by any other file since theoffline POS was removed from ERPNext.
The Lease messages that were written as
_(f"...")now translate the templateand interpolate afterwards, so the strings are extractable again. The scheduler's
own
frappe.db.commit()is kept and annotated — it is a batch entry point, not adocument hook.
Verification
All 27 propms desk surfaces (forms, list views, the Property tree, the Point of
Sale page, all query reports and the workspace) were loaded in Chrome against
the v16 site and report zero console errors.
Merge order: this PR first, then #lease-invoicing, then #report-sql, then the
test suite.