FIX adding tms and datec to email templates #36189

JonBendtsen · 2025-11-09T11:10:02Z

FIX #32457 adding tms and datec to email templates

This PR adds tms and datec to email templates. It's added but not shown on creation and modify, where as it is shown when just listing email templates. The fields already existed in the database. A few more hurl tests are introduced to ensure that API post and create are not done with wrong fields.

This PR also adds id="" fields to the forms and the tables - so it easier to find them if you inspect the html.

This PR fixes #32457 but changing the Code column to Label which matches what you get in the json using the API

JonBendtsen · 2025-11-09T11:37:45Z

build STILL complains about non escaped string, but you clearly see that it is being escaped

1) CodingPhpTest::testPHP with data set #170 (array('mails_templates.php', '/home/travis/build/Dolibarr/d.../admin', 'admin', 'admin/mails_templates.php', '/home/travis/build/Dolibarr/d...es.php', '', '', 'file'))
Found non escaped string in building of a sql request (case 3) in admin/mails_templates.php: $sql .= '"'.$db->escap - Bad.
Failed asserting that false is true.

Why?

JonBendtsen · 2025-11-09T11:43:34Z

@hregis You commented on my other PR with changes to the database, so maybe you can help me here with the build system? Why does it complain about a non escaped string when it clearly says escape in the string it complains about?

hregis · 2025-11-09T12:04:11Z

@JonBendtsen don't use dol_print_date
Check another code to have an example

JonBendtsen · 2025-11-09T12:08:31Z

@JonBendtsen don't use dol_print_date Check another code to have an example

why not? it is a function in Dolibarr?

And I even escape the output?

What other code to check?

JonBendtsen · 2025-11-09T12:54:38Z

Change Code to label to match the database content

AND this also matches what is shown in the json from the API

{
  "module": "",
  "id": 59,
  "entity": 1,
  "contacts_ids_internal": null,
  "model_pdf": null,
  "date_creation": 1762687782,
  "tms": "2025-11-09 12:29:55",
  "totalpaid_multicurrency": null,
  "type_template": "all",
  "datec": 2025,
  "active": 1,
  "enabled": "1",
  "defaultfortype": 0,
  "label": "1229",
  "fk_user": 0,
  "private": 0,
  "topic": "1229 edit",
  "content": "1229 edit",
  "content_lines": "",
  "lang": "",
  "joinfiles": 0,
  "email_from": "",
  "email_to": "",
  "email_tocc": "",
  "email_tobcc": "",
  "position": 1
}

Fixes #29116

hregis · 2025-11-09T13:30:41Z

@JonBendtsen use this code, and remove the field "tms"

$now = dol_now();

.......

$sql .= "'".$db->idate(now)."'";

JonBendtsen · 2025-11-09T13:35:01Z

@JonBendtsen use this code, and remove the field "tms"

I replaced tms with Modif. Date something I find valuable to know when I edited the field

JonBendtsen · 2025-11-09T13:38:34Z

now using idate in almost all places @hregis

hregis · 2025-11-09T14:07:48Z

@JonBendtsen What I meant was that the SQL server should handle the timestamp of the "tms" field. The "tms" field is only used in SELECT statements.

JonBendtsen · 2025-11-09T14:12:33Z

@JonBendtsen What I meant was that the SQL server should handle the timestamp of the "tms" field. The "tms" field is only used in SELECT statements.

Okay, but didn't both you @hregis and @eldy state that you wanted to shift away from letting the database server handle timestamps since the database server might be in a different time zone?

hregis · 2025-11-09T14:17:55Z

@JonBendtsen For datec we use dol_now, but for tms we let the SQL server handle it.

hregis · 2025-11-09T14:18:27Z

@JonBendtsen i send a fix for the travis error : #36195

eldy · 2025-11-09T16:55:17Z

@JonBendtsen For datec we use dol_now, but for tms we let the SQL server handle it.

In a future we will have to manage the tms field in php code too. Just not the current priority.

eldy · 2025-11-09T17:01:33Z

@JonBendtsen For datec we use dol_now, but for tms we let the SQL server handle it.

In a future we will have to manage the tms field in php code too. Just not the current priority, so for the moment we must foll[own regis comment: datec using php and tms still using current timestamp (with in mind that it will be managed by php in future).

eldy · 2025-11-09T18:25:41Z

htdocs/admin/mails_templates.php

 $tabname = array();
 $tabname[25] = MAIN_DB_PREFIX."c_email_templates";

+// Used by the GUI during creation so we don't ask the user to fill in tms and datec


I don't think we need to introduce another table.
We know that tms and datec are date update and creation and should not be requested.
We can just add a hard coded if on this 2 fields when we forge the form to not show them.

@eldy requested changes has been made, it builds cleanup and test works

…o make the build system happy?

JonBendtsen · 2025-11-09T19:21:06Z

adding

After adding

JonBendtsen · 2025-11-09T19:22:16Z

editing

after edit

hregis · 2025-11-09T21:11:22Z

@JonBendtsen

You're told not to modify TMS, and you modify TMS anyway!
How do you have to tell you?

JonBendtsen · 2025-11-09T21:28:27Z

@JonBendtsen

You're told not to modify TMS, and you modify TMS anyway! How do you have to tell you?

@hregis Please comment in the code the line you mean - that is much easier to find the line you think is erroneous

JonBendtsen · 2025-11-09T21:42:02Z

htdocs/admin/mails_templates.php

-				if (GETPOST($keycode) == '' && $keycode != 'langcode') {
+				if ($keycode == 'datec') {
+					$sql .= "'".$db->idate($now)."'";
+				} elseif ($keycode == 'tms') {


@hregis you probably mean this line nr 439 - 441

This is during

if ($ok && GETPOST('actionadd')) {

in line 396, meaning this is during INSERT, and the configuration of the tms field is

So it only uses CURRENT_TIMESTAMP() during update, not during INSERT. So if I remove the tms line, then it does not get a value in tms, and I'd argue that creation of an object is a modification of the current state.

hregis · 2025-11-10T07:35:13Z

@JonBendtsen remove "datec" from the update function, datec is never update

hregis · 2025-11-10T07:37:47Z

@JonBendtsen

1 remove tms
2 use datec only in create and select

JonBendtsen · 2025-11-10T18:16:58Z

@JonBendtsen

1 remove tms 2 use datec only in create and select

This is the new INSERT line from dolibarr.log - no tms

sql=INSERT INTO llx_c_email_templates (label,lang,type_template,fk_user,private,position,topic,email_from,joinfiles,defaultfortype,content,datec,entity, active, enabled) VALUES('001912', '', 'all', null, 0, 1, '001912', null, 0, 0, '001912', '2025-11-10 19:13:07', 1, 1, 1)

But, the database still has a value in the tms column?

Is that the expected?

…atec

JonBendtsen · 2025-11-10T18:30:02Z

latest hurl tests


JonSweet16:hurl jonbendtsen$ ./run.sh 
----- Run hurl test on APIs ---
First we run tests that do not require authentication
Success gui/00_HOME.hurl (1 request(s) in 44 ms)
Success api/00_foobar.hurl (1 request(s) in 49 ms)
Success api/00_explorer.hurl (1 request(s) in 53 ms)
Success api/status/00_status.hurl (1 request(s) in 61 ms)
Success public/payment/00_payment_newpayment.hurl (1 request(s) in 73 ms)
Success public/onlinesign/00_onlinesign_newonlinesign.hurl (1 request(s) in 75 ms)
Success api/login/00_login_POST.hurl (1 request(s) in 1065 ms)
Success api/login/00_login_GET.hurl (1 request(s) in 1066 ms)
--------------------------------------------------------------------------------
Executed files:    8
Executed requests: 8 (7.5/s)
Succeeded files:   8 (100.0%)
Failed files:      0 (0.0%)
Duration:          1067 ms

Now we are ready to run API tests that do require authentication
Success api/status/10_status.hurl (1 request(s) in 84 ms)
Success api/setup/10_setup_company.hurl (1 request(s) in 483 ms)
Success api/setup/10_setup_conf.hurl (1 request(s) in 490 ms)
Success api/users/10_groups.hurl (15 request(s) in 3059 ms)
Success api/mailings/10_mailings.hurl (34 request(s) in 3138 ms)
Success api/users/10_users.hurl (17 request(s) in 3354 ms)
Success api/emailtemplates/10_emailtemplates.hurl (23 request(s) in 3659 ms)
Success api/contracts/10_contracts.hurl (22 request(s) in 3915 ms)
Success api/warehouses/10_warehouses.hurl (34 request(s) in 4543 ms)
Success api/setup/10_setup_modules.hurl (14 request(s) in 5168 ms)
--------------------------------------------------------------------------------
Executed files:    10
Executed requests: 162 (31.3/s)
Succeeded files:   10 (100.0%)
Failed files:      0 (0.0%)
Duration:          5172 ms

JonBendtsen force-pushed the adding_tms_datec_to_email_templates branch from 0bfbe2f to cc12caf Compare November 9, 2025 11:59

eldy reviewed Nov 9, 2025

View reviewed changes

eldy added the Discussion Some questions or discussions are opened and wait answers of author or other people to be processed label Nov 9, 2025

Jon Bendtsen added 7 commits November 9, 2025 19:42

NEW: adding tms and datec to email templates

d25d55a

Escaping a timestamp string I just generated myself

725a6b3

switching single and double quotes in escape line for tms and datec t…

cbbc1a4

…o make the build system happy?

Add an id to the table

9481fbf

showing better names than tms and datec

5bd5c0e

Changing code to label to fix Dolibarr#29116

19644bc

using hregis advice to use idate not escape the string with date

52b1215

JonBendtsen force-pushed the adding_tms_datec_to_email_templates branch from f725f0f to 52b1215 Compare November 9, 2025 18:42

Jon Bendtsen added 2 commits November 9, 2025 20:16

requested changes

d5db220

forgot to add my name to editors

cc0fdcc

had to make class changes for API to work

14e8ddd

setting datec back to int|string

f578a74

JonBendtsen marked this pull request as ready for review November 9, 2025 21:04

JonBendtsen commented Nov 9, 2025

View reviewed changes

removing unused code lines and using idate in the api file

cc46b22

Jon Bendtsen added 2 commits November 10, 2025 19:17

no tms in create or update, but select. No datec in update

e9b1e25

expanding hurl tests to prevent post with id or tms, put with id or d…

7b8b648

…atec

Uh oh!

FIX adding tms and datec to email templates #36189

Are you sure you want to change the base?

FIX adding tms and datec to email templates #36189

Conversation

JonBendtsen commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

FIX #32457 adding tms and datec to email templates

Uh oh!

JonBendtsen commented Nov 9, 2025

Uh oh!

JonBendtsen commented Nov 9, 2025

Uh oh!

hregis commented Nov 9, 2025

Uh oh!

JonBendtsen commented Nov 9, 2025

Uh oh!

JonBendtsen commented Nov 9, 2025

Uh oh!

hregis commented Nov 9, 2025

Uh oh!

JonBendtsen commented Nov 9, 2025

Uh oh!

JonBendtsen commented Nov 9, 2025

Uh oh!

hregis commented Nov 9, 2025

Uh oh!

JonBendtsen commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hregis commented Nov 9, 2025

Uh oh!

hregis commented Nov 9, 2025

Uh oh!

eldy commented Nov 9, 2025

Uh oh!

eldy commented Nov 9, 2025

Uh oh!

eldy Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

JonBendtsen Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

JonBendtsen commented Nov 9, 2025

adding

After adding

Uh oh!

JonBendtsen commented Nov 9, 2025

editing

after edit

Uh oh!

hregis commented Nov 9, 2025

Uh oh!

JonBendtsen commented Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonBendtsen Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

hregis commented Nov 10, 2025

Uh oh!

hregis commented Nov 10, 2025

Uh oh!

JonBendtsen commented Nov 10, 2025

Uh oh!

JonBendtsen commented Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JonBendtsen commented Nov 9, 2025 •

edited

Loading

JonBendtsen commented Nov 9, 2025 •

edited

Loading

JonBendtsen commented Nov 9, 2025 •

edited

Loading