Summary:
When I create new repos, I always forget which is which (between Callsign and Short Name)
This adds helpful text as a placeholder and as control instructions
I am not sure if this layout is 100% better, but this is an option as well
**Add new Repository**
{F3169761 width=800}
**Edit existing Repository**
{F3169775 width=800}
Closes T15742
Test Plan: Added a new repository and saw these control instructions
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15742
Differential Revision: https://we.phorge.it/D25898
Summary: Make things a bit easier to understand.
Test Plan: Check parameters and return values for types and classes.
Reviewers: O1 Blessed Committers, Cigaryno, valerio.bozzolan
Reviewed By: O1 Blessed Committers, Cigaryno, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25906
Summary:
The packages base uri is /packages/.
Previously, the base uri pointed to the packages search engine.
Because that prevents navigating to the publishers search engine
and the versions search engine and because the new base uri is a 404,
add a console, similar to Almanac or Nuance to link to these
endpoints. This makes the forms for publishers and versions
accessible without manually navigating there.
Test Plan:
* Clicked on the Packages application in Applications and saw a Console
* Clicked on the options in the console and saw all search engines
* Clicked on the packages crumb and got sent to the console
* Created a package and clicked on Edit Package and didn't see a 404
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25881
Summary: In `HarbormasterManagementRestartWorkflow`, `logSkip()` is called. A method with such a name has never existed in `PhabricatorManagementWorkflow`. Given the five available `log*()` methods, replace the call with `logInfo()` which seems most appropriate.
Test Plan: Run static code analysis which complains about `Call to an undefined method HarbormasterManagementRestartWorkflow::logSkip()`; read the code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Tags: #almanac_drydock_harbormaster
Differential Revision: https://we.phorge.it/D25742
Summary: Make things a bit easier to understand, for the start.
Test Plan: Check return values for their types and 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/D25901
Summary:
Followup to rPf8b8d88
Rename uncalled `newNoMenuItemsView()` to `newNoContentView()` overwriting this method in the parent class, to fix showing more specific UI messages for empty portals.
Test Plan:
Code:
* Grep the code for non-existing `newNoMenuItemsView`
* Check code of parent class `PhabricatorProfileMenuEngine`
Workflows:
* Create an empty portal via http://phorge.localhost/portal/edit/form/default/, see correct (more specific) string "Use "Edit Menu" to add menu items to this portal." displayed
* Click "Edit Menu" to go to http://phorge.localhost/portal/view/1/configure/global/ of that Portal just created, on the right in "Add New Menu Item..." select "Divider" and click "Create Menu Item" button, go back to portal, see correct (more specific) string "None of the visible menu items in this portal can render any content." displayed
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Tags: #dashboards
Differential Revision: https://we.phorge.it/D25776
Summary:
`PhabricatorFeedQuery::withEpochInRange()` returns zero results when passing parameters in the wrong order.
Instead return a PhutilArgumentUsageException which makes it clear why there are no results.
Test Plan:
On an early morning without coffee supply, write custom code like
```
$query = id(new PhabricatorFeedQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withFilterPHIDs(array($user->getPHID()))
->withEpochInRange(time(), time() - 86400)
->setReturnPartialResultsOnOverheat(true);
$stories = $query->execute();
```
and wonder why you get zero results. Optionally, feel stupid for a moment.
Apply patch, now get an "Unhandled Exception ("PhutilArgumentUsageException") - Minimum range must be lower than maximum range."
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/D25891
Summary:
rPd6bce34a5db1a838a988440f09f7728747c9e067 failed to replace all occurrences of `$this->getApplicationURI('update/'.$conpherence->getID().'/')` with `$this->getApplicationURI('edit/'.$conpherence->getID().'/')`. Thus editing a Conpherence room on mobile crashes due to using an old invalid path.
Closes T15513
Test Plan:
* Run `grep -r "update/" . | grep onpherenc` vs `grep -r "edit/" . | grep onpherenc` and read rPd6bce34a5db1a838a988440f09f7728747c9e067
* Go to a Conpherence room in mobile view and select "Edit Room"
* Go to a Conpherence room in desktop view and select "Edit Room"
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15513
Differential Revision: https://we.phorge.it/D25879
Summary:
Passing `null` as the first parameter to `ltrim()` is deprecated since PHP 8.1.
Thus change the optional parameter of the function from null to an empty string.
```
ERROR 8192: ltrim(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/src/applications/nuance/source/NuanceSourceDefinition.php:211]
```
Closes T16000
Test Plan:
* Create a Nuance queue at `/nuance/queue/edit/form/default/`
* Create a Nuance "Web Form" type source at `/nuance/source/edit/form/default/`
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16000
Differential Revision: https://we.phorge.it/D25890
Summary:
The order of items in the main panel on a user profile page is currently (depending on which Phorge applications are installed): `Profile, Badges, Tasks, Revisions, Commits, Manage.`
This order does not reflect the common main interests of most users in my installation who try to find and do work, I'm afraid.
Thus change it to `Profile, Tasks, Revisions, Commits, Badges, Manage.`
Test Plan: Go to http://phorge.localhost/people/manage/1/ and look at the panel on the left. See that Badges is not listed second anymore but is listed last but one.
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/D25876
Summary:
Allow the creation of versions in Packages
by specifying a dummy Package to allow the
extended policy checks to pass. Versions
require that the package they're a version
of be accessible to the viewer, but that
means a package is required during creation.
This also initializes the attached package
property to the un-attached state, which
was missing. This should make it easier to
fix when misused.
Currently trying to create versions in Packages throws:
```
EXCEPTION: (Error) Call to a member function getPHID() on null at [<phorge>/src/applications/policy/filter/PhabricatorPolicyFilter.php:404]
```
Test Plan:
* Visit /applications/view/PhabricatorPackagesApplication/ and enable the Packages prototype application
* Visit /packages/version/edit/form/default
* Observe an edit form instead of an exception
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25882
Summary:
Adds a new filter to the page /token/given/ to specify the type of token awarded.
Closes T15988
Test Plan:
Visited /token/ to see it works as before.
Specified a token type in the search menu, and checked that the results only include that token type.
Activate DarkConsole and inspect the Services tab to check the query with "burminate" has sense, and you see this:
SELECT * FROM `token_given` WHERE (tokenPHID IN ('PHID-TOKN-misc-4')) ORDER BY `id` DESC LIMIT 101
Removed the token type in the search menu, the results still include everything, like before.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15988
Differential Revision: https://we.phorge.it/D25865
Summary:
Now the page /token/given/ allows to sort tokens by newest (default) and oldest.
The default sort is unchanged.
The given tokens are also now easily usable into any Dashboard.
This introduces creative space to add future filters.
refs T15988
Test Plan:
Tested that /token/ still renders fine.
Order by Creation (Newest First): it works.
Order by Creation (Oldest First): it works.
Activate the DarkConsole's top bar and open the tab Services to inspect the generated queries, that are like this, and not anything alien,
accordingly to the current "order by":
SELECT * FROM `token_given` ORDER BY `id` DESC LIMIT 101
SELECT * FROM `token_given` ORDER BY `id` ASC LIMIT 101
Test the page /token/given/ as logged-out: it the Tokens app is configured as public, it still works like before.
From the page /token/given/ order by "Creation (oldest first)", use {nav Use Results > Add Dashboard} and see that it works.
From the same Dashboard: {nav Create Panel > Query Panel} and select "Tokens Given" with Limit=1 and title "Most Recent Tokenzzz" and see that it works as expected.
Visit the Leader Board page at /token/leaders/: it still works like before.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15988
Differential Revision: https://we.phorge.it/D25863
Summary:
Going to e.g. the workboard of an archived project, there is no indication when the project has been archived (the blurred project icon is only displayed if the viewport width is >920px). This can lead to confusion why a workboard is completely empty.
Thus render an archived project as strike-through in the navigation bread crumbs.
Closes T15890
Test Plan:
* Go to the {Profile, Workboard, Reports, Members, Subprojects, Reports} pages of an active and an archived project and look at the navigation crumbs bar below Phorge's global top bar.
* Create an active subproject and active milestone of an archived project (and vice versa) and look at the navigation crumbs.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Tags: #projects
Maniphest Tasks: T15890
Differential Revision: https://we.phorge.it/D25774
Summary:
Trying to set a large image as a project profile image, Phorge displays the "it is a mystery" placeholder image without errors or explanation.
Thus communicate the maximum file dimensions for transforming thumbnails, like Phorge already does for supported file format types.
Closes T15984
Test Plan:
* Go to http://phorge.localhost/project/picture/1/ and http://phorge.localhost/people/picture/1/ and set an image with 4096×4096px and an image with 4097×4097px and observe resulting image.
* Apply this patch, then go to http://phorge.localhost/project/picture/1/ and http://phorge.localhost/people/picture/1/ and see the additional "Maximum image dimensions: 4096×4096 pixels." in the "Upload New Picture" section, set an image with 4096×4096px and an image with 4097×4097px.
* Also test on http://phorge.localhost/phame/post/header/1/ and http://phorge.localhost/phame/blog/header/1/ but realize that these codepaths do not transform larger images, thus no problem. I remain clueless how to trigger PhameBlogProfilePictureController, DiffusionRepositoryProfilePictureController with similar code.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15984
Differential Revision: https://we.phorge.it/D25862
Summary: Calls to this private function were removed in https://we.phorge.it/rPeded19a5c6d3e5fa4e0d655bc8271d2a5baa5a34.
Test Plan: Grep the code; run static code analysis.
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/D25787
Test Plan:
* Create a new phriction document, observe the history
* Apply this patch
* Observe the history again, noting that the word "document" now replaces "object"
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25858
Summary:
| Before | After |
|----------|----------|
|{F2669862}|{F2669861}|
Closes T15970
Test Plan:
* Go to http://phorge.localhost/applications/view/PhabricatorPhurlApplication/ and see two additional settings "Default View Policy" and "Default Edit Policy"
* Go to http://phorge.localhost/applications/view/PhabricatorPhurlApplication/ and select "Edit Policies" to see and set two additional settings
* Go to http://phorge.localhost/phurl/url/edit/form/default/ and check the "Visible To" and "Editable By" fields now showing the default page
* Check existing Phurls which did not have their policies changed as that's stored in the database
* run `arc liberate` again and see that it's already updated
Reviewers: O1 Blessed Committers, aklapper, valerio.bozzolan
Reviewed By: O1 Blessed Committers, aklapper, valerio.bozzolan
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15970
Differential Revision: https://we.phorge.it/D25850
Summary:
When viewing the history of a Mercurial diffusion repository the server return a 500. This is
because a function call to array_slice requires the offset to be set to an integer. So when it is
not specified it can be set to 0 as a default.
The error is as follows:
```
ERROR 8192: array_slice(): Passing null to parameter #2 ($offset) of type int is deprecated at [/usr/local/www/phorge/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:167]; PHP message: arcanist(head=master, ref.master=3cb117684f4e), phorge(head=master, ref.master=4bf5c452eb28); PHP message: #0 array_slice(array, NULL) called at [<phorge>/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php:167]
```
Test Plan: The history tab should not return a 500 when not specifying an offset
Reviewers: O1 Blessed Committers, valerio.bozzolan, speck
Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck
Subscribers: aklapper, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25673
Summary:
Make sure that Repository Diffusion Identities "Automatically Detected User " are not created from unverified emails.
Closes T15965
Test Plan:
Find at least one identity that is assigned to nobody:
http://phorge.localhost/diffusion/identity/
(For example, you may easily find an identity of "GitHub <noreply@github.com>")
(Double check that its "Assigned To" is unset or make sure it's unset for this test)
Be evil: add *that* email in your {nav Profile > Settings > Email addresses}. So, for example add "noreply@github.com", like a rogue. The email can stay unverified.
Run this command to immediately cause an effect:
./bin/repository rebuild-identities --all-identities
- before this change, you can reproduce that you successfully steal that identity and you become "GitHub" or whoever
- after this change, you see that "Automatically Detected User" is unset again
- after this change, any other identity manually assigned, is still assigned to that value
- after this change, any other identity automatically assigned to verified emails, are still "Automatically Detected User"
Reviewers: O1 Blessed Committers, speck, 20after4
Reviewed By: O1 Blessed Committers, speck, 20after4
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15965
Differential Revision: https://we.phorge.it/D25845
Summary:
Closes T15966
Force Diffusion commands to be executed in the "LC_ALL=C" language mode, that is, English,
defusing whatever custom language you have in your LANGUAGE variable.
Related reading:
https://we.phorge.it/T15872
Test Plan:
Make sure Subversion is installed on your system. Explicitly set `LC_ALL`.
Finally, this unit test works, even if you have something esoteric in your LANGUAGE:
arc unit ./src/applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php
Before, it was crashing, like mentioned in T15966.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15966
Differential Revision: https://we.phorge.it/D25846
Summary:
Uploaded an example email with a lot of accents called 'test_accents.mbox' and
expected headers in the file 'test_accents.headers.txt'.
Better than nothing.
This change also includes a minor refactor in the library loading.
Ref T15960
Test Plan:
Manually run the new unit test and see green lights:
arc unit src/applications/metamta/externals/__tests__/PhabricatorExternalMimeMailParserTestCase.php
Double-check that the new class is already recorded:
arc liberate
Just as extra care, re-apply the same test plan of:
D25839
So, for example, run this, and see no exceptions:
./scripts/mail/mail_handler.php < src/applications/metamta/externals/__tests__/data/test_accents.mbox
Reviewers: aklapper, taavi, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15960
Differential Revision: https://we.phorge.it/D25844
Summary: Fixes T15371
Test Plan:
- Save the text `{{#translation:}}` in remarkup and see that it renders.
- Create a project or projects with the hashtags `a`, `b`, `ab`, `foo`, `f.o.o`.
- Observe that both before and after this patch you can link to all of them by hashtag.
- Create a project or projects with the hashtags `a.`, `.b`, `.foo`, `foo.`.
- Observe that both before and after this patch you can link to none of them by hashtag.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15371
Differential Revision: https://we.phorge.it/D25838
Summary:
The "Delete Repository" button should be disabled, just like any other admin button in the page,
if you cannot edit the repository, so to do not cause anxiety situations to users without
editing permissions.
Moreover, probably, if you still want to click that disabled button,
you should still be allowed to see the super-nice related information message,
just because it's super-lovely, super-informative and it does nothing wrong.
Closes T15964
Test Plan:
Be normal user. See the button finally disabled. Click it, you finally see its related info popup instead "You shall not pass",
so you unlock extra documentation to read.
Be admin user. See the button still enabled. Click it, you still can see the info popup.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15964
Differential Revision: https://we.phorge.it/D25843
Summary:
Avoid a database exception at user account registration when users enter very long real names by setting a maximum length.
This does not affect existing account data as it is only called in the account registration code.
Fixes T15962
Test Plan: Go to http://phorge.localhost/auth/register/ and enter long values into the "Real Name" field
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: l2dy, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15962
Differential Revision: https://we.phorge.it/D25841
Summary:
Custom Select field values are rendered as `Unknown Object (????)` when set as Herald rule conditions and editing the Herald rule.
This is unhelpful; at least display their key value.
Closes T15860
Test Plan: Create a `"type": "select"` custom field via `/config/edit/maniphest.custom-field-definitions/`, set up a Herald rule with a condition based on that custom field, edit the Herald rule.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15860
Differential Revision: https://we.phorge.it/D25696
Summary:
Calendar search form allows users to define date ranges. Entering gibberish data leads to a cryptic exception due to calling `format()` on `null`, as `AphrontFormDateControlValue::getDateTime()` can return `null` instead of a `DateTime` object.
Also add some additional PhpDoc as a result of playing with this code.
Note that other calendar query forms are more lenient and still accepts gibberish after applying this patch. The intention behind this patch is replacing a cryptic exception with a more appropriate and descriptive error; this patch does not attempt to introduce validation everywhere.
```
EXCEPTION: (Error) Call to a member function format() on null at [<phorge>/src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php:469]
```
Closes T15943
Test Plan:
* On http://phorge.localhost/calendar/query/month/, click "Edit Query", check "Occurs After", replace default date value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/month/, click "Edit Query", check "Occurs After", replace default time value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/month/, click "Edit Query", check "Occurs Before", replace default date value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/month/, click "Edit Query", check "Occurs Before", replace default time value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/day/, click "Edit Query", check "Occurs After", replace default date value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/day/, click "Edit Query", check "Occurs After", replace default time value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/day/, click "Edit Query", check "Occurs Before", replace default date value with "abcde", click "Search" button
* On http://phorge.localhost/calendar/query/day/, click "Edit Query", check "Occurs Before", replace default time value with "abcde", click "Search" button
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15943
Differential Revision: https://we.phorge.it/D25825
Summary:
Include task title and metadata in Conduit's `maniphest.gettasktransactions` output.
This data can for example be used by IRC bots to improve their output (e.g. conditions which changes [not] to emit or text coloring based on metadata).
Upstreaming from https://phabricator.wikimedia.org/rPHABf2fd14dc1edeb41aa2874336548cfaa7fa0e87a0
NOTE: this specific API is in "frozen" status. This method is frozen and will eventually be deprecated. New code should use "transaction.search" instead.
Closes T15916
Test Plan: Call `maniphest.gettasktransactions` on a task, see two additional fields.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15916
Differential Revision: https://we.phorge.it/D25777
Summary:
Per https://www.php.net/manual/en/function.ldap-connect.php the signature `$conn = @ldap_connect($host, $this->port);` is deprecated since PHP 8.3.
Thus pass a full LDAP URI as the only parameter.
```
ERROR 8192: Usage of ldap_connect with two arguments is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/adapter/PhutilLDAPAuthAdapter.php:308]
```
Closes T15724
Test Plan: Set up LDAP as auth provider, try to log in.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15724
Differential Revision: https://we.phorge.it/D25792
Summary:
Due to code additions in rPcb957f8d and rPadbd7d4f required due to rPd321cc81, the code intertwines handling legacy data with handling/creating modern data.
Make things more understandable by clearly separating between both (handle one after the other) and by renaming some variables for clarity, so it will become slightly easier in the future to investigate this bottleneck (it is the only code querying the ManiphestTransaction table, leading to timeouts in large Phorge installations).
Also add a specific reference to the corresponding code change in a code comment, instead of a vague "late 2017".
Also, don't use the variable name `$table` for two different things (database vs AphrontTableView) in the same function.
Test Plan: Carefully read the code. Optionally, play with http://phorge.localhost/maniphest/report/burn/ with and without setting a project filter having tasks created in the codebase before 2017-11-22, and compare that the output is still the same.
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25828
Summary:
Followup to rPdb61eb20 and rPf3d49f74. This patch should cover all remaining issues now that PHPStan covers it (instead of the previous trial-and-error approach).
Implicitly nullable parameter declarations are deprecated in PHP 8.4:
https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated
The proposed syntax was introduced in PHP 7.1 and Phorge requires PHP 7.2 now.
Test Plan: Run static code analysis.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25832
Summary:
Implicitly nullable parameter declarations are deprecated in PHP 8.4:
https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated
The proposed syntax was introduced in PHP 7.1.
This patch is a followup on top of D25814.
Refs T15935
Test Plan:
* Try to view a task in the browser
* Try to run `./bin/auth recover` on the CLI
* Try to browse projects in the browser
* Try to create a new project without a name in the browser
Reviewers: O1 Blessed Committers, chris
Reviewed By: O1 Blessed Committers, chris
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15935
Differential Revision: https://we.phorge.it/D25816
Summary:
Implicitly nullable parameter declarations are deprecated in PHP 8.4:
https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated
The proposed syntax was introduced in PHP 7.1.
Note: This code change does *not* fix all occurrences but only allows passing `./bin/storage upgrade`.
Refs T15935
Test Plan: Try to successfully run `./bin/storage upgrade` with PHP 8.4
Reviewers: O1 Blessed Committers, chris
Reviewed By: O1 Blessed Committers, chris
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15935
Differential Revision: https://we.phorge.it/D25814
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/PhabricatorJupyterDocumentEngine.php:326]
```
Closes T15951
Test Plan: See T15951
Reviewers: O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15951
Differential Revision: https://we.phorge.it/D25830
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 [/srv/phorge/phorge/src/applications/diffusion/query/DiffusionCommitQuery.php:779]
```
Closes T15936
Test Plan: Unclear.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15936
Differential Revision: https://we.phorge.it/D25821
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.
| Before | After |
|---------------------|---------------------|
| {F313288,size=full} | {F313289,size=full} |
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.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15489
Differential Revision: https://we.phorge.it/D25824
Summary:
Passing null instead of a string or array to `str_replace()` deprecated since PHP 8.1.
Thus do not create a title array with a `null` entry in `DivinerFindController` when there is no `$query_text`, later to be read via `$this->titles` in `DivinerAtomRef`.
```
ERROR 8192: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated at [/var/www/html/phorge/phorge/src/applications/diviner/atom/DivinerAtomRef.php:205]
#0 str_replace(string, string, NULL) called at [<phorge>/src/applications/diviner/atom/DivinerAtomRef.php:205]
#1 DivinerAtomRef::normalizeTitleString(NULL) called at [<phorge>/src/applications/diviner/query/DivinerAtomQuery.php:344]
```
Credits to valerio.bozzolan for finding the right spot in the code.
Closes T15911
Test Plan: Go to http://phorge.localhost/diviner/find/ (not passing a `name` URI parameter), optionally with D25768 applied to avoid another exception
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15911
Differential Revision: https://we.phorge.it/D25769
Summary:
Per https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md the valid keyword is `$this`.
Thus replace `this` to make output of static code analysis slightly less noisy.
(See rARC4b7ee1985b3c15b4af81b5480522ae24c0afd6a2 for its Arcanist counterpart.)
Test Plan: Read the docs.
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/D25819
Summary:
The question mark in `@param type? $foo Desc` is a custom notation not consistently applied across the codebase and not necessarily obvious to the reader (because custom and not mentioned in https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md ).
Instead, explicitly state "optional" in the parameter description for clarity.
Closes T15925.
Test Plan: Run PHPStan, see no `PHPDoc @param has invalid value (type? [...]` style output anymore.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15925
Differential Revision: https://we.phorge.it/D25797
Summary:
It is only possible `throw` an `Exception` class. `Aphront400Response` is not an Exception class but a `Phobject`.
Thus `return` it, like the `Aphront404Response`s within the same method.
Test Plan:
Check parent classes; run static code analysis. Hack the code to force that return,
and successfully obtain an HTTP 400 status code 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/D25780
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/provider/PhabricatorOAuth1AuthProvider.php:163]
```
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorOAuth1AuthProvider.php:178]
```
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/provider/PhabricatorOAuthAuthProvider.php:147]
```
Closes T15912
Test Plan: As an admin, create an OAuth provider (such as using Bitbucket) and check the error logs or Dark Console.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15912
Differential Revision: https://we.phorge.it/D25771
