Summary: Two unconditional return statements lead to never reaching the second one.
Test Plan: Read the code of this method.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25729
Summary: `setRequest()` and `getRequest` in this class access the undefined property `$request`. Thus define this property.
Test Plan: Run PHPStan static code analysis; grep the code in this class.
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/D25716
Summary:
The commit rP7bde92b9c9806c5967df617fb4e843dea315ecf7 (2016) introduced `private function validateNavigationMenuItem()`.
The only call to this method was removed in rP950e9d085b482bc3484e2066591ac07c20154c9a.
Thus remove this unused method.
Test Plan:
Read/grep the code in `PhabricatorProfileMenuEngine`.
Run also `grep -RF 'validateNavigationMenuItem' --include="*.php" .` to verify that this is an orphan method.
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/D25719
Summary:
The expression `$all_tags[$tag]['count']` does not do anything. Reading the surrounding code, intention is to display a "Common" section on `/settings/panel/emailpreferences/` listing MailTags defined by at least two different applications. (This is currently not the case anyway as Phorge prefixes all MailTags with their corresponding application but might be a future use case.)
This change fixes the logic accordingly.
Closes T15874
Test Plan: Apply the patch; replace the value of a random `const MAILTAG_*` line in the codebase with the value of another random `const MAILTAG_*` line in the codebase to make it "common", now see that `/settings/panel/emailpreferences/` displays this MailTag in a "Common" section on top of the options.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15874
Differential Revision: https://we.phorge.it/D25727
Summary:
The timeline for an EditEngine form says `created this object`. It should say `created this form`.
Closes T15880
Test Plan: Go to http://phorge.localhost/transactions/editengine/maniphest.task/ and create an EditEngine form, scroll down to its timeline.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15880
Differential Revision: https://we.phorge.it/D25726
Summary: `$capbilities` has a typo.
Test Plan: Slowly spell the word "capabilities"; compare with the top of the file.
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/D25714
Summary:
When you drag and drop a file in a Conpherence chat,
the file was only visible by the author.
Now the file is also attached to that chat, making it visible.
This is a follow-up from:
https://we.phorge.it/D25705?id=2178
Refs T15106
Test Plan:
1) open a Conpherence chat or create a new one
2) drag and drop file in it and send the message
3) verify file is attached to the chat
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, Cigaryno, Matthew, valerio.bozzolan, tobiaswiese
Maniphest Tasks: T15106
Differential Revision: https://we.phorge.it/D25709
Summary:
The methods `newRequiredTransasctionError()` [sic!] and `newInvalidTransactionError()` are not called anywhere in the codebase. Both are `final protected`, thus remove them.
Incidentally, speedup Diviner generation by at least 0.000000000000000000000001 milliseconds.
Test Plan: Grep 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/D25643
Summary: Seems to be unused. Plus it calls `Filesystem::digestForIndex()` which does not exist. So if this method was ever called then it would through an exception and not reach the `parent::save()` call either in the line after.
Test Plan:
Grep and read the codebase.
Tried things that may use cache and they still work, including:
- running `./bin/cache purge --caches user` successfully shows `Purging "user" cache...`
- the button "Purge cache" in /config/cache/ still work.
- editing a user
- changing user picture using default one
- uploading new user picture
- creating Calendar event and inviting yourself
- visiting user settings
- changing a user setting, including Timezone, Display preferences
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/D25645
Summary:
When you drag and drop a file in the Phriction editor and then save the
corresponding wiki document, the file was only visible by the author.
Now the file is also attached to that document, making it visible.
Refs T15106
Test Plan:
1) open the Phriction editor to edit an existing or create a new wiki document
2) drag and drop file in it
3) save wiki document
4) verify file is attached to wiki document (other user can see file in wiki page)
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15106
Differential Revision: https://we.phorge.it/D25705
Summary: Remove incorrect book scope from numerous `find` URIs on the "Rendering HTML" Diviner page in order to get results instead of 404 pages.
Test Plan:
Refresh Diviner documentation from Phorge directory:
./bin/diviner generate
Go to https://we.phorge.it/book/contrib/article/rendering_html/ and click random links on that page.
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/D25688
Summary:
When awarding a badge to a user without having actually selected a batch to award, do not fail on running a database query with no parameter. Instead, silently fail without an error and reload the page, similar to the behavior of `Add Recipients` on `/badges/recipients/1/` in this case.
Do not throw an Aphront404Response as it would make the `Award Badge` button in the dialog inaccessible and keep the dysfunctional overlay dialog displayed.
```
EXCEPTION: (AphrontParameterQueryException) Array for %Ls conversion is empty. Query: badges.phid IN (%Ls) at [<phorge>/src/infrastructure/storage/xsprintf/qsprintf.php:383]
```
Closes T15825
Test Plan: Go to a user's badges, click the `Award Badge` button, in the dialog do not select any Badge and click the `Award` button.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15825
Differential Revision: https://we.phorge.it/D25636
Summary:
`PhabricatorApplicationTransactionCommentView` performs `$uri = id(new PhutilURI('/login/'))->replaceQueryParam('next', (string)$this->getRequestURI())` but the constructor call in `PhabricatorEditEngine` does not set any `RequestURI` so the `Log In to Comment` button at the bottom of an object view does not redirect back to the object URI after login.
Thus pass a `RequestURI` to `PhabricatorApplicationTransactionCommentView` in the constructor call in `PhabricatorEditEngine`.
Closes T15837
Test Plan: As a Phorge admin, set `policy.allow-public` to true and create a Maniphest task with `Public` view policy. As an anonymous user, visit the task and click the `Log In to Comment` button at the bottom of the single task view, log in, and see that you get redirected to the task.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15837
Differential Revision: https://we.phorge.it/D25667
Summary:
If a single transaction performs column moves on multiple different boards (which is permitted in the API), the rendering logic in the feed currently fails. Make it render properly.
Same fix as rPa3f4cbd7484b591425e83bbfc7642bccb04d0d57 for `getTitle()` which left out also fixing `getTitleForFeed()`.
Test Plan: Carefully read the if/else logic.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25646
Summary:
Now anonymous people can visit this page:
http://phorge.localhost/home/menu/new/custom/motivator/
And you do not see anymore this crash:
Undefined variable $custom_phid
phorge applications/search/engine/PhabricatorProfileMenuEngine.php : 61 PhutilErrorHandler::handleError()
phorge applications/search/engine/PhabricatorProfileMenuEngine.php : 916 PhabricatorProfileMenuEngine::getEditModeCustomPHID()
phorge applications/search/engine/PhabricatorProfileMenuEngine.php : 269 PhabricatorProfileMenuEngine::buildItemNewContent()
phorge applications/home/controller/PhabricatorHomeMenuItemController.php : 39 PhabricatorProfileMenuEngine::buildResponse()
phorge aphront/configuration/AphrontApplicationConfiguration.php : 284 PhabricatorHomeMenuItemController::handleRequest()
phorge aphront/configuration/AphrontApplicationConfiguration.php : 203 AphrontApplicationConfiguration::processRequest()
Closes T15867
Test Plan:
Visit the mentioned page as logged-in. It still works.
Visit the mentioned page as logged-out. No crash anymore. You are redirected to the login page.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15867
Differential Revision: https://we.phorge.it/D25700
Summary:
The Remarkup Help page is currently pointing to a HTML page on we.phorge.it.
Because of this it can not be customized/extended by custom extensions.
This diff allows you to implement a `RemarkupSyntaxDocumentationProvider` interface
to your custom class, which will implement a getDocumentation method.
This getDocumentation should return the custom help text in Remarkup format.
This diff also contains some extra help for the cowsay and figlet interpreters.
The local URLs for the local references are:
* /reference/remarkup/
* /reference/cowsay/
* /reference/figlet/
Closes T15401
Test Plan: Open the Phriction wiki editor and click on the Book (Remarkup Reference) icon in the toolbar and validate URL afterwards
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15401
Differential Revision: https://we.phorge.it/D25698
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/transactions/editengine/PhabricatorEditEngine.php:1923]
```
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/transactions/editengine/PhabricatorEditEngine.php:2023]
```
Closes T15864
Test Plan: See steps in https://we.phorge.it/T15864
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15864
Differential Revision: https://we.phorge.it/D25699
Summary:
Premising that I still consider Almanac, Drydock and Harbormaster as dark magic, and premising that all my knowledge about these components is here:
https://en.wikibooks.org/wiki/Phabricator_Administrator%27s_Handbook/Continuous_integration
(Above guide linked from [[Community Resources]])
Hoping to be useful, I've added some welcome pages, precious to newcomers, with the goal to lower a bit the learning barrier to these tricky components.
## Drydock Blueprints
Screenshot of the new welcome page in {nav Drydock > Blueprints}:
{F2243109}
Since Drydock Blueprints are deeply dependent to {nav Almanac}, we added an extra blue button to see its homepage.
So, you are then 2-clicks away from...
## Almanac Services
Screenshot of the new welcome page in {nav Almanac > Services}:
{F2243110}
Since Almanac Services are deeply dependent to {nav Almanac > Devices}, we added an extra blue button to see them.
So, you are then 1-click away from...
## Almanac Devices
Screenshot of the new welcome page in {nav Almanac > Devices}:
{F2243112}
Since Almanac Devices are deeply dependent to {nav Almanac > Networks}, we added an extra blue button to see them.
So, you are then 1-click away from...
## Almanac Networks
Screenshot of the new welcome page in {nav Almanac Networks}:
{F2243111}
And, that's it.
## Notes
Ref T15018
----
After this change, at least 1 newcomer will love Phorge at the maximum level. and they will never again be able to use the proprietary GitHub.
bwahaha
Test Plan:
Visit all the following pages, forcing the welcome "new user interface" mode:
http://phorge.localhost/drydock/blueprint/?nux=1http://phorge.localhost/almanac/service/?nux=1http://phorge.localhost/almanac/device/?nux=1http://phorge.localhost/almanac/network/?nux=1
Test all action buttons.
Write a comment with "yesyes" or "bwahaha" or "chadyes" in production too.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15018
Differential Revision: https://we.phorge.it/D25675
Summary:
`PhabricatorDashboardPortalProfileMenuEngine` calls `$this->newEmptyValue()` but neither `PhabricatorDashboardPortalProfileMenuEngine` nor its parent `PhabricatorProfileMenuEngine` define a `newEmptyValue()` method.
Thus use existing `newNoContentView()` in parent `PhabricatorProfileMenuEngine` (which according to its string output is supposed to cover this case).
Test Plan:
Go to a Portal with no content. Before applying the patch, Phorge shows the message defined in the parent class `PhabricatorProfileMenuEngine`: `No Content`; `No visible menu items can render content.`
After applying this change, it correctly shows the message defined in `PhabricatorDashboardPortalProfileMenuEngine`: `No Portal Content`; `None of the visible menu items in this portal can render any content.`
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25648
Summary:
Display a placeholder text in the text comment field of a Maniphest task if the task status has been set to Duplicate.
This makes it clearer to users (who may have not checked the task status at the top of the page) not to fragment conversations.
Closes T15749
Test Plan:
* Be logged in and go to a task which is closed as a duplicate and see the placeholder text in the field to add a comment.
* Be logged in and go to tasks which are not closed as a duplicate and see no placeholder text in the field to add a comment.
* Go to other places whose code calls a `PhabricatorApplicationTransactionCommentView` constructor and check that it still renders correctly, for example Ponder in http://phorge.localhost/Q1, Slowvote in http://phorge.localhost/V1, Differential in http://phorge.localhost/D1
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15749
Differential Revision: https://we.phorge.it/D25546
Summary: I obviously failed to run `./bin/celerity map` one more time before running `arc diff`. Sorry for that.
Test Plan: Run `./bin/celerity map` and see differences.
Reviewers: O1 Blessed Committers, 20after4, chris
Reviewed By: O1 Blessed Committers, 20after4, chris
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25697
Summary:
Update the d3 library to its last 6.x version available on https://github.com/d3/d3/releases
This also requires updating the tooltip event handling of dots in `Chart.js` to avoid an `Uncaught TypeError: d3.event is undefined` per https://observablehq.com/@d3/d3v6-migration-guide#event-management linked from https://github.com/d3/d3/releases/tag/v6.0.0
Closes T15820
Test Plan:
* Enable the Facts application, go to the Reports of a Project with task changes over time, look at charts, hover over data points, read the tooltip - e.g. on http://phorge.localhost/project/reports/1/ or http://phorge.localhost/maniphest/report/burn/
* Check HTML source of above URIs for the `<script type="text/javascript">` loading `d3.min.js` and open the JS file to verify the d3 version number bump.
* Check Console of web browser's developer tools for no errors.
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15820
Differential Revision: https://we.phorge.it/D25631
Summary:
The PHPDoc in https://we.phorge.it/source/phorge/browse/master/src/infrastructure/storage/lisk/LiskMigrationIterator.php$14 defines `* @task storage`.
That means the value of `$task` in the DivinerAtomController class is the single string `storage` and there is no whitespace to `explode` on and there is no second key in the array to set as `$title`.
Thus only call `explode` when `$task` contains a whitespace.
```
EXCEPTION: (RuntimeException) Undefined array key 1 at [<arcanist>/src/error/PhutilErrorHandler.php:273]
#0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/diviner/controller/DivinerAtomController.php:450]
```
Closes T15854
Test Plan: Go to https://we.phorge.it/book/contrib/article/database/#primary-keys and click the `LiskMigrationIterator` link pointing to https://we.phorge.it/book/dev/class/LiskMigrationIterator/
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15854
Differential Revision: https://we.phorge.it/D25690
Summary:
Allow "Standard" (config-based") custom fields to be displayed in search-results.
Depends on D25548. Ref T15750.
Test Plan: Set `maniphest.custom-field-definitions` to the value of P32, and start playing with custom values on tasks.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15750
Differential Revision: https://we.phorge.it/D25549
Summary:
Display same items in mobile menu on User Profile page as on desktop. (The code did not make too much sense anyway here?)
Also remove special but dysfunctional "Calendar" item in mobile view: The resulting URI does not work even if the calendar application is installed. If it ever worked, it should be moved to the default desktop sidebar anyway and not be a mobile-only menu item.
Closes T15224
Test Plan: Open http://phorge.localhost/people/ and http://phorge.localhost/p/username and http://phorge.localhost/people/manage/1/, change screen width to less and more than 920px, check at menu items in left bar (>920px) and right upper corner (<920px).
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15224
Differential Revision: https://we.phorge.it/D25687
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.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/htdocs/phorge/phorge/src/applications/metamta/adapter/PhabricatorMailSMTPAdapter.php:65]
```
Closes T15857
Test Plan: Send Welcome email via 'Manage' user screen and Run `./bin/phd log` afterwards to validate email action
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15857
Differential Revision: https://we.phorge.it/D25692
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
