Summary: Minor, moves to `newPage`
Test Plan: Test both pages, still work
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15582
Summary: Little straggler here, updates to `newPage`
Test Plan: Review a document, no visibile changes
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15580
Summary: Updates Conpherence pages to use `newPage`
Test Plan: View a Room, view list of joined rooms.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15578
Summary: Just clearing these all out.
Test Plan: Visit channel list and log page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15577
Summary:
Ref T10709. Two issues:
- If a user sends an invalid `!command`, we can throw, which means we don't return HTTP 200. This makes Mailgun re-send the mail later.
- We don't parse headers of the modern API correctly, so the "Message-ID" failsafe doesn't work. Parse them correctly. I //believe// Mailgun's API changed at some point.
Test Plan:
This is difficult to test exhaustively in isolation. I used Mailgun's web tools to verify the format of the hook request, and faked some requests locally.
I'll keep an eye on this as it goes to production and make sure the fix is correct there.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10709
Differential Revision: https://secure.phabricator.com/D15575
Summary:
Ref T10709. Two issues:
- If a user sends an invalid `!command`, we can throw, which means we don't return HTTP 200. This makes Mailgun re-send the mail later.
- We don't parse headers of the modern API correctly, so the "Message-ID" failsafe doesn't work. Parse them correctly. I //believe// Mailgun's API changed at some point.
Test Plan:
This is difficult to test exhaustively in isolation. I used Mailgun's web tools to verify the format of the hook request, and faked some requests locally.
I'll keep an eye on this as it goes to production and make sure the fix is correct there.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10709
Differential Revision: https://secure.phabricator.com/D15575
Summary: Modernize and use newer UI
Test Plan: Bounce around various views.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15574
Summary: Updates various Phriction pages to match new UI
Test Plan: New Document, Edit Document, View History, Revert Change
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15572
Summary: Swap over to modern components, `newPage` and `handleRequest`.
Test Plan: `arc lint` :(
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D15571
Summary: Runs through Meta, cleaned up policies and editing email addresses to new UI
Test Plan: Set a new Email address for Maniphest, edit policies.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15569
Summary: Fixes T10704. This is just bad copy-paste -- "O" for "old" should be "N" for "new".
Test Plan:
- Followed steps on T10704.
- Applied patch.
- Marked inline done, replied, etc. No more JS errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10704
Differential Revision: https://secure.phabricator.com/D15566
Summary: Cleans up Pholio, moves to two column layout, fix some transaction inconsistencies. This moves "Image" to the MainColumn, which feels fine, but I think we'll likely want some sort of "fullscreen" option for Pholio V2 like we have on workboards perhaps.
Test Plan:
New Mock, Edit Mock, View Mock.
{F1200450}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15564
Summary: Ref T9007
Test Plan: Navigate to "Advanced Search" in Badges, order by rarity, then by commonality. Rarest and most common badges should be ordered, respectively.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Maniphest Tasks: T9007
Differential Revision: https://secure.phabricator.com/D15555
Summary:
Ref T10698. Currently, we show the spacer/continuation lines around previews, but these don't make sense in previews.
(Other stuff also uses this code so I can't simply remove `spacer`.)
Test Plan:
Before:
{F1199924}
After:
{F1199925}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10698
Differential Revision: https://secure.phabricator.com/D15562
Summary: This doesn't hit the ambiguous case in Diffusion so it seems fine to make it more consistent.
Test Plan: Looked at a little task-o.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15561
Summary: Walks through various object, rule, create forms and transcripts in Herald. Slightly nicer looking.
Test Plan: Make rules, see rules, edit rules, see transcripts.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15559
Summary: Fixes T10672. Cleaning this up myself since I was responsible for the implementation.
Test Plan: Leave a comment, Edit a badge, create a badge.
Reviewers: lpriestley, epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10672
Differential Revision: https://secure.phabricator.com/D15556
Summary: Adds basic commenting to Fund Initiatives.
Test Plan: Leave a comment, see comment.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15554
Summary: Uses BLUE_PROPERTY on Recipients box, removes redundent properties since we render the badge itself already.
Test Plan: View a badge with and without a description.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15553
Summary: [WIP] Tossing this up for safety and to read through it. Need to test, update some of the other flows. This updates everything in Auth for new UI and modern conventions.
Test Plan: Loooots of random testing, new providers, edit providers, logging out, forgot password... more coming.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15550
Summary:
[WIP] Allows awarding a badge from a user profile. Unsure of the interactions here if a user can't award any badges, or if we should just hide this.
Fixes T10688
Fixes T10318
Test Plan: Award some badges. Steal them back.
Reviewers: lpriestley, epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10318, T10688
Differential Revision: https://secure.phabricator.com/D15544
Summary: Ref T8941
Test Plan: Create an object and create multiple transactions, some time apart to ensure that time clumping isn't interfering. Make sure that events that are large enough to have a dropdown menu show badges under author pic.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T8941
Differential Revision: https://secure.phabricator.com/D15543
Summary:
Ref T10537. Currently, Nuance has a `NuanceRequestor` object, intended to represent the external user who created content (e.g., a GitHub account or a Twitter account or whatever).
This object is currently almost unused, and its design predates Doorkeeper. In D15541, I chose to use doorkeeper objects instead of NuanceRequestor objects to represent requestors.
I don't currently anticipate a need for such an object, given that we have Doorkeeper. If we do need it in the future for some reason, it would be fairly easy to restore it, create a requestor type which wraps a Doorkeeper object, and then migrate. Not super thrilling to do that, but not a huge mess.
`NuanceItem` still has a `requestorPHID`, but this is now a less formal object PHID instead of a more formal Requestor-object PHID, and holds a doorkeeper exeternal object PHID for GitHub events.
Test Plan:
- Grepped for `nuancerequestor`.
- Ran `bin/storage upgrade -f`.
- Grepped for `requestor`, remaining uses of this term seem reasonable/correct.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15546
Summary: This is kinda bad in terms of UI (It just makes a json of the thing and diffs that), but it's a start.
Test Plan: edit rule, create rule, add/remove/edit conditions, actions
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15542
Summary:
Ref T10538. Ref T10537. This creates PHIDs which represent GitHub users, and uses them as the actors for synchronized comments.
I've just made them Doorkeeper objects. There are three major kinds of objects they //could// possibly be:
- Nuance requestor objects.
- External account objects.
- Doorkeeper objects.
I don't think we actually need distinct nuance requestor objects. These don't really do anything right now, and were originally created before Doorkeeper. I think Doorkeeper is a superset of nuance requestor functionality, and better developed and more flexible.
Likewise, doorkeeper objects are much more flexible than external account objects, and it's nice to imagine that we can import from Twootfeed or whatever without needing to build full OAuth for it. I also like less stuff touching auth code, when possible.
Making these separate from external accounts does make it a bit harder to reconcile external users with internal users, but I think that's OK, and that it's generally desirable to show the real source of a piece of content. That is, if I wrote a comment on GitHub but also have a Phabricator account, I think it's good to show "epriestley (GitHub)" (the GitHub user) as the author, not "epriestley" (the Phabricator user). I think this is generally less confusing overall, and we can add more linkage later to make it clearer.
Test Plan:
{F1194104}
{F1194105}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537, T10538
Differential Revision: https://secure.phabricator.com/D15541
Summary: Ref T10538. This probably gets push events where GitHub does not recognize the author wrong, but I don't have any of those yet.
Test Plan: Added and ran unit tests.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10538
Differential Revision: https://secure.phabricator.com/D15540
Summary: Ref T10537. Add a new content source for Nuance. Prepare for better author attribution.
Test Plan: {F1194038}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15539
Summary: Fixes T10670, for users with exclusively archived badges, user profile should show "no badges" message instead of blank box
Test Plan: Award badge to user with no badges, archive badge, user profile should show "no badges" message under badges.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10670
Differential Revision: https://secure.phabricator.com/D15538
Summary:
Fixes T10234. This is a more thorough fix.
Root issue is that some time around D13589, we started hitting an object cache for `loadCustomInlineRules()`, but didn't adjust the code to account for that.
So if a page created multiple similar engines, we'd return the same `$rule` object for multiple engines, call `setEngine()` on it with different engines, and then possibly try to render using an already-expired engine the second time through.
Instead, create a separate `$rule` object for each separate `$engine`.
Test Plan:
Repro is something like this:
- Create a custominlinerule which uses an engine.
- Purge the remarkup cache.
- Load a page which uses the rule in two engines (e.g., in a revision description, and also in an inline comment).
- Before change: second one could fatal. After change: clean load.
Reviewers: thoughtpolice, chad
Reviewed By: thoughtpolice, chad
Subscribers: thoughtpolice, eadler
Maniphest Tasks: T10234
Differential Revision: https://secure.phabricator.com/D15535
Summary:
Ref T10537.
- Let nuance items render custom curtain panels.
- Add a custom panel linking to the imported task, if one exists.
- Actually extract comments properly.
Test Plan:
Unit tests, plus:
{F1193800}
{F1193801}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10537
Differential Revision: https://secure.phabricator.com/D15537
Summary: Ref T10677, Awarding/revoking badge should create a feed story on homepage with badge handle recipient handles
Test Plan: Award/revoke badge, open Feed, should see story with badge link and recipient links.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10677
Differential Revision: https://secure.phabricator.com/D15534
Summary: Ref T10677, awarding/revoking a badge should create timeline entries with titles that are more clear (excludes homepage feed stories)
Test Plan: Award/revoke a badge to single or multiple users. See timeline entries that reflect those actions.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10677
Differential Revision: https://secure.phabricator.com/D15533
Summary: Cleans up EditEngine, adds new layout to EditEngine and descendents
Test Plan: Test creating a new form, reordering, marking and unmarking defaults. View new forms.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15531
