Strip surrounding whitespace from project and task titles
Needs ReviewPublic
Actions

Authored by aklapper on Oct 29 2024, 10:34.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Summary

When creating or renaming a project or a task, copy and paste can lead to unwanted whitespace.
Thus strip any whitespace from the beginning and end of the title string by using trim() when running validateTransactions().

Test Plan

Create and rename projects and tasks with whitespace at the beginning and the end of their names. Confirm that the whitespace gets removed. Get correctly sorted database query search results, don't get notification mail saying Foo added a parent task: Txxxx: Title . anymore which annoys pedants like me.
Create and rename projects and tasks by setting the title to include whitespace only. Confirm that the error about being empty is shown.

Diff Detail

Repository

rP Phorge

Branch

stripTitleWhitespace (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1616
Build 1616: arc lint + arc unit

Event Timeline

aklapper created this revision.Oct 29 2024, 10:34

Owners added a reviewer: O1: Blessed Committers.Oct 29 2024, 10:34

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptOct 29 2024, 10:34

aklapper requested review of this revision.Oct 29 2024, 10:34

Harbormaster completed remote builds in B1615: Diff 2515.Oct 29 2024, 10:34

Restore an empty line

Harbormaster completed remote builds in B1616: Diff 2516.Oct 29 2024, 10:38

aklapper edited the test plan for this revision. (Show Details)Oct 29 2024, 10:40

Thanks. Seems good to me since trim($new) does not receive null so cannot create PHP 8.1 problems, since we already execute strlen($new) and mb_strlen($new) and that was not reporting errors in your updated install

This revision is now accepted and ready to land.Oct 29 2024, 12:11

This needs rework as the logic currently does not properly handle entering a title consisting of whitespace only. (Outcome gets unpleasant afterwards.)

aklapper edited the test plan for this revision. (Show Details)Tue, Nov 26, 13:00

Handle whitespace-only task and project titles correctly

This revision is now accepted and ready to land.Tue, Nov 26, 13:02

Harbormaster completed remote builds in B1627: Diff 2531.Tue, Nov 26, 13:02

aklapper requested review of this revision.Tue, Nov 26, 13:03

@valerio.bozzolan: Feel free to give this revised version another review :)

valerio.bozzolan added inline comments.Wed, Dec 4, 06:59

src/applications/project/xaction/PhabricatorProjectNameTransaction.php
75	Maybe we can restore line 75 and put early just after line 74 $new_value = trim($new_value); In this way line 76 is triggered on the new length that I guess it's a better message if you write " lol " So it's saved as "lol" without the related error.