shopdb-flask/CONTRIBUTING.md

Contributing to ShopDB Flask

Coding Standards

Naming principles

1. Spell it out. If a non-technical user looking at a report would not understand a column at a glance, rename it.
2. No project-specific shorthand. A name either uses a word in full, or uses an acronym from the allowed list below.
3. Consistency beats cleverness. Once a name is committed and used in code, do not rename it for marginal improvement.
4. When unsure, ask: "would a new shop-floor IT person understand this?"

Database

• Table names: lowercase, concatenated, plural. No dashes, no underscores.
  • Good: machines, pctypes, networkdevices, businessunits, auditlogs
  • Bad: machine_types, network-devices, BusinessUnits
• Column names: lowercase, concatenated, singular. No dashes, no underscores.
  • Good: machineid, machinenumber, lastzabbixsync, isactive
  • Bad: machine_id, last_zabbix_sync, IsActive
• Foreign keys: referenced table name (singular) + id.
  • Good: locationid references locations, vendorid references vendors
• Booleans: prefix with is or has.
  • Good: isactive, isshopfloor, hasprinter
• Index names: idx_<table>_<column>. Underscores allowed here only because indexes are infrastructure, not data.
  • Good: idx_machines_locationid

Migrations and deploy ordering

• A model change ships WITH its migration in the same change. Adding or changing a column/table on a SQLAlchemy model without the matching Alembic migration is a broken change, even if it passes tests against a freshly built schema.
• Code and schema must never deploy out of step. A model that references a column its migration has not applied yet makes MySQL raise 1054 Unknown column, which 500s every query on that table (this is not a degraded read - the whole endpoint fails).
• Bringing up a new image MUST run migrations before serving traffic, in order: flask db upgrade (core chain) then flask plugin upgrade-all (per-plugin chains). A rebuilt image with new model code but an un-migrated database will 500 until this runs.
• Core schema lives in migrations/versions/; each plugin owns its own chain under plugins/<name>/migrations/. New schema = a new migration with an idempotent guard and a real downgrade, clean on strict MySQL 8.

Allowed acronyms (closed list, expand by PR)

• Universal: id, url, api, http, https, json, jwt, sql, os, ip, dns, csv, pdf, cors, ttl, uuid, html, css, orm
• Domain (shop floor / IT infra): cmm, cnc, pc, usb, vnc, winrm, ssh, ssl, tls, tcp, udp, smtp, ldap, vlan, sso, dnc, focas, clm, mtconnect

Any acronym not on these lists must be spelled out. Proposing a new acronym is a separate PR that updates this list.

Third-party imports are exempt. Library and stdlib module paths cannot be renamed. from sqlalchemy.orm import ..., import importlib.util, from werkzeug.utils import ... are all fine even if they contain words that would otherwise be banned.

Banned shorthand

cfg, ctx, mgr, req, res, env, util, helper, and any other domain-specific shortening. Spell it out: config, context, manager, request, response, environment, utilities. Note: db as a standalone variable name is banned, but db as a prefix in database or as part of project name shopdb is fine.

Suffix exception. Banned shorthand applies to standalone identifiers. As a suffix where the prefix establishes meaning, it is acceptable:

• printers_bp, auth_bp, network_bp are fine. The prefix names what blueprint, the _bp suffix is conventional for Flask blueprints.
• request_obj is fine when disambiguating from imported request.
• bp alone as a variable name is not fine. Always pair with a meaningful prefix.

Code

Python

• Variables and functions holding a database value: match the column name exactly. No conversion to snake_case.
  • DB column machineid -> Python variable machineid, attribute Machine.machineid, dict key {"machineid": 1}
  • DB column lastzabbixsync -> Python variable lastzabbixsync
• Pure code variables and functions (loop counters, helpers, anything that does not mirror a DB field): snake_case per PEP 8.
  • Good: loop_count, current_user, validate_input()
• Classes: PascalCase. Spell out fully.
  • Good: PrinterData, AssetType, NetworkDevice
• Modules / file names: lowercase. Underscores allowed where it improves readability (PEP 8 permits this for modules).
  • Good: employee_service.py, asset_routes.py

JavaScript / Vue

• Variables: camelCase per JS convention.
  • Good: currentUser, isLoading
• Variables holding API field values: match the API field name exactly. Do NOT convert to camelCase.
  • API returns {"machineid": 1} -> JS response.machineid, NOT response.machineId
  • API returns {"lastzabbixsync": "..."} -> JS device.lastzabbixsync
• Components: PascalCase. Spell out fully.
  • Good: AssetDetail.vue, MachineForm.vue, PrinterList.vue
• CSS classes: lowercase with dashes.
  • Good: asset-detail, machine-form

API

• Endpoints: lowercase, plural nouns. No underscores or dashes.
  • Good: /api/machines, /api/networkdevices, /api/businessunits
• Query parameters: lowercase, concatenated. Match column names where applicable.
  • Good: ?locationid=5, ?isactive=true
• Response keys: match database column names exactly.
  • Good: {"machineid": 1, "lastzabbixsync": "2026-01-12T10:00:00Z"}

Examples

# Good - DB-mirrored fields keep column names; pure code uses snake_case
class PrinterData(db.Model):
    __tablename__ = 'printerdata'

    machineid = db.Column(db.Integer, db.ForeignKey('machines.machineid'))
    lastzabbixsync = db.Column(db.DateTime)
    isnetworkprinter = db.Column(db.Boolean)


def list_printers_by_location(locationid: int) -> list[dict]:
    printers = PrinterData.query.filter_by(locationid=locationid).all()
    result_count = len(printers)
    return [{"machineid": p.machineid, "isnetworkprinter": p.isnetworkprinter} for p in printers]

# Bad - PEP 8-style underscores on DB fields, banned shorthand, project acronym not on allowed list
class PrinterData(db.Model):
    __tablename__ = 'printer_data'

    machine_id = db.Column(db.Integer)
    last_zabbix_sync = db.Column(db.DateTime)
    is_net_prn = db.Column(db.Boolean)


def list_prns_by_loc(loc_id):
    bp_ctx = get_context()
    cfg = load_cfg()
    return PrinterData.query.filter_by(loc_id=loc_id).all()

Style policy (non-naming)

• No emojis in code, comments, documentation, string literals, or UI.
• No em-dashes (U+2014), en-dashes (U+2013), Unicode arrows, or smart quotes anywhere. Plain ASCII only.
• Comments default to none. Add one only when the WHY is non-obvious. Keep comments terse (lite caveman style is fine for inline # and // comments). Docstrings stay normal English.
• Dark theme is default. Keep UI functional and professional.

File structure

• Core models: shopdb/core/models/
• Core API routes: shopdb/core/api/
• Core services: shopdb/core/services/
• Plugin code: plugins/<plugin>/{models,api,services,schemas}/

Plugin development

When creating a new plugin:

1. Create directory plugins/<name>/. Use the scaffold (flask plugin new <name>) when available.
2. Include manifest.json with metadata. This is the single source of truth for plugin name, version, dependencies, api_prefix.
3. Extend BasePlugin class.
4. Follow naming conventions above.
5. Run flask plugin install <name> to install.

See the PLUGIN-QUICKSTART wiki page (when published) for the 30-minute walkthrough.

A plugin must not hold a DB session across external I/O

Rule: finish with the database before you call anything outside the process - an HTTP API (Dell warranty, ServiceNow, Zabbix, Azure/Intune), a subprocess, a file on a network share - and open a fresh session afterwards to write the result.

Why it is a rule and not a preference. The app runs gevent workers, so a request that blocks on a network call does not block its worker, and the call can take minutes without anything complaining. What it DOES hold for those minutes is a pooled database connection. Each worker owns a pool of DB_POOL_SIZE + DB_MAX_OVERFLOW connections (see shopdb/config.py and the arithmetic beside --workers in docker-compose.yml), and hundreds of greenlets can enter the same slow path at once. Drain the pool and every other request in that worker blocks on checkout and then fails - the whole site, every plugin, while the CPU sits idle and MySQL reports itself healthy. That is the worst diagnostic signature there is.

The shape to write:

# Read what you need, then let the session go.
servicetags = [row.servicetag for row in query.all()]
db.session.remove()

results = provider.bulk_lookup(servicetags)   # slow, no connection held

for servicetag, result in results.items():    # new session, short write
    ...
db.session.commit()

Rules of thumb: no requests, urllib, subprocess or network-share access between a query and its commit(); cap every external call with a timeout; and if a sync is inherently long, chunk it so each chunk is its own read / call / write cycle.

Testing

Run tests with:

pytest tests/

Tests are required for:

• New routes (smoke + happy path + 1-2 edge cases)
• New plugin hooks (contract test)
• Schema migrations (before/after data integrity)

Running the gates on Windows

Three commands in this repository resolve to the wrong program on a Windows dev box. Each one fails in a way that looks like a code fault, which is why all three are written out here rather than left to be rediscovered.

& "C:\Program Files\Git\bin\bash.exe" scripts/check-naming-and-style.sh
cd frontend; npm.cmd test; npm.cmd run build; cd ..
python -m pytest tests/ -q

• bash resolves to WSL. C:\Windows\System32\bash.exe is on PATH by default. With no distribution installed it fails with execvpe(/bin/bash) failed: No such file or directory, and with one installed it cannot read the Windows path you handed it. Always give the Git Bash path in full. The same trap bit the test suite: tests/test_naming_gate_migrations.py used bare bash, and its 13 failures were miscategorised as "environmental" for weeks. It now resolves Git Bash explicitly and probes it before use.
• npm resolves to npm.ps1. PowerShell prefers the .ps1 over the .cmd, and the default execution policy is Restricted, so bare npm dies with "npm.ps1 cannot be loaded because running scripts is disabled on this system" before any test runs. Use npm.cmd. Nothing in the repo causes this and nothing in the repo can fix it; it is machine policy. If you would rather fix it once, Set-ExecutionPolicy -Scope CurrentUser RemoteSigned also works, but that is a machine-level change and npm.cmd is free.
• npm test, not npx vitest run. The pretest hook runs stage-frontend.mjs; bare vitest never stages plugin frontends and silently skips their specs.

Keeping the environment matched to the lockfiles

python scripts/check-dependency-drift.py

Editing a lockfile changes what a FUTURE install produces. It does not touch an already-populated site-packages. Commit a8f8580 raised eight pins carrying advisories; no install was re-run afterwards, so that workstation kept the old libraries for three weeks while requirements.txt said otherwise. Every local test run in that window exercised different libraries than a container build would have.

Run this after any pull that touches requirements.txt or requirements-dev.txt, and fix drift with:

python -m pip install --require-hashes -r requirements.txt
python -m pip install --require-hashes -r requirements-dev.txt

The check reports MISSING separately from MISMATCHED, because absent packages are the class that gets missed: pytest-xdist was simply not installed, which surfaced only as pytest: error: unrecognized arguments: -n - a message that reads as a broken pytest.ini and invites deleting -n auto instead of installing the plugin.

scripts/check-naming-and-style.sh runs this check too, advisory by default. CI sets DEPENDENCY_DRIFT_ENFORCE=1 to make it fail the build.

If you use the repo-local env/ virtualenv, check it specifically - it drifts independently of the global interpreter, and both were stale in the incident above:

env/Scripts/python.exe scripts/check-dependency-drift.py

Branches: what belongs on main, and what does not

main is the product every site runs and the publication mirror exports. <site> branches hold work that is not ready for other sites to see. There are two: wj here, and wilm on the corporate GitHub, where the other site develops the .NET endpoint agent.

A site branch is not a fork. Both feed main, and the sooner they do it the cheaper it is. A branch the dev box cannot see is still a branch: wilm is not visible from here because this machine has no GitHub account, which is why fetch-wilm-branch.cmd on the share exists to bring it back for review.

Most work is product. ADR-015 gives site differences three homes on main: a setting with a neutral default, seed data, or a site-namespaced directory (scripts/site_imports/<site>/, which the style gate exempts). That is the only site-specific home in the tree, so a site branch is about TIMING, not locality: things that are not ready yet, rather than things that are permanently ours.

Say which at commit time, with a trailer, because the decision does not survive being deferred - months later it is a list of subject lines and a guess:

git commit -m "Add the thing" -m "Scope: product"   # main should have it
git commit -m "Add the thing" -m "Scope: site"      # this site alone

Then:

tools/wj-status.sh                          # what is unpromoted, and where it belongs
tools/promote-to-main.sh --dry-run <sha>    # what files would reach main
tools/promote-to-main.sh <sha> [<sha> ...]  # oldest first; runs main's gates
tools/git-push.sh main                      # you push, deliberately

Windows equivalents are tools\git-push.cmd and tools\promote-to-main.cmd.

Promotion cherry-picks NAMED commits rather than merging the branch. Merging takes the site work along with the product work, which is how a site branch quietly becomes the product.

The failure to watch for is not what you think. Promoting something site-specific by accident is caught by the ADR-015 gate on the way into main. The real risk is product work sitting on a site branch until bringing it back is a port rather than a merge - one such round trip cost a MySQL dialect fix, a contract violation and a plugin install.

So rebase the site branch on main often, and promote as you go rather than in one batch at the end.

Code review checklist

• No underscores in table or column names
• No banned shorthand or unapproved acronyms in any new identifier
• DB-mirrored Python and JS variables match column names exactly
• API response keys match column names exactly
• No em-dashes, en-dashes, smart quotes, Unicode arrows, or emojis (run grep -rP '[^\x00-\x7F]' --include='*.py' --include='*.vue' --include='*.js' .)
• Plugin follows BasePlugin interface and contract version
• Tests included for new functionality