Store and serve a maintenance date as a date
The machine maintenance panel showed every date one calendar day early on this
site, and would show it a day late east of UTC.
lastmaintenancedate and nextmaintenancedate were DATETIME columns holding
midnight, and BaseModel.to_dict serialises a datetime as isoformat + 'Z', so
2026-12-15 00:00:00 went out as "2026-12-15T00:00:00Z" and the browser
converted it into local time. There is no time in a maintenance date: the form
offers a date picker, the CSV importer takes '2026-06-01', and the value is only
ever compared against other dates. Every sibling already models it correctly -
measuringtools.lastcalibrationdate, warranties.startdate and enddate are all
db.Date - so machines were the odd one out.
Migration 7d40 converts both columns; MySQL truncates a midnight time so no
data moves, and SQLite neither enforces nor needs it.
That exposed two more things. The route was assigning the raw request string to
the column, which a DATE column rejects outright - it now goes through the
strict parser from wave 2, so a mistyped maintenance date is refused with a 400
rather than stored or nulled. And BaseModel.to_dict had no branch for a plain
date, so one reached Flask's encoder and came out as an HTTP-date
("Thu, 15 Jan 2026 00:00:00 GMT") - which a browser parses back into a moment
and can still render as the day before. Dates now serialise as "2026-01-15",
with datetime checked first because it is a subclass of date.
Verified against MySQL on a live instance: the column is DATE, the API serves
2026-01-15, a mistyped date is refused with 400, and a rename on a machine that
has maintenance dates saves - the 500 from the audit-log fix in 5d81f50c, now
confirmed end to end rather than only in tests.
Scope: product
This commit is contained in:
@@ -0,0 +1,73 @@
|
|||||||
|
"""Store a maintenance date as a DATE, because that is what it is.
|
||||||
|
|
||||||
|
lastmaintenancedate and nextmaintenancedate were DATETIME columns holding
|
||||||
|
midnight. BaseModel.to_dict serialises a datetime as isoformat + 'Z', so
|
||||||
|
2026-12-15 00:00:00 went out as "2026-12-15T00:00:00Z" and a browser west of
|
||||||
|
UTC rendered it as 14 December. The maintenance panel showed every date one
|
||||||
|
calendar day early on this site, and would show it a day LATE east of UTC.
|
||||||
|
|
||||||
|
There is no time in a maintenance date. Nobody records that a machine was
|
||||||
|
serviced at midnight; the form offers a date picker, the CSV importer takes
|
||||||
|
'2026-06-01', and the value is compared against other dates. Every sibling
|
||||||
|
already models it correctly - measuringtools.lastcalibrationdate,
|
||||||
|
warranties.startdate and enddate are all db.Date - so this is machines being
|
||||||
|
the odd one out rather than a new decision.
|
||||||
|
|
||||||
|
As a Date, to_dict emits "2026-12-15" with no timezone marker at all, and
|
||||||
|
nothing can shift it.
|
||||||
|
|
||||||
|
MySQL truncates the (midnight) time component on the ALTER, so no data moves.
|
||||||
|
SQLite stores what it is given and does not enforce column types, so the
|
||||||
|
conversion is a no-op there and is skipped.
|
||||||
|
|
||||||
|
Revision ID: 7d40_machine_maintenance_dates
|
||||||
|
Revises: 7d39_audit_retention_key_merge
|
||||||
|
"""
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
|
||||||
|
revision = '7d40_machine_maintenance_dates'
|
||||||
|
down_revision = '7d39_audit_retention_key_merge'
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
COLUMNS = ('lastmaintenancedate', 'nextmaintenancedate')
|
||||||
|
|
||||||
|
|
||||||
|
def _machines_table(bind):
|
||||||
|
inspector = sa.inspect(bind)
|
||||||
|
if 'machines' not in inspector.get_table_names():
|
||||||
|
return None
|
||||||
|
return {column['name'] for column in inspector.get_columns('machines')}
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade():
|
||||||
|
bind = op.get_bind()
|
||||||
|
if bind.dialect.name == 'sqlite':
|
||||||
|
return
|
||||||
|
present = _machines_table(bind)
|
||||||
|
if not present:
|
||||||
|
return
|
||||||
|
for column in COLUMNS:
|
||||||
|
if column in present:
|
||||||
|
op.alter_column('machines', column,
|
||||||
|
existing_type=sa.DateTime(),
|
||||||
|
type_=sa.Date(),
|
||||||
|
existing_nullable=True)
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade():
|
||||||
|
bind = op.get_bind()
|
||||||
|
if bind.dialect.name == 'sqlite':
|
||||||
|
return
|
||||||
|
present = _machines_table(bind)
|
||||||
|
if not present:
|
||||||
|
return
|
||||||
|
for column in COLUMNS:
|
||||||
|
if column in present:
|
||||||
|
op.alter_column('machines', column,
|
||||||
|
existing_type=sa.Date(),
|
||||||
|
type_=sa.DateTime(),
|
||||||
|
existing_nullable=True)
|
||||||
@@ -7,7 +7,8 @@ from shopdb.api import db, Asset, AssetType, AuditLog, Vendor, success_response,
|
|||||||
|
|
||||||
from ..models import Machine, MachineType
|
from ..models import Machine, MachineType
|
||||||
|
|
||||||
from shopdb.api import require_permission, apply_import_timestamps, intarg
|
from shopdb.api import (require_permission, apply_import_timestamps, intarg,
|
||||||
|
DateValueError, parse_date_fields)
|
||||||
|
|
||||||
machines_bp = Blueprint('machines', __name__)
|
machines_bp = Blueprint('machines', __name__)
|
||||||
|
|
||||||
@@ -336,6 +337,14 @@ def create_machine():
|
|||||||
- mapx, mapy, notes
|
- mapx, mapy, notes
|
||||||
"""
|
"""
|
||||||
data = request.get_json()
|
data = request.get_json()
|
||||||
|
# Refused rather than stored raw or nulled: these are db.Date columns
|
||||||
|
# (migration 7d40), so a string would be rejected by the engine and a
|
||||||
|
# mistyped one used to become NULL further down the stack.
|
||||||
|
try:
|
||||||
|
maintenancedates = parse_date_fields(
|
||||||
|
data, ('lastmaintenancedate', 'nextmaintenancedate'))
|
||||||
|
except DateValueError as bad:
|
||||||
|
return error_response(ErrorCodes.VALIDATION_ERROR, str(bad))
|
||||||
|
|
||||||
if not data:
|
if not data:
|
||||||
return error_response(ErrorCodes.VALIDATION_ERROR, 'No data provided')
|
return error_response(ErrorCodes.VALIDATION_ERROR, 'No data provided')
|
||||||
@@ -388,8 +397,8 @@ def create_machine():
|
|||||||
modelnumberid=data.get('modelnumberid'),
|
modelnumberid=data.get('modelnumberid'),
|
||||||
requiresmanualconfig=data.get('requiresmanualconfig', False),
|
requiresmanualconfig=data.get('requiresmanualconfig', False),
|
||||||
islocationonly=data.get('islocationonly', False),
|
islocationonly=data.get('islocationonly', False),
|
||||||
lastmaintenancedate=data.get('lastmaintenancedate'),
|
lastmaintenancedate=maintenancedates.get('lastmaintenancedate'),
|
||||||
nextmaintenancedate=data.get('nextmaintenancedate'),
|
nextmaintenancedate=maintenancedates.get('nextmaintenancedate'),
|
||||||
maintenanceintervaldays=data.get('maintenanceintervaldays'),
|
maintenanceintervaldays=data.get('maintenanceintervaldays'),
|
||||||
controllervendorid=data.get('controllervendorid'),
|
controllervendorid=data.get('controllervendorid'),
|
||||||
controllermodelid=data.get('controllermodelid')
|
controllermodelid=data.get('controllermodelid')
|
||||||
@@ -427,6 +436,14 @@ def update_machine(machine_id: int):
|
|||||||
)
|
)
|
||||||
|
|
||||||
data = request.get_json()
|
data = request.get_json()
|
||||||
|
# Refused rather than stored raw or nulled: these are db.Date columns
|
||||||
|
# (migration 7d40), so a string would be rejected by the engine and a
|
||||||
|
# mistyped one used to become NULL further down the stack.
|
||||||
|
try:
|
||||||
|
maintenancedates = parse_date_fields(
|
||||||
|
data, ('lastmaintenancedate', 'nextmaintenancedate'))
|
||||||
|
except DateValueError as bad:
|
||||||
|
return error_response(ErrorCodes.VALIDATION_ERROR, str(bad))
|
||||||
if not data:
|
if not data:
|
||||||
return error_response(ErrorCodes.VALIDATION_ERROR, 'No data provided')
|
return error_response(ErrorCodes.VALIDATION_ERROR, 'No data provided')
|
||||||
|
|
||||||
@@ -471,7 +488,7 @@ def update_machine(machine_id: int):
|
|||||||
# Update machine fields
|
# Update machine fields
|
||||||
machine_fields = ['machinetypeid', 'vendorid', 'modelnumberid',
|
machine_fields = ['machinetypeid', 'vendorid', 'modelnumberid',
|
||||||
'requiresmanualconfig', 'islocationonly',
|
'requiresmanualconfig', 'islocationonly',
|
||||||
'lastmaintenancedate', 'nextmaintenancedate', 'maintenanceintervaldays',
|
'maintenanceintervaldays',
|
||||||
'controllervendorid', 'controllermodelid']
|
'controllervendorid', 'controllermodelid']
|
||||||
for key in machine_fields:
|
for key in machine_fields:
|
||||||
if key in data:
|
if key in data:
|
||||||
@@ -481,6 +498,14 @@ def update_machine(machine_id: int):
|
|||||||
changes[key] = {'old': old_val, 'new': new_val}
|
changes[key] = {'old': old_val, 'new': new_val}
|
||||||
setattr(mach, key, data[key])
|
setattr(mach, key, data[key])
|
||||||
|
|
||||||
|
# The maintenance dates are real dates now (migration 7d40), so they are
|
||||||
|
# parsed rather than assigned as whatever string arrived - and a mistyped
|
||||||
|
# one is refused instead of stored or nulled, like every other date.
|
||||||
|
for key, parsed in maintenancedates.items():
|
||||||
|
if getattr(mach, key) != parsed:
|
||||||
|
changes[key] = {'old': getattr(mach, key), 'new': parsed}
|
||||||
|
setattr(mach, key, parsed)
|
||||||
|
|
||||||
# Audit log if there were changes
|
# Audit log if there were changes
|
||||||
if changes:
|
if changes:
|
||||||
AuditLog.log('updated', 'Machine', entityid=mach.machineid,
|
AuditLog.log('updated', 'Machine', entityid=mach.machineid,
|
||||||
|
|||||||
@@ -73,8 +73,12 @@ class Machine(BaseModel):
|
|||||||
)
|
)
|
||||||
|
|
||||||
# Maintenance tracking
|
# Maintenance tracking
|
||||||
lastmaintenancedate = db.Column(db.DateTime, nullable=True)
|
# Dates, not datetimes: there is no time in a maintenance date, and a
|
||||||
nextmaintenancedate = db.Column(db.DateTime, nullable=True)
|
# DATETIME went out of to_dict as "...T00:00:00Z", which a browser west of
|
||||||
|
# UTC rendered as the previous day. measuringtools and warranties already
|
||||||
|
# model theirs as db.Date. See migration 7d40.
|
||||||
|
lastmaintenancedate = db.Column(db.Date, nullable=True)
|
||||||
|
nextmaintenancedate = db.Column(db.Date, nullable=True)
|
||||||
maintenanceintervaldays = db.Column(db.Integer, nullable=True)
|
maintenanceintervaldays = db.Column(db.Integer, nullable=True)
|
||||||
|
|
||||||
# Controller info (for CNC machines)
|
# Controller info (for CNC machines)
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
"""Base model class with common fields."""
|
"""Base model class with common fields."""
|
||||||
|
|
||||||
from datetime import datetime, timezone
|
from datetime import date, datetime, timezone
|
||||||
from shopdb.extensions import db
|
from shopdb.extensions import db
|
||||||
|
|
||||||
|
|
||||||
@@ -63,6 +63,13 @@ class BaseModel(db.Model):
|
|||||||
value = getattr(self, c.name)
|
value = getattr(self, c.name)
|
||||||
if isinstance(value, datetime):
|
if isinstance(value, datetime):
|
||||||
value = value.isoformat() + 'Z'
|
value = value.isoformat() + 'Z'
|
||||||
|
elif isinstance(value, date):
|
||||||
|
# A date, not an instant: no time and no timezone marker. Left
|
||||||
|
# to Flask's encoder it became an HTTP-date
|
||||||
|
# ("Thu, 15 Jan 2026 00:00:00 GMT"), which a browser parses
|
||||||
|
# back into a moment and can render as the day before.
|
||||||
|
# datetime is a subclass of date, so this must stay second.
|
||||||
|
value = value.isoformat()
|
||||||
result[c.name] = value
|
result[c.name] = value
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
|||||||
@@ -238,3 +238,39 @@ def test_the_status_breakdown_counts_both_bays_when_collapse_is_off(
|
|||||||
headers=auth_headers).get_json()['data']
|
headers=auth_headers).get_json()['data']
|
||||||
assert summary['total'] == 2
|
assert summary['total'] == 2
|
||||||
assert sum(row['count'] for row in summary['bystatus']) == 2
|
assert sum(row['count'] for row in summary['bystatus']) == 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_maintenance_date_is_served_as_a_plain_date(client, db, auth_headers):
|
||||||
|
"""It was a DATETIME holding midnight, serialised as '...T00:00:00Z', so a
|
||||||
|
browser west of UTC rendered every maintenance date one day early. A date
|
||||||
|
has no time and no timezone."""
|
||||||
|
from plugins.machines.models import Machine
|
||||||
|
|
||||||
|
# The create route resolves the 'machine' AssetType, which this module's
|
||||||
|
# fixture only seeds as part of building a pair.
|
||||||
|
if not AssetType.query.filter_by(assettype='machine').first():
|
||||||
|
_db.session.add(AssetType(assettype='machine'))
|
||||||
|
_db.session.commit()
|
||||||
|
|
||||||
|
created = client.post('/api/machines', headers=auth_headers, json={
|
||||||
|
'assetnumber': 'MAINT-DATE-1',
|
||||||
|
'name': 'Maintenance date probe',
|
||||||
|
'lastmaintenancedate': '2026-01-15',
|
||||||
|
'nextmaintenancedate': '2026-12-15',
|
||||||
|
})
|
||||||
|
assert created.status_code == 201, created.get_json()
|
||||||
|
payload = created.get_json()['data']
|
||||||
|
machineid = payload.get('machineid') or payload['machine']['machineid']
|
||||||
|
|
||||||
|
fetched = client.get('/api/machines/%d' % machineid,
|
||||||
|
headers=auth_headers).get_json()['data']
|
||||||
|
extension = fetched['machine']
|
||||||
|
assert extension['lastmaintenancedate'] == '2026-01-15'
|
||||||
|
assert extension['nextmaintenancedate'] == '2026-12-15'
|
||||||
|
for value in (extension['lastmaintenancedate'],
|
||||||
|
extension['nextmaintenancedate']):
|
||||||
|
assert 'T' not in value and 'Z' not in value, \
|
||||||
|
'a date must not carry a time or a timezone marker'
|
||||||
|
|
||||||
|
stored = db.session.get(Machine, machineid)
|
||||||
|
assert str(stored.lastmaintenancedate) == '2026-01-15'
|
||||||
|
|||||||
Reference in New Issue
Block a user