Bug-Fix 31206 -Update ticket.class.php markAsRead function

[cbeasley-sbb]

Fix Issue #31206

Sets the object's "fk_statut" to Ticket::STATUS_READ. In the event of rollback reverts the object's "fk_statut" to its old value.

NOTE: I need this fix to be applied to v18.x if and when approved.

[lvessiller-opendsi]

Thanks for your remarks
Why not only set the status before db commit : (line 1735)

if (!$error) {
        $this->fk_statut = Ticket::STATUS_READ;
	$this->db->commit();
	return 1;
}

[cbeasley-sbb]

Thanks for your remarks Why not only set the status before db commit : (line 1735)

if (!$error) {
        $this->fk_statut = Ticket::STATUS_READ;
	$this->db->commit();
	return 1;
}

That could work. However, in other trigger situations the primary transaction for the event has already been executed but not committed. This way is more consistent with that pattern. With the approach you suggest $object->old_copy will be identical to the object that has supposedly changed during trigger function execution(s).

1. change executed
2. run_triggers called
3. Trigger functions evaluate the transition with custom logic; Presumably deltas can be identified by comparing $object->X to $object->old_copy->X
4. main function commits the changes if no errors.

[eldy]

A fix was pushed into v18. When merging to higher branch, upstream code was modified to use the ->oldcopy instead of $oldStatus var.
16ab86a

@lvessiller-opendsi "Why not only set the status before db commit : (line 1735)"
The set is not required just before the commit because we must do it (the set of values) at begin to have all the new value loaded into the object for the manipulation that will come later (update, trigger, ...)
So setting at end in case of commit should be useless, however, we must revert value in case of rollback using the old value defined into ->oldcopy.