compta/paiement/document.php and fourn/paiement/document.php expose
$hookmanager via the @var docblock but never call initHooks, so the
include of core/actions_linkedfiles.inc.php and the rest of the page
can't run any custom-module hook context.
Add an initHooks() right after loadLangs, mirroring the pattern used
by commande/document.php (orderdocument, globalcard). Use
paymentdocument / paymentsupplierdocument for the two pages.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
PrintIPP just calls $ipp->setData($fileprint) and never tells CUPS what
the file actually is, so the job arrives as application/octet-stream
and CUPS treats ODT/PDF/PS as raw bytes - the ASCII garbage symptom
the reporter sees.
Map the file extension to a MIME and call $ipp->setMimeMediaType()
when the extension is recognised. Unknown extensions fall through to
the existing default, so the change is non-breaking.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
When $arraydefaultmessage->email_from is set but $arraydefaultmessage->id
is empty, FormMail built a 'from_template_' option key with no trailing
id, and the same string was passed as the preselected value of the
fromtype select. The downstream parser in core/actions_sendmails.inc.php
matches /from_template_(\d+)/ which requires at least one digit, so the
match failed and the from address fell back through the else branch.
Guard the option emission and the selected-value computation against
empty $arraydefaultmessage->id so the malformed key is never produced.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
isStringVarMatching() wraps the input regex with '/^' and '/' before
preg_match, but callers (custom modules, older code, dol_eval rules
that accept both '(aaa|bbb)' and '/^(aaa|bbb)/') sometimes already
pass the full regex with delimiters. The double wrap produces
'/^/^(aaa|bbb)//' which trips preg_match with 'Unknown modifier ^'
on every page load.
Strip a leading '/' (optionally followed by '^') and any trailing
'$?/' plus PCRE flags before reusing the pattern, so both bare and
already-delimited forms work the same way.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
When a paid deposit invoice is auto-converted into a discount, the per-rate
buckets in Paiement::create were keyed on tva_tx alone. Lines that shared a
rate but had distinct vat_src_code values were merged into one bucket, and
the generated discount was always created with an empty vat_src_code.
A later 'Apply discount' on the standard invoice therefore inserts a line
without the source VAT code, while the deposit lines keep it. The PDF then
shows two VAT lines for the same rate (one with the code, one without).
Bucket by tva_tx + vat_src_code instead, parse both back when looping the
buckets, and assign vat_src_code on the generated discount before create().
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The various-payments section of compta/resultat/clientfourn.php fetches
two rows from the GROUP BY p.sens query (debit, credit). When one of the
two sens has no payment in the period the corresponding fetch_object()
returns null, but the subsequent print statements still dereferenced
$obj->amount outside the isset() guard used for the totals.
Capture the amount into a local that defaults to 0 when the row is
absent, and print from that local. Behaviour is unchanged when both rows
are present, the warning disappears in the missing-row case, and the
debit/credit value printed in that case is now 0 instead of an undefined
read.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The signed-PDF builder branched only when the bank PDF model is exactly
'sepamandate' and left $sourcefile empty in every other case. The
downstream dol_is_file($sourcefile) check then failed silently, the
signed PDF was never built, and the public download link kept pointing
at the unsigned original.
Add an else fallback that takes the last generated main document path
and derives the signed name by appending _signed-<date> before the
extension, so non-default bank PDF models (eg. 'ban') produce a signed
copy too.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
draw_chart() built tmpykey by indexing array_of_ykeys[$i + 1] without
checking the key existed first, and the y_<i> fallback used
$valarray[$i + 1] the same way. Both fire PHP 8.2 'Undefined array key'
warnings when a $this->data row is shorter than the longest one (custom
modules sometimes feed sparse rows).
Resolve ykey via isset() before exploding, and add a ?? null fallback on
the value lookup so both warnings stop without changing the value
returned in the well-formed case.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
* Fix#38358 guard cache_availability lookup in form_availability
When form_availability() is called with a non-zero $selected that is
not a known c_availability rowid (e.g. -1 from a row whose fk_availability
was left at the legacy 'not set' sentinel), the direct array access
emits two PHP warnings on PHP 8+:
- Undefined array key -1
- Trying to access array offset on value of type null
Check isset() before printing the label and fall back to nbsp like the
$selected==0 branch already does.
Signed-off-by: Dolicraft <contact@dolicraft.com>
* Fix#38358 also drop excess args on two dolPrintHTMLForAttribute calls
phpstan flags two pre-existing 6-arg calls in the same file once any
edit forces a re-check. dolPrintHTMLForAttribute() takes 1-3 args, so
the extra '', 0, 1 are dropped. The default $escapeonlyhtmltags=0 and
$allowothertags=array() match the intent of the original calls.
Signed-off-by: Dolicraft <contact@dolicraft.com>
---------
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
When the 'by' column is hidden in the project timespent list, the form does
not emit a userid_line input. The updateline handler then read
GETPOSTINT('userid_line') as 0 and overwrote timespent_fk_user. The downstream
'in_array($object->timespent_fk_user, $childids)' guard with fk_user=0
blocked the update and the user saw a confusing 'Field User is required' for
the unchanged user assignment.
Only overwrite timespent_fk_user when the form actually posted userid_line,
mirroring the date column fix from #34511.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The shared confirm_delete branch at htdocs/core/actions_addupdatedelete.inc.php:468
fired on action=confirm_delete with permission, but never checked the
$confirm value. Every other confirm_* branch in the same file checks
$confirm == 'yes'. As a result, a card.php using the standard
$form->formconfirm() flow without the ajax handler would delete the
object when the user clicks 'No' on the confirmation dialog.
Align with the rest of the file by requiring $confirm == 'yes'.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
ExpenseReport::hasDelay() compares the validation date to (now -
warning_delay). When MAIN_DELAY_EXPENSEREPORTS (or its 'to pay' twin)
is not configured, $conf->expensereport->approve->warning_delay is 0,
so the test becomes "date_valid < now" and every validated report
older than a moment is flagged as late.
Treat warning_delay <= 0 as 'no delay configured' and return false in
that case, so an unconfigured threshold no longer marks the whole list
as overdue.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
llx_actioncomm has no fk_user_done column - it was migrated to
fk_user_action by the 3.5 -> 3.6 schema change. The 2023 introduction
of the upcoming-events box (commit e0ef2870a4) added a third predicate
'a.fk_user_done = $user->id' to the visibility WHERE clause, which
fails with 'Unknown column' on both MySQL and PostgreSQL (PG just
surfaces it visibly).
Drop the extra OR. fk_user_action already covers the 'owner of action'
case, which is what fk_user_done used to mean before the column rename.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
Task::create()'s INSERT statement omitted the rang column. Project clone
flows go through Task::createFromClone -> Task::create with the rang
copied from the source task, but because the INSERT never wrote it the
new tasks always ended up with rang=0 (the column default) and the
duplicated project lost the drag/drop ordering.
Include rang in the INSERT, mirroring the update() statement at line 692
that already does '($this->rang ?: 0)'.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The customer-side line picker preselects the product's default measuring
unit when a product is chosen, via /product/ajax/products.php returning
fk_unit and a small jQuery snippet in objectline_create.tpl.php (added
for issue #34610). The supplier-side line picker, which uses
/fourn/ajax/getSupplierPrices.php and a separate $.post callback,
never received fk_unit on the wire, so the #units dropdown was always
left at its default and the reporter's products with a configured unit
showed the wrong unit on save.
Three coordinated edits:
- list_product_fournisseur_price() now selects p.fk_unit alongside the
other product columns and assigns it on the returned ProductFournisseur
instance as $prodfourn->fk_unit.
- getSupplierPrices.php exposes $productSupplier->fk_unit on each row
of the JSON response so the client can read it.
- objectline_create.tpl.php (the supplier callback after the
fournprice_predef rebuild) walks the response, picks the first
non-pmp/non-costprice row's fk_unit (all rows for one product carry
the same fk_unit), and sets #units to that value when
PRODUCT_USE_UNITS is on. Mirrors the customer-side logic already
in place for issue #34610.
Same code path on develop.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The existing guard at line 1531 of Product::update re-clones $this->oldcopy
with native=1 (which preserves methods) when oldcopy is null or has no
id, but does not catch the case where a caller assigned a stdClass via
dol_clone($obj, 2). dol_clone with native=2 copies the scalar properties
of $srcobject onto a fresh stdClass, including $obj->id, so
empty($this->oldcopy->id) is false. Execution then reaches
$this->oldcopy->hasbatch() at line 1716 and fatals with
'Call to undefined method stdClass::hasbatch()'.
Add a third branch to the guard: re-clone whenever oldcopy is not an
instance of Product. The native=1 reclone restores the proper Product
type and the hasbatch() / isEmpty() calls below work as intended.
Reproduced on PHP 8.2 in Docker: assigned oldcopy = dol_clone($obj, 2)
(stdClass) then called update(), vanilla path fatals on
stdClass::hasbatch(), patched path returns 1 cleanly.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
`getOnlineSignatureUrl()` ends each public signature URL with
`&securekey=` followed by the output of `dol_hash(..., '0')`. With the
default `MAIN_SECURITY_HASH_ALGO = password_hash`, that output is a
bcrypt string whose alphabet is `[./A-Za-z0-9]`, so a non-trivial share
of generated URLs legitimately end with `.` or `/`.
Gmail and several mobile mail clients autolink plain-text URLs but
drop trailing punctuation, so the recipient clicks on a URL with the
last character missing and gets a securekey mismatch error
(`Bad value for securitykey`). The user-facing symptom is exactly the
one in #31464.
When multicompany is enabled, the URL already ends with `&entity=N`,
which sidesteps the issue. For installations without multicompany,
append a stable trailing `&_=1` so the URL never ends with the hash.
The receiver in `public/onlinesign/newonlinesign.php` does not read
`_`, only `source`, `ref`, `securekey` and (when multicompany) `entity`,
so adding the parameter has no behavior impact and existing links keep
verifying as before. Only the real URL (mode 0) is affected, not the
mode 1 preview shown in the admin setup.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The minimum-price guard on the customer invoice, customer order and
customer proposal forms compares the entered unit price to the
product's price_min. When the document is in a foreign currency the
unit price is provided in the foreign currency ($pu_ht_devise), so
it must first be converted back to the base (company) currency before
the comparison.
card.php was multiplying by $currency_tx:
$pu_equivalent = (float) $pu_ht_devise * $currency_tx;
but multicurrency_tx is defined as 'units of foreign currency per
1 unit of base', exactly as price.lib.php uses it
(calcul_price_total at line 181):
$pu = $pu_devise / $multicurrency_tx;
With the reporter's example (1 MXN = 0.055 USD, so tx = 0.055,
product min price = 1000 MXN, entered 55 USD), the wrong direction
gives 55 * 0.055 = 3.025 MXN < 1000 MXN and the form errors out with
'CantBeLessThanMinPrice', even though 55 USD is exactly 1000 MXN.
Dividing yields 55 / 0.055 = 1000 MXN and the check passes.
Mirror calcul_price_total's direction at all four invoice sites
(htdocs/compta/facture/card.php lines 2503/2506 and 2684/2687) plus
the corresponding sites in htdocs/commande/card.php and
htdocs/comm/propal/card.php. Documents in the base currency
($pu_ht_devise empty) are not affected because the conversion
branch is gated on $pu_ht_devise being non-empty.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The per-card Send by email button on Propal, Commande and Facture
already honours the hidden constants
`PROPOSAL_SENDBYEMAIL_FOR_ALL_STATUS`,
`COMMANDE_SENDBYEMAIL_FOR_ALL_STATUS` and
`FACTURE_SENDBYEMAIL_FOR_ALL_STATUS` to allow sending drafts. The
mass Send by email action in the corresponding lists silently dropped
draft objects regardless of those constants, so an administrator who
intentionally enabled the override saw inconsistent behaviour between
the card and the list.
Apply the same constant check on the three draft-skip guards in
`htdocs/core/actions_massactions.inc.php` so the mass action behaves
the same way as the single-object button. Default behaviour is
unchanged: with none of the constants set, drafts are still skipped
with the existing 'OnlyXxxNotDraftCanBeSentInMassAction' error
message.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
* Fix#38491 Name the field in the ticket too-long error message
When a ticket message exceeds 65000 characters, Ticket::verify() and
Ticket::update() appended the bare string "ErrorFieldTooLong" to
$this->errors. setEventMessages forwards that string and
dol_htmloutput_mesg applies $langs->trans() to it, which yields the
raw template "Field %s is too long." with the %s placeholder still
visible because no argument was passed.
Replace the bare key with a pre-translated message that names the
field ("Initial message"), the same pattern already used in
htdocs/barcode/printsheet.php for the same lang key. Two identical
sites in the file are updated (one in verify, called by create, and
one in update). The "errors" and "ticket" lang files are loaded
defensively before translation because both methods can run before
the page-level loadLangs.
Reported by @ocemag, expected field name confirmed by @aspangaro.
Signed-off-by: Dolicraft <contact@dolicraft.com>
* Fix phan: declare $langs as global in ticket too-long branches
phan level 10 raised PhanUndeclaredVariable and PhanNonClassMethodCall
on the two branches that translate the new error message. The enclosing
Ticket::verify() and Ticket::update() do not declare global $langs;
add it where needed instead of opening the method scope further.
Signed-off-by: Dolicraft <contact@dolicraft.com>
---------
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Alexandre SPANGARO <alexandre.spangaro@gmail.com>
ExtraFields::showInputField normalises the legacy "(field=123)"
filter syntax of "List from a table" extrafields into the modern
USF "(field:=:123)" form. The compatibility regex
"/^\(?([a-z0-9]+)([=<>]+)(\d+)\)?$/i" only accepts alphanumeric
field names, so a filter like "fk_typent=2" does not match, the
filter is left as is, and forgeSQLFromUniversalSearchCriteria later
fails with "Filter error" because the raw expression is not valid
universal search syntax.
Accept the underscore character in the field name class, the rest
of the predicate is untouched. The two identical occurrences in
the file (one per branch of the dispatch) are updated.
Reported and root cause pinpointed by @stephanebourdon-cmyk.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
Updating a product selling price via /product/price.php triggered a
fatal "Call to undefined method stdClass::isEmpty()" at
Product::update line 1457 of 21.0. In the update_price flow,
$this->oldcopy is a stdClass (not a Product instance), so isEmpty()
does not exist on it. The page returned HTTP 500 and the open
transaction was forcibly closed by DoliDBMysqli::close.
Replace the isEmpty() call with empty($this->oldcopy->id), which is
the form already shipped in 22.0, 23.0 and develop after upstream
commit ef94452f77. The accompanying comment from that commit is
also brought in.
Same root cause as issue 37792 (closed, fixed in 23.0.3 and
forward); the fix was never backported to 21.0 even though the bug
is still triggered on that branch.
Reported by @YOYOKMC.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
Categorie::containing() returns an integer (-1) when its SQL query fails
(e.g. the categorie_xxx table does not exist). The $remove_existing=false
branch of setCategoriesCommon() passed that int straight to array_diff(),
producing "array_diff(): Expected parameter 2 to be an array, int given"
and "Invalid argument supplied for foreach()" (a fatal TypeError on PHP 8),
which in turn broke the redirect with "headers already sent".
Normalize $existing to an empty array right after the containing() call so
both branches are safe, and drop the now-redundant inner is_array() guard.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix#27987 refuse to save signature when too-many-links sanitizer fires
When the user signature contains more inline links/images than
MAIN_SECURITY_MAX_IMG_IN_HTML_CONTENT allows, GETPOST('signature',
'restricthtml') returns the literal string 'ErrorTooManyLinksIntoHTMLString'
instead of the sanitized HTML (see dol_string_unicode in
htdocs/core/lib/functions.lib.php:8755-8757). On the user/card.php save
sites, that literal was assigned verbatim to $object->signature and
persisted; the next outgoing email then used the literal as the
signature block, so the customer received an email whose entire
signature was the string 'ErrorTooManyLinksIntoHTMLString'.
Detect the literal at both save sites (action == 'add' around line 316
and action == 'update' around line 499), increment $error, surface a
clear setEventMessages and route the user back to the form so the
signature is not persisted. The MAIN_SECURITY_MAX_IMG_IN_HTML_CONTENT
setting is the legitimate guard; this PR only ensures its rejection
output never reaches the database or downstream emails.
Signed-off-by: Dolicraft <contact@dolicraft.com>
* Add ErrorTooManyLinksIntoHTMLString en_US translation
pre-commit CI on PR #38534 (Fix#27987) flagged the new lang key
ErrorTooManyLinksIntoHTMLString used in user/card.php as missing.
Add the en_US entry so the existing helper text from the constant
MAIN_SECURITY_MAX_IMG_IN_HTML_CONTENT is reachable through
$langs->trans().
Signed-off-by: Dolicraft <contact@dolicraft.com>
---------
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
* Fix#29799 widen RUM truncation so customer code fits
BonPrelevement::buildRumNumber() builds an auto-generated SEPA UMR
when no UMR was supplied on a new bank-mandate row. The format is
{RUM/UMR}-{yymmddHHMM}-{drum_id-customer_code}
with the tail truncated to 13 characters. That truncation is too
aggressive: typical Dolibarr customer codes (CU1908-00002,
CU2509-00012, ...) are already 12 characters, so any drum id at all
pushes the tail past 13 and the customer code is cut mid-string.
The generated UMR then no longer matches the actual customer code,
which is the symptom in the report ("customer code CU1908-00002
becomes 1-CU1908-0000 in the UMR").
The SEPA standard allows up to 35 characters for MndtId. The
non-truncatable prefix (RUM/UMR + '-' + 10-char date + '-') uses
15 characters, leaving up to 20 characters for the tail before hitting
the 35-char limit, and 17 before hitting the more conservative 32-char
target the original comment referenced. Bump the truncation to 20:
the typical drum-id + 12-char customer code (14 chars) now fits
cleanly, and longer codes still get truncated but at a much later
point.
Only the generation of newly created mandates is affected. Existing
$companybankaccount->rum values stored in the database are not
touched, so no migration is needed.
Signed-off-by: Dolicraft <contact@dolicraft.com>
* Update bonprelevement.class.php
---------
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
`Facture::createFromClone()` calls `Facture::fetch($fromid)` to
populate the in-memory object, then immediately runs
`$object->create($user)` to persist the clone. `fetch()` explicitly
sets `$this->thirdparty = null;` (see facture.class.php fetch()), and
the clone path never calls `fetch_thirdparty()` before `create()`.
When `create()` walks the cloned lines it goes through
`addLine()` -> `getLocalTaxesFromRate($txtva, 0, $this->thirdparty,
$mysoc)` -> `get_localtax($vatrate, 2, $buyer, $seller)`. The buyer
argument is the now-null $this->thirdparty, and `get_localtax()`
guards on `!$thirdparty_buyer->localtax2_assuj` and returns 0 as soon
as that property is unreadable. `calcul_price_total()` then computes
total_localtax2 = 0, and addLine() resets the line's localtax2_tx to
0 ("if ($total_localtax2)" branch in addLine()). The cloned line is
inserted with the IRPF (localtax2) rate stripped, which is the symptom
the reporter and the four other thread participants observed.
Call `fetch_thirdparty()` once in createFromClone() so the buyer is
populated for the rest of the clone flow. The fix is local to the
clone path; the regular "create from form" flow already had a
thirdparty set by its caller and is not affected.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
ProductCombination::update() called $this->updateProperties($parent,
$user) at line 439 and discarded the return value. updateProperties()
walks the parent's price levels, computes the variant price from the
combination's variation_price / variation_price_percentage and calls
Product::updatePrice() on the child product. updatePrice() refuses
to set a price below the configured price_min (returns -1 with
'ErrorPriceCantBeLowerThanMinPrice'), but ProductCombination::update()
still returned 1 because the failure was thrown away.
createProductCombination() initialises the new variant product with
price = price_ttc = price_min = price_min_ttc = 0 to keep a clean
price history, then commits the combination by calling
$newcomb->update($user). When the parent product has a non-zero
minimum price and the variation drops the computed price below it,
updateProperties() returns -1, update() reports success anyway, the
combination is committed and the variant product stays at price 0.
The card displays price 0 and minimum 0 with no error message, which
is exactly the behaviour the reporter described and that @JonBendtsen
confirmed was still present on develop after the v22 release.
Capture updateProperties()'s return value and propagate the negative
result so the calling createProductCombination() sees the failure,
rolls back the transaction and surfaces the underlying
'ErrorPriceCantBeLowerThanMinPrice' message. The variant is no longer
silently created with a 0 price; users get either the legitimate
adjusted price (when the variation is permissible) or a clear error
prompting them to drop the parent's minimum price first.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
The single subscription form (htdocs/adherents/subscription.php
lines 219-220 and 262) computes the subscription end date from the
selected AdherentType: it uses adht->duration_value and
adht->duration_unit to derive the end date with dol_time_plus_duree,
and only falls back to '1 year' when those fields are empty.
The mass action on the member list (htdocs/adherents/list.php
$action == 'createsubscription_confirm') skipped that step and
called Adherent::subscription($now, $amount, ..., $datesubend = 0)
with $datesubend left at 0 for any member. Adherent::subscription()
treats $datesubend = 0 as 'no end date supplied' and defaults to
+1 year - 1 day, so a member type configured for 6 months ended up
with a 12-month subscription whenever the mass action was used.
Compute the per-member end date before the call, mirroring the
single form: fetch the AdherentType of the current member, take
duration_value / duration_unit, and add them to $now. The previously
shipped MEMBER_SUBSCRIPTION_SUGGEST_END_OF_MONTH /
MEMBER_SUBSCRIPTION_SUGGEST_END_OF_YEAR override (commit b1d906d6d3)
still wins when set, because $datesubend keeps its computed value;
the per-type fallback only kicks in when no global override was used.
Same code path on 21.0, 22.0, 23.0 and develop; PR targets 21.0.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
Facture::addline()'s sixteenth positional argument is $type, which
defaults to 0 (TYPE_PRODUCT). When the addline call passes a non-zero
$fk_product, addline overrides $type from the fetched Product object,
so the type the caller passed is just a default for the no-product
case.
htdocs/projet/tasks/time.php had three invoice-line creation paths
for the project time-spent flow:
- line 567 (one line per user per day)
- line 666 (one line per time-spent declaration)
- line 764 (one line per task)
Only line 764 explicitly passed type=1. The other two relied on the
addline default of 0, so when a time entry had no product attached
and the chosen "service to use on lines by default" did not match a
product row that addline could fetch (for example if the service id
got reset between sessions or the product was disabled), the line
was stored with product_type=0. The PDF then computed
'Delivery of goods' in the category-of-operations mention even
though the invoice was for time spent. The reporter confirmed the
'one line per user' and 'one line per task' modes already worked,
which lines up with mode 567 (no product on the time entry) and
mode 764 (explicit type=1).
Pass Product::TYPE_SERVICE as the type at the two affected call
sites, matching the explicit value already used by the per-task
mode. The product fetch inside addline still overrides this when a
real service product is bound to the line, so the change is a
no-op in the happy path and only fixes the leak when no product is
resolved.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
htdocs/holiday/card.php fired the 'HolidaysToValidateAlertSolde' note
on the approval email whenever the number of requested open days
exceeded the current balance, even when the leave type was configured
in c_holiday_types with affect=0 (the type does not deduct from the
holiday balance). For non-deducted types, by definition, the balance
cannot drop, so the warning is bogus and misleading for the approver.
Look up c_holiday_types.affect via getDictionaryValue() and only
compute the balance comparison when affect=1. The
HOLIDAY_HIDE_APPROVER_ABOUT_NEGATIVE_BALANCE global keeps its previous
effect of silencing the warning entirely.
Same code path on 21.0, 22.0, 23.0 and develop; PR targets 21.0.
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
* Fix#38559 reset lifecycle timestamps when cloning a ticket
Ticket::createFromClone() resets id, status, ref, track_id and progress
but leaves datec, date_read, date_close and resolution carrying over
from the source. The clone is then persisted with the source's creation
timestamp (Ticket::create only fills datec via dol_now() when it is
empty, line 494-495), the source's date_read, the source's date_close
and the source's resolution, even though the clone is a brand new
ticket that has never been read, closed or resolved.
For workflows that duplicate a closed ticket to open a fresh request
(the use case in the issue: a Purchasing queue that reuses past tickets
as templates), this means the duplicate appears as if it was opened
months ago and was closed at duplication time.
Set the four properties to null before create(). create() then fills
datec with dol_now() and writes date_read, date_close and resolution
as SQL NULL.
Signed-off-by: Dolicraft <contact@dolicraft.com>
* Fix#38559 use 0 + unset to clear timestamps to satisfy phpstan
PR #38561 CI surfaced phpstan complaints: Ticket::$datec, $date_read
and $date_close are declared @var int in this class, so assigning
null fails the type check.
Use the Dolibarr-idiomatic alternatives:
- $datec = 0 so Ticket::create()'s 'if (empty($this->datec))' guard
still fires and fills it with dol_now(); isDolTms(0) is true but we
never reach the writer with 0 because create() rewrites it first.
- unset() for date_read / date_close / resolution so the writers'
'!isset' branch emits SQL NULL.
Behaviour-identical to the previous commit; only the form changes.
Signed-off-by: Dolicraft <contact@dolicraft.com>
---------
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
