Summary: pre-patch, these fatal, since we overwrite $content to be just a string so methods fail later in the code. Instead, write a $content_str to keep $content as the proper data.
Test Plan: editing a document and on save it showed me the view page! (as opposed to fataling and staying on the eidt page)
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10426
Summary: Ref T3307. Only one I thought was tricky was Excel; I went with bytes there like it was email.
Test Plan: played around on a few endpoints but mostly thought carefully
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T3307
Differential Revision: https://secure.phabricator.com/D10392
Summary: Adds Phriction to list of apps that use Source Sans as default font in addition to Legalpad and Diviner.
Test Plan: Tested various layouts imported from secure. Should be reasonably tested, but will follow up on secure.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10064
Summary: Ref T5655. Rename `PhabricatorPHIDType` subclasses for clarity (see discussion in D9839). I'm not too keen on some of the resulting class names, so feel free to suggest alternatives.
Test Plan: Ran unit tests.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin, hach-que
Maniphest Tasks: T5655
Differential Revision: https://secure.phabricator.com/D9986
Summary: Provide an implementation for the `getName` method rather than automagically determining the application name.
Test Plan: Saw reasonable application names in the launcher.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10027
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
Summary: Instead of implementing the `getTypeConstant` method in all subclasses of `PhabricatorPHIDType`, provide a `final` implementation in the base class which uses reflection. See D9837 for a similar implementation.
Test Plan: Ran `arc unit`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9985
Summary: Ref T5655. The `PhabricatorDestructibleInterface` interface is misspelled as `PhabricatorDestructableInterface`. Fix the spelling mistake.
Test Plan: `grep`. Seeing as this interface is fairly recent, I don't expect that this would cause any widespread breakages.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5655
Differential Revision: https://secure.phabricator.com/D9988
Summary: Allow `PhrictionDocument` to be permanently deleted with `./bin/remove destroy`.
Test Plan:
Deleted a Phriction document with `./bin/remove` and verified that the database was in the expected state.
```
> ./bin/remove destroy PHID-WIKI-auj57rauigvcqvv5feh6
IMPORTANT: OBJECTS WILL BE PERMANENTLY DESTROYED!
There is no way to undo this operation or ever retrieve this data.
These 1 object(s) will be completely destroyed forever:
- PHID-WIKI-auj57rauigvcqvv5feh6 (PhrictionDocument)
Are you absolutely certain you want to destroy these 1 object(s)? [y/N] y
Destroying objects...
Destroying PhrictionDocument PHID-WIKI-auj57rauigvcqvv5feh6...
Permanently destroyed 1 object(s).
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9976
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
Summary: Remarkup rules can not safely use arbitrary text in tag attributes,
because it may include tokens which are later replaced. Precedence rules
should prevent this in general. Use flat text assertions and adjust precedence
rules in cases where they may not prevent tokens from appearing in attributes.
Auditors: btrahan
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
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.
Test Plan: Eyeballed it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9431
Summary: This went smoother than expeced. Makes the rounded Card the default, also tweaked selected state a little.
Test Plan:
Test UIExamples, Maniphest, Home, Differential, Harbormaster, Audit. Everything seems normal
{F163971}
{F163973}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9408
Summary:
This does some backend cleanup of the tile stuff, and some general cleanup of other application things:
- Users who haven't customized preferences get a small, specific set of pinned applications: Differential, Maniphest, Diffusion, Audit, Phriction, Projects (and, for administrators, Auth, Config and People).
- Old tile size methods are replaced with `isPinnnedByDefault()`.
- Shortened some short descriptions.
- `shouldAppearInLaunchView()` replaced by less ambiguous `isLaunchable()`.
- Added a marker for third-party / extension applications.
Test Plan: Faked away my preferences and viewed the home page, saw a smaller set of default pins.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9358
Summary: Fixes T5250. This needs some general cleanup, but fix the fatal.
Test Plan:
- Viewed moved document.
- Viewed moved-from-nonexistent-source document.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5250
Differential Revision: https://secure.phabricator.com/D9357
Summary:
Merge "Organization" and "Communication" into "Core". The split between these three was always tenuous, and this is easier to use and nicer looking on the new launcher.
Merge "Miscellaneous" into "Utilities" since they're basically the same thing.
Test Plan: Looked at app launcher.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9334
Summary: Fixes T5156. If a document has been moved but the new one does not exist or can't be seen by the viewer, render a generic message.
Test Plan: Viewed moved-plus-visible and moved-plus-nonvisible documents.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5156
Differential Revision: https://secure.phabricator.com/D9254
Summary:
Fixes T5144. This was incorrectly checking the //content// version, not the //head// version, so reverts would raise the "conflict" warning.
Also fix a couple of FontAwesome icons.
Test Plan:
- Edited a document.
- Reverted a document.
- Opened two edit tabs. Edited one, tried to edit #2, got a warning.
- Opened two revert tabs. Reverted in one, tried to revert in #2, got a warning.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5144
Differential Revision: https://secure.phabricator.com/D9249
Summary: Ref T4029. When checking the view policy of a document, require the viewer to also be able to see all of the ancestors.
Test Plan:
- Hard-coded `/x/y/` to "no one".
- Checked that `/x/y/` is not visible.
- Checked that `/x/y/z/` is not visible.
- Checked that `/x/`, `/x/q/`, etc., are still visible.
- Tested project pages and sub-pages for project visibility.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4029
Differential Revision: https://secure.phabricator.com/D9199
Summary: Ref T4029. We use a lot of very outdated content loading in Phriction, which blocks T4029.
Test Plan:
- Called phriction.info
- Called phriction.history
- Called phriction.edit
- Viewed document list.
- Deleted a document.
- Viewed history.
- Viewed a diff.
- Created a document.
- Edited a document.
- Moved a document.
- Tried to overwrite a document with "new".
- Tried to overwrite a document with "move".
- Viewed a moved document note.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: shadowhand, epriestley
Maniphest Tasks: T4029
Differential Revision: https://secure.phabricator.com/D9194
Summary: The removes the sprite sheet 'icons' and replaces it with FontAwesome fonts.
Test Plan:
- Grep for SPRITE_ICONS and replace
- Grep for sprite-icons and replace
- Grep for PhabricatorActionList and choose all new icons
- Grep for Crumbs and fix icons
- Test/Replace PHUIList Icon support
- Test/Replace ObjectList Icon support (foot, epoch, etc)
- Browse as many pages as I could get to
- Remove sprite-icons and move remarkup to own sheet
- Review this diff in Differential
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9052
Summary: Ref T4986. Getting closer. Nothing out of the ordinary in this group.
Test Plan:
For each application:
- Viewed the normal search results.
- Created a panel version and viewed it.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9024
Summary:
Fixes T4917. Currently, if a user doesn't have access to, e.g., Phriction, they still get a checkbox in the search results to search for Wiki Documents. Those results will be filtered anyway, so this is confusing at best.
Instead, bind PHID types to applications. This is a relatively tailored fix; some areas for potential future work:
- Go through every PHID type and bind them all to applications. Vaguely nice to have, but doesn't get us anything for now.
- If no searchable application is installed, we don't show you an error state. This isn't currently possible ("People" is always installed) but in the interest of generality we could throw an exception or something at least.
- The elasticserach thing could probably constrain types to visible types, but we don't have a viewer there easily right now.
Test Plan: Uninstalled Phriction, saw the checkbox vanish.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4917
Differential Revision: https://secure.phabricator.com/D8904
Summary: A small but appreciable number of users find flavor on buttons confusing. Remove this flavor. This retains flavor in headers, error messages, etc., which doesn't cause confusion.
Test Plan: Looked at a revision, task, paste, macro, etc.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8812
Summary: Uses cards, fixes bgcolors.
Test Plan: View edit history on a few documents.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, chad
Differential Revision: https://secure.phabricator.com/D8648
Summary:
- Point them at the new Diviner.
- Make them a little less cumbersome to write.
Test Plan: Found almost all of these links in the UI and clicked them.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8553
Summary:
There are quite a few tests in Arcanist, libphutil and Phabricator that do something similar to `$this->assertEqual(false, ...)` or `$this->assertEqual(true, ...)`.
This is unnecessarily verbose and it would be cleaner if we had `assertFalse` and `assertTrue` methods.
Test Plan: I contemplated adding a unit test for the `getCallerInfo` method but wasn't sure if it was required / where it should live.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8460
Summary:
Ref T4379. I want project subscriptions to work like this (yell if this seems whacky, since it makes subscriptions mean somethign a little different for projects than they do for other objects):
- You can only subscribe to a project if you're a project member.
- When you're added as a member, you're added as a subscriber.
- When you're removed as a member, you're removed as a subscriber.
- While you're a member, you can optionally unsubscribe.
From a UI perspective:
- We don't show the subscriber list, since it's going to be some uninteresting subset of the member list.
- We don't show CC transactions in history, since they're an uninteresting near-approximation of the membership transactions.
- You only see the subscription controls if you're a member.
To do this, I've augmented `PhabricatorSubscribableInterface` with two new methods. It would be nice if we were on PHP 5.4+ and could just use traits for this, but we should get data about version usage before we think about this. For now, copy/paste the default implementations into every implementing class.
Then, I implemented the interface in `PhabricatorProject` but with alternate defaults.
Test Plan:
- Used the normal interaction on existing objects.
- This has no actual effect on projects, verified no subscription stuff mysteriously appeared.
- Hit the new error case by fiddling with the UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T4379
Differential Revision: https://secure.phabricator.com/D8165
Summary: Ref T4365. Two diffs from now, I'm changing the UI a bit to let you search for closed and unowned documents more explcitly. To support this in ElasticSearch and more easily in MySQL search, make these explicit, positive relationships.
Test Plan: `bin/search index --all`
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4365
Differential Revision: https://secure.phabricator.com/D8122
Summary: Fixes T3857. Earlier work made this trivial and just left product questions, which I've answered by requiring the daemons to run on reasonable installs.
Test Plan: Ran `bin/search index` and `bin/search index --background`. Observed indexes write in the former case and tasks queue in the latter case. Commented with a unique string on a revision and searched for it a moment later, got exactly one result (that revision), verifying that reindexing works correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3857
Differential Revision: https://secure.phabricator.com/D7966
Summary: This removes the bulk of the "Form Errors" text, some variations likely exists. These are a bit redundant and space consuming. I'd also like to back ErrorView more into PHUIObjectBox.
Test Plan: Test out the forms, see errors without the text.
Reviewers: epriestley, btrahan
CC: Korvin, epriestley, aran, hach-que
Differential Revision: https://secure.phabricator.com/D7924
Summary: We currently have a lot of calls to `addCrumb(id(new PhabricatorCrumbView())->...)` which can be expressed much more simply with a convenience method. Nearly all crumbs are only textual.
Test Plan:
- This was mostly automated, then I cleaned up a few unusual sites manually.
- Bunch of grep / randomly clicking around.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: hach-que, aran
Differential Revision: https://secure.phabricator.com/D7787
Summary:
These just got copy/pasted like crazy, the base class has the correct default implementation.
(I'm adding "H" for Herald Rules, which is why I was in this code.)
I also documented the existing prefixes at [[ Object Name Prefixes ]].
Test Plan: Verified base implementation. Typed some object names into the jump nav.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Differential Revision: https://secure.phabricator.com/D7785
Summary:
we were just checking if projects/ was in the URI before barfing. Use some more fun utility functions such that we only complain if there is no project.
Fixes T4071.
Test Plan: made a subpage under a project - success! tried to make a project wiki page where there was no project - successful failure! tried to make a project wiki sub page where there was no project - successful failure!
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4071
Differential Revision: https://secure.phabricator.com/D7527
Summary: This adds pht's and such english to Phriction email body.
Test Plan: Edited a Document, Moved a Document. Got new emails. Such Wow.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7477
Summary: Fixes T4028, makes the table a bit easier to read and navigate changesets.
Test Plan: Tested a big history, back to original and latest.
Reviewers: epriestley, btrahan
CC: Korvin, epriestley, aran
Maniphest Tasks: T4028
Differential Revision: https://secure.phabricator.com/D7438
Summary: See title. Fixes T1809.
Test Plan:
verified each type that has flaggable interface still can be flagged
verified that new custom query filter works
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T1809
Differential Revision: https://secure.phabricator.com/D7392
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
Summary:
Fixes T3675.
- Maniphest had a couple of old non-event listeners; move them to events.
- Make most of the similar listeners a little more similar.
- Add checks for access to the application.
Test Plan:
- Viewed profile, project, task, revision.
- Clicked all the actions.
- Blocked access to various applications and verified the actions vanished.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3675
Differential Revision: https://secure.phabricator.com/D7365
Summary:
Ref T3675. Some of these listeners shouldn't do their thing if the viewer doesn't have access to an application (for example, users without access to Differential should not be able to "Edit Tasks"). Set the stage for that:
- Introduce `PhabricatorEventListener`, which has an application.
- Populate this for event listeners installed by applications.
- Rename the "PeopleMenu" listeners to "ActionMenu" listeners, which better describes their modern behavior.
This doesn't actually change any behaviors.
Test Plan: Viewed Maniphest, Differntial, People.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3675
Differential Revision: https://secure.phabricator.com/D7364
