1. 20 Aug, 2013 1 commit
  2. 19 Aug, 2013 3 commits
    • Bob Trahan's avatar
      make commit message worker savvier around revision ids · e1d2523e
      Bob Trahan authored
      Summary: Fixes T2836
      
      Test Plan: make a diff, get it approved, arc land, verify things okay. ask users on T2836 to try.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      CC: Korvin, aran
      
      Maniphest Tasks: T2836
      
      Differential Revision: https://secure.phabricator.com/D6770
      e1d2523e
    • Bob Trahan's avatar
      Pholio - make create stories include the mock description, if there is one. · 3f0ffaa9
      Bob Trahan authored
      Summary:
      This diff accomplishes this task by adding an arbitrary metadata store to PhabricatorObjectHandle. This seemed like it would be "necessary eventually"; for example if / when we decide we want to show images in these stories we'd need to add some more arbitrary data. A point of debate is this technique will yield the _current_ data and not the data at the time the transaction was originally made. I can see this being both desirable and non-desirable.
      
      Otherwise, the best way to do this is to make a new transaction type specifically for create and store exactly what data we think we would need.
      
      (and there's probably many other ways but they require much more work...)
      
      Test Plan: viewed some pholio create stories and yes, they had the description showing.
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      CC: Korvin, aran
      
      Maniphest Tasks: T3685
      
      Differential Revision: https://secure.phabricator.com/D6767
      3f0ffaa9
    • epriestley's avatar
      Add followers to Asana diff tasks silently · e8c2b2c3
      epriestley authored
      Summary: Ref T2852. Asana is launching some kind of silent follow thing today; I don't know what the API is but it's probably something like this. I'll update this to actually make the right call once the call exists, this is mostly just a placeholder so I don't forget about it.
      
      Test Plan: None yet, this API isn't documented or live and doesn't work yet so it can't be tested.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran, moskov
      
      Maniphest Tasks: T2852
      
      Differential Revision: https://secure.phabricator.com/D6740
      e8c2b2c3
  3. 17 Aug, 2013 2 commits
    • epriestley's avatar
      Modernize Releeph branch edit and create interfaces · bd17fac9
      epriestley authored
      Summary: Ref T3092. Fixes T3724. Use modern/flexible UI for these interfaces. Removes the ability to retarget an existing branch (you can just close it and open a new one if you made a mistake).
      
      Test Plan: {F54437} {F54438}
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Maniphest Tasks: T3092, T3724
      
      Differential Revision: https://secure.phabricator.com/D6765
      bd17fac9
    • epriestley's avatar
      Use ApplicationSearch for Releeph branch lists · 210e30c2
      epriestley authored
      Summary:
      Releeph branch lists in project views have a bunch of custom UI right now; give them more standard UI and ApplicationSearch.
      
      This drops a small piece of functionality: we now show only a total open request count instead of a detailed enumeration of each request status. I assume this is reasonable (that is, the important piece is "is there something to do on this branch?"), but we can muck with it if the more detailed status is important.
      
      Test Plan: {F54344}
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: LegNeato, aran
      
      Maniphest Tasks: T3656
      
      Differential Revision: https://secure.phabricator.com/D6764
      210e30c2
  4. 15 Aug, 2013 4 commits
    • Bob Trahan's avatar
      Integrate Pholio with Herald · f909a295
      Bob Trahan authored
      Summary: Ref T2766. Does the integration via ApplicationTransactionsEditor. Only did addCC and Flag for proof of concept.
      
      Test Plan: Made a rule to cc, made a rule to flag. They worked!  (will attach screens to diff)
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      CC: Korvin, aran
      
      Maniphest Tasks: T2766
      
      Differential Revision: https://secure.phabricator.com/D6766
      f909a295
    • epriestley's avatar
      Add src/extensions/ to .gitignore · fde0d1f1
      epriestley authored
      Summary: Oops -- forgot to do this in D6759.
      
      Test Plan: `git status`
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Differential Revision: https://secure.phabricator.com/D6760
      fde0d1f1
    • epriestley's avatar
      Log and continue when trying to register an invalid event listener · 13d18376
      epriestley authored
      Summary: We currently die if an event listener throws when registering (e.g., because it is misconfigured), but this prevents you from running `bin/config` to fix it, which is a bit silly.
      
      Test Plan: Created this revision with an invalid listener in config.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Differential Revision: https://secure.phabricator.com/D6761
      13d18376
    • Bob Trahan's avatar
      Conpherence - fix conpherence create · 3d8eb641
      Bob Trahan authored
      Summary: somewhere along the line this broke. Before this patch we fail the visibility check since its based on Conpherence Participants which don't get created and attached until applyExternalEffects. Believe it or not, this was the least gross fix I could come up with; since the permission check is done SO early most other ideas I had involved creating a dummy participant object to pass the check then handling things for real later on...  Ref T3723.
      
      Test Plan: created a conpherence with myself - great success
      
      Reviewers: epriestley
      
      Reviewed By: epriestley
      
      CC: chad, Korvin, aran
      
      Maniphest Tasks: T3723
      
      Differential Revision: https://secure.phabricator.com/D6762
      3d8eb641
  5. 14 Aug, 2013 15 commits
    • epriestley's avatar
      Use ApplicationSearch in ReleephBranchView · 23e68ee8
      epriestley authored
      Summary:
      Ref T3721. Releeph currently attempts to implement a flexible, field-driven search for branches, but it's building all of its own infrastructure and it ends up heading down some weird paths. In particular, it loads **every** request and then makes calls into fields to filter them. It also tries to be very very general, which isn't really necessary (for example, I think it's reasonable for us to assume that we won't let you disable the "requestor" field).
      
      ApplicationSearch and CustomField provide more scalable approaches to this problem; move search on top of them. The query still ends up doing some filtering in-process, but it's now far more limited in scope and can be denormalized later.
      
      Test Plan: {F54304}
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: chad, aran
      
      Maniphest Tasks: T3721
      
      Differential Revision: https://secure.phabricator.com/D6758
      23e68ee8
    • epriestley's avatar
      Add `src/extensions/` to Phabricator · 42a81554
      epriestley authored
      Summary: I added this easier-extension mechanism a while ago but only added the actual directories to libphutil and arcanist. This one should work, it just wasn't checked into the repository.
      
      Test Plan: Added a file with an exception in it to the directory, verified the exception was thrown at runtime.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Differential Revision: https://secure.phabricator.com/D6759
      42a81554
    • epriestley's avatar
      Use PhabricatorCustomField infrastructure to render Releeph custom fields on the edit screen · 7821c980
      epriestley authored
      Summary:
      Ref T3718. This moves custom field rendering on the edit screen to PhabricatorCustomField and makes all the APIs conformant.
      
      We still run through edit with both old-school and new-school sets of fields, because the actual editing isn't on the new stuff yet. That will happen in a diff or two.
      
      Test Plan: Edited a request; intentionally introduced errors and verified the form behaved as expected.
      
      Reviewers: btrahan, testuser1122344
      
      Reviewed By: btrahan
      
      CC: aran
      
      Maniphest Tasks: T3718
      
      Differential Revision: https://secure.phabricator.com/D6756
      7821c980
    • Chad Little's avatar
      Add hovercard on/off option to PhabricatorFeedStory · d02eb46a
      Chad Little authored
      Summary: Defaults hovercards off everywhere feed stories are shown. I tried to find where to put this in so /feed/ could display them, but got horribly lost and confused in SearchQueryLandView
      
      Test Plan: turn hovercards on and off, inspect elements.
      
      Reviewers: epriestley, btrahan
      
      Reviewed By: epriestley
      
      CC: Korvin, aran
      
      Differential Revision: https://secure.phabricator.com/D6757
      d02eb46a
    • epriestley's avatar
      Delete "Risk" field specification from Releeph · e3f3017b
      epriestley authored
      Summary: Ref T3718. This is not used and does not seem particularly useful.
      
      Test Plan: Grep.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: LegNeato, aran
      
      Maniphest Tasks: T3718
      
      Differential Revision: https://secure.phabricator.com/D6755
      e3f3017b
    • epriestley's avatar
      Partially fix a policy issue with ApplicationTransactions · 8c3d1af6
      epriestley authored
      Summary: Currently, we check that the user can view and edit their own transaction, which is always true. Instead, check that they can view the object. I'll fix this with a more tailored check against the EDIT capability that's per-transaction later.
      
      Test Plan: Applying no transactions no longer fatals with undefined `$xaction`.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Differential Revision: https://secure.phabricator.com/D6754
      8c3d1af6
    • epriestley's avatar
      Make ReleephRequest implement PhabricatorCustomFieldInterface · f7b289e3
      epriestley authored
      Summary: Ref T3718. Doesn't do anything yet.
      
      Test Plan: Viewed and edited a request.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Maniphest Tasks: T3718
      
      Differential Revision: https://secure.phabricator.com/D6753
      f7b289e3
    • epriestley's avatar
      Further simplify PhabricatorCustomFieldInterface · 026137f9
      epriestley authored
      Summary:
      Ref T1703. Ref T3718. This introduces `PhabricatorCustomFieldAttachment`, which is just a fancy `array()`. The goal here is to simplify `PhabricatorCustomFieldInterface` as much as possible.
      
      In particular, it can now use common infrastructure (`assertAttached()`) and is more difficult to get wrong.
      
      Test Plan: Edited custom fields on profile.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Maniphest Tasks: T1703, T3718
      
      Differential Revision: https://secure.phabricator.com/D6752
      026137f9
    • epriestley's avatar
      Simplify and improve PhabricatorCustomField APIs · 938b63aa
      epriestley authored
      Summary:
      Ref T1703. Ref T3718. The `PhabricatorCustomFieldList` seems like a pretty good idea. Move more code into it to make it harder to get wrong.
      
      Also the sequencing on old/new values for these transactions was a bit off; fix that up.
      
      Test Plan: Edited standard and custom profile fields.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Maniphest Tasks: T1703, T3718
      
      Differential Revision: https://secure.phabricator.com/D6751
      938b63aa
    • epriestley's avatar
      Partially move Releeph custom fields to PhabricatorCustomField · 74de2490
      epriestley authored
      Summary:
      Fixes T3661. Ref T3718. This makes Releeph custom fields extend PhabricatorCustomField so we can start moving over other pieces of infrastructure (rendering, storage, etc) to run through the same pathways. It's roughly the minimum amount of work required to be able to move forward.
      
      NOTE: This removes per-project custom field selectors. Fields are now configured for an entire install. My understanding is that Facebook does not use this feature, and modern field infrastructure has moved away from selectors.
      
      Test Plan: Viewed and edited projects, branches, and requests in Releeph. Grepped for removed config. Grepped for `field_selector`.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: LegNeato, aran
      
      Maniphest Tasks: T3661, T3718
      
      Differential Revision: https://secure.phabricator.com/D6750
      74de2490
    • epriestley's avatar
      Support configuration-driven custom fields · ca0115b3
      epriestley authored
      Summary:
      Ref T1702. Ref T3718. There are a couple of things going on here:
      
      **PhabricatorCustomFieldList**: I added `PhabricatorCustomFieldList`, which is just a convenience class for dealing with lists of fields. Often, current field code does something like this inline in a Controller:
      
        foreach ($fields as $field) {
          // do some junk
        }
      
      Often, that junk has some slightly subtle implications. Move all of it to `$list->doSomeJunk()` methods (like `appendFieldsToForm()`, `loadFieldsFromStorage()`) to reduce code duplication and prevent errors. This additionally moves an existing list-convenience method there, out of `PhabricatorPropertyListView`.
      
      **PhabricatorUserConfiguredCustomFieldStorage**: Adds `PhabricatorUserConfiguredCustomFieldStorage` for storing custom field data (like "ICQ Handle", "Phone Number", "Desk", "Favorite Flower", etc).
      
      **Configuration-Driven Custom Fields**: Previously, I was thinking about doing these with interfaces, but as I thought about it more I started to dislike that approach. Instead, I built proxies into `PhabricatorCustomField`. Basically, this means that fields (like a custom, configuration-driven "Favorite Flower" field) can just use some other Field to actually provide their implementation (like a "standard" field which knows how to render text areas). The previous approach would have involed subclasssing the "standard" field and implementing an interface, but that would mean that every application would have at least two "base" fields and generally just seemed bleh as I worked through it.
      
      The cost of this approach is that we need a bunch of `proxy` junk in the base class, but that's a one-time cost and I think it simplifies all the implementations and makes them a lot less magical (e.g., all of the custom fields now extend the right base field classes).
      
      **Fixed Some Bugs**: Some of this code hadn't really been run yet and had minor bugs.
      
      Test Plan:
      {F54240}
      {F54241}
      {F54242}
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: aran
      
      Maniphest Tasks: T1702, T1703, T3718
      
      Differential Revision: https://secure.phabricator.com/D6749
      ca0115b3
    • epriestley's avatar
      Remove "cut point commit identifier" from Releeph · 9f410326
      epriestley authored
      Summary: Ref T3656. Releeph denormalizes branch cut point identifiers into Branch objects, but this information isn't useful or used for sorting, filtering, or enforcing unique constraints. Instead, derive it via noramlized pathways from the `cutPointCommitPHID`.
      
      Test Plan: Ran storage upgrade. Ran `releephwork.getbranch` and `releeph.getbranches`. Grepped for `cutPointCommitIdentifier`.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: LegNeato, aran
      
      Maniphest Tasks: T3656
      
      Differential Revision: https://secure.phabricator.com/D6636
      9f410326
    • epriestley's avatar
      Remove "Project ID" from Releeph Projects · a84cc777
      epriestley authored
      Summary:
      Fixes T3660. Releeph Projects currently have an unused one-to-one mapping to Phabricator projects. This isn't consistent with other applications and has no integrations or uses. Get rid of it.
      
      NOTE: Waiting for signoff from @legneato on T3660 before pulling the trigger here.
      
      Test Plan: Created and edited Releeph projects. Grepped for references to project ID; there are a dozen or so but they're all either Releeph projects or Arcanist projects.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: LegNeato, aran
      
      Maniphest Tasks: T3660
      
      Differential Revision: https://secure.phabricator.com/D6635
      a84cc777
    • epriestley's avatar
      Remove ReleephProject->repositoryID writes · 296818e1
      epriestley authored
      Summary: Ref T3655. Depends on D6633. This removes the writes and the column.
      
      Test Plan: Created a project, edited a project. Verified the table doesn't have any keys including this column.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: LegNeato, aran
      
      Maniphest Tasks: T3655
      
      Differential Revision: https://secure.phabricator.com/D6634
      296818e1
    • epriestley's avatar
      Remove all reads of `ReleephProject->repositoryID` · a7955e91
      epriestley authored
      Summary:
      Ref T3655. ReleephProject currently has both `repositoryID` and `repositoryPHID`, which point to the same object and are reudundant. Get rid of all reads of `repositoryID`.
      
      NOTE: This makes project loads depend on repository loads. The eventual rule here will be that you must be able to see a repository in order to see projects for that repository, which seems like a reasonable rule. We might need to tailor it more than this (e.g., if there are branch read permissions down the line) but this seems like a reasonable minimum.
      
      Test Plan: Grepped for `repositoryID` in `releeph/`. Called `releeph.getbranches`.
      
      Reviewers: btrahan
      
      Reviewed By: btrahan
      
      CC: LegNeato, aran
      
      Maniphest Tasks: T3655
      
      Differential Revision: https://secure.phabricator.com/D6633
      a7955e91
  6. 13 Aug, 2013 12 commits
  7. 12 Aug, 2013 3 commits
    • Bob Trahan's avatar
      Pholio - back end for image re-ordering · 65b875d2
      Bob Trahan authored
      Summary:
      companion diff to D6729. This is the back-end stuff, plus calls the JS in D6729 for when images are removed, un-removed, uploaded, or replaced.
      
      Fixes T3640.
      
      Test Plan: messed around with images. hit save - new order! temporarily showed these stories and got text about re-ordering stuff.
      
      Reviewers: epriestley, chad
      
      Reviewed By: epriestley
      
      CC: Korvin, aran
      
      Maniphest Tasks: T3640
      
      Differential Revision: https://secure.phabricator.com/D6731
      65b875d2
    • Chad Little's avatar
      Clean up Notification colors a smidge · fe087340
      Chad Little authored
      Summary: Picked better colors and hover states.
      
      Test Plan: test new colors, stare intently.
      
      Reviewers: epriestley, btrahan
      
      Reviewed By: epriestley
      
      CC: Korvin, aran
      
      Differential Revision: https://secure.phabricator.com/D6730
      fe087340
    • epriestley's avatar
      Allow Pholio mock images to be drag-reordered · a167d746
      epriestley authored
      Summary:
      Ref T3640. JS part only, should give you a list in `imageOrder` on the server that you can read with `$request->getStrList('imageOrder')`.
      
      NOTE: You can't drag images into the first position; this is an existing thing that I just need to fix with DraggableList.
      
      @chad might have some design feedback.
      
      Test Plan: Dragged images around, things seemed to work?
      
      Reviewers: btrahan, chad
      
      Reviewed By: btrahan
      
      CC: aran
      
      Maniphest Tasks: T3640
      
      Differential Revision: https://secure.phabricator.com/D6729
      a167d746