commits
Summary:
In Diffusion repositories, project tags associated with the repository are only shown on the "Manage > Edit Basic Information" page. (?)
Also display them in the repositories main page, alongside the existing tags (Status, Visibility).
Note that a project tag does not line-wrap, so a long project name can move the "Pattern Search" field out of the viewport on smaller screen widths due to T16287 and `.phui-tag-view {white-space: nowrap;}`.
Closes T15526
Test Plan:
1. Create a Diffusion repository; create some project tags
2. Go to http://phorge.localhost/diffusion/36/manage/ and select "Edit Basic Information"
3. Under "Tags", add several project tags, and "Save Changes"
4. Go to http://phorge.localhost/diffusion/36/ and see the project tags displayed next to the "Active/Disabled" and "Public/All Users/Custom Policy" tags.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15526
Differential Revision: https://we.phorge.it/D26735
Summary:
On small screens, the view policy tag in the page subheader (e.g. "Public", "All Users", "Custom Policy", "Administrators") can get wrapped across three lines.
Avoid that and keep the tag on one line.
Closes T16287
Test Plan:
Look at tags in subheaders on object pages in all three different CSS layouts that Phorge offers, ideally with "long" tags
* http://phorge.localhost/p/user/ (e.g. with a user who also has MFA enabled)
* http://phorge.localhost/diffusion/1/
* http://phorge.localhost/book/flavor/
* http://phorge.localhost/T1 (e.g. with Story Point tags also enabled)
* http://phorge.localhost/D1
* http://phorge.localhost/phame/blog/view/1/
* http://phorge.localhost/w/somepage/
* http://phorge.localhost/book/dev/class/PhorgeI18nTestCase/#method/key (Diviner; the "Inherited" tag uses this CSS)
* http://phorge.localhost/M1
* http://phorge.localhost/F1
* http://phorge.localhost/B1
* http://phorge.localhost/C1
* http://phorge.localhost/uiexample/view/PHUIBoxExample/ ("Fancy Box" example at the bottom)
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16287
Differential Revision: https://we.phorge.it/D26736
Summary:
This adds CSS to prevent the document view from overlapping to the blogs and drafts boxes in the right column.
Ref T16473
Test Plan:
- Browse to phame home view using a screen width of at least 1792px
- See that now the links to the individual blogs are fully clickable
- See that all other views (Phriction and Phame posts) are unchanged
Reviewers: #blessed_committers, O1 Blessed Committers, aklapper
Reviewed By: #blessed_committers, O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16473
Differential Revision: https://we.phorge.it/D26715
Summary:
Do not allow setting an invalid project color via the `project.edit` Conduit API but validate the value.
Same game as in D26430, D26459.
Closes T16237
Test Plan:
* Run `echo '{"transactions":[{"type":"name","value":"projectWithInvalidColor"},{"type":"color","value":"purpleyellowgreyish"},{"type":"description","value":"project project new new new"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" project.edit --`
* Succeed before applying this patch
* Fail after applying this patch:
```
{
"error": "ERR-CONDUIT-CORE",
"errorMessage": "ERR-CONDUIT-CORE: <project.edit> Validation errors:\n - Value for \"project:color\" is invalid: \"purpleyellowgreyish\".",
"response": null
}
```
* Still succeed with a valid name like `{"type":"color","value":"checkered"}`
* Manually set a color in the project using the frontend. It works.
* Change every single other color, like Red, Orange, Yellow, ... Grey, Checkered, they all works
* Set color to Orange. Archive the project. Activate the project. It still works, you still see the original color Orange.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16323, T16237
Differential Revision: https://we.phorge.it/D26460
Summary:
Do not allow setting an invalid project icon via the `project.edit` Conduit API but validate the value.
Same game as in D26430.
Closes T16322
Test Plan:
* Run `echo '{"transactions":[{"type":"name","value":"projectWithInvalidIcon"},{"type":"icon", "value":"noexxxist"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" project.edit --`
* Succeed before applying this patch
* Fail after applying this patch:
```
{
"error": "ERR-CONDUIT-CORE",
"errorMessage": "ERR-CONDUIT-CORE: <project.edit> Validation errors:\n - Value for \"project:icon\" is invalid: \"noexxxist\".",
"response": null
}
```
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16322
Differential Revision: https://we.phorge.it/D26459
Summary:
Do not allow setting an invalid user icon via the `user.edit` Conduit API but validate the value.
This code path is executed because `PhabricatorApplicationTransactionEditor::validateTransaction()` for `case PhabricatorTransactions::TYPE_CUSTOMFIELD` calls `$field->validateApplicationTransactions` and because `PhabricatorUserIconField` is a child class of `PhabricatorCustomField`.
Closes T16307
Test Plan:
* Run `echo '{ "objectIdentifier":"@myusername", "transactions":[{"type":"icon","value":"nonexistingicon"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost/ --conduit-token "cli-mytoken" user.edit --` before and after this, and get a new error after this patch:
```
{
"error": "ERR-CONDUIT-CORE",
"errorMessage": "ERR-CONDUIT-CORE: <user.edit> Validation errors:\n - Value for \"Icon\" is invalid: \"nonexistingicon\".",
"response": null
}
```
* Optionally, check database value of `SELECT u.userName, up.icon FROM phabricator_user.user_profile up JOIN phabricator_user.user u ON up.userPHID = u.phid WHERE u.userName = "myusername";` in the database before and after applying patch and compare what `PhabricatorPeopleIconSet` actually offers
* Run `echo '{ "objectIdentifier":"@myusername", "transactions":[{"type":"icon","value":"spy"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost/ --conduit-token "cli-mytoken" user.edit --` and still successfully change your user icon
* Run `echo '{ "objectIdentifier":"@notmyusername", "transactions":[{"type":"icon","value":"spy"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost/ --conduit-token "cli-mytoken" user.edit --` and still get the expected error that you are not that user
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16307
Differential Revision: https://we.phorge.it/D26430
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
According to https://web.archive.org/web/20171001040455/https://www.php.net/manual/en/language.types.array.php, `Null will be cast to the empty string, i.e. the key null will actually be stored under ""` at least since PHP 7.2, which we require.
Closes T16495
Test Plan: Run `../arcanist/bin/arc unit ./src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php` in PHP 8.5
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16495
Differential Revision: https://we.phorge.it/D26748
Summary:
Throw a ConduitException when no objectPHID is passed instead of silently failing when trying to delete a token being `null`.
As a side effect, fix a PHP 8.5 deprecation warning.
Closes T16436
Test Plan: Go to http://phorge.localhost/conduit/method/token.give/ and press "Call Method". Try also after entering a random string in the "tokenPHID" field.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16436
Differential Revision: https://we.phorge.it/D26647
Summary:
Bump our copy of external mimemailparser library from 8.0.4 to 9.0.1.
See https://github.com/php-mime-mail-parser/php-mime-mail-parser/releases for the upstream changelog.
Closes T16458
Test Plan: tbd
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16458
Differential Revision: https://we.phorge.it/D26685
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
```
ERROR 8192: Using null as an array offset is deprecated, use an empty string instead at [/var/www/html/phorge/phorge/src/applications/macro/markup/PhabricatorIconRemarkupRule.php:70]
```
Closes T16446
Test Plan: Unknown
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16446
Differential Revision: https://we.phorge.it/D26764
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
```
ERROR 8192: Using null as an array offset is deprecated, use an empty string instead at [/var/www/html/phorge/phorge/src/applications/project/engine/PhabricatorProjectEditEngine.php:128]
```
Closes T16504
Test Plan:
* PHP 8.5
* Go to an existing parent project
* Select "Subprojects" in the sidebar
* Click "Create Milestone" on the right
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16504
Differential Revision: https://we.phorge.it/D26765
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/legalpad/xaction/LegalpadDocumentTitleTransaction.php:22]
```
Closes T16505
Test Plan:
* Create a Legalpad at http://phorge.localhost/legalpad/edit/
* Go to http://phorge.localhost/feed/transactions/
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16505
Differential Revision: https://we.phorge.it/D26766
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
See the same game nine lines lower.
Closes T16497
Test Plan: Run `./bin/arc unit ./src/applications/differential/__tests__/DifferentialParseRenderTestCase.php` on PHP 8.5
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16497
Differential Revision: https://we.phorge.it/D26763
Summary:
Link the new "Policies User Guide" added in rPc5e33a500c19 also from the "Understanding Policies" section in the Forms and Projects guides.
Followup to rPc5e33a500c19.
Test Plan: `./bin/diviner generate`
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26762
Summary:
Remove code around some conditions which are always true/false anyway.
Same game as rP1b9fafc7af72.
Test Plan: Read earlier code in each file; run static code analysis.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26760
Summary: In prep for T15277, write some user docs about the existing Policies - how they work and how to use them.
Test Plan: look at it until blurry.
Reviewers: O1 Blessed Committers, valerio.bozzolan, aklapper
Reviewed By: O1 Blessed Committers, valerio.bozzolan, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26759
Summary:
Given the absolute CSS position of the global top bar search parent object, there is no reason to truncate search scope names that early.
Change `width: 200px` to `max-width: 260px`.
(I'd also generally like to advertise https://www.w3.org/International/articles/article-text-size.en)
Closes T16369
Test Plan: See steps in T16369; test on different screen widths
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16369
Differential Revision: https://we.phorge.it/D26539
Summary:
Fully modularizing DifferentialDiffTransaction, and then fix T16472 by attaching any binary files uploaded to the diff object.
Refs T16485 T16070.
Test Plan:
Set the default view-policy of new files to something that isn't very public.
Create new diffs using `arc diff` that includes a picture of a small cat. Other kinds of images are out of scope.
- Users that don't have permission to see the revision should not be able to see the file
- Users that have permission to see the revision, should be able to see the file.
- Changing the policies on the created file to "no one" will not be make a difference.
- The file's page will show the diff in the Attachment list.
Also:
- Herald can still block the creation of the diffs.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno, jdelker
Maniphest Tasks: T16485, T16472, T16070
Differential Revision: https://we.phorge.it/D26724
Summary: Some PhpDoc corrections and additions as I've been poking some code.
Test Plan: Read code; run static code analysis.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26756
Summary:
While poking a CSS bug in Diviner I was checking where else `PHUITagView::TYPE_STATE` is used.
Turns out that this use can be removed, as any reading of the $tag variable got removed in rP27e13ea0.
Test Plan: Search for `$tag` within that very file.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26757
Summary: Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
Test Plan:
Run static code analysis:
```
/src/applications/transactions/view/PhabricatorApplicationTransactionView.php:372 Possibly invalid array key type int|string|null.
```
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26679
Summary:
Move the phui-info-view box about Query Errors below the phui-form-view Search form. That's below the `#R` anchor that the view automatically scrolls to. People do not scroll up again to expect an error message up there so show the error message down here.
Closes T16089
Test Plan: Go to http://phorge.localhost/maniphest/query/advanced/ and enter something in the Query field that will trigger a Query Error, such as `help:now` or a very long string. Press the Search button.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16089
Differential Revision: https://we.phorge.it/D26672
Summary:
Going to http://phorge.localhost/maniphest/query/advanced/, after `if ($this->queryKey == 'advanced') { $query_key = $request->getStr('query');` in `PhabricatorApplicationSearchController::processSearchRequest()`, `$query_key` remains null. Afterwards, `if ($engine->isBuiltinQuery($query_key))` calls PhabricatorApplicationSearchEngine::isBuiltinQuery($query_key) which tries `$builtins[$query_key]`.
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
```
ERROR 8192: Using null as an array offset is deprecated, use an empty string instead at [/var/www/html/phorge/phorge/src/applications/search/engine/PhabricatorApplicationSearchEngine.php:757]
```
Closes T16361
Test Plan:
Go to http://phorge.localhost/maniphest/query/advanced/. Enter a term, run a query, save it.
Go to http://phorge.localhost/mail/query/advanced/ which triggers the issue.
Reviewers: O1 Blessed Committers, valerio.bozzolan, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16361
Differential Revision: https://we.phorge.it/D26530
Summary:
Make Phorge also look for the "magick" binary which supersedes "convert" since ImageMagick version 7.
Closes T16474
Test Plan:
1. Rename the commands in the two "Filesystem::binaryExists()" checks to some nonsense
2. As an admin, set http://phorge.localhost/config/edit/files.enable-imagemagick/ to `enabled`
3. See in top bar a new setup warning pointing to http://phorge.localhost/config/issue/files.enable-imagemagick/ about "'magick' or 'convert' binary not found or Imagemagick is not installed."
4. Correct the two commands in the two "Filesystem::binaryExists()" checks, as they are in this patch
5. Upload an animated GIF image file `F1`, go to http://phorge.localhost/file/transforms/1/ and manually trigger a transform by clicking a "Regenerate" button, get a result (optionally insert phlog() statements in PhabricatorFileImageTransform::applyImagemagick() to prove that ImageMagick is used)
6. Probably create some Macro meme? I did not.
7. Check the "Behavioral Changes" section on https://imagemagick.org/script/porting.php and do not spot any commands used in the Phorge codebase
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16474
Differential Revision: https://we.phorge.it/D26742
Summary: Throw a proper error code and error_info, like similar `file.download` and `file.info` already do.
Test Plan:
* Go to http://phorge.localhost/conduit/method/file.uploadchunk and http://phorge.localhost/conduit/method/file.querychunks , click "Call Method"
* Before this patch, get a generic "ERR-CONDUIT-CORE" error code.
* After this patch, get a "ERR-BAD-PHID" error code and the error info `No such file 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/D26754
Summary:
Do not allow to unset the image title from Pholio mock images,
in order to avoid at least 3 related UX bugs shown in T16478.
Also remove the broken method 'shouldHide()',
because it always returns false, which is the default from
the parent call, PhabricatorModularTransactionType#shouldHide().
The 'shouldHide()' was also broken in PHP 8.5.
Also improve one error message to talk about "image titles"
(and not "image names"), so to be consistent with the "Title"
field in the UX.
Thanks to @aklapper for the original troubleshooting.
Closes T16478
Ref T16460
Test Plan:
Create a Pholio mock with at least one image and save.
Try to unset one image title (not the mock title, but the image title)
and enjoy the new validation message: "Mock images must have a name."
Reviewers: aklapper, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, aklapper, Cigaryno
Maniphest Tasks: T16478, T16460
Differential Revision: https://we.phorge.it/D26717
Summary:
It might be vaguely useful to know what happens to your audio
once you press save.
Screenshot after the change:
{F6759938}
Test Plan:
Visit a specific macro/meme from this link:
http://phorge.localhost/macro/
Click on Edit Audio. Enjoy the new caption.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16451
Differential Revision: https://we.phorge.it/D26723
Summary:
Type hints for parameters can be generic, such as `array<string,int>`.
Some of these contain spaces, like `array<string, int>`, which was parsed incorrectly previously.
This resulted in a line like ` * @param map<string, string> $attributes (optional) A map of tag attributes.` rendering as
`map<string, | $attributes | string> $attributes (optional) A map of tag attributes.` instead of
`map<string,string> | $attributes | $attributes (optional) A map of tag attributes.`.
Closes T16499
Test Plan:
* Run `./bin/diviner generate` on git master
* Look at the "Parameters" box on http://phorge.localhost/book/dev/function/phutil_tag/
* Apply this patch
* Run `./bin/diviner generate --clean`
* Look at the "Parameters" box on http://phorge.localhost/book/dev/function/phutil_tag/
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16499
Differential Revision: https://we.phorge.it/D26752
Summary:
`PhabricatorProjectCoreTestCase` creates projects that it doesn't save to the database. As a result, they don't have a PHID. This causes issues because `::getPHID()` will return null instead, which in PHP 8.5 results in deprecation warnings.
Closes T16496
Test Plan:
Run this unit test on PHP 8.5 and see no 'ERROR 8192' anymore:
arc unit src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
Alternatively, for those in the stone age, add a null check to `PhabricatorProjectMembersPolicyRule` for `$object->getPHID()` on lines 34 & 70. (It's me, I'm in the stone age)
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16496
Differential Revision: https://we.phorge.it/D26751
Summary:
As of PHP 8.1.0, calling `setAccessible()` has no effect; all properties are accessible by default.
See https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_reflectionsetaccessible and https://www.php.net/manual/en/reflectionproperty.setaccessible.php
Refs T16494
Test Plan: Run `./bin/arc unit --everything` on PHP 8.5
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16494
Differential Revision: https://we.phorge.it/D26746
Summary:
Do not execute a PhabricatorSavedQueryQuery in the database when a queryKey is passed which is obviously bogus.
Closes T16482
Test Plan:
* Adjust the newly added exception error message, go to http://phorge.localhost/conduit/method/project.column.search/ and enter a "string" in the queryKey field which does not have 12 characters.
* Adjust the newly added exception error message, go to http://phorge.localhost/maniphest/query/nonsense/#R and still get a 404.
* Optionally, check in DarkConsole that there is one less DB query (I did not).
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16482
Differential Revision: https://we.phorge.it/D26720
Summary:
Fix T16483.
1. Fix the page to actually load things
2. Add a way to query for a specific object
3. Add link from object pages to the full history (under "Advanced")
Test Plan:
- Visit `/feed/transactions/` and see looots of entries.
- enter object names and phids into the query, list is filtered to these objects only.
- Visit a random task object, hit Advanced -> View All Transactions, see full history including things that are normally hidden.
Reviewers: aklapper, valerio.bozzolan, O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16483
Differential Revision: https://we.phorge.it/D26732
Summary:
* Last call to private `PhrictionTransactionEditor::setNewContent()` was removed in rP64cee4a9.
* Last call to private `PhabricatorRepository::isSSHProtocol()` was removed in rP24ebac8a.
Test Plan: Grep the codebase.
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/D26740
Summary:
Revert rPbabeed68 which can lead to an error when commenting on a task:
```
Subscriber transactions must be existing PHIDs or usernames (found key "0" on transaction of type "core:subscribers").
```
Looks like Phorge is pretty inconsistent in its array keys when passing subscriber arrays around - sometimes the keys are default integers, sometimes they duplicate the value.
Closes Q238
Reopens T16311
Test Plan:
* Comment on a task which you are not yet subscribed to via the web interface.
* Follow test plan of original rPbabeed68 for the Conduit part.
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16311
Differential Revision: https://we.phorge.it/D26741
Summary:
Fixes the bug in the third paragraph of D25861#39133
Depends on D25861
Test Plan: Follow the test plan setup for D25861. Try to edit an event and see that you can now edit it normally and don't get "end date is invalid"/"start date is invalid" errors.
Reviewers: aklapper, O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26544
Summary: Fixes T15811. Alternative to D25618
Test Plan: Observe that the lengthy steps to reproduce in T15811 no longer reproduce. Observe that translated date strings (e.p.p.) still appear in most parts of the interface
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: avivey, aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15811
Differential Revision: https://we.phorge.it/D25861
Summary: Closes T16398
Test Plan: Go to some object's timeline, look at the timestamp anchor, also click that timestamp to get background highlighting.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16398
Differential Revision: https://we.phorge.it/D26594
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/feed/conduit/FeedQueryConduitAPIMethod.php:79]
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/feed/conduit/FeedQueryConduitAPIMethod.php:84]
```
Closes T16419
Test Plan:
* PHP 8.1+
* Go to http://phorge.localhost/conduit/method/feed.query/
* Press the "Call Method" button
* Go to http://phorge.localhost/conduit/method/feed.query/ and enter values in the `before` and `after` fields
* Press the "Call Method" button
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16419
Differential Revision: https://we.phorge.it/D26631
Summary:
For years, when editing a Pholio Mock and causing an image title
validation error, then the breadcrumb and the page header were altered.
The cause was a variable, $title, re-used by mistake in this corner case.
After this change, the "Edit Mock: The mock name" is not overwritten by mistake.
Closes T16492
Test Plan:
Edit an already-existing Pholio Mock.
http://phorge.localhost/pholio/edit/1/
In the field "Name", set "The mock name".
Then change the image title (field "Title") setting a very long title,
more than 128 characters, like this:
THE IMAGE VERY LONG TITLE BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLABLA BLA BLA BLA BLA BLA BLA BLA!
Save. You will see the usual error, but now you finally see the normal breadcrumb, like this:
{nav Pholio > M1 > Edit Mock: The mock name}
So, you don't see anymore this nonsense breadcrumb:
{nav Pholio > M1 > THE IMAGE VERY LONG TITLE BLA BLA BLA BLA BLA B...}
Plus, you finally see this page header:
Edit Mock: The mock name
So, you don't see anymore this nonsense page header:
Edit Mock: THE IMAGE VERY LONG TITLE BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLABLA BLA BLA BLA BLA BLA BLA BLA!
Such wow!
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16492
Differential Revision: https://we.phorge.it/D26734
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.
```
strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [PhameBlogParentDomainTransaction.php:18]
strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [PhameBlogFullDomainTransaction.php:25]
```
Ref T15064
Test Plan: Unknown, presumably change the parent domain of a Phame blog. These only became visible after D26732 was applied.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15064
Differential Revision: https://we.phorge.it/D26737
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/PhabricatorAuthNeedsMultiFactorController.php:39]
```
Closes T16491
Test Plan:
* Be logged into Phorge
* Run `/var/www/html/phorge/phorge/bin/config set security.require-multi-factor-auth true`
* Reload a Phorge page to get the content of http://phorge.localhost/auth/enroll/multifactor/ displayed
* Visit also http://phorge.localhost/auth/
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16491
Differential Revision: https://we.phorge.it/D26733
Summary:
For more than 10 years it was possible to troll your coworkers with
infinite audio loops, by typing a macro/meme in a comment box,
previewing, and then writing something else. This was happening
since macros may become hidden but still loaded in the DOM and,
in that case, they may play forever, until the page is destroyed.
The annoying sounds were also continuing when enjoying a meme and then
clicking on a different link, since page loads are often done by ajax,
effectively causing invisible memes in the DOM playing rickroll forever.
After this patch, no more audio from invisible macro/meme, effectively
implementing the beloved "STFU" alghorithm.
https://en.wikipedia.org/wiki/STFU
Note: you may appreciate that the audio volume still decreases
if you scroll up/down distant from a meme.
So, sounds are not really that annoying. Not even audio loops!
If a particular macro is annoying, then scroll down, or file a
severe complaint to your favorite Phorge admin to de-activate
that audio.
This patch is designed after D26674, to be not destructive.
Additionally, done small refactor to avoid 'statics.items[ii].element',
which can be written as just 'item.element'.
Closes T16486
Closes T16451
Test Plan:
Preamble: always click somewhere in the page for all tests,
otherwise modern browser webs like Firefox will likely deny audio,
even if you scroll straight to a meme.
-----
Locally, create a weird image macro (http://phorge.localhost/macro/create/)
for example called "freebird" and upload the sound of an epic
mp3 file, like the solo of Free Bird by Lynyrd Skynyrd, which is
somehow long. Example mp3:
https://quicksounds.com/sound/23840/free-bird-solo
Now, do the tests:
Write 'freebird' in a comment in a preview. Enjoy audio.
Remove 'freebird' from the preview, enjoy NO AUDIO \o/
Write 'freebird' in a comment and save. Enjoy audio.
Click a different link (e.g. 'Edit Task'). enjoy NO AUDIO \o/
Check for regressions in normal workflows:
Write 'freebird' in a comment and save. Enjoy audio.
Scroll distant from the meme, enjoy decreasing volume.
Scroll nearby the meme, enjoy increasing volume.
If it's a loop, the audio continues.
If it's not a loop, the audio terminates after the first run.
Reviewers: aklapper, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16451, T16486
Differential Revision: https://we.phorge.it/D26722
Summary:
Remove the nonsense method 'shouldHide()' because it always returns
false, which is the same desired value from the parent class.
In fact, the old value is always something like this:
array( 'PHID-PIMG-xegbj5j4fjnmp7xzakk3' => '' )
Or this:
array( 'PHID-PIMG-xegbj5j4fjnmp7xzakk3' => 'asd' )
So it's never this:
counterexample
array( null => null )
(Which - by the way, is always internally converted by PHP to this...)
array( '' => null )
Plus, the class 'PholioImageDescriptionTransaction' should be like
the class 'ManiphestTaskDescriptionTransaction', so, both without
this method 'shouldHide()'.
The 'shouldHide()' here was also broken in PHP 8.5.
Thanks to Aklapper for the initial troubleshooting.
Closes T16479
Ref T16460
Test Plan:
These feeds absolutely do not change A/B:
- create a Pholio mock with an image with a description and save
- remove that image description and save
- re-add a image description and save
Reviewers: aklapper, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16479, T16460
Differential Revision: https://we.phorge.it/D26718
Summary:
Prefer a field label to a field name for the aria-label parameter value of a textarea element, as that is more descriptive.
Closes T16216
Test Plan:
1. Go to http://phorge.localhost/ponder/question/edit/form/default/
2. Inspect the <textarea> tag of `Answer Summary` which now says `aria-label="Answer Summary"` instead of `aria-label="answerWiki"`
3. Go to http://phorge.localhost/maniphest/task/edit/form/default/ while having a custom text field `Custom Field` defined
4. Inspect the <textarea> tag of `Description` which now says `aria-label="Description"` instead of `aria-label="description"`
5. Inspect the <textarea> tag of `Custom Field` which now says `aria-label="Custom Field"` instead of `aria-label="std:maniphest:custom.field"`
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16216
Differential Revision: https://we.phorge.it/D26730
Summary: Cross-reference a TODO comment in PhabricatorJavelinLinter with related open tasks
Test Plan: None. It is a comment only.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26731
Summary:
The example.com domain is intentionally meant for such purposes.
Same game as rPab272edc or rP674ef80b.
Test Plan: None.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26729
Summary:
These message refer to user accounts, not users. Thus avoid any ambiguity.
(`PhabricatorDisabledUserController` already does it right by setting `Account Disabled`.)
Refs T16252
Test Plan: Look at strings.
Reviewers: O1 Blessed Committers, valerio.bozzolan, haylin
Reviewed By: O1 Blessed Committers, valerio.bozzolan, haylin
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16252
Differential Revision: https://we.phorge.it/D26417
Summary:
Session-based auth got superseded by token-based authentication long ago.
Refs T16487
Test Plan:
* Go to http://phorge.localhost/conduit/method/conduit.connect/ and http://phorge.localhost/conduit/method/conduit.getcertificate/
* See a "Stability" entry saying "Deprecated Method"
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16487
Differential Revision: https://we.phorge.it/D26728
Summary:
Use default browser arrow, unless someone has strong opinions.
Refs T15056
Test Plan: Go to a Maniphest task or Differential revision, in Default mode and Dark mode, and look at the "Add Action..." dropdown.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15056
Differential Revision: https://we.phorge.it/D26665
Summary: Followup to D25743
Test Plan: 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/D26726
Summary:
Static code analysis complained `Cannot unset offset 'Authorization'|'Host' on array<int<0, max>, array{string, mixed}>.`
Per avivey's look at the code, we only ever get headers in the form of X-Hgargs-* in $headers anyway, so the special-case here won't ever happen. Delete this code.
Also see similar code at the bottom of `AphrontHTTPProxyResponse.php`.
Test Plan: Read the code.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25743
Summary:
Passing null to `json_encode()` is deprecated since PHP 8.1.
```
ERROR 8192: json_decode(): Passing null to parameter #1 ($json) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php:35]
```
In addition, also check for `name` not being null to avoid yet another similar AphrontQueryException here and throw a proper ConduitException instead.
Closes T16426
Test Plan:
* PHP 8.1+
* Go to http://phorge.localhost/conduit/method/differential.setdiffproperty/
* Press the "Call Method" button
* Go to http://phorge.localhost/conduit/method/differential.setdiffproperty/ and set `1` in the `diff_id` field and leave the `name` field empty
* Press the "Call Method" button
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16426
Differential Revision: https://we.phorge.it/D26637
Summary:
Passing bogus values to the Conduit endpoints 'subscribers.remove', 'subscribers.add', 'subscribers.set', do not silently fail or do not emit confusing `Edges are not available for objects of type '????'!`, but throw a useful error message on empty/invalid values like
`"ERR-CONDUIT-CORE: <maniphest.edit> Subscriber transactions must be existing PHIDs or usernames (found key \"nonsense\" on transaction of type \"core:subscribers\")."`
Closes T16311
Test Plan:
* Edit the subscribers of an existing task in the web UI, e.g. add/remove existing users and projects
* Run a bunch of API calls passing valid and invalid usernames/project tags and see that they now fail with informative error output telling you which value is invalid, e.g.
```
echo '{"transactions": [{"type": "subscribers.add", "value":["user1","user2","someExistingProjectName"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
echo '{"transactions": [{"type": "subscribers.add", "value":["PHID-xxxinvalid","user1",""]},{"type": "subscribers.remove", "value":["user4","usernonexisting"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
echo '{"transactions": [{"type": "subscribers.add", "value":["user9"]},{"type": "subscribers.remove", "value":["user3","usernonexisting"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
echo '{"transactions": [{"type": "subscribers.add", "value":["user9"]},{"type": "subscribers.remove", "value":["user3"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
```
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16311
Differential Revision: https://we.phorge.it/D26696
Summary:
The current `overflow: hidden;` in the "Close as Duplicate" dialog clips content.
`word-break: break-word;` breaks within words when needed and keeps text inside the table cell boundaries defined by the dialog's fixed width of 860px.
Closes T16476
Test Plan:
# Create a new task with some title which has a long string without whitespace, for example `TypeError: Phorge\Extension\Translate\PhorgeExtensionTranslateGroupProcessing\CachedMessageGroupExtensionLoader::Phorge\Extension\Translate\PhorgeMessageGroupProcessing\{closure}(): Argument #1 ($value) must be of type FooBar, __PHP_Incomplete_Class given`
# Go to another existing task and select `Edit Related Tasks > Close as Duplicate`
# In the search field, enter the task ID of the new task from step 1
# Click on that task in the search results
# Look at the search results and the "Close this task as a duplicate of" section at the bottom of the overlay dialog before and after applying this patch
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16476
Differential Revision: https://we.phorge.it/D26716
Test Plan: Run `arc unit`
Reviewers: O1 Blessed Committers, aklapper, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26704
Summary:
A project workboard may have existed before but got disabled. When trying to access this workboard, do not trigger `buildInitializeContent()` which will create yet another default workboard column (and duplicates the existing column on the disabled workboard) but call `buildEnableContent()` which will display the "Workboard Disabled" panel instead allowing to re-enable the workboard.
Also fix some wrong indentation in a section below.
Closes T16475
Test Plan:
1. Create a new project.
2. Select "Workboard" from the left sidebar.
3. Select default "Columns: New Empty Board" for that project, and click the "Create Workboard" button.
4. In the right upper corner, select {nav icon=cog}, click {nav icon=cog,name=Manage Workboard}, and select {nav icon=ban,name=Disable Workboard}
5. End up on resulting http://phorge.localhost/project/board/1/, or go explicitly to that URI by selecting "Workboard" in the project navigation panel on the left
6. Before this patch, it shows the "Create Workboard" panel, allowing you to again select "Columns: New Empty Board", click the "Create Workboard" button, and see now two default columns both named "Backlog (Default)" on the (re-enabled) workboard
7. After this patch, it shows the "Disabled Workboard" panel with a button to re-enable the workboard
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16475
Differential Revision: https://we.phorge.it/D26714
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
```
ERROR 8192: Using null as the key parameter for array_key_exists() is deprecated, use an empty string instead at [/var/www/html/phorge/phorge/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:287]
```
Closes T16377
Test Plan:
* PHP 8.5
* Set `/config/edit/policy.allow-public` to `true`
* Create a Maniphest task with View Policy = Public
* Access that task while not logged in
* Get two errors less
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16377
Differential Revision: https://we.phorge.it/D26551
Summary:
Add the PHPAST linter in Phorge, to support newer PHP syntax.
Requires that D26516 be applied to rARC locally.
Test Plan:
* Run `arc patch D26516` on `arcanist/`
* Run `arc lint --everything --never-apply-patches --severity disabled --output summary`
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16309
Differential Revision: https://we.phorge.it/D26517
Summary:
Older MySQL versions had performance issues in query optimization for tuple comparisons in an IN clause.
These issues were mostly fixed for MySQL 5.6; see https://bugs.mysql.com/bug.php?id=31188
While there are still some issues (e.g. https://bugs.mysql.com/bug.php?id=115190), nowadays tuple search index usage is generally supported.
Phorge requires MySQL 8 since rP555fb3a8 / T16107. So this sentence is now moot.
Test Plan:
* Search the interwebs
* Run `./bin/diviner generate`, read http://phorge.localhost/book/contrib/article/database/
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/D26472
Summary:
Has never had any translations other than a translation of its own locale name. It's not clear if there was once a translation in some third-party library that I can't find, or if it was one of old upstream's tired jokes.
In any event, nobody plans to do anything with this, so, as cool of an idea this is, it has no point to it.
Test Plan: Don't see emoji in limited translations (or, this needs a companion arcanist patch, so don't it once both are merged)
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/D26711
Summary:
In Diffusion repositories, project tags associated with the repository are only shown on the "Manage > Edit Basic Information" page. (?)
Also display them in the repositories main page, alongside the existing tags (Status, Visibility).
Note that a project tag does not line-wrap, so a long project name can move the "Pattern Search" field out of the viewport on smaller screen widths due to T16287 and `.phui-tag-view {white-space: nowrap;}`.
Closes T15526
Test Plan:
1. Create a Diffusion repository; create some project tags
2. Go to http://phorge.localhost/diffusion/36/manage/ and select "Edit Basic Information"
3. Under "Tags", add several project tags, and "Save Changes"
4. Go to http://phorge.localhost/diffusion/36/ and see the project tags displayed next to the "Active/Disabled" and "Public/All Users/Custom Policy" tags.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15526
Differential Revision: https://we.phorge.it/D26735
Summary:
On small screens, the view policy tag in the page subheader (e.g. "Public", "All Users", "Custom Policy", "Administrators") can get wrapped across three lines.
Avoid that and keep the tag on one line.
Closes T16287
Test Plan:
Look at tags in subheaders on object pages in all three different CSS layouts that Phorge offers, ideally with "long" tags
* http://phorge.localhost/p/user/ (e.g. with a user who also has MFA enabled)
* http://phorge.localhost/diffusion/1/
* http://phorge.localhost/book/flavor/
* http://phorge.localhost/T1 (e.g. with Story Point tags also enabled)
* http://phorge.localhost/D1
* http://phorge.localhost/phame/blog/view/1/
* http://phorge.localhost/w/somepage/
* http://phorge.localhost/book/dev/class/PhorgeI18nTestCase/#method/key (Diviner; the "Inherited" tag uses this CSS)
* http://phorge.localhost/M1
* http://phorge.localhost/F1
* http://phorge.localhost/B1
* http://phorge.localhost/C1
* http://phorge.localhost/uiexample/view/PHUIBoxExample/ ("Fancy Box" example at the bottom)
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16287
Differential Revision: https://we.phorge.it/D26736
Summary:
This adds CSS to prevent the document view from overlapping to the blogs and drafts boxes in the right column.
Ref T16473
Test Plan:
- Browse to phame home view using a screen width of at least 1792px
- See that now the links to the individual blogs are fully clickable
- See that all other views (Phriction and Phame posts) are unchanged
Reviewers: #blessed_committers, O1 Blessed Committers, aklapper
Reviewed By: #blessed_committers, O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16473
Differential Revision: https://we.phorge.it/D26715
Summary:
Do not allow setting an invalid project color via the `project.edit` Conduit API but validate the value.
Same game as in D26430, D26459.
Closes T16237
Test Plan:
* Run `echo '{"transactions":[{"type":"name","value":"projectWithInvalidColor"},{"type":"color","value":"purpleyellowgreyish"},{"type":"description","value":"project project new new new"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" project.edit --`
* Succeed before applying this patch
* Fail after applying this patch:
```
{
"error": "ERR-CONDUIT-CORE",
"errorMessage": "ERR-CONDUIT-CORE: <project.edit> Validation errors:\n - Value for \"project:color\" is invalid: \"purpleyellowgreyish\".",
"response": null
}
```
* Still succeed with a valid name like `{"type":"color","value":"checkered"}`
* Manually set a color in the project using the frontend. It works.
* Change every single other color, like Red, Orange, Yellow, ... Grey, Checkered, they all works
* Set color to Orange. Archive the project. Activate the project. It still works, you still see the original color Orange.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16323, T16237
Differential Revision: https://we.phorge.it/D26460
Summary:
Do not allow setting an invalid project icon via the `project.edit` Conduit API but validate the value.
Same game as in D26430.
Closes T16322
Test Plan:
* Run `echo '{"transactions":[{"type":"name","value":"projectWithInvalidIcon"},{"type":"icon", "value":"noexxxist"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" project.edit --`
* Succeed before applying this patch
* Fail after applying this patch:
```
{
"error": "ERR-CONDUIT-CORE",
"errorMessage": "ERR-CONDUIT-CORE: <project.edit> Validation errors:\n - Value for \"project:icon\" is invalid: \"noexxxist\".",
"response": null
}
```
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16322
Differential Revision: https://we.phorge.it/D26459
Summary:
Do not allow setting an invalid user icon via the `user.edit` Conduit API but validate the value.
This code path is executed because `PhabricatorApplicationTransactionEditor::validateTransaction()` for `case PhabricatorTransactions::TYPE_CUSTOMFIELD` calls `$field->validateApplicationTransactions` and because `PhabricatorUserIconField` is a child class of `PhabricatorCustomField`.
Closes T16307
Test Plan:
* Run `echo '{ "objectIdentifier":"@myusername", "transactions":[{"type":"icon","value":"nonexistingicon"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost/ --conduit-token "cli-mytoken" user.edit --` before and after this, and get a new error after this patch:
```
{
"error": "ERR-CONDUIT-CORE",
"errorMessage": "ERR-CONDUIT-CORE: <user.edit> Validation errors:\n - Value for \"Icon\" is invalid: \"nonexistingicon\".",
"response": null
}
```
* Optionally, check database value of `SELECT u.userName, up.icon FROM phabricator_user.user_profile up JOIN phabricator_user.user u ON up.userPHID = u.phid WHERE u.userName = "myusername";` in the database before and after applying patch and compare what `PhabricatorPeopleIconSet` actually offers
* Run `echo '{ "objectIdentifier":"@myusername", "transactions":[{"type":"icon","value":"spy"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost/ --conduit-token "cli-mytoken" user.edit --` and still successfully change your user icon
* Run `echo '{ "objectIdentifier":"@notmyusername", "transactions":[{"type":"icon","value":"spy"}]}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost/ --conduit-token "cli-mytoken" user.edit --` and still get the expected error that you are not that user
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16307
Differential Revision: https://we.phorge.it/D26430
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
According to https://web.archive.org/web/20171001040455/https://www.php.net/manual/en/language.types.array.php, `Null will be cast to the empty string, i.e. the key null will actually be stored under ""` at least since PHP 7.2, which we require.
Closes T16495
Test Plan: Run `../arcanist/bin/arc unit ./src/infrastructure/env/__tests__/PhabricatorEnvTestCase.php` in PHP 8.5
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16495
Differential Revision: https://we.phorge.it/D26748
Summary:
Throw a ConduitException when no objectPHID is passed instead of silently failing when trying to delete a token being `null`.
As a side effect, fix a PHP 8.5 deprecation warning.
Closes T16436
Test Plan: Go to http://phorge.localhost/conduit/method/token.give/ and press "Call Method". Try also after entering a random string in the "tokenPHID" field.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16436
Differential Revision: https://we.phorge.it/D26647
Summary:
Bump our copy of external mimemailparser library from 8.0.4 to 9.0.1.
See https://github.com/php-mime-mail-parser/php-mime-mail-parser/releases for the upstream changelog.
Closes T16458
Test Plan: tbd
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16458
Differential Revision: https://we.phorge.it/D26685
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
```
ERROR 8192: Using null as an array offset is deprecated, use an empty string instead at [/var/www/html/phorge/phorge/src/applications/macro/markup/PhabricatorIconRemarkupRule.php:70]
```
Closes T16446
Test Plan: Unknown
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16446
Differential Revision: https://we.phorge.it/D26764
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
```
ERROR 8192: Using null as an array offset is deprecated, use an empty string instead at [/var/www/html/phorge/phorge/src/applications/project/engine/PhabricatorProjectEditEngine.php:128]
```
Closes T16504
Test Plan:
* PHP 8.5
* Go to an existing parent project
* Select "Subprojects" in the sidebar
* Click "Create Milestone" on the right
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16504
Differential Revision: https://we.phorge.it/D26765
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/legalpad/xaction/LegalpadDocumentTitleTransaction.php:22]
```
Closes T16505
Test Plan:
* Create a Legalpad at http://phorge.localhost/legalpad/edit/
* Go to http://phorge.localhost/feed/transactions/
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16505
Differential Revision: https://we.phorge.it/D26766
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
See the same game nine lines lower.
Closes T16497
Test Plan: Run `./bin/arc unit ./src/applications/differential/__tests__/DifferentialParseRenderTestCase.php` on PHP 8.5
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16497
Differential Revision: https://we.phorge.it/D26763
Summary:
Link the new "Policies User Guide" added in rPc5e33a500c19 also from the "Understanding Policies" section in the Forms and Projects guides.
Followup to rPc5e33a500c19.
Test Plan: `./bin/diviner generate`
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26762
Summary:
Remove code around some conditions which are always true/false anyway.
Same game as rP1b9fafc7af72.
Test Plan: Read earlier code in each file; run static code analysis.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26760
Summary: In prep for T15277, write some user docs about the existing Policies - how they work and how to use them.
Test Plan: look at it until blurry.
Reviewers: O1 Blessed Committers, valerio.bozzolan, aklapper
Reviewed By: O1 Blessed Committers, valerio.bozzolan, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26759
Summary:
Given the absolute CSS position of the global top bar search parent object, there is no reason to truncate search scope names that early.
Change `width: 200px` to `max-width: 260px`.
(I'd also generally like to advertise https://www.w3.org/International/articles/article-text-size.en)
Closes T16369
Test Plan: See steps in T16369; test on different screen widths
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16369
Differential Revision: https://we.phorge.it/D26539
Summary:
Fully modularizing DifferentialDiffTransaction, and then fix T16472 by attaching any binary files uploaded to the diff object.
Refs T16485 T16070.
Test Plan:
Set the default view-policy of new files to something that isn't very public.
Create new diffs using `arc diff` that includes a picture of a small cat. Other kinds of images are out of scope.
- Users that don't have permission to see the revision should not be able to see the file
- Users that have permission to see the revision, should be able to see the file.
- Changing the policies on the created file to "no one" will not be make a difference.
- The file's page will show the diff in the Attachment list.
Also:
- Herald can still block the creation of the diffs.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno, jdelker
Maniphest Tasks: T16485, T16472, T16070
Differential Revision: https://we.phorge.it/D26724
Summary: Some PhpDoc corrections and additions as I've been poking some code.
Test Plan: Read code; run static code analysis.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26756
Summary:
While poking a CSS bug in Diviner I was checking where else `PHUITagView::TYPE_STATE` is used.
Turns out that this use can be removed, as any reading of the $tag variable got removed in rP27e13ea0.
Test Plan: Search for `$tag` within that very file.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26757
Summary: Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
Test Plan:
Run static code analysis:
```
/src/applications/transactions/view/PhabricatorApplicationTransactionView.php:372 Possibly invalid array key type int|string|null.
```
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26679
Summary:
Move the phui-info-view box about Query Errors below the phui-form-view Search form. That's below the `#R` anchor that the view automatically scrolls to. People do not scroll up again to expect an error message up there so show the error message down here.
Closes T16089
Test Plan: Go to http://phorge.localhost/maniphest/query/advanced/ and enter something in the Query field that will trigger a Query Error, such as `help:now` or a very long string. Press the Search button.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16089
Differential Revision: https://we.phorge.it/D26672
Summary:
Going to http://phorge.localhost/maniphest/query/advanced/, after `if ($this->queryKey == 'advanced') { $query_key = $request->getStr('query');` in `PhabricatorApplicationSearchController::processSearchRequest()`, `$query_key` remains null. Afterwards, `if ($engine->isBuiltinQuery($query_key))` calls PhabricatorApplicationSearchEngine::isBuiltinQuery($query_key) which tries `$builtins[$query_key]`.
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
```
ERROR 8192: Using null as an array offset is deprecated, use an empty string instead at [/var/www/html/phorge/phorge/src/applications/search/engine/PhabricatorApplicationSearchEngine.php:757]
```
Closes T16361
Test Plan:
Go to http://phorge.localhost/maniphest/query/advanced/. Enter a term, run a query, save it.
Go to http://phorge.localhost/mail/query/advanced/ which triggers the issue.
Reviewers: O1 Blessed Committers, valerio.bozzolan, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16361
Differential Revision: https://we.phorge.it/D26530
Summary:
Make Phorge also look for the "magick" binary which supersedes "convert" since ImageMagick version 7.
Closes T16474
Test Plan:
1. Rename the commands in the two "Filesystem::binaryExists()" checks to some nonsense
2. As an admin, set http://phorge.localhost/config/edit/files.enable-imagemagick/ to `enabled`
3. See in top bar a new setup warning pointing to http://phorge.localhost/config/issue/files.enable-imagemagick/ about "'magick' or 'convert' binary not found or Imagemagick is not installed."
4. Correct the two commands in the two "Filesystem::binaryExists()" checks, as they are in this patch
5. Upload an animated GIF image file `F1`, go to http://phorge.localhost/file/transforms/1/ and manually trigger a transform by clicking a "Regenerate" button, get a result (optionally insert phlog() statements in PhabricatorFileImageTransform::applyImagemagick() to prove that ImageMagick is used)
6. Probably create some Macro meme? I did not.
7. Check the "Behavioral Changes" section on https://imagemagick.org/script/porting.php and do not spot any commands used in the Phorge codebase
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16474
Differential Revision: https://we.phorge.it/D26742
Summary: Throw a proper error code and error_info, like similar `file.download` and `file.info` already do.
Test Plan:
* Go to http://phorge.localhost/conduit/method/file.uploadchunk and http://phorge.localhost/conduit/method/file.querychunks , click "Call Method"
* Before this patch, get a generic "ERR-CONDUIT-CORE" error code.
* After this patch, get a "ERR-BAD-PHID" error code and the error info `No such file 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/D26754
Summary:
Do not allow to unset the image title from Pholio mock images,
in order to avoid at least 3 related UX bugs shown in T16478.
Also remove the broken method 'shouldHide()',
because it always returns false, which is the default from
the parent call, PhabricatorModularTransactionType#shouldHide().
The 'shouldHide()' was also broken in PHP 8.5.
Also improve one error message to talk about "image titles"
(and not "image names"), so to be consistent with the "Title"
field in the UX.
Thanks to @aklapper for the original troubleshooting.
Closes T16478
Ref T16460
Test Plan:
Create a Pholio mock with at least one image and save.
Try to unset one image title (not the mock title, but the image title)
and enjoy the new validation message: "Mock images must have a name."
Reviewers: aklapper, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, aklapper, Cigaryno
Maniphest Tasks: T16478, T16460
Differential Revision: https://we.phorge.it/D26717
Summary:
It might be vaguely useful to know what happens to your audio
once you press save.
Screenshot after the change:
{F6759938}
Test Plan:
Visit a specific macro/meme from this link:
http://phorge.localhost/macro/
Click on Edit Audio. Enjoy the new caption.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16451
Differential Revision: https://we.phorge.it/D26723
Summary:
Type hints for parameters can be generic, such as `array<string,int>`.
Some of these contain spaces, like `array<string, int>`, which was parsed incorrectly previously.
This resulted in a line like ` * @param map<string, string> $attributes (optional) A map of tag attributes.` rendering as
`map<string, | $attributes | string> $attributes (optional) A map of tag attributes.` instead of
`map<string,string> | $attributes | $attributes (optional) A map of tag attributes.`.
Closes T16499
Test Plan:
* Run `./bin/diviner generate` on git master
* Look at the "Parameters" box on http://phorge.localhost/book/dev/function/phutil_tag/
* Apply this patch
* Run `./bin/diviner generate --clean`
* Look at the "Parameters" box on http://phorge.localhost/book/dev/function/phutil_tag/
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16499
Differential Revision: https://we.phorge.it/D26752
Summary:
`PhabricatorProjectCoreTestCase` creates projects that it doesn't save to the database. As a result, they don't have a PHID. This causes issues because `::getPHID()` will return null instead, which in PHP 8.5 results in deprecation warnings.
Closes T16496
Test Plan:
Run this unit test on PHP 8.5 and see no 'ERROR 8192' anymore:
arc unit src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
Alternatively, for those in the stone age, add a null check to `PhabricatorProjectMembersPolicyRule` for `$object->getPHID()` on lines 34 & 70. (It's me, I'm in the stone age)
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16496
Differential Revision: https://we.phorge.it/D26751
Summary:
As of PHP 8.1.0, calling `setAccessible()` has no effect; all properties are accessible by default.
See https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_reflectionsetaccessible and https://www.php.net/manual/en/reflectionproperty.setaccessible.php
Refs T16494
Test Plan: Run `./bin/arc unit --everything` on PHP 8.5
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16494
Differential Revision: https://we.phorge.it/D26746
Summary:
Do not execute a PhabricatorSavedQueryQuery in the database when a queryKey is passed which is obviously bogus.
Closes T16482
Test Plan:
* Adjust the newly added exception error message, go to http://phorge.localhost/conduit/method/project.column.search/ and enter a "string" in the queryKey field which does not have 12 characters.
* Adjust the newly added exception error message, go to http://phorge.localhost/maniphest/query/nonsense/#R and still get a 404.
* Optionally, check in DarkConsole that there is one less DB query (I did not).
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16482
Differential Revision: https://we.phorge.it/D26720
Summary:
Fix T16483.
1. Fix the page to actually load things
2. Add a way to query for a specific object
3. Add link from object pages to the full history (under "Advanced")
Test Plan:
- Visit `/feed/transactions/` and see looots of entries.
- enter object names and phids into the query, list is filtered to these objects only.
- Visit a random task object, hit Advanced -> View All Transactions, see full history including things that are normally hidden.
Reviewers: aklapper, valerio.bozzolan, O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16483
Differential Revision: https://we.phorge.it/D26732
Summary:
* Last call to private `PhrictionTransactionEditor::setNewContent()` was removed in rP64cee4a9.
* Last call to private `PhabricatorRepository::isSSHProtocol()` was removed in rP24ebac8a.
Test Plan: Grep the codebase.
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/D26740
Summary:
Revert rPbabeed68 which can lead to an error when commenting on a task:
```
Subscriber transactions must be existing PHIDs or usernames (found key "0" on transaction of type "core:subscribers").
```
Looks like Phorge is pretty inconsistent in its array keys when passing subscriber arrays around - sometimes the keys are default integers, sometimes they duplicate the value.
Closes Q238
Reopens T16311
Test Plan:
* Comment on a task which you are not yet subscribed to via the web interface.
* Follow test plan of original rPbabeed68 for the Conduit part.
Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16311
Differential Revision: https://we.phorge.it/D26741
Summary:
Fixes the bug in the third paragraph of D25861#39133
Depends on D25861
Test Plan: Follow the test plan setup for D25861. Try to edit an event and see that you can now edit it normally and don't get "end date is invalid"/"start date is invalid" errors.
Reviewers: aklapper, O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26544
Summary: Fixes T15811. Alternative to D25618
Test Plan: Observe that the lengthy steps to reproduce in T15811 no longer reproduce. Observe that translated date strings (e.p.p.) still appear in most parts of the interface
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: avivey, aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15811
Differential Revision: https://we.phorge.it/D25861
Summary: Closes T16398
Test Plan: Go to some object's timeline, look at the timestamp anchor, also click that timestamp to get background highlighting.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16398
Differential Revision: https://we.phorge.it/D26594
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/feed/conduit/FeedQueryConduitAPIMethod.php:79]
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/feed/conduit/FeedQueryConduitAPIMethod.php:84]
```
Closes T16419
Test Plan:
* PHP 8.1+
* Go to http://phorge.localhost/conduit/method/feed.query/
* Press the "Call Method" button
* Go to http://phorge.localhost/conduit/method/feed.query/ and enter values in the `before` and `after` fields
* Press the "Call Method" button
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16419
Differential Revision: https://we.phorge.it/D26631
Summary:
For years, when editing a Pholio Mock and causing an image title
validation error, then the breadcrumb and the page header were altered.
The cause was a variable, $title, re-used by mistake in this corner case.
After this change, the "Edit Mock: The mock name" is not overwritten by mistake.
Closes T16492
Test Plan:
Edit an already-existing Pholio Mock.
http://phorge.localhost/pholio/edit/1/
In the field "Name", set "The mock name".
Then change the image title (field "Title") setting a very long title,
more than 128 characters, like this:
THE IMAGE VERY LONG TITLE BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLABLA BLA BLA BLA BLA BLA BLA BLA!
Save. You will see the usual error, but now you finally see the normal breadcrumb, like this:
{nav Pholio > M1 > Edit Mock: The mock name}
So, you don't see anymore this nonsense breadcrumb:
{nav Pholio > M1 > THE IMAGE VERY LONG TITLE BLA BLA BLA BLA BLA B...}
Plus, you finally see this page header:
Edit Mock: The mock name
So, you don't see anymore this nonsense page header:
Edit Mock: THE IMAGE VERY LONG TITLE BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLA BLABLA BLA BLA BLA BLA BLA BLA BLA!
Such wow!
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16492
Differential Revision: https://we.phorge.it/D26734
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.
```
strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [PhameBlogParentDomainTransaction.php:18]
strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [PhameBlogFullDomainTransaction.php:25]
```
Ref T15064
Test Plan: Unknown, presumably change the parent domain of a Phame blog. These only became visible after D26732 was applied.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15064
Differential Revision: https://we.phorge.it/D26737
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/PhabricatorAuthNeedsMultiFactorController.php:39]
```
Closes T16491
Test Plan:
* Be logged into Phorge
* Run `/var/www/html/phorge/phorge/bin/config set security.require-multi-factor-auth true`
* Reload a Phorge page to get the content of http://phorge.localhost/auth/enroll/multifactor/ displayed
* Visit also http://phorge.localhost/auth/
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16491
Differential Revision: https://we.phorge.it/D26733
Summary:
For more than 10 years it was possible to troll your coworkers with
infinite audio loops, by typing a macro/meme in a comment box,
previewing, and then writing something else. This was happening
since macros may become hidden but still loaded in the DOM and,
in that case, they may play forever, until the page is destroyed.
The annoying sounds were also continuing when enjoying a meme and then
clicking on a different link, since page loads are often done by ajax,
effectively causing invisible memes in the DOM playing rickroll forever.
After this patch, no more audio from invisible macro/meme, effectively
implementing the beloved "STFU" alghorithm.
https://en.wikipedia.org/wiki/STFU
Note: you may appreciate that the audio volume still decreases
if you scroll up/down distant from a meme.
So, sounds are not really that annoying. Not even audio loops!
If a particular macro is annoying, then scroll down, or file a
severe complaint to your favorite Phorge admin to de-activate
that audio.
This patch is designed after D26674, to be not destructive.
Additionally, done small refactor to avoid 'statics.items[ii].element',
which can be written as just 'item.element'.
Closes T16486
Closes T16451
Test Plan:
Preamble: always click somewhere in the page for all tests,
otherwise modern browser webs like Firefox will likely deny audio,
even if you scroll straight to a meme.
-----
Locally, create a weird image macro (http://phorge.localhost/macro/create/)
for example called "freebird" and upload the sound of an epic
mp3 file, like the solo of Free Bird by Lynyrd Skynyrd, which is
somehow long. Example mp3:
https://quicksounds.com/sound/23840/free-bird-solo
Now, do the tests:
Write 'freebird' in a comment in a preview. Enjoy audio.
Remove 'freebird' from the preview, enjoy NO AUDIO \o/
Write 'freebird' in a comment and save. Enjoy audio.
Click a different link (e.g. 'Edit Task'). enjoy NO AUDIO \o/
Check for regressions in normal workflows:
Write 'freebird' in a comment and save. Enjoy audio.
Scroll distant from the meme, enjoy decreasing volume.
Scroll nearby the meme, enjoy increasing volume.
If it's a loop, the audio continues.
If it's not a loop, the audio terminates after the first run.
Reviewers: aklapper, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16451, T16486
Differential Revision: https://we.phorge.it/D26722
Summary:
Remove the nonsense method 'shouldHide()' because it always returns
false, which is the same desired value from the parent class.
In fact, the old value is always something like this:
array( 'PHID-PIMG-xegbj5j4fjnmp7xzakk3' => '' )
Or this:
array( 'PHID-PIMG-xegbj5j4fjnmp7xzakk3' => 'asd' )
So it's never this:
counterexample
array( null => null )
(Which - by the way, is always internally converted by PHP to this...)
array( '' => null )
Plus, the class 'PholioImageDescriptionTransaction' should be like
the class 'ManiphestTaskDescriptionTransaction', so, both without
this method 'shouldHide()'.
The 'shouldHide()' here was also broken in PHP 8.5.
Thanks to Aklapper for the initial troubleshooting.
Closes T16479
Ref T16460
Test Plan:
These feeds absolutely do not change A/B:
- create a Pholio mock with an image with a description and save
- remove that image description and save
- re-add a image description and save
Reviewers: aklapper, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T16479, T16460
Differential Revision: https://we.phorge.it/D26718
Summary:
Prefer a field label to a field name for the aria-label parameter value of a textarea element, as that is more descriptive.
Closes T16216
Test Plan:
1. Go to http://phorge.localhost/ponder/question/edit/form/default/
2. Inspect the <textarea> tag of `Answer Summary` which now says `aria-label="Answer Summary"` instead of `aria-label="answerWiki"`
3. Go to http://phorge.localhost/maniphest/task/edit/form/default/ while having a custom text field `Custom Field` defined
4. Inspect the <textarea> tag of `Description` which now says `aria-label="Description"` instead of `aria-label="description"`
5. Inspect the <textarea> tag of `Custom Field` which now says `aria-label="Custom Field"` instead of `aria-label="std:maniphest:custom.field"`
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16216
Differential Revision: https://we.phorge.it/D26730
Summary: Cross-reference a TODO comment in PhabricatorJavelinLinter with related open tasks
Test Plan: None. It is a comment only.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26731
Summary:
The example.com domain is intentionally meant for such purposes.
Same game as rPab272edc or rP674ef80b.
Test Plan: None.
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D26729
Summary:
These message refer to user accounts, not users. Thus avoid any ambiguity.
(`PhabricatorDisabledUserController` already does it right by setting `Account Disabled`.)
Refs T16252
Test Plan: Look at strings.
Reviewers: O1 Blessed Committers, valerio.bozzolan, haylin
Reviewed By: O1 Blessed Committers, valerio.bozzolan, haylin
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16252
Differential Revision: https://we.phorge.it/D26417
Summary:
Session-based auth got superseded by token-based authentication long ago.
Refs T16487
Test Plan:
* Go to http://phorge.localhost/conduit/method/conduit.connect/ and http://phorge.localhost/conduit/method/conduit.getcertificate/
* See a "Stability" entry saying "Deprecated Method"
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16487
Differential Revision: https://we.phorge.it/D26728
Summary:
Use default browser arrow, unless someone has strong opinions.
Refs T15056
Test Plan: Go to a Maniphest task or Differential revision, in Default mode and Dark mode, and look at the "Add Action..." dropdown.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15056
Differential Revision: https://we.phorge.it/D26665
Summary:
Static code analysis complained `Cannot unset offset 'Authorization'|'Host' on array<int<0, max>, array{string, mixed}>.`
Per avivey's look at the code, we only ever get headers in the form of X-Hgargs-* in $headers anyway, so the special-case here won't ever happen. Delete this code.
Also see similar code at the bottom of `AphrontHTTPProxyResponse.php`.
Test Plan: Read the code.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25743
Summary:
Passing null to `json_encode()` is deprecated since PHP 8.1.
```
ERROR 8192: json_decode(): Passing null to parameter #1 ($json) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php:35]
```
In addition, also check for `name` not being null to avoid yet another similar AphrontQueryException here and throw a proper ConduitException instead.
Closes T16426
Test Plan:
* PHP 8.1+
* Go to http://phorge.localhost/conduit/method/differential.setdiffproperty/
* Press the "Call Method" button
* Go to http://phorge.localhost/conduit/method/differential.setdiffproperty/ and set `1` in the `diff_id` field and leave the `name` field empty
* Press the "Call Method" button
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16426
Differential Revision: https://we.phorge.it/D26637
Summary:
Passing bogus values to the Conduit endpoints 'subscribers.remove', 'subscribers.add', 'subscribers.set', do not silently fail or do not emit confusing `Edges are not available for objects of type '????'!`, but throw a useful error message on empty/invalid values like
`"ERR-CONDUIT-CORE: <maniphest.edit> Subscriber transactions must be existing PHIDs or usernames (found key \"nonsense\" on transaction of type \"core:subscribers\")."`
Closes T16311
Test Plan:
* Edit the subscribers of an existing task in the web UI, e.g. add/remove existing users and projects
* Run a bunch of API calls passing valid and invalid usernames/project tags and see that they now fail with informative error output telling you which value is invalid, e.g.
```
echo '{"transactions": [{"type": "subscribers.add", "value":["user1","user2","someExistingProjectName"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
echo '{"transactions": [{"type": "subscribers.add", "value":["PHID-xxxinvalid","user1",""]},{"type": "subscribers.remove", "value":["user4","usernonexisting"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
echo '{"transactions": [{"type": "subscribers.add", "value":["user9"]},{"type": "subscribers.remove", "value":["user3","usernonexisting"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
echo '{"transactions": [{"type": "subscribers.add", "value":["user9"]},{"type": "subscribers.remove", "value":["user3"]}],"objectIdentifier":"T3955"}' | /var/www/html/phorge/arcanist/bin/arc call-conduit --conduit-uri http://phorge.localhost --conduit-token "cli-xxx" maniphest.edit --
```
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16311
Differential Revision: https://we.phorge.it/D26696
Summary:
The current `overflow: hidden;` in the "Close as Duplicate" dialog clips content.
`word-break: break-word;` breaks within words when needed and keeps text inside the table cell boundaries defined by the dialog's fixed width of 860px.
Closes T16476
Test Plan:
# Create a new task with some title which has a long string without whitespace, for example `TypeError: Phorge\Extension\Translate\PhorgeExtensionTranslateGroupProcessing\CachedMessageGroupExtensionLoader::Phorge\Extension\Translate\PhorgeMessageGroupProcessing\{closure}(): Argument #1 ($value) must be of type FooBar, __PHP_Incomplete_Class given`
# Go to another existing task and select `Edit Related Tasks > Close as Duplicate`
# In the search field, enter the task ID of the new task from step 1
# Click on that task in the search results
# Look at the search results and the "Close this task as a duplicate of" section at the bottom of the overlay dialog before and after applying this patch
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16476
Differential Revision: https://we.phorge.it/D26716
Summary:
A project workboard may have existed before but got disabled. When trying to access this workboard, do not trigger `buildInitializeContent()` which will create yet another default workboard column (and duplicates the existing column on the disabled workboard) but call `buildEnableContent()` which will display the "Workboard Disabled" panel instead allowing to re-enable the workboard.
Also fix some wrong indentation in a section below.
Closes T16475
Test Plan:
1. Create a new project.
2. Select "Workboard" from the left sidebar.
3. Select default "Columns: New Empty Board" for that project, and click the "Create Workboard" button.
4. In the right upper corner, select {nav icon=cog}, click {nav icon=cog,name=Manage Workboard}, and select {nav icon=ban,name=Disable Workboard}
5. End up on resulting http://phorge.localhost/project/board/1/, or go explicitly to that URI by selecting "Workboard" in the project navigation panel on the left
6. Before this patch, it shows the "Create Workboard" panel, allowing you to again select "Columns: New Empty Board", click the "Create Workboard" button, and see now two default columns both named "Backlog (Default)" on the (re-enabled) workboard
7. After this patch, it shows the "Disabled Workboard" panel with a button to re-enable the workboard
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16475
Differential Revision: https://we.phorge.it/D26714
Summary:
Setting null as an array key is deprecated since PHP 8.5 per https://www.php.net/releases/8.5/en.php: "Using null as an array offset or when calling array_key_exists() is now deprecated. Use an empty string instead."
```
ERROR 8192: Using null as the key parameter for array_key_exists() is deprecated, use an empty string instead at [/var/www/html/phorge/phorge/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:287]
```
Closes T16377
Test Plan:
* PHP 8.5
* Set `/config/edit/policy.allow-public` to `true`
* Create a Maniphest task with View Policy = Public
* Access that task while not logged in
* Get two errors less
Reviewers: O1 Blessed Committers, mainframe98
Reviewed By: O1 Blessed Committers, mainframe98
Subscribers: mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16377
Differential Revision: https://we.phorge.it/D26551
Summary:
Add the PHPAST linter in Phorge, to support newer PHP syntax.
Requires that D26516 be applied to rARC locally.
Test Plan:
* Run `arc patch D26516` on `arcanist/`
* Run `arc lint --everything --never-apply-patches --severity disabled --output summary`
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16309
Differential Revision: https://we.phorge.it/D26517
Summary:
Older MySQL versions had performance issues in query optimization for tuple comparisons in an IN clause.
These issues were mostly fixed for MySQL 5.6; see https://bugs.mysql.com/bug.php?id=31188
While there are still some issues (e.g. https://bugs.mysql.com/bug.php?id=115190), nowadays tuple search index usage is generally supported.
Phorge requires MySQL 8 since rP555fb3a8 / T16107. So this sentence is now moot.
Test Plan:
* Search the interwebs
* Run `./bin/diviner generate`, read http://phorge.localhost/book/contrib/article/database/
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/D26472
Summary:
Has never had any translations other than a translation of its own locale name. It's not clear if there was once a translation in some third-party library that I can't find, or if it was one of old upstream's tired jokes.
In any event, nobody plans to do anything with this, so, as cool of an idea this is, it has no point to it.
Test Plan: Don't see emoji in limited translations (or, this needs a companion arcanist patch, so don't it once both are merged)
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/D26711