Summary: Ref T3886. Now that a custom field can emit a core transaction, just emit a subscribers transaction.
Test Plan: {F116014}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3886
Differential Revision: https://secure.phabricator.com/D8289
Summary:
Ref T3886. Ref T418.
- Adds "View Policy" and "Edit Policy" fields.
- Allows CustomFields to produce arbitrary types of transactions, so these fields can produce standard view/edit policy transactions and get all the strings and validation associated with them.
Test Plan: {F116001}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T418, T3886
Differential Revision: https://secure.phabricator.com/D8287
Summary: Ref T3886. Moves some of the "required" logic to the base class ("DifferentialCoreField") so Title and Test Plan can share it.
Test Plan: Edited revisions using "pro" editor, saw test plan transactions.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3886
Differential Revision: https://secure.phabricator.com/D8285
Summary:
Ref T3886. Ref T418. For fields like "Summary" and "Test Plan" where changes can't be summarized in one line, allow CustomField to provide a "(Show Details)" link and render a diff.
Also consolidate some of the existing copy/paste, and simplify this featuer slightly now that we've move to dialogs.
Test Plan:
{F115918}
- Viewed "description"-style field changes in phlux, pholio, legalpad, maniphest, differential, ponder (questions), ponder (answers), and repositories.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3886, T418
Differential Revision: https://secure.phabricator.com/D8284
Summary:
Ref T3886.
- Adds "Summary" field.
- Adds "CoreField" for fields stored on the actual object, to reduce code duplication a bit for the main fields.
Test Plan: {F115902}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3886
Differential Revision: https://secure.phabricator.com/D8283
Summary: Ref T2222. Ref T1790. I partially modernized this recently, but bring it to the mail version too.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: zeeg, aran
Maniphest Tasks: T1790, T2222
Differential Revision: https://secure.phabricator.com/D8294
Summary: Fixes T4466. We do an excessively strict effect check now, which means that these fields changing from (string) "16" to (int) 16 generates a transaction. Instead, compare integer values if the field has data in it.
Test Plan:
{F116261}
(Also made updates without changing the number, which did not appear in the transaction log anymore.)
Reviewers: btrahan, richardvanvelzen
Reviewed By: richardvanvelzen
CC: aran
Maniphest Tasks: T4466
Differential Revision: https://secure.phabricator.com/D8292
Summary:
Fixes T4463. When your VCS or account password is not set, we test it for upgrade anyway. This doesn't make sense and throws shortly into the process because the empty hash isn't parseable.
Instead, only show upgrade prompts when the password exists.
Test Plan:
- Added a password to an existing account with no password via password reset.
- Added a VCS password to an existing account with no VCS password.
- Observed no fatals / nonsense behaviors.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T4463
Differential Revision: https://secure.phabricator.com/D8282
Summary: When we removed the shadow, we no longer needed two containers.
Test Plan: Browsed Box example, a diff, a task, and other random pages. Grep for phui-box-inner, not used elsewhere.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8281
Summary: so it was said in IRC and so it is true
Test Plan: saving maniphest tasks with custom fields no longer barfs
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8279
Summary:
Error:
Fatal error: Call to a member function getRefType() on a non-object in /opt/phabricator/phabricator/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php on line 197
Test Plan: No more error in daemon.log afterwards
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8278
Summary:
Ref T3496. Currently, we call loadAssets() on each revision table, which invokes a new revision query and a pile of subqueries.
Instead, add `needFlags()` and `needDrafts()` to `RevisionQuery`. Some day these could perhaps be more generic.
Test Plan:
- Viewed home, differential, etc., no longer saw 9203809238 queries being run for no reason.
- Drafts and flags still appear properly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3496
Differential Revision: https://secure.phabricator.com/D8277
Summary:
Ref T3886. I spent a few hours trying to make `DifferentialFieldSpecification` extend `PhabricatorCustomField` so I could be more blunt in my approach here and just swap the whole thing over in one go (more or less like I did with Maniphest) but we have a ton of custom fields and things felt really shaky and the change was enormous and hard to keep track of.
Instead, I'm going to do this more gradually and go field-by-field. This implements a CustomField version of the "Title" field.
(There are no links to this in the UI.)
Test Plan:
{F115353}
{F115354}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3886
Differential Revision: https://secure.phabricator.com/D8276
Summary: Ref T4361. Projects are mailable now, so let them show up in mail contexts.
Test Plan: Added a project as a CC to a task, filtered by project CCs, etc.
Reviewers: btrahan, zeeg, dctrwatson
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4361
Differential Revision: https://secure.phabricator.com/D8274
Summary:
Ref T3886. Fixes the removed TODO. This also implements the generally reasonable policy "you have to be able to see an object in order to see its transactions". That was implicit before (we never load transactions without loading an object first) but is now explicit.
This fixes bad (nonspecialized) rendering of custom field transactions in Projects, and shortly in Differential, where stories would read "alincoln edited this object." instead of a more specific string.
Test Plan: Viewed a project edit, saw a more specific string. Browed ApplicationTransaction applications.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3886
Differential Revision: https://secure.phabricator.com/D8273
Summary: ...do it somewhat generically, so we could fairly easily add this to other applications. Fixes T3496. I got a wee bit lazy and decided not to migrate existing drafts. My excuses aside from laziness are doing it this way will let us see if anyone complains, we can always do a migration later if people do complain, and there's likely to be a lot of garbage data for older / bigger installs, and the migration didn't seem worth itgiven it would also likely be expensive in these cases.
Test Plan: made a draft inline comment on DX and observed DX had a note icon on Differential home page. made a draft comment on DX and observed DX had a note icon on Differential home page. deleted a draft inline comment and noted icon disappeared from Differential homepage. Submitted a draft comment + inline comment and noted icon disappeared.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T3496
Differential Revision: https://secure.phabricator.com/D8275
Summary: Fixes T4443. Plug VCS passwords into the shared key stretching. They don't use any real stretching now (I anticipated doing something like T4443 eventually) so we can just migrate them into stretching all at once.
Test Plan:
- Viewed VCS settings.
- Used VCS password after migration.
- Set VCS password.
- Upgraded VCS password by using it.
- Used VCS password some more.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4443
Differential Revision: https://secure.phabricator.com/D8272
Summary:
Ref T4443. In addition to performing upgrades from, e.g., md5 -> bcrypt, also allow sidegrades from, e.g., bcrypt(cost=11) to bcrypt(cost=12). This allows us to, for example, bump the cost function every 18 months and stay on par with Moore's law, on average.
I'm also allowing "upgrades" which technically reduce cost, but this seems like the right thing to do (i.e., generally migrate password storage so it's all uniform, on average).
Test Plan:
- Fiddled the bcrypt cost function and saw appropriate upgrade UI, and upgraded passwords upon password change.
- Passwords still worked.
- Around cost=13 or 14 things start getting noticibly slow, so bcrypt does actually work. Such wow.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4443
Differential Revision: https://secure.phabricator.com/D8271
Summary:
Ref T4443.
- Add a `password_hash()`-based bcrypt hasher if `password_hash()` is available.
- When a user logs in using a password, upgrade their password to the strongest available hash format.
- On the password settings page:
- Warn the user if their password uses any algorithm other than the strongest one.
- Show the algorithm the password uses.
- Show the best available algorithm.
Test Plan: As an md5 user, viewed password settings page and saw a warning. Logged out. Logged in, got upgraded, no more warning. Changed password, verified database rehash. Logged out, logged in.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4443
Differential Revision: https://secure.phabricator.com/D8270
Summary:
Ref T4443. Make hashing algorithms pluggable and extensible so we can deal with the attendant complexities more easily.
This moves "Iterated MD5" to a modular implementation, and adds a tiny bit of hack-glue so we don't need to migrate the DB in this patch. I'll migrate in the next patch, then add bcrypt.
Test Plan:
- Verified that the same stuff gets stored in the DB (i.e., no functional changes):
- Logged into an old password account.
- Changed password.
- Registered a new account.
- Changed password.
- Switched back to master.
- Logged in / out, changed password.
- Switched back, logged in.
- Ran unit tests (they aren't super extensive, but cover some of the basics).
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, kofalt
Maniphest Tasks: T4443
Differential Revision: https://secure.phabricator.com/D8268
Summary: Crumbs everywhere. Not sure how to better format the date, let me know.
Test Plan: Browse everything I can see in Calendar
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4440
Differential Revision: https://secure.phabricator.com/D8265
Summary: Let's people know what the event is, specifically.
Test Plan: View an Event Page
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8267
Summary: Ref T4324. Currently, it's a bit of a pain to send yourself notifications, and often involves multiple browsers. Instead, add a button to send them.
Test Plan: {F114495}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8255
Summary:
Ref T4324. Add a real `Application` class. Use modern UI elements.
@chad, we could use an icon :3
Test Plan: {F114477}
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8254
Summary:
Ref T4324. The server code is probably going to get a fair amount more complicated, so allow it to load Javelin classes in a mostly-reasonable way.
This integration has a few warts, but should be good enough to let us manage complexity through the next iteration of the server.
(Mostly I just want the concicse Javelin mechanism for defining new classes.)
Version bump is just so I can figure stuff out if this creates any issues for users based on which version of things they're running.
Test Plan: Started server, posted some messages through it.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8253
Summary: Ref T4324. Centralize communication with the notification server. This will probably get less messy eventually.
Test Plan: Posted some messages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8252
Summary: Ref T4324. Add some version information to the server status output, and setup checks to test for an unreachable or out-of-date server.
Test Plan:
- With server down, hit reasonable setup check.
- With server up and at a bad version, hit reasonable setup check.
- Viewed `/notification/status/`.
- The CSS thing fixes this:
{F114445}
Reviewers: btrahan, chad
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T4324
Differential Revision: https://secure.phabricator.com/D8251
Summary:
Ref T1812. This cleans up most of the easy hard-coded references to specific statuses:
- The "fixes" language moves into ManiphestTaskStatus.
- Add a method to list open statuses.
- Add a method to test if a status is open.
- Add a method to get default status for new tasks.
Test Plan: Browsed around, lint, grep, created, filtered and updated tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1812
Differential Revision: https://secure.phabricator.com/D8264
Summary: Fixes T3872. Ref T1812. Ref T3886. Modernize the "closes x as y" string parser, and use all the new parsers instead of the old ones.
Test Plan: Made a commit full of a pile of these trigger strings, then used `scripts/repository/reparse.php --message` to reparse it. Verified that parses came back as expected using a bunch of `var_dump()`.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1812, T3872, T3886
Differential Revision: https://secure.phabricator.com/D8263
Summary:
Ref T3886. See D8261. This brings the "reverts x" phrase to modern infrastructure. It isn't actually called by the real parser yet, I'm going to do that in one go at the end so I can test everything more easily.
This had unit tests; port most of them forward.
Test Plan: Added and executed unit tests.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3886
Differential Revision: https://secure.phabricator.com/D8262
Summary: Fixes T3497.
Test Plan: on conpherence 1, typed some stuff. clicked conpherence 2 - observed some stuff gone. clicked conpherence 1 - stuff came back! submitted conpherence 1 and reloaded - stuff did not come back. (Generally played around a bunch like this)
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T3497
Differential Revision: https://secure.phabricator.com/D8266
Summary:
Ref T3886. Ref T3872. Ref T1812. We have several parsers which look for textual references to other objects, like:
Closes Tx.
Depends on Dy.
Reverts Dz.
Currently, these are pretty hard coded, don't get all the edge cases right, and don't generalize well. They're also implemented in the middle of Differential's field code. So I want to:
- Share more code so that, e.g., "Tx, Ty" always works (only some rules support it right now);
- fix bugs in the parser, like T3872;
- make this a modular, extensible process which runs against custom fields, not a builtin part of fields;
- make the internals more flexible to accommodate custom stuff like T1812.
This implements the "Verbs optional-noun Object, Optional Other Objects optional-as-something." grammar in a general way so subclasses can just plug in their keywords. Runtime code doesn't touch this yet.
Test Plan: Ran unit tests.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3872, T1812, T3886
Differential Revision: https://secure.phabricator.com/D8261
Summary: Ref T4446. Some discussion in IRC. Prior to hitting the 1.0.0 issue, we hit and resolved this issue; this is a leftover call from bringing ApplicationSearch to main search.
Test Plan: User confirmed this fixes the issue.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4446
Differential Revision: https://secure.phabricator.com/D8260
Summary: Ref T4426. This moves "Edit Details", "Edit Picture", and "Archive" to a separate "Edit" interface. "History" becomes part of this UI.
Test Plan:
{F114417}
{F114418}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T4426
Differential Revision: https://secure.phabricator.com/D8248
Summary: See <https://github.com/facebook/phabricator/pull/510>.
After the repository rework, the Create New Repository link in Repositories goes straight to creating a phabricator hosted repo(diffusion/create), rather than the chooser create/import (diffusion/new)
I updated it to point to diffusion/new the same as the New Repository link in Diffusion.
As a side note, the Repositories page could probably use the Crumbs treatment.
Reviewed by: epriestley
Summary: Updates Calendar View to more modern components.
Test Plan: Browse Calendar Forward and Back, Create a Status, Get Excited, Get PUMPED.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4392
Differential Revision: https://secure.phabricator.com/D8247
Summary: This applies a minor fix for the Phragment ZIP controller where it would raise missing index errors when attempting to map deleted fragments to file PHIDs (since deleted fragments no longer have files).
Test Plan: Tested this patch on a live server and saw the issue go away.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8243
Summary:
Ref T2222. I want to stage a "later" patch to drop this column, but get rid of the last few references to it.
One of these methods has no callers, and the other stuff I've updated to use the modern fields.
Test Plan: Created some inlines, hit "edit", submitted them, `grep.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8240
Summary:
Ref T4420. When a result list contains both open and closed results, hide the closed results. I think this has a good chance of almost always working, and feeling very intuitive. It has a small chance of being a weird mess. It feels reasonable to me so far
The one bad case I can come up with here is that if you have results which shadow each other, like "Apples" (a closed project) and "Apples and Bananas" (an open project), it is impossible to get "Apples" in the result list, because "Apples and Bananas" will always shadow it. Let's wait for someone to hit this before we figure out how to deal with it.
Test Plan: Typed through open stuff to hit closed stuff.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8238
Summary: Ref T2222. I accidentally changed the beahvior of this in D8229.
Test Plan: Revisions with no comments no longer with too little space between the main object box and the "Revision Update History".
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8239
Summary: See thread; these are just bugs. Handles and main search do not mark projects correctly as open/closed.
Test Plan: Searched for projects and observed they respect the open/closed flags properly after reidnexing.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D8237
Summary: Ref T2222. Use new code for rendering. Delete `DifferentialRevisionCommentView`, which has no remaining callsites.
Test Plan: Went through all the different actions and verified the previews rendered correctly. Reloaded page to test draft behavior.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8236
Summary:
Allows to keep really wide graphs inside the div:
dot (width=100%) {{{ }}}
Test Plan:
Created a graph that is wider than a phriction page.
Added `(width=100%)` and now the images stays within the div.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8235
Summary:
Ref T4420. Tokenizers currently operate in "preload" or "ondemand" modes. In the former mode, which is default, they'll try to load the entire result list when a page loads.
The theory here was that this would slightly improve the experience for small installs, and once they got big enough they could switch to "ondemand". In practice, several issues have arisen:
- We generally don't have a good mechanism for telling installs that they should tweak perf config -- `metamta.send-immediately` is the canonical example here. Some large installs are probably affected negatively by not knowing to change this setting, and having settings like this is generally annoying.
- We have way way too much config now.
- With the advent of ApplicationSearch, pages like Maniphest make many redundant loads to prefill sources like projects. Most of the time, this data is not used. It's far simpler to switch everything to ondemand than try to deal with this, and dealing with this would mean creating two very complex divergent pathways in the codebase for a mostly theoretical performance benefit which only impacts tiny installs.
- We've been using `tokenizer.ondemand` forever on `secure.phabricator.com` since we have many thousands of user accounts, and it doesn't seem sluggish and works properly.
Removing this config is an easy fix which makes the codebase simpler.
I've retained the ability to use preloaded sources, since they may make sense in some cases (in at least one case -- task priorities -- adding a static source pathway might make sense), and they're part of Javelin itself. However, the code will no longer ever go down that pathway.
Test Plan: Used `secure.phabricator.com` for years with this setting enabled.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8232
Summary:
Ref T4420. This is mostly a design change, but addresses two functional issues:
# Many sources exclude disabled accounts, system agents, archived projects, etc. It is rare to select these, but excluding them completely is too severe, and we've made more than a handful of changes over time to replace a "users" endpoint with an "accounts" endpoint (to include disabled users) or similar. Instead, always show these results, but sort them last and use a special style to clearly mark them as closed, disabled, or otherwise unusual.
- As a practical consequence, all the similar endpoints can now be merged, so "accounts" and "users" return the exact same result sets.
# Increasingly, sources can return multiple object types in a single list. For example, "CC" can have a user or mailing list, and soon a project or repository. However, the result list is fairly homogenous across types and it isn't easy to quickly pick out projects vs users. To help with this, add icons showing the result type.
Test Plan:
{F113079}
(The main search results get touched here too, I verified they didn't blow up.)
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran, mbishopim3
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8231
Summary:
Ref T1279. The new dual-mode user/project tokenizers are a bit disorienting. Provide content type hints.
Very open to any suggestions here, most of this patch is just getting the right data in the right places. We can change things up pretty easily.
- I like the little icons in the tokens themselves, I think they look good and are useful.
- I'm less sold on the '(Project)' thing I did in the dropdown. We can easily make this richer if you have thoughts on it -- we could put icons in the left column maybe? Or right-justify the types?
- I made it always sort users above projects.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: chad, aran, carl
Maniphest Tasks: T4420, T1279
Differential Revision: https://secure.phabricator.com/D7250
