1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 08:42:41 +01:00
Commit graph

15631 commits

Author SHA1 Message Date
Andre Klapper
935d7120ee PHP 8.1 "preg_match(null)" exception in javelin/markup.php when http_action not set
Summary:
Accessing a project's workboard URL of a non-existing workboard shows RunTimeException in PHP 8.1:

    preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated

Closes T15262

Test Plan: Page whether to create a workboard for a project was displayed after this change

Reviewers: O1 Blessed Committers, avivey, valerio.bozzolan

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

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

Maniphest Tasks: T15262

Differential Revision: https://we.phorge.it/D25131
2023-04-25 15:45:57 +02:00
Dylan F
98c1267e5f Countdown: fix PhutilMissingSymbolException
Summary:
A previous change added a couple of Countdown-related classes:

7ed35123a3

This change updates the related library map (arc liberate) in order to
fix any related regression (like PhutilMissingSymbolException).

Closes T15271

Test Plan:
- you run `arc liberate` and the file does not change
- you do not see any `PhutilMissingSymbolException` exception

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

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

Maniphest Tasks: T15271

Differential Revision: https://we.phorge.it/D25133
2023-04-25 06:37:38 -07:00
Dylan F
90f9da643d Add and use new RemarkupMetadata class
Summary:
Add a new RemarkupMetadata class and use with upload button as well as drag and drop pathways.

With this change, files are now attached automatically to the object. This this could solve
several issues, for example when a person uploads a file but only that author is able to see it.

Ref T15106

Test Plan:
- Drag and drop file, upload file with button. Check that both files are attached with "attachedFilePHIDs" values.
    - drop file in a Task description (now works)
    - drop file in a Task comment (now works)
    - drop file in an edited Task comment (still not supported)
- This was already tested in Wikimedia Foundation: https://phabricator.wikimedia.org/D1203

Reviewers: O1 Blessed Committers, avivey, Matthew

Reviewed By: O1 Blessed Committers, avivey, Matthew

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

Tags: #remarkup

Maniphest Tasks: T15106

Differential Revision: https://we.phorge.it/D25052
2023-04-24 01:53:16 +00:00
Mukunda Modell
7ed35123a3 Countdown: add a capability to decide who can Create
Summary:
This change adds the `countdown.create` capability to
allow to limit countdown creation.

This change comes from the Wikimedia Phabricator and
was originally proposed by v.bozzolan and implemented
by the kind user 20after4.

https://phabricator.wikimedia.org/T258599

Closes T15208
Ref T15081

Test Plan:
- go to the page /applications/view/PhabricatorCountdownApplication/
- play a bit with the "Can Create Countdowns" option
- note that it is respected

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Tags: #countdown, #policy_archived

Maniphest Tasks: T15208, T15081

Differential Revision: https://we.phorge.it/D25101
2023-04-23 15:15:50 -05:00
Valerio Bozzolan
1ba5c8c260 Fix some PHP 8.1 issues in Diviner generator
Summary:
After this change I was able to generate again the Diviner
documentation from PHP 8.2 using the related command:

    ./bin/diviner generate

Closes T15255

Test Plan:
- run `./bin/diviner generate` - great success
- tested the advanced search in various ways - it still works

Reviewers: O1 Blessed Committers, Matthew

Reviewed By: O1 Blessed Committers, Matthew

Subscribers: Ekubischta, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15226, T15255

Differential Revision: https://we.phorge.it/D25124
2023-04-20 18:00:31 +02:00
Aviv Eyal
996a853fc4 Update "how to file bugs"
Summary:
We want users to use Ponder as primary support vector.

T15084

Test Plan: Generated diviner, saw that the reference works correctly.

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/D25125
2023-04-18 08:21:33 -07:00
Valerio Bozzolan
1b9da964e9 Add setup check to avoid git version 2.5.0 and below (May 2015)
Summary:
After this change, if you have a deprecated git version,
now you see a related information config warning:

{F276840}

Having said that the git version 1.8.3.1 - that for example
is provided by CentOS 7 - is surely problematic;

Some sources indicate all git versions *before* 2.5.0 as not
able to handle the '--' escape argument. The '--' arg
is used to separate normal git flags from user arguments,
so that they cannot be mistakenly exchanged. Kind of:

    git <flags> -- <arguments>

The problem is, Phabricator/Phorge at the moment executes
this kind of git commands when you surf a git repository from
the web interface:

    git cat-file -t -- <hash>:<file>

But, if your git version does not support "--", you can
get yourself into unhappy situations since "--" could be
just interpreted as a "wrong value", causing other
considerations and misleading exceptions, such as:

    This path was a submodule at <repo>:<hash>

If you have seen the above error while surfing a git repo,
and if you have 2.5.0 or before, and if you are sure that
the mentioned path it's not a submodule, you probably
need a git update.

It is not yet clear whether Phabricator/Phorge should
support the possibility of this lack of support for "--".
In the meanwhile, just update your git on the server.

AFAIK no particular version is required on your clients.

Related information:

- https://unix.stackexchange.com/a/740621/85666
- b48158ac94

If you disagree, please do not simply ignore the
warning but share your experience in the Task.

Ref T15179

Test Plan:
- open Phorge, you see no warnings since you are
  already up to date. Nice.
- Otherwise, you see a nice config message, and you
  can ignore it as usual.

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15179

Differential Revision: https://we.phorge.it/D25089
2023-04-10 11:10:02 +02:00
Valerio Bozzolan
0a82ccb8a4 AphrontFileResponse: avoid alien usages of setDownload()
Summary:
I noticed that - historically - setDownload() could also be used with
false, true, or other weird values to activate a download filename.
So, this change continues to give full compatibility to PHP 8.1 but
with extra validations.

This also adds a bit of inline documentation to put this more explicit.

For more context see the previous version:

{5952b0a31b6aac0718bc23aefe43560b9bfe8cc5}

{96ae4ba13acbf0e2f8932e950a92af0495f034d7}

Ref T15190

Test Plan:
- Drop a file in a comment, click, download
- See that it still works

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15190

Differential Revision: https://we.phorge.it/D25113
2023-04-09 22:46:04 +02:00
Aviv Eyal
d46aab8321 Add GitHub mirror to list of known mirrors
Summary: Ref T15046

Test Plan: Clone phorge from github mirror, visit `/config/`, see correct branch-point in `Version Information`

Reviewers: O1 Blessed Committers, Cigaryno, valerio.bozzolan

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

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

Maniphest Tasks: T15046

Differential Revision: https://we.phorge.it/D25115
2023-04-09 05:28:25 -07:00
Valerio Bozzolan
b587865ce7 People: add a permalink to list Administrators
Summary:
This change introduces a builting query to easily list
Administrators of your beautiful Phorge instance:

{F277912}

Note that any builtin query can be disabled anytime or
reordered, so this is not a problem if you already have
a similar saved query. But probably you have not.

The goal is not to just "have a link" (this was already
possible) but to have a predictable link to be used in
any instance-agnostic document, including but not
limited to Diviner or Phorge's user interface messages.

Closes T15238

Test Plan: Visit /people/query/admin/ and enjoy.

Reviewers: O1 Blessed Committers, Cigaryno, Matthew

Reviewed By: O1 Blessed Committers, Cigaryno, Matthew

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15238

Differential Revision: https://we.phorge.it/D25116
2023-04-08 09:47:12 +02:00
Valerio Bozzolan
3a8ee271d9 Render all Removed Comments a little more docile
Summary:
This small restyle makes any Removed Comment a little less
prominent than normal ones, with the goal of decreasing a
bit your in-page distractions and increase your individual
productivity in your business by at least 250 milliseconds
every 48 hours of hard work in front of your monitor.

| Before              | After               |
|---------------------|---------------------|
| {F274834,size=full} | {F274835,size=full} |

This implementation (which is called "Kasper on Diet")
contains these specific changes for Removed Comments:

- user icon visibility: reduced by ~50% (-> Kasper)
- black "trash" icon: reduced by ~50% (-> Diet)
- texts: visibility reduced by ~50%
- vertical padding: reduced from 16px down to 4px

Note that if your Phorge is under the Serious Business Mode,
it seems it is still technically possible to manually
activate the "Decaying Curse" proposal mentioned in the Task.

Closes T15192

Test Plan:
- Add a Comment "I love Phorge"
- Add a Comment "I love Phabricator"
- Mark the second Comment as Removed
- Call a person at your desk
- Plug that person to an eyeball tracker

If the general attention focuses first on a normal Comment and then
on the Removed Comment, this change works perfectly.

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Tags: #comments

Maniphest Tasks: T15192

Differential Revision: https://we.phorge.it/D25096
2023-04-06 14:46:35 +02:00
Valerio Bozzolan
524579fe64 Diffusion: fix missing mobile "Actions" from Manage Repo
Summary:
After this change, the page "/manage/" related to Diffusion
repositories has an "Actions" button for mobile users:

{F276113}

Before this change, that button was missing only from this
specific page, and so it was really difficult for them
(if not impossible) to edit repository's basic information:

{F276115}

Do not confuse the hamburger menu on the sidebar with this
"Actions". The hamburger contains queries like Active Repo,
All Repo, etc. Instead the Actions contains
Edit Basic Info, Allow Dangerous Changes, etc.

It seems the cause of this problem is originated from an
apparently incomplete rollback occurred in Sep 6 2017 from
Phabricator:

e1fd74ddb5

As you can see, the DiffusionRepositoryBasicsManagementPanel
was probably not completely restored from the previous
version, at least for the part related to the method
newActionList():

bb369c7b71

The method newActionList() was restored in every other
panel but no the one related to the Basics Management. So,
a new PhabricatorActionListView object was manually
created, but without generating an ID from the Celerity
utilities.

In short, this change assigns an ID to this Curtain, in
order to correctly generates a related "Actions" menu
again. Otherwise, on mobile the curtain (with all the
actions) is automatically hidden, without any alternative.

Closes T15067

Test Plan:
- Repository > Actions > Manage Repository
- Reduce the page until "Actions" appears (yeeh!)

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Tags: #diffusion, #mobile

Maniphest Tasks: T15067

Differential Revision: https://we.phorge.it/D25109
2023-04-06 10:58:43 +02:00
Valerio Bozzolan
787a84969f Phriction: clarify its search results as "Wiki page"
Summary:
This patch changes a bit how your search results from Phriction
are described in the autocomplete component (the "Typehead"):

{F276843}

After this change, Phriction search results will also contain the word
"Wiki page" so that users can better understand what the result
actually is. Just like a Diffusion repository mentions that it's
a "Repository", and just like a Project mentions that it's a
"Project" and so on.

Before this change, the Typehead entries were only mentioning
the slug of that wiki page.

| Before    | After     |
| {F274820} | {F272278} |

It's unclear if the previous behavior was a mistake (since the
internal parameter of the Typehead is called "type", and so, it
is supposed to mention the application type, not the slug.

Anyway, as a compromise, the slug is still mentioned.

To be honest this is just an excuse to put the middle dot /
aka interpunct character in this project again. Yeah, here the
middle point was used as separator. The apparent reason is:
because the middle point was already in use elsewhere in Phorge.
The real reason is: I'm a lobbyist in a corporation that sells
interpuncts worldwide as our core business, and so, I designed
thisvil plan to put the following middle point - again - in
Phorge (evil laugh).

Closes T15213

Test Plan:
- Search "Change Log" in the up-right bar (or similar)
- You see "Wiki Page · change_log/" (or similar)

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Tags: #phriction

Maniphest Tasks: T15213

Differential Revision: https://we.phorge.it/D25102
2023-04-05 15:39:41 +02:00
Valerio Bozzolan
306ce1c0b7 Fix "Search" Application description
Summary:
This description was somehow broken since 2019:

{F276845}

Probably this was caused by a Remarkup breaking change introduced by this
specific XSS security fix:

{54bcbdaba94a3573e128c6498816dbfa41d3a9cb}

Maybe in the future the backtick operator will be supported again
in Remarkup. Or, maybe not. So, let's just workaround to make that
specific page "less exploded" again.

Closes T15230

Test Plan:
- visit the page /applications/view/PhabricatorSearchApplication/
- check that now the Remarkup is rendered perfectly

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15230

Differential Revision: https://we.phorge.it/D25110
2023-04-05 15:37:46 +02:00
Valerio Bozzolan
96ae4ba13a PHP 8.1: fixes for strlen() not accepting NULL anymore, part 2
Summary:
This is a fix for PHP 8.1 deprecation of strlen(NULL), for these Phorge components:

- scripts
- aphront
- project

The strlen() was used in Phabricator to check if a generic value was a non-empty string.
For this reason, Phorge adopts phutil_nonempty_string() that checks that.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If your phutil_nonempty_string() throws an exception, just
report it to Phorge to evaluate and fix together that specific corner case.

Closes T15223
Ref T15190
Ref T15064

Test Plan: - check with your big eyes that there are no obvious typos

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15223, T15190, T15064

Differential Revision: https://we.phorge.it/D25105
2023-04-01 15:20:13 +02:00
Valerio Bozzolan
d25d630fe5 PHP 8.2: fixes for strlen() not accepting NULL anymore, part 1
Summary:
This change avoids some unnecessary uses of the strlen() function,
actually fixing some deprecation warnings in PHP 8.2.

In short, this is the suggested universal replace:

    -if(strlen($v))
    +if(phutil_nonempty_string($v))

And, if you know PHP, this is also another adoptable replace, but
only for cases where you are sure that the string "0" is not useful:

    -if(strlen($v))
    +if($v))

As usual the optimal solution depends on the contest.

Other similar patches will probably follow.

Closes T15222
Ref T15190

Test Plan:
- for the first time in my life, with this change, the unit tests are passed in PHP 8.2
- check with your big eyes that there are no obvious typos

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15199, T15190, T15222

Differential Revision: https://we.phorge.it/D25104
2023-03-31 22:05:52 +02:00
Valerio Bozzolan
fc9bbb9949 Workboard: fix regression caused by tags= prefiller
Summary:
This change fixes a regression introduced here:

rP5e2b3677157889104a7e540d7772a04f13164037

Thank you to the user @dadalha for auditing that commit
and sharing a stack trace to easily fix the issue.

In short, if you see this exception you are affected:

```
EXCEPTION: (InvalidArgumentException)
Value provided to "replaceQueryParam()" for key "tags" is NULL.
Use "removeQueryParam()" to remove a query parameter.
at [<arcanist>/src/parser/PhutilURI.php:341]
```

This change just fixes that specific problem.

Closes T15221

Test Plan:
1. Create a new Project
2. Create a new Milestone inside
3. Create the Milestone's Workboard
4. Visit the Milestone's Workboard and note that now it works

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno, dadalha

Tags: #workboard

Maniphest Tasks: T15221

Differential Revision: https://we.phorge.it/D25103
2023-03-31 11:05:22 +02:00
Valerio Bozzolan
6036079e88 PHP 8.2: fix deprecated use of "parent" in callables
Summary: Closes T15200

Test Plan:
Test 1:

I was able to run `arc unit --everything` without the error thrown from T15200

Test 2:

I also tried this and it still works:

```
php -a
require 'src/applications/metamta/exception/PhabricatorMetaMTAReceivedMailProcessingException.php';
$asd = new PhabricatorMetaMTAReceivedMailProcessingException("1", "TEST MESSAGE");
var_dump($asd->getMessage());
```

You get:

```
string(12) "TEST MESSAGE"
```

Reviewers: O1 Blessed Committers, Matthew

Reviewed By: O1 Blessed Committers, Matthew

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15200

Differential Revision: https://we.phorge.it/D25099
2023-03-29 20:37:25 +02:00
Valerio Bozzolan
5eac69b79d Ponder: strike code of resolved questions in query results
Summary:
After this change, search results with resolved
questions are striked, just like any Closed Maniphest
Tasks is already striked, etc.

https://we.phorge.it/T15166

There is still the same thing to be done at Remarkup.

Ref T15166

Test Plan: - go in the page /ponder/query/all/ and see that closed questions are now striked

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15166

Differential Revision: https://we.phorge.it/D25086
2023-03-27 20:55:18 +02:00
Valerio Bozzolan
780e86acf8 Herald Rule Creation Page: fix Back button in some cases
Summary:
If you want to create a new Herald Rule, sometime,
like for new "Object" Herald Rules, the Back button just
sends you to the very same page you already were.

The risk is that an employee could receive a specific
instruction, such as "Hey Alfreda, Go back, now"
and at this point Alfreda goes into an incredible loop
continuing to click the Back button until the duration
of the universe (since Phorge is so stable that it's able
to handle that "Back spike" forever - really).

https://we.phorge.it/T15184

Note that this also tries to avoid to change the base URI
to just go Back. For example, before this change, the Back
button was trying to send from /create/ to /new/, but
apparently they are just aliases.

Closes T15184

Test Plan:
- Visit the page /herald/create/?adapter=commit
- Click on "Object"
- Click on "Back"
- Verify that you only went back one screen, and not just forward in time

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15184

Differential Revision: https://we.phorge.it/D25087
2023-03-27 20:54:28 +02:00
Valerio Bozzolan
f0618d54e8 Installation Guide: mention that git 2.5.0 (2015) is needed server-side
Summary:
Note that the git version 2.5.0 is surely already assumed
as required as implicit fact, so it's just better to clarify
this in the documentation to prevent weird situations and
unclear related exceptions.

Note that if you have seen this Phabricator/Phorge error
in your installation:

    This path was a submodule at R123:5378198ea123asdlol

This is probably that kind of weird errors that will be
fixed just updating your git version to 2.5.0, so that
the git escape sequence ('--') is correctly parsed by git.

Example of command that will fail with outdated git versions:

    git cat-file -t -- <hash>:<file>

If you do not want to update git, but you want to fix,
you can convert that command to this one, manually patching
your installation:

    git cat-file -t <hash>:<file>

Related material:

- https://we.phorge.it/T15179
- https://unix.stackexchange.com/a/740621/85666
- 6f9504c48e/Documentation/RelNotes/2.5.0.txt

Wontfix T15179

Test Plan:
- open Diviner page "Installation Guide"
- see that the git version is not explicit - yup!

Reviewers: O1 Blessed Committers, Cigaryno, avivey

Reviewed By: O1 Blessed Committers, Cigaryno, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15179

Differential Revision: https://we.phorge.it/D25081
2023-03-27 20:51:24 +02:00
dylsss
5e2b367715 Workboard: improve the Create Task link to mention the Project slug
Summary:
This expands a specific link in a specific menu of Workboard Columns.

You may never notice this difference unless you like to open links in another tab.

If you go to a Workboard, and you open its context menu, and you hover your mouse on the
Create Task action, you see this URL or a similar one:

    http://example.com/maniphest/task/edit/form/default/

After this change, you see this URL or a similar one:

    http://example.com/maniphest/task/edit/form/default/?tags=test

You see that the PhutilURI class was used to add the Project slug to the 'tags' query param so
that users can still open the URL in a new tab and have the form prefilled with the Project Tag.

Closes T15147

Test Plan:
- visit a Workboard
- open the context menu of a Column (the pencil icon)
- see that the Create Task link has the Project slug is in the URL of its prefilled form

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

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

Maniphest Tasks: T15147

Differential Revision: https://we.phorge.it/D25068
2023-03-27 08:18:48 +02:00
Valerio Bozzolan
d98c47041f PHP 8.2: fix deprecated creation of dynamic properties
Summary:
This change fixes a typo that, in PHP 8.2, causes this exception:

    Creation of dynamic property PhabricatorAuthPasswordException::$confirmError is deprecated

Closes T15201

Test Plan: - I checked that "error" was spelled with two "r"

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15201

Differential Revision: https://we.phorge.it/D25100
2023-03-27 07:10:04 +02:00
Valerio Bozzolan
03c9bf575e PHP 8.2: fixes for deprecated use of ${var} in strings
Summary:
Note that PHP 8.2 implemented this deprecation:

    Using ${var} in strings is deprecated, use {$var} instead

This change fixes some known cases.

Ref T15196

Test Plan: - I checked with my big eyes that everything is as it should be

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15196

Differential Revision: https://we.phorge.it/D25098
2023-03-26 22:08:22 +02:00
Valerio Bozzolan
42a8b73adb Feed Differential retitled: do not repeat the title twice
Summary:
This is a cosmetic modification for a specific Feed.

https://we.phorge.it/T15183

If you retitle a Diff (and you do nothing else), its Feed
is somehow too much verbose:

    ... retitled NEW_TITLE from OLD_TITLE to NEW_TITLE

In this specific case, the Feed now becomes shorter so that
the NEW_TITLE is not repeated twice:

    ... retitled NEW_TITLE from OLD_TITLE

Note that, if the title changes again, the Feed naturally comes
back as before so to mention the change and obviously without
any repetition. Example:

    ... retitled CURRENT_TITLE from OLD_TITLE to NEW_TITLE

Closes T15183

Test Plan:
- take a Diff named "OLD_TITLE" and retitle to "NEW_TITLE"
- visit the homepage and see "retitled NEW_TITLE from OLD_TITLE"
  (this means the new behavior works)
- retitle the Diff from "NEW_TITLE" to "CURRENT_TITLE"
- visit the homepage again and see "retitled CURRENT_TITLE from OLD_TITLE to NEW_TITLE"
  (this means the old behavior still works)

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15183

Differential Revision: https://we.phorge.it/D25094
2023-03-25 14:21:10 +01:00
Valerio Bozzolan
9bc3c16b6e Fix information page about Read-only status
Summary:
This change fixes the following page on your Phorge/Phabricator:

    /readonly/config/

If you visit that page (with or without read-only mode), the
following crash will happen:

    Call to undefined method PlatformSymbols::getPlatformServerSymbol()

Fixes T15185

Probably the regression was introduced in this point:

rP4d3f0dc7c727e6fdc0e8cba0e8e8f2b8ef1bd16c

This change also renamed "administrator" to "Administrator" in the messages
from that specific page, both because I have great respect for Administrators
and also because that is how this word is spelled in most other places in Phorge.

Test Plan:
- Visit the page /readonly/config/
- It does not crash anymore and you can see that beautiful message
  that - it seems - no person in the world has ever seen since a
  whole year. Take a good look at this page: it is a precious treasure.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15185

Differential Revision: https://we.phorge.it/D25091
2023-03-25 12:06:35 +01:00
Valerio Bozzolan
5cba56182f Diviner: fix "Javascript" -> "JavaScript" and minor change
Test Plan:
- look at Diviner and say "Java..."
- Diviner will look at you saying "...Script"
- you land, satisfied

If Diviner says "...script" instead (lowercase "S"), abandon the ship.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25075
2023-03-16 15:28:14 +01:00
Valerio Bozzolan
66192a5b83 Fix "Undefined index: icon" when visiting Search Servers using MySQL
Summary:
Fix "Undefined index: icon" when visiting Search Servers using MySQL

NOTE: This patch just fixes the exception at my best but this section probably deserves more improvement to show a better default.

Closes T15155

Test Plan:
- use the default Search Server configuration (that is MySQL)
- open the page Search Servers (/config/cluster/search/)
- verify that it does not explode anymore but it displays something unuseful

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15155

Differential Revision: https://we.phorge.it/D25070
2023-03-03 12:16:51 +01:00
Valerio Bozzolan
36dba82224 Show the API summary on the top of each Conduit API page
Summary:
Show the API summary on the top of each Conduit API page.

Before this change, the summary was only displayed from the Conduit APIs list.

Closes T15141

Test Plan:
I've opened 20 random Conduit API pages from the web
interface and I checked that now there is an amazing
"Summary" field.
I also double-checked that there were not green peppers.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15141

Differential Revision: https://we.phorge.it/D25063
2023-02-11 11:11:20 +01:00
Valerio Bozzolan
20fb93d1a4 Add small documentation details
Summary:
Add some small Diviner documentation details like:

* mention PHP 8.1 (thanks https://we.phorge.it/Q18)
* mention the fact that you need multiple databases (it may be not obvious from the startup)
* mention MariaDB and not just MySQL (thanks MariaDB community!)
* mention "Administrator" and not just "User" for the "Instructions for installing, configuring, and using Phorge."

I was not bold enough to do more changes.

Test Plan: I tested locally and Diviner does not look bad.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25064
2023-02-11 11:10:09 +01:00
Valerio Bozzolan
83672ba949 Allow to prefill name and description fields of a new Passphrase
Summary:
Allow to prefill name and description fields of a new Passphrase

Closes T15142

Test Plan:
I've visited this URL in my local test installation:

/passphrase/edit/?type=password&username=user&name=super&description=mario

And I was able to see the Name and Description fields prefilled,
just like the username.

I also tried to put the query string on an already-existing element
and it was working as expected, so, without any prefilled value from
the query string.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15142

Differential Revision: https://we.phorge.it/D25065
2023-02-11 11:08:12 +01:00
Valerio Bozzolan
02a4f8b0c8 Fix editing a Calendar import ICS URI
Summary:
Before this change it was only possible to create a Calendar import ICS URI.
After this change it's possible to also edit already-existing elements.

This change fixes this specific exception when visiting similar pages:

        /calendar/import/edit/5/

        Argument 2 passed to PhabricatorCalendarImport::initializeNewCalendarImport()
        must be an instance of PhabricatorCalendarImportEngine, null given, called in
        phorge/src/applications/calendar/editor/PhabricatorCalendarImportEditEngine.php
        on line 45

Before this change it was only theorically possible to edit the name, policies, etc.
but not the URI.

This change also introduces the ability to edit the specific ICS URI,
in order to change legitimate parts of the URI, like the secret token.

Closes T15137

Test Plan:
I tested in my own installation with success lints.

To test, visit the Calendar, create an import ICS URI, and then try to edit it again.
It will work only after this change.

I was not able to conclude the "arc diff" since it tries to connect
to an unexisting database owned by root.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, Cigaryno, speck, tobiaswiese, Matthew

Maniphest Tasks: T15137

Differential Revision: https://we.phorge.it/D25061
2022-12-17 12:17:37 +01:00
k__nard
d5040f9a8f updating twitch to latest api (Helix)
Summary:
api doc : https://dev.twitch.tv/docs/api/reference
oauth2 doc : https://dev.twitch.tv/docs/authentication

Test Plan: I have successfully setup OAuth2 authentication against Twitch

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

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

Tags: #auth

Maniphest Tasks: T15122

Differential Revision: https://we.phorge.it/D25057
2022-12-08 15:41:09 -07:00
Valerio Bozzolan
dc558b5538 Fix NULL pointer exception in some circumstances from Calendar's homepage
Summary:
After importing specific weird events, for example from Google Calendar (bleah), it can happen that the Calendar's homepage becomes broken.

This was the Exception error shown to video:

    "Call to a member function getEventsBetween() on null"

It was happening since this method can return NULL:

    PhabricatorCalendarEventQuery#newRecurrenceSet()

This changeset verifies this condition from the Calendar's homepage.

Closes T15136

Test Plan: I tried in my server. I've executed the syntax lint. On my local machine I was not able to run "arc diff" since it tries to connect to root@localhost for some reasons.

Reviewers: O1 Blessed Committers, 20after4

Reviewed By: O1 Blessed Committers, 20after4

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

Tags: #calendar

Maniphest Tasks: T15136

Differential Revision: https://we.phorge.it/D25060
2022-12-08 15:32:27 -07:00
Thomas Iguchi
a8a38835e1 Fix broken file PHID extraction that causes Pholio uploads to crash
Summary:
A commit earlier this year modified the structure of the file upload transaction data value, by nesting the array of file upload PHIDs in another array.
The `extractFilePHIDs` method was not updated to deal with that change though, therefore new mock uploads via Pholio would crash.
This patch fixes that method so it can process the updated transaction data.

Resolves T15105

Test Plan: Patched my live Phabricator installation with this fix and successfully uploaded new Pholio mockups.

Reviewers: O1 Blessed Committers, Cigaryno, Matthew

Reviewed By: O1 Blessed Committers, Cigaryno, Matthew

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

Tags: #pholio

Maniphest Tasks: T15105

Differential Revision: https://we.phorge.it/D25058
2022-11-11 13:10:16 -07:00
ekubischta
4aedc206c7 Update PassphraseCredential to support Mentions
Summary:
This revision allows us to see where passphrase credentials are mentioned in other parts of phabricator

This is very useful to "What is this credential for?"  If we can see where it was been mentioned

Test Plan: Mentioned a passphrase and saw it in the transaction list

Reviewers: O1 Blessed Committers, Matthew

Reviewed By: O1 Blessed Committers, Matthew

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

Differential Revision: https://we.phorge.it/D25032
2022-10-09 03:30:07 -07:00
Vedran Miletić
f8ffa393c4 Added cross-platform default fonts
Summary:
`system-ui` is provided by modern browsers as [a cross-platform default font](https://caniuse.com/font-family-system-ui). Using this font [was a controversial choice back in 2017](https://infinnie.github.io/blog/2017/systemui.html). because it did not address i18n well and therefore it was removed from Bootstrap [twbs/bootstrap#22377](https://github.com/twbs/bootstrap/pull/22377). However, it was added back in Bootstrap v5 [twbs/bootstrap#30561](https://github.com/twbs/bootstrap/pull/30561) since it got better over time.

Fixes T15049.

Test Plan: Tested locally for now.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: Matthew, avivey, Cigaryno, 20after4, Ekubischta, speck, tobiaswiese, valerio.bozzolan

Maniphest Tasks: T15049

Differential Revision: https://we.phorge.it/D25021
2022-10-09 03:30:07 -07:00
Aviv Eyal
377ac059d6 Update default wordmark and logo
Summary:
The current default wordmark is "Phabricator" which is trademarked and the
default logo is also copyright.

(This change was made by @speck directly in the deployed instnace, bringing it into `master` now).

Test Plan: should be fine.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, speck

Differential Revision: https://we.phorge.it/D25048
2022-08-25 01:31:18 -07:00
Aviv Eyal
dbf7cd2b4a Rebrand: Rename library and update "version" logic
Summary: Make the "version" data in Config show information. Also rename the library.

Test Plan: See version "branched from" information.

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew

Maniphest Tasks: T15006

Differential Revision: https://we.phorge.it/D25047
2022-08-25 01:26:21 -07:00
Aviv Eyal
c3f0c0b0f7 Merge Phacility/master into phorge 2022-07-25 11:50:26 -07:00
epriestley
9426765a2c Flatten "RemarkupValue" objects when setting field defaults for custom forms
Summary:
Ref T13685. "RemarkupEditField" uses a "RemarkupValue" internally, and it currently attempts to serialize into the database unsuccessfully.

Instead, flatten the value before returning it for storage.

Test Plan:
  - Edited the default description of a task in a custom form.

Maniphest Tasks: T13685

Differential Revision: https://secure.phabricator.com/D21854
2022-06-14 09:37:35 -07:00
epriestley
1fc04e238f Fix repository URI generation for SVN commit identifiers represented with integer types
Summary:
Ref T13588. See PHI2198. Some pathways represent SVN commit identifiers (which are numeric) with integers; these raise an exception in `phutil_nonempty_string()`, which is strict.

(Ideally, commit identifiers wouldn't have mixed types, but that's a dramatically less practical change.)

Test Plan: Will deploy.

Maniphest Tasks: T13588

Differential Revision: https://secure.phabricator.com/D21852
2022-06-03 15:01:04 -07:00
epriestley
944b257d5d Fix a policy issue where permissions were not properly checked when disabling global builtin queries
Summary: See <https://hackerone.com/reports/1573143>. The pathway for disabling global builtin queries is missing a policy check. Add it.

Test Plan:
  - Accessed the "/search/delete/id/.../" URI for a global builtin query as a non-administrator.
  - Before patch: could improperly disable queries.
   -After patch: proper policy exception.

Differential Revision: https://secure.phabricator.com/D21851
2022-05-31 11:00:53 -07:00
epriestley
3052ed1484 Remove obsolete, policy-violating "owners.query" API method
Summary: See <https://hackerone.com/reports/1584409>. This obsolete API method is technically policy-violating: Owners packages have had policies since D15275. See also T10360.

Test Plan: Viewed Conduit API console, no longer saw API method available.

Differential Revision: https://secure.phabricator.com/D21850
2022-05-31 10:44:45 -07:00
epriestley
835cd7f270 Improve remarkup metadata robustness against debris in the database
Summary:
Ref T13682. Versioned drafts may have missing or unexpected metadata:

  - versioned drafts from an older version of Phabricator may be missing metadata;
  - versioned drafts created by an older UI against a newer version of Phabricator may have `null` metadata.

Generally, make these workflows robust to metadata in unexpected formats, so database debris doesn't break the UI.

Test Plan: Simulated debris, interacted with UI.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21849
2022-05-28 06:29:43 -07:00
epriestley
809ae81752 Update Slowvote poll status to use sensible string constants
Summary: Ref T13682. This prepares for modernizing Slowvote and exposing a more usable API.

Test Plan: Ran migrations, opened and closed polls.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21848
2022-05-27 10:15:01 -07:00
epriestley
c1887f0c1d Separate Slowvote poll status onto a dedicated object
Summary: Ref T13682. Prepares for use of API-friendly string constants rather than opaque integers.

Test Plan: Created and edited polls, opening and closing them. Grepped for affected methods.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21847
2022-05-27 10:15:01 -07:00
epriestley
03d3d1889d Update Slowvote voting methods to use sensible string constants
Summary: Ref T13682. Use API-friendly string constants instead of opaque integers in Slowvote voting methods.

Test Plan: Created, edited, and voted in polls with various voting methods. Examined database after migrations.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21846
2022-05-26 10:27:01 -07:00
epriestley
9dad49472c Move Slowvote vote types to a separate object
Summary: Ref T13682. Extract Slowvote vote types to a separate object, to prepare for turning them into API-friendly string constants.

Test Plan: Created, edited, and voted in Slowvote polls. Grepped for affected constants.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21845
2022-05-26 10:17:36 -07:00
epriestley
9f075839a2 Update Slowvote to use sensible string constants for response visibility
Summary: Ref T13682. Migrate and update Slowvote to use API-friendly string constants for response visibility, not opaque integers.

Test Plan: Created and edited slowvotes, changing response visibility.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21844
2022-05-26 09:58:48 -07:00
epriestley
23094b4950 Move Slowvote response visibility to a separate object
Summary: Ref T13682. This change supports modifying these constants to be sensible strings instead of opaque integers.

Test Plan: Created and edited polls.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21843
2022-05-26 09:58:48 -07:00
epriestley
b1533e5468 Migrate "SlowvotePoll" to modern mailkey storage
Summary:
Ref T13682. Ref T13065. Migrate and update "SlowvotePoll" to use more modern mailkey storage (in the mail application, rather than onboard).

Also make other minor modernization changes.

Test Plan: Created a new poll. Ran migration, verified mail key copied properly in database.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13682, T13065

Differential Revision: https://secure.phabricator.com/D21842
2022-05-26 09:29:05 -07:00
epriestley
d91727e66c Update nonstandard PHID key on "SlowvotePoll"
Summary: Ref T13682. Use a more modern PHID key name for this key.

Test Plan: Ran `bin/storage adjust`, got a modern key.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21841
2022-05-26 09:29:05 -07:00
epriestley
7e5f7b9640 Update "Files" attachment table to show more attachment details and support detachment
Summary: Ref T13682. Make the "Attached" list in Files a bit more detailed, and add a "Detach" button.

Test Plan: Tried to detach unrelated, referenced, and attached files. Saw attached files detach.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21840
2022-05-25 12:56:37 -07:00
epriestley
5aa159a830 Pass "object" to modular transaction methods explicitly
Summary:
Ref T13682. When editing a comment, the edit pathway may invoke file PHID extraction without first formalizing an object on the Editor.

Pass the "object" explicitly to generally improve the robustness of this pathway.

Test Plan: Edited a comment.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21839
2022-05-25 10:25:18 -07:00
epriestley
8e703c8c35 Provide a default "loadPage()" implementation on "CursorPagedPolicyAwareQuery"
Summary: Ref T13682. Many subclasses of "CursorPagedPolicyAwareQuery" have the same implementation of "loadPage()", and this is a sensible default behavior.

Test Plan: Examined changes to verify that all removed methods have the same behavior.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21838
2022-05-24 10:18:53 -07:00
epriestley
5493f028dc Provide a simple "Attach File" explicit workflow for files referenced but not attached
Summary: Ref T13682. Allow users to manually attach files which are referenced (but not attached) via the UI.

Test Plan: Reference files via `{F...}`, then attached them via the UI workflow.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21837
2022-05-24 09:49:59 -07:00
epriestley
021e5ab933 Provide a rudimentary "Attached Files" curtain UI panel
Summary: Ref T13682. Provide a basic UI element for showing file attached to an object.

Test Plan: Viewed objects with attached files in the UI.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21836
2022-05-23 14:08:15 -07:00
epriestley
5033ef6f88 Give "FileAttachment" policy support and a query object
Summary: Ref T13682. This supports an "Attached Files" curtain UI element.

Test Plan: See next change.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21835
2022-05-23 14:08:15 -07:00
epriestley
631c36aee3 Don't consider file references inside quoted text blocks to grant "attachment intent"
Summary:
Ref T13682. This is a marginal case, but if you quote someone else's text and it has a file reference embedded in it, that isn't good enough to generate "attachment intent".

Since you need both a reference and an explicit attachment, this should never actually affect any user-visible behavior today, but makes the ruleset more thorough.

Test Plan: Dragged and dropped a file, referencing it in either a quoted or unquoted block. Saw it attach only for a quoted block.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21833
2022-05-23 14:08:15 -07:00
epriestley
2a0feb3de0 Only attach files that are both referenced in Remarkup and attached by explicit metadata
Summary:
Ref T13682. When a user uploads a file, then changes their mind and deletes the reference to the file, we don't actually want to attach the file.

When choosing which files to attach, only attach files which are both referenced in Remarkup and explicitly attached in remarkup metadata.

Test Plan:
  - Dropped a file into a comment, submitted it, saw it attach normally.
  - Dropped a file into a comment, deleted the reference, submitted it, saw no attachment.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21832
2022-05-23 14:08:14 -07:00
epriestley
8cd02e6727 Provide missing default attachment list for Files transactions
Summary: Ref T13682. This property may not exist, and should defualt to an empty array if not present.

Test Plan: Created a new object.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21834
2022-05-23 09:24:38 -07:00
epriestley
cc44ae32c5 Remove obsolete "setDisableMacros()" on "PhabricatorRemarkupControl"
Summary:
Ref T13682. This method originated in D4781 and no longer has any callers; remove it.

(I believe this was from an era where macros didn't have to be on their own line to trigger.)

Test Plan: Grepped for method, found no callers.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21831
2022-05-20 10:27:41 -07:00
epriestley
42876de60d Generate file attachment transactions for explicit Remarkup attachments on common edit pathways
Summary:
Ref T13603. On common edit pathways, extract explicit file attachments from Remarkup. These pathways are affected:

  - Objects that use EditEngine and expose a remarkup area via "RemarkupEditField".
  - Objects that use EditEngine to generate a comment area.

This is the vast majority of pathways, but not entirely exhaustive.

Test Plan: Created and commented on a task, explicitly attaching images. Saw images attach properly.

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21830
2022-05-19 13:21:04 -07:00
epriestley
fee8297121 Remove file attachment edge types, including API access
Summary:
Ref T13603. This removes the file attachment edge types, which no longer have readers or writers.

It does not delete the underlying data.

This indirectly removes API access to this edge. As far as I know, this was only used by one customer (see D21480) who has migrated away from Phabricator.

Test Plan: Grepped for edge constants.

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21823
2022-05-19 13:21:04 -07:00
epriestley
b872640c16 Remove all "FileHasObject" edge reads and writes
Summary: Ref T13603. Removes all reads and writes to the other half of the legacy edge.

Test Plan:
  - Verified I could still see file content accessible only through object attachment.
  - This fixes a unit test broken by removal of only half the edge access in the previous change.

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21820
2022-05-19 13:21:04 -07:00
epriestley
c25a8fabfc Remove all "ObjectHasFile" edge reads and writes
Summary: Ref T13603. Migrate all code which interacts with the "ObjectHasFile" edge to use the "Attachments" table instead.

Test Plan:
  - Edited a paste's view policy, confirmed associated file secret was scrambled.
  - Verified I could still view paste content as a user who could not naturally view the underlying file.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21819
2022-05-19 13:21:04 -07:00
epriestley
4a2d961e76 Adjust paste content transaction to support earlier file PHID extraction
Summary: Ref T13603. New transactional file attachment extracts PHIDs earlier than the older mechanism did. Allow either pathway to save content and extract a PHID.

Test Plan:
  - Created and modified a paste.
  - Confirmed both pathways do old and new storage writes.

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21817
2022-05-19 13:21:03 -07:00
epriestley
d017f3f210 Double-write file attachment to old "edge" storage and new "attachment" storage
Summary: Ref T13603. This adds a second write to new "attachment" storage to all writers except one in Paste, which creates the file inline.

Test Plan:
  - Updated a macro image, confirmed a write to "attachment" storage (transaction pathway).
  - Updated a blog profile image, confirmed a write to "attachment" storage (legacy pathway).

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21816
2022-05-19 13:21:03 -07:00
epriestley
7fcc0f9ebd Remove "PhabricatorFile->detachFromObject()"
Summary:
Ref T13603. Currently, files are sometimes detached from objects. For example, when you change the image for a Macro, the old image is detached.

This is wrong: the image should remain attached so users who can view the macro can view the complete "alice change the image from X to Y" transaction. To be able to understand the change that was applied, you need to be able to view both files.

All workflows which currently detach files aren't conistent with the modern way applications behave, except maybe one callsite in a unit test, and that one's kind of moot.

Get rid of this stuff and just use PHID extraction to perform file attachment in all cases.

Test Plan: Created and edited macros, verified files were properly attached and remained attached across edits.

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21815
2022-05-19 13:21:03 -07:00
epriestley
cfa42c5e65 Add database storage for a dedicated file attachment table
Summary: Ref T13603. Prepare to move this relationship out of edge storage into dedicated storage so it is easier to formalize better in the UI.

Test Plan: Ran `bin/storage upgrade`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21813
2022-05-19 13:21:02 -07:00
epriestley
6fea5e5ce7 Do not extract file PHIDs from remarkup blocks for attachment
Summary: Ref T13603. When processing transactions that include remarkup blocks, stop extracting file references from them for attachment.

Test Plan: Submitted a remarkup block with a file reference, no longer saw automatic file attachment.

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21812
2022-05-19 13:21:02 -07:00
epriestley
ee6e2a396f Fix some test suite stragglers with PHP 8.1 compatibility
Summary: Ref T13588. This allows "arc unit --everything" to more or less run cleanly on PHP 8.1.

Test Plan: Ran "arc unit --everything".

Maniphest Tasks: T13588

Differential Revision: https://secure.phabricator.com/D21821
2022-05-19 12:04:09 -07:00
epriestley
7693a711a7 Persist remarkup metadata in "VersionedDrafts" and record explicit file uploads
Summary:
Ref T13603. Allow "VersionedDraft" to persist remarkup comment area metadata from stacked actions controls.

When files are dragged and dropped, record them as explicit uploads in comment metadata.

Test Plan: Dragged and dropped files into Remarkup stacked action text areas (e.g., in Maniphest), reloaded page, saw metadata persist across reloads.

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21828
2022-05-19 12:03:18 -07:00
epriestley
33a0731619 Give "RemarkupControl" support for tracking metadata
Summary: Ref T13603. This will support explicit handling of attached files.

Test Plan: Adjusted new input to have "text" input type, used it alongside additional upcoming changes, saw sensible metadata behavior.

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21827
2022-05-19 12:03:17 -07:00
epriestley
147b48b934 Fix an exception in cursor pagination of Conpherence threads
Summary: Ref T13680. Conpherence may pass values with an integer type to this layer of the stack. These are "supposed" to be strings, but just be accepting.

Test Plan:
  - Wrote 100+ messages to a Conpherence room.
  - Clicked "Show Older Messages".
  - Before: exception, int passed to "phutil_nonempty_string()".
  - After: older messages loaded.

Maniphest Tasks: T13680

Differential Revision: https://secure.phabricator.com/D21824
2022-05-17 16:31:01 -07:00
epriestley
431612023d Remove "feed.publish" API
Summary:
Ref T13681. This was introduced in D593, never used, and doesn't make sense or have any uses in modern Phabricator.

It also does some pretty direct writes that can technically do things that at least //look// like they violate policies, so remove it.

Test Plan:
  - Checked the API console, no longer saw "feed.publish".
  - Grepped for "feed.publish", no hits.

Maniphest Tasks: T13681

Differential Revision: https://secure.phabricator.com/D21826
2022-05-17 16:30:53 -07:00
epriestley
698ada2470 Correct overbroad automatic capability grant of global settings objects
Summary:
Ref T13679. In D16983, global settings objects were given an exception to let logged-out users see them, even on installs with no "public" user role.

This exception is too broad and grants everyone all capabilities, not just "CAN_VIEW". In particular, it incorrectly grants "CAN_EDIT", so any user can edit global settings defaults.

Restrict this grant to "CAN_VIEW".

Test Plan:
  - As a non-administrator, tried to edit global settings.
  - Before: could.
  - After: could not.

Maniphest Tasks: T13679

Differential Revision: https://secure.phabricator.com/D21811
2022-05-09 15:10:50 -07:00
epriestley
01253d533b Prevent embedded remarkup content from cycling when it contains embedded self-references
Summary: Ref T13678. When remarkup content embeds other remarkup content, detect and degrade if the references have nesting depth greater than 1. This is a coarse cycle detector, since rendering shallow (but technically non-cycling) trees doesn't seem valuable.

Test Plan: Created various objects with self-references, saw everything degrade properly (after one level of embedding) when embedded in itself and in other contexts. See attached screenshot.

Maniphest Tasks: T13678

Differential Revision: https://secure.phabricator.com/D21810
2022-05-09 11:21:42 -07:00
epriestley
a640a4a493 In Drydock, yield for reclaiming resources in the "released" state
Summary: Ref T13677. This was an accidental change in D21807: when reclaiming a resource, wait for it to be completely destroyed before allowing a lease to reclaim another resource.

Test Plan: Reverts accidental behavior change in D21807.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13677

Differential Revision: https://secure.phabricator.com/D21809
2022-05-09 10:45:35 -07:00
epriestley
1835ca1918 Remove the "25% of active pool" growth rate throttle from Drydock
Summary:
Ref T13677. Drydock has a hard-coded and fairly arbitrary limit which prevents a resource pool from growing more than 25% at once.

This is vaguely reasonable for resources which allocate quickly, but suffocating for slower resources. It's also wholly arbitrary, and the "one per lease" limit introduced in D21807 should do a better job of covering the same ground while generally being more reasonable, expected, and predictable.

Test Plan: Ran Drydock allocations without the throttle, saw faster pool growth.

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13677

Differential Revision: https://secure.phabricator.com/D21808
2022-05-09 10:40:08 -07:00
epriestley
62360ea406 Adjust the Drydock allocator to limit each pending lease to one allocating resource
Summary:
Ref T13677. Currently, one lease may cause multiple resources to allocate simultaneously if it starts allocating one, then wakes up from a yield later on and still sees no available resources.

This is never desired -- or, at least, produces desirable behavior only entirely by accident. Normally, it causes an excess of resources to allocate.

This is not a catastrophic problem: the extra resources usually get used sooner or later or cleaned up; and the total amount of badness is limited by overall resource allocation limits.

However, this behavior is also suppressed by an artificial "25% of current pool size" growth limit throttle which I intend to remove. Removing this throttle without fixing the allocator behavior could make this "too many resources" problem worse.

Change the allocator so that a lease that has started allocating a resource won't allocate another resource until the first resource leaves the "pending" state.

This also fixes some general oddness with the allocator and attempts to simplify the structure.

Test Plan:
  - Ran 8 taskmasters.
  - Destroyed all resources and leases.
  - Leased 4 working copies.
    - Saw exactly 4 resources build and lease, all simultaneously.
  - Destroyed all resources and leases.
  - Leased 32 working copies.
    - Saw exactly 32 resources build and lease, approximately 8 at a time (limited by taskmasters).
  - Destroyed all leases (but not resources).
  - Leased 32 working copies, saw them  satisfied by existing resources.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13677

Differential Revision: https://secure.phabricator.com/D21807
2022-05-09 10:40:08 -07:00
epriestley
13a37282bc Formalize some more Drydock conditions and bookkeeping
Summary:
Ref T13677. Track which resources a given lease has begun allocating or reclaiming in a more formal way, and add logging for waiting actions.

The "allocating" mechanism is new. This will replace an existing similar "reclaiming" mechanism in a future change.

Test Plan: See followup changes.

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13677

Differential Revision: https://secure.phabricator.com/D21806
2022-05-09 10:40:08 -07:00
epriestley
1b6727ac3d Add "--all" flags to "release-lease" and "release-resource" workflows in "bin/drydock"
Summary: Ref T13677. These flags increase the convenience of testing in a development environment.

Test Plan: Used new "--all" flags to release all resources and leases.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13677

Differential Revision: https://secure.phabricator.com/D21805
2022-05-09 10:40:08 -07:00
epriestley
6417e950f6 Don't reclaim resources that have a destroyed lease less than 3 minutes old
Summary:
Ref T13676. The 3-minute grace period when a resource can not be reclaimed after its leases are released currently doesn't work reliably because the Resource object usually isn't actually updated when a lease is released.

Add an additional check for recently-destroyed leases, and extend the grace period if we find any.

Test Plan:
  - See T13676. Ran reproduction sequence there, observed immediate resource reclamation.
  - Applied patch.
  - Ran sequence again, observed repository B wait 3 minutes to reclaim a repository A resource.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13676

Differential Revision: https://secure.phabricator.com/D21803
2022-05-03 15:57:16 -07:00
epriestley
dfdbe7a6be Allow "bin/drydock lease ..." to select particular blueprints with "--blueprint"
Summary: Ref T13676. See discussion in that task.

Test Plan: Tried nonsense blueprint identifiers, duplicate identifiers, incompatible identifiers, and valid identifiers. Got apparently sensible behavior in all cases.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13676

Differential Revision: https://secure.phabricator.com/D21802
2022-05-03 15:57:16 -07:00
epriestley
25cf955a89 Use the same logic in "bin/drydock lease" and LeaseUpdateWorker to identify candidate blueprints
Summary:
Ref T13676. Currently, "bin/drydock lease" just creates a lease that permits any blueprint.

To prepare for "use specific blueprint X", unify the logic between this workflow and LeaseUpdateWorker so we select only blueprints which at least have coarse compatibility (e.g., if we're leasing a host, only select enabled blueprints of classes that can allocate hosts).

Test Plan: Used `bin/drydock lease` to try to lease a nonsense type, got sensible error. Leased a host.

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13676

Differential Revision: https://secure.phabricator.com/D21801
2022-05-03 15:57:16 -07:00
epriestley
30c3d1e929 Allow "bin/drydock lease" to acquire many identical leases with "--count N"
Summary: Ref T13676. This makes it easier to create resource pressure without juggling a big pile of terminals.

Test Plan: Used `bin/drydock lease --count 5 ...` to acquire 5 leases.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13676

Differential Revision: https://secure.phabricator.com/D21800
2022-05-03 15:57:15 -07:00
epriestley
9ec6677c02 Update "bin/drydock command" help text to use more standard quoting
Summary: Ref T13676. Modern conventions quote flags in help text.

Test Plan: Read text.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13676

Differential Revision: https://secure.phabricator.com/D21797
2022-05-03 15:57:15 -07:00
epriestley
d1fd2975b0 Fail in a more comprehensible way when a WorkingCopy lease omits or mangles "repositories.map"
Summary: Ref T13676. When the required "repositories.map" attribute is omitted, `bin/drydock lease` currently fatals in an unhelpful way when trying to lease a working copy.

Test Plan:
Ran `bin/drydock lease --type working-copy` with no attributes, after following steps in T13676.

```
<Allocation Failed> One or more blueprints promised a new resource, but failed when allocating: [PhutilAggregateException] All blueprints failed to allocate a suitable new resource when trying to allocate lease ("PHID-DRYL-orbtwtlinksm3xqpyhmw").
    - Exception: Working copy lease is missing required attribute "repositories.map".

      Attribute "repositories.map" should be a map of repository specifications.
```

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13676

Differential Revision: https://secure.phabricator.com/D21796
2022-05-03 15:57:15 -07:00
epriestley
00a20d3cdc Fix various "strlen(null)" PHP 8.1 issues on "bin/phd" and "bin/drydock" pathways
Summary: Ref T13676. Ref T13588. Fix some issues that prevent "bin/phd" and "bin/drydock" from executing under PHP 8.1, broadly because `null` is being passed to `strlen()`.

Test Plan: Ran `bin/phd debug task` and `bin/drydock ...` under PHP 8.1.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13676, T13588

Differential Revision: https://secure.phabricator.com/D21795
2022-05-03 15:57:15 -07:00
epriestley
c415622923 Remove the "Phragment" application
Summary:
Ref T5479. Ref T13658. This was a contributed application from the early days of Phabricator which never had customers or users in the wild. The contributor moved on from the project many years ago.

Any capabilities in this general role would look different today. It also has one or two product name literal strings, so this is as good a time as any to remove it.

This change does not remove storage; I'll issue upgrade guidance and do that separately after some time.

Test Plan: Grepped for "phragment", got no relevant hits.

Subscribers: hach-que, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658, T5479

Differential Revision: https://secure.phabricator.com/D21793
2022-04-25 16:46:27 -07:00
epriestley
84b0c5a669 Remove the "Releeph" application
Summary:
Ref T9530. Ref T13658. The "Releeph" application was never useful outside of Facebook and any application providing release support would not resemble it much.

It has some product name literal strings, so now is as good a time as any to get rid of it.

This application never left prototype and I'm not aware of any install in the wild that uses it (or has ever used it).

I did not destroy the database itself. I'll issue upgrade guidance and destroy the database in some future release, just in case.

Test Plan: Grepped for "releeph", found no relevant/removable hits.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658, T9530

Differential Revision: https://secure.phabricator.com/D21792
2022-04-25 16:46:27 -07:00
epriestley
27b512e284 Remove product literal strings in "pht()", part 25
Summary: Ref T13658.

Test Plan: Static checks only.

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21791
2022-04-25 16:46:26 -07:00
epriestley
bc7f87591f Remove product literal strings in "pht()", part 24
Summary: Ref T13658.

Test Plan: Static checks only.

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21790
2022-04-25 16:46:26 -07:00
epriestley
0c5ab2c164 Remove product literal strings in "pht()", part 23
Summary: Ref T13658.

Test Plan: Static checks only.

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21789
2022-04-25 16:46:26 -07:00
epriestley
7fd8263185 Remove product literal strings in "pht()", part 22
Summary: Ref T13658.

Test Plan: Static checks only.

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21788
2022-04-25 16:46:26 -07:00
epriestley
b1079b79e5 Remove product literal strings in "pht()", part 21
Summary: Ref T13658.

Test Plan: Static checks only.

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21787
2022-04-25 16:46:26 -07:00
epriestley
07723b4627 Remove product literal strings in "pht()", part 20
Summary: Ref T13658.

Test Plan: Static checks only.

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21786
2022-04-25 16:46:25 -07:00
epriestley
2f4ac2a804 Remove product literal strings in "pht()", part 19
Summary: Ref T13658.

Test Plan: Static checks only.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21785
2022-04-25 16:46:25 -07:00