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

15011 commits

Author SHA1 Message Date
epriestley
5ed90b2235 Only validate form subtype edits if subtype transactions are present
Summary: Fixes T12347. Ref T12314. Validation gets called no matter what, but is only relevant if the form supports subtypes.

Test Plan: Marked/unmarked a Paste form as editable.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12347, T12314

Differential Revision: https://secure.phabricator.com/D17457
2017-03-03 13:44:32 -08:00
Chad Little
d2a420d13a Remove needRecipients and needAwards from Badges
Summary: Fixes T10798. Separates these two since they don't need to be combined and it allows for more flexibility / scalability.

Test Plan:
- Add Badge
- Edit Badge
- Add myself as Recipient
- Remove myself
- Go to my profile
- Award Badge from there
- Assign myself a badge, try to re-assign it, see validation error.

Also, validation errors on dialog forms are ugly.

{F3495630}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10798, T12270

Differential Revision: https://secure.phabricator.com/D17447
2017-03-03 08:41:58 -08:00
epriestley
c102620a29 Lock files.video-mime-types config option for consistency
Summary:
This is a consistency change to make this option consistent with `audio-mime-types`, `image-mime-types` and `icon-mime-types`, all of which are locked.

(They're locked because SVG is definitely dangerous, and other types might be dangerous or might become dangerous in the future, although I'm not aware of any actual dangers from video types today.)

Test Plan: Viewed `files.video-mime-types` in Config, saw it was locked.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17454
2017-03-03 08:38:02 -08:00
epriestley
0e7a5623e3 Allow task statuses to "lock" them, preventing additional comments and interactions
Summary:
Ref T12335. See that task for discussion. Here are the behavioral changes:

  - Statuses can be flagged with `locked`, which means that tasks in that status are locked to further discussion and interaction.
  - A new "CAN_INTERACT" permission facilitates this. For most objects, "CAN_INTERACT" is just the same as "CAN_VIEW".
  - For tasks, "CAN_INTERACT" is everyone if the status is a normal status, and no one if the status is a locked status.
  - If a user doesn't have "Interact" permission:
    - They can not submit the comment form.
    - The comment form is replaced with text indicating "This thing is locked.".
    - The "Edit" workflow prompts them.

This is a mixture of advisory and hard policy checks but sholuld represent a reasonable starting point.

Test Plan: Created a new "Locked" status, locked a task. Couldn't comment, saw lock warning, saw lock prompt on edit. Unlocked a task.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12335

Differential Revision: https://secure.phabricator.com/D17453
2017-03-02 16:57:10 -08:00
epriestley
0a0ac1302f Prevent users from taking "edit"-like actions via comment forms if they don't have edit permission
Summary:
Ref T12335. Fixes T11207. Edit-like interactions which are not performed via "Edit <object>" are a bit of a grey area, policy-wise.

For example, you can correctly do these things to an object you can't edit:

  - Comment on it.
  - Award tokens.
  - Subscribe or unsubscribe.
  - Subscribe other users by mentioning them.
  - Perform review.
  - Perform audit.
  - (Maybe some other stuff.)

These behaviors are all desirable and correct. But, particularly now that we offer stacked actions, you can do a bunch of other stuff which you shouldn't really be able to, like changing the status and priority of tasks you can't edit, as long as you submit the change via the comment form.

(Before the advent of stacked actions there were fewer things you could do via the comment form, and more of them were very "grey area", especially since "Change Subscribers" was just "Add Subscribers", which you can do via mentions.)

This isn't too much of a problem in practice because we won't //show// you those actions if the edit form you'd end up on doesn't have those fields. So on intalls like ours where we've created simple + advanced flows, users who shouldn't be changing task priorities generally don't see an option to do so, even though they technically could if they mucked with the HTML.

Change this behavior to be more strict: unless an action explicitly says that it doesn't need edit permission (comment, review, audit) don't show it to users who don't have edit permission and don't let them take the action.

Test Plan:
  - As a user who could not edit a task, tried to change status via comment form; received policy exception.
  - As a user who could not edit a task, viewed a comment form: no actions available (just "comment").
  - As a user who could not edit a revision, viewed a revision form: only "review" actions available (accept, resign, etc).
  - Viewed a commit form but these are kind of moot because there's no separate edit permission.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12335, T11207

Differential Revision: https://secure.phabricator.com/D17452
2017-03-02 16:56:57 -08:00
Chad Little
08b18ac5f5 Remove needBadges from PhabricatorUser
Summary: Ref T12270. We don't really need these, timeline does it's own thing, badges is now a profile page, and hovercards have been removed.

Test Plan: Visit timeline, still see badges, visit my profile page, bask in the warmth of fake awards.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17448
2017-03-02 06:30:23 -08:00
Chad Little
664d9fa3ed Touch up Badges emails
Summary: Ref T12270. Adds the name of the badge to the subject, fixes the double description.

Test Plan: Edit lots of badges with and without descriptions, see good emails.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17449
2017-03-02 06:30:04 -08:00
Chad Little
87304e360f Remove dashboard footer
Summary: Doesn't seem popular, will rethink dashboard editing again in the future at some point.

Test Plan: Review a dashboard, edit, install.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17450
2017-03-02 06:29:39 -08:00
epriestley
6f7bb8c91a On workboards, provide all of the supported "create task" forms in the dropdown
Summary:
Ref T12314. Ref T6064. Ref T11580. If an install defines several different task create forms (like "Create Plant" and "Create Animal"), allow any of them to be created directly onto a workboard column.

This is just a general consistency improvement that makes Custom Forms and Workboards work together a bit better. We might do something fancier eventually for T6064 (which wants fewer clicks) and/or T11580 (which wants per-workboard control over forms or defaults).

Test Plan:
  - Created several different types of tasks directly onto a workboard.
  - Faked just one create form, saw the UI unchanged (except that it respects any renaming).

{F3492928}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314, T11580, T6064

Differential Revision: https://secure.phabricator.com/D17446
2017-03-02 04:24:40 -08:00
epriestley
7eab75410a When editing a subtyped object, use edit forms of the same subtype
Summary:
Ref T12314. When we pick an "Edit" form for a subtyped object, only consider forms with the same subtype.

For example, editing an "Animal" uses the forms with subtype "animal" which are marked as edit forms.

This also makes "Create Subtask" carry the parent task's type.

Test Plan:
  - Edited an Animal, got an animal edit form.
  - Edited a normal task, got a normal task form.
  - Edited a paste, got the normal workflow.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17445
2017-03-02 04:24:28 -08:00
epriestley
4948a21959 Allow tasks to be searched by subtype
Summary:
Ref T12314. Allow tasks to be queried by subtype using a typeahead.

Open to a better default icon. I'll probably let you configure them later.

Just hide this constraint if there's only one subtype.

Test Plan:
  - Searched for subtypes.
  - Verified that the control hides if there is only one subtype.

{F3492293}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17444
2017-03-02 04:20:38 -08:00
epriestley
4a061b1def When an object which supports subtypes is created, set its subtype to the creating form's subtype
Summary:
Ref T12314. If you set a form to have the "plant" subtype, then create a task with it, save "plant" as the task subtype.

For Conduit, the default subtype is used by default, but a new "subtype" transaction is exposed. You can apply this transaction at create time to create an object of a certain subtype, or at any later time to change the subtype of an object.

This still doesn't do anything particularly useful or interesting.

Test Plan:
  - Created a non-subtyped object (a Paste).
  - Created "task" and "plant" tasks via different forms.
  - Created "default" and "plant" tasks via Conduit.
  - Changed the subtype of a task via Conduit.
  - Tried to set a bad subtype.

{F3492061}

{F3492066}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17443
2017-03-02 04:18:23 -08:00
epriestley
b9d60d2653 Allow EditEngine forms for objects which support subtyping to have a subtype configured
Summary:
Ref T12314. This adds a "Change Form Subtype" workflow to the EditEngine form configuration screen, for forms that edit/create objects which support subtyping (for now, only tasks).

For example, this allows you to switch a form from being a "task" form to a "plant" or "animal" form.

Doing this doesn't yet do anything useful or interesting. I'm also not showing it in the UI yet since I'm not sure what we should make that look like (presumably, we should just echo whatever UI we end up with on tasks).

Test Plan:
  - Changed the subtype of a task form.
  - Verified that the "Change Subtype" action doesn't appear on other forms (for example, those for Pastes).

{F3491374}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17442
2017-03-02 04:18:06 -08:00
epriestley
dc7ecf5875 Add "subtype" storage to Maniphest tasks
Summary: Ref T12314. Provides a field on tasks for storing subtypes. Does nothing interesting yet.

Test Plan:
  - Ran storage upgrade.
  - Created some tasks.
  - Looked in the database.
  - Used Conduit to query some tasks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17441
2017-03-02 04:17:47 -08:00
epriestley
1b96f2fc28 Add maniphest.subtypes for configuring task subtypes
Summary:
Ref T12314. Builds toward letting you define "animal" and "plant" tasks.

This just adds some configuration. I'll probably add some more quality-of-life options (like "icon") later but these are the only bits I'm sure I'll need.

Test Plan:
  - Configured sensible subtypes.
  - Tried to configure bad subtypes: bad key, missing "default", duplicate keys. Got sensible error messages.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17440
2017-03-02 04:16:51 -08:00
epriestley
91ef237290 Add a "subtype" field to EditEngine forms
Summary:
Ref T12314. This adds storage so EditEngine forms can later be marked as edit fields for particular types of objects (like an "animal edit form" vs a "plant edit form").

We'll take you to the right edit form when you click "Edit" by selecting among forms with the same subtype as the task.

This doesn't do anything very interesting on its own.

Test Plan:
  - Ran `bin/storage upgrade`.
  - Verified database got the field with proper values.
  - Created a new form, checked the database.
  - Ran unit tests.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12314

Differential Revision: https://secure.phabricator.com/D17439
2017-03-02 04:16:27 -08:00
Joshua Spence
fcd8c9c240 Update phd launch
Summary: Ref T12298. `phd launch` was missed in D17390 and thus broken by D17389.

Test Plan: Launched a daemon with great success.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T12298

Differential Revision: https://secure.phabricator.com/D17429
2017-03-02 21:37:02 +11:00
Christopher Wetherill
5fad7eb1f9 Get line count before truncating Paste snippets
Summary: Fixes T12338. Resolves an issue where long pastes would be truncated before getting a line count, resulting in an inaccurate line count being returned.

Test Plan: Made a large paste, verified that it displayed the correct number of lines.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T12338

Differential Revision: https://secure.phabricator.com/D17438
2017-03-01 22:30:18 +00:00
Chad Little
3f1ee67972 Add a tooltip option to Link menu items
Summary: Ref T12174. Let's users add a tooltip to LinkProfileMenuItem

Test Plan: Add a tooltip, remove tooltip. Menu appears as expected

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12174

Differential Revision: https://secure.phabricator.com/D17437
2017-03-01 11:16:25 -08:00
Chad Little
bf0a7cbec6 Remove "disabled" look to subprojects/workboard nav items
Summary: Fixes T12330. Minor UI nit, since we use "disabled" to usually mean "no permission". Makes these links always normal looking.

Test Plan: Review a new project in sandbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12330

Differential Revision: https://secure.phabricator.com/D17436
2017-03-01 09:20:48 -08:00
epriestley
dbe3d5e57f Fix a stray piece of remarkup syntax in arc documentation
Summary: This is incorrectly starting an ordered sublist.

Test Plan: (o_O)

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17434
2017-02-28 12:09:30 -08:00
epriestley
216f6be11e (stable) Add "--pool" and "--duration" flags to daemon CLI tools
Summary: Ref T12331. These changes are intended to make it easier to debug T12331 since I'm having difficulty reproducing the issue locally.

Test Plan:
  - Ran `bin/phd debug task --pool 4` and got an autoscaling pool.
  - Ran `bin/worker flood --duration 3` and got some 3-second-long tasks to execute with `bin/worker execute ...`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12331

Differential Revision: https://secure.phabricator.com/D17431
2017-02-28 07:44:27 -08:00
epriestley
90ec21f999 Add "--pool" and "--duration" flags to daemon CLI tools
Summary: Ref T12331. These changes are intended to make it easier to debug T12331 since I'm having difficulty reproducing the issue locally.

Test Plan:
  - Ran `bin/phd debug task --pool 4` and got an autoscaling pool.
  - Ran `bin/worker flood --duration 3` and got some 3-second-long tasks to execute with `bin/worker execute ...`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12331

Differential Revision: https://secure.phabricator.com/D17431
2017-02-28 07:43:46 -08:00
Chad Little
54059b0a9d Add fulltext search results panel back for dashboards
Summary: Ref T12324. Adds back this query for search results in dashboards.

Test Plan: Use panel in Dashboard.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12324

Differential Revision: https://secure.phabricator.com/D17428
2017-02-27 12:45:17 -08:00
epriestley
a9cd146745 Filter archived packages out of the "controlling packages" query earlier
Summary:
Ref T12319. Currently, we end up filtering archived packages out once for each path. This shows up on a profile from an install as meaningfully expensive:

https://secure.phabricator.com/xhprof/profile/PHID-FILE-7kmpevyr22aih4s2vyln/?symbol=PhabricatorOwnersPackage::isArchived

Instead, filter them out before we do any work.

Test Plan:
Viewed a revision, still saw packages.

{F3425553}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12319

Differential Revision: https://secure.phabricator.com/D17427
2017-02-27 12:37:08 -08:00
Chad Little
05377bea19 Add an avatar builtin file generator
Summary: Ref T10319. This builds out a reasonably decent avatar generator. 256 colors x 74 images x 2 borders, 38k options. Not completely sure though how names disburse though, so likely half that number. I can add lowercase lettering to double the footprint if needed though.

Test Plan:
UIExamples. Color generator here: http://tools.medialab.sciences-po.fr/iwanthue/

{F3416622}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10319

Differential Revision: https://secure.phabricator.com/D17418
2017-02-27 11:09:30 -08:00
epriestley
6c21646b5f Put revisions waiting on other reviewers in their own bucket
Summary: Fixes T12323. See that task for discussion.

Test Plan: {F3424441}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12323

Differential Revision: https://secure.phabricator.com/D17425
2017-02-27 10:47:15 -08:00
epriestley
c5fa7421c2 Allow commits to be queried by repository using the tagged(...) typehaead function
Summary:
Fixes T12322. Allows you to search for commits using the `tagged(...)` repository function, so you can find "any commmit in any repository tagged with android" or similar.

I moved the function from Differential (which was the application using it) to Diffusion (which is more accurately the application which provides it).

I fixed a bug where searching for `tagged(xyz)` would have no effect (constraint was ignored) if there were no repositories tagged with "xyz". The fix isn't perfectly clean, but should work properly for the moment.

Test Plan:
  - Searched with `tagged(...)` in Diffusion and Differential.
  - Searched by repository.
  - Searched with `tagged(...)` for a project with no tagged repositories.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12322

Differential Revision: https://secure.phabricator.com/D17426
2017-02-27 10:46:55 -08:00
epriestley
3bea3fbb12 When computing revision ownership, cache some intermediate results for performance
Summary:
Ref T12319. With large datasets, the computation of which packages own paths in a revision is needlessly slow.

Improve performance through caching:

  - Cache which paths belong to each repository.
  - Cache the split fragments of each path.
  - Cache the path fragment counts.
  - Micro-optimize accessing `$this->path`.

Test Plan:
  - Used `bin/lipsum` to generate 4,000 packages with 150,000 paths.
  - Created a revision affecting 100 paths in `phabricator/` (these paths mostly overlap with `bin/lipsum` path rules, since Lipsum uses Phabricator-like rules to generate paths).
  - Before optimizations, this revision spent about 5.5 seconds computing paths.
  - After optimizations, it spends about 275ms.

{F3423414}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12319

Differential Revision: https://secure.phabricator.com/D17424
2017-02-27 09:11:57 -08:00
epriestley
b9568646ac Add an owners package generator for Lipsum
Summary: Ref T12319. Allow `bin/lipsum generate` to generate owners packages.

Test Plan: Generated ~4,000 packages with ~150,000 paths.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12319

Differential Revision: https://secure.phabricator.com/D17423
2017-02-27 09:11:04 -08:00
epriestley
5cb4c76bef Add a lipsum generator for Badges
Summary: Ref T12319. Ref T12270. Allow badges to be generated with `bin/lipsum`. These aren't hugely sophisticated but I'm not sure about the fate of T9010 yet or what's happening with the quality levels, and didn't want to make those changes more difficult.

Test Plan:
  - Used `bin/lipsum generate badges --force --quickly` to generate badges.
  - Made some coffee and came back to 20K badges.

{F3422200}

Reviewers: chad

Reviewed By: chad

Subscribers: cspeckmim

Maniphest Tasks: T12319, T12270

Differential Revision: https://secure.phabricator.com/D17422
2017-02-27 09:10:05 -08:00
epriestley
3b8ccb0b78 Add "--force" and "--quickly" flags to bin/lipsum
Summary:
Ref T12319.

  - Lipsum can trash an install by creating a lot of junk that's hard to get rid of, so we're cautious about letting you run it. Add a `--force` flag if you're sure you know what you're doing. This makes the edit/test cycle a bit easier when actually writing Lipsum generators.
  - Lipsum normally sleeps for a second before creating objects, to give users more control over how much stuff they create and limit the amount of damage caused by mistakes. Sometimes, you want to generate a LOT of stuff because you want to reproduce a performance/scale issue (like T12319). Add a `--quickly` flag to generate objects as fast as possible.
  - When loading random users (used as authors, assignees, etc), also load user settings so we can `ConduitCall` with them.
  - Allow generators to return a PHID instead of an actual object (more convenient for Conduit-based generators).

Test Plan:
  - With next change, ran `lipsum generate badges --force --quickly`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12319

Differential Revision: https://secure.phabricator.com/D17421
2017-02-27 09:09:41 -08:00
epriestley
99bcf5f112 Make bin/lipsum generate hanldle generator keys and arguments more clearly
Summary:
Ref T12319. Currently, `bin/lipsum` uses substring matches against human-readable text to chose which objects to generate.

Instead:

  - Use separate selector keys which are guaranteed to be unique.
  - When a match is exact, select only that generator.
  - When a match is ambiguous, fail and warn the user.

Test Plan: Generated several types of objects, tried to generate ambiguous objects like "e".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12319

Differential Revision: https://secure.phabricator.com/D17420
2017-02-27 09:09:28 -08:00
epriestley
1b2c047ce0 Correct spelling of "phabrictor" in Lipsum and elsewhere
Summary: Ref T12319. The product name is misspelled in some methods, and a few places in the documentation.

Test Plan: `grep`

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12319

Differential Revision: https://secure.phabricator.com/D17419
2017-02-27 09:09:13 -08:00
Chad Little
be785d1e8a Add some pre-built alphanumeric characters
Summary: Ref T10319. Builds out some decent looking letters and numbers.

Test Plan: Review alignment in Photoshop.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10319

Differential Revision: https://secure.phabricator.com/D17417
2017-02-26 11:39:05 -08:00
Chad Little
44b307f28d Add some higher resolution default user images
Summary: Looks nicer on profiles, cards. Added some additional colors.

Test Plan: change my avatar a few times

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: avivey, Korvin

Differential Revision: https://secure.phabricator.com/D17416
2017-02-26 09:56:21 -08:00
Chad Little
59207fcfac Fix italics issue with nux state on homepage
Summary: We moved to having "no data" strings render in italics, but sometimes it doesn't make sense. This renders out the panel a little more expected.

Test Plan: Clean install of Phabricator, read home page activity box.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17415
2017-02-25 15:30:17 -08:00
epriestley
c8f5fb1205 (stable) Promote 2017 Week 8 2017-02-24 18:17:58 -08:00
Chad Little
c15501fc9b Add clearer language and options to Arcanist install guides
Summary: Fixes T12315. Reworks the copy placement a little, adds more links.

Test Plan: Read in sandbox

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12315

Differential Revision: https://secure.phabricator.com/D17413
2017-02-24 16:37:32 -08:00
Chad Little
eec6cd865c Miscellanous badge fixes
Summary: Ref T12270. Add transaction validation for name, alias, award, revoke. Change auto subscribe for authors. Fix some typos.

Test Plan: Add badge, award badge, revoke badge, edit badge.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17412
2017-02-24 15:51:26 -08:00
Chad Little
730d88c414 Re-center timeline badges
Summary: This layout inadvertantly changed, so make timeline badges always center.

Test Plan: Review in sandbox.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17411
2017-02-24 14:04:13 -08:00
Chad Little
80cccebca2 Build a Badges page for Profiles
Summary: Ref T12270. Moves badges into their own page and menu item. Capable of displaying hundreds of useful tokens of appreciation and dedication.

Test Plan:
Test blank state, mobile, awards badges.

{F3284139}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17410
2017-02-24 13:15:42 -08:00
epriestley
4270649abe Increase the size of the Diffusion commit cache
Summary:
Ref T12296. This cache is used to cache Git ref heads (branches, tags, etc). Reasonable repositories may have more than 2048 of these.

When we miss the cache, we need to single-get refs to check them, which is relatively expensive.

Increasing the size of the cache to 65535 should only require about 7.5MB of RAM.

Additionally, fill only as much of the cache as actually fits. The FIFO nature of the cache can get us into trouble otherwise.

If we insert "A, B, C, D" and then lookup A, B, C, D, but the cache has maximum size 3, we get this:

  - Insert A, B, C, D: cache is now "B, C, D".
  - Lookup A: miss, single get, insert, purge, cache is now "C, D, A".
  - Lookup B: miss, singel get, insert, purge, cache is now "D, A, B".

Test Plan:
  - Reduced cache size to 5, observed reasonable behavior on the `array_slice()` locally with `bin/repository update` + `var_dump()`.
  - Used this script to estimate the size of 65535 cache entries as 7.5MB:

```
epriestley@orbital ~ $ cat size.php
<?php

$cache = array();

$mem_start = memory_get_usage();
for ($ii = 0; $ii < 65535; $ii++) {
  $cache[sha1($ii)] = true;
}

echo number_format(memory_get_usage() - $mem_start)." bytes\n";
epriestley@orbital ~ $ php -f size.php
7,602,176 bytes
```

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12296

Differential Revision: https://secure.phabricator.com/D17409
2017-02-24 10:54:19 -08:00
epriestley
40cc403d23 Allow the Trigger daemon to hibernate, reducing processes to 0
Summary:
Ref T12298. The trigger daemon already has routine long-term sleep, and few external events can impact when it should ideally wake up. The relevant events are:

  - Someone creates a new Nuance source (ideally, we should wake up right away and start polling it).
  - Someone creates a Calendar event about 16 minutes in the future (ideally, we should send them a reminder in about a minute).
  - Someone changes GC config to be extremely aggressive (ideally, we should immediately respect the change).

None of these cases are very important. We don't hibernate for more than 3 minutes, so the worst case is that your Nuance source takes 3 minutes to start importing or your Calendar notification comes two minutes too late (13 minutes before the event instead of 15).

This change makes GC sightly more CPU-expensive on average: currently, we do a GC sweep every 4 hours. After this change, we'll end up doing one every 3 minutes, because we lose the fact that we did a sweep recently when the daemon restarts.

We could fix this by keeping track of when the last GC sweep was in the database, instead of in the Daemon process, but the cost of a sweep is normally very small so I don't plan to do this anytime soon.

Test Plan:
  - Ran `bin/phd debug trigger`, saw daemon go through 3-minute hibernate + restart cycles.
  - Ran `bin/phd debug task`, saw daemon run normally.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12298

Differential Revision: https://secure.phabricator.com/D17408
2017-02-24 10:54:05 -08:00
Chad Little
d38ee2d79a Update Phurl for modular transactions
Summary: Ref T6049. This moves Phurl to modular transactions.

Test Plan: Everything works here, add phurl, edit phurl, use phurl. Test various error states. Left a TODO on the validate dupe keys, not sure how to implement that in modular-land.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T6049

Differential Revision: https://secure.phabricator.com/D17405
2017-02-24 08:30:47 -08:00
epriestley
89d1403fe8 Explicitly decline to add commit authors as auditors from Herald
Summary:
Fixes T12304. If you have a Herald rule which tries to add a commit author as an auditor, it fails validation when trying to apply.

Stop trying to apply these transactions, and explicitly tell the user why. Differential already uses a similar ruleset around reviewers, but Audit was using older code.

Test Plan:
  - Wrote a Herald rule to add A, B and C as auditors.
  - Committed as A.
  - After change, saw B and C added with transacript guidance that A was the author.

{F3235660}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12304

Differential Revision: https://secure.phabricator.com/D17404
2017-02-23 15:19:23 -08:00
epriestley
3b6a651b69 Merge multiple Auditors transactions from Herald
Summary:
Fixes T12302. Currently, we aren't merging multiple "AddAuditors" transactions correctly.

This can occur when Herald triggers multiple auditor rules.

Instead, merge them.

Test Plan:
  - Wrote two different Herald rules that add auditors.
  - Pushed a commit which triggered them.
  - After the change, saw all the auditors get added correctly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12302

Differential Revision: https://secure.phabricator.com/D17403
2017-02-23 15:14:58 -08:00
Chad Little
3eae9a368d Modular Transactions for Badges
Summary: Ref T12270. This converts Badges to modular transactions for editing and awarding.

Test Plan: Add Badge, edit badge, award and revoke... Still going to test this some more but feel free to comment on anything obviously wrong?

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12270

Differential Revision: https://secure.phabricator.com/D17402
2017-02-23 14:22:52 -08:00
epriestley
ee2d8ce94b Allow XHProf profiles to be drag-and-dropped to upload them
Summary: Ref T12297. This could be fancier, but should make pulling profiles off `admin.phacility.com` significantly more realistic.

Test Plan: Dragged and dropped some profiles to upload them, then reviewed them via web UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12297

Differential Revision: https://secure.phabricator.com/D17401
2017-02-23 11:16:19 -08:00
epriestley
4254702271 Use ApplicationSearch in XHProf
Summary:
Ref T12297. This slightly modernizes the XHProf UI. Not included here:

  - Some of the code acts like samples have PHIDs, but they currently do not. I plan to add them in the next change.
  - I've intentionally left the actual list untouched for now -- it has some old/buggy code (like `flag-6` is no longer an icon) that I'll fix in a future change.

Test Plan: {F3224264}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12297

Differential Revision: https://secure.phabricator.com/D17400
2017-02-23 11:15:58 -08:00