Page MenuHomePhorge

PhabricatorModularTransactionType: fix regression
ClosedPublic

Authored by valerio.bozzolan on May 12 2023, 17:07.
Tags
None
Referenced Files
F1415847: D25220.id725.diff
Sun, Feb 25, 07:37
F1415844: D25220.id727.diff
Sun, Feb 25, 07:37
F1415365: D25220.id.diff
Sun, Feb 25, 06:54
F1415131: D25220.diff
Sun, Feb 25, 06:46
Unknown Object (File)
Fri, Feb 16, 23:03
Unknown Object (File)
Fri, Feb 16, 20:05
Unknown Object (File)
Tue, Feb 13, 00:27
Unknown Object (File)
Mon, Feb 12, 21:44

Details

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

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/applications/transactions/storage/PhabricatorModularTransactionType.php
342

I am also glad since making a code like ! nonempty really makes me semantically mad

This revision is now accepted and ready to land.May 12 2023, 18:37