Page MenuHomePhorge

Diffusion commit feed: fix commit title repeated twice
ClosedPublic

Authored by valerio.bozzolan on Sep 14 2024, 14:11.

Details

Summary

Before this change any commit in your web feed had a duplicated commit
title. Old example:

FooBar committed REPOFOO 132abc: add documentation (authored by FooBar)
add documentation

After this change the commit title "add documentation" is repeated only once.

BeforeAfter
Diffusion feed before.png (307×718 px, 24 KB)
Diffusion feed after.png (307×718 px, 21 KB)

So the web feed is slimmer and less distracting, more space for more useful info.

Having a NULL body seems OK. In fact, the upstream caller already skips
body rendering in that case:

https://we.phorge.it/source/phorge/browse/master/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php;0fe0b9f681d0da79b313e0907933665930074704$157-160

P.S.

If you think it would be nice to show a second line in the body web feed,
we think so too! Look at the mentioned task and please propose that feature.
This is just an early UX fix to avoid to repeat the same info twice.

Closes T15489

Test Plan

Before this change, look at your web feeds about commits and reproduce
the original problem.

Apply this change and restart phd and do some commits and appreciate
that you do not see anymore duplicated commit titles in each commit feed.

Your email notifications are unchanged.

Diff Detail

Repository
rP Phorge
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1593
Build 1593: arc lint + arc unit

Event Timeline

aklapper subscribed.

Works locally; this makes sense to me (though I'm surprised it's in an Audit related source file).

PhabricatorApplicationTransactionFeedStory checks for if (nonempty($body)) so it can be null instead of an empty string, same for handling in PhabricatorApplicationTransaction

This revision is now accepted and ready to land.Sep 16 2024, 09:04