recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

PhabricatorModularTransactionType: fix regression

Summary:
Fix a regression in this specific point:

phutil_nonempty_string(integer) called at [<phorge>/src/applications/transactions/storage/PhabricatorModularTransactionType.php:342]

This regression was causing a broken Almanac page and maybe others.

Note: The function phutil_nonempty_string() is well-known to be very angry and
throws for any alien value. This is by design, and in many cases
this is appropriate. But not here.

The business logic here handles very generic types like integers
and really probably whatever scalar value coming from an user input
and then normalized to something else, not necessarily a string, but definitely
something that can be cast to string.

If you have better ideas about how to handle these cases, please join T15190.

Closes T15385

Test Plan:
To test Almanac:

1. go to `/almanac/network/` and create at least one network (example: "foo")
2. go to `/almanac/device/` and create at least one device (example: "bar")
3. visit that Device
4. Add Interface
- test the creation of an empty Interface
- test the creation of a right Interface (example: Network "foo", Address 127.0.0.1, Port 80)
- nothing esplodes anymore

Reviewers: arnold, O1 Blessed Committers, avivey

Reviewed By: arnold, O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15385

Differential Revision: https://we.phorge.it/D25220

Valerio Bozzolan 2df7ea13 4d1adf69

+17 -1
+17 -1
src/applications/transactions/storage/PhabricatorModularTransactionType.php
··· 334 334 return $this->getEditor()->getIsNewObject(); 335 335 } 336 336 337 + /** 338 + * Check whenever a new transaction's value is considered an "empty text" 339 + * @param mixed $value A string, null, an integer... 340 + * @param array $xactions Transactions 341 + */ 337 342 final protected function isEmptyTextTransaction($value, array $xactions) { 338 343 foreach ($xactions as $xaction) { 339 344 $value = $xaction->getNewValue(); 340 345 } 341 346 342 - return !phutil_nonempty_string($value); 347 + // The $value can be a lot of stuff, null, string, integer and maybe more. 348 + // We cast to string to answer the question "Is this string empty?". 349 + // Note: Don't use phutil_nonempty_stringlike() since it was not designed 350 + // for integers. 351 + // Note: Don't use phutil_nonempty_scalar() since very probably we could 352 + // receive a boolean, causing exceptions. 353 + // https://we.phorge.it/T15239 354 + $value_clean = phutil_string_cast($value); 355 + 356 + // We made our lives easier and we don't need strlen(something) 357 + // and we should not. 358 + return $value_clean === ''; 343 359 } 344 360 345 361 /**