1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-24 14:30:56 +01:00
Commit graph

463 commits

Author SHA1 Message Date
Bob Trahan
5f33aa5b4f Dashboards - add ability to install dashboard as home
Summary:
See title. Adds PhabricatorDashboardInstall data object which scopes installs to objectPHID + applicationClass. This is because we already have a collision for user home pages and user profiles. Assume only one dashboard per objectPHID + applicationClass though at the database level.

Fixes T5076.

Test Plan: From dashboard view, installed a dashboard - success! Went back to dashboard view and uninstalled it!

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5076

Differential Revision: https://secure.phabricator.com/D9206
2014-05-19 16:09:31 -07:00
lkassianik
3d457a53be Close pholio mocks
Summary: Fixes T4299, Add status dropdown to mock edit view

Test Plan: Edit mock, close mock, thumbnail title should read (Disabled). Default mocks list should show only open mocks.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: chad, epriestley, Korvin

Maniphest Tasks: T4299

Differential Revision: https://secure.phabricator.com/D9145
2014-05-19 11:34:23 -07:00
epriestley
15c408be1d Change LONGTEXT cache column to BINARY
See T4898. As it turns out, `latin1` LONGTEXT is not approximately equivalent
to LONGBLOB.
2014-05-17 22:38:56 -07:00
epriestley
d744d5d859 Fix binary/utf8 issues with Differential changeset parse cache
Summary:
Fixes T4898. After we increased the strictness of the `%s` conversion, most `serialize()` output is rejected from the cache.

Drop the cache, change the column type to latin1_bin, and then use `%B` to mark the data as binary during query construction.

Test Plan: Viewed Differential, saw cache fills.

Reviewers: btrahan, spicyj

Reviewed By: spicyj

Subscribers: epriestley

Maniphest Tasks: T4898

Differential Revision: https://secure.phabricator.com/D9171
2014-05-17 16:34:13 -07:00
epriestley
a74545c9da Provide a rough, unstable API for reporting coverage into Diffusion
Summary:
Ref T4994. This stuff works:

  - You can dump a blob of coverage information into `diffusion.updatecoverage`. This wipes existing coverage information and replaces it.
  - It shows up when viewing files.
  - It shows up when viewing commits.

This stuff does not work:

  - When viewing files, the Javascript hover interaction isn't tied in yet.
  - We always show this information, even if you're behind the commit where it was generated.
  - You can't do incremental updates.
  - There's no aggregation at the file (this file has 90% coverage), diff (the changes in this commit are 90% covered), or directory (the code in this directory has 90% coverage) levels yet.
  - This is probably not the final form of the UI, storage, or API, so you should expect occasional changes over time. I've marked the method as "Unstable" for now.

Test Plan:
  - Ran `save_lint.php` to check for collateral damage; it worked fine.
  - Ran `save_lint.php` on a new branch to check creation.
  - Published some fake coverage information.
  - Viewed an affected commit.
  - Viewed an affected file.

{F151915}

{F151916}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: jhurwitz, epriestley, zeeg

Maniphest Tasks: T5044, T4994

Differential Revision: https://secure.phabricator.com/D9022
2014-05-17 16:10:54 -07:00
Tal Shiri
43d45c4956 can now tell phabricator you trust an auth provider's emails (useful for Google OAuth), which will mark emails as "verified" and will skip email verification.
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
2014-05-16 14:14:06 -07:00
Bob Trahan
6300955661 Dashboards - add layout mode to dashboards
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
2014-05-15 19:12:40 -07:00
Aviv Eyal
f2c0e94ea8 Show command transactions in Harbormaster builds
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
2014-05-15 07:04:34 -07:00
epriestley
4a7499f230 Fix credential upgrade issue after introduction of isLocked column
Summary: Fixes T5035. This migration isn't forward compatible after schema mutation.

Test Plan: Ran locally, will get reporting user to confirm.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: gera, epriestley

Maniphest Tasks: T5035

Differential Revision: https://secure.phabricator.com/D9101
2014-05-13 12:14:27 -07:00
epriestley
95eab2f3b0 Record parent relationships when discovering commits
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
2014-05-12 11:47:22 -07:00
Bob Trahan
e96c363eef Add SMS support
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
2014-05-09 12:47:21 -07:00
epriestley
bd7420c4bb Allow pastes to be edited
Summary: Fixes T4814.

Test Plan: Edited pastes from the web UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4814

Differential Revision: https://secure.phabricator.com/D8970
2014-05-04 11:11:46 -07:00
epriestley
7fb89686d4 Upstream a patch to cover a migration issue
Summary: Fixes T4928. I'm not sure how this column was missing, but this patch can't hurt.

Test Plan: Reasoned about behavior.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4928

Differential Revision: https://secure.phabricator.com/D8967
2014-05-04 10:48:25 -07:00
lkassianik
d7b7b19337 Add a "Lock Permanently" action to Passphrase
Summary: Fixes T4931. Each new credential should come with the ability to lock the credential permanently, so that no one can ever edit again. Each existing credential must allow user to lock existing credential.

Test Plan: Create new credential, verify that you can lock it before saving it. Open existing unlocked credential, verify that option to lock it exists. Once credential is locked, the option to reveal it should be disabled, and editing the credential won't allow username/password updates.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4931

Differential Revision: https://secure.phabricator.com/D8947
2014-05-02 18:21:51 -07:00
epriestley
2022a70e16 Implement bin/remove, for structured destruction of objects
Summary:
Ref T4749. Ref T3265. Ref T4909. Several goals here:

  - Move user destruction to the CLI to limit the power of rogue admins.
  - Start consolidating all "destroy named object" scripts into a single UI, to make it easier to know how to destroy things.
  - Structure object destruction so we can do a better and more automatic job of cleaning up transactions, edges, search indexes, etc.
  - Log when we destroy objects so there's a record if data goes missing.

Test Plan: Used `bin/remove destroy` to destroy several users.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3265, T4749, T4909

Differential Revision: https://secure.phabricator.com/D8940
2014-05-01 18:23:31 -07:00
epriestley
50376aad04 Require multiple auth factors to establish web sessions
Summary:
Ref T4398. This prompts users for multi-factor auth on login.

Roughly, this introduces the idea of "partial" sessions, which we haven't finished constructing yet. In practice, this means the session has made it through primary auth but not through multi-factor auth. Add a workflow for bringing a partial session up to a full one.

Test Plan:
  - Used Conduit.
  - Logged in as multi-factor user.
  - Logged in as no-factor user.
  - Tried to do non-login-things with a partial session.
  - Reviewed account activity logs.

{F149295}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4398

Differential Revision: https://secure.phabricator.com/D8922
2014-05-01 10:23:02 -07:00
epriestley
941f0ba7ae Allow panels to appear on dashboards
Summary:
Ref T3583. Adds edges, query relationships, etc. Lots of debugging/temporary UI.

My general intent here is to use edges to track where panels appear, and then put additional data on the dashboard itself to control layout, positioning, etc.

Dashboards don't actually render yet so this is still pretty boring.

Test Plan:
{F149175}

{F149176}

{F149177}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3583

Differential Revision: https://secure.phabricator.com/D8916
2014-04-30 14:28:55 -07:00
epriestley
ea954c37e4 Add dashboard panel types
Summary: Ref T3583. These will be the primary class carrying panel implementations.

Test Plan:
{F149125}

{F149126}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3583

Differential Revision: https://secure.phabricator.com/D8912
2014-04-30 14:28:20 -07:00
epriestley
17709bc167 Add multi-factor auth and TOTP support
Summary:
Ref T4398. This is still pretty rough and isn't exposed in the UI yet, but basically works. Some missing features / areas for improvement:

  - Rate limiting attempts (see TODO).
  - Marking tokens used after they're used once (see TODO), maybe. I can't think of ways an attacker could capture a token without also capturing a session, offhand.
  - Actually turning this on (see TODO).
  - This workflow is pretty wordy. It would be nice to calm it down a bit.
  - But also add more help/context to help users figure out what's going on here, I think it's not very obvious if you don't already know what "TOTP" is.
  - Add admin tool to strip auth factors off an account ("Help, I lost my phone and can't log in!").
  - Add admin tool to show users who don't have multi-factor auth? (so you can pester them)
  - Generate QR codes to make the transfer process easier (they're fairly complicated).
  - Make the "entering hi-sec" workflow actually check for auth factors and use them correctly.
  - Turn this on so users can use it.
  - Adding SMS as an option would be nice eventually.
  - Adding "password" as an option, maybe? TOTP feels fairly good to me.

I'll post a couple of screens...

Test Plan:
  - Added TOTP token with Google Authenticator.
  - Added TOTP token with Authy.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4398

Differential Revision: https://secure.phabricator.com/D8875
2014-04-28 09:27:11 -07:00
epriestley
f42ec84d0c Add "High Security" mode to support multi-factor auth
Summary:
Ref T4398. This is roughly a "sudo" mode, like GitHub has for accessing SSH keys, or Facebook has for managing credit cards. GitHub actually calls theirs "sudo" mode, but I think that's too technical for big parts of our audience. I've gone with "high security mode".

This doesn't actually get exposed in the UI yet (and we don't have any meaningful auth factors to prompt the user for) but the workflow works overall. I'll go through it in a comment, since I need to arrange some screenshots.

Test Plan: See guided walkthrough.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4398

Differential Revision: https://secure.phabricator.com/D8851
2014-04-27 17:31:11 -07:00
lkassianik
9a827096a7 Ability to close poll
Summary: Fixes T3566 List of poll actions should include ability to close an open poll or reopen a closed poll.

Test Plan: Poll author should be able to close/reopen poll. Non-author should get policy screen when attempting to close/reopen poll.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T3566

Differential Revision: https://secure.phabricator.com/D8846
2014-04-24 12:02:56 -07:00
epriestley
2ac8457cb9 [Later] Drop the project profile table
Summary: Going to sit on this for a bit so we can fall back to it if needbe, but this table no longer has any reads or writes in the application.

Test Plan: Applied patch locally and poked around.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: aran

Differential Revision: https://secure.phabricator.com/D8190
2014-04-24 08:15:24 -07:00
epriestley
4bbd2d5203 Make an old migration a little more robust
Summary: See https://github.com/facebook/phabricator/pull/507

Test Plan: This is hard to test since the migration no longer runs against HEAD, but pull 507 strongly implies this is the correct fix.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8837
2014-04-23 14:22:02 -07:00
epriestley
1a3ac09975 Add "requestedObjectPHID" to ReleephRequest
Summary:
Ref T3551. Currently, ReleephRequests don't have a direct concept of the //object// being requested. You can request `D123`, but that is just a convenient way to write `rXyyyy`.

When the UI wants to display information about a revision, it deduces it by examining the commit.

This is primarily an attack on T3551, so we don't need to load <commit -> edge -> revision> (in an ad-hoc way) to get revisions. Instead, when you request a revision we keep track of it and can load it directly later.

Later, this will let us do more things: for example, if you request a branch, we can automatically update the commits (as GitHub does), etc. (Repository branches will need PHIDs first, of course.)

This adds and populates the column but doesn't use it yet. The second part of the migration could safely be run while Phabricator is up, although even for Facebook this table is probably quite small.

Test Plan:
  - Ran migration.
  - Verified existing requests associated sensibly.
  - Created a new commit request.
  - Created a new revision request.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3551

Differential Revision: https://secure.phabricator.com/D8822
2014-04-20 11:55:18 -07:00
epriestley
4918773afe Drop nonsense buildStatus field from Buildable
Summary:
Ref T4809. Buildables currently have buildStatus and buildableStatus. Neither are used, and no one knows why we have two.

I'm going to use buildableStatus shortly, but buildStatus is meaningless; burn it.

Test Plan: `grep`, examined similar get/set calls, created a new buildable, ran storage upgrade.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8796
2014-04-17 16:01:06 -07:00
epriestley
ab7d89edc8 Use better secrets in generating account tokens
Summary:
When we generate account tokens for CSRF keys and email verification, one of the inputs we use is the user's password hash. Users won't always have a password hash, so this is a weak input to key generation. This also couples CSRF weirdly with auth concerns.

Instead, give users a dedicated secret for use in token generation which is used only for this purpose.

Test Plan:
  - Ran upgrade scripts.
  - Verified all users got new secrets.
  - Created a new user.
  - Verified they got a secret.
  - Submitted CSRF'd forms, they worked.
  - Adjusted the CSRF token and submitted CSRF'd forms, verified they don't work.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8748
2014-04-10 11:45:10 -07:00
epriestley
582ec54465 Add a checkbox to the LDAP auth configuration UI to "Always Search"
Summary: Fixes T3208. This forces us to bind+search even if there are no anonymous credentials.

Test Plan: Checked the box, saved the form. Unchecked the box, saved the form. LDAP??

Reviewers: Firehed

Reviewed By: Firehed

Subscribers: epriestley

Maniphest Tasks: T3208

Differential Revision: https://secure.phabricator.com/D8723
2014-04-08 11:36:23 -07:00
epriestley
847b7977c1 Add semi-generic rate limiting infrastructure
Summary:
This adds a system which basically keeps a record of recent actions, who took them, and how many "points" they were worth, like:

  epriestley email.add 1 1233989813
  epriestley email.add 1 1234298239
  epriestley email.add 1 1238293981

We can use this to rate-limit actions by examining how many actions the user has taken in the past hour (i.e., their total score) and comparing that to an allowed limit.

One major thing I want to use this for is to limit the amount of error email we'll send to an email address. A big concern I have with sending more error email is that we'll end up in loops. We have some protections against this in headers already, but hard-limiting the system so it won't send more than a few errors to a particular address per hour should provide a reasonable secondary layer of protection.

This use case (where the "actor" needs to be an email address) is why the table uses strings + hashes instead of PHIDs. For external users, it might be appropriate to rate limit by cookies or IPs, too.

To prove it works, I rate limited adding email addresses. This is a very, very low-risk security thing where a user with an account can enumerate addresses (by checking if they get an error) and sort of spam/annoy people (by adding their address over and over again). Limiting them to 6 actions / hour should satisfy all real users while preventing these behaviors.

Test Plan:
This dialog is uggos but I'll fix that in a sec:

{F137406}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8683
2014-04-03 11:22:38 -07:00
epriestley
f239d81c1c Allow very long notes on flags
Summary: Fixes T4703. This is a VARCHAR(255) for no particular reason.

Test Plan: {F136160}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4703

Differential Revision: https://secure.phabricator.com/D8652
2014-03-30 19:51:46 -07:00
epriestley
d3dbbec88d Rename Releeph "Project" transactions to "Product"
Summary: Ref T3549. This table isn't written to yet; rename it and the DAOs and modernize the history controller.

Test Plan: Viewed history page for a product.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3549

Differential Revision: https://secure.phabricator.com/D8633
2014-03-29 09:15:09 -07:00
Bob Trahan
655ac9927f Workboards - add column detail page
Summary: followup to D8544. This ends up creating an editor + transactions to get the job done.

Test Plan: made a column - saw a nice created transaction. edited the name - saw a nice name edit. deleted the column - saw a deleted transaction, updated "deleted" ui, and hte action change to activate. "Activated" the column and saw a transaction and updated UI. Tried to delete a column with tasks in it and got an error.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8620
2014-03-26 14:40:47 -07:00
epriestley
543a313387 Skip a very old project reindex migration
Summary:
During migration of very old installs, this script no longer runs properly since at HEAD it can't index against older schemas.

Since it's pretty fluff, just toss it. Installs can run `bin/search index --type PROJ` after finishing migrations to achieve the same effect, if necessary.

Test Plan: eyeballed it

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8619
2014-03-26 13:51:49 -07:00
epriestley
a5f55d506f Provide a real object ("PhabricatorRepositoryPushEvent") to represent an entire push transaction
Summary:
Ref T4677. Currently, we record individual actions in a push as PhabricatorRepositoryPushLogs, but tie them together only loosely with a `transactionKey`.

Provide a real PushEvent object, and move some of the denormalized fields to it. This primarily just gives us more robust infrastructure for building, e.g., email about pushes, for T4677, since we can act on real PHIDs rather than passing awkward identifiers around.

Test Plan:
  - Performed migration.
  - Looked at database for consistency.
  - Browsed/queried push logs.
  - Pushed a bunch of stuff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4677

Differential Revision: https://secure.phabricator.com/D8615
2014-03-26 13:51:06 -07:00
epriestley
d6b937ca27 Allow external systems to send messages to build targets
Summary:
Ref T1049. Allows external systems to send a message to a build target. The primary intended use case is:

  - You make an HTTP request to Jenkins.
  - The build goes into a "waiting" state.
  - Later, Jenkins calls `harbormaster.sendmessage` to report that the target passed or failed.
  - The build continues as appropriate.

This is deceptively complicated because:

  - There are a lot of race concerns. We might get a message back from an external system before it even responds to the request we made. We want to make sure we process these messages no matter when we receive them.
  - These messages need to be sent to a build target (vs a build or buildable) because we'll get into trouble with parallelization later on otherwise (Jenkins is told to do 3 builds; we can't tell which ones failed or what overall state is unless the message are sent to targets).
  - I initially thought about implementing this as a separate "Wait for a response from an external system" build step. This gets a lot more complicated for users once we do parallelization, though. Particularly, in the case where you've told Jenkins to do 3 builds, the three "wait" steps need to know which target they're waiting for (and jenkins needs to know some unique identifier for each target). So this pretty much boils down to a more complicated, more error-prone version of using target PHIDs.

This makes the already-muddy Build UI a bit worse, but it needs a general clarity pass anyway (it's showing way too much uninteresting data, and should show a better summary of results instead).

Test Plan:
  - This doesn't really do anything interesting yet.
  - Used Conduit to send messages to build plans.
  - Viewed the messages on the build screen.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8604
2014-03-25 16:11:28 -07:00
epriestley
cec8d10731 Rename concrete Harbormaster step implementations
Summary: Ref T1049. For consistency, rename these to "Harbormaster...".

Test Plan: Ran migration, ran builds, everything still works fine.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8602
2014-03-25 16:09:51 -07:00
epriestley
a246c85c6b Use ApplicationTransactions and CustomField to implement build steps
Summary:
Ref T1049. Fixes T4602. Moves all the funky field stuff to CustomField. Uses ApplicationTransactions to apply and record edits.

This makes "artifact" fields a little less nice (but still perfectly usable). With D8599, I think they're reasonable overall. We can improve this in the future.

All other field types are better (e.g., fixes weird bugs with "bool", fixes lots of weird behavior around required fields), and this gives us access to many new field types.

Test Plan:
Made a bunch of step edits. Here's an example:

{F133694}

Note that:

  - "Required" fields work correctly.
  - the transaction record is shown at the bottom of the page.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4602, T1049

Differential Revision: https://secure.phabricator.com/D8600
2014-03-25 16:08:40 -07:00
epriestley
0a76d82a7c Use string constants, not integer constants, to represent task status internally
Summary:
Ref T1812. I think integer constants are going to be confusing and error prone for users to interact with. For example, because we use 0-5, adding a second "open" status like "needs verification" without disrupting the existing statuses would require users to define a status with, e.g., constant `6`, but order it between constants `0` and `1`. And if they later remove statuses, they need to avoid reusing existing constants.

Instead, use more manageable string constants like "open", "resolved", etc.

We must migrate three tables:

  - The task table itself, to update task status.
  - The transaction table, to update historic status changes.
  - The saved query table, to update saved queries which specify status sets.

Test Plan:
  - Saved a query with complicated status filters.
  - Ran migrations.
  - Looked at the query, at existing tasks, and at task transactions.
  - Forced migrations to run again to verify idempotentcy/safety.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1812

Differential Revision: https://secure.phabricator.com/D8583
2014-03-25 13:58:14 -07:00
Bob Trahan
809e5a0389 Workboards - let users delete columns
Summary: Fixes T4408. I had to add a "status" to colum. I think we'll need this once we get fancier anyway but for now we have "active" and deleted.

Test Plan: deleted a column. noted reloaded workboard with all those tasks back in the default colun. loaded a task and saw the initial transaction had a "Disabled" icon next to the deleted workboard. also saw the new transaction back to the default column worked.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4408

Differential Revision: https://secure.phabricator.com/D8544
2014-03-18 10:40:31 -07:00
epriestley
5b2887b69b Add "Date Updated" query fields for Maniphest
Summary:
Fixes T4637.

  - We already allow you to order by this column but don't have a key on it. Add one.
  - Expose UI for querying on ranges.

Test Plan:
  - Ran some queries, got reasonable-looking results and no table scans.

Reviewers: btrahan, bigo

Reviewed By: bigo

Subscribers: bigo, epriestley

Maniphest Tasks: T4637

Differential Revision: https://secure.phabricator.com/D8557
2014-03-17 15:53:07 -07:00
epriestley
be92bf182f Drop Maniphest legacy transaction table
Summary: Ref T2217. I'll hold this for a month or so, but once we're confident the migration didn't ruin anything we should nuke this old data -- it's just an insurance policy against discovering migration issues.

Test Plan: Will run in a month or so.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7104
2014-03-12 06:04:45 -07:00
epriestley
2dbfb1d5fb Remove DifferentialComment
Summary: Ref T2222. Remove this; no more callsites.

Test Plan: `grep`

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8477
2014-03-11 13:02:33 -07:00
epriestley
03abde0b25 Fix Diviner links to articles by title
Summary:
Ref T988. This fixes the biggest current problem with Diviner, which is dead links to articles.

In the new Diviner, articles can have both a "name" (derived from the file name, and used in the URI) and a "title" (optional, specified explicitly). For example, we have one document with the name "feedback" and the title "Give Feedback! Get Support!".

On disk, we want to use the name for the actual file where the text lives ("feedback.diviner"). We also want to use the name in the URI, to generate a clean URI and to allow us to retitle the document slightly without breaking links to it (for example, we renamed the "Backup" document to "Backups and Migrations").

However, when displaying the article we want to use the title.

Currently, you can //only// link to the name, not the title. This is inconvenient:

  - We have a bunch of existing docs which link to titles.
  - It's natural/intuitive to link to titles.
  - Linking to titles makes it easier/cheaper to generate documentation, because we don't need to be able to resolve things at render time.

To remedy this, allow links to target either names or titles. If we miss on a name query, we'll do a title query. This is implemented with a slug hash to allow approximately correct titles (wrong case/spacing/punctuation, e.g.) and sidestep all the UTF8/column length issues.

(In the long run, atom resolution should theoretically be more sophistiated than it is now, and we should do render-time lookups on at least some documents to catch bad links. However, this is fairly complicated and a relatively advanced feature, and I think allowing links to titles is desirable no matter what.)

Test Plan: The user documentation book now has valid links to articles when the titles and names differ.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T988

Differential Revision: https://secure.phabricator.com/D8407
2014-03-05 12:07:26 -08:00
epriestley
bca0ad8fdd Make "EditPro" controller work with diff updates
Summary:
Ref T2222. Make the "EditPro" controller accommodate diff updates, and support the transaction type. This one is pretty straightforward.

Also make `revisionPHID` in the comments table nullable to fix the "Edit" action.

Test Plan:
  - Created new revision.
  - Updated revision.
  - Tried to do some invalid stuff.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8376
2014-02-28 16:49:22 -08:00
epriestley
cd7171ec6e Migrate old AuxiliaryField storage to modern CustomField storage
Summary: Ref T2222. Ref T3886. Differential has a legacy storage table for auxiliary fields; move the data to modern storage.

Test Plan:
  - Ran migration.
  - Verified fields still worked properly afterward (view, edit, etc).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3886, T2222

Differential Revision: https://secure.phabricator.com/D8355
2014-02-26 16:52:30 -08:00
Joshua Spence
6270114767 Various linter fixes.
Summary:
- Removed trailing newlines.
- Added newline at EOF.
- Removed leading newlines.
- Trimmed trailing whitespace.
- Spelling fix.
- Added newline at EOF

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: hach-que, chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8344
2014-02-26 12:44:58 -08:00
epriestley
0f0673b9e5 Remove "dateCommitted" field from DifferentialRevision
Summary: Ref T2222. This is obsolete and no longer used. We could deduce it from transactions or commits in modern Phabricator if we wanted it. We may implement a more general mechanism for T4434.

Test Plan: `grep`

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8330
2014-02-25 12:36:14 -08:00
epriestley
70b008d18d Add test coverage that our definition of BMP agrees with MySQL
Summary:
Ref T1191. Test that MySQL's rules match those of `phutil_is_utf8_with_only_bmp_characters()`:

  - Build a string with //every// character that we consider to be a BMP character.
  - Write it into MySQL.
  - Read it back out.
  - Make sure MySQL didn't truncate it.

Test Plan: Ran unit test. This test runs pretty quickly (50ms), the string with every character isn't all that enormous.

Reviewers: btrahan, arice

Reviewed By: arice

CC: chad, arice, aran

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D8314
2014-02-23 16:20:38 -08:00
Bob Trahan
dcd7a316d2 Differential - add DifferentialDraft to track whether revisions have draft feedback or not
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
2014-02-18 16:25:16 -08:00
epriestley
b96ab5aadf Modernize VCS password storage to use shared hash infrastructure
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
2014-02-18 14:09:36 -08:00
epriestley
5778627e41 Provide more storage space for password hashes and migrate existing hashes to "md5:"
Summary: Ref T4443. Provide more space; remove the hack-glue.

Test Plan: Logged out, logged in, inspected database.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4443

Differential Revision: https://secure.phabricator.com/D8269
2014-02-18 14:09:36 -08:00