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

3109 commits

Author SHA1 Message Date
epriestley
1b24b486f5 Manage object mailKeys automatically in Mail instead of storing them on objects
Summary:
Ref T13065. `mailKey`s are a private secret for each object. In some mail configurations, they help us ensure that inbound mail is authentic: when we send you mail, the "Reply-To" is "T123+456+abcdef".

  - The `T123` is the object you're actually replying to.
  - The `456` is your user ID.
  - The `abcdef` is a hash of your user account with the `mailKey`.

Knowing this hash effectively proves that Phabricator has sent you mail about the object before, i.e. that you legitimately control the account you're sending from. Without this, anyone could send mail to any object "From" someone else, and have comments post under their username.

To generate this hash, we need a stable secret per object. (We can't use properties like the PHID because the secret has to be legitimately secret.)

Today, we store these in `mailKey` properties on the actual objects, and manually generate them. This results in tons and tons and tons of copies of this same ~10 lines of code.

Instead, just store them in the Mail application and generate them on demand. This change also anticipates possibly adding flags like "must encrypt" and "original subject", which are other "durable metadata about mail transmission" properties we may have use cases for eventually.

Test Plan:
  - See next change for additional testing and context.
  - Sent mail about Herald rules (next change); saw mail keys generate cleanly.
  - Destroyed a Herald rule with a mail key, saw the mail properties get nuked.
  - Grepped for `getMailKey()` and converted all callsites I could which aren't the copy/pasted boilerplate present in 50 places.
  - Used `bin/mail receive-test --to T123` to test normal mail receipt of older-style objects and make sure that wasn't broken.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13065

Differential Revision: https://secure.phabricator.com/D19399
2018-04-25 06:46:58 -07:00
epriestley
33da9f833f Fix odd line number line wrapping on embedded pastes ({Pxxx})
Summary: Ref T13126. After SourceView changes, embedded pastes with the `{Pxxx}` syntax are line-wrapping line numbers in Safari, at least. Put a stop to this.

Test Plan: Viewed a `{Pxxx}` with more than 10 lines. Before: weird line wrapping; after: nice consistent display.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13126

Differential Revision: https://secure.phabricator.com/D19393
2018-04-20 14:20:20 -07:00
Austin McKinley
4dc8e2de56 Add unique constraint to AlmanacInterfaces
Summary: See discussion in D19379. The 4-tuple of (device, network, address, port) should be unique.

Test Plan: Created lots of duplicate interfaces, bound those interfaces to various services, observed migration script clean things up correctly.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19388
2018-04-19 19:16:50 -07:00
epriestley
19403fdb8e Improve color use in "[+++- ]" element for colorblind users
Summary:
Ref T13127. Users with red/green colorblindness may have difficulty using this element in its current incarnation.

We could give it different behavior if the "Accessibility" option is set for red/green colorblind users, but try a one-size-fits-all approach since the red/green aren't wholly clear anwyay.

Test Plan: {F5530050}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13127

Differential Revision: https://secure.phabricator.com/D19385
2018-04-19 17:24:44 -07:00
epriestley
70d67a3908 Fix the most significant "phantom notification" badness
Summary:
Ref T13124. Ref T13131. Fixes T8953. See PHI512.

When you receieve a notification about an object and then someone hides that object from you (or deletes it), you get a phantom notification which is very difficult to clear.

For now, test that notifications are visible when you open the menu and clear any that are not.

This could be a little more elegant than it is, but the current behavior is very clearly broken. This unbreaks it, at least.

Test Plan:
  - As Alice, configured task stuff to notify me (instead of sending email).
  - As Bailey, added Alice as a subscriber to a task, then commented on it.
  - As Alice, loaded home and saw a notification count. Didn't click it yet.
  - As Bailey, set the task to private.
  - As Alice, clicked the notification bell menu icon.
    - Before change: no unread notifications, bell menu is semi-stuck in a phantom state which you can't clear.
    - After change: bad notifications automatically cleared.

{F5530005}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13131, T13124, T8953

Differential Revision: https://secure.phabricator.com/D19384
2018-04-19 17:24:19 -07:00
Austin McKinley
e81b2173ad Add edge tables for Phlux
Summary: Fixes T13129. This at least makes the existing UI work again before we banish Phlux to the shadow realm.

Test Plan: Edited the visibility for a Phlux variable, didn't get an error. Nothing showed up in the edge tables when I made those changes, but at least it doesn't error out anymore.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13129

Differential Revision: https://secure.phabricator.com/D19387
2018-04-19 15:49:08 -07:00
Austin McKinley
0a83f253ed Add unique constraint for Almanac network names
Summary:
The name of networks should be unique.

Also adds support for exact-name queries for AlamanacNetworks.

Test Plan: Applied migration with existing duplicates, saw networks renamed, attempted to add duplicates, got a nice error message.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D19379
2018-04-19 13:41:15 -07:00
epriestley
665529ab60 Restore coverage reporting to Diffusion browse UI
Summary:
Depends on D19377. Ref T13125. Ref T13124. Ref T13105. Coverage reporting in Diffusion didn't initially survive the transition to Document Engine; restore it.

This adds some tentative/theoretical support for multiple columns of coverage, but no way to actually produce them in the UI. For now, the labels, codes, and colors are hard coded.

Test Plan:
Added coverage with `diffusion.updatecoverage`, saw coverage in the UI:

{F5525542}

Hovered over coverage, got labels and highlighting.

Double-checked labels for "N" (Not Executable) and "U" (Uncovered). See PHI577.

Faked some multi-column coverage, but you can't currently get this yourself today:

{F5525544}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13125, T13124, T13105

Differential Revision: https://secure.phabricator.com/D19378
2018-04-17 14:51:47 -07:00
epriestley
21bb0215db Remove obsoleted "diffusion-browse-file" behavior for coverage
Summary: Ref T13105. After moving Diffusion to DocumnentEngine, this no longer has callers. It will become part of the document behavior.

Test Plan: Grepped for calls to the `diffusion-browse-file` behavior, found none.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19377
2018-04-17 14:51:12 -07:00
epriestley
37a03402bc When following a link to a particular line ("/example.txt$12"), scroll to that line
Summary:
Depends on D19349. Ref T13105. This was the behavior in Diffusion before with a little hard-coded snippet.

Remove that snippet ("diffusion-jump-to") and add a more general-purpose snippet to SourceView.

This is a tiny bit hacky still (and probably doesn't work quite right with Quicksand) but gets things working again and works in all of Files, Paste, and Diffusion.

Test Plan: Followed links to particular lines in Paste, Files and Diffusion; got scrolled to the right place.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19350
2018-04-11 17:29:22 -07:00
epriestley
5b3a351852 Use pseudoelements, not Zero Width Space, to implement copy/paste behavior in Paste/Diffusion
Summary:
Depends on D19348. Ref T13105. When copying text from Paste or Diffusion, we'd like to copy only source, not line numbers.

We currently accomplish this with zero-width spaces plus a trigger that fires on "copy" in Paste and Diffusion. This is quite gross.

In the new-style Harbormaster logs, we use an approach that seems slightly better: CSS psuedoelements.

This isn't a complete solution (see also PHI504 / T5032) but puts us in a slightly better place.

Use it in Paste/Files/Diffusion too.

This gives us good behavior in all browsers in Files and Paste.

This gives us good behavior in Chrome and Firefox in Diffusion. Safari will copy (but not visually select) blame information in Diffusion. I think we can live with that for now.

Test Plan: Selected and copy/pasted stuff in Diffusion, Files, and Paste. Got good behavior everywhere except Safari + Diffusion.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19349
2018-04-11 17:28:46 -07:00
epriestley
c5c53e277a Make line selection in source code views less fragile and more consistent
Summary:
Depends on D19347. Ref T13105. See PHI565. The "highlight lines" behavior is interacting poorly with the new blame element in Diffusion.

Make the behavior a little simpler and hopefully more robust.

Test Plan:
  - Clicked commit/revision links in Diffusion, saw the links get followed instead of the lines highlighted.
  - Highlighted lines in Diffusion, saw just the line/code highlight instead of the whole thing.
  - Highlighted lines in Paste and new-style Harbormaster build logs, saw consistent behavior.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19348
2018-04-11 17:27:34 -07:00
epriestley
ac570fd4bc When you make the file tree huge, scroll to the right, and then toggle it, stop it from growing
Summary: Depends on D19346. Ref PHI568. I love Javascript.

Test Plan:
  - Viewed a revision.
  - Dragged file tree view really wide.
  - Scrolled document to the right.
  - Toggled file tree off and on by pressing "f" twice.
    - Before patch: file tree grew wider and wider after it was toggled.
    - After patch: file tree stayed the same size after it was toggled.
  - Dragged to various widths and reloaded to make sure the "sticky across reloads" behavior still works.
  - Scrolled right, dragged the tree a bit, then reloaded and didn't see it flip out.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19347
2018-04-11 17:25:31 -07:00
epriestley
55619e8964 Restore an explicit white background color to files in Paste
Summary:
Ref T13105. Previously, the "source code" view in Paste rendered on a brown/orange-ish background. I've been using this element in more contexts (Files, Diffusion) and removed the colored background to make text (particularly syntax-highlighted text) easier to read and reduce visual noise with the new blame colors.

In Diffusion the view is in a box with a white background so removing the background left us with white, but in Paste it's just directly on the page so the background was bleeding through. Instead, set it to white explicitly.

Test Plan: Viewed source files in Files, Diffusion and Paste; saw text on a white background.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19346
2018-04-11 17:21:33 -07:00
epriestley
d6ef32a7b7 Give the "Filetree" UI element an explicit background color
Summary:
See PHI568. If you make the file tree UI very wide so that the page generates a horizontal scrollbar and then scroll the page, the page content can paint underneath the menu.

The menu already has a z-index to make it render above the content, but doesn't actually have a background. Give it a background.

The "transparent" rule was added in D16346 but I don't see any reason why we actually need it there, so I think this probably won't break anything.

Test Plan: {F5518822}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13120

Differential Revision: https://secure.phabricator.com/D19344
2018-04-11 10:42:41 -07:00
epriestley
f9c6a69d9c Add skeleton code for Almanac Interfaces to have real transactions
Summary:
Depends on D19322. Ref T13120. Ref T12414.

Currently, `AlmanacDevice` has a bit of a beast of a `TYPE_INTERFACE` transaction that fully creates a complex Interface object. This isn't very flexible or consistent, and Interfaces are complex enough to reasonably have their own object behaviors (for example, they have their own PHIDs).

The complexity of this transaction makes modularizing `AlmanacDevice` transactions tricky. To simplify this, move Interface toward having its own set of normal transactions.

This change just adds some reasonable-looking transactions; it doesn't actually hook them up in the UI or make them reachable. I'll test that they actually work as I swap the UI over.

We may also have some code using the `TYPE_INTERFACE` transaction in Phacility support stuff, so that may need to wait a week to actually phase out.

Test Plan: Ran `bin/storage upgrade` and `arc liberate`. This code isn't reachable yet.

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13120, T12414

Differential Revision: https://secure.phabricator.com/D19323
2018-04-11 10:29:26 -07:00
epriestley
4c4a5a7656 Fix the wrapping/padding behavior of Remarkup code block headers more thoroughly?
Summary: Ref T13118. The first fix there fixed Safari, but made Chrome weird. Try this?

Test Plan: Viewed a code block with `name=...` in Safari, Firefox and Chrome and saw consistent display without weird wrappping.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13118

Differential Revision: https://secure.phabricator.com/D19319
2018-04-10 04:37:14 -07:00
epriestley
472bc3d90a Colorize lines in blame under DocumentEngine, to show relative age of changes
Summary:
Depends on D19313. Ref T13105. Fixes T13015. We lost the coloration for ages in the switch to Document Engine.

Restore it, and use a wider range of colors to make the information more clear.

Test Plan: Viewed some blame, saw a nice explosion of bright colors. This is a cornerstone of good design.

Maniphest Tasks: T13105, T13015

Differential Revision: https://secure.phabricator.com/D19314
2018-04-09 06:11:47 -07:00
epriestley
cf75d63b49 When lines 12, 13, 14, etc all blame to the same change, only show it once
Summary:
Depends on D19312. Ref T13105. For readability, render only one link for each contiguous block of changes.

Also make the actual rendering logic a little more defensible.

Test Plan: Viewed some files with blame, saw one render per chunk instead of one per line.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19313
2018-04-09 06:11:06 -07:00
epriestley
eb80f0a2d9 When you swap between document rendering engines, populate or redraw blame if appropriate
Summary: Depends on D19311. Ref T13105. Currently, blame only renders on the initial request. Instead, redraw blame after swapping views.

Test Plan: Swapped from "Source -> Hexdump -> Source" and "Hexdump -> Source". Saw blame on source in all cases.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19312
2018-04-09 06:10:41 -07:00
epriestley
11664277b3 Make DocumentEngine source line linking behavior better when blame is shown
Summary: Ref T13105. The line linker behavior currently has trouble identifying the line number when blame is active. Improve this, albeit not the most cleanly.

Test Plan: Selected lines with blame on.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19310
2018-04-09 06:09:40 -07:00
epriestley
09c6d42b95 Mostly make blame work with DocumentEngine
Summary: Ref T13105. This needs refinement but blame sort of works again, now.

Test Plan: Viewed files in Diffusion and Files; saw blame in Diffusion when viewing in source mode.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19309
2018-04-09 04:48:21 -07:00
epriestley
90a614778c Make repository symbol references work with DocumentEngine
Summary: Ref T13105. Ref T13047. This makes symbol indexes work with DocumentEngine in Files, and restores support in Diffusion.

Test Plan: Command-clicked stuff, got taken to the symbol index with reasonable metadata in Diffusion, Differential and Files.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105, T13047

Differential Revision: https://secure.phabricator.com/D19307
2018-04-09 04:47:28 -07:00
epriestley
6dea2ba3b3 Fix DocumentEngine line behaviors in Diffusion
Summary:
Ref T13105. Fixes some issues with line linking and highlighting under DocumentEngine:

  - Adding `$1-3` to the URI didn't work correctly with query parameters.
  - Reading `$1-3` from the URI didn't work correctly because Diffusion parses them slightly abnormally.

Test Plan: Clicked/dragged lines to select them. Observed URI. Reloaded page, got the right selection.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19305
2018-04-09 04:46:47 -07:00
epriestley
fc103f71e9 Fix very odd wrapping / linebreaking for Remarkup code block headers in Safari
Summary: Fixes T13118. Ref T13120. This construction is a little odd; I'm not entirely sure why Safari is doing what it's doing, but this appears to fix it.

Test Plan: Viewed blocks like those in T13118 in Safari. Before the patch, weird last-letter wrapping. After the patch, sensible behavior.

Maniphest Tasks: T13118, T13120

Differential Revision: https://secure.phabricator.com/D19303
2018-04-08 06:15:58 -07:00
epriestley
e70c9f72a4 Show revision sizes using a perplexing, inexplicable symbol code
Summary: Ref T13110. See PHI230. Show revision sizes on a roughly logarithmic scale from 1-7 stars. See D16322 for theorycrafting on this element.

Test Plan: Looked at some revisions, saw plausible-looking size markers.

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19294
2018-04-03 12:49:27 -07:00
epriestley
615d27c8e9 Show an additional "Draft" tag on non-broadcasting revisions in a non-draft state
Summary:
Depends on D19284. Ref T13110. It's now possible to get a revision into a "Abandoned + But, Never Promoted From Draft" state. Show this in the header and provide the draft hint above the comment area.

Also, remove `shouldBroadcast()`. The method `getShouldBroadcast()` now has the same meaning.

Finally, migrate existing drafts to `shouldBroadcast = false` and default `shouldBroadcast` to `true`. If we don't do this, every older revision becomes a non-broadcasting revision because this flag was not explicitly set on revision creation before, only on promotion out of draft.

Test Plan: Ran migration; abandoned draft revisions and ended up in a draft + abandoned state.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19285
2018-04-03 11:09:49 -07:00
epriestley
7189cb7ba8 Support text encoding and syntax highlighting options in document rendering
Summary: Depends on D19273. Ref T13105. Adds "Change Text Encoding..." and "Highlight As..." options when rendering documents, and makes an effort to automatically detect and handle text encoding.

Test Plan:
  - Uploaded a Shift-JIS file, saw it auto-detect as Shift-JIS.
  - Converted files between encodings.
  - Highlighted various things as "Rainbow", etc.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19274
2018-03-30 11:28:52 -07:00
epriestley
7eaa27683e Make closed/disabled results in the remarkup autocomplete more visually clear
Summary:
Ref T13114. See PHI522. Although it looks like results are already ordered correctly, the override rendering isn't accommodating disabled results gracefully.

Give closed results a distinctive look (grey + strikethru) so it's clear when you're autocompleting `@mention...` into a disabled user.

Test Plan: {F5497621}

Maniphest Tasks: T13114

Differential Revision: https://secure.phabricator.com/D19272
2018-03-30 08:47:00 -07:00
epriestley
b7d3101e7c Minor document rendering fixes: dropdown for synchronous files, URI normalization for default renderers
Summary:
Depends on D19258. Ref T13105.

  - When the default renderer is an Ajax renderer, don't replace the URI. For example, when viewing a Jupyter notebook, the URI should remain `/F123`, not instantly change to `/view/123/jupyter/`.
  - Fix an issue where non-ajax renderers could fail to display the dropdown menu properly.

Test Plan:
  - Viewed a Jupyter notebook, stayed on the same URI.
  - Changed rendering, got different URIs.
  - Viewed a JSON file and toggled renderers via dropdown.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19259
2018-03-28 15:07:21 -07:00
epriestley
f583406ba9 Drop uniqueness constraint on PushEvent request ID
Summary: See <https://discourse.phabricator-community.org/t/pushing-to-mercurial-repository-fails/1275/1>. Mercurial may invoke hooks multiple times per push.

Test Plan: Pushed to Mercurial, saw key constraint failure.

Differential Revision: https://secure.phabricator.com/D19257
2018-03-26 07:02:15 -07:00
epriestley
bba1b185f8 Improve minor client behaviors for document rendering
Summary:
Ref T13105. This adds various small client-side improvements to document rendering.

  - In the menu, show which renderer is in use.
  - Make linking to lines work.
  - Make URIs persist information about which rendering engine is in use.
  - Improve the UI feedback for transitions between document types.
  - Load slower documents asynchronously by default.
  - Discard irrelevant requests if you spam the view menu.

Test Plan: Loaded files, linked to lines, swapped between modes, copy/pasted URLs.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19256
2018-03-23 14:09:31 -07:00
epriestley
d2727d24da Add an abstract "Text" document engine and a "Source" document engine
Summary: Ref T13105. Allow normal text files to be rendered as documents, and add a "source code" rendering engine.

Test Plan: Viewed some source code.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19254
2018-03-23 12:28:43 -07:00
epriestley
cbf3d3c371 Add a very rough, proof-of-concept Jupyter notebook document engine
Summary:
Depends on D19252. Ref T13105. This very roughly renders Jupyter notebooks.

It's probably better than showing the raw JSON, but not by much.

Test Plan:
  - Viewed various notebooks with various cell types, including markdown, code, stdout, stderr, images, HTML, and Javascript.
  - HTML and Javascript are not live-fired since they're wildly dangerous.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19253
2018-03-23 07:14:45 -07:00
epriestley
fb4ce851c4 Add a PDF document "rendering" engine
Summary:
Depends on D19251. Ref T13105. This adds rendering engine support for PDFs.

It doesn't actually render them, it just renders a link which you can click to view them in a new window. This is much easier than actually rendering them inline and at least 95% as good most of the time (and probably more-than-100%-as-good some of the time).

This makes PDF a viewable MIME type by default and adds a narrow CSP exception for it. See also T13112.

Test Plan:
  - Viewed PDFs in Files, got a link to view them in a new tab.
  - Clicked the link in Safari, Chrome, and Firefox; got inline PDFs.
  - Verified primary CSP is still `object-src 'none'` with `curl ...`.
  - Interacted with the vanilla lightbox element to check that it still works.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19252
2018-03-23 07:14:17 -07:00
epriestley
8b658706a8 Add a basic Remarkup document rendering engine
Summary:
Ref T13105. Although Markdown is trickier to deal with, we can handle Remarkup easily.

This may need some support for encoding options.

Test Plan: Viewed `.remarkup` files, got remarkup document presentation by default. Viewed other text files, got an option to render as remarkup.

Reviewers: avivey

Reviewed By: avivey

Subscribers: mydeveloperday, avivey

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19251
2018-03-23 07:07:50 -07:00
epriestley
df3c937dab Record lock timing information on PushEvents
Summary:
Depends on D19249. Ref T13109. Add timing information to the `PushEvent`:

  - `writeWait`: Time spent waiting for a write lock.
  - `readWait`: Time spent waiting for a read lock.
  - `hostWait`: Roughly, total time spent on the leaf node.

The primary goal here is to see if `readWait` is meaningful in the wild. If it is, that motivates smarter routing, and the value of smarter routing can be demonstrated by looking for a reduction in read wait times.

Test Plan: Pushed some stuff, saw reasonable timing values in the table. Saw timing information in "Export Data".

Maniphest Tasks: T13109

Differential Revision: https://secure.phabricator.com/D19250
2018-03-22 13:46:01 -07:00
epriestley
69bff489d4 Generate a random unique "Request ID" for SSH requests so processes can coordinate better
Summary:
Depends on D19247. Ref T13109. When we receive an SSH request, generate a random unique ID for the request. Then thread it down through the process tree.

The immediate goal is to let the `ssh-exec` process coordinate with `commit-hook` process and log information about read and write lock wait times. Today, there's no way for `ssh-exec` to interact with the `PushEvent`, but this is the most helpful place to store this data for users.

Test Plan: Made pushes, saw the `PushEvent` table populate with a random request ID. Exported data and saw the ID preserved in the export.

Maniphest Tasks: T13109

Differential Revision: https://secure.phabricator.com/D19249
2018-03-22 13:44:30 -07:00
epriestley
e010aaca43 accidentally a word
Summary: Sometimes I dream I am a small turtle.

Test Plan: squeak squeak

Differential Revision: https://secure.phabricator.com/D19248
2018-03-22 13:43:10 -07:00
epriestley
c8583b016d When workflow dialog buttons are clicked, disable the button
Summary:
Depends on D19245. Fixes T11145. Ref T13108. See PHI488. Disable workflow buttons when they're clicked to prevent accidental client-side double submission.

This might have some weird side effects but we should normally never need to re-use a workflow dialog form so it's not immediately obvious that this can break anything.

Test Plan:
  - Added `sleep(1)` to the Mute controller and the Maniphest task controller.
  - Added `phlog(...)` to the Mute controller.
  - Opened the mute dialog, mashed the button a thousand times.
    - Before: Saw a bunch of logs.
    - After: Button immediately disables, saw only one log.

Maniphest Tasks: T13108, T11145

Differential Revision: https://secure.phabricator.com/D19246
2018-03-21 11:58:13 -07:00
epriestley
9e278a89ba If a Workflow form receives a redirect response, don't re-enable the submit buttons
Summary:
See PHI488. Ref T13108. Currently, there is a narrow window between when the response returns and when the browser actually follows the redirect where the form is live and you can click the button again.

This is relativey easy if Phabricator is running //too fast// since the button may be disabled only momentarily. This seems to be easier in Firefox/Chrome than Safari.

Test Plan:
  - In Firefox and Chrome, spam-clicked a comment submit button.
    - Before: could sometimes get a double-submit.
    - After: couldn't get a double-submit.
    - This could probably be reproduced more reliabily by adding a `sleep(1)` to whatever we're redirecting //to//.
  - Submitted an empty comment, got a dialog plus a still-enabled form (so this doesn't break the non-redirect case).

Maniphest Tasks: T13108

Differential Revision: https://secure.phabricator.com/D19245
2018-03-21 11:56:21 -07:00
epriestley
4aafce6862 Add filesize limits for document rendering engines and support partial/complete rendering
Summary:
Depends on D19238. Ref T13105. Give document engines some reasonable automatic support for degrading gracefully when someone tries to hexdump a 100MB file or similar.

Also, make "Video" sort above "Audio" for files which could be rendered either way.

Test Plan: Viewed audio, video, image, and other files. Adjusted limits and saw full, partial, and fallback/error rendering.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19239
2018-03-19 15:18:34 -07:00
epriestley
f646153f4d Add an async driver for document rendering and a crude "Hexdump" document engine
Summary: Depends on D19237. Ref T13105. This adds a (very basic) "Hexdump" engine (mostly just to have a second option to switch to) and a selector for choosing view modes.

Test Plan: Viewed some files, switched between audio/video/image/hexdump.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19238
2018-03-19 15:18:05 -07:00
epriestley
01f22a8d06 Roughly modularize document rendering in Files
Summary:
Ref T13105. This change begins modularizing document rendering. I'm starting in Files since it's the use case with the smallest amount of complexity.

Currently, we hard-coding the inline rendering for images, audio, and video. Instead, use the modular engine pattern to make rendering flexible and extensible.

There aren't any options for switching modes yet and none of the renderers do anything fancy. This API is also probably very unstable.

Test Plan: Viewwed images, audio, video, and other files. Saw reasonable renderings, with "nothing can render this" for any other file type.

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19237
2018-03-19 15:17:04 -07:00
epriestley
dbc72a05bc Correct the behavior of "Desktop Only" in Notifications preferences
Summary:
See <https://discourse.phabricator-community.org/t/desktop-only-notifications-mode-is-broken/1234>. Ref T13102. The "Desktop Only" mode for notifications currently shows both desktop and web notifications.

In fact, `JX.Notification` currently has no ability to render notifications as desktop-only. Make this work.

Note that many of the variables and parameters here, including `showAnyNotification`, `web_ready`, and `desktop_ready`, are named in an incorrect or misleading way. However, the new behavior appears to be correct.

Test Plan:
  - Emitted test notifications in "No Notifications", "Web Only", "Web and Desktop", and "Desktop" modes.
  - Saw appropriate notifications appear in the UI.

Maniphest Tasks: T13102

Differential Revision: https://secure.phabricator.com/D19233
2018-03-16 15:17:49 -07:00
epriestley
2b5c73fc3d In "Analyze Query Plans" mode, collect service call stack traces in DarkConsole
Summary: Ref T13106. When profiling service queries, there's no convenient way to easily get a sense of why a query was issued. Add a mode to collect traces for each query to make this more clear. This is rough, but works well enough to be useful.

Test Plan: Clicked "Analyze Query Plans", got stack traces for each service call.

Maniphest Tasks: T13106

Differential Revision: https://secure.phabricator.com/D19221
2018-03-14 20:34:34 -07:00
epriestley
dc7e40ff3f Fix the DarkConsole inline error log stack trace expansion behavior for Content-Security-Policy
Summary:
See PHI451. Ref T13102. DarkConsole uses an ancient inline "onclick" handler to expand the stack traces for errors.

The new Content-Security-Policy prevents this from functioning.

Replace this with a more modern behavior-driven action instead.

Test Plan:
  - Clicked some errors in DarkConsole, saw stack traces appear.
  - Grepped for `onclick` and `jsprintf()` to see if I could find any more of these, but came up empty.

Maniphest Tasks: T13102

Differential Revision: https://secure.phabricator.com/D19218
2018-03-13 16:45:20 -07:00
epriestley
0bf8e33bb6 Issue setup guidance recommending MySQLi and MySQL Native Driver
Summary:
Fixes T12994. We need `MYSQLI_ASYNC` to implement client-side query timeouts, and we need MySQLi + MySQL Native Driver to get `MYSQLI_ASYNC`.

Recommend users install MySQLi and MySQL Native Driver if they don't have them. These are generally the defaults and best practice anyway, but Ubuntu makes it easy to use the older stuff.

All the cases we're currently aware of stem from `apt-get install php5-mysql` (which explicitly selects the non-native driver) so issue particular guidance about `php5-mysqlnd`.

Test Plan:
  - Faked both issues locally, reviewed the text.
  - Will deploy to `secure`, which currently has the non-native driver.

Maniphest Tasks: T12994

Differential Revision: https://secure.phabricator.com/D19216
2018-03-13 12:38:09 -07:00
epriestley
2b19f91936 Allow Doorkeeper references to have multiple display variations (full, short, etc.)
Summary:
Ref T13102. An install has a custom rule for bridging JIRA references via Doorkeeper and would like to be able to render them as `JIRA-123` instead of `JIRA JIRA-123 Full JIRA title`.

I think it's reasonable to imagine future support upstream for `JIRA-123`, `{JIRA-123}`, and so on, although we do not support these today. We can take a small step toward eventual support by letting the rendering pipeline understand different view modes.

This adds an optional `name` (the default text rendered before we do the OAuth sync) and an optional `view`, which can be `short` or `full`.

Test Plan:
I tested this primarily with Asana, since it's less of a pain to set up than JIRA. The logic should be similar, hopefully.

I changed `DoorkeeperAsanaRemarkupRule` to specify `name` and `view`, e.g `'view' => (mt_rand(0, 1) ? 'short' : 'full')`. Then I made a bunch of Asana references in a comment and saw them randomly go short or long.

Maniphest Tasks: T13102

Differential Revision: https://secure.phabricator.com/D19215
2018-03-13 11:29:52 -07:00
epriestley
1e93b49b1b Allow custom actions in Differential to explicitly override "accept" stickiness
Summary:
See PHI431. Ref T13102. An install is interested in a custom "non-sticky" accept action, roughly.

On the one hand, this is a pretty hacky patch. However, I suspect it inches us closer to T731, and I'm generally comfortable with exploring the realms of "Accept Next Update", "Unblock Without Accepting", etc., as long as most of it doesn't end up enabled by default in the upstream.

Test Plan:
  - Accepted and updated revisions normally, saw accepts respect global stickiness.
  - Modified the "Accept" action to explicitly be unsticky, saw nonsticky accept behavior after update.

Maniphest Tasks: T13102

Differential Revision: https://secure.phabricator.com/D19211
2018-03-12 17:10:43 -07:00