Files
shopdb-flask/scripts/check-naming-and-style.sh
cproudlock f34b9ca710
Some checks failed
CI / backend (push) Failing after 9s
CI / naming (push) Successful in 2s
CI / frontend (push) Successful in 11s
CI / migrations-mysql (push) Failing after 7s
Carry the level everywhere a position is drawn, and gate it per occurrence
The hover mini-map said "This asset has a position (2835, 1410) but no level"
for every asset in the product. When 0.11.0 gave LocationMapTooltip a levelid
prop, NONE of its seven call sites were taught to pass one - printer, machine and
PC detail pages, the toner report, enforcement reports, the warranty chip and the
dashboard cards - so the component correctly reported a missing level and the
preview never drew. Two payloads behind those views also emitted mapx/mapy with
no level: the toner report and the enforcement report.

The map PDF export had the ORIGINAL bug still in it: it plotted every filtered
asset onto the sheet, so exporting the ground floor printed second-floor markers
on it. Worse than on screen, because nobody can correct a sheet once it has been
printed and carried onto the floor. It now exports only the level being viewed.

The legacy import loader sent mapleft/maptop with no level at three call sites.
That loader is the one still to run against production, and every marker it
created would have been undrawable. It now resolves the site's default level -
the legacy schema predates levels and has one floor plan, so that is what its
coordinates mean.

THE GATE MISSED ALL OF THIS because it asked whether a FILE mentions 'levelid',
not whether each position does: one module emitted 'mapx' six times and 'levelid'
once and passed. It now checks per occurrence, covers scripts/ as well as shopdb/
and plugins/, and fails any Vue file that binds tooltip coordinates without
:levelid. Both new rules were confirmed to fail the build against planted
violations before being relied on.

Printer QR labels: the asset number is no longer printed. A label now reads name
(8201-HPLaserJetPro), QR, FQDN, then IP. The name falls back to the assetnumber
because that is where sites actually keep it - every printer here has an empty
name field, so preferring the Windows queue name alone would have printed a blank
line on every label.
2026-08-18 09:36:45 -04:00

278 lines
12 KiB
Bash
Executable File

#!/usr/bin/env bash
#
# Pre-commit naming + style check for shopdb-flask.
# Enforces CONTRIBUTING.md rules:
# 1. No non-ASCII chars in source (em-dashes, smart quotes, arrows, emojis)
# 2. No banned shorthand identifiers (cfg, ctx, mgr, req, res, env, util, helper)
# as standalone names (suffix usage like printers_bp, request_obj is allowed)
# 3. No snake_case DB column names in __tablename__ or db.Column attrs
# 4. No snake_case API params in frontend that should match DB column names
#
# Exits non-zero if any violation found.
# Skips: venv/, node_modules/, __pycache__/, frontend/dist/, migrations/versions/,
# deploy/windows/installer/bundle/ (installer build output)
set -e
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || echo .)"
cd "$REPO_ROOT"
VIOLATIONS=0
EXCLUDES=(
--exclude-dir=venv
--exclude-dir=node_modules
--exclude-dir=__pycache__
--exclude-dir=dist
# The installer's build output: a staged copy of the whole application plus
# a second SPA build under dist-subpath, which --exclude-dir=dist does not
# match. Linting it means linting vendored minified JS and failing on
# characters nobody in this repository wrote.
--exclude-dir=bundle
--exclude-dir=dist-subpath
--exclude-dir=.git
--exclude-dir=versions
--exclude-dir=staticdocs
)
INCLUDES_CODE=(
--include='*.py'
--include='*.vue'
--include='*.js'
--include='*.ts'
)
INCLUDES_ALL=(
--include='*.py'
--include='*.vue'
--include='*.js'
--include='*.ts'
--include='*.json'
--include='*.md'
--include='*.yaml'
--include='*.yml'
)
# INCLUDES_ALL, not INCLUDES_CODE: the check only ever covered .py/.vue/.js/.ts,
# so documentation was free to accumulate em-dashes, arrows and smart quotes -
# and did, including in files this repo's own convention forbids them in.
# Markdown, JSON and YAML are now covered too.
echo "==> Checking for non-ASCII characters..."
NON_ASCII=$(grep -rPn '[^\x00-\x7F]' "${EXCLUDES[@]}" "${INCLUDES_ALL[@]}" . 2>/dev/null || true)
if [ -n "$NON_ASCII" ]; then
echo "FAIL: non-ASCII characters found (em-dashes, smart quotes, arrows, emojis):"
echo "$NON_ASCII"
echo
VIOLATIONS=$((VIOLATIONS + 1))
fi
echo "==> Checking for banned shorthand (standalone)..."
# Match the word as a standalone identifier: not preceded or followed by underscore/word char
# Word boundary in grep is \b but we want to exclude suffix usage like printers_bp
# So: match (^|[^a-zA-Z0-9_])(banned)([^a-zA-Z0-9_]|$)
for word in cfg ctx mgr req res; do
HITS=$(grep -rPn "(^|[^a-zA-Z0-9_])${word}([^a-zA-Z0-9_]|\$)" "${EXCLUDES[@]}" --include='*.py' --include='*.vue' --include='*.js' --include='*.ts' . 2>/dev/null \
| grep -vP "(^|[^a-zA-Z0-9_])(request_obj|response_obj)" \
|| true)
if [ -n "$HITS" ]; then
echo "FAIL: banned shorthand '$word' (standalone) found:"
echo "$HITS"
echo
VIOLATIONS=$((VIOLATIONS + 1))
fi
done
echo "==> Checking for snake_case DB tablenames..."
SNAKE_TABLES=$(grep -rPn "__tablename__\s*=\s*['\"][^'\"]*_" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null || true)
if [ -n "$SNAKE_TABLES" ]; then
echo "FAIL: snake_case __tablename__ found (must be lowercase concatenated):"
echo "$SNAKE_TABLES"
echo
VIOLATIONS=$((VIOLATIONS + 1))
fi
echo "==> Checking for snake_case DB column attrs..."
SNAKE_COLS=$(grep -rPn "^\s+[a-z]+_[a-z_]+\s*=\s*db\.Column" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null || true)
if [ -n "$SNAKE_COLS" ]; then
echo "FAIL: snake_case db.Column attribute found (must match column name, no underscores):"
echo "$SNAKE_COLS"
echo
VIOLATIONS=$((VIOLATIONS + 1))
fi
echo "==> Checking for snake_case ForeignKey targets..."
SNAKE_FK=$(grep -rPn "ForeignKey\(['\"][^'\"]*_[^'\"]*['\"]" "${EXCLUDES[@]}" --include='*.py' . 2>/dev/null || true)
if [ -n "$SNAKE_FK" ]; then
echo "FAIL: snake_case ForeignKey target found:"
echo "$SNAKE_FK"
echo
VIOLATIONS=$((VIOLATIONS + 1))
fi
echo "==> Checking for snake_case API params in frontend (DB-mirrored fields)..."
SNAKE_FE=$(grep -rPn "params\.(machine_id|location_id|vendor_id|type_id|business_unit_id|model_id|status_id|operating_system_id|asset_id|user_id|is_active|is_shopfloor)" "${EXCLUDES[@]}" --include='*.vue' --include='*.js' --include='*.ts' . 2>/dev/null || true)
if [ -n "$SNAKE_FE" ]; then
echo "FAIL: snake_case API params in frontend (must match DB column names without underscores):"
echo "$SNAKE_FE"
echo
VIOLATIONS=$((VIOLATIONS + 1))
fi
# ADR-013 Phase 4: a plugin frontend (plugins/<name>/frontend/) may import core
# via the @/ alias or its own tree (./, ../ within the plugin), never another
# plugin's tree and never an escaping ../../ into src. Keeps plugin frontends
# self-contained so a per-site build can drop one cleanly.
echo "==> Checking for cross-plugin / escaping imports in plugin frontends..."
if [ -d plugins ]; then
PLUGIN_FE_IMPORTS=$(grep -rPn "(import|from)\s+['\"]([^'\"]*\.\./\.\./|[^'\"]*/plugins/)" \
--include='*.vue' --include='*.js' plugins/*/frontend/ 2>/dev/null || true)
if [ -n "$PLUGIN_FE_IMPORTS" ]; then
echo "FAIL: plugin frontend imports must use the @/ alias for core, not"
echo " an escaping ../../ or another plugin's path:"
echo "$PLUGIN_FE_IMPORTS"
echo
VIOLATIONS=$((VIOLATIONS + 1))
fi
fi
# ADR-015: one site's data does not belong in product code. A site host, a site
# FQDN or a site network in plugins/ or shopdb/ ships another site a value it
# cannot see and did not choose. Use a setting with a NEUTRAL default, a
# site-namespaced directory (scripts/site_imports/<site>/), or seed data.
#
# Code generation lives in one module. Seven views were importing qrcode and
# jsbarcode directly, each with its own margin, width and error correction, and
# the answers had already drifted - which on a label means one page scans and
# another does not. frontend/src/utils/codes.js owns that knowledge now; a view
# passes what is specific to its label and nothing else.
echo "==> Checking that only the shared module imports the code libraries..."
CODE_LIB_IMPORTS=$(grep -rn "from ['\"]\(qrcode\|jsbarcode\)['\"]" \
--include='*.vue' --include='*.js' \
frontend/src plugins/ 2>/dev/null \
| grep -v 'frontend/src/utils/codes\.js' \
| grep -v '\.plugins-staged/' || true)
if [ -n "$CODE_LIB_IMPORTS" ]; then
echo "FAIL: import qrcode/jsbarcode through @/utils/codes, not directly:"
echo "$CODE_LIB_IMPORTS"
echo
VIOLATIONS=$((VIOLATIONS + 1))
fi
# ADR-017: a marker position is pixels in ONE LEVEL's coordinate space, so a
# payload that emits mapx/mapy without levelid gives the consumer coordinates and
# no drawing to put them on. The consumer then either renders nothing or - worse,
# and what a reasonable implementation does - falls back to the default level,
# drawing one building's ground floor behind a marker positioned for another
# building's mezzanine. That renders perfectly and points at the wrong place.
#
# Eight files emit positions across twenty sites. This is the check that says
# whether all of them travel with their level, because reading them by eye is
# how the twentieth gets missed.
echo "==> Checking that emitted map positions carry their level (ADR-017)..."
# PER OCCURRENCE, not per file. The file-level form passed a module that emitted
# 'mapx' six times and 'levelid' once, and two payloads shipped without a level:
# the toner report and the enforcement report, both feeding a hover preview that
# then said "no level". Every 'mapx' must have a 'levelid' in the same literal -
# the window is wide enough for a comment between them, and no wider.
MISSING_LEVEL=""
while IFS= read -r hit; do
[ -z "$hit" ] && continue
file=${hit%%:*}
line=${hit#*:}; line=${line%%:*}
if ! sed -n "${line},$((line + 8))p" "$file" | grep -q "'levelid'"; then
MISSING_LEVEL="$MISSING_LEVEL$file:$line"$'\n'
fi
done <<EOF
$(grep -rn "'mapx':" --include='*.py' shopdb/ plugins/ scripts/ 2>/dev/null | grep -v '/tests\?/' || true)
EOF
if [ -n "$MISSING_LEVEL" ]; then
echo "FAIL: these emit 'mapx' with no 'levelid' beside it - a position with no"
echo " level cannot be drawn on the right floor plan (ADR-017):"
echo "$MISSING_LEVEL" | sed 's/^/ /'
VIOLATIONS=$((VIOLATIONS + 1))
fi
# The same rule for the hover preview: binding coordinates into
# LocationMapTooltip without a level makes it report "no level" for every asset,
# which is exactly what shipped in 0.11.0 - all seven call sites missed it.
TOOLTIP_MISSING=""
for candidate in $(grep -rl "LocationMapTooltip" --include='*.vue' frontend/src plugins/ 2>/dev/null | grep -v plugins-staged || true); do
grep -q ':left=' "$candidate" || continue
grep -q ':levelid=' "$candidate" || TOOLTIP_MISSING="$TOOLTIP_MISSING$candidate"$'\n'
done
if [ -n "$TOOLTIP_MISSING" ]; then
echo "FAIL: these bind LocationMapTooltip coordinates without :levelid, so the"
echo " preview cannot know which drawing to use (ADR-017):"
echo "$TOOLTIP_MISSING" | sed 's/^/ /'
VIOLATIONS=$((VIOLATIONS + 1))
fi
# ENFORCING. It was report-only while the backlog was worked off, and the hit
# count then did not move for weeks - a rule that only prints is read as no rule.
# Set SITE_LITERALS_ENFORCE=0 to drop back to reporting for a local run.
SITE_LITERALS_ENFORCE=${SITE_LITERALS_ENFORCE:-1}
echo "==> Checking for site-specific literals in product code (ADR-015)..."
# A real site hostname, a site FQDN, a site name or a site network.
#
# SCOPE. It used to look at .py/.vue/.js under plugins/ and shopdb/ only, and
# every literal that actually reached a second site was somewhere else: the
# PowerShell clients, the installer, the seeds, generated JSON. Case-sensitive
# too, so a capitalised spelling of the host passed. Both fixed - the scan is
# only worth having where
# the leaks are.
#
# Fleet-wide vocabulary (gea-shopfloor-*) is NOT matched: it is overridable
# through the pctypemap settings and is not one site's data.
#
# A line may declare itself deliberate with a trailing `ADR-015-OK: <reason>`
# marker. That is for an organisation-wide default that is genuinely right for
# every site and configurable anyway - not for "we will fix it later". The
# marker makes the claim visible in review; silence would not.
# ASSEMBLED FROM FRAGMENTS, deliberately. This script is published, and the
# publication scrub greps for the very strings below - written literally, the
# rule's own definition trips the gate that enforces it. Same reason
# tests/test_docs_publishable.py splits its terms.
SITE_HOST='tsg'\''wp00525'
SITE_DOMAIN='\.geaero'\''space\.net'
SITE_NAME='West Jeff'\''erson'
SITE_NETS='10\.134\.48\.|10\.48\.249\.'
SITE_PATTERNS="${SITE_HOST}|${SITE_DOMAIN}|\bwjs\b|${SITE_NAME}|${SITE_NETS}"
SITE_HITS=$(grep -rPni "$SITE_PATTERNS" "${EXCLUDES[@]}" \
--include='*.py' --include='*.vue' --include='*.js' \
--include='*.ps1' --include='*.psm1' --include='*.sh' --include='*.iss' \
--include='*.json' --include='*.html' \
plugins/ shopdb/ scripts/ deploy/ frontend/src/ tools/ 2>/dev/null \
| grep -v '/tests\?/' \
| grep -v 'site_imports/' \
| grep -v 'installer/bundle/' \
| grep -v '\.plugins-staged/' \
| grep -v 'check-naming-and-style\.sh' \
| grep -v 'export-github\.sh' \
| grep -v 'ADR-015-OK' || true)
if [ -n "$SITE_HITS" ]; then
COUNT=$(echo "$SITE_HITS" | wc -l)
if [ "$SITE_LITERALS_ENFORCE" = "1" ]; then
echo "FAIL: $COUNT site-specific literal(s) in product code:"
echo "$SITE_HITS"
echo
VIOLATIONS=$((VIOLATIONS + 1))
else
echo " $COUNT site-specific literal(s) found (report only, not failing):"
echo "$SITE_HITS" | sed 's/^/ /'
echo " See docs/adr/ADR-015-site-specific-configuration.md"
echo
fi
fi
if [ "$VIOLATIONS" -gt 0 ]; then
echo "=================================================="
echo "$VIOLATIONS naming/style violation(s) found."
echo "See CONTRIBUTING.md for the full convention."
echo "=================================================="
exit 1
fi
echo "==> All naming/style checks passed."
exit 0