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

3503 commits

Author SHA1 Message Date
Valerio Bozzolan
821df3364e Hide the "hidden" fields on custom form previews
Summary:
Hide the "hidden" fields on custom form previews.

Before this change, Phabricator's custom form preview doesn't actually hide the
hidden fields, instead it shows them at 50% opacity. It looks cluttered and
doesn't provide a very useful "preview" at all.

This just hides some fields via CSS.

Cherry picked from:

https://phabricator.wikimedia.org/rPHABbba62cf5243538af9e37cc1211a01d247294f9f7

Upstream Task:

https://phabricator.wikimedia.org/T209743

Test Plan: Tested in Wikimedia's fork. I believe it results in better UX.

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

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

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

Maniphest Tasks: T15081

Differential Revision: https://we.phorge.it/D25037
2023-03-30 12:11:48 +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
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
53c31b7b13 Fix middle-click, CTRL+click, right-click etc. on Typehead search results
Summary:
Fix middle-click, CTRL+click, right-click etc. on Typehead search results.

Closes T15149

Test Plan:
Try the following actions on various typeheads:

- right-click
- middle-click
- CTRL+click
- normal click

Demonstration video (2M) showing After patch (on localhost) and Before patch (here on we.phorge.it), where I middle-click and normal-click on menu entries:

{F256610}

Notes:

- the middle click now works (opening in new tab)
- the CTRL+click (or "command" key + click) now works (opening in new tab)
- the right click now opens the context menu (previously broken)
- the normal click should just click (as usual)

Try on:

- search results while typing in main search bar
- search results when editing a Task Tags / assigned to, etc.
- try to click on other weird places
- $$$

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15149

Differential Revision: https://we.phorge.it/D25069
2023-03-03 12:12:45 +01:00
Aviv Eyal
6a563a68b5 Celerify
Summary: Fix unit tests in master

 #cowboycommit

Test Plan: arc unit

Reviewers: O1 Blessed Committers!

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

Differential Revision: https://we.phorge.it/D25062
2023-01-28 06:53:03 -08: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
Leon Eckardt
69b2710af9 Prevent Line Numbers in Diffusion being copied as Tabs
Summary: Marks Line Numbers in Diffusion File Preview as unselectable

Test Plan:
- Select multiple Lines from a File Preview in Diffusion
- Copy them into a Text Editor
- The Leading Tabs should no longer included

Reviewers: O1 Blessed Committers, speck, Ekubischta

Reviewed By: O1 Blessed Committers, speck, Ekubischta

Subscribers: Ekubischta, speck, tobiaswiese, valerio.bozzolan

Differential Revision: https://we.phorge.it/D25024
2022-09-21 11:33:20 -06: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
c3f0c0b0f7 Merge Phacility/master into phorge 2022-07-25 11:50:26 -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
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
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
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
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
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
0329df0fa8 Copy file attachment edges to new storage
Summary: Ref T13603. Copy all existing file attachment data into the new storage table.

Test Plan: Ran migration, checked attachment table for general reasonableness.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13603

Differential Revision: https://secure.phabricator.com/D21818
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
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
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
039cbec155 Remove product literal strings in "pht()", part 11
Summary: Ref T13658.

Test Plan: Static checks only, none of these looked easily reachable.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21777
2022-04-25 16:46:24 -07:00
epriestley
3e82ab5adb Remove product literal strings in "pht()", part 1
Summary: Ref T13658. I used the linter in D21763 to identify these and `split` them into arbitrary groups of 10 files.

Test Plan:
This test plan is non-exhaustive, because some of these strings are difficult to reach.

  - Looked at "Create Service" in Almanac.
  - Used "bin/auth" to go through a one-time auth workflow (not all related strings can be hit on a single workflow).
  - Started the "Generate Keypair" worfklow in "SSH Public Keys".

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21765
2022-04-25 12:22:25 -07:00
epriestley
aae23f0204 Give Phame blog posts configurable interact policies, with a default policy of "Same as Blog"
Summary: Ref T13661. This allows posts to have comments disabled (or restricted) on a per-post basis, and makes them inherit the containing blog policy by default.

Test Plan: Locked a post by editing its policy explicitly; locked a post by editing the containing blog policy.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13661

Differential Revision: https://secure.phabricator.com/D21754
2022-04-01 12:52:58 -07:00
epriestley
c25595417f Give Phame blogs mutable interact policies
Summary:
Ref T13661.

I'm fairly sure these policies don't actually do anything (you can't "interact" with a blog) but the primarily support a Phame Post object policy of "Same as Parent Blog", which is the "natural" interact policy for a post.

Most of this is infrastructure support for mutable interact policies: today, only Maniphest has interact mutability and only via indirect effects (locking tasks), not through a directly mutable "Can Interact" policy.

Test Plan:
Ran storage upgrade, edited interact policy of a blog, saw appropriate persistence and transactions.

Created and edited a task to make sure there's no weird fallout from increasing what can be done with interact policies.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13661

Differential Revision: https://secure.phabricator.com/D21751
2022-04-01 12:52:56 -07:00
epriestley
28b12fb3cd Make Phame blog policies non-nullable
Summary: Ref T6203. Ref T13661. These policies are incorrectly nullable, although it's likely that no pathway exists in the application to write NULL into them. Fix the schema.

Test Plan: Ran `bin/storage upgrade`.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13661, T6203

Differential Revision: https://secure.phabricator.com/D21748
2022-04-01 12:52:55 -07:00
Leon Eckardt
b293e6ffed Make Datepicker translatable
Summary: This adds the Ablillity to translate the Content of the Date Picker. It is possible to display more than one Letter for the Day Captions.

Test Plan:
 - Change the Language in the User Preferences, make sure it translates 'S|M|T|W|T|F|S' and Months or add the following to the translation.override config to get German Translation:

```
{
    'S|M|T|W|T|F|S': 'So|Mo|Di|Mi|Do|Fr|Sa',
    'January': 'Januar',
    'February': 'Februar',
    'March': 'März',
    'May': 'Mai',
    'June': 'Juni',
    'July': 'Juli',
    'October': 'Oktober',
    'December': 'Dezember'
}
```
 - Open Datepicker on a Datefield (Edit Event or Edit for Custom Field of Type date)
 - The Day Headers and the Months should now be translated

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: CSharp, 0, speck, tobiaswiese

Tags: #calendar

Differential Revision: https://we.phorge.it/D25016
2021-09-21 20:45:32 -04:00
James Brown
a4948ec800 Add conduit endpoints for querying legalpad
Summary:
This diff adds conduit methods for searching for legalpad documents and signatures. This is very helpful for auditing who's actually signed a document. It also fixes the "contributorPHIDs" constraint in the existing search engine.

In order to expose legalpad signatures through Conduit, this adds a `phid` column to the `legalpad_documentsignature` table. It includes a migration (in the style of many previous phid-adding migrations) to actually populate the column.

Test Plan: We run this on my company's internal fork and it seems to work okay. I don't think any other conduit methods anywhere have tests (???), but if you can point me at one I'm glad to write a unit test!

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: 20after4, speck, tobiaswiese

Differential Revision: https://we.phorge.it/D25018
2021-09-04 13:53:13 -04:00
epriestley
3df1e17527 Add a side nav to Conduit API method console pages
Summary: Ref T13072. Make large Conduit doc pages a bit more navigable. This prepares for updating "harbormaster.sendmessage" to support sending messages to builds.

Test Plan: Viewed various Conduit API documentation pages, clicked links.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21696
2021-07-21 14:16:59 -07:00
epriestley
8bbee92139 Modularize individual Harbormaster build messages
Summary:
Ref T13072. Further modularize build messages by applying each one in a separate transaction type.

This makes it easier to add new types of messages (although I have no particular plans to do this, offhand) and reduces the amount of switch-boilerplate.

This will probably also simplify validating "harbormaster.sendmessage".

Test Plan:
  - Applied all commands.
  - Ran migration, saw transactions render properly

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21690
2021-07-21 14:16:56 -07:00
epriestley
b48d4fabaf Merge the "HarbormasterBuildCommand" table into "HarbormasterBuildMessage"
Summary:
Ref T13072. These two similar tables don't make sense to keep separate. Instead, make Build a valid receiver for BuildMessage objects.

These tables are practically the same, so this is straightforward: just copy the rows in and then drop the old table.

(This table was trivial and ephemeral anyway, so I'm not bothering to do the usual "keep it around for a couple years just in case".)

Test Plan:
  - Populated BuildCommand table, ran migration, saw Builds end up in the proper transitional state (e.g., pausing, aborting, restarting) with appropriate queued messages.
  - Queued new messages by clicking UI buttons.
  - Ran BuildWorkers, saw them process messages and mark them as consumed.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13072

Differential Revision: https://secure.phabricator.com/D21684
2021-07-21 14:16:53 -07:00
Mukunda Modell
a0b91de535 New MediaWiki logo for the icon that represents the MediaWiki oauth provider.
Summary: Replaced the old logo with a new png.

Test Plan: I only tested that the new image was the same dimensions, format and filename as the old one.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D21661
2021-07-11 20:27:58 -05:00
epriestley
5521f76fe4 Remove some "Phacility" and "epriestley" references
Summary:
Ref T13658. This just scrubs some of the simple references from the codebase.

Most of what's left is in documentation which won't be relevant for a fork and/or which I need to separately revise (or more-or-less delete) at some point anyway.

I removed the "install RHEL" and "install Ubuntu" scripts outright since I don't have any reasonable way to test them and don't plan to maintain them.

Test Plan: Grepped for "phacility", "epriestley"; ran unit tests.

Reviewers: cspeckmim

Reviewed By: cspeckmim

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13658

Differential Revision: https://secure.phabricator.com/D21678
2021-07-08 10:46:17 -07:00
epriestley
a641ec82a3 Add an "Authority" control to Packages to support "Watcher" packages
Summary: See T13657. An install has "watcher" packages which should not allow owners to "Force Accept" other packages.

Test Plan:
  - Created package A, which I own, on "/", with "Weak" authority.
  - Created package B, which I do not own, on "/src".
  - Created a revision which touches "/src" and added package B as a reviewer.
  - Attempted to accept the revision...
    - Before patch: permitted to "Force Accept" for package B.
    - After patch: not allowed to "Force Accept" for package B.
  - Verified that setting package "A" back to "Strong" authority allows a force-accept for package B.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D21674
2021-06-25 13:48:46 -07:00
Dmitri Iouchtchenko
1b54afdce5 Saturate day of month in datepicker
Summary:
The datepicker could step by the wrong number of months, due to the date rolling over to the next month when the number of days in the month is exceeded. For example, going forward from January 31 would jump to March 3, while going backward from July 31 would only go to July 1.

Push the date back to ensure that the datepicker stays in the correct month when switching.

Test Plan: Changed months starting from an assortment of dates.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: artms, Korvin

Differential Revision: https://secure.phabricator.com/D21673
2021-06-24 20:02:51 -04:00
Matthew Bowker
af36da4741 Rename example sshd files
Test Plan: Looked at new files, made sure the only changes were to rename the files in line with the documentation

Reviewers: O1 Blessed Committers, eax

Reviewed By: O1 Blessed Committers, eax

Subscribers: speck, tobiaswiese

Maniphest Tasks: T15017

Differential Revision: https://we.phorge.it/D25010
2021-06-20 23:32:17 -06:00
epriestley
1308a5555f Update client logic for inline comment "Save" and "Cancel" actions
Summary: Ref T13559. Substantially correct the client logic for "Save" and "Cancel" actions to handle unusual cases.

Test Plan:
Quoting behavior:

  - Quoted a comment.
  - Cancelled the quoted comment without modifying anything.
  - Reloaded page.
    - Before changes: quoted comment still exists.
    - After changes: quoted comment is deleted.
  - Looked at comment count in header, saw consistent behavior (before: weird behavior).

Empty suggestion behavior:

  - Created a new comment on a suggestable file.
  - Clicked "Suggest Edit" to enable suggestions.
  - Without making any text or suggestion changes, clicked "Save".
    - Before changes: comment saves, but is empty.
    - After changes: comment deletes itself without undo.

General behavior:

  - Created and saved an empty comment (deletes itself).
  - Created and saved a nonempty comment (saves as draft).
  - Created and saved an empty comment with an edit suggestion (saves).
  - Created and saved an empty comment with a suggestion to entirely delete lines -- that is, no suggestion text (saves).
  - Edited a comment, saved without changes (save).
  - Edited a comment, save deleting all text (saves -- note that this is intentionally without undo, since this is a lot of steps to do by accident).
  - Cancel editing an unchanged comment (cancels without undo).
  - Cancel editing a changed comment (cancels with undo).
    - Undo'd, got text back.
  - Cancel new comment with no text (deletes without undo).
  - Cancel new comment with text (deletes with undo).
    - Undo'd, got text back.
  - Saved a quoted comment with no changes (saves -- note that this is intentionally not a "delete", since just quoting someone seems fine if you click "Save" -- maybe you want to come back to it later).

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21654
2021-03-29 09:00:27 -07:00
epriestley
6fd55d692f Formally track "initial", "committed", and "active" states for inline comments
Summary:
Ref T13559. Various client decisions depend on the "initial" or "committed" states of inline comments. Previously, these were informally constructed from "mostly similar" available values, or glossed over in some cases.

On the server, save the initial state when creating a comment. Save the committed state when applying a "save" operation. Send all three states to the client.

On the client, load and track all three states explicitly.

Test Plan: Created inlines, etc. See followups.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21653
2021-03-29 09:00:27 -07:00
epriestley
b75517918d When creating an inline comment, populate the content state with the default suggestion text
Summary:
Ref T13559. Currently, the default text for inline comment side-loads in a bizarre way. Instead, when a user creates an inline comment, load the inline context and set it as part of the initial content state.

This allows the side channel (and the code that puts the text in place at the last second on the client) to be removed.

Test Plan: Created inlines, clicked "Suggest Edit". See followup changes.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21652
2021-03-29 09:00:26 -07:00
epriestley
5efe7fb4c1 On inline comments, track an explicit "committed" content state
Summary:
Ref T13559. To allow the client to make correct decisions about what buttons mean, track an explicit "Committed" content state.

This is the last version of the comment that has been saved on the server, and does not exist if the comment has never been saved.

Test Plan: Created comments, etc. See followups.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21651
2021-03-29 09:00:26 -07:00
epriestley
d30c3a961c Make the client authoritative for "Cancel" actions
Summary:
Ref T13559. When the user clicks the "Cancel" button, we sometimes take it to mean "delete" (when the comment is empty).

Both the client and server make a decision about this, and they may not agree, which causes the client to fall out of sync.

Make the client responsible for deciding whether it wants to interpret a click on the "Cancel" button as a "revert" or a "delete".

Test Plan: Cancelled empty and nonempty comments, etc. See followup changes.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21649
2021-03-29 09:00:25 -07:00
epriestley
60e869f411 Make the client authoritative for "Save" actions
Summary:
Ref T13559. When you click "Save" on an inline comment and it's empty, we may actually delete the comment.

Currently, the client and server both make decisions about whether the comment should be deleted. These decisions may not agree, causing the client state to fall out of sync.

Make the client authoritative about whether it wants to handle the user clicking the "Save" button as an intent to save or an intent to delete.

Test Plan: Saved empty and nonempty inlines. See followup changes.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21648
2021-03-29 09:00:25 -07:00
epriestley
0f04d9e584 Remove direct reads of form state from main Inline client code
Summary:
Ref T13559. Instead of directly reading form state, make all callers use the "active" state instead. The state reads the form.

No functional changes, just clarifying responsiblites.

Test Plan: Created inlines, etc. See followup changes.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21647
2021-03-29 09:00:24 -07:00
epriestley
cb00cb99e2 Make client inlines track an "active" state
Summary:
Ref T13559. Rather than reading from the document, make client inlines actively track their current "active" state.

The "active" state is what the user currently sees in the client UI.

Test Plan: Created inlines, etc. See followups.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21646
2021-03-29 09:00:24 -07:00
epriestley
b964731b6a Make inline "ContentState" a client object, and track "hasSuggestion" on it
Summary:
Ref T13559. In an effort to ultimately fix the "quote + cancel" bug, begin formalizing content states on the client.

This creates a "ContentState" client object and moves the authoritative storage for the "hasSuggestion" property to it.

Test Plan: Created inlines, etc. See followups.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21645
2021-03-29 09:00:23 -07:00
epriestley
87c6c270b4 Fix an issue where inlines could be duplicated in the client list
Summary:
Ref T13559. D21261 added caching here, but the logic in rebuilding inlines wasn't quite correct, and could lead to us double-appending.

Instead, when rebuilding, unconditionally discard the old list.

Test Plan:
  - Added inline comments to a file in Differential.
  - Marked some done.
  - Scrolled so the inline comment header was visible, saw "X / Y Comments" button in header.
  - Clicked "Show 20 more lines" on the changeset with inlines (or toggle "View Unified" / "View Side-by-Side", or other interactions likely work too).
    - Before: saw "X / Y" change improperly (because inlines in that file were double-counted).
    - After: saw stable count.
  - Grepped for "differential-inline-comment-refresh", got no hits, concluded this event has no listeners.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21642
2021-03-29 09:00:23 -07:00
epriestley
db9191f9a8 Correct minor "jump to symbol" behavior in Differential
Summary:
Ref T13644. Ref T13638.

  - Double-encode the symbol that is used as a path component, similar to Diffusion.
  - Fix an outdated reference to ".path", which provided context for symbol lookup.
  - Prevent command-clicking headers from looking up the path as a symbol.

Test Plan:
  - Command-clicked headers, no longer got a symbol.
  - Command-clicked stuff with "/", saw it double-encoded and decoded properly.
  - Command-clicked normal symbols, saw "path" populate correctly.

Maniphest Tasks: T13644, T13638

Differential Revision: https://secure.phabricator.com/D21641
2021-03-17 15:32:07 -07:00
epriestley
d6ed9392d4 Replace Differential "lint stars" with icons
Summary:
Ref T9764. These stars are inconsistent, not accessible, and generally weird. They predate icons.

Update them to use icons instead.

Test Plan:
{F8545721}

{F8545722}

{F8545723}

Maniphest Tasks: T9764

Differential Revision: https://secure.phabricator.com/D21640
2021-03-17 13:45:52 -07:00
epriestley
3267859aee Modernize "mailKey" on Fund initiatives
Summary: Ref T13065. Migrate "mailKey" and drop the column.

Test Plan: Ran "bin/storage upgrade", got a clean upgrade, saw migrated values in mail properties table.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13065

Differential Revision: https://secure.phabricator.com/D21634
2021-03-16 15:51:50 -07:00
epriestley
eeb009b4fa Modernize "mailKey" for Calendar Event
Summary: Ref T13065. Migrate "mailKey" and drop the column.

Test Plan: Ran "bin/storage upgrade", got a clean report, saw migrated data in mail properties table.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13065

Differential Revision: https://secure.phabricator.com/D21633
2021-03-16 15:51:50 -07:00