Summary: Fixes T3044. We currently don't add these to the index.
Test Plan: Made a unique inline comment on a commit, then searched for it.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T3044
Differential Revision: https://secure.phabricator.com/D9170
Summary:
Fixes T3854. Subversion allows commits with no message, and in other cases we might not have imported the message yet. In these cases, we may not render any text inside the link.
When we hit these cases, render appropriate replacement text.
Test Plan: {F156229}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T3854
Differential Revision: https://secure.phabricator.com/D9169
Summary: Fixes T5093. Ref T4830.
Test Plan:
- As a logged out user, viewed a public countdown detail page.
- (Tried to view a nonpublic one, got asked to login.)
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4830, T5093
Differential Revision: https://secure.phabricator.com/D9162
Summary: Changes to using FontAwesome
Test Plan:
Testing UIExamples and each of the pages (except releelph)
{F155942}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9157
Summary: This is useful when you're trying to onboard an entire office and you end up using the Google OAuth anyway.
Test Plan: tested locally. Maybe I should write some tests?
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9150
Summary: This doesn't have any impact on anything right now (we don't persist the query) but could in the future, so I just left it as-is but fixed the typo.
Test Plan: looked at it carefully
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: WikiChad, epriestley
Differential Revision: https://secure.phabricator.com/D9155
Summary:
I have added a dialog box which pops up when a user starts or stops tracking time on an issue with Phrequent. These dialogs allow the user to modify the time if it so happens that they forgot to either clock in or out.
I have also added a Note field in the dialog when a user stops tracking time. This allows them to enter a note about the time, and is entered into the database, but is currently (as far as I know) not visible anywhere in Phabricator.
I have made these changes according to the suggestions found in T3568
Also, upon clocking in or out, if the time entered is a future time, an error is returned and the user is asked to enter a valid time.
Test Plan:
Start tracking time and edit the start date/time, then end the time and edit that timestamp as well.
Also, try entering future dates/times and ensure that the dialog reports an error and asks for the time again.
Ensure that these edited times are recorded properly.
Reviewers: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T3568
Differential Revision: https://secure.phabricator.com/D9147
Summary:
Fixes T5081. This block enforces policies and prevents you from seeing groups associated with projects you can't see. However, it incorrectly removes tasks which end up with no group key. This can happen in two cases:
- The task isn't in a project.
- The task is in a project, but the query includes an "In All Projects: <that project>" constraint. In that case, we don't show the group becuase it would always contain every task.
Test Plan:
Replicated the setup in T5081, saw an "Ungrouped" group with "Task A":
{F155766}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5081
Differential Revision: https://secure.phabricator.com/D9152
Summary: we should do a redirect if its not an ajax request, which solves this problem since folks with no javascript aren't sending ajax requests. Fixes T5049.
Test Plan: inverted the predicate, tried ajax requests, and got redirected
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5049
Differential Revision: https://secure.phabricator.com/D9149
Summary:
Ref T4986. This mostly just makes tab panels a little nicer.
Maybe this will be modal (header = "none", "edit", "view") in a few diffs but we can clean it up then if so.
Test Plan: {F155491}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9143
Summary:
Ref T4986. Ref T4983. Panels will soon be able to contain other panels, either via Remarkup (`{W1}`) or maybe through new types of meta-panels.
Allow panels to detect that they are being rendered very deeply and/or within themselves.
Test Plan: Faked some errors, got failed panel renders. Since panels can't //really// contain other panels yet, this doesn't really have an impact.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T4983, T4986
Differential Revision: https://secure.phabricator.com/D9140
Summary:
Ref T4986. One note:
- I've disabled syntax highlighting in the previews. When we miss caches this is just way way too slow and has frustrated me several times in the past. The value of syntax highlighting these snippets is not huge. We could maybe ajax this in or use it //if// we get a cache hit in the future, but just kill it for the moment.
Test Plan: Viewed pastes. Created a paste panel.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9138
Summary:
Ref T4986. One note:
- We have a separate "browse directory" capability, to provide some soft privacy for users of public installs. Respect that policy within the SearchEngine.
- Also restore some other icons I missed earlier.
Test Plan:
- Viewed people list.
- Build people panel.
- Verified people panel was just me without browse capability.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9137
Summary:
Ref T4986. Swap this in. Two minor notes:
- I adjusted the SearchEngine to add an additional constraint when the viewer isn't an admin. This mostly stops us from doing a bunch of unnecessary work.
- I fixed the settings panel to paginate (currently loads all results, slow in production).
Test Plan: Viewed logs; viewed settings panel; created a dashboard panel.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9136
Summary:
This gets us the ability to specify a "layout mode" and which column a panel should appear in at panel add time. Changing the layout mode from a multi column view to a single column view or vice versa will reset all panels to the left most column.
You can also drag and drop where columns appear via the "arrange" mode.
We also have a new dashboard create flow. Create dashboard -> arrange mode. (As opposed to view mode.) This could all possibly use massaging.
Fixes T4996.
Test Plan:
made a dashboard with panels in multiple columns. verified correct widths for various layout modes
re-arranged collumns like whoa.
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4996
Differential Revision: https://secure.phabricator.com/D9031
Summary: Fixes T5062. See inlines.
Test Plan: Did not test whatsoever.
Reviewers: hach-que
Reviewed By: hach-que
Subscribers: epriestley
Maniphest Tasks: T5062
Differential Revision: https://secure.phabricator.com/D9132
Summary:
Create transaction, editor, etc, and move command generation over to editor.
Show in a timeline in the buildable page.
Also prevent Engine from creating an empty transaction when build starts (Fixes T4885).
Fixes T4886.
Test Plan: Restart builds and buildables, look at timeline.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4885, T4886
Differential Revision: https://secure.phabricator.com/D9110
Summary: Ref T5058. The use of "enum" is confusing; we mean "choose one of these specific string constants". Make this more clear.
Test Plan: Viewed each call from the web UI.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5058
Differential Revision: https://secure.phabricator.com/D9127
Summary: Fixes T5050. This might not be 100% right in all edge cases, but it worked on everything I tried.
Test Plan:
- Pushed a branch deletion.
- Pushed a branch creation.
- Pushed a brnach creation + deletion.
- Pushed a brnach deletion + creation.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5050
Differential Revision: https://secure.phabricator.com/D9122
Summary: Ref T5050. This fixes the immediate error (bad pht()) but doesn't fix the other error (can't `--close-branch`) yet.
Test Plan: Pushed a `--close-branch` commit, got a first-level error instead of an error about an error.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5050
Differential Revision: https://secure.phabricator.com/D9119
Summary: 71 new icons! Pied Piper!
Test Plan: tested new icons on UIExample. Perused a few other pages in Maniphest, Differential. No issues noted.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9125
Summary: yay public policy. Ref T4830.
Test Plan: browsed about calendar from a logged out browser
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4830
Differential Revision: https://secure.phabricator.com/D9120
Summary:
Administrators may encounter this UI for the first time when configuring a repository, without ever having used Passphrase before. If they end up here on a workflow like this, the meaning of "Lock Permanently" may not be totally clear. Add some explanatory text.
(See also T4721, which discusses Passphrase documentation more generally.)
Test Plan: {F154209}
Reviewers: lpriestley
Reviewed By: lpriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9102
Summary:
Allows you to quickly search for files within a repository. Roughly:
- We build a big tree of everything and ship it to the client.
- The client implements a bunch of Sublime-ish magic to find paths.
Test Plan: {F154007}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley, zeeg
Differential Revision: https://secure.phabricator.com/D9087
Summary: Ref T4986. Move push logs to a View, then have all the stuff that needs to use it use that View.
Test Plan: Viewed push logs and transaction detail in Diffusion. Created a panel.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9104
Summary: Ref T2683. This field is //almost// entirely redundant with `symbolicCommit`. Improve how some of the diff query stuff works a bit, then remove it.
Test Plan: Browsed around in all interfaces, looked at a bunch of diffs, etc.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9099
Summary:
Ref T2683. Currently, DiffusionRequest has four different "commitey" things:
- `commit`
- `rawCommit`
- `symbolicCommit`
- `stableCommit`
Of these, only two are actually distinct, useful values: `symbolicCommit` (which holds the value the request originally contained, if one existed) and `stableCommit` (which resolves that value, or the value implied by its omission, into a stable, permanent commit identifier).
- `rawCommit` is equivalent to `symbolicCommit` and can be simply removed.
- `commit` has some sketchy magic around it that needs to be pulled out before it can be jettisoned.
Test Plan: Viewed SVN, Git, and Mercurial repositories. Viewed brwose/history/change/tag/branch/etc views.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9098
Summary: Ref T2683. Throw a more tailored exception to allow callers to distinguish between bad refs (which are expected, if users try to visit garbage branches) and other types of errors.
Test Plan: Tried to view branch "alksndfklansdf". Viewed branch "master".
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9094
Summary:
Ref T2683. This should probably just be `diffusion.filecontentquery` but keep things as they are for now.
This method uses a commit, so accept one. Soon, this will save a bit of work.
Test Plan: Viewed readmes in main and browse views.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9093
Summary:
Ref T2683. The old name was a bit confusing because it meant "the type of the thing the symbol represents": a "commit type" should logically always be "commit".
(Currently, this is only used to detect when we're looking at a tag.)
Test Plan: Looked at a tag. Looked at some other non-tag things. Browsed around, `grep`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9092
Summary:
Ref T2683. This is closely related to "symbolicCommit", but has an inconsistent "name" on the end.
Also, `diffusion.searchquery` uses this parameter inconsistently.
Test Plan:
- `grep`ed for callsites.
- Ran searches in Git and Mercurial repositories.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9091
Summary:
Ref T2683. This has no callsites, and the functionality is covered by the `initFromConduit` flag.
This simplifies the code and reduces then number of internal `diffusion.resolverefs` calls we make on, e.g., the Git repository page from 7 to 2.
Test Plan: Grepped for these symbols.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9090
Summary: Don't get me wrong, "Debate Paste Accuracy" is witty, but we can aspire to better.
Test Plan: Consume glue, obviously.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, chrisbolt, Korvin
Differential Revision: https://secure.phabricator.com/D9106
Summary: Ref T4986. Allows the Drydock search engines to render as panels.
Test Plan: Viewed affected interfaces in Drydock. Created panels from each engine.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9103
Summary: Add a prebuilt filter to show all of the viewer's commits across all repositories. I could go either way on this, but it seems maybe-useful (?), and we have similar prebuilt filters elsewhere.
Test Plan: scoped it out <.< >.>
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8881
Summary: Did a more exhaustive grep on setIcon and found 99.9% of the icons.
Test Plan: I verified icon names on UIExamples, but unable to test some of the more complex flows visually. Mostly a read and replace.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9088
Summary:
Currently, Diffusion has very complex views. After three years I'm not really used to them and rarely use many of these options.
Simplify the browse and history views:
- Put the browse view on top.
- Move dates to the right.
- Remove "History" and "Edit" links from the browse view. You can access these actions by clicking the file/path.
- Remove "Browse" link from the history view. You can access this action by clicking the commit.
- Remove "Change Type", which is essentially never useful, from the history view.
- Add some tweaks for mobile.
Test Plan: {F153931}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley, zeeg
Differential Revision: https://secure.phabricator.com/D9085
Summary: Remove white app icons, no longer in use as far as grep/memory serve. These were for list hover states.
Test Plan: Rebuild sprites, celerity. Grep for appIcon use (only feed). Verify all action lists are driven by FontAwesome.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9078
Summary: Most requests examine the same buckets, especially the first bucket. Let them just read it out of request cache.
Test Plan: Observed most bucket fetches resolving in <10us instead of <10ms.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9080
Summary: Ref T2683. This is a small optimization, but it has low complexity: don't rebuild a bucket more than once in the same request, since it will almost always be the same. Bucket rebuilds are pretty cheap, but this saves a few queries.
Test Plan:
- After discovering (but before parsing) a commit, viewed its browse view. Verified that this patch causes us to perform only one bucket rebuild, and therefore reduces the number of queries we issue.
- Parsed the commit and viewed the browse view again, got successful rebuild and then fills from cache.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9055
Summary: Ref T2683. Normally not a big deal, but if a readme has some codeblocks missing the cache can slow things down.
Test Plan:
- Verified we hit the cache.
- Verified TOC still works.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5028, T2683
Differential Revision: https://secure.phabricator.com/D9049
Summary: Ref T2683. By resolving the stable name earlier, we can save a resolve when viewing branch heads. This is ~100ms in Mercurial, and roughly 25% of page weight. It's less bad in Git.
Test Plan: Saw page cost go down in "Services" tab, particularly for Mercurial browse views.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9048
Summary:
Ref T2683. At least locally, browse views are now nearly instantaneous, even in Mercurial. We also fall back to what we were doing before if we miss or take too long, so this shouldn't make things very much worse even in extreme cases.
For a local `hg` repo, the time we spend pulling browse stuff has dropped from ~3,000ms to ~20ms. This is probably atypical, but not completely crazy or rigged or anything.
Test Plan: Viewed Git, Subversion and Mercurial repositories and observed dramatically better performance in Git and Mercurial as they took advantage of the cache.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, jhurwitz
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9047
Summary:
Ref T2683. Further reduces query count of last modified loads; we're now at 11 instead of 200+.
(This works in SVN but could be further optimized.)
Test Plan:
Loaded SVN, Mercurial, Git:
{F34864}
{F34865}
{F34866}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, vrana, aran
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D5256
Summary:
This code is currently quite complicated because we pull history data inline for SVN files, and via ajax for everything else (SVN dirs, everything in Git and Hg).
Always pull over ajax; batch some of the queries.
Test Plan: {F34860}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, vrana, aran
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D5255
Summary: Ref T2683. Instead of sending one request for each path's history, send one request for all of it. This permits optimizations which are not currently available to us. It degrades the user experience a tiny bit in theory, but on my machine it's actually way faster already.
Test Plan: Loaded a browse page.
Reviewers: vrana, btrahan
Reviewed By: btrahan
Subscribers: epriestley, aran
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D5254
Summary: Ref T2683. This query is currently unbatched and happens inside a view. Leave it inside the view for now, but separate it and make it batchable.
Test Plan: {F34848}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, vrana, aran
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D5252
Summary:
Ref T2683. This is a refinement and simplification of D5257. In particular:
- D5257 only cached the commit chain, not path changes. This meant that we had to go issue an awkward query (which was slow on Facebook's install) periodically while reading the cache. This was reasonable locally but killed performance at FB scale. Instead, we can include path information in the cache. It is very rare that this is large except in Subversion, and we do not need to use this cache in Subversion. In other VCSes, the scale of this data is quite small (a handful of bytes per commit on average).
- D5257 required a large, slow offline computation step. This relies on D9044 to populate parent data so we can build the cache online at will, and let it expire with normal LRU/LFU/whatever semantics. We need this parent data for other reasons anyway.
- D5257 separated graph chunks per-repository. This change assumes we'll be able to pull stuff from APC most of the time and that the cost of switching chunks is not very large, so we can just build one chunk cache across all repositories. This allows the cache to be simpler.
- D5257 needed an offline cache, and used a unique cache structure. Since this one can be built online it can mostly use normal cache code.
- This also supports online appends to the cache.
- Finally, this has a timeout to guarantee a ceiling on the worst case: the worst case is something like a query for a file that has never existed, in a repository which receives exactly 1 commit every time other repositories receive 4095 commits, on a cold cache. If we hit cases like this we can bail after warming the cache up a bit and fall back to asking the VCS for an answer.
This cache isn't perfect, but I believe it will give us substantial gains in the average case. It can often satisfy "average-looking" queries in 4-8ms, and pathological-ish queries in 20ms on my machine; `hg` usually can't even start up in less than 100ms. The major thing that's attractive about this approach is that it does not require anything external or complicated, and will "just work", even producing reasonble improvements for users without APC.
In followups, I'll modify queries to use this cache and see if it holds up in more realistic workloads.
Test Plan:
- Used `bin/repository cache` to examine the behavior of this cache.
- Did some profiling/testing from the web UI using `debug.php`.
- This //appears// to provide a reasonable fast way to issue this query very quickly in the average case, without the various issues that plagued D5257.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, jhurwitz
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D9045
Summary:
Ref T4455. This adds a `repository_parents` table which stores `<childCommitID, parentCommitID>` relationships.
For new commits, it is populated when commits are discovered.
For older commits, there's a `bin/repository parents` script to rebuild the data.
Right now, there's no UI suggestion that you should run the script. I haven't come up with a super clean way to do this, and this table will only improve performance for now, so it's not important that we get everyone to run the script right away. I'm just leaving it for the moment, and we can figure out how to tell admins to run it later.
The ultimate goal is to solve T2683, but solving T4455 gets us some stuff anyway (for example, we can serve `diffusion.commitparentsquery` faster out of this cache).
Test Plan:
- Used `bin/repository discover` to discover new commits in Git, SVN and Mercurial repositories.
- Used `bin/repository parents` to rebuild Git and Mercurial repositories (SVN repos just exit with a message).
- Verified that the table appears to be sensible.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: jhurwitz, epriestley
Maniphest Tasks: T4455
Differential Revision: https://secure.phabricator.com/D9044
Summary: The removes the sprite sheet 'icons' and replaces it with FontAwesome fonts.
Test Plan:
- Grep for SPRITE_ICONS and replace
- Grep for sprite-icons and replace
- Grep for PhabricatorActionList and choose all new icons
- Grep for Crumbs and fix icons
- Test/Replace PHUIList Icon support
- Test/Replace ObjectList Icon support (foot, epoch, etc)
- Browse as many pages as I could get to
- Remove sprite-icons and move remarkup to own sheet
- Review this diff in Differential
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9052
Summary: Fixes T4728, first pass, Make real name optional on user accounts
Test Plan: Default real name config should be false (not required). Create new user, real name should not be required. Toggle config, real name should be required. Users with no real name should be always listed by their usernames.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4728
Differential Revision: https://secure.phabricator.com/D9027
Summary: Fixes T5025. We label this action in two different ways ("Clear All Notifications", "Mark All Read"). The latter is probably a better description.
Test Plan: looked at the UI.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5025
Differential Revision: https://secure.phabricator.com/D9073
Summary: `''` is not a valid integer.
Test Plan: Used `bin/accountadmin` to turn bot flag on and off for a user.
Reviewers: btrahan, Firehed
Reviewed By: Firehed
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9046
Summary: This doesn't look like it is spelled correctly.
Test Plan: Seems more correct now.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9043
Summary: See <http://fab.wmflabs.org/T88>. While this issue is on MW's side, these links are kind of ugly and have more readable alternate forms now. Update them to use proper modern forms.
Test Plan: See inlines.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9042
Summary:
See <https://github.com/facebook/phabricator/issues/596>
Broadly, Facebook would like to bring Pull Requests from GitHub into Phabricator.
In the long term we can do this properly via Doorkeeper/Nuance, but that's probably a ways off. This seems like a reasonable low-budget compromise for now.
I'm a little hesitant to add a ton of parameters to this call, but `repositoryPHID` seems pretty reasonable, and is notable because it also controls default policies.
Test Plan:
- Created a diff with no repositoryPHID.
- Created a diff with a repositoryPHID.
- Verified it carried over when the diff was used to create a revision.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: ptarjan, jamesgpearce, epriestley
Differential Revision: https://secure.phabricator.com/D9023
Summary: Some profiling using XHProf in the Dark Console showed me that Diffusion was wasting a ton of time on array_merge. This change sped up the loading of a large file in Diffusion from 16.8 seconds to 2.4 seconds.
Test Plan: Load files in Diffusion. They all look good. Also, use a PHP shell to try to manually verify that I still kinda remember some PHP and, yes, this is functionally equivalent to what was there before.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9032
Summary:
Provides a working SMS implementation with support for Twilio.
This version doesn't really retry if we get any gruff at all. Future versions should retry.
Test Plan: used bin/sms to send messages and look at them.
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: aurelijus, epriestley, Korvin
Maniphest Tasks: T920
Differential Revision: https://secure.phabricator.com/D8930
Summary: Ref T4986. I think this is the last of the easy ones, there are about 10 not-quite-so-trivial ones left.
Test Plan:
- Viewed app results.
- Created panels.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9025
Summary: Ref T4986. Getting closer. Nothing out of the ordinary in this group.
Test Plan:
For each application:
- Viewed the normal search results.
- Created a panel version and viewed it.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9024
Summary: Ref T4986. These are mostly mechanical now, I skipped a couple of slightly tricky ones. Still a bunch to go.
Test Plan:
For each engine:
- Viewed the application;
- created a panel to issue the query.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9017
Summary:
Ref T4986. This one needs `getApplicationURI()` so make it a little beefier to deal with that.
(It would be vaguely nice to somehow share the handle and application stuff between Controllers and Engine classes like this, but I don't immediately see a clean way to do it without traits. Not a big deal, in any case.)
Test Plan:
- Viewed Calendar.
- Made a Calendar panel.
- Viewed feed.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9012
Summary:
Ref T4986. Updates audit.
Slightly tweaks on method visibility.
Just used a HandleQuery since we have to rebuild the whole view thing otherwise; this is an unusual case.
Test Plan:
- Checked Audit.
- Checked Feed.
- Checked Slowvote.
{F151555}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9011
Summary:
Ref T4986. This adds a bit of structure for handles, since we used to have Controller utilities but no longer do.
Hopefully these will start going faster soon...
Test Plan:
- Checked feed for collateral damage.
- Checked slowvote for collateral damage.
- Made a slowvote panel.
{F151550}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9010
Summary:
Ref T4986. We need to introduce alternate views to make this more pleasant, but let rendering move to engines so it can be shared between panels and controllers.
I also moved some of the pagination logic in to avoid duplicating that.
So far, only Feed works. I'm going to do these gradually since we have ~40-50 of them.
Test Plan:
- Used global search to check for collateral damage.
- Used not-global search too.
- Used normal feed.
{F151541}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9008
Summary: Ref T4986. This isn't pretty/usable yet (I need to move rendering out of ListController classes and into SearchEngine classes, I think) but does pull the correct results.
Test Plan: {F151537}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9007
Summary:
This plugin provides an OAuth authentication provider to authenticate users using WordPress.com Connect.
This diff corresponds to github pull request https://github.com/facebook/phabricator/pull/593/ and had its libphutil counterpart reviewed in D9004.
Test Plan: Configured WordPress.com as an authentication provider, saw it show up on the login screen, registered a new account, got expected defaults for my username/name/email/profile picture.
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9019
Summary:
**WIP**
This sets a shadow fixed to the full screen of the browser for which panels to live in. I have some minor things to fix, but play with it.
Specifically I haven't been able to dig up when stuff that overflowwed, doesn't always render even after scroll.
Test Plan: Test on a board with many tasks, many panels, small screens. Test on mobile too!
Reviewers: epriestley, btrahan
Reviewed By: btrahan
Subscribers: qgil, epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9005
Summary: Fixes T4995. This uses commas so it shouldn't have `[]`. I think I just derped this a while ago.
Test Plan: fiddled
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4995
Differential Revision: https://secure.phabricator.com/D9009
Summary: Fixes T4914. We currently have a finite limit on column displays which caused T4914. This fixes T4914 by no longer using a fluid layout. Rather, we use a fixed column width layout which does not have a 7 column limit. Future work - see T4054 for an example - will likely make the fluid layout thing work with infinite columns, and / or other work may re-jigger project workboards directly.
Test Plan: had a project like in T4914 that wouldn't load and it loaded post this change! added more columns and using javascript inspector noted proper width being set
Reviewers: epriestley, chad
Reviewed By: epriestley
Subscribers: joshuaspence, epriestley, Korvin
Maniphest Tasks: T4054, T4914
Differential Revision: https://secure.phabricator.com/D8942
Summary: There was a typo.
Test Plan: Now there is no typo.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: aran, epriestley
Differential Revision: https://secure.phabricator.com/D8995
Summary: D1239 got it mostly right, but some versions of Outlook apparently put a '> ' in front of the 'Original Message' marker, which the parser couln't grok.
Test Plan: Added a test case to the unit tests, applied the patch to my install and asked one of my heathen Outlook using colleagues to reply to a Conpherence post.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D8998
Summary: Fixes T4970.
Test Plan: left a comment on a paste
Reviewers: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4970
Differential Revision: https://secure.phabricator.com/D8996
Summary:
Ref T4119.
- Trim newlines off the quoted text before quoting it; otherwise we can end up with a staircase of ">" at the end of a quote.
- Allow image macros to have leading whitespace, so multiple consecuitive quoted macros work properly.
Test Plan: I QUOTED MACROS A LOT OF TIMES
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4119
Differential Revision: https://secure.phabricator.com/D8983
Summary:
man I sure hate Javascript
I removed the ajax-edit and ajax-remove interactions, becuase they were prohibitively complex to get working given that the entire menu has to change too. Instead, the page just reloads. This works perfectly fine in practice.
If we want to restore these in the future, we should have the server re-render the entire transaction group or something. I think very little is lost here, though.
Test Plan:
- Took all the actions.
- Used existing dropdown menus.
{F150196}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8966
Summary:
See D8966, D8973. Replace PhabricatorDropdownMenu with PHUIXDropdownMenu.
These new menus look weird on mobile because all action lists pick up a bunch of weird styles on mobile and we're now reusing the CSS.
Test Plan:
{F150425}
{F150426}
Reviewers: btrahan, chad
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8974
Summary:
Ref T4119. This is ugly for now, but technically works.
The comment area and transaction log don't realy know about each other, so for the moment the linking is a bit manual. Differential/Maniphest are special cases anyway.
Test Plan: {F149992}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4119
Differential Revision: https://secure.phabricator.com/D8957
Summary:
Fixes T4909. Adds a "remove" link next to the edit link, which permanently hides a comment. Addresses two use cases:
- Allowing administrators to clean up spam.
- Allowing users to try to put the genie back in the bottle if they post passwords or sensitive links, etc.
The user who removed the comment is named in the removal text to enforce some level of administrative accountability.
No data is deleted, but there's currently no method to restore these comments. We'll see if we need one.
This is cheating a little bit by storing "removed" as "2" in the isDeleted field. This doesn't seem tooooo bad for now.
Test Plan:
- Removed some of my comments.
- As an administrator, removed other users' comments.
- Failed to view history of a removed comment.
- Failed to edit a removed comment.
- Failed to remove a removed comment.
- Verified feed doesn't show the old comment after comment removal.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: qgil, chad, epriestley
Maniphest Tasks: T4909
Differential Revision: https://secure.phabricator.com/D8945
Summary:
Fixes T4960. Users `chmod +x` this, and then bash chokes on it.
Phabricator "owns" this file anyway, so there is no real ambiguity here: this should never be a hook script.
Test Plan:
- Did `chmod +x README`.
- Made a commit.
- Added `z.sh`, got blocked.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4960
Differential Revision: https://secure.phabricator.com/D8981
Summary: A user in IRC hit this; this looks like a typo.
Test Plan: Created a new blog with a public view policy, a non-public join policy, and a custom domain.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: legien, epriestley
Differential Revision: https://secure.phabricator.com/D8980
Summary:
Ref T4843.
- The token award buttons are just icons.
- The tokens themselves are just icons.
- Also spread the tokens out a tiny bit, they feel a little tight to me right now.
Test Plan: Used VoiceOver to read out tokens and token actions. Looked at an object with several token awards.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4843
Differential Revision: https://secure.phabricator.com/D8982
Summary:
Ref T4814. Although this approach made sense at one point, we have more file infrastructure now and T4814 will be easier if we just pass a PHID in.
Also swap Conduit over to use the Editor.
Test Plan:
- Created a paste.
- Created a paste via Conduit.
- Verified that files had correct permissions and appropriate object links in Files.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4814
Differential Revision: https://secure.phabricator.com/D8969
Summary:
Fixes T4941. If a diff has had trailing whitespace stripped, we will fail to handle empty lines correctly (previously, these lines had a leading space when the original tool emitted them).
(This probably stopped working around the time we began retaining newlines.)
Test Plan: The diff in T4941 now parses and renders correctly.
Reviewers: asherkin, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4941
Differential Revision: https://secure.phabricator.com/D8968
