Summary: Closes T15840
Test Plan: Send an I forgot my password email with serious business mode off and see the updated email
Reviewers: O1 Blessed Committers, valerio.bozzolan, aklapper
Reviewed By: O1 Blessed Committers, valerio.bozzolan, aklapper
Subscribers: sirocyl, aklapper, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15840
Differential Revision: https://we.phorge.it/D25671
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/diviner/controller/DivinerAtomController.php:440]
```
Closes T15853
Test Plan: Run `./bin/diviner generate` and visit a Diviner page about a class which has no PHPDoc `@task` block, for example access http://phorge.localhost/diviner/find/?name=PhutilSafeHTML&type=class&jump=1
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15853
Differential Revision: https://we.phorge.it/D25689
Test Plan:
This archive contains a small demo: {F2142567}
When installed, an application named D25571 appears under "More Applications".
When you start it, you will see 2 messageboxes: one should be a single lined text, the other a multilined text.
Steps:
1) unpack archive in some directory
2) add ext-D25571/src/ path to load-libraries in local.json
3) bin/arc liberate
4) bin/cache purge --all
5) bin/celerity map
6) restart httpd
7) start D25571 application from More Applications
Also:
arc unit ./src/applications/celerity/__tests__/CelerityResourceTransformerTestCase.php
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15774
Differential Revision: https://we.phorge.it/D25571
Summary:
`$cache_key` is unconditionally called in `$cache->setKey($cache_key, $data)` but is only defined if the previous condition `if ($is_cacheable && $is_locally_cacheable && !$dev_mode)` was true.
Thus initialize the variable to avoid a theoretical exception. (No additional null check is needed as `$cache` gets defined in the same condition.)
Test Plan: Carefully read the code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25642
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Passing `null` to `preg_match()` is deprecated behavior since PHP 8.1.
Thus only call `preg_match()` when the value is set.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:238]
```
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:245]
```
Closes T15852
Test Plan: Import an ICS file which lists attendees without a name but only with an email address into the calendar via http://phorge.localhost/calendar/import/ > "Import Events" > "Import .ics File"
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15852
Differential Revision: https://we.phorge.it/D25686
Summary:
https://we.phorge.it/source/phorge/browse/master/src/view/page/PhabricatorStandardPageView.php$908 tries to "find some navigational menu items to create a mobile navigation menu from" but does not succeed too much it seems:
On a user profile page, that very code calls `buildApplicationMenu()` in `PhabricatorPeopleController` which calls `buildSideNavView()` in the same class.
This code unconditionally displays a "Calendar" menu item, no matter if the Calendar application is installed or not. Thus check first to avoid offering a link ending up in a 404.
For the time being this change renders the menu empty (apart from a link to the page we are already on) but that's more acceptable than all those tears shed by devastated users after clicking the Calendar menu item while being excited and full of anticipation, just to end up on a 404 page instead.
Related: T15224
Test Plan:
* As an admin, install/uninstall Calendar prototype application via http://phorge.localhost/applications/view/PhabricatorCalendarApplication/
* Go to a user profile page like http://phorge.localhost/username/ on a screen with 920px or less width and click the hamburger menu item in the upper right corner. Check/click the "Calendar" entry.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25683
Summary:
rPb48a22bf50a32895566e9aa66c1d7aff87a492b8 moved the call `$this->getTransactionCommentForSave()->save()` from within `PhabricatorAuditInlineComment` to the new (parent) class `PhabricatorInlineComment.php`, however the later does not define this function.
As the two child classes which extend `PhabricatorInlineComment` both implement `getTransactionCommentForSave()`, declare an abstract function in their parent class.
Test Plan: Carefully read the code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25644
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/dashboard/editfield/PhabricatorDashboardQueryPanelQueryEditField.php:41]
```
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/dashboard/editfield/PhabricatorDashboardQueryPanelQueryEditField.php:59]
```
Closes T15791
Test Plan: Successfully create a Dashboard query panel searching for "Diffusion Raw Commits" (which has no Query defined per T15790) and try to edit it afterwards under PHP 8.1 or later.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15791
Differential Revision: https://we.phorge.it/D25596
Summary:
rPe1b8532e2478395bd1f36baecb998adeef5bea3d introduced an implementation of `newWorkCommands()` in the child class `NuanceFormItemType` which is called in parent `NuanceItemType::buildWorkCommands()` without being defined in that parent class.
Thus define `newWorkCommands()` as an abstract class in parent class `NuanceItemType` and add an implementation returning null in the other child class `NuanceGitHubEventItemType` to make static code analyzers happy.
Test Plan: Carefully read the code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25655
Summary:
`DiffusionRepositoryClusterEngine` calls `$this->getDisplayName()` which is not defined in this class or its parent class.
Looking at the variable use in the companying error message, getting the name of the `$repository` was intended instead.
Test Plan: Carefully read the code and check available methods in its parent classes.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25653
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/dashboard/menuitem/PhabricatorDashboardPortalMenuItem.php:34]
```
Closes T15847
Test Plan: Go to `/portal/edit/form/default/`, set a name and click `Create Portal`.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15847
Differential Revision: https://we.phorge.it/D25681
Summary:
The PHPDoc for the abstract function `loadObject()` defines `@return void`, however the function's return value is used twice in `PhabricatorObjectMailReceiver` and all 17 `loadObject()` implementations in child classes return an object provided by some type of `PhabricatorCursorPagedPolicyAwareQuery`.
Thus correct the PHPDoc to make static code analyzers happier.
Test Plan: Carefully read and compare the code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25654
Summary:
`PhabricatorIconSet::getIcons()` calls `$this->newIcons()` which is undefined.
As all six child classes which extend `PhabricatorIconSet` implement `newIcons()`, declare an abstract function in their parent class.
Test Plan: Carefully read the code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25647
Summary:
A saved query can have tokens that require a valid current viewer. For example, this token:
viewer()
Before this change, visiting such saved queries would cause this:
This datasource ("PhabricatorPeopleUserFunctionDatasource") can not evaluate the function "viewer(...)".
After this change, instead of that, you are just redirected to the login page,
so, after you do the login, you are redirected back to that saved query and it works.
This fix was boosted during the Wikimedia Hackaton (wmhack) in Tallinn. Thanks Tallinn!
https://phabricator.wikimedia.org/T356384
Fixes T15704
Test Plan:
Go to Maniphest > Advanced Search > Assigned to > "Viewer". It still works.
Visit the same page in a new anonymous tab: now it redirects to the login page. You login,
and that page works again.
Do the same specific test for all these cases:
- Maniphest
- Assigned To: viewer
- Tags: current Viewer's Projects
- Authors: viewer
- Subscribers: ...
- Closed by
- Badges
- Subscribers
- Differential
- Responsible Users
- Authors
- Reviewers
- Subscribers
- Tags
- Dashboards
- Authored By
- Tags
- Dashboard Panels
- Authored By
- Dashboard Portals
- Tags
- Calendar:
- Hosts
- Invited
- Subscribers
- Tags
- Countdown
- Authors
- Diffusion
- Tags
- Subscribers
- Tags
- Diffusion commit
- Responsible Users
- Authors
- Subscribers
- Tags
- Diffusion identities
- Matching Users
- Feed
- Include Users
- Include Projects (interestingly it does not support "current Viewer's Projects")
- Files
- Authors
- Herald
- Authors
- Subscribers
- Legalpad
- Subscribers
- Nuance (none of their entity support search by token)
- Passphrase
- Subscribers
- Paste
- Authors
- Subscribers
- Tags
- Phame
- Subscribers
- Tags
- Pholio
- Authors
- Subscribers
- Tags
- Phrequent
- Users (interestingly it does not support "viewer")
- Ponder
- Authors
- Answered By
- Projects
- Members
- Watchers
- Transactions - /feed/transactions/
- Authors
- General search at /search/query/
- Authors
- Owners
- Subscribers
- Tags
All the above fields were tested in a clean search, one at a time, both logged-in and logged-out, with the function "viewer" or anything similar like "current Viewer's Projects":
For all cases, the login page appeared successfully where needed, instead of a crash.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, avivey, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15704
Differential Revision: https://we.phorge.it/D25621
Summary:
The method getNewUserBody() was never designed for a list controller.
The method was just orphan. Now, instead, it's non-orphan, and it works
as expected.
This bug was highlighted by Andre Klapper, since their linter was screaming
about inconsistences in the current nonsense usage of getNewUserBody():
D25649
The code was just moved from the list controller, to the search engine.
- PhabricatorDashboardPanelListController (from)
- PhabricatorDashboardPanelSearchEngine (to)
Bonus point:
Adjusted the icon and the title to talk about "Panels" and not "Dashboards".
Added also some inline documentation with an hint about `?nux=1`.
So now we can easily remember how to test this kind of things.
Closes T15844
Test Plan:
In both these pages, you finally see a welcome message, not just in Maniphest:
- http://phorge.localhost/dashboard/panel/?nux=1
- http://phorge.localhost/maniphest/?nux=1
In both these pages, if you have no elements, you finally see a welcome message,
and not just in Maniphest:
- http://phorge.localhost/dashboard/panel/
- http://phorge.localhost/maniphest/
Check that the welcome message helps you in creating a new Panel, and that
talks about Panels, and that have the lovely icon of a Panel.
Reviewers: aklapper, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15844
Differential Revision: https://we.phorge.it/D25674
Summary:
After removing or resetting a custom date field value or points value initially set at task creation there is no way to find and restore the previous value.
As Phorge adds a transaction entry anyway, include the previous value in the transaction title.
This makes it easier to revert an accidental removal of such data and is also more aligned with the behavior of other custom field types.
Closes T15846
Test Plan:
* Define a custom date field in Maniphest via http://phorge.localhost/config/edit/maniphest.custom-field-definitions/ and also enable http://phorge.localhost/config/edit/maniphest.points/
* Create a task with those fields set to non-custom values via a create task form which allows editing/changing these values
* Edit the task and remove/reset the values of these custom fields
* Look at the task and its transaction entries
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15846
Differential Revision: https://we.phorge.it/D25678
Summary:
Nothing in the codebase calls or reads this function.
If it was ever used in some implementation of a custom field, then it was likely in context of the `Similar Task` functionality removed in 52f7446eea.
Test Plan: Grep the codebase.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25677
Summary:
Required parameters $password and $username follow the optional parameter $tval.
This is deprecated since PHP 8.0 per https://www.php.net/manual/en/migration80.deprecated.php
Define defaults for those required parameters to fix the issue - https://github.com/PHPMailer/PHPMailer/blob/master/src/POP3.php#L199 seems to also have survived so far without complaints by PHP 8 users (and PHPStan also does not complain anymore after this change).
Closes T15834
Test Plan: Carefully read the code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15834
Differential Revision: https://we.phorge.it/D25660
Summary:
Maniphest expects the values of the `ids` URL parameter to be integers. Example: http://phorge.localhost/maniphest/?ids=1,2,3,4 to show a list of tasks with those IDs.
When passing monograms instead (`T` prefix, like `T123` instead of `123`), a cryptic message `Expected a numeric scalar or null for %Ld conversion` is shown.
Thus check if `$this->taskIDs` consists of integers only; if not throw a `PhutilSearchQueryCompilerSyntaxException` with an explanatory error message.
Closes T15838
Test Plan: Go to http://phorge.localhost/maniphest/?ids=T1,T2,T3,T4 before and after applying the patch.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15838
Differential Revision: https://we.phorge.it/D25669
Summary:
`'doc'` is listed twice in the `$mimes` array. Remove one key.
The second key was also removed in 6f919df65e.
Test Plan: Read/grep the code.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25666
Summary:
`each()` has been deprecated since PHP 7.2 and got removed in PHP 8.0 per https://www.php.net/manual/en/function.each.php
Replace it with a `foreach` construction.
Closes T15833
Test Plan: Carefully read the code; locally testing that construction per D25659#18155; compare to similar rP9623e667458821f696eb92e9dac5a2c9720af7af
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15833
Differential Revision: https://we.phorge.it/D25659
Summary: No idea if PHP ever offered such a method. In any case, this very check was removed in 6f919df65e (diff-2d294eae28fb0d70fc32da8369c4fe33d301c99ca3eeb4bd41da18ad99344d68) in 2012.
Test Plan: Not sure - how to test a removed check for a method which does not exist anyway? :)
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25663
Summary:
Correct PHPDoc when none of the method calls try to handle a return value.
Add missing fallback `return` statements (which may never be reached if previous condition checks were always true).
This reduces the amount of errors reported by static code analysis tools.
Test Plan: Carefully read the code and check all places in which a function gets called whether a return value is expected.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25650
Summary:
Make the one `new QRbitrtream()` look like the other nine `new QRbitstream()`.
Interestingly, this bug was reported upstream since 2018, and still not fixed:
https://github.com/t0k4rt/phpqrcode/issues/47
Test Plan:
Read the code aloud and realize that this word sound weird.
Verify that the class `QRbitrtream` does not exist, and `QRbitstream` exists.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25651
Summary:
The transaction types `TYPE_TITLE` and `TYPE_CONTENT` were removed from `PonderQuestionTransaction` in rPdff028c4907dd1959859733ea0d947f244559e7f.
Thus update `PonderQuestionCreateMailReceiver` to use the corresponding modular transactions `PonderQuestionTitleTransaction` and `PonderQuestionContentTransaction` instead.
Test Plan: Carefully read the code, grep the code base, use the test case in D25656#18162.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25656
Summary:
This is a natural expansion of the Workboard Trigger system
to support actions about Subscribers.
Context: even before this change - Workboard users
were already able to move Tasks to Columns with
nice automatisms. For instance, you can already:
- Play a nice Sound
- Change Task Priority
- Change Task Status
- Add or Remove a Task Owner
- Add or Remove a Task Project Tag
With this change, you can also:
- Add or Remove Task Subscribers
If you need inspiration, this feature is useful for adding
more eyes on a given work area; lighten the notifications
of certain people after certain workflows have already been
done, to increase happiness and mitigate Burnout.
If your goal is to oppress yourself or your coworkers - of
course you can also use this feature to increase the Burnout
of yourself or your coworkers, adding random people as
Subscriber.
Note: every trigger action can be confusing in general, but
it is the fault of Triggers in general and not related to
this specific feature - that is totally loveable,
just like you.
Closes T15162
Test Plan:
- Workboard > Column > Create Trigger
- Add Action > Add Subscriber > (your best friend who hates NodeJS)
- Add Action > Remove Subscriber > (your antagonist who hates PHP)
- Move some Tasks there and here, and note that Add/Remove works
Reviewers: O1 Blessed Committers, Cigaryno, aklapper
Reviewed By: O1 Blessed Committers, Cigaryno, aklapper
Subscribers: aklapper, speck, tobiaswiese, Matthew, Cigaryno
Tags: #workboard
Maniphest Tasks: T15162
Differential Revision: https://we.phorge.it/D25080
Summary:
Create a Maniphest task, Resolved, High priority.
Before this change, this was the feed in the object page:
- a.lincoln closed this task as Resolved.
- a.lincoln triaged this task as High priority.
- a.lincoln __created this task__.
After this change, this is the feed:
- a.lincoln __created this task__.
- a.lincoln closed this task as Resolved.
- a.lincoln triaged this task as High priority.
This also improves the situation in the /feed/ page. So you see the creation now.
Ref T15816
Test Plan:
Create an object in any mentioned application below, setting all fields.
Then, check the history of the object itself. Check also the /feed/ page.
Check that the creation action is now listed first.
This change improves the situation in:
- Maniphest
- Phriction
This change has no impact on these that seem already OK:
- Calendar
- Dashboard
- Dashboard Panel
- Diffusion
- Macro
- Paste
- Ponder
- Project
- Pholio
This change has no impact on these that seem still in the wrong order:
- Differential
The above situation in Differential is noted, but it's unrelated,
since it has a different legacy structure and it deserves more triaging.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15816
Differential Revision: https://we.phorge.it/D25626
Summary:
Pull the names of Priority field values instead of hardcoding them.
Closes T15799
Test Plan:
* As an admin, go to `/config/edit/maniphest.priorities/` and change the value of `"name"` of a Priority value with a value < `50` (e.g.: Low, Wishlist)
* Open `/maniphest/report/project/`
* Hover over the `Oldest (Pri)` column and check the tooltip text before and after applying this patch
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15799
Differential Revision: https://we.phorge.it/D25602
Summary:
Remove deprecated `buildStandardPageView()` and `buildStandardPageResponse()` methods.
They have been deprecated since rP300c74c49dc9d9f3ef1d7bbcc1b2400a3ded0c41 in November 2015.
cf rPd26cca27d70b69080c10e1df92a10419fffbfc7e, rP1b00ef08a039d48adb6bbbcd6956f900b6303b5b.
Test Plan: Compare http://phorge.localhost/xhpast/ before and after; paste and parse some PHP code into http://phorge.localhost/xhpast/; compare resulting http://phorge.localhost/xhpast/view/1/ before and after. Spot no differences and see no errors in DarkConsole.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25634
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/files/document/render/PhabricatorDocumentRenderingEngine.php:211]
```
Closes T15826
Test Plan: Surf a Diffusion repository and play with {nav View Options}.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15826
Differential Revision: https://we.phorge.it/D25638
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/controller/PhabricatorAuthStartController.php:34]
```
Closes T15832
Test Plan: Run `arc unit` locally, probably also don't be logged in.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15832
Differential Revision: https://we.phorge.it/D25639
Summary: Followup to rP67b7181f522673b05435083ce28106bdfa320e18
Test Plan: Run `./bin/celerity map` and see changes due to my previous commit.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25637
Summary:
Always render dates in tooltips when hovering over data points in Burndown charts etc of the Facts application as `YYYY-MM-DD` instead of `YYYY-M-D` by converting the integer to a string and if its length is only one character, prepend a zero.
Closes T15819
Test Plan:
* Enable the Facts application, go to the Reports of a Project with task changes over time, hover over data points and read the tooltip.
* Check Console of web browser's developer tools for no errors.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15819
Differential Revision: https://we.phorge.it/D25630
Summary:
Allow PHP-coded Custom Fields to show things in Lists.
Also add Repository to Revision List and Flags to Maniphest lists.
Closes T15133. Ref T15512, T15750
Test Plan: Look at Repository List and Task lists that have flags.
Reviewers: O1 Blessed Committers, aklapper, valerio.bozzolan
Reviewed By: O1 Blessed Committers, aklapper, valerio.bozzolan
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15750, T15512, T15133
Differential Revision: https://we.phorge.it/D25548
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/config/check/PhabricatorSetupCheck.php:125]
```
Closes T15823
Test Plan: Run `./bin/cache purge --all`, check any Phorge page in browser before and after applying this patch.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15823
Differential Revision: https://we.phorge.it/D25633
Summary:
The foreach starting at old line 1305-1311 was designed to look for a specific transaction type.
When we have found that, let's stop that loop, so, we save some CPU cycles.
Test Plan:
Note that getTransactionType() has not side-effects.
Test some applications - like Pholio - that use the TransactionEditor:
- Create a new Pholio and Save
- Save it again without changes.
- Save it again with changes.
Everything still working normally.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25627
Summary:
When creating a Dashboard Text Panel, from example from here:
http://phorge.localhost/dashboard/panel/edit/?panelType=text
After you change its Description,
Before this change, this was the feed (but you must have Developer Mode):
foo.bar edited this object (transaction type "text.text").
After this change, this is the feed:
foo.bar edited the the panel text. (Show Details)
So, not only we show a nice title, but we also allow to see details.
| Before | After |
|------------|------------|
| {F2182938} | {F2182943} |
Credits:
- LegalpadDocumentTextTransaction for the getTitle() method
- ManiphestTaskDescriptionTransaction for the details
Closes T15817
Test Plan:
- Create a Text Panel from /dashboard/panel/edit/?panelType=text
- Change description
- See the new feed
- Click on "Show Details". Also that works. You see A/B changes.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15817
Differential Revision: https://we.phorge.it/D25625
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php:154]
```
See also similar rPe65ac7b8800d3397abfced7824a8d316ec573d8f
Closes T15818
Test Plan: Go to http://phorge.localhost/conpherence/search/query/all/
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15818
Differential Revision: https://we.phorge.it/D25629
Summary:
When the persistent chat is minimized to the bottom of your window, as mentioned in T15626,
the chat bar still has an invisible square area that does nothing if you click it by mistake:
{F2184778}
That area was reserved for the Settings Icon, that appears only if you de-minimize the chat.
The Settings Icon was toggled incorrectly from CSS. It was just hiding the Font Awesome icon,
(.fa-gear), instead of hiding its clickable container.
After this change, chat can be de-minimized even if you click in that specific evil spot.
Original credit to @roberto.urbani for the original troubleshooting and original patch.
Follow-up from: D25428
Closes T15626
Test Plan:
Show the Chat, then:
- when the chat is de-collapsed:
- you see the Settings icon (as before)
- you can click on the Settings Icon (as before)
- it shows the Settings menu (as before)
- when the chat is collapsed:
- you can click in whatever point, and it de-collapse (better than before)
Additionally, use the HTML inspector in your browser (`F12` from Firefox),
to double-check that now that `<li>` has display: none when the chat is collapsed.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno, roberto.urbani
Maniphest Tasks: T15626
Differential Revision: https://we.phorge.it/D25628
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:
1. Create a new Pholio mockup via `/pholio/create/`
2. Go to `/feed/transactions/query/all/`
3. Compare entries in the `Transaction` column before (empty ones) and after (no empty ones) applying the patch.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15679
Differential Revision: https://we.phorge.it/D25600
Summary:
Do not extract strings from test case files for localization / translation.
They are not exposed to users, and localizing wastes translators' time.
Closes T15815
Test Plan:
* Install Wikimedia downstream `translations` extension via `git clone https://gerrit.wikimedia.org/r/phabricator/translations` and put it on the same filesystem level as `phorge` and `arcanist`, and follow https://we.phorge.it/w/docs/extensions/install/
* Apply https://gerrit.wikimedia.org/r/c/phabricator/translations/+/1028887 on top, to make the `translations` extension work with upstream Phorge.
* In the downstream `translations` extension path, run `./export.sh`, then run `grep -r "__tests__" .` and get a list of `qqq.json` files with context for strings from test files.
* Apply this patch.
* In the downstream `translations` extension path, run `./export.sh`, then run `grep -r "__tests__" .` to verify that no `qqq.json` files include any strings from test files anymore.
Reviewers: O1 Blessed Committers, aklapper, avivey
Reviewed By: O1 Blessed Committers, aklapper, avivey
Subscribers: avivey, aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15815
Differential Revision: https://we.phorge.it/D25624
Summary:
When trying to rename a user, properly handle when the new username is the old username to avoid an exception.
```
EXCEPTION: (PhabricatorApplicationTransactionNoEffectException) Transactions have no effect:
- Transaction (of type "user.rename") has no effect. at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2823]
```
Closes T15795
Test Plan:
* As an admin, go to `/people/manage/123/` and select `Change Username`. Use the old username as the new username and select `Rename User`. Without the patch, get an exception. With the patch, get a proper error message within the dialog, as if you tried to set the new username to an empty string.
* As an admin, go to `/people/create/`, select `Create Standard User`, and successfully create a new user account.
* In an anonymous browser window, go to `/auth/start/`, click `Register New Account`, and successfully create a new user account.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15795
Differential Revision: https://we.phorge.it/D25601
Summary:
Charts in the Facts prototype application (e.g. as a Burndown chart on a project's Reports page) remained empty for users not logged in: payload response was a login dialog instead of actual data to render. Thus set `shouldAllowPublic()` (as for many other Controllers).
Closes T15436
Test Plan:
* As an admin, enable the Facts prototype application.
* As an admin, set "Can Use Application" to "Public" for Projects on http://phorge.localhost/applications/view/PhabricatorProjectApplication/
* As an admin, set "Can Use Application" to "Public" for Facts on http://phorge.localhost/applications/view/PhabricatorFactApplication/
* Go to a project with tasks; select "Reports (Prototype)" in the sidebar
* Both as a logged in and as an anonymous user, check if the charts render.
* Click the `View Chart` option and both as a logged in and as an anonymous user, check if the charts render at an URI like http://phorge.localhost/fact/chart/a1b2c3d4e5f6/
* As an admin, restrict "Can Use Application" to "Users" for Facts on `/applications/view/PhabricatorFactApplication/`
* As an anonymous user, check that http://phorge.localhost/fact/chart/a1b2c3d4e5f6/ requires a login now.
* As an anonymous user, check that charts on http://phorge.localhost/project/reports/1/ remain empty boxes (which is not ideal but not a big difference to beforehand).
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15436
Differential Revision: https://we.phorge.it/D25608
