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: Cleans up some language, colors, etc.
Test Plan: Write lots of new posts, hide them, edit them, check history.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14914
Summary: Right now you can't create two blogs without a domain name, since it has a unique key on the column. Removing the key.
Test Plan: Create two blogs with no domain name, works as expected. Create two blogs with `cat.dog` as domain name, get duplicate domain error.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14915
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: We're checking for drafts even though we already know there are no blogs, just skip the query.
Test Plan: trucate phame_blogs; See proper blank state.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14908
Summary: Fixes T10058. We don't need to continue on this check if no path changes are being applied.
Test Plan: Archived an owners package.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10058
Differential Revision: https://secure.phabricator.com/D14906
Summary:
Fixes T10057. Root issue is:
- In the past, you could give tokens to objects of type X (here, Ponder answers).
- Now, you can't.
- If you try to load a token on an object of type X, we do a bad call to attach it and fatal.
Instead, make sure objects implement the proper interface before we attach them, and just pretend the token does not exist otherwise.
Test Plan: Faked the exception in T10057, applied patch, got clean tokens page.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10057
Differential Revision: https://secure.phabricator.com/D14905
Summary:
Ref T9897. This one is a little more involved because of how getting a post on a blog works.
I also changed moving posts to be a real transaction (which shows up in history, now).
Test Plan: Created posts from web UI and conduit.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D14902
Summary: Ref T9897. Mostly straightforward, but also modernize/fixup the Query a little so that posts never load with no blog.
Test Plan: Queried posts via API.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D14901
Summary: Ref T9897. Adds basic blog query support.
Test Plan: Ran some queries.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D14900
Summary: Ref T9897.
Test Plan: Used API to make a few changes to a blog.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D14899
Summary: Ref T9897. We can now provide modern `search` and `edit` endpoints (I'll do this next).
Test Plan: Grepped for removed methods.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D14898
Summary: Allows create and edit workflows through EditEngine. Not sure I did the 'blog' stuff correct.
Test Plan: Create a new post, edit a post, move a post.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14802
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: Ref T10004. This primarily supports moving Phame to EditEngine.
Test Plan: {F1045166}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14887
Summary:
Ref T9994.
- Allow errors to be dismissed.
- Tailor messaging for closed/abandoned revisions.
- Reduce scare messaging on land dialog, since it's not really that scary anymore.
Test Plan:
- Dismissed errors.
- Hit new warnings.
- Wasn't as scared when landing.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9994
Differential Revision: https://secure.phabricator.com/D14886
Summary: This is linking to the wrong article.
Test Plan: O.O
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14885
Summary: I'll revert this within 24 hours, before I promote.
Test Plan: Had completely functional but greatly enhanced Phabricator experience in Firefox, Safari and Chrome.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14884
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. Previously, if you had like `T123T123T123` and waved your mouse over them, we wouldn't move the card. Now, move the card.
Test Plan: Waved mouse. Saw the card move.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8980
Differential Revision: https://secure.phabricator.com/D14881
Summary: Ref T8980. This calculation was not quite right and you could sneak one off screen if you tried carefully.
Test Plan: Couldn't sneak one off screen anymore.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8980
Differential Revision: https://secure.phabricator.com/D14880
Summary:
Ref T8980. Move away from events to EngineExtensions.
This also simplifies hovercards a bit:
- Removes tasks from revision cards.
- Removes blockers/blocked from task cards.
- Removes "Send Message" from user cards.
These mostly felt cluttery to me. Open to arguments to retain them. I think we can make better use of the space, though (e.g., flags, projects + board columns).
Test Plan:
- Viewed people, task, revision, commit and project hovercards.
{F1043256}
{F1043257}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8980
Differential Revision: https://secure.phabricator.com/D14878
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: Ref T10032. This is sufficent to hit NUX without doing anything bad.
Test Plan:
- Visited NUX.
- Browsed normally.
{F1043191}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10032
Differential Revision: https://secure.phabricator.com/D14876
Summary:
After certain types of errors, we may deadlock when trying to destroy test databases.
Specifically, we still have connections open to, say, `phabricator_unittest_abasonaknlbaklnasb_herald` (or whatever) and MySQL sometimes (not sure exactly when?) waits for them before destorying the database.
Test Plan:
- Added `$m = null; $m->method()` to a fixture test to force a fatal.
- Saw consistent deadlock, with `storage destroy` never exiting.
- Added `--trace` to the `storage destroy` command and made it use `phutil_passthru()` so I could see what was happening.
- Saw it hang on some arbitrary database.
- Conneced to MySQL, used `show full processlist;` to see what was wrong.
- Saw the `DROP DATABASE ...` command waiting for locks to release on the database, and other connections still open.
- Applied patch.
- Saw consistent success.
- Used `storage destroy --unittest-fixtures` to clean up extra databases.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14875
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: I feel like the daemon configuration should be locked from editing from the web UI, given that much of it won't work unless the daemons are restarted anyway.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14867
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
