Skip to content

[3.0] Stops drafts from silently discarding every change after the first save - #9339

Merged
jdarwood007 merged 2 commits into
SimpleMachines:release-3.0from
albertlast:3.0/draft-poster-time
Aug 2, 2026
Merged

[3.0] Stops drafts from silently discarding every change after the first save#9339
jdarwood007 merged 2 commits into
SimpleMachines:release-3.0from
albertlast:3.0/draft-poster-time

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Description

Editing an existing draft never saves. The first save creates the row, and every save after that is thrown away — no error, no message, and Draft::save() still reports success.

The cause is a copy-paste slip in Draft::__construct(), from 5520b18 ("Ensure values have the right type when loading drafts"):

// These have to be ints
case 'type':
case 'poster_time':
	$this->type = (int) $value;
	break;

read() does SELECT *, so poster_time arrives right after type and overwrites it. Every loaded draft therefore ends up with a unix timestamp in $this->type, and $this->poster_time stays 0.

The next save writes that timestamp straight back into user_drafts.type, which the schema declares as a tinyint. On PostgreSQL that is a smallint:

smf=# UPDATE smf_user_drafts SET type = 1785615651, body = 'changed' WHERE id_draft = 5;
ERROR:  smallint out of range

The whole UPDATE fails, so the draft keeps its old body. saveToDatabase() sets Utils::$context['draft_saved'] = true before looking at anything, so save() returns true anyway, and the PostgreSQL layer does not raise on a failed query, so nothing reaches the error log either. MySQL does not fail the statement in the same way, but it stores a nonsense type and the draft stops matching the AND type = {int:type} filter in read(), so it disappears from the editor.

Two knock-on effects go with it, and both are fixed by the same line:

  • the five second autosave throttle in save() tests $this->poster_time, which was always 0, so it never fired;
  • $this->type never equalled 1 for a PM draft.

That second one needs the follow-up commit. saveToDatabase() returns the draft ID in $context['id' . ($this->type === 1 ? '_pm' : '') . '_draft'], which is how 2.1 named it. Nothing in 3.0 reads id_pm_draftPM::compose2(), DraftPM::prepare() and the hidden field Editor.php builds all use id_draft for both kinds of draft. The broken type was the only reason that expression never took the _pm branch, so fixing the first bug on its own would have left PM drafts with id_draft=0 in the form and created a new draft on every save. The dead key is dropped.

Testing

Verified against the Docker environment on PostgreSQL 17, before and after, in the same session:

before after
post draft, second save row unchanged, nothing logged body and poster_time updated, type still 0
PM draft, first save row created, form comes back with id_draft=7
PM draft, second save draft 7 updated in place, type still 1, no duplicate row

vendor/bin/phpunit — 108 tests, 157 assertions, OK.

Not covered here: the PostgreSQL API's query() swallows failures entirely (@pg_query() with no error path), which is why this was invisible for a year and a half, and saveToDatabase() reports success without checking anything. Both are worth their own look; neither is needed to fix this.

Found while splitting #7933, but this is server-side only and touches no theme file — it applies equally to the current default theme and the new one.

Issues References (Fixes|Related|Closes)

Related to #7933, #9337

Loading an existing draft assigned poster_time to $this->type, so the type
of every loaded draft became a unix timestamp and poster_time stayed 0.

The next save then wrote that timestamp back into user_drafts.type, which
is a tinyint. On PostgreSQL that is a smallint, so the whole UPDATE fails
with "smallint out of range": the draft keeps its old body, and because
saveToDatabase() sets $context['draft_saved'] before looking at anything,
save() still reports success. Autosaving an existing draft therefore threw
the changes away without a word.

It also meant $this->type never equalled 1 for a loaded PM draft, so the
autosave response filled in $context['id_draft'] instead of
$context['id_pm_draft'], and the five second autosave throttle in save()
never fired, since it tests a poster_time that was always 0.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
saveToDatabase() returned the new draft ID in $context['id_pm_draft'] for PM
drafts, which is how 2.1 named it. Everything in 3.0 - PM::compose2(),
DraftPM::prepare(), the hidden field Editor.php builds - uses id_draft for
both kinds of draft, so that value went nowhere.

It only stayed unnoticed because $this->type never actually held 1: the
previous commit was what made the PM branch of that expression reachable.
Without this, saving a new PM draft would leave id_draft at 0 in the form
and the next save would create a second draft instead of updating the
first.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@jdarwood007 jdarwood007 added this to the 3.0 Alpha 5 milestone Aug 2, 2026
@jdarwood007
jdarwood007 merged commit fcbc3cb into SimpleMachines:release-3.0 Aug 2, 2026
4 checks passed
@albertlast
albertlast deleted the 3.0/draft-poster-time branch August 2, 2026 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants