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

35 commits

Author SHA1 Message Date
epriestley
3386920971 Add Herald support for blocking ref changes
Summary: Ref T4195. Allows users to write Herald rules which block ref changes. For example, you can write a rule like `alincoln can not create branches`, or `no one can push to the branch "frozen"`.

Test Plan:
This covers a lot of ground. I created and pushed a bunch of rules, then looked at transcripts, in general. Here are some bits in detail:

Here's a hook-based reject message:

  >>> orbital ~/repos/POEMS $ git push
  Counting objects: 5, done.
  Delta compression using up to 8 threads.
  Compressing objects: 100% (3/3), done.
  Writing objects: 100% (3/3), 274 bytes, done.
  Total 3 (delta 2), reused 0 (delta 0)
  remote: +---------------------------------------------------------------+
  remote: |      * * * PUSH REJECTED BY EVIL DRAGON BUREAUCRATS * * *     |
  remote: +---------------------------------------------------------------+
  remote:             \
  remote:              \                    ^    /^
  remote:               \                  / \  // \
  remote:                \   |\___/|      /   \//  .\
  remote:                 \  /V  V  \__  /    //  | \ \           *----*
  remote:                   /     /  \/_/    //   |  \  \          \   |
  remote:                   @___@`    \/_   //    |   \   \         \/\ \
  remote:                  0/0/|       \/_ //     |    \    \         \  \
  remote:              0/0/0/0/|        \///      |     \     \       |  |
  remote:           0/0/0/0/0/_|_ /   (  //       |      \     _\     |  /
  remote:        0/0/0/0/0/0/`/,_ _ _/  ) ; -.    |    _ _\.-~       /   /
  remote:                    ,-}        _      *-.|.-~-.           .~    ~
  remote:   \     \__/        `/\      /                 ~-. _ .-~      /
  remote:    \____(Oo)           *.   }            {                   /
  remote:    (    (--)          .----~-.\        \-`                 .~
  remote:    //__\\  \ DENIED!  ///.----..<        \             _ -~
  remote:   //    \\               ///-._ _ _ _ _ _ _{^ - - - - ~
  remote:
  remote:
  remote: This commit was rejected by Herald pre-commit rule H24.
  remote: Rule: No Branches Called Blarp
  remote: Reason: "blarp" is a bad branch name
  remote:
  To ssh://dweller@localhost/diffusion/POEMS/
   ! [remote rejected] blarp -> blarp (pre-receive hook declined)
  error: failed to push some refs to 'ssh://dweller@localhost/diffusion/POEMS/'

Here's a transcript, showing that all the field values populate sensibly:

{F90453}

Here's a rule:

{F90454}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4195

Differential Revision: https://secure.phabricator.com/D7782
2013-12-17 15:23:55 -08:00
James Rhodes
7ffea0463e Use herald to trigger builds of revisions and commits.
Summary:
Depends on D7500.

This seemed like a pretty good idea once I thought of it.  Instead of having some custom triggering logic instead Harbormaster, I figured it best to leverage all of Herald's power so that users can create rules to apply builds to commits and differential revisions.  This gives the added advantage that they can trigger off builds for particular types of revisions and commits, which seems like it could be really useful (e.g. run extra tests against revisions that touch sensitive areas of the code).

Test Plan: Ran the usual daemons + the Harbormaster daemon.  Pushed a commit to the repository and saw both the buildable and build get created when the commit worked picked it up.  Submitted a diff and saw both the buildable and build get created when the Herald rules were evaluated for the diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, hwinkel

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7501
2013-11-08 16:58:39 -08:00
epriestley
19968e31f4 Generalize Herald account sources
Summary: The "user" and "user/project" sources exclude system agents and disabled users, but should not.

Test Plan: Added system agents to Herald rules.

Reviewers: btrahan, bigo

Reviewed By: bigo

CC: aran

Differential Revision: https://secure.phabricator.com/D7319
2013-10-14 19:38:35 -07:00
epriestley
650dc0cc30 Remove the "create rules" Herald capability
Summary:
Ref T603. In thinking about this, I think I went mad with power in creating this capability. I can't imagine any reason to give users access to Herald but not let them create rules.

We can restore this later if some install comes up with a good reason to have it, but in the interest of keeping policies as simple as possible, I think we're better off without it. In particular, if you don't want a group of users creating rules, just lock them out of the application entirely.

The "Manage Global Rules" capability is still around, I think that one's super good.

Test Plan: Edited Herald policies, created a rule.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7268
2013-10-09 13:55:44 -07:00
epriestley
7a97a71e20 Move Herald application capabilities to newer infrastructure
Summary: Ref T603. Use the new hotness.

Test Plan: Edited Herald in Applications, tried to create rules / global rules without capabilities, got reasonable error messages.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7263
2013-10-09 13:44:41 -07:00
epriestley
4c0ec01ce5 Allow Herald rules to add reviewers
Summary:
Ref T1279. Although I think this is a bad idea in general (we once supported it, removed it, and seemed better off for it) users expect it to exist and want it to be available. Give them enough rope to shoot themselves in the foot.

I will probably write some lengthy treatise on how you shouldn't use this rule later.

Implementation is straightforward because Differential previously supported this rule.

This rule can also be used to add project reviewers.

Test Plan: Made some "add reviewers" rules, created revisions, saw reviewers trigger.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1279

Differential Revision: https://secure.phabricator.com/D7235
2013-10-05 14:10:51 -07:00
epriestley
c8127edfe9 Tighten up some policy interactions in Herald
Summary:
Ref T603. Herald is a bit of a policy minefield right now, although I think pretty much everything has straightforward solutions. This change:

  - Introduces "create" and "create global" permisions for Herald.
    - Maybe "create" is sort of redundant since there's no reason to have access to the application if not creating rules, but I think this won't be the case for most applications, so having an explicit "create" permission is more consistent.
  - Add some application policy helper functions.
  - Improve rendering a bit -- I think we probably need to build some `PolicyType` class, similar to `PHIDType`, to really get this right.
  - Don't let users who can't use application X create Herald rules for application X.
  - Remove Maniphest/Pholio rules when those applications are not installed.

Test Plan:
  - Restricted access to Maniphest and uninstalled Pholio.
  - Verified Pholio rules no longer appear for anyone.
  - Verified Maniphest ruls no longer appear for restricted users.
  - Verified users without CREATE_GLOBAL can not create global ruls.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7219
2013-10-04 15:15:48 -07:00
Bob Trahan
477d4e9db1 Herald - add support for "content source" conditions
Summary: ...and deploy on Maniphest. Ref T1638.

Test Plan: created a herald rule to be cc'd for tasks created via web. made a task via web and another via email and was cc'd appropriately. edited the herald to be cc'd for tasks created via not web. made 2 tasks again and got cc'd appropriately

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T1638

Differential Revision: https://secure.phabricator.com/D7145
2013-09-26 14:20:56 -07:00
Chad Little
9be7a948f9 Move PHUIFormBoxView to PHUIObjectBoxView
Summary: I'd like to reuse this for other content areas, renaming for now. This might be weird to keep setForm, but I can fix that later if we need.

Test Plan: reload a few forms in maniphest, projects, differential

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7120
2013-09-25 11:23:29 -07:00
epriestley
0ae7335316 Add missing (int) casts to Herald rule editing
Summary: Fixes T3792. These raise errors if the database is in strict mode and you try to create an "any" rule.

Test Plan: Created a rule with "any".

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3792

Differential Revision: https://secure.phabricator.com/D6883
2013-09-04 12:07:26 -07:00
Chad Little
fe2a96e37f Update Form Layouts
Summary:
This attempts some consistency in form layouts. Notably, they all now contain headers and are 16px off the sides and tops of pages. Also updated dialogs to the same look and feel. I think I got 98% of forms with this pass, but it's likely I missed some buried somewhere.

TODO: will take another pass as consolidating these colors and new gradients in another diff.

Test Plan: Played in my sandbox all week. Please play with it too and let me know how they feel.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6806
2013-08-26 11:53:11 -07:00
epriestley
751cd547c2 Remove dust from page construction
Summary:
  ^\s+(['"])dust\1\s*=>\s*true,?\s*$\n

Test Plan: Looked through the diff.

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6769
2013-08-19 18:09:35 -07:00
epriestley
ce163536ca Add a normal "view" page for Herald rules
Summary:
Ref T2769. This will house the transaction list and replace the "edit log" stuff.

The UI is a little bit rough and can probably share more code with the transaction history, but seems mostly-reasonable.

Test Plan: {F53253}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6690
2013-08-07 18:04:39 -07:00
epriestley
75e43513c2 Remove HeraldActionConfig, HeraldFieldConfig
Summary:
Ref T2769. Move all of this stuff into Adapters and get rid of the hard-coded classes.

I cheated in two places.

Test Plan: Edited and activated Herald rules.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6688
2013-08-07 18:04:37 -07:00
epriestley
2c2fcc58ca Remove HeraldConditionConfig
Summary: Ref T2769. Moves all traces of HeraldConditionConfig into Adapters.

Test Plan: Edited rules and used Test Console to exercise both affected code paths. Tried to save invalid rules to hit error pat.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6679
2013-08-07 18:04:36 -07:00
epriestley
ca66eeb07c Remove HeraldContentTypeConfig and move repetition to Adapters
Summary: Ref T2769. Get rid of the last use of `HeraldContentTypeConfig` by moving repetition options into Adapters.

Test Plan: Viewed / edited Herald rules.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6664
2013-08-07 18:04:35 -07:00
epriestley
78eb81ffd0 Remove almost all instances of HeraldContentTypeConfig
Summary: Ref T2769. This cleans up almost every use of the HeraldContentTypeConfig class.

Test Plan: Viewed and edited Herald rules.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6662
2013-08-07 18:04:33 -07:00
epriestley
9c637604a2 Move most Herald actions and values into dynamic adapters
Summary: Ref T2769. Shift the bulk of value and action config into Adapters.

Test Plan: Viewed and edited Herald list and rules.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, vrana

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6660
2013-08-07 18:03:54 -07:00
epriestley
2e87f9f53c Move most Herald condition config into dynamic adapters
Summary: Ref T2769. Pushes most condition configuration into Adapters, out of the hard-coded class.

Test Plan: Looked at, edited, and dry-run'd Herald rules.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6658
2013-08-07 18:03:53 -07:00
epriestley
3490b6dd11 Move most Herald field configuration into dynamic Adapters
Summary: Ref T2769. Herald has a giant hard-coded list of fields. Primarily make these dynamic and adapter-based.

Test Plan: Viewed and edited Herald rules.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6657
2013-08-07 18:03:52 -07:00
epriestley
a7ce55e3ca Remove extra side navs in Herald
Summary: Ref T2769. Removes some nonstandard side navs.

Test Plan: Viewed affected pages.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6651
2013-08-07 18:03:46 -07:00
epriestley
ceb7f830a4 Make HeraldRuleQuery policy-aware
Summary: Ref T2769. dem policy checks

Test Plan: Loaded `/herald/`; loaded rule editor.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

Differential Revision: https://secure.phabricator.com/D6650
2013-08-07 18:03:45 -07:00
Chad Little
c5e7222f9e Modernize Herald
Summary: Convert to responsive layout, pht, etc.

Test Plan: Test Herald on desktop and mobile.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5976
2013-05-20 08:24:07 -07:00
Chad Little
43ff24b0f3 Update form styles, implement in many places
Summary:
This creates a common form look and feel across the site. I spent a bit of time working out a number of kinks in our various renderings. Some things:

- Font Styles are correctly applied for form elements now.
- Everything lines up!
- Selects are larger, easier to read, interact.
- Inputs have been squared.
- Consistant CSS applied glow (try it!)
- Improved Mobile Responsiveness
- CSS applied to all form elements, not just Aphront
- Many other minor tweaks.

I tried to hit as many high profile forms as possible in an effort to increase consistency. Stopped for now and will follow up after this lands. I know Evan is not a super fan of the glow, but after working with it for a week, it's way cleaner and responsive than the OS controls. Give it a try.

Test Plan: Tested many applications, forms, mobile and tablet.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5860
2013-05-07 14:07:06 -07:00
epriestley
a22bea2a74 Apply lint rules to Phabricator
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such

Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D5002
2013-02-19 13:33:10 -08:00
vrana
2f508bf0dc Delete some phutil_safe_html()
Test Plan: Displayed revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4829
2013-02-05 15:52:48 -08:00
vrana
6bb7a282b1 Convert AphrontFormControl to safe HTML
Summary: Everything here now should properly handle plain strings and safe HTML.

Test Plan: /settings/panel/display/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4826
2013-02-05 15:52:46 -08:00
epriestley
820c6279f3 More render_tag -> tag (misc, form inset view)
Summary: Converts some callsites.

Test Plan: Grepped; looked -- see inlines.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4688
2013-01-28 18:42:57 -08:00
epriestley
3093d1663d Add javelin_tag(), convert easy callsites
Summary:
  - Implements `javelin_tag()`, which is `javelin_render_tag()` on top of `phutil_tag()` instead of `phutil_render_tag()`.
  - Manually converts all or almost all of the trivial callsites.

Test Plan:
  - Site does not seem any more broken than before.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4639
2013-01-25 12:57:17 -08:00
Chad Little
117589c160 Clean up Login, Responsive Forms
Summary: Removes the panel-view on login and adds additonal responsive styles for mobile forms.

Test Plan: View in mobile browser, resize page.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4530
2013-01-19 14:30:26 -08:00
Chad Little
c2b31149ab Removie panelview backgrounds from Herald.
Summary: Prior to re-doing responsive form CSS, update the Herald app for no panel backgrounds.

Test Plan: Went through Herald application, see no panels.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4545
2013-01-19 12:15:41 -08:00
vrana
ef85f49adc Delete license headers from files
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).

We are removing the headers for these reasons:

- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.

This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).

Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.

Reviewers: epriestley, davidrecordon

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2035

Differential Revision: https://secure.phabricator.com/D3886
2012-11-05 11:16:51 -08:00
vrana
8ff52c0b6c Set viewer for all handles loaded in controllers
Summary:
I've replaced all `id(new PhabricatorObjectHandleData(...))->loadHandles()` by `$this->loadViewerHandles(...)`.
Lint caught one usage in a static method.

Test Plan: Displayed revision with sporadic author.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3432
2012-09-04 23:14:26 -07:00
epriestley
f5e738b7ca Minor, fix a name collision between HeraldRuleController and PhabricatorController.
Auditors: btrahan
2012-08-15 14:11:40 -07:00
vrana
6cc196a2e5 Move files in Phabricator one level up
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.

NOTE: `arc diff` timed out so I'm pushing it without review.

Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.

Auditors: epriestley

Maniphest Tasks: T1103
2012-06-01 12:32:44 -07:00
Renamed from src/applications/herald/controller/rule/HeraldRuleController.php (Browse further)