Summary:
- Add inline comments to Audits, like Differential.
- Creates new storage for the comments in the Audits database.
- Creates a new PhabricatorAuditInlineComment class, similar to DifferentialInlineComment.
- Defines an Interface which Differential and Audit comments conform to.
- Makes consumers of DifferentialInlineComments consume objects which implement that interface instead.
- Adds save
NOTE: Some features are still missing! Wanted to cut this off before it got crazy:
- Inline comments aren't shown in the main comment list.
- Inline comments aren't shown in the emails.
- Inline comments aren't previewed.
I'll followup with those but this was getting pretty big.
@vrana, does the SQL change look correct?
Test Plan:
- Created, edited, deleted, replied to, reloaded and saved inline comments in Diffusion, on the left and right side of diffs.
- Created, edited, deleted, replied to, reloaded and saved inline comments in Differentila, on the left and right side of primary and diff-versus-diff diffs.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D1898
Summary:
When the user loads a page with an anchor on it like #thing, or clicks a link to
#thing, and #thing doesn't exist, keep trying to navigate to #thing for a few
seconds.
This allows anchors to work when the target is in content which is later ajaxed
in. In particular, this affects inline comments in Differential.
Test Plan: Opened inline comment links in a new tab, was in the right place when
I switched tabs.
Reviewers: nh, btrahan, jungejason
Reviewed By: btrahan
CC: aran, btrahan, epriestley
Maniphest Tasks: T492
Differential Revision: https://secure.phabricator.com/D1327
Summary:
Provide a catchall mechanism to find unprotected writes.
- Depends on D758.
- Similar to WriteOnHTTPGet stuff from Facebook's stack.
- Since we have a small number of storage mechanisms and highly structured
read/write pathways, we can explicitly answer the question "is this page
performing a write?".
- Never allow writes without CSRF checks.
- This will probably break some things. That's fine: they're CSRF
vulnerabilities or weird edge cases that we can fix. But don't push to Facebook
for a few days unless you're prepared to deal with this.
- **>>> MEGADERP: All Conduit write APIs are currently vulnerable to CSRF!
<<<**
Test Plan:
- Ran some scripts that perform writes (scripts/search indexers), no issues.
- Performed normal CSRF submits.
- Added writes to an un-CSRF'd page, got an exception.
- Executed conduit methods.
- Did login/logout (this works because the logged-out user validates the
logged-out csrf "token").
- Did OAuth login.
- Did OAuth registration.
Reviewers: pedram, andrewjcg, erling, jungejason, tuomaspelkonen, aran,
codeblock
Commenters: pedram
CC: aran, epriestley, pedram
Differential Revision: 777
Summary:
Although I think the recent changes here improved things, the "Unsaved Draft"
language is continuing to confuse new users. Try to find some less-confusing
langauge. Open to suggestions here, too.
Test Plan:
Viewed unsubmitted inline comments.
Reviewed By: jungejason
Reviewers: aran, jungejason, gregprice
CC: aran, epriestley, jungejason
Differential Revision: 501
Summary:
See attached tasks. See D459 for the ability to merge tasks.
Test Plan:
Looked at posted and unposted inline comments.
Reviewed By: aran
Reviewers: edward, viyer, aran, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 461
Summary:
They currently have "Next", "Previous" and "Reply" links which don't work. Don't
render these links.
Test Plan:
Looked at inline previews, didn't see any silly/nonfunctional links.
Reviewed By: tuomaspelkonen
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 419
Summary:
When a user hits 'cancel' on a 'new', 'edit', or 'reply' operation, add a little
"Changes discarded. __Undo__" insert so they can get their change back. No undo
for delete since there's an explicit prompt. Once this lands we can make
'escape' work again to close dialogs.
This change started feeling really good when I was merging all the duplicate
code and making things more consistent, but by the time I started writing client
rendering it felt gross. I'm not really thrilled with it but I guess it's a step
forward? The feature seems pretty OK in practice. Let me know how much barfing
this causes and I can try to remedy the most acute concerns.
This also fixes a bug where replies always (?) appear on the 'new' side of the
diff (I think?).
Test Plan:
Applied 'new', 'edit', 'delete' and 'reply' operations, pressed 'cancel' and
'okay' in each case, with and without changing text where relevant. All
behaviors seem to conform with expectations, except that canceling out of 'edit'
without changing the text gives you an option to undo when it shouldn't really.
There's no super easy way to get at the original text right now.
Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: simpkins, aran, epriestley
Differential Revision: 406
Summary:
Browsing comments was a bit difficult without the possibllity to jump
between comments. These links will make the browsing easier.
Test Plan:
Tested on multiple diffs that the links were working correctly.
Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, aran, tuomaspelkonen, epriestley
Differential Revision: 266
Summary:
This makes some of the line spacing, paragraph spacing and layout
less terrible. In particular, fixes code blocks inside Differential inline
comments.
Test Plan:
Looked at Maniphest Tasks, Differential Revisions and Differential
inline comments with various flavors of remarkup in them.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason
CC: jungejason
Differential Revision: 89
Summary:
- Prevent long comments from expanding the inline box.
- Make anchor links to inlines work properly.
- Get rid of "pre" white-space formatting in inline comments.
Test Plan: Viewed a revision with crazy comments in it.
Reviewers:
CC:
