Summary:
In preparation for adding another search engine (see T355):
- Rename "executor" to "engine".
- Move all engine-specific operations into the engine. Specifically, this
means that indexing moves out of the document store and into the engine (it was
sort of silly where it was before).
- Split choice of an engine into an overridable "selector" class, a base API,
and a concrete MySQL implementation (just like storage engine selection).
- Make all callers go through the indirection layer.
The default selector just unconditionally selects the MySQL engine, but now
(with D786) I can build an Elastic Search engine and you guys can build a
multi-target engine if you want and I don't get there fast enough.
Test Plan:
- Created a new document (task).
- Searched for and found it.
- Viewed index reconstruction.
Reviewed By: jungejason
Reviewers: jungejason, amckinley, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 788
Summary:
This is a very small step toward building a Status and possibly an Oncall tool.
Build a calendar view which renders months.
Much of my hesitance to bang these tools out is that dealing with
dates/calendaring is basically horrible, so I'm trying to ease into it.
This calendar is locale-aware and all that jazz.
Test Plan:
- See:
https://secure.phabricator.com/file/view/PHID-FILE-c07a9c663a7d040d2529/
- Verified that months have the right number of days, today is the right day
of the week, months begin on the day after previous months end on, etc.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
Commenters: cwbeck, jungejason
CC: blair, aran, epriestley, cwbeck, jungejason
Differential Revision: 791
Summary:
Provide a {T123} syntax which pulls in the entire name of an object, not just a
link to it. A major use for this is organizing projects using wiki pages. Since
handle links show object status now, this lets you organize stuff in an ad-hoc
way and get a reasonable overview of it. We can make handles richer in the
future, too.
The performance on this isn't perfect (it adds some potential single gets) but I
think it's okay for now and I don't want to make remarkup engine even more
complex until the preprocess/postprocess stuff has had a chance to settle and
I'm more confident it works.
In Differential and Maniphest we'll also incorrectly cache the object
state/name, but that'll fix itself once I move the cache code to use
preprocess/postprocess correctly.
Test Plan:
- See https://secure.phabricator.com/file/view/PHID-FILE-5f9ca32407bec20899b9/
for an example.
- Generated and looked over the documentation.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, hunterbridges
CC: skrul, aran, jungejason, epriestley
Differential Revision: 784
Summary:
This accomplishes two goals:
- Data fetching is now grouped across blocks.
- Demonstrates that D737 actually works.
Test Plan: Used @mentions in Phriction preview, they rendered properly. Verified
only one service call was being made across blocks.
Reviewed By: jungejason
Reviewers: hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 738
Summary:
Right now, the "SimpleEmailService" class uses trigger_error() to communicate
error messages. This means they get lost in the error logs and aren't visible in
the MetaMTA interface.
Provide a flag to strengthen them into exceptions, instead.
(I've attempted to emulate the prevailing style so I can offer this upstream.)
Test Plan: Faked an error condition and got a detailed stack trace in MetaMTA
instead of an empty "Message" field.
Reviewed By: jungejason
Reviewers: hunterbridges, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 783
Summary:
Add a new "Create Subtask" action that allows you to quickly split a task into
dependent subtasks, using the parent task as a template.
Followups:
- Cause "workflow=<parent>" to change "Create Another Task" into "Create
Another Subtask" (after D736).
- Show parent tasks (requires some schema stuff).
Test Plan:
- Created a new task.
- Created a new subtask.
Reviewed By: codeblock
Reviewers: hunterbridges, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, codeblock
Differential Revision: 774
Summary:
We use ObjectHandles as proxy objects which can refer to any other object in the
system. Add the concept of the underlying object's "status" (e.g., open, closed
or busy).
This allows us to render completed tasks and revisions with strikethrough. In
the future, if we implement OOO or something, we could render users with a
"busy" status if they're on vacation, etc.
Test Plan: Viewed a task with closed revisions and dependencies:
https://secure.phabricator.com/file/view/PHID-FILE-6183e81286fa3288d33d/
Reviewed By: codeblock
Reviewers: codeblock, hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, codeblock
Differential Revision: 772
Summary:
Not totally sure I'm in love with this but I think it's somewhat non-terrible,
despite the lack of lens flare.
Also made "Cancel" take you back to the task if you got to "Create" from "Create
Another Task".
Test Plan:
- Style:
https://secure.phabricator.com/file/view/PHID-FILE-ad37d3c1f3b2c7a7a7d1/
- Hit "Cancel" from "Create Another", got sent back to task.
- Hit "Cancel" from normal create, got sent back to list.
- Tried to save an invalid task after making changes to CC/Projects, changes
were preserved.
Reviewed By: codeblock
Reviewers: hunterbridges, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock
Differential Revision: 736
Summary: Implements an S3 storage engine option for Phabricator.
Test Plan:
- Uploaded files to S3.
- Looked at them.
- Verified they appeared in S3 using the S3 file browser.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 752
Summary:
This allows you to edit dependencies. It is a better patch than it used to be.
It depends on D725.
- If you create a cycle, it just throws an exception and aborts the workflow.
It should not do this.
- Tasks which depend on the current task aren't shown in the UI. Need to add a
new table for this.
- Transaction text says "attached Task" but should probably say "added a
dependency on task".
Test Plan: Created valid and invalid dependencies between tasks. Created valid
and invalid dependencies between revisions.
Reviewed By: tuomaspelkonen
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
Commenters: codeblock
CC: aran, codeblock, tuomaspelkonen, epriestley
Differential Revision: 595
Test Plan: Add fields to config based on specification on T335. View on Task
Edit and Task Detail. Supported types are string, int and select
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley, hunterbridges
Differential Revision: 753
Summary: See T382. I think bill also asked about this (T308); there's no way to
figure it out right now.
Test Plan:
- Hit "?", verified the dialog now reveals that escape closes it.
- Hit escape, dialog vanished.
Reviewed By: jungejason
Reviewers: fzamore, bill, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 763
Summary:
@skrul reports receiving multiple copies of notification emails since
@hunterbridges configured some bizarre dystopian email replication factory on
their outbound route. Two fixes:
- Ensure "To" and "Cc" are unique. Email shouldn't be replicated for "To:
x@y.com, x@y.com" but it's silly that we do this.
- Remove "To" addresses from "Cc". Email shouldn't be replicated here either,
but we don't really lose anything by accommodating this.
Test Plan:
Sent a mail to the same to/cc, verified I was to'd only and not cc'd when the
mail was delivered.
@hunterbridges, can you apply this patch locally and verify it fixes the issue?
You can test by going to MetaMTA -> Send New Message and sending a message to
yourself as both To and CC.
Reviewed By: skrul
Reviewers: skrul, hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, skrul, epriestley
Differential Revision: 751
Summary: See T389. If you're looking at tasks in project X, default new tasks
into project X.
Test Plan:
- Created a new task without associated projects.
- Created a new task from a filter view with 1 and 2 projects; defaults were
filled in.
- Submitted task with invalid title, projects were correctly preserved.
Reviewed By: skrul
Reviewers: skrul, hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, skrul, epriestley
Differential Revision: 757
Summary: Try to break this apart a little better in preparation for D595. No
functional changes, just refactored the relatively large processRequest()
method.
Test Plan:
- Attached and detached revisions from tasks.
- Attached and detached tasks from revisions.
- Merged tasks.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 725
Summary:
This gets all the major pieces working. Allows you to drag-and-drop files in
Differential and Phriction, and embed files in remarkup with {Fxxx} references.
See also task.
I'm explicitly not documenting this yet since it's still pretty rough.
Test Plan: Dragged and dropped stuff into Differential and Phriction.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, tomo
Commenters: tomo
CC: aran, tomo, jungejason
Differential Revision: 674
Summary:
See T372. Always render commit messages on one display line, so the table
doesn't jump around as they AJAX in on browse views.
The goal here is to have the cell choose a size naturally and for its content to
render with "overflow: hidden" if the natural size isn't large enough to contain
the content. "white-space: pre" or "white-space: nowrap" would prevent wrapping
but potentially make the table exceed the display width when a better behavior
is to hide some of the commit message.
Also use utf8-aware shortening, now that we have a function for it.
Casting a wide net in case anyone has a better way to do the CSS here. It's kind
of nasty that we have to use so many DOM nodes.
Test Plan:
- Resized window while viewing browse and history views in Safari, Chrome and
Firefox. Table exhibited described behavior.
- Verified summaries render sensibly and are properly truncated to 100
characters.
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen, tomo, mroch, cpojer
CC: aran, epriestley
Differential Revision: 750
Summary:
- Allow the console to handle abstract classes correctly.
- Move paste dictionary generation to an abstract base class.
- Add paste.create.
- Add 'objectName', 'parentPHID', and 'content' to Paste info dictionaries
(you can use filePHID with file.download to get the content but I think just
always sending it back is reasonable).
Test Plan:
- Use paste.create to create new pastes.
- Used paste.info to get existing pastes.
- Checked console UI to make sure "paste." didn't show up or anything
silly/dumb like that.
- Tried to call the method "paste" and got the right exception.
Reviewed By: codeblock
Reviewers: codeblock, jungejason, tuomaspelkonen, aran
CC: aran, codeblock
Differential Revision: 747
Summary:
- There's no way you can figure out the ID of a file right now. Expose that
more prominently.
- Put the drag-and-drop uploader on the main page so you don't have to click
through.
- Restore the basic uploader so IE users can theoretically use the suite I
guess? Added author info to basic uploader.
- Show author information in the table.
- Show date information in the table.
- Link file names.
- Rename table for filter views.
- When you upload one file, just jump to it. When you upload multiple files,
jump to your uploads and highlight them.
- Add an "arc download" hint.
Test Plan: Uploaded single files, groups of files, and files via simple
uploader.
Reviewers: codeblock, jungejason, tuomaspelkonen, aran
Commenters: codeblock
CC: aran, codeblock, epriestley
Differential Revision: 746
Summary: See T368. The current rendering result can cause some confusion for the
first/last chunks, make their behavior more explicit.
Test Plan:
- Clicked various "show more" links on a bunch of top/bottom/middle omitted
context blocks in a variety of diffs.
- Located a @generated shielded file and verified the initial render is
correct when the entire file is default-hidden.
Reviewed By: avitaloliver
Reviewers: avitaloliver, jungejason, tuomaspelkonen, aran
CC: aran, avitaloliver
Differential Revision: 744
Summary:
Need this for 'arc upload' and 'arc download'.
Given a file PHID or ID, provides information about it.
Test Plan:
- Implemented 'arc upload' and 'arc download' based on 'file.info'.
- Used Conduit API console to test method.
Reviewed By: codeblock
Reviewers: codeblock, jungejason, tuomaspelkonen, aran
CC: aran, codeblock, epriestley
Differential Revision: 741
Summary:
We just weren't handling these at all reasonably, must have dropped the logic
when they got ported.
This still isn't perfect: we have some display glitches around file names, so
the 'away' part renders as "This file was moved to .". I'll see if I can follow
up and fix that, but this resolves the more immediate issue of the interface
just not working at all.
Test Plan: Moved and copied files in my test repository, verified they rendered
somewhat correctly.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, jungejason, epriestley
Differential Revision: 699
Summary: create the page by getting data from the search result.
Test Plan:
load page with url /author/, /author/valid_username, and
/uathor/invalid_username, and verified that it works as expected.
Reviewed By: epriestley
Reviewers: epriestley, tuomaspelkonen
Commenters: tuomaspelkonen
CC: hwang, aran, tuomaspelkonen, epriestley, jungejason
Differential Revision: 723
Test Plan: Look at a task detail. Some dummy attributes are automatically added in ManiphestTaskDetailController.
Reviewed By: epriestley
Differential Revision: 730
Summary:
Python people don't seem to like the 'ignore-all' as default. Provide a way
to configure which file types should not use 'ignore-all'.
Test Plan:
Tested that it worked with bunch of Python of files and non-python
files. Cache was disabled during the test.
Reviewed By: jungejason
Reviewers: epriestley, jungejason
Commenters: epriestley
CC: aran, jungejason, epriestley
Differential Revision: 713
Summary:
I want clean this up enough that I can land D595 without making a complete mess,
here's a small simplification.
Move object load logic into PhabricatorObjectHandleData.
Test Plan: Attached tasks and revisions, merged tasks.
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran
Differential Revision: 724
Summary: See T266. Combine these interfaces into one and move it to settings.
Test Plan: Edited my profile and account.
Reviewers: codeblock, tcook, jungejason, tuomaspelkonen, aran
CC:
Differential Revision: 722
Summary: We omit a 'status' dropdown when creating a new task since it's silly
to create a non-open task and would just clutter the interface, but there's no
reason not to allow status to be edited on the "Edit" interface.
Test Plan: Created a new task (no status dropdown). Edited a task, including
changing the task status.
Reviewed By: jungejason
Reviewers: toulouse, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 720
interface
Summary:
See T58. When I originally wrote this interface you couldn't make a form that
looked like a form but wasn't really a form, so I made it a real form. That
meant you could hit "return" in the text input and submit the form, which didn't
do anything but is vaguely silly.
After D422 there's a layout-only form element, so switch to that.
Test Plan: Loaded the tab, hit return in the textarea, it didn't do a form
submit.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 719
Summary:
It makes more sense to just make this a settings panel rather than a standalone
app, particularly since setting panels are relatively well separated now.
Also default-disabled the SSH Keys interface since it won't currently be useful
for most installs.
Test Plan: Edited preferences.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, jungejason
Differential Revision: 716
Summary:
Add a new column to PhrictionContent called 'comment' or 'description' or
something
Add an optional field to the Phriction document editing interface that
allows you to add a comment
Test Plan:
Run the sql patch to modify phriction_content table
Create a new wiki page in Phriction
Type in words in description field and save the page
Visit this page and click "Edit Page" button
The content in the desciption field is saved
Reviewed By: epriestley
Reviewers: epriestley, hsb, codeblock
Commenters: codeblock
CC: aran, codeblock, hwang, epriestley
Differential Revision: 709
Summary:
With the sshd-vcs thing I hacked together, this will enable Phabricator to host
repositories without requiring users to have SSH accounts.
I also fixed "subporjects" and added an explicit ENGINE to it.
Test Plan: Created, edited and deleted public keys. Attempted to add the same
public key twice. Attempted to add invalid and unnamed public keys.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran, cadamo, codeblock
CC: aran, epriestley
Differential Revision: 711
Summary:
Currently, you can only edit your own affiliation to projects. Enable users to
be managed in a more reasonable batched way.
I'll lock this down to admins/owners and add a transaction log at some point.
Test Plan: Edited project affiliations. Verified Herald still works.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 677
Summary:
See T344. Currently, there's a hard-coded 12MB filesize limit and some awkward
interactions with MySQL's max_allowed_packet. Make this system generally more
robust:
- Move the upload limit to configuration.
- Add setup steps which reconcile max_allowed_packet vs MySQL file storage
limits.
- Add a layer of indirection between uploading files and storage engines.
- Allow the definition of new storage engines.
- Define a local disk storage engine.
- Add a "storage engine selector" class which manages choosing which storage
engines to put files in.
- Document storage engines.
- Document file storage classes.
Test Plan:
Setup mode:
- Disabled MySQL storage engine, misconfigured it, configured it correctly.
- Disabled file storage engine, set it to something invalid, set it to
something valid.
- Verified max_allowed_packet is read correctly.
Application mode:
- Configured local file storage.
- Uploaded large and small files.
- Verified larger files were written to local storage.
- Verified smaller files were written to MySQL blob storage.
Documentation:
- Read documentation.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 695
Summary:
I want to do two things here:
- Add SSH Keys
- Move "Preferences" into this panel
But this controller was pretty gigantic and messy. Split it apart and use
delegation instead.
There are no functional changes. I changed some of the conduit certificate text
to simplify it since no one should need to go through that workflow anymore,
given the existence of "arc install-certificate".
Test Plan:
- Edited realname, including attempting to remove it.
- Edited profile picture.
- Edited timezone.
- Edited email, including attempting to remove it.
- Regenerated condiut certificate.
- Linked and unlinked an OAuth account.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 688
Summary:
You can only call setHeader() on a Panel once. Otherwise the last sticks. Move the "forks of this paste" stuff to its own panel (only shown if there are, indeed, forks), and make the columns look nicer.
Test Plan:
Viewed previously forked pastes, forked a paste and looked at the original, and looked at a non-forked paste. All looked sane.
Reviewers:
epriestley
CC:
Differential Revision: 700
Summary:
Share code with the new PhabricatorDifferenceEngine, which handles diffs with no
changes correctly.
(This isn't the same issue as file moves, but I ran into it while generating a
repro case.)
Test Plan: Previously, changes which didn't change file content (e.g., property
changes) would throw. Now they work.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 698
Summary:
See T325. While this is a touch hacky it ends up being fairly clean, and we can
now do initial imports much more quickly and this actually cleaned up some of
the code. I also made the repository edit interface a little less foreboding.
@tuomaspelkonen, did you get anywhere with that bug you were chasing down a
couple days ago? We can hold this if it throws a wrench into stuff you're
working on.
Test Plan:
- Imported a subdirectory of a midsized SVN project (jQuery UI).
- Commit discovery for ~3500/4500 commits took just a few seconds.
- Commit discovery correctly ignored commits which didn't affect this
directory.
- Commit discovery correctly stopped at commit 13.
- Browse interface shows an incomplete listing, but that's fine, and
everything is otherwise functionally correct. We can add a note or something
later ("this is a view of commits affecting a subdirectory, some paths aren't
available"), but this behavior probably won't be too startling to users.
- Edited Git and SVN repositories to test form logic.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran, Girish
Commenters: tuomaspelkonen
CC: jcleveley, aran, jungejason, tuomaspelkonen
Differential Revision: 696
Summary:
Quite basic subproject attachment.
Test Plan:
Go to some proj
ect and add another as a subproject.
Play around the ui.
Reviewers: epr
iestley
CC:
Summary:
This is really rough and needs work (particularly, there's some diff code I
really need to refactor since I sort-of-copy-pasted it) but basically
functional.
Show text changes between diffs and allow users to revert to earlier versions.
Differential's line-oriented diff style isn't ideal for large blocks of text but
I'm betting this is probably good enough in most cases. We can see how bad it is
in practice and then fix it if needbe.
I added a bunch of support for "description" but didn't add the feature in this
diff, I'll either follow up or task it out since it should be a pretty
straightforward change.
Test Plan: Looked at history for several Phriction documents, clicked "previous
change" / "next change", clicked revert buttons.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen
CC: aran, hsb, epriestley
Differential Revision: 687
Summary:
Show line count, arcanist project and base revision.
This adds a little clutter but I think we're still okay and I can play around
with it later.
Test Plan: Looked at a couple of revisions. I'm actually not 100% sure about the
SVN logic but maybe I will test that before committing.
Reviewed By: tomo
Reviewers: tomo, jungejason, tuomaspelkonen, aran
CC: aran, tomo
Differential Revision: 685
Summary: Preview Phriction documents as they are edited, similar to how
Differential/Maniphest work.
Test Plan: Mashed my keyboard while editing a Phriction document.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, hsb
Differential Revision: 684
Summary: Pull the next couple levels of hierarchy and render them at the bottom
of the page. This might need some tweaking but it seems OK at first glance.
Test Plan:
https://secure.phabricator.com/file/info/PHID-FILE-ef0af5d4dc6dceaeb2e3/
Also reduced limit to 1 and verified the "more" behavior worked properly.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, hsb
Differential Revision: 676
Summary:
Added a 'parent' field which stores a PHID of another paste. If it is not NULL show a list of children pastes on view.
Also did some misc. refactoring to clean up the code a bit, specifically in the Create controller.
Test Plan:
- Checked old pastes, they were not affected.
- Added a paste, successfully.
- Forked it, successfully.
- Went to the original paste, saw the child paste listed.
- Forked it again, saw the new one added to the list.
Reviewers:
epriestley
CC:
Differential Revision: 672
Summary: So you don't have to edit if you don't want to.
Test Plan: Hit "cancel" on an edit of an existing page and an edit of a new
page.
Reviewed By: codeblock
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, codeblock
Differential Revision: 675
Summary: This fixes a bug where doing an "Add CC" on a task you were not CC'd on
would remove all the CCs except yourself. It also simplifies the CC handling
code a lot.
Test Plan:
- Added myself and another user to a task neither of us were CC'd on (old
behavior: added both then removed them; new behavior: added both)
- Added a user to CC with @mentions.
- Made a comment on a task I wasn't CC'd on (I was CC'd).
- Closed a task I wasn't assigned or CC'd on (I was not CC'd, but was
assigned).
- Made an "Add CC" with new CCs and comment text (ccs added, text appeared).
- Made an "Add CC" with no CCs and comment text (text appeared, transaction
correctly downgraded to "comment").
- Made an "Add CC" with exsiting CCs and comment text (text appeared,
transaction correctly downgraded to "comment").
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 668
Summary:
We currently cycle CSRF tokens every hour and check for the last two valid ones.
This means that a form could go stale in as little as an hour, and is certainly
stale after two.
When a stale form is submitted, you basically get a terrible heisen-state where
some of your data might persist if you're lucky but more likely it all just
vanishes. The .js file below outlines some more details.
This is a pretty terrible UX and we don't need to be as conservative about CSRF
validation as we're being. Remedy this problem by:
- Accepting the last 6 CSRF tokens instead of the last 1 (i.e., pages are
valid for at least 6 hours, and for as long as 7).
- Using JS to refresh the CSRF token every 55 minutes (i.e., pages connected
to the internet are valid indefinitely).
- Showing the user an explicit message about what went wrong when CSRF
validation fails so the experience is less bewildering.
They should now only be able to submit with a bad CSRF token if:
- They load a page, disconnect from the internet for 7 hours, reconnect, and
submit the form within 55 minutes; or
- They are actually the victim of a CSRF attack.
We could eventually fix the first one by tracking reconnects, which might be
"free" once the notification server gets built. It will probably never be an
issue in practice.
Test Plan:
- Reduced CSRF cycle frequency to 2 seconds, submitted a form after 15
seconds, got the CSRF exception.
- Reduced csrf-refresh cycle frequency to 3 seconds, submitted a form after 15
seconds, got a clean form post.
- Added debugging code the the csrf refresh to make sure it was doing sensible
things (pulling different tokens, finding all the inputs).
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley
Differential Revision: 660
Summary:
Provides a slowvote.poll.info method.
Test Plan:
Web console - seemed to work fine.
Reviewers:
epriestley, phuzion
CC:
Differential Revision: 659
Summary: Basic integration between Phriction and feed.
Test Plan: Created and edited some documents, they published to feed.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, hsb
Differential Revision: 653
Summary:
A few minor things:
- Add Phriction to the homepage.
- Link to remarkup reference on the edit screen.
- Add a help tab with a super basic help document.
Test Plan: Cursory inspection.
Reviewed By: codeblock
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, codeblock
Differential Revision: 655
Summary: Document linking and some general layout improvements. I'd like to
eventually do more meta-dataey things with links (like store them separately and
check them for 404s) but this is a decent start.
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-d756b94a06b69c273fce/
Reviewed By: jungejason
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 650
Summary: Index Phriction documents in search, and allow
PhabricatorObjectHandleData to load them.
Test Plan: Searched for "pokemons", got my wiki page as a search result.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, hsb
Differential Revision: 648
Summary:
This thing services every app but it lives inside Differential right now. Pull
it out, and separate the factory interfaces per-application.
This will let us accommodate changes we need to make for Phriction to support
wiki linking.
Test Plan: Tested remarkup in differential, diffusion, maniphest, people,
slowvote.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, hsb
Differential Revision: 646
Summary: Pretty much ripped from D636, but somewhat simplified. Lists all the
documents in the system.
Test Plan: Looked at both of the views, seems to work correctly.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, hsb, epriestley
Differential Revision: 645
Summary: Provide a (mostly useless, currently) table of document edits.
Test Plan: Looked at document history for several of my high-quality sandbox
wiki pages.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, aran, tuomaspelkonen
CC: aran, hsb
Differential Revision: 644
Summary:
This is another chunk of D636, I just simplified it a bit and added slugs.
When you go to a page like /w/pokemon/, it allows you to create or edit the
page.
Title vs slug stuff is a little funky but I think mostly-reasonable.
Test Plan: Created and edited /w/, /w/pokemon/, etc.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, aran, tuomaspelkonen
CC: aran, hsb
Differential Revision: 643
Summary: Depends on D628. Provides a config option so you can set up a public
feed, which you can iframe. This needs some work but sort of works.
Test Plan: Loaded the public feed as a logged-out user.
Reviewed By: codeblock
Reviewers: jungejason, tuomaspelkonen, aran, codeblock
CC: aran, codeblock
Differential Revision: 635
Summary:
- Services: Show summary panel of total service call costs and relative page weight.
- Services: Add "Analyze Query Plans" button, which issues EXPLAIN for each query and flags problems.
- XHPRof: iframe the profile.
Test Plan: Used the new query plan analysis to find missing keys causing table scans, see D627.
Reviewers: jungejason, tuomaspelkonen, aran
CC:
Differential Revision: 628
Summary:
Basically a copy/paste of parts of D636, but with two changes:
- Fully separate the index table ("document") from the content table
("content"). I think this will be a cleaner solution in the long run.
- Build slugs into the document structure.
This doesn't do anything useful, it just normalizes slugs and lays some
groundwork.
Test Plan:
- Visited various /w/ pages and saw them normalize correctly.
- Verified the DAO works by inserting dummy rows.
Reviewed By: codeblock
Reviewers: hsb, codeblock, jungejason, aran, tuomaspelkonen
CC: aran, codeblock, epriestley
Differential Revision: 638
Summary: see title
Test Plan: ran "arc amend" to ensure that task ids where being included in the commit message
Reviewers: epriestley
CC: dpepper
Differential Revision: 637
Summary: Basic hookup for Differential -> Feed. Also introduces "one-line"
stories for less-important stuff.
Test Plan: Interacted with some revisions, got feed stories out of it.
Reviewed By: jungejason
Reviewers: jungejason, aran, tuomaspelkonen, codeblock
CC: aran, jungejason
Differential Revision: 632
Summary:
We don't currently add CCs, but should (similar to how Differential works).
This also fixes some problems where you can get no-op CC transactions, and makes
mentions a little less aggressively colored.
Test Plan: Applied a bunch of CC/mention transactions to tasks and observed the
behavior.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, codeblock
CC: aran, jungejason
Differential Revision: 634
Summary:
fix two issues. First is that the field in the query is not
correct; the other is to make indexing handle deleted repository.
Test Plan: indexed some commits and the search result looks correct.
Reviewed By: epriestley
Reviewers: epriestley, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 633
Summary: No change in functionality, just splitting this method up a bit
Test Plan: Loaded list and looked at all three views.
Reviewed By: jungejason
Reviewers: jungejason, codeblock, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 618
Summary: Make this more usable. Also fix a bug where $choices got overriden by a
loop variable.
Test Plan: Looked at a vote with multiple respondents.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran, codeblock
CC: aran, jungejason
Differential Revision: 629
Summary: We need to perform an explicit test for public reply support.
Previously, the existence of a valid result here was a sufficient implicit test
for public reply support, but it no longer is.
Test Plan: With an unmodified configuration, sent email. It generated with the
correct reply-to (me). Restored my original configuration and sent an email, it
generated with the correct (routed) reply-to.
Reviewed By: codeblock
Reviewers: codeblock
CC: aran, codeblock
Differential Revision: 626
Summary:
This is not very useful and not exposed on the web UI. It's also the only caller
for PhabricatorPHIDConstants::getTypes().
I originally wrote this to test PHID allocation when I built the PHID system but
it's no longer really useful in any way.
phid.allocate might be useful to expose over Conduit eventually but the
implementation is trivial.
Test Plan: Grepped for controller and method names, came up empty.
Reviewed By: codeblock
Reviewers: jungejason, tuomaspelkonen, aran, codeblock
CC: aran, codeblock
Differential Revision: 625
Summary: No change in functionality, just split up this megamethod
Test Plan: Looked at several votes.
Reviewed By: jungejason
Reviewers: jungejason, codeblock, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 619
Summary:
Add the differential parse cache to the GC. This is the largest object in the
system by a wide margin, I think.
This table is potentially gigantic which is why the script truncates it before
doing a schema change.
Test Plan: Ran the GC daemon, it cleaned up some parse caches.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
Commenters: tuomaspelkonen
CC: aran, jungejason, tuomaspelkonen, epriestley
Differential Revision: 620
Summary:
Show language of pastes in the list view.
Test Plan:
Saw language of pastes in the list view.
Reviewers:
epriestley
CC:
Differential Revision: 617
Summary:
Now that we store language with pastes, we can include this in Conduit.
Test Plan:
Tried it on a paste with a blank language, and one with a specified language.
16:14:50 <@CodeBlock> P1
16:14:51 <@codeblock-phabot> P1: http://phabricator.local/P1 - test.php
16:15:05 <@CodeBlock> P43
16:15:06 <@codeblock-phabot> P43: http://phabricator.local/P43 - sadoijfoisaf (php)
Reviewers:
epriestley, Ttech
CC:
Differential Revision: 616
Summary:
See T303. Enable comment panel haunting.
I hid the preview for the sticky panel, which I think is reasonable?
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-64713fa8a7c2a22e5b93/
Reviewed By: broofa
Reviewers: broofa, jungejason, aran, tomo, tuomaspelkonen
CC: aran, broofa
Differential Revision: 615
Summary: Port slowvote. This has some style/layout roughness but gets us most of
the way there. I'll followup to fix some of the markup issues.
Test Plan: Created and voted in several different kinds of poll.
Reviewed By: codeblock
Reviewers: codeblock, tomo, jungejason, aran, tuomaspelkonen
Commenters: aran, jungejason
CC: aran, codeblock, jungejason, epriestley
Differential Revision: 613
Summary:
- have files be uploaded by drag+drop instead of browse.
- Files are named by their uploaded filename, the user isn't given a chance to enter a file name. Is this bad?
- Store author PHID now with files
- Allow an ?author=<username> to limit the /files/ list by author.
- If one file is uploaded, the user is taken to its info page.
- If several are uploaded, they are taken to a list of their files.
Test Plan:
- Quickly tested everything and it still worked, I'd recommend some people try this out before it gets committed though. It's a rather huge revision.
Reviewers:
epriestley, Ttech
CC:
Differential Revision: 612
Summary: This is pretty basic but gets us most of the way there I think. Could
use some style tweaks at some point.
Test Plan: Looked at a project page with open tasks, and one without open tasks.
Reviewed By: tuomaspelkonen
Reviewers: cadamo, aran, jungejason, tuomaspelkonen
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 609
Summary: We decided to move away from driving everything through the search
engine since it doesn't scale terribly well, so use ManiphestTaskQuery instead.
Also link the open count and tweak some display stuff.
Test Plan: Looked at project list, clicked open tasks link
Reviewed By: tuomaspelkonen
Reviewers: cadamo, aran, jungejason, tuomaspelkonen
CC: aran, tuomaspelkonen
Differential Revision: 608
Summary: JX.JSON was recently changed to use JSON.stringify (the native
implementation) if it is available. The native implementation has a behavioral
difference from the Javelin implementation, in that it does not compact sparse
arrays. Ignore nulls resulting from removals when processing the encoded action
and condition lists.
Test Plan: Removed conditions from Herald rules.
Reviewed By: aran
Reviewers: aran, cpojer, jungejason
CC: aran, epriestley
Differential Revision: 606
Summary:
My earlier diff refactored some code without completely
respecting the semantics, sometimes resulting in duplicate field names
returned from differential.getcommitmessage. This fixes that.
Test Plan:
ran "arc diff" with diff causing the bug (commit message
had an empty Revert Plan: field) and verified no duplicate fields
Reviewed By: epriestley
Reviewers: epriestley
CC: aran, dpepper, epriestley
Differential Revision: 610
Summary:
People want to see all the tasks they have subscribed to in one
view. A new table was added for this to make queries faster.
Test Plan:
Tested that the view was initially empty. After running the
reindex_maniphest.php script, I saw the correct tasks there. Added
myself as a subscriber to one task and made sure the view was
updated. Removed myself as a subscriber from one task and made sure
the view was updated again.
Reviewed By: epriestley
Reviewers: epriestley, jungejason, codeblock
CC: aran, rm, epriestley
Differential Revision: 603
Summary:
Send the user where they were intending to go after github and localized logins.
Before, because Github didn't send oauthState, we would force / upon them.
Test Plan:
Tried all three methods of login successfully.
Reviewers:
epriestley
CC:
Differential Revision: 602
Summary: This defines an extremely basic version of an activity feed, like
Facebook's news feed. It doesn't do much of interest yet.
Test Plan: Published some feed stories:
https://secure.phabricator.com/file/view/PHID-FILE-5061aa72105bbdc05b21/
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
Commenters: codeblock, jungejason
CC: aran, epriestley, codeblock, tuomaspelkonen, jungejason
Differential Revision: 593
Summary: Reduce the amount of code duplication here and allow for an override
configuration on the filename.map stuff.
Test Plan: Checked paste, diffusion and differential syntax highlighting and
everything appeared reasonable.
Reviewed By: codeblock
Reviewers: tuomaspelkonen, codeblock, jungejason, aran
CC: aran, codeblock, epriestley
Differential Revision: 601
Summary: I'll clean some of this stuff up in a followup too, but update the
callers to use the new explicit filename-based API.
Test Plan: Looked at paste, Diffusion and Differential.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, codeblock, jungejason, aran
CC: aran, tuomaspelkonen
Differential Revision: 600
Summary:
We deleted a repository (I don't remember which one). Some
differential revisions where committed to this repository, and opening
these revisions show a blank page, because the repository_id is not in
the database anymore.
This causes the 'ERROR 8: Undefined index: 12 at
[phabricator/src/applications/phid/handle/data/PhabricatorObjectHandleData.php:209]
' in our log.
Test Plan:
Opened a revision which where committed to multiple repositories including
the deleted one. Made sure that the page was rendered correctly and there
were no messages in the error log.
Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley
Differential Revision: 598
Summary: This lets you configure an email address which will create tasks when
emails are sent to it. It's pretty basic but should get us most of the way
there.
Test Plan: Configured an address and created a task via email. Replied to a task
via email to check that I didn't break that.
Reviewed By: tuomaspelkonen
Reviewers: davidreuss, jungejason, tuomaspelkonen, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 590