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