Summary:
Ref T9360. Moves PhamePost to CommentEditEngine.
[x] HTTP Parameters dropdown on New Post goes to 404
[x] Implement EditEngine Comments
Test Plan: Make Post, Make Comment, Laugh.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D16222
Summary: Fixes T11280. I extracted this at the last minute and got the constant flipped.
Test Plan: Archived, then activated a paste. Observed correct timeline stories/icons/etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11280
Differential Revision: https://secure.phabricator.com/D16240
Test Plan: Tested with a transactionType from an extension.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9789
Differential Revision: https://secure.phabricator.com/D16236
Summary: Fixes T11275. This search query doesn't actually have any options so these links are a little pointless, but generate valid links instead of 404s.
Test Plan: Clicked "Advanced Search" and "Edit Queries" from `/settings/`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11275
Differential Revision: https://secure.phabricator.com/D16238
Summary: Fixes T11276. This feels slightly iffy (we `attachRepository()` here, and also when applying the TYPE_REPOSITORY transaction) but simpler than trying to reorder things.
Test Plan: Created a repository URI with transactions in `["uri", "repository"]` order.
Reviewers: chad, avivey
Reviewed By: avivey
Maniphest Tasks: T11276
Differential Revision: https://secure.phabricator.com/D16237
Summary:
Fixes T11269. The basic issue is that `git log` in an empty repository exits with an error message.
Prior to recent Git (2.6?), this message reads:
> fatal: bad default revision 'HEAD'
This message was somewhat recently changed by <ce11360467>. After that, it reads:
> fatal: your current branch 'master' does not have any commits yet
This change isn't //technically// a //complete// fix because you could still hit this issue like this:
- Create an empty repository.
- Push some stuff to `master`.
- Delete `master`.
However, this is very rare and even in this case the repository will fix itself once you push something again. We can try to fix that if any users ever actually hit it.
Test Plan:
- Created a new empty Git repository.
- Ran `bin/repository update Rxx`.
- Before patch: "git log" error because of the empty repository.
- After patch: clean update.
- Also ran `repository update` on a non-empty repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11269
Differential Revision: https://secure.phabricator.com/D16234
Summary:
Fixes T11274. When task titles are long, we currently wrap stuff and the trace graph renders real weird.
Instead, prevent taks/revision titles from wrapping/overflowing.
(This works in a slightly weird way, and `text-overflow: ellipsis;` has no apparent effect on any of the containers.)
Test Plan: {F1712394}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11274
Differential Revision: https://secure.phabricator.com/D16233
Summary: Ref T9640. Fixes T9888. Decline to support PHP 7 until the async signal handling issue in T11270 is resolved.
Test Plan: Faked local version, got helpful error message.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9640, T9888
Differential Revision: https://secure.phabricator.com/D16231
Summary: Fixes T11243. Seems reasonable to open this stuff in a new window so you don't put any application state in Herald, etc., at risk -- looking in this menu for help with a currently-executing workflow is reasonable and normal.
Test Plan: Clicked a help menu link, saw it open in a new page.
Reviewers: avivey, chad
Reviewed By: chad
Maniphest Tasks: T11243
Differential Revision: https://secure.phabricator.com/D16230
Summary: Fixes T11267. This data was coming back weird (in reverse order relative to the graph itself). Previously it worked OK anyway, but the new logic is a little more sensitive to the input.
Test Plan: Viewed a Mercurial repository with linear history, saw linear history.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11267
Differential Revision: https://secure.phabricator.com/D16229
Summary:
Ref T5267. Two general changes:
- Make string extraction use a cache, so that it doesn't take several minutes every time you change something. Minor updates now only take a few seconds (like `arc liberate` and similar).
- Instead of dumping a sort-of-template file out, write out to a cache (`src/.cache/i18n_strings.json`). I'm planning to add more steps to read this cache and do interesting things with it (emit translatewiki strings, generate or update standalone translation files, etc).
Test Plan:
- Ran `bin/i18n extract`.
- Ran it again, saw it go a lot faster.
- Changed stuff, ran it, saw it only look at new stuff.
- Examined caches.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5267
Differential Revision: https://secure.phabricator.com/D16227
Summary: Ref T11244. 8 more tokens. Probably need better math on the selector?
Test Plan: Award Dat Boi.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: putnam, Korvin
Maniphest Tasks: T11244
Differential Revision: https://secure.phabricator.com/D16228
Summary: Fixes T11159. We get two different values here (`NULL` and `0`) with different meanings.
Test Plan:
- Ran `STOP SLAVE;`.
- Saw this:
{F1710181}
- Ran `START SLAVE;`.
- Back to normal.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11159
Differential Revision: https://secure.phabricator.com/D16225
Summary:
Ref T10855. This can't replace the old edit flow yet, but get the basics in place.
(This is actually much closer to just being able to swap than I anticipated since CustomFields sort of just work, but the exiting flow has some "clone existing panel" / "place directly on dashboard" stuff that this doesn't yet.)
Test Plan: Created and edited a panel by manually using the "editpro" flow.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10855
Differential Revision: https://secure.phabricator.com/D16226
Summary: Ref T4788. It's not easy to tell at a glance which objects are open vs closed. Try to make that a bit more clear. This could probably use some more tweaking.
Test Plan: {F1708330}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16219
Summary:
Ref T4788. As it turns out, our tasks are very tightly connected.
Instead of loading every parent/child task, then every parent/child of those tasks, etc., etc., only load tasks in the "same direction" that we're already heading.
For example, we load children of children, but not parents of children. And we load parents of parents, but not children of parents.
Basically we only go "up" and "down" now, but not "out" as much. This should reduce the gigantic multiple-thousand-node graphs currently shown in the UI.
I still discover the whole graph for revisiosn, because I think it's probably more useful and always much smaller. That might need adjustment too, though.
Test Plan: Seems fine locally??
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16218
Summary: Ref T9360. This makes these transactional.
Test Plan: Set new header, delete header. Set new profile image, reset profile image.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D16217
Summary:
Ref T4788. This fixes all the bugs I was immediately able to catch:
- "Directory-Like" graph shapes could draw too many vertical lines.
- "Reverse-Directory-Like" graph shapes could draw too few vertical lines.
- Terminated, branched graph shapes drew the very last line to the wrong place.
This covers the behavior with tests, so we should be able to fix more stuff later without breaking anything.
Test Plan:
- Added failing tests and made them pass.
{F1708158}
{F1708159}
{F1708160}
{F1708161}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16216
Summary: Ref T4788. This seems reasonable locally, but not sure how it will feel on real data. Might need some tweaks, or might just be a terrible idea.
Test Plan: {F1708059}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16214
Summary:
Ref T4788. This separates the revision graph view into a base class with core logic and a revision class with Differential-specific logic, so I can subclass it in Maniphest, etc., and try using it in other applications to show similar graphs.
Not sure if we'll stick with it, but even if we don't this makes the code a bit cleaner and gets custom rendering logic out of the RevisionViewController, which is nice.
Test Plan: Viewed revisions, saw the stack UI completely unchanged.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16213
Summary: New tokens, slightly larger (18x18 vs 16x16). I think these all feel decent, I might tweak the thumbs icons a little more color-wise.
Test Plan:
Use Tokens.
{F1707411}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11244
Differential Revision: https://secure.phabricator.com/D16211
Summary: Ref T10628. Turn these into tabs in a single box, since "local commits" and "similar revisions" are of particularly rare use.
Test Plan: {F1707196}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10628
Differential Revision: https://secure.phabricator.com/D16209
Summary: Ref T10628. Cleans up remaining weird, unused tab behaviors in ObjectBoxView to simplify ObjectBox.
Test Plan: Toggled tabs in Files.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10628
Differential Revision: https://secure.phabricator.com/D16208
Summary: Ref T10628. This moves everything else over. I'll clean up the cruft in the next diff.
Test Plan:
- Viewed Conduit API page, toggled tabs.
- Viewed Harbormaster build, toggled tabs.
- Viewed a Drydock lease, swapped tabs.
- Viewed a Drydock resource, swapped tabs.
- Viewed mail, swapped tabs.
- Grepped for `addPropertyList(...)`, looked for any remaining calls with a second argument.
- Also checked rSAAS for any calls, but we don't have anything there that uses tabs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10628
Differential Revision: https://secure.phabricator.com/D16207
Summary:
Ref T10628. Switch this to be nicer and more modern.
- When there's only one tab, add an option to hide it.
Test Plan:
- Viewed normal revisions (no tabs).
- Viewed X vs Y revisions (two tabs, rightmost tab selected by default).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10628
Differential Revision: https://secure.phabricator.com/D16206
Summary:
Ref T10628. Currently, tabs are part of ObjectBoxes. However, the code is a bit of a mess and I want to use them in some other contexts, notably the "prose diff" dialog to show "old raw, new raw, diff".
Pull them out, and update Files to use the new stuff. My plan is:
- Update all callsites to this stuff.
- Remove the builtin-in ObjectBox integration to simplify ObjectBox a bit.
- Move forward with T10628.
This is pretty straightforward. A couple of the sigils are a little weird, but I'll update the JS later. For now, the same JS can drive both old and new tabs.
Test Plan: Viewed files, everything was unchanged.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10628
Differential Revision: https://secure.phabricator.com/D16205
Summary:
Fixes T11242. See that task for detailed discussion.
Previously, it didn't particularly matter that we don't MIME detect chunked files since they were all just big blobs of junk (PSDs, zips/tarballs, whatever) that we handled uniformly.
However, videos are large and the MIME type also matters.
- Detect the overall mime type by detecitng the MIME type of the first chunk. This appears to work properly, at least for video.
- Skip mime type detection on other chunks, which we were performing and ignoring. This makes uploading chunked files a little faster since we don't need to write stuff to disk.
Test Plan:
Uploaded a 50MB video locally, saw it as chunks with a "video/mp4" mime type, played it in the browser in Phabricator as an embedded HTML 5 video.
{F1706837}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11242
Differential Revision: https://secure.phabricator.com/D16204
Summary:
Ref T4788. When closing a task as a duplicate of another task, you can only select one task, since it doesn't really make sense to merge one task into several other tasks (this operation is //possible//, but probably not what anyone ever wants to do, I think?).
Make the UI understand this: after you select a task, disable all of the "select" buttons in the UI to make this clear.
Test Plan:
- Used "Close as Duplicate", only allowed to select 1 task.
- Used other editors like "Merge Duplicates In", allowed to select lots of tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16203
Summary: Ref T9360. This adds ability to search posts by blog(s) and by type better.
Test Plan:
Create some posts, search for them.
{F1705961}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D16199
Summary:
Ref T4788. Fixes T10703.
In the longer term I want to put this on top of ApplicationSearch, but that's somewhat complex and we're at a fairly good point to pause this feature for feedback.
Inch toward that instead: provide more appropriate filters and defaults without rebuilding the underlying engine. Specifically:
- No "assigned" for commits (barely makes sense).
- No "assigned" for mocks (does not make sense).
- Default to "open" for parent tasks, subtasks, close as duplicate, and merge into.
Also, add a key to the `search_document` table to improve the performance of the "all open stuff of type X" query. "All Open Tasks" is about 100x faster on my machine with this key.
Test Plan:
- Clicked all object relationships, saw more sensible filters and defaults.
- Saw "open" query about 100x faster locally (300ms to 3ms).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788, T10703
Differential Revision: https://secure.phabricator.com/D16202
Summary: Fixes T11240. Also simplify things a little and share a bit more code.
Test Plan:
- Viewed revisions and tasks, opened submenu.
- Viewed as a user without edit permission, saw the menus greyed out.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11240
Differential Revision: https://secure.phabricator.com/D16201
Summary:
Ref T4788. Fixes T7820. This updates the "Merge Duplicates In" interaction, and adds a "Close as Duplicate" action.
These are the last interactions that were using the old code, so it removes that code.
Merges are now recorded as real edges, so we can show them in the UI later on (originally from T9390, etc).
Also provides more general support for relationships which need EDIT permission, not-undoable relationships like merges, preventing relating an object to itself, and relationship side effects like merges.
Finally, fixes a couple of behaviors around typing an exact object name (like `T123`) to find the related object.
Test Plan:
- Merged tasks into the current task.
- Closed the current task as a duplicate of another task.
- Edited other relationships.
- Searched for tasks, commits, etc., by object monogram.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788, T7820
Differential Revision: https://secure.phabricator.com/D16196
Summary:
The first dialog was being given the wrong user (`$user`, should be `$viewer`), leading to a CSRF issue.
(The CSRF token it generated was invalid in all validation contexts, so this wasn't a security problem or a way to capture CSRF tokens for other users.)
Use `newDialog()` instead.
(This seems completely unrelated to the vaguely-similar-looking issues we saw earlier this week.)
Test Plan:
- Added a new email address.
- Clicked "Done" on the last step.
- Completed workflow instead of getting a CSRF error.
Reviewers: chad, tide
Reviewed By: tide
Differential Revision: https://secure.phabricator.com/D16200
Summary: Fixes T11236, breaks long words and inlines the spans.
Test Plan:
Use a diff with super long text like SSH keys, set Font to 24/48 Impact
{F1705910}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11236
Differential Revision: https://secure.phabricator.com/D16198
Summary:
Ref T9360.
[x] View Live useless on archived blogs
[x] Edit Blog Image treatment like profiles
[x] Pager next/prev should keep you on whatever view you're on
[x] Unset user titles aren't falling back properly
[x] Add captions to edit fields for better clarification
Test Plan: Archive a blog, Edit a photo, verify pager on live and internal blogs, check empty titles, and view new edit form instructions.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D16197
Summary: Ref T4788. This moves everything except "merge" to the new code.
Test Plan:
- Edited relationships in Differential, Diffusion, and Pholio.
- Uninstalled Pholio, made sure "Edit Mocks..." actions vanished.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16193
Summary:
Ref T4788. Fixes T9232. This moves the "search for stuff to attach to this object" flow away from hard-coding and legacy constants and toward something more modular and flexible.
It also adds an "Edit Commits..." action to Maniphest, resolving T9232. The behavior of the search for commits isn't great right now, but it will improve once these use real ApplicationSearch.
Test Plan: Edited a tasks' related commits, mocks, tasks, etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788, T9232
Differential Revision: https://secure.phabricator.com/D16189
Summary:
Ref T11232. The cluster connection pathway specifies a timeout when connecting, but this connection pathway does not. (I'm not sure if we just never did or if it got lost at some point.)
Soon, T11044 will obsolete this and unify the database connection pathways, but that's a more complicated change.
I'm not sure if this will fix T11232, but it can't hurt.
Test Plan: Put a `throw` on timeout specifications. Before the change: did not hit it in non-cluster configurations. After the change: hit it.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11232
Differential Revision: https://secure.phabricator.com/D16194
Summary: Fixes T11223. I missed a few of these; most of them kept working anyway because we have redirects in place, but make them a bit more modern/not-hard-coded.
Test Plan:
- Generated and revoked API tokens for myself.
- Generated and revoked API tokens for bots.
- Revoked temporary tokens for myself.
- Clicked the link to the API tokens panel from the Conduit console.
- Clicked all the cancel buttons in all the dialogs, too.
In all cases, everything now points at the correct URIs. Previously, some things pointed at the wrong URIs (mostly dealing with stuff for bots).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11223
Differential Revision: https://secure.phabricator.com/D16185
Summary:
Fixes T11225. The primary issue here is that this rule is bleeding down too far. It appears that it's only intended to put space around the inline as a whole.
Spacing still isn't //perfect// since a few other rules are bleeding, but it feels reasonable now instead of being clearly broken.
Test Plan:
- Added "background: red;" to figure out what was being affected.
- Before:
{F1704081}
- After:
{F1704084}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11225
Differential Revision: https://secure.phabricator.com/D16184
Summary: love to wordsmith
Test Plan: read it
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16183
