Summary: Closes T8023, Calendar event edit view should validate that start time preceeds end time .
Test Plan: Create Calendar event, add details, make end time be earlier than start time, try to save, get error, make sure all previously entered details are populated correctly.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T8023
Differential Revision: https://secure.phabricator.com/D12656
Summary: Fixes T7918. Update hard-coded ApplicationSearch URIs for parameterized typeaheads.
Test Plan: Found all these links and clicked 'em. Probably.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7918
Differential Revision: https://secure.phabricator.com/D12554
Summary:
Ref T4100.
- Make it easy to choose all open or closed tasks.
- Make "special" tokenizers composable.
- Get `viewer()` generating documentation properly.
Test Plan:
- Ran queries with new tokens.
- Browsed new tokens.
- Viewed docs on new tokens.
- Used plain status tokens.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12530
Summary:
Ref T4100.
- Removes the "with unowned" checkbox in favor of the "no owners" function.
- Support functions in "Authors" and "Owners".
Test Plan:
- Ran various global search and Maniphest queries.
{F379931}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12523
Summary:
See M1433. Fixes T7266. Fixes T4475. Ref T7314.
Future work/notes/etc:
- Write the User Guide (see TODO).
- This might needs some design tweaks -- I think it's functionally almost-equivalent to the mock, but the UI isn't quite the same.
- (Mobile design is a touch off-looking I think?)
- When you use a custom query, the duplicate "magnifying glass" icons are a little weird. Maybe change one or the other.
- Maybe worth adding an "Open Documents in Current Application" option? Planning to wait for feedback on that.
- Need a Quicksand integration to change the current application at some point.
- Searching in "Current Application" from, e.g., the 404 page just searches all documents. Current plan is to just document this behavior, since the icon is a pretty good callout and it seems plausible that this is intuitive enough that users won't have a hard time with it.
Test Plan:
New dropdown:
{F379150}
Device-ish:
{F379151}
Normal search (current application, from maniphest, selects tasks):
{F379153}
Application search from non-application:
{F379154}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: johnny-bit, epriestley
Maniphest Tasks: T7266, T7314, T4475
Differential Revision: https://secure.phabricator.com/D12509
Summary: Ref T4100. Let datasources specify a more meaningful title than the class name.
Test Plan: Browsed some sources.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: chad, epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12469
Summary: Ref T4100. Implement members() in terms of the real projects datasource.
Test Plan: Used members() functions in Differential, browsed members() functions.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12459
Summary:
Ref T4100. This is like members(), but is implemented on top of the raw datasource. This is a lot simpler and involves way less code duplication.
I'll go back and implement members() like this, too.
Nothing actually uses this yet.
Test Plan:
- Used browse view to browse datasource.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12458
Summary:
Ref T4100. Ref T5595. This allows PolicyAwareQuery to write all the logic for AND, OR, NOT, and NULL (i.e., "not in any projects") queries against any edge type.
It accepts an edge type and a list of constraints (which are basically just operator-value pairs, like `<NOT, PHID-X-Y>`, meaning the results must not have an edge connecting them to `PHID-X-Y`).
This doesn't actually do anything yet; see future diffs.
Test Plan: `arc unit --everything`
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4100, T5595
Differential Revision: https://secure.phabricator.com/D12455
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
Summary:
Ref T4100. Overall:
- Use token background color to communicate token type (blue = object, yellow = function, grey = disabled/closed, red = invalid).
- Use token icon color to make color choices consistent (specifically, use project icon colors in project tokens).
- For functions, use token icon to communicate function result type (e.g., viewer() has a user icon; members(...) has a group icon), since we don't need the icon to indicate "this is a function" anymore.
Test Plan:
{F374615}
{F374616}
{F374617}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12446
Summary:
Ref T4100. This is still a bit rough around the edges, but mostly does what we're after.
- Implements viewer() and members(...) functions.
- The new browse workflow makes these discoverable.
Test Plan: {F374201}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12444
Summary: Ref T5750. Fix this query so it works better with the browse view. In particular, make it return results before the user types a query.
Test Plan:
- Used a people tokenizer.
- Used global search tokenizer to search for people.
- Ran new version of query in production to make sure it wasn't crazy.
- Browsed through results in browse view.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5750
Differential Revision: https://secure.phabricator.com/D12427
Summary:
Ref T7803. Ref T5873. I want to drive Conduit through more shared infrastructure, but can't currently add parameters automatically.
Put a `getX()` around the `defineX()` methods so the parent can provide default behaviors.
Also like 60% of methods don't define any special error types; don't require them to implement this method. I want to move away from this in general.
Test Plan:
- Ran `arc unit --everything`.
- Called `conduit.query`.
- Browsed Conduit UI.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: hach-que, epriestley
Maniphest Tasks: T5873, T7803
Differential Revision: https://secure.phabricator.com/D12380
Summary:
Ref T7803. Some Query subclasses implement getPagingColumn() in a trivial way, usually to provide a table alias.
Formalize the concept of a primary table alias, and remove obsoleted getPagingColumn() implementations.
Test Plan: Issued affected queries.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7803
Differential Revision: https://secure.phabricator.com/D12356
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
Summary:
Ref T7689, which discusses some of the motivation here. Briefly, these methods are awkward:
- Controller->loadHandles()
- Controller->loadViewerHandles()
- Controller->renderHandlesForPHIDs()
This moves us toward better semantics, less awkwardness, and a more reasonable attack on T7688 which won't double-fetch a bunch of data.
Test Plan:
- Added unit tests.
- Converted one controller to the new stuff.
- Viewed countdown lists, saw handles render.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7689
Differential Revision: https://secure.phabricator.com/D12202
Summary: Renames the method in PHUIObjectBoxView to match the new PHUIInfoView class.
Test Plan: grepped codebase. Went to Calendar and tried a new status.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12005
Summary: Makes this just a plain box (we don't have UI for table-in-form just yet).
Test Plan: Faked some connection data to view the display.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7427
Differential Revision: https://secure.phabricator.com/D11982
Summary:
Currently, PhortuneAccounts have a very open default policy to allow merchants to see and interact with them.
This has the undesirable side effect of leaking their names in too many places, because all users are allowed to load the handles for the accounts. Although this information is not super sensitive, we shouldn't expose it.
I went through about 5 really messy diffs trying to fix this. It's very complicated because there are a lot of objects and many of them are related to PhortuneAccounts, but PhortuneAccounts are not bound to a specific merchant. This lead to a lot of threading viewers and merchants all over the place through the call stack and some really sketchy diffs with OmnipotentUsers that weren't going anywhere good.
This is the cleanest approach I came up with, by far:
- Introduce the concept of an "Authority", which gives a user more powers as a viewer. For now, since we only have one use case, this is pretty open-ended.
- When a viewer is acting as a merchant, grant them authority through the merchant.
- Have Accounts check if the viewer is acting with merchant authority. This lets us easily implement the rule "merchants can see this stuff" without being too broad.
Then update the Subscription view to respect Merchant Authority.
I partially updated the Cart views to respect it. I'll finish this up in a separate diff, but this seemed like a good checkpoint that introduced the concept without too much extra baggage.
This feels pretty good/clean to me, overall, even ignoring the series of horrible messes I made on my way here.
Test Plan:
- Verified I can see everything I need to as a merchant (modulo un-updated Cart UIs).
- Verified I can see nothing when acting as a normal user.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11945
Summary: Since this element isn't strictly about errors, re-label as info view instead.
Test Plan: Grepped for all callsites, tested UIExamples and a few other random pages.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11867
Summary: Feed currently returns nothing is there are no stories, we can present a better view here by allowing a base and customizable set of errors. Fixes T7383
Test Plan:
Test a Project feed with no noDataSting and People with a noDataSting
{F321700}
{F321701}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7383
Differential Revision: https://secure.phabricator.com/D11897
Summary: Fixes T7382, specifically we were drawing double navs, removed those from each page and added the correct CSS rule.
Test Plan: Test a number of people and profile pages with a footer set.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7382
Differential Revision: https://secure.phabricator.com/D11895
Summary: The mobile menu on people profiles has the incorrect order in the URLs and thus, 404s.
Test Plan: Went to a profile on a mobile display, click on feed and calendar links, got to correct place.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11847
Summary: Ref T7263. We need this in the oauth case and otherwise it makes sense to include.
Test Plan: used the conduit console and saw my email address included in the results!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7263
Differential Revision: https://secure.phabricator.com/D11791
Summary:
At least one user wanted to type more text here, and it seems reasonable that administrators may want to write a couple of paragraphs.
I didn't make this short for any particular reason, I just wasn't sure what the workflow would look like as I was building it.
Test Plan: Loaded page, saw normal height text area.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11779
Summary: Fixes T7159.
Test Plan:
Created a legalpad document that needed a signature and I was required to sign it no matter what page I hit. Signed it and things worked! Added a new legalpad document and I had to sign again!
Ran unit tests and they passed!
Logged out as a user who was roadblocked into signing a bunch of stuff and it worked!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7159
Differential Revision: https://secure.phabricator.com/D11759
Summary: Ref T7152. Gives us an event hook so we can go make users a member of any instance they've been invited to as soon as they verify an email address.
Test Plan:
- Used `bin/auth verify` to trigger the event.
- Build out the invite flow in rSERVICES.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7152
Differential Revision: https://secure.phabricator.com/D11752
Summary:
Ref T7152. Ref T1139. This updates Phabricator so third-party libraries can translate their own stuff. Also:
- Hide "All Caps" when not in development mode, since some users have found this a little confusing.
- With other changes, adds a "Raw Strings" mode (development mode only).
- Add an example silly translation to make sure the serious business flag works.
- Add a basic British English translation.
- Simplify handling of translation overrides.
Test Plan:
- Flipped serious business / development on and off and saw silly/development translations drop off.
- Switched to "All Caps" and saw all caps.
- Switched to Very English, Wow!
- Switched to British english and saw "colour".
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7152, T1139
Differential Revision: https://secure.phabricator.com/D11747
Summary:
Ref T7152. This substantially completes the upstream login flow. Basically, we just cookie you and push you through normal registration, with slight changes:
- All providers allow registration if you have an invite.
- Most providers get minor text changes to say "Register" instead of "Login" or "Login or Register".
- The Username/Password provider changes to just a "choose a username" form.
- We show the user that they're accepting an invite, and who invited them.
Then on actual registration:
- Accepting an invite auto-verifies the address.
- Accepting an invite auto-approves the account.
- Your email is set to the invite email and locked.
- Invites get to reassign nonprimary, unverified addresses from other accounts.
But 98% of the code is the same.
Test Plan:
- Accepted an invite.
- Verified a new address on an existing account via invite.
- Followed a bad invite link.
- Tried to accept a verified invite.
- Reassigned an email by accepting an unverified, nonprimary invite on a new account.
- Verified that reassigns appear in the activity log.
{F291493}
{F291494}
{F291495}
{F291496}
{F291497}
{F291498}
{F291499}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7152
Differential Revision: https://secure.phabricator.com/D11737
Summary:
Ref T7152. Ref T3554.
- When an administrator clicks "send invites", queue tasks to send the invites.
- Then, actually send the invites.
- Make the links in the invites work properly.
- Also provide `bin/worker execute` to make debugging one-off workers like this easier.
- Clean up some UI, too.
Test Plan:
We now get as far as the exception which is a placeholder for a registration workflow.
{F291213}
{F291214}
{F291215}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T3554, T7152
Differential Revision: https://secure.phabricator.com/D11736
Summary:
Ref T7152. This implements the administrative UI for the upstream email invite workflow.
Pieces of this will be reused in Instances to implement the instance invite workflow, although some of it is probably going to be a bit copy/pastey.
This doesn't actually create or send invites yet, and they still can't be carried through registration.
Test Plan:
{F290970}
{F290971}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7152
Differential Revision: https://secure.phabricator.com/D11733
Summary:
Ref T7152. This builds the core of email invites and implements all the hard logic for them, covering it with a pile of tests.
There's no UI to create these yet, so users can't actually get invites (and administrators can't send them).
This stuff is a complicated mess because there are so many interactions between accounts, email addresses, email verification, email primary-ness, and user verification. However, I think I got it right and got test coverage everwhere.
The degree to which this is exception-driven is a little icky, but I think it's a reasonable way to get the testability we want while still making it hard for callers to get the flow wrong. In particular, I expect there to be at least two callers (one invite flow in the upstream, and one derived invite flow in Instances) so I believe there is merit in burying as much of this logic inside the Engine as is reasonably possible.
Test Plan: Unit tests only.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7152
Differential Revision: https://secure.phabricator.com/D11723
Summary: Adds core and apps grouping to configuration options, makes it somewhat easier to browse config options.
Test Plan: Set each option, review list. Breakdown is nearly 50/50 apps/core.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11722
Summary: Fixes T7142. Make old permission mean "make (non-bot) users" and then nuance the UI for those administrators who can make bot accounts.
Test Plan: loaded up admin a with full powers and admin b with restricted powers. noted admin a could make a full user. noted admin b could not make a full user. noted admin b got an error even via clever uri hacking.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7142
Differential Revision: https://secure.phabricator.com/D11702
Summary: This got clobbered in D11547. Revive the code but move it up from the base class to the PeopleList controller which is presumably all the main "admin" views. Fixes T7181.
Test Plan: Saw the button once more on /people/...!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7181
Differential Revision: https://secure.phabricator.com/D11698
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
Summary: Ref T7094. We already had and were mostly using "needProfileImage" on the people query class. Only real trick in this diff is deleting a conduit end point that has been marked deprecated for the better part of 3 years.
Test Plan: clicked around the people action and profiles and calendars loaded nicely.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7094
Differential Revision: https://secure.phabricator.com/D11630
Summary: Revamps Profile to be like Projects, a mini portal and side nav with icons.
Test Plan: Viewed my own profile, as well as others. Test seeing my commits, tasks, diffs, and upcoming events. Checked mobile navigation.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11547
Summary: This sets an icon for each config, makes it easier to scan.
Test Plan:
Reload Config page, see all new icons
{F281089}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11619
Summary: Clean up the error view styling.
Test Plan:
Tested as many as I could find, built additional tests in UIExamples
{F280452}
{F280453}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11605
Summary: Ref T3404. The only mildly sketchy bit is these codepaths all load the application email directly, by-passing privacy. I think this is necessary because not getting to see an application doesn't mean you should be able to break the application by registering a colliding email address.
Test Plan:
Tried to add a registered application email to a user account via the web ui and got a pretty error.
Ran unit tests.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T3404
Differential Revision: https://secure.phabricator.com/D11565
Summary: Add a setBorder call to CrumbsView to be more deliberate when a border is drawn. Could not find any CSS hacks to set it conditionally CSS.
Test Plan: Browsed every application that called crumbs and make a design decision. Also fixed a few bad layouts.
Reviewers: btrahan, epriestley
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11533