Summary: Fixes T12682.
Test Plan: Ran `bin/storage upgrade --dryrun` repeatedly with un-applied patches, saw it not apply them and not mark them applied.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12682
Differential Revision: https://secure.phabricator.com/D17837
Summary: This was mis-tested by only using one account, which could always see the image. External transaction moved file attachment to the modular transaction for file and audio instead.
Test Plan: Test adding audio and a macro on a pleb account, visit with normal account and see macro fine.
Reviewers: epriestley, amckinley
Reviewed By: amckinley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17836
Summary: Not sure when these stopped, also fixed mailtag contants.
Test Plan: Close an initiative, see story, fund initiative, see story.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17835
Summary: Fixes T12627. Updates FundInitiative and FundBacker with modular transactions.
Test Plan: Create an Initiative, back it with fake monies, close initiative, reopen, edit various fields.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12627
Differential Revision: https://secure.phabricator.com/D17782
Summary:
Fixes T12623. Adds new modular transactions to Slowvote. Also converts
the `shuffle` column to `bool` for consistency with other boolean-ish columns.
Test Plan:
Create a new vote, modified everything that could be modified from the web UI,
observed expected timeline.
Example timeline: {F4938843}
Example transaction values in DB: {F4938850}
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T12623
Differential Revision: https://secure.phabricator.com/D17830
Summary:
We are submitting `epriestley (Evan Priestley) <noreply@meta.phacility.com>`, but should be submitting `"epriestley (Evan Priestley)" <noreply@meta.phacility.com>`.
Add the missing quotes.
Test Plan: Locally, this makes the API calls work against the Mailgun sandbox domain.
Reviewers: chad, amckinley
Reviewed By: chad, amckinley
Differential Revision: https://secure.phabricator.com/D17831
Summary: Updates the Spaces application for modular transactions, seemed easy to bang out.
Test Plan: Create a space, edit a space, archive a space. Verify default space works as intended.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17829
Summary: In remarkup lists, it can be hard to clearly see which items still need to be completed. This makes completed items a little lighter for clarity.
Test Plan:
Review a long list with checked and unchecked items in a task.
{F4938611}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17828
Summary: Update Legalpad for modular transactions
Test Plan:
- New Document (no sign)
- New Document (individual)
- New Document (corp)
- Require Signature - get prompted to sign before I can do anything.
- Edit Documents
- Sign Documents
- Comment on Documents
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17826
Summary: Updates Passphrase for modular transactions.
Test Plan: Create, edit, lock, view, lots of different types of Passphrases. Enable Conduit, Lock Passphrases, Destroy Secrets from the interface and verify from the DB it was eradicated.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17824
Summary: Still needs some cleanup, but ready for review in broad outline form.
Test Plan:
Made lots of policy changes to the Badges application and confirmed expected rows in `application_xactions`, confirmed expected changes to `phabricator.application-settings`.
See example output (not quite working for custom policy objects) here:
{F4922240}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, chad, epriestley
Maniphest Tasks: T11476
Differential Revision: https://secure.phabricator.com/D17757
Summary: Moves over to transaction commenting.
Test Plan: Leave a comment (tested with TYPE_COMMENT still present).
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17823
Summary: See D17812, etc. We can figure this out by looking at the object carefully. We don't need to go delete all the old TYPE_COMMENT (it doesn't hurt anything) but can nuke it when we see it.
Test Plan:
- Made a comment in Slowvote (supports commenting).
- Viewed an Almanac device (does not support commenting).
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17822
Summary: Just a small touch up to move this to edit engine.
Test Plan:
- Create a question
- Edit a question
- Close question
- Test NUX state
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17812
Summary: Updates PhamePost for modular transactions.
Test Plan:
- Create a post
- Edit a post
- Add a header image
- Delete header image
- Award Token
- Leave comment
- Unpublish post
- Check History page
- Move post
- Archive post
{F4936456}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17818
Summary: Updates Macro to use EditEngine. Also removes "URL" field for adding a Macro, which I think it's worth pursuing.
Test Plan:
- Create a Macro
- Forget to name it
- Try a PDF
- Use a Macro
- Edit a macro (not working)
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17821
Summary: Moves PhameBlog over to the wonderful world of modular transactions and the riches that lay beyond...
Test Plan:
- Create Blog
- Edit Blog
- Set Header
- Delete Header
- Add picture
- Archive blog
- Set incorrect domain values
- Be irresponsible with subtitle length
- Activate blog
- Change description
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17815
Summary:
Fixes T12661.
When changing the start date of an event from some time in the past to some time significantly in the future (more than 24 hours), we'd invalidate only future caches and leave users in an "away" state. Instead, just invalidate all past and future caches (this is simpler than trying to figure out a narrower window, and should not make us do too much extra work).
When uninviting users from events, their caches also didn't get cleared correctly. Instead, clear them.
Test Plan:
- Changed an event from "Apr 1 - June 1" to "May 15 - June 1", saw availablity clear properly.
- Uninvited user `@dog` from an ongoing event, saw availability clear properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12661
Differential Revision: https://secure.phabricator.com/D17809
Summary: T12656, mark these methods as frozen and use conpherence.edit instead.
Test Plan: Visit conduit, check status is displayed.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17808
Summary: Fixes T12659. Previously this would lead to an error when trying to run `arc diff` on a revision that had a milestone as a reviewer (or any non-octothorpe'd Object Name)
Test Plan: Followed repro steps in T12659 and didn't get the error described. Also clicked around and didn't notice any obvious regressions in projects or differential
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, yelirekim
Maniphest Tasks: T12659
Differential Revision: https://secure.phabricator.com/D17807
Summary:
Fixes T12642. Currently, writing "Fixes T..." in a comment gets picked up as a formal "fixes".
This is a bit confusing, and can also give you a "no effect" error if you "fixes ..." a task which is already "fixes"'d.
We could make the duplicate action a non-error, but just prevent the text from having an effect instead, which seems cleaner.
Test Plan:
- Wrote "Fixes ..." in a summary, saw a "fixes" relationship established.
- Wrote "Fixes ..." in a comment, got a "mention" instead.
- `var_dump()`'d some stuff as a sanity check, looked reasonable.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12642
Differential Revision: https://secure.phabricator.com/D17805
Summary: Builds a basic room generator for Conpherence, picks a random name, adds 10 random users to it, sets view and edit policy to all users.
Test Plan:
`bin/lipsum generate conpherence`
{F4928815}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17699
Summary: Moves Conpherence to use EditEngine. This removes the "First Message" field, but I think that's ok until we have direct messaging of some sort, then maybe have built-ins cover that case.
Test Plan:
- Visit /new/ and /edit/ for creating new rooms.
- Edit a room in full conpherence
- Edit a room in durable column
- grep for METADATA calls
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11729
Differential Revision: https://secure.phabricator.com/D16677
Summary:
- Change column type from `sort128` to `sort`.
- Remove `originalName`. This column is unused. Long ago, we used it to generate a `Thread-Topic` header for mail, but just use PHIDs now (the value just needs to be stable for a given object, users normally don't see it).
Test Plan:
- Created a package with a beautifully long name. Magnificent!
- Grepped for `originalName` / `getOriginalName()`, found no Owners hits.
- Verified that there isn't any name-length validation code to remove.
{F4925637}
Reviewers: chad, amckinley
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17798
Summary:
Ref T12646.
- Use "wb1" instead of "wb" to use level 1 gzip compression (faster, less compressy). Locally, this went about 2x faster and the output only grew 4% larger.
- LinesOfALargeExecFuture does a lot of unnecessary string operations, and can boil down to a busy wait. The process is pretty saturated by I/O so this isn't the end of the world, but just use raw ExecFuture with FutureIterator so that we wait in `select()`.
- Also, nice the process to +19 so we try to give other things CPU.
Test Plan:
- Ran `bin/storage dump --compress --output ...`.
- Saw CPU time for my local database drop from ~240s to ~90s, with a 4% larger output. Most of this was adding the `1`, but the ExecFuture thing helped a little, too.
- I'm not sure what a great way to test `nice` in a local environment is and it's system dependent anyway, but nothing got worse / blew up.
- Used `gzcat | head` and `gzcat | tail` on the result to sanity-check that everything was preserved.
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12646
Differential Revision: https://secure.phabricator.com/D17795
Summary:
Send-on-enter and autocomplete both listen for "return" keypresses, and could race. Have autocomplete let other handlers take a shot at the action before it does.
Also, fix a case where ":)" and the suffix list (which lets you type `someone is 100% to blame here (@epriestley)` and get the results you want) interacted badly, so ":)" cancels the autocompleter like ":3" does.
Test Plan:
- Typed "@xxx" and mashed return real fast over and over again while reloading the page. Before: sometimes handlers raced and text submitted. After: always handled by autocomplete behavior.
- Typed ":", ")", "<return>", sent an emoticon (previously: no).
Reviewers: chad, amckinley
Reviewed By: chad
Subscribers: xxx
Differential Revision: https://secure.phabricator.com/D17794
Summary: Ref T12600. Basically all the property (not path) information on a hovercard for owner packages.
Test Plan:
Create a package with LOTS OF RULES. Test it as open and archived states.
{F4923441}
{F4923444}
Reviewers: epriestley, jmeador
Reviewed By: jmeador
Subscribers: jmeador, Korvin
Maniphest Tasks: T12600
Differential Revision: https://secure.phabricator.com/D17793
Summary: When a notice is in a table view in a two column layout, reset the margins.
Test Plan: Visit OwnerDetails with no paths set.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17792
Summary: I can't believe I've been staring at this page for so long without noticing this typo
Test Plan: doitlive
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D17791
Summary: Ref T12600. This makes the background solid when in hovercard context.
Test Plan: Close a task, view hovercard, see solid background
Reviewers: epriestley, amckinley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12600
Differential Revision: https://secure.phabricator.com/D17788
Summary:
Ref T12613. Currently, the SVNTEST and HGTEST repositories are improperly configured on `secure`. These repositories use VCS systems which do not support synchronization, so they can not be served from cluster services with multiple hosts.
However, I've incorrectly configured them the same way as all the Git repositories, which support synchronization. This causes about 50% of requests to randomly fail (when they reach the wrong host).
Detect this issue and warn the user that the configuration is not valid.
It should be exceptionally difficult for normal installs to run into this.
Test Plan:
- Mostly faked these conditions locally, verified that `secure` really has this configuration.
- I'll push this, verify that the issue is detected correctly in production, then fix the config which should resolve the intermittent issues with SVNTEST.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12613
Differential Revision: https://secure.phabricator.com/D17774
Summary:
Depends on D17785. Fixes T12635. There was a bug where users could verify their primary email without getting the "isEmailVerified" flag set on their accounts.
D17785 fixes this bug. This change migrates affected account to fix their state, now that they can't get in trouble any more (hopefully).
Test Plan:
- Explicitly removed this flag from a bunch of accounts.
- Ran migration, saw the accounts get fixed.
- Ran migration again (`storage upgrade --apply ...`), saw the accounts not get touched.
- We have 117 affected accounts on `secure`, so I'll verify that this fixes them.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12635
Differential Revision: https://secure.phabricator.com/D17786
Summary:
Ref T12635. See that task for discussion.
You can currently end up with a verified primary address but no "verified" flag on your account through an unusual sequence of address mutations.
Test Plan:
- Registered without verifying, using address "A".
- Added a second email address, address "B".
- Verified B (most easily with `bin/auth verify`).
- Changed my primary email to B.
- Before patch: account not verified.
- After patch: account verified.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12635
Differential Revision: https://secure.phabricator.com/D17785
Summary:
Fixes T10954. This got hidden underneath things at some point.
Use `pointer-events: none` to make the mouse ignore the element so that hover/select/edit/click still work "through" the element.
Design could probably be improved here, maybe I'll make it more-visible when you press {key n} and then have it fade quickly so it kind of gets out of your way once you find the block you want to read.
Test Plan: {F4921746}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10954
Differential Revision: https://secure.phabricator.com/D17784
Summary: Swaps out hovercard boring view for super cool workboard card view. Will have more diffs to add additional information down the road.
Test Plan: {F4921092}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17769
Summary:
Ref T12611. Currently, the HTTP/SSH logs don't have an option to include the instance name.
Add such an option.
Leave it out of the default logs because most installs don't use this.
Test Plan: See next changes.
Reviewers: chad, amckinley
Reviewed By: chad
Maniphest Tasks: T12611
Differential Revision: https://secure.phabricator.com/D17776
Summary:
The quickstart SQL hasn't been regenrated in a while, mildly impacting unit test and instance startup times.
- Use `bin/storage quickstart` to regenerate quickstart.
- Manually set the FULLTEXT tables back to `MyISAM` until we deal with T11741.
Test Plan:
- Saw database setup drop from ~10,500ms to ~7,500ms locally.
- Visually inspected diff, changes looked expected.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17775
Summary: Fixes T12614.
Test Plan:
- Ran tests, saw them pass.
- Changed a thumbnail to 800x800, saw tests detect that the default image was missing.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12614
Differential Revision: https://secure.phabricator.com/D17773
Summary:
Fixes T12628. After later changes to `PhabricatorFile`, this migration no longer runs if you upgrade through it to a recent `HEAD` while your data has some room images.
Since this isn't critical and has been available for ~6 months, I just nuked it as a first pass. I can find a more careful approach which lets us continue to run this migration instead if you're hesitant to skip this step, although it may be a little involved.
In 95% of cases we avoid this by updating the storage table as it existed at the time the migraiton ran, but Files are much too complicated for that to be realistic.
Test Plan: Ran `bin/storage upgrade -f --apply phabricator:20161005.conpherence.image.2.php`, saw it do nothing.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12628
Differential Revision: https://secure.phabricator.com/D17770
Summary: Ref T12622.
Test Plan: As a logged-out and logged-in user, loaded Conpherence threads.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12622
Differential Revision: https://secure.phabricator.com/D17768
Summary: Fixes T12622. This variable is mis-named.
Test Plan: Visit a room I have not joined.
Reviewers: epriestley, amckinley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12622
Differential Revision: https://secure.phabricator.com/D17767
Summary:
Ref T12612. This updates the rate limiting code to:
- Support a customizable token, like the client's X-Forwarded-For address, rather than always using `REMOTE_ADDR`.
- Support APCu.
- Report a little more rate limiting information.
- Not reference nonexistent documentation (removed in D16403).
I'm planning to put this into production on `secure` for now and then we can deploy it more broadly if things work well.
Test Plan:
- Enabled it locally, used `ab -n 100` to hit the limit, saw the limit enforced.
- Waited a while, was allowed to browse again.
Reviewers: chad, amckinley
Reviewed By: amckinley
Maniphest Tasks: T12612
Differential Revision: https://secure.phabricator.com/D17758
Summary: Fixes T12619.
Test Plan: Faked `return array()` in ConpherneceThreadQuery, got a NUX instead of fatal.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12619
Differential Revision: https://secure.phabricator.com/D17764
