mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-14 23:18:40 +00:00
Merge branch 'develop' into fix-variant-of-filter-item-list
This commit is contained in:
248
.github/POSTGRES_COMPATIBILITY.md
vendored
Normal file
248
.github/POSTGRES_COMPATIBILITY.md
vendored
Normal file
@@ -0,0 +1,248 @@
|
||||
# PostgreSQL compatibility — review guide
|
||||
|
||||
ERPNext targets **both MariaDB and PostgreSQL from a single codebase**. The full server
|
||||
test suite passes on both, but the PostgreSQL CI job is **label-gated** (it does not run on
|
||||
every PR), so until it is required this guide is the always-on guard. Greptile loads it as
|
||||
review context (`.greptile/config.json`).
|
||||
|
||||
When reviewing a PR, flag any **new or changed query** (raw `frappe.db.sql`, `frappe.qb`,
|
||||
`frappe.get_all/get_list/get_value`, report SQL) that would **error on PostgreSQL** or
|
||||
**return different results on the two engines**.
|
||||
|
||||
## The one rule that governs everything
|
||||
|
||||
**MariaDB behaviour must not change; PostgreSQL is brought into line with MariaDB — never the
|
||||
reverse.** A "fix" that changes the value, row count, or ordering MariaDB produced is a
|
||||
regression, even if the new behaviour looks more correct. The only accepted MariaDB-output
|
||||
change is replacing a genuinely *undefined/arbitrary* result with a deterministic one (row
|
||||
count preserved) — and that should be called out explicitly.
|
||||
|
||||
There are two failure modes to watch for:
|
||||
1. **Hard breaks** — PostgreSQL raises an exception; MariaDB is green. Easy to catch in CI,
|
||||
but the gated job may not run.
|
||||
2. **Silent divergences** — both engines succeed but return *different* results. CI on one
|
||||
engine stays green; the bug only shows on a PostgreSQL site. These are the dangerous ones.
|
||||
|
||||
---
|
||||
|
||||
## 1. Hard breaks — would error on PostgreSQL
|
||||
|
||||
Flag a changed query that uses any of these:
|
||||
|
||||
- **Loose `GROUP BY`** — selecting/ordering a column that is neither in `GROUP BY` nor wrapped
|
||||
in an aggregate. MariaDB tolerates it; PostgreSQL errors (`must appear in the GROUP BY
|
||||
clause or be used in an aggregate function`). This **also covers an aggregate (`Sum`/`Count`/…)
|
||||
selected alongside bare columns with NO `.groupby()` at all** — MariaDB silently collapses
|
||||
every row into one arbitrary-valued row (often a *wrong-output* bug there too), PostgreSQL
|
||||
errors. Fix: add the bare column to `GROUP BY` **if it is functionally dependent on the group
|
||||
key**, otherwise wrap it in `Max()`/`Min()`. **See §3 — the row-count trap — before suggesting
|
||||
"add it to GROUP BY".**
|
||||
- **MySQL-only functions** — `TIMESTAMP(date,time)`, `TIMEDIFF`, `STR_TO_DATE`, `DATE_FORMAT`,
|
||||
`DATE_ADD/SUB`, `GROUP_CONCAT`, `PERIOD_DIFF`, SQL `IF(cond,a,b)`. Use the portable
|
||||
`frappe.query_builder.functions` equivalents (`CombineDatetime`, `DateDiff`, `Case`,
|
||||
`GroupConcat`, …) or a precomputed column (e.g. `posting_datetime`).
|
||||
- **`UPDATE … JOIN`** — not valid on PostgreSQL. Rewrite as `UPDATE … WHERE name IN (subquery)`.
|
||||
- **`HAVING` referencing a `SELECT` alias** — PostgreSQL rejects output-column aliases in
|
||||
`HAVING` (regardless of whether the query has a `GROUP BY`; MariaDB allows them). Repeat the
|
||||
underlying expression in `HAVING`, or move a non-aggregate predicate into `WHERE`.
|
||||
- **`SELECT DISTINCT … ORDER BY <expr not in the select list>`** — add the expr to the select
|
||||
**only if it is single-valued per distinct row**; otherwise it grows the `DISTINCT` key and the
|
||||
MariaDB row count (see §3) — drop the SQL `ORDER BY` and sort in Python instead.
|
||||
- **Single-quoted column alias** `AS 'x'` — PostgreSQL reads `'x'` as a string literal. Use an
|
||||
unquoted (or double-quoted) alias.
|
||||
- **`varchar | varchar`** (bitwise OR misused as a coalesce) — errors on PostgreSQL. Use
|
||||
`Coalesce(...)`.
|
||||
- **Capital-cased identifiers** used as column/field names in `get_value(dt, dn, "Status")`,
|
||||
`get_all(dt, fields=["Account"])`, and similar — PostgreSQL quotes the identifier and matches
|
||||
it case-sensitively; a stored column named `status`/`account` won't match `"Status"`/`"Account"`
|
||||
(`column "Account" does not exist`). Use the exact stored (lower-case) fieldname.
|
||||
- **Boolean passed where an integer column is expected** — `frappe.db.set_value(dt, dn,
|
||||
check_field, True)`, `doc.db_set(field, False)`, or `frappe.qb.update(dt).set(check_field, True)`
|
||||
emit `SET col = true`, which PostgreSQL rejects on a `smallint`/`Check` column
|
||||
(`column is of type smallint but expression is of type boolean`). Pass `1`/`0`.
|
||||
- **`.like()`/`.ilike()` (or raw `LIKE`) on a NON-text column** — `idx`, `docstatus`, a date, etc.
|
||||
frappe maps `.like()` → `ILIKE`, and PostgreSQL has no `bigint ILIKE text` operator (`operator
|
||||
does not exist: bigint ~~* unknown`). Cast the column to text first — **`Cast_(col, "varchar")`**,
|
||||
not `Cast(col, "char")` (see below). MariaDB coerces the int implicitly, so the cast is a no-op there.
|
||||
- **`CAST(… AS CHAR)` / `Cast(x, "char")`** — on PostgreSQL bare `CHAR` is `character(1)`, so
|
||||
`CAST(12 AS CHAR)` → `'1'` (silently truncates multi-digit values); MariaDB gives the full string.
|
||||
Use `VARCHAR` / `Cast_(x, "varchar")`.
|
||||
- **`.rlike()` / raw `RLIKE`** — frappe rewrites `REGEXP` → `~*` on PostgreSQL but does **not**
|
||||
translate `RLIKE` (no such PostgreSQL operator). Use `.regexp()` (or `.like()` for a simple prefix).
|
||||
- **`IfNull`/`Coalesce` of a typed column with a different-typed literal** — `IfNull(asset.disposal_date, 0)`
|
||||
renders `COALESCE("disposal_date", 0)`, coalescing a **DATE** with an **integer**. PostgreSQL requires
|
||||
`COALESCE` args to share a type (`DatatypeMismatch: COALESCE types date and integer cannot be matched`);
|
||||
MariaDB's `IFNULL` is permissive. The common shape is `IfNull(date_col, 0) != 0 / == 0` as a presence test —
|
||||
replace with `date_col.isnotnull()` / `date_col.isnull()` (identical, and valid on both). Otherwise coalesce
|
||||
to a **same-type** default (`Coalesce(date_col, '1900-01-01')`, `Coalesce(text_col, '')`).
|
||||
- **Division by a possibly-zero divisor** — `Sum(a) / Sum(b)`, `x / col`, etc. where the
|
||||
divisor can be `0`/empty. MariaDB returns `NULL` for division by zero; PostgreSQL raises
|
||||
`division by zero` and aborts the query. Wrap the divisor in `NullIf(divisor, 0)` — that
|
||||
yields `NULL` on both engines, matching MariaDB's value. (Only the *literal* `/ 0` is a parse
|
||||
constant; the trap is a divisor that is an aggregate or column the data can drive to zero.)
|
||||
|
||||
---
|
||||
|
||||
## 2. Silent divergences — succeeds on both, returns different results
|
||||
|
||||
These don't error, so a one-engine CI stays green. Flag them:
|
||||
|
||||
- **Case sensitivity on text equality** — `==`, `.isin()`, `Strpos`/`Locate` on free-text
|
||||
columns are case-**sensitive** on PostgreSQL but case-**insensitive** under MariaDB's default
|
||||
collation. `Lower()` both sides. *(Not `.like()`/`["like", …]` — those already render as
|
||||
`ILIKE` on PostgreSQL; see §4.)*
|
||||
- **Case sensitivity in a doc-`name` lookup** — lower-casing a value then using it as a
|
||||
document name in `get_value`/`get_doc`/`exists` misses on PostgreSQL (names are
|
||||
case-sensitive). Keep original case for the identifier; lower-case only comparison operands.
|
||||
- **Empty string vs NULL** — PostgreSQL stores a blank link/data field as `NULL` on some paths
|
||||
while MariaDB keeps `''`; `Concat`/`Concat_ws` then diverge. Prefer the stored full value, or
|
||||
`Coalesce(col, '')` per argument.
|
||||
- **NULL ordering** — MariaDB sorts `NULL` first, PostgreSQL sorts it last. For
|
||||
`ORDER BY … LIMIT 1`/`[0]` on a nullable column, guard with `Coalesce`/`isnotnull()`.
|
||||
- **`ORDER BY … LIMIT 1` with no unique tiebreaker** — when rows tie on the ordered column the
|
||||
two engines may pick different rows. Add a `creation`/`name` tiebreaker **only if it does not
|
||||
change MariaDB's current pick** (see §4).
|
||||
- **Integer division** — `int / int` truncates on PostgreSQL but is decimal on MariaDB, e.g.
|
||||
`COUNT(...) / COUNT(...) * 100` → `0`, or `manufacturing_time_in_mins / 1440` flooring a
|
||||
lead-time to whole days. Force float: multiply by `100.0`, or make a literal a float
|
||||
(`/ 1440` → `/ 1440.0`), or cast an operand. (Only SQL-level `/` on integer **columns/literals**
|
||||
— Python `/` is already float.)
|
||||
- **`DISTINCT` list ordering** — `frappe.get_all(distinct=True, order_by=…)` /
|
||||
`SELECT DISTINCT … ORDER BY`: frappe's `db_query` **silently drops `ORDER BY` for distinct
|
||||
queries on PostgreSQL**, so the result is unordered there. Sort in Python instead — and use
|
||||
`key=str.casefold`, because bare `sorted()` is case-sensitive (ASCII) while MariaDB's
|
||||
collation is case-insensitive, so a plain sort reorders MariaDB's output.
|
||||
- **Engine-specific function rewrites** — e.g. a PostgreSQL `regexp_replace` branch
|
||||
reimplementing MariaDB's `CAST(SUBSTRING_INDEX(name,' ',-1) AS UNSIGNED)` (leading digits of
|
||||
the last whitespace token). Verify the rewrite matches MariaDB on edge cases (`"X - 3a"→3`,
|
||||
`"X - 1.5"→1`) by diffing both engines on literal rows.
|
||||
- **`UnixTimestamp(date)` / date→epoch** is timezone-dependent (midnight in the DB session TZ),
|
||||
so a strict `epoch <= now` bound is flaky on PostgreSQL.
|
||||
|
||||
---
|
||||
|
||||
## 3. The row-count trap — `GROUP BY` **and** `DISTINCT` (the single most important rule)
|
||||
|
||||
When making a loose `GROUP BY` PostgreSQL-valid, **do not add a non-functionally-dependent
|
||||
column to the `GROUP BY` just to satisfy PostgreSQL** — that turns one group row into N and
|
||||
**changes the MariaDB row count** (a regression). The classic traps are adding the **child/row
|
||||
primary key** or an **editable per-row field**. Instead **`Max()`/`Min()`-wrap** the offending
|
||||
column: the row count is preserved and the value goes from arbitrary (MariaDB's old loose pick)
|
||||
to deterministic.
|
||||
|
||||
**Judge functional dependence by the source table, not the column name:**
|
||||
- A column from a **master joined on the group key** (`t3.x` where `t1.key = t3.name`) is FD →
|
||||
safe to keep in `GROUP BY`.
|
||||
- A descriptive field on the **transaction** table (`t1.supplier_name`, `t1.territory`,
|
||||
`t1.item_name` — fetched/editable, can differ across historical rows for the same key) is
|
||||
**not** FD even though it looks master-derived → `Max()`-wrap it.
|
||||
|
||||
Conversely, do **not** suggest changing a `Max()`/`Min()`-wrapped column to `Sum()` (or vice
|
||||
versa) to make a number "more correct" — that changes the MariaDB value. The wrap reproduces
|
||||
MariaDB's prior one-value-per-group output; a different aggregate is a product change, out of
|
||||
scope for a portability fix.
|
||||
|
||||
**The same trap applies to `SELECT DISTINCT`.** To satisfy PostgreSQL's "an `ORDER BY` expr must
|
||||
appear in the select list under `DISTINCT`" rule, **do not blindly add the ordered column to the
|
||||
select** — if it is not single-valued per existing distinct row, the `DISTINCT` key grows and
|
||||
MariaDB returns **more rows** (a regression), exactly as adding a non-FD column to `GROUP BY` does.
|
||||
Add it only when it is functionally dependent on the existing select columns; otherwise drop the
|
||||
SQL `ORDER BY` and **sort in Python** (`key=str.casefold`, per §2) so the distinct row set is
|
||||
unchanged.
|
||||
|
||||
### 3.1 Second-order traps — when the `Max()`/`Min()` wrap itself is the bug
|
||||
|
||||
The wrap is only a no-op when the column is provably single-valued per group (**"`Max()` means
|
||||
provably constant"**). When the column can genuinely vary, the wrap is a decision, and a full
|
||||
audit of these fixes found four recurring mistakes:
|
||||
|
||||
- **Incoherent pair** — two semantically-coupled columns (a flag + a link:
|
||||
`is_phantom_item` + `bom_no`; a discriminator + its value) aggregated with *independent*
|
||||
`Max()`/`Min()` can pair values from **different rows** — a chimera row that never existed.
|
||||
MariaDB's loose pick was at least row-coherent. Fix: group by the pair (when consumers
|
||||
tolerate the extra rows), or select one **representative row** (`Min(child.name)` subquery +
|
||||
join-back) so every column comes from the same line.
|
||||
- **NULL-skipping** — `MAX`/`MIN` ignore NULLs, so `Max()` over a mostly-NULL discriminator
|
||||
(an `original_item`-style column) *deterministically* returns the non-NULL value where
|
||||
MariaDB could return NULL — deterministically wrong where the old behavior was only
|
||||
intermittently wrong. Flag it wherever "no value" is a meaningful state (fallback gates,
|
||||
dict keys).
|
||||
- **Fabricated arithmetic** — `Sum(x) * Max(y)` where `y` varies within the group invents a
|
||||
number no row ever had (and `Max` biases it upward) — poisonous when it feeds validation,
|
||||
budgets, valuation, or GL/stock values. Fix per-row: `Sum(x * y)`.
|
||||
- **Wrong bound** — where the value has a semantic, pick the bound deliberately:
|
||||
`Min(schedule_date)` for a "required by", `Min(idx)` for first-line ordering, a qty-weighted
|
||||
average for a rate. A blind `Max` can understate urgency or overstate a figure.
|
||||
|
||||
Review heuristic: **if choosing between `Max` and `Min` would change the answer, the column is
|
||||
not functionally dependent** — wrapping either is the wrong fix. Group by it, restructure, or
|
||||
pick a bound for a stated reason, and cover the varying-group case with a test.
|
||||
|
||||
---
|
||||
|
||||
## 4. False positives — do NOT flag these
|
||||
|
||||
These are auto-handled by the framework and are **not** breaks:
|
||||
|
||||
- **`.like()` / `["like", …]`** already renders as `ILIKE` on PostgreSQL — not a
|
||||
case-sensitivity bug. *(Exception: `.like()` on a **non-text** column — `idx`, `docstatus` —
|
||||
is a hard break, `bigint ILIKE`; see §1.)*
|
||||
- **Raw `ifnull(...)`** inside `frappe.db.sql()` is rewritten to `coalesce(...)` on all engines.
|
||||
- **Backticks**, **`LOCATE`**, **`REGEXP`** / **`.regexp()`** in raw SQL are auto-translated on
|
||||
PostgreSQL (`REGEXP` → `~*`). **But `RLIKE` / `.rlike()` is NOT translated** — that one is a
|
||||
hard break (see §1).
|
||||
- **An `ORDER BY … LIMIT 1` tie where the two engines already agree**, or where adding a
|
||||
tiebreaker would *change* MariaDB's current pick — leave it; "fixing" it would either change
|
||||
MariaDB or has no observable effect.
|
||||
|
||||
---
|
||||
|
||||
## 5. Transaction / runtime (not query-shape, still PostgreSQL-only)
|
||||
|
||||
- **Catch-and-continue inserts** — on PostgreSQL a failed `insert()` aborts the **whole
|
||||
transaction**, so code that swallows a duplicate and keeps going dies on the next statement
|
||||
with `InFailedSqlTransaction` (frappe dropped its blanket per-statement savepoint in
|
||||
frappe#40075). Such a handler must wrap the fallible insert in `frappe.db.savepoint(name)` +
|
||||
`rollback(save_point=name)` — unless it re-`throw`s with no DB call before the throw, or the
|
||||
insert uses `ignore_if_duplicate=True` / `autoname="hash"` (→ `ON CONFLICT DO NOTHING`).
|
||||
- **Recover the txn with a *scoped* savepoint, not a full `frappe.db.rollback()`, if any prior work
|
||||
must survive.** A full rollback un-poisons the txn but also discards every row the handler committed
|
||||
*before* the failure — which MariaDB kept (it has no statement-abort), so it's a **silent MariaDB
|
||||
regression**. **"The background job / whitelist entrypoint owns the txn" does NOT make a full rollback
|
||||
safe** if it did multiple inserts in a loop first — it drops the partial results MariaDB retained. A
|
||||
full rollback is safe only when it (a) immediately re-`throw`s/`raise`s (MariaDB rolls back anyway),
|
||||
(b) has nothing successful before it (a single op), or (c) the batch is genuinely meant to be
|
||||
**atomic** (a partial result is an invalid state → rollback + mark *Failed* is correct). Otherwise use
|
||||
a **per-iteration / per-record savepoint** — and keep the function's success/`None` return contract:
|
||||
do **not** return the doc when the savepoint was rolled back.
|
||||
|
||||
---
|
||||
|
||||
## 6. Refactors and raw-SQL→ORM conversions are not automatically 1:1
|
||||
|
||||
A commit labeled a **refactor** or a **raw-`frappe.db.sql` → `frappe.qb`/ORM conversion** is meant
|
||||
to preserve behaviour — but it easily doesn't, and the change passes the static checker and a
|
||||
one-engine green run. **Diff the `WHERE`/predicate, the `JOIN`/`ON` conditions, and the resulting
|
||||
row set — not just the `SELECT` shape.** A conversion that silently widens or narrows the filter
|
||||
changes the rows touched on **both** engines and is a regression hiding under a "refactor" label.
|
||||
|
||||
Real example: an `UPDATE` whose bound was `posting_datetime > X` gained an
|
||||
`OR (posting_datetime == X AND creation > args.creation)` branch during a "`sql` → `qb` refactor",
|
||||
widening the rows updated on both engines. Even when such a change is a deliberate bug-fix it must
|
||||
be called out and tested — it is **not** the no-op the refactor label implies. Confirm the
|
||||
converted query touches exactly the same rows with the same values MariaDB produced before.
|
||||
|
||||
---
|
||||
|
||||
## How to review
|
||||
|
||||
For every changed query: does it (a) use a construct from §1 (would error on PostgreSQL),
|
||||
(b) match a divergence in §2/§3 (different result across engines), or (c) change the row set under
|
||||
a refactor/conversion label (§6)? If so, comment with the
|
||||
portable fix and confirm it leaves **MariaDB output unchanged**. Skip the §4 false positives.
|
||||
Prefer a comment that names the rule (e.g. "loose GROUP BY — Max()-wrap, don't add to GROUP BY:
|
||||
splits the row count") so the fix is unambiguous.
|
||||
|
||||
The static pre-commit checker (`.github/helper/postgres_compat.py`) catches the *mechanical*
|
||||
§1 breaks; the **semantic** §2/§3 divergences and the §6 refactor/conversion row-set changes are
|
||||
exactly what a reviewer (and this guide) must cover, because no static check can see them.
|
||||
72
.github/helper/hydrate.sh
vendored
Executable file
72
.github/helper/hydrate.sh
vendored
Executable file
@@ -0,0 +1,72 @@
|
||||
#!/bin/bash
|
||||
#
|
||||
# Hydrate a test shard from the setup job's artifact.
|
||||
#
|
||||
# The bench (apps, venv, node_modules, sites) is already on disk at ~/frappe-bench — the
|
||||
# workflow untar'd it from the artifact the setup job built. So there is NO bench init, no
|
||||
# asset build, and no reinstall here: just bring the DB up on the baked datadir and start redis
|
||||
# so tests can run. The whole point is that the expensive work happened ONCE in the setup job.
|
||||
#
|
||||
set -e
|
||||
|
||||
ci_user="${ERPNEXT_CI_USER:-frappe}"
|
||||
db_host="${DB_HOST:-127.0.0.1}"
|
||||
|
||||
# Re-exec as the ci user (uid 1001) so bench/cache ownership matches the artifact, same as
|
||||
# install.sh. The workflow untar'd as root with -p, so the files are already owned by ci.
|
||||
if [ "$(id -u)" = "0" ] && [ "${SKIP_SYSTEM_SETUP:-0}" = "1" ] && [ "$ci_user" != "root" ]; then
|
||||
exec su -m "$ci_user" -s /bin/bash -c \
|
||||
"ERPNEXT_CI_USER='$ci_user' DB_HOST='$db_host' DB='${DB:-}' bash '$0'"
|
||||
fi
|
||||
|
||||
cd ~/frappe-bench
|
||||
|
||||
# Start the DB on the datadir baked into the artifact. It's already populated (the setup job
|
||||
# reinstalled into this very datadir), so there is NO restore — the server comes up on the
|
||||
# existing files. This is what replaces the per-shard SQL replay.
|
||||
bash ~/frappe-bench/start-db.sh
|
||||
|
||||
# Bring up redis (lightmode unit tests need cache + queue). In the self-hosted container we use the
|
||||
# full `bench start` (web/workers too, like install.sh). On the bare GitHub Postgres shard
|
||||
# `bench start` (honcho) lagged — it blocks the redis procs behind web/worker procs the lightmode
|
||||
# suite never uses, so the wait below burned its full timeout (~4m). There, start the two redis
|
||||
# instances directly: fast and deterministic.
|
||||
if [ "${DB:-mariadb}" = "postgres" ]; then
|
||||
# Start redis directly as daemons — reliable and persists across steps. Do NOT route it through
|
||||
# `bench start`: honcho tears the whole process group down if any one Procfile proc dies on the
|
||||
# bare shard, which took redis with it (redis @ 13000 refused in Run Tests). Keeping redis
|
||||
# independent is what makes it survive. The web server (for PDF tests) is NOT started here — a
|
||||
# backgrounded server doesn't survive into the next step; it's started inside the Run Tests step.
|
||||
for conf in redis_cache redis_queue; do
|
||||
[ -f ~/frappe-bench/config/$conf.conf ] && redis-server ~/frappe-bench/config/$conf.conf --daemonize yes
|
||||
done
|
||||
else
|
||||
bench start >> ~/frappe-bench/bench_start.log 2>&1 &
|
||||
fi
|
||||
|
||||
# Wait for redis, failing fast instead of silently burning minutes if it never comes up.
|
||||
cfg=~/frappe-bench/sites/common_site_config.json
|
||||
if [ -f "$cfg" ]; then
|
||||
ports=$(python - "$cfg" <<'PY'
|
||||
import json, re, sys
|
||||
try:
|
||||
cfg = json.load(open(sys.argv[1]))
|
||||
except Exception:
|
||||
sys.exit(0)
|
||||
for key in ("redis_cache", "redis_queue"):
|
||||
m = re.search(r":(\d+)", str(cfg.get(key, "")))
|
||||
if m:
|
||||
print(m.group(1))
|
||||
PY
|
||||
)
|
||||
for port in $ports; do
|
||||
up=0
|
||||
for _ in $(seq 1 60); do
|
||||
if (exec 3<>"/dev/tcp/127.0.0.1/$port") 2>/dev/null; then exec 3>&- 3<&-; up=1; break; fi
|
||||
sleep 1
|
||||
done
|
||||
[ "$up" = "1" ] || { echo "redis did not come up on port $port"; exit 1; }
|
||||
done
|
||||
fi
|
||||
|
||||
echo "Hydrated: DB up on baked datadir, redis up — ready for tests."
|
||||
338
.github/helper/install.sh
vendored
338
.github/helper/install.sh
vendored
@@ -7,21 +7,106 @@ cd ~ || exit
|
||||
githubbranch=${GITHUB_BASE_REF:-${GITHUB_REF##*/}}
|
||||
frappeuser=${FRAPPE_USER:-"frappe"}
|
||||
frappecommitish=${FRAPPE_BRANCH:-$githubbranch}
|
||||
db_host=${DB_HOST:-"127.0.0.1"}
|
||||
db_user_host=${DB_USER_HOST:-"localhost"}
|
||||
wkhtmltox_deb=${WKHTMLTOX_DEB:-"/tmp/wkhtmltox.deb"}
|
||||
bench_cache_dir=${BENCH_CACHE_DIR:-}
|
||||
|
||||
run_as_ci_user_if_needed() {
|
||||
if [ "$(id -u)" != "0" ] || [ "${SKIP_SYSTEM_SETUP:-0}" != "1" ] || [ "${ERPNEXT_CI_NON_ROOT:-0}" = "1" ]; then
|
||||
return
|
||||
fi
|
||||
|
||||
local missing_packages=()
|
||||
if ! command -v pkg-config >/dev/null 2>&1; then
|
||||
missing_packages+=("pkg-config")
|
||||
fi
|
||||
if ! command -v mariadb_config >/dev/null 2>&1 && ! command -v mysql_config >/dev/null 2>&1; then
|
||||
missing_packages+=("libmariadb-dev")
|
||||
fi
|
||||
if ! command -v crontab >/dev/null 2>&1; then
|
||||
missing_packages+=("cron")
|
||||
fi
|
||||
|
||||
if [ "${#missing_packages[@]}" -gt 0 ]; then
|
||||
apt-get update
|
||||
apt-get install -y --no-install-recommends "${missing_packages[@]}"
|
||||
fi
|
||||
|
||||
local ci_user="${ERPNEXT_CI_USER:-frappe}"
|
||||
|
||||
if ! id "$ci_user" >/dev/null 2>&1; then
|
||||
useradd --home-dir "$HOME" --no-create-home --shell /bin/bash "$ci_user"
|
||||
fi
|
||||
|
||||
rm -rf ~/frappe ~/frappe-bench
|
||||
|
||||
local ci_dirs=(
|
||||
"$HOME"
|
||||
"$GITHUB_WORKSPACE"
|
||||
"$HOME/.cache"
|
||||
"${PIP_CACHE_DIR:-$HOME/.cache/pip}"
|
||||
"${npm_config_cache:-$HOME/.npm}"
|
||||
"${YARN_CACHE_FOLDER:-$HOME/.cache/yarn}"
|
||||
"$HOME/.yarn"
|
||||
"${UV_CACHE_DIR:-$HOME/.cache/uv}"
|
||||
"$(dirname "$wkhtmltox_deb")"
|
||||
)
|
||||
if [ -n "$bench_cache_dir" ]; then
|
||||
ci_dirs+=("$bench_cache_dir")
|
||||
fi
|
||||
|
||||
# Create + own (non-recursively) the home/cache/workspace dirs before dropping to
|
||||
# the ci user. We deliberately do NOT wipe the yarn/uv caches here so a persistent
|
||||
# cache (mounted volume or baked image layer) stays warm across runs.
|
||||
mkdir -p "${ci_dirs[@]}" "$HOME/.yarn"
|
||||
chown "$ci_user:$ci_user" "${ci_dirs[@]}" "$HOME/.yarn"
|
||||
|
||||
export ERPNEXT_CI_NON_ROOT=1
|
||||
exec su -m "$ci_user" -s /bin/bash -c "cd '$HOME' && bash '$GITHUB_WORKSPACE/.github/helper/install.sh'"
|
||||
}
|
||||
|
||||
run_as_ci_user_if_needed
|
||||
|
||||
run_ci_step() {
|
||||
local label=$1
|
||||
shift
|
||||
|
||||
echo "::group::${label}"
|
||||
date -u
|
||||
local exit_code=0
|
||||
timeout --foreground "${CI_INSTALL_STEP_TIMEOUT:-1800}" "$@" || exit_code=$?
|
||||
date -u
|
||||
echo "::endgroup::"
|
||||
return "$exit_code"
|
||||
}
|
||||
|
||||
if [ -n "${GITHUB_WORKSPACE:-}" ]; then
|
||||
git config --global --add safe.directory "$GITHUB_WORKSPACE" || true
|
||||
git config --global --add safe.directory "$GITHUB_WORKSPACE/.git" || true
|
||||
fi
|
||||
|
||||
rm -rf ~/frappe ~/frappe-bench
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 1 — parallelise the three slow, independent setup steps:
|
||||
# a) system packages b) frappe-bench pip install c) frappe git fetch
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
sudo apt update
|
||||
if [ "${SKIP_SYSTEM_SETUP:-0}" != "1" ]; then
|
||||
sudo apt-get update
|
||||
|
||||
# apt remove/install must run sequentially but can overlap with pip and git.
|
||||
sudo apt remove mysql-server mysql-client
|
||||
sudo apt install libcups2-dev redis-server mariadb-client libmariadb-dev &
|
||||
apt_pid=$!
|
||||
# apt remove/install must run sequentially but can overlap with pip and git.
|
||||
sudo apt-get remove -y mysql-server mysql-client
|
||||
sudo apt-get install -y libcups2-dev redis-server mariadb-client libmariadb-dev &
|
||||
apt_pid=$!
|
||||
|
||||
pip install frappe-bench &
|
||||
pip_pid=$!
|
||||
pip install frappe-bench &
|
||||
pip_pid=$!
|
||||
else
|
||||
apt_pid=
|
||||
pip_pid=
|
||||
fi
|
||||
|
||||
mkdir frappe
|
||||
(
|
||||
@@ -32,76 +117,243 @@ mkdir frappe
|
||||
) &
|
||||
clone_pid=$!
|
||||
|
||||
wait $apt_pid
|
||||
wait $pip_pid
|
||||
if [ -n "$apt_pid" ]; then wait $apt_pid; fi
|
||||
if [ -n "$pip_pid" ]; then wait $pip_pid; fi
|
||||
wait $clone_pid
|
||||
|
||||
pushd frappe
|
||||
git checkout FETCH_HEAD
|
||||
popd
|
||||
frappe_sha=$(git -C frappe rev-parse HEAD)
|
||||
|
||||
get_bench_cache_archive() {
|
||||
if [ -z "$bench_cache_dir" ]; then
|
||||
return
|
||||
fi
|
||||
|
||||
mkdir -p "$bench_cache_dir"
|
||||
|
||||
# Keyed on tool versions only (NOT the frappe SHA): any recent base bench works, because
|
||||
# restore_warm_bench fast-forwards it to the exact live develop SHA. This is what lets a
|
||||
# constantly-moving develop still hit the cache.
|
||||
local cache_key
|
||||
cache_key=$(
|
||||
{
|
||||
uname -m
|
||||
python --version
|
||||
node --version
|
||||
bench --version
|
||||
} | sha256sum | awk '{print $1}'
|
||||
)
|
||||
|
||||
echo "${bench_cache_dir}/frappe-bench-base-${cache_key}.tar.zst"
|
||||
}
|
||||
|
||||
restore_warm_bench() {
|
||||
bench_cache_archive=$(get_bench_cache_archive)
|
||||
[ -n "$bench_cache_archive" ] && [ -f "$bench_cache_archive" ] || return 1
|
||||
|
||||
echo "Restoring base bench from ${bench_cache_archive}"
|
||||
tar --use-compress-program=unzstd -xf "$bench_cache_archive" -C ~ || return 1
|
||||
[ -d ~/frappe-bench/apps/frappe/.git ] || return 1
|
||||
mkdir -p ~/frappe-bench/sites ~/frappe-bench/logs
|
||||
[ -f ~/frappe-bench/sites/apps.txt ] || printf "frappe\n" > ~/frappe-bench/sites/apps.txt
|
||||
[ -f ~/frappe-bench/sites/common_site_config.json ] || printf "{}\n" > ~/frappe-bench/sites/common_site_config.json
|
||||
|
||||
# Fast-forward the restored frappe to the EXACT live develop SHA fetched in phase 1, then
|
||||
# rebuild only what changed. The editable install means the venv tracks the new code with
|
||||
# no reinstall. Any failure returns non-zero so the caller falls back to a full bench init.
|
||||
if ! (
|
||||
cd ~/frappe-bench/apps/frappe || exit 1
|
||||
# Phase 1 already fetched ~/frappe to the exact live develop SHA. Fetch that commit
|
||||
# straight from it (bench init names the remote 'upstream', not 'origin', and points
|
||||
# it at this local clone — so a plain `git fetch origin` does not work).
|
||||
git fetch --no-tags "$HOME/frappe" HEAD || exit 1
|
||||
git checkout --force FETCH_HEAD || exit 1
|
||||
); then
|
||||
echo "Fast-forward to ${frappe_sha} failed; falling back to full init"
|
||||
rm -rf ~/frappe-bench
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Pick up any frappe dependency changes since the base was built (cached → fast if none),
|
||||
# so a develop commit that bumped requirements doesn't leave a stale venv.
|
||||
if ! ~/frappe-bench/env/bin/python -m pip install -q -e ~/frappe-bench/apps/frappe; then
|
||||
echo "frappe dependency refresh failed; falling back to full init"
|
||||
rm -rf ~/frappe-bench
|
||||
return 1
|
||||
fi
|
||||
|
||||
( cd ~/frappe-bench && CI=Yes bench build --app frappe ) || { rm -rf ~/frappe-bench; return 1; }
|
||||
return 0
|
||||
}
|
||||
|
||||
save_warm_bench() {
|
||||
if [ -z "${bench_cache_archive:-}" ] || [ -f "$bench_cache_archive" ]; then
|
||||
return
|
||||
fi
|
||||
|
||||
if [ -n "$bench_cache_dir" ] && [ ! -w "$bench_cache_dir" ]; then
|
||||
echo "Skipping warm bench save because ${bench_cache_dir} is not writable"
|
||||
return
|
||||
fi
|
||||
|
||||
local tmp_archive
|
||||
tmp_archive="${bench_cache_archive}.${$}.tmp"
|
||||
|
||||
echo "Saving warm bench to ${bench_cache_archive}"
|
||||
# Keep sites/common_site_config.json (the redis ports live there — dropping it makes the
|
||||
# restore path fall back to a default redis port that bench start never bound, so reinstall
|
||||
# fails with "redis ... connection refused"). Only the rebuildable sites/assets is excluded;
|
||||
# restore_warm_bench runs `bench build` to regenerate it.
|
||||
tar \
|
||||
--use-compress-program="zstd -T0 -3" \
|
||||
--exclude="frappe-bench/logs" \
|
||||
--exclude="frappe-bench/sites/assets" \
|
||||
-cf "$tmp_archive" \
|
||||
-C ~ frappe-bench
|
||||
mv "$tmp_archive" "$bench_cache_archive"
|
||||
}
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 2 — bench init and site setup
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
bench init --skip-assets --frappe-path ~/frappe --python "$(which python)" frappe-bench
|
||||
install_whktml() {
|
||||
# Re-use the .deb if the wkhtmltopdf cache step already restored it.
|
||||
if [ ! -f "$wkhtmltox_deb" ]; then
|
||||
wget -O "$wkhtmltox_deb" https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb
|
||||
fi
|
||||
sudo apt-get install -y "$wkhtmltox_deb"
|
||||
}
|
||||
if [ "${SKIP_WKHTMLTOX_SETUP:-0}" != "1" ]; then
|
||||
install_whktml &
|
||||
wkpid=$!
|
||||
else
|
||||
wkpid=
|
||||
fi
|
||||
|
||||
mkdir ~/frappe-bench/sites/test_site
|
||||
if ! restore_warm_bench; then
|
||||
bench init --skip-assets --frappe-path ~/frappe --python "$(which python)" frappe-bench
|
||||
|
||||
cd ~/frappe-bench || exit
|
||||
|
||||
sed -i 's/watch:/# watch:/g' Procfile
|
||||
sed -i 's/schedule:/# schedule:/g' Procfile
|
||||
sed -i 's/socketio:/# socketio:/g' Procfile
|
||||
sed -i 's/redis_socketio:/# redis_socketio:/g' Procfile
|
||||
|
||||
CI=Yes bench build --app frappe
|
||||
save_warm_bench
|
||||
fi
|
||||
|
||||
if [ -n "$wkpid" ]; then wait $wkpid; fi
|
||||
|
||||
mkdir -p ~/frappe-bench/sites/test_site
|
||||
|
||||
if [ "$DB" == "mariadb" ];then
|
||||
cp -r "${GITHUB_WORKSPACE}/.github/helper/site_config_mariadb.json" ~/frappe-bench/sites/test_site/site_config.json
|
||||
if [ "$db_host" != "127.0.0.1" ]; then
|
||||
sed -i "s/\"db_host\": \"127.0.0.1\"/\"db_host\": \"${db_host}\"/" ~/frappe-bench/sites/test_site/site_config.json
|
||||
fi
|
||||
else
|
||||
cp -r "${GITHUB_WORKSPACE}/.github/helper/site_config_postgres.json" ~/frappe-bench/sites/test_site/site_config.json
|
||||
fi
|
||||
|
||||
|
||||
if [ "$DB" == "mariadb" ];then
|
||||
mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "SET GLOBAL character_set_server = 'utf8mb4'"
|
||||
mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "SET GLOBAL collation_server = 'utf8mb4_unicode_ci'"
|
||||
for _ in {1..60}; do
|
||||
if mariadb-admin ping --host "$db_host" --port 3306 -u root -proot --silent; then
|
||||
break
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
mariadb-admin ping --host "$db_host" --port 3306 -u root -proot --silent
|
||||
|
||||
# Belt-and-suspenders: also set performance variables at runtime in case
|
||||
# MARIADB_EXTRA_FLAGS was not honoured by the container image.
|
||||
mariadb --host 127.0.0.1 --port 3306 -u root -proot \
|
||||
mariadb --host "$db_host" --port 3306 -u root -proot -e "SET GLOBAL character_set_server = 'utf8mb4'"
|
||||
mariadb --host "$db_host" --port 3306 -u root -proot -e "SET GLOBAL collation_server = 'utf8mb4_unicode_ci'"
|
||||
|
||||
# Throwaway-DB durability tuning at runtime. (innodb_doublewrite is read-only on MariaDB
|
||||
# 10.6, so it can't be disabled here — would need a server startup flag.)
|
||||
mariadb --host "$db_host" --port 3306 -u root -proot \
|
||||
-e "SET GLOBAL innodb_flush_log_at_trx_commit=0; SET GLOBAL sync_binlog=0;"
|
||||
|
||||
mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "CREATE USER 'test_frappe'@'localhost' IDENTIFIED BY 'test_frappe'"
|
||||
mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "CREATE DATABASE test_frappe"
|
||||
mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "GRANT ALL PRIVILEGES ON \`test_frappe\`.* TO 'test_frappe'@'localhost'"
|
||||
# Opt-in DDL speedup: a shared tablespace avoids a create+fsync per DocType table during
|
||||
# reinstall — a big win under disk contention. But ROW_FORMAT=DYNAMIC must be accepted in
|
||||
# the system tablespace on this MariaDB. Enable with CI_INNODB_SHARED_TABLESPACE=1; if
|
||||
# reinstall then errors on table creation, unset it (off by default — zero risk).
|
||||
if [ "${CI_INNODB_SHARED_TABLESPACE:-0}" = "1" ]; then
|
||||
mariadb --host "$db_host" --port 3306 -u root -proot -e "SET GLOBAL innodb_file_per_table=0;"
|
||||
fi
|
||||
|
||||
mariadb --host 127.0.0.1 --port 3306 -u root -proot -e "FLUSH PRIVILEGES"
|
||||
mariadb --host "$db_host" --port 3306 -u root -proot -e "CREATE USER 'test_frappe'@'${db_user_host}' IDENTIFIED BY 'test_frappe'"
|
||||
mariadb --host "$db_host" --port 3306 -u root -proot -e "CREATE DATABASE test_frappe"
|
||||
mariadb --host "$db_host" --port 3306 -u root -proot -e "GRANT ALL PRIVILEGES ON \`test_frappe\`.* TO 'test_frappe'@'${db_user_host}'"
|
||||
|
||||
mariadb --host "$db_host" --port 3306 -u root -proot -e "FLUSH PRIVILEGES"
|
||||
fi
|
||||
|
||||
if [ "$DB" == "postgres" ];then
|
||||
echo "travis" | psql -h 127.0.0.1 -p 5432 -c "CREATE DATABASE test_frappe" -U postgres;
|
||||
echo "travis" | psql -h 127.0.0.1 -p 5432 -c "CREATE USER test_frappe WITH PASSWORD 'test_frappe'" -U postgres;
|
||||
|
||||
# Durability-off for speed (no fsync/synchronous_commit/full_page_writes) is applied by
|
||||
# start-db.sh's postgres `-o` flags on every start — setup job AND each test shard — so it is
|
||||
# NOT repeated here. The postgres workflow runs in-runner via start-db.sh, not a service
|
||||
# container.
|
||||
fi
|
||||
|
||||
|
||||
install_whktml() {
|
||||
# Re-use the .deb if the wkhtmltopdf cache step already restored it.
|
||||
if [ ! -f /tmp/wkhtmltox.deb ]; then
|
||||
wget -O /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb
|
||||
fi
|
||||
sudo apt install /tmp/wkhtmltox.deb
|
||||
}
|
||||
install_whktml &
|
||||
wkpid=$!
|
||||
|
||||
|
||||
cd ~/frappe-bench || exit
|
||||
|
||||
sed -i 's/watch:/# watch:/g' Procfile
|
||||
sed -i 's/schedule:/# schedule:/g' Procfile
|
||||
sed -i 's/socketio:/# socketio:/g' Procfile
|
||||
sed -i 's/redis_socketio:/# redis_socketio:/g' Procfile
|
||||
run_ci_step "Get payments app" bench get-app payments --branch develop
|
||||
|
||||
bench get-app payments --branch develop
|
||||
bench get-app erpnext "${GITHUB_WORKSPACE}"
|
||||
# Opt-in: skip building erpnext's frontend assets. Server tests don't need them, but PDF
|
||||
# tests (print formats) do — they pass only if the PDF renderer ignores missing assets.
|
||||
# Enable with CI_SKIP_ERPNEXT_ASSETS=1 to test; if PDF tests fail, unset it.
|
||||
erpnext_get_app_args=()
|
||||
if [ "${CI_SKIP_ERPNEXT_ASSETS:-0}" = "1" ]; then erpnext_get_app_args=(--skip-assets); fi
|
||||
run_ci_step "Get erpnext app" bench get-app erpnext "${GITHUB_WORKSPACE}" "${erpnext_get_app_args[@]}"
|
||||
|
||||
if [ "$TYPE" == "server" ]; then bench setup requirements --dev; fi
|
||||
if [ "$TYPE" == "server" ]; then run_ci_step "Setup dev requirements" bench setup requirements --dev; fi
|
||||
|
||||
wait $wkpid
|
||||
bench start >> ~/frappe-bench/bench_start.log 2>&1 &
|
||||
|
||||
bench start &>> ~/frappe-bench/bench_start.log &
|
||||
CI=Yes bench build --app frappe &
|
||||
bench --site test_site reinstall --yes
|
||||
# Under heavy concurrency, gunicorn's startup can delay redis coming up. reinstall and the
|
||||
# tests need redis, so wait for it (best-effort, bounded) instead of racing — contention
|
||||
# then slows the job rather than failing it.
|
||||
wait_for_redis() {
|
||||
local cfg=~/frappe-bench/sites/common_site_config.json
|
||||
[ -f "$cfg" ] || return 0
|
||||
local ports port
|
||||
ports=$(python - "$cfg" <<'PY'
|
||||
import json, re, sys
|
||||
try:
|
||||
cfg = json.load(open(sys.argv[1]))
|
||||
except Exception:
|
||||
sys.exit(0)
|
||||
for key in ("redis_cache", "redis_queue"):
|
||||
match = re.search(r":(\d+)", str(cfg.get(key, "")))
|
||||
if match:
|
||||
print(match.group(1))
|
||||
PY
|
||||
)
|
||||
for port in $ports; do
|
||||
local up=0
|
||||
for _ in $(seq 1 120); do
|
||||
if (exec 3<>"/dev/tcp/127.0.0.1/$port") 2>/dev/null; then
|
||||
exec 3>&- 3<&-; up=1
|
||||
break
|
||||
fi
|
||||
sleep 1
|
||||
done
|
||||
# Fail clearly instead of letting reinstall die later on a vague socket-connection error
|
||||
# when redis never bound.
|
||||
[ "$up" = "1" ] || { echo "redis did not come up on port $port"; return 1; }
|
||||
done
|
||||
}
|
||||
wait_for_redis
|
||||
|
||||
# Site setup: build the schema (~1000 DocTypes) into the DB. This is the single-threaded-Python
|
||||
# bottleneck, but the fan-out amortises it — it runs once here in the setup job, and the test
|
||||
# shards start the DB on the baked datadir instead of repeating the reinstall.
|
||||
run_ci_step "Reinstall test site" bench --site test_site reinstall --yes
|
||||
|
||||
241
.github/helper/postgres_compat.py
vendored
Executable file
241
.github/helper/postgres_compat.py
vendored
Executable file
@@ -0,0 +1,241 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Static guard against MySQL-only SQL that breaks on PostgreSQL.
|
||||
|
||||
The Postgres test job is label-gated, so it does not run on every PR. This pre-commit
|
||||
hook is the always-on first line of defence: it flags the *mechanical* Postgres breaks
|
||||
that static analysis can catch reliably with a low false-positive rate.
|
||||
|
||||
It deliberately does NOT try to catch the *semantic* divergences (loose GROUP BY,
|
||||
case-sensitive ==/IN, NULL ordering, ORDER BY ... LIMIT 1 tiebreakers, integer-division
|
||||
intent, division by a possibly-zero divisor, savepoint discipline) — those genuinely need
|
||||
the test suite or a human/Greptile reviewer. Run the full suite on a Postgres site for those.
|
||||
|
||||
Escape hatch: put `# pg-ok` anywhere on the offending statement's line span (e.g. on a
|
||||
`SHOW INDEX` query that lives inside an `if frappe.db.db_type == "mariadb":` branch).
|
||||
|
||||
Usage: postgres_compat.py <file.py> [<file.py> ...] (pre-commit passes staged files)
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
import re
|
||||
import sys
|
||||
|
||||
IGNORE = "pg-ok"
|
||||
|
||||
# Strings are only scanned for the patterns below when they have real SQL *structure*
|
||||
# (not just an English word like "select" or "from"), to keep false positives near zero.
|
||||
SQL_HINT = re.compile(
|
||||
r"\bselect\b[\s\S]{0,800}\bfrom\b" # SELECT ... FROM
|
||||
r"|\bupdate\b[\s\S]{0,400}\bset\b" # UPDATE ... SET
|
||||
r"|\bdelete\s+from\b"
|
||||
r"|\binsert\s+into\b"
|
||||
r"|\bshow\s+(?:index|tables|columns)\b"
|
||||
r"|\bfrom\s+[\"'`]?tab", # FROM `tabDocType`
|
||||
re.I,
|
||||
)
|
||||
|
||||
# MySQL-only constructs with NO frappe auto-translation. (frappe.db.sql already rewrites
|
||||
# ifnull->coalesce on all engines and backtick/locate/REGEXP on Postgres, and .like()
|
||||
# renders ILIKE — so those are NOT listed here; flagging them would be false positives.)
|
||||
SQL_PATTERNS: list[tuple[re.Pattern, str]] = [
|
||||
(re.compile(r"\btimestamp\s*\(\s*[^,()]+,", re.I),
|
||||
"timestamp(date, time) is MySQL-only -> use CombineDatetime() or a precomputed datetime column"),
|
||||
(re.compile(r"\btimediff\s*\(", re.I),
|
||||
"timediff() is MySQL-only -> compute the delta in Python"),
|
||||
(re.compile(r"\bstr_to_date\s*\(", re.I),
|
||||
"str_to_date() is MySQL-only -> parse in Python and pass a real date"),
|
||||
(re.compile(r"\bdate_format\s*\(", re.I),
|
||||
"date_format() is MySQL-only -> filter on a date range instead"),
|
||||
(re.compile(r"\bdate_(add|sub)\s*\(", re.I),
|
||||
"date_add()/date_sub() are MySQL-only -> use Python date math or interval arithmetic"),
|
||||
(re.compile(r"\bgroup_concat\s*\(", re.I),
|
||||
"group_concat() is MySQL-only -> use GroupConcat (string_agg) or aggregate in Python"),
|
||||
(re.compile(r"\bperiod_diff\s*\(", re.I),
|
||||
"period_diff() is MySQL-only -> compute in Python"),
|
||||
(re.compile(r"\bshow\s+index\b", re.I),
|
||||
"SHOW INDEX is MySQL-only -> use frappe.db.has_index() / get_column_index()"),
|
||||
(re.compile(r"\bshow\s+(tables|columns)\b", re.I),
|
||||
"SHOW TABLES/COLUMNS is MySQL-only -> use frappe.db.get_tables()/table_columns / information-schema helpers"),
|
||||
(re.compile(r"\bas\s+'[^']+'", re.I),
|
||||
"single-quoted column alias breaks on Postgres -> use a bare or double-quoted alias"),
|
||||
(re.compile(r"\bif\s*\(", re.I),
|
||||
"SQL IF() is MySQL-only -> use CASE WHEN ... THEN ... ELSE ... END (frappe.qb.Case())"),
|
||||
(re.compile(r"\brlike\b", re.I),
|
||||
"RLIKE is MySQL-only -> frappe rewrites REGEXP->~* on Postgres but NOT RLIKE; use REGEXP / .regexp() / ~"),
|
||||
(re.compile(r"\bcast\s*\(.+?\bas\s+char\b", re.I | re.S), # .+? spans nested parens, e.g. CAST(ABS(x) AS CHAR)
|
||||
"CAST(... AS CHAR) is character(1) on Postgres and truncates -> CAST AS VARCHAR (frappe Cast_(x, 'varchar'))"),
|
||||
]
|
||||
|
||||
# UPDATE ... JOIN: both keywords in the same SQL string.
|
||||
UPDATE_JOIN = (re.compile(r"\bupdate\b", re.I), re.compile(r"\bjoin\b", re.I))
|
||||
|
||||
MYSQL_RESULT_KEYS = {"Column_name", "Key_name", "Seq_in_index", "Non_unique", "Index_type"}
|
||||
|
||||
SET_BOOL_FUNCS = {"set_value", "db_set"}
|
||||
|
||||
# query-builder cast helpers: pypika Cast / frappe Cast_. A "char" target type is character(1)
|
||||
# on Postgres (truncates); "varchar" is the full-length cast.
|
||||
CAST_FUNCS = {"Cast", "Cast_"}
|
||||
|
||||
# frappe.get_all / get_list: frappe's db_query SILENTLY drops ORDER BY for `distinct` queries on
|
||||
# Postgres (the ORDER BY column must appear in the SELECT-DISTINCT list), so `distinct=True` together
|
||||
# with a literal `order_by` is a no-op on PG and the result comes back unordered.
|
||||
DISTINCT_ORDER_FUNCS = {"get_all", "get_list"}
|
||||
|
||||
|
||||
def _docstring_ids(tree: ast.AST) -> set[int]:
|
||||
"""ids of Constant nodes that are docstrings (so prose describing the rules isn't flagged)."""
|
||||
ids: set[int] = set()
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, (ast.Module, ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
|
||||
body = getattr(node, "body", None)
|
||||
if body and isinstance(body[0], ast.Expr) and isinstance(body[0].value, ast.Constant) and isinstance(body[0].value.value, str):
|
||||
ids.add(id(body[0].value))
|
||||
return ids
|
||||
|
||||
|
||||
class Visitor(ast.NodeVisitor):
|
||||
def __init__(self, lines: list[str], docstrings: set[int]):
|
||||
self.lines = lines
|
||||
self.docstrings = docstrings
|
||||
self.violations: list[tuple[int, str]] = []
|
||||
|
||||
def _ignored(self, node: ast.AST) -> bool:
|
||||
start = getattr(node, "lineno", 1)
|
||||
end = getattr(node, "end_lineno", start) or start
|
||||
# honour `# pg-ok` anywhere on the node's line span, the line just above (the enclosing
|
||||
# call, e.g. `frappe.db.sql( # pg-ok`), or the line just below (a multi-line call's `) # pg-ok`).
|
||||
lo = max(0, start - 2)
|
||||
return any(IGNORE in self.lines[i] for i in range(lo, min(end + 1, len(self.lines))))
|
||||
|
||||
def _flag(self, node: ast.AST, msg: str) -> None:
|
||||
if not self._ignored(node):
|
||||
self.violations.append((getattr(node, "lineno", 1), msg))
|
||||
|
||||
def _scan_sql(self, text: str, node: ast.AST) -> None:
|
||||
if not SQL_HINT.search(text):
|
||||
return
|
||||
for pattern, msg in SQL_PATTERNS:
|
||||
if pattern.search(text):
|
||||
self._flag(node, msg)
|
||||
if UPDATE_JOIN[0].search(text) and UPDATE_JOIN[1].search(text):
|
||||
self._flag(node, "UPDATE ... JOIN is MySQL-only -> use a correlated subquery (WHERE ... IN/EXISTS)")
|
||||
|
||||
def visit_Constant(self, node: ast.Constant) -> None:
|
||||
# plain string literals, incl. `"...".format()` and `"..." % (...)` templates
|
||||
if isinstance(node.value, str) and id(node) not in self.docstrings:
|
||||
self._scan_sql(node.value, node)
|
||||
self.generic_visit(node)
|
||||
|
||||
def visit_JoinedStr(self, node: ast.JoinedStr) -> None:
|
||||
# f-string: scan its STATIC text (interpolated values become a placeholder) so MySQL-isms
|
||||
# in dynamic SQL are caught, without flagging safe interpolation of identifiers.
|
||||
text = "".join(
|
||||
v.value if isinstance(v, ast.Constant) and isinstance(v.value, str) else " ? "
|
||||
for v in node.values
|
||||
)
|
||||
self._scan_sql(text, node)
|
||||
# don't recurse: child literal chunks would otherwise be re-scanned individually
|
||||
|
||||
def visit_Call(self, node: ast.Call) -> None:
|
||||
fn = node.func
|
||||
name = fn.attr if isinstance(fn, ast.Attribute) else (fn.id if isinstance(fn, ast.Name) else "")
|
||||
|
||||
# row.get("Column_name") — MySQL SHOW INDEX result key
|
||||
if name == "get" and node.args and isinstance(node.args[0], ast.Constant) and node.args[0].value in MYSQL_RESULT_KEYS:
|
||||
self._flag(node, f'"{node.args[0].value}" is a MySQL SHOW INDEX result key -> use frappe.db.has_index()/get_column_index()')
|
||||
|
||||
# set_value(..., True) / db_set("field", True) on a Check (int) column.
|
||||
# Only the field *value* arg carries bool->smallint risk — NOT trailing flags like
|
||||
# update_modified. db_set(field, value, update_modified, ...) -> value at args[1] (or a dict
|
||||
# at args[0]); set_value(dt, dn, field, value, ...) -> value at args[3] (or a dict at args[2]).
|
||||
if name in SET_BOOL_FUNCS:
|
||||
value_idx, dict_idx = (1, 0) if name == "db_set" else (3, 2)
|
||||
dict_arg = (
|
||||
node.args[dict_idx]
|
||||
if len(node.args) > dict_idx and isinstance(node.args[dict_idx], ast.Dict)
|
||||
else None
|
||||
)
|
||||
if dict_arg is not None:
|
||||
for v in dict_arg.values:
|
||||
if isinstance(v, ast.Constant) and isinstance(v.value, bool):
|
||||
self._flag(node, f"{name}(...) sets an int/Check column with a bool in a dict -> pass 1/0 (Postgres rejects bool->smallint)")
|
||||
elif len(node.args) > value_idx:
|
||||
a = node.args[value_idx]
|
||||
if isinstance(a, ast.Constant) and isinstance(a.value, bool):
|
||||
self._flag(node, f"{name}(..., {a.value}) sets an int/Check column with a bool -> pass 1/0 (Postgres rejects bool->smallint)")
|
||||
|
||||
# frappe.get_all/get_list(..., distinct=True, order_by="<col>") -> ORDER BY is silently dropped
|
||||
# for distinct queries on Postgres, so the result is unordered there. Sort in python instead
|
||||
# (e.g. sorted(frappe.get_all(..., distinct=True), key=str.casefold)). An empty order_by="" (the
|
||||
# explicit "suppress the injected default" idiom) and a dynamic/variable order_by are not flagged.
|
||||
if name in DISTINCT_ORDER_FUNCS:
|
||||
has_distinct = any(
|
||||
kw.arg == "distinct" and isinstance(kw.value, ast.Constant) and kw.value.value
|
||||
for kw in node.keywords
|
||||
)
|
||||
order_kw = next((kw for kw in node.keywords if kw.arg == "order_by"), None)
|
||||
has_literal_order = (
|
||||
order_kw is not None
|
||||
and isinstance(order_kw.value, ast.Constant)
|
||||
and isinstance(order_kw.value.value, str)
|
||||
and order_kw.value.value.strip()
|
||||
)
|
||||
if has_distinct and has_literal_order:
|
||||
self._flag(node, f"{name}(distinct=True, order_by=...) -> frappe drops ORDER BY for distinct queries on Postgres; sort in python instead, e.g. sorted(..., key=str.casefold)")
|
||||
|
||||
# query-builder .rlike(...): pypika emits the MySQL-only RLIKE operator, which frappe does
|
||||
# NOT translate for Postgres (it rewrites only REGEXP -> ~*).
|
||||
if name == "rlike":
|
||||
self._flag(node, ".rlike() emits MySQL-only RLIKE (not translated on Postgres) -> use .regexp() (rewritten to ~*) or .like()")
|
||||
|
||||
# Cast(col, "char") / Cast_(col, "char"): on Postgres a bare CHAR is character(1) and truncates
|
||||
# (e.g. CAST(12 AS CHAR) -> '1'); use "varchar" for a full-length string cast.
|
||||
if name in CAST_FUNCS:
|
||||
for arg in (*node.args, *(kw.value for kw in node.keywords)):
|
||||
if isinstance(arg, ast.Constant) and isinstance(arg.value, str) and arg.value.strip().lower() == "char":
|
||||
self._flag(node, f"{name}(..., 'char') is character(1) on Postgres and truncates -> use 'varchar'")
|
||||
|
||||
self.generic_visit(node)
|
||||
|
||||
def visit_Subscript(self, node: ast.Subscript) -> None:
|
||||
key = node.slice
|
||||
if isinstance(key, ast.Constant) and key.value in MYSQL_RESULT_KEYS:
|
||||
self._flag(node, f'"{key.value}" is a MySQL SHOW INDEX result key -> use frappe.db.has_index()/get_column_index()')
|
||||
self.generic_visit(node)
|
||||
|
||||
|
||||
def check_file(path: str) -> list[str]:
|
||||
try:
|
||||
# nosemgrep: frappe-semgrep-rules.rules.security.frappe-security-file-traversal -- dev-only lint tool; `path` is a source file supplied by pre-commit, not user input
|
||||
src = open(path, encoding="utf-8").read()
|
||||
except (OSError, UnicodeDecodeError):
|
||||
return []
|
||||
try:
|
||||
tree = ast.parse(src, filename=path)
|
||||
except SyntaxError:
|
||||
return [] # check-ast hook reports real syntax errors
|
||||
v = Visitor(src.splitlines(), _docstring_ids(tree))
|
||||
v.visit(tree)
|
||||
return [f"{path}:{line}: [pg-compat] {msg}" for line, msg in sorted(set(v.violations))]
|
||||
|
||||
|
||||
def main(argv: list[str]) -> int:
|
||||
out: list[str] = []
|
||||
for path in argv:
|
||||
if path.endswith(".py"):
|
||||
out.extend(check_file(path))
|
||||
if out:
|
||||
print("\n".join(out))
|
||||
print(
|
||||
f"\n{len(out)} PostgreSQL-incompatibility issue(s). Fix them, or add `# pg-ok` to a "
|
||||
"line that is intentionally MariaDB-only (e.g. inside an `if frappe.db.db_type == 'mariadb':` branch)."
|
||||
)
|
||||
return 1
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main(sys.argv[1:]))
|
||||
2
.github/helper/site_config_postgres.json
vendored
2
.github/helper/site_config_postgres.json
vendored
@@ -13,6 +13,6 @@
|
||||
"root_login": "postgres",
|
||||
"root_password": "travis",
|
||||
"host_name": "http://test_site:8000",
|
||||
"install_apps": ["erpnext"],
|
||||
"install_apps": ["payments", "erpnext"],
|
||||
"throttle_user_limit": 100
|
||||
}
|
||||
|
||||
79
.github/helper/start-db.sh
vendored
Executable file
79
.github/helper/start-db.sh
vendored
Executable file
@@ -0,0 +1,79 @@
|
||||
#!/bin/bash
|
||||
#
|
||||
# Run MariaDB INSIDE the runner container, on a datadir we control. Because the datadir can be
|
||||
# packaged into the bench artifact, test shards start an already-loaded server instead of
|
||||
# replaying a SQL dump (the ~60s hydrate restore). Each shard gets its own copy → isolation kept.
|
||||
#
|
||||
# CI_DB_DATADIR picks the path:
|
||||
# - setup job: /home/ci/db-data (OUTSIDE the bench, so install.sh's `rm -rf ~/frappe-bench`
|
||||
# doesn't wipe it; it's moved into the bench just before packaging)
|
||||
# - test shard: ~/frappe-bench/mariadb-data (where the artifact untar'd it)
|
||||
#
|
||||
# Idempotent: inits a fresh datadir if absent (setup), else starts on the existing one (shards).
|
||||
#
|
||||
set -e
|
||||
|
||||
ci_user="${ERPNEXT_CI_USER:-frappe}"
|
||||
|
||||
# Re-exec as the ci user so mariadbd and the datadir are owned consistently (root mariadbd is
|
||||
# refused anyway). Mirrors install.sh's user switch.
|
||||
if [ "$(id -u)" = "0" ] && [ "${SKIP_SYSTEM_SETUP:-0}" = "1" ] && [ "$ci_user" != "root" ]; then
|
||||
exec su -m "$ci_user" -s /bin/bash -c \
|
||||
"ERPNEXT_CI_USER='$ci_user' CI_DB_DATADIR='${CI_DB_DATADIR:-}' DB='${DB:-}' bash '$0'"
|
||||
fi
|
||||
|
||||
# --- PostgreSQL (GitHub-hosted CI): run in-runner on a PGDATA so it bakes into the artifact,
|
||||
# same idea as the mariadb datadir. Trust auth (throwaway CI) skips password setup; durability
|
||||
# off for speed. Postgres is preinstalled on ubuntu-latest under /usr/lib/postgresql/<ver>/bin.
|
||||
if [ "${DB:-mariadb}" = "postgres" ]; then
|
||||
PG_BIN=$(ls -d /usr/lib/postgresql/*/bin 2>/dev/null | sort -V | tail -1)
|
||||
[ -n "$PG_BIN" ] && export PATH="$PG_BIN:$PATH"
|
||||
PGDATA="${CI_DB_DATADIR:-$HOME/frappe-bench/pgdata}"
|
||||
if [ ! -d "$PGDATA/base" ]; then
|
||||
initdb -D "$PGDATA" -U postgres --auth-local=trust --auth-host=trust >/dev/null
|
||||
echo "host all all 127.0.0.1/32 trust" >> "$PGDATA/pg_hba.conf"
|
||||
fi
|
||||
pg_ctl -D "$PGDATA" -w -o "-p 5432 -c listen_addresses=127.0.0.1 -c unix_socket_directories=$PGDATA -c fsync=off -c synchronous_commit=off -c full_page_writes=off" start
|
||||
echo "PostgreSQL up in-runner (pgdata=$PGDATA)"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# --- MariaDB ---
|
||||
DATADIR="${CI_DB_DATADIR:-$HOME/frappe-bench/mariadb-data}"
|
||||
SOCK="$DATADIR/mysqld.sock"
|
||||
fresh=0
|
||||
|
||||
if [ ! -d "$DATADIR/mysql" ]; then
|
||||
mkdir -p "$DATADIR"
|
||||
mariadb-install-db --no-defaults --datadir="$DATADIR" \
|
||||
--auth-root-authentication-method=normal --skip-test-db >/dev/null 2>&1
|
||||
fresh=1
|
||||
fi
|
||||
|
||||
# Throwaway-CI durability off; bind TCP 127.0.0.1:3306 so bench/install.sh connect as usual.
|
||||
mariadbd --no-defaults --datadir="$DATADIR" --socket="$SOCK" --pid-file="$DATADIR/mysqld.pid" \
|
||||
--port=3306 --bind-address=127.0.0.1 \
|
||||
--innodb-flush-log-at-trx-commit=0 --sync-binlog=0 --skip-log-bin \
|
||||
> "$HOME/mariadb.log" 2>&1 &
|
||||
|
||||
up=0
|
||||
for _ in $(seq 1 60); do
|
||||
if mariadb-admin --socket="$SOCK" ping --silent 2>/dev/null; then up=1; break; fi
|
||||
sleep 1
|
||||
done
|
||||
# Fail loudly instead of letting the loop fall through (exit 0 of the last `sleep`) into SQL that
|
||||
# would error with a vague socket-connection failure.
|
||||
[ "$up" = "1" ] || { echo "mariadbd did not come up on $SOCK"; cat "$HOME/mariadb.log" 2>/dev/null; exit 1; }
|
||||
|
||||
if [ "$fresh" = "1" ]; then
|
||||
# A fresh datadir has only a password-less root@localhost. Give it the password install.sh
|
||||
# uses, plus a TCP-reachable root@127.0.0.1, so the rest of install.sh works unchanged.
|
||||
mariadb --no-defaults --socket="$SOCK" -u root <<'SQL'
|
||||
ALTER USER 'root'@'localhost' IDENTIFIED BY 'root';
|
||||
CREATE USER IF NOT EXISTS 'root'@'127.0.0.1' IDENTIFIED BY 'root';
|
||||
GRANT ALL PRIVILEGES ON *.* TO 'root'@'127.0.0.1' WITH GRANT OPTION;
|
||||
FLUSH PRIVILEGES;
|
||||
SQL
|
||||
fi
|
||||
|
||||
echo "MariaDB up in-container (datadir=$DATADIR, fresh=$fresh)"
|
||||
2
.github/workflows/linters.yml
vendored
2
.github/workflows/linters.yml
vendored
@@ -21,7 +21,7 @@ jobs:
|
||||
cache: pip
|
||||
|
||||
- name: Install and Run Pre-commit
|
||||
uses: pre-commit/action@v3.0.0
|
||||
uses: pre-commit/action@v3.0.1
|
||||
|
||||
semgrep:
|
||||
name: semgrep
|
||||
|
||||
33
.github/workflows/patch.yml
vendored
33
.github/workflows/patch.yml
vendored
@@ -65,6 +65,22 @@ jobs:
|
||||
- name: Add to Hosts
|
||||
run: echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
|
||||
|
||||
# The v14 baseline backup is a fixed published file — cache it instead of re-downloading
|
||||
# it from the GitHub release every run.
|
||||
- name: Cache erpnext v14 backup
|
||||
id: cache-v14
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: ~/erpnext-v14.sql.gz
|
||||
key: erpnext-v14-sql-gz
|
||||
|
||||
- name: Download erpnext v14 backup
|
||||
if: steps.cache-v14.outputs.cache-hit != 'true'
|
||||
run: |
|
||||
curl -fSL --retry 5 --retry-all-errors --retry-delay 5 \
|
||||
-o ~/erpnext-v14.sql.gz \
|
||||
https://github.com/frappe/erpnext/releases/download/v14-baseline/erpnext-v14.sql.gz
|
||||
|
||||
- name: Cache pip
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
@@ -113,12 +129,20 @@ jobs:
|
||||
jq 'del(.install_apps)' ~/frappe-bench/sites/test_site/site_config.json > tmp.json
|
||||
mv tmp.json ~/frappe-bench/sites/test_site/site_config.json
|
||||
|
||||
wget https://frappe.io/files/erpnext-v14.sql.gz
|
||||
bench --site test_site --force restore ~/frappe-bench/erpnext-v14.sql.gz
|
||||
bench --site test_site --force restore ~/erpnext-v14.sql.gz
|
||||
|
||||
git -C "apps/frappe" remote set-url upstream https://github.com/frappe/frappe.git
|
||||
git -C "apps/erpnext" remote set-url upstream https://github.com/frappe/erpnext.git
|
||||
|
||||
# Start every bench process except the background workers. If workers run during a
|
||||
# migrate, they pick up the orphan-link cleanup jobs it enqueues and race its schema
|
||||
# changes, which fails with MySQL 1412 "Table definition has changed". Redis and the
|
||||
# other services stay up; the queued jobs simply wait and are harmless here.
|
||||
function start_bench_without_workers() {
|
||||
local procs
|
||||
procs=$(awk -F: '/^[a-z_]+:/ && $1 !~ /worker/ {print $1}' ~/frappe-bench/Procfile)
|
||||
honcho start -f ~/frappe-bench/Procfile $procs &>> ~/frappe-bench/bench_start.log &
|
||||
}
|
||||
|
||||
function update_to_version() {
|
||||
version=$1
|
||||
@@ -134,10 +158,11 @@ jobs:
|
||||
|
||||
# Resetup env and install apps
|
||||
pgrep honcho | xargs kill
|
||||
sleep 10
|
||||
rm -rf ~/frappe-bench/env
|
||||
bench -v setup env --python python$2
|
||||
bench pip install -e ./apps/erpnext
|
||||
bench start &>> ~/frappe-bench/bench_start.log &
|
||||
start_bench_without_workers
|
||||
|
||||
bench --site test_site migrate
|
||||
}
|
||||
@@ -154,7 +179,7 @@ jobs:
|
||||
rm -rf ~/frappe-bench/env
|
||||
bench -v setup env
|
||||
bench pip install -e ./apps/erpnext
|
||||
bench start &>> ~/frappe-bench/bench_start.log &
|
||||
start_bench_without_workers
|
||||
|
||||
bench --site test_site migrate
|
||||
|
||||
|
||||
@@ -22,4 +22,4 @@ jobs:
|
||||
pull-requests: write
|
||||
|
||||
steps:
|
||||
- uses: alyf-de/po-review-action@v1.0.0
|
||||
- uses: alyf-de/po-review-action@v1.1.0
|
||||
|
||||
203
.github/workflows/server-tests-mariadb.yml
vendored
203
.github/workflows/server-tests-mariadb.yml
vendored
@@ -31,51 +31,49 @@ on:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
packages: read
|
||||
|
||||
concurrency:
|
||||
group: server-mariadb-develop-${{ github.event_name }}-${{ github.event.number || github.event_name == 'workflow_dispatch' && github.run_id || '' }}
|
||||
cancel-in-progress: true
|
||||
|
||||
# Shared across both jobs. Both run in the SAME CI image so the bench lives at the identical
|
||||
# path (/home/ci/frappe-bench) on the setup runner and the test shards — that's what makes the
|
||||
# packaged Python venv portable between them.
|
||||
env:
|
||||
TZ: 'Asia/Kolkata'
|
||||
DEBIAN_FRONTEND: noninteractive
|
||||
NODE_ENV: "production"
|
||||
WITH_COVERAGE: ${{ github.event_name != 'pull_request' }}
|
||||
ERPNEXT_CI_USER: ci
|
||||
PIP_CACHE_DIR: /home/ci/.cache/pip
|
||||
npm_config_cache: /home/ci/.cache/npm
|
||||
YARN_CACHE_FOLDER: /home/ci/.cache/yarn
|
||||
UV_CACHE_DIR: /home/ci/.cache/uv
|
||||
|
||||
jobs:
|
||||
test:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 60
|
||||
env:
|
||||
TZ: 'Asia/Kolkata'
|
||||
NODE_ENV: "production"
|
||||
WITH_COVERAGE: ${{ github.event_name != 'pull_request' }}
|
||||
|
||||
strategy:
|
||||
fail-fast: false
|
||||
|
||||
matrix:
|
||||
container: [1, 2, 3, 4]
|
||||
|
||||
name: Python Unit Tests
|
||||
|
||||
services:
|
||||
mysql:
|
||||
image: mariadb:10.6
|
||||
env:
|
||||
TZ: 'Asia/Kolkata'
|
||||
MARIADB_ROOT_PASSWORD: 'root'
|
||||
# Disable durability guarantees that are unnecessary in a throwaway CI container.
|
||||
# innodb_flush_log_at_trx_commit=0 avoids an fsync on every commit (biggest win).
|
||||
# sync_binlog=0 skips binary-log syncs; innodb_doublewrite=0 skips the doublewrite buffer.
|
||||
MARIADB_EXTRA_FLAGS: --innodb-flush-log-at-trx-commit=0 --sync-binlog=0 --innodb-doublewrite=0
|
||||
ports:
|
||||
- 3306:3306
|
||||
options: --health-cmd="mariadb-admin ping" --health-interval=5s --health-timeout=2s --health-retries=3
|
||||
|
||||
# Build the bench (clone + pip + yarn + assets) and reinstall test_site ONCE, on a free
|
||||
# GitHub-hosted runner, then publish the whole bench (with a DB dump baked in) as an artifact.
|
||||
# The expensive, non-parallelisable work happens here exactly once instead of on every shard.
|
||||
setup:
|
||||
name: Build & reinstall (setup)
|
||||
# Dedicated scale set (fat cpu request) so the build+reinstall runs at full speed, uncontended
|
||||
# by the many thin test shards. Same CI image + /home/ci path + 127.0.0.1 DB as the shards,
|
||||
# so the packaged bench (and its venv) transplants cleanly.
|
||||
runs-on: erpnext-arc-setup
|
||||
timeout-minutes: 40
|
||||
container:
|
||||
image: ghcr.io/frappe/erpnext-ci-mariadb:py3.14-node24
|
||||
credentials:
|
||||
username: ${{ secrets.GHCR_USERNAME || github.actor }}
|
||||
password: ${{ secrets.GHCR_TOKEN || github.token }}
|
||||
defaults:
|
||||
run:
|
||||
shell: bash
|
||||
steps:
|
||||
- name: Clone
|
||||
uses: actions/checkout@v6
|
||||
|
||||
- name: Setup Python
|
||||
uses: actions/setup-python@v6
|
||||
with:
|
||||
python-version: '3.14'
|
||||
|
||||
- name: Check for valid Python & Merge Conflicts
|
||||
run: |
|
||||
python -m compileall -fq "${GITHUB_WORKSPACE}"
|
||||
@@ -84,53 +82,17 @@ jobs:
|
||||
exit 1
|
||||
fi
|
||||
|
||||
- name: Setup Node
|
||||
uses: actions/setup-node@v6
|
||||
with:
|
||||
node-version: 24
|
||||
check-latest: true
|
||||
|
||||
- name: Add to Hosts
|
||||
run: echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
|
||||
|
||||
- name: Cache pip
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: ~/.cache/pip
|
||||
key: ${{ runner.os }}-pip-${{ hashFiles('**/*requirements.txt', '**/pyproject.toml') }}
|
||||
restore-keys: |
|
||||
${{ runner.os }}-pip-
|
||||
${{ runner.os }}-
|
||||
|
||||
- name: Cache node modules
|
||||
uses: actions/cache@v4
|
||||
# MariaDB runs in-container on a datadir OUTSIDE the bench, because install.sh's next step
|
||||
# does `rm -rf ~/frappe-bench`. After the reinstall, the datadir is moved into the bench so
|
||||
# it ships in the artifact — test shards then start an already-loaded server (no restore).
|
||||
- name: Start DB
|
||||
run: bash ${GITHUB_WORKSPACE}/.github/helper/start-db.sh
|
||||
env:
|
||||
cache-name: cache-node-modules
|
||||
with:
|
||||
path: ~/.npm
|
||||
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
|
||||
restore-keys: |
|
||||
${{ runner.os }}-build-${{ env.cache-name }}-
|
||||
${{ runner.os }}-build-
|
||||
${{ runner.os }}-
|
||||
|
||||
- name: Get yarn cache directory path
|
||||
id: yarn-cache-dir-path
|
||||
run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT
|
||||
|
||||
- uses: actions/cache@v4
|
||||
id: yarn-cache
|
||||
with:
|
||||
path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
|
||||
key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
|
||||
restore-keys: |
|
||||
${{ runner.os }}-yarn-
|
||||
|
||||
- name: Cache wkhtmltopdf
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: /tmp/wkhtmltox.deb
|
||||
key: wkhtmltox-0.12.6.1-2-jammy-amd64
|
||||
SKIP_SYSTEM_SETUP: "1"
|
||||
CI_DB_DATADIR: /home/ci/db-data
|
||||
|
||||
- name: Install
|
||||
run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh
|
||||
@@ -139,9 +101,88 @@ jobs:
|
||||
TYPE: server
|
||||
FRAPPE_USER: ${{ github.event.inputs.user }}
|
||||
FRAPPE_BRANCH: ${{ github.event.client_payload.sha || github.event.inputs.branch }}
|
||||
DB_HOST: 127.0.0.1
|
||||
DB_USER_HOST: '%'
|
||||
WKHTMLTOX_DEB: /tmp/wkhtmltox.deb
|
||||
SKIP_SYSTEM_SETUP: "1"
|
||||
SKIP_WKHTMLTOX_SETUP: "1"
|
||||
|
||||
- name: Warm up test data
|
||||
run: |
|
||||
su -m "${ERPNEXT_CI_USER:-frappe}" -s /bin/bash <<'EOF'
|
||||
cd ~/frappe-bench/
|
||||
bench --site test_site run-tests --lightmode --module erpnext.tests.bootstrap_test_data
|
||||
EOF
|
||||
|
||||
# Clean shutdown (consistent InnoDB datadir), then stage it inside the bench for packaging.
|
||||
- name: Stop DB and stage datadir
|
||||
run: |
|
||||
mariadb-admin -h 127.0.0.1 -P 3306 -u root -proot shutdown || true
|
||||
for _ in $(seq 1 30); do [ -f /home/ci/db-data/mysqld.pid ] || break; sleep 1; done
|
||||
# Don't bake a dirty datadir — fail if mariadbd didn't finish stopping, rather than ship
|
||||
# an inconsistent datadir the shards would have to crash-recover.
|
||||
[ -f /home/ci/db-data/mysqld.pid ] && { echo "mariadbd did not shut down cleanly"; exit 1; }
|
||||
mv /home/ci/db-data /home/ci/frappe-bench/mariadb-data
|
||||
|
||||
# Package the whole bench (apps, venv, node_modules, sites, the DB dump, and hydrate.sh)
|
||||
# into one artifact for the test shards to consume.
|
||||
# Single-node hand-off: stage the bench on a node-local hostPath instead of round-tripping
|
||||
# through GitHub artifact storage (~60s/shard). Setup and shards share the same disk, so
|
||||
# the shards just untar it locally. NOTE: this assumes one node — a shard on a different
|
||||
# node could not read this path (then you'd need GitHub artifacts or an NFS/RWX volume).
|
||||
- name: Stage bench on node (hostPath)
|
||||
run: |
|
||||
cp "${GITHUB_WORKSPACE}/.github/helper/hydrate.sh" /home/ci/frappe-bench/hydrate.sh
|
||||
cp "${GITHUB_WORKSPACE}/.github/helper/start-db.sh" /home/ci/frappe-bench/start-db.sh
|
||||
mkdir -p /opt/ci-bench-staging
|
||||
# self-clean: drop bench tars from runs older than 2h
|
||||
find /opt/ci-bench-staging -maxdepth 1 -name '*.tar.gz' -mmin +120 -delete 2>/dev/null || true
|
||||
# Exclude .git/node_modules; the mariadb-data datadir IS included (the pre-loaded DB).
|
||||
tar czpf "/opt/ci-bench-staging/${GITHUB_RUN_ID}.tar.gz" -C /home/ci \
|
||||
--exclude='.git' --exclude='node_modules' frappe-bench
|
||||
ls -lh "/opt/ci-bench-staging/${GITHUB_RUN_ID}.tar.gz"
|
||||
|
||||
# Fan-out: each shard downloads the bench, untars it, starts MariaDB on the baked datadir, and
|
||||
# runs its slice of the suite. No clone, no build, no reinstall, no DB dump restore on the shards.
|
||||
test:
|
||||
name: Python Unit Tests
|
||||
needs: setup
|
||||
runs-on: erpnext-arc
|
||||
timeout-minutes: 60
|
||||
container:
|
||||
image: ghcr.io/frappe/erpnext-ci-mariadb:py3.14-node24
|
||||
credentials:
|
||||
username: ${{ secrets.GHCR_USERNAME || github.actor }}
|
||||
password: ${{ secrets.GHCR_TOKEN || github.token }}
|
||||
defaults:
|
||||
run:
|
||||
shell: bash
|
||||
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
container: [1, 2, 3, 4]
|
||||
|
||||
steps:
|
||||
- name: Add to Hosts
|
||||
run: echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
|
||||
|
||||
# Read the bench straight from the node-local hostPath the setup job staged it on — no
|
||||
# GitHub download. -p preserves the ci (uid 1001) ownership so bench runs as ci cleanly.
|
||||
- name: Untar bench from node (hostPath)
|
||||
run: |
|
||||
tar xzpf "/opt/ci-bench-staging/${GITHUB_RUN_ID}.tar.gz" -C /home/ci
|
||||
ls -ld /home/ci/frappe-bench
|
||||
|
||||
- name: Hydrate (start DB on baked datadir + bench start)
|
||||
run: bash /home/ci/frappe-bench/hydrate.sh
|
||||
env:
|
||||
DB_HOST: 127.0.0.1
|
||||
SKIP_SYSTEM_SETUP: "1"
|
||||
|
||||
- name: Run Tests
|
||||
run: |
|
||||
su -m "${ERPNEXT_CI_USER:-frappe}" -s /bin/bash <<'EOF'
|
||||
cd ~/frappe-bench/
|
||||
coverage_flag=""
|
||||
if [ "$WITH_COVERAGE" = "true" ]; then coverage_flag="--with-coverage"; fi
|
||||
@@ -149,10 +190,10 @@ jobs:
|
||||
--total-builds ${{ strategy.job-total }} \
|
||||
--build-number ${{ matrix.container }} \
|
||||
$coverage_flag
|
||||
EOF
|
||||
env:
|
||||
TYPE: server
|
||||
|
||||
|
||||
- name: Show bench output
|
||||
if: ${{ always() }}
|
||||
run: cat ~/frappe-bench/bench_start.log || true
|
||||
@@ -162,11 +203,11 @@ jobs:
|
||||
uses: actions/upload-artifact@v4
|
||||
with:
|
||||
name: coverage-${{ matrix.container }}
|
||||
path: /home/runner/frappe-bench/sites/coverage.xml
|
||||
path: /home/ci/frappe-bench/sites/coverage.xml
|
||||
|
||||
coverage:
|
||||
name: Coverage Wrap Up
|
||||
needs: test
|
||||
needs: [test]
|
||||
if: ${{ github.event_name != 'pull_request' }}
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
|
||||
194
.github/workflows/server-tests-postgres.yml
vendored
194
.github/workflows/server-tests-postgres.yml
vendored
@@ -1,7 +1,12 @@
|
||||
name: Server (Postgres)
|
||||
|
||||
on:
|
||||
schedule:
|
||||
# 03:00 AM IST daily (21:30 UTC the previous day)
|
||||
- cron: "30 21 * * *"
|
||||
pull_request:
|
||||
# 'labeled' so adding the 'postgres' label to an already-open PR re-triggers the run.
|
||||
types: [opened, reopened, synchronize, labeled]
|
||||
paths-ignore:
|
||||
- '**.js'
|
||||
- '**.md'
|
||||
@@ -9,7 +14,7 @@ on:
|
||||
- 'crowdin.yml'
|
||||
- '.coderabbit.yml'
|
||||
- '.mergify.yml'
|
||||
types: [opened, labelled, synchronize, reopened]
|
||||
workflow_dispatch:
|
||||
|
||||
concurrency:
|
||||
group: server-postgres-develop-${{ github.event_name }}-${{ github.event.number || github.event_name == 'workflow_dispatch' && github.run_id || '' }}
|
||||
@@ -18,41 +23,31 @@ concurrency:
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
# Postgres CI stays on GitHub-hosted (free, full-speed VM per shard) but follows the same fan-out
|
||||
# we built for MariaDB: build the bench + reinstall ONCE in the setup job, bake the PostgreSQL
|
||||
# PGDATA into the artifact, and have 4 test shards start Postgres on that datadir — no per-shard
|
||||
# clone/build/reinstall/restore. Python is pinned so the venv transplants between VMs.
|
||||
env:
|
||||
TZ: 'Asia/Kolkata'
|
||||
NODE_ENV: "production"
|
||||
PYTHON_VERSION: '3.14'
|
||||
|
||||
jobs:
|
||||
test:
|
||||
if: ${{ contains(github.event.pull_request.labels.*.name, 'postgres') }}
|
||||
setup:
|
||||
name: Build & reinstall (setup)
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 60
|
||||
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
container: [1]
|
||||
|
||||
name: Python Unit Tests
|
||||
|
||||
services:
|
||||
postgres:
|
||||
image: postgres:13.3
|
||||
env:
|
||||
POSTGRES_PASSWORD: travis
|
||||
options: >-
|
||||
--health-cmd pg_isready
|
||||
--health-interval 10s
|
||||
--health-timeout 5s
|
||||
--health-retries 5
|
||||
ports:
|
||||
- 5432:5432
|
||||
|
||||
# Runs on the daily schedule (and workflow_dispatch). On PRs it runs ONLY when the PR carries
|
||||
# the 'postgres' label — the test job needs setup, so it's skipped too when this is.
|
||||
if: github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'postgres')
|
||||
timeout-minutes: 40
|
||||
steps:
|
||||
|
||||
- name: Clone
|
||||
uses: actions/checkout@v6
|
||||
|
||||
- name: Setup Python
|
||||
uses: actions/setup-python@v6
|
||||
with:
|
||||
python-version: '3.14'
|
||||
python-version: ${{ env.PYTHON_VERSION }}
|
||||
|
||||
- name: Check for valid Python & Merge Conflicts
|
||||
run: |
|
||||
@@ -71,48 +66,133 @@ jobs:
|
||||
- name: Add to Hosts
|
||||
run: echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
|
||||
|
||||
- name: Cache pip
|
||||
- name: Cache deps (uv/pip/npm/yarn)
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: ~/.cache/pip
|
||||
key: ${{ runner.os }}-pip-${{ hashFiles('**/*requirements.txt', '**/pyproject.toml') }}
|
||||
restore-keys: |
|
||||
${{ runner.os }}-pip-
|
||||
${{ runner.os }}-
|
||||
path: |
|
||||
~/.cache/uv
|
||||
~/.cache/pip
|
||||
~/.npm
|
||||
~/.cache/yarn
|
||||
key: ${{ runner.os }}-deps-${{ hashFiles('**/*requirements.txt', '**/pyproject.toml', '**/yarn.lock') }}
|
||||
restore-keys: ${{ runner.os }}-deps-
|
||||
|
||||
- name: Cache node modules
|
||||
# Warm-bench cache (the big one): install.sh saves the built base bench — frappe + env +
|
||||
# node_modules + assets — here as frappe-bench-base-*.tar.zst. Later runs restore it and only
|
||||
# fast-forward to the live develop SHA + rebuild the delta, so the bench BUILD is near-free and
|
||||
# only the test_site reinstall (per-run DB, uncacheable) stays slow — matching the self-hosted
|
||||
# box. The first run after a deps change populates it; every run after that is fast.
|
||||
- name: Cache warm bench (base build)
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: ~/bench-cache
|
||||
key: ${{ runner.os }}-warmbench-v2-${{ hashFiles('**/*requirements.txt', '**/pyproject.toml', '**/yarn.lock') }}
|
||||
restore-keys: ${{ runner.os }}-warmbench-v2-
|
||||
|
||||
# Postgres runs in-runner on a PGDATA OUTSIDE the bench (install.sh wipes ~/frappe-bench);
|
||||
# after the reinstall it's moved into the bench so it ships in the artifact.
|
||||
- name: Start DB
|
||||
run: bash ${GITHUB_WORKSPACE}/.github/helper/start-db.sh
|
||||
env:
|
||||
cache-name: cache-node-modules
|
||||
with:
|
||||
path: ~/.npm
|
||||
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
|
||||
restore-keys: |
|
||||
${{ runner.os }}-build-${{ env.cache-name }}-
|
||||
${{ runner.os }}-build-
|
||||
${{ runner.os }}-
|
||||
|
||||
- name: Get yarn cache directory path
|
||||
id: yarn-cache-dir-path
|
||||
run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT
|
||||
|
||||
- uses: actions/cache@v4
|
||||
id: yarn-cache
|
||||
with:
|
||||
path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
|
||||
key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
|
||||
restore-keys: |
|
||||
${{ runner.os }}-yarn-
|
||||
DB: postgres
|
||||
CI_DB_DATADIR: /home/runner/pgdata
|
||||
|
||||
- name: Install
|
||||
run: bash ${GITHUB_WORKSPACE}/.github/helper/install.sh
|
||||
env:
|
||||
DB: postgres
|
||||
TYPE: server
|
||||
FRAPPE_BRANCH: develop
|
||||
BENCH_CACHE_DIR: /home/runner/bench-cache
|
||||
|
||||
- name: Warm up test data
|
||||
run: |
|
||||
cd ~/frappe-bench/
|
||||
bench --site test_site run-tests --lightmode --module erpnext.tests.bootstrap_test_data
|
||||
|
||||
- name: Stop DB and stage datadir
|
||||
run: |
|
||||
PG_BIN=$(ls -d /usr/lib/postgresql/*/bin | sort -V | tail -1)
|
||||
# Clean shutdown so the baked datadir is consistent. Do NOT swallow a failed stop with
|
||||
# `|| true`: moving and tarring a still-running cluster ships a torn datadir the shards
|
||||
# cannot crash-recover (full_page_writes is off). Fail the job instead — mirrors the
|
||||
# MariaDB sister's "don't bake a dirty datadir" guard.
|
||||
"$PG_BIN/pg_ctl" -D /home/runner/pgdata -m fast -w stop
|
||||
mv /home/runner/pgdata /home/runner/frappe-bench/pgdata
|
||||
|
||||
- name: Package bench for test shards
|
||||
run: |
|
||||
cp "${GITHUB_WORKSPACE}/.github/helper/hydrate.sh" /home/runner/frappe-bench/hydrate.sh
|
||||
cp "${GITHUB_WORKSPACE}/.github/helper/start-db.sh" /home/runner/frappe-bench/start-db.sh
|
||||
tar czpf "${GITHUB_WORKSPACE}/bench.tar.gz" -C /home/runner \
|
||||
--exclude='.git' --exclude='node_modules' frappe-bench
|
||||
ls -lh "${GITHUB_WORKSPACE}/bench.tar.gz"
|
||||
|
||||
- name: Upload bench artifact
|
||||
uses: actions/upload-artifact@v4
|
||||
with:
|
||||
name: bench-pg
|
||||
path: bench.tar.gz
|
||||
retention-days: 1
|
||||
compression-level: 0
|
||||
|
||||
test:
|
||||
name: Python Unit Tests
|
||||
needs: setup
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 60
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
container: [1, 2, 3, 4]
|
||||
steps:
|
||||
- name: Download bench artifact
|
||||
uses: actions/download-artifact@v4
|
||||
with:
|
||||
name: bench-pg
|
||||
|
||||
- name: Setup Python
|
||||
uses: actions/setup-python@v6
|
||||
with:
|
||||
python-version: ${{ env.PYTHON_VERSION }}
|
||||
|
||||
- name: Add to Hosts
|
||||
run: echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts
|
||||
|
||||
# The bench CLI (frappe-bench) and redis are global/system tools — not in the bench tarball.
|
||||
# The setup runner got them via install.sh; the MariaDB shards get them from the arc5 image.
|
||||
# GitHub-hosted PG shards install them here (cheap vs the build+reinstall that setup did once).
|
||||
- name: Install shard runtime (bench CLI + redis + wkhtmltopdf)
|
||||
run: |
|
||||
pip install frappe-bench
|
||||
command -v redis-server >/dev/null || { sudo apt-get update -qq && sudo apt-get install -y -qq redis-server; }
|
||||
# wkhtmltopdf (patched-qt build) for print-format / PDF tests — same .deb install.sh uses.
|
||||
if ! command -v wkhtmltopdf >/dev/null; then
|
||||
wget -qO /tmp/wkhtmltox.deb https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-2/wkhtmltox_0.12.6.1-2.jammy_amd64.deb
|
||||
sudo apt-get install -y -qq /tmp/wkhtmltox.deb
|
||||
fi
|
||||
|
||||
- name: Untar bench
|
||||
run: |
|
||||
tar xzpf "${GITHUB_WORKSPACE}/bench.tar.gz" -C /home/runner
|
||||
ls -ld /home/runner/frappe-bench
|
||||
|
||||
- name: Hydrate (start Postgres on the baked datadir)
|
||||
run: bash /home/runner/frappe-bench/hydrate.sh
|
||||
env:
|
||||
DB: postgres
|
||||
DB_HOST: 127.0.0.1
|
||||
|
||||
- name: Run Tests
|
||||
run: cd ~/frappe-bench/ && bench --site test_site run-parallel-tests --app erpnext --use-orchestrator
|
||||
run: |
|
||||
cd ~/frappe-bench/
|
||||
# print-format / PDF tests are engine-independent (they exercise wkhtmltopdf rendering,
|
||||
# not postgres SQL — the MariaDB CI already covers them). They only fetch the static asset
|
||||
# bundles from http://test_site:8000/assets/..., so a plain static file server over sites/
|
||||
# satisfies wkhtmltopdf without the frappe web server (which never bound on a bare runner).
|
||||
( cd ~/frappe-bench/sites && nohup python3 -m http.server 8000 --bind 127.0.0.1 > ~/frappe-bench/web.log 2>&1 & )
|
||||
for _ in $(seq 1 15); do (exec 3<>/dev/tcp/127.0.0.1/8000) 2>/dev/null && { exec 3>&- 3<&-; break; }; sleep 1; done
|
||||
bench --site test_site run-parallel-tests --lightmode --app erpnext \
|
||||
--total-builds ${{ strategy.job-total }} --build-number ${{ matrix.container }}
|
||||
env:
|
||||
TYPE: server
|
||||
CI_BUILD_ID: ${{ github.run_id }}
|
||||
ORCHESTRATOR_URL: http://test-orchestrator.frappe.io
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -48,7 +48,6 @@ repos:
|
||||
cypress/.*|
|
||||
.*node_modules.*|
|
||||
.*boilerplate.*|
|
||||
erpnext/public/js/controllers/.*|
|
||||
erpnext/templates/pages/order.js|
|
||||
erpnext/templates/includes/.*
|
||||
)$
|
||||
@@ -66,6 +65,18 @@ repos:
|
||||
- id: ruff-format
|
||||
name: "Run ruff formatter"
|
||||
|
||||
- repo: local
|
||||
hooks:
|
||||
- id: postgres-compat
|
||||
name: "PostgreSQL compatibility (static check)"
|
||||
description: "Flags MySQL-only SQL that breaks on Postgres; the label-gated PG test job is the backstop for semantic divergences."
|
||||
entry: .github/helper/postgres_compat.py
|
||||
language: script
|
||||
files: ^erpnext/.*\.py$
|
||||
# patches/ are historical, version-gated migrations (skipped on fresh Postgres installs);
|
||||
# out of scope for the always-on gate.
|
||||
exclude: ^erpnext/patches/
|
||||
|
||||
ci:
|
||||
autoupdate_schedule: weekly
|
||||
skip: []
|
||||
|
||||
@@ -14,35 +14,35 @@
|
||||
"@dnd-kit/core": "^6.3.1",
|
||||
"@dnd-kit/sortable": "^10.0.0",
|
||||
"@dnd-kit/utilities": "^3.2.2",
|
||||
"@tailwindcss/vite": "^4.3.0",
|
||||
"@tailwindcss/vite": "^4.3.2",
|
||||
"@tanstack/react-table": "^8.21.3",
|
||||
"@tanstack/react-virtual": "^3.13.24",
|
||||
"@vitejs/plugin-react": "^6.0.1",
|
||||
"@vitejs/plugin-react": "^6.0.3",
|
||||
"chrono-node": "^2.9.1",
|
||||
"class-variance-authority": "^0.7.1",
|
||||
"clsx": "^2.1.1",
|
||||
"cmdk": "^1.1.1",
|
||||
"date-fns": "^4.1.0",
|
||||
"dayjs": "^1.11.20",
|
||||
"frappe-react-sdk": "^1.15.0",
|
||||
"frappe-react-sdk": "^1.17.0",
|
||||
"fuse.js": "^7.3.0",
|
||||
"jotai": "^2.20.0",
|
||||
"jotai-family": "^1.0.1",
|
||||
"jotai": "^2.20.1",
|
||||
"jotai-family": "^1.0.2",
|
||||
"lodash.isplainobject": "^4.0.6",
|
||||
"lucide-react": "^1.14.0",
|
||||
"radix-ui": "^1.4.3",
|
||||
"react": "^19.2.6",
|
||||
"radix-ui": "^1.6.1",
|
||||
"react": "^19.2.7",
|
||||
"react-currency-input-field": "^4.0.5",
|
||||
"react-day-picker": "9.14.0",
|
||||
"react-dom": "^19.2.6",
|
||||
"react-dom": "^19.2.7",
|
||||
"react-dropzone": "^15.0.0",
|
||||
"react-hook-form": "^7.75.0",
|
||||
"react-hotkeys-hook": "^5.3.2",
|
||||
"react-markdown": "^10.1.0",
|
||||
"react-router": "^7.15.0",
|
||||
"react-router-dom": "^7.15.0",
|
||||
"react-router": "^8.1.0",
|
||||
"rehype-raw": "^7.0.0",
|
||||
"remark-gfm": "^4.0.1",
|
||||
"safe-expr-eval": "^1.0.4",
|
||||
"sonner": "^2.0.7",
|
||||
"tailwind-merge": "^3.5.0",
|
||||
"tailwindcss": "^4.3.0",
|
||||
@@ -51,15 +51,15 @@
|
||||
"vite": "^8.0.16"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@eslint/js": "^9.39.1",
|
||||
"@eslint/js": "^9.39.4",
|
||||
"@types/node": "^25.3.0",
|
||||
"@types/react": "^19.2.7",
|
||||
"@types/react-dom": "^19.2.3",
|
||||
"eslint": "^9.39.1",
|
||||
"eslint-plugin-react-hooks": "^7.1.1",
|
||||
"eslint-plugin-react-refresh": "^0.4.24",
|
||||
"eslint-plugin-react-refresh": "^0.5.3",
|
||||
"globals": "^16.5.0",
|
||||
"typescript": "~5.9.3",
|
||||
"typescript-eslint": "^8.48.0"
|
||||
"typescript-eslint": "^8.62.1"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { lazy, useEffect } from 'react'
|
||||
import { BrowserRouter, Navigate, Route, Routes } from 'react-router-dom'
|
||||
import { BrowserRouter, Navigate, Route, Routes } from 'react-router'
|
||||
import { FrappeProvider } from 'frappe-react-sdk'
|
||||
import { Toaster } from '@/components/ui/sonner'
|
||||
import BankReconciliation from '@/pages/BankReconciliation'
|
||||
|
||||
@@ -2,7 +2,6 @@ import { useAtomValue } from "jotai"
|
||||
import { MissingFiltersBanner } from "./MissingFiltersBanner"
|
||||
import { bankRecDateAtom, SelectedBank, selectedBankAccountAtom } from "./bankRecAtoms"
|
||||
import { useCurrentCompany } from "@/hooks/useCurrentCompany"
|
||||
import { Paragraph } from "@/components/ui/typography"
|
||||
import type { ColumnDef } from "@tanstack/react-table"
|
||||
import { useCallback, useMemo, useState } from "react"
|
||||
import { useFrappeGetCall, useFrappePostCall, useSWRConfig } from "frappe-react-sdk"
|
||||
@@ -26,6 +25,7 @@ import { Form } from "@/components/ui/form"
|
||||
import { useForm } from "react-hook-form"
|
||||
import { DateField } from "@/components/ui/form-elements"
|
||||
import { Empty, EmptyMedia, EmptyHeader, EmptyTitle, EmptyDescription } from "@/components/ui/empty"
|
||||
import MarkdownRenderer from "@/components/ui/markdown"
|
||||
|
||||
const BankClearanceSummary = () => {
|
||||
const bankAccount = useAtomValue(selectedBankAccountAtom)
|
||||
@@ -203,14 +203,14 @@ const BankClearanceSummaryView = () => {
|
||||
[accountCurrency, bankAccount, companyID, mutate, onCopy],
|
||||
)
|
||||
|
||||
const content = _("Below is a list of all accounting entries posted against the bank account {0} between {1} and {2}.", [`<strong>${bankAccount?.account}</strong>`, `<strong>${formattedFromDate}</strong>`, `<strong>${formattedToDate}</strong>`])
|
||||
|
||||
return <div className="space-y-4 py-2">
|
||||
|
||||
<div>
|
||||
<Paragraph className="text-sm">
|
||||
<span dangerouslySetInnerHTML={{
|
||||
__html: _("Below is a list of all accounting entries posted against the bank account {0} between {1} and {2}.", [`<strong>${bankAccount?.account}</strong>`, `<strong>${formattedFromDate}</strong>`, `<strong>${formattedToDate}</strong>`])
|
||||
}} />
|
||||
</Paragraph>
|
||||
<span className="text-p-sm">
|
||||
<MarkdownRenderer content={content} />
|
||||
</span>
|
||||
</div>
|
||||
|
||||
{error && <ErrorBanner error={error} />}
|
||||
|
||||
@@ -18,6 +18,7 @@ import { useMultiFileUploadProgress } from "@/hooks/useMultiFileUploadProgress"
|
||||
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table"
|
||||
import { Checkbox } from "@/components/ui/checkbox"
|
||||
import { ArrowDownRight, ArrowUpRight, Plus, Trash2 } from "lucide-react"
|
||||
import { evaluateAmountFormula } from "@/lib/amountFormula"
|
||||
import { flt, formatCurrency } from "@/lib/numbers"
|
||||
import { cn } from "@/lib/utils"
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"
|
||||
@@ -215,38 +216,13 @@ const BankEntryForm = ({ selectedTransaction }: { selectedTransaction: Unreconci
|
||||
})
|
||||
} else {
|
||||
|
||||
/**
|
||||
* The debit and credit amounts can also be expressions - like "transaction_amount * 0.5"
|
||||
* So we need to compute the value of the expression
|
||||
* We can use the eval function to do this. But we need to expose certain variables to the expression.
|
||||
* One of them is transaction_amount which is the unallocated amount of the selected transaction
|
||||
* @param expression - The expression to compute
|
||||
* @returns The computed value
|
||||
*/
|
||||
const computeExpression = (expression: string) => {
|
||||
|
||||
const script = `
|
||||
const transaction_amount = ${selectedTransaction.unallocated_amount ?? 0}
|
||||
${expression};
|
||||
`
|
||||
|
||||
let value = 0;
|
||||
|
||||
try {
|
||||
value = window.eval(script);
|
||||
} catch (error: unknown) {
|
||||
console.error(error);
|
||||
value = 0;
|
||||
}
|
||||
|
||||
return value;
|
||||
}
|
||||
const transactionAmount = selectedTransaction.unallocated_amount ?? 0
|
||||
if (!acc?.debit && !acc?.credit) {
|
||||
hasTotallyEmptyRowEarlier = true;
|
||||
}
|
||||
|
||||
const computedDebit = acc?.debit ? flt(computeExpression(acc.debit), 2) : 0
|
||||
const computedCredit = acc?.credit ? flt(computeExpression(acc.credit), 2) : 0
|
||||
const computedDebit = acc?.debit ? flt(evaluateAmountFormula(acc.debit, transactionAmount), 2) : 0
|
||||
const computedCredit = acc?.credit ? flt(evaluateAmountFormula(acc.credit, transactionAmount), 2) : 0
|
||||
|
||||
totalDebits = flt(totalDebits + computedDebit, 2)
|
||||
totalCredits = flt(totalCredits + computedCredit, 2)
|
||||
|
||||
@@ -2,7 +2,6 @@ import { useAtomValue } from "jotai"
|
||||
import { MissingFiltersBanner } from "./MissingFiltersBanner"
|
||||
import { bankRecDateAtom, selectedBankAccountAtom } from "./bankRecAtoms"
|
||||
import { useCurrentCompany } from "@/hooks/useCurrentCompany"
|
||||
import { Paragraph } from "@/components/ui/typography"
|
||||
import { useCallback, useMemo } from "react"
|
||||
import type { ColumnDef } from "@tanstack/react-table"
|
||||
import { useFrappeGetCall } from "frappe-react-sdk"
|
||||
@@ -19,6 +18,7 @@ import _ from "@/lib/translate"
|
||||
import { toast } from "sonner"
|
||||
import { useCopyToClipboard } from "usehooks-ts"
|
||||
import { Empty, EmptyDescription, EmptyHeader, EmptyMedia, EmptyTitle } from "@/components/ui/empty"
|
||||
import MarkdownRenderer from "@/components/ui/markdown"
|
||||
|
||||
const BankReconciliationStatement = () => {
|
||||
const bankAccount = useAtomValue(selectedBankAccountAtom)
|
||||
@@ -189,14 +189,14 @@ const BankReconciliationStatementView = () => {
|
||||
return data.message.result.filter((row: BankClearanceSummaryEntry) => Boolean(row.payment_entry))
|
||||
}, [data])
|
||||
|
||||
const content = _("Below is a list of all entries posted against the bank account {0} which have not been cleared till {1}.", [`<strong>${bankAccount?.account}</strong>`, `<strong>${formatDate(dates.toDate)}</strong>`])
|
||||
|
||||
return <div className="space-y-4 py-2">
|
||||
|
||||
<div>
|
||||
<Paragraph className="text-sm">
|
||||
<span dangerouslySetInnerHTML={{
|
||||
__html: _("Below is a list of all entries posted against the bank account {0} which have not been cleared till {1}.", [`<strong>${bankAccount?.account}</strong>`, `<strong>${formatDate(dates.toDate)}</strong>`])
|
||||
}} />
|
||||
</Paragraph>
|
||||
<span className="text-p-sm">
|
||||
<MarkdownRenderer content={content} />
|
||||
</span>
|
||||
</div>
|
||||
|
||||
{error && <ErrorBanner error={error} />}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { useAtomValue, useSetAtom } from "jotai"
|
||||
import { MissingFiltersBanner } from "./MissingFiltersBanner"
|
||||
import { bankRecDateAtom, bankRecUnreconcileModalAtom, selectedBankAccountAtom } from "./bankRecAtoms"
|
||||
import { Paragraph } from "@/components/ui/typography"
|
||||
import { formatDate } from "@/lib/date"
|
||||
import { ListView, type ListViewColumnMeta } from "@/components/ui/list-view"
|
||||
import { formatCurrency, getCurrencyFormatInfo } from "@/lib/numbers"
|
||||
@@ -23,6 +22,7 @@ import { useCallback, useMemo, useState } from "react"
|
||||
import { Link } from "react-router"
|
||||
import { Empty, EmptyTitle, EmptyHeader, EmptyMedia, EmptyDescription, EmptyContent } from "@/components/ui/empty"
|
||||
import { InputGroup, InputGroupAddon } from "@/components/ui/input-group"
|
||||
import MarkdownRenderer from "@/components/ui/markdown"
|
||||
|
||||
const BankTransactions = () => {
|
||||
const selectedBank = useAtomValue(selectedBankAccountAtom)
|
||||
@@ -243,14 +243,14 @@ const BankTransactionListView = () => {
|
||||
|
||||
}, [data, search, amountFilter, typeFilter, status])
|
||||
|
||||
const content = _("Below is a list of all bank transactions imported in the system for the bank account {0} between {1} and {2}.", [`<strong>${bankAccount?.account_name}</strong>`, `<strong>${formattedFromDate}</strong>`, `<strong>${formattedToDate}</strong>`])
|
||||
|
||||
return <div className="space-y-2 py-2">
|
||||
|
||||
<div className="flex gap-2 justify-between items-center">
|
||||
<Paragraph className="text-sm">
|
||||
<span dangerouslySetInnerHTML={{
|
||||
__html: _("Below is a list of all bank transactions imported in the system for the bank account {0} between {1} and {2}.", [`<strong>${bankAccount?.account_name}</strong>`, `<strong>${formattedFromDate}</strong>`, `<strong>${formattedToDate}</strong>`])
|
||||
}} />
|
||||
</Paragraph>
|
||||
<span className="text-p-sm">
|
||||
<MarkdownRenderer content={content} />
|
||||
</span>
|
||||
|
||||
<Button size='md' variant='subtle' asChild>
|
||||
<Link to="/statement-importer">
|
||||
|
||||
@@ -2,7 +2,6 @@ import { useAtomValue } from "jotai"
|
||||
import { MissingFiltersBanner } from "./MissingFiltersBanner"
|
||||
import { bankRecDateAtom, selectedBankAccountAtom } from "./bankRecAtoms"
|
||||
import { useCurrentCompany } from "@/hooks/useCurrentCompany"
|
||||
import { Paragraph } from "@/components/ui/typography"
|
||||
import type { ColumnDef } from "@tanstack/react-table"
|
||||
import { useCallback, useMemo } from "react"
|
||||
import { useFrappeGetCall, useFrappePostCall } from "frappe-react-sdk"
|
||||
@@ -18,6 +17,7 @@ import { PartyPopper } from "lucide-react"
|
||||
import ErrorBanner from "@/components/ui/error-banner"
|
||||
import _ from "@/lib/translate"
|
||||
import { Empty, EmptyTitle, EmptyDescription, EmptyMedia, EmptyHeader } from "@/components/ui/empty"
|
||||
import MarkdownRenderer from "@/components/ui/markdown"
|
||||
|
||||
const IncorrectlyClearedEntries = () => {
|
||||
const companyID = useCurrentCompany()
|
||||
@@ -177,22 +177,22 @@ const IncorrectlyClearedEntriesView = () => {
|
||||
[accountCurrency, onClearClick],
|
||||
)
|
||||
|
||||
const content = _("This report shows all entries in the system where the <strong>clearance date is before the posting date</strong> which is incorrect.")
|
||||
|
||||
const entriesContent = _("Entries below have a posting date after {0} but the clearance date is before {1}.", [`<strong>${formattedToDate}</strong>`, `<strong>${formattedToDate}</strong>`])
|
||||
|
||||
return <div className="space-y-4 py-2">
|
||||
|
||||
<div>
|
||||
<Paragraph className="text-sm">
|
||||
<span dangerouslySetInnerHTML={{
|
||||
__html: _("This report shows all entries in the system where the <strong>clearance date is before the posting date</strong> which is incorrect.")
|
||||
}} />
|
||||
<span className="text-p-sm">
|
||||
<MarkdownRenderer content={content} />
|
||||
<br />
|
||||
{data && data.message.result.length > 0 && <span>
|
||||
<span dangerouslySetInnerHTML={{
|
||||
__html: _("Entries below have a posting date after {0} but the clearance date is before {1}.", [`<strong>${formattedToDate}</strong>`, `<strong>${formattedToDate}</strong>`])
|
||||
}} />
|
||||
<MarkdownRenderer content={entriesContent} />
|
||||
<br />
|
||||
{_("You can reset the clearing dates of these entries here.")}
|
||||
</span>}
|
||||
</Paragraph>
|
||||
</span>
|
||||
</div>
|
||||
|
||||
{error && <ErrorBanner error={error} />}
|
||||
|
||||
@@ -11,6 +11,7 @@ import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"
|
||||
import { H4, Paragraph } from "@/components/ui/typography"
|
||||
import { today } from "@/lib/date"
|
||||
import { evaluateAmountFormula } from "@/lib/amountFormula"
|
||||
import _ from "@/lib/translate"
|
||||
import { cn } from "@/lib/utils"
|
||||
import { BankTransactionRule } from "@/types/Accounts/BankTransactionRule"
|
||||
@@ -445,11 +446,10 @@ const AmountFormulaRenderer = ({ value }: { value?: string }) => {
|
||||
// If it's a string and cannot be a number, then show it as a formula
|
||||
|
||||
if (isNaN(Number(value))) {
|
||||
|
||||
let calculatedValue = "";
|
||||
|
||||
try {
|
||||
calculatedValue = window.eval(`const transaction_amount = 200; ${value}`);
|
||||
calculatedValue = String(evaluateAmountFormula(value ?? "", 200));
|
||||
} catch (error: unknown) {
|
||||
console.error(error);
|
||||
calculatedValue = "Error";
|
||||
|
||||
@@ -14,7 +14,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip
|
||||
import { useFrappeEventListener, useFrappePostCall } from 'frappe-react-sdk'
|
||||
import { toast } from 'sonner'
|
||||
import ErrorBanner from '@/components/ui/error-banner'
|
||||
import { Link, useNavigate } from 'react-router-dom'
|
||||
import { Link, useNavigate } from 'react-router'
|
||||
import { useMemo, useState } from 'react'
|
||||
import { Progress } from '@/components/ui/progress'
|
||||
import { useSetAtom } from 'jotai'
|
||||
|
||||
26
banking/src/lib/amountFormula.ts
Normal file
26
banking/src/lib/amountFormula.ts
Normal file
@@ -0,0 +1,26 @@
|
||||
import { Parser } from 'safe-expr-eval'
|
||||
|
||||
const parser = new Parser()
|
||||
|
||||
const PLAIN_NUMBER_PATTERN = /^-?\d+(\.\d+)?$/
|
||||
|
||||
export function evaluateAmountFormula(expression: string, transactionAmount: number): number {
|
||||
const trimmed = expression.trim()
|
||||
if (!trimmed) {
|
||||
return 0
|
||||
}
|
||||
|
||||
if (PLAIN_NUMBER_PATTERN.test(trimmed)) {
|
||||
return Number(trimmed)
|
||||
}
|
||||
|
||||
try {
|
||||
const result = parser.parse(trimmed).evaluate({ transaction_amount: transactionAmount })
|
||||
if (typeof result !== 'number' || !Number.isFinite(result)) {
|
||||
return 0
|
||||
}
|
||||
return result
|
||||
} catch {
|
||||
return 0
|
||||
}
|
||||
}
|
||||
2655
banking/yarn.lock
2655
banking/yarn.lock
File diff suppressed because it is too large
Load Diff
@@ -3,8 +3,6 @@ import inspect
|
||||
from typing import TypeVar
|
||||
|
||||
import frappe
|
||||
from frappe.model.document import Document
|
||||
from frappe.utils.user import is_website_user
|
||||
|
||||
__version__ = "17.0.0-dev"
|
||||
|
||||
@@ -155,6 +153,8 @@ def allow_regional(fn):
|
||||
|
||||
|
||||
def check_app_permission():
|
||||
from frappe.utils.user import is_website_user
|
||||
|
||||
if frappe.session.user == "Administrator":
|
||||
return True
|
||||
|
||||
@@ -175,9 +175,16 @@ def normalize_ctx_input(T: type) -> callable:
|
||||
- Casting the result to the specified type T
|
||||
"""
|
||||
|
||||
from frappe.model.document import Document
|
||||
|
||||
def decorator(func: callable):
|
||||
# conserve annotations for frappe.utils.typing_validations
|
||||
@functools.wraps(func, assigned=(a for a in functools.WRAPPER_ASSIGNMENTS if a != "__annotations__"))
|
||||
@functools.wraps(
|
||||
func,
|
||||
assigned=(
|
||||
a for a in functools.WRAPPER_ASSIGNMENTS if a not in ("__annotations__", "__annotate__")
|
||||
),
|
||||
)
|
||||
def wrapper(ctx: T | Document | dict | str, *args, **kwargs):
|
||||
if isinstance(ctx, Document):
|
||||
ctx = T(**ctx.as_dict())
|
||||
|
||||
129
erpnext/accounts/bulk_payment.py
Normal file
129
erpnext/accounts/bulk_payment.py
Normal file
@@ -0,0 +1,129 @@
|
||||
import frappe
|
||||
from frappe import _
|
||||
from frappe.utils import flt
|
||||
|
||||
from erpnext.accounts.doctype.payment_entry.payment_entry import (
|
||||
get_outstanding_reference_documents,
|
||||
get_payment_entry,
|
||||
)
|
||||
from erpnext.utilities.bulk_transaction import transaction_processing
|
||||
|
||||
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def create_payment_entries(
|
||||
grouped_invoices: str | list | None = None,
|
||||
ungrouped_invoices: str | list | None = None,
|
||||
):
|
||||
"""Create draft Payment Entries from AP report invoice selection."""
|
||||
frappe.has_permission("Payment Entry", "create", throw=True)
|
||||
|
||||
grouped_invoices = [d for d in frappe.parse_json(grouped_invoices or "[]") if d.get("voucher_no")]
|
||||
ungrouped_invoices = [d for d in frappe.parse_json(ungrouped_invoices or "[]") if d.get("voucher_no")]
|
||||
|
||||
if not grouped_invoices and not ungrouped_invoices:
|
||||
frappe.throw(_("No Purchase Invoices selected"))
|
||||
|
||||
if ungrouped_invoices:
|
||||
data = [{"name": d["voucher_no"]} for d in ungrouped_invoices]
|
||||
transaction_processing(data, "Purchase Invoice", "Payment Entry")
|
||||
|
||||
if grouped_invoices:
|
||||
groups = {}
|
||||
for d in grouped_invoices:
|
||||
key = (d["supplier"], d["party_account"])
|
||||
groups.setdefault(
|
||||
key, {"supplier": d["supplier"], "party_account": d["party_account"], "vouchers": []}
|
||||
)["vouchers"].append(d["voucher_no"])
|
||||
|
||||
frappe.msgprint(
|
||||
_("Started a background job to create {0} Grouped Payment Entries").format(len(groups))
|
||||
)
|
||||
frappe.enqueue(
|
||||
make_grouped_payment_entries,
|
||||
queue="long",
|
||||
timeout=1500,
|
||||
groups=list(groups.values()),
|
||||
)
|
||||
|
||||
|
||||
def make_grouped_payment_entries(groups):
|
||||
created, failed = 0, 0
|
||||
|
||||
for group in groups:
|
||||
supplier = group["supplier"]
|
||||
try:
|
||||
frappe.db.savepoint("bulk_pe")
|
||||
pe = _build_grouped_payment_entry(supplier, group["party_account"], group["vouchers"])
|
||||
if not pe:
|
||||
frappe.db.rollback(save_point="bulk_pe")
|
||||
failed += 1
|
||||
frappe.log_error(
|
||||
title=_("Bulk Payment Entry skipped for {0}").format(supplier),
|
||||
message=_(
|
||||
"No outstanding invoices found for the selected vouchers in account {0}"
|
||||
).format(group["party_account"]),
|
||||
)
|
||||
continue
|
||||
|
||||
pe.flags.ignore_validate = True
|
||||
pe.set_title_field()
|
||||
pe.insert(ignore_mandatory=True)
|
||||
created += 1
|
||||
except Exception:
|
||||
frappe.db.rollback(save_point="bulk_pe")
|
||||
failed += 1
|
||||
frappe.log_error(title=_("Bulk Payment Entry creation failed for {0}").format(supplier))
|
||||
|
||||
message = _("Created {0} draft Grouped Payment Entries").format(created)
|
||||
|
||||
if failed:
|
||||
message += " — " + _("{0} skipped (see Error Log)").format(failed)
|
||||
|
||||
frappe.publish_realtime(
|
||||
"msgprint",
|
||||
{"message": message, "title": _("Bulk Payment Entries"), "indicator": "green"},
|
||||
user=frappe.session.user,
|
||||
after_commit=True,
|
||||
)
|
||||
|
||||
|
||||
def _build_grouped_payment_entry(supplier, party_account, names):
|
||||
pe = get_payment_entry("Purchase Invoice", names[0])
|
||||
pe.set("references", [])
|
||||
|
||||
refs = get_outstanding_reference_documents(
|
||||
{
|
||||
"party_type": "Supplier",
|
||||
"party": supplier,
|
||||
"party_account": party_account,
|
||||
"company": pe.company,
|
||||
"vouchers": [frappe._dict(voucher_type="Purchase Invoice", voucher_no=n) for n in names],
|
||||
}
|
||||
)
|
||||
|
||||
for r in refs:
|
||||
if r.voucher_type != "Purchase Invoice":
|
||||
continue
|
||||
pe.append(
|
||||
"references",
|
||||
{
|
||||
"reference_doctype": r.voucher_type,
|
||||
"reference_name": r.voucher_no,
|
||||
"bill_no": r.get("bill_no"),
|
||||
"due_date": r.get("due_date"),
|
||||
"payment_term": r.get("payment_term"),
|
||||
"total_amount": r.invoice_amount,
|
||||
"outstanding_amount": r.outstanding_amount,
|
||||
"allocated_amount": r.outstanding_amount,
|
||||
"exchange_rate": r.get("exchange_rate") or 1,
|
||||
},
|
||||
)
|
||||
|
||||
if not pe.references:
|
||||
return None
|
||||
|
||||
# received_amount is in paid_to account currency; convert to paid_from account currency for paid_amount
|
||||
pe.received_amount = sum(r.allocated_amount for r in pe.references)
|
||||
pe.paid_amount = flt(pe.received_amount * pe.target_exchange_rate, pe.precision("paid_amount"))
|
||||
pe.set_amounts()
|
||||
return pe
|
||||
@@ -582,6 +582,7 @@ def make_gl_entries(
|
||||
frappe.db.commit()
|
||||
except Exception as e:
|
||||
if frappe.in_test:
|
||||
frappe.db.rollback()
|
||||
doc.log_error(f"Error while processing deferred accounting for Invoice {doc.name}")
|
||||
raise e
|
||||
else:
|
||||
|
||||
@@ -234,7 +234,7 @@ class Account(NestedSet):
|
||||
if not frappe.db.get_value(
|
||||
"Account", {"account_name": self.account_name, "company": ancestors[0]}, "name"
|
||||
):
|
||||
frappe.throw(_("Please add the account to root level Company - {}").format(ancestors[0]))
|
||||
frappe.throw(_("Please add the account to root level Company - {0}").format(ancestors[0]))
|
||||
elif self.parent_account:
|
||||
descendants = get_descendants_of("Company", self.company)
|
||||
if not descendants:
|
||||
@@ -671,7 +671,7 @@ def _ensure_idle_system():
|
||||
if last_gl_update > add_to_date(None, minutes=-5):
|
||||
frappe.throw(
|
||||
_(
|
||||
"Last GL Entry update was done {}. This operation is not allowed while system is actively being used. Please wait for 5 minutes before retrying."
|
||||
"Last GL Entry update was done {0}. This operation is not allowed while system is actively being used. Please wait for 5 minutes before retrying."
|
||||
).format(pretty_date(last_gl_update)),
|
||||
title=_("System In Use"),
|
||||
)
|
||||
|
||||
@@ -236,9 +236,9 @@ frappe.treeview_settings["Account"] = {
|
||||
function () {
|
||||
let root_company = treeview.page.fields_dict.root_company.get_value();
|
||||
if (root_company) {
|
||||
frappe.throw(__("Please add the account to root level Company - {0}"), [
|
||||
root_company,
|
||||
]);
|
||||
frappe.throw(
|
||||
__("Please add the account to root level Company - {0}", [root_company])
|
||||
);
|
||||
} else {
|
||||
treeview.new_node();
|
||||
}
|
||||
|
||||
@@ -137,7 +137,7 @@ def get_charts_for_country(country: str, with_standard: bool = False):
|
||||
|
||||
def _get_chart_name(content):
|
||||
if content:
|
||||
content = json.loads(content)
|
||||
content = frappe.parse_json(content)
|
||||
if (
|
||||
content and content.get("disabled", "No") == "No"
|
||||
) or frappe.local.flags.allow_unverified_charts:
|
||||
|
||||
@@ -37,6 +37,10 @@
|
||||
"account_type": "Stock",
|
||||
"account_category": "Stock Assets"
|
||||
},
|
||||
"Stock Delivered But Not Billed": {
|
||||
"account_type": "Stock Delivered But Not Billed",
|
||||
"account_category": "Stock Assets"
|
||||
},
|
||||
"account_type": "Stock",
|
||||
"account_category": "Stock Assets"
|
||||
},
|
||||
@@ -223,10 +227,6 @@
|
||||
"Stock Received But Not Billed": {
|
||||
"account_type": "Stock Received But Not Billed",
|
||||
"account_category": "Trade Payables"
|
||||
},
|
||||
"Stock Delivered But Not Billed": {
|
||||
"account_type": "Stock Delivered But Not Billed",
|
||||
"account_category": "Trade Payables"
|
||||
}
|
||||
},
|
||||
"Duties and Taxes": {
|
||||
|
||||
@@ -1,10 +1,59 @@
|
||||
# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
|
||||
# import frappe
|
||||
|
||||
from erpnext.accounts.doctype.account_closing_balance.account_closing_balance import (
|
||||
aggregate_with_last_account_closing_balance,
|
||||
generate_key,
|
||||
)
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
|
||||
def entry(**overrides):
|
||||
row = {"debit": 0, "credit": 0, "debit_in_account_currency": 0, "credit_in_account_currency": 0}
|
||||
row.update(overrides)
|
||||
return row
|
||||
|
||||
|
||||
class TestAccountClosingBalance(ERPNextTestSuite):
|
||||
pass
|
||||
"""The closing-balance snapshot is built by merging this period's entries with the
|
||||
previous period's. These lock the merge/key logic that drives that carry-forward."""
|
||||
|
||||
def test_matching_entries_are_summed(self):
|
||||
# this is how a prior-period balance carries forward into the current one
|
||||
merged = aggregate_with_last_account_closing_balance(
|
||||
[
|
||||
entry(account="Cash - _TC", debit=100, debit_in_account_currency=100),
|
||||
entry(
|
||||
account="Cash - _TC",
|
||||
debit=50,
|
||||
credit=20,
|
||||
debit_in_account_currency=50,
|
||||
credit_in_account_currency=20,
|
||||
),
|
||||
],
|
||||
[],
|
||||
)
|
||||
self.assertEqual(len(merged), 1)
|
||||
row = next(iter(merged.values()))
|
||||
self.assertEqual(row["debit"], 150)
|
||||
self.assertEqual(row["credit"], 20)
|
||||
# the account-currency columns are accumulated in the same pass
|
||||
self.assertEqual(row["debit_in_account_currency"], 150)
|
||||
self.assertEqual(row["credit_in_account_currency"], 20)
|
||||
|
||||
def test_entries_are_kept_separate_per_dimension(self):
|
||||
merged = aggregate_with_last_account_closing_balance(
|
||||
[
|
||||
entry(account="Cash - _TC", cost_center="CC1", debit=100, debit_in_account_currency=100),
|
||||
entry(account="Cash - _TC", cost_center="CC2", debit=40, debit_in_account_currency=40),
|
||||
],
|
||||
[],
|
||||
)
|
||||
self.assertEqual(len(merged), 2)
|
||||
|
||||
def test_period_closing_flag_is_part_of_the_key(self):
|
||||
# a P&L reversal (flag 0) and a closing-account entry (flag 1) for the same
|
||||
# account must not merge, so the flag has to distinguish their keys
|
||||
key_reversal, _ = generate_key(entry(account="Sales - _TC", is_period_closing_voucher_entry=0), [])
|
||||
key_closing, _ = generate_key(entry(account="Sales - _TC", is_period_closing_voucher_entry=1), [])
|
||||
self.assertNotEqual(key_reversal, key_closing)
|
||||
|
||||
@@ -224,7 +224,7 @@ def disable_dimension(doc: str):
|
||||
|
||||
|
||||
def toggle_disabling(doc):
|
||||
doc = json.loads(doc)
|
||||
doc = frappe.parse_json(doc)
|
||||
|
||||
if doc.get("disabled"):
|
||||
df = {"read_only": 1}
|
||||
|
||||
@@ -6,7 +6,7 @@ frappe.ui.form.on("Accounting Dimension Filter", {
|
||||
let help_content = `<table class="table table-bordered" style="background-color: var(--scrollbar-track-color);">
|
||||
<tr><td>
|
||||
<p>
|
||||
<i class="fa fa-hand-right"></i>
|
||||
<svg class="icon icon-sm"><use href="#icon-info"></use></svg>
|
||||
{{__('Note: On checking Is Mandatory the accounting dimension will become mandatory against that specific account for all accounting transactions')}}
|
||||
</p>
|
||||
</td></tr>
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
import frappe
|
||||
from frappe import _
|
||||
from frappe.model.document import Document
|
||||
from frappe.utils import getdate, nowdate
|
||||
|
||||
|
||||
class OverlapError(frappe.ValidationError):
|
||||
@@ -36,8 +37,20 @@ class AccountingPeriod(Document):
|
||||
# end: auto-generated types
|
||||
|
||||
def validate(self):
|
||||
self.validate_dates()
|
||||
self.validate_overlap()
|
||||
|
||||
def validate_dates(self):
|
||||
if getdate(self.start_date) > getdate(self.end_date):
|
||||
frappe.throw(_("Start Date cannot be after End Date"))
|
||||
|
||||
if getdate(self.end_date) > getdate(nowdate()):
|
||||
frappe.throw(
|
||||
_(
|
||||
"Accounting Period cannot be created for a future date. End Date {0} is after today."
|
||||
).format(frappe.bold(frappe.format(self.end_date, "Date")))
|
||||
)
|
||||
|
||||
def before_insert(self):
|
||||
self.bootstrap_doctypes_for_closing()
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
from frappe.utils import add_months, nowdate
|
||||
from frappe.utils import nowdate
|
||||
|
||||
from erpnext.accounts.doctype.accounting_period.accounting_period import (
|
||||
ClosedAccountingPeriod,
|
||||
@@ -93,7 +93,7 @@ def create_accounting_period(**args):
|
||||
|
||||
accounting_period = frappe.new_doc("Accounting Period")
|
||||
accounting_period.start_date = args.start_date or nowdate()
|
||||
accounting_period.end_date = args.end_date or add_months(nowdate(), 1)
|
||||
accounting_period.end_date = args.end_date or nowdate()
|
||||
accounting_period.company = args.company or "_Test Company"
|
||||
accounting_period.period_name = args.period_name or "_Test_Period_Name_1"
|
||||
accounting_period.append("closed_documents", {"document_type": "Sales Invoice", "closed": 1})
|
||||
|
||||
@@ -10,7 +10,7 @@ frappe.ui.form.on("Accounts Settings", {
|
||||
},
|
||||
};
|
||||
});
|
||||
if (!frm.naming_controller) frm.naming_controller = new erpnext.NamingSeriesController(frm);
|
||||
if (!frm.naming_controller) frm.naming_controller = new frappe.ui.NamingSeriesController(frm);
|
||||
|
||||
frm.naming_controller.render_table("transaction_naming_html", get_transactions(frm));
|
||||
},
|
||||
|
||||
@@ -87,6 +87,7 @@
|
||||
"period_closing_settings_section",
|
||||
"ignore_account_closing_balance",
|
||||
"use_legacy_controller_for_pcv",
|
||||
"pcv_job_timeout",
|
||||
"column_break_25",
|
||||
"reports_tab",
|
||||
"remarks_section",
|
||||
@@ -612,6 +613,14 @@
|
||||
"fieldtype": "Check",
|
||||
"label": "Use legacy controller for Period Closing Voucher"
|
||||
},
|
||||
{
|
||||
"default": "3600",
|
||||
"depends_on": "eval: !doc.use_legacy_controller_for_pcv",
|
||||
"description": "Timeout (in seconds) for each background job enqueued by Process Period Closing Voucher",
|
||||
"fieldname": "pcv_job_timeout",
|
||||
"fieldtype": "Int",
|
||||
"label": "PCV Job Timeout (seconds)"
|
||||
},
|
||||
{
|
||||
"description": "Users with this role will be notified if the asset depreciation gets failed",
|
||||
"fieldname": "role_to_notify_on_depreciation_failure",
|
||||
@@ -756,7 +765,7 @@
|
||||
"index_web_pages_for_search": 1,
|
||||
"issingle": 1,
|
||||
"links": [],
|
||||
"modified": "2026-06-03 13:11:54.721495",
|
||||
"modified": "2026-06-24 12:59:41.868865",
|
||||
"modified_by": "Administrator",
|
||||
"module": "Accounts",
|
||||
"name": "Accounts Settings",
|
||||
|
||||
@@ -90,6 +90,7 @@ class AccountsSettings(Document):
|
||||
make_payment_via_journal_entry: DF.Check
|
||||
merge_similar_account_heads: DF.Check
|
||||
over_billing_allowance: DF.Currency
|
||||
pcv_job_timeout: DF.Int
|
||||
preview_mode: DF.Check
|
||||
receivable_payable_fetch_method: DF.Literal["Buffered Cursor", "UnBuffered Cursor"]
|
||||
receivable_payable_remarks_length: DF.Int
|
||||
|
||||
@@ -188,7 +188,7 @@ def get_closing_balance_as_per_statement(bank_account: str, date: str):
|
||||
return {"balance": 0, "date": None}
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def set_closing_balance_as_per_statement(bank_account: str, date: str | datetime.date, balance: float):
|
||||
"""
|
||||
Set the closing balance as per statement for a bank account and date
|
||||
|
||||
@@ -1,8 +1,76 @@
|
||||
# Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
from frappe.utils import flt
|
||||
|
||||
from erpnext.accounts.doctype.bank_guarantee.bank_guarantee import get_voucher_details
|
||||
from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
BANK = "_Test BG Bank"
|
||||
|
||||
|
||||
class TestBankGuarantee(ERPNextTestSuite):
|
||||
pass
|
||||
"""Bank Guarantee records a guarantee issued/received against a customer or
|
||||
supplier. validate() needs a party; on_submit() needs the bank details filled in."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
if not frappe.db.exists("Bank", BANK):
|
||||
frappe.get_doc({"doctype": "Bank", "bank_name": BANK}).insert()
|
||||
|
||||
def make_bg(self, **args):
|
||||
args = frappe._dict(args)
|
||||
doc = frappe.new_doc("Bank Guarantee")
|
||||
doc.bg_type = args.bg_type or "Receiving"
|
||||
doc.amount = args.amount if args.amount is not None else 1000
|
||||
doc.start_date = args.start_date or "2026-06-01"
|
||||
if args.end_date:
|
||||
doc.end_date = args.end_date
|
||||
doc.customer = args.get("customer", "_Test Customer")
|
||||
doc.supplier = args.get("supplier")
|
||||
# fields on_submit requires — present by default, cleared per-test to assert the guard
|
||||
doc.bank_guarantee_number = args.get("bank_guarantee_number", "BG-001")
|
||||
doc.name_of_beneficiary = args.get("name_of_beneficiary", "Test Beneficiary")
|
||||
doc.bank = args.get("bank", BANK)
|
||||
return doc
|
||||
|
||||
def test_validate_requires_customer_or_supplier(self):
|
||||
doc = self.make_bg(customer=None)
|
||||
self.assertRaises(frappe.ValidationError, doc.insert)
|
||||
|
||||
def test_submit_requires_guarantee_number(self):
|
||||
doc = self.make_bg(bank_guarantee_number="")
|
||||
doc.insert()
|
||||
self.assertRaises(frappe.ValidationError, doc.submit)
|
||||
|
||||
def test_submit_requires_beneficiary_name(self):
|
||||
doc = self.make_bg(name_of_beneficiary="")
|
||||
doc.insert()
|
||||
self.assertRaises(frappe.ValidationError, doc.submit)
|
||||
|
||||
def test_submit_requires_bank(self):
|
||||
doc = self.make_bg(bank="")
|
||||
doc.insert()
|
||||
self.assertRaises(frappe.ValidationError, doc.submit)
|
||||
|
||||
def test_valid_guarantee_submits(self):
|
||||
doc = self.make_bg()
|
||||
doc.insert()
|
||||
doc.submit()
|
||||
self.assertEqual(frappe.db.get_value("Bank Guarantee", doc.name, "docstatus"), 1)
|
||||
|
||||
def test_get_voucher_details_for_receiving(self):
|
||||
so = make_sales_order()
|
||||
details = get_voucher_details("Receiving", so.name)
|
||||
self.assertEqual(details.customer, so.customer)
|
||||
self.assertEqual(flt(details.grand_total), flt(so.grand_total))
|
||||
|
||||
def test_end_date_before_start_date_is_not_validated(self):
|
||||
# SUSPECTED BUG: validate() never checks that end_date >= start_date, so a
|
||||
# guarantee that expires before it starts saves cleanly. Locking the current
|
||||
# (wrong) behaviour so a future fix that adds the check trips this test.
|
||||
doc = self.make_bg(start_date="2026-06-30", end_date="2026-06-01")
|
||||
doc.insert()
|
||||
self.assertTrue(frappe.db.exists("Bank Guarantee", doc.name))
|
||||
|
||||
@@ -116,7 +116,7 @@ def get_account_balance(bank_account: str, till_date: str | date, company: str):
|
||||
return flt(balance_as_per_system) - flt(total_debit) + flt(total_credit) + amounts_not_reflected_in_system
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def update_bank_transaction(
|
||||
bank_transaction_name: str, reference_number: str, party_type: str | None = None, party: str | None = None
|
||||
):
|
||||
@@ -146,7 +146,7 @@ def update_bank_transaction(
|
||||
)[0]
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def create_journal_entry_bts(
|
||||
bank_transaction_name: str,
|
||||
reference_number: str | None = None,
|
||||
@@ -305,7 +305,7 @@ def create_journal_entry_bts(
|
||||
return reconcile_vouchers(bank_transaction_name, vouchers, is_new_voucher=True)
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def create_payment_entry_bts(
|
||||
bank_transaction_name: str,
|
||||
reference_number: str | None = None,
|
||||
@@ -500,7 +500,7 @@ def create_bulk_internal_transfer(bank_transaction_names: list[str | int], bank_
|
||||
return output
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def create_internal_transfer(
|
||||
bank_transaction_name: str | int,
|
||||
posting_date: str | date,
|
||||
@@ -1057,10 +1057,10 @@ def get_auto_reconcile_message(partially_reconciled, reconciled):
|
||||
return alert_message, indicator
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
def reconcile_vouchers(bank_transaction_name: str | int, vouchers: str, is_new_voucher: bool = False):
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def reconcile_vouchers(bank_transaction_name: str | int, vouchers: str | list, is_new_voucher: bool = False):
|
||||
# updated clear date of all the vouchers based on the bank transaction
|
||||
vouchers = json.loads(vouchers)
|
||||
vouchers = frappe.parse_json(vouchers)
|
||||
transaction = frappe.get_doc("Bank Transaction", bank_transaction_name)
|
||||
transaction.add_payment_entries(vouchers, is_new_voucher)
|
||||
transaction.validate_duplicate_references()
|
||||
|
||||
@@ -8,6 +8,7 @@ from frappe.utils import add_days, today
|
||||
|
||||
from erpnext.accounts.doctype.bank_reconciliation_tool.bank_reconciliation_tool import (
|
||||
auto_reconcile_vouchers,
|
||||
get_auto_reconcile_message,
|
||||
get_bank_transactions,
|
||||
)
|
||||
from erpnext.accounts.doctype.payment_entry.test_payment_entry import create_payment_entry
|
||||
@@ -97,3 +98,40 @@ class TestBankReconciliationTool(ERPNextTestSuite, AccountsTestMixin):
|
||||
# assert API output post reconciliation
|
||||
transactions = get_bank_transactions(self.bank_account, from_date, to_date)
|
||||
self.assertEqual(len(transactions), 0)
|
||||
|
||||
def make_bank_transaction(self, date, deposit=100):
|
||||
return (
|
||||
frappe.get_doc(
|
||||
{
|
||||
"doctype": "Bank Transaction",
|
||||
"date": date,
|
||||
"deposit": deposit,
|
||||
"bank_account": self.bank_account,
|
||||
"currency": "INR",
|
||||
}
|
||||
)
|
||||
.save()
|
||||
.submit()
|
||||
)
|
||||
|
||||
def test_get_bank_transactions_excludes_dates_after_to_date(self):
|
||||
self.make_bank_transaction(date=today())
|
||||
names = [t.name for t in get_bank_transactions(self.bank_account, to_date=add_days(today(), -1))]
|
||||
self.assertEqual(names, [])
|
||||
|
||||
def test_auto_reconcile_message_for_no_matches(self):
|
||||
message, indicator = get_auto_reconcile_message([], [])
|
||||
self.assertEqual(indicator, "blue")
|
||||
self.assertIn("No matches", message)
|
||||
|
||||
def test_auto_reconcile_message_counts_and_pluralizes(self):
|
||||
# reconciled count is reported and the indicator turns green
|
||||
message, indicator = get_auto_reconcile_message([], ["t1", "t2"])
|
||||
self.assertEqual(indicator, "green")
|
||||
self.assertIn("2 Transaction(s) Reconciled", message)
|
||||
|
||||
# partially-reconciled label is singular for one, plural for many
|
||||
singular, _ = get_auto_reconcile_message(["p1"], [])
|
||||
self.assertIn("1 Transaction Partially Reconciled", singular)
|
||||
plural, _ = get_auto_reconcile_message(["p1", "p2"], [])
|
||||
self.assertIn("2 Transactions Partially Reconciled", plural)
|
||||
|
||||
@@ -221,12 +221,12 @@
|
||||
"default": "0",
|
||||
"fieldname": "import_mt940_fromat",
|
||||
"fieldtype": "Check",
|
||||
"label": "Import MT940 Fromat"
|
||||
"label": "Import MT940 Format"
|
||||
}
|
||||
],
|
||||
"hide_toolbar": 1,
|
||||
"links": [],
|
||||
"modified": "2026-05-31 00:41:11.251215",
|
||||
"modified": "2026-06-19 14:18:00.000000",
|
||||
"modified_by": "Administrator",
|
||||
"module": "Accounts",
|
||||
"name": "Bank Statement Import",
|
||||
|
||||
@@ -290,7 +290,7 @@ def update_mapping_db(bank, template_options):
|
||||
for d in bank.bank_transaction_mapping:
|
||||
d.delete()
|
||||
|
||||
for d in json.loads(template_options)["column_to_field_map"].items():
|
||||
for d in frappe.parse_json(template_options)["column_to_field_map"].items():
|
||||
bank.append("bank_transaction_mapping", {"bank_transaction_field": d[1], "file_field": d[0]})
|
||||
|
||||
bank.save()
|
||||
|
||||
@@ -829,7 +829,9 @@ def compute_final_transactions(transaction_rows: list, date_format: str, amount_
|
||||
|
||||
if amount_format == 'Amount column has "CR"/"DR" values':
|
||||
amount = transaction_row.get("amount")
|
||||
float_amount = get_float_amount(amount)
|
||||
|
||||
# If the amount column has CR/DR in it - we should remove any signs (negative or positive) from the amount
|
||||
float_amount = abs(get_float_amount(amount) or 0)
|
||||
if "cr" in amount.lower():
|
||||
return 0, float_amount
|
||||
else:
|
||||
@@ -932,14 +934,18 @@ def extract_pdf_tables(content: bytes, password: str | None = None) -> list[dict
|
||||
from pypdf import PdfReader
|
||||
|
||||
reader = PdfReader(io.BytesIO(content))
|
||||
if reader.is_encrypted and (not password or not reader.decrypt(password)):
|
||||
frappe.throw(
|
||||
_(
|
||||
"This PDF is password protected. Please set the correct statement password on the"
|
||||
" Bank Account and try again."
|
||||
),
|
||||
title=_("Password Required"),
|
||||
)
|
||||
if reader.is_encrypted:
|
||||
# Try opening the PDF with a password - if no password is provided, try with a blank password
|
||||
if not password:
|
||||
password = ""
|
||||
if not reader.decrypt(password):
|
||||
frappe.throw(
|
||||
_(
|
||||
"This PDF is password protected. Please set the correct statement password on the"
|
||||
" Bank Account and try again."
|
||||
),
|
||||
title=_("Password Required"),
|
||||
)
|
||||
|
||||
text_settings = {"vertical_strategy": "text", "horizontal_strategy": "text"}
|
||||
tables = []
|
||||
@@ -1183,8 +1189,7 @@ def update_pdf_tables(statement_import_id: str, tables: list | str):
|
||||
if doc.status == "Completed":
|
||||
frappe.throw(_("This statement has already been imported."), title=_("Already Imported"))
|
||||
|
||||
if isinstance(tables, str):
|
||||
tables = json.loads(tables)
|
||||
tables = frappe.parse_json(tables)
|
||||
|
||||
doc.apply_pdf_tables(tables)
|
||||
|
||||
@@ -1204,8 +1209,7 @@ def reextract_pdf_table(statement_import_id: str, page: int, table_index: int, b
|
||||
if doc.status == "Completed":
|
||||
frappe.throw(_("This statement has already been imported."), title=_("Already Imported"))
|
||||
|
||||
if isinstance(bbox, str):
|
||||
bbox = json.loads(bbox)
|
||||
bbox = frappe.parse_json(bbox)
|
||||
|
||||
page = int(page)
|
||||
table_index = int(table_index)
|
||||
@@ -1290,8 +1294,7 @@ def update_column_mapping(statement_import_id: str, column_mapping: list | str):
|
||||
if doc.status == "Completed":
|
||||
frappe.throw(_("This statement has already been imported."), title=_("Already Imported"))
|
||||
|
||||
if isinstance(column_mapping, str):
|
||||
column_mapping = json.loads(column_mapping)
|
||||
column_mapping = frappe.parse_json(column_mapping)
|
||||
|
||||
doc.apply_column_mapping(column_mapping)
|
||||
doc.save()
|
||||
|
||||
@@ -397,7 +397,7 @@ def unreconcile_transaction(transaction_name: str | int):
|
||||
frappe.get_doc(voucher["doctype"], voucher["name"]).cancel()
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def unreconcile_transaction_entry(bank_transaction_id: str | int, voucher_type: str, voucher_id: str | int):
|
||||
"""
|
||||
Removes a single payment entry from a bank transaction - for example only undoing one voucher instead of undoing the entire transaction
|
||||
@@ -440,7 +440,7 @@ def get_clearance_details(transaction, payment_entry, bt_allocations, gl_entries
|
||||
|
||||
if bt_bank_account != gl_bank_account:
|
||||
frappe.throw(
|
||||
_("Bank Account {} in Bank Transaction {} is not matching with Bank Account {}").format(
|
||||
_("Bank Account {0} in Bank Transaction {1} is not matching with Bank Account {2}").format(
|
||||
bt_bank_account, payment_entry.payment_entry, gl_bank_account
|
||||
)
|
||||
)
|
||||
@@ -449,7 +449,7 @@ def get_clearance_details(transaction, payment_entry, bt_allocations, gl_entries
|
||||
|
||||
if gl_bank_account not in gl_entries:
|
||||
frappe.throw(
|
||||
_("{} {} is not affecting bank account {}").format(
|
||||
_("{0} {1} is not affecting bank account {2}").format(
|
||||
payment_entry.payment_document, payment_entry.payment_entry, gl_bank_account
|
||||
)
|
||||
)
|
||||
@@ -457,7 +457,7 @@ def get_clearance_details(transaction, payment_entry, bt_allocations, gl_entries
|
||||
allocable_amount = gl_entries.pop(gl_bank_account) or 0
|
||||
if allocable_amount <= 0.0:
|
||||
frappe.throw(
|
||||
_("Invalid amount in accounting entries of {} {} for Account {}: {}").format(
|
||||
_("Invalid amount in accounting entries of {0} {1} for Account {2}: {3}").format(
|
||||
payment_entry.payment_document, payment_entry.payment_entry, gl_bank_account, allocable_amount
|
||||
)
|
||||
)
|
||||
|
||||
@@ -34,19 +34,20 @@ def upload_bank_statement():
|
||||
return {"columns": columns, "data": data}
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
def create_bank_entries(columns: str, data: str, bank_account: str):
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def create_bank_entries(columns: str, data: str | list, bank_account: str):
|
||||
header_map = get_header_mapping(columns, bank_account)
|
||||
|
||||
success = 0
|
||||
errors = 0
|
||||
for d in json.loads(data):
|
||||
for d in frappe.parse_json(data):
|
||||
if all(item is None for item in d) is True:
|
||||
continue
|
||||
fields = {}
|
||||
for key, value in header_map.items():
|
||||
fields.update({key: d[int(value) - 1]})
|
||||
|
||||
frappe.db.savepoint("bank_entry")
|
||||
try:
|
||||
bank_transaction = frappe.get_doc({"doctype": "Bank Transaction"})
|
||||
bank_transaction.update(fields)
|
||||
@@ -56,7 +57,8 @@ def create_bank_entries(columns: str, data: str, bank_account: str):
|
||||
bank_transaction.submit()
|
||||
success += 1
|
||||
except Exception:
|
||||
bank_transaction.log_error("Bank entry creation failed")
|
||||
frappe.db.rollback(save_point="bank_entry")
|
||||
frappe.log_error(title="Bank entry creation failed")
|
||||
errors += 1
|
||||
|
||||
return {"success": success, "errors": errors}
|
||||
@@ -66,7 +68,7 @@ def get_header_mapping(columns, bank_account):
|
||||
mapping = get_bank_mapping(bank_account)
|
||||
|
||||
header_map = {}
|
||||
for column in json.loads(columns):
|
||||
for column in frappe.parse_json(columns):
|
||||
if column["content"] in mapping:
|
||||
header_map.update({mapping[column["content"]]: column["colIndex"]})
|
||||
|
||||
|
||||
@@ -9,6 +9,48 @@ from frappe.model.document import Document
|
||||
|
||||
from erpnext.accounts.doctype.bank_transaction.bank_transaction import BankTransaction
|
||||
|
||||
PLAIN_NUMBER_PATTERN = re.compile(r"^-?\d+(\.\d+)?$")
|
||||
# Tokens accepted by safe-expr-eval on the frontend (must stay in sync).
|
||||
ALLOWED_FORMULA_TOKEN = re.compile(r"\s+|transaction_amount|\d+(?:\.\d+)?|[+\-*/%^()]")
|
||||
PYTHON_ONLY_OPERATORS = ("**", "//")
|
||||
|
||||
|
||||
def _is_expr_eval_formula(formula: str) -> bool:
|
||||
position = 0
|
||||
while position < len(formula):
|
||||
match = ALLOWED_FORMULA_TOKEN.match(formula, position)
|
||||
if not match:
|
||||
return False
|
||||
position = match.end()
|
||||
|
||||
return formula.count("(") == formula.count(")")
|
||||
|
||||
|
||||
def validate_amount_formula(formula: str) -> None:
|
||||
if not formula:
|
||||
return
|
||||
|
||||
stripped = formula.strip()
|
||||
if PLAIN_NUMBER_PATTERN.match(stripped):
|
||||
return
|
||||
|
||||
if any(operator in stripped for operator in PYTHON_ONLY_OPERATORS):
|
||||
frappe.throw(_("Invalid debit/credit formula: {0}").format(formula))
|
||||
|
||||
if not _is_expr_eval_formula(stripped):
|
||||
frappe.throw(_("Invalid debit/credit formula: {0}").format(formula))
|
||||
|
||||
# expr-eval uses ^ for exponentiation; translate for a smoke-test evaluation only.
|
||||
python_formula = stripped.replace("^", "**")
|
||||
|
||||
try:
|
||||
result = frappe.safe_eval(python_formula, eval_globals=None, eval_locals={"transaction_amount": 1})
|
||||
except Exception:
|
||||
frappe.throw(_("Invalid debit/credit formula: {0}").format(formula))
|
||||
|
||||
if not isinstance(result, (int | float)):
|
||||
frappe.throw(_("Invalid debit/credit formula: {0}").format(formula))
|
||||
|
||||
|
||||
class BankTransactionRule(Document):
|
||||
# begin: auto-generated types
|
||||
@@ -66,7 +108,7 @@ class BankTransactionRule(Document):
|
||||
frappe.throw(_("Party type is required to create a payment entry."))
|
||||
|
||||
if not self.party:
|
||||
frappe.throw(_("Party is required create a payment entry."))
|
||||
frappe.throw(_("Party is required to create a payment entry."))
|
||||
|
||||
if not self.account:
|
||||
frappe.throw(_("Party account is required to create a payment entry."))
|
||||
@@ -86,6 +128,11 @@ class BankTransactionRule(Document):
|
||||
frappe.throw(
|
||||
_("The last account row must not have any debit or credit amounts set.")
|
||||
)
|
||||
else:
|
||||
if account.debit:
|
||||
validate_amount_formula(account.debit)
|
||||
if account.credit:
|
||||
validate_amount_formula(account.credit)
|
||||
|
||||
# Validate regex
|
||||
for rule in self.description_rules:
|
||||
|
||||
@@ -231,3 +231,45 @@ class TestBankTransactionRule(ERPNextTestSuite, AccountsTestMixin):
|
||||
doc = self._rule("bad_rx", [{"check": "Regex", "value": "["}])
|
||||
with self.assertRaises(ValidationError):
|
||||
doc.insert()
|
||||
|
||||
def _multiple_accounts_rule(self, prefix: str, accounts, **fields):
|
||||
return self._rule(
|
||||
prefix,
|
||||
[{"check": "Contains", "value": "x"}],
|
||||
classify_as="Bank Entry",
|
||||
bank_entry_type="Multiple Accounts",
|
||||
accounts=accounts,
|
||||
**fields,
|
||||
)
|
||||
|
||||
def test_validate_bank_entry_multiple_valid_amount_formulas(self):
|
||||
doc = self._multiple_accounts_rule(
|
||||
"be_formula",
|
||||
accounts=[
|
||||
{"account": self.bank, "debit": "200", "credit": ""},
|
||||
{"account": self.cash, "debit": "", "credit": "transaction_amount * 0.25"},
|
||||
{"account": self.cash, "debit": "", "credit": ""},
|
||||
],
|
||||
)
|
||||
doc.insert()
|
||||
self.assertTrue(doc.name)
|
||||
|
||||
def test_validate_bank_entry_multiple_invalid_amount_formulas(self):
|
||||
malicious_formulas = [
|
||||
"__import__('os')",
|
||||
"eval('1+1')",
|
||||
"open('/etc/passwd')",
|
||||
"transaction_amount ** 2",
|
||||
"transaction_amount // 2",
|
||||
]
|
||||
for formula in malicious_formulas:
|
||||
with self.subTest(formula=formula):
|
||||
doc = self._multiple_accounts_rule(
|
||||
"be_bad_formula",
|
||||
accounts=[
|
||||
{"account": self.bank, "debit": formula, "credit": ""},
|
||||
{"account": self.cash, "debit": "", "credit": ""},
|
||||
],
|
||||
)
|
||||
with self.assertRaises(ValidationError):
|
||||
doc.insert()
|
||||
|
||||
@@ -184,7 +184,7 @@ class BisectAccountingStatements(Document):
|
||||
self.get_report_summary()
|
||||
self.update_node()
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def bisect_left(self):
|
||||
if self.current_node is not None:
|
||||
cur_node = frappe.get_doc("Bisect Nodes", self.current_node)
|
||||
@@ -198,7 +198,7 @@ class BisectAccountingStatements(Document):
|
||||
else:
|
||||
frappe.msgprint(_("No more children on Left"))
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def bisect_right(self):
|
||||
if self.current_node is not None:
|
||||
cur_node = frappe.get_doc("Bisect Nodes", self.current_node)
|
||||
@@ -212,7 +212,7 @@ class BisectAccountingStatements(Document):
|
||||
else:
|
||||
frappe.msgprint(_("No more children on Right"))
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def move_up(self):
|
||||
if self.current_node is not None:
|
||||
cur_node = frappe.get_doc("Bisect Nodes", self.current_node)
|
||||
|
||||
@@ -1,11 +1,47 @@
|
||||
# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
|
||||
# import frappe
|
||||
import datetime
|
||||
|
||||
import frappe
|
||||
from frappe.utils import getdate
|
||||
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
|
||||
class TestBisectAccountingStatements(ERPNextTestSuite):
|
||||
pass
|
||||
"""The tool bisects a date range into a tree of Bisect Nodes down to single days.
|
||||
These cover the date validation and that the bisection cleanly partitions the range."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
frappe.db.delete("Bisect Nodes")
|
||||
|
||||
def _leaf_days(self):
|
||||
leaves = frappe.get_all(
|
||||
"Bisect Nodes",
|
||||
filters={"left_child": ["is", "not set"]},
|
||||
fields=["period_from_date", "period_to_date"],
|
||||
)
|
||||
# every leaf spans a single day
|
||||
for leaf in leaves:
|
||||
self.assertEqual(getdate(leaf.period_from_date), getdate(leaf.period_to_date))
|
||||
return sorted(getdate(leaf.period_from_date) for leaf in leaves)
|
||||
|
||||
def test_validate_dates_rejects_reversed_range(self):
|
||||
doc = frappe.new_doc("Bisect Accounting Statements")
|
||||
doc.from_date = "2026-01-08"
|
||||
doc.to_date = "2026-01-01"
|
||||
self.assertRaises(frappe.ValidationError, doc.validate)
|
||||
|
||||
def test_bfs_partitions_range_into_single_days(self):
|
||||
doc = frappe.new_doc("Bisect Accounting Statements")
|
||||
doc.bfs(datetime.datetime(2026, 1, 1), datetime.datetime(2026, 1, 8))
|
||||
|
||||
# the 8-day span Jan 1..Jan 8 becomes exactly 8 contiguous single-day leaves
|
||||
self.assertEqual(self._leaf_days(), [getdate(f"2026-01-0{n}") for n in range(1, 9)])
|
||||
|
||||
def test_dfs_produces_the_same_partition_as_bfs(self):
|
||||
doc = frappe.new_doc("Bisect Accounting Statements")
|
||||
doc.dfs(datetime.datetime(2026, 1, 1), datetime.datetime(2026, 1, 8))
|
||||
self.assertEqual(self._leaf_days(), [getdate(f"2026-01-0{n}") for n in range(1, 9)])
|
||||
|
||||
@@ -162,9 +162,9 @@ class Budget(Document):
|
||||
frappe.throw(_("Account {0} does not belong to company {1}").format(self.account, self.company))
|
||||
elif account_details.report_type != "Profit and Loss":
|
||||
frappe.throw(
|
||||
_("Budget cannot be assigned against {0}, as it's not an Income or Expense account").format(
|
||||
self.account
|
||||
)
|
||||
_(
|
||||
"Budget cannot be assigned against {0}, as its Root Type is not of Income or Expense"
|
||||
).format(self.account)
|
||||
)
|
||||
|
||||
def set_null_value(self):
|
||||
@@ -878,7 +878,7 @@ def get_fiscal_year_date_range(from_fiscal_year, to_fiscal_year):
|
||||
return from_year.year_start_date, to_year.year_end_date
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def revise_budget(budget_name: str):
|
||||
old_budget = frappe.get_doc("Budget", budget_name)
|
||||
|
||||
|
||||
@@ -1,8 +1,67 @@
|
||||
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
|
||||
from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
DATE = "2026-06-15"
|
||||
|
||||
|
||||
class TestCashierClosing(ERPNextTestSuite):
|
||||
pass
|
||||
"""Cashier Closing reconciles a shift: it pulls outstanding invoices in a
|
||||
date/time window and rolls payments, expense, custody and returns into net_amount."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
|
||||
def make_invoice_in_window(self, rate=100):
|
||||
si = create_sales_invoice(rate=rate, qty=1, posting_date=DATE, do_not_submit=True)
|
||||
si.posting_time = "10:30:00"
|
||||
si.submit()
|
||||
si.reload() # read outstanding_amount as persisted after submit
|
||||
return si
|
||||
|
||||
def make_closing(self, user="Administrator", payments=None, **args):
|
||||
doc = frappe.new_doc("Cashier Closing")
|
||||
doc.user = user
|
||||
doc.date = args.get("date", DATE)
|
||||
doc.from_time = args.get("from_time", "09:00:00")
|
||||
doc.time = args.get("time", "18:00:00")
|
||||
for amount in payments or []:
|
||||
doc.append("payments", {"mode_of_payment": "Cash", "amount": amount})
|
||||
doc.expense = args.get("expense", 0)
|
||||
doc.custody = args.get("custody", 0)
|
||||
doc.returns = args.get("returns", 0)
|
||||
return doc
|
||||
|
||||
def test_from_time_must_be_before_to_time(self):
|
||||
doc = self.make_closing(from_time="18:00:00", time="09:00:00")
|
||||
self.assertRaises(frappe.ValidationError, doc.save)
|
||||
|
||||
def test_equal_from_and_to_time_is_rejected(self):
|
||||
# validate_time uses >=, so a zero-length window is also blocked
|
||||
doc = self.make_closing(from_time="09:00:00", time="09:00:00")
|
||||
self.assertRaises(frappe.ValidationError, doc.save)
|
||||
|
||||
def test_net_amount_rolls_up_outstanding_and_adjustments(self):
|
||||
si = self.make_invoice_in_window(rate=100)
|
||||
doc = self.make_closing(payments=[500], expense=50, custody=30, returns=20)
|
||||
doc.save()
|
||||
|
||||
# the in-window invoice is picked up as outstanding
|
||||
self.assertEqual(doc.outstanding_amount, si.outstanding_amount)
|
||||
# net = payments + outstanding + expense - custody + returns
|
||||
self.assertEqual(doc.net_amount, 500 + si.outstanding_amount + 50 - 30 + 20)
|
||||
|
||||
def test_outstanding_is_scoped_to_the_invoice_owner(self):
|
||||
# The invoice is created by Administrator; a closing for a different user does
|
||||
# not see it. NOTE: get_outstanding keys on Sales Invoice.owner (the document
|
||||
# creator) rather than an explicit cashier/POS-user field, which is fragile when
|
||||
# invoices are created by a shared or system user.
|
||||
self.make_invoice_in_window(rate=100)
|
||||
doc = self.make_closing(user="Guest", payments=[500])
|
||||
doc.save()
|
||||
self.assertEqual(doc.outstanding_amount, 0)
|
||||
self.assertEqual(doc.net_amount, 500)
|
||||
|
||||
@@ -63,8 +63,8 @@ def validate_company(company: str):
|
||||
)
|
||||
|
||||
if parent_company and (not allow_account_creation_against_child_company):
|
||||
msg = _("{} is a child company.").format(frappe.bold(company)) + " "
|
||||
msg += _("Please import accounts against parent company or enable {} in company master.").format(
|
||||
msg = _("{0} is a child company.").format(frappe.bold(company)) + " "
|
||||
msg += _("Please import accounts against parent company or enable {0} in company master.").format(
|
||||
frappe.bold(_("Allow Account Creation Against Child Company"))
|
||||
)
|
||||
frappe.throw(msg, title=_("Wrong Company"))
|
||||
|
||||
@@ -1,8 +1,54 @@
|
||||
# Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
|
||||
from erpnext.accounts.doctype.chart_of_accounts_importer.chart_of_accounts_importer import (
|
||||
build_forest,
|
||||
validate_columns,
|
||||
validate_missing_roots,
|
||||
)
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
# columns: account_name, parent_account, account_number, parent_account_number,
|
||||
# is_group, account_type, root_type, account_currency
|
||||
ROOT = ["Assets", "Assets", "", "", 1, "", "Asset", "INR"]
|
||||
CHILD = ["Cash", "Assets", "", "", 0, "Cash", "Asset", "INR"]
|
||||
|
||||
|
||||
class TestChartofAccountsImporter(ERPNextTestSuite):
|
||||
pass
|
||||
"""The importer parses an uploaded CoA into a nested tree and validates its
|
||||
shape. These cover the parsing/validation helpers without a file upload."""
|
||||
|
||||
def test_validate_columns_rejects_blank_file(self):
|
||||
self.assertRaises(frappe.ValidationError, validate_columns, [])
|
||||
|
||||
def test_validate_columns_requires_eight_columns(self):
|
||||
self.assertRaises(frappe.ValidationError, validate_columns, [["a", "b", "c"]])
|
||||
# the standard template width passes
|
||||
validate_columns([ROOT])
|
||||
|
||||
def test_build_forest_nests_child_under_parent(self):
|
||||
forest = build_forest([ROOT, CHILD])
|
||||
self.assertIn("Assets", forest)
|
||||
self.assertIn("Cash", forest["Assets"])
|
||||
|
||||
def test_build_forest_rejects_unknown_parent(self):
|
||||
orphan = ["Cash", "Missing Parent", "", "", 0, "Cash", "Asset", "INR"]
|
||||
self.assertRaises(frappe.ValidationError, build_forest, [orphan])
|
||||
|
||||
def test_build_forest_requires_account_name(self):
|
||||
nameless = ["", "Assets", "", "", 0, "Cash", "Asset", "INR"]
|
||||
self.assertRaises(frappe.ValidationError, build_forest, [ROOT, nameless])
|
||||
|
||||
def test_validate_missing_roots_requires_all_root_types(self):
|
||||
present = ("Asset", "Liability", "Expense", "Income") # Equity missing
|
||||
self.assertRaises(
|
||||
frappe.ValidationError,
|
||||
validate_missing_roots,
|
||||
[{"root_type": rt} for rt in present],
|
||||
)
|
||||
# all five root types present -> no error
|
||||
validate_missing_roots(
|
||||
[{"root_type": rt} for rt in ("Asset", "Liability", "Expense", "Income", "Equity")]
|
||||
)
|
||||
|
||||
@@ -46,7 +46,7 @@ class ChequePrintTemplate(Document):
|
||||
pass
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def create_or_update_cheque_print_format(template_name: str):
|
||||
frappe.only_for("System Manager")
|
||||
|
||||
|
||||
@@ -173,3 +173,66 @@ class TestCouponCode(ERPNextTestSuite):
|
||||
|
||||
# Clean up
|
||||
coupon.delete()
|
||||
|
||||
def test_validate_coupon_code_rejections(self):
|
||||
from frappe.utils import add_days, nowdate
|
||||
|
||||
from erpnext.accounts.doctype.pricing_rule.utils import validate_coupon_code
|
||||
|
||||
pricing_rule = frappe.db.get_value("Pricing Rule", {"title": "_Test Pricing Rule for _Test Item"})
|
||||
|
||||
def make_coupon(name, **kwargs):
|
||||
frappe.delete_doc_if_exists("Coupon Code", name)
|
||||
return frappe.get_doc(
|
||||
{
|
||||
"doctype": "Coupon Code",
|
||||
"coupon_name": name,
|
||||
"coupon_code": name,
|
||||
"coupon_type": "Promotional",
|
||||
"pricing_rule": pricing_rule,
|
||||
**kwargs,
|
||||
}
|
||||
).insert(ignore_permissions=True)
|
||||
|
||||
with self.subTest("validity not yet started"):
|
||||
make_coupon("_Test Coupon Future", valid_from=add_days(nowdate(), 5))
|
||||
self.assertRaises(frappe.ValidationError, validate_coupon_code, "_Test Coupon Future")
|
||||
|
||||
with self.subTest("validity expired"):
|
||||
make_coupon("_Test Coupon Expired", valid_upto=add_days(nowdate(), -5))
|
||||
self.assertRaises(frappe.ValidationError, validate_coupon_code, "_Test Coupon Expired")
|
||||
|
||||
with self.subTest("maximum use exhausted"):
|
||||
make_coupon("_Test Coupon Exhausted", maximum_use=2, used=2)
|
||||
self.assertRaises(frappe.ValidationError, validate_coupon_code, "_Test Coupon Exhausted")
|
||||
|
||||
with self.subTest("valid coupon passes"):
|
||||
make_coupon("_Test Coupon Valid", maximum_use=5, used=1, valid_upto=add_days(nowdate(), 5))
|
||||
validate_coupon_code("_Test Coupon Valid") # no raise
|
||||
|
||||
def test_update_coupon_code_count_cancel_and_exhaust(self):
|
||||
from erpnext.accounts.doctype.pricing_rule.utils import update_coupon_code_count
|
||||
|
||||
pricing_rule = frappe.db.get_value("Pricing Rule", {"title": "_Test Pricing Rule for _Test Item"})
|
||||
frappe.delete_doc_if_exists("Coupon Code", "_Test Coupon Count")
|
||||
frappe.get_doc(
|
||||
{
|
||||
"doctype": "Coupon Code",
|
||||
"coupon_name": "_Test Coupon Count",
|
||||
"coupon_code": "_Test Coupon Count",
|
||||
"coupon_type": "Promotional",
|
||||
"pricing_rule": pricing_rule,
|
||||
"maximum_use": 2,
|
||||
"used": 1,
|
||||
}
|
||||
).insert(ignore_permissions=True)
|
||||
|
||||
# cancelling a transaction releases one use
|
||||
update_coupon_code_count("_Test Coupon Count", "cancelled")
|
||||
self.assertEqual(frappe.db.get_value("Coupon Code", "_Test Coupon Count", "used"), 0)
|
||||
|
||||
# using up to the maximum is allowed, beyond it is rejected
|
||||
update_coupon_code_count("_Test Coupon Count", "used")
|
||||
update_coupon_code_count("_Test Coupon Count", "used")
|
||||
self.assertEqual(frappe.db.get_value("Coupon Code", "_Test Coupon Count", "used"), 2)
|
||||
self.assertRaises(frappe.ValidationError, update_coupon_code_count, "_Test Coupon Count", "used")
|
||||
|
||||
@@ -90,7 +90,7 @@ class CurrencyExchangeSettings(Document):
|
||||
try:
|
||||
response = requests.get(api_url, params=params)
|
||||
except requests.exceptions.RequestException as e:
|
||||
frappe.throw("Error: " + str(e))
|
||||
frappe.throw(_("Error: {0}").format(str(e)))
|
||||
|
||||
response.raise_for_status()
|
||||
value = response.json()
|
||||
|
||||
@@ -85,7 +85,7 @@ class Dunning(AccountsController):
|
||||
if invoice_currency != self.currency:
|
||||
frappe.throw(
|
||||
_(
|
||||
"The currency of invoice {} ({}) is different from the currency of this dunning ({})."
|
||||
"The currency of invoice {0} ({1}) is different from the currency of this dunning ({2})."
|
||||
).format(
|
||||
frappe.get_desk_link(
|
||||
"Sales Invoice",
|
||||
@@ -248,8 +248,7 @@ def get_dunning_letter_text(dunning_type: str, doc: str | dict, language: str |
|
||||
DOCTYPE = "Dunning Letter Text"
|
||||
FIELDS = ["body_text", "closing_text", "language"]
|
||||
|
||||
if isinstance(doc, str):
|
||||
doc = json.loads(doc)
|
||||
doc = frappe.parse_json(doc)
|
||||
|
||||
if not language:
|
||||
language = doc.get("language")
|
||||
|
||||
@@ -12,6 +12,7 @@ from erpnext.accounts.doctype.sales_invoice.mapper import (
|
||||
create_dunning as create_dunning_from_sales_invoice,
|
||||
)
|
||||
from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import (
|
||||
create_sales_invoice,
|
||||
create_sales_invoice_against_cost_center,
|
||||
)
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
@@ -152,6 +153,37 @@ class TestDunning(ERPNextTestSuite):
|
||||
dunning.reload()
|
||||
self.assertEqual(dunning.status, "Unresolved")
|
||||
|
||||
@ERPNextTestSuite.change_settings(
|
||||
"Accounts Settings", {"allow_multi_currency_invoices_against_single_party_account": 1}
|
||||
)
|
||||
def test_dunning_outstanding_uses_transaction_currency(self):
|
||||
"""
|
||||
Regression for #56006: dunning outstanding must be in the invoice transaction
|
||||
currency, not in the party account currency.
|
||||
|
||||
A USD invoice posted against an INR receivable account stores
|
||||
outstanding_amount in INR (party account currency). The overdue payment
|
||||
row on the resulting Dunning must carry the USD amount, not the INR amount.
|
||||
"""
|
||||
si = create_sales_invoice(
|
||||
posting_date=add_days(today(), -10),
|
||||
currency="USD",
|
||||
conversion_rate=50,
|
||||
rate=100,
|
||||
debit_to="Debtors - _TC",
|
||||
)
|
||||
|
||||
# Sanity-check the invoice state before creating the dunning
|
||||
self.assertEqual(si.currency, "USD")
|
||||
self.assertEqual(si.outstanding_amount, 5000.0) # INR (party account currency)
|
||||
self.assertEqual(si.payment_schedule[0].outstanding, 100.0) # USD (transaction currency)
|
||||
|
||||
dunning = create_dunning_from_sales_invoice(si.name)
|
||||
|
||||
self.assertEqual(len(dunning.overdue_payments), 1)
|
||||
# Must reflect 100 USD, not 5000 INR mislabelled as USD
|
||||
self.assertEqual(dunning.overdue_payments[0].outstanding, 100.0)
|
||||
|
||||
def test_dunning_not_affected_by_standalone_credit_note(self):
|
||||
"""
|
||||
Test that dunning is NOT resolved when a credit note has update_outstanding_for_self checked.
|
||||
|
||||
@@ -136,7 +136,7 @@ frappe.ui.form.on("Exchange Rate Revaluation Account", {
|
||||
var get_account_details = function (frm, cdt, cdn) {
|
||||
var row = frappe.get_doc(cdt, cdn);
|
||||
if (!frm.doc.company || !frm.doc.posting_date) {
|
||||
frappe.throw(__("Please select Company and Posting Date to getting entries"));
|
||||
frappe.throw(__("Please select Company and Posting Date to get entries"));
|
||||
}
|
||||
frappe.call({
|
||||
method: "erpnext.accounts.doctype.exchange_rate_revaluation.exchange_rate_revaluation.get_account_details",
|
||||
|
||||
@@ -73,7 +73,7 @@ class ExchangeRateRevaluation(Document):
|
||||
|
||||
def validate_mandatory(self):
|
||||
if not (self.company and self.posting_date):
|
||||
frappe.throw(_("Please select Company and Posting Date to getting entries"))
|
||||
frappe.throw(_("Please select Company and Posting Date to get entries"))
|
||||
|
||||
def before_submit(self):
|
||||
self.remove_accounts_without_gain_loss()
|
||||
@@ -350,12 +350,14 @@ class ExchangeRateRevaluation(Document):
|
||||
zero_balance_jv = self.make_jv_for_zero_balance()
|
||||
if zero_balance_jv:
|
||||
frappe.msgprint(
|
||||
f"Zero Balance Journal: {get_link_to_form('Journal Entry', zero_balance_jv.name)}"
|
||||
_("Zero Balance Journal: {0}").format(get_link_to_form("Journal Entry", zero_balance_jv.name))
|
||||
)
|
||||
|
||||
revaluation_jv = self.make_jv_for_revaluation()
|
||||
if revaluation_jv:
|
||||
frappe.msgprint(f"Revaluation Journal: {get_link_to_form('Journal Entry', revaluation_jv.name)}")
|
||||
frappe.msgprint(
|
||||
_("Revaluation Journal: {0}").format(get_link_to_form("Journal Entry", revaluation_jv.name))
|
||||
)
|
||||
|
||||
return {
|
||||
"revaluation_jv": revaluation_jv.name if revaluation_jv else None,
|
||||
@@ -599,17 +601,22 @@ def calculate_exchange_rate_using_last_gle(company, account, party_type, party):
|
||||
.select(gl.voucher_type, gl.voucher_no)
|
||||
.where(Criterion.all(conditions))
|
||||
.orderby(gl.posting_date, order=Order.desc)
|
||||
.orderby(gl.name, order=Order.desc)
|
||||
.limit(1)
|
||||
.run()[0]
|
||||
)
|
||||
|
||||
last_exchange_rate = (
|
||||
qb.from_(gl)
|
||||
.select((gl.debit - gl.credit) / (gl.debit_in_account_currency - gl.credit_in_account_currency))
|
||||
.select(
|
||||
(gl.debit - gl.credit)
|
||||
/ NullIf(gl.debit_in_account_currency - gl.credit_in_account_currency, 0)
|
||||
)
|
||||
.where(
|
||||
(gl.voucher_type == voucher_type) & (gl.voucher_no == voucher_no) & (gl.account == account)
|
||||
)
|
||||
.orderby(gl.posting_date, order=Order.desc)
|
||||
.orderby(gl.name, order=Order.desc)
|
||||
.limit(1)
|
||||
.run()[0][0]
|
||||
)
|
||||
|
||||
@@ -298,3 +298,65 @@ class TestExchangeRateRevaluation(ERPNextTestSuite, AccountsTestMixin):
|
||||
|
||||
for key, _val in expected_data.items():
|
||||
self.assertEqual(expected_data.get(key), account_details.get(key))
|
||||
|
||||
|
||||
class TestExchangeRateRevaluationValidation(ERPNextTestSuite):
|
||||
"""Validation and gain/loss calculation paths, exercised on the document directly
|
||||
so they don't need the multi-currency GL setup the integration tests above build."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
self.company = "_Test Company"
|
||||
|
||||
def _revaluation_with_rows(self, rows, rounding_loss_allowance=0.05):
|
||||
doc = frappe.new_doc("Exchange Rate Revaluation")
|
||||
doc.company = self.company
|
||||
doc.posting_date = today()
|
||||
doc.rounding_loss_allowance = rounding_loss_allowance
|
||||
for row in rows:
|
||||
doc.append("accounts", row)
|
||||
return doc
|
||||
|
||||
def test_rounding_loss_allowance_must_be_between_0_and_1(self):
|
||||
for bad in (-0.1, 1, 1.5):
|
||||
doc = self._revaluation_with_rows([], rounding_loss_allowance=bad)
|
||||
self.assertRaises(frappe.ValidationError, doc.validate)
|
||||
# values inside [0, 1) are accepted, at the lower bound and mid-range
|
||||
for good in (0.0, 0.5):
|
||||
self._revaluation_with_rows([], rounding_loss_allowance=good).validate()
|
||||
|
||||
def test_gain_loss_computed_and_split_by_zero_balance(self):
|
||||
doc = self._revaluation_with_rows(
|
||||
[
|
||||
# open (unbooked) row: base balance moved 1000 -> 1100, a 100 gain
|
||||
{"zero_balance": 0, "balance_in_base_currency": 1000, "new_balance_in_base_currency": 1100},
|
||||
# already-settled (zero_balance) row carries a booked loss of 40
|
||||
{"zero_balance": 1, "gain_loss": -40},
|
||||
]
|
||||
)
|
||||
doc.validate()
|
||||
|
||||
# gain_loss is derived only for open rows; the zero-balance row keeps its value
|
||||
self.assertEqual(doc.accounts[0].gain_loss, 100)
|
||||
self.assertEqual(doc.gain_loss_unbooked, 100)
|
||||
self.assertEqual(doc.gain_loss_booked, -40)
|
||||
self.assertEqual(doc.total_gain_loss, 60)
|
||||
|
||||
def test_before_submit_drops_rows_without_gain_loss(self):
|
||||
doc = self._revaluation_with_rows(
|
||||
[
|
||||
{"zero_balance": 0, "balance_in_base_currency": 1000, "new_balance_in_base_currency": 1100},
|
||||
{"zero_balance": 0, "balance_in_base_currency": 500, "new_balance_in_base_currency": 500},
|
||||
]
|
||||
)
|
||||
doc.validate() # second row nets to a 0 gain_loss
|
||||
doc.remove_accounts_without_gain_loss()
|
||||
self.assertEqual(len(doc.accounts), 1)
|
||||
self.assertEqual(doc.accounts[0].gain_loss, 100)
|
||||
|
||||
def test_before_submit_requires_at_least_one_gain_loss_row(self):
|
||||
doc = self._revaluation_with_rows(
|
||||
[{"zero_balance": 0, "balance_in_base_currency": 500, "new_balance_in_base_currency": 500}]
|
||||
)
|
||||
doc.validate()
|
||||
self.assertRaises(frappe.ValidationError, doc.remove_accounts_without_gain_loss)
|
||||
|
||||
@@ -12,8 +12,9 @@ from typing import Any, Union
|
||||
import frappe
|
||||
from frappe import _
|
||||
from frappe.database.operator_map import OPERATOR_MAP
|
||||
from frappe.model import numeric_fieldtypes
|
||||
from frappe.query_builder import Case
|
||||
from frappe.query_builder.functions import Sum
|
||||
from frappe.query_builder.functions import Cast_, Sum
|
||||
from frappe.utils import cstr, date_diff, flt, getdate
|
||||
from frappe.utils.xlsxutils import XLSXMetadata, XLSXStyleBuilder
|
||||
from pypika.terms import Bracket, LiteralValue
|
||||
@@ -864,8 +865,15 @@ class FilterExpressionParser:
|
||||
field = getattr(table, field_name, None)
|
||||
operator_fn = OPERATOR_MAP.get(operator.casefold())
|
||||
|
||||
if "like" in operator.casefold() and "%" not in value:
|
||||
value = f"%{value}%"
|
||||
if "like" in operator.casefold():
|
||||
if "%" not in value:
|
||||
value = f"%{value}%"
|
||||
# Postgres has no LIKE/ILIKE operator for non-text columns; MariaDB implicitly casts
|
||||
# the numeric column to text. Cast a numeric/Check Account field to varchar so the
|
||||
# match runs on both engines and reproduces MariaDB's result.
|
||||
meta_field = frappe.get_meta("Account").get_field(field_name)
|
||||
if meta_field and meta_field.fieldtype in numeric_fieldtypes:
|
||||
field = Cast_(field, "varchar")
|
||||
|
||||
return operator_fn(field, value)
|
||||
|
||||
@@ -1024,8 +1032,7 @@ class FormulaFieldUpdater:
|
||||
def get_filtered_accounts(company: str, account_rows: str | list):
|
||||
frappe.has_permission("Financial Report Template", ptype="read", throw=True)
|
||||
|
||||
if isinstance(account_rows, str):
|
||||
account_rows = json.loads(account_rows, object_hook=frappe._dict)
|
||||
account_rows = [frappe._dict(row) for row in frappe.parse_json(account_rows)]
|
||||
|
||||
return DataCollector.get_filtered_accounts(company, account_rows)
|
||||
|
||||
|
||||
@@ -107,6 +107,9 @@ def auto_create_fiscal_year():
|
||||
)
|
||||
|
||||
for d in fiscal_year:
|
||||
# savepoint so a duplicate-year INSERT (Fiscal Year autoname=field:year) that aborts the
|
||||
# statement doesn't poison the whole scheduler transaction on Postgres and kill the next iteration
|
||||
frappe.db.savepoint("auto_create_fiscal_year")
|
||||
try:
|
||||
current_fy = frappe.get_doc("Fiscal Year", d[0])
|
||||
|
||||
@@ -127,7 +130,7 @@ def auto_create_fiscal_year():
|
||||
|
||||
new_fy.insert(ignore_permissions=True)
|
||||
except frappe.NameError:
|
||||
pass
|
||||
frappe.db.rollback(save_point="auto_create_fiscal_year")
|
||||
|
||||
|
||||
def get_from_and_to_date(fiscal_year):
|
||||
|
||||
@@ -471,6 +471,25 @@ def on_doctype_update():
|
||||
frappe.db.add_index("GL Entry", ["posting_date", "company"])
|
||||
frappe.db.add_index("GL Entry", ["party_type", "party"])
|
||||
|
||||
if frappe.db.db_type == "postgres":
|
||||
# Postgres-only partial/covering indexes for the financial reports (General Ledger, Trial
|
||||
# Balance, Balance Sheet, P&L), which always filter `is_cancelled = 0` and scope by company.
|
||||
# `where`/`include` are no-ops on MariaDB and its optimizer ignores these anyway, so they are
|
||||
# added only on postgres to avoid dead write overhead on this insert-hot table.
|
||||
frappe.db.add_index(
|
||||
"GL Entry",
|
||||
["company", "posting_date", "account"],
|
||||
index_name="gle_active_detail",
|
||||
where="is_cancelled = 0",
|
||||
)
|
||||
frappe.db.add_index(
|
||||
"GL Entry",
|
||||
["company", "account", "posting_date"],
|
||||
index_name="gle_active_cover",
|
||||
where="is_cancelled = 0",
|
||||
include=["debit", "credit"],
|
||||
)
|
||||
|
||||
|
||||
def rename_gle_sle_docs():
|
||||
for doctype in ["GL Entry", "Stock Ledger Entry"]:
|
||||
|
||||
@@ -317,8 +317,8 @@ class InvoiceDiscounting(AccountsController):
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
def get_invoices(filters: str):
|
||||
filters = frappe._dict(json.loads(filters))
|
||||
def get_invoices(filters: str | dict):
|
||||
filters = frappe._dict(frappe.parse_json(filters))
|
||||
si = frappe.qb.DocType("Sales Invoice")
|
||||
di = frappe.qb.DocType("Discounted Invoice")
|
||||
|
||||
|
||||
@@ -1,8 +1,62 @@
|
||||
# Copyright (c) 2018, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
COMPANY = "_Test Company"
|
||||
TAX_ACCOUNT = "_Test Account VAT - _TC"
|
||||
RECEIVABLE_ACCOUNT = "Debtors - _TC"
|
||||
|
||||
|
||||
class TestItemTaxTemplate(ERPNextTestSuite):
|
||||
pass
|
||||
"""Item Tax Template validates its tax rows: each account must belong to the
|
||||
company, be a tax-like account type, and appear only once."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
|
||||
def make_template(self, rows, title="_Test ITT"):
|
||||
doc = frappe.new_doc("Item Tax Template")
|
||||
doc.title = f"{title} {frappe.generate_hash(length=6)}"
|
||||
doc.company = COMPANY
|
||||
for account, rate, not_applicable in rows:
|
||||
doc.append(
|
||||
"taxes",
|
||||
{"tax_type": account, "tax_rate": rate, "not_applicable": not_applicable},
|
||||
)
|
||||
return doc
|
||||
|
||||
def test_valid_template_saves_and_is_named_with_abbr(self):
|
||||
doc = self.make_template([(TAX_ACCOUNT, 9, 0)])
|
||||
doc.insert()
|
||||
self.assertTrue(doc.name.endswith(" - _TC"))
|
||||
self.assertTrue(doc.name.startswith(doc.title))
|
||||
|
||||
def test_duplicate_tax_type_throws(self):
|
||||
doc = self.make_template([(TAX_ACCOUNT, 9, 0), (TAX_ACCOUNT, 5, 0)])
|
||||
self.assertRaises(frappe.ValidationError, doc.insert)
|
||||
|
||||
def test_account_of_wrong_company_throws(self):
|
||||
other_account = frappe.db.get_value("Account", {"company": "_Test Company 1", "is_group": 0}, "name")
|
||||
self.assertTrue(other_account, "need a non-group account in _Test Company 1")
|
||||
doc = self.make_template([(other_account, 9, 0)])
|
||||
self.assertRaises(frappe.ValidationError, doc.insert)
|
||||
|
||||
def test_disallowed_account_type_throws(self):
|
||||
# a Receivable account is not Tax/Chargeable/Income/Expense
|
||||
doc = self.make_template([(RECEIVABLE_ACCOUNT, 9, 0)])
|
||||
self.assertRaises(frappe.ValidationError, doc.insert)
|
||||
|
||||
def test_not_applicable_row_has_rate_zeroed(self):
|
||||
doc = self.make_template([(TAX_ACCOUNT, 18, 1)])
|
||||
doc.insert()
|
||||
self.assertEqual(doc.taxes[0].tax_rate, 0)
|
||||
|
||||
def test_negative_tax_rate_is_accepted(self):
|
||||
# SUSPECTED BUG: validate never bounds tax_rate, so a negative (or >100) rate
|
||||
# saves silently. Locking the current (wrong) behaviour.
|
||||
doc = self.make_template([(TAX_ACCOUNT, -5, 0)])
|
||||
doc.insert()
|
||||
self.assertEqual(doc.taxes[0].tax_rate, -5)
|
||||
|
||||
@@ -29,7 +29,7 @@ frappe.ui.form.on("Journal Entry", {
|
||||
|
||||
refresh(frm) {
|
||||
if (frm.doc.reversal_of && (frm.is_new() || frm.doc.docstatus == 0)) {
|
||||
frm.set_read_only();
|
||||
erpnext.journal_entry.lock_reversal_entry(frm);
|
||||
}
|
||||
|
||||
erpnext.toggle_naming_series();
|
||||
@@ -232,6 +232,13 @@ Object.assign(erpnext.journal_entry, {
|
||||
}
|
||||
},
|
||||
|
||||
lock_reversal_entry(frm) {
|
||||
frm.fields
|
||||
.filter((field) => field.has_input)
|
||||
.forEach((field) => frm.set_df_property(field.df.fieldname, "read_only", 1));
|
||||
frm.set_df_property("accounts", "read_only", 1);
|
||||
},
|
||||
|
||||
add_custom_buttons(frm) {
|
||||
if (frm.doc.docstatus > 0) {
|
||||
frm.add_custom_button(
|
||||
|
||||
@@ -45,6 +45,20 @@ class JournalEntryTemplate(Document):
|
||||
|
||||
def validate(self):
|
||||
self.validate_party()
|
||||
self.validate_account_company()
|
||||
|
||||
def validate_account_company(self):
|
||||
"""Each row's account must belong to the template's company."""
|
||||
for account in self.accounts:
|
||||
if (
|
||||
account.account
|
||||
and frappe.get_cached_value("Account", account.account, "company") != self.company
|
||||
):
|
||||
frappe.throw(
|
||||
_("Row {0}: Account {1} does not belong to company {2}").format(
|
||||
account.idx, account.account, self.company
|
||||
)
|
||||
)
|
||||
|
||||
def validate_party(self):
|
||||
"""
|
||||
|
||||
@@ -1,9 +1,45 @@
|
||||
# Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
# import frappe
|
||||
|
||||
import frappe
|
||||
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
COMPANY = "_Test Company"
|
||||
|
||||
|
||||
class TestJournalEntryTemplate(ERPNextTestSuite):
|
||||
pass
|
||||
"""Journal Entry Template's only real rule is validate_party: party_type is
|
||||
allowed only on Receivable/Payable accounts, and a party needs a party_type."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
|
||||
def make_template(self, rows, company=COMPANY):
|
||||
doc = frappe.new_doc("Journal Entry Template")
|
||||
doc.template_title = f"_Test JET {frappe.generate_hash(length=6)}"
|
||||
doc.company = company
|
||||
doc.voucher_type = "Journal Entry"
|
||||
doc.naming_series = frappe.get_meta("Journal Entry").get_field("naming_series").options.split("\n")[0]
|
||||
for row in rows:
|
||||
doc.append("accounts", row)
|
||||
return doc
|
||||
|
||||
def test_party_type_only_on_receivable_or_payable_account(self):
|
||||
# Cash is neither Receivable nor Payable, so a party_type here is invalid
|
||||
doc = self.make_template([{"account": "Cash - _TC", "party_type": "Customer"}])
|
||||
self.assertRaises(frappe.ValidationError, doc.validate)
|
||||
|
||||
def test_party_requires_party_type(self):
|
||||
doc = self.make_template([{"account": "Debtors - _TC", "party": "_Test Customer"}])
|
||||
self.assertRaises(frappe.ValidationError, doc.validate)
|
||||
|
||||
def test_account_from_other_company_is_rejected(self):
|
||||
other_receivable = frappe.db.get_value(
|
||||
"Account", {"company": "_Test Company 1", "account_type": "Receivable", "is_group": 0}, "name"
|
||||
)
|
||||
self.assertTrue(other_receivable, "need a receivable account in _Test Company 1")
|
||||
doc = self.make_template(
|
||||
[{"account": other_receivable, "party_type": "Customer", "party": "_Test Customer"}]
|
||||
)
|
||||
self.assertRaises(frappe.ValidationError, doc.insert)
|
||||
|
||||
@@ -65,6 +65,7 @@ def start_merge(docname):
|
||||
total = len(ledger_merge.merge_accounts)
|
||||
for row in ledger_merge.merge_accounts:
|
||||
if not row.merged:
|
||||
frappe.db.savepoint("ledger_merge_row")
|
||||
try:
|
||||
merge_account(
|
||||
row.account,
|
||||
@@ -79,8 +80,7 @@ def start_merge(docname):
|
||||
{"ledger_merge": ledger_merge.name, "current": successful_merges, "total": total},
|
||||
)
|
||||
except Exception:
|
||||
if not frappe.in_test:
|
||||
frappe.db.rollback()
|
||||
frappe.db.rollback(save_point="ledger_merge_row")
|
||||
ledger_merge.log_error("Ledger merge failed")
|
||||
finally:
|
||||
if successful_merges == total:
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
from frappe.query_builder.functions import Sum
|
||||
from frappe.utils import today
|
||||
|
||||
from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice
|
||||
@@ -63,13 +64,9 @@ class TestLoyaltyPointEntry(ERPNextTestSuite):
|
||||
self.assertEqual(doc.loyalty_points, -7)
|
||||
|
||||
# Check balance
|
||||
balance = frappe.db.sql(
|
||||
"""
|
||||
SELECT SUM(loyalty_points)
|
||||
FROM `tabLoyalty Point Entry`
|
||||
WHERE customer = %s
|
||||
""",
|
||||
(self.customer_name,),
|
||||
)[0][0]
|
||||
lpe = frappe.qb.DocType("Loyalty Point Entry")
|
||||
balance = (
|
||||
frappe.qb.from_(lpe).select(Sum(lpe.loyalty_points)).where(lpe.customer == self.customer_name)
|
||||
).run()[0][0]
|
||||
|
||||
self.assertEqual(balance, 3) # 10 added, 7 redeemed
|
||||
|
||||
@@ -8,7 +8,7 @@ frappe.ui.form.on("Loyalty Program", {
|
||||
var help_content = `<table class="table table-bordered" style="background-color: var(--scrollbar-track-color);">
|
||||
<tr><td>
|
||||
<h4>
|
||||
<i class="fa fa-hand-right"></i>
|
||||
<svg class="icon icon-sm"><use href="#icon-info"></use></svg>
|
||||
${__("Notes")}
|
||||
</h4>
|
||||
<ul>
|
||||
@@ -36,7 +36,7 @@ frappe.ui.form.on("Loyalty Program", {
|
||||
)}
|
||||
</li>
|
||||
<li>
|
||||
${__("One customer can be part of only single Loyalty Program.")}
|
||||
${__("One customer can be part of only a single Loyalty Program.")}
|
||||
</li>
|
||||
</ul>
|
||||
</td></tr>
|
||||
@@ -62,7 +62,7 @@ frappe.ui.form.on("Loyalty Program", {
|
||||
refresh: function (frm) {
|
||||
if (frm.doc.loyalty_program_type === "Single Tier Program" && frm.doc.collection_rules.length > 1) {
|
||||
frappe.throw(
|
||||
__("Please select the Multiple Tier Program type for more than one collection rules.")
|
||||
__("Please select the Multiple Tier Program type for more than one collection rule.")
|
||||
);
|
||||
}
|
||||
},
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import unittest
|
||||
|
||||
import frappe
|
||||
from frappe.query_builder.functions import Sum
|
||||
from frappe.utils import cint, flt, getdate, today
|
||||
|
||||
from erpnext.accounts.doctype.loyalty_program.loyalty_program import (
|
||||
@@ -262,14 +263,12 @@ class TestLoyaltyProgram(ERPNextTestSuite):
|
||||
|
||||
def get_points_earned(self):
|
||||
def get_returned_amount():
|
||||
returned_amount = frappe.db.sql(
|
||||
"""
|
||||
select sum(grand_total)
|
||||
from `tabSales Invoice`
|
||||
where docstatus=1 and is_return=1 and ifnull(return_against, '')=%s
|
||||
""",
|
||||
self.name,
|
||||
)
|
||||
si = frappe.qb.DocType("Sales Invoice")
|
||||
returned_amount = (
|
||||
frappe.qb.from_(si)
|
||||
.select(Sum(si.grand_total))
|
||||
.where((si.docstatus == 1) & (si.is_return == 1) & (si.return_against == self.name))
|
||||
).run()
|
||||
return abs(flt(returned_amount[0][0])) if returned_amount else 0
|
||||
|
||||
lp_details = get_loyalty_program_details_with_points(
|
||||
|
||||
@@ -60,6 +60,6 @@ class ModeofPayment(Document):
|
||||
|
||||
if pos_profiles:
|
||||
message = _(
|
||||
"POS Profile {} contains Mode of Payment {}. Please remove them to disable this mode."
|
||||
"POS Profile {0} contains Mode of Payment {1}. Please remove them to disable this mode."
|
||||
).format(frappe.bold(", ".join(pos_profiles)), frappe.bold(str(self.name)))
|
||||
frappe.throw(message, title=_("Not Allowed"))
|
||||
|
||||
@@ -5,9 +5,59 @@ import frappe
|
||||
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
COMPANY = "_Test Company"
|
||||
|
||||
|
||||
class TestModeofPayment(ERPNextTestSuite):
|
||||
pass
|
||||
"""Mode of Payment validates its per-company default accounts (account company
|
||||
must match the row, no company twice) and blocks disabling while a POS Profile
|
||||
still references it."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
|
||||
def make_mop(self, accounts=None, enabled=1):
|
||||
doc = frappe.new_doc("Mode of Payment")
|
||||
doc.mode_of_payment = f"_Test MoP {frappe.generate_hash(length=6)}"
|
||||
doc.type = "General"
|
||||
doc.enabled = enabled
|
||||
for company, account in accounts or []:
|
||||
doc.append("accounts", {"company": company, "default_account": account})
|
||||
return doc
|
||||
|
||||
def test_valid_mode_of_payment_saves(self):
|
||||
doc = self.make_mop(accounts=[(COMPANY, "Cash - _TC")])
|
||||
doc.insert()
|
||||
self.assertTrue(doc.name)
|
||||
|
||||
def test_account_of_wrong_company_throws(self):
|
||||
other_account = frappe.db.get_value("Account", {"company": "_Test Company 1", "is_group": 0}, "name")
|
||||
self.assertTrue(other_account, "need a non-group account in _Test Company 1")
|
||||
doc = self.make_mop(accounts=[(COMPANY, other_account)])
|
||||
self.assertRaises(frappe.ValidationError, doc.insert)
|
||||
|
||||
def test_repeating_company_throws(self):
|
||||
doc = self.make_mop(accounts=[(COMPANY, "Cash - _TC"), (COMPANY, "Debtors - _TC")])
|
||||
self.assertRaises(frappe.ValidationError, doc.insert)
|
||||
|
||||
def test_disabling_mode_referenced_by_pos_profile_is_not_blocked(self):
|
||||
# SUSPECTED BUG: validate_pos_mode_of_payment queries "Sales Invoice Payment"
|
||||
# rows with parenttype "POS Profile", but a POS Profile's payments are stored
|
||||
# as "POS Payment Method" rows. The filter never matches, so the guard is dead
|
||||
# and a mode still referenced by a POS Profile disables without complaint.
|
||||
# Locking the current (wrong) behaviour so a fix to the guard trips this test.
|
||||
from erpnext.accounts.doctype.pos_profile.test_pos_profile import make_pos_profile
|
||||
|
||||
make_pos_profile() # its payments row references the "Cash" mode of payment
|
||||
cash = frappe.get_doc("Mode of Payment", "Cash")
|
||||
cash.enabled = 0
|
||||
cash.save()
|
||||
self.assertEqual(frappe.db.get_value("Mode of Payment", "Cash", "enabled"), 0)
|
||||
|
||||
def test_disabling_unreferenced_mode_succeeds(self):
|
||||
doc = self.make_mop(accounts=[(COMPANY, "Cash - _TC")], enabled=0)
|
||||
doc.insert()
|
||||
self.assertEqual(doc.enabled, 0)
|
||||
|
||||
|
||||
def set_default_account_for_mode_of_payment(mode_of_payment, company, account):
|
||||
|
||||
@@ -1,8 +1,67 @@
|
||||
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
from frappe.utils import getdate
|
||||
|
||||
from erpnext.accounts.doctype.monthly_distribution.monthly_distribution import (
|
||||
get_percentage,
|
||||
get_periodwise_distribution_data,
|
||||
)
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
|
||||
class TestMonthlyDistribution(ERPNextTestSuite):
|
||||
pass
|
||||
"""Monthly Distribution spreads an amount across months. validate() enforces a
|
||||
100% total; get_percentage() sums the months that fall inside a period window."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
|
||||
def make_distribution(self, allocations):
|
||||
doc = frappe.new_doc("Monthly Distribution")
|
||||
doc.distribution_id = f"_Test MD {frappe.generate_hash(length=6)}"
|
||||
for month, pct in allocations:
|
||||
doc.append("percentages", {"month": month, "percentage_allocation": pct})
|
||||
return doc
|
||||
|
||||
def test_get_months_populates_twelve_even_rows(self):
|
||||
doc = frappe.new_doc("Monthly Distribution")
|
||||
doc.distribution_id = "_Test MD Even"
|
||||
doc.get_months()
|
||||
|
||||
self.assertEqual(len(doc.percentages), 12)
|
||||
self.assertEqual(doc.percentages[0].month, "January")
|
||||
self.assertEqual(doc.percentages[-1].month, "December")
|
||||
self.assertEqual([d.idx for d in doc.percentages], list(range(1, 13)))
|
||||
for d in doc.percentages:
|
||||
self.assertAlmostEqual(d.percentage_allocation, 100.0 / 12, places=4)
|
||||
# the auto-populated rows round to exactly 100 and pass validation
|
||||
doc.validate()
|
||||
|
||||
def test_validate_rejects_total_other_than_100(self):
|
||||
doc = self.make_distribution([("January", 50), ("February", 30)]) # sums to 80
|
||||
self.assertRaises(frappe.ValidationError, doc.insert)
|
||||
|
||||
def test_get_percentage_sums_period_window(self):
|
||||
doc = self.make_distribution([("January", 50), ("February", 30), ("March", 20)])
|
||||
doc.insert() # total is 100, so validate passes
|
||||
|
||||
# a quarter starting in January covers Jan+Feb+Mar
|
||||
self.assertEqual(get_percentage(doc, getdate("2026-01-01"), 3), 100)
|
||||
# a single month picks up only that month
|
||||
self.assertEqual(get_percentage(doc, getdate("2026-02-01"), 1), 30)
|
||||
# months with no row simply contribute 0 (there is no guard that all 12 exist)
|
||||
self.assertEqual(get_percentage(doc, getdate("2026-04-01"), 1), 0)
|
||||
|
||||
def test_periodwise_distribution_maps_each_period(self):
|
||||
doc = self.make_distribution([("January", 50), ("February", 30), ("March", 20)])
|
||||
doc.insert()
|
||||
|
||||
period_list = [
|
||||
frappe._dict(key="q1", from_date=getdate("2026-01-01")),
|
||||
frappe._dict(key="q2", from_date=getdate("2026-04-01")),
|
||||
]
|
||||
data = get_periodwise_distribution_data(doc.name, period_list, "Quarterly")
|
||||
self.assertEqual(data["q1"], 100) # Jan+Feb+Mar
|
||||
self.assertEqual(data["q2"], 0) # Apr+May+Jun carry no allocation
|
||||
|
||||
@@ -74,29 +74,31 @@ frappe.ui.form.on("Opening Invoice Creation Tool", {
|
||||
},
|
||||
|
||||
setup_company_filters: function (frm) {
|
||||
frm.set_query("cost_center", "invoices", function (doc, cdt, cdn) {
|
||||
return {
|
||||
filters: {
|
||||
company: doc.company,
|
||||
},
|
||||
};
|
||||
frm.events.apply_company_query_filter(frm, "cost_center", "invoices", { is_group: 0 });
|
||||
frm.events.apply_company_query_filter(frm, "project", "invoices");
|
||||
frm.events.apply_company_query_filter(frm, "project");
|
||||
frm.events.apply_company_query_filter(frm, "cost_center", undefined, { is_group: 0 });
|
||||
frm.events.apply_company_query_filter(frm, "temporary_opening_account", "invoices", {
|
||||
account_type: "Temporary",
|
||||
is_group: 0,
|
||||
});
|
||||
},
|
||||
|
||||
frm.set_query("cost_center", function (doc) {
|
||||
apply_company_query_filter: function (frm, field_name, child_doctype = null, filters = {}) {
|
||||
const query = function (doc) {
|
||||
return {
|
||||
filters: {
|
||||
company: doc.company,
|
||||
...filters,
|
||||
},
|
||||
};
|
||||
});
|
||||
};
|
||||
|
||||
frm.set_query("temporary_opening_account", "invoices", function (doc, cdt, cdn) {
|
||||
return {
|
||||
filters: {
|
||||
company: doc.company,
|
||||
},
|
||||
};
|
||||
});
|
||||
if (child_doctype) {
|
||||
frm.set_query(field_name, child_doctype, query);
|
||||
} else {
|
||||
frm.set_query(field_name, query);
|
||||
}
|
||||
},
|
||||
|
||||
company: function (frm) {
|
||||
@@ -120,11 +122,6 @@ frappe.ui.form.on("Opening Invoice Creation Tool", {
|
||||
},
|
||||
|
||||
invoice_type: function (frm) {
|
||||
$.each(frm.doc.invoices, (idx, row) => {
|
||||
row.party_type = frm.doc.invoice_type == "Sales" ? "Customer" : "Supplier";
|
||||
frappe.model.set_value(row.doctype, row.name, "party", "");
|
||||
frappe.model.set_value(row.doctype, row.name, "party_name", "");
|
||||
});
|
||||
frm.clear_table("invoices");
|
||||
frm.refresh_fields();
|
||||
frm.trigger("update_party_labels");
|
||||
@@ -219,7 +216,19 @@ frappe.ui.form.on("Opening Invoice Creation Tool Item", {
|
||||
});
|
||||
},
|
||||
|
||||
invoices_add: (frm) => {
|
||||
invoices_add: (frm, cdt, cdn) => {
|
||||
const row = frappe.get_doc(cdt, cdn);
|
||||
const field_copy = [];
|
||||
|
||||
["project", "cost_center"].forEach((fieldname) => {
|
||||
if (frm.doc[fieldname]) {
|
||||
frappe.model.set_value(cdt, cdn, fieldname, frm.doc[fieldname]);
|
||||
} else {
|
||||
field_copy.push(fieldname);
|
||||
}
|
||||
});
|
||||
|
||||
frm.script_manager.copy_from_first_row("invoices", row, field_copy);
|
||||
frm.trigger("update_invoice_table");
|
||||
},
|
||||
});
|
||||
|
||||
@@ -110,7 +110,7 @@ class OpeningInvoiceCreationTool(Document):
|
||||
def validate_mandatory_invoice_fields(self, row):
|
||||
if self.create_missing_party:
|
||||
if not row.party and not row.party_name:
|
||||
frappe.throw(_("Row #{}: Either Party ID or Party Name is required").format(row.idx))
|
||||
frappe.throw(_("Row #{0}: Either Party ID or Party Name is required").format(row.idx))
|
||||
|
||||
if not row.party and row.party_name:
|
||||
row.party = self.add_party(row.party_type, row.party_name)
|
||||
@@ -120,10 +120,10 @@ class OpeningInvoiceCreationTool(Document):
|
||||
|
||||
else:
|
||||
if not row.party:
|
||||
frappe.throw(_("Row #{}: Party ID is required").format(row.idx))
|
||||
frappe.throw(_("Row #{0}: Party ID is required").format(row.idx))
|
||||
if not frappe.db.exists(row.party_type, row.party):
|
||||
frappe.throw(
|
||||
_("Row #{}: {} {} does not exist.").format(
|
||||
_("Row #{0}: {1} {2} does not exist.").format(
|
||||
row.idx, frappe.bold(row.party_type), frappe.bold(row.party)
|
||||
)
|
||||
)
|
||||
@@ -133,6 +133,17 @@ class OpeningInvoiceCreationTool(Document):
|
||||
if not row.get(scrub(d)):
|
||||
frappe.throw(mandatory_error_msg.format(row.idx, d, self.invoice_type))
|
||||
|
||||
self.validate_temporary_opening_account(row)
|
||||
|
||||
def validate_temporary_opening_account(self, row):
|
||||
account_type = frappe.get_cached_value("Account", row.temporary_opening_account, "account_type")
|
||||
if account_type != "Temporary":
|
||||
frappe.throw(
|
||||
_("Row #{0}: {1} account is not of type {2}").format(
|
||||
row.idx, row.temporary_opening_account, "Temporary"
|
||||
)
|
||||
)
|
||||
|
||||
def get_invoices(self):
|
||||
invoices = []
|
||||
for row in self.invoices:
|
||||
@@ -203,6 +214,7 @@ class OpeningInvoiceCreationTool(Document):
|
||||
"description": row.item_name or "Opening Invoice Item",
|
||||
income_expense_account_field: row.temporary_opening_account,
|
||||
"cost_center": cost_center,
|
||||
"project": row.get("project") or self.get("project"),
|
||||
}
|
||||
)
|
||||
|
||||
@@ -295,7 +307,7 @@ def start_import(invoices):
|
||||
doc.log_error("Opening invoice creation failed")
|
||||
if errors:
|
||||
frappe.msgprint(
|
||||
_("You had {} errors while creating opening invoices. Check {} for more details").format(
|
||||
_("You had {0} errors while creating opening invoices. Check {1} for more details").format(
|
||||
errors, "<a href='/app/List/Error Log' class='variant-click'>Error Log</a>"
|
||||
),
|
||||
indicator="red",
|
||||
|
||||
@@ -2,10 +2,12 @@
|
||||
# See license.txt
|
||||
|
||||
import frappe
|
||||
from frappe.utils import add_days, today
|
||||
|
||||
from erpnext.accounts.doctype.opening_invoice_creation_tool.opening_invoice_creation_tool import (
|
||||
get_temporary_opening_account,
|
||||
)
|
||||
from erpnext.projects.doctype.project.test_project import make_project
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
|
||||
@@ -14,21 +16,26 @@ class TestOpeningInvoiceCreationTool(ERPNextTestSuite):
|
||||
self,
|
||||
invoice_type="Sales",
|
||||
company=None,
|
||||
party_1=None,
|
||||
party_2=None,
|
||||
invoice_number=None,
|
||||
invoices=None,
|
||||
project=None,
|
||||
cost_center=None,
|
||||
department=None,
|
||||
return_doc=False,
|
||||
):
|
||||
doc = frappe.get_single("Opening Invoice Creation Tool")
|
||||
args = get_opening_invoice_creation_dict(
|
||||
invoice_type=invoice_type,
|
||||
company=company,
|
||||
party_1=party_1,
|
||||
party_2=party_2,
|
||||
invoice_number=invoice_number,
|
||||
invoices=invoices,
|
||||
project=project,
|
||||
cost_center=cost_center,
|
||||
department=department,
|
||||
)
|
||||
doc.update(args)
|
||||
|
||||
if return_doc:
|
||||
return doc
|
||||
|
||||
return doc.make_invoices()
|
||||
|
||||
def test_opening_sales_invoice_creation(self):
|
||||
@@ -37,8 +44,8 @@ class TestOpeningInvoiceCreationTool(ERPNextTestSuite):
|
||||
self.assertEqual(len(invoices), 2)
|
||||
expected_value = {
|
||||
"keys": ["customer", "outstanding_amount", "status"],
|
||||
0: ["_Test Customer", 300, "Overdue"],
|
||||
1: ["_Test Customer 1", 250, "Overdue"],
|
||||
0: ["_Test Customer", 200, "Overdue"],
|
||||
1: ["_Test Customer 1", 200, "Overdue"],
|
||||
}
|
||||
self.check_expected_values(invoices, expected_value)
|
||||
|
||||
@@ -55,48 +62,34 @@ class TestOpeningInvoiceCreationTool(ERPNextTestSuite):
|
||||
for field_idx, field in enumerate(expected_value["keys"]):
|
||||
self.assertEqual(si.get(field, ""), expected_value[invoice_idx][field_idx])
|
||||
|
||||
def test_opening_invoice_requires_temporary_account_type(self):
|
||||
doc = self.make_invoices(company="_Test Opening Invoice Company", return_doc=True)
|
||||
doc.invoices[0].temporary_opening_account = "Sales - _TOIC"
|
||||
self.assertRaises(frappe.ValidationError, doc.make_invoices)
|
||||
|
||||
def test_opening_purchase_invoice_creation(self):
|
||||
invoices = self.make_invoices(invoice_type="Purchase", company="_Test Opening Invoice Company")
|
||||
|
||||
self.assertEqual(len(invoices), 2)
|
||||
expected_value = {
|
||||
"keys": ["supplier", "outstanding_amount", "status"],
|
||||
0: ["_Test Supplier", 300, "Overdue"],
|
||||
1: ["_Test Supplier 1", 250, "Overdue"],
|
||||
0: ["_Test Supplier", 200, "Overdue"],
|
||||
1: ["_Test Supplier 1", 200, "Overdue"],
|
||||
}
|
||||
self.check_expected_values(invoices, expected_value, "Purchase")
|
||||
|
||||
def test_opening_sales_invoice_creation_with_missing_debit_account(self):
|
||||
company = "_Test Opening Invoice Company"
|
||||
party_1, party_2 = make_customer("Customer A"), make_customer("Customer B")
|
||||
|
||||
old_default_receivable_account = frappe.db.get_value("Company", company, "default_receivable_account")
|
||||
frappe.db.set_value("Company", company, "default_receivable_account", "")
|
||||
old_default_receivable_account = frappe.db.get_value(
|
||||
"Company", "_Test Opening Invoice Company", "default_receivable_account"
|
||||
)
|
||||
frappe.db.set_value("Company", "_Test Opening Invoice Company", "default_receivable_account", "")
|
||||
|
||||
if not frappe.db.exists("Cost Center", "_Test Opening Invoice Company - _TOIC"):
|
||||
cc = frappe.get_doc(
|
||||
{
|
||||
"doctype": "Cost Center",
|
||||
"cost_center_name": "_Test Opening Invoice Company",
|
||||
"is_group": 1,
|
||||
"company": "_Test Opening Invoice Company",
|
||||
}
|
||||
)
|
||||
cc.insert(ignore_mandatory=True)
|
||||
cc2 = frappe.get_doc(
|
||||
{
|
||||
"doctype": "Cost Center",
|
||||
"cost_center_name": "Main",
|
||||
"is_group": 0,
|
||||
"company": "_Test Opening Invoice Company",
|
||||
"parent_cost_center": cc.name,
|
||||
}
|
||||
)
|
||||
cc2.insert()
|
||||
|
||||
frappe.db.set_value("Company", company, "cost_center", "Main - _TOIC")
|
||||
|
||||
self.make_invoices(company="_Test Opening Invoice Company", party_1=party_1, party_2=party_2)
|
||||
self.make_invoices(
|
||||
company="_Test Opening Invoice Company",
|
||||
invoices=[{"party": party_1}, {"party": party_2}],
|
||||
)
|
||||
|
||||
# Check if missing debit account error raised
|
||||
error_log = frappe.db.exists(
|
||||
@@ -106,71 +99,107 @@ class TestOpeningInvoiceCreationTool(ERPNextTestSuite):
|
||||
self.assertTrue(error_log)
|
||||
|
||||
# teardown
|
||||
frappe.db.set_value("Company", company, "default_receivable_account", old_default_receivable_account)
|
||||
|
||||
def test_renaming_of_invoice_using_invoice_number_field(self):
|
||||
company = "_Test Opening Invoice Company"
|
||||
party_1, party_2 = make_customer("Customer A"), make_customer("Customer B")
|
||||
self.make_invoices(
|
||||
company=company, party_1=party_1, party_2=party_2, invoice_number="TEST-NEW-INV-11"
|
||||
frappe.db.set_value(
|
||||
"Company",
|
||||
"_Test Opening Invoice Company",
|
||||
"default_receivable_account",
|
||||
old_default_receivable_account,
|
||||
)
|
||||
|
||||
sales_inv1 = frappe.get_all("Sales Invoice", filters={"customer": "Customer A"})[0].get("name")
|
||||
sales_inv2 = frappe.get_all("Sales Invoice", filters={"customer": "Customer B"})[0].get("name")
|
||||
self.assertEqual(sales_inv1, "TEST-NEW-INV-11")
|
||||
def test_renaming_of_invoice_using_invoice_number_field(self):
|
||||
party_1, party_2 = make_customer("Customer A"), make_customer("Customer B")
|
||||
invoices = self.make_invoices(
|
||||
company="_Test Opening Invoice Company",
|
||||
invoices=[
|
||||
{"party": party_1, "invoice_number": "TEST-NEW-INV-11"},
|
||||
{"party": party_2},
|
||||
],
|
||||
)
|
||||
|
||||
# teardown
|
||||
for inv in [sales_inv1, sales_inv2]:
|
||||
doc = frappe.get_doc("Sales Invoice", inv)
|
||||
doc.cancel()
|
||||
self.assertEqual(invoices[0], "TEST-NEW-INV-11")
|
||||
|
||||
def test_opening_invoice_with_accounting_dimension(self):
|
||||
invoices = self.make_invoices(
|
||||
invoice_type="Sales", company="_Test Opening Invoice Company", department="Sales - _TOIC"
|
||||
)
|
||||
|
||||
expected_value = {
|
||||
"keys": ["customer", "outstanding_amount", "status", "department"],
|
||||
0: ["_Test Customer", 300, "Overdue", "Sales - _TOIC"],
|
||||
1: ["_Test Customer 1", 250, "Overdue", "Sales - _TOIC"],
|
||||
}
|
||||
self.check_expected_values(invoices, expected_value, invoice_type="Sales")
|
||||
for invoice in invoices:
|
||||
self.assertEqual(frappe.db.get_value("Sales Invoice", invoice, "department"), "Sales - _TOIC")
|
||||
|
||||
def test_opening_entry_project_linking(self):
|
||||
doc = self.make_invoices(
|
||||
company="_Test Opening Invoice Company", invoice_type="Sales", return_doc=True
|
||||
)
|
||||
project_1 = make_project(
|
||||
{"project_name": "Test Opening Invoice projecty 01", "company": "_Test Opening Invoice Company"}
|
||||
)
|
||||
project_2 = make_project(
|
||||
{"project_name": "Test Opening Invoice projecty 02", "company": "_Test Opening Invoice Company"}
|
||||
)
|
||||
doc.invoices[0].project = project_1.name
|
||||
doc.invoices[1].project = project_2.name
|
||||
invoices = doc.make_invoices()
|
||||
sales_invoice_1 = frappe.get_doc("Sales Invoice", invoices[0])
|
||||
sales_invoice_2 = frappe.get_doc("Sales Invoice", invoices[1])
|
||||
|
||||
self.assertEqual(sales_invoice_1.items[0].project, project_1.name)
|
||||
self.assertEqual(sales_invoice_2.items[0].project, project_2.name)
|
||||
|
||||
|
||||
def get_opening_invoice_creation_dict(**args):
|
||||
party = "Customer" if args.get("invoice_type", "Sales") == "Sales" else "Supplier"
|
||||
company = args.get("company", "_Test Company")
|
||||
default_invoices = []
|
||||
default_invoice_rows = [
|
||||
{
|
||||
"qty": 1.0,
|
||||
"outstanding_amount": 200,
|
||||
"party": f"_Test {party}",
|
||||
"item_name": "Opening Item",
|
||||
"due_date": add_days(today(), -10),
|
||||
"posting_date": add_days(today(), -15),
|
||||
"temporary_opening_account": get_temporary_opening_account(company),
|
||||
},
|
||||
{
|
||||
"qty": 1.0,
|
||||
"outstanding_amount": 200,
|
||||
"party": f"_Test {party} 1",
|
||||
"item_name": "Opening Item",
|
||||
"due_date": add_days(today(), -10),
|
||||
"posting_date": add_days(today(), -15),
|
||||
"temporary_opening_account": get_temporary_opening_account(company),
|
||||
},
|
||||
]
|
||||
|
||||
for row in args.get("invoices") or default_invoice_rows:
|
||||
default_invoices.append(
|
||||
{
|
||||
"qty": row.get("qty") or 1.0,
|
||||
"outstanding_amount": row.get("outstanding_amount") or 200,
|
||||
"party": row.get("party") or f"_Test {party}",
|
||||
"item_name": row.get("item_name") or "Opening Item",
|
||||
"due_date": row.get("due_date") or add_days(today(), -10),
|
||||
"posting_date": row.get("posting_date") or add_days(today(), -15),
|
||||
"temporary_opening_account": row.get("temporary_opening_account")
|
||||
or get_temporary_opening_account(company),
|
||||
"invoice_number": row.get("invoice_number"),
|
||||
"project": row.get("project"),
|
||||
"cost_center": row.get("cost_center"),
|
||||
}
|
||||
)
|
||||
|
||||
invoice_dict = frappe._dict(
|
||||
{
|
||||
"company": company,
|
||||
"invoice_type": args.get("invoice_type", "Sales"),
|
||||
"invoices": [
|
||||
{
|
||||
"qty": 1.0,
|
||||
"outstanding_amount": 300,
|
||||
"party": args.get("party_1") or f"_Test {party}",
|
||||
"item_name": "Opening Item",
|
||||
"due_date": "2016-09-10",
|
||||
"posting_date": "2016-09-05",
|
||||
"temporary_opening_account": get_temporary_opening_account(company),
|
||||
"invoice_number": args.get("invoice_number"),
|
||||
},
|
||||
{
|
||||
"qty": 2.0,
|
||||
"outstanding_amount": 250,
|
||||
"party": args.get("party_2") or f"_Test {party} 1",
|
||||
"item_name": "Opening Item",
|
||||
"due_date": "2016-09-10",
|
||||
"posting_date": "2016-09-05",
|
||||
"temporary_opening_account": get_temporary_opening_account(company),
|
||||
"invoice_number": None,
|
||||
},
|
||||
],
|
||||
"project": args.get("project"),
|
||||
"cost_center": args.get("cost_center"),
|
||||
"invoices": default_invoices,
|
||||
}
|
||||
)
|
||||
|
||||
invoice_dict.update(args)
|
||||
invoice_dict.invoices = default_invoices
|
||||
return invoice_dict
|
||||
|
||||
|
||||
|
||||
@@ -21,7 +21,8 @@
|
||||
"qty",
|
||||
"accounting_dimensions_section",
|
||||
"cost_center",
|
||||
"dimension_col_break"
|
||||
"dimension_col_break",
|
||||
"project"
|
||||
],
|
||||
"fields": [
|
||||
{
|
||||
@@ -125,11 +126,17 @@
|
||||
"fieldtype": "Data",
|
||||
"in_list_view": 1,
|
||||
"label": "Party Name"
|
||||
},
|
||||
{
|
||||
"fieldname": "project",
|
||||
"fieldtype": "Link",
|
||||
"label": "Project",
|
||||
"options": "Project"
|
||||
}
|
||||
],
|
||||
"istable": 1,
|
||||
"links": [],
|
||||
"modified": "2026-03-20 02:11:42.023575",
|
||||
"modified": "2026-04-29 17:08:15.617047",
|
||||
"modified_by": "Administrator",
|
||||
"module": "Accounts",
|
||||
"name": "Opening Invoice Creation Tool Item",
|
||||
|
||||
@@ -26,6 +26,7 @@ class OpeningInvoiceCreationToolItem(Document):
|
||||
party_name: DF.Data | None
|
||||
party_type: DF.Link | None
|
||||
posting_date: DF.Date | None
|
||||
project: DF.Link | None
|
||||
qty: DF.Data | None
|
||||
supplier_invoice_date: DF.Date | None
|
||||
temporary_opening_account: DF.Link | None
|
||||
|
||||
@@ -37,7 +37,7 @@ class PartyLink(Document):
|
||||
)
|
||||
if existing_party_link:
|
||||
frappe.throw(
|
||||
_("{} {} is already linked with {} {}").format(
|
||||
_("{0} {1} is already linked with {2} {3}").format(
|
||||
self.primary_role,
|
||||
bold(self.primary_party),
|
||||
self.secondary_role,
|
||||
@@ -50,7 +50,7 @@ class PartyLink(Document):
|
||||
)
|
||||
if existing_party_link:
|
||||
frappe.throw(
|
||||
_("{} {} is already linked with another {}").format(
|
||||
_("{0} {1} is already linked with another {2}").format(
|
||||
self.secondary_role, self.secondary_party, existing_party_link[0]
|
||||
)
|
||||
)
|
||||
@@ -60,7 +60,7 @@ class PartyLink(Document):
|
||||
)
|
||||
if existing_party_link:
|
||||
frappe.throw(
|
||||
_("{} {} is already linked with another {}").format(
|
||||
_("{0} {1} is already linked with another {2}").format(
|
||||
self.primary_role, self.primary_party, existing_party_link[0]
|
||||
)
|
||||
)
|
||||
|
||||
@@ -1,9 +1,67 @@
|
||||
# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
# import frappe
|
||||
|
||||
import frappe
|
||||
|
||||
from erpnext.accounts.doctype.party_link.party_link import create_party_link
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
CUSTOMER = "_Test Customer"
|
||||
SUPPLIER = "_Test Supplier"
|
||||
SUPPLIER_2 = "_Test Supplier 1"
|
||||
|
||||
|
||||
class TestPartyLink(ERPNextTestSuite):
|
||||
pass
|
||||
"""Party Link ties a Customer and a Supplier together as one underlying party.
|
||||
validate() constrains the primary role and blocks duplicate links."""
|
||||
|
||||
def setUp(self):
|
||||
frappe.set_user("Administrator")
|
||||
|
||||
def test_create_party_link_with_customer_primary(self):
|
||||
link = create_party_link("Customer", CUSTOMER, SUPPLIER)
|
||||
self.assertEqual(link.primary_role, "Customer")
|
||||
self.assertEqual(link.secondary_role, "Supplier")
|
||||
self.assertEqual(link.primary_party, CUSTOMER)
|
||||
self.assertEqual(link.secondary_party, SUPPLIER)
|
||||
self.assertTrue(frappe.db.exists("Party Link", link.name))
|
||||
|
||||
def test_create_party_link_with_supplier_primary(self):
|
||||
link = create_party_link("Supplier", SUPPLIER, CUSTOMER)
|
||||
self.assertEqual(link.primary_role, "Supplier")
|
||||
self.assertEqual(link.secondary_role, "Customer")
|
||||
self.assertEqual(link.primary_party, SUPPLIER)
|
||||
self.assertEqual(link.secondary_party, CUSTOMER)
|
||||
self.assertTrue(frappe.db.exists("Party Link", link.name))
|
||||
|
||||
def test_primary_role_must_be_customer_or_supplier(self):
|
||||
doc = frappe.new_doc("Party Link")
|
||||
doc.primary_role = "Employee"
|
||||
doc.primary_party = CUSTOMER
|
||||
doc.secondary_role = "Supplier"
|
||||
doc.secondary_party = SUPPLIER
|
||||
# validate() alone isolates the role rule from the dynamic-link checks
|
||||
self.assertRaises(frappe.ValidationError, doc.validate)
|
||||
|
||||
def test_duplicate_link_throws(self):
|
||||
create_party_link("Customer", CUSTOMER, SUPPLIER)
|
||||
dup = frappe.new_doc("Party Link")
|
||||
dup.primary_role = "Customer"
|
||||
dup.primary_party = CUSTOMER
|
||||
dup.secondary_role = "Supplier"
|
||||
dup.secondary_party = SUPPLIER
|
||||
self.assertRaises(frappe.ValidationError, dup.insert)
|
||||
|
||||
def test_party_can_wrongly_be_primary_in_two_links(self):
|
||||
# SUSPECTED BUG: the uniqueness checks are asymmetric - a party already a
|
||||
# *primary* in another link isn't blocked, so one customer can be linked to two
|
||||
# different suppliers, breaking the 1:1 mapping. Locking the current (wrong)
|
||||
# behaviour so a fix that blocks primary reuse trips this test.
|
||||
create_party_link("Customer", CUSTOMER, SUPPLIER)
|
||||
link2 = frappe.new_doc("Party Link")
|
||||
link2.primary_role = "Customer"
|
||||
link2.primary_party = CUSTOMER
|
||||
link2.secondary_role = "Supplier"
|
||||
link2.secondary_party = SUPPLIER_2
|
||||
link2.insert()
|
||||
self.assertTrue(frappe.db.exists("Party Link", link2.name))
|
||||
|
||||
@@ -414,21 +414,17 @@ frappe.ui.form.on("Payment Entry", {
|
||||
|
||||
show_general_ledger: function (frm) {
|
||||
if (frm.doc.docstatus > 0) {
|
||||
frm.add_custom_button(
|
||||
__("Ledger"),
|
||||
function () {
|
||||
frappe.route_options = {
|
||||
voucher_no: frm.doc.name,
|
||||
from_date: frm.doc.posting_date,
|
||||
to_date: moment(frm.doc.modified).format("YYYY-MM-DD"),
|
||||
company: frm.doc.company,
|
||||
categorize_by: "",
|
||||
show_cancelled_entries: frm.doc.docstatus === 2,
|
||||
};
|
||||
frappe.set_route("query-report", "General Ledger");
|
||||
},
|
||||
"fa fa-table"
|
||||
);
|
||||
frm.add_custom_button(__("Ledger"), function () {
|
||||
frappe.route_options = {
|
||||
voucher_no: frm.doc.name,
|
||||
from_date: frm.doc.posting_date,
|
||||
to_date: moment(frm.doc.modified).format("YYYY-MM-DD"),
|
||||
company: frm.doc.company,
|
||||
categorize_by: "",
|
||||
show_cancelled_entries: frm.doc.docstatus === 2,
|
||||
};
|
||||
frappe.set_route("query-report", "General Ledger");
|
||||
});
|
||||
}
|
||||
},
|
||||
|
||||
@@ -754,17 +750,21 @@ frappe.ui.form.on("Payment Entry", {
|
||||
frm.set_paid_amount_based_on_received_amount = true;
|
||||
let company_currency = frappe.get_doc(":Company", frm.doc.company).default_currency;
|
||||
|
||||
if (frm.doc.base_received_amount && frm.doc.source_exchange_rate) {
|
||||
frm.set_value("base_paid_amount", frm.doc.base_received_amount);
|
||||
if (frm.doc.paid_amount && frm.doc.source_exchange_rate) {
|
||||
frm.set_value("base_paid_amount", flt(frm.doc.paid_amount) * flt(frm.doc.source_exchange_rate));
|
||||
frm.set_value("base_received_amount", frm.doc.base_paid_amount);
|
||||
|
||||
// target exchange rate should always be same as source if both account currencies is same
|
||||
if (frm.doc.paid_from_account_currency == frm.doc.paid_to_account_currency) {
|
||||
frm.set_value("target_exchange_rate", frm.doc.source_exchange_rate);
|
||||
frm.set_value("received_amount", frm.doc.paid_amount);
|
||||
} else {
|
||||
frm.set_value(
|
||||
"paid_amount",
|
||||
flt(frm.doc.base_paid_amount) / flt(frm.doc.source_exchange_rate)
|
||||
);
|
||||
const target_rate =
|
||||
flt(frm.doc.target_exchange_rate) ||
|
||||
(company_currency == frm.doc.paid_to_account_currency ? 1 : 0);
|
||||
if (target_rate) {
|
||||
frm.set_value("received_amount", flt(frm.doc.base_received_amount) / target_rate);
|
||||
}
|
||||
}
|
||||
|
||||
// set_unallocated_amount is called by below method,
|
||||
@@ -780,18 +780,23 @@ frappe.ui.form.on("Payment Entry", {
|
||||
target_exchange_rate: function (frm) {
|
||||
let company_currency = frappe.get_doc(":Company", frm.doc.company).default_currency;
|
||||
|
||||
if (frm.doc.base_paid_amount && frm.doc.target_exchange_rate) {
|
||||
frm.set_value("base_received_amount", frm.doc.base_paid_amount);
|
||||
if (
|
||||
!frm.doc.source_exchange_rate &&
|
||||
frm.doc.paid_from_account_currency == frm.doc.paid_to_account_currency
|
||||
) {
|
||||
if (frm.doc.received_amount && frm.doc.target_exchange_rate) {
|
||||
frm.set_value(
|
||||
"base_received_amount",
|
||||
flt(frm.doc.received_amount) * flt(frm.doc.target_exchange_rate)
|
||||
);
|
||||
frm.set_value("base_paid_amount", frm.doc.base_received_amount);
|
||||
|
||||
if (frm.doc.paid_from_account_currency == frm.doc.paid_to_account_currency) {
|
||||
frm.set_value("source_exchange_rate", frm.doc.target_exchange_rate);
|
||||
frm.set_value("paid_amount", frm.doc.received_amount);
|
||||
} else {
|
||||
frm.set_value(
|
||||
"received_amount",
|
||||
flt(frm.doc.base_received_amount) / flt(frm.doc.target_exchange_rate)
|
||||
);
|
||||
const source_rate =
|
||||
flt(frm.doc.source_exchange_rate) ||
|
||||
(company_currency == frm.doc.paid_from_account_currency ? 1 : 0);
|
||||
if (source_rate) {
|
||||
frm.set_value("paid_amount", flt(frm.doc.base_paid_amount) / source_rate);
|
||||
}
|
||||
}
|
||||
|
||||
// set_unallocated_amount is called by below method,
|
||||
@@ -968,7 +973,7 @@ frappe.ui.form.on("Payment Entry", {
|
||||
let to_field = fields[key][1];
|
||||
|
||||
if (filters[from_field] && !filters[to_field]) {
|
||||
frappe.throw(__("Error: {0} is mandatory field", [to_field.replace(/_/g, " ")]));
|
||||
frappe.throw(__("Error: {0} is a mandatory field", [to_field.replace(/_/g, " ")]));
|
||||
} else if (filters[from_field] && filters[from_field] > filters[to_field]) {
|
||||
frappe.throw(
|
||||
__("{0}: {1} must be less than {2}", [
|
||||
|
||||
@@ -514,10 +514,12 @@ class PaymentEntry(AccountsController):
|
||||
invoice_names.add((ref.reference_doctype, ref.reference_name))
|
||||
|
||||
for doctype, name in invoice_names:
|
||||
frappe.db.savepoint("subscription_update")
|
||||
try:
|
||||
doc = frappe.get_doc(doctype, name)
|
||||
doc.refresh_subscription_status()
|
||||
except Exception:
|
||||
frappe.db.rollback(save_point="subscription_update")
|
||||
frappe.log_error(_("Failed to update subscription status for {0} {1}").format(doctype, name))
|
||||
|
||||
def set_missing_values(self):
|
||||
@@ -621,7 +623,7 @@ class PaymentEntry(AccountsController):
|
||||
|
||||
def validate_payment_type(self):
|
||||
if self.payment_type not in ("Receive", "Pay", "Internal Transfer"):
|
||||
frappe.throw(_("Payment Type must be one of Receive, Pay and Internal Transfer"))
|
||||
frappe.throw(_("Payment Type must be one of Receive, Pay, or Internal Transfer"))
|
||||
|
||||
def validate_party_details(self):
|
||||
if self.party and not frappe.db.exists(self.party_type, self.party):
|
||||
@@ -678,7 +680,7 @@ class PaymentEntry(AccountsController):
|
||||
|
||||
elif d.reference_name:
|
||||
if not frappe.db.exists(d.reference_doctype, d.reference_name):
|
||||
frappe.throw(_("{0} {1} does not exist").format(d.reference_doctype, d.reference_name))
|
||||
frappe.throw(_("{0} {1} does not exist").format(_(d.reference_doctype), d.reference_name))
|
||||
|
||||
ref_doc = frappe.get_lazy_doc(d.reference_doctype, d.reference_name)
|
||||
|
||||
@@ -1805,8 +1807,7 @@ class PaymentEntry(AccountsController):
|
||||
if not self.references or not matched_payment_requests:
|
||||
return
|
||||
|
||||
if isinstance(matched_payment_requests, str):
|
||||
matched_payment_requests = json.loads(matched_payment_requests)
|
||||
matched_payment_requests = frappe.parse_json(matched_payment_requests)
|
||||
|
||||
# modify matched_payment_requests
|
||||
# like (reference_doctype, reference_name, allocated_amount): payment_request
|
||||
@@ -2011,8 +2012,7 @@ def validate_inclusive_tax(tax, doc):
|
||||
|
||||
@frappe.whitelist()
|
||||
def get_outstanding_reference_documents(args: str | dict, validate: bool = False):
|
||||
if isinstance(args, str):
|
||||
args = json.loads(args)
|
||||
args = frappe.parse_json(args)
|
||||
|
||||
if args.get("party_type") == "Member":
|
||||
return
|
||||
@@ -2685,7 +2685,7 @@ def get_payment_entry(
|
||||
|
||||
# only Purchase Invoice can be blocked individually
|
||||
if doc.doctype == "Purchase Invoice" and doc.invoice_is_blocked():
|
||||
frappe.msgprint(_("{0} is on hold till {1}").format(doc.name, doc.release_date))
|
||||
frappe.msgprint(_("{0} is on hold until {1}").format(doc.name, doc.release_date))
|
||||
else:
|
||||
if doc.doctype in (
|
||||
"Sales Invoice",
|
||||
@@ -2795,6 +2795,9 @@ def get_open_payment_requests_for_references(references=None):
|
||||
.where(PR.docstatus == 1)
|
||||
.where(PR.outstanding_amount > 0) # to avoid old PRs with 0 outstanding amount
|
||||
.orderby(Coalesce(PR.transaction_date, PR.creation), order=frappe.qb.asc)
|
||||
# unique tiebreaker so PRs sharing a transaction_date allocate in the same order on both engines
|
||||
.orderby(PR.creation, order=frappe.qb.asc)
|
||||
.orderby(PR.name, order=frappe.qb.asc)
|
||||
).run(as_dict=True)
|
||||
|
||||
if not response:
|
||||
@@ -3089,7 +3092,7 @@ def apply_early_payment_discount(paid_amount, received_amount, doc, party_accoun
|
||||
if total_discount:
|
||||
currency = doc.get("currency") if is_multi_currency else doc.company_currency
|
||||
money = frappe.utils.fmt_money(total_discount, currency=currency)
|
||||
frappe.msgprint(_("Discount of {} applied as per Payment Term").format(money), alert=1)
|
||||
frappe.msgprint(_("Discount of {0} applied as per Payment Term").format(money), alert=1)
|
||||
|
||||
return paid_amount, received_amount, total_discount, valid_discounts
|
||||
|
||||
|
||||
@@ -246,6 +246,62 @@ class TestPaymentEntry(ERPNextTestSuite):
|
||||
outstanding_amount = flt(frappe.db.get_value("Sales Invoice", pi.name, "outstanding_amount"))
|
||||
self.assertEqual(outstanding_amount, 0)
|
||||
|
||||
def test_pay_multiple_purchase_invoices_in_one_entry(self):
|
||||
pi1 = make_purchase_invoice() # outstanding 250
|
||||
pi2 = make_purchase_invoice() # outstanding 250
|
||||
|
||||
pe = get_payment_entry("Purchase Invoice", pi1.name, bank_account="_Test Cash - _TC")
|
||||
pe.append(
|
||||
"references",
|
||||
{
|
||||
"reference_doctype": "Purchase Invoice",
|
||||
"reference_name": pi2.name,
|
||||
"total_amount": pi2.grand_total,
|
||||
"outstanding_amount": pi2.outstanding_amount,
|
||||
"allocated_amount": pi2.outstanding_amount,
|
||||
},
|
||||
)
|
||||
pe.paid_amount = pe.received_amount = (
|
||||
pe.references[0].allocated_amount + pe.references[1].allocated_amount
|
||||
)
|
||||
pe.insert()
|
||||
pe.submit()
|
||||
|
||||
self.assertEqual(pe.total_allocated_amount, 500)
|
||||
self.assertEqual(frappe.db.get_value("Purchase Invoice", pi1.name, "outstanding_amount"), 0)
|
||||
self.assertEqual(frappe.db.get_value("Purchase Invoice", pi2.name, "outstanding_amount"), 0)
|
||||
|
||||
def test_unallocated_amount_on_overpaid_purchase_payment(self):
|
||||
pi = make_purchase_invoice() # outstanding 250
|
||||
|
||||
pe = get_payment_entry("Purchase Invoice", pi.name, bank_account="_Test Cash - _TC")
|
||||
pe.paid_amount = pe.references[0].allocated_amount + 200 # overpay -> 200 advance
|
||||
pe.received_amount = pe.paid_amount
|
||||
pe.insert()
|
||||
pe.submit()
|
||||
|
||||
self.assertEqual(pe.docstatus, 1)
|
||||
self.assertEqual(pe.unallocated_amount, 200)
|
||||
|
||||
# end-to-end: submitting posts a balanced GL for the full paid amount (250
|
||||
# settling the invoice + 200 advance)
|
||||
gl_entries = frappe.get_all(
|
||||
"GL Entry",
|
||||
filters={"voucher_no": pe.name, "is_cancelled": 0},
|
||||
fields=["debit", "credit"],
|
||||
)
|
||||
self.assertTrue(gl_entries, "Submitted payment produced no GL entries")
|
||||
self.assertEqual(flt(sum(e.debit for e in gl_entries)), flt(sum(e.credit for e in gl_entries)))
|
||||
self.assertEqual(flt(sum(e.debit for e in gl_entries)), 450)
|
||||
|
||||
def test_overallocation_against_purchase_invoice_throws(self):
|
||||
pi = make_purchase_invoice() # outstanding 250
|
||||
|
||||
pe = get_payment_entry("Purchase Invoice", pi.name, bank_account="_Test Cash - _TC")
|
||||
pe.references[0].allocated_amount += 100 # 350 > 250 outstanding
|
||||
pe.paid_amount = pe.received_amount = pe.references[0].allocated_amount
|
||||
self.assertRaises(frappe.ValidationError, pe.insert)
|
||||
|
||||
def test_payment_against_sales_invoice_to_check_status(self):
|
||||
si = create_sales_invoice(
|
||||
customer="_Test Customer USD",
|
||||
@@ -2317,3 +2373,65 @@ def create_customer(name="_Test Customer 2 USD", currency="USD"):
|
||||
customer.save()
|
||||
customer = customer.name
|
||||
return customer
|
||||
|
||||
|
||||
class TestPaymentEntryValidation(ERPNextTestSuite):
|
||||
"""Field-level validations invoked on the document directly, covering branches the
|
||||
integration suite above doesn't reach (no GL / reconciliation setup needed)."""
|
||||
|
||||
def make_pe(self, **fields):
|
||||
doc = frappe.new_doc("Payment Entry")
|
||||
doc.update(fields)
|
||||
return doc
|
||||
|
||||
def test_payment_type_must_be_a_known_value(self):
|
||||
self.assertRaises(frappe.ValidationError, self.make_pe(payment_type="Foo").validate_payment_type)
|
||||
self.make_pe(payment_type="Receive").validate_payment_type() # valid value passes
|
||||
|
||||
def test_nonexistent_party_is_rejected(self):
|
||||
doc = self.make_pe(party_type="Customer", party="__No Such Customer__")
|
||||
self.assertRaises(frappe.ValidationError, doc.validate_party_details)
|
||||
|
||||
def test_amount_and_exchange_rate_fields_are_mandatory(self):
|
||||
# every field but target_exchange_rate is set, so that missing one raises
|
||||
doc = self.make_pe(
|
||||
paid_amount=100, received_amount=100, source_exchange_rate=1, target_exchange_rate=0
|
||||
)
|
||||
self.assertRaises(frappe.ValidationError, doc.validate_mandatory)
|
||||
|
||||
def test_received_amount_cannot_exceed_paid_in_same_currency(self):
|
||||
doc = self.make_pe(
|
||||
paid_from_account_currency="INR",
|
||||
paid_to_account_currency="INR",
|
||||
paid_amount=100,
|
||||
received_amount=150,
|
||||
)
|
||||
self.assertRaises(frappe.ValidationError, doc.validate_received_amount)
|
||||
# received <= paid is fine
|
||||
doc.received_amount = 50
|
||||
doc.validate_received_amount()
|
||||
|
||||
def test_duplicate_reference_rows_are_rejected(self):
|
||||
doc = self.make_pe()
|
||||
for _ in range(2):
|
||||
doc.append(
|
||||
"references",
|
||||
{"reference_doctype": "Sales Invoice", "reference_name": "SI-X", "allocated_amount": 100},
|
||||
)
|
||||
self.assertRaises(frappe.ValidationError, doc.validate_duplicate_entry)
|
||||
|
||||
def test_receive_from_customer_against_negative_outstanding_is_rejected(self):
|
||||
doc = self.make_pe(party_type="Customer", payment_type="Receive")
|
||||
doc.append(
|
||||
"references",
|
||||
{"reference_doctype": "Sales Invoice", "reference_name": "SI-Y", "allocated_amount": -100},
|
||||
)
|
||||
self.assertRaises(frappe.ValidationError, doc.validate_payment_type_with_outstanding)
|
||||
|
||||
def test_bank_transaction_requires_a_reference_number(self):
|
||||
doc = self.make_pe(payment_type="Pay", paid_from="_Test Bank - _TC")
|
||||
self.assertRaises(frappe.ValidationError, doc.validate_transaction_reference)
|
||||
# supplying the reference details clears the requirement
|
||||
doc.reference_no = "TXN-1"
|
||||
doc.reference_date = "2026-06-15"
|
||||
doc.validate_transaction_reference()
|
||||
|
||||
@@ -6,8 +6,10 @@ import frappe
|
||||
from frappe import _, msgprint, qb
|
||||
from frappe.model.document import Document
|
||||
from frappe.model.meta import get_field_precision
|
||||
from frappe.permissions import get_allowed_docs_for_doctype, get_user_permissions
|
||||
from frappe.query_builder import Case, Criterion
|
||||
from frappe.query_builder.custom import ConstantColumn
|
||||
from frappe.query_builder.functions import IfNull
|
||||
from frappe.utils import flt, fmt_money, get_link_to_form, getdate, nowdate, today
|
||||
|
||||
import erpnext
|
||||
@@ -73,6 +75,7 @@ class PaymentReconciliation(Document):
|
||||
self.accounting_dimension_filter_conditions = []
|
||||
self.ple_posting_date_filter = []
|
||||
self.dimensions = get_dimensions(with_cost_center_and_project=True)[0]
|
||||
self.user_permissions = get_user_permissions(frappe.session.user)
|
||||
|
||||
def load_from_db(self):
|
||||
# 'modified' attribute is required for `run_doc_method` to work properly.
|
||||
@@ -153,6 +156,22 @@ class PaymentReconciliation(Document):
|
||||
|
||||
self.add_payment_entries(non_reconciled_payments)
|
||||
|
||||
def get_permitted_dimension_values(self, document_type, reference_doctype):
|
||||
return get_allowed_docs_for_doctype(self.user_permissions.get(document_type, []), reference_doctype)
|
||||
|
||||
def validate_permitted_dimension_value(self, document_type, value, allowed):
|
||||
if value and allowed and value not in allowed:
|
||||
frappe.throw(
|
||||
_("You do not have enough permission to access {0}: {1}").format(_(document_type), value),
|
||||
frappe.PermissionError,
|
||||
)
|
||||
|
||||
def get_user_permission_dimension_condition(self, field, allowed):
|
||||
value_condition = field.isin(allowed)
|
||||
if frappe.get_system_settings("apply_strict_user_permissions"):
|
||||
return value_condition
|
||||
return (IfNull(field, "") == "") | value_condition
|
||||
|
||||
def get_payment_entries(self):
|
||||
party_account = [self.receivable_payable_account]
|
||||
|
||||
@@ -176,8 +195,13 @@ class PaymentReconciliation(Document):
|
||||
dimensions = {}
|
||||
for x in self.dimensions:
|
||||
dimension = x.fieldname
|
||||
if self.get(dimension):
|
||||
dimensions.update({dimension: self.get(dimension)})
|
||||
allowed = self.get_permitted_dimension_values(x.document_type, "Payment Entry")
|
||||
if value := self.get(dimension):
|
||||
self.validate_permitted_dimension_value(x.document_type, value, allowed)
|
||||
dimensions[dimension] = value
|
||||
elif allowed:
|
||||
dimensions[dimension] = allowed
|
||||
|
||||
condition.update({"accounting_dimensions": dimensions})
|
||||
|
||||
payment_entries = get_advance_payment_entries_for_regional(
|
||||
@@ -201,8 +225,12 @@ class PaymentReconciliation(Document):
|
||||
# Dimension filters
|
||||
for x in self.dimensions:
|
||||
dimension = x.fieldname
|
||||
if self.get(dimension):
|
||||
conditions.append(jea[dimension] == self.get(dimension))
|
||||
allowed = self.get_permitted_dimension_values(x.document_type, "Journal Entry Account")
|
||||
if value := self.get(dimension):
|
||||
self.validate_permitted_dimension_value(x.document_type, value, allowed)
|
||||
conditions.append(jea[dimension] == value)
|
||||
elif allowed:
|
||||
conditions.append(self.get_user_permission_dimension_condition(jea[dimension], allowed))
|
||||
|
||||
if self.payment_name:
|
||||
conditions.append(je.name.like(f"%%{self.payment_name}%%"))
|
||||
@@ -748,8 +776,15 @@ class PaymentReconciliation(Document):
|
||||
ple = qb.DocType("Payment Ledger Entry")
|
||||
for x in self.dimensions:
|
||||
dimension = x.fieldname
|
||||
if self.get(dimension) and frappe.db.has_column("Payment Ledger Entry", dimension):
|
||||
self.accounting_dimension_filter_conditions.append(ple[dimension] == self.get(dimension))
|
||||
if frappe.db.has_column("Payment Ledger Entry", dimension):
|
||||
allowed = self.get_permitted_dimension_values(x.document_type, "Payment Ledger Entry")
|
||||
if value := self.get(dimension):
|
||||
self.validate_permitted_dimension_value(x.document_type, value, allowed)
|
||||
self.accounting_dimension_filter_conditions.append(ple[dimension] == value)
|
||||
elif allowed:
|
||||
self.accounting_dimension_filter_conditions.append(
|
||||
self.get_user_permission_dimension_condition(ple[dimension], allowed)
|
||||
)
|
||||
|
||||
def build_qb_filter_conditions(self, get_invoices=False, get_return_invoices=False):
|
||||
self.common_filter_conditions.clear()
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
|
||||
|
||||
import frappe
|
||||
from frappe.utils import add_days, add_years, flt, getdate, nowdate, today
|
||||
from frappe.utils import add_days, add_years, cint, flt, getdate, nowdate, today
|
||||
from frappe.utils.data import getdate as convert_to_date
|
||||
|
||||
from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry
|
||||
@@ -1102,6 +1102,101 @@ class TestPaymentReconciliation(ERPNextTestSuite):
|
||||
payment_vouchers = [x.get("reference_name") for x in pr.get("payments")]
|
||||
self.assertCountEqual(payment_vouchers, [je2.name, pe2.name])
|
||||
|
||||
def test_user_permission_on_accounting_dimension_filters_vouchers(self):
|
||||
test_user = "test@example.com"
|
||||
permitted_ccs = ["_Test Cost Center - _TC", "_Test Cost Center 2 - _TC"]
|
||||
restricted_cc = "_Test Write Off Cost Center - _TC"
|
||||
existing_apply_strict_user_permissions = cint(
|
||||
frappe.db.get_single_value("System Settings", "apply_strict_user_permissions")
|
||||
)
|
||||
self.addCleanup(
|
||||
frappe.db.set_single_value,
|
||||
"System Settings",
|
||||
"apply_strict_user_permissions",
|
||||
existing_apply_strict_user_permissions,
|
||||
)
|
||||
transaction_date = nowdate()
|
||||
rate = 100
|
||||
|
||||
def make_invoice(cost_center):
|
||||
si = self.create_sales_invoice(
|
||||
qty=1, rate=rate, posting_date=transaction_date, do_not_submit=True
|
||||
)
|
||||
si.cost_center = cost_center
|
||||
for row in si.items:
|
||||
row.cost_center = cost_center
|
||||
return si.submit()
|
||||
|
||||
def make_payment(cost_center):
|
||||
pe = self.create_payment_entry(posting_date=transaction_date, amount=rate)
|
||||
pe.cost_center = cost_center
|
||||
return pe.save().submit()
|
||||
|
||||
def make_journal(cost_center):
|
||||
je = self.create_journal_entry(
|
||||
self.bank, self.debit_to, 100, transaction_date, cost_center=cost_center
|
||||
)
|
||||
je.accounts[1].party_type = "Customer"
|
||||
je.accounts[1].party = self.customer
|
||||
return je.save().submit()
|
||||
|
||||
# Vouchers tagged with the two permitted cost centers
|
||||
si_allowed = make_invoice(permitted_ccs[0])
|
||||
pe_allowed = make_payment(permitted_ccs[1])
|
||||
je_allowed = make_journal(permitted_ccs[0])
|
||||
|
||||
# Vouchers tagged with the restricted cost center
|
||||
si_restricted = make_invoice(restricted_cc)
|
||||
pe_restricted = make_payment(restricted_cc)
|
||||
je_restricted = make_journal(restricted_cc)
|
||||
|
||||
# Payment entry with a BLANK cost center
|
||||
pe_blank = make_payment(None)
|
||||
|
||||
for cc in permitted_ccs:
|
||||
frappe.permissions.add_user_permission("Cost Center", cc, test_user)
|
||||
|
||||
# Without strict user permissions
|
||||
frappe.db.set_single_value("System Settings", "apply_strict_user_permissions", 0)
|
||||
with self.set_user(test_user):
|
||||
pr = self.create_payment_reconciliation()
|
||||
pr.get_unreconciled_entries()
|
||||
|
||||
invoice_numbers = [x.get("invoice_number") for x in pr.get("invoices")]
|
||||
payment_vouchers = [x.get("reference_name") for x in pr.get("payments")]
|
||||
self.assertIn(si_allowed.name, invoice_numbers)
|
||||
self.assertIn(pe_allowed.name, payment_vouchers)
|
||||
self.assertIn(je_allowed.name, payment_vouchers)
|
||||
self.assertIn(pe_blank.name, payment_vouchers)
|
||||
self.assertNotIn(si_restricted.name, invoice_numbers)
|
||||
self.assertNotIn(pe_restricted.name, payment_vouchers)
|
||||
self.assertNotIn(je_restricted.name, payment_vouchers)
|
||||
|
||||
# With strict user permissions
|
||||
frappe.db.set_single_value("System Settings", "apply_strict_user_permissions", 1)
|
||||
with self.set_user(test_user):
|
||||
pr = self.create_payment_reconciliation()
|
||||
pr.get_unreconciled_entries()
|
||||
|
||||
invoice_numbers = [x.get("invoice_number") for x in pr.get("invoices")]
|
||||
payment_vouchers = [x.get("reference_name") for x in pr.get("payments")]
|
||||
self.assertIn(si_allowed.name, invoice_numbers)
|
||||
self.assertIn(pe_allowed.name, payment_vouchers)
|
||||
self.assertIn(je_allowed.name, payment_vouchers)
|
||||
self.assertNotIn(pe_blank.name, payment_vouchers)
|
||||
self.assertNotIn(si_restricted.name, invoice_numbers)
|
||||
self.assertNotIn(pe_restricted.name, payment_vouchers)
|
||||
self.assertNotIn(je_restricted.name, payment_vouchers)
|
||||
|
||||
# with restricted dimension as a filter
|
||||
with self.set_user(test_user):
|
||||
pr = self.create_payment_reconciliation()
|
||||
pr.cost_center = restricted_cc
|
||||
self.assertRaises(frappe.PermissionError, pr.get_unreconciled_entries)
|
||||
|
||||
for cc in permitted_ccs:
|
||||
frappe.permissions.remove_user_permission("Cost Center", cc, test_user)
|
||||
|
||||
@ERPNextTestSuite.change_settings(
|
||||
"Accounts Settings",
|
||||
{
|
||||
|
||||
@@ -718,7 +718,7 @@ class PaymentRequest(Document):
|
||||
row_number += TO_SKIP_NEW_ROW
|
||||
|
||||
|
||||
@frappe.whitelist()
|
||||
@frappe.whitelist(methods=["POST"])
|
||||
def make_payment_request(**args):
|
||||
"""Make payment request"""
|
||||
|
||||
@@ -740,7 +740,7 @@ def make_payment_request(**args):
|
||||
|
||||
# Schedule-based PRs are allowed only if no Payment Entry exists for this document.
|
||||
# Any existing Payment Entry forces legacy (amount-based) flow.
|
||||
selected_payment_schedules = json.loads(args.get("schedules")) if args.get("schedules") else []
|
||||
selected_payment_schedules = frappe.parse_json(args.get("schedules")) if args.get("schedules") else []
|
||||
|
||||
# Backend guard:
|
||||
# If any Payment Entry exists, schedule-based PRs are not allowed.
|
||||
@@ -931,7 +931,7 @@ def apply_payment_references(pr, payment_reference):
|
||||
|
||||
|
||||
def set_payment_references(payment_schedules):
|
||||
payment_schedules = json.loads(payment_schedules) if payment_schedules else []
|
||||
payment_schedules = frappe.parse_json(payment_schedules) if payment_schedules else []
|
||||
payment_reference = []
|
||||
|
||||
for row in payment_schedules:
|
||||
|
||||
@@ -41,21 +41,17 @@ frappe.ui.form.on("Period Closing Voucher", {
|
||||
|
||||
refresh: function (frm) {
|
||||
if (frm.doc.docstatus > 0) {
|
||||
frm.add_custom_button(
|
||||
__("Ledger"),
|
||||
function () {
|
||||
frappe.route_options = {
|
||||
voucher_no: frm.doc.name,
|
||||
from_date: frm.doc.period_start_date,
|
||||
to_date: frm.doc.period_end_date,
|
||||
company: frm.doc.company,
|
||||
categorize_by: "",
|
||||
show_cancelled_entries: frm.doc.docstatus === 2,
|
||||
};
|
||||
frappe.set_route("query-report", "General Ledger");
|
||||
},
|
||||
"fa fa-table"
|
||||
);
|
||||
frm.add_custom_button(__("Ledger"), function () {
|
||||
frappe.route_options = {
|
||||
voucher_no: frm.doc.name,
|
||||
from_date: frm.doc.period_start_date,
|
||||
to_date: frm.doc.period_end_date,
|
||||
company: frm.doc.company,
|
||||
categorize_by: "",
|
||||
show_cancelled_entries: frm.doc.docstatus === 2,
|
||||
};
|
||||
frappe.set_route("query-report", "General Ledger");
|
||||
});
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
@@ -360,12 +360,15 @@ class TestPeriodClosingVoucher(ERPNextTestSuite):
|
||||
|
||||
self.make_period_closing_voucher(posting_date="2021-03-31")
|
||||
|
||||
# Passed posting_date is after PCV end date, so cancellation should not fail.
|
||||
make_reverse_gl_entries(
|
||||
voucher_type="Journal Entry",
|
||||
voucher_no=jv.name,
|
||||
posting_date="2022-01-01",
|
||||
)
|
||||
frappe.db.set_value("Company", "Test PCV Company", "accounts_frozen_till_date", "2021-12-31")
|
||||
|
||||
try:
|
||||
make_reverse_gl_entries(
|
||||
voucher_type="Journal Entry",
|
||||
voucher_no=jv.name,
|
||||
)
|
||||
finally:
|
||||
frappe.db.set_value("Company", "Test PCV Company", "accounts_frozen_till_date", None)
|
||||
|
||||
totals_after_cancel = frappe.get_all(
|
||||
"GL Entry",
|
||||
|
||||
@@ -121,13 +121,13 @@ class POSClosingEntry(StatusUpdater):
|
||||
continue
|
||||
if pos_invoice.pos_profile != self.pos_profile:
|
||||
invalid_row.setdefault("msg", []).append(
|
||||
_("POS Profile doesn't match {}").format(frappe.bold(self.pos_profile))
|
||||
_("POS Profile doesn't match {0}").format(frappe.bold(self.pos_profile))
|
||||
)
|
||||
if pos_invoice.docstatus != 1:
|
||||
invalid_row.setdefault("msg", []).append(_("POS Invoice is not submitted"))
|
||||
if pos_invoice.owner != self.user:
|
||||
invalid_row.setdefault("msg", []).append(
|
||||
_("POS Invoice isn't created by user {}").format(frappe.bold(self.owner))
|
||||
_("POS Invoice isn't created by user {0}").format(frappe.bold(self.owner))
|
||||
)
|
||||
|
||||
if invalid_row.get("msg"):
|
||||
@@ -139,7 +139,7 @@ class POSClosingEntry(StatusUpdater):
|
||||
error_list = []
|
||||
for row in invalid_rows:
|
||||
for msg in row.get("msg"):
|
||||
error_list.append(_("Row #{}: {}").format(row.get("idx"), msg))
|
||||
error_list.append(_("Row #{0}: {1}").format(row.get("idx"), msg))
|
||||
|
||||
frappe.throw(error_list, title=_("Invalid POS Invoices"), as_list=True)
|
||||
|
||||
@@ -186,13 +186,13 @@ class POSClosingEntry(StatusUpdater):
|
||||
invalid_row.setdefault("msg", []).append(_("Sales Invoice is not created using POS"))
|
||||
if sales_invoice.pos_profile != self.pos_profile:
|
||||
invalid_row.setdefault("msg", []).append(
|
||||
_("POS Profile doesn't match {}").format(frappe.bold(self.pos_profile))
|
||||
_("POS Profile doesn't match {0}").format(frappe.bold(self.pos_profile))
|
||||
)
|
||||
if sales_invoice.docstatus != 1:
|
||||
invalid_row.setdefault("msg", []).append(_("Sales Invoice is not submitted"))
|
||||
if sales_invoice.owner != self.user:
|
||||
invalid_row.setdefault("msg", []).append(
|
||||
_("Sales Invoice isn't created by user {}").format(frappe.bold(self.owner))
|
||||
_("Sales Invoice isn't created by user {0}").format(frappe.bold(self.owner))
|
||||
)
|
||||
|
||||
if invalid_row.get("msg"):
|
||||
@@ -204,7 +204,7 @@ class POSClosingEntry(StatusUpdater):
|
||||
error_list = []
|
||||
for row in invalid_rows:
|
||||
for msg in row.get("msg"):
|
||||
error_list.append(_("Row #{}: {}").format(row.get("idx"), msg))
|
||||
error_list.append(_("Row #{0}: {1}").format(row.get("idx"), msg))
|
||||
|
||||
frappe.throw(error_list, title=_("Invalid Sales Invoices"), as_list=True)
|
||||
|
||||
|
||||
@@ -1425,19 +1425,23 @@
|
||||
"width": "50%"
|
||||
},
|
||||
{
|
||||
"depends_on": "eval:doc.sales_partner",
|
||||
"fetch_from": "sales_partner.commission_rate",
|
||||
"fetch_if_empty": 1,
|
||||
"fieldname": "commission_rate",
|
||||
"fieldtype": "Float",
|
||||
"label": "Commission Rate (%)",
|
||||
"no_copy": 1,
|
||||
"oldfieldname": "commission_rate",
|
||||
"oldfieldtype": "Currency",
|
||||
"print_hide": 1
|
||||
},
|
||||
{
|
||||
"depends_on": "eval:doc.sales_partner",
|
||||
"fieldname": "total_commission",
|
||||
"fieldtype": "Currency",
|
||||
"label": "Total Commission",
|
||||
"no_copy": 1,
|
||||
"oldfieldname": "total_commission",
|
||||
"oldfieldtype": "Currency",
|
||||
"options": "Company:company:default_currency",
|
||||
@@ -1528,9 +1532,11 @@
|
||||
"print_hide": 1
|
||||
},
|
||||
{
|
||||
"depends_on": "eval:doc.sales_partner",
|
||||
"fieldname": "amount_eligible_for_commission",
|
||||
"fieldtype": "Currency",
|
||||
"label": "Amount Eligible for Commission",
|
||||
"no_copy": 1,
|
||||
"options": "Company:company:default_currency",
|
||||
"read_only": 1
|
||||
},
|
||||
@@ -1636,7 +1642,7 @@
|
||||
"icon": "fa fa-file-text",
|
||||
"is_submittable": 1,
|
||||
"links": [],
|
||||
"modified": "2026-05-28 12:22:50.253090",
|
||||
"modified": "2026-06-21 12:46:13.250145",
|
||||
"modified_by": "Administrator",
|
||||
"module": "Accounts",
|
||||
"name": "POS Invoice",
|
||||
|
||||
@@ -279,7 +279,7 @@ class POSInvoice(SalesInvoice):
|
||||
limit=1,
|
||||
)
|
||||
frappe.throw(
|
||||
_("You need to cancel POS Closing Entry {} to be able to cancel this document.").format(
|
||||
_("You need to cancel POS Closing Entry {0} to be able to cancel this document.").format(
|
||||
get_link_to_form("POS Closing Entry", pos_closing_entry[0])
|
||||
),
|
||||
title=_("Not Allowed"),
|
||||
@@ -498,7 +498,7 @@ class POSInvoice(SalesInvoice):
|
||||
if d.get("qty") > 0:
|
||||
frappe.throw(
|
||||
_(
|
||||
"Row #{}: You cannot add positive quantities in a return invoice. Please remove item {} to complete the return."
|
||||
"Row #{0}: You cannot add positive quantities in a return invoice. Please remove item {1} to complete the return."
|
||||
).format(d.idx, frappe.bold(d.item_code)),
|
||||
title=_("Invalid Item"),
|
||||
)
|
||||
@@ -526,7 +526,7 @@ class POSInvoice(SalesInvoice):
|
||||
bold_serial_no = frappe.bold(sr)
|
||||
frappe.throw(
|
||||
_(
|
||||
"Row #{}: Serial No {} cannot be returned since it was not transacted in original invoice {}"
|
||||
"Row #{0}: Serial No {1} cannot be returned since it was not transacted in original invoice {2}"
|
||||
).format(d.idx, bold_serial_no, bold_return_against)
|
||||
)
|
||||
|
||||
@@ -541,7 +541,7 @@ class POSInvoice(SalesInvoice):
|
||||
and frappe.get_cached_value("Account", self.account_for_change_amount, "company") != self.company
|
||||
):
|
||||
frappe.throw(
|
||||
_("The selected change account {} doesn't belongs to Company {}.").format(
|
||||
_("The selected change account {0} does not belong to Company {1}.").format(
|
||||
self.account_for_change_amount, self.company
|
||||
)
|
||||
)
|
||||
@@ -571,12 +571,12 @@ class POSInvoice(SalesInvoice):
|
||||
invoice_total = self.rounded_total or self.grand_total
|
||||
total_amount_in_payments = flt(total_amount_in_payments, self.precision("grand_total"))
|
||||
if total_amount_in_payments and total_amount_in_payments < invoice_total:
|
||||
frappe.throw(_("Total payments amount can't be greater than {}").format(-invoice_total))
|
||||
frappe.throw(_("Total payments amount can't be greater than {0}").format(-invoice_total))
|
||||
|
||||
def validate_company_with_pos_company(self):
|
||||
if self.company != frappe.db.get_value("POS Profile", self.pos_profile, "company"):
|
||||
frappe.throw(
|
||||
_("Company {} does not match with POS Profile Company {}").format(
|
||||
_("Company {0} does not match with POS Profile Company {1}").format(
|
||||
self.company, frappe.db.get_value("POS Profile", self.pos_profile, "company")
|
||||
)
|
||||
)
|
||||
@@ -663,7 +663,6 @@ class POSInvoice(SalesInvoice):
|
||||
def set_pos_fields(self, for_validate=False):
|
||||
"""Set retail related fields from POS Profiles"""
|
||||
from erpnext.stock.get_item_details import (
|
||||
ItemDetailsCtx,
|
||||
get_pos_profile,
|
||||
get_pos_profile_item_details_,
|
||||
)
|
||||
@@ -736,7 +735,7 @@ class POSInvoice(SalesInvoice):
|
||||
for item in self.get("items"):
|
||||
if item.get("item_code"):
|
||||
profile_details = get_pos_profile_item_details_(
|
||||
ItemDetailsCtx(item.as_dict()), profile.get("company"), profile
|
||||
frappe._dict(item.as_dict()), profile.get("company"), profile
|
||||
)
|
||||
for fname, val in profile_details.items():
|
||||
if (not for_validate) or (for_validate and not item.get(fname)):
|
||||
@@ -1036,8 +1035,7 @@ def make_sales_return(source_name: str, target_doc: Document | str | None = None
|
||||
def make_merge_log(invoices: str | list):
|
||||
import json
|
||||
|
||||
if isinstance(invoices, str):
|
||||
invoices = json.loads(invoices)
|
||||
invoices = frappe.parse_json(invoices)
|
||||
|
||||
if len(invoices) == 0:
|
||||
frappe.throw(_("At least one invoice has to be selected."))
|
||||
|
||||
@@ -10,9 +10,9 @@ from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_pu
|
||||
|
||||
class TestPOSInvoiceMerging(POSInvoiceTestMixin):
|
||||
def clear_pos_data(self):
|
||||
frappe.db.sql("delete from `tabPOS Opening Entry`;")
|
||||
frappe.db.sql("delete from `tabPOS Closing Entry`;")
|
||||
frappe.db.sql("delete from `tabPOS Invoice`;")
|
||||
frappe.db.delete("POS Opening Entry")
|
||||
frappe.db.delete("POS Closing Entry")
|
||||
frappe.db.delete("POS Invoice")
|
||||
|
||||
def setUp(self):
|
||||
self.clear_pos_data()
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and Contributors
|
||||
# See license.txt
|
||||
|
||||
# Regression test for https://github.com/frappe/erpnext/issues/56501
|
||||
# AttributeError: 'POSInvoice' object has no attribute 'is_created_using_pos'
|
||||
# when calling reset_mode_of_payments on a draft POS Invoice.
|
||||
|
||||
from erpnext.accounts.doctype.pos_invoice.test_pos_invoice import (
|
||||
POSInvoiceTestMixin,
|
||||
create_pos_invoice,
|
||||
)
|
||||
from erpnext.accounts.doctype.pos_opening_entry.test_pos_opening_entry import create_opening_entry
|
||||
|
||||
|
||||
class TestPOSInvoiceResetModeOfPayments(POSInvoiceTestMixin):
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
create_opening_entry(self.pos_profile, self.test_user.name)
|
||||
|
||||
def test_reset_mode_of_payments_does_not_raise_attribute_error(self):
|
||||
"""Calling reset_mode_of_payments on a draft POS Invoice must not raise
|
||||
AttributeError for the missing is_created_using_pos attribute.
|
||||
|
||||
update_multi_mode_option accesses doc.is_created_using_pos, which is a
|
||||
field on SalesInvoice but does not exist on POSInvoice, causing the error
|
||||
reported in #56501 when a user tries to edit a saved draft order.
|
||||
"""
|
||||
inv = create_pos_invoice(do_not_submit=True)
|
||||
|
||||
# This call must not raise AttributeError on the missing field.
|
||||
inv.reset_mode_of_payments()
|
||||
|
||||
# Payments should have been repopulated from the POS profile.
|
||||
self.assertTrue(len(inv.payments) > 0, "Payments should be populated after reset")
|
||||
@@ -70,7 +70,7 @@ class POSInvoiceMergeLog(Document):
|
||||
for d in self.pos_invoices:
|
||||
if d.customer != self.customer:
|
||||
frappe.throw(
|
||||
_("Row #{}: POS Invoice {} is not against customer {}").format(
|
||||
_("Row #{0}: POS Invoice {1} is not against customer {2}").format(
|
||||
d.idx, d.pos_invoice, self.customer
|
||||
)
|
||||
)
|
||||
@@ -85,11 +85,11 @@ class POSInvoiceMergeLog(Document):
|
||||
bold_status = frappe.bold(status)
|
||||
if docstatus != 1:
|
||||
frappe.throw(
|
||||
_("Row #{}: POS Invoice {} is not submitted yet").format(d.idx, bold_pos_invoice)
|
||||
_("Row #{0}: POS Invoice {1} is not submitted yet").format(d.idx, bold_pos_invoice)
|
||||
)
|
||||
if status == "Consolidated":
|
||||
frappe.throw(
|
||||
_("Row #{}: POS Invoice {} has been {}").format(d.idx, bold_pos_invoice, bold_status)
|
||||
_("Row #{0}: POS Invoice {1} has been {2}").format(d.idx, bold_pos_invoice, bold_status)
|
||||
)
|
||||
if (
|
||||
is_return
|
||||
@@ -101,14 +101,14 @@ class POSInvoiceMergeLog(Document):
|
||||
if return_against_status != "Consolidated":
|
||||
# if return entry is not getting merged in the current pos closing and if it is not consolidated
|
||||
msg = _(
|
||||
"Row #{}: The original Invoice {} of return invoice {} is not consolidated."
|
||||
"Row #{0}: The original Invoice {1} of return invoice {2} is not consolidated."
|
||||
).format(d.idx, bold_return_against, bold_pos_invoice)
|
||||
msg += " "
|
||||
msg += _(
|
||||
"The original invoice should be consolidated before or along with the return invoice."
|
||||
)
|
||||
msg += "<br><br>"
|
||||
msg += _("You can add the original invoice {} manually to proceed.").format(
|
||||
msg += _("You can add the original invoice {0} manually to proceed.").format(
|
||||
bold_return_against
|
||||
)
|
||||
frappe.throw(msg)
|
||||
@@ -330,7 +330,7 @@ class POSInvoiceMergeLog(Document):
|
||||
|
||||
if not dimension_value and (dimension.mandatory_for_pl or dimension.mandatory_for_bs):
|
||||
frappe.throw(
|
||||
_("Please set Accounting Dimension {} in {}").format(
|
||||
_("Please set Accounting Dimension {0} in {1}").format(
|
||||
frappe.bold(dimension.label),
|
||||
frappe.get_desk_link("POS Profile", invoice.pos_profile),
|
||||
)
|
||||
|
||||
Some files were not shown because too many files have changed in this diff Show More
Reference in New Issue
Block a user