Ports 37 of Benjamin Baker's commits from the wilm branch, mirrored from the
corporate GitHub at 7905917c. wilm rides the publication lineage and shares no
ancestor with wj, so this is a filtered tree patch rather than a merge:
94 files, and every path the publication export strips (docs/, tools/, mcp/,
CLAUDE.md, start-*.sh, .gitea/, the three excluded tests) is held back, because
a raw diff would delete each of them from this tree.
What comes across:
* Endpoint telemetry, phase 2. Physical disk health from two sources, NIC link
state, boot type, Defender health, processor/memory/chassis inventory, the
listening TCP set, mapped network drives and Windows Update history. Five
additive migrations, 0012 through 0016, off the 0011 head.
* Mapped drives are collected by a probe running in the interactive user's
session, because they do not exist in the agent's session 0. Two Windows
sources can disagree about one drive and the worse status wins, so a
half-broken share is never rendered as healthy.
* Windows events are reported once each, keyed on the channel's own record id,
instead of being re-sent whenever a report overlapped the last one.
* PC Detail rework: Details cards flow, the hand-written Network card gives way
to a full address history, Storage and disk health merge into one card with
two sections, Operator Reports and Corporate Tools get their own tabs.
* An opt-in restic backup overlay with a third container, restore gated on the
dump's own data revision rather than the code head, and backup/restore history.
* The agent's version is read from the binary's PE version rather than trusted
from the csproj, and the Fleet view gains an Agent Version column.
* The enqueue route now asks caller_holds instead of the owner's permissions,
which closes the scoped-token elevation on run_script, update_app and
system_restart.
Held back deliberately:
* agent/installer/install-shopdbagent.ps1 stays deleted, as upstream deleted it.
The installer that reaches a bay is generated by _install_script() in
plugins/endpoints/api/siteagent.py; the standalone script had stopped being
shipped and was collecting fixes no endpoint ever ran.
* The .gitignore hunk adding docs/*.md. Correct on the publication lineage,
where docs/ becomes the wiki and its absence is how the gates detect a
published checkout. Here it would silently ignore every new doc.
* Their deletions of docs/DEV-BUILD-TEST.md and docs/PLUGIN-HOOKS.md. Upstream
those were stubs; here PLUGIN-HOOKS.md is the 31KB document CLAUDE.md points
readers at.
* Five zero-byte files committed by an accidental `git add -A`, a run log from
the author's own machine, and docs/notes/, which is working scratch that says
so in its own header and belongs in devnotes/ on this side.
Carried with the import so it lands green: the 17 new routes are described in
docs/api-inventory.json with the spec and project map regenerated, and two new
frontend specs read fixtures through fileURLToPath(URL) which node rejects under
jsdom's own URL global.
Suite 2591 passed / 9 skipped; frontend 391 passed; naming, style and doc gates
clean.
Scope: product
231 lines
11 KiB
Markdown
231 lines
11 KiB
Markdown
# 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
|
|
|
|
```python
|
|
# 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]
|
|
```
|
|
|
|
```python
|
|
# 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.
|
|
|
|
## Testing
|
|
|
|
Run tests with:
|
|
|
|
```bash
|
|
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)
|
|
|
|
## 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:
|
|
|
|
```bash
|
|
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:
|
|
|
|
```bash
|
|
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
|