Mark created Pholio mockups as new objects to fix empty Feed transaction entries
ClosedPublic
Actions

Authored by aklapper on Fri, Apr 19, 16:30.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15679: Pholio image upload triggers Passing null is deprecated at PhabricatorPolicyQuery.php:314

Commits

rPaad704d0fce5: Mark created Pholio mockups as new objects to fix empty Feed transaction entries

Summary

When creating a Pholio mockup, setIsCreateTransaction(true) to avoid an empty Transaction field in the Feed and avoid strncmp() complaining about a null value being passed.

This is very similar to T15659 about Differential Diffs.

ERROR 8192: strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/policy/query/PhabricatorPolicyQuery.php:314]

Closes T15679

Test Plan

Create a new Pholio mockup via /pholio/create/
Go to /feed/transactions/query/all/
Compare entries in the Transaction column before (empty ones) and after (no empty ones) applying the patch.

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Fri, Apr 19, 16:30

Owners added a reviewer: O1: Blessed Committers.Fri, Apr 19, 16:30

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptFri, Apr 19, 16:30

aklapper requested review of this revision.Fri, Apr 19, 16:30

Harbormaster completed remote builds in B1192: Diff 1889.Fri, Apr 19, 16:30

Screenshot before (M17) and after (M18) applying D25600:

Screenshot from 2024-04-19 18-20-53.png (834×1 px, 162 KB)

Probably we need some help to understand the semantic difference between these two:

PholioTransaction#setIsCreateTransaction(bool)
PholioMockEditor#setIsCreate(bool)

In any case, whenever it's the first one or the second (or both??) I'm quite sure we should set this only if $id is set.

valerio.bozzolan awarded a token.Fri, Apr 26, 10:29

Thanks again! This looks perfect in the current state of PholioMockEditor, since it's not an EditEngine and since this is not an appropriate moment to learn all the differences between an "Editor" and an "EditEngine" and/or it's not an appropriate moment to do that kind of migration here now (if ever needed).

sgtm

src/applications/pholio/controller/PholioMockEditController.php
111	✅ Calling `setIsCreateTransaction(true)` is useful thanks to `PhabricatorApplicationTransactionEditor` that looks for at least one transaction with that creation flag, and applies that creation flag to every other transaction: https://we.phorge.it/source/phorge/browse/master/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php;29cbb0a6580fa2a94e8fae13fd4edef78efdeab0$1303-1317