Summary:
Because most threads are private, this query can overheat the policy filter (today, probably only on this install).
Improve the common case by skipping "Visible To: Room Participants" threads if the viewer isn't a participant. This means they don't hit the application and don't count toward overheating the filter.
Test Plan: Viewed Conpherence threads.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16740
Summary: Ref T11766. When users run `git pull` or similar, log the operation in the pull log.
Test Plan: Performed SSH pulls, got a log in the database. Today, this event log is purely diagnostic and has no UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11766
Differential Revision: https://secure.phabricator.com/D16738
Summary: Ref T11773. Not committed to this implementation, but adds some "Developer" query actions to jump to the nux/overheated states without needing to know secret magic URL variables.
Test Plan: {F1878984}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11773
Differential Revision: https://secure.phabricator.com/D16736
Summary:
See D16734.
- Add ".ico" files to the Celerity map.
- Add a formal route for "/favicon.ico".
- Remove instructions to configure `/rsrc/` and `/favicon.ico` rewrite rules.
Long ago, we served resources directly via `/rsrc/` in at least some cases. As we added more features, this stopped working more and more often (for example, Apache can never serve CSS this way, because it doesn't know how to post-process `{$variables}`).
In modern code (until this change), only `/favicon.ico` is still expected to be served this way.
Instead, serve it with an explicit route via controller (this allows different Sites to have different favicons, for example).
Remove the instructions suggesting the old rewrite rules be configured. It's OK if they're still in place -- they won't break anything, so we don't need to rush to get users to delete them.
We should keep "webroot/favicon.ico" in place for now, since it needs to be there for users with the old rewrite rule.
Test Plan:
- Ran celerity map.
- Loaded `/favicon.ico`, got resource via route.
- Used `celerity_generate_resource_uri()` to get paths to other icons, loaded them, got icons.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16737
Summary: Ref T11773. This is an initial first step toward a more complete solution, but should make the worst case much less bad: prior to this change, the worst case was "30 second exeuction timeout". After this patch, the worst case is "no results + explanatory message", which is strictly better.
Test Plan:
Made all feed stories fail policy checks, loaded home page.
- Before adding overheating: 9,600 queries / 20 seconds
- After adding overheating: 376 queries / 800ms
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11773
Differential Revision: https://secure.phabricator.com/D16735
Summary: Ref T10747. This doesn't have a "keep up to date" option yet, but can, e.g., fetch a Google Calendar URI
Test Plan: Fetched a Google Calendar URI, got some events imported.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16730
Summary:
Ref T10747. Previously, importing a recurring event failed to mark the instnaces of the event as imported.
Now, we copy the source/UID/importer over.
Test Plan: Imported a recurring event, viewed event series, saw all of them marked imported.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16728
Summary:
Ref T10747. When viewing an imported event:
- Make it more clear that it is imported and where it is from.
- Add some explicit "this is imported" help.
Test Plan: Viewed imported and normal events.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16727
Summary: Ref T10747. When viewing an import detail page, show a little more information about what you're looking at.
Test Plan: {F1876957}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16726
Summary: Fixes T11764. Moves rendering of the column to client-side, which can skip if it detects we're on mobile.
Test Plan: Open column on desktop, switch to mobile, don't see column. Toggle column on mobile on and off. Switch back to desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11764
Differential Revision: https://secure.phabricator.com/D16725
Summary: Ref T10747. Although I could possibly imagine some very selective cases where we do this eventually, these are read-only for now and not interesting to publish/mail about. The presumption is that the original/authoritative system has already notified relevant parties or they're subscribing passively.
Test Plan: Imported some name changes for events, saw no more mail/feed stuff.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16723
Summary:
Ref T10747. When a user drops a ".ics" file or a bunch of ".ics" files into a calendar view, import the events.
(Possibly we should just do this if you drop ".ics" files into any application, but we can look at that later.)
Test Plan: Dropped some .ics files into calendar views, got imports.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16722
Summary:
Ref T10747. When we hit an ICS parser error, render it into a log instead of fataling.
(This will be more important in the future with subscription-based URL ICS import.)
Test Plan: {F1875292}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16721
Summary:
Ref T10747. If you accidentally import the wrong thing, you can clean up the big mess you made.
These imported events are read-only so it's OK to destroy them completely (vs disable/hide/archive).
Test Plan: Destroyed some imported events.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16720
Summary:
Ref T10747.
- Look at more than 25 logs!
- Review your favorite logs. Heartwarming! :)
Test Plan: Looked at logs. Wow! Logs!
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16719
Summary: Ref T10747. Don't let users import SECONDLY events, or events outside of the range of a signed 32-bit integer (these are likely not too hard to support, but they're more headaches than we need right now).
Test Plan: Tried to import these no-good problem events, got helpful import errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16716
Summary: There isn't any link back to all your joined rooms when on mobile, add it here.
Test Plan: Pull up mobile, click on menu, see list of threads, click on other room.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16717
Summary:
Looks like the logic was there already but some minor parts were missing.
Fixes T8082.
Test Plan:
- Create document `/w/foo`
- Delete document `/w/foo`
- Create document `/w/bar`
- Move document `/w/bar` for `/w/foo`
No error was displayed and document `/w/bar` was moved to `/w/foo`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T8082
Differential Revision: https://secure.phabricator.com/D16713
Summary:
Fixes T11748. This option currently implies a line limit (e.g., inline patches that are less than 100 lines long). This breaks down if a diff has a 10MB line, like a huge blob of JSON all on one line.
For now, imply a reasonable byte limit (256 bytes per line).
See T11767 for future work to make this and related options more cohesive.
Test Plan:
- With option at `1000`: sent Differential email, saw patches inlined.
- With option at `10`: sent Differential email, saw patches dropped because of the byte limit.
- `var_dump()`'d the actual limits and used `bin/worker execute --id ...` to sanity check that things were working properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11748
Differential Revision: https://secure.phabricator.com/D16714
Summary: This removes 'full-display', 'minimal-display' from Conpherence, which I recall was because we had 2 UIs for column and regular chat. I'm also tossing in slightly nicer search results, with a link to the actual message and the full date shown for context.
Test Plan: Post a message in mobile, tablet, full conpherence, and in durable column. Clean up UI in durable column. Do a search in Full UI, click on result date, get taken to the message... usually. My test data is a little wonky, but I think this works most of the time.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16710
Summary: Ref T11730. Removes the unused column, seen no issues during past week migrations.
Test Plan: Run migration, check database no longer contains column.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11730
Differential Revision: https://secure.phabricator.com/D16711
Summary: Basically all here, but still probably needs some polish (links to jump? full dates?). Looks much better, still duplicates messages though sometimes. Needs to debug that more.
Test Plan:
Revisit search UI inside Conpherence, outside Conpherence, and normal room searches in Conpherence.
{F1870748}
{F1870749}
{F1870750}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16708
Summary: I passed this in as a config, but need to parse it live when threads change, otherwise the wrong room could be searched.
Test Plan: Search in one room, click a second, search again, see correct results.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16707
Summary: Ref T10747. When stuff goes wrong (or right) let the user know what happened.
Test Plan: {F1870139}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16704
Summary: Accept Conduit parameter values as strings (e.g. from `curl`) and convert to required type.
Test Plan:
Call conduit method with int/bool parameter iusing `curl` and make sure it does not result in validation error, e.g.
```
$ curl http://$PHABRICATOR_HOST/api/maniphest.search -d api.token=$CONDUIT_TOKEN -d constraints[modifiedEnd]=$(date +%s) -d constraints[hasParents]=true -d limit=1
```
Fixes T10456.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T10456
Differential Revision: https://secure.phabricator.com/D16694
Summary: Converts Owners package transactions to modular transactions.
Test Plan:
- created a new package
- edited all simple properties from the web ui
- checked that project and user owners were added as reviewers appropriately to new diffs
- inspected the change details for various types of path add / remove / update / reorder changes
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D16651
Summary:
This search engine ports cleanly to Conduit out of the box.
Ref T11694
Test Plan: called the API method from the console, browsed blueprints in the ui
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T11694
Differential Revision: https://secure.phabricator.com/D16593
Summary: Adds a search bar toggle and results for searching inside a Conpherence Room. The UI of the results itself are not styled yet, and will follow up with another diff.
Test Plan: Go to Conpherence, search for "asdf", get lots of results. Search for nothing, get no change, search for something fictitious, get no threads found (will follow up with search result UI).
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16697
Summary: Ref T10747. Show which events a source imported, and link to the full list as a query result.
Test Plan: {F1870049}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16703
Summary: Ref T10747. This doesn't do much for ICS file imports (you can't disable them since it doesn't do anything meaningful) but will matter more for ICS-subscription imports later.
Test Plan: Clicked "Disable" on an ICS file import, got explanatory dialog.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16702
Summary:
Ref T10747.
- Apply what changes we can with transactions, so you can see how an event has changed and import actions are more explicit.
- I'll hide these from email/feed soon: I want them to appear on the event, but not generate notifications, since that could be especially annoying for automated events.
- When importing, try to update existing events if we can.
Test Plan:
Imported a ".ics" file several times with minor changes, saw them reflected in the UI with transactions.
{F1870027}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16701
Summary: Ref T11730. Removes unused code since this is now it's own page.
Test Plan: rebuild maps, grep for javelin code, classnames
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11730
Differential Revision: https://secure.phabricator.com/D16700
Summary: Ref T10747. This barely works, but can technically import some event data.
Test Plan: Used import flow to import a ".ics" document.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16699
Summary:
Ref T10747. Adds a bunch of stuff so we can keep track of which events we've imported from external sources.
This doesn't do anything yet: you can't actually import anything.
Test Plan:
- Ran `bin/storage upgrade`.
- Clicked "Imports", saw an empty wasteland.
- Created/edited events.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16696
Summary: We currently fetch 15 transactions for 5 rooms, which leads to some room subtitles in the notification panel to being blank since nothing was fetched. I don't think this is a great fix, but moves the bar much further. Maybe there is a more accurate fix that isn't 5 SQL queries?
Test Plan: Review notification panel in sandbox, ensure all threads have some additional information.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16695
Summary: Ref T3165. Builds an ngram table for Conpherence Room titles, allowing a tokenizer for searching a subset of rooms.
Test Plan: Say `Gabbert` in two different rooms, search all, see two rooms returned. Search specific room, see specific result.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3165
Differential Revision: https://secure.phabricator.com/D16692
Summary: Background is now always white, spacing in header is more consistent
Test Plan: test mobile, table, desktop application search apps.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16691
Summary:
Ref T11706. Add some casts so we don't return `"0"` for `false`.
Also I forgot to document one of the things.
Test Plan: Called `calendar.event.search`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11706
Differential Revision: https://secure.phabricator.com/D16690
Summary:
`DrydockAuthorizationSearchEngine` was being used solely to display authorizations for a specific blueprint from the web UI and consequently expected that callers set a specific blueprint before performing a query. Here we check to see if a blueprint has been set in cases where the engine could be operating from either Conduit or the web.
Ref T11694
Test Plan:
- called the API method from the console
- approved an authorization
- followed the "view all" link from a blueprint page
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Maniphest Tasks: T11694
Differential Revision: https://secure.phabricator.com/D16592
Summary: We have more space here for last 8.
Test Plan: Reload, see 8.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16689
Summary: Fixes T11706. I think this approach (roughly: provide the information in a few different formats) is generally reasonable, and should let clients choose how much date/time magic they want to do.
Test Plan: Called `calenadar.event.search`, viewed results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11706
Differential Revision: https://secure.phabricator.com/D16688
Summary:
Fixes T11746. The opcache docs are on a different page, so point there if we're raising opcache issues.
(It's possible for a setup issue to say "configure X, or configure Y", where X is opcache and Y is non-opcache, so we may want to render both links.)
Test Plan: {F1867109}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11746
Differential Revision: https://secure.phabricator.com/D16685
Summary: Fixes T11745. I just missed this while juggling some of the internal storage.
Test Plan: Created a new event with recurrence behavior.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11745
Differential Revision: https://secure.phabricator.com/D16684
Summary:
Ref T10747. This adds disable/enable to exports.
Mostly useful if you leak a URI by accident.
Test Plan:
- Disabled and enabled exports.
- Verified that disabled exports don't actually export any data.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16681
Summary:
Ref T10747. This explains how exports work.
Also make mail exports use the same logic as other stuff.
Test Plan: Read documentation. Did some exports.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16680
Summary:
Ref T10747. This:
- Exports recurring events properly, with RRULE + RECURRENCE-ID.
- When exporting a part of an event series, export the whole series to ICS so it is represented faithfully.
- Make the subscribable URL for "Export" objects work.
Test Plan:
- Downloaded the ".ics" for a normal event, imported it into Calendar.app and Google Calendar.
- Downloaded the ".ics" for a recurring event, imported it into Calendar.app and Google Calendar.
- Defined an ".ics" Export of my events, subscribed to them in Calendar.app.
- Edited an event in Phabricator.
- Hit {key Command R} in Calendar.app, saw changes. (MAGIC!)
- This export included recurring events, which appeared the same way in Calendar.app and Phabricator.
- Can't import into Google Calendar from my local install easily since Google's servers can't hit my laptop, but I'll test once we deploy.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16679
Summary:
Ref T10747.
- Adds a "Use Results..." dropdown to query result pages, with actions you can take with search results (today: create export; in future: bulk edit, export as excel, make dashboard panel, etc).
- Allows you to create an export against a query key.
- I'm just using a text edit field for this for now.
- Fleshes out export modes. I plan to support: public (as though you were logged out), privileged (as though you were logged in) and availability (event times, but not details).
This does not actually export stuff yet.
Test Plan: Created some exports. Viewed and listed exports.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16676
Summary:
Ref T10747. Rough flow is:
- Run a query.
- Select a new "Export Events..." action.
- This lets you define an "Export", which has a unique URL you can paste into Google Calendar or Calendar.app or whatever.
Most of this does nothing yet but here's the boilerplate.
Test Plan: Doesn't do anything yet.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16675
Summary:
Ref T11326. This reorders sections:
- Description (if present)
- Recurring event series info (if recurring)
- Invitees (this also has custom stuff, if it exists)
Test Plan: Viewed some events, saw more sensible order.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11326
Differential Revision: https://secure.phabricator.com/D16671
Summary:
Ref T10747.
- Store recurrence as RRULEs internally.
- Use RRULE constants.
- Migrate existing rules to RRULEs.
Test Plan: Ran migration, nothing seemed broken?
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16670
Summary: Ref T10747. This drives event queries through RRULE, too.
Test Plan: Created recurring events, saw them appear correctly on the calendar.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16668
Summary:
Ref T10737. Today, we evalute recurrence twice: once when querying, and once in all other cases. This converts the second case to use the RRULE engine.
Next up is making the query use the RRULE engine, too.
Test Plan: Created a new recurring event, iterated through it by clicking "next instance", viewed it on Calendar view.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10737
Differential Revision: https://secure.phabricator.com/D16667
Summary:
Ref T10747. This deprecates "dateFrom", "dateTo", "allDayDateFrom", "allDayDateTo", and "recurrenceEndDate".
They are replaced with "utc*Epoch" fields (for querying) and CalendarDateTime objects (for start, end, until). These objects can represent the full range of dates and times expressible in ICS format, allowing us to import a wider range of ICS events.
Test Plan:
Ran migrations, viewed/edited Calendar, didn't catch anything catastrophcially broken.
This likely needs some followups, I'll keep it local for a bit until I'm confident I didn't break anything too catastrophically. I'm retaining the old data for now so we can likely fix things if it turns out there is some sort of issue.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16664
Summary: Ref T10747. Moves away from getDateFrom() / getDateTo() and makes a few more date/time methods more consistent.
Test Plan: Created, edited, viewed events.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16663
Summary: Ref T10747. The CalendarDateTime object now carries the viewer timezone as part of its state, so we don't need to have separate accessors.
Test Plan:
- Viewed events, checked that crumbs render properly.
- Edited events.
- Created new events.
- Viewed calendar.
- Viewed event detail pages.
- Viewed profile mini-calendar.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16662
Summary:
Ref T10747. This does double-writes and starts generating/writing CalendarDateTimes.
This greater flexibility is necessary to support the full range of ICS-specifiable events, including "floating" events.
This doesn't do anything yet.
Test Plan: Created and edited events, verified sensible representations of corresponding datetimes appeared in the database.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16661
Summary:
Ref T10747. Currently, Calendar events are mostly epoch-based and cheat a little bit for all-day events.
This already felt a little flimsy, and can't reasonably accommodate the full range of `.ics` events, which include "floating" events (e.g., occurs at 3PM regardless of timezone, like "Tea Time").
As a secondary issue, we identify instances of a recurring event by instance number (1, 2, 3, etc.). This can't accommodate the full range of `.ics` events, which include arbitrary additional "RDATE" events (e.g., recurrs every week, and also on these specific extra days).
However, we do need to store some epoch information so we can do query windowing: when the user looks at "October 2016", we want to select the smallest number of events that we can from the database initially, before refining them down to generate instances. We can't reasonably query the actual dates no matter how we store them because this depends on computing things like UNTIL, COUNT, initial dates, whether events are recurring or not, timezones, etc.
Instead, when we save an event compute the earliest second it occurs on in UTC and the latest second it occurs on in UTC. We can then query for a small superset of possible events in "October 2016" for any viewer pretty easily.
Also, start laying the groundwork for using fewer epochs in the rest of the code, and for reducing the role of sequence indexes (I plan to keep some sequences indexes around, probably, since they're nice in the UI, but not all child events will have indexes since there's no index for an RDATE event).
This doesn't migrate existing events yet or actually read these new columns -- that will come later once the new code is a little more solid.
Test Plan:
- Ran `bin/storage upgrade`.
- Created a new event.
- Saved an existing event.
- Viewed database, saw sensible-looking "UTC Epoch" values.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16652
Summary: This moves room pictures out of the dialog and into it's own PictureController. Also adds a standard image (and removes the "last person to chat" picture (though we could add that back. My plan is though that direct messages use auto use the other person's photo, after we have editengine and room pictures will have a plain, replaceable image.
Test Plan: Set a new room picture, remove a picture. Run migration, see old images properly set with new image.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11730
Differential Revision: https://secure.phabricator.com/D16669
Summary: Ref T11730. Removes the front end crop feature. Will follow up with proper removal, but this seems broken outright.
Test Plan:
Edit a room, don't seen "Crop" feature. Upload new photo, works fine.
- grep for `ConpherencePicCropControl`
- grep for `aphront-crop`
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11730
Differential Revision: https://secure.phabricator.com/D16665
Summary: This adds the room image to the main header in full Conpherence. It's nice, plus I plan to move the image edit workflow to it to simplify the move to EditEngine. I plan to build some default images for Conpherence which should be better about denoting the room, not just the last person writing.
Test Plan: Click on lots of rooms with and without topics.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16666
Summary: Not sure this ever worked correctly, but now once we have a supported action, skip the rest of the transactions. Currently you'll see a random old post.
Test Plan: Test multiple rooms in various states with new messages, edits, new room titles, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16660
Summary: Unclear these are worth sending, but mostly seems useful. Returns `getTitle` for the transaction if it's not a message. Fixes T10683
Test Plan: Leave rooms, change names, add pictures.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10683
Differential Revision: https://secure.phabricator.com/D16658
Summary: Provide higher resolution for Conpherence room images. Fixes T11728
Test Plan:
Upload a new photo, see it pulls in 200px image as background.
{F1858660}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11728
Differential Revision: https://secure.phabricator.com/D16659
Summary: Depending on when packages loaded, this CSS sometimes gets overwritten. Make it more specific and always present.
Test Plan: Reload a lot
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16657
Summary: Remove policy icons from durable column, create a basic nux layout and style.
Test Plan: leave all rooms, pop open chat, see helpful text and button.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16655
Summary:
Fix typo 'Branches' in the panel header for the Diffusion Actions
management panel.
Test Plan: Saw 'Actions' in the panel heading
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16654
Summary: More work to do here on the JS side, but this at least makes sure users with a small chat window have some notification marked that new replies have not been seen.
Test Plan: Open two windows. Window 1 has durable minimized, Window 2 is full conpherence. Send a message from Window 2, see header count in Window 1 increase. Repeat with durable open, see no change in window.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16650
Summary: A bit better styling, this adds an indication icon for if you're connected or not (and later, away, etc).
Test Plan: Test in Notifications menu, Conpherence full, Durable Column.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16647
Summary: This exposes the chat window to a larger audience beside people who accidentaly hit `\`.
Test Plan:
Lots of clicks and reloads.
{F1856043}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16643
Summary: Creates a background that renders inside the Quicksand frame, through sorcery.
Test Plan: Turn on Quicksand, visit lots of pages. See correct background colors. This probably blows something up I'm not testing.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16642
Summary: I missed removing this during the file purge of '16. Fixes T11717
Test Plan: Will test live
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11717
Differential Revision: https://secure.phabricator.com/D16639
Summary:
Since I plan to add collapsing, this widens the chat window and moves the switcher to the side, for more visual space for conversation.
TODO: make a magical minimizer so I can always have it open.
Test Plan:
Tested on my large display and little Macbook.
{F1854092}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16635
Summary: Fixes T11712. This is somewhat misleading with encryption enabled.
Test Plan: Viewed chunked and unchunked files.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11712
Differential Revision: https://secure.phabricator.com/D16636
Summary: Adds a CSS class if comments come in from the same user in the past 2 minutes for cleaner UI. Note will have to find some better display UI when comment editing comes.
Test Plan: Test lots of random Conpherence messages with different transactions, different people, and quick commenting.
Reviewers: scp, epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16632
Summary: This feels pretty reasonable with little effort, and I think I'd use it more than the full column.
Test Plan:
Chat a lot on various pages.... still some quicksand quirks around various pages.
{F1853487}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: scp, Korvin
Differential Revision: https://secure.phabricator.com/D16627
Summary: Adds a connection status message in Conpherence
Test Plan: Check status
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16625
Summary:
Fixes T11705. I did not realize that `ON DUPLICATE KEY UPDATE` was order-dependent, so the "reset" clause of this `IF(...)` never actually worked.
Reorder it so we check if we're changing the message type //first//, then actually change the message type.
This makes the count reset properly when a failing repository succeeds, or a working repository fails.
Test Plan:
- On `master`, forced a working repository to fail a `bin/repository update`, saw the message change types (expected) but keep the old count (wrong!).
- With this patch, repeated the process and saw the count reset properly.
- Ran the patch, verified counts reset to 0.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11705
Differential Revision: https://secure.phabricator.com/D16623
Summary:
Looking at IPs who recently registered more than one account in
Phabricator and trying to figure out whether they are spam bots
or just all on the same university network, I often want to check
recent user activity of these accounts. Hence linking the entries
in the User column to their user page comes in handy.
Test Plan: Tested on local instance and works as expected.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D16620
Summary: Ref T11217. This just adds the table that we'll store tokens in. It doesn't make use of the table at all yet. This is mostly pulled from this diff (D16178). Specifically I mostly followed Evan's instructions related to the token table here: D16178#189120.
Test Plan: I ran `./bin/storage upgrade` successfully and there were no schema errors.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T11217
Differential Revision: https://secure.phabricator.com/D16621
Summary: Fixes T11623. Enables send-on-enter and shift-enter for linebreaks, per durable column. Also cleaned up UI for Joining Room or Logging In.
Test Plan: See room I can join, click Join Room. Leave Room, Log out, visit room with login prompt. Login, Join Room again.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11623
Differential Revision: https://secure.phabricator.com/D16595
Summary: Fixes T10715. Badges on the profile view now link to the badge view
Test Plan: Went to the profile view and clicked the link.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim
Maniphest Tasks: T10715
Differential Revision: https://secure.phabricator.com/D16604
Summary: Fixes T9063. Removes the "Application" field from the search because it was largely redundant with the 'Name Contains' field.
Test Plan: Went to `/conduit/query/modern/`, clicked on `Edit Query` and noted that there is no "Application" field anymore. The 'Name Contains' field still works however.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T9063
Differential Revision: https://secure.phabricator.com/D16602
Summary: Fixes T10681. Adds a search API endpoint and an edit API endpoint for Phurl URLs. I still need to add the ability to search by name, alias, URL, and maybe description.
Test Plan: Test the methods through `/conduit/method/phurls.search/` and `/conduit/method/phurls.edit/`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T10681
Differential Revision: https://secure.phabricator.com/D16600
Summary:
Ref T4190. Added the remarkup rule to embed images:
Syntax is as follows:
`{image <IMAGE_URL>}`
Parameters are also supported, like:
`{image uri=<IMAGE_URI>, width=500px, height=200px, alt=picture of a moose, href=google.com}`
URLs without a protocol are not supported.
Test Plan: Tested with many of the syntax variations. If the provided URL doesn't point to an image, then a broken image icon will be shown.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T4190
Differential Revision: https://secure.phabricator.com/D16597
Summary: Somehow this got through last week :( It's a bug that causes the controller to... *ahem*... just not work. Luckily nothing uses this yet so nothing was really affected.
Test Plan: Hit `/file/imageproxy/?uri=http://i.imgur.com/nTvVrYN.jpg` and are served a nice picture of a bird
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim
Differential Revision: https://secure.phabricator.com/D16598
Summary:
Ref T11672. At low loads, this causes us to use more connections, which is pushing some installs over the default limits.
Rather than trying to walk users through changing `max_connections`, `open_files_limit`, `fs.file-max`, `ulimit`, etc., just put things back for now. After T11044 we should have headroom to use persistent connections within the default limits on all reasonable systems..
Test Plan: Loaded Phabricator, poked around.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11672
Differential Revision: https://secure.phabricator.com/D16591
Summary:
Ref T4190. Currently only have the endpoint and controller working. I added caching so subsequent attempts to proxy the same image should result in the same redirect URL. Still need to:
- Write a remarkup rule that uses the endpoint
Test Plan: Hit /file/imageproxy/?uri=http://i.imgur.com/nTvVrYN.jpg and are served the picture
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T4190
Differential Revision: https://secure.phabricator.com/D16581
Summary: Ref T11687. Subscription to Blogs comes with many additional features, don't lock people in.
Test Plan: Saw I was no longer subscribed.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11687
Differential Revision: https://secure.phabricator.com/D16589
Summary:
Fixes T11679. This application is probably vanishing into the aether eventually, but stop it from fataling for now.
Here's the glyph: ▛
It's like a fragment of a block of file data! Right? Obviously.
Test Plan: Visited `/phragment/` with glpyhs on, saw the glyph.
Reviewers: chad, avivey
Reviewed By: avivey
Subscribers: avivey, hach-que
Maniphest Tasks: T11679
Differential Revision: https://secure.phabricator.com/D16588
Summary: Fixes T11685. We missed this one straggler the recent conversion of Phurl to EditEngine, in T10673.
Test Plan: Visited `/phurl/?nux=1`, clicked "Shorten a URL".
Reviewers: chad, jcox
Reviewed By: jcox
Maniphest Tasks: T11685
Differential Revision: https://secure.phabricator.com/D16587
Summary:
Fixes T11683. Likely as a result of the persitent connections change, more users are seeing MySQL connection limit errors.
The persistent connections change means we use //fewer// connections at the high end, but I'm guessing PHP is keeping some more connections around in the pool, so while high-traffic hosts use fewer connections, low-traffic hosts now use more.
Raise an explicit setup warning about this. Users should be adjusting it anyway, there's no value to leaving it at extremely low default and connections are baiscally free until you run out of outbound ports.
Test Plan: {F1844630}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11683
Differential Revision: https://secure.phabricator.com/D16586
Summary:
Fixes T11677. This makes two minor adjustments to the repository import daemons:
- The first step ("Message") now queues at a slightly-lower-than-default (for already-imported repositories) or very-low (for newly importing repositories) priority level.
- The other steps now queue at "default" priority level. This is actually what they already did, but without this change their behavior would be to inherit the priority level of their parents.
This has two effects:
- When adding new repositories to an existing install, they shouldn't block other things from happening anymore.
- The daemons will tend to start one commit and run through all of its steps before starting another commit. This makes progress through the queue more even and predictable.
- Before, they did ALL the message tasks, then ALL the change tasks, etc. This works fine but is confusing/uneven/less-predictable because each type of task takes a different amount of time.
Test Plan:
- Added a new repository.
- Saw all of its "message" steps queue at priority 4000.
- Saw followups queue at priority 2000.
- Saw progress generally "finish what you started" -- go through the queue one commit at a time, instead of one type of task at a time.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11677
Differential Revision: https://secure.phabricator.com/D16585
Summary: Fixes T10673. Set up Phurl to use Edit Engine. There's no way this is all I needed to do to get it working, so I'll be making another pass at it and testing more thoroughly...
Test Plan: Ran through the Phurl URL creation/edit/deletion process.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T10673
Differential Revision: https://secure.phabricator.com/D16573
Ref T11665.
Without `-n 1`, this logs the ENTIRE history of the repository. We
actually get the right result, but this is egregiously slow. Add `-n 1`
to return only one result.
It appears that I wrote this wrong way back in 2011, in D953. This
query is rarely used (until recently) which is likely why it has
escaped notice for so long.
Test Plan: Used Conduit console to execute `diffusion.rawdiffquery`.
Got the same results but spent 8ms instead of 200ms executing this
command, in a very small repository.
Summary:
The commit which added checks for the old homepage options (now in
Dashboard) in rP9d9a47e9cf, added them to the auth section, where they
would present:
This option has been migrated to the "Auth" application. Your old
configuration is still in effect, but now stored in "Auth" instead of
configuration. Going forward, you can manage authentication from the
web UI.
Remove them from the moved-to-Auth list, and coalesce the multiple
definitions of the help text into one.
Test Plan:
- set maniphest.priorities.unbreak-now to something
- observe the setup issue reported
- hope it tells you the right thing
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, chad
Differential Revision: https://secure.phabricator.com/D16576
Summary:
Ref T11665. Currently, when a repository hits an error, we retry it after 15s. This is correct if the error was temporary/transient/config-related (e.g., bad network or administrator setting up credentials) but not so great if the error is long-lasting (completely bad authentication, invalid URI, etc), as it can pile up to a meaningful amount of unnecessary load over time.
Instead, record how many times in a row we've hit an error and adjust backoff behavior: first error is 15s, then 30s, 45s, etc.
Additionally, when computing the backoff for an empty repository, use the repository creation time as though it was the most recent commit. This is a good proxy which gives us reasonable backoff behavior.
This required removing the `CODE_WORKING` messages, since they would have reset the error count. We could restore them (as a different type of message), but I think they aren't particularly useful since cloning usually doesn't take too long and there's more status information avilable now than there was when this stuff was written.
Test Plan:
- Ran `bin/phd debug pull`.
- Saw sensible, increasing backoffs selected for repositories with errors.
- Saw sensible backoffs selected for empty repositories.
Reviewers: chad
Maniphest Tasks: T11665
Differential Revision: https://secure.phabricator.com/D16575
Summary:
Ref T11665. Fixes T7865. When we restart the daemons, the repository pull daemon currently resets the cooldowns on all of its pulls. This can generate a burst of initial load when restarting a lot of instance daemons (as in the Phacility cluster), described in T7865. This smooths things out so that recent pulls are considered, and any repositories which were waiting keep waiting.
Somewhat counterintuitively, hosted repositories write `TYPE_FETCH` status messages, so this should work equally well for hosted and observed repositories.
This also paves the way for better backoff behavior on repository errors, described in T11665. The error backoff now uses the same logic that the standard backoff does. The next change will make backoff computation consider recent errors.
(This is technically too large for repositories which have encountered one error and have a low commit rate, but I'll fix that in the following change; this is just a checkpoint on the way there.)
Test Plan: Ran `bin/phd debug pull`, saw the daemon compute reasonable windows based on previous pull activity.
Reviewers: chad
Maniphest Tasks: T7865, T11665
Differential Revision: https://secure.phabricator.com/D16574
Summary: Ref T8628.
Test Plan: Performed an action that uses the redirect controller (trying to visit a repo page while not logged in). Logged in and was redirected as expected
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim
Maniphest Tasks: T8628
Differential Revision: https://secure.phabricator.com/D16571
Summary: Ref T8628
Test Plan: Updated DarkConsoleDataController and observed that the darkconsole still works as expected
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T8628
Differential Revision: https://secure.phabricator.com/D16570
Summary: Fixes T11642. Added a 'name' field to the results from harbormaster.build.search.
Test Plan: Went to `/conduit/method/harbormaster.build.search/` and ran a search that would yield results (because otherwise there will be nothing there). Noted that there was, in fact, a name in the results.
Reviewers: yelirekim, #blessed_reviewers, epriestley
Reviewed By: yelirekim, #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim
Maniphest Tasks: T11642
Differential Revision: https://secure.phabricator.com/D16569
Summary: I believe these are left over from widgets, when we added a "Files" widget that kept track of everything added to the window.
Test Plan: Added files to a Conpherece, Set an image when editing. Anything else?
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16567
Summary: Roughly, if user isn't in any rooms, search for joinable ones. If no results, show big NUX banner.
Test Plan: Left all rooms, got fallback, joined room, left room. Create new instance, see new NUX. Set instance to public, visit Conpherence with and without public rooms.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16563
Summary: [Draft] Posting this up because feed is pulling `getTitle` and not `getTitleForFeed` and I'm super confused. Restarted phd and apache.
Test Plan: Create a new room, see link in feed. Change topic, see story, add people, don't see story.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11645
Differential Revision: https://secure.phabricator.com/D16561
Summary:
Ref T11650. Currently, we load packages and then discard the archived ones.
However, this gets "dominion" rules (where a more-general package gives up ownership if a more-specific package exists) wrong if the more-specific package is archived: we incorrectly give up ownership.
Instead, just ignore these packages completely when loading affected packages. This is slightly simpler.
(There are technically two pieces of code we have to do this for, which should be a single piece of code but which haven't yet been unified.)
Test Plan:
- Created packages:
- Package A, on "/" (strong dominion, autoreview).
- Package B, on "/x/" (weak dominion, autoreview).
- Package C, on "/x/y" (archived, autoreview).
- Create a revision affecting "/x/y".
- Saw correct path ownership in table of contents ("B", strongest package only).
- Saw correct autoreview behavior (A + B).
- (Prior to patch, in `master`, reproduced the problem behaviors described in T11650, with bad dominion rules and failure to autoreview B.)
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11650
Differential Revision: https://secure.phabricator.com/D16564
Summary: We no longer have widgets, don't need these calls for simplicity.
Test Plan: Add a user to a room, remove a user from a room. Grep for `needWidgetData`
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16562
Summary: Mostly quality of life in renames and moving everything to the view class. Minor CSS tweaks. Fix room handles getting added when adding a new user.
Test Plan: Add to room, remove from room.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16560
Summary:
Ref T10747.
- Remove the warning dialog since these files don't seem to do anything confusing/problematic in Calendar.app or Google Calendar. Those importers generally need to be defensive about how they handle random ".ics" files from arbitrary third parties anyway, and this makes testing imports easier since we have a GET-table ".ics" URI for public events.
- Attach ".ics" files to email.
Test Plan:
- Clicked "Export as .ics", got an ICS file.
- Used "bin/mail show-outbound" to review an ICS attachment, although I don't actually have real mail set up locally so this may still be a little funky.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16559
Summary: Ref T10747. This exports these sections when generating an ".ics" file.
Test Plan: {F1832214}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16558
Summary: Darkens the topic, adds fun colors to action icons.
Test Plan: Hover over new action icons. See darker topic text.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16556
Summary: Ref T10747. Allows you to grab an event as a (basic) ICS file.
Test Plan:
- Exported a normal event.
- Exported an all-day event.
{F1830577}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16553
Summary:
Minor rebuild / redesign of Conpherence. Most of this is new UX and tossing out things like widgets, device fallbacks. I expect some of the UI to get more polished after next pass, but most everything here is in place.
- Removed "Widgets", now just a single Participants pane
- Added "Topic"
- New header
- Settings, Edit are action icons
- Removed a lot of JS
- Simplified CSS as much as I could
Test Plan:
Desktop, Tablet, Mobile. Adding and removing people. Setting new topics, new rooms.
{F1828662}
{F1828669}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16550
Summary: These floats were wonky in FF, adding some hardening to make sure they clear.
Test Plan:
Firefox, Chrome, IE.
{F1815981}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16547
Summary: We currently try to build crumbs out even if the user has no membership in any. This removes these useless controls.
Test Plan: Leave all Rooms, see NUX state with no "Edit Room", blank title, or "Collapse Column" controls.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16546
Summary:
Fixes T11586. First pass at a class for displaying invisible characters. Still need to:
- Write a couple unit tests
- Add some styling to the .invisible-special spans
- Actually start using the class when displaying form errors to users
Currently this makes the string `"\nab\x00c\x01d\te\nf"` look like:
{F1812711}
Test Plan:
Unit tests all pass and run in <1ms:
{F1812998}
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T11586
Differential Revision: https://secure.phabricator.com/D16541
Summary: This adds a "column" icon into crumbs, like in workboards, for expanding or hiding the "Widget Pane". This is per user sticky and defaults to off.
Test Plan: View a Conpherence Room, see no widgets by default. Toggle it on, see widget. Reload page, see widget stick. Verify mobile, tablets ignore hiding.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10364
Differential Revision: https://secure.phabricator.com/D16533
Summary:
Moves search and new room out of crumb bar and into the thread column. This opens up space to add "Edit Room" into the crumbs area as an action link. Also removed 'widget-edit' pathways and javascript.
Fixes T8972
Test Plan:
Search, New, Edit rooms. Click different rooms, see correct URL each time.
{F1813226}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T8972
Differential Revision: https://secure.phabricator.com/D16543
Summary:
Fixes T11627.
Beyond being complex, I have no real reason to believe these checks even work (and they don't test repositories, file storage, logfiles, etc).
Test Plan:
Faked the error:
{F1813433}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11627
Differential Revision: https://secure.phabricator.com/D16544
Summary:
Ref T11613. In D16503/T11598 I refined the setup flow to improve messaging for early-stage setup issues, but failed to fully untangle things.
We sometimes still try to access a cache which uses configuration before we build configuration, which causes an error.
Instead, store "are we in flight / has setup ever worked?" in a separate cache which doesn't use the cache namespace. This stops us from trying to read config before building config.
Test Plan:
Hit bad extension error with a fake extension, got a proper setup help page:
{F1812803}
Solved the error, reloaded, broke things again, got a "friendly" page:
{F1812805}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11613
Differential Revision: https://secure.phabricator.com/D16542
Summary:
* All Applications are useing the description "Tags", Phriction actually uses "Projects"
* Changed "Projects" to "Tags" for phriction
Ref T11614
Test Plan:
* Edited a document, took a look at the description for that input line
* Added a project to a page and saved it
Reviewers: chad, #blessed_reviewers
Reviewed By: chad, #blessed_reviewers
Subscribers: avivey, chad, epriestley
Tags: #phriction
Maniphest Tasks: T11614
Differential Revision: https://secure.phabricator.com/D16537
Summary: Fixes T10155
Test Plan: View an empty repository in diffusion, check for the exception.
See T10155 for steps to reproduce
Reviewers: epriestley
Subscribers:
Summary: Cleans up a number of inconsistencies, makes Conpherence lighter, easier to read, make names larger, darker.
Test Plan: Review Conpherence chat in Main App, Durable Column, Mobile, Tablet
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16531
Summary: Just changes the default here. Main reasoning is if I'm searching for a room, show me all of them first. "My Rooms" is where I just came from.
Test Plan: Click Search in Conpherence, see All Rooms as default.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16527
Summary: Fixes T10781. Re-uses sidenav styles from global space, though a little hacky. Fix crumb border, fix nux experience. Fiddle spacing.
Test Plan:
Use Conpherence as new user, make a room, post some messages.
{F1805658}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10781
Differential Revision: https://secure.phabricator.com/D16528
Summary: Fixes T10131. Adds new CSS to better present document pages for printing. Added a print link to Phriction.
Test Plan: Tested Phriction, Phame, and Legalpad.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10131
Differential Revision: https://secure.phabricator.com/D16524
Summary:
Fixes T11610. Clean up some sketchy old code from long ago.
If you had rules that use conditions like "Accepted revision exists" and ran them in the test console, we'd never load the "CommitData" and fatal.
Instead, load CommitData in `newTestAdapter()` and generally make these pathways a little more modern.
Test Plan:
- Wrote an "Accepted Revision Exists" rule.
- Ran a commit in the test console.
- Before patch, got fatal from T11610.
- After patch, got clean test result.
- Also pushed a commit and reviewed the transcript to make sure the rule ran properly.
Reviewers: joshuaspence, chad
Reviewed By: chad
Maniphest Tasks: T11610
Differential Revision: https://secure.phabricator.com/D16522
Summary: Fixes T11567. This way people can use things like `sans-serif` and `-webkit-small-control` for their "monospaced" font
Test Plan:
I added the hyphen to the regex then was able to set my Monospaced Font to be anything with a hyphen in it.
I also tried to break it pretty extensively, but couldn't find anything that would let me write malicious CSS or JS.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim
Maniphest Tasks: T11567
Differential Revision: https://secure.phabricator.com/D16519
Summary: Fixes T11595. Previously if a user didn't have permissions to view an application it would still appear in the application typeahead in various menus. This change will prevent that by checking if the app is installed for the viewer before displaying it as an option
Test Plan: I went to the "Pin Applications" menu and typed in "Conpherence" and saw it appear as an option. I then went to the "Edit Policies" menu as an admin and removed permissions for my user to use Conpherence. Once I did that, it no longer showed up in the typeahead menu
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T11595
Differential Revision: https://secure.phabricator.com/D16518
Summary:
Fixes T11396. Currently, you can keep clicking "Next >" forever to generate infinite instances of an event, even if it has a set end date.
Likewise, you can visit `/E123/999999` or whatever to stub out the 999999th instance of an event.
Instead:
- Before creating a new stub, make sure it happens before any end date.
- 404 stubs if we can't create them.
- Disable the "Next >" button if it isn't valid.
Test Plan:
- Visited `/E123/9999` for an event with a recurrence end date, got 404.
- Clicked "Next >" on an event with an end date, got new events until I hit the end date.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11396
Differential Revision: https://secure.phabricator.com/D16517
Summary: If you don't have any rooms, we currently show no menu and users have no logical means of closing the column. This lets "Hide Column" at least still appear. Fixes T9195
Test Plan: Fresh install, hit {key \}, see menu. Close Column.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9195
Differential Revision: https://secure.phabricator.com/D16516
Summary:
Fixes T11604. If we send two requests to render a brand new tag at about the same time (say, 50ms apart) but JIRA takes more than 50ms to return from its API call, the two processes will race one another and try to save the same external object.
If they do, have whichever one lost the race just load the object the other one created.
Apply this to other bridges, too.
Test Plan:
- Created a new task in JIRA.
- Referenced it for the first time in Differential, in a comment.
- This causes two tag renders to fire. This //might// be a bug but I spend 30 seconds on it without figuring out what was up. Regardless, we should fix the race even if the reason it's triggering so easily legitimately is a bug.
- Before patch: big error dialog (as in T11604).
- After patch: smooth sailing.
{F1804008}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11604
Differential Revision: https://secure.phabricator.com/D16514
Summary: In new PHUITwoColumnLayout, some new colors were used that aren't in the High Contrast PostProcessor. Adding them and proper fallbacks.
Test Plan: High Contrast on/off on a timeline page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16515
Summary: It's super
Test Plan: Set lots of project icons
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16513
Summary: This fixes a bug where the icon won't display since it doesn't include `fa-`.
Test Plan: Change icon on a project, revisit timeline and see icon.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16512
Summary:
Ref T11593. When you call a `*.search` method like `maniphest.search`, we don't currently validate that all the constraints you pass are recognized.
I think there were two very weak arguments for not doing this:
- It makes compatibility in `arc` across versions slightly easier: if we add a new constraint, we could add it to `arc` but also do client-side filtering for a while.
- Conduit parameter types //could//, in theory, accept multiple inputs or optional/alias inputs.
These reasons are pretty fluff and T11593 is a concrete issue caused by not validating. Just validate instead.
Test Plan:
- Made a `maniphest.search` call with a bogus constraint, got an explicit error about the bad constraint.
- Made a `maniphest.search` call with a valid constraint (`"ids"`).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11593
Differential Revision: https://secure.phabricator.com/D16507
Summary:
Ref T11589. When we hit a fatal setup issue (essentially always a connection failure) //after// we've already survived them on at least one request, we can be pretty sure a server went down and that the problem is not a setup/configuration issue.
In this case, show a friendlier error page instead of the fairly detailed technical one.
Test Plan:
- Broke MySQL config.
- Restarted Apache.
- Got the "admin/setup" error page:
{F1803268}
- Fixed the MySQL config.
- Loaded any page, to put us "in flight".
- Broke MySQL config.
- Loaded any page.
- Got the friendly "in flight" error page:
{F1803271}
If you want to design this better, easiest way to get to it is:
- Set `mysql.port` to `9999` in `conf/local/local.json`.
- Reload any page while already running (don't restart).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11589
Differential Revision: https://secure.phabricator.com/D16503
Summary:
Ref T11589. Previously, when we failed to load database configuration we just continued anyway, in order to get to setup checks so we could raise a better error.
There was a small chance that this could lead to pages running in a broken state, where ONLY that connection failed and everything else worked. This was accidentally fixed by narrowing the exceptions we continue on in D16489.
However, this "fix" meant that users no longer got helpful setup instructions. Instead:
- Keep throwing these exceptions: it's bad to continue if we've failed to connect to the database.
- However, catch them and turn them into setup errors.
- Share all the setup code so these errors and setup check errors work the same way.
Test Plan:
- Intentionally broke `mysql.host` and `mysql.pass`.
- Loaded pages.
- Got good setup errors.
- Hit normal setup errors too.
- Put everything back.
- Swapped into cluster mode.
- Intentionally broke cluster mode, saw failover to readonly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11589
Differential Revision: https://secure.phabricator.com/D16501
Summary:
Ref T11589. This runs:
- preflight checks (critical checks: PHP version stuff, extensions);
- configuration;
- normal checks.
The PHP checks are split into critical ("bad version") and noncritical ("sub-optimal config").
I tidied up the extension checks slightly, we realistically depend on `cURL` nowadays.
Test Plan:
- Faked a preflight failure.
- Hit preflight check.
- Got expected error screen.
- Loaded normal pages.
- Hit a normal setup check.
- Used DarkConsole "Startup" tab to verify that preflight checks take <1ms to run (we run them on every page without caching, at least for now, but they only do trivial checks like PHP versions).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11589
Differential Revision: https://secure.phabricator.com/D16500
Summary:
Ref T11596. When exporting data from the Phacility cluster, we `bin/files migrate` data from S3 into a database dump on the `aux` tier.
With current semantics, this //moves// the data and destroys it in S3.
Add a `--copy` flag to //copy// the data instead. This leaves the old copy around, which is what we want for exports.
Test Plan:
- Ran `bin/files migrate` to go from `blob` to `disk` with `--copy`. Verified a copy was left in the database.
- Copied it back, verified a copy was left on disk (total: 2 database copies, 1 disk copy).
- Moved it back without copy, verified database was destroyed and disk was created (total: 1 database copy, 2 disk copies).
- Moved it back without copy, verified local disk was destroyed and blob was created (total: 2 datbabase copies, 1 disk copy).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11596
Differential Revision: https://secure.phabricator.com/D16497
Summary:
Ref T11589. Currently, initialization order is a bit tangled: we load configuration from the database, then later test if we can connect to the database.
Instead, I'm going to do: preflight checks ("PHP Version OK?", "Extensions installed?"), then configuration, then normal setup checks.
To prepare for this, flag core checks as "preflight" and add a setup panel to visually confirm that I didn't miss anything.
Test Plan: {F1803210}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11589
Differential Revision: https://secure.phabricator.com/D16499
Summary: For phabricator. Adds a Slack auth adapater and icon.
Test Plan:
Create a new Slack Application for login, generate id and secret. Activate login and registration for Slack. Create a new account with Slack credentials. Log out. Log in with Slack credentials. Set my avatar with Slack. Slack. Slack.
{F1802649}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16496
Summary:
Two minor issues that I caught in the log while fixing Phame permissions:
- We had a JS bug which would cause us to immediately generate two comment previews at the exact same time -- one for loading the page, and one for "switching to desktop". Instead, only generate the "switch to desktop" preview if we really switched to desktop from a different device layout.
- These two requests could end up reading/writing the VersionedDraft table at exactly the same time fairly often (e.g., after a comment submission, the page would load, send two preview requests at exactly the same time, and they'd race fairly reliably for me locally). If we do race, recover from the race.
Test Plan:
Submitted some Phame comments.
- No more error log errors about VersionedDraft keys.
- Saw only one preview request when loading the page instead of two.
Here's the specific stack trace I caught:
```
[Mon Sep 05 12:15:33.639930 2016] [:error] [pid 50608] [client 127.0.0.1:55278] [2016-09-05 14:15:33] EXCEPTION: (AphrontDuplicateKeyQueryException) #1062: Duplicate entry 'PHID-POST-fknnpzjnsdgc3rqobhst-PHID-USER-pr5rjpuilpfserepsd2k-13' for key 'key_object' at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:314]
[Mon Sep 05 12:15:33.640801 2016] [:error] [pid 50608] [client 127.0.0.1:55278] arcanist(head=master, ref.master=9e82ef979e81), corgi(head=master, ref.master=5b9171222bc9), instances(head=stable, ref.master=485bc8128198, ref.stable=2983bc917601), ledger(head=master, ref.master=4da4a24b8779), libcore(), phabricator(head=phame2, ref.master=4b6da9735ba7, ref.phame2=4b6da9735ba7), phutil(head=stable, ref.master=97f05269fdb1, ref.stable=c14343ee620e), services(head=stable, ref.master=1fcb5cdb7582, ref.stable=2d8088a5b4b3)
[Mon Sep 05 12:15:33.640815 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #0 <#2> AphrontBaseMySQLDatabaseConnection::throwCommonException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:348]
[Mon Sep 05 12:15:33.640830 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #1 <#2> AphrontBaseMySQLDatabaseConnection::throwQueryCodeException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:289]
[Mon Sep 05 12:15:33.640833 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #2 <#2> AphrontBaseMySQLDatabaseConnection::throwQueryException(mysqli) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:185]
[Mon Sep 05 12:15:33.640836 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #3 <#2> AphrontBaseMySQLDatabaseConnection::executeRawQuery(string) called at [<phutil>/src/xsprintf/queryfx.php:8]
[Mon Sep 05 12:15:33.640839 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #4 <#2> queryfx(AphrontMySQLiDatabaseConnection, string, string, string, array, string)
[Mon Sep 05 12:15:33.640841 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #5 <#2> call_user_func_array(string, array) called at [<phutil>/src/aphront/storage/connection/AphrontDatabaseConnection.php:42]
[Mon Sep 05 12:15:33.640844 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #6 <#2> AphrontDatabaseConnection::query(string, string, string, array, string) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1261]
[Mon Sep 05 12:15:33.640846 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #7 <#2> LiskDAO::insertRecordIntoDatabase(string) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1106]
[Mon Sep 05 12:15:33.640849 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #8 <#2> LiskDAO::insert() called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1075]
[Mon Sep 05 12:15:33.640851 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #9 <#2> LiskDAO::save() called at [<phabricator>/src/applications/draft/storage/PhabricatorVersionedDraft.php:65]
[Mon Sep 05 12:15:33.640854 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #10 <#2> PhabricatorVersionedDraft::loadOrCreateDraft(string, string, integer) called at [<phabricator>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1669]
[Mon Sep 05 12:15:33.640857 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #11 <#2> PhabricatorEditEngine::buildCommentResponse(PhamePost) called at [<phabricator>/src/applications/transactions/editengine/PhabricatorEditEngine.php:894]
[Mon Sep 05 12:15:33.640859 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #12 <#2> PhabricatorEditEngine::buildResponse() called at [<phabricator>/src/applications/phame/controller/post/PhamePostEditController.php:60]
[Mon Sep 05 12:15:33.640862 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #13 <#2> PhamePostEditController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:237]
[Mon Sep 05 12:15:33.640865 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #14 phlog(AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php:27]
[Mon Sep 05 12:15:33.640868 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #15 PhabricatorAjaxRequestExceptionHandler::handleRequestException(AphrontRequest, AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:644]
[Mon Sep 05 12:15:33.640870 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #16 AphrontApplicationConfiguration::handleException(AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:242]
[Mon Sep 05 12:15:33.640873 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #17 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:149]
[Mon Sep 05 12:15:33.640879 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #18 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:17]
```
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16494
Summary:
Fixes T11584. This controller does unnecessary CAN_EDIT policy checks.
These checks are enforced by `EditEngine`, and you can make certain types of edits (including comments) even without full-blown edit permission.
Test Plan:
- Commented as a user without edit permission.
- Tried to edit as a user without edit permission, was rebuffed with a policy dialog.
- Edited as a user with edit permission.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11584
Differential Revision: https://secure.phabricator.com/D16493
Summary:
Fixes T11590. Currently, we incorrectly consider cluster repository versions that are (or were) on devices which are no longer part of the active cluster service when building this status screen.
Instead, ignore them. This is just a display bug; the actual `ClusterEngine` already had similar logic.
Test Plan:
- Added a bad leader record to `repository_workingcopyversion`.
- Before patch, got a bad "Partial (1w)" sync:
{F1802292}
- After patch, got a good "Sycnchronized":
{F1802293}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11590
Differential Revision: https://secure.phabricator.com/D16492
