Files
shopdb-flask/CONTRIBUTING.md
cproudlock bfc30f4c7c
Some checks failed
CI / naming (push) Successful in 3s
CI / frontend (push) Successful in 1m37s
CI / migrations-mysql (push) Successful in 42s
CI / backend (push) Failing after 4m49s
docs: branch model documents one branch per instance
CONTRIBUTING.md still said "main is the product every site runs and the
publication mirror exports" and framed wj/wilm as site branches feeding it.
That was true when main was the only thing anyone deployed. It is not now:
WJ production runs wj, the /ops instance runs dev, and main deploys nowhere.
Nothing in the repository mentioned dev at all, so the branch a deploy script
refuses to run without was undocumented.

Replaces that section with the branch/instance table and the dev -> wj -> main
promotion order, and says why promoting before validating is the wrong way
round. Keeps the wilm note but moves it up with the reason it cannot be
merged locally: it sits on the publication lineage and shares no ancestor.

CLAUDE.md gains the branch-to-server pointer and records that
export-github.sh now builds a dist only for wj and dev.

docs/DEPLOY.md is deliberately left alone: `git pull origin main` is correct
there. That document is the generic connected-Docker deploy another site
follows, and main is exactly what another site adopts.

Scope: product
2026-09-03 18:52:55 -04:00

248 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 curated product other sites adopt, and what the publication
mirror exports. It is a release decision: something reaching `main` says other
sites should take it. **`main` is not what this site runs.**
Each branch answers to one instance:
| branch | runs on | contains |
|---|---|---|
| `dev` | the dev instance | work being validated |
| `wj` | this site's PRODUCTION | everything this site runs |
| `main` | nothing directly | the curated release for other sites |
| `wilm` | the other site | their work, on the corporate GitHub |
So work flows `dev` -> `wj` -> `main`: land it on `dev`, validate it on the dev
instance, take it to `wj` for production, and promote to `main` what you are
happy for another site to run. Promoting first and back-filling the site branch
is the wrong way round - it puts unvalidated work in front of other sites and
leaves production behind.
`wilm` is not visible from the dev box because that machine has no account on
the corporate GitHub, which is why `fetch-wilm-branch.cmd` exists to bring it
back for review. It sits on the publication lineage, so it shares no ancestor
with `main`/`wj`/`dev` and cannot be merged locally.
A site branch is not a fork. Both feed `main`, and the sooner they do it the
cheaper it is.
**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