Page MenuHomePhorge

PhabricatorModularTransactionType: fix regression
ClosedPublic

Authored by valerio.bozzolan on May 12 2023, 17:07.

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
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 402
Build 402: arc lint + arc unit

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