NEW Link manage extrafields and phpunit test for the class (#39514)

* NEW: Add extrafields support to Link class

Link now declares isextrafieldmanaged and wires fetch_optionals()/
insertExtraFields()/deleteExtraFields() into fetch(), fetchAll(),
create(), update() and delete(), matching the llx_links_extrafields
table added previously.

* FIX: Several bugs in Link class review

- create(): wrong duplicate-record error message (copy-pasted from
  Societe, referenced undefined $this->name) replaced by the generic
  ErrorDuplicateField, consistent with update().
- create()/update(): missing "NoURL" translation key replaced by the
  standard ErrorFieldRequired pattern.
- fetch(): guard against running an unfiltered query (no rowid, no
  hashforshare) that could silently return an arbitrary link; a caller
  in actions_linkedfiles.inc.php could hit this when 'linkid' was
  missing from the request.
- actions_linkedfiles.inc.php: check fetch() result with `> 0` instead
  of a truthy test, since -1 (error) is truthy in PHP.
- delete(): add User type hint (consistent with create()/update()) and
  a $notrigger parameter to optionally skip the LINK_DELETE trigger.
- update(): fix copy-pasted docblock ("third party" -> "link").

* NEW: Add PHPUnit test for Link class

Covers create/fetch/update/fetchAll/count/delete, plus regression
tests for the two bugs fixed in the previous commit: create() rejects
an empty url, and fetch() rejects a call with neither rowid nor
hashforshare instead of returning an arbitrary record.

* FIX Phan false positive on $object in actions_linkedfiles.inc.php

Phan's ambient type inference for the loosely-typed global $object in
this shared include file was picking up CommonSocialNetworks (an
unrelated trait, not even a class), reported as undeclared
->id/->entity/->addThumbs()/->delThumbs() in a real CI Phan run on
this branch, once this file was analyzed on its own via the
changed-files file-list (this file has no prior baseline entry, so it
was apparently never previously exercised in isolation like this).

Force $object's type explicitly to CommonObject via the same
@phan-var-force string-literal idiom this file already uses for
$upload_dir/$upload_dirold/$confirm/$forceFullTextIndexation -
CommonObject genuinely declares addThumbs()/delThumbs(), which
resolves those two errors outright. $id/$entity remain reported as
PhanUndeclaredProperty (CommonObject itself does not declare them,
only its concrete subclasses do at runtime) - baseline-suppress that
for this file the same way it is already suppressed for the sibling
shared-include files actions_addupdatedelete.inc.php,
actions_massactions.inc.php and actions_sendmails.inc.php, which have
the exact same $object typing situation.

dev/tools/phan/baseline.txt

@@ -47,6 +47,7 @@ return [
         'htdocs/compta/tva/class/paymentvat.class.php' => ['PhanUndeclaredProperty'],
         'htdocs/compta/tva/clients.php' => ['PhanTypeArraySuspiciousNull', 'PhanTypeInvalidDimOffset'],
         'htdocs/core/actions_addupdatedelete.inc.php' => ['PhanUndeclaredProperty'],
+        'htdocs/core/actions_linkedfiles.inc.php' => ['PhanUndeclaredProperty'],
         'htdocs/core/actions_massactions.inc.php' => ['PhanUndeclaredProperty'],
         'htdocs/core/actions_sendmails.inc.php' => ['PhanUndeclaredProperty'],
         'htdocs/core/ajax/ajaxdirtree.php' => ['PhanTypeMismatchArgument'],

htdocs/core/actions_linkedfiles.inc.php

@@ -37,6 +37,7 @@
  * @var	string $forceFullTextIndexation
  */
 '
+@phan-var-force CommonObject $object
 @phan-var-force string $upload_dir
 @phan-var-force string $upload_dirold
 @phan-var-force string $confirm
@@ -224,7 +225,7 @@ if ($action == 'confirm_deletefile' && $confirm == 'yes' && !empty($permissionto
 
 	$link = new Link($db);
 	$f = $link->fetch(GETPOSTINT('linkid'));
-	if ($f) {
+	if ($f > 0) {
 		$link->url = GETPOST('link', 'alpha');
 		if (substr($link->url, 0, 7) != 'http://'
 			&& substr($link->url, 0, 8) != 'https://'

htdocs/core/class/link.class.php

@@ -1,7 +1,7 @@
 <?php
 /* Copyright (C) 2013       Cédric Salvador         <csalvador@gpcsolutions.fr>
  * Copyright (C) 2024-2026	MDW						<mdeweerd@users.noreply.github.com>
- * Copyright (C) 2024       Frédéric France         <frederic.france@free.fr>
+ * Copyright (C) 2024-2026  Frédéric France         <frederic.france@free.fr>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -40,6 +40,11 @@ class Link extends CommonObject
 	 */
 	public $table_element = 'links';
 
+	/**
+	 * @var int<0,1> Does object support extrafields ? 0=No, 1=Yes
+	 */
+	public $isextrafieldmanaged = 1;
+
 	/**
 	 * @var int Entity
 	 */
@@ -102,7 +107,7 @@ class Link extends CommonObject
 		global $langs, $conf;
 
 		$error = 0;
-		$langs->load("errors");
+		$langs->loadLangs(array("errors", "admin"));
 		// Clean parameters
 		if (empty($this->label)) {
 			$this->label = trim(basename($this->url));
@@ -116,7 +121,7 @@ class Link extends CommonObject
 
 		// Check parameters
 		if (empty($this->url)) {
-			$this->error = $langs->trans("NoURL");
+			$this->error = $langs->trans("ErrorFieldRequired", $langs->transnoentitiesnoconv("URL"));
 			return -1;
 		}
 
@@ -137,12 +142,20 @@ class Link extends CommonObject
 			$this->id = $this->db->last_insert_id($this->db->prefix()."links");
 
 			if ($this->id > 0) {
+				// Actions on extra fields
+				$result = $this->insertExtraFields();
+				if ($result < 0) {
+					$error++;
+				}
+
+				if (!$error) {
 					// Call trigger
 					$result = $this->call_trigger('LINK_CREATE', $user);
 					if ($result < 0) {
 						$error++;
 					}
 					// End call triggers
+				}
 			} else {
 				$error++;
 			}
@@ -158,7 +171,7 @@ class Link extends CommonObject
 			}
 		} else {
 			if ($this->db->errno() == 'DB_ERROR_RECORD_ALREADY_EXISTS') {
-				$this->error = $langs->trans("ErrorCompanyNameAlreadyExists", (string) $this->name);
+				$this->error = $langs->trans("ErrorDuplicateField");
 				$result = -1;
 			} else {
 				$this->error = $this->db->lasterror();
@@ -170,7 +183,7 @@ class Link extends CommonObject
 	}
 
 	/**
-	 *  Update parameters of third party
+	 *  Update parameters of link
 	 *
 	 *  @param  User	$user            			User executing update
 	 *  @param  int		$call_trigger    			0=no, 1=yes
@@ -181,14 +194,14 @@ class Link extends CommonObject
 		global $langs, $conf;
 		require_once DOL_DOCUMENT_ROOT.'/core/lib/functions2.lib.php';
 
-		$langs->load("errors");
+		$langs->loadLangs(array("errors", "admin"));
 		$error = 0;
 
 		dol_syslog(get_class($this)."::Update id = ".$this->id." call_trigger = ".$call_trigger);
 
 		// Check parameters
 		if (empty($this->url)) {
-			$this->error = $langs->trans("NoURL");
+			$this->error = $langs->trans("ErrorFieldRequired", $langs->transnoentitiesnoconv("URL"));
 			return -1;
 		}
 
@@ -216,7 +229,13 @@ class Link extends CommonObject
 		dol_syslog(get_class($this)."::update sql = ".$sql);
 		$resql = $this->db->query($sql);
 		if ($resql) {
-			if ($call_trigger) {
+			// Actions on extra fields
+			$result = $this->insertExtraFields();
+			if ($result < 0) {
+				$error++;
+			}
+
+			if (!$error && $call_trigger) {
 				// Call trigger
 				$result = $this->call_trigger('LINK_MODIFY', $user);
 				if ($result < 0) {
@@ -291,6 +310,10 @@ class Link extends CommonObject
 					$link->objectid = $obj->objectid;
 					$link->share = $obj->share;
 					$link->share_pass = $obj->share_pass;
+
+					// Retrieve all extrafields for link
+					$link->fetch_optionals();
+
 					$links[] = $link;
 				}
 				return 1;
@@ -345,6 +368,11 @@ class Link extends CommonObject
 			$rowid = $this->id;
 		}
 
+		if (empty($rowid) && empty($hashforshare)) {
+			$this->error = 'ErrorBadParameters';
+			return -1;
+		}
+
 		$sqlwhere = [];
 
 		$sql = "SELECT rowid, entity, datea, url, label, objecttype, objectid, share, share_pass FROM ".$this->db->prefix()."links";
@@ -377,6 +405,10 @@ class Link extends CommonObject
 				$this->objectid = $obj->objectid;
 				$this->share = $obj->share;
 				$this->share_pass = $obj->share_pass;
+
+				// Retrieve all extrafields for link
+				$this->fetch_optionals();
+
 				return 1;
 			} else {
 				return 0;
@@ -390,16 +422,18 @@ class Link extends CommonObject
 	/**
 	 *    Delete a link from database
 	 *
-	 *	  @param	User		$user		Object suer
+	 *	  @param	User		$user		Object user
+	 *    @param	int<0,1>	$notrigger	1=Does not execute triggers, 0=Execute triggers
 	 *    @return	int						Return integer <0 if KO, 0 if nothing done, >0 if OK
 	 */
-	public function delete($user)
+	public function delete(User $user, $notrigger = 0)
 	{
 		dol_syslog(get_class($this)."::delete", LOG_DEBUG);
 		$error = 0;
 
 		$this->db->begin();
 
+		if (!$notrigger) {
 			// Call trigger
 			$result = $this->call_trigger('LINK_DELETE', $user);
 			if ($result < 0) {
@@ -407,6 +441,7 @@ class Link extends CommonObject
 				return -1;
 			}
 			// End call triggers
+		}
 
 		// Remove link
 		$sql = "DELETE FROM ".$this->db->prefix()."links";
@@ -418,6 +453,14 @@ class Link extends CommonObject
 			$this->error = $this->db->lasterror();
 		}
 
+		// Removed extrafields
+		if (!$error) {
+			$result = $this->deleteExtraFields();
+			if ($result < 0) {
+				$error++;
+			}
+		}
+
 		if (!$error) {
 			$this->db->commit();
 

test/phpunit/AllTests.php

@@ -288,6 +288,9 @@ class AllTests
 		require_once dirname(__FILE__).'/CategorieTest.php';
 		$suite->addTestSuite('CategorieTest');
 
+		require_once dirname(__FILE__).'/LinkTest.php';
+		$suite->addTestSuite('LinkTest');
+
 		require_once dirname(__FILE__).'/ProjectTest.php';
 		$suite->addTestSuite('ProjectTest');
 		require_once dirname(__FILE__).'/CommentTest.php';

test/phpunit/LinkTest.php

@@ -0,0 +1,267 @@
+<?php
+/* Copyright (C) 2026 Frédéric France  <frederic.france@free.fr>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ * or see https://www.gnu.org/
+ */
+
+/**
+ *      \file       test/phpunit/LinkTest.php
+ *      \ingroup    test
+ *      \brief      PHPUnit test
+ *      \remarks    To run this script as CLI:  phpunit filename.php
+ */
+
+global $conf,$user,$langs,$db;
+//define('TEST_DB_FORCE_TYPE','mysql');	// This is to force using mysql driver
+//require_once 'PHPUnit/Autoload.php';
+require_once dirname(__FILE__).'/../../htdocs/master.inc.php';
+require_once dirname(__FILE__).'/../../htdocs/core/class/link.class.php';
+require_once dirname(__FILE__).'/CommonClassTest.class.php';
+
+if (empty($user->id)) {
+	print "Load permissions for admin user nb 1\n";
+	$user->fetch(1);
+	$user->loadRights();
+}
+$conf->global->MAIN_DISABLE_ALL_MAILS = 1;
+
+
+/**
+ * Class for PHPUnit tests
+ *
+ * @backupGlobals disabled
+ * @backupStaticAttributes enabled
+ * @remarks	backupGlobals must be disabled to have db,conf,user and lang not erased.
+ */
+class LinkTest extends CommonClassTest
+{
+	/**
+	 * testLinkCreate
+	 *
+	 * @return int
+	 */
+	public function testLinkCreate()
+	{
+		global $conf,$user,$langs,$db;
+		$conf = $this->savconf;
+		$user = $this->savuser;
+		$langs = $this->savlangs;
+		$db = $this->savdb;
+
+		$localobject = new Link($db);
+		$localobject->url = 'https://www.dolibarr.org';
+		$localobject->label = 'Specimen link';
+		$localobject->objecttype = 'societe';
+		$localobject->objectid = 1;
+
+		$result = $localobject->create($user);
+		print __METHOD__." result=".$result."\n";
+		$this->assertGreaterThan(0, $result);
+
+		return $result;
+	}
+
+	/**
+	 * testLinkCreateWithoutUrl
+	 *
+	 * A link with no url must be rejected.
+	 *
+	 * @return void
+	 *
+	 * @depends testLinkCreate
+	 * The depends says test is run only if previous is ok
+	 */
+	public function testLinkCreateWithoutUrl()
+	{
+		global $conf,$user,$langs,$db;
+		$conf = $this->savconf;
+		$user = $this->savuser;
+		$langs = $this->savlangs;
+		$db = $this->savdb;
+
+		$localobject = new Link($db);
+		$localobject->objecttype = 'societe';
+		$localobject->objectid = 1;
+
+		$result = $localobject->create($user);
+		print __METHOD__." result=".$result."\n";
+		$this->assertLessThan(0, $result);
+	}
+
+	/**
+	 * testLinkFetch
+	 *
+	 * @param   int $id     Id of link
+	 * @return  Link
+	 *
+	 * @depends testLinkCreate
+	 * The depends says test is run only if previous is ok
+	 */
+	public function testLinkFetch($id)
+	{
+		global $conf,$user,$langs,$db;
+		$conf = $this->savconf;
+		$user = $this->savuser;
+		$langs = $this->savlangs;
+		$db = $this->savdb;
+
+		$localobject = new Link($db);
+		$result = $localobject->fetch($id);
+
+		print __METHOD__." id=".$id." result=".$result."\n";
+		$this->assertGreaterThan(0, $result);
+		$this->assertEquals('https://www.dolibarr.org', $localobject->url);
+		$this->assertEquals('Specimen link', $localobject->label);
+
+		return $localobject;
+	}
+
+	/**
+	 * testLinkFetchWithoutParameters
+	 *
+	 * A fetch() with neither rowid nor hashforshare must fail instead of
+	 * returning an arbitrary record.
+	 *
+	 * @return void
+	 *
+	 * @depends testLinkCreate
+	 * The depends says test is run only if previous is ok
+	 */
+	public function testLinkFetchWithoutParameters()
+	{
+		global $conf,$user,$langs,$db;
+		$conf = $this->savconf;
+		$user = $this->savuser;
+		$langs = $this->savlangs;
+		$db = $this->savdb;
+
+		$localobject = new Link($db);
+		$result = $localobject->fetch(0);
+
+		print __METHOD__." result=".$result."\n";
+		$this->assertLessThan(0, $result);
+	}
+
+	/**
+	 * testLinkUpdate
+	 *
+	 * @param   Link    $localobject    Link
+	 * @return  Link
+	 *
+	 * @depends testLinkFetch
+	 * The depends says test is run only if previous is ok
+	 */
+	public function testLinkUpdate($localobject)
+	{
+		global $conf,$user,$langs,$db;
+		$conf = $this->savconf;
+		$user = $this->savuser;
+		$langs = $this->savlangs;
+		$db = $this->savdb;
+
+		$localobject->label = 'Specimen link updated';
+		$result = $localobject->update($user);
+
+		print __METHOD__." id=".$localobject->id." result=".$result."\n";
+		$this->assertGreaterThan(0, $result);
+
+		$localobject->fetch($localobject->id);
+		$this->assertEquals('Specimen link updated', $localobject->label);
+
+		return $localobject;
+	}
+
+	/**
+	 * testLinkFetchAll
+	 *
+	 * @param   Link    $localobject    Link
+	 * @return  Link
+	 *
+	 * @depends testLinkUpdate
+	 * The depends says test is run only if previous is ok
+	 */
+	public function testLinkFetchAll($localobject)
+	{
+		global $conf,$user,$langs,$db;
+		$conf = $this->savconf;
+		$user = $this->savuser;
+		$langs = $this->savlangs;
+		$db = $this->savdb;
+
+		$links = array();
+		$tmpobject = new Link($db);
+		$result = $tmpobject->fetchAll($links, $localobject->objecttype, $localobject->objectid);
+
+		print __METHOD__." result=".$result."\n";
+		$this->assertGreaterThan(0, $result);
+		$this->assertGreaterThan(0, count($links));
+
+		return $localobject;
+	}
+
+	/**
+	 * testLinkCount
+	 *
+	 * @param   Link    $localobject    Link
+	 * @return  Link
+	 *
+	 * @depends testLinkFetchAll
+	 * The depends says test is run only if previous is ok
+	 */
+	public function testLinkCount($localobject)
+	{
+		global $conf,$user,$langs,$db;
+		$conf = $this->savconf;
+		$user = $this->savuser;
+		$langs = $this->savlangs;
+		$db = $this->savdb;
+
+		$nb = Link::count($db, $localobject->objecttype, $localobject->objectid);
+
+		print __METHOD__." nb=".$nb."\n";
+		$this->assertGreaterThan(0, $nb);
+
+		return $localobject;
+	}
+
+	/**
+	 * testLinkDelete
+	 *
+	 * @param   Link    $localobject    Link
+	 * @return  int
+	 *
+	 * @depends testLinkCount
+	 * The depends says test is run only if previous is ok
+	 */
+	public function testLinkDelete($localobject)
+	{
+		global $conf,$user,$langs,$db;
+		$conf = $this->savconf;
+		$user = $this->savuser;
+		$langs = $this->savlangs;
+		$db = $this->savdb;
+
+		$result = $localobject->delete($user);
+
+		print __METHOD__." id=".$localobject->id." result=".$result."\n";
+		$this->assertGreaterThan(0, $result);
+
+		$resultFetch = $localobject->fetch($localobject->id);
+		print __METHOD__." resultFetch=".$resultFetch."\n";
+		$this->assertEquals(0, $resultFetch);
+
+		return $result;
+	}
+}