1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 22:10:55 +01:00
Commit graph

17319 commits

Author SHA1 Message Date
sten
007e7621f2 Fix HarbormasterBuildUnitMessage PHP 8.1 strlen(null) error
Summary:
Fix HarbormasterBuildUnitMessage PHP 8.1 strlen(null) error.

Fixes T15572

Test Plan: Do an 'arc diff' to a PHP 8.1 Phorge server when doing so will trigger unit tests.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15572

Differential Revision: https://we.phorge.it/D25365
2023-07-29 13:04:11 +01:00
sten
1802ebd2cc Fix PHP 8.1 Diffusion history errors
Summary:
Fix multiple PHP 8.1 errors when viewing Diffusion history.

Fixes T15573

Test Plan: View a diffusion history page. Eg https://my.php81.phorge.site/source/myrepo/history/master/

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15573

Differential Revision: https://we.phorge.it/D25366
2023-07-29 13:03:48 +01:00
Andre Klapper
fa687b4ba0 Always ellipsize long filename in file upload dialog
Summary:
Firefox does not ellipsize long filenames after selecting them in the File Upload dialog; Chromium does (for unknown reasons).

Could hardcode the "remaining" width for the `<input>` element itself (approx. 340px) based on calculating the CSS widths and margins of all surrounding elements but that is error-prone if CSS for one of those surrounding elements ever changed.
Thus instead use `max-width: stretch` for the `<input>` element itself. Per its limited support (see https://caniuse.com/?search=max-width), set also `-moz-available` (https://bugzilla.mozilla.org/show_bug.cgi?id=1495868) and `-webkit-fill-available` (see https://bugs.chromium.org/p/chromium/issues/detail?id=611857) aliases.

Closes T15553

Test Plan:
* Both in Firefox and in Chromium, go to an existing task, select "File Upload", select a file with a long name.
* See that the filename is now correctly ellipsized in Firefox.
* See no changes in Chromium.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15553

Differential Revision: https://we.phorge.it/D25347
2023-07-29 12:14:50 +03:00
c7574226
189a5d6047 Fix strlen(null) PHP 8.1 error when pushing into a fresh repository
Summary:
Doing a git push into a fresh repository over https fails with a PHP 8.1 strlen(null) error.

This fixes it.

Fixes T15569

Test Plan:
* Create an empty repository on a PHP 8.1 Phorge instance
* Clone it
* Add some data
* git push

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15569

Differential Revision: https://we.phorge.it/D25361
2023-07-28 18:22:55 +01:00
Valerio Bozzolan
1aa532a476 Workboard Columns: avoid page reload on Edit > Cancel
Summary:
The rationale for this change is that reloading a page unnecessarily wastes
resources that we can save.

This includes human resources (time) but also computational resources
as well. We probably save a fawn for every 1,000 clicks on that button.
A message to Greta Thunberg: now Phorge is with you.

This is the involved "Cancel" button, that does not reload the page anymore:

{F309894}

Closes T15471

Test Plan:
Workboard > Column > Edit > Cancel:

The popup closes gracefully instead of a page refresh.

Also try opening the workflow in another tab (/project/board/1/hide/2/) > Cancel:

You turn back to your Workboard just as before.

Reviewers: O1 Blessed Committers, Matthew

Reviewed By: O1 Blessed Committers, Matthew

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15471

Differential Revision: https://we.phorge.it/D25302
2023-07-28 19:02:40 +02:00
Valerio Bozzolan
1e3c9bdfee Update Diff: fix dead-end when not yours
Summary:
Fix "Update Patch" for generic Differential revisions.

You can now help a coworker to update their patch, from the web.

So, if you visit Diff 111 (not yours) and you click "Update Diff", and you paste a valid diff,
you arrive here:

    https://we.phorge.it/differential/diff/222/?revisionID=111

That page as default was suggesting some Diffs that are yours. Plus, now it shows Diff 111.

Omitting the Diff from which the workflow was started made no sense and it's certainly an oversight.

Any follow-up change is welcome to modernize the selector using AphrontFormTokenizerControl.

Closes T15538

Test Plan:
- create a Diff on Differential with user A
- click on Update Diff from user B (pasting a valid diff) and Continue

Now that patch is selected (and selectable). No dead-end anymore.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15538

Differential Revision: https://we.phorge.it/D25337
2023-07-25 14:15:03 +02:00
Aviv Eyal
bf025c5fb4 Aphlict - search for default config in the right place
Summary: Fix T15502.

Test Plan: Renamed `phorge` dir few times, run `./bin/aphlict status` to see message.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15502

Differential Revision: https://we.phorge.it/D25344
2023-07-24 00:42:06 -07:00
Valerio Bozzolan
0f07c07b3b Attach Diff to Revision: fix Repository asked twice
Summary:
If you manually upload a Diff (patch) from the Differential web
interface you can select the Repository.

We fix the default value of the Repository field during creation mode,
guessing the repo from the potentially already-available diff.

Closes T15537

Test Plan:
- Create a cute Differential Revision from web
- Pick a Repository > Continue
- Create a new Revision > Continue

At this point you should still have that repository instead of nothing.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15537

Differential Revision: https://we.phorge.it/D25338
2023-07-22 13:55:41 +02:00
Valerio Bozzolan
0dfc84e2ab Ponder Answer Comments: indent textarea as Comments
Summary:
In a Ponder Answer, when adding a Comment, the textarea indentation was a
a little bit unintuitive.

After this change, the textarea is aligned as a Comment:

| Before    | After     |
|-----------|-----------|
| {F286220} | {F286221} |

Other changes are welcome but this seems to me a good minimum.

I also hope this could help not to confuse the Comment field with the Answer field.

Closes T15350

Test Plan:
- create a Ponder Question
- create an Answer
- look at the Comments input field that now should be more nicely indented
- the normal Response field is not changed at all
- the mobile view is not changed at all
- click on "Pin Form On Screen" and it must be as before

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15350

Differential Revision: https://we.phorge.it/D25192
2023-07-19 21:28:24 +02:00
Steve Campbell
cb9fcf18d5 Fix PHP 8.1 "strlen(null)" exception trying to stop maintenance mode of Diffusion repo
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.

Use `isset` instead to check if the argument "start" was passed to the CLI command.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=repoMaintStartHistory, ref.master=1c098c273d06, ref.repoMaintStartHistory=0a4a34143528)
  #0 PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/management/PhabricatorRepositoryManagementMaintenanceWorkflow.php:45]
```

Closes T15476

Test Plan: After applying this change, executing `./bin/repository maintenance --stop R1` on the CLI shows `Took repository "R1" out of maintenance mode.` as expected, instead of an exception.

Reviewers: O1 Blessed Committers, valerio.bozzolan, aklapper, avivey

Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey

Subscribers: Sten, avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15476

Differential Revision: https://we.phorge.it/D25296
2023-07-18 09:06:25 +01:00
Steve Campbell
27fa498966 Fix PHP 8.1 strlen(null) error in PhabricatorFile::newChunkedFile()
Summary:
Fix PHP 8.1 strlen(null) error in PhabricatorFile::newChunkedFile().

Fixes T15499

Test Plan:
Added a unit test which replicated the fault, so you can test simply by 'arc unit'
Alternatively, see test in T15499

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15499

Differential Revision: https://we.phorge.it/D25352
2023-07-17 14:24:41 +01:00
Steve Campbell
dbc101ca8a Fix PHP 8.1 substr(null) in People > Activity Logs
Summary:
When someone tries to log on with invalid credentials, a record is inserted in the phabricator_user.user_log table with action = login-fail.
The session value of this record is set to null in this case.

When you open the People → Activity Logs screen, you will see an exception because of this.

Fixes T15555

Test Plan: Go to https://my.phorge.site/people/logs/ after a login failure and a login success.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15555

Differential Revision: https://we.phorge.it/D25351
2023-07-17 14:23:30 +01:00
Steve Campbell
849e18ccbb Fix PhabricatorRepository generateURI PHP 8.1 strlen(null) errors
Summary:
When viewing a repository in Diffusion, clicking on a folder (eg https://my.phorge.site/source/myrepo/browse/master/myfolder/) will generate multiple strlen(null) exceptions under PHP 8.1

Fix is to replace all the strlen() calls with phutil_nonempty_string()

Fixes T15532

Test Plan: View a folder in a repo in Diffusion. Eg https://my.phorge.site/source/myrepo/browse/master/myfolder/

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15532

Differential Revision: https://we.phorge.it/D25336
2023-07-17 11:07:25 +01:00
Steve Campbell
8af1624692 Fix passing null to trim() error in DiffusionBrowseTableView render()
Summary:
When viewing the top level of a diffusion repository, $request->getPath() returns null, which then causes trim() to throw an error.

Fix is to update DiffusionRequest getPath() such that it returns '' instead of null, thus fixing it's return type.

Fixes T15522

Test Plan: Go to https://my.phorge.site/source/myrepo

Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey

Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15522

Differential Revision: https://we.phorge.it/D25329
2023-07-17 11:05:18 +01:00
Steve Campbell
65e57fe23d Fix PHP 8.1 PhabricatorEditorURIEngine::newForViewer() trim(NULL) error
Summary:
Under PHP 8.1, PhabricatorEditorURIEngine::newForViewer() is throwing a trim(NULL) error when trying to view a diff.
This is because it tries to apply string operations to a user setting which will be null by default.

Fixes T15518

Test Plan:
Unit test added -
  arc unit

Or just view a diff. Eg:
	https://my.phorge.site/D1234

Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey

Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15518

Differential Revision: https://we.phorge.it/D25324
2023-07-17 11:03:52 +01:00
Andre Klapper
682fe971a4 Do not crop user avatar image in top bar when no Real Name set
Summary:
Increase required height for avatar image

Patch provided by Stang in https://phabricator.wikimedia.org/T307492

Closes T15552

Test Plan:
* Set `/config/edit/user.require-real-name/` to `Make real names optional`
* Log in as a user without a real name set
* Open the user menu in the top bar and look at the user avatar image

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15552

Differential Revision: https://we.phorge.it/D25348
2023-07-17 12:03:23 +02:00
Steve Campbell
8cbbc9f961 Fix strlen(null) in DifferentialChangesetViewController loadCoverage()
Summary:
DifferentialChangesetViewController loadCoverage() fails under PHP 8.1 when test coverage is provided for some but not all modified files.

Update the code to replace strlen() with phutil_nonempty_string().

Fixes T15521

Test Plan: View a diff with test coverage for some but not all files.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15521

Differential Revision: https://we.phorge.it/D25328
2023-07-17 11:02:38 +01:00
Steve Campbell
f7dd8f579a Fix /dashboard/adjust/remove/ PHP 8.1 strlen(null) errors
Summary:
/dashboard/adjust/remove/ POST requests fail under PHP 8.1 with strlen(null) errors.

This change fixes it.

Fixes T15549

Test Plan:
View a dashboard which has some panels. Eg https://my.phorge.site/dashboard/view/1/
Click on a 'Manage Panel' dropdown and click on 'Remove Panel'

Confirm the panel is removed and no strlen(null) errors are generated.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15549

Differential Revision: https://we.phorge.it/D25345
2023-07-15 12:43:52 +01:00
Andre Klapper
85539ce255 Blur profile image when user account disabled
Summary:
Add blur and sepia filter to disabled user profile image.

Before this change, the image was just greyed out but still fully recognizable.

Closes T15515

Test Plan:
* Create an account with a custom profile picture
* Disable the account
* Go to user profile of the account
* Go to a task with activity of that account and hover over the account to open card view

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15515

Differential Revision: https://we.phorge.it/D25322
2023-07-12 22:05:56 +02:00
Steve Campbell
3703588e9d Fix PHP 8.1 strlen(null) in DiffusionView linkBrowse()
Summary:
Fix PHP 8.1 strlen(null) in DiffusionView linkBrowse().

Fixes T15542

Test Plan:
# Go into a diff (eg https://my.phorge.site/D1234)
# Click on the Revision Contents - History tab
# Click on a base link (the short git revision ID)

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15542

Differential Revision: https://we.phorge.it/D25340
2023-07-12 13:38:23 +01:00
Andre Klapper
fb822ba849 Update Facebook icon on Auth Provider page
Summary: Closes T15545

Test Plan: Go to `/auth/config/new/` in browser and look at icons.

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15545

Differential Revision: https://we.phorge.it/D25342
2023-07-11 13:00:36 +02:00
Valerio Bozzolan
7bebfa289a Diffusion commits: add some edge phrases about Revisions
Summary:
This change implements some messages that were only present
from this class:

    DifferentialRevisionHasCommitEdgeType.php

But not in its inverse class:

    DiffusionCommitHasRevisionEdgeType.php

In short:

| Before    | After     |
|-----------|-----------|
| {F313282} | {F313281} |

Closes T15314

Test Plan:
Visit a Diffusion commit and use the sidebar menu "Edit Related Objects" to:

- add 1 revision
- add 2 revisions
- remove 1 revision
- remove 2 revisions
- add 1 revision, remove 1 revision
- add 2 revisions, remove 2 revisions

Check that you don't see phrases like "added an edge" anymore.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15314

Differential Revision: https://we.phorge.it/D25306
2023-07-06 13:32:21 +02:00
Valerio Bozzolan
92861cc1f0 Dashboard Tabs: fix action "Move Right"
Summary:
It seems that if you use the "Move Right" action, you can get
an unrelated error message about "Move Left":

    This is already the first tab. It can not move any farther to the left.

This happened because there was an incomplete condition.

Closes T15493

Test Plan:
- visit /dashboard/panel/ and create at least two panels (please, owned by you)
- visit a Dashboard, and add a Tabs Panel
- take the first Tab: "Move Right"

After this change, "Move Right" now always works.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15493

Differential Revision: https://we.phorge.it/D25307
2023-07-06 13:31:32 +02:00
Valerio Bozzolan
cae1246359 Dashboard Tabs: render "Move" buttons grayed out more consistently
Summary:
If you happen to create a Dashboard by retrieving other people's panels,
you were probably affected by this visual bug.

It seems that you cannot Move things, but you really can (and that is OK).

The reason is, to move things in a Parent Panel, you only need edit permission there,
and you do not need to also have edit permissions on every Child Panel.

Closes T15492

Test Plan:
- From user A:
    - Create two panels - editable by: User A
- From user B:
    - Create a "Tabs Panel" in a Dashboard
    - Attach the panels from User A (so, editable by, not you)

The meaningful "Move" actions should be not disabled anymore.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15492

Differential Revision: https://we.phorge.it/D25308
2023-07-06 13:31:14 +02:00
Steve Campbell
b3ac5ceb38 Fix DifferentialGetCommitMessageConduitAPIMethod execute strlen(null)
Summary:
When iterating through the fields of a differential commit, the DifferentialGetCommitMessageConduitAPIMethod execute method explicitly allows a value to be either a string or a null. It then calls strlen upon this possibly null value.

We could replace the strlen with phutil_nonempty_string, but as the code has already eliminated variable types other than string or null, it is more efficient to explicitly check for null or ''
```
$value === null or $value == ''
```

Fixes T15527

Test Plan:
Run
```
arc diff
```

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15527

Differential Revision: https://we.phorge.it/D25332
2023-07-06 09:42:20 +01:00
Aviv Eyal
d5a28e12a0 Project Hovercards: Show Description
Summary: Fixes T15275.

Test Plan:
Project pages have "Developer > View Hovercard" button. Or hover over a project handle, I guess.

Also test for empty description and with the description field disabled.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15275

Differential Revision: https://we.phorge.it/D25331
2023-07-05 13:31:55 -07:00
Andre Klapper
71f38a90a3 Fix PHP 8.1 "strlen(null)" exception on History page of Diffusion repo after setting maintenance mode
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.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=master, ref.master=1c098c273d06)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/repository/xaction/PhabricatorRepositoryMaintenanceTransaction.php:28]
```

Closes T15475

Test Plan:
Play a bit with maintenance messages:

    ./bin/repository maintenance --start "Start"      R1
    ./bin/repository maintenance --start "Start Yeah" R1
    ./bin/repository maintenance --stop               R1

Then visit `/diffusion/1/manage/history/` and look at `Diffusion put this repository into maintenance mode.` as expected, instead of an exception.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15475

Differential Revision: https://we.phorge.it/D25295
2023-07-05 19:41:51 +02:00
Andre Klapper
f45d6421a1 Fix PHP 8.1 "strlen(null)" exception opening a File overlay (Lightbox comment frame)
Summary:
The error was caused by the overlay lightbox, since it gives the possibility to share a comment on any file
in any general Remarkup text, but that comment textarea is indeed empty as default.

`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.

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418), phorge(head=T15498, ref.master=a25af8ccef69, ref.T15498=a25af8ccef69)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/files/controller/PhabricatorFileLightboxController.php:23]
```

Closes T15498

Test Plan: Visit a blog post with a file/image dropped in the content and click on the thumbnail. No error overlay dialog is displayed anymore.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15498

Differential Revision: https://we.phorge.it/D25310
2023-07-05 19:38:44 +02:00
Steve Campbell
9d512595c0 Fix PHP 8.1 DifferentialBranchField getBranchDescription strlen(null) error
Summary:
Fix PHP 8.1 strlen(null) error in DifferentialBranchField getBranchDescription()

Fixes T15531

Test Plan:
Simply viewing a diff (eg https://my.phorge.site/D1234) on a PHP 8.1 server generated the error fixed by this diff.

Also created a unit test.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15531

Differential Revision: https://we.phorge.it/D25335
2023-07-05 16:06:26 +01:00
Steve Campbell
9bf5e17352 PHP8.1 fix for DiffusionServeController serveRequest()
Summary:
When a 'git pull' is done to an https git URL, the $_SERVER variables PHP_AUTH_USER and PHP_AUTH_PW will be unset, causing PHP 8.1 to throw strlen(null) errors.

This update fixes the issue by defaulting the values to '', which results in $have_user and $have_pass having false values as desired.

Fixes T15520

Test Plan: arc unit

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15520

Differential Revision: https://we.phorge.it/D25327
2023-07-05 15:37:45 +01:00
Steve Campbell
5e48e16f77 Update DiffusionDocumentRenderingEngine to cope with null code coverage in PHP 8.1
Summary:
Update DiffusionDocumentRenderingEngine to cope with null code coverage in PHP 8.1

Fixes T15523

Test Plan: https://my.phorge.site/source/myrepo/browse/master/myfile

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15523

Differential Revision: https://we.phorge.it/D25330
2023-07-05 08:44:24 +01:00
Valerio Bozzolan
80484b76a5 Remarkup Code-block: parse language specifier in markdown
Summary:
We add support to code blocks with the language expressed as GitLab/GitHub/StackOverflow/...
"flavored markdown".

So we support this syntax: (to avoid confusion see it online on the Diff)

    lang=text
    ```php
    $asd = 1;
    ```

Before this change, this was the only supposed syntax in Remarkup, with an explicit "lang=":

    lang=text
    ```lang=php
    $asd = 1;
    ```

This change introduces a minor risk to eat legitimate Remarkup content, since Remarkup allows
to do a multi-line in this way:

    lang=text
    ```$asd = 1;
    $asd = 2;```

The above example still works, but, there is a chance that hardcore Remarkup people
have a problem when doing a code block to mention programming languages.

In short, this can be problematic since "cpp" will be eaten from this list:

    COUNTEREXAMPLE
    ```cpp
    php
    python
    ```

Using the above example is not socially nice because it is not usable in GitLab, GitHub and Stack Overflow.

If your first line is eaten:

Just *add* a newline on the top to reach a valid raw Markdown list (suggested, valid in Remarkup + Markdown):

    lang=text
    ```
    cpp
    php
    python
    ```

Or, just add "text" to specify that as language (suggested, valid in Remarkup + Markdown):

    lang=text
    ```text
    cpp
    php
    python
    ```

Or, just *remove* a newline from the bottom to reach a valid raw Remarkup list (Remarkup-only):

    lang=text
    ```cpp
    php
    python```

Or, just specify that you are writing in the language "text" (Remarkup-only):

    lang=text
    ```lang=text
    cpp
    php
    python```

To reduce impact and help you, the logic of this strict implementation is:

- must have backticks
- must not have any valid remarkup option, like lang=, counterexample, etc.
- must not have content in the same line of the last backticks
- must have a known language in our proposed subset

If everything is OK, we remove that language from the content since it would be otherwise displayed.

Interestingly, this could improve performance when rendering README files or snippets from external
websites, since - in case - we do not need to guess the language using our deep dark magic.

Closes T15481

Test Plan:
We added some nice unit tests. Ensure that this test passes:

    PhutilRemarkupEngineTestCase::testEngine

Optionally, take vision of these, before and after:

https://we.phorge.it/P16

Change the test plan slightly every time, to make sure it is not in your cache.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15481

Differential Revision: https://we.phorge.it/D25299
2023-07-04 18:23:14 +02:00
Valerio Bozzolan
4a0d3ba3e1 Locate File: allow to search './path/to/something.txt'
Summary:
Before this change, if you search a file using Locate File,
this was the only accepted syntax for a full-path:

    path/to/something.txt

After this change, some relative/absolute Unix-like variants
are also accepted:

    ./path/to/something.txt
    /path/to/something.txt

Similar prefixes can be frequent when you quickly copy-paste things
from shell commands like 'grep' or 'find' etc.

Ref T15508

Test Plan:
Visit a lovely repository like Phorge and use Locate File with these:

1. src/applications/maniphest/editor/ManiphestEditEngine.php
2. /src/applications/maniphest/editor/ManiphestEditEngine.php
3. ./src/applications/maniphest/editor/ManiphestEditEngine.php

After this change, also 2. and 3. return the expected file.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15508

Differential Revision: https://we.phorge.it/D25320
2023-07-04 17:12:57 +02:00
Aviv Eyal
603cf474ee Fix int fields for now
Summary: Fix T15516

Test Plan: Viewed an "int" custom field, no boom.

Reviewers: speck, valerio.bozzolan, O1 Blessed Committers

Reviewed By: valerio.bozzolan, O1 Blessed Committers

Subscribers: tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15516

Differential Revision: https://we.phorge.it/D25325
2023-07-04 02:26:46 -07:00
Aviv Eyal
d725ffaa77 Fix "Register with Approval flow" for php 8
Summary: Ref T15064.

Test Plan:
- Register as a new user
- as admin, view user's page and approve it
- As new user, hit "wait patiently" to go back to Home

No crashes in this flow.

Reviewers: O1 Blessed Committers, chris

Reviewed By: O1 Blessed Committers, chris

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15064

Differential Revision: https://we.phorge.it/D25317
2023-07-02 08:49:12 -07:00
Aviv Eyal
9aafbb42ef fix strlen in PhabricatorMailManagementShowOutboundWorkflow
Summary: Ref T15064

Test Plan: run `./bin/mail show-outbound --id 1` on a non-html email with php 8.1, no crash.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Tags: #php_8_support

Maniphest Tasks: T15064

Differential Revision: https://we.phorge.it/D25275
2023-07-02 08:49:02 -07:00
MacFan4000
aebd7a2f72 Update a couple of lingering instances of secure.phabricator.com to we.phorge.it
Summary: We were still pointint to secure.phabricator.com in a couple places. This fixes that.

Test Plan: No testing needed

Reviewers: O1 Blessed Committers, speck, Cigaryno

Reviewed By: O1 Blessed Committers, speck, Cigaryno

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25316
2023-06-30 12:01:54 -04:00
Steve Campbell
73253489bf Make src/infrastructure/javelin/markup.php phabricator_form PHP 8.1 compliant
Summary:
src/infrastructure/javelin/markup.php phabricator_form() performs a strcasecmp() operation on an optional attribute withoiut checking to see if it exists first. This causes an 'Passing null to parameter #1 ($string1) of type string is deprecated' error.

Because we subsequenly check to see that the value equals /POST/i, all we need to do is check that the value has some 'true' value before doing the strcasecmp(). If it's not true, then it won't be POST!

Fixes T15509

Test Plan: View a diff. Eg: https://my.phorge.site/D1234

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15509

Differential Revision: https://we.phorge.it/D25319
2023-06-30 15:13:20 +01:00
Steve Campbell
101ce834b8 Fix PHP 8.1 strlen() error when database replication Seconds_Behind_Master is NULL
Summary:
When MySQL database replication is configured, but not running, the slave status Seconds_Behind_Master is NULL.
When this value is passed to strlen() an error is thrown.

Fixes T15507

Test Plan:
* Configure database replication such that Seconds_Behind_Master is NULL
* Restart php-fpm
* Go to Phorge web site.

Reviewers: O1 Blessed Committers, chris

Reviewed By: O1 Blessed Committers, chris

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15507

Differential Revision: https://we.phorge.it/D25318
2023-06-30 13:49:32 +01:00
Taavi Väänänen
0a17287e08
Generalize references to PHP 5.x Debian/Ubuntu packages
Summary:
Both Debian and Ubuntu have been shipping PHP 7.x (or now 8.x) for a while.
This patch replaces the versioned php5-foo package references with more general
php-foo package names.

Test Plan: Navigated to the documentation pages that this patch updates.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25315
2023-06-29 15:58:54 +03:00
Valerio Bozzolan
8ffa604462 Diviner: mention how to flag Arcanist and Phorge as "safe" git repos
Summary:
This tip is useful if you are affected by the following error in git or similar:

    fatal: detected dubious ownership in repository at '/var/www/phorge'

Just follow the new tip from Diviner to fix at system level.

Ref T15282

Test Plan:
- see if you like the documentation tip
- try to execute the command, and verify that your computer does not explode
- eventually visit the /config/ page with and without that change (with and without a /etc/gitconfig)

Reviewers: O1 Blessed Committers, avivey, Cigaryno

Reviewed By: O1 Blessed Committers, avivey, Cigaryno

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15282

Differential Revision: https://we.phorge.it/D25236
2023-06-28 23:14:31 +02:00
Valerio Bozzolan
1005e38160 Fix notification message when Aphlict is not configured
Summary:
Before this change the footer in the notification bar was a bit
weird on the message "Notification server not enabled":

| Before    | After     |
|-----------|-----------|
| {F315024} | {F315001} |

Also a bit wrong on the "Connecting..." message:

| Before    | After     |
|-----------|-----------|
| {F315061} | {F315072} |

The fix involves a refactor in the HTML structure in order to
match the graphic rendered when Aphlict is configured and running.

In short, we pass from this:

    COUNTEREXAMPLE
    <span class="connection-status-text aphlict-connection-status-notenabled">
      <span class="visual-only phui-icon-view phui-font-fa fa-circle-o grey"></span>
      Notification server not enabled
    </span>

To this:

    <span class="visual-only phui-icon-view phui-font-fa fa-circle-o grey"></span>
    <span class="connection-status-text aphlict-connection-status-notenabled">Notification server not enabled</span>

See the Task for additional details.

Closes T15415

Test Plan:
Follow the official Notification documentation about the config `notification.servers`,
open the Notification top-bar, and check the message in these conditions:

- without the config
- with the config
- with/without Aphlict running

https://we.phorge.it/book/phorge/article/notifications/

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15415

Differential Revision: https://we.phorge.it/D25312
2023-06-27 13:30:11 +02:00
Valerio Bozzolan
659e13fa6d Fix UX regression in Dashboard Tabs Panel
Summary:
Before this change, it seems that the Dashboard Tabs Panel could be empty
as default. This was the regression:

cbc0e66154

The variable $is_selected was defined inside a foreach loop, but that loop
was terminated when it was then read again. This redefines that variable in
the right scope.

| Before    | After     |
|-----------|-----------|
| {F312797} | {F312798} |

Closes T15474

Test Plan: - see that now you have something visibile in your Dashboard Tabs Panel

Reviewers: speck, O1 Blessed Committers

Reviewed By: speck, O1 Blessed Committers

Subscribers: tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15474, T15146

Differential Revision: https://we.phorge.it/D25291
2023-06-27 08:15:16 +02:00
Andre Klapper
e4fe54edf0 Fix typo "recipeint" in metamta.one-mail-per-recipient description
Test Plan: Go to `/config/settings/all` and look at the description of `metamta.one-mail-per-recipient`

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25311
2023-06-26 14:28:01 +02:00
Valerio Bozzolan
a25af8ccef Diviner: add a minimal systemd example
Summary:
After this change the section "Managing Daemons with phd"
mentions a minimally viable systemd configuration file
to make systemd-users happier and systemd-haters saddest.

Closes T15467

Test Plan: - read the page, see systemd, don't cry

Reviewers: O1 Blessed Committers, 20after4

Reviewed By: O1 Blessed Committers, 20after4

Subscribers: 20after4, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15467

Differential Revision: https://we.phorge.it/D25293
2023-06-23 08:50:16 +02:00
Andre Klapper
7c1487e4dc Fix Pholio RuntimeException: Undefined variable $dictionary (when adding an empty Inline Comment)
Summary:
Trying to create an empty inline comment in a Pholio mock, `$dictionary` does not get set as both `strlen($v_content)` and `$inline->getID()` are not true.

Thus show a more explanatory error message ('Comment cannot be empty.') to users instead of exposing internal variable names.

```
EXCEPTION: (RuntimeException) Undefined variable $dictionary at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=b325304b6e52), phorge(head=master, ref.master=980293b707a0)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<phorge>/src/applications/pholio/controller/PholioInlineController.php:117]
```

Closes T15456

Test Plan: After applying this change, try to add an empty Inline Comment in a Pholio mock. See that the error message is now "Comment cannot be empty." instead of "Undefined variable $dictionary".

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15456

Differential Revision: https://we.phorge.it/D25281
2023-06-19 14:13:36 +02:00
Andre Klapper
1c59b65421 Dashboards: add capability who can create Dashboards
Summary:
Add a `dashboard.create` capability to allow limiting dashboard creation (and creation of related panels).
This can reduce spam in open Phorge installations in which anyone can create an account while you still want anyone to be able to view existing dashboards.

Closes T15438

Test Plan:
1. As an admin, go to `/applications/view/PhabricatorDashboardApplication/`
2. See only two options "Can Use Application: All Users" and "Can Configure Application: Administrators"
3. Apply patch
4. As an admin, go to `/applications/view/PhabricatorDashboardApplication/` and see the new option "Can Create Dashboards" set to "All Users"
5. As an average user, go to `/dashboard/` and successfully create a new Dashboard
6. As an admin, go to `/applications/edit/PhabricatorDashboardApplication/` and change "Can Create Dashboards" from "All Users" to "Administrators", select "Save Policies" button
7. As an average user, go to `/dashboard/` and see that "Create Dashboard" is disabled
8. As an average user, go to an existing dashboard not created by this user and see that selecting "Edit Dashboard" shows "You do not have permission to edit this object."
9. As an average user, go to the existing dashboard that you created yourself and see that the Edit Policy is set to this user.
10. As an average user, go to the existing dashboard that you created yourself and see that you can still create or add panels.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15438

Differential Revision: https://we.phorge.it/D25270
2023-06-19 12:12:22 +02:00
Andre Klapper
324470e39b Give embedded image files with transparency a checkered background
Summary:
Have a consistent rendering experience between File page and file embedded in a task for PNG image files with an alpha channel:
Show a checkered background on the embedded image instead of a white background; show full contrast when hovering over the embedded image.

Closes T15473

Test Plan:
See steps in T15473 - compare an image file embedded in a task before and after applying this patch.

- test in Task description
- test in Task comment
- test in Task comment inside something weird like a table

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15473

Differential Revision: https://we.phorge.it/D25294
2023-06-19 11:57:03 +02:00
Mukunda Modell
327ac921c2 Conduit column.search: add status, sequence and isDefault to API results
Summary:
This seems like a fairly obvious oversight with the column.search API.

Knowing:

1. isHidden - boolean to indicate Active vs Archived
2. isDefaultColumn - the one that Tasks get dropped in by default, usually called "Backlog"
3. sequence - numerical order on the Workboard

are all necessary for a lot of things that very sensible real-world API clients need to do when working with columns.

Partial cherry-pick from:

https://phabricator.wikimedia.org/rPHABebfe30890b52784d222ec4ed36c05462b2a57f92

Closes T15484

Test Plan:
Tested on phabricator.wikimedia.org over many months and used by real client apps.

To do additional tests, visit this page:

/conduit/method/project.column.search/

Check that the new fields are returned correctly and nothing explodes.

Reviewers: #blessed_committers, O1 Blessed Committers, 0, avivey, valerio.bozzolan

Reviewed By: #blessed_committers, O1 Blessed Committers, 0, valerio.bozzolan

Subscribers: Cigaryno, 0, speck, tobiaswiese, valerio.bozzolan, Matthew

Maniphest Tasks: T15081, T15484

Differential Revision: https://we.phorge.it/D25038
2023-06-19 08:58:52 +02:00
Andre Klapper
478012cd46 Add serious business string for defrocking a user (remove admin rights)
Summary:
Allow humans to understand what happened when `phabricator.serious-business` mode is enabled: The administrator role was removed from a user account.

Closes T15480

Test Plan: Enable `phabricator.serious-business` mode. Enable and disable an account as admin. Check that account's feed and the general feed.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15480

Differential Revision: https://we.phorge.it/D25298
2023-06-16 13:58:28 +02:00