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

8263 commits

Author SHA1 Message Date
epriestley
f1889aa942 Use a custom quit message when gracefully shutting down the IRC bot
Summary: Fixes T3173. This doesn't actually fix T3173 but I'm going to redirect that. It does make the bot quit IRC gracefully, with a nicer quit message, which can be customized.

Test Plan: Got a bot to quit IRC with nice messages.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3173

Differential Revision: https://secure.phabricator.com/D10257
2014-08-13 15:33:38 -07:00
epriestley
1ad0e84518 Show image dimensions and some other metadata in Differential
Summary: Fixes T2101. When viewing an image change, show image dimensions, MIME type, and filesize.

Test Plan:
{F190189}

{F190190}

very utility

such wow

Reviewers: mailson, btrahan, chad

Reviewed By: chad

Subscribers: epriestley, Korvin, aran

Maniphest Tasks: T2101

Differential Revision: https://secure.phabricator.com/D5206
2014-08-13 14:41:06 -07:00
epriestley
df28d751f5 Add an X-Phabricator-Projects header to outbound mail
Summary: Fixes T4973. For `PhabricatorProjectInterface` objects, add a header to let clients do mail filtering.

Test Plan: Saw `X-Phabricator-Projects: <#goat_farm>` in outbound mail.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: rush898, epriestley

Maniphest Tasks: T4973

Differential Revision: https://secure.phabricator.com/D10256
2014-08-13 14:40:54 -07:00
epriestley
a37dc68b0a Show projects on feed stories
Summary: Fixes T1922. When a story is about a primary object associated with projects, show those projects on the feed story.

Test Plan: {F190171}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T1922

Differential Revision: https://secure.phabricator.com/D10255
2014-08-13 11:24:56 -07:00
epriestley
bcdadf5947 Add autocomplete=off to all non-login password forms
Summary: Fixes T5579. Modern browsers aggressively autofill credentials, but at least Firefox still behaves slightly better with this flag. Hopefully other browsers will follow suit.

Test Plan: Browsed various interfaces, verifying that login interfaces allow autocomplete while non-login interfaces do not.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5579

Differential Revision: https://secure.phabricator.com/D10253
2014-08-13 10:06:48 -07:00
epriestley
a5d2460974 Probably fix bad method call in Diffusion
Summary: Fixes T5869. Ref T4896. This `setID()` method no longer exists.

Test Plan: (WARNING) This is a pain to reproduce locally so I'm just winging it. I'm 99% sure this ID is only used to generate an anchor link. This is a hack to start with, and T4896 will eventualy clean it up properly.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896, T5869

Differential Revision: https://secure.phabricator.com/D10254
2014-08-13 10:06:41 -07:00
James Rhodes
e07011d37d Disable known hosts when SSH'ing through Drydock
Summary: This prevents SSH from saving the host key into known_hosts; StrictHostKeyChecking only prevents it from prompting for unknown hosts, but it will still verify hosts against what it has previously saved.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10252
2014-08-13 15:03:10 +10:00
James Rhodes
ca8f7cdaa5 Execute commands under Powershell on Windows for Harbormaster
Summary:
Resolves T5831.  This modifies the Drydock SSH interface to execute commands under Powershell when the target host platform is Windows.  Powershell is far more featured than cmd.exe, and more closely resembles a UNIX shell.

Currently Powershell outputs stderr as an XML blob on a line, and while this code currently doesn't use that, it will allow us in the future (planned next week) to redirect that output to the stderr log instead of having it all merged in with stdout under cmd (where there is no way to distinguish it).

Test Plan:
Ran various native commands and PowerShell commands from a Harbormaster build, including things like:

```
Write-Host ("my test" + ${build.id})
```

and saw:

```
my test679
```

in the output.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5831

Differential Revision: https://secure.phabricator.com/D10248
2014-08-13 12:48:52 +10:00
epriestley
8ef1ea63dd Add "unblock" and "column" mail tags to Maniphest
Summary: Fixes T5769. Fixes T5861. Add mail tags for "unblock" and "column change".

Test Plan: Did unblocks and column changes, verified the mail got the right mailtags and recipient nondelivery flags.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5861, T5769

Differential Revision: https://secure.phabricator.com/D10241
2014-08-12 14:07:37 -07:00
cpettet
0420754d73 allow setting view and edit policy with maniphest.createtask
Summary: allowing explicit maniphest policy on api creation

Test Plan:
tested with arc cli

Example:

> echo '{"title": "mc6", "editPolicy": "users", "viewPolicy": "users"}' | \
> arc call-conduit maniphest.createtask

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10243
2014-08-12 14:07:27 -07:00
epriestley
5183bf3c49 Detect macros more strictly in bot handler
Summary: Fixes T5863. Like Remarkup, only identify macros if the entire chat line is the macro.

Test Plan: https://secure.phabricator.com/chatlog/channel/6/?at=158229

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5863

Differential Revision: https://secure.phabricator.com/D10242
2014-08-12 14:07:23 -07:00
epriestley
c443913c0b Allow users to set notifications to "Email", "Notification", or "Ignore"
Summary:
Ref T5861. Ref T5769. If users don't care at all about something, allow them to ignore it.

We have some higher-volume notifications either built now (column changes) or coming (mentions) which users might reasonably want to ignore completely.

Test Plan:
Ignored some notifications, then took appropraite actions. Saw my user culled from the notification subscriber list.

{F189531}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5769, T5861

Differential Revision: https://secure.phabricator.com/D10240
2014-08-12 12:29:03 -07:00
epriestley
f6f9d78f3a Modularize mail tags
Summary:
Ref T5861. Currently, mail tags are hard-coded; move them into applications. Each Editor defines its own tags.

This has zero impact on the UI or behavior.

Test Plan:
  - Checked/unchecked some options, saved form.
  - Swapped back to `master` and saw exactly the same values.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5861

Differential Revision: https://secure.phabricator.com/D10238
2014-08-12 12:28:41 -07:00
epriestley
d011f8fdc6 Add a setting to disable all notification email
Summary: Ref T5861. Adds an option to opt out of all notification email. We'll still send you password resets, email verifications, etc.

Test Plan:
{F189484}

  - Added unit tests.
  - With preference set to different things, tried to send myself mail. Mail respected preferences.
  - Sent password reset email, which got through the preference.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: rush898, epriestley

Maniphest Tasks: T5861

Differential Revision: https://secure.phabricator.com/D10237
2014-08-12 12:28:29 -07:00
epriestley
0196f53f9d Separate email formatting options into a new panel
Summary:
Ref T5861. These two options are complex, rarely useful, and not directly related to controlling what mail you receive.

Move them to a separate panel to make way for more stuff on the preferences panel. We'll probably add an "HTML" option to this new panel eventually, too.

Test Plan:
{F189474}

  - Used both panels.
  - Tested with multiplexing off.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5861

Differential Revision: https://secure.phabricator.com/D10236
2014-08-12 12:28:17 -07:00
epriestley
500506bfef Persist excluded recipients when saving mail
Summary:
Fixes T5185. The fundamental issue is that this `excludePHIDs` property was not saved, so the logic went like this:

  - Generate `excludePHIDs` correctly.
  - Pass `excludePHIDs` through the stack.
  - Perform some other computations correctly.
  - Queue the mail for the daemons, throwing it away. {icon bomb}
  - Daemons process mail with empty `excludePHIDs` list.

Store it in the persistent properties array instead.

Also remove the "override self mail" thing, since it's only used by `bin/mail send-test` and suffers from the same issue. I think it's too useless to fix, since even if you get caught by it, `bin/mail` makes it clear why the message was dropped.

Test Plan:
Notable:

  - `exclude` present in properties
  - Exclusion reason under RECIPIENTS header

{P1229}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5185

Differential Revision: https://secure.phabricator.com/D10234
2014-08-12 12:28:07 -07:00
epriestley
c9835c4492 Publish stories about objects in projects as related to projects
Summary:
Fixes T5456. We lost this logic in the transition to applicationtransactions.

When publishing a feed story, mark all of the object's projects as related, so the project filter in feed works.

Test Plan: Made a comment on a task associated with a project, saw the story in filtered feed.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: timor, epriestley

Maniphest Tasks: T5456

Differential Revision: https://secure.phabricator.com/D10233
2014-08-12 12:27:24 -07:00
epriestley
e4049e8797 Fix verbose email addresses being passed to mail adapters
Summary:
Fixes T5233.

  - The mail adapter API currently expects plain addresses (like `a@b.com`) in `addTos()`, and some adapters can not accept fancy verbose addresses (like `"name" <a@b.com>`).
  - When we try to send error email, we pass the entire "From" header into the API. This is incorrect.
  - Since it would be nice to make this just work in the future, fix it inside the API.
  - Specifically, this is reached with: send email -> generates error -> we try to send you an email back -> we send it to your "From" -> some mailers choke on the fancy name if you have one.

Test Plan: Processed an errorneous email with a fancy "From", got a response error.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5233

Differential Revision: https://secure.phabricator.com/D10232
2014-08-12 12:27:13 -07:00
epriestley
dedcfd0c91 Fix broken handle rendering in Ponder
Summary: Ref T5817. This just fixes the markup in emails, the overall behavior still isn't great. I don't want to spend to much time on Ponder until it ends up somewhere nearer the top of the priority queue.

Test Plan: Viewed feed stories and emails, no stray/clearly-broken HTML.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5817

Differential Revision: https://secure.phabricator.com/D10231
2014-08-12 12:25:58 -07:00
epriestley
a3a72c1c7d Use transactions properly when building tasks from email
Summary: Fixes T5859. This doesn't change much, but makes the transaction record a little more accurate and activates stuff like `#hashtags` and `{F123}` causing policy associations.

Test Plan: Used `bin/mail receive-test` and mail receiver script to send bug mail, saw hashtags imply projects.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5859

Differential Revision: https://secure.phabricator.com/D10229
2014-08-12 12:25:47 -07:00
epriestley
ec9eaabfbd Allow repo updates to recover after force push + garbage collection
Summary:
Fixes T5839. If a repository has been force pushed and garbage collected, we might have a ref cursor in the database which still points at the old commit (which no longer exists).

We'll then run a command like `git log <new hash> --not <old hash>` to figure out which commits are newly pushed, and this will bomb out because `<old hash>` is invalid.

Instead, validate all the `<old hash>` values before we try to make use of them.

Test Plan:
  - Forced a repository into a bad state by mucking with the datbase, generating a reproducible failure similar to the one in T5839.
  - Applied patch.
  - `bin/repository update <callsign> --trace` filtered the bad commit and put the repository into the right state.
  - Saw new commits recognized correctly.
  - Ran `bin/repository update <callsign>` for a Mercurial and SVN repo as a sanity check.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5839

Differential Revision: https://secure.phabricator.com/D10226
2014-08-12 12:25:24 -07:00
epriestley
394250397e Improve "unblock task" feed stories
Summary:
Fixes T5184. Fixes T5008. Three issues with stories/notifications about changing the status of tasks which block other tasks:

**Bad Feed Stories**

  - Problem: Feed story rendering was confusing (T5184).
  - Solution: fix it to provide context.

**Too Many Feed Stories**

  - Problem: Feed gets a story for the original task's close ("a closed x"), and a story for each blocked task ("a closed x, a task blocking y").
  - "Solution": Punt. These are redundant in the full feed but not in filtered feeds. Right solution is display-time aggregation. No users have really complained about this.

**Too Many Notifications**

  - Problem: Users subscribed to both tasks get notified about the clsoe, and also about the unblocked task. These notifications are redundant.
  - "Solution": Punt. This is easy to fix by silencing notifications for the sub-editor, but I'm worried it would be confusing. Users haven't complained. Display-time aggregation might be a better fix.

Test Plan: {F189463}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5008, T5184

Differential Revision: https://secure.phabricator.com/D10235
2014-08-12 09:27:26 -07:00
epriestley
94389fcd9f Allow projects to be filtered by icon and color
Summary: Ref T5819. Implements basic icon and color filtering for projects.

Test Plan: {F189350}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5819

Differential Revision: https://secure.phabricator.com/D10230
2014-08-12 08:04:38 -07:00
epriestley
9309723ac4 Send graceful shutdown signals to daemons in Phabricator
Summary:
Fixes T5855. Adds a `--graceful N` flag to `phd stop` and `phd restart`.

`phd` will send SIGINT, wait `N` seconds, SIGTERM, wait 15 seconds, and SIGKILL. By default, `N` is 15.

Test Plan:
  - Ran `bin/phd debug ...` and used `^C` to interrupt daemons. Saw graceful shutdown behavior, and abrupt termination on multiple `^C`.
  - Ran `bin/phd start`, `bin/phd stop` and `bin/phd restart` with `--graceful` set to various things, notably `0`. Saw graceful shutdowns on the CLI and in the web UI. With `0`, abrupt shutdowns.

Reviewers: btrahan, hach-que

Reviewed By: hach-que

Subscribers: epriestley

Maniphest Tasks: T5855

Differential Revision: https://secure.phabricator.com/D10228
2014-08-11 20:18:31 -07:00
James Rhodes
aab0ed1c50 Implement artifact release for Harbormaster
Summary: Resolves T5836.  This automatically releases artifacts when Harbormaster builds finish (either passing or failing).  This allows Harbormaster to release the Drydock leases it has for hosts.

Test Plan: Tested it with a build plan that passes and fails; tested it with lots of builds running in parallel.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5836

Differential Revision: https://secure.phabricator.com/D10208
2014-08-12 09:15:16 +10:00
James Rhodes
d2111214f2 Allow timeouts to be specified on Drydock SSH connections
Summary: This allows timeouts to be specified on SSH connections that Drydock makes.  Used in the EC2 allocator to poll for the SSH server starting.

Test Plan: Used in EC2 allocator diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10225
2014-08-12 09:14:47 +10:00
James Rhodes
e48aaa563a Allow Drydock blueprints to define and use custom fields
Summary: This allows Drydock blueprints to define custom fields for blueprint settings.

Test Plan: Pulled out of EC2 allocator diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10224
2014-08-12 08:39:00 +10:00
James Rhodes
efc82c727b Measure how long build targets take in Harbormaster
Summary:
Ref T1049.  This keeps track of how long a build target takes to execute in Harbormaster and displays it in the build view page.  I'm not sure whether "Started" is really that useful once the target has completed?

Also, I change the name of the time taken depending on whether or not the target has completed; if it's still in progress it's called "Elapsed" and if it's completed then it's "Duration".  The primary reason for this is that "Duration" sounds like post tense, whereas "Elapsed" is current tense.  I'm not sure whether this is okay or not?

Test Plan: Ran a Sleep build step and saw the target dates / times appear correctly.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: talshiri, epriestley, Korvin

Maniphest Tasks: T5824, T1049

Differential Revision: https://secure.phabricator.com/D10174
2014-08-12 08:34:43 +10:00
Caleb Anderson
60fca1d7f4 Updated PhabricatorManiphestTaskTestDataGenerator to assign projects to the generated tasks.
Summary: To assist with {T5245}, I have added projects back into the lipsum maniphest generator with the edge infrastructure.

Test Plan: Run the lipsum script for PhabricatorManiphestTaskTestDataGenerator and make sure it generates project data.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10202
2014-08-11 13:27:33 -07:00
epriestley
0292793d4d Account for preempting events on the Phrequent list view
Summary: Fixes T5850. Also fixes some logic where the wrong preempting events could be attached during a bulk query.

Test Plan: Phrequent list now shows preemption-aware times.

Reviewers: hach-que, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5850

Differential Revision: https://secure.phabricator.com/D10223
2014-08-11 12:30:48 -07:00
epriestley
fc814647e6 Improve usability of Phrequent "Stop Time" dialog
Summary:
Fixes T5848.

  - Disallow tracking negative time.
  - Preserve note if there's an error with the time selection.
  - Show start time and duration.
  - Slightly better error messages.

Test Plan: Started and stopped time. Tried to select future/negative ranges.

Reviewers: hach-que, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5848

Differential Revision: https://secure.phabricator.com/D10218
2014-08-11 12:30:01 -07:00
epriestley
31e1653a4e Convert pushlog and refcursor to BLOB storage
Summary:
Fixes T5840. Some time ago I incorrectly believed that `latin1_bin` collation was synonymous with "binary". It is not, and does not permit UTF8 characters outside of BMP, among other sequences.

These two tables currently have `LONGTEXT` columns which should be `LONGBLOB`. The table design is explicilty intended to accommodate invalid/unreasonably long ref names, but the collation prevents this from working properly.

After T1191, we'll have a general system for resolving this, but a user hit an issue yesterday (T5840) with a brnach name containing Chinese characters.

Test Plan:
  - Tried emoji inserts into both tables, was rebuffed.
  - Ran migration.
  - Performed emoji inserts into both tables.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5840

Differential Revision: https://secure.phabricator.com/D10217
2014-08-11 12:29:46 -07:00
epriestley
98cd2cd799 Move object monogram rules later in the parse order
Summary:
Fixes T5837. The problem is that the hash is being recognized as a commit hash. We currently fire the object monogram rules fairly early, but there's no real reason to do this. Move them after all of the hyperlink rules:

    0 PhutilRemarkupEscapeRemarkupRule
    100 PhutilRemarkupMonospaceRule
    150 PhutilRemarkupDocumentLinkRule
    175 PhrictionRemarkupRule

  <<< OLD OBJECT RULE POSITION

    200 PhabricatorIconRemarkupRule
    200 PhabricatorMemeRemarkupRule
    200 DivinerSymbolRemarkupRule
    350 DoorkeeperRemarkupRuleJIRA
    350 PhabricatorYoutubeRemarkupRule
    350 DoorkeeperRemarkupRuleAsana
    400 PhutilRemarkupHyperlinkRule

  >>> NEW OBJECT RULE POSITION

    500 PhabricatorImageMacroRemarkupRule
    500 CustomInlineJIRA5Rule
    500 PhabricatorMentionRemarkupRule
    500 CustomInlineCodeRule
    1000 PhutilRemarkupDelRule
    1000 PhutilRemarkupBoldRule
    1000 PhutilRemarkupItalicRule
    1000 PhutilRemarkupUnderlineRule

  - The disadvantage of this approach is that `{F123, alt=go look at http://lol.com/ omg}` will parse the URL first, and then fail to resolve the object embed. This seems very rare / unusual.
  - The advantage is that all URLs which happen to have monograms in them work.

In the future, we could refine this by separating the rules, so the embed (`{...}`) versions fired at priority 200, while the normal versions fired at priority 450. We can wait for use cases, though. This is a little messy because the same code implements both rules.

Test Plan:
  - Verified example in T5837.
  - Marked up object rules like `F123` (works), `[[ asdf | F123 ]]` (works), `{F123, alt=http://example.com}` (does not work).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5837

Differential Revision: https://secure.phabricator.com/D10212
2014-08-11 12:29:23 -07:00
epriestley
448020a573 Rename Lisk internal getProperties() methods to avoid collisions
Summary:
See some discussion here:

24a6eeb8d8 (commitcomment-7334892)

The `protected $properties;` storage parameter added to `ProjectColumn` is shadowed by `getProperties()` in the base class.

Although this works correctly for me, it's ambiguous and worth fixing. Make the base class methods explicit.

Test Plan: Used `grep` to find callers for both methods and renamed them.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10210
2014-08-11 12:24:41 -07:00
epriestley
2b909fb3a1 Minor, fix one more Actor -> Acting As PHID
Summary: See D10221, I missed one of these callsites.

Auditors: btrahan
2014-08-11 12:20:25 -07:00
epriestley
6232e9676c Don't send reset links to unverified addresses on accounts with verified addresses
Summary:
Via HackerOne. If a user adds an email address and typos it, entering `alinculne@gmailo.com`, and it happens to be a valid address which an evil user controls, the evil user can request a password reset and compromise the account.

This strains the imagination, but we can implement a better behavior cheaply.

  - If an account has any verified addresses, only send to verified addresses.
  - If an account has no verified addresses (e.g., is a new account), send to any address.

We've also received several reports about reset links not being destroyed as aggressively as researchers expect. While there's no specific scenario where this does any harm, revoke all outstanding reset tokens when a reset link is used to improve the signal/noise ratio of the reporting channel.

Test Plan:
  - Tried to send a reset link to an unverified address on an account with a verified address (got new error).
  - Tried to send a reset link to a verified adddress on an account with a verified address (got email).
  - Tried to send a reset link to an invalid address (got old error).
  - Tried to send a reset link to an unverified address on an account with only unverified addresses -- a new user (got email).
  - Requested several reset links, used one, verified all the others were revoked.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10206
2014-08-11 12:13:09 -07:00
epriestley
7513c70e2c Show a very basic purchase history in Phortune
Summary: Ref T2787. This is very basic and just helps me know that the data is inserting correctly.

Test Plan: {F187765}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2787

Differential Revision: https://secure.phabricator.com/D10205
2014-08-11 12:10:09 -07:00
epriestley
d38e89ef6b Fix several issues with application interactions while importing commits
Summary:
  - Fixes T5851. Currently, if a commit has `Fixes T123`, we generate an email with just that before generating the commit email. Don't send/publish transactions about a commit before it imports (this is a tiny bit hacky, but well-contained and I don't think it causes any problems).
  - Fixes T4864. Currently, we try to parse Differential information even if Differential is not installed. Instead, do this only if Differential is installed.
  - Fixes T5771. Currently, if we can't figure out who the committer/author of a commit is, we don't publish a `Fixes T123` transaction. Instead, fall back to acting as "Diffusion" if we can't find a better actor. Most of this diff expands the role of application actors. The existing application actors (Herald and Harbormaster) seem to be working well.

Test Plan:
  - Pushed a commit with `Fixes T123` and verified it did not generate email directly. (The task half of the transaction still does, correctly.)
  - Uninstalled Differential and pushed a commit, got a clean import instead of an exception.
  - Commented out author/committer PHIDs and pushed stuff, saw a "Diffusion" actor.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5771, T4864, T5851

Differential Revision: https://secure.phabricator.com/D10221
2014-08-11 12:08:24 -07:00
epriestley
d09d7ffe1f Fix string construction in Conduit exceptions
Summary:
Fixes T5838.

  - We currently try to use a `ConduitAPIMethod` object as a string.
  - We then pass that string to the parent's `__construct()` method as `$message`.

Test Plan: Uninstalled Maniphest, then tried to execute `maniphest.createtask`. Got a useful exception message instead of an error during message construction.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5838

Differential Revision: https://secure.phabricator.com/D10211
2014-08-11 12:08:06 -07:00
epriestley
aa67a5ffc8 Make payment method management somewhat more reasonable in Phortune
Summary: Ref T2787. Shows somewhat-useful information, allows payment methods to be disabled and renamed.

Test Plan: Created, renamed, disabled payment methods.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2787

Differential Revision: https://secure.phabricator.com/D10203
2014-08-11 12:07:35 -07:00
epriestley
eb9dcd6fba Consolidate handling of special properties for newly uploaded files
Summary:
Fixes T5849. When a new file is created, we might have to actually write the data to a storage engine, or we might be able to just point at data which is already there.

Currently, these two paths handle `$params` with different code and mild behavioral differences. Instead, have them call the same code so they get the same behavior.

Test Plan:
  - Uploaded the same file multiple times to home page.
  - Uploaded the same file multiple times as profile picture.
  - Generated files via Diffusion.
  - All the files got the expected properties, whether they were reusing data or not.

Reviewers: btrahan, 20after4

Reviewed By: 20after4

Subscribers: epriestley

Maniphest Tasks: T5849

Differential Revision: https://secure.phabricator.com/D10216
2014-08-11 09:39:40 -07:00
epriestley
a9f2c07345 Generate a 403 page with a nice dialog when a file token is invalid
Summary:
Ref T5685. Currently we just 403 on an invalid token, but we can be a little more helpful.

The issues here are:

  - If we **do** redirect you on this page and something goes wrong, you might get stuck in a redirect loop.
  - If we **don't** redirect you, copy/pasting the link to someone (or reloading the page) gives them a pretty confusing result, since the link doesn't work any more. Prior to this diff, they get a 403.

To mitigate this, do a little better than a bare 403: give them a link to auth and generate a new URI for the file.

If this is still confusing, the next best thing I can come up with is something like this:

  - Put some modulous of the timestamp in the URI.
  - If the current time is within 2 seconds of the generation time, show this dialog.
  - Otherwise, redirect.

That seems like it would be okay, but I worry that "2" has to be small (so links you copy/paste -> chat -> click still work) and a small value means that a small amount of clock skew breaks things. We could use the database clock, but ehhh.

Other ideas:

  - Put a hash of the remote IP in the URI, redirect if it doesn't match. Fails for companies behind a NAT gateway but should work in a lot of other cases.
  - Just redirect always, there's no reason it should ever loop and browsers don't really do anything bad when there's a loop (they'll show an error after too many redirects).

I'm leaning toward letting this stabilize in the wild for a bit, then trying "always redirect".

Test Plan: {F188914}

Reviewers: btrahan, 20after4

Reviewed By: 20after4

Subscribers: epriestley

Maniphest Tasks: T5685

Differential Revision: https://secure.phabricator.com/D10215
2014-08-11 09:39:25 -07:00
epriestley
5a630f84de Show file cacheability in Files application
Summary: Ref T5685. We've added a new `canCDN` flag to control whether or not files can be cached and delivered over a CDN. Show this flag in the UI.

Test Plan: Viewed several files, saw correct/expected UI values.

Reviewers: btrahan, 20after4

Reviewed By: 20after4

Subscribers: epriestley

Maniphest Tasks: T5685

Differential Revision: https://secure.phabricator.com/D10213
2014-08-11 09:39:06 -07:00
Chad Little
b97ccd88ec Add Phund app icon
Summary: Super important. This single icon unlocks untold riches and tens of corporate dollars.

Test Plan: Photoshop

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10219
2014-08-11 09:26:41 -07:00
Mukunda Modell
25ae4c458d Protect file data with a one-time-token
Test Plan: currently untested work in progress

Reviewers: #blessed_reviewers, epriestley

Subscribers: rush898, aklapper, Korvin, epriestley

Projects: #wikimedia

Maniphest Tasks: T5685

Differential Revision: https://secure.phabricator.com/D10054
2014-08-11 07:32:17 -07:00
epriestley
c0919be0ec Fix dashboard list if there are no results
Summary:
We'll fire a bad query if there are no dashboards in the result list, see:

http://pastie.org/private/j0f8tzbdahwragxjsk8qxq

Test Plan: Viewed result list with no dashboards.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10207
2014-08-09 19:08:16 -07:00
epriestley
0e98e33b33 When dragging stuff around, compensate for changes in scroll position
Summary:
Ref T5240. This fixes dragging stuff and using the mousewheel or keyboard to scroll the window during the drag.

(It does not fix "dragging near the edge of the container should scroll it" yet.)

Test Plan: Dragged stuff around on task list and workboards in Safari, Firefox and Chrome. Used mousewheel and shift + mousewheel to scroll the document and containers during drag. Object remained under the cursor.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5240

Differential Revision: https://secure.phabricator.com/D10186
2014-08-09 08:49:01 -07:00
epriestley
c0dc5ca898 Clean up remaining default column logic for boards
Summary: See D10189. We should never hit this anymore, so clean it up.

Test Plan:
  - Reloaded a board, saw everything stay where it was before the change.
  - Added a new task to the project, saw it show up in backlog.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D10200
2014-08-08 16:22:11 -07:00
epriestley
664507e450 Use ManiphestTaskQuery instead of ad-hoc load in Maniphest reports
Summary: Fixes T5829. This stuff is old and busted, but keep it working for now.

Test Plan: No more fatal when there are recently closed tasks.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5829

Differential Revision: https://secure.phabricator.com/D10201
2014-08-08 16:21:53 -07:00
epriestley
24a6eeb8d8 Allow the workboard backlog column to be reordered
Summary:
Fixes T5677.

  - Instead of using `sequence == 0` to mean "this is the backlog column", flag the column explicitly.
  - Migrate existing sequence 0 columns to have the flag.
  - Add the flag when initializing or copying a board.
  - Remove special backlog logic when reordering columns.

Test Plan:
  - Migrated columns, viewed some boards, they looked identical.
  - Reordered the backlog column a bunch of times (first, last, middle, dragged other stuff around).
  - Added tasks to a project, saw them show up in the reordered backlog.
  - Initialized a new board and saw a backlog column show up.
  - Copied an existing board and saw the backlog column come over.
  - Tried to hide a backlog column.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5677

Differential Revision: https://secure.phabricator.com/D10189
2014-08-08 15:50:36 -07:00