Summary:
Ref T1806. Ref T7173. Depends on D14047.
Currently, all exception handling is in this big messy clump in `AphrontDefaultApplicationConfiguration`.
Split it out into modular classes. This will let a future change add new classes in the Phacility cluster which intercept particular exceptions we care about and replaces the default, generic responses with more useful, tailored responses.
Test Plan:
{F777391}
- Hit a Conduit error (made a method throw).
- Hit an Ajax error (made comment preview throw).
- Hit a high security error (tried to edit TOTP).
- Hit a rate limiting error (added a bunch of email addresses).
- Hit a policy error (tried to look at something with no permission).
- Hit an arbitrary exception (made a randomc ontroller throw).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T1806, T7173
Differential Revision: https://secure.phabricator.com/D14049
Summary:
Ref T1806. Ref T7173. Context here is that I want to fix "you can not log in to this instance" being a confusing mess with an opaque error. To do this without hacks, I want to:
- clean up some exception handling behavior (this diff);
- modularize exception handling (next diff);
- replace confusing, over-general exceptions with tailored ones in the Phacility cluster, using the new modular stuff.
This cleans up an awkward "AphrontUsageException" which does some weird stuff right now. In particular, it is extensible and extended in one place in Diffusion, but that extension is meaningless.
Realign this as "AphrontMalformedRequestException", which is a better description of what it is and does: raises errors before we can get as far as normal routing and site handling.
Test Plan: Hit some of these exceptions, saw the expected "abandon all hope" error page.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T1806, T7173
Differential Revision: https://secure.phabricator.com/D14047
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 T9080. We try to list alternatives for the current ref (for example, if you're viewing a branch named "master" but there's also a tag named "master", or, in Mercurial, there are several branches named "master") but fail to abruptly if we can't get the list.
It's fine if we can't get the list; just continue. This is common when the repository hasn't cloned yet.
Test Plan: In a local repository with bad credentials, tried to do anything before and after. Before: completely blocked by error; after: things work normally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9080
Differential Revision: https://secure.phabricator.com/D14044
Summary:
Ref T1806. Ref T5752. Currently, `handleRequest()` needs to return an `AphrontResponse`, but sometimes it's really convenient to return some other object, like a Dialog, and let that convert into a response elsewhere.
Formalize this and clean up some of the existing hacks for it so there's less custom/magical code in Phabricator-specific classes and more general code in Aphront classes.
More broadly, I want to clean up T5752 before pursuing T9132, since I'm generally happy with how `SearchEngine` works except for how it interacts with side navs / application menus. I want to fix that first so a new Editor (which will have a lot in common with SearchEngine in terms of how controllers interact with it) doesn't make the problem twice as bad.
Test Plan:
- Loaded a bunch of normal pages.
- Loaded dialogs.
- Loaded proxy responses (submitted empty comments in Maniphest).
Reviewers: chad
Reviewed By: chad
Subscribers: joshuaspence
Maniphest Tasks: T1806, T5752
Differential Revision: https://secure.phabricator.com/D14032
Summary: See D14025. In all cases where we compare hashes, use strict, constant-time comparisons.
Test Plan: Logged in, logged out, added TOTP, ran Conduit, terminated sessions, submitted forms, changed password. Tweaked CSRF token, got rejected.
Reviewers: chad
Reviewed By: chad
Subscribers: chenxiruanhai
Differential Revision: https://secure.phabricator.com/D14026
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:
Ref T8320. I missed this a while ago and then it came to me in a dream.
Only consider paths in the same repo when looking at ownership.
(I think this is rarely reachable in practice.)
Test Plan: Verified that files and commits still listed ownership properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8320
Differential Revision: https://secure.phabricator.com/D14022
Summary:
Fixes T9218. Fixes T8320. Fixes T8661. This isn't exhaustive but documents the stuff that cropped up in this iteration as needing documentation. In particular:
- Be explicit about multiple ownership.
- Explain value of having one place to update your giant regexp of a trillion paths.
Test Plan: Read documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8320, T8661, T9218
Differential Revision: https://secure.phabricator.com/D14023
Summary:
Fixes T9279. Modernizes the SearchEngine and Query classes. User-facing changes:
- Added order by commit date, default to order by commit date with newest commits first.
- Added explicit "Needs Audit by".
- Added new `packages(...)` typeahead function.
- Picked up automatic subscribers, projects, and order fields.
This changes behavior a little bit: we previously attempted to exclude, e.g., commits which a package you own needs to audit, but which you have resigned from. This is difficult in general and I think it needs a more comprehensive solution. This shouldn't impact users much, anyway.
Test Plan: {F767628}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9279
Differential Revision: https://secure.phabricator.com/D14013
Summary: Ref T9089. This link leads to a detail page, not an edit page, and is always visible by users with permission to see the column.
Test Plan: Clicked "Column Details" with and without edit permission.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9089
Differential Revision: https://secure.phabricator.com/D14016
Summary: Fixes T9090. You don't need to be able to edit a project to create tasks on its workboard. Being able to view the project is sufficient, and the user certianly can if they got this far.
Test Plan: Viewed workboard, hit "Create Task".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9090
Differential Revision: https://secure.phabricator.com/D14015
Summary: Fixes T9278. Logged out viewers shouldn't see a form field to answer, just a login button.
Test Plan: Log out, go to question, click Login to Answer, login, get redirected back.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9278
Differential Revision: https://secure.phabricator.com/D14012
Summary:
This enables CORGI.
Currently, `AphrontSite` subclasses can't really have their own routes. They can do this sort of hacky rewriting of paths, but that's a mess and not desirable in the long run.
Instead, let subclasses build their own routing maps. This will let CORP and ORG have their own routing maps.
I was able to get rid of the `PhameBlogResourcesSite` since it can really just share the standard resources site.
Test Plan:
- With no base URI set, and a base URI set, loaded main page and resources (from main site).
- With file domain set, loaded resources from main site and file site.
- Loaded a skinned blog from a domain.
- Loaded a skinned blog from the main site.
- Viewed "Request" tab of DarkConsole to see site/controller info.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14008
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: Adds a notice reminding viewers of their own question to resolve it and mark the correct answer.
Test Plan:
View my own open question, see notice. Resolve question, notice goes away.
{F743481}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13958
Summary: Updates Releeph callsites to handleRequest
Test Plan: Bounce around Releeph, cut a branch, edit a product, view history
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14001
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: Ref T9134. It looks like this functionality was removed in D13848.
Test Plan: Submitted a diff successfully.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9134
Differential Revision: https://secure.phabricator.com/D13869
Summary:
Email is so exciting I can't wait 30 days for initial results.
ref T9161
Test Plan:
* `./bin/mail volume --days 60` took longer and gave plausibly larger
results.
* `./bin/mail volume --days 0` quickly told me no mail had been sent.
* `./bin/mail volume` Said it was still looking 30 days back.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Maniphest Tasks: T9161
Differential Revision: https://secure.phabricator.com/D13901
Summary:
We currently detect tab panels embedding themselves, but do not detect text panels embedding themselves with `{Wxx}`.
Detect these self-embedding panels.
I had to add a bit of a hack to pass the parent panel PHIDs to the rule. Generally, I got the Markup API kind of wrong and want to update it, I'll file a followup with details about how I'd like to move forward.
Test Plan:
Created a text panel embedding itself, a tab panel embedding a text panel embedding itself, a tab panel embedding a text panel embedding the tab panel, etc.
Rendered all panels standalone and as `{Wxx}` from a different context.
{F761158}
{F761159}
{F761160}
{F761161}
{F761162}
Reviewers: chad, jbeta
Reviewed By: chad, jbeta
Differential Revision: https://secure.phabricator.com/D13999
Summary:
Fixes T9268. Currently, we try to match any string like "a2f313f1" as a commit/revision, so short hashes will get picked up.
However, we don't require a word boundary or terminal after the match, so for input like "aaa...aaaaz" the engine can get stuck trying to split the string into sub-matches.
That is, in the original case, the input "aaaz" had valid matches against `[rA-Z0-9a-f]+` up to "z" of:
aaa
aa a
a aa
a a a
All of these will fail once it hits "z", but it has to try them all. This complexity is explosive with longer strings.
Instead, require a word boundary or EOL after the match, so this is the only valid match:
aaa
Then the engine sees the "z", says "nope, no match" and doesn't have to backtrack across all possible combinations.
Test Plan: Added a failing unit test, applied patch, clean test.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9268
Differential Revision: https://secure.phabricator.com/D13997
Summary:
Fixes T9251. Old mail could get saved with bad parameters for two reasons that I can come up with:
- Nothing ever set a parameter on it -- not sure this could ever actually happen; or
- some field contained non-UTF8 data prior to D13939 and we silently failed to encode it.
My guess is that the second case is probably the culprit here.
In any case, recover from this so `20150622.metamta.5.actor-phid-mig.php` can proceed.
Test Plan: Same effective patch as user patch in T9251; looked at some mail to make sure it was still pulling parameters properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9251
Differential Revision: https://secure.phabricator.com/D13990
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: Ref T2015. This fixes issues where the Drydock queries wouldn't filter (or throw an exception) when passed empty arrays for their `with` methods. In addition, this also adds `array_unique` to the resource and lease subqueries so that we don't pull in a bunch of stuff if logs or leases have the same related objects.
Test Plan: Tested it by using DarkConsole on the log controller.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: joshuaspence, Korvin, epriestley
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D10879
Summary: Ref T2015. This allows searching based on blueprints, resources or leases when viewing the logs, which helps when searching for events that occured to a particular blueprint / resource / lease. Unlike the logs shown on the resource / lease pages, the search engine supports paging properly, which means it can be used to find entries in the past.
Test Plan: Used the Drydock log search page.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: joshuaspence, Korvin, epriestley
Maniphest Tasks: T2015
Differential Revision: https://secure.phabricator.com/D10874
Summary: Show the time in addition to the date in the Drydock logs.
Test Plan: Brought forward from D10479.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: joshuaspence, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10909
Summary: Ref T1049. This ensures the Harbormaster build target is associated with leases, so in the future we can query things and find out whether builds are still running with associated leases.
Test Plan: Leased a host, checked the DB and saw the field populated.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: joshuaspence, Korvin, epriestley
Maniphest Tasks: T1049
Differential Revision: https://secure.phabricator.com/D10870
Summary:
Ref T8783.
The "View" UI is where a user would check their request for feedback or a resolution, if it's something that makes sense for them to interact with from the web UI.
The "Edit" UI is the manage/admin UI where you'd respond to a request. It's similar to the view UI but will have actions and eventually some queue UI, etc.
(I don't think items need a normal "Edit" UI -- it doesn't make sense to "Edit" a tweet or inbound email -- but maybe this will shuffle around a little eventually.)
Test Plan:
View
{F747218}
Edit
{F747219}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8783
Differential Revision: https://secure.phabricator.com/D13980
Summary:
Fixes T9245. These picked up some possibly-confusing metadata, like in the screenshot on T9245 where "Subscribers" appears in the middle of the page for no obvious reason.
- Make these pages a little cleaner by removing elements which aren't important for signing agreements.
- Use the last time the actual document text was updated as the modification time, not the last time the "Document" object was modified. The latter will change for trivial things like altering the view/edit policy, but that could be confusing if you see that a TOS was "last updated yesterday" but can't figure out what actually changed (since nothing changed).
Test Plan: Viewed signature page for a document.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9245
Differential Revision: https://secure.phabricator.com/D13982
Summary:
Ref T8783. If you have a source (like a "report bug" form), let it put a link (like "View Form") on the source detail page.
This also straightens out getting definitions from sources, which had a bug with the modern way we do `PhutilClassMapQuery`.
Specifically, if you called the old mechanism on two different sources, they'd return the same definition object, but they need to return different definitions.
Test Plan:
{F747093}
{F747092}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8783
Differential Revision: https://secure.phabricator.com/D13966
Summary: Ref T8783. There's nothing at `/nuance/` right now, put something basic there.
Test Plan: {F747078}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8783
Differential Revision: https://secure.phabricator.com/D13965
Summary:
Ref T8672. Ref T9187. Root issue in at least one case is:
- User makes a commit including a file with some non-UTF8 text (say, a Japanese file full of Shift-JIS).
- We pass the file to the TransactionEditor so it can inline or attach the patch if the server is configured for these things.
- When inlining patches, we convert them to UTF8 before inlining. We must do this since the rest of the mail is UTF8.
- When attaching patches, we send them in the original encoding (as file attachments). This is correct, and means we need to give the worker the raw patch in whatever encoding it was originally in: we can't just convert it to utf8 earlier, or we'd attach the wrong patch in some cases.
- TransactionEditor does its thing (e.g., creates the commit), then gets ready to send mail about whatever it did.
- The publishing work now happens in the daemon queue, so we prepare to queue a PublishWorker and pass it the patch (with some other data).
- When we queue workers, we serialize the state data with JSON.
So far, so good. But this is where things go wrong:
- JSON can't encode binary data, and can't encode Shift-JIS. The encoding silently fails and we ignore it.
Then we get to the worker, and things go wrong-er:
- Since the data is bad, we fatal. This isn't a permanent failure, so we continue retrying the task indefinitely.
This applies several fixes:
# When queueing tasks, fail loudly when JSON encoding fails.
# In the worker, fail permanently when data can't be decoded.
# Allow Editors to specify that some of their data is binary and needs special handling.
This is fairly messy, but some simpler alternatives don't seem like good ways forward:
- We can't convert to UTF8 earlier, because we need the original raw patch when adding it as an attachment.
- We could encode //only// this field, but I suspect some other fields will also need attention, so that adding a mechanism will be worthwhile. In particular, I suspect filenames //may// be causing a similar problem in some cases.
- We could convert task data to always use a serialize()-based binary safe encoding, but this is a larger change and I think it's correct that things are UTF8 by default, even if it makes a bit of a mess. I'd rather have an explicit mess like this than a lot of binary data floating around.
The change to make `LiskDAO` will almost certainly catch some other problems too, so I'm going to hold this until after `stable` is cut. These problems were existing problems (i.e., the code was previously breaking or destroying data) so it's definitely correct to catch them, but this will make the problems much more obvious/urgent than they previously were.
Test Plan:
- Created a commit with a bunch of Shift-JIS stuff in a file.
- Tried to import it.
Prior to patch:
- Broken PublishWorker with distant, irrelevant error message.
With patch partially applied (only new error checking):
- Explicit, local error message about bad key in serialized data.
With patch fully applied:
- Import went fine and mail generated.
Reviewers: chad
Reviewed By: chad
Subscribers: devurandom, nevogd
Maniphest Tasks: T8672, T9187
Differential Revision: https://secure.phabricator.com/D13939
Summary: Fixes T9241. Users have a tendancy to assume Ponder is a "forum", make replying to your own question take an additional click.
Test Plan:
View my own question, see notice, click open answer box, reply. Visit not my question, see box as normal.
{F743412}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: cspeckmim, Korvin
Maniphest Tasks: T9241
Differential Revision: https://secure.phabricator.com/D13957
Summary: Ref T8588. It looks like something slow is happening //before// we start DarkConsole. Add some crude reporting to try to narrow it down.
Test Plan: {F743050}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8588
Differential Revision: https://secure.phabricator.com/D13956
Summary:
Fixes T9237. A while ago, the old (more-pink) indigo got split into "pink" (more pink) and "indigo" (more purple), but we didn't change this color config in Maniphest.
This generally made the color more purple, and it's now pretty simliar to the "needs triage" color (violet).
Make it "pink" instead.
Test Plan: {F742617}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9237
Differential Revision: https://secure.phabricator.com/D13954
Summary: Should fix all email reply issues, but no solid means of testing at home (how do you local reply test?)
Test Plan: Check for answer mail in /mail/ and see proper headers. Make sure question mail works too.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3846
Differential Revision: https://secure.phabricator.com/D13951
Summary: Fixes T9234. The joins method was still the old method and the builtin was calling the wrong key.
Test Plan: Test authored builtin, custom search
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9234
Differential Revision: https://secure.phabricator.com/D13953
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: Fixes T9226, shows the update time, not the creation time.
Test Plan: Update an answer, see new date.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9226
Differential Revision: https://secure.phabricator.com/D13945
Summary:
Ref T9218. See discussion there for rationale; I think this is the right behavior to pursue.
The screenshot below is pretty ugly. I think it's a lot worse than most real-world cases will be, since you have to sort of opt-in to having crazy levels of overlapping packages, and it's perfectly normal/reasonable for files owned by one package. Owners is powerful enough to let you specify sub-packages with exclusive ownership.
That said, this may be more typical than I hope. I don't think we can reduce the complexity here much for free, but it would might be reasonable to add some view options (e.g.: group by package?, show only packages I own?, show packages as icons with a tooltip?) if it's an issue.
Test Plan: {F734956}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9218
Differential Revision: https://secure.phabricator.com/D13940
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: This algorithm isn't quite right with respect to ordering more-specific packages correctly.
Test Plan: {F729864}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D13929
Summary: Pending other diffs, this actually removes Ponder as a prototype. Fixes T3578
Test Plan: No longer listed as a prototype
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D13920
Summary: Ref T9205. This is a likely fix.
Test Plan: This isn't straightforward to test in the upstream unless you have custom code on top of it.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9205
Differential Revision: https://secure.phabricator.com/D13928
Summary: Fixes T9207. I think this is correct? Sorts based on vote count and reverses the order of the array so higher is first.
Test Plan: Test vote ordering on a number of sample tasks.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9207
Differential Revision: https://secure.phabricator.com/D13924
Summary: Ref T9068, Ref T3846. Maybe fixes both, but I'm having issues testing email replies in a sandbox. Moves answer feed/mail generation to the AnswerEditor, hides it in QuestionEditor.
Test Plan: Write an answer, see feed story, check /mail/ for mail generation.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3846, T9068
Differential Revision: https://secure.phabricator.com/D13905
Summary:
Fixes T8004.
- For paths which are part of a package, show the package.
- Highlight paths which are part of a package you (the viewer) have authority over.
Test Plan:
{F725418}
- Viewed owned and unowned chagnes in Diffusion and Differential.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8004
Differential Revision: https://secure.phabricator.com/D13923
Summary: These were a little silly, simplify them
Test Plan: New countdown, edit preferences, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13888
Summary:
Ref T8320. This field doesn't have any special effects and has no reason to exist. Primary owners are always already owners.
This makes one minor implicit change to Owners: it is now possible to create a package with no owners. That seems fine; it is reasonable to create a package purely as an organizational tool and use it in, e.g., Herald.
Test Plan: Created and edited packages.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8320
Differential Revision: https://secure.phabricator.com/D13921
Summary:
Ref T9201. At the root of a repository the current path is `null`, but the OwnersQuery wants strings.
This could be resolved a couple different ways, but just cast the arguments to strings since that seems reasonable enough.
Test Plan: Browsed root of a repository.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9201
Differential Revision: https://secure.phabricator.com/D13919
Summary: When using the active query in Macro, all Macros are returned. Properly set the status query.
Test Plan: Review active macros, don't see archived macros.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13912
Summary: Ref T8700, I don't believe we need to be specific here about the object, since it displays on the object.
Test Plan: Change policy a few times on a task, see new translation
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T8700
Differential Revision: https://secure.phabricator.com/D13913
Summary:
Ref T8320. Ref T8004. This just tries to generally modernize
It also replaces the nonfunctional "Find Owners" link with a new property that just shows owning packages.
Test Plan:
- Created and edited packages.
{F720478}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8004, T8320
Differential Revision: https://secure.phabricator.com/D13911
Summary:
Fixes T2183. We now use the same rendering element in both places.
Intentional changes:
- Package highlighting is out, coming back to both apps in next diff.
- removed redundant-feeling "Change" link. The information is now shown with a character ("M", "V", etc.) and the page is a click away under "History". Clicking the path also jumps you to substantially similar content. (We could restore it fairly easily, I just think it's probably the least useful thing in the table right now.)
Test Plan: Viewed a bunch of commits in Diffusion.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2183
Differential Revision: https://secure.phabricator.com/D13910
Summary:
Ref T2183. Introduces a new View which can (in theory) unify the Revision, Diff and Commit table of contents views.
This has the same behavior as before, but accepts slightly more general primitives and parameters and has somewhat cleaner code.
I've made one intentinoal behavior change: removing the "Open All in Editor" button. I suspect this is essentially unused, and is a pain to keep around. We can look at restoring it if anyone notices.
Test Plan: Looked at a bunch of revisions, no changes from before.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2183
Differential Revision: https://secure.phabricator.com/D13908
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 T5791. This is still very basic (no global actions, no support for matching headers/bodies/recipients/etc) but gets the core in.
Test Plan:
{F715209}
{F715211}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13897
Summary: Ref T2183. These properties are not used and not useful.
Test Plan: Grepped for callsites and uses. Loaded revisions.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T2183
Differential Revision: https://secure.phabricator.com/D13906
Summary: Fixes T8659. This isn't //explicitly// documented but I'm going to wait for a bit until the "Harbormaster" doc splits into internal/external builds to add docs for it. There's other similar stuff coming soon anyway.
Test Plan:
{F716439}
{F716440}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8659
Differential Revision: https://secure.phabricator.com/D13903
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: I derped here and broke new questions, also remove old Answer constants
Test Plan: Ask a new question, see it save
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13899
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 T5791. This collects outbound mail status in one place and makes the list view a little spiffier.
Test Plan: Looked at list and detail views. Grepped for changed classes/constants.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5791
Differential Revision: https://secure.phabricator.com/D13884
Summary: Ref T3578, Changes "Ask Away" to just "Submit", Changes Description to Details, check for is_new when offering closed state on question edit.
Test Plan: New Question, Edit Question
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D13891
Summary: Use `PhutilClassMaQuery` instead of `PhutilSymbolLoader`, mostly for consistency. Depends on D13588.
Test Plan: Poked around a bunch of pages.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D13589
Summary: Ref T3578 Adds mailtags to Ponder, answer stuff not quite ready, but that's another diff.
Test Plan: set preferences to notify, second account updates a question, get notification on first.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D13886
Summary:
Ref T9141. Ref T5791. Ref T7013. Major changes here is:
- Currently, we don't store the headers we actually sent, or the reasons we actually did or did not deliver a mail.
- Start storing these (as `headers.sent` and `actors.sent`).
- Show them in the web UI.
- Show them in `bin/mail show-outbound` (previously, we sort of re-computed them in a hacky way).
- Take them into account in `bin/mail volume`.
Then some minor changes:
- Show mail bodies.
- Show more mail information.
- Start renaming "MetaMTA" to "Mail", at least in the web UI.
Test Plan:
{F707501}
{F707502}
{F707503}
{F707504}
{F707505}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5791, T7013, T9141
Differential Revision: https://secure.phabricator.com/D13878
Summary:
Ref T9099.
- Remove unused function
- Fix celerity resource include
- Add a note when an answer's been closed
- Clean up architecture a little
- Add a better notice when you've already answered.
Test Plan: Test with and without answers, with answering, with being closed.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9099
Differential Revision: https://secure.phabricator.com/D13875
Summary: In Ponder, I think the right default query is "good" questions. That being recently asked, and either open or resolved. For browsing this seems correct. For moderators, I presume they use saved queries accordingly. This is similar to stack overflow and quora.
Test Plan: Review queries in my dev box, all seems to run correctly
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13877
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: Fixes T9128. I missed this when converting other controllers to look in Harbormaster for lint/unit data.
Test Plan: Copy/pasted a diff successfully.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9128
Differential Revision: https://secure.phabricator.com/D13865
Summary:
Ref T8096. This modernizes the last thing which was reading the old datasource.
Also fix a bug where it didn't work.
Test Plan: {F698405}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13852
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 T8096. This information has moved into Harbormaster.
Test Plan:
Pushed some test results in; see right margin:
{F698394}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13850
Summary:
Ref T8096.
Long ago, support for "postponed" lint and unit tests got hacked in. `arc` would publish a bunch of ghost results, and then something else would fill the results in later.
This was always a hack. It is not nearly as powerful or flexible as having a real build system, and is obsolete with Harbormaster, which supports these operations in a more reasonable and straightforward way.
This was used (only? almost only?) at Facebook.
- Remove `differential.finishpostponedlinters`. This only served to update postponed linters.
- Remove lint magic in `differential.setdiffproperty`. This magic only made sense in the context of postponed linters.
- Remove `differential.updateunitresults`. The only made sense for postponed unit tests.
And one minor change: when a diff contains >100 affected files, we hide the content by default, but show content for files with inline comments. Previously, we'd do this for lint inlines, too. I don't tink this is too useful, and it's much simpler to just remove it. We could add it back at some point, but I think large changes often trigger a lot of lint and no one actually cares. The behavior for actual human inlines is retained.
Test Plan: `grep`
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13848
Summary: Ref T8096. These are still reading out of the old diff property, but should read from Harbormaster instead.
Test Plan: {F698346}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13847
Summary:
Ref T8096. Hit this on IRC:
> epriestley: what's in the Messages tab?
> jdoe: what Messages tab??!
> epriestley: ughhhh
Test Plan: Clicked "Messages" tab, saw helpful "no messages" message.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13846
Summary:
Fixes T7370. Two changes:
- Make the default to show nothing, instead of showing all the data. This is a better default because the data is sometimes sensitive. Workers should have to opt in to revealing it.
- For TargetWorkers, link to the target (technically the build, for now, since there's no dedicated target detail page).
Test Plan: {F698325}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7370
Differential Revision: https://secure.phabricator.com/D13845
Summary: Ref T8096. Like stop/start, autoplans are pushed into the system from outside (normally by `arc`) so it doesn't make any sense to run them manually.
Test Plan:
- Tried to run an autoplan from web UI, got an error.
- Ran a normal plan from web UI.
- Tried to run an autoplan from CLI, got an error.
- Ran a noraml plan from CLI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8096
Differential Revision: https://secure.phabricator.com/D13844
Summary: Ref T6920, this adds a basic controller for marking an answer as helpful and removes the negative voting. Any current positive vote is kept as helpful. New UI is needed here, but there is a separate task for redesigning Ponder overall.
Test Plan: Mark an answer as helpful, see count go up, remove helpful, see count go down. Test endpoint manually.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T6920
Differential Revision: https://secure.phabricator.com/D13834
Summary: Updates the Daemon application calls
Test Plan: Click on various things in the Daemon app
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D13835
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
