Summary: Ref T8888. Cool man, like, more flags.
Test Plan: Flag it, Tag it, and Bag it.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8888
Differential Revision: https://secure.phabricator.com/D13653
Summary: Ref T8888, makes these two objects flaggable.
Test Plan: Set Flag, See Flag.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8888
Differential Revision: https://secure.phabricator.com/D13652
Summary: This isn't used much (once?) and it's not needed. Fixes T8876
Test Plan: Grep for setObjectName and see if its PHUIHeaderView.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8876
Differential Revision: https://secure.phabricator.com/D13645
Summary:
Ref T8726. Some adapters now have a large number of fields, and we lost the sort-of-human-readable implicit ordering when fields were modularized.
Instead, group and sort fields.
Test Plan: {F603066}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13619
Summary: Ref T8726. This gets rid of all the `VALUE_*` constants and lets Fields provide arbitrary typeaheads without upstream/JS changes.
Test Plan: Used all tokenizers. Used "Another Herald Rule". Grepped for all removed constants.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13616
Summary:
Ref T8726. I'm primarily trying to modularize tokenizer values so we don't have to update JS to add a new one.
This is ultimately the blocker for "select" custom fields working in Herald.
This inches us toward that. I'm //not// modularizing conditions or control types in this round, but hope to end up with hard-coded conditions (which are highly general and very rarely change), hard-coded control types (which are also highly general and very rarely change) and completely modular fields and values (which have mid-to-low generality and change frequently).
Test Plan: Used UI to interact with "none", "text", and new-style "select" controls. No actual support for tokenizers yet.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13613
Summary:
Ref T8726. I want to modularize values and reduce how hard-coded / copypasta'd they are.
- Rename `get...StandardCondition()` to `get...StandardType()`, since we can drive both conditions and values from it.
- Rename `STANDARD_LIST` to `STANDARD_PHID_LIST` for consistency: all "lists" are lists of PHIDs.
- For all standard types which don't require typehaeads, lift their logic into the base class.
- I'll lift typeaheads soon, but need to generalize them first.
Test Plan: Edited various Herald rules, saw value UI generate correctly.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13612
Summary: Ref T7149. These formalize the local path adjustment step for cluster imports, rather than relying on an ad-hoc script.
Test Plan: Used `list-paths` and `move-paths` to list and move repositories.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7149
Differential Revision: https://secure.phabricator.com/D13621
Summary: Implements T8734
Test Plan: Verify that countdowns can have spaces associated with them. Verify that non-default spaces are displayed in the header of the countdowns in the result of the query.
Reviewers: lpriestley, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13634
Summary: Implements T8733
Test Plan: Verify that spaces policies can be implemented in the creation and editing of polls.
Reviewers: lpriestley, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13537
Summary: Ref T8726. The gruntwork part of this is finally over.
Test Plan:
- Made a huge rule with every field.
- Applied migration.
- Verified the rule was still the same.
- Pushed a bunch of commits and verified transcripts.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13601
Summary: Ref T8726. Just (pre-commit content) one more left.
Test Plan:
- Created a big rule with every field.
- Migrated it.
- Verified the rule was still the same.
- Pushed a bunch of changes and reviewed the transcripts.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13600
Summary: Ref T8726. Make all the DifferentialRevision stuff modular.
Test Plan:
- Created a rule with all fields.
- Ran upgrade.
- Saw all fields preserved with new modular versions.
- Used test console to run rule with all fields, verified field values as broadly sensible.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13598
Summary: Ref T8726. This deals with all the Differential diff fields, same deal as previous changes.
Test Plan:
- Wrote a rule with every field.
- Migrated it.
- Saw the same rule working.
- Rigged the hell out of transcripts (diffs normally do not generate transcripts, because the only action is "block" and they don't exist yet when Herald runs).
- Verified that all fields looked sensible in the transcript.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13590
Summary: Releeph does some really old sketchy stuff in result rendering. Modernize it, remove the holdover/oldschool interface (these were the last two implementors) and make errors a little softer.
Test Plan: Viewed Releeph products and branches.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13604
Summary: Fixes T7604. This is the big scary change which drops the "arcanist project" fields from the database permanently.
Test Plan:
`grep`ped for the following to ensure that I had found all remaining references:
- `/arcanistProject/i`
- `/arcanist_project/i`
- `/projectName/i`
- `/project_name/i`
- `/project_id/i`
WARNING: Wait at least one month before landing this.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T7604
Differential Revision: https://secure.phabricator.com/D12899
Summary: "Enrage in Deliberations" button on Slowvote should say "Engage in Deliberations".
Test Plan: Make sure the button says the right thing now.
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13591
Summary: Ref T8780. I think this fixes the issue.
Test Plan: @chad
Reviewers: chad
Reviewed By: chad
Subscribers: chad, epriestley
Maniphest Tasks: T8780
Differential Revision: https://secure.phabricator.com/D13584
Summary: These detail pages don't have a "pageObjects" configuration, so we don't clear, e.g., token stories correctly when users load them.
Test Plan: Will clear a token story in production.
Reviewers: btrahan, lpriestley, chad
Reviewed By: chad
Subscribers: chad, epriestley
Differential Revision: https://secure.phabricator.com/D13582
Summary: Ref T8726.
Test Plan:
Created a giant rule with every commit field:
{F594686}
Ran the upgrade, got the same rule with new fields:
{F594688}
Used "Test Console" to run transcripts, saw all the fields populate correctly.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13567
Summary:
Fixes T8774. When an ApplicationSearch page returned an error (e.g., using a "viewer()" query while logged out), we would try to add action links to a box without a header. Instead:
- If a box has no header, but has show/hide actions, just create an empty header so the view renders. This is a little silly looking but does what the caller asks.
- Always set the title on the result box, so we get a header.
Test Plan: Did an invalid (logged out, with viewer()) search. Did a valid search.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8774
Differential Revision: https://secure.phabricator.com/D13576
Summary: This rule isn't quite right.
Test Plan: Shipped email in with a rule in effect, saw correct value in transcript.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13578
Summary: Adds a trivial test case to ensure that `HeraldField` subclasses are properly implemented.
Test Plan: `arc unit`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13577
Summary: This was broken in D13374. Also rearranged a few classes.
Test Plan: `arc lint`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13575
Summary: Use `PhutilClassMapQuery` where appropriate.
Test Plan: Browsed around the UI to verify things seemed somewhat working.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13429
Summary:
- Set a default edit policy to match view policy for projects
- It seems like initializeNew* is a better init function than new Phabricator*,
so ported two classes to use it and spot-checked the others (this also fixed
a fatal.)
- Other minor bugs
Test Plan: Ran lipsum for a while without any fatals
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, jhurwitz, Korvin
Differential Revision: https://secure.phabricator.com/D13559
Summary: Ref T8726. Use modular fields for the Pholio adapter.
Test Plan:
- Created rule using all the old fields.
- Migrated, saw upgrade apply correctly.
- Created mock, reviewed transcript.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13513
Summary:
Ref T8726. The only notable bit here is that the "body" / "title" fields (which are currently shared across a bunch of types) are getting split into application variants.
Among other things, this will let us label the field "Commit message" for commits, for example.
Test Plan:
- Created a rule using all four fields.
- Applied patch, saw rule break ("unknown field").
- Ran storage upgrade, saw rule fix itself in the migration.
- Edited tasks, triggered rule, viewed transcripts.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13501
Summary: Ref T8726. These are a bit involved because they have custom rendering and editor values.
Test Plan: Created new rule using these fields, edited tasks to trigger them, viewed transcripts.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13500
Summary:
Ref T8726. There's no interface we can check for this, so the adapter needs to opt in.
Also fix a spelling mistake.
Test Plan: Created rules with "Application Email" fields.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13499
Summary: Ref T8726. No clue what this was.
Test Plan: Grepped for usage, found none.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13498
Summary: Ref T8726. The existing implementation is largely made redundant by the newer modular implementation.
Test Plan: Created a custom field rule, set custom field to various values until it matched, saw effect and proper transcripts.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13497
Summary: Ref T8726. Continue making Herald fields more modular than they currently are.
Test Plan:
- Created a rule using all the affected fields.
- Ran the rule.
- Saw reasonable object field values.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13495
Summary:
Ref T8726. This lays groundwork for modularizing Herald fields and modularizes the "Content source" field.
This doesn't touch conditions or values yet.
Test Plan: Created a rule with a "content source" field, created a new task, saw rule apply.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13488
Summary: Return `$this` from setter methods for consistency. I started writing a [[https://secure.phabricator.com/differential/diff/32506/ | linter rule]] to detect this, but I don't think it is trivial to do this properly.
Test Plan: Eyeball it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13422
Summary: Move some `PhabricatorSearchField` subclasses to be adjacent to the application to which they belong. This seems generally better to me than lumping them all together in the `src/applications/search/field/` directory. I was also wondering if it makes sense to rename these subclasses as `PhabricatorXSearchField` rather than `PhabricatorSearchXField` (as per T5655), but wasn't really sure if these objects are meant to be search-fields, or just fields belonging to the #search application.
Test Plan: N/A.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13374
Summary: Modern mozilla browsers use min-resolution.
Test Plan: Regernate sprites, use site in Firefox, see correct header image.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13555
Summary: Ref T8099, Fixes layout of the Pager, all adds distinction to the current pager button.
Test Plan: Edit a Phriction document 100 times. Get a pager.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13549
Summary: Ref T8449. Also, fix annoying inconsistency between "Spaces" and "spaces": "Spaces" is the application, the things it creates are just "spaces", similar to how "Maniphest" creates "tasks".
Test Plan: Mostly reading. Viewed `/applications/`.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: eadler, epriestley
Maniphest Tasks: T8449
Differential Revision: https://secure.phabricator.com/D13547
Summary:
Ref T8449. If a user doesn't have access to any spaces, most applications just don't work, and they fail in confusing ways.
Just lock users out of everything explicitly up front with a clear message instead of letting them stumble into a big broken mess.
Test Plan: Locked a user out of all spaces, saw error to that effect.
Reviewers: btrahan, eadler
Reviewed By: eadler
Subscribers: eadler, epriestley
Maniphest Tasks: T8449
Differential Revision: https://secure.phabricator.com/D13545
Summary: We access this variable, but may never initialize it.
Test Plan: Viewed SVN repository.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13544
Summary: Fixes T8740. This data is required for now, and didn't get brought across properly from the original standalone foreground editor.
Test Plan: Did a bulk editor "add project", got a clean result.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8740
Differential Revision: https://secure.phabricator.com/D13533
Summary: I got a couple of tasks stuck in my local queue a while ago when touching Nuance, fix a couple minor issues to clean them up.
Test Plan: Ran tasks in queue, got clean results.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13532
Summary: Fixes T8736. It's OK for duration to be something that comes out of JSON as an int, like `1`.
Test Plan: Will make @hach-que verify.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, hach-que
Maniphest Tasks: T8736
Differential Revision: https://secure.phabricator.com/D13531
Summary: Ref T8099, Missed a few various edge cases here. Cleans up the 'next' UI.
Test Plan: Review a current setup issue, then ignore it.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13529
Summary:
Ref T6817. Fixes T8731. On the old `secure` host, `feed.public` was set to `true`. I didn't bring the option over, which caused the secondary issue in T8731.
Specifically, when `feed.public` is off, a logged-out user looking at feed can't see //any// stories, so they query all of feed until they hit the time limit.
To fix this immediately, just use the most open policy, which is basically equivalent but always correct.
To fix this more thoroughly:
- Remove `feed.public`, which violates policies and has been slated for removal for a while (see T6817).
- Clean up policy handling.
Test Plan:
- As a logged-out user, viewed feed on a public install with `feed.public` off; no longer saw all stories get queried + no feed shown.
- Grepped for `feed.public`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T6817, T8731
Differential Revision: https://secure.phabricator.com/D13518
Summary: This correct some bad strings for translations
Test Plan:
merge similar stings
removes some word wraps
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8700
Differential Revision: https://secure.phabricator.com/D13464
Summary: Ref T8099. I think these colors make sense, but if any seem wrong, lmk. Colors the type of file change.
Test Plan: Test a few diffs locally, read carefully, ask epriestley.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13502
Summary: Ref T8099, Fixes T8339. This lets us pull in the status icon and display in lists and object headers.
Test Plan: Look at a list of open and closed tasks, see icons match state (duplicate, spite, defaults).
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099, T8339
Differential Revision: https://secure.phabricator.com/D13490
Summary: Closes T8696, Commenting on instances of a recurring event should create a new event with its own comments
Test Plan: Open recurring event instance (not exception), should see no timeline, leave a comment, an exception to event should be created.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8696
Differential Revision: https://secure.phabricator.com/D13481
Summary:
Fixes T8727. When I added `setParentQuery()`, I increased the cache hit rate but also accidentally propagated policy exception settings.
Instead, make the policy exception beahvior explicit: no exceptions should be raised when querying handles.
Test Plan: Will test production.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8727
Differential Revision: https://secure.phabricator.com/D13489
Summary:
Ref T8631. The query plan for feed stories is really bad right now, because we miss caches we should be hitting:
- The workspace cache is stored at each query, so adjacent queries can't benefit from the cache (only subqueries). Feed has primarily sibling queries.
- There is no technical reason to do this. Store the workspace cache on the root query, so sibling queries can hit it.
- In `ObjectQuery`, we check the workspace once, then load all the PHIDs. When the PHIDs are a mixture of transactions and objects, we always miss the workspace and load the objects twice.
- Instead, check the workspace after loading each type of object.
- `HandleQuery` does not set itself as the parent query for `ObjectQuery`, so handles never hit the workspace cache.
- Pass it, so they can hit the workspace cache.
- Feed's weird `PhabricatorFeedStory::loadAllFromRows()` method does not specify a parent query on its object/handle queries.
- Just declare the object query to be the "root" query until this eventually gets cleaned up.
Test Plan: Saw queries for each object drop from 4-6x to 1x in `/feed/`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8631
Differential Revision: https://secure.phabricator.com/D13479
Summary: Ref T8631. Projects may not have a profile image PHID; don't try to load it if they don't.
Test Plan: Saw `WHERE phid IN ('')` query stop happening on a local feed story page.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8631
Differential Revision: https://secure.phabricator.com/D13476
Summary: Fixes T8719. It's fine to subscribe to objects that don't have all their required fields filled in.
Test Plan: Subscribed/unsubscribed from an object.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8719
Differential Revision: https://secure.phabricator.com/D13484
Summary: Ref T8099, Fixes T8338. This allows re-ordering of Maniphest Tasks in the redesign. Somehow seems more fragile, but I couldn't break anything with it.
Test Plan: Try ordering into first position after header, last position, changing priority outright, everything I can drag.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099, T8338
Differential Revision: https://secure.phabricator.com/D13487
Summary: Ref T8099, Missed this when testing Spaces. Adds proper list formatting.
Test Plan: Review Policy when object is higher or lower than default.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13478
Summary: Ref T8099, Simplifies the button bar with a `borderless` option and implements in Differential Inline Commenting.
Test Plan:
Review new and old comments, submitted, unsubmitted, ghosts, done.
{F562765}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13475
Summary:
Fixes T5702. The path here is long and windy:
- I want to move `blog.phacility.com` to the new `secure` host.
- That host has `security.require-https` set, which I want to keep set (before, this was handled in a sort of hacky way at the nginx/preamble level, but I've cleaned up everything else now).
- Currently, that setting forces blogs to HTTPS too, which won't work.
- To let blogs be individually configurable, we need to either modularize site config or make things hackier.
- Modularize rather than increasing hackiness.
- Also add a little "modules" panel in Config. See T6859. This feels like a reasonable middle ground between putting this stuff in Applications and burying it in `bin/somewhere`.
Test Plan:
- Visited normal site.
- Visited phame on-domain site.
- Visited phame off-domain site.
- Viewed static resources.
{F561897}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5702
Differential Revision: https://secure.phabricator.com/D13474
Summary: Ref T8099, Cleans up UI issues, adds `appendList` and renders lists and paragraphs with Remarkup UI.
Test Plan: Test Policy Dialogs, other various dialogs.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13463
Summary: Ref T8099, functionally I prefer to be able to set anything 'table-like' with `setTable` for design consistency. This looses the restriction and did some light grepping for other missed cases.
Test Plan: Test new UI, grep for other missing cases.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13471
Summary: Fixes T8703. The URI handling here was a little unusual.
Test Plan:
- Edited and deleted comments in several applications, including Macro.
- As an admin, deleted others' comments.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8703
Differential Revision: https://secure.phabricator.com/D13469
Summary: Fixes T8698. This isn't actually a table.
Test Plan: Viewed push log, no more fatal.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8698
Differential Revision: https://secure.phabricator.com/D13468
Particularly, in the cluster, it needs to be able to see Almanac services.
This permission may be conflated with the logged-out viewer, since neither
have PHIDs.
Auditors: btrahan
Summary: Ref T8686. This method has the wrong name and calls a nonexistent method on `$service`.
Test Plan: Set page size to 5, paged locally, hit error, applied changes, got clean paging.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8686
Differential Revision: https://secure.phabricator.com/D13460
Summary: Closes T8028, Set event policies like other event properties
Test Plan: Create and edit events, make sure edit and view policies get populated and saved correctly.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T8028
Differential Revision: https://secure.phabricator.com/D13415
Summary: Ref T8099, Ref T8614. Reasonable attempt at a larger font size PostProcessor, will need additional UI cleanup (places where we fix height) but overall very decent.
Test Plan: Use lots of the UI elements with new font sizes.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099, T8614
Differential Revision: https://secure.phabricator.com/D13457
Summary:
We use a non-standard way to invoke the edit dialogue (title, picture, etc) via the crumb. Stop doing that and instead use the widget technology to invoke the dialogue.
This requires making the widget handling code a bit more nuanced as nothing has wanted to pop a dialogue before. I plan to clean this up as I add the action to "Mark as Favorite" to the UI. In particular, I want to stop rendering the un-used DOM and make a workflow-based widget action a property as opposed to something hardcoded. This may be too ambitious depending on how similar these workflows are....
This also updates the ThreadSearchEngine to be a bit more modern. Additionally, go through making some user-facing strings a bit more sensical.
Test Plan: changed settings from conpherence full and durable column successfully.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13449
Summary: Fixes T8675, Set display mode for Upcoming Events builtin query so that no `$max_range` is set on the query
Test Plan: Open Upcoming Events builtin query, max events returned should be size of the page (100 objects)
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T8675
Differential Revision: https://secure.phabricator.com/D13447
Summary:
Ref T8488, T8469, T8485.
This is done in regards to T8488 as far as users are concerned. There's still some classes, and etc. that should be re-named probably. T8469 and T8485 are basically moot now though.
Rather than having "Send Message" exposed, just expose "Create Room". Users get the full form. One change is "title" is now required.
This diff removes the concept of "isRoom" entirely.
Test Plan: Verifed a user with no conpherences had sensible data in both column view and full conpherence view. Created rooms with various policies and things worked well.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: chad, epriestley, Korvin
Maniphest Tasks: T8469, T8485, T8488
Differential Revision: https://secure.phabricator.com/D13351
Summary: Ref T8099, A cleaner, more standard UI for Slowvotes. Because Slowvote is important.
Test Plan: Test a number of poll layouts and embeds.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13445
Summary: Ref T8650. This should stop the problem, but isn't a root cause fix. See discussion on the task.
Test Plan: Made some local diffs, but this is a bit hard to reproduce reliably.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8650
Differential Revision: https://secure.phabricator.com/D13441
Summary: Ref T8670. The spec for "coverage" is incorrect, it's a map of paths to coverage information.
Test Plan: See next diff.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8670
Differential Revision: https://secure.phabricator.com/D13435
Summary: Ref T8658. Caught this in the logs. This value may be set to `null`. Handle that gracefully.
Test Plan: Will check logs.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8658
Differential Revision: https://secure.phabricator.com/D13432
Summary:
I pulled this out of the logs; not sure how anyone hit it, but this controller could bump into an undefined variable here:
```
2015/06/24 22:59:55 [error] 10150#0: *172493 FastCGI sent in stderr: "PHP message: [2015-06-24 22:59:55] EXCEPTION: (RuntimeException) Undefined variable: subscriber_phids at [<phutil>/src/error/PhutilErrorHandler.php:210]
PHP message: arcanist(head=master, ref.master=b697a3b80bdc), phabricator(head=redesign-2015, ref.master=0fd0f171f10f, ref.redesign-2015=1a5f986d73dd), phutil(head=master, ref.master=74c9cb3a266e)
PHP message: #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phabricator>/src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php:32]
PHP message: #1 <#2> PhabricatorSubscriptionsListController::processRequest() called at [<phabricator>/src/aphront/AphrontController.php:33]
PHP message: #2 <#2> AphrontController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:226]
PHP message: #3 phlog(RuntimeException) called at [<phabricator>/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php:229]
PHP message: #4 AphrontDefaultApplicationConfiguration::handleException(RuntimeException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:230]
PHP message: #5 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:140]
PHP message: #6 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:21]" while reading response header from upstream, client: 167.114.156.198, server: , request: "GET /subscriptions/list/phid-wiki-366842d394398204f305/ HTTP/1.1", upstream: "fastcgi://unix:/core/var/pool/fpm.sock:", host: "secure.phabricator.com"
```
Clean things up a bit.
Test Plan: Clicked "6 others" subscriber link on a task with a bunch of subscribers.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13430
Summary: Ref T8099, Moving to Roboto Slab for Document Headers. It's a little less serif-y and fits well with Lato. Also took a pass at cleaning up edge cases in Documents, Diviner, Phriction, Legalpad, and Welcome Screen.
Test Plan: Test Phriction, Diviner, Legalpad, Welcome Screen
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13439
Summary: Ref T8099, Fixes padding and spacing when status tags are coming from Maniphest. Also, maybe remove this?
Test Plan: Test closed tasks in Maniphest.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13434
Summary: Fixes T8655. This isn't actually a table -- just use `setContent()`.
Test Plan: Loaded leases in redesign-2015.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8655
Differential Revision: https://secure.phabricator.com/D13431
Summary: Ref T5791. This edge table grows 2+X faster than the corresponding mail table depending on usage. Ergo, lets make sure to clean that up too in the delete code.
Test Plan: careful thought
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13408
Summary:
Ref T8493. Diffusion is probably the strongest upstream use case we have for Spaces right now, so I want to get us on it to kick the tires a bit.
Small amount of hackiness around the multi-page form thing but it shouldn't create any problems.
Test Plan:
- Created a new repo.
- Edited a repo.
- Tried invalid edits, saw value preserved.
- Viewed edit full detail screen, saw space info.
- Viewed repo detail view, saw space.
- Viewed repo list view, saw space.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8493
Differential Revision: https://secure.phabricator.com/D13414
Summary: Ref T5791. Makes the result page a little less ugly and adds the time created. Makes the detail page include more useful information about the message.
Test Plan: viewed results page and detail page and they looked better
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13410
Summary:
Ref T8637. This does nothing interesting, just has empty scaffolding for a bulk job queue.
Basic idea is that when you do something like a batch edit in Maniphest, we:
- Create a BulkJob with all the details.
- Queue a worker to start the job.
- Send you to a progress bar page for the job.
In the background:
- The "start job" worker creates a ton of Task objects, then queues worker tasks to do the work.
In the foreground:
- Fancy ajax animates the progress bar and it goes wooosh.
In general:
- Big jobs actually work.
- Jobs get logged.
- You can monitor jobs.
- Terrible junk like T8637 should be much harder to write and much easier to catch and diagnose.
Test Plan:
No interesting code/beahavior yet. Clean `storage adjust`.
{F526411}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8637
Differential Revision: https://secure.phabricator.com/D13392
Summary:
Ref T8096. Various tweaks here:
- Sort result lists by importance (even lint -- "errors first" seems better than "alphabetical by file", I think?).
- Do sane stuff with display limits.
- Add a "view all" view.
- Don't show a huge table of passing tests in Differential.
- Link to full results.
Test Plan: See screenshots.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13407
Summary: Ref T8096. We don't currently link to the buildable, which I think contributes to Harbormaster feeling a little scattered.
Test Plan: {F528095}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13405
Summary: Ref T8096. No functional changes, just a bit less code.
Test Plan: Viewed some revisions, saw the same stuff as before.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13404
Summary:
Ref T5791. This diff adds a "sensitive" flag to `PhabricatorMetaMTAMail`, defaults it to true in the constructor, and then sets it to false in teh application transaction editor. Assumption here is that sensitive emails are basically all the emails that don't flow through the application transaction editor.
This diff also gets a basic "mail view" page up and going.
This diff also fixes a bug writing recipient edges; the actor was being included.
This bug also fixes a querying bug; we shouldn't do the automagic join of $viewer is recipient or $viewer is actor if folks are querying for recipients or actors already. The bug manifested itself as having the "inbox" be inbox + outbox.
Test Plan: viewd list of messages. viewed message detail.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13406
Summary:
Ref T5791. This diff does a few things...
- Adds code to write recipients to edges on save
- Makes Query performance for policy filtering okay-ish
- Adds a Search Engine for PhabricatorMetaMTAMail
- Adds "working" List Controller
- Inbox and Outbox both work
- Adds stub View Controller
Test Plan: ran `./bin/storage upgrade` and saw my inbox and outbox start getting data. played with application and saw new entries in inbox and outbox
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13397
Summary: Fixes T8095. Still needs UI/UX work (see T8096) but this has all the core features now.
Test Plan: Saw Harbormaster lint/unit data as though it was Differential lint-unit data.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13401
Summary: Ref T8096. Show basic lint/unit info on builds. This is still pretty rough.
Test Plan: {F524839}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13383
Summary:
Ref T8096. Fixes a few bugs and glitches.
- Set build completion time when handling a message.
- Format duration information in a more human-readable way.
- Use a table for build variables.
- Fix up container PHIDs on diffs (a touch hacky, should be OK for now though).
Test Plan: Browsed around the UI.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13382
Summary:
Ref T8095. When build results are reported for a target, allow them to include unit and lint results.
There is no real way to see this stuff in the UI yet, either in Harbormaster or Differential.
Test Plan: Manually called this method with some results, saw Harbormaster update appropriately.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13380
Summary: Ref T8095. This weird grey table has no remaining callsites and can be removed.
Test Plan: Grepped for symbols.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13379
Summary:
Ref T8095. Same as D13377, but for unit results.
This is a bit rough and there's some duplication between this and unit results. I'll likely merge them later, but I think some of it is superficial since these iterations are still a little crude.
Test Plan: {F523499}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13378
Summary:
Ref T8095. Render lint results in a future-ready way.
This makes the renderer accept `HarbormasterBuildLintMessage` objects. If we have legacy data instead, it converts it into `HarbormasterBuildLintMessage` objects.
Design is a bit rough but will be cleaned up later after T7739.
This moves away from "postponed linters", which are obsolete after Harbormaster (and were only ever used by Facebook).
Test Plan: {F523429}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13377
Summary: closes T8486
Test Plan: Create a pholio mock. In an already existing object mention the pholio mock. Verify that the reference to the object exists in the pholio mock's timeline.
Reviewers: lpriestley, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8486
Differential Revision: https://secure.phabricator.com/D13402
Summary: Ref T8099, This adds a consistent background color to object and policy tags, and highlights them when they deviate from the normal. Still likely worth revamping 'closed' and 'review' state colors.
Test Plan: Review lots of diffs and tasks.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13399
Summary: Closes T8639, Formatting event dates in list view
Test Plan: List view should show dates in wide and narrow lists.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8639
Differential Revision: https://secure.phabricator.com/D13398
Summary: Ref T5791. This should make performance snappy wrt policy checks in some future diff where the Query is updated and in use somewhere in the application.
Test Plan: ran `./bin/storage upgrade`. commented on a task and saw actorPHID populated correctly in underlying MetaMTAMail object database entry
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13396
Summary: Fixes T8642, This is a table, but not returned as one. Set it to Content for now with a Collapsed layout.
Test Plan: Test /people/logs/
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8642
Differential Revision: https://secure.phabricator.com/D13395
Summary:
Ref T5791. This does a few bits there. Namely:
- Adds PHID column to PhabricatorMetaMTAMail
- Implements a PhabricatorMetaMTAMailPHIDType
- Script to backpopulate them.
- Makes PhabricatorMetaMTAMail implement PolicyInterface.
- View policy is NOONE and the author and recipients have automatic view capabilities
- No edit capability.
- Adds a PhabricatorMetaMTAMailQuery for PhabricatorMetaMTAMail.
Test Plan: ran `./bin/storage upgrade` successfully. commented on a maniphest task and verifed the metamta mail object in the database was created successfully with a shiny new phid
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13394
Summary: Ref T7950, Refactor Calendar Search, and implement Projects on events
Test Plan: Verify that all queries in Calendar search still work, and that events can now have associated Projects that you can search by in Calendar Search.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T7950
Differential Revision: https://secure.phabricator.com/D13393
Summary:
Fixes T4139. Adds a "Desktop Notifications" panel to settings. For now, we start with "Send Desktop Notifications Too" functionality. We can try to be fancy later and only send desktop notifications if the web app doesn't have focus, etc.
Test Plan:
Made some comments as a test user on a task and got purdy desktop notifications using Chrome. Then did it again with Firefox.
Played around with permissions form with Chrome and got helpful information about what was up. Played around with Firefox and got similar results, except canceling the dialogue didn't invoke my handler code somehow. Oh Firefox!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: rbalik, tycho.tatitscheff, joshuaspence, epriestley, Korvin
Maniphest Tasks: T4139
Differential Revision: https://secure.phabricator.com/D13219
Summary: Ref T8637. This class has some really old parameter handling which can send `withIDs(array())` down a "fetch everything" pathway. Clean up most of it.
Test Plan: Issued every ApplicationSearch query.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8637
Differential Revision: https://secure.phabricator.com/D13390
Summary:
Ref T8637. If a user tries to batch edit a list of tasks which can't be edited, we fall through to `withIDs(array())`, which can affect //everything//.
Explicitly stop batch editing if we don't have valid IDs or valid tasks.
The UI sort-of warns you that something is wrong, but this is ultimately a pretty severe UX issue. I'll fix the underlying Query in the next diff.
Test Plan: Tried to batch edit a list of tasks I didn't have permission to edit.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: lloyd.oliver, epriestley
Maniphest Tasks: T8637
Differential Revision: https://secure.phabricator.com/D13388
Summary:
Fixes T6787. I'm kind of cheating a little bit here by not unifying default selection with `initializeNew(...)` methods, but I figure we can let this settle for a bit and then go do that later. It's pretty minor.
Since we're not doing templates I kind of want to swap the `'template'` key to `'type'` so maybe I'll do that too at some point.
@chad, freel free to change these, I was just trying to make them pretty obvious. I //do// think it's good for them to stand out, but my approach is probably a bit inconsistent/heavy-handed in the new design.
Test Plan:
{F525024}
{F525025}
{F525026}
{F525027}
Reviewers: btrahan, chad
Reviewed By: btrahan
Subscribers: johnny-bit, joshuaspence, chad, epriestley
Maniphest Tasks: T6787
Differential Revision: https://secure.phabricator.com/D13387
Summary:
Fixes T5135. Currently, when you create a credential, we default the policies to your PHID. This means we can't have an application-level configurable default because there's no way to select "the actor's PHID" as a policy.
Start tracking the credential author's PHID and add an object policy for it, so there is such a setting.
Then, add policy defaults.
This mostly unblocks T6787. This obsoletes T6860.
Test Plan:
- Created a credential with "Credential Author" policy.
- Verified I can see/edit it, but other users can not.
- Changed default policies to something else.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5135
Differential Revision: https://secure.phabricator.com/D13385
Summary: Prepares for bringing spaces and a new object policy here.
Test Plan: Used all search controls dozens of times.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D13384
Summary:
Ref T8095. Two general problems:
- I want Harbormaster to own all lint and unit test results.
- I don't want users to have to configure anything for `arc` to keep working automatically.
These are in conflict because generic lint/unit test ownership in Harbormaster requires that build targets exist which we can attach build results to. However, we can't currently create build targets on demand: Harbormaster assumes it is responsible for creating targets, then running code or making third-party service calls to actually run the builds.
I considered two broad approaches to let `arc` push results into Harbormaster without requiring administrators to configure some kind of "arc results" build plan:
# Add magic target PHIDs like `PHID-MAGIC-this-is-really-arc-unit`.
# Add new code to build real targets with real PHIDs.
(1) is probably a bit less work to get off the ground, but I think it's worse overall and very likely to create more problems in the long run. I particularly worry that it will lead to a small amount of special casing in a very large number of places, which seems more fragile.
(2) is more work upfront but I think does a better job of putting all the special casing in one place that we can, e.g., more reasonably unit test, and letting the rest of the code rarely/never care about this case since it's just dealing with normal plans/steps/targets as far as it can tell.
This diff introduces "autoplans", which are source templates for plans/steps. This let us "push" these targets into Harbormaster. Hypthetically, any process "like" arc can use autoplans to upload test/lint/etc results. In practice, probably only `arc` will ever use this, but I think it's still quite a bit cleaner than the alternative despite all the generality.
Workflow is basically:
- `arc` creates a diff.
- `arc` calls `harbormaster.queryautotargets`, passing the diff PHID and saying "I have some lint and unit results I want to stick on this thing".
- Harbormaster builds the plan, steps, and targets (if any of them don't already exist), and hands back the target PHIDs so `arc` has a completely standard-looking place to put results.
- `arc` uploads the test results to the right targets, as though Harbormaster had asked it to run unit/lint in the first place.
(This doesn't actually do any of that yet, just sets things up.)
I'll maybe doc turn that ^^^^^^ into a doc for posterity since I think it's hard to guess what an "autotarget" is, but I'm going to grab some lunch first.
Test Plan:
- Added unit tests to make sure we can build these things properly.
- Used `harbormaster.queryautotargets` to build autotargets for a bunch of diffs.
- Verified targets come up in "waiting for message" state.
- Verified plans and steps are not editable.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: hach-que, epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13345
Summary:
Ref T8095. This is just general groundwork for more exciting changes:
- Use more modern conventions around controllers, UI elements, and dialogs.
- Provide real CAN_EDIT policies and policy checks (they just don't do anything yet).
Test Plan:
- Used all affected controllers.
- Faked CAN_EDIT to POLICY_NOONE and verified everything was greyed out and unselectable.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13344
Summary:
Ref T8095.
Harbormaster has a `BuildItem` class, but it has no table and is unused. This was an earlier idea about representing lint/unit results and some other possible types of messages, but I think we want to be more specific than this.
Remove `BuildItem` and add `Lint` and `Unit` storage. These tables roughly parallel how we store lint/unit messages today, with some guesses about how where they'll go in the future.
Test Plan: Ran `bin/storage upgrade` and got a clean adjust out of it.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: hach-que, epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13329
Summary: Ref T8099, Now that we have cleaner headers, we can add more pop to important items, like the state of an Object. This makes it easier to just note the color and generally understand the state of the object (closed, returned, accepted, open). @epriestley, I think you previously thought this was a bug, but if it still feels bad, let me know.
Test Plan:
Review Differential, Maniphest, Polls for various object states.
{F520973}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13369
Summary: Ref T8099, Adds more infomation to the icon in Maniphest Task Lists.
Test Plan: Hover over icons in a Maniphest Task
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13367
Summary: Fixes T8626. I guess that `$this->getViewer()->renderHandle(null)` renders as "Unknown Object", which is somewhat odd to me.
Test Plan: Viewed an atom from a book without a repository linked to it.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T8626
Differential Revision: https://secure.phabricator.com/D13370
Summary: Not sure if we want this, but it seems to work fine.
Test Plan: {F516736}
Reviewers: joshuaspence, chad
Reviewed By: joshuaspence, chad
Subscribers: joshuaspence, epriestley
Differential Revision: https://secure.phabricator.com/D13363
Summary: Ref T8617. Provide general documentation with tools for debugging hangs and slow pages. Update DarkConsole docs and discuss how to use Services and XHProf. Explain what Multimeter is for and how to use it. Update XHProf docs and provide some usage hints.
Test Plan: Read documentation.
Reviewers: joshuaspence, btrahan
Reviewed By: joshuaspence, btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T8617
Differential Revision: https://secure.phabricator.com/D13359
Summary: Fixes T6839. Sometimes, worker tasks go astray for whatever reason. This automates the step of `bin/repository importing | xargs | mangle mangle | bin/repostiory reparse`.
Test Plan: Ran various flavors of the command, got good looking results.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6839
Differential Revision: https://secure.phabricator.com/D13362
Summary: Ref T8099, uses original bgcolor when dark headers are used.
Test Plan: Switch between light and dark Phabricator, see new colors.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13361
Summary: Ref T8096. This shows the build plan name on the Harbormaster build plan view controller. Without this, the name is not displayed anywhere on the page when you're viewing a build plan's configuration (which makes things confusing if you're updating a bunch of build plans at once).
Test Plan: Viewed a build plan, saw the build plan name on the page.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Projects: #harbormaster
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13356
Summary: Ref T8099, Cleans up profile photos around the site in a more consistent way.
Test Plan: grep for 'background-size: 100%'
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13360
Ref T8618. I think this was just typoed in D13070.
Immediate issue is that we're looking for symbols with the book name as a "phid".
Auditors: joshuaspence
Summary: Closes T8032
Test Plan: Verify that when editing a calendar event's description, there is a UI bar helping with adding markup to the description. Also verify that markup is displayed correctly on the event page once the event has been updated.
Reviewers: lpriestley, #blessed_reviewers, epriestley
Reviewed By: lpriestley, #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8032
Differential Revision: https://secure.phabricator.com/D13355
Summary:
Minor layout updates to Ref T8099
- Timeline tweaks
- Use Lato headers in Document Obj Headers
- Minor Remarkup
- Add Audit Icons
(Unclear if Audit is "correct", ie Status vs. Commit Status) But added icons anyways if needed.
Test Plan: Review each of the updated elements
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13357
Summary: Ref T8099. This adds a new class which all search engines return for layout. I thought about this a number of ways, and I think this is the cleanest path. Each Engine can return whatever UI bits they needs, and AppSearch or Dashboard picks and lays the bits out as needed. In the AppSearch case, interfaces like Notifications, Calendar, Legalpad all need more custom layouts. I think this also leaves a resonable path forward for NUX as well. Also, not sure I implemented the class correctly, but assume thats easy to fix?
Test Plan: Review and do a search in each application changed. Grep for all call sites.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13332
Summary: Fixes T8352. Associate Diviner books and atoms with a repository. This relationship is not really surfaced anywhere in the UI but provides metadata that contextualises search results. Depends on D13091.
Test Plan: Ran `diviner generate --repository ARC` and then went to `/diviner/book/arcanist/`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7703, T8352
Differential Revision: https://secure.phabricator.com/D13070
Summary: This drops the Windows-specific escaping code for the creation of directories when acquiring a lease. This is basically the change from D10378 without the other, no longer necessary changes.
Test Plan: This code hasn't been run in a production environment for a while (any instances of Phabricator using Drydock / Harbormaster with Windows have had this code removed by the D10378 patch for a while).
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Projects: #drydock
Differential Revision: https://secure.phabricator.com/D13341
Summary: Ref T2015. This code is only relevant when attempting to run commands on a Windows host over SSH. Since SSH on Windows is extremely fragile and hard to maintain, and WinRM is a better long-term solution, drop this code (which will end up being unused when later diffs introduce the WinRM command interface).
Test Plan: This code won't be used when D10495 lands.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Projects: #drydock
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D13340
Summary: Ref T8320. Fixes T8427. This is still a little funky because Owners has weird name rules, but should fix the bugs (unselectable packages) in T8427.
Test Plan: Browsed Owners typaheads, used various search functions.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8320, T8427
Differential Revision: https://secure.phabricator.com/D13349
Summary:
Fixes T8601. To reproduce the problem:
- Set your time preference to `""` (the empty string). This isn't possible from the modern UI, but can be done with "Right Click > Inspect Element", or users may have carried it forward from an older setting (this is the case with me and @hach-que on this install).
- Load Calendar with some events.
- This parses an epoch, which sets `valueTime` to `""` (since there are no format characters in the preference) and then `getEpoch()` fails because `strlen($time)` is 0.
- Since `getEpoch()` failed, `getDateTime()` also fails.
To fix this:
- Only permit the date and time preferences to have valid values.
Test Plan:
- Loaded page before patch, saw fatal.
- Applied patch.
- No more fatal.
- Viewed tooltips, dates/times, dates/times in other apps.
- Changed my preferences, saw them respected.
Reviewers: lpriestley
Reviewed By: lpriestley
Subscribers: epriestley, hach-que
Maniphest Tasks: T8601
Differential Revision: https://secure.phabricator.com/D13346
Summary:
Fixes T8597. Second issue there is that if you look at a huge file in Diffusion (like `/path/to/300MB.pdf`) we pull the whole thing over Conduit upfront, then try to shove it into file storage.
Instead, pull no more than the chunk limit (normally 4MB) and don't spend more than 10s pulling data.
If we get 4MB of data and/or time out, just fail with a message in the vein of "this is a really big file".
Eventually, we could improve this:
- We can determine the //size// of very large files correctly in at least some VCSes, this just takes a little more work. This would let us show the true filesize, at least.
- We could eventually stream the data out of the VCS, but we can't stream data over Conduit right now and this is a lot of work.
This is just "stop crashing".
Test Plan: Changed limits to 0.01 seconds and 8 bytes and saw reasonable errors. Changed them back and got normal beahvior.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8597
Differential Revision: https://secure.phabricator.com/D13348
Summary: Fixes T8603. For automatic 'order' parameters provided by infrastructure en route to T7715, clarify that they are optional (we will use the default builtin order for the underlying Query if an order is not provided).
Test Plan: Used web UI to see "optional" hint.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8603
Differential Revision: https://secure.phabricator.com/D13342
Summary:
Ref T8597. If a page issues a large query (like inserting a blob into file storage), we may try to utf8ize the entire thing. This is slow and pointless.
Instead, truncate tab data after 4096 bytes before sanitizing.
Test Plan: Adjusted limit to 256 bytes, saw long queries get truncated reasonably.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8597
Differential Revision: https://secure.phabricator.com/D13347
Summary: Closes T8050, Format Calendar list objects
Test Plan: Open Calendar list, check that new formatting is true to mocks.
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8050
Differential Revision: https://secure.phabricator.com/D13318
Summary: Ref T8577, More correct fix - set default value array() for `getParameter` call.
Test Plan: Make sure saved queries like Month View, Day View, and Upcoming Events, still work.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T8577
Differential Revision: https://secure.phabricator.com/D13330
Summary: Fixes T8589. Adds a bunch of new languages to the syntax highlighting config options so that they are supported by #paste.
Test Plan: Saw new filetypes in Paste.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8589
Differential Revision: https://secure.phabricator.com/D13337
Summary:
Ref T8449. Before we show a policy hint in the header of an object, compare it to the space policy (if one exists).
If the space policy is strictly stronger (more restrictive -- for example, the Space policy is 'members of X', and the object policy is 'public'), show the space policy instead.
See discussion on T8376.
Test Plan: {F509126}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8449
Differential Revision: https://secure.phabricator.com/D13328
Summary:
Ref T8575. We run a big "(A) UNION (B)" query on the home page and on the main Differential page.
"A" can always be improved by using `%Ls`, so it can use the second half of the `(authorPHID, status)` key.
"B" can sometimes be improved if the fraction of open revisions is smaller than the fraction of revisions you are reviewing. This is true for me on secure.phabricator.com (I'm a reviewer, either directly or via 'Blessed Reviewers', on about 80% of revisions, but <5% are open). In these cases, a `(status, phid)` key is more efficient.
Test Plan: Tweaked queries and added keys on this server, saw less derpy query plans and performance.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8575
Differential Revision: https://secure.phabricator.com/D13325
Summary:
Ref T8575. Although we cache spaces as a whole, we don't cache viewer spaces. This can still do a lot of work if they have complex policies.
Instead, cache them in the request cache.
Test Plan: Saw this account for 37% of a page in produciton (303ms on on 835ms).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8575
Differential Revision: https://secure.phabricator.com/D13324
Summary: These class properties don't seem to be used.
Test Plan: Eyeball it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13295
Summary: Ref T8575. We check if users can see applications frequently, and caching on the Query isn't especially effective. Use the new Request cache instead.
Test Plan:
- Saw `/feed/` drop 7% (from ~830ms to ~770ms) on profiles.
Reviewers: btrahan, avivey
Reviewed By: avivey
Subscribers: avivey, epriestley
Maniphest Tasks: T8575
Differential Revision: https://secure.phabricator.com/D13321
Summary:
Ref T8574. Currently, failures during mail body construction, feed publishing, or search indexing could cause us to retry the publishing task and potentially send duplicate mail.
Instead, build (but do not send) the mail first, then send all the mail at the very end.
This isn't completley perfect, but should make it enormously harder for duplicate mail to be generated.
Test Plan: Sent some mail, ran the daemons, saw it show up normally in the outbound queue.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8574
Differential Revision: https://secure.phabricator.com/D13320
Summary:
Ref T8574. This could fail because the target is disabled (as here), or doesn't have access to the API, or the API request needs to be satisfied by a different host which isn't available.
In any of these cases, just show the failure and continue generating the mail. This field isn't important enough to block the mail, and many of these errors are permanent.
(I'll follow up on T8574 with some more permanent ideas to address this class of issue.)
Test Plan: Faked API call failure, generated mail, saw clean generation of mail with a failure message.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8574
Differential Revision: https://secure.phabricator.com/D13319
