1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-18 19:40:55 +01:00
Commit graph

76 commits

Author SHA1 Message Date
Christopher Wetherill
5fad7eb1f9 Get line count before truncating Paste snippets
Summary: Fixes T12338. Resolves an issue where long pastes would be truncated before getting a line count, resulting in an inaccurate line count being returned.

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

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T12338

Differential Revision: https://secure.phabricator.com/D17438
2017-03-01 22:30:18 +00:00
Jakub Vrana
a778151f28 Fix errors found by PHPStan
Test Plan: Ran `phpstan analyze -a autoload.php phabricator/src`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D17371
2017-02-17 10:10:15 +00:00
Christopher Wetherill
bee043b163 Display paste line count alongside snippets
Summary: Fixes T11547. I //think// this mostly gets about addressing @epriestley's comments in D16465 and stores each paste's line count in its snippet so that we can display the actual number of lines in the paste rather than '5 Lines'. Let me know if this is on the right track!

Test Plan: Open /paste and see that each paste's actual line count is reported.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T11547

Differential Revision: https://secure.phabricator.com/D17256
2017-01-26 19:06:23 -05:00
epriestley
0f1785c0aa Allow EditEngine to build NUX buttons that point at the right place
Summary:
Fixes T11812.

  - Pull the logic for building the "Create Whatever" dropdown out.
  - Use it to generate NUX buttons, too.
  - Use the new logic in Paste and Maniphest.

Test Plan:
  - Viewed Paste NUX, button worked.
  - Viewed Maniphest NUX with multiple create forms, button worked.

Reviewers: chad, avivey

Reviewed By: avivey

Subscribers: avivey

Maniphest Tasks: T11812

Differential Revision: https://secure.phabricator.com/D16797
2016-11-04 09:51:25 -07:00
Josh Cox
f5537fdff6 Fix the feed line items for autodetect paste languages
Summary:
Fixes T11555. Previously changing the extension of a paste wouldn't change the syntax highlight language. Now it does.
Also, feed items involving autodetect weren't rendering in a readable way.

Test Plan: Created a paste named `paste.php` and let it autodetect language. Then edited the paste to be named paste.rainbow. It should now be highlighted in raiinnboow

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley, yelirekim

Maniphest Tasks: T11555

Differential Revision: https://secure.phabricator.com/D16474
2016-08-29 22:29:29 -04:00
Chad Little
fe5cd4ca2c Move FontIcon calls to Icon
Summary: Normalizes all `setFontIcon` calls to `setIcon`.

Test Plan: UIExamples, Almanac, Apps list, etc.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, hach-que, yelirekim

Differential Revision: https://secure.phabricator.com/D15129
2016-01-28 08:48:45 -08:00
Chad Little
bde9ac43e7 Add various NUX states
Summary: Adds basic NUX UI to Countdown, Paste, Phurl, Ponder, Slowvote, Macro, and Pholio.

Test Plan: Review each with ?nux=true. Click on Create Button.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D14840
2015-12-21 09:55:54 -08:00
epriestley
0282ce74ab Flesh out Conduit types for Paste search fields
Summary: Ref T9964. This fills in types and descriptions for ApplicationSearch fields in Paste.

Test Plan:
Got this nice table now:

{F1023999}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

Differential Revision: https://secure.phabricator.com/D14765
2015-12-14 04:23:28 -08:00
epriestley
05a798e3ac Add basic typechecking support to Conduit
Summary:
Ref T9964. I want to show users what we're expecting in "constraints", and let constraints like "authors=epriestley" work to make things easier.

I'm generally very happy with the "HTTPParameterType" stuff from EditEngine, so add a parallel set of "ConduitParameterType" classes. These are a little simpler than the HTTP ones, but have a little more validation logic.

Test Plan:
This is really just a proof of concept; some of these fields are now filled in:

{F1023845}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

Differential Revision: https://secure.phabricator.com/D14763
2015-12-14 04:21:39 -08:00
Giedrius Dubinskas
421c2453e5 Truncate long source lines in Paste search result list snippets
Summary:
An attempt to resolve T9600.

- `PhabricatorPasteQuery` builds truncated snippet when requested using `needSnippet()`.
- `PhabricatorPasteSearchEngine` uses Paste snippet istead of content.
- `PhabricatorSourceCodeView` accepts truncated source and type instead of line limit.

Test Plan: Generated some content for Paste application and also added huge JSON oneliner. Checked Paste application pages in browser.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin

Maniphest Tasks: T9600

Differential Revision: https://secure.phabricator.com/D14313
2015-10-20 19:07:04 +00:00
Chad Little
3d1783b2da Add application search for Paste status
Summary: Fixes T9076. This adds the ability to search for active, archived, or all Pastes.

Test Plan: Search for active, archived, all Pastes.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9076

Differential Revision: https://secure.phabricator.com/D13809
2015-08-08 09:46:21 -07:00
Joshua Spence
acb1eb81cc Move some PhabricatorSearchField subclasses
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
2015-07-06 22:52:05 +10:00
Chad Little
801607381d [Redesign] PhabricatorApplicationSearchResultView
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
2015-06-19 11:46:20 +01:00
epriestley
bf87976d25 Support ordering in SearchField
Summary:
Ref T8441. Ref T7715. Automatically generate a modern "Order" control in ApplicationSearch for engines which fully support SearchField.

Notably, this allows the standard "Order" control to automatically support custom field orders. We do this in Maniphest today, but in an ad-hoc way.

Test Plan: Performed order-by queries in Almanac (Services), Pholio, Files, People, Projects, and Paste.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13193
2015-06-08 12:21:48 -07:00
epriestley
3cdaf52ce9 Make Subscribers automatically provide working SearchFields
Summary:
Ref T8441. Ref T7715. For modern Query classes, automatically make subscriber queries and SearchField integrations work.

In particular, we can just drive this query with EdgeLogic and don't need to do anything specific on these Query classes beyond making sure they're implemented in a way that picks up all of the EdgeLogic clauses.

Test Plan:
  - Searched for subscribers in Pholio, Files, Paste, and Projects.
  - Searched for all other fields in Projects to check that Query changes are OK.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13191
2015-06-08 12:20:53 -07:00
Joshua Spence
259b40fc28 Fix some method visibilities
Summary: See D13185, the `testMethodVisibility()` test was not being executed.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13186
2015-06-08 09:52:14 +10:00
epriestley
2492fef029 Move Pholio to SearchFields
Summary:
Ref T8441. Ref T7715.

  - Update PholioSearchEngine.
  - Automatically add project fields.
  - Update Paste to support project search.
  - Simplify common Query class construction.

Test Plan:
  - Searched for pastes.
  - Searched for mocks.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13174
2015-06-07 07:31:28 -07:00
epriestley
b5dfd34e4a Support PhabricatorSpacesInterface in ApplicationSearch UI
Summary: Ref T8441. Does what it says, provided other conditions (like using the new SearchField stuff) are fulfilled.

Test Plan:
{F473836}

{F473837}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8441

Differential Revision: https://secure.phabricator.com/D13171
2015-06-05 11:21:45 -07:00
epriestley
65b891b095 Move some value massaging into SearchFields
Summary: Ref T8441. Ref T7715. Let SearchFields do some value massaging before buiding queries so we don't have to work as hard in each SearchEngine. Particularly, they can handle evaluateTokens() from Tokenizers.

Test Plan: Paste automatically gained access to `viewer()`, etc.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13170
2015-06-05 11:21:22 -07:00
epriestley
08966ba311 Begin modularizing fields in SearchEngines
Summary:
Ref T8441. Ref T7715. Add a layer of indirection to fields in search engines. This will allow us to:

  - Simplify SearchEngine code, which has collected a lot of duplication around expressing what is effectively field types.
  - Automatically add fields like "Spaces" and "Projects" (primary driver for T8441).
  - Reorder or hide fields (not sure if we really want to do this, but it seems plausible, and this will let us play around with it, at least).
  - Drive Conduit Query methods via SearchEngines, so the same code specifies both the search UI and the `application.query` endpoint (primary driver in T7715).

Test Plan:
  - Searched for stuff in Paste, everything behaved exaclty like it used to (except that I removed the "no language" checkbox, which seemed like fluff from a bygone era).
  - Searched for stuff in other applications, saw no changes.
  - Hit date field errors.
  - Used query strings to specify values.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

Differential Revision: https://secure.phabricator.com/D13169
2015-06-05 11:21:08 -07:00
epriestley
c1c897b961 Provide core policy support for Spaces
Summary:
Ref T8424. No UI or interesting behavior yet, but integrates Spaces checks:

  - `PolicyFilter` now checks Spaces.
  - `PolicyAwareQuery` now automatically adds Spaces constraints.

There's one interesting design decision here: **spaces are stronger than automatic capabilities**. That means that you can't see a task in a space you don't have permission to access, //even if you are the owner//.

I //think// this is desirable. Particularly, we need to do this in order to exclude objects at the query level, which potentially makes policy filtering for spaces hugely more efficient. I also like Spaces being very strong, conceptually.

It's possible that we might want to change this; this would reduce our access to optimizations but might be a little friendlier or make more sense to users later on.

For now, at least, I'm pursuing the more aggressive line. If we stick with this, we probably need to make some additional UI affordances (e.g., show when an owner can't see a task).

This also means that you get a hard 404 instead of a policy exception when you try to access something in a space you can't see. I'd slightly prefer to show you a policy exception instead, but think this is generally a reasonable tradeoff to get the high-performance filtering at the Query layer.

Test Plan:
  - Added and executed unit tests.
  - Put objects in spaces and viewed them with multiple users.
  - Made the default space visible/invisible, viewed objects.
  - Checked the services panel and saw `spacePHID` constraints.
  - Verified that this adds only one query to each page.

Reviewers: btrahan, chad

Reviewed By: btrahan

Subscribers: chad, epriestley

Maniphest Tasks: T8424

Differential Revision: https://secure.phabricator.com/D13156
2015-06-04 17:46:32 -07:00
epriestley
763b63a0fb Add spacePHID infrastructure and implement in Paste
Summary:
Ref T8424. I'm using Paste as a testbed application because Spaces make some degree of sense for it but it's also flat/simple.

This doesn't do anything interesting or useful and mostly just making the next (more interesting) diff smaller.

Test Plan:
  - Ran `bin/storage upgrade -f`.
  - Browsed pastes.
  - Created a paste.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8424

Differential Revision: https://secure.phabricator.com/D13154
2015-06-04 17:45:24 -07:00
epriestley
b0862a8176 Filter pastes at the query level if we're unable to load their content
Summary:
Fixes T8230. I think I broke this in D12354 since I missed the fact that this could filter results, because it's constructed in a sort of weird way. Try to make the logic a little more clear, while retaining the relevant properties:

  - Pastes with unloadable content should be discarded;
  - the input order should be retained in the result set.

Test Plan: Faked content misses and saw things work instead of fatal.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8230

Differential Revision: https://secure.phabricator.com/D12909
2015-05-18 13:57:20 -07:00
epriestley
f5580c7a08 Make buildWhereClause() a method of AphrontCursorPagedPolicyAwareQuery
Summary:
Ref T4100. Ref T5595.

To support a unified "Projects:" query across all applications, a future diff is going to add a set of "Edge Logic" capabilities to `PolicyAwareQuery` which write the required SELECT, JOIN, WHERE, HAVING and GROUP clauses for you.

With the addition of "Edge Logic", we'll have three systems which may need to build components of query claues: ordering/paging, customfields/applicationsearch, and edge logic.

For most clauses, queries don't currently call into the parent explicitly to get default components. I want to move more query construction logic up the class tree so it can be shared.

For most methods, this isn't a problem, but many subclasses define a `buildWhereClause()`. Make all such definitions protected and consistent.

This causes no behavioral changes.

Test Plan: Ran `arc unit --everything`, which does a pretty through job of verifying this statically.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: yelirekim, hach-que, epriestley

Maniphest Tasks: T4100, T5595

Differential Revision: https://secure.phabricator.com/D12453
2015-04-20 10:06:09 -07:00
epriestley
e6174ed45c Fix an issue where pastes could be reordered as a side effect of cache fills
Summary: Ref T7803. Pastes which needed a cache fill would incorrectly be dropped to the bottom of the list. Stop doing that.

Test Plan: Loaded a list of pastes with some that needed cache fills, saw them appear in the correct order.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12354
2015-04-13 11:58:16 -07:00
epriestley
d403700e1f Convert all tokenizers to take token/scalar inputs
Summary: Ref T7689. Ref T4100. This advances the goals of removing `loadViewerHandles()` (only 67 callsites remain!) and letting tokenizers some day take token functions like `viewer()` and `members(differential)`.

Test Plan:
- Sent a new message; used "To".
  - I simplified the cancel URI construction slightly because it's moot in all normal cases.
- Edited a thread; used "Add Participants".
- Searched rooms; used "Participants".
- Searched countdowns; used "Authors".
- Created a diff; used "Repository".
- Edited a revision; edited "Projects"; edited "Reveiwers"; edited "Subscribers".
- Searched for revisions; edited "responsible users"; "authors"; "reviwers"; "subscribers"; "repositories".
- Added revision comments; edited "Add Reveiwers"; "Add Subscribers".
- Commented on a commit; edited "Add Auditors"; "Add subscribers".
- Edited a commit; edited "Projects".
- Edited a repository; edited "Projects".
- Searched feed, used "include Users"; "include Proejcts".
- Searched files, used "authors".
- Edited initiative; edited "Projects".
- Searched backers; used "Backers".
- Searched initiatives; used "Owners".
- Edited build plans; edited "Run Command".
- Searched Herald; used "Authors".
- Added signature exemption in Legalpad.
- Searhced legalpad; used "creators"; used "contributors".
- Searched signatures; used "documents"; used "signers".
- Created meme.
- Searched macros; used "Authors".
- Used "Projects" in Maniphest reports.
- Used Maniphest comment actions.
- Edited Maniphest tasks; edited "Assigned To"; edited "CC"; edited "projects".
- Used "parent" in Maniphest task creation workflow.
- Searched for projects; used "assigned to"; "in any projec"; "in all projects"; "not in projects"; "in users' projects"; "authors"; "subscribers".
- Edited Maniphest bug filing domains, used "Default Author".
- Searched for OAuth applications, used "Creators".
- Edited Owners pacakge; edited "Primary Owner"; edited "Owners".
- Searched for Owners packages; used "Owner".
  - OMG this UI is OLD
- Edited a paste; edited "Projects".
- Searched for paste; used "Authors".
- Searched user activity log; used "Actors"; used "Users".
- Edited a mock; edited "Projects"; edited "CC".
- Searched for mocks; used "Authors".
- Edited Phortune account; edited "Members".
- Edited Phortune merchant account; edited "Members".
- Searched Phrequent; used "Users".
- Edited Ponder question; sued "projects".
- Searched Ponder; used "Authors"; used "Answered By".
- Added project members.
- Searched for projects; used "Members".
- Edited a Releeph product; edited "Pushers".
- Searched pull requests; searched "Requestors".
- Edited an arcanist project; used "Uses Symbols From".
- Searhced push logs; used "Repositories"; used "Pushers".
- Searched repositories; used "In nay project".
- Used global search; used Authors/owners/Subscribers/In Any Project.
- Edited a slowvote; used "Projects".
- Searched slovotes; used "Authors".
- Created a custom "Users" field; edited and searched for it.
- Made a whole lot of typos in this list. ^^^^^^

Did not test:

- Lint is nontrivial to test locally, I'll test it in production.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100, T7689

Differential Revision: https://secure.phabricator.com/D12224
2015-03-31 14:10:55 -07:00
Bob Trahan
5a9df1a225 Policy - filter app engines where the user can't see the application from panel editing
Summary: Fixes T7118. This does the basic "filter the list" thing, though it ends up being a little manual since I guess this hasn't come up before? There is also potential weird behavior if the user was using an app and lost access to it - they will have nothing selected on edit - but I think this is actually correct behavior in this circumstance.

Test Plan:
used a user who couldn't get access to the "quick create" apps and noted that the dropdown list on dashboard panel create was missing the expected engines

ran `arc unit --everything` to verify abstract method implemented everywhere

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7118

Differential Revision: https://secure.phabricator.com/D11687
2015-02-04 15:47:48 -08:00
Joshua Spence
e448386d39 Fix method visibility for PhabricatorApplicationSearchEngine methods
Summary: Ref T6822.

Test Plan: Visual inspection. These methods are only called from within the `PhabricatorApplicationSearchEngine` class.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11242
2015-01-07 07:34:52 +11:00
Bob Trahan
3bc391fbc7 Paste - add created time to pastes in a list
Summary: Fixes T5943. We do this differently in different places; Audit / Differential do something like this while Pholio expands the "byLine" to include a timestamp. Go with the Audit / Differential approach, as presumably having the date as a top line, easily scannable metadata is the goal here.

Test Plan: viewed a list of pastes and saw a timestamp of creation at the top.

Reviewers: epriestley, chad

Reviewed By: chad

Subscribers: epriestley, Korvin

Maniphest Tasks: T5943

Differential Revision: https://secure.phabricator.com/D10338
2014-08-22 13:15:49 -07:00
Bob Trahan
ed98a1cc84 Paste - fix caching mechanism for S3-stored files
Summary: Fixes T5798. We basically weren't using the caching mechanism. Also adds service calls for S3 stuff, and support for seeing a little info like you can for conduit.

Test Plan: uploaded a paste, looked at paste list - no s3 service calls. edited the paste, looked at paste list - no s3 service calls and edited content properly shown

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5798

Differential Revision: https://secure.phabricator.com/D10294
2014-08-19 12:01:17 -07:00
Joshua Spence
86c399b657 Rename PhabricatorApplication subclasses
Summary: Ref T5655. Some discussion in D9839. Generally speaking, `Phabricator{$name}Application` is clearer than `PhabricatorApplication{$name}`.

Test Plan:
# Pinned and uninstalled some applications.
# Applied patch and performed migrations.
# Verified that the pinned applications were still pinned and that the uninstalled applications were still uninstalled.
# Performed a sanity check on the database contents.

Reviewers: btrahan, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: hach-que, epriestley, Korvin

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D9982
2014-07-23 10:03:09 +10:00
epriestley
dcc6997793 Modernize "users" typeahead datasource
Summary: Ref T4420. Modernize users.

Test Plan:
- Edited "Commit Authors" on Audit search.
- Edited "Created By" on calendar search.
- Edited "invited" on calendar search.
- Edited "To" on "New conpherence message".
- Edited user on "Add user to conpherence thread".
- Edited "Authors" on countdown search.
- Edited "Author" on differential search.
- Edited "Responsible users" on differential search.
- Edited "Owner" on Diffusion lint search.
- Edited "include users" on Feed search.
- Edited "Authors" on file search.
- Edited "Authors" on Herald rule search.
- Edited a couple of user-selecting Herald fields on rules.
- Edited "user" on legalpad signature exemption.
- Edited "creator" on legalpad search.
- Edited "contributors" on legalpad search.
- Edited "signers" on legalpad signature search.
- Edited "Authors" on macro search.
- Edited "Reassign/claim" on task detail.
- Edited "assigned to" on task edit.
- Edited "assigned to", "users projects", "authors" on task search.
- Edited "creators" on oauthserver.
- Edited "authors" on paste search.
- Edited "actors" and "users" on activity log search.
- Edited "authors" on pholio search.
- Edited "users" on phrequent search.
- Edited "authors", "answered by" on Ponder search.
- Edited "add members" on project membership editor.
- Edited "members" on project search.
- Edited "pushers" on releeph product edit.
- Edited "requestors" on releeph request search.
- Edited "pushers" on diffusion push log.
- Edited "authors", "owners", "subscribers" on global search.
- Edited "authors" on slowvote search.
- Edited users in custom policy.
- Grepped for "common/authors", no hits.
- Grepped for "common/users", no (relevant) hits.
- Grepped for "common/accounts", no (relevant) hits.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4420

Differential Revision: https://secure.phabricator.com/D9885
2014-07-17 15:44:18 -07:00
Joshua Spence
8756d82cf6 Remove @group annotations
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.

Test Plan: Eye-ball it.

Reviewers: #blessed_reviewers, epriestley, chad

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9859
2014-07-10 08:12:48 +10:00
epriestley
b8bc0aa2b0 Allow users to select QueryPanel search engines from a list
Summary: Ref T4986. Instead of requiring users to know the name of an application search engine class, let them select from a list.

Test Plan:
Created a new panel.

{F165468}

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4986

Differential Revision: https://secure.phabricator.com/D9500
2014-06-12 13:22:20 -07:00
epriestley
d653fa13de Move paste rendering into SearchEngine
Summary:
Ref T4986. One note:

  - I've disabled syntax highlighting in the previews. When we miss caches this is just way way too slow and has frustrated me several times in the past. The value of syntax highlighting these snippets is not huge. We could maybe ajax this in or use it //if// we get a cache hit in the future, but just kill it for the moment.

Test Plan: Viewed pastes. Created a paste panel.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4986

Differential Revision: https://secure.phabricator.com/D9138
2014-05-15 19:17:22 -07:00
epriestley
bd7420c4bb Allow pastes to be edited
Summary: Fixes T4814.

Test Plan: Edited pastes from the web UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4814

Differential Revision: https://secure.phabricator.com/D8970
2014-05-04 11:11:46 -07:00
epriestley
2a5c987c71 Lock policy queries to their applications
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.

This has several parts:

  - For PolicyAware queries, provide an application class name method.
  - If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
  - For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.

Test Plan:
  - Added a unit test to verify I got all the class names right.
  - Browsed around, logged in/out as a normal user with public policies on and off.
  - Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7367
2013-10-21 17:20:27 -07:00
epriestley
8c1c6fec5a Modernize policies in Paste and Macro
Summary:
Ref T603. Fixes T2823. This updates Paste and Macro.

  - **Paste**
    - Added default view policy.
    - I didn't add a "create" policy, since I can't come up with any realistic scenario where you'd give users access to pastes but not let them create them.
  - **Macro**
    - Added a "manage" policy, which covers creating and editing macros. This lets an install only allow "People With An Approved Sense of Humor" or whatever to create macros.
    - Removed the "edit" policy, since giving individual users access to specific macros doesn't make much sense to me.
    - Changed the view policy to the "most public" policy the install allows.
    - Added view policy information to the header.

Also fix a couple of minor things in Maniphest.

Test Plan:
  - Set Paste policy, created pastes via web and Conduit, saw they got the right default policies.
  - Set Macro policy, tried to create/edit macros with valid and unauthorized users.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2823, T603

Differential Revision: https://secure.phabricator.com/D7317
2013-10-16 10:35:52 -07:00
epriestley
c4abf160cc Fix some file policy issues and add a "Query Workspace"
Summary:
Ref T603. Several issues here:

  1. Currently, `FileQuery` does not actually respect object attachment edges when doing policy checks. Everything else works fine, but this was missing an `array_keys()`.
  2. Once that's fixed, we hit a bunch of recursion issues. For example, when loading a User we load the profile picture, and then that loads the User, and that loads the profile picture, etc.
  3. Introduce a "Query Workspace", which holds objects we know we've loaded and know we can see but haven't finished filtering and/or attaching data to. This allows subqueries to look up objects instead of querying for them.
    - We can probably generalize this a bit to make a few other queries more efficient. Pholio currently has a similar (but less general) "mock cache". However, it's keyed by ID instead of PHID so it's not easy to reuse this right now.

This is a bit complex for the problem being solved, but I think it's the cleanest approach and I believe the primitive will be useful in the future.

Test Plan: Looked at pastes, macros, mocks and projects as a logged-in and logged-out user.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7309
2013-10-14 14:36:06 -07:00
epriestley
0598600476 Always pass handles to tokenizers, not <phid -> name> maps
Summary: Ref T1279. Prerequisite for adding icons or other type information to tokenizers, since we don't currently have enough information to prefill them when rendering things from the server side. By passing handles in, the tokenizer can extract type information.

Test Plan:
- Searched by user in Audit.
- Sent Conpherence from profile page.
- Tried to send an empty conpherence.
- Searched Countdown by user.
- Edited CCs in Differential.
- Edited reviewers in Differential.
- Edited a commit's projects.
- Searched lint by owner.
- Searched feed by owner/project.
- Searched files by owner.
- Searched Herald by owner.
- Searched Legalpad by owner.
- Searched Macro by owner.
- Filtered Maniphest reports by project.
- Edited CCs in Maniphest.
- Searched Owners by owner.
- Edited an Owners package.
- Searched Paste by owner.
- Searched activity logs by owner.
- Searched for mocks by owner.
- Edited a mock's CCs.
- Searched Ponder by owner.
- Searched projects by owner.
- Edited a Releeph project's pushers.
- Searched Releeph by requestor.
- Edited "Uses Symbols" for an Arcanist project.
- Edited all tokenizers in main search.
- Searched Slowvote by user.

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1279

Differential Revision: https://secure.phabricator.com/D7248
2013-10-07 12:51:24 -07:00
epriestley
13dae05193 Make most file reads policy-aware
Summary: Ref T603. Swaps out most `PhabricatorFile` loads for `PhabricatorFileQuery`.

Test Plan:
  - Viewed Differential changesets.
  - Used `file.info`.
  - Used `file.download`.
  - Viewed a file.
  - Deleted a file.
  - Used `/Fnnnn` to access a file.
  - Uploaded an image, verified a thumbnail generated.
  - Created and edited a macro.
  - Added a meme.
  - Did old-school attach-a-file-to-a-task.
  - Viewed a paste.
  - Viewed a mock.
  - Embedded a mock.
  - Profiled a page.
  - Parsed a commit with image files linked to a revision with image files.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7178
2013-09-30 09:38:13 -07:00
Bob Trahan
b902005bed Kill PhabricatorObjectDataHandle
Summary: Ref T603. Killing this class is cool because the classes that replace it are policy-aware. Tried to keep my wits about me as I did this and fixed a few random things along the way. (Ones I remember right now are pulling a query outside of a foreach loop in Releeph and fixing the text in UIExample to note that the ace of hearts if "a powerful" card and not the "most powerful" card (Q of spades gets that honor IMO))

Test Plan: tested the first few changes (execute, executeOne X handle, object) then got real mechanical / careful with the other changes.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran, FacebookPOC

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D6941
2013-09-11 12:27:28 -07:00
epriestley
f1c75a6382 Allow construction of ApplicationSearch queries with GET
Summary:
Ref T3775 (discussion here). Ref T2625.

T3775 presents two problems:

  # Existing tools which linked to `/differential/active/epriestley/` (that is, put a username in the URL) can't generate search links now.
  # Humans can't edit the URL anymore, either.

I think (1) is an actual issue, and this fixes it. I think (2) is pretty fluff, and this doesn't really try to fix it, although it probably improves it.

The fix for (1) is:

  - Provide a helper to read a parameter containing either a list of user PHIDs or a list of usernames, so `/?users[]=PHID-USER-xyz` (from a tokenizer) and `/?users=alincoln,htaft` (from an external program) are equivalent inputs.
  - Rename all the form parameters to be more digestable (`authorPHIDs` -> `authors`). Almost all of them were in this form already anyway. This just gives us `?users=alincoln` instead of `userPHIDs=alincoln`.
  - Inside ApplicationSearch, if a request has no query associated with it but does have query parameters, build a query from the request instead of issuing the user's default query. Basically, this means that `/differential/` runs the default query, while `/differential/?users=x` runs a custom query.

Test Plan: {F56612}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625, T3775

Differential Revision: https://secure.phabricator.com/D6840
2013-08-29 11:52:29 -07:00
Bob Trahan
37a5c4b11a Paste - add transactions
Summary: Ref T3650. This adds a create transaction, transactions for metadata (title, langauge, view policy), and comments. Editor is used on all create /edit paths.

Test Plan: made some pastes via web and email - yay. edited pastes - yay. verified txns showed up on pastes and in feed correctly.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3516, T3650

Differential Revision: https://secure.phabricator.com/D6645
2013-08-02 12:56:58 -07:00
epriestley
9383abc6b0 Remove unnecessary empty checks from willFilterPage()
Summary: Fixes T3600. These checks are obsolete after D6512.

Test Plan: Syntax / static / inspection.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3600

Differential Revision: https://secure.phabricator.com/D6563
2013-07-24 15:30:26 -07:00
epriestley
b483c11d46 Fix a bug where PhabricatorPasteQuery incorrectly rekeys results
Summary: I made changes here recently to improve robustness in the presence of missing files, but accidentally caused the results to re-key. Some callers depend on the mapping, and every other query is consistent about it. Restore the original behavior.

Test Plan: `Pnnn` works again in remarkup.

Reviewers: dctrwatson, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6134
2013-06-05 11:49:32 -07:00
epriestley
fb765b8c93 Add language and date ranges to Paste queries
Summary:
Ref T2625. Ref T3273. This is mostly a UI foil for T3273. Right now, to find tasks without owners or without projects you search for the magic strings "upforgrabs" and "noproject". Unsurprisingly, no users have ever figured this out. I want to get rid of it. Instead, these interfaces will look like:

      Assigned: [ Type a user name... ]
                [ X ] Find unassigned tasks.
      Projects: [ Type a project name... ]
                [ X ] Find tasks with no projects.

Seems reasonable, I think?

Test Plan: Searched for "rainbow, js", "rainbow + no language", "no language", date ranges, etc.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T2625, T3273

Differential Revision: https://secure.phabricator.com/D6085
2013-05-30 18:55:04 -07:00
epriestley
d82e135cde Implement generalized application search in Macros
Summary: Ref T2625. Works out the last kinks of generalization and gives Macros the more powerful new query engine. Overall, this feels pretty good to me.

Test Plan: Executed, saved and edited a bunch of Macro queries.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6078
2013-05-30 14:09:37 -07:00
epriestley
5d94a8a338 Use delegation to generalize application search controllers
Summary:
Ref T2625. Lifts almost all of the search logic out of Paste controllers and into Search.

This uses controller delegation for generalization. We use this in a few places, but don't use it very much yet. I think it's pretty reasonable as-is, but I might be able to make even more stuff free.

There are some slightly rough edges around routes, still, but I want to hit Phame and Differential (which both have multiple application search engines) before trying to generalize that.

Test Plan: Executed, browsed and managed Paste searches.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6073
2013-05-30 14:09:02 -07:00
epriestley
b91c863780 When a paste's file is deleted, drop it from query results
Summary:
We allow files to be deleted from the web UI, including paste files. When a paste's file is deleted, we currently continue to show it in the paste list and then 400 when it's clicked on.

Instead, remove it from the list. (Note that it may stick around if it's cached.)

Test Plan: Purged general cache, deleted a paste's file, viewed paste list, saw no pastes.

Reviewers: dctrwatson, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3265

Differential Revision: https://secure.phabricator.com/D6067
2013-05-29 06:28:47 -07:00