Summary: Minor cleanup, this logic can be simpler. Instead of special-casing inlines as having an effect if the have a comment, just consider any transaction with a comment to have an effect. I'm fairly certain this is always true.
Test Plan: Made inlines, tried to submit empty comments. Behavior unchanged.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18468
Summary:
Ref T5873. See PHI14. I don't want to just expose internal transaction data to Conduit by default, since it's often: unstable, unusable, sensitive, or some combination of the three.
Instead, let ModularTransactions opt in to providing additional data to Conduit, similar to other infrastructure. If a transaction doesn't, the API returns an empty skeleton for it. This is generally fine since most transactions have no real use cases, and I think we can fill them in as we go.
This also probably builds toward T5726, which would likely use the same format, and perhaps simply not publish stuff which did not opt in.
This doesn't actually cover "comment" or "inline comment", which are presumably what PHI14 is after, since neither is modular. I'll probably just put a hack in place for this until they can modularize since I suspect modularizing them here is difficult.
Test Plan: Ran `transaction.search` on a revision, saw some transactions (title and status transactions) populate with values.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5873
Differential Revision: https://secure.phabricator.com/D18467
Summary:
Ref T5873. See PHI14. This does the basics that are shared across everything (IDs, PHIDs, dates, comments).
It doesn't do types (I think I don't necessarily want to expose internal types over the API?) or transaction-specific data.
In the next change, I'm going to add ways to let ModularTransactions "opt-in" to providing more data to Conduit. I'll use this to flesh out the actual desired transaction types (comments, presumably inline comments) and likely leave the rest as skeletons for now until use cases arise so we don't create a backward compatibility issue (or a security issue!) by exposing tons of internal stuff as public-facing API.
Test Plan:
Ran queries, used paging. Retrieved an edited, deleted, and normal comment.
{F5120060}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5873
Differential Revision: https://secure.phabricator.com/D18466
Summary:
Fixes T12970. This is easier than I expected, and appears to occur in only one place.
This prevents a change from ever generating with an anchor like `#12345678`, which is ambiguous because it may be a comment anchor.
Test Plan: Viewed a revision, saw new `change-xxxyyyzzz` anchors, clicked one, got jumped to the right place.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12970
Differential Revision: https://secure.phabricator.com/D18465
Summary:
Ref T12956. After this change, individual users will no longer be able to modify builtin queries on a user-by-user basis: they will always appear at the bottom of the list, under their personal queries, and can only be managed by administrators.
To support this, clean up the old rows which could be hanging around from before: delete any personal saved queries where the saved query is a builtin query.
To ease this transition, try to pin the query we're deleting //if// the user had reordered things to put it on top.
Test Plan:
- Ran the migration, saw no changes in the UI but fewer rows.
- Went back to `master`, reordered queries to put a builtin one on top.
- Ran the migration.
- Saw that builtin one drop to the bottom (since it can't be on top anymore) but be pinned, preserving the behavior of `/maniphest/`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12956
Differential Revision: https://secure.phabricator.com/D18464
Summary:
Ref T12956. UI changes:
- Administrators get a new `[X] Save as global query` option when saving a query.
- "Edit Queries..." is split into "Personal" and "Global" sections. For administrators, each section can be edited. For non-admins, only the top section can be edited, but any query can be pinned.
A couple notes:
- This doesn't support "pin for everyone by default". New users just get the first query from the bottom set. That seems reasonable for now.
- Reordering is currently a little buggy (it works if you've reordered before, but not if you're reordering for the first time), but I need to migrate before I can fix / test that properly. So that'll get cleaned up in the next change or two.
Test Plan:
- As an admin and non-admin, viewed, edited, disabled, saved-as-personal and saved-as-global various queries.
{F5098581}
{F5098582}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12956
Differential Revision: https://secure.phabricator.com/D18426
Summary:
Ref T12956. Currently, when you visit `/maniphest/` (or any other ApplicationSearch application) we execute the first query in the list by default.
In T12956, I plan to make changes so that personal queries are always first, then global/builtin queries. Without changing the "default query" rule, this will make it harder to have, for example, some custom queries in Differential but still run a global query like "Active" by default. To make this work, you'd have to save a personal copy of the "Active" query, then put it at the top.
This feels a bit cumbersome and this rule is kind of implicit and a little weird anyway. To make this work a little better as we make changes here, add an explicit pinning action, like the one we have in Project ProfileMenus.
You can now explicitly choose a query to make default.
Test Plan:
- Browsed without pinning anything, saw normal behavior.
- Pinned queries, viewed `/maniphest/`, saw a non-initial query selected by default.
- Pinned a query, deleted it, nothing exploded.
{F5098484}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12956
Differential Revision: https://secure.phabricator.com/D18422
Summary: This is wierd and I can't think of a use for it? Causing issues on hover states.
Test Plan: Action list, menus, dropdowns, etc. Labels shouldn't have hover states.
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Differential Revision: https://secure.phabricator.com/D18463
Summary:
See PHI42. Currently, `maniphest.search` incorrectly applies this default (group by priority) to all queries via Conduit.
The correct behavior is to apply no grouping constraint.
I think this is also a reasonable general behavior, and the current code seems to date from D6960 in 2013 and didn't seem particularly carefully considered.
This is a minor compatibility break -- saved queries which are more than 4 years old might change their group behavior. I'll note this in the change logs but expect essentially no one to be affected.
Test Plan: Ran a `maniphest.search` Conduit call and observed the underlying query. Before this change, it executed `ORDER BY priority, id`. After this change, it correctly executed `ORDER BY id` only.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18459
Summary: Using icons and dropdown buttons without text looks a little wonky, this resets the CSS a bit.
Test Plan: Review button with icon and text, just icon, just test, and dropdowns.
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Differential Revision: https://secure.phabricator.com/D18461
Summary: These items should be floated, not display: block.
Test Plan: Test blame view with commits AND revisions, check they display inline.
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Differential Revision: https://secure.phabricator.com/D18462
Summary:
Ref T12970. See PHI43. Currently, the "Show Older Comments" link gets auto-clicked if the user visits **any** anchor. This is not correct.
Instead, only auto-click it if the user visits a numeric anchor. This fixes the behavior approximately 98% of the time. See T12970 for a followup on the remaining ambiguous cases.
Test Plan:
- Viewed a revision with some folded transactions and a "Show Older Comments" link.
- Clicked a link to a file in the table of contents, with a hash like `#1234abcd`.
- Before: Timeline expanded and I ended up somewhere bad.
- After: Timeline no longer expanded.
- Manually changed hash to `#1234` (purely numeric), saw timeline expand.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12970
Differential Revision: https://secure.phabricator.com/D18458
Summary:
See PHI39. This adds support for editing parents and subtasks of a task via Conduit.
It might be nice to tie this into the `PhabricatorObjectRelationship` stuff eventually, but I think we'd effectively end up in the same place anyway in terms of what the API looks like.
Test Plan: {F5116163}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18456
Summary: Fixes T12792. Expands the Notifications to "web, desktop, both, or none" for real-time notifications in settings.
Test Plan: Test with "test notifications" button, and while logged into two accounts with each of the 4 settings.
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Maniphest Tasks: T12792
Differential Revision: https://secure.phabricator.com/D18457
Summary: I missed an anchor tag here, adds it back
Test Plan: View blame, click a previous version of the file, click Back to HEAD link.
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Differential Revision: https://secure.phabricator.com/D18451
Summary: Ref T12824, adds more information to the blame view, exposes date, commit summary, lighter colors.
Test Plan:
Review many diffs with and without blame on.
{F5111758}
{F5111759}
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Maniphest Tasks: T12824
Differential Revision: https://secure.phabricator.com/D18452
Summary: Fixes T12969. If you disable "Home" but leave it at the top, we still load it.
Test Plan: Disabled "Home". Move Dashboard into first position, see correct home layout.
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Maniphest Tasks: T12969
Differential Revision: https://secure.phabricator.com/D18455
Summary: From Z1336, we don't currently document anywhere how the default dashboard works. I should also update the copy in the UI. Ref T12969
Test Plan: regenerate docs, read carefully
Reviewers: epriestley
Reviewed By: epriestley
Spies: Korvin
Maniphest Tasks: T12969
Differential Revision: https://secure.phabricator.com/D18454
Summary:
See PHI36. APCu originally had `apc_` methods, but at some point dropped these and only provides `apcu_` methods.
When the `apcu_` method is present, use it. It may not be present for older versions of APCu, so keep the fallback.
Test Plan:
- With modern APCu, clicked "Purge Caches" in Config > Caches.
- Before: fatal on bad `apc_clear_caches` call.
- After: Valid cache clear.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18449
Summary: This adds a very very basic view count to Phame, so bloggers can get some idea which posts are more popular than others. Anything more than this I think should be Facts or Google Analytics.
Test Plan: Write a new post, see post count. Reload page, post count goes up. Archive post, post count stays the same.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18446
Summary: Ref T12964. This feels like a cheat, but works well. Just redirect the user back to the form they came from instead of to the key page.
Test Plan: Add a key to a user profile, add a key to an Alamanac device. Grep for PhabricatorAuthSSHKeyTableView and check all locations.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12964
Differential Revision: https://secure.phabricator.com/D18445
Summary: Moves browseFile to single column, implements Owners as a list under the file (and now directory as well), improved information listed in Owners, and moves actions into the Diffusion action bar instead of the header.
Test Plan:
Test browsing directories, files, text, images, binaries, enabling blame. Mobile and desktop.
{F5111045}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18448
Summary: Adds some basic UI for open / closed state when viewing a list of branches in Mercurial. Fixes T12838
Test Plan: Close and open branches, view list.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12838
Differential Revision: https://secure.phabricator.com/D18447
Summary: Better table layouts here for branches view
Test Plan: Test git, hg repositories. See column go away.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18444
Summary: These can wrap on iPhone 5 screens, no need for so much padding.
Test Plan: iPhone 5 simulator, tablet and mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18437
Summary: This is in the crumbs, but a little hidden. Puts branch name at the top of the browse table header.
Test Plan: Review a few branchs, change branch, see new name.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18441
Summary: Adds an icon for default branch, status for branch status
Test Plan: Review `hg` and `git` repositories, change default branch, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18443
Summary:
Ref T12966. See that task for a description and reproduction steps.
If you put Phabricator in a master/replica configuration and then restart it, we may fatal here if the master is unreachable. Instead, we should survive setup checks.
Test Plan: Put Phabricator in a master/replica configuration, explicitly disabled the master by misconfiguring the port, restarted Phabricator. Before: fatal; after: login screen in read-only mode.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12966
Differential Revision: https://secure.phabricator.com/D18442
Summary:
Ref T12965. See that task for discussion, and PHI36 for context.
This sweeps the fatal under the rug by skipping it, letting things move forward for now.
Test Plan: Followed instructions in T12965, got a read-only recovery after restart instead of a fatal.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12965
Differential Revision: https://secure.phabricator.com/D18440
Summary: Simplifies the page, adds base support for PHUITwoColumn fixed from Instances (which I'll delete css there).
Test Plan:
click on every settings page, UI seems in tact, check mobile, desktop, mobile menus.
{F5102572}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18436
Summary: Moves the method up to DiffusionController, so it can be more universally used. Also now center aligns tabs on mobile. Still todo, get search nicely toggled on mobile
Test Plan: Test mobile, desktop. Test search from home, from browse, and browsing a specific path.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18432
Summary: Moving this down the the "bar" to allow pattern search on home. Rebuilds the mobile layout a little.
Test Plan:
Test actions on mobile, desktop, tablet.
{F5100460}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18431
Summary: Roughs this in a little, kinda basic. Allows for grouping results by page. A bit better on mobile. Would like more content return from conduit though.
Test Plan:
Test `CMS`, `cms`, and `OMGLOLWTFBBQ`, desktop and mobile
{F5099081}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18429
Summary: This is only on browse pages, but I think could be global (home) also. Moves it from a button, field, to just a field.
Test Plan:
Review search on desktop, mobile.
{F5098886}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18428
Summary: Removing this cleanly in event we want to put it back later. 99% of these cases are likely workable either by command line or the typeahead. Will gauge feedback if users notice.
Test Plan: Reload page, perform file grep search.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18425
Summary: I wrote "free" since "test" only give us 7 days to confirm the issue, but "free" no longer exists and "test" should be good enough.
Test Plan: o_O
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D18423
Summary: Uses `fire`, `underline`. Sets text that overflows to ellipsis.
Test Plan:
Test searching for CMS in Phabricator. Check other typeaheads, tokenizers.
{F5098496}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18424
Summary: Getting to the straight browse view went away, this adds a link back. I'll look at more long term solution for getting to grep search.
Test Plan: Click on header, get take to browse view.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18421
Summary: Ref T12956. No real behavioral changes here, just slightly more modern code.
Test Plan: Reviewed named queries in Maniphest and "Edit Queries...".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12956
Differential Revision: https://secure.phabricator.com/D18420
Summary:
Ref T2543. This updates and migrates the status change transactions:
- All storage now records the modern modular transaction ("differential.revision.status"), not the obsolete non-modular transaction ("differential:status").
- All storage now records the modern constants ("accepted"), not the obsolete numeric values ("2").
Test Plan:
- Selected all the relevant rows before/after migration, data looked sane.
- Browsed around, reviewed timelines, no changes after migration.
- Changed revision states, saw appropriate new transactions in the database and timeline rendering.
- Grepped for `differential:status`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18419
Summary:
Ref T2543. Rewrites all the storage to use constants.
Note that transactions still use legacy values, I'll migrate and update them separately.
Test Plan:
- Ran migration.
- Browsed around, changed revision states, viewed dashboard, etc.
- Selected `DISTINCT()` and `GROUP_CONCAT()` of the `status` field in the database, saw sane/expected before and after values.
- Verified that old Conduit methods still return numeric constants for compatibility.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18418
Summary: Ref T2543. All writers now write modern statuses. Make all readers explicit about whether they are reading modern or legacy statuses, so I can swap the storage format.
Test Plan:
- Grepped for `getStatus()`, scanned the list. Other applications have methods with this name so it's possible I missed something.
- Browed around, changed revision statuses.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18417
Summary:
Ref T2543. These are the last `ArcanistDifferentialRevisionStatus` callsites.
This removes the very old legacy `precommitRevisionStatus` field, which has no other readers. This was obsoleted by the `CLOSED_FROM_ACCEPTED` stuff, but retained for compatibility.
Test Plan:
- Poked these with the test console, although they're a little tricky to be sure about.
- Grepped for `ArcanistDifferentialRevisionStatus`, no more hits.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18416
Summary: Ref T2543. Update these for the modern stuff.
Test Plan: Created a new revision, got a revision in the right state ("Needs Review"). Accepted, planned, requested, abandoned revision; state transitions looked good.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18415
Summary: Ref T2543. Swaps these over to modern constants.
Test Plan: Viewed dashboard, no chagnes to bucketing.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2543
Differential Revision: https://secure.phabricator.com/D18414
