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 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 T5240. We don't currently use any of these options and I don't think we have any plans to use them. Strip them out for now to make fixing drag-and-drop stuff easier.
Test Plan: Grepped for removed stuff, no hits.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5240
Differential Revision: https://secure.phabricator.com/D14864
Summary:
Ref T10004. This might be a little too funky or magical, but is probably not necessary too often and easier than doing new layout with the CSS.
Also make the desktop icons a little meatier.
Test Plan: Swiped things away on mobile. Clicked things away on desktop.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14857
Summary:
Fixes T1895. Now that we have on-demand prviews, we can use them on mobile. On mobile:
- don't show live previews;
- only save drafts every 10 seconds.
Also, show fewer remarkup buttons on mobile to try to make sure the more important ones (preview, e.g.) fit.
Test Plan:
- Made window narrower and wider to trigger preview/no-preview behavior.
- Used DarkConsole to verify request rate.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T1895
Differential Revision: https://secure.phabricator.com/D14856
Summary:
Ref T3967. This gives us a reasonable baseline for doing remarkup previews inline in all contexts, and works in weird/constrained context including:
- inline comments;
- conpherence; and
- custom fields.
It would be nicer to go beyond this in contexts like Phame posts, but this is a start, at least.
Test Plan:
{F1040877}
{F1040878}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T3967
Differential Revision: https://secure.phabricator.com/D14855
Summary:
Fixes T10039. We add text to textareas in two cases:
- Users clicking assitance buttons in Remarkup text areas.
- Drag-and-drop file uploads.
In case (1), it makes sense to highlight the text (it shows the user what we inserted, and lets them undo the action easily if it isn't what they wanted).
In case (2), it does not. Users almost never want to delete or edit a file reference. It is slightly nice to have the reference as a visual callout, but I don't think this is a big deal.
Change the behavior so that we highlight only for remarkup buttons, not for drag-and-drop files.
Test Plan:
- Clicked an "isnert quote" button on remarkup assist area, got highlighted example text.
- Dragged and dropped a file, got text inserted with no highlight.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10039
Differential Revision: https://secure.phabricator.com/D14851
Summary:
Ref T9979. This uses ngrams (specifically, trigrams) to build a reasonably efficient index for substring matching. Specifically, for a package like "Example", with ID 123, we store rows like this:
```
< ex, 123>
<exa, 123>
<xam, 123>
<amp, 123>
<mpl, 123>
<ple, 123>
<le , 123>
```
When the user searches for `exam`, we join this table for packages with tokens `exa` and `xam`. MySQL can do this a lot more efficiently than it can process a `LIKE "%exam%"` query against a huge table.
When the user searches for a one-letter or two-letter string, we only search the beginnings of words. This is probably what they want, the only thing we can do quickly, and a reasonable/expected behavior for typeaheads.
Test Plan:
- Ran storage upgrades and search indexer.
- Searched for stuff with "name contains".
- Used typehaead and got sensible results.
- Searched for `aabbccddeeffgghhiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz` and saw only 16 joins.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14846
Summary:
Fixes T9890. This allows IndexExtensions to emit an object version.
Before we build indexes, we check if the indexed version is the same as the current version. If it is, we just don't call that extension.
T9890 has a case where this is useful: a script went crazy and posted thousands of comments to a single task.
Without versioning, that results in the same comments being indexed over and over again. With versioning, most of the queue could just exit without doing any work.
Test Plan:
- Added a `sleep(1)` to the actual indexing, used `bin/search index --background` to queue up a lot of tasks, ran them with `bin/phd debug task`, saw them complete very quickly with only one actual index operation performed.
- Used `bin/search index --trace` and `bin/search index --trace --background` to observe the behavior of queries against the index version store, which looked sensible.
- Made comments/transactions, saw versions update.
- Used `bin/remove destroy`, verified index versions were purged.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9890
Differential Revision: https://secure.phabricator.com/D14845
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: Currently it's difficult to notice posts may be much longer than the summary (in most cases, actually), Adds a consistent "Read More" link to the full post. Also made sure "violet" was more consistantly used.
Test Plan:
Review summaries in Phame
{F1031799}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14827
Summary: Ref T9967
Test Plan:
Ran migrations.
Verified database populated properly with PHIDs (SELECT * FROM auth_sshkey;).
Ran auth.querypublickeys conduit method to see phids show up
Ran bin/remove destroy <phid>.
Viewed the test key was gone.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9967
Differential Revision: https://secure.phabricator.com/D14823
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:
Fixes T10012. The permissions here are little weird: you need edit permission on the //configurations//, not the //engines//. I was checking edit permission on the engines only.
I should possibly make this a bit more consistent, the engine edit permission is just very convenient to use to enforce object create permission right now. I'll likely clean this up after T9789.
Test Plan:
- Tried to reorder forms as a less-privileged user, got proper policy errors.
- Reordered forms normally as a regular user.
Reviewers: chad
Reviewed By: chad
Subscribers: Luke081515.2
Maniphest Tasks: T10012
Differential Revision: https://secure.phabricator.com/D14824
Summary: Fixes T9871. Ref T10004. These won't win any awards but it fixes them being incredibly weird and confusing.
Test Plan:
{F1029090}
- Tried to use controls, got reasonable behavior.
- Used normal controls to make sure I didn't break anything.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9871, T10004
Differential Revision: https://secure.phabricator.com/D14814
Summary:
Ref T10004. After a user logs in, we send them to the "next" URI cookie if there is one, but currently don't always do a very good job of selecting a "next" URI, especially if they tried to do something with a dialog before being asked to log in.
In particular, if a logged-out user clicks an action like "Edit Blocking Tasks" on a Maniphest task, the default behavior is to send them to the standalone page for that dialog after they log in. This can be pretty confusing.
See T2691 and D6416 for earlier efforts here. At that time, we added a mechanism to //manually// override the default behavior, and fixed the most common links. This worked, but I'd like to fix the //default// beahvior so we don't need to remember to `setObjectURI()` correctly all over the place.
ApplicationEditor has also introduced new cases which are more difficult to get right. While we could get them right by using the override and being careful about things, this also motivates fixing the default behavior.
Finally, we have better tools for fixing the default behavior now than we did in 2013.
Instead of using manual overrides, have JS include an "X-Phabricator-Via" header in Ajax requests. This is basically like a referrer header, and will contain the page the user's browser is on.
In essentially every case, this should be a very good place (and often the best place) to send them after login. For all pages currently using `setObjectURI()`, it should produce the same behavior by default.
I'll remove the `setObjectURI()` mechanism in the next diff.
Test Plan: Clicked various workflow actions while logged out, saw "next" get set to a reasonable value, was redirected to a sensible, non-confusing page after login (the page with whatever button I clicked on it).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14804
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 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:
Fixes T9995. I think letting users customize slugs is not a hugely compelling as a product feature, and this fixes the issue with slugs that have "/" characters in them and makes the move to EditEngine easier since I don't have to deal with the weird JS thing.
Instead, just generate slugs automatically. No more JS, no more separate field, things automatically update if you rename a blog, and now that URIs have IDs in them the old URI will still work after a rename.
Test Plan:
- Applied migration.
- Created new posts.
- Edited existing posts.
- Visited various posts.
- Created a post with a bunch of "/" in the title, things still worked fine.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9995
Differential Revision: https://secure.phabricator.com/D14792
Summary:
Fixes T9984. When a tokenizer only allows one selection (like "Task Owner:" or "Land Onto Branch:"), keep the browse button active but have it //replace// values.
Also, have "Create Subtask" default to the system default status, so subtasks of closed tasks are not also closed.
Test Plan:
- Browsed an empty limit=1 tokenizer.
- Replaced a full limit=1 tokenizer.
- Browsed an empty no-limit tokenizer.
- Browsed more tokens into the no-limit tokenizer.
- Typed some tokens normally.
- Created a subtask of a closed task.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9984
Differential Revision: https://secure.phabricator.com/D14785
Summary: Better color, space for Blog List.
Test Plan: Review PhameHome at mobile, tablet and desktop breakpoints.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14749
Summary: Ref T9927. Adds a "Blogs" section to PhameHome. Removes "New Post" Controller. Adds flipped layout for PHUITwoColumnView
Test Plan:
Test PhameHome, Ponder, New Post, etc. Mobile and Desktop states.
{F1022080}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: johnny-bit, Korvin
Maniphest Tasks: T9927
Differential Revision: https://secure.phabricator.com/D14744
Summary: Ref T7858. Don't let users add multiple values to single-value tokenizers by using the "Browse" button.
Test Plan:
- Added a token, browse button was disabled.
- Removed the token, browse button was enabled again.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7858
Differential Revision: https://secure.phabricator.com/D14738
Summary: Ref T9952. See discussion there. This change is primarily aimed at letting me build a typeahead of branches in a repository so that we can land to arbitrary branches a few diffs from now.
Test Plan:
- Ran migrations.
- Verified database populated properly with PHIDs (`SELECT * FROM repository_refcursor;`).
- Ran `bin/repository update`.
- Viewed a Git repository in Diffusion.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9952
Differential Revision: https://secure.phabricator.com/D14731
Summary: Makes this more consistent. Also clean up spacing. Ref T9414
Test Plan: Archive/Activate Paste, Edit Paste
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9414
Differential Revision: https://secure.phabricator.com/D14724
Summary:
Ref T9132. Ref T9908. Puts reordering UI in place:
- For create forms, this just lets you pick a UI display order other than alphabetical. Seems nice to have.
- For edit forms, this lets you create a hierarchy of advanced-to-basic forms and give them different visibility policies, if you want.
Test Plan:
{F1017842}
- Verified that "Edit Thing" now takes me to the highest-ranked edit form.
- Verified that create menu and quick create menu reflect application order.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132, T9908
Differential Revision: https://secure.phabricator.com/D14704
Summary:
Ref T9908. Simplify some of the policies here:
- If you can edit an application (currently, always "Administrators"), you can view and edit all of its forms.
- You must be able to edit an application to create new forms.
- Improve some error messages.
- Get about halfway through letting users reorder forms in the "Create" menu if they want to sort by something weird since it'll need schema changes and I can do them all in one go here.
Test Plan:
- Tried to create and edit forms as an unprivileged user.
- Created and edited forms as an administrator.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14700
Summary:
Fixes T7848. @jasonfsmitty discussed an issue in great detail there and in D14359, and I completely missed it. Specifically:
- If you save a "Change status to: Open" rule in Maniphest, and then edit it again, the token shows "Unknown Object (???)" instead of the correct token.
- That's because loadHandles() has no idea what to do with the value "open", since it's not a real PHID.
The way we render tokenizer tokens in Herald is quite hacky right now. Fortunately, I wrote a //slightly// better way for EditEngine yesterday or the day before. Use the slightly better way to fix the issue with D14359.
This could still be better than it is, but the badness is mostly hidden now and can be cleaned up later without impacting anything.
Test Plan: Edited a Herald rule with projects and status changes, saw proper tokens.
Reviewers: chad
Reviewed By: chad
Subscribers: jasonfsmitty
Maniphest Tasks: T7848
Differential Revision: https://secure.phabricator.com/D14682
Summary:
Ref T9132. Fixes T4580. Thhat might actually have been fixed a while ago or something since it describes a buggy/bad interaction which doesn't reproduce for me at HEAD.
This saves and restores all the stacked actions (subscribers, projects, etc) so that you don't lose anything if you close a window by accident.
Test Plan:
Added a bunch of actions in various states, reloaded the page, draft stuck around.
Submitted form, actions didn't stick around anymore.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4580, T9132
Differential Revision: https://secure.phabricator.com/D14675
Summary:
Ref T9132. We currently have an old preview/draft behavior and a new actions behavior.
Let the actions behavior do drafts/previews too, so we can eventually throw away the old thing.
This is pretty much just copying the old behavior into the new one, but with a few tweaks. The major change is that we submit all the stacked actions behavior now, so the preview reflects everything the change will do (and, soon, we can save it in the draft in a consistent way).
Also includes one hack-fix that I'll clean up at some point.
Test Plan: Added a bunch of stacked actions and observed meaningful previews.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14672
Summary:
Ref T9132. This is kind of a mess because the tokenizer rewrite left rendering tokenizers in Javascript a little rough. This causes bugs like icons not showing up on tokens in the "Policy" dialog, which there's a task for somewhere I think.
I think I've fixed it enough that the beahavior is now correct (i.e., icons show up properly), but some of the code is a bit iffy. I'll eventually clean this up properly, but it's fairly well contained for now.
Test Plan:
- Reassigned a task.
- Put a task up for grabs.
- No reassign on closed tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14669
Summary: Ref T9132. Supports selects in stacked actions and adds "Change Status" + "Change Priority".
Test Plan: Changed status and priority from stacked actions.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14667
Summary: Ref T9132. Shhh this never happened shhhhhhh.
Test Plan: Selected multiple actions, saw them add at the bottom.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14664
Summary:
Ref T9132. Like D14659, I'll hold this until after the cut.
This swaps commenting in Maniphest over to EditEngine / stackable actions. New code doesn't have parity yet, although none of the things we're missing should technically be //strictly mandatory//. There's a list inline. I'll restore these in the next diffs.
Briefly -- comments, subscribers and projects work. Status, owners and priority do not yet.
Test Plan:
- Made comments and added subscribers and projects.
- Read through the old code to look for missing features and tried to document them all.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14663
Summary: Provides a real 2x avatar and offers new built in images for profile pictures.
Test Plan: reload profile, see sharper image, pick eevee, see eevee
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14668
Summary: Updates some internal builtin image defaults for 2x, and adds some new ones for profiles
Test Plan: View my profile, still works
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14661
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:
Ref T9132. Fixes T5031. This approximately implements the plan described in T5031#67988:
When we recieve a preview request, don't write a draft if the form is from a version of the object before the last update the viewer made.
This should fix the race-related (?) zombie draft comments that sometimes show up.
I just added a new object for this stuff to make it easier to do stacked actions (or whatever we end up with) a little later, since I needed to do some schema adjustments anyway.
Test Plan:
- Typed some text.
- Reloaded page.
- Draft stayed there.
- Tried real hard to get it to ghost by submitting stuff in multiple windows and typing a lot and couldn't, although I didn't bother specifically narrowing down the race condition.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5031, T9132
Differential Revision: https://secure.phabricator.com/D14640
Summary: Better mobile spacing, post spacing.
Test Plan: Eyeball it.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14620
Summary: Provides more information that a post is a draft.
Test Plan:
Add a draft post, see new style. Check Blog as non-editor, don't see draft post.
{F1008655}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14613
Summary: Creates a new PhameBlogView which is more of a blog landing page with the latest posts. Management has moved to PhameManageController with a new timeline.
Test Plan:
Edit Blog, Publish, Subscribe, view posts.
{F1008400}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14608
Summary: Will use these more in the upcoming unbeta design of PhameBlog, likely. Also curious how this works.
Test Plan: Add an image to a blog, remove an image from a blog.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14587
Summary: Lists that have task objects look bad since the objects bleed into each other vertically. This provides slightly better spacing.
Test Plan: Review T9379 with new spacing.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14588
Summary: This makes document views a little more automatic, and a little more style to the page. The Document itself remains on a pure white centered background, but footer and preceeding objects go back to the original body color. This provides a bit more depth and separation over content and definitions/comments.
Test Plan:
Tested Phriction, Diviner, Legalpad, Phame, Email Commands, HTTP Commands, with and without a footer.
{F1005853}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14582
Summary: Darken up the Preamble UI and provide more //dramatic// spacing. Fixes T9866
Test Plan: Review a preamble.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9866
Differential Revision: https://secure.phabricator.com/D14579
Summary: Moves to showing Legalpad previews using PHUIDocumentViewPro
Test Plan: Create a new document, edit an existing document
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14550
Summary: New icons
Test Plan: Use new icons
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14568
Summary: Reuse PHUIMarkupPreviewView in Phame for consistency, less custom code. Also, doesn't work (JS issue).
Test Plan: New Post, Edit Post, Save Post
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14552
Summary: This paragraph shouldn't get extra margin when inside a blockquote.
Test Plan: Quote some text, see less margin.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14549
Summary: This rule should correctly target the last //direct// descendent child of .phabricator-remarkup, and not grandchildren. Fixes T9833
Test Plan: Test commenting, documents, descriptions, and other remarkup areas. All spacing seems consistent.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9833
Differential Revision: https://secure.phabricator.com/D14548
Summary: Fixes T9793, the selector is triggering on the last child of every child of phabricator-remarkup, not the last child of phabricator-remarkup
Test Plan: Checkout latest changelog
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9793
Differential Revision: https://secure.phabricator.com/D14544
Summary:
Ref T9132. Allows fields to be locked (shown, but not modifiable) and hidden (not shown).
In both cases, default values are still respected.
This lets you do things like create a form that generates objects with specific projects, policies, etc.
Test Plan:
- Set defaults.
- Locked and hid a bunch of fields.
- Created new objects using the resulting form.
{F975801}
{F975802}
{F975803}
{F975804}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14509
Summary:
Ref T9132. This just makes edited forms do //something//, albeit not anything very useful yet.
You can now edit a form and:
- Retitle it;
- add a preamble (instructions on top of the form); and
- reorder the form's fields.
Test Plan:
{F974632}
{F974633}
{F974634}
{F974635}
{F974636}
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14503
Summary:
Addresses T9814. Adds SVG files to Celerity maps. Adds a mask-icon.svg file that
I made by pulling the existing favicon into Illustrator and running trace on it.
This hardcodes the header color from the default theme, and doesn't pay attention
to customizations of the header.
Test Plan: I pinned the tab in Safari.
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: chad, Korvin
Maniphest Tasks: T9814
Differential Revision: https://secure.phabricator.com/D14527
Summary: Removes "delete" and uses "archive/activate" instead for Phame Blogs. Ref T9756
Test Plan: Archive a blog, see in search, activate blog, see in other search.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: joshuaspence, Korvin
Maniphest Tasks: T9756
Differential Revision: https://secure.phabricator.com/D14465
Summary:
Ref T9787. To fix this, I want to change how file PHIDs are extracted slightly: specifically, I'm going to extract them later in the editing process.
Before doing this, clean up a couple of bad implementations:
- Owners extracts its description as a file PHID. This is an error.
- Extract the description as a remarkup block instead.
- Add an edge table so stuff like file attachment works properly.
- Differential has a no-op extract method. This is presumably just a copy/paste issue from long ago.
Test Plan:
- Edited a revision in Differential.
- Dropped a file into the description of an Owners package.
- Before change: this did not attach the file.
- After change: the file now attaches properly and shows up as "Attached" in the file details.
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Subscribers: joshuaspence
Maniphest Tasks: T9787
Differential Revision: https://secure.phabricator.com/D14493
Summary: Fixes T9757.
Test Plan: Created a Herald rule and then subscribed to it with a different account.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9757
Differential Revision: https://secure.phabricator.com/D14468
Summary: Rename the XHPAST database from `{$NAMESPACE}_xpastview` to `{$NAMESPACE}_xhpast`.
Test Plan: Ran `./bin/storage --namespace test upgrade --no-quickstart`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14442
Summary: Ref T7053. Remove the `envHash` and `envInfo` fields, which are no longer used now that the daemons restart automagically. Depends on D14458.
Test Plan: Saw no more setup issues.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: tycho.tatitscheff, epriestley
Maniphest Tasks: T7053
Differential Revision: https://secure.phabricator.com/D14446
Summary:
Ref T9132. This diff doesn't do anything interesting, it just lays the groundwork for more interesting future diffs.
Broadly, the idea here is to let you create multiple views of each edit form. For example, we might create several different "Create Task" forms, like:
- "New Bug Report"
- "New Feature Request"
These would be views of the "Create Task" form, but with various adjustments:
- A form might have additional instructions ("how to file a good bug report").
- A form might have prefilled values for some fields (like particular projects, subscribers, or policies).
- A form might have some fields locked (so they can not be edited) or hidden.
- A form might have a different field order.
- A form might have a limited visibility policy, so only some users can access it.
This diff adds a new storage object (`EditEngineConfiguration`) to keep track of all those customizations and represent "a form which has been configured to look and work a certain way".
This doesn't let these configurations do anything useful/interesting, and you can't access them directly yet, it's just all the boring plumbing to enable more interesting behavior in the future.
Test Plan:
ApplicationEditor forms now let you manage available forms and edit the current form:
{F959025}
There's a new (bare bones) list of all available engines:
{F959030}
And if you jump into an engine, you can see all the forms for it:
{F959038}
The actual form configurations have standard detail/edit pages. The edit pages are themselves driven by ApplicationEditor, of course, so you can edit the form for editing forms.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14453
Summary: Adds commenting to Phame Posts, also testing a new "document comment style". Unsure about it but Phame is a prototype so good place to explore.
Test Plan: Leave some comments, see some comments, test show/hide.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9746
Differential Revision: https://secure.phabricator.com/D14451
Summary:
This diff adds a new mode argument to the Diffusion Conduit API with two options:
- "overwrite": the default, maintains the current behavior of deleting all coverage
in the specified branch before uploading the new coverage
- "update": does not delete old coverage, but will overwrite previous
coverage information if it's for the same file and commit
`DiffusionRequest::loadCoverage` already loads a file's coverage from the
latest available commit, so uploading coverage for different files in different
commits with "update" will result in seeing the latest uploaded coverage in
Diffusion.
Test Plan: manual local verification
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14428
Summary: Drops Join Policy, uses Edit Policy where needed. Allows anyone with Blog Edit permissions to post and edit any post on that blog. Fixes T5371
Test Plan: Draft Post as chad, see post, log in with notchad, edit that post and publish it.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T5371
Differential Revision: https://secure.phabricator.com/D14444
Summary: Currently, a bunch of developers are using #xhpast for writing custom linter rules. As such, we end up with a fair few `XHPASTSyntaxErrorException` in our PHP error logs. I think that throwing an exception is not quite correct in this case because it is somewhat expected that invalid PHP may be entered. Instead, catch the exception and show the user a helpful message.
Test Plan: This doesn't quite work yet... the stream and tree views render as blank but the exceptions still propogate to the error logs. Mostly, I'm not sure how the exception should be rendered for display.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14028
Summary: Add some mailkeys, allow feed stories to be published.
Test Plan: New Blog, Edit Blog
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14434
Summary: Larger (open) installs may want to restrict Blog to formal entities, like with Phriction.
Test Plan: Set policy to administrators, have notchad try to create a blog. See error.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14433
Summary: Allows feed stories and mail for new Phame Posts.
Test Plan: Write Post, Get Mail
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14426
Summary: Updates "View Post" to use PHUIDocumentViewPro, updates calls to `newPage` and other minor modernizations. Edit Page updated to show proper document display as well. Ref T9545
Test Plan:
Write a blog post, edit it.
{F945897}
{F945896}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9545
Differential Revision: https://secure.phabricator.com/D14415
Summary:
Fixes T9675.
Fixes the calculation for the difference between the first day of the month and the first day of the week. It was previously possible for this to be negative, which caused the subsequent loop to hang the browser.
Test Plan:
- Set week to start on Sunday. Observed that the calendar still renders fine.
- Set week to start on Monday. Observed that the browser no longer hangs on November 2015.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9675
Differential Revision: https://secure.phabricator.com/D14409
Summary: This is a bit too grey, and doesn't match our theme well (see sequence navs)
Test Plan: Remarkup reference article, sequence navs
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14408
Summary: This just reads easier to me.
Test Plan: Highlight some text, see new padding.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14407
Summary: This margin rule is bleeding in some cases where we wrap `phabricator-remarkup` around a large amount of other content (like Dialogs). This restricts the rule to the first direct child.
Test Plan: Go to a workboard, edit a task, see proper alignment of icons.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14406
Summary:
Fix T9662.
Record who initiated the build, and allow this information as a parameter.
In this implementation, a 're-run' keeps the original initiator, which we maybe not desired?
Test Plan:
Make a HTTP step with initiator.phid, trigger manually, via HM, via ./bin/harbormaster build.
Look at requests made.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T9662
Differential Revision: https://secure.phabricator.com/D14380
Summary: Use in MailCommands and HTTP Parameters
Test Plan: Tested MailCommands in Paste, HTTP Parameters in Paste, Legalpad, Diviner. Mobile and Desktop breakpoints.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14397
Summary:
Ref T9132. Although forms do generally support prefilling right now, you have to guess how to do it.
Provide an explicit action showing you which values are supported and how to prefill them. This is generated automatically when an application switches to ApplicationEditor.
Test Plan:
{F939804}
{F939805}
{F939806}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14392
Summary: These fonts are functionally very similar, but in diagnosing a problem with mobile Safari/Chrome, it turned out that our use of "bold" with the "normal" font build created a "semibold" look when on desktop and a "normal" look on mobile. The "semibold" feel is more important, so finding a lighter "bold" font was the impetus for this font switch. As it turns out **Aleo** is built by the same author as **Lato** (our other font) and is intended as it's companion. So stylistically, this is the more correct font.
Test Plan:
Test Phriction, Legalpad, Diviner, Desktop and Mobile
{F938013}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14391
Summary: Generated Roboto Slab with the same settings as Lato, which produces a smaller file. Also hope it fixes mobile kerning.
Test Plan: View various documents, fonts look identical.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14388
Summary: Adds some basic style to new !!Remarkup Highlighter!! Ref T5560
Test Plan: Wait for next diff.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T5560
Differential Revision: https://secure.phabricator.com/D14383
Summary: Rolls out PHUIDocumentViewPro to Legalpad. Minor tweaks to provide space around Preamble and Signature blocks. Otherwise, straight forward.
Test Plan:
Build a new document with and without Preamble, sign document.
{F933386}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14377
Summary:
This implements `PHUIDocumentViewPro` which should move to be the base for all documents (Phame, Phriction, Legalpad, Diviner). Overall this feels really good to me, but I'd like to roll it out into Diviner specifically first to work through the issues and then move into other apps and drop `PHUIDocumentView` once everything is converted. Some features are:
- White Background, no border on page
- Table of Contents is move to hidden menu (more space for documentation)
- Property List sits under the document
Some design decisions above are in anticipation of Phriction v3 and Unbeta Phame, specifically commenting and maybe some cool new Remarkup text layout options for Phame.
Test Plan:
Went through tons of pages on Diviner on Desktop, Tablet, Mobile. Bounce back to Phriction to make sure DocumentView CSS changes actually look better there.
{F930518}
{F930519}
{F930520}
{F930521}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: tycho.tatitscheff, joshuaspence, Korvin
Differential Revision: https://secure.phabricator.com/D14374
Summary: We haven't seen any issues here, remove the table and schema spec.
Test Plan: Not yet tested.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14067
Summary: This makes PHUIPropertyList display wider when an ActionList isn't present.
Test Plan: Review Diff Details in a Diff. Test mobile and desktop layouts.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13568
Summary: Fixes T9609, applies style to a wrapping div for long tables in Remarkup
Test Plan: Build a long table in Phriction, get scrollbar.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9609
Differential Revision: https://secure.phabricator.com/D14355
Summary: Better formatting for object lists when in a dialog (like subscribers).
Test Plan:
Test a subscription list.
{F911522}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14353
Summary:
Ref T182. When viewing a revision, if there are several error operations and then a success operation, we currently show the last error. This is misleading.
Instead, don't show anything if there's a success (this may require tuning eventually if you can land multiple times onto different branches or whatever, but should be reasonable for now).
Also make the table a little nicer, particularly for merge failure output.
Test Plan: {F910385}
Reviewers: chad, Mnkras
Reviewed By: Mnkras
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14348
Summary:
Fixes T9519. Right now, build steps go straight from the build to the edit screen.
This means that there's no way to see their edit history or review details without edit permission. In particular, this makes it a bit harder to catch the Drydock Blueprint authorization warnings from T9519.
- Add a standard view screen.
- Add a little warning callout to blueprint authorizations.
This also does a bit of a touchup on the weird dropshadow element from T9586. Maybe not totally design-approved now but it's less ugly, at least.
Test Plan:
{F906695}
{F906696}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9519
Differential Revision: https://secure.phabricator.com/D14330
Summary:
Ref T9614. Currently, a lot of Build Plan behavior is covered by a global "can manage" policy.
One install in particular is experiencing difficulty with warring factions within engineering aborting one another's builds.
As a first step to remedy this, and also generally make Harbormaster more flexible and bring it in line with other applications in terms of policy power:
- Give Build Plans normal view/edit policies.
- Require "Can Edit" to run a plan manually.
Having "Can View" on plans may be a little weird in some cases (the status of a Buildable might be bad because of a build you can't see) but we can cross that bridge when we come to it.
Next change here will require "Can Edit" to abort a build. This will reasonably allow installs to reserve pause/abort for administrators/adults. (I might let anyone restart a plan, though?)
Test Plan:
- Created a new build plan.
- Verified defaults were inherited from application defaults (swapped them around, too).
- Saved build plan.
- Edited policies.
- Verified autoplans get the right policies.
- Verified old plans got migrated properly.
- Tried to run a plan I couldn't edit (denied).
- Ran a plan from CLI with `bin/harbormaster`.
- Tried to create a plan with an unprivileged user.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9614
Differential Revision: https://secure.phabricator.com/D14321
Summary:
It's hard for us to predict how long patches and migrations will take in the general case since it varies a lot from install to install, but we can give installs some kind of rough heads up about longer patches. I'm planning to just put a sort of hint for things in the changelog, something like this:
{F905579}
To make this easier, start storing how long stuff took. I'll write a little script to dump this into a table for the changelog.
Test Plan:
Ran `bin/storage status`:
{F905580}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14320
Summary:
Ref T182. Replace the total mess we had before with a sort-of-reasonable element.
This automatically updates using "javascript".
Test Plan:
{F901983}
{F901984}
Used "Land Revision", saw the land status go from "Waiting" -> "Working" -> "Landed" without having to mash reload over and over again.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14314
Summary: Ref T9336. Links the timeline photo to user profile. Presume this always exists?
Test Plan: Review a few timelines, click on heads.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9336
Differential Revision: https://secure.phabricator.com/D14283
Summary:
Ref T182. This doesn't do anything interesting yet and is mostly scaffolding, but here's roughly the workflow. From previous revision, you can configure "Repository Automation" for a repository:
{F875741}
If it's configured, a new "Land Revision" button shows up:
{F875743}
Once you click it you get a big warning dialog that it won't work, and then this shows up at the top of the revision (completely temporary/placeholder UI, some day a nice progress bar or whatever):
{F875747}
If you're lucky, the operation eventually sort of works:
{F875750}
It only runs `git show` right now, doesn't actually do any writes or anything.
Test Plan:
- Clicked "Land Revision".
- Watched `phd debug task`.
- Saw it log `git show` to output.
- Verified operation success in UI (by fiddling URL, no way to get there normally yet).
Reviewers: chad
Reviewed By: chad
Subscribers: revi
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14266
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 T9519. When acquiring leases on resources:
- Only consider resources created by authorized blueprints.
- Only consider authorized blueprints when creating new resources.
- Fail with a tailored error if no blueprints are allowed.
- Fail with a tailored error if missing authorizations are causing acquisition failure.
One somewhat-substantial issue with this is that it's pretty hard to figure out from the Harbormaster side. Specifically, the Build step UI does not show field value anywhere, so the presence of unapproved blueprints is not communicated. This is much more clear in Drydock. I'll plan to address this in future changes to Harbormaster, since there are other related/similar issues anyway.
Test Plan: {F872527}
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9519
Differential Revision: https://secure.phabricator.com/D14254
Summary:
Ref T9519. This is like 80% of the way there and doesn't fully work yet, but roughly shows the shape of things to come. Here's how it works:
First, there's a new custom field type for blueprints which works like a normal typeahead but has some extra logic. It's implemented this way to make it easy to add to Blueprints in Drydock and Build Plans in Harbormaster. Here, I've added a "Use Blueprints" field to the "WorkingCopy" blueprint, so you can control which hosts the working copies are permitted to allocate on:
{F869865}
This control has a bit of custom rendering logic. Instead of rendering a normal list of PHIDs, it renders an annotated list with icons:
{F869866}
These icons show whether the blueprint on the other size of the authorization has approved this object. Once you have a green checkmark, you're good to go.
On the blueprint side, things look like this:
{F869867}
This table shows all the objects which have asked for access to this blueprint. In this case it's showing that one object is approved to use the blueprint since I already approved it, but by default new requests come in here as "Authorization Requested" and someone has to go approve them.
You approve them from within the authorization detail screen:
{F869868}
You can use the "Approve" or "Decline" buttons to allow or prevent use of the blueprint.
This doesn't actually do anything yet -- objects don't need to be authorized in order to use blueprints quite yet. That will come in the next diff, I just wanted to get the UI in reasonable shape first.
The authorization also has a second piece of state, which is whether the request from the object is active or inactive. We use this to keep track of the authorization if the blueprint is (maybe temporarily) deleted.
For example, you might have a Build Plan that uses Blueprints A and B. For a couple days, you only want to use A, so you remove B from the "Use Blueprints: ..." field. Later, you can add B back and it will connect to its old authorization again, so you don't need to go re-approve things (and if you're declined, you stay declined instead of being able to request authorization over and over again). This should make working with authorizations a little easier and less labor intensive.
Stuff not in this diff:
- Actually preventing any allocations (next diff).
- Probably should have transactions for approve/decline, at least, at some point, so there's a log of who did approvals and when.
- Maybe should have a more clear/loud error state when no blueprints are approved?
- Should probably restrict the typeahead to specific blueprint types.
Test Plan:
- Added the field.
- Typed some stuff into it.
- Saw the UI update properly.
- Approved an authorization.
- Declined an authorization.
- Saw active authorizations on a blueprint page.
- Didn't see any inactive authroizations there.
- Clicked "View All Authorizations", saw all authorizations.
Reviewers: chad, hach-que
Reviewed By: chad
Maniphest Tasks: T9519
Differential Revision: https://secure.phabricator.com/D14251
Summary:
Ref T9524. Because fetching the last time files were modified in Diffusion can be slow, we bring it in over Ajax.
The logic to fetch and paint the table is kind of fragile because there are two different definitions of the columns right now and we break in a bad way if they differ.
In particular, calling `diffusion.updatecoverage` can populate a "lint commit" for a repository, which tries to generate lint information in one of the views (but not the other one).
In the longer run I think we're removing some of the concepts here and this rendering should be rebuilt to not have two separate column definitions, but just make it degrade gracefully for now since those are larger changes.
Test Plan:
Reproduced the issue in T9524 by calling `diffusion.updatecoverage` on a repostiory. Specifically, this has a side effect of creating a "lint commit" which triggers a "lint" column in this table, sort of.
Applied this patch, got a clean render.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9524
Differential Revision: https://secure.phabricator.com/D14243
Summary:
Fixes T9500. All the code is fine in D13836, but the value of the constant got updated (from "open" to "active") and the migration still used the old value.
Correct any affected dashboards to use the proper constant.
This only affected old dashboards: newly created ones use the right constant.
Test Plan: Ran migration, verified that all active dashboards appeared on "Active Dashboards".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9500
Differential Revision: https://secure.phabricator.com/D14223
Summary: Ref T9352. See D13635. Build targets can have variables already, but let builds have them too. This mostly enables future use cases (sub-builds, more sophisticated build triggers).
Test Plan: With a custom Herald rule + action like the one in T9352, updated a revision and saw it generate multiple builds with varying parameters.
Reviewers: chad, hach-que
Reviewed By: hach-que
Maniphest Tasks: T9352
Differential Revision: https://secure.phabricator.com/D14222
Summary:
Ref T9252. Long ago you sometimes manually created resources, so they had human-enterable names. However, users never make resources manually any more, so this field isn't really useful any more.
In particular, it means we write a lot of untranslatable strings like "Working Copy" to the database in the default locale. Instead, do the call at runtime so resource names are translatable.
Also clean up a few minor things I hit while kicking the tires here.
It's possible we might eventually want to introduce a human-choosable label so you can rename your favorite resources and this would just be a default name. I don't really have much of a use case for that yet, though, and I'm not sure there will ever be one.
Test Plan:
- Restarted a Harbormaster build, got a clean build.
- Released all leases/resources, restarted build, got a clean build with proper resource names.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14213
Summary:
Ref T9252. Several general changes here:
- Moves logs to use PHIDs instead of IDs. This generally improves flexibility (for example, it's a lot easier to render handles).
- Adds `blueprintPHID` to logs. Although you can usually figure this out from the leasePHID or resourcePHID, it lets us query relevant logs on Blueprint views.
- Instead of making logs a top-level object, make them strictly a sub-object of Blueprints, Resources and Leases. So you go Drydock > Lease > Logs, etc., to get to logs.
- I might restore the "everything" view eventually, but it doesn't interact well with policies and I'm not sure it's very useful. A policy-violating `bin/drydock log` might be cleaner.
- Policy-wise, we always show you that logs exist, we just don't show you log content if it's about something you can't see. This is similar to seeing restricted handles in other applications.
- Instead of just having a message, give logs "type" + "data". This will let logs be more structured and translatable. This is similar to recent changes to Herald which seem to have worked well.
Test Plan:
Added some placeholder log writes, viewed those logs in the UI.
{F855199}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14196
Summary: Updates to their new logo
Test Plan: review in photoshop
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14199
Summary: I haven't regenerated this for a while and it makes instances and unit tests a little faster.
Test Plan:
- Manually reviewed changes for sanity.
- Ran `arc unit --everything`.
- Observed runtime drop from ~15-16 seconds to ~12-13 seconds.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14192
Summary:
Fixes T6569. This implements an expiry mechanism for Drydock resources which parallels the mechanism for leases.
A few things are missing that we'll probably need in the future:
- An "EXPIRES" command to update the expiration time. This would let resources be permanent while leased, then expire after, say, 24 hours without any leases.
- A callback like `shouldActuallyExpireRightNow()` for resources and leases that lets them decide not to expire at the last second.
- A callback like `didAcquireLease()` for resource blueprints, to parallel `didReleaseLease()`, letting them clear or extend their timer.
However, this stuff would mostly just let us tune behaviors, not really open up new capabilities.
Test Plan: Changed host resources to expire after 60 seconds, leased one, saw it vanish 60 seconds later.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T6569
Differential Revision: https://secure.phabricator.com/D14176
Summary: Ref T9252. If you have a blueprint and you do not like that blueprint very much, you can disable it.
Test Plan: Disabled / enabled some blueprints.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14156
Summary:
Ref T9252. Drydock currently uses integer statuses, but there's no reason for this (they don't need to be ordered) and it makes debugging them, working with them, future APIs, etc., more cumbersome.
Switch to string instead.
Also rename `STATUS_OPEN` to `STATUS_ACTIVE` and `STATUS_CLOSED` to `STATUS_RELEASED` for consistency. This makes resources and leases have more similar states, and gives resource states more accurate names.
Test Plan: Browsed web UI, grepped for changed constants, applied patch, inspected database.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14153
Summary:
Ref T9252. Leases currently have a `resourceID`, but this is a bit nonstandard and generally less flexible than giving them a `resourcePHID`.
In particular, a `resourcePHID` is easier to use when rendering interfaces, since you can get handles out of a PHID.
Add a PHID column, copy over all the PHIDs that correspond to existing IDs, then drop the ID column.
Test Plan:
- Browsed web UIs.
- Inspected database during/after migration.
- Grepped for `resourceID`.
- Allocated a new lease with `bin/drydock lease`.
Reviewers: chad, hach-que
Reviewed By: hach-que
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14151
Summary:
Ref T9464. If an ancient transaction doesn't have array values for whatever reason, we fail here.
Instead, just recover as gracefully as we can. We may get the transaction "wrong" in some sense, but this only impacts what is rendered in the transaction log.
Test Plan: This is nearly a year old and there's no real way to test it.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9464
Differential Revision: https://secure.phabricator.com/D14149
Summary:
Ref T9252. This simplifies some Drydock code.
Most of this code relates to the old notion of Drydock being able to enumerate all the tasks it needs to complete in order to acquire a lease. The code has stepped back from this, since it's unnecessary, the queue is more powerful than it used to be, and it would be a lot of work to keep track of.
The ~only thing that should ever wait for leases in modern code is `bin/drydock lease`, and it's fine for it to just sit there sleeping, so this just does that.
This reduces the granularity of logging, but I'll address that separately in future logging-focused changes.
Test Plan: Used `bin/drydock lease` to acquire a lease, saw it acquire cleanly.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14147
Summary:
Ref T9252. Broadly, Drydock currently races on releasing objects from the "active" state. To reproduce this:
- Scatter some sleep()s pretty much anywhere in the release code.
- Release several times from web UI or CLI in quick succession.
Resources or leases will execute some release code twice or otherwise do inconsistent things.
(I didn't chase down a detailed reproduction scenario for this since inspection of the code makes it clear that there are no meaningful locks or mechanisms preventing this.)
Instead, add a Harbormaster-style command queue to resources and leases. When something wants to do a release, it adds a command to the queue and schedules a worker. The workers acquire a lock, then try to consume commands from the queue.
This guarantees that only one process is responsible for writes to active resource/leases.
This is the last major step to giving resources and leases a single writer during all states:
- Resource, Unsaved: AllocatorWorker
- Resource, Pending: ResourceWorker (Possible rename to "Allocated?")
- Resource, Open: This diff, ResourceUpdateWorker. (Likely rename to "Active").
- Resource, Closed/Broken: Future destruction worker. (Likely rename to "Released" / "Broken"; maybe remove "Broken").
- Resource, Destroyed: No writes.
- Lease, Unsaved: Whatever wants the lease.
- Lease, Pending: AllocatorWorker
- Lease, Acquired: LeaseWorker
- Lease, Active: This diff, LeaseUpdateWorker.
- Lease, Released/Broken: Future destruction worker (Maybe remove "Broken"?)
- Lease, Expired: No writes. (Likely rename to "Destroyed").
In most phases, we can already guarantee that there is a single writer without doing any extra work. This is more complicated in the "Active" case because the release buttons on the web UI, the release tools on the CLI, the lease requestor itself, the garbage collector, and any other release process cleaning up related objects may try to effect a release. All of these could race one another (and, in many cases, race other processes from other phases because all of these get to act immediately) as this code is currently written. Using a queue here lets us make sure there's only a single writer in this phase.
One thing which is notable is that whatever acquires a lease **can not write to it**! It is never the writer once it queues the lease for activation. It can not write to any resources, either. And, likewise, Blueprints can not write to resources while acquiring or releasing leases.
We may need to provide a mechinism so that blueprints and/or resource/lease holders get to attach some storage to resources/leases for bookkeeping. For example, a blueprint might need to keep some kind of cache on a resource to help it manage state. But I think we can cross that bridge when we come to it, and nothing else would need to write to this storage so it's technically straightforward to introduce such a mechanism if we need one.
Test Plan:
- Viewed buttons in web UI, checked enabled/disabled states.
- Clicked the buttons.
- Saw commands show up in the command queue.
- Saw some daemon stuff get scheduled.
- Ran CLI tools, saw commands get consumed and resources/leases release.
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14143
Summary:
See discussion in D10304. There's a lot of context there, but the general idea is:
- Blueprints should manage locks in a granular way during the actual allocation/acquisition phase.
- Optimistic "slot locks" might a pretty good primitive to make that easy to implement and reason about in most cases.
The way these locks work is that you just pick some name for the lock (like the PHID of a resource) and say that it needs to be acquired for the allocation/acquisition to work:
```
...
->needSlotLock("mylock(PHID-XYZQ-...)")
...
```
When you fire off the acquisition or allocation, it fails unless it could acquire the slot with that name. This is really simple (no explicit lock management) and a pretty good fit for most of the locking that blueprints and leases need to do.
If you need to do limit-based locks (e.g., maximum of 3 locks) you could acquire a lock like this:
```
mylock(whatever).slot(2)
```
Blueprints generally only contend with themselves, so it's normally OK for them to pick whatever strategy works best for them in naming locks.
This may not work as well if you have a huge number of slots (e.g., 100TB you want to give out in 1MB chunks), or other complex needs for locks (like you have to synchronize access to some external resource), but slot locks don't need to be the only mechanism that blueprints use. If they run into a problem that slot locks aren't a good fit for, they can use something else instead. For now, slot locks seem like a good fit for the problems we currently face and most of the problems I anticipate facing.
(The release workflows have other race issues which I'm not addressing here. They work fine if nothing races, but aren't race-safe.)
Test Plan:
To create a race where the same binding is allocated as a resource twice:
- Add `sleep(10)` near the beginning of `allocateResource()`, after the free bindings are loaded but before resources are allocated.
- (Comment out slot lock acquisition if you have this patch.)
- Run `bin/drydock lease ...` in two windows, within 10 seconds of one another.
This will reliably double-allocate the binding because both blueprints see a view of the world where the binding is free.
To verify the lock works, un-comment it (or apply this patch) and run the same test again. Now, the lock fails in one process and only one resource is allocated.
Reviewers: hach-que, chad
Reviewed By: hach-que, chad
Differential Revision: https://secure.phabricator.com/D14118
Summary: These already have a larger font, the extra height isn't needed. Make them the same size as `td`
Test Plan: Look at a bunch of tables.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14135
Summary: These currently use different yellows, and more importantly when pygmentized, is hard to read. This lightens up code embeds and pastes.
Test Plan:
Review Diviner docs, Pastes, and normal code embeds.
{F820471}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: tycho.tatitscheff, Korvin
Differential Revision: https://secure.phabricator.com/D14123
Summary: Builds a container of paste, makes it smaller on mobile.
Test Plan: View on desktop, tablet, mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14122
Summary: Wraps entire element in the anchor tag, gives a hover state, makes icons bounce.
Test Plan: Hover and click.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14124
Summary: Adds full ROYGBIVP color spectrum, adds basic overflow, collapse protection.
Test Plan: Review small and large panels are various breakpoints.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14120
Summary: Making these a little more fun, a little more flexible and better looking. Will have an update for rSAAS in a bit.
Test Plan:
Make lots of them. Click.
{F815658}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14115
Summary:
Ref T7785. This prepares for (but does not yet use) a pure PHP implementation of Figlet parsing and rendering.
Figlet is somewhat complex, but a parser already exists in PEAR. I'll make sure it's suitable and hook it up in the next diff.
Test Plan: N/A, code not reachable
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9408, T7785
Differential Revision: https://secure.phabricator.com/D14101
Summary:
Ref T7785. Convert the Cowsay Remarkup rule to use a PHP implementation so we don't have to execute an external `cowsay` binary.
I removed some of the default ".cow" files that come with Cowsay because they:
- include Perl code which we can not interpret; or
- are primarily in-jokes or standalone visual puns or artwork rather than usable actors on the grand stage of cowsay; or
- offended my delicate sensibilities.
Users can add new cows to `resources/cows/custom/` if they want to make new cows available.
I have included a majestic original artwork depicting the "Companion Cube" character from //Portal//.
Test Plan: {F802535}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9408, T7785
Differential Revision: https://secure.phabricator.com/D14100
Summary:
This reverts commit 1583738842.
See T8646 for discussion. This version of the feature feels terrible on real data.
Test Plan: Strict revert.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14097
Summary:
Fixes T9126. In particular:
- Add "Browse" links to all history views.
- Use icons to show "Browse" and "History" links, instead of text.
- Use FontAwesome.
- Generally standardize handling of these elements.
This might need a little design attention, but I think it's an improvement overall.
Test Plan:
- Viewed repository history.
- Viewed branch history.
- Viewed file history.
- Viewed table of contents on a commit.
- Viewed merged changes on a merge commit.
- Viewed a directory containing an external.
- Viewed a deleted file.
{F788419}
{F788420}
{F788421}
{F788422}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9126
Differential Revision: https://secure.phabricator.com/D14096
Summary:
Ref T8646. This is fairly rough:
This interface is very niche, and not really flexible enough to accommodate other result customization (but I don't think we have any plans here)?
I'm just //summarizing// the content of documents, basically showing the first paragraph of their content, summary, etc. This isn't what Google does: it shows snippets surrounding the actual search terms. However, this is more involved and might be less useful in structured data: for example, I'd imagine that the first line of most phriciton documents, maniphest tasks and Differential revisions really might be the best machine-generatable summary of them. The actual contextual snippeting in Google doesn't often seem hugely useful to me. But this might also not be very useful.
There's not much design, not sure if you had any ideas.
I only implemented this for tasks, revisions and the wiki since those seem most useful.
I'm generally on the fence about this, but it's not a ton of work to swap out for something else later. Maybe we can see how it feels? But happy to toss it or rethink the approach.
Test Plan: {F788026}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8646
Differential Revision: https://secure.phabricator.com/D14095
Summary: Fixes T9351. This is straightforward since this application is now relatively modern and doesn't have any bizarre craziness.
Test Plan:
{F787981}
{F787982}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9351
Differential Revision: https://secure.phabricator.com/D14093
Summary: Fixes T8572. Ideally we would probably just permit this, but clean up the behavior until the day arrives when inline code is actually rewritten.
Test Plan:
- Tried to launch editors in Differential and Diffusion while comments were already open.
- Verified that "Jump to inline" works in both cases.
{F788008}
{F788009}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8572
Differential Revision: https://secure.phabricator.com/D14094
Summary:
Ref T9324. When you upload a normal file, we call `willUpload` and then `didUpload`.
When you upload a chunked file, we never call `willUpload`. This can get things out of sync. Make sure we invoke this event for both chunked and non-chunked uploads.
Test Plan: Got cleaner behavior (redirect after all uploads finish) in new Phacility file upload area.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9324
Differential Revision: https://secure.phabricator.com/D14083
Summary:
- Fix missing space before "For example:".
- Fix instruction to run `bin/config set value` instead of `bin/config set key value`.
- Minor cleanup.
Test Plan: Tried to set `files.image-mime-types`, `load-libraries`.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14080
Summary: This is required by Aphront now but not given a default implementation in the base class.
Test Plan: CORGI sites now work.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14079
Summary: Tables headers break all kinds of funky even though we set `word-break` to word presumably because width is not calculated. Just use normal breaks.
Test Plan: Test developer documentation table in sand box, table is readable on desktop, phone break points.
Reviewers: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14063
Summary:
One of the migrations in rPa335004a91 (`20150730.herald.5.sql`) incorrectly swapped "add" and "add blocking" Differential Herald rules.
Swap any rules last modified before this patch was applied back. This is the best we can do without possibly overwriting more recent, intentional data. I'll issue some guidance on this in the changelog.
Test Plan:
- Made a rule, ran patch, no change.
- Changed rule modified time to a few months ago, ran patch, saw swap from non-blocking to blocking.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14061
Summary:
Ref T7148. I can do most of the export stuff by only modifying the Instances codebase, but want to upload all the backups and exports as temporary files and can't currently do this via the API.
Make the necessary API changes so that the export workflow can use them when it gets built out.
Test Plan: See next diff. Uploaded files with `arc upload --temporary` and saw them upload as temporary files.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7148
Differential Revision: https://secure.phabricator.com/D14055
Summary: There are a handful of places I've been wanting to use a button here. Adds that ability and uses in app launcher.
Test Plan:
Test Applicatons->Launcher at desktop, mobile, tablet breakpoints
{F780453}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14059
Summary:
Fixes T9339.
- Don't show edit control for locked config at all.
- Don't show a "Cancel" button either.
- Change "Value" label to "Database Value" for non-custom config.
- Highlight effective value.
- Move examples under current state.
- Tweak some formatting.
Test Plan: {F777878}
Reviewers: chad, avivey
Reviewed By: chad, avivey
Subscribers: avivey
Maniphest Tasks: T9339
Differential Revision: https://secure.phabricator.com/D14054
Summary: Fixes T9314. Functionally phui-status-list should get moved off a table, but that's another day. This catches many other possible issues.
Test Plan: Review changes on a narrow browser.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9314
Differential Revision: https://secure.phabricator.com/D14036
Summary: Fixes T9264. I'm surprised this hasn't come up before, but any long string or URL in remarkup will overflow and cause remarkup areas to scroll. Prefer breaking these words.
Test Plan:
Review a timeline feed in Differential and a Ponder answer.
{F768105}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9264
Differential Revision: https://secure.phabricator.com/D14018
Summary: Makes the New Comment, See Comments more obviously placed to find.
Test Plan: Review new CSS, answer question, comment, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14043
Summary:
Ref T9309. This is a minor quality of life improvement, hopefully. We already have print CSS, just expose it more clearly.
Also, hide actions (these never seem useful?) and footers from printable versions. I opened the printable version in a new window since it now doesn't have any actions.
Test Plan:
{F777241}
{F777242}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9309
Differential Revision: https://secure.phabricator.com/D14045
Summary:
Fixes T9302. This datasource wasn't resolving package PHIDs correctly for the actual query.
Also fixes an issue with the "Affected packages that need audit" Herald rule.
Test Plan: Ran a "Needs Audit" query with only packages, and only `packages(user)`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9302
Differential Revision: https://secure.phabricator.com/D14029
Summary: Fixes T8901 by adding in additional colors used by icons. Plus fire. Fire is cool.
Test Plan: Try out new colors in maniphest priorities.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T8901
Differential Revision: https://secure.phabricator.com/D14020
Summary: Adds an additional field for questions, an answer wiki, should should usually be community editable.
Test Plan: New question, edit question, no wiki, lots of wiki.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14003
Summary: Fixes T9217, adds detection for logged in users and adjusts the layout accordingly.
Test Plan: View logged in and logged out Conpherence
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9217
Differential Revision: https://secure.phabricator.com/D14002
Summary: Until we have a proper close as duplicate workflow for Ponder, remove the option with something more sensible.
Test Plan: Closed a question as invalid, saw it closed and in feed.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14007
Summary: Fix T8710. I had hopes of doing something cleaver with `highlighted` (Like trying to understand `foo.bar` when clicking `bar`, but I obviously didn't do it.
Test Plan: ctrl-click.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: joshuaspence, epriestley, gena2x, Korvin
Maniphest Tasks: T8710
Differential Revision: https://secure.phabricator.com/D13550
Summary:
Fixes T8501.
When losing focus while holding ctrl, we never get a key-up event; ctrl-f/d/tab make the browser tab lose focus.
So treat 'blur' (unfocus) as if the user released ctrl.
Test Plan: ctrl-f/ctrl-d/ctrl-tab, ctrl-click-outside-of-window, and move mouse over the content - see no help suggestions.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Maniphest Tasks: T8501
Differential Revision: https://secure.phabricator.com/D13260
Summary:
Ref T8783. Sort out some relationships and fields:
- Make Items 1:1 with Queues: each item is always in exactly one queue. Minor discussion on T8783. I think this is easier to understand and reason about (and implement!) and can't come up with any real cases where it isn't powerful enough.
- Remove "QueueItem", which allowed items to be in multiple queues at once.
- Remove "dateNuanced", which is equivalent to "dateCreated" in all cases.
Then add really basic routing:
- Add "Default Queue" for Sources. New items from the source route into that queue.
- (Some day there will be routing rules, but for now the rule is "always route into the default queue".)
- Show queue on items.
- Show more / more useful edit history and transactions in several UIs.
Test Plan:
{F749445}
{F749446}
{F749447}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8783
Differential Revision: https://secure.phabricator.com/D13988
Summary: There is still some general buginess with answer comments, trying to work them out. This replaces timeline rendering into one offs (less performant) but resolves many bugs. Or if there is a more performant way, let me know? Also when leaving an answer comment, you currently get redirected back to the page, but both the comment form is still populated and you dont see your answer without a reload. I feel like I'm missing some magical parameter to pass, so just redirecting back to the question itself.
Test Plan: Leave lots of answer comments.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13946
Summary: If we have lots ot `text in monospace` everywhere, the padding bleeds. Reduce 1px.
Test Plan: eyeball it
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13947
Summary: Fixes T8428. Adds status to packages, allows setting and application search. I presume though these need checked elsewhere?
Test Plan: New package, edit package, archive package, run search queries.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T8428
Differential Revision: https://secure.phabricator.com/D13925
Summary: Fixes T9180, removes the background color and wraps the text.
Test Plan: View on a shrunked Chrome.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9180
Differential Revision: https://secure.phabricator.com/D13915
Summary: Fixes T9154, tables have special needs with widths and pres
Test Plan: Test a long piece of code in a Ponder question
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9154
Differential Revision: https://secure.phabricator.com/D13904
Summary: Standardize color and spacing.
Test Plan:
`Wrote a test plan`
{F720873}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13916
Summary: Fixes T9099, I think this is as much as I can come up with for unbeta. Cleans up the answer header (profile image, smaller font, smaller header). Cleans up voting (new, with color), and makes it a bit more readable.
Test Plan:
Review a number of answers in ponder with and without votes, comments.
{F720189}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9099
Differential Revision: https://secure.phabricator.com/D13907
Summary:
Ref T8659. In the general case, this eventually allows build processes to do things like:
- Upload build results (like a ".app" or ".exe" or other binary).
- Pass complex results between build steps (e.g., build step A does something hard and build step B uses it to do something else).
Today, we're a long way away from having the infrastructure for that. However, it is useful to let third party build processes (like Jenkins) upload URIs that link back to the external build results.
This adds `harbormaster.createartifact` so they can do that. The only useful thing to do with this method today is have your Jenkins build do this:
params = array(
"uri": "https://jenkins.mycompany.com/build/23923/details/",
"name": "View Build Results in Jenkins",
"ui.external": true,
);
harbormaster.createartifact(target, 'uri', params);
Then (after the next diff) we'll show a link in Differential and a prominent link in Harbormaster. I didn't actually do the UI stuff in this diff since it's already pretty big.
This change moves a lot of code around, too:
- Adds PHIDs to artifacts.
- It modularizes build artifact types (currently "file", "host" and "URI").
- It formalizes build artifact parameters and construction:
- This lets me generate usable documentation about how to create artifacts.
- This prevents users from doing dangerous or policy-violating things.
- It does some other general modernization.
Test Plan:
{F715633}
{F715634}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8659
Differential Revision: https://secure.phabricator.com/D13900
Summary: Ref T9173, adds basic hide support for answers. Answer authors and Moderators can hide answers, unhide them.
Test Plan: Hide answer, log into other account, see hidden message. Mark as visible, see answer again.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9173
Differential Revision: https://secure.phabricator.com/D13894
Summary: Ref T9099, A step forward for the main Ponder UI. Mostly moving stuff into View classes and reducing clutter. Took a pass at keeping comments and helpfuls, but unclear what the 'final' UI will be (I'm just designing as I use the product).
Test Plan:
Review a number of questions and answers.
{F702495}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9099
Differential Revision: https://secure.phabricator.com/D13872
Summary: Ref T9099. Testing out a two column layout in Ponder, with the main idea being creating a more browsable, discoverable product. I'd like the side column though to be a little smarter and provide project based searching. Ideally, if I'm reading Resolved Maniphest questions, other Resolved Maniphest questions are likely interesting. Another scenario is if I'm answering questions, in which case browsing more Open questions would also be interesting. Ponder "Main Column" still needs to be redesigned.
Test Plan: Browse open questions, resolved questions.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9099
Differential Revision: https://secure.phabricator.com/D13849
Summary: Ref T6920, This removes the PonderVotableInterface from PonderQuestion and assocaited code. Also... never used?
Test Plan: Visit Ponder, See List, New Question, Add Answer.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T6920
Differential Revision: https://secure.phabricator.com/D13833
Summary: Ref T6920, This just removes the old voting UI from Ponder.
Test Plan: Visit a Question, no voting UI
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T6920
Differential Revision: https://secure.phabricator.com/D13827
Summary: This allows installs to essentially set a "moderator" for Ponder, who can clean up answers. Fixes T9098
Test Plan: Edit an answer I don't own.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9098
Differential Revision: https://secure.phabricator.com/D13818
Summary: This fails to apply on my second sandbox with incorrect DOUBLE value. Reran SQL, works as expected.
Test Plan: Rerun new SQL on ponder_question table
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13830
Summary: Ref T9096. This is a first cut at adding additional statuses, happy to add or subtract as needed... maybe even configurable? Also, the dialog doesn't seem to fire, I'll keep debugging.
Test Plan: Close and Reopen many questions. Test applicationSearch params by seeing resolved questions, all questions.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9096
Differential Revision: https://secure.phabricator.com/D13826
Summary: This is causing mis-alignment and is no longer needed.
Test Plan: Check alignment in Photoshop
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13815
Summary: Ref T9076, adds basic plumbing for setting the state of a Paste.
Test Plan: Archive Paste, Activate Paste, New Paste
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9076
Differential Revision: https://secure.phabricator.com/D13801
Summary: Ref T8493, Ref T3578. Adds spaces support to ponder.
Test Plan: Ask a question in a new space, see new question.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3578, T8493
Differential Revision: https://secure.phabricator.com/D13792
Summary: Ref T3578, adds ability to set a default edit and view policy for questions. Not sure what to set viewPolicy to ?
Test Plan: Test an old question, edit policy still on myself. Test a new question, see new default.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D13791
Summary: Code blocks in Conpherence can be massive and disruptive to a chat-like experience. This caps the height.
Test Plan: Add a ginormous code block to Conpherence, see it hit max-height
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13790
Summary: Ref T3846. Adds mailkey generation and migration.
Test Plan: Ran the migration, see keys in mysql.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3846
Differential Revision: https://secure.phabricator.com/D13785
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. No surprises.
Test Plan:
Created rules using both action variants, applied upgrade, saw rules still work correctly.
{F658842}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13701
Summary: Ref T8726. Converts these actions to be modular. No real surprises in this change.
Test Plan:
{F658709}
- Wrote some rules.
- Migrated them forward.
- Used a bunch of these rules.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13699
Summary:
Ref T8726. This modularizes "Mark with flag", plus rebuilds transcripts in a more modern/flexible way. The big transcript stuff is:
- Transcripts are now translatable.
- Transcripts can now show multiple outputs from a single action. For example, an action like "add A, B, C to subscribers" can now say "added A; B is invalid; C was already subscribed".
Test Plan: {F637784}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, eadler, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13649
Summary:
Ref T8726. Herald actions are technically sort-of modular already, but make them more aggressively modular similar to `HeraldField`.
I plan to obsolete and replace `HeraldCustomAction`.
Test Plan: Saw actions in nice groups; created and ran a "Do Nothing" action. Transcripts are a bit rough for now.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13646
Summary: New Calendar icons!
Test Plan: Try new icons. See new icons.
Reviewers: lpriestley, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13761
Summary: Allows a rough setting of a number and noun for object item list view.
Test Plan:
Use in countdown, set various times.
{F670267}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13742
Summary: For smaller feed columns, make countdown embed attempt to fit.
Test Plan: Narrow Feed Column on Dashboard.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13741
Summary: Adds the launch date, normalize spacing a bit.
Test Plan:
Test a normal and embeded countdowns.
{F670199}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13740
Summary:
Fixes T8958. I renamed this class and table (in D13644), but the migration `20150606.mlist.1.php` needs to load records from the table before the table rename applies.
Put the table back and accept a tiny bit of cruft here to fix this problem. We can no-op the migration after a while.
I'll cherry-pick this to `stable`.
Test Plan: Applied migration. Created Herald rules. Reviewed existing Herald rules.
Reviewers: btrahan
Subscribers: epriestley
Maniphest Tasks: T8958
Differential Revision: https://secure.phabricator.com/D13736
Summary: Put super long text for title and flavor text on badges and resolve breaking issues.
Test Plan: Tested Supercalifragilisticexpialidocious for title and flavor.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13730
Summary: Touch up Countdown timer in page, remarkup, and mobile.
Test Plan: Test in page, remarkup, and mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13732
Summary: Fixes T8964 and a few other details. Cards float to 100%, headers too, policy moved to it's own line.
Test Plan: Mobile workboards at 320 and 360px wide.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8964
Differential Revision: https://secure.phabricator.com/D13722
Summary: Drops the sql column.
Test Plan: View logged in and logged out badges, edited some too.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13715
Summary: Still doesn't mail yet, but the settings now show up.
Test Plan: View email settings, see Badges options.
Reviewers: eadler, epriestley
Reviewed By: eadler, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13712
Summary: DRAFT - throw together Phurl skeleton.
Test Plan: The idea is that `some/long/url` will become `install/Udet4d` and can be viewed and edited at `install/Udet4d/view` and `install/Udet4d/edit`, respectively?
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: joshuaspence, chad, epriestley, Korvin
Maniphest Tasks: T6049
Differential Revision: https://secure.phabricator.com/D13681
Summary: Fixes T8949. Adds the ability to render honors on those who have fought and received badges of distinction and honor.
Test Plan: Write 'asdf'. See 'asdf'.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8949
Differential Revision: https://secure.phabricator.com/D13704
Summary:
Ref T8726. I want to modularize actions like fields, but the base class should be "HeraldAction".
Eventually, "HeraldCondition" should probably be "HeraldFieldRecord", and then both Action and Condition should just be rolled into Rule, probably, but that can wait and doesn't block anything.
Test Plan: Ran migration, poked around UI, used `git grep`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13644
Summary: Allows countdowns to have a description.
Test Plan: Use description, edit description. Check timeline, etc.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13691
Summary:
Basic plumbing for Badges application.
- You can make Badges.
- You can look at a list of them.
- They can be edited.
- They can be assigned to people.
- You can revoke them from people.
- You can subscribe to them.
Test Plan: Make Badges with various options. Give them to people. Take them away from people.
Reviewers: lpriestley, epriestley
Reviewed By: epriestley
Subscribers: tycho.tatitscheff, johnny-bit, epriestley, Korvin
Maniphest Tasks: T6526
Differential Revision: https://secure.phabricator.com/D13626
Summary: These icons weren't actually centering. Widen the width and actually center the text.
Test Plan: Review centering very carefully. Test home desktop / mobile
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13648
Summary: No idea why this needed regenerated, but it fixes the issue. Fixes T8898
Test Plan: Fake a BB account
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: epriestley, Korvin
Maniphest Tasks: T8898
Differential Revision: https://secure.phabricator.com/D13661
Summary: We use different background colors on <body> depending on ui.header. This makes the workpanel header transparent to fit all cases.
Test Plan: Switch to `dark` header color and check work boards for no UI glitches.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13658
Summary: Fixes T8885. This view is restricted enough on mobile that we can widen and center the icon.
Test Plan: Test workboards, pinning apps, viewing action menus.
Reviewers: btrahan, joshuaspence, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8885
Differential Revision: https://secure.phabricator.com/D13647
Summary: Fixes T8827. Firefox and other browsers differ here. Going to make the header relative so boards always slide under it.
Test Plan: Test workboard with scrolling in FF and Chrome.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: epriestley, Korvin
Maniphest Tasks: T8827
Differential Revision: https://secure.phabricator.com/D13630
Summary: Fixes T8829. Moves the padding and background to the outer container instead of the table itself.
Test Plan: Test Calendar Month View, Calendar Day View, A Text Panel with Multiple Dashboards.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8829
Differential Revision: https://secure.phabricator.com/D13631
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'm primarily trying to modularize tokenizer values so we don't have to update JS to add a new one.
This is ultimately the blocker for "select" custom fields working in Herald.
This inches us toward that. I'm //not// modularizing conditions or control types in this round, but hope to end up with hard-coded conditions (which are highly general and very rarely change), hard-coded control types (which are also highly general and very rarely change) and completely modular fields and values (which have mid-to-low generality and change frequently).
Test Plan: Used UI to interact with "none", "text", and new-style "select" controls. No actual support for tokenizers yet.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13613
Summary: Implements T8734
Test Plan: Verify that countdowns can have spaces associated with them. Verify that non-default spaces are displayed in the header of the countdowns in the result of the query.
Reviewers: lpriestley, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13634
Summary: Implements T8733
Test Plan: Verify that spaces policies can be implemented in the creation and editing of polls.
Reviewers: lpriestley, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13537
Summary: Ref T8726. The gruntwork part of this is finally over.
Test Plan:
- Made a huge rule with every field.
- Applied migration.
- Verified the rule was still the same.
- Pushed a bunch of commits and verified transcripts.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13601
Summary: Ref T8726. Just (pre-commit content) one more left.
Test Plan:
- Created a big rule with every field.
- Migrated it.
- Verified the rule was still the same.
- Pushed a bunch of changes and reviewed the transcripts.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13600
Summary: Ref T8726. Make all the DifferentialRevision stuff modular.
Test Plan:
- Created a rule with all fields.
- Ran upgrade.
- Saw all fields preserved with new modular versions.
- Used test console to run rule with all fields, verified field values as broadly sensible.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13598
Summary: Ref T8726. This deals with all the Differential diff fields, same deal as previous changes.
Test Plan:
- Wrote a rule with every field.
- Migrated it.
- Saw the same rule working.
- Rigged the hell out of transcripts (diffs normally do not generate transcripts, because the only action is "block" and they don't exist yet when Herald runs).
- Verified that all fields looked sensible in the transcript.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13590
Summary: Fixes T7604. This is the big scary change which drops the "arcanist project" fields from the database permanently.
Test Plan:
`grep`ped for the following to ensure that I had found all remaining references:
- `/arcanistProject/i`
- `/arcanist_project/i`
- `/projectName/i`
- `/project_name/i`
- `/project_id/i`
WARNING: Wait at least one month before landing this.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T7604
Differential Revision: https://secure.phabricator.com/D12899
Summary: Fixes T8789. This makes status icons centered in the PHUI object list view.
Test Plan: Looked at it.
Reviewers: #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: epriestley
Maniphest Tasks: T8789
Differential Revision: https://secure.phabricator.com/D13592
Summary: Ref T8726.
Test Plan:
Created a giant rule with every commit field:
{F594686}
Ran the upgrade, got the same rule with new fields:
{F594688}
Used "Test Console" to run transcripts, saw all the fields populate correctly.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13567
Summary:
Ref T8746. See discussion there. We could try harder to fix this but I suspect the number of affected installs is exceedingly small since it's ~18 months old. Requiring reconfiguration of credentials seems reasonable-ish.
Also fix a merge issue with Conpherence search.
Test Plan: N/A
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8746
Differential Revision: https://secure.phabricator.com/D13565
Summary: These should all be shades now instead of pure colors.
Test Plan: Fake an error, see correct muted color.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13580
Summary: Fixes T8751, Swaps out links to Diffusion. Also upped font size on Document monospace.
Test Plan: Rebuild Diviner docs, click on new links.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8751
Differential Revision: https://secure.phabricator.com/D13566
Summary: Ref T8726. Use modular fields for the Pholio adapter.
Test Plan:
- Created rule using all the old fields.
- Migrated, saw upgrade apply correctly.
- Created mock, reviewed transcript.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13513
Summary:
Ref T8726. The only notable bit here is that the "body" / "title" fields (which are currently shared across a bunch of types) are getting split into application variants.
Among other things, this will let us label the field "Commit message" for commits, for example.
Test Plan:
- Created a rule using all four fields.
- Applied patch, saw rule break ("unknown field").
- Ran storage upgrade, saw rule fix itself in the migration.
- Edited tasks, triggered rule, viewed transcripts.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, joshuaspence, epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13501
Summary:
Ref T8726. There's no interface we can check for this, so the adapter needs to opt in.
Also fix a spelling mistake.
Test Plan: Created rules with "Application Email" fields.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8726
Differential Revision: https://secure.phabricator.com/D13499
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: This border is meant to be darker, make it relative to background color.
Test Plan: Test and see new darker border.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13564
Summary:
About 1/3 the size. The "Full" Lato set is over 3000 Glyphs, this was a handbuilt set of Latin, Latin-A, and Latin-B. Specific Unicode set:
```U+0100-024F, U+1E00-1EFF, U+20A0-20AB, U+20AD-20CF, U+2C60-2C7F, U+A720-A7FF, U+0000-00FF, U+0131, U+0152-0153, U+02C6, U+02DA, U+02DC, U+2000-206F, U+2074, U+20AC, U+2212, U+2215, U+E0FF, U+EFFD, U+F000```
This unicode set is what Google Fonts uses as "Latin,Latin-Ext"
Test Plan: Built test pages in Phriction for Latin-A, Latin-B. Checked Chrome, Firefox, Mac and PC. If issues get reported, can default back to larger font pack.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8755
Differential Revision: https://secure.phabricator.com/D13558
Summary: Rebuilt Lato font for Firefox, x-height to 100%. Ref T8755
Test Plan: Test Firefox //without// having a local copy of Lato installed on my Mac.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8755
Differential Revision: https://secure.phabricator.com/D13556
Summary: Modern mozilla browsers use min-resolution.
Test Plan: Regernate sprites, use site in Firefox, see correct header image.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13555
Summary: Ref T8755, Adds full fallback support for Lato, the default Phabricator font and champion of Azeroth.
Test Plan: Verify fonts are being used everywhere as expected.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8755
Differential Revision: https://secure.phabricator.com/D13553
Summary: Ref T8099, Fixes layout of the Pager, all adds distinction to the current pager button.
Test Plan: Edit a Phriction document 100 times. Get a pager.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13549
Summary: Ref T8755. Hunting down some display issues with the fonts on certain computers (Ubuntu, Windows, Chrome Mobile). Hopefully having the full font fallback tree will resolve the issues.
Test Plan: Review Phriction on my Mac, no distinguishable difference. Will test Ubuntu after it lands.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8755
Differential Revision: https://secure.phabricator.com/D13552
Summary: Ref T8099, Better colors and spacing on the Object Selector.
Test Plan: Hover and test new spacing.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13524
Summary: Ref T8099, Ref T8449. Adds a min width to selects (can't think of anything this breaks in ui) and adds space around space select.
Test Plan: Review a number of selects in different UIs, forms, dialogs.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099, T8449
Differential Revision: https://secure.phabricator.com/D13523
Summary: Ref T8099, Missed a few various edge cases here. Cleans up the 'next' UI.
Test Plan: Review a current setup issue, then ignore it.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13529
Summary: Ref T8099, An attempt to consolidate spacing, size, and border styles of various dropdown menus and list elements.
Test Plan: Look at as many lists as I could find, probably worth collapsing some of the duplicates sometime.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13517
Summary: Ref T8099. Better alignment with TODO and Ghost Icons.
Test Plan: Test more states of inline comments, ghosts, todos, unsubmitted, etc.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13515
Summary: Ref T8099. This is a little more resilient to font size changes in the main UI.
Test Plan: Change font size in settings, items grow and align correctly.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13492
Summary: Ref T8099, Remove final border, break words in titles.
Test Plan: See line vanish before my eyes!
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13491
Summary: Ref T8099, Fixes T8338. This allows re-ordering of Maniphest Tasks in the redesign. Somehow seems more fragile, but I couldn't break anything with it.
Test Plan: Try ordering into first position after header, last position, changing priority outright, everything I can drag.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099, T8338
Differential Revision: https://secure.phabricator.com/D13487
Summary: Ref T8099, This feels a little more consistent with the Document.
Test Plan: Review Phriction, Legalpad, Diviner
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13477
Summary: Ref T8099, Simplifies the button bar with a `borderless` option and implements in Differential Inline Commenting.
Test Plan:
Review new and old comments, submitted, unsubmitted, ghosts, done.
{F562765}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13475
Summary: Ref T8099, Cleans up UI issues, adds `appendList` and renders lists and paragraphs with Remarkup UI.
Test Plan: Test Policy Dialogs, other various dialogs.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13463
Summary: Ref T8099, makes the home nav elements more resilient to font size changes, fixes alignment on mobile.
Test Plan: Review home nav with smaller and larger font, check hover states, check phone layouts.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13465
Summary: Ref T8099, test a few themes and fix some inconsistencies.
Test Plan: Lots of changing headers, checking colors.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13466
Summary: Ref T8099, These got bigger in the redesign (font?), and didn't fit in certain spots (Conpherence, Dialogs)
Test Plan: Test new button sizes in Conpherence, Durable Column, Dialogs
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13467
Summary: Fixes T8605, Popup datepicker should respect user preferred week start day
Test Plan: Edit event, open datepicker, calendar weeks should start on Sunday by default, and another day, if specified in User Preferences.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8605
Differential Revision: https://secure.phabricator.com/D13419
Summary: Ref T8099, Ref T8614. Reasonable attempt at a larger font size PostProcessor, will need additional UI cleanup (places where we fix height) but overall very decent.
Test Plan: Use lots of the UI elements with new font sizes.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099, T8614
Differential Revision: https://secure.phabricator.com/D13457
Summary: Ref T8099, lots of little tweaks, mostly to icon nav.
Test Plan: Test side navigation elements on home, workboards, conpherence, and maniphest
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13456
Summary: Ref T8099, Fixes T8678. Breaks overly long words in headers.
Test Plan: Make a long header on mobile.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099, T8678
Differential Revision: https://secure.phabricator.com/D13453
Summary: Ref T8099, Feel like this got reverted somewhere, but list elements should have more space (for lists of objects).
Test Plan: Review a list of objects.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13452
Summary:
We use a non-standard way to invoke the edit dialogue (title, picture, etc) via the crumb. Stop doing that and instead use the widget technology to invoke the dialogue.
This requires making the widget handling code a bit more nuanced as nothing has wanted to pop a dialogue before. I plan to clean this up as I add the action to "Mark as Favorite" to the UI. In particular, I want to stop rendering the un-used DOM and make a workflow-based widget action a property as opposed to something hardcoded. This may be too ambitious depending on how similar these workflows are....
This also updates the ThreadSearchEngine to be a bit more modern. Additionally, go through making some user-facing strings a bit more sensical.
Test Plan: changed settings from conpherence full and durable column successfully.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13449
Summary: Ref T8099, Move these to alpha channels so different header colors pop the color up equally.
Test Plan: Test Documents, Maniphest, Workboards
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13440
Summary:
Ref T8488, T8469, T8485.
This is done in regards to T8488 as far as users are concerned. There's still some classes, and etc. that should be re-named probably. T8469 and T8485 are basically moot now though.
Rather than having "Send Message" exposed, just expose "Create Room". Users get the full form. One change is "title" is now required.
This diff removes the concept of "isRoom" entirely.
Test Plan: Verifed a user with no conpherences had sensible data in both column view and full conpherence view. Created rooms with various policies and things worked well.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: chad, epriestley, Korvin
Maniphest Tasks: T8469, T8485, T8488
Differential Revision: https://secure.phabricator.com/D13351
Summary: Ref T8099, A cleaner, more standard UI for Slowvotes. Because Slowvote is important.
Test Plan: Test a number of poll layouts and embeds.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13445
Summary: Ref T8099, Moving to Roboto Slab for Document Headers. It's a little less serif-y and fits well with Lato. Also took a pass at cleaning up edge cases in Documents, Diviner, Phriction, Legalpad, and Welcome Screen.
Test Plan: Test Phriction, Diviner, Legalpad, Welcome Screen
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13439
Summary: Ref T8099, Instead of Latin subset, this uses full 3000+ Glyph set for rendering.
Test Plan: Test a few various languages in my Phriction documents.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13437
Summary:
Ref T8493. Diffusion is probably the strongest upstream use case we have for Spaces right now, so I want to get us on it to kick the tires a bit.
Small amount of hackiness around the multi-page form thing but it shouldn't create any problems.
Test Plan:
- Created a new repo.
- Edited a repo.
- Tried invalid edits, saw value preserved.
- Viewed edit full detail screen, saw space info.
- Viewed repo detail view, saw space.
- Viewed repo list view, saw space.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8493
Differential Revision: https://secure.phabricator.com/D13414
Summary:
Ref T8637. This does nothing interesting, just has empty scaffolding for a bulk job queue.
Basic idea is that when you do something like a batch edit in Maniphest, we:
- Create a BulkJob with all the details.
- Queue a worker to start the job.
- Send you to a progress bar page for the job.
In the background:
- The "start job" worker creates a ton of Task objects, then queues worker tasks to do the work.
In the foreground:
- Fancy ajax animates the progress bar and it goes wooosh.
In general:
- Big jobs actually work.
- Jobs get logged.
- You can monitor jobs.
- Terrible junk like T8637 should be much harder to write and much easier to catch and diagnose.
Test Plan:
No interesting code/beahavior yet. Clean `storage adjust`.
{F526411}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8637
Differential Revision: https://secure.phabricator.com/D13392
Summary:
Ref T8096. Various tweaks here:
- Sort result lists by importance (even lint -- "errors first" seems better than "alphabetical by file", I think?).
- Do sane stuff with display limits.
- Add a "view all" view.
- Don't show a huge table of passing tests in Differential.
- Link to full results.
Test Plan: See screenshots.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13407
Summary: Fixes T8095. Still needs UI/UX work (see T8096) but this has all the core features now.
Test Plan: Saw Harbormaster lint/unit data as though it was Differential lint-unit data.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13401
Summary: Ref T8095. This weird grey table has no remaining callsites and can be removed.
Test Plan: Grepped for symbols.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13379
Summary: Ref T8099, These should align out to 4px from the edge of the line, not 10px.
Test Plan: Test dashboards, Maniphest, Workboards
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13400
Summary: Ref T8099, This adds a consistent background color to object and policy tags, and highlights them when they deviate from the normal. Still likely worth revamping 'closed' and 'review' state colors.
Test Plan: Review lots of diffs and tasks.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13399
Summary: Ref T5791. This should make performance snappy wrt policy checks in some future diff where the Query is updated and in use somewhere in the application.
Test Plan: ran `./bin/storage upgrade`. commented on a task and saw actorPHID populated correctly in underlying MetaMTAMail object database entry
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13396
Summary:
Ref T5791. This does a few bits there. Namely:
- Adds PHID column to PhabricatorMetaMTAMail
- Implements a PhabricatorMetaMTAMailPHIDType
- Script to backpopulate them.
- Makes PhabricatorMetaMTAMail implement PolicyInterface.
- View policy is NOONE and the author and recipients have automatic view capabilities
- No edit capability.
- Adds a PhabricatorMetaMTAMailQuery for PhabricatorMetaMTAMail.
Test Plan: ran `./bin/storage upgrade` successfully. commented on a maniphest task and verifed the metamta mail object in the database was created successfully with a shiny new phid
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13394
Summary:
Fixes T4139. Adds a "Desktop Notifications" panel to settings. For now, we start with "Send Desktop Notifications Too" functionality. We can try to be fancy later and only send desktop notifications if the web app doesn't have focus, etc.
Test Plan:
Made some comments as a test user on a task and got purdy desktop notifications using Chrome. Then did it again with Firefox.
Played around with permissions form with Chrome and got helpful information about what was up. Played around with Firefox and got similar results, except canceling the dialogue didn't invoke my handler code somehow. Oh Firefox!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: rbalik, tycho.tatitscheff, joshuaspence, epriestley, Korvin
Maniphest Tasks: T4139
Differential Revision: https://secure.phabricator.com/D13219
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:
Fixes T5135. Currently, when you create a credential, we default the policies to your PHID. This means we can't have an application-level configurable default because there's no way to select "the actor's PHID" as a policy.
Start tracking the credential author's PHID and add an object policy for it, so there is such a setting.
Then, add policy defaults.
This mostly unblocks T6787. This obsoletes T6860.
Test Plan:
- Created a credential with "Credential Author" policy.
- Verified I can see/edit it, but other users can not.
- Changed default policies to something else.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5135
Differential Revision: https://secure.phabricator.com/D13385
Summary:
Ref T8095. Two general problems:
- I want Harbormaster to own all lint and unit test results.
- I don't want users to have to configure anything for `arc` to keep working automatically.
These are in conflict because generic lint/unit test ownership in Harbormaster requires that build targets exist which we can attach build results to. However, we can't currently create build targets on demand: Harbormaster assumes it is responsible for creating targets, then running code or making third-party service calls to actually run the builds.
I considered two broad approaches to let `arc` push results into Harbormaster without requiring administrators to configure some kind of "arc results" build plan:
# Add magic target PHIDs like `PHID-MAGIC-this-is-really-arc-unit`.
# Add new code to build real targets with real PHIDs.
(1) is probably a bit less work to get off the ground, but I think it's worse overall and very likely to create more problems in the long run. I particularly worry that it will lead to a small amount of special casing in a very large number of places, which seems more fragile.
(2) is more work upfront but I think does a better job of putting all the special casing in one place that we can, e.g., more reasonably unit test, and letting the rest of the code rarely/never care about this case since it's just dealing with normal plans/steps/targets as far as it can tell.
This diff introduces "autoplans", which are source templates for plans/steps. This let us "push" these targets into Harbormaster. Hypthetically, any process "like" arc can use autoplans to upload test/lint/etc results. In practice, probably only `arc` will ever use this, but I think it's still quite a bit cleaner than the alternative despite all the generality.
Workflow is basically:
- `arc` creates a diff.
- `arc` calls `harbormaster.queryautotargets`, passing the diff PHID and saying "I have some lint and unit results I want to stick on this thing".
- Harbormaster builds the plan, steps, and targets (if any of them don't already exist), and hands back the target PHIDs so `arc` has a completely standard-looking place to put results.
- `arc` uploads the test results to the right targets, as though Harbormaster had asked it to run unit/lint in the first place.
(This doesn't actually do any of that yet, just sets things up.)
I'll maybe doc turn that ^^^^^^ into a doc for posterity since I think it's hard to guess what an "autotarget" is, but I'm going to grab some lunch first.
Test Plan:
- Added unit tests to make sure we can build these things properly.
- Used `harbormaster.queryautotargets` to build autotargets for a bunch of diffs.
- Verified targets come up in "waiting for message" state.
- Verified plans and steps are not editable.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: hach-que, epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13345
Summary:
Ref T8095.
Harbormaster has a `BuildItem` class, but it has no table and is unused. This was an earlier idea about representing lint/unit results and some other possible types of messages, but I think we want to be more specific than this.
Remove `BuildItem` and add `Lint` and `Unit` storage. These tables roughly parallel how we store lint/unit messages today, with some guesses about how where they'll go in the future.
Test Plan: Ran `bin/storage upgrade` and got a clean adjust out of it.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: hach-que, epriestley
Maniphest Tasks: T8095
Differential Revision: https://secure.phabricator.com/D13329
Summary: Ref T8099, Now that we have cleaner headers, we can add more pop to important items, like the state of an Object. This makes it easier to just note the color and generally understand the state of the object (closed, returned, accepted, open). @epriestley, I think you previously thought this was a bug, but if it still feels bad, let me know.
Test Plan:
Review Differential, Maniphest, Polls for various object states.
{F520973}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13369
Summary: Ref T8099, Timeline images should be 28 and 50, I previously had a border adding the additonal size, but changed that to an inset shadow.
Test Plan: Staring. Intensifies.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13368
Summary: Not sure if we want this, but it seems to work fine.
Test Plan: {F516736}
Reviewers: joshuaspence, chad
Reviewed By: joshuaspence, chad
Subscribers: joshuaspence, epriestley
Differential Revision: https://secure.phabricator.com/D13363
Summary: Ref T8099, these grew during the Oswald days, this calms them down a bit.
Test Plan: Review fonts sizes in both descriptions and in documents.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13364
Summary: Ref T8099, uses original bgcolor when dark headers are used.
Test Plan: Switch between light and dark Phabricator, see new colors.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13361
Summary: Ref T8099, Cleans up profile photos around the site in a more consistent way.
Test Plan: grep for 'background-size: 100%'
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13360
Summary:
Minor layout updates to Ref T8099
- Timeline tweaks
- Use Lato headers in Document Obj Headers
- Minor Remarkup
- Add Audit Icons
(Unclear if Audit is "correct", ie Status vs. Commit Status) But added icons anyways if needed.
Test Plan: Review each of the updated elements
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13357
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 T8352. Associate Diviner books and atoms with a repository. This relationship is not really surfaced anywhere in the UI but provides metadata that contextualises search results. Depends on D13091.
Test Plan: Ran `diviner generate --repository ARC` and then went to `/diviner/book/arcanist/`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7703, T8352
Differential Revision: https://secure.phabricator.com/D13070
Summary: Fixes T8565, Quoted text remarkup should be smart enough to know when to add a '>' and when to add '> '
Test Plan: Open an object with remarkup comments, add 'quote', select that text click the quote button in the remarkup menu, text should become '> quote'. Select and click again, text should become '>> quote'.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8565
Differential Revision: https://secure.phabricator.com/D13334
Summary:
Ref T4558. This diff modernizes the #diviner application. Basically:
- Add an edit controller, accessible at `/book/$BOOK/edit/`.
- Add edit/view policies.
- Added an action menu to the `DivinerBookController` to expose the edit interface.
- Allows projects to be associated with books.
- Implement edges and transactions.
- Implemented `PhabricatorApplicationTransactionInterface` in `DivinerLiveBook`.
Test Plan:
- Generated a Diviner book with `./bin/diviner generate`.
- Added projects to a book and ensured that they persisted.
- Changed the view policy on a book and made sure it was effective.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4558
Differential Revision: https://secure.phabricator.com/D13091
Summary: Ref T8099, Gives some additional prominence to a Space (darkred) in headers and obj lists.
Test Plan: Test a sample space. Some concern on workboards if space name is long.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13312
Summary: Closes T8460.
Test Plan: In the Calendar application, create a recursive event. After editing a recursion of the event, verify that the "Recurrence of Event" property is of the form "<event sequence number> of <parent event>".
Reviewers: lpriestley, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T8460
Differential Revision: https://secure.phabricator.com/D13307
Summary:
Ref T8099, these should sit in a 4px gutter.
Also:
- remove bad route on feed
- Fix issue with custom logos
- Fix unit error
Test Plan: Review on dashboards and on Maniphest homepage
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13310
Summary: This should default to light, also made the icons on the dark headers more transparent.
Test Plan: Tested light, dark, blue, blindigo, and default.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13306
Summary:
- Give preference to Segoe on Windows
- Move to Lato headers except documents
- Fix diviner issues
- Fix feed overwriting line-height
- Fix apps launcher
- Fix infoview + listview
- Make tall headers less tall
- Legalpad, Diviner tweaks
Test Plan: Random surfing. @epriestley can you just commit this for me, connection generally shitty.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13305
Summary: Couple of reasons here. Mainly, we're looking at Serif fonts for Phacility, so having headers tie back seems worth exploring. Also Oswals was a bit heavy, and in trying to find a replacement with similiar distinction, was unable to find anything suitable. Trying out Slabo here and have been pleasantly surprised how well the Serif/Sans-Serif combination works here, typographically.
Test Plan:
Test Objects, Documents, and Dashboards.
{F500207}
{F500208}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13279
Summary:
Ref T5681. Policy rules can now select objects they can apply to, so a rule like "task author" only shows up where it makes sense (when defining task policies).
This will let us define rules like "members of thread" in Conpherence, "subscribers", etc., to make custom policies more flexible.
Notes:
- Per D13251, we need to do a little work to get the right options for policies like "Maniphest > Default View Policy". This should allow "task" policies.
- This implements a "task author" policy as a simple example.
- The `willApplyRule()` signature now accepts `$objects` to support bulk-loading things like subscribers.
Test Plan:
- Defined a task to be "visible to: task author", verified author could see it and other users could not.
- `var_dump()`'d willApplyRule() inputs, verified they were correct (exactly the objects which use the rule).
- Set `default view policy` to a task-specific policy.
- Verified that other policies like "Can Use Bulk Editor" don't have these options.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5681
Differential Revision: https://secure.phabricator.com/D13252
Summary: Working towards a more unified look and feel. This brings in Lato as a complete base font over Helvetica Neue, as well as removing Source Sans Pro from DocumentView and Conpherence. Design-wise Lato provides the nice readability at larger font sizes that Source Sans Pro did, with the ability to scale down to tables and UI widgets with ease. This gives us one font instead of two, and now Object descriptions and Timeline posts all can benefit from a consistent, readable font.
Test Plan:
Test main UI, smaller elements like tables, menus, DocumentViews, Previews, Conpherence.
{F498135}
{F498136}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13276
Summary: Ref T8498. This editor is an artifact of the Old World at this point, but it still works fine.
Test Plan: Moved tasks between spaces using the batch editor.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8498
Differential Revision: https://secure.phabricator.com/D13249
Summary:
Ref T8498. Allow ApplicationEmail addresses to be put into spaces:
- You can only see and send to addresses in Spaces you have access to.
- Objects are created into the same space their address is associated with.
Test Plan:
- Used `bin/mail receive-test` to send mail to various `xyz-bugs@...` addresses.
- Saw objects created in the proper space.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8498
Differential Revision: https://secure.phabricator.com/D13247
Summary:
Ref T8498. I want to add Spaces to these, and the logic for getting Spaces right is a bit tricky, so swap these to ApplicationTransactions.
One new piece of tech: made it easier for Editors to raise DuplicateKeyException as a normal ValidationException, so callers don't have to handle this case specially.
One behavioral change: we no longer require these addresses to be at the `auth.email-domains` domains -- I think this wasn't quite right in the general case. It's OK to require users to have `@mycompany.com` addresses but add `@phabricator.mycompany-infrastructure.com` addresses here if you want.
Test Plan:
- Tried to create a duplicate email.
- Tried to create an empty email.
- Tried to create an invalid email.
- Created a new email.
- Deleted an email.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8498
Differential Revision: https://secure.phabricator.com/D13246
Summary: Ref T8498. Allow Herald rules to act on the Space which contains an object.
Test Plan:
- Wrote a "Space is any of..." rule, created tasks that matched and failed the rule.
- Also created a Pholio rule with the "Space..." condition.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8498
Differential Revision: https://secure.phabricator.com/D13242
Summary:
Ref T8377. This adds a standard disable/enable feature to Spaces, with a couple of twists:
- You can't create new stuff in an archived space, and you can't move stuff into an archived space.
- We don't show results from an archived space by default in ApplicationSearch queries. You can still find these objects if you explicitly search for "Spaces: <the archived space>".
So this is a "put it in a box in the attic" sort of operation, but that seems fairly nice/reasonable.
Test Plan:
- Archived and activated spaces.
- Used ApplicationSearch, which omitted archived objects by default but allowed searches for them, specifically, to succeed.
- Tried to create objects into an archived space (this is not allowed).
- Edited objects in an archived space (this is OK).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8377
Differential Revision: https://secure.phabricator.com/D13238
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:
Ref T8493. Add Spaces support to Pholio.
This is straightforward; Pholio has no clone/copy/fork or weird parent/child stuff going on.
Test Plan: Created a mock, put it in a space, looked at it as another user, searched for stuff in spaces, viewed Macros.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8493
Differential Revision: https://secure.phabricator.com/D13231
Summary: Fixes T8489. Regression in D13058. Re-write this so a) works and b) works as well cross browser as possible. (big guess on b)
Test Plan: visited /Z1 vs /Z1?settings and saw people widget vs settings widget as respective defaults.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T8489
Differential Revision: https://secure.phabricator.com/D13228
Summary:
Fixes T8483. I did this incorrectly in D13159, by doing it correctly first and then editing it carelessly. For most transaction types, it didn't matter, but did for inline state.
Also, clean up any bad inline state transactions.
Test Plan:
- Ran migration, bad transactions vanished.
- Marked some inline comments as done.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8483
Differential Revision: https://secure.phabricator.com/D13226
Summary: Fixes T6713. Though I've said that before. =D Looks like this handler wasn't upgraded earlier and was still updating the DOM; removing the DOM updating code and let the central spot handle everything and this works fine.
Test Plan: open up two browsers with durable column on same room. send messages in browser a and observe 1 copy of each message showing up in browser b. send messages in browswer b and observe one copy in browser a. browser a was chrome and browser b was firefox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T6713
Differential Revision: https://secure.phabricator.com/D13214
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 T8455. It looks like for at least some installs, there are a lot of rules which use mailing lists and they aren't easy to just manually go fix.
Migrate conditions and actions of known types which contain mailing list PHIDs from old MLST PHIDs to new USER PHIDs.
Test Plan:
- Created a "Subscribers include..." condition using a mailing list, migrated it forward into a user.
- Created a "add ccs..." action including a mailing list, migrated it forward into a user.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8455
Differential Revision: https://secure.phabricator.com/D13184
Summary:
Ref T8424. This adds crude integration with Paste's edit/view workflows: you can change the space a Paste appears in, see transactions, and get a policy callout.
Lots of rough edges and non-obviousness but it pretty much works.
Test Plan:
- Created and updated Pastes.
- Moved them between spaces, saw policy effects.
- Read transactions.
- Looked at feed.
- Faked query to return no spaces, saw control and other stuff vanish.
- Faked query to return no spaces, created pastes.
- Tried to submit bad values and got errors.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8424
Differential Revision: https://secure.phabricator.com/D13159
Summary:
Ref T8424. I'm using Paste as a testbed application because Spaces make some degree of sense for it but it's also flat/simple.
This doesn't do anything interesting or useful and mostly just making the next (more interesting) diff smaller.
Test Plan:
- Ran `bin/storage upgrade -f`.
- Browsed pastes.
- Created a paste.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8424
Differential Revision: https://secure.phabricator.com/D13154
Summary: Ref T8099, hashtag#yolo. Adds back the original gradients plus a 'light' theme. Unclear which should be default, but we can play with it until a decision needs to be made.
Test Plan: Change colors a lot, turn on durable column.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13146
Summary: Ref T8377. These were picked up by tests.
Test Plan: `arc unit --everything`
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8377
Differential Revision: https://secure.phabricator.com/D13130
Summary:
Ref T8387. This migrates lists in the database to users, and replaces all subscriptions.
This won't update Herald rules or saved search queries, but they're presumably rare and infeasibly complex to migrate.
Test Plan: This migration is relatively re-runnable, so I ran it a bunch of times with different setups using `bin/storage adjust --apply`. It successfully migrated lists into users and replaced them in all the places they were subscribed.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13128
Summary:
Ref T8387. Adds new mailing list users.
This doesn't migrate anything yet. I also need to update the "Email Addresses" panel to let administrators change the list address.
Test Plan:
- Created and edited a mailing list user.
- Viewed profile.
- Viewed People list.
- Searched for lists / nonlists.
- Grepped for all uses of `getIsDisabled()` / `getIsSystemAgent()` and added relevant corresponding behaviors.
- Hit the web/api/ssh session blocks.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: eadler, tycho.tatitscheff, epriestley
Maniphest Tasks: T8387
Differential Revision: https://secure.phabricator.com/D13123
Summary: Ref T8099, I want to live with this for a few weeks. I've grow to like that it gives more distinction to ObjectBox Headers and fits very nicely as headers in DocumentView. Even if we don't use them for all headers, Documents are much nicer to me at least with a more visually impactful font (Source Sans doesn't make a great header).
Test Plan:
Review dashboards, objects, Diviner, Phriction, Legalpad, and misc other pages.
{F466410}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13127
Summary: Ref T8099, Moves AphrontPagerView to PHUIPagerView, converts to standard PHUIButtons and adds some additional features for icon placement on buttons.
Test Plan: Tested Advanced Search and Searching files in Diffusion. Works as expected.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8342, T8099
Differential Revision: https://secure.phabricator.com/D13092
Summary: Ref T8099, Mostly a Feed cleanup, removing old CSS, relying on modern display objects, adds back the feed to profile (I miss it, but maybe you don't).
Test Plan: Visit Feed on Profiles, Projects, Feed, and Dashboards. Same UI Everywhere. TODO, "Public Feed".
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13101
Summary: Fixes T8375. This column is `repositoryID`, not `repository`.
Test Plan: Examined schema.
Reviewers: joshuaspence, btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T8375
Differential Revision: https://secure.phabricator.com/D13097
Summary: Ref T7604. Change two migrations to query arcanist project information using `queryfx` directly to avoid the need for the `LiskDAO` fields to exist.
Test Plan:
Ran the following commands to verify that things weren't majorly broken:
- `./bin/storage upgrade --apply phabricator:20150503.repositorysymbols.2.php`
- `./bin/storage upgrade --no-quickstart --namespace test`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7604
Differential Revision: https://secure.phabricator.com/D13011
Summary: Ref T8099, We moved to `phui-header-header` for targeting the internal title in the header vs. the entire header-view. There were some pieces that didn't get converted over. Also resolved some dashboard feed issues.
Test Plan: Test lots of dashboards.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13086
Summary: Ref T8099, This centers, but floats, all pinboards items based on screen width.
Test Plan:
Test desktop, mobile break points. Items scale nicely.
{F459680}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13085
Summary: Ref T8099, adds a little excitement to searching for things... with fire!
Test Plan: Search for items, easier to locate searched copy.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13082
Summary: Ref T8099, Adds a bit more color/contract to Paste.
Test Plan: Review a list of Pastes, embedded Pastes, and the main Paste UI.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13083
Summary: Fixes T8354. Adds a class to each `th` and `td` for specificity.
Test Plan: Write a large table as an inline comment, see it wraps property on smaller screens.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8354
Differential Revision: https://secure.phabricator.com/D13076
Summary: Ref T8099, Adds ability to set header as "tall" and provide better control over height of actual header text. Also fixed some color issues with multiple object boxes.
Test Plan: Review a commit, review dashboards, review a diff, review home.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13077
Summary: Ref T8099. Makes subheaders in object lists darker with a background. Moves to lighter icons in the header with a dark instead of blue hover state. Gives a nicer blue selected state for sidenavs.
Test Plan: Just eyeballing a lot of this, but with some level of consistency.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13065
Summary: Ref T8099, No specific reason for these to be small buttons.
Test Plan: Test desktop and mobile layouts.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13081
Summary: Ref T8099, Adds a border to ConpherenceView, but not other Views. Removed some old styles.
Test Plan: Visit Conpherence Room, Thread, and Search.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13062
Summary:
Refs T8302.
V1 of the implementation. This replaces the previous mode, but I guess there's no real reason we can't have
some symbols always clickable and the rest require modifier.
I'm also a little concerned about discoverability; Holding down ctrl/cmd will make the cursor change, so there's
some hint that something might be up, but that's probably not obvious enough.
Test Plan:
Tested in diffusion and differential and differential comments on:
- Windows/Chrome,
- Windows/IE 11
- LInux/Firefox 38
- Mac/Chrome
- Mac/Safari
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, joshuaspence
Maniphest Tasks: T8302
Differential Revision: https://secure.phabricator.com/D13034
Summary: Fixes T8329. I was able to figure out a reasonable way to have the full conpherence default to the email settings panel. I think this is cleaner than making things a dialogue as I rambled about in the description for T8329.
Test Plan: using /bin/mail to verify correct email links were generated for conpherence notifications and maniphest (general) notifications.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8329
Differential Revision: https://secure.phabricator.com/D13058
Summary: Fixes T7696.
Test Plan: hit the quote button with no text - it worked. highlighted some text and hit the quote button - it worked. hit the list item button with no text - it worked. hit the list item button with text selected - it worked
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7696
Differential Revision: https://secure.phabricator.com/D13060
Summary: Ref T8099, minor spacing improvements and more icons for feed.
Test Plan:
Test feed in Feed, in a Dashboard, Profile, and Project
{F443366}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13045
Summary: Ref T8099. Not sure if this was accidentally removed, but `overflow: hidden` saves the day once again. Fixes an issue where different pages had navs starting at different spots based on crumbs or object boxes.
Test Plan: Test Projects, People, Dashboards, lots of other navs. All start at 8px now.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13047
Summary: Ref T8099. I use this app daily, so 30 minutes of love is good enough for me.
Test Plan:
Tested with a fake chat.
{F443522}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13046
Summary: Ref T8099. We removed the background column, so this JS wasn't able to fire and make the filetree movable.
Test Plan: Set Filetree on, resize it. Scroll through a diff.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13048
Summary: Fixes T8328. Somewhere along the line we stopped posting to the server with no text. Make sure if the action is join_room that we ping the server even if no text is specified.
Test Plan: tried to send an empty message and failed; nothing happened when I clicked. tried to join a room with an empty message and it worked
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8328
Differential Revision: https://secure.phabricator.com/D13041
Summary: Ref T8099. Adds back basic header color options, which change the logo color instead. Also RAINBOW.
Test Plan:
tested each of the new colors.
{F442284}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8099
Differential Revision: https://secure.phabricator.com/D13040
Summary:
Ref T8320. Fixes T8317. Fixes T2831. Fixes T8073. Fixes T7127.
There was a bug with this line:
for ($ii = 0; $ii < count($paths); $ii++) {
...because the array may be sparse if there have been deletes, so `count($paths)` might be 3, but the real keys could be `1`, `5` and `6`. I think this was the primary issue behind T7127.
The old Editor did a lot of work to try to validate paths. When a path failed to validate, it silently discarded it. This was silly and pointless: it's incredibly bad UX; and it's totally fine if users saves "invalid" paths. This was likely the cause of T8317, and probably the cause of T8073.
T2831 I'm less sure about, but I can't reproduce it and I rewrote all the logic so I suspect it's gone.
This also records and shows edits, so if stuff does keep happening it should be more clear what's going on.
I removed some adjacent stuff:
- I removed the ability to delete packages. I'll add "disable" in a future diff, plus `bin/remove destroy`, like other objects. Getting rid of this now let me get rid of all the mail stuff.
- I removed "path validation" where packages would try to automatically update in response to commits. This doesn't necessarily make sense in Git/Mercurial, is sketchy, could easily have been the source of T2831, and seems generally complicated and not very valuable. We could maybe restore it some day, but I'd like to get Owners stable before trying to do crazy stuff like that.
Test Plan: {F437687}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8317, T8073, T7127, T2831, T8320
Differential Revision: https://secure.phabricator.com/D13032
