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.
This patch also fixes another similar strlen() occurrence in the same source file.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/auth/management/PhabricatorAuthManagementCachePKCS8Workflow.php:35]
```
Test Plan: Run `../phorge/bin/auth cache-pkcs8`: Get no `strlen()` error anymore but only expected output `Usage Exception: You must specify the path to a public keyfile with --public.`
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/D25587
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.
This patch also fixes another similar strlen() occurrence in the same source file.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/herald/management/HeraldTestManagementWorkflow.php:33]
```
Test Plan: Run `../phorge/bin/herald test`: Get no `strlen()` error anymore but only expected output `Usage Exception: Specify an object to test rules for with "--object".`
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/D25586
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.
This patch also fixes two similar `strlen()` occurrences in the same source file.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/conduit/management/PhabricatorConduitCallManagementWorkflow.php:44]
```
Test Plan: Run `../phorge/bin/conduit call`: Get no `strlen()` error anymore but only expected output `Usage Exception: Specify a method to call with "--method".`
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25584
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/files/management/PhabricatorFilesManagementEncodeWorkflow.php:45]
```
Test Plan: Run `../phorge/bin/files encode F1`: Get no `strlen()` error anymore but only expected output `Usage Exception: Use --as <format> to select a target encoding format. Available formats are: aes-256-cbc, rot13, raw.`
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/D25595
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/files/management/PhabricatorFilesManagementMigrateWorkflow.php:65]
```
Test Plan: Run `../phorge/bin/files migrate`: Get no `strlen()` error anymore but only expected output `Usage Exception: Specify an engine to migrate to with `--engine`. Use `files engines` to get a list of engines.`
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/D25594
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/herald/management/HeraldRuleManagementWorkflow.php:36]
```
Test Plan: Run `../phorge/bin/herald rule`: Get no `strlen()` error anymore but only expected output `Usage Exception: Specify a rule to edit with "--rule <id|monogram>".`
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/D25585
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/search/management/PhabricatorSearchManagementQueryWorkflow.php:24]
```
Test Plan: Run `../phorge/bin/search query`: Get no `strlen()` error anymore but only expected output `Usage Exception: Specify a query with --query.`
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/D25589
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/management/PhabricatorAuthManagementRefreshWorkflow.php:37]
```
Test Plan: Run `../phorge/bin/auth refresh`: Get no `strlen()` error anymore but only expected output `Found 3 account(s) to refresh.` (or similar)
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/D25583
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/management/PhabricatorAuthManagementUnlimitWorkflow.php:30]
```
Test Plan: Run `../phorge/bin/auth unlimit`: Get no `strlen()` error anymore but only expected output `Usage Exception: Use --user to choose a user to reset actions for.`
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/D25582
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/nuance/management/NuanceManagementWorkflow.php:69]
```
Test Plan: Run `../phorge/bin/nuance update`: Get no `strlen()` error anymore but only expected output `Usage Exception: Specify a item with --item.`
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/D25581
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/nuance/management/NuanceManagementWorkflow.php:8]
```
Test Plan: Run `../phorge/bin/nuance import`: Get no `strlen()` error anymore but only expected output `Usage Exception: Specify a source with --source.`
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/D25580
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
```
ERROR 8192: strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/applications/files/management/PhabricatorFilesManagementGenerateKeyWorkflow.php:23]
```
Test Plan: Run `../phorge/bin/files generate-key`: Get no `strlen()` error anymore but only expected output `Usage Exception: Specify the type of key to generate with --type.`
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/D25579
Summary:
When a column value to export to Tab-Separated Text is empty, `null` is passed to `addcslashes()` which is deprecated behavior since PHP 8.1.
Thus only call `addclashes()` when the value is set.
```
ERROR 8192: addcslashes(): Passing null to parameter #1 ($string) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/export/format/PhabricatorTextExportFormat.php:45]
```
Closes T15771
Test Plan: Export a Maniphest task list of query results to Tab-Separated Text.
Reviewers: O1 Blessed Committers, 20after4, speck
Reviewed By: O1 Blessed Committers, 20after4, speck
Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15771
Differential Revision: https://we.phorge.it/D25568
Summary:
Older git versions allowed the `--skip` to get empty string as value and interpreted that to an int.
We were sending `null` in some places where we wanted a `0`, so it all worked out.
Since https://git.kernel.org/pub/scm/git/git.git/commit/revision.c?id=71a1e94821666909b7b2bd62a36244c601f8430e git fails when provided empty string for an integer.
See Q124 and T15783.
Test Plan: Have recent git version (that fails with `--skip ''`, navigate to `/diffusion/1/history/master/`) - see history.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25575
Summary: The signature and return type of the `getAllTypesForApplication()` function differed from its docs.
Test Plan: Carefully read the code of the function and compare it to the `getAllInstalledTypes()` function above from which the docs were incorrectly copied.
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/D25572
Summary: T15772.
Test Plan: Add a `throw` statement in graph calculation, load a task that has a graph, see message.
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/D25570
Summary:
* Properly handle when no mail headers at all can be parsed
* Properly handle when mail headers can be parsed but no subject line can be found
```
EXCEPTION: (RuntimeException) Undefined index: subject
```
Closes T15769
Test Plan: See T15769
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15769
Differential Revision: https://we.phorge.it/D25565
Summary:
When a column value to export to CSV is empty, `null` is passed to `preg_match()` which is deprecated behavior since PHP 8.1.
Thus only call `preg_match()` when the value is set.
```
ERROR 8192: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/export/format/PhabricatorCSVExportFormat.php:51]
```
```
ERROR 8192: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated at [/var/www/html/phorge/phorge/src/infrastructure/export/format/PhabricatorCSVExportFormat.php:55]
```
Closes T15770
Test Plan: Export a Maniphest task list of query results to CSV.
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15770
Differential Revision: https://we.phorge.it/D25567
Summary:
Certain web servers (or cache layers such as Varnish) strip the body
of GET requests, resulting in the setup check failing when it would
otherwise function normally if a request is sent as POST.
Test Plan:
Phorge will no-longer mark the test as failing due to the body of a GET request being dropped.
As per RFC7231, certain implementations of HTTP/1.1 may reject the request or drop the payload
due to a previous revision of the spec where the body was supposed to be ignored (RFC2616, section 4.3)
Reviewers: O1 Blessed Committers, 20after4, speck, avivey
Reviewed By: O1 Blessed Committers, 20after4, speck, avivey
Subscribers: speck, 20after4, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25566
Summary:
The tab menu in a repository can be a bit confusing. The first tab will be rendered active for two distinctive states:
- the home view in the currently selected branch
- the browse view in the currently selected branch
This diff will give the home view a dedicated extra tab. When switching the branches, we won't switch between home and browse view like it is done now.
And I have changed the crumb text for the browse view to "Code" to match it up with the tab name.
Additionally the readme is now the first content on the home view.
Fixes T15728
Test Plan: Browse in your favorite repos and feel an UX elevation.
Reviewers: O1 Blessed Committers, avivey, valerio.bozzolan
Reviewed By: O1 Blessed Committers, avivey, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15728
Differential Revision: https://we.phorge.it/D25538
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/project/typeahead/PhabricatorProjectDatasource.php:99]
```
Closes T15761
Test Plan: Go to a task, enter `#` and start typing a project tag of a project which has no additional slugs defined.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15761
Differential Revision: https://we.phorge.it/D25556
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/metamta/storage/PhabricatorMetaMTAReceivedMail.php:527]
```
Closes T15767
Test Plan: See T15767
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15767
Differential Revision: https://we.phorge.it/D25564
Summary:
This minimal changes seems the natural expansion of this search function,
that "seems" designed to only search with a single value, but the backend
is designed to receive an array of possible values.
Look at the same method in PhabricatorStandardCustomFieldLink that already
works also for array inputs.
To get extra confidence look at the method withApplicationSearchContainsConstraint()
that works perfectly with arrays (to be honest it only works with arrays in mind...)
This feature allows to avoid crashes when extension developers tries to run "IN" queries
because for example they try to follow our performance guidelines:
https://we.phorge.it/book/contrib/article/n_plus_one/
Closes T15752
Test Plan:
Have a nice custom integer field, like this one for Maniphest:
{
"mycompany.estimated-hours": {
"name": "Estimated Hours",
"type": "int",
"caption": "Estimated number of hours this will take.",
"search": true
}
}
You can reproduce T15752 before the change. It just works after the change.
Or, just trust your instincts by looking at the same method in PhabricatorStandardCustomFieldLink.
Also, use the "normal" frontend search page. Still works as usual.
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15752
Differential Revision: https://we.phorge.it/D25554
Summary:
Add a documentation link in a specific Config page, that is:
Config > Setting > projects.custom-field-definitions
Comparison example:
| User | People |
|-----------|------------|
|{F1633919} | {F1633920} |
This change is a boring follow-up of this one:
https://we.phorge.it/D25507
Test Plan:
Visit the page and click on the new link:
- /config/edit/projects.custom-field-definitions/
Visit this page that was the inspirational page:
- /config/edit/user.custom-field-definitions/
Check that no nuclear implosion is raised.
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25552
Summary:
Fix T15762. There's a complex interaction with project's AutoCapability requiring the membership info
to be already loaded, and the Mention rule assuming any project is fully loaded before trying to check
permissions.
As far as I can tell, the "contextObject" is currently used only to make the mention handle gray if
the mentioned user can't see it.
Test Plan: Load a Project's hovercard that @mentions a user that can't see the project - no crash.
Reviewers: aklapper, O1 Blessed Committers
Reviewed By: aklapper, O1 Blessed Committers
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15762
Differential Revision: https://we.phorge.it/D25562
Summary:
The file name is a sufficient source of information, if available.
Closes T15729
Test Plan: The new unit test is green. Old unit tests are green.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15729
Differential Revision: https://we.phorge.it/D25560
Summary:
Interestingly Phorge allows to paste a Phorge URL in the search bar to be redirected there...
Now this interesting feature was crashing the whole search engine for simple queries like "asdasd"
with the following exception message:
Refusing to redirect to local resource "asdasd". This URI is not formatted in a recognizable way.
You are affected by this crash after this commit:
328aee033f
This specific commit is probably legitimate since "#asdasd" and "/asdasd" are indeed internal URIs,
plus, yes, there are some undocumented and untested cases like "asdasd" that may be considered
internal URIs as well. Or not. But this is just an undocumented corner-case.
In short:
PhabricatorDatasourceURIEngineExtension should not be built over undocumented corner-cases and
should require an absolute URI, with at least a domain.
Note that PhutilURI#getDomain() is always a string and never null, so we can do a strict check.
This fix also involves a micro-optimization to avoid duplicate URI parsing. Since the method was calling
the constructor PhutilURI(string) twice (line 13, line 24). Now just once.
Closes T15763
Test Plan:
Search "lol", no crash anymore.
Copy a random Phorge URL and paste that in your search bar. It still redirects there.
Reviewers: zhe, avivey, O1 Blessed Committers, speck
Reviewed By: O1 Blessed Committers, speck
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15763
Differential Revision: https://we.phorge.it/D25561
Summary:
This is supposed to render in plain text instead of crashing or giving an error, after D25415:
a {{{a}}}
The related unit test is updated accordingly.
We have unnoticed this, because of the well-known bug T15500.
Ref T15372
See D25415
Test Plan:
This makes sense with the introduced functionality in D25415.
Unit tests are happy again.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15372
Differential Revision: https://we.phorge.it/D25559
Summary:
Sorting a Maniphest search query by custom fields throws a "Map returned omits required key" exception.
The `isCustomFieldOrderKey()` check still tested against legacy field key format (for example `[custom:]std:maniphest:deadline.due`) while the code passes modern field key format (for example `custom.deadline.due`).
After fixing this, `PhutilTypeSpec::checkMap()` throws an exception when a non-optional (extra) key `$column` is in `$columns` but not in the array of type parameters below to check against:
`"Got unexpected parameters: customfield, customfield.index.table, customfield.index.key"`
Thus add optional types for customfields in `buildPagingClauseFromMultipleColumns()` to allow them instead of throwing another exception.
Closes T15631
Test Plan:
* Have a custom field (with search: true) defined via http://phorge.localhost/config/edit/maniphest.custom-field-definitions/
* Have two tasks have the custom field set
* Go to http://phorge.localhost/maniphest/query/ , select the custom field (e.g. "Due Date (Reversed)") in the "Order By" field, and click the "Search" button. Iterate through the results via the "Next" button if needed
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15631
Differential Revision: https://we.phorge.it/D25504
Summary:
Example cases that were wrongly considered external URLs:
- "#foo"
- "/foo"
Note that Phorge usually does not change stable things. In fact this thing
was not documented even inline, and was not even in the unit tests.
So this is a fix, and not a breaking change. Probably no one had ever tried it.
Closes T15182
Test Plan:
The already existing unit tests still work.
The added ones make sense.
There are no problems doing other random things.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15182
Differential Revision: https://we.phorge.it/D25555
Summary:
Sometime you just want to flag something as Resolved and keep that task claimed by nobody.
But, there are some task statuses that can auto-claim, and "Resolved" is one of these.
So, if you "Resolved", Phorge tries to set yourself as claimer.
Keeping that "claimed by nobody" is a bit tricky and also generates a confusing warning.
In fact, after you "Resolved", you can override the defaults with:
- Add Action > Assign / Claim > (nobody)
The problem is, on saving, the above action causes this warning:
> **Action With No Effect**
> One of your actions has no effect:
> The task already has the selected owner.
> Apply remaining actions?
> [ Cancel ] [ Apply Remaining Actions ]
That warning "The task already has the selected owner" really means
"The task is already claimed by nobody" and, indeed, that is exactly what the user wants.
This patch intercepts the above action, and prevents the related confusing "non-effect" warning.
Thanks to hard troubleshooting from user https://we.phorge.it/p/aklapper/
See also https://we.phorge.it/D25476
Closes T15164
Test Plan:
Task 1 open unassigned:
1. Change Status to Resolved
2. Preview yourself as Claimer
3. Add Action > Assign / Claim, and **set <nothing>**
4. Save and, instead of any confusing warning, only the Status changes.
Task 2 open unassigned:
1. Change Status to Resolved
2. Preview yourself as Claimer
3. Add Action > Assign / Claim, keep it as-is
4. Save and, it works as expected (just like before)
Task 3 open unassigned:
1. Change Status to Resolved
2. Preview yourself as Claimer
3. Add Action > Assign / Claim, set to somebody else
4. Save and, it works as expected (just like before)
Reviewers: O1 Blessed Committers, aklapper
Reviewed By: O1 Blessed Committers, aklapper
Subscribers: tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15164
Differential Revision: https://we.phorge.it/D25550
Summary:
This change is probably needed after 892f527b9f.
At that time, the unit test was successfully completed probably because of a lack
of git rebase master.
Test Plan: The unit test PhabricatorCelerityTestCase::testCelerityMaps is not screaming.
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25553
Summary:
To prevent spammers from abusing this feature on a public server, do not include query parameters in the generated filenames. See <d8bb7d91b7>.
Ref T15665.
Test Plan: Download raw diff from a revision and check filename in URL.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15665
Differential Revision: https://we.phorge.it/D25478
Signed-off-by: Zero King <l2dy@icloud.com>
Summary:
`./bin/lipsum` allows to fill up a Phorge instance with bogus data for testing. When creating a project, the code creates a project title, project status, random subscribers/members and picks a random project creator, but does not set a project description.
Make this code also set a project description (which is implemented as a custom field).
Closes T15748
Test Plan:
* On the CLI, use `./bin/lipsum generate projects` to create a bogus project.
* In the web browser, go to `/project/query/all/` and open the project page of the recently created bogus project: See no project description.
* Apply this change.
* On the CLI, use `./bin/lipsum generate projects` to create another bogus project.
* In the web browser, go to `/project/query/all/` and open the project page of the recently created bogus project: See a random project description.
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15748
Differential Revision: https://we.phorge.it/D25545
Summary:
PhabricatorDefaultSyntaxStyle works now with 'CSS-color-aliases' instead of fixed colors.
The aliases are translated in the corresponding themes CelerityPostprocessor.
| Before | After |
|-----------|-----------|
| {F709419} | {F709421} |
Ref T15056
Test Plan:
1) bin/celerity syntax
2) bin/celerity map
3) bin/cache purge --all
4) sign in
5) go to user's Settings > Display Preferences and select the Accessibility (user interface) "Dark Mode".
6) go to Phriction or Maniphest and add a codeblock.
Example:
``` lang=php
<?php
final class Example extends PhabricatorApplication {
}
```
7) verify that codeblock content is readable
8) also check that with/without this, nothing changes in "normal" theme
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/D25509
Summary:
One preview is better than a thousand of words:
{F270656}
This Sound is probably universally recognized thanks to
videogames like Super Mario® or other popular platforms; but
this specific Coin Sound is not proprietary: it's released
with a Free as in Freedom of Speach license: Creative Commons 0.
As if that were not enough, this Coin sound, instead of
potentially creating frustration like many others - it positively
stimulates part of the human cerebral cortex involved in making
humans more addicted to Phorge. To be precise, this Coin sound is
particularly suitable to be associated with Trigger actions
such as:
- Mark as Resolved
- Move on to the next Milestone
- etc.
But the only limitation here, as usual in Phorge, is
your imagination.
Insert a Coin and have fun with Phorge.
Complete sound credits:
https://freesound.org/people/Jocabundus/sounds/678385/
2023, Jocabundus, Creative Commons Zero (public domain dedication)
This change also introduces a Credits page in Diviner to mentions this
kind of things. Future contributions in that page are encouraged.
Insert a Coin. Play again with Phorge.
Ref T15178
Ref T15248
Test Plan:
- Create a Project with a Workboard
- Create a Column
- Column > Gear > New Trigger...
- Action > Play Sound > "Coin"
- Create a Task on the Backlog column, and drop it on your new Column
- Turn on audio, have fun!
Reviewers: O1 Blessed Committers, Cigaryno, avivey, aklapper
Reviewed By: O1 Blessed Committers, Cigaryno, avivey, aklapper
Subscribers: avivey, aklapper, chris, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15178, T15248
Differential Revision: https://we.phorge.it/D25079
Summary:
Improve remarkup reference documentation.
* Added a heading for `Code Blocks`
* Added a sentence mentioning the ##```lang## syntax which was added in {T15481}
Test Plan:
* Run `bin/diviner generate` to regen diviner docs.
* Check out the code blocks section of [[/book/phorge/article/remarkup/#code-blocks|Remarkup Reference]]
Reviewers: valerio.bozzolan, O1 Blessed Committers
Reviewed By: valerio.bozzolan, O1 Blessed Committers
Subscribers: tobiaswiese, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25547
Summary:
If you are reading this commit message, it probably happened to you this situation:
1. Somebody published a comment that shows the file F123
2. You Submit another comment that shows F123 again
After you click "Submit", a wild warning may appear:
> Post as Comment
> ----
> The action you are taking has no effect:
> Transaction (of type "core:file") has no effect.
> Do you want to post your comment anyway?
> [Undo] [Continue]
In this case you probably contact your favorite Phorge administrator to translate that stuff.
This is an example unofficial translation:
> Post as Comment
> ----
> You mentioned a file. Phorge loves files. Phorge attach files. But somebody else already
> mentioned that file, successfully. So, since Phorge attach Files, Phorge will result in
> doing nothing special, since the file is already perfectly attached by Phorge.
> Phorge will just post your comment.
> Would you like me to just STFU and post your comment?
> [OMG I'm scared] [Post comment and STFU]
After this change you don't see anymore that warning. So it's even rarer in your Pokedex.
Closes T15293
Test Plan:
- Create a comment showing F123 (an existing file)
- Create another comment showing F123
- No wild warnings
Reviewers: O1 Blessed Committers, 20after4
Reviewed By: O1 Blessed Committers, 20after4
Subscribers: 20after4, speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15293
Differential Revision: https://we.phorge.it/D25544
Summary:
As of February 2024, phurl.io (which is not run by Phorge.it) shows HTTP 503 errors instead of redirecting to the target URIs.
Thus replace any URIs pointing to phurl.io with the corresponding target URIs, based on the mapping listed on https://secure.phabricator.com/phurl/.
Closes T15746
Test Plan: Carefully read the diff.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15746
Differential Revision: https://we.phorge.it/D25543
Summary: Ref Q112, ref T15064.
Test Plan:
Enter the text `[[hello]]` in a remarkup field - without this change, it causes a deprecation warning in the log.
This doesn't happen if the content looks like a valid URL - this is the "Wiki Link" syntax.
Reviewers: bekay, O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15064
Differential Revision: https://we.phorge.it/D25539
Summary:
This diff adds a copy button to every repo uri in the clone repo modal. I have made the button to select the text to a merely structural span before the input - it just shows the type of the repository uri. When you click inside the input, the entire uri will be selected. Also I have uncluttered the HTML structure. A table is not needed here, nothing a flex block can't handle.
| Before | After |
|-----------|-----------|
| {F1360344} | {F1368592} |
While at it, I have extended the used javascript copy behavior. First of all: `document.execCommand('copy')` [[ https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand | could stop working every moment in every browser ]]. The [[ https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Interact_with_the_clipboard | new clipboard API ]] is the way to go, so I have implemented it as the preferred method. The old method is kept as a fallback. And I have added a very nice feature: If defined, the behavior will now issue success or error notifications. See the changed UIExamples for that.
To support the shrinking of JS code with async functions I have patched the JsShrink source.
Test Plan: Go to a repository, hit the clone button and use the new copy button. You will see a shiny notification as a reward.
Reviewers: O1 Blessed Committers, avivey, valerio.bozzolan
Reviewed By: O1 Blessed Committers, avivey, valerio.bozzolan
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Differential Revision: https://we.phorge.it/D25536
Summary:
When GD is not installed, trying to set a custom image for a project/blog/repository/user/etc displays unhelpful error messages (`This server only supports these image formats: .` and `Supported formats: `) due to the array of supported image formats being empty.
Display clearer messages instead.
Closes T15720
Test Plan: Do not have php-gd installed, go to `/project/manage/1/`, take a look at the string below the "Upload Picture" button, select {nav icon=picture, name=Edit Picture} in the sidebar, select `Custom: Choose Icon and Color...`, `Choose Background Color` and `Choose Icon`, then click the `Save Image` button. Also try to upload a custom image and look at the error message.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15720
Differential Revision: https://we.phorge.it/D25525
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/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php:56]
```
Closes T15685
Test Plan: After configuring a custom `int` field and a dashboard panel to query and listed the latest created tasks, access the panel and check the PHP error log.
Reviewers: O1 Blessed Committers, speck, valerio.bozzolan
Reviewed By: O1 Blessed Committers, speck, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15685
Differential Revision: https://we.phorge.it/D25489
Summary:
While many Phorge applications can be uninstalled (such as Differential for pre-merge commit review), this has not been the case for Audit (post-merge commit review) since rP11861265fe94fa97e4d0563c5bdb7b8cac27282d.
In installations which do not use Audit in their workflows, exposing Audit related UI elements creates additional clutter and complexity.
Fixes T15129
Test Plan:
* Go to `/applications/view/PhabricatorAuditApplication/` and click "Uninstall"
* View an individual merged commit in Diffusion, click "Edit Commit" in the sidebar to go to `/diffusion/commit/edit/1/`, see that "Auditors" datasource is not displayed
* View an individual merged commit in Diffusion, click the "Add Action..." dropdown, see that the "Audit Action" section and its entries "Accept Commit" and "Raise Concern" are not displayed
* View an individual merged commit in Diffusion, click the "Add Action..." dropdown, see that the "Change Auditors" entry is not displayed
* Go to `/conduit/` and see that the API method "audit.query" and the entire section "audit" is not displayed
* Go to `/diffusion/commit/` and see no "Active Audits" and "Audited" searches in the left side bar
* Go to `/diffusion/commit/query/all/` and see that no "Auditors" entry is shown for each commit
* Go to `/diffusion/commit/query/all/` and see that no audit related entry is shown in the grey column on the right (if Harbormaster is not installed, there will be no grey column at all)
* Go to `/herald/edit/?content_type=commit&rule_type=global` and see that no Condition entry "Auditors" and no Condition entry "Affected packages that need audit" is displayed
* Go to `/herald/edit/?content_type=commit&rule_type=personal` and see that no "Add me as an auditor" Herald action and no "Added Auditors" Herald condition is displayed
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15129
Differential Revision: https://we.phorge.it/D25503
