Summary:
Ref T10054. This does a big chunk of the legwork to let users reconfigure profile menus (currently, just project menus).
This includes:
- Editing builtin items (e.g., you can rename the default items).
- Creating new items (for now, only links are available).
This does not yet include:
- Hiding items.
- Reordering items.
- Lots of fancy types of items (dashboards, etc).
- Any UI changes.
- Documentation (does feature: TODO link for documentation).
Test Plan:
{F1060695}
{F1060696}
{F1060697}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15010
Summary: Ref T10054. This mostly makes sure that mobile gets to have the same profile menu that Desktop does.
Test Plan:
- Viewed menus on mobile, saw all profile menu actions available.
- Viewed/used changed pages (column detail, edit column, edit picture).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15008
Summary: Ref T10054. This has no product impact, but prepares us for customizable side nav on "profiles" (today, projects; probably users some day; and maybe other stuff down the road).
Test Plan: Clicked all links on a profile, everything was exactly the same as before.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15007
Summary: Fixes T10096. The `**` was being parsed as part of the hashtag, so `**#asdf**` interpreted `#asdf**` as a hashtag.
Test Plan: Unit test; bolded stuff with hashy contents.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10096
Differential Revision: https://secure.phabricator.com/D15006
Summary:
Fixes T10135. When the viewer is a member of no projects, specify the constraint type as a new "EMPTY" type.
When a query has an "EMPTY" constraint, fail fast with no results.
Test Plan:
- Viewed a viewerprojects() query result set as a user in no projects.
- Before patch: got a lot of hits. After patch: no hits.
- Viewed a normal result set, no changes.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10135
Differential Revision: https://secure.phabricator.com/D15003
Summary: A lot of confusing stuff happens when you create a milestone or subproject which isn't explained clearly in the UI. This is causing more harm than good on the balance since we're still figuring out how to move forward here. Just turn it off for now until we're closer to pushing it forward.
Test Plan: Viewed a project, no more UI for subprojects or milestones.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14999
Summary: I missed this in the recent icon customziation thing.
Test Plan: Typehaead'ed some projects, saw icons properly.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14975
Summary:
Ref T10010. Ref T5819. General alignment of the stars:
- There were some hacks in Conduit around stripping `fa-...` off icons when reading and writing that I wanted to get rid of.
- We probably have room for a subtitle in the new heavy nav, and using the icon name is a good starting point (and maybe good enough on its own?)
- The project list was real bad looking with redundant tag/names, now it is very slightly less bad looking with non-redundant types?
- Some installs will want to call Milestones something else, and this gets us a big part of the way there.
- This may slightly help to reinforce "tag" vs "policy" vs "group" stuff?
---
I'm letting installs have enough rope to shoot themselves in the foot (e.g., define 100 icons). It isn't the end of the world if they reuse icons, and is clearly their fault.
I think the cases where 100 icons will break down are:
- Icon selector dialog may get very unwieldy.
- Query UI will be pretty iffy/huge with 100 icons.
We could improve these fairly easily if an install comes up with a reasonable use case for having 100 icons.
---
The UI on the icon itself in the list views is a little iffy -- mostly, it's too saturated/bold.
I'd ideally like to try either:
- rendering a "shade" version (i.e. lighter, less-saturated color); or
- rendering a "shade" tag with just the icon in it.
However, there didn't seem to be a way to do the first one right now (`fa-example sh-blue` doesn't work) and the second one had weird margins/padding, so I left it like this for now. I figure we can clean it up once we build the thick nav, since that will probably also want an identical element.
(I don't want to render a full tag with the icon + name since I think that's confusing -- it looks like a project/object tag, but is not.)
Test Plan:
{F1049905}
{F1049906}
Reviewers: chad
Reviewed By: chad
Subscribers: 20after4, Luke081515.2
Maniphest Tasks: T5819, T10010
Differential Revision: https://secure.phabricator.com/D14918
Summary:
Fixes T10089. This did work at one point, but was broken by D12868, which got too aggressive about mailing members.
We don't want to send mail to all members by default, only those who are subscribed. The parent implementation of `getMailCC()` handles this for us.
Test Plan:
Joined a project as users A and B. Unsubscribed with B. Made an edit.
Before patch: both A and B got mail. After patch: only A got mail.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10089
Differential Revision: https://secure.phabricator.com/D14955
Summary:
Ref T10010.
Currently, milestone subproject have editable icons/colors, but I don't think this is likely to be used much (the expectation is that milestones will be common and homogenous, and it doesn't make much sense to pick different icons for "Sprint 32" vs "Sprint 33", I think).
Locking the icon and color lets us simplify the form, make milestones more distinct, and potentially reuse the color later for other things (e.g., active/future/past or on time / overdue or whatever else) or just give them a special color to make them more visible.
The best argument for retaining this that I can come up with is that certain milestones may be special (e.g., Sprint 19 is a major release?), but you can just name it "Sprint 19 (v3.0!)" or something, which seems pretty good for now.
Also don't show milestones on task browse/list view.
Test Plan: {F1048532}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14912
Summary: Ref T10004. This control doesn't disable visually or behaviorally, e.g. when locked in an EditEngine configuration.
Test Plan:
- Locked field for Projects.
- Reviewed form in EditEngine.
- Created/edited a project.
- Swapped default.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14911
Summary:
Ref T10010. I think this is the desired/expected default behavior (e.g., searching for "Maniphest" should find tasks in any subproject or sprint of that project).
I'll probably add an "exact(...)" function later to mean "only the Maniphest superproject, exactly, not any of its children".
Test Plan:
- Added and executed unit tests.
- Ran various queries from the web UI.
- Got sensible-seeming results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14910
Summary:
Ref T10010.
- Default name to "Milestone X".
- Remove policy controls, which have no effect.
- Don't generate slugs for milestones since this is a big pain where they all generate as `#milestone_1` by default (you can add one if you want). I plan to add some kind of syntax like `#parent/32` to mean "Milestone 32 in Parent" later.
- Don't require projects to have unique names (again, 900 copies of "Milestone X"). I think we can trust users to sort this out for themselves since modern Phabricator has "Can Create Projects" permission, etc.
Test Plan: Created some milestones, had a less awful experience.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14909
Summary:
Ref T10010. This has a lot of UI/UX problems but I think it:
- technically allows subproject creation;
- technically allows milestone creation;
- doesn't let users unwittingly destroy their installs (probably).
Test Plan:
- Created milestones.
- Created subprojects.
- Created and edited normal projects.
- Observed some reasonable interactions (e.g., you can't create milestones for a milestone or edit a superproject's members).
- Observed plenty of silly/confusing interactions that need additional work.
{F1046657}
{F1046658}
{F1046655}
{F1046656}
{F1046654}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14904
Summary: Ref T10010. This is pretty straightforward with a couple of very minor new behaviors, like the icon selector edit field.
Test Plan:
- Created projects.
- Edited projects.
- Saw "Create Project" in quick create menu.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14896
Summary:
Ref T10010. Three motivations:
- Primarily: this makes conversion to EditEngine easier since I don't have to convert this weird control.
- This probably needs to have "Lock", "Unlock" and "Use Parent Project Setting" values after subprojects? But maybe just locking any parent locks all the children? Anyway, doesn't make sense to put it on the main edit form if it's weird like this, I think, since we'll want some kind of explanatory text.
- I probably want to move this to the "Members" tab anyway, and this won't be available on milestone projects at all.
Test Plan: Locked, unlocked, edited projects.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14895
Summary:
Ref T10010. This will allow us to find superprojects with `withMemberPHIDs(...)` queries.
- Copy all the current real member edges to materialized member edges.
- Redirect all reads to look at materialized members.
- This table is already kept in sync by earlier work with indexing.
Basically, flow is:
- Writes (joining, leaving, adding/removing members) write to the real member edge type.
- After a project's members change, they're copied to the materialized member edge type for that project and all of its superprojects.
- Reads look at materialized members, so "Parent" sees the members of "Child" and "Grandchild" as its own members, but we still have the "real members" edge type to keep track of "natural" or "direct" members.
Test Plan:
- Ran migration.
- Ran unit tests.
- Saw the same projects as projects I was a member of.
- Added some `var_dump()` stuff to verify the Owners changed.
- Used `grep` to look for other readers of this edge type.
- Made some project updates.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14893
Summary:
Ref T10010. When the viewer is logged-out or omnipotent, we can skip this query.
(Currently we issue a silly query like `src = X AND type = Y AND dst = ''`, which will never return results.)
Test Plan:
- Viewed projects as normal user and logged-out user.
- Ran unit tests.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14892
Summary: Ref T10010. This is primarily to make "Parent > Child > Grandchild" navigation more manageable for subprojects, at least for now.
Test Plan: Viewed profile, members, feed; saw crumbs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14891
Summary: Ref T10010. Several controlers currently have similar logic for handling tags and slugs, loading projects, and canonicalizing URIs. Clean it up a bit.
Test Plan:
- Visited profile, boards, feed.
- Visited by ID and by tag.
- Visited by non-normal tag (redircted).
- Visited by alternate tag (redirected).
- Visited non-policy project by non-normal tag (redirected into policy error).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14890
Summary: Ref T10010. This still needs support for attachments (to get members) and more constraints (like slugs), but mostly works.
Test Plan: Ran query, saw basically sensible results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14889
Summary:
Ref T10010. We currently require `withSlugs()` to have properly formatted slugs, but this leads to similar code in several places.
Instead: accept any slug, normalize slugs in the query, return a map so callers can figure out what happened if they want.
This tends to do the right thing by default, while keeping enough information around to do more complex things if necessary. A similar approach for querying commits has worked well in Diffusion.
Test Plan: Added and executed unit tests.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14888
Summary: Fixes T8482, or something. I can't actually repro that but I think it should be fixed either here or earlier
Test Plan:
```
$ ./bin/lipsum generate paste
GENERATORS Selected generators: Pastes.
WARNING This command generates synthetic test data, including user accounts. It is intended for use in development environments so you can test features more easily. There is no easy way to delete this data or undo the effects of this command. If you run it in a production environment, it will pollute your data with large amounts of meaningless garbage that you can not get rid of.
Are you sure you want to generate piles of garbage? [y/N] y
LIPSUM Generating synthetic test objects forever. Use ^C to stop when satisfied.
Generated "Paste": P223 forgotten_memory_disks_backup.java
Generated "Paste": P224 backup_disk_tables_and_administrate_backup_memory_account.java
Generated "Paste": P225 sync_backup_disk_and_undo_memory.php
Generated "Paste": P226 administrate_memory_shard_helper.php
Generated "Paste": P227 cancel_disk_users
Generated "Paste": P228 backups_pro.txt
Generated "Paste": P229 undo_host.txt
Generated "Paste": P230 accelerate_database_accounts.java
Generated "Paste": P231 entomb_accounts.java
Generated "Paste": P232 legendary_legendary_shards_helper.java
Generated "Paste": P233 compact_backup_and_user_and_purge_memory
Generated "Paste": P234 account_script_script_backup_helper_helper.java
Generated "Paste": P235 purge_disk.php
Generated "Paste": P236 forgotten_elder_account.txt
Generated "Paste": P237 ancient_ancient_disks.txt
Generated "Paste": P238 disk_user.php
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8482
Differential Revision: https://secure.phabricator.com/D14883
Summary:
Ref T8980. This isn't 100% coverage but should be pretty much all of the common ones.
These feel a touch iffy to me at first glance so I didn't go crazy trying to hunt all of them down. I have some other plans for them so maybe they'll feel better by the end of it.
Test Plan: Hovered over author, reviewers, blocked tasks, projects, etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8980
Differential Revision: https://secure.phabricator.com/D14877
Summary:
Fixes T9156.
- Fix hashtag generation.
- Fix various badnesses.
- Improve project name generator.
Test Plan:
```
$ ./bin/lipsum generate projects
GENERATORS Selected generators: Projects.
WARNING This command generates synthetic test data, including user accounts. It is intended for use in development environments so you can test features more easily. There is no easy way to delete this data or undo the effects of this command. If you run it in a production environment, it will pollute your data with large amounts of meaningless garbage that you can not get rid of.
Are you sure you want to generate piles of garbage? [y/N] y
LIPSUM Generating synthetic test objects forever. Use ^C to stop when satisfied.
Generated "Project": Self-Flying Data Center Swag Performance
Generated "Project": Optimize Cars
Generated "Project": Triaging Culture Optimization
Generated "Project": Automating Experience
Generated "Project": Accelerating NUX Performance
Generated "Project": Optimizing Culture Optimization
Generated "Project": Optimize Hardware
```
{F1042949}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9156
Differential Revision: https://secure.phabricator.com/D14874
Summary: Ref T9156. This makes the UX a little more modern/standard/safe.
Test Plan:
```
epriestley@orbital ~/dev/phabricator $ ./bin/lipsum generate
Choose which type or types of test data you want to generate, or select "all".
- Differential Revisions
- Files
- Maniphest Tasks
- Pastes
- Pholio Mocks
- Projects
- User Accounts
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9156
Differential Revision: https://secure.phabricator.com/D14873
Summary:
Fixes T9019. Pretty much ripped from D14467. I added the "policy hint" stuff so that you can create a project with this policy immediately.
I really dislike how the "hint" code works, but we //almost// never need to use it and the badness feels fairly well-contained.
Also pick up a quick feedback fix from D14863.
Test Plan:
- Added test coverage, got it to pass.
- Created a project with "Visible To: Project Members".
Reviewers: joshuaspence, chad
Reviewed By: chad
Maniphest Tasks: T9019
Differential Revision: https://secure.phabricator.com/D14869
Summary: Ref T8509. We currently give you a fairly obtuse error when trying to name a project something like "!!". The error is correct, but not as helpful as it could be. Give users a more specific, more helpful error.
Test Plan: {F1042883}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8509
Differential Revision: https://secure.phabricator.com/D14872
Summary:
Fixes T8509. Changes these behaviors:
- If you create a project named "QQQ" and add "qqq" as a hashtag at the same time, it fails in an unhelpful way. (Now: succeeds.)
- If you add "qqq" as a hashtag to a project with primary hashtag "qqq", it fails in a correct but probably unnecessary way (Now: just works).
We could make one or both of these behaviors show the user an error instead, but I think it's likely that this behavior is just what they always want.
Test Plan:
- Added failing tests and made them pass.
- Executed both scenarios described above from the web UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8509
Differential Revision: https://secure.phabricator.com/D14871
Summary:
Ref T8509. This fixes three issues:
- Adding a slug like `UPPERCASE` would not give you a normalized slug. (Now: normalizes as `uppercase`.)
- Adding a slug like `UPPERCASE` would allow you to give two different projects the different tags `UPPERCASE` and `uppercase` (and `UpPeRcAsE`, etc). (Now: second tag is rejected as a duplicate.)
- Adding multiple identical or similar slugs would produce a duplicate key exception. (Now: ignores the duplicates.)
Test Plan:
- Added test coverage.
- Made tests pass.
- Hit these cases in the UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8509
Differential Revision: https://secure.phabricator.com/D14870
Summary: See D14467. Just teasing this apart so I can be a little more confident in my commandeering. These are the unambiguous cleanup changes from D14467.
Test Plan: inspection / clicked stuff / no impact
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Differential Revision: https://secure.phabricator.com/D14868
Summary:
Ref T10010. Subprojects have the following general membership rule: if you are a member of a subproject ("Engineering > Backend"), you are also a member of the parent project.
It would be unreasonably difficult to implement this rule directly in SQL when querying `withMemberPHIDs()`, because we'd have to do an arbitrarily large number of arbitrarily deep joins, or fetch and then requery a lot of data.
Instead, introduce "materailized members", which are just a copy of all the effective members of a project. When a subproject has a membership change, we go recompute the effective membership of all the parent projects. Then we can just JOIN to satisfy `withMemberPHIDs()`.
Having this process avialable will also be useful in the future, when a project's membership might be defined by some external source.
Also make milestones mostly work like we'd expect them to with respect to membership and visibility.
Test Plan:
- Added and executed unit tests.
- Changed project members, verified materialized members populated correctly in the database.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14863
Summary:
Ref T10010. This adds infrastructure for querying projects by type, depth, parent and ancestor.
I needed to revise the "extended policy check" cycle detection rules. When, e.g., querying a grandchild, they incorrectly detected a cycle because both the child and grandchild needed to check the policy of the grandparent.
Instead, simplify it to just do a basic runaway calldepth check. There are many other safety mechanisms to make it so this can't ever occur.
(Cycle detection does have existing test coverage, and those tests still pass, it just takes a little longer to detect the cycle internally.)
There is still no way to create subprojects in the UI.
Test Plan: Added and executed unit tests.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14862
Summary:
Ref T10010. This implements technical groundwork for subprojects. Specifically, it implements policy rules like Phriction:
- to see a project, you must be able to see all of its parents (and the project itself).
- you can edit a project if you can edit any of its parents (or the project itself).
To facilitiate this, we load all project ancestors when querying projects so we can do the view/edit checks.
This does NOT yet implement:
- proper membership rules for these projects (up next);
- any kind of UI to let users create subprojects.
Test Plan:
- Added unit tests.
- Executed unit tests.
- Browsed Projects (no change in behavior is expected).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14861
Summary:
Ref T10010. Currently, we do an unusual JOIN to make testing for viewer membership in projects a little cheaper.
This won't work as-is once we have subprojects, so standardize, simplify, and cover it with more tests for now. (I may be able to get a similar optimization later, but want a correct implementation first.)
Test Plan:
This change should create no behavioral differences.
- Added tests.
- Ran tests.
- Viewed projects.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14859
Summary: Ref T9979. This simplifies/standardizes the code a bit, but mostly gives us more consistent class names and structure.
Test Plan:
- Used `bin/search index --type ...` to index documents of every indexable type.
- Searched for documents by unique text, found them.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14842
Summary: Ref T9979. This is going to become `FulltextEngine`, but pave the way for that by pulling extensions out of it.
Test Plan:
{F1036624}
- Used `bin/search index Txxx`, saw projects, subscribers and custom fields rebuild in the index.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14835
Summary: Adds basic NUX to Dashboards, Herald, Repositories, Maniphest. Note Herald and Dashboard Panels don't fine the nux for some reason, assume they will when modernized?
Test Plan: Read text, click buttons.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14844
Summary:
Ref T10010. This does some cleanups on the schema:
- `viewPolicy`, `editPolicy` and `joinPolicy` were nullable, but should never be `null`. Set them to defaults if they're null, then make the column non-nullable.
- Rename `phrictionSlug` to `primarySlug` and stop adding and removing trailing slashes from it.
- Add new columns to support milestones and non-milestone subprojects.
- Drop very old subprojectPHIDs column. This hasn't done anything in the UI for years and years, and isn't particularly realistic to migrate forward.
The new columns aren't reachable from the UI.
Test Plan:
- Applied patches.
- Grepped for `phrictionSlug`.
- Grepped for `subprojectPHIDs`.
- Created tasks.
- Edited tasks.
- Verified existing tasks still had primary slugs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14825
Summary:
Ref T10004. After D14804, we get this behavior by default and no longer need to set it explicitly.
(If some endpoint did eventually need to set it explicitly, it could just change what it passes to `setHref()`, but I believe we currently have no such endpoints and do not foresee ever having any.)
Test Plan:
- As a logged out user, clicked various links in Differential, Maniphest, Files, etc., always got redirected to a sensible place after login.
- Grepped for `setObjectURI()`, `getObjectURI()` (there are a few remaining callsites, but to a different method with the same name in Doorkeeper).
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14805
Summary:
Ref T9992. This is a step on the path to getting EditEngine working in Badges, Projects and Calendar.
This doesn't add a new `EditField` for icons yet, just standardizes the old stuff. New stuff is more general and I saved 150 lines of code.
I put the endpoint in Files because the similar "choose a profile picture" endpoint will definitely go there, and this endpoint might eventually feature, like, "draw your own icon~~" or something.
Test Plan:
- Created events, projects and badges with custom icons.
- Edited events, projects and badges, changing their icons.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9992
Differential Revision: https://secure.phabricator.com/D14799
Summary:
Ref T9964. Three goals here:
- Make it easier to supply Conduit documentation.
- Make automatic documentation for `*.edit` endpoints more complete, particularly for custom fields.
- Allow type resolution via Conduit types, so you can pass `["alincoln"]` to "subscribers" instead of needing to use PHIDs.
Test Plan:
- Viewed and used all search and edit endpoints, including custom fields.
- Used parameter type resolution to set subscribers to user "dog" instead of "PHID-USER-whatever".
- Viewed HTTP parameter documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14796
Summary:
Ref T9908. Fixes T6205.
This is largely some refactoring to improve the code. The new structure is:
- Each EditField has zero or one "submit" (normal edit form) controls.
- Each EditField has zero or one "comment" (stacked actions) controls.
- If we want more than one in the future, we'd just add two fields.
- Each EditField can have multiple EditTypes which provide Conduit transactions.
- EditTypes are now lower-level and less involved on the Submit/Comment pathways.
Test Plan:
- Added and removed projects and subscribers.
- Changed task statuses.
- In two windows: added some subscribers in one, removed different ones in the other. The changes did not conflict.
- Applied changes via Conduit.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6205, T9908
Differential Revision: https://secure.phabricator.com/D14789
Summary: Ref T9964. Builds on D14772. Allows callers to get the raw content of pastes as an attachment.
Test Plan:
- Read docs.
- Executed attachment query.
- Saw raw paste content.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14774
Summary: Ref T9964. Builds on D14772. Allows callers to request project PHIDs for objects.
Test Plan: {F1025468}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14773
Summary:
Ref T9964. Building tables in Remarkup is kind of neat-ish but ends up feeling kind of hacky, and requires weird workarounds if any of the values have `|` in them.
Switch to normal elements instead.
Also move the magic "ids" and "phids" to be more like real fields. I'll clean this up fully in a diff or two, it's just a little tricky because Maniphest has an "ids" field.
Test Plan: {F1024294}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14768
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
Summary:
Ref T9964. ApplicationSearch currently has a bunch of hard-coded `if ($object instanceof thing)` stuff.
Pull that out so it can live in extensions.
Test Plan:
- Searched by spaces, subscribers, projects.
{F1023921}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14764
Summary: Ref T9908. Move all the "pro" stuff into the old locations.
Test Plan: Created/edited tasks, looked at URIs, saw non-pro ones. Grepped for `editpro`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14717
Summary: Ref T9908. This is the last of the things that need to swap over.
Test Plan:
- Created tasks from a workboard.
- Created tasks in different columns.
- Edited tasks.
- Used `?parent=..`.
- Verified that default edit form config now affects comment actions.
- No more weird comment thing on forms, at least for now.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14715
Summary: Ref T9908. This drives workboard card edits through the new stuff. Bit of copy-paste but the old one will get deleted soon.
Test Plan:
- Edited some cards.
- Changed priority on a priority-sorted board, saw proper re-sort
- Removed board project, saw card vanish properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14712
Summary:
Ref T9132. Ref T9908. Fixes T5622. This allows you to copy some fields (projects, subscribers, custom fields, some per-application) from another object when creating a new object by passing the `?template=xyz` parameter.
Extend "copy" support to work with all custom fields.
Test Plan:
- Created new pastes, packages, tasks using `?template=...`
- Viewed new template docs page.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5622, T9132, T9908
Differential Revision: https://secure.phabricator.com/D14699
Summary: Ref T9132. This still has a lot of rough edges but the basics seem to work OK.
Test Plan: {F1012627}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14653
Summary:
Fixes T9881. This one had Quicksand spelled as "QuickSand" (with capital "S") so it probably didn't get hit by `grep`.
Didn't need to do any special magic with the footer, as far as I can tell.
Test Plan: Loaded project board view, seemed to work OK (no footer, nav works, title works, mobile menu sane).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9881
Differential Revision: https://secure.phabricator.com/D14626
Summary: Ref T9132. I had some hacks in place for dealing with Edge/Subscribers stuff. Clean that up so it's structured a little better.
Test Plan:
- Edited subscribers and projects.
- Verified things still show up in Conduit.
- Made concurrent edits (added a project in one window, removed it in another window, got a clean result with a correct merge of the two edits).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14601
Summary:
Ref T9132. Currently, EditEngine had some branchy-`instanceof` code like this:
```
if ($object instanceof Whatever) {
do_magic();
}
if ($object instanceof SomethingElse) {
do_other_magic();
}
```
...where `Whatever` and `SomethingElse` are first-party applications like ProjectsInterface and SubscribersInterface.
This kind of code is generally bad because third-parties can't add new stuff, and it suggest something is kind of hacky in its architecture. Ideally, we would eventually get rid of almost all of this.
T9789 is a similar discussion of this for the next layer down (`TransactionEditor`) and plans to get rid of branchy-instanceofs there too.
Since I'm about to add more stuff here (for Custom Fields), split it out first so I'm not digging us any deeper than I already dug us.
Broadly, this allows third-party extensions to add fields to every EditEngine UI if they want, like we do for Policies, Subscribers, Projects and Comments today (and CustomFields soon).
Test Plan:
{F1007575}
- Observed that all fields still appear on the form and seem to work correctly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14599
Summary:
Fixes T9832. We currently refuse to recognize project hashtags in remarkup if they begin with a digit. This is motivated by attempting to not recognize them if they contain only digits.
I don't think we really gain anything by this. Although most `#123` in text are probably not project references, the cost of doing a lookup for them is quite small, and //some// of them are.
In cases where users use `#123` to refer to tasks in an external system, they can use a rule for that with higher precedence than this one or not give their projects conflicting hashtags.
Test Plan:
- This is well-covered by unit tests.
- Referenced a `#3u1`, per T9832.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9832
Differential Revision: https://secure.phabricator.com/D14547
Summary: Move some `PhabricatorPolicyRule` implementations to a subdirectory of the parent application.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14478
Summary: fix T9718.
Test Plan: view project page when maniphest is and isn't. Look for Workboards.
Reviewers: #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: epriestley
Maniphest Tasks: T9718
Differential Revision: https://secure.phabricator.com/D14438
Summary: Make Workboard initialization more restrictive.
Test Plan: Log out, see "No Workboard", Log in with permissions, see "New Workboard", Log in with notchad, see "No Workboard".
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T7410
Differential Revision: https://secure.phabricator.com/D14306
Summary: Right now logged out users can enable a workboard on a project.
Test Plan: Log out, view a public project, click on Workboard, get not set up dialog. Click Cancel, return to project details.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14304
Summary:
Ref T9551. We currently use the same logic for generating project hashtags and Phriction slugs, but should be a little more conservative with project hashtags.
Stop them from generating with stuff that won't parse in a "Reviewers:" field or generally in commments (commas, colons, etc).
Test Plan:
Created a bunch of projects with nonsense in them and saw them generate pretty reasonable hashtags.
{F873456}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9551
Differential Revision: https://secure.phabricator.com/D14261
Summary:
Ref T9551. To set things up:
- Name a project `aa bb`. This will have the tag `aa_bb`.
- Try to visit `/tag/aa%20bb`.
Here's what happens now:
- You get an Aphront redirect error as it tries to add the trailing `/`. Add `phutil_escape_uri()` so that works again.
- Then, you 404, even though this tag is reasonably equivalent to the real project tag and could be redirected. Add a fallback to lookup, resolve, and redirect if we can find a hit for the tag.
This also fixes stuff like `/tag/AA_BB/`.
Test Plan: Visited URIs like `/tag/aa%20bb`, `/tag/aa%20bb/`, `/tag/Aa_bB/`, etc. None of them worked before and now they all do.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9551
Differential Revision: https://secure.phabricator.com/D14260
Summary:
Fixes T9279. Modernizes the SearchEngine and Query classes. User-facing changes:
- Added order by commit date, default to order by commit date with newest commits first.
- Added explicit "Needs Audit by".
- Added new `packages(...)` typeahead function.
- Picked up automatic subscribers, projects, and order fields.
This changes behavior a little bit: we previously attempted to exclude, e.g., commits which a package you own needs to audit, but which you have resigned from. This is difficult in general and I think it needs a more comprehensive solution. This shouldn't impact users much, anyway.
Test Plan: {F767628}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9279
Differential Revision: https://secure.phabricator.com/D14013
Summary: Ref T9089. This link leads to a detail page, not an edit page, and is always visible by users with permission to see the column.
Test Plan: Clicked "Column Details" with and without edit permission.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9089
Differential Revision: https://secure.phabricator.com/D14016
Summary: Fixes T9090. You don't need to be able to edit a project to create tasks on its workboard. Being able to view the project is sufficient, and the user certianly can if they got this far.
Test Plan: Viewed workboard, hit "Create Task".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9090
Differential Revision: https://secure.phabricator.com/D14015
Summary: Ref T8726. Convert these to be modular.
Test Plan:
- Created rules using these actions.
- Upgraded them.
- Verified they still work.
{F659266}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13705
Summary:
Ref T8726. Some adapters now have a large number of fields, and we lost the sort-of-human-readable implicit ordering when fields were modularized.
Instead, group and sort fields.
Test Plan: {F603066}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13619
Summary: Ref T8726. This gets rid of all the `VALUE_*` constants and lets Fields provide arbitrary typeaheads without upstream/JS changes.
Test Plan: Used all tokenizers. Used "Another Herald Rule". Grepped for all removed constants.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13616
Summary:
Ref T8726. I want to modularize values and reduce how hard-coded / copypasta'd they are.
- Rename `get...StandardCondition()` to `get...StandardType()`, since we can drive both conditions and values from it.
- Rename `STANDARD_LIST` to `STANDARD_PHID_LIST` for consistency: all "lists" are lists of PHIDs.
- For all standard types which don't require typehaeads, lift their logic into the base class.
- I'll lift typeaheads soon, but need to generalize them first.
Test Plan: Edited various Herald rules, saw value UI generate correctly.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13612
Summary:
- Set a default edit policy to match view policy for projects
- It seems like initializeNew* is a better init function than new Phabricator*,
so ported two classes to use it and spot-checked the others (this also fixed
a fatal.)
- Other minor bugs
Test Plan: Ran lipsum for a while without any fatals
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, jhurwitz, Korvin
Differential Revision: https://secure.phabricator.com/D13559
Summary: Ref T8726. Continue making Herald fields more modular than they currently are.
Test Plan:
- Created a rule using all the affected fields.
- Ran the rule.
- Saw reasonable object field values.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13495
Summary: Move some `PhabricatorSearchField` subclasses to be adjacent to the application to which they belong. This seems generally better to me than lumping them all together in the `src/applications/search/field/` directory. I was also wondering if it makes sense to rename these subclasses as `PhabricatorXSearchField` rather than `PhabricatorSearchXField` (as per T5655), but wasn't really sure if these objects are meant to be search-fields, or just fields belonging to the #search application.
Test Plan: N/A.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13374
Summary: Ref T8631. Projects may not have a profile image PHID; don't try to load it if they don't.
Test Plan: Saw `WHERE phid IN ('')` query stop happening on a local feed story page.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8631
Differential Revision: https://secure.phabricator.com/D13476
Summary:
Fixes T6787. I'm kind of cheating a little bit here by not unifying default selection with `initializeNew(...)` methods, but I figure we can let this settle for a bit and then go do that later. It's pretty minor.
Since we're not doing templates I kind of want to swap the `'template'` key to `'type'` so maybe I'll do that too at some point.
@chad, freel free to change these, I was just trying to make them pretty obvious. I //do// think it's good for them to stand out, but my approach is probably a bit inconsistent/heavy-handed in the new design.
Test Plan:
{F525024}
{F525025}
{F525026}
{F525027}
Reviewers: btrahan, chad
Reviewed By: btrahan
Subscribers: johnny-bit, joshuaspence, chad, epriestley
Maniphest Tasks: T6787
Differential Revision: https://secure.phabricator.com/D13387
Summary: Ref T8099. This adds a new class which all search engines return for layout. I thought about this a number of ways, and I think this is the cleanest path. Each Engine can return whatever UI bits they needs, and AppSearch or Dashboard picks and lays the bits out as needed. In the AppSearch case, interfaces like Notifications, Calendar, Legalpad all need more custom layouts. I think this also leaves a resonable path forward for NUX as well. Also, not sure I implemented the class correctly, but assume thats easy to fix?
Test Plan: Review and do a search in each application changed. Grep for all call sites.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13332
Summary: Fixes T8524, T8550. These were both missing "subscribers" and Phriction was also the only application with no "other".
Test Plan:
- Project
- set user A to notify only for project subscriber changes
- made a project with user A
- subscribed user B to project
- verified notification sent to user A
- used ./bin/mail to make sure no mail was sent to user A because "This mail has tags which control which users receive it, and this recipient has not elected to receive mail with any of the tags on this message (Settings > Email Preferences)."
- Phriction
- set user A to notify only for phriction document subscriber changes
- made a document with user A
- subscribed user B to document
- verified notification sent to user A
- used ./bin/mail to make sure no mail was sent to user A because "This mail has tags which control which users receive it, and this recipient has not elected to receive mail with any of the tags on this message (Settings > Email Preferences)."
- observed option for "other" in email preferences
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8524, T8550
Differential Revision: https://secure.phabricator.com/D13313
Summary: All classes should extend from some other class. See D13275 for some explanation.
Test Plan: `arc unit`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13283
Summary: Self-explanatory.
Test Plan: Viewed the project page as a user without edit permissions and saw the link greyed out.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13255
Summary:
Ref T5681. Ref T6860. This doesn't do anything interesting on its own, just makes the next diff smaller.
In the next diff, policies become aware of the types of objects they're acting on. We need to specify which object type all the "Default View/Edit" settings are for so they get the right rules.
For example, a rule like "Allow task author" is OK for "View Policy" on a task, and also OK for "Default View Policy" on ManiphestApplication. But it's not OK for "Can Create Tasks" on ManiphestApplication.
So annotate all the "template"/"default" policies with their types. The next diff will use these to let you select appropriate rules for the given object type.
Test Plan:
- Used `grep` to find these.
- This change has no effect.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5681, T6860
Differential Revision: https://secure.phabricator.com/D13251
Summary:
Ref T8493. Tricks:
- "Create Similar Task" and "Create Subtask" should copy the parent's Space.
- Normal list view + workboard card view.
Test Plan:
- Created a task, edited space, etc.
- Viewed tasks with different users.
- Created a "Similar Task" (saw proper Space).
- Created a subtask (saw proper Space).
- Viewed workboard.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8493
Differential Revision: https://secure.phabricator.com/D13232
Summary: See also D13186.
Test Plan: Ran `arc unit --everything`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13201
Summary: Ref T8441. Ref T7715. Ref T7909. Clean up all the ordering and grouping hacks in Maniphest so we can drive it through normal infrastructure, move it to SearchField, introduce Spaces, and eventually modernize the Conduit API.
Test Plan:
- Executed all grouping/ordering queries, including custom queries.
- Forced execution with old aliases; got modern results.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7909, T7715, T8441
Differential Revision: https://secure.phabricator.com/D13197
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
