Summary: Fixes T11113. On the 2nd+ page, we could end up with an ambiguous `id` WHERE clause because we don't define a primary table alias on this query. Define one.
Test Plan:
Changed SearchEngine to return pages of size 5, searched for my threads, toggled to second page, no exception.
Used DarkConsole to examine that second-page query, saw that it had `thread.id` explicitly instead of `id` implicitly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11113
Differential Revision: https://secure.phabricator.com/D16080
Summary:
Via HackerOne. This page fatals if accessed directly while logged out.
The "shouldRequireLogin()" check is wrong; this is a logged-in page.
Test Plan:
Viewed the page while logged out, no more fatal.
Faked my way through the actual verification flow.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16077
Summary:
Ref T6916. Current rules tend to make videos gigantic. Just embed them at actual size, scaling them down if they're too big to fit.
Browsers generally provide some kind of "expand / fullscreen" element automatically anyway.
Test Plan: Viewed videos locally, saw them sized a little more reasonably.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6916
Differential Revision: https://secure.phabricator.com/D16076
Summary: Fixes T11107. The URI change here meant we were dropping the "key" parameter, which allows you to set a new password without knowing your old one.
Test Plan: Reset password, didn't need to provide old one anymore.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11107
Differential Revision: https://secure.phabricator.com/D16075
Summary: Ref T6916. Added video to remarkup using D7156 as reference.
Test Plan:
- Viewed video files (MP4, Ogg) in Safari, Chrome, Firefox (some don't work, e.g., OGG in Safari, but nothing we can really do about that).
- Used `alt`.
- Used `autoplay`.
- Used `loop`.
- Used `media=audio`.
- Viewed file detail page.
Reviewers: nateguchi2, chad, #blessed_reviewers
Reviewed By: chad, #blessed_reviewers
Subscribers: asherkin, ivo, joshuaspence, Korvin, epriestley
Tags: #remarkup
Maniphest Tasks: T6916
Differential Revision: https://secure.phabricator.com/D11297
Summary:
Fixes T10402.
I tried about 50 variations on the wording and notification layout, this seemed by far the most reasonable.
Didn't implement a way to ignore the warning, which might be required - but figured this is serious and broken enough while being completely invisible 99% of the time that it's worth shouting about.
Test Plan: Messed around with $_SERVER['HTTPS'] on the server side and client_uri on the client side - saw reasonable results in all combinations.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Maniphest Tasks: T10402
Differential Revision: https://secure.phabricator.com/D16064
Summary:
Ref T11098. This primarily fixes Conduit calls to `*.edit` methods failing when trying to access user preferences.
(The actual access is a little weird, since it seems like we're building some UI stuff inside a policy query, but that's an issue for another time.)
To fix this, consolidate the "we're about to run some kind of request with this user" code and run it consistently for web, conduit, and SSH sessions.
Additionally, make sure we swap things to the user's translation.
Test Plan:
- Ran `maniphest.edit` via `arc call-conduit`, no more settings exception.
- Set translation to ALL CAPS, got all caps output from `ssh` and Conduit.
Reviewers: avivey, chad
Reviewed By: chad
Maniphest Tasks: T11098
Differential Revision: https://secure.phabricator.com/D16066
Summary:
Ran into this while fixing T11098#179088.
The "Transaction Type" details in the conduit autogenerated documentation for `*.edit` endpoints still wraps incorrectly.
Test Plan: Purged remarkup cache, reloaded page, got full-width text.
Reviewers: avivey, chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16065
Summary:
Ref T7643.
- When a transaction edits a text block, add a link to the changes (for HTML mail).
- Also, inline the changes in the mail (for HTML mail).
- Do nothing for text mail since I don't think we really have room? And I don't know how we can make the diff look any good.
Test Plan:
Edited a task description, generated mail, examined mail.
- It contained a link leading to a prose diff.
- It had a more-or-less reasonable inline text diff.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7643
Differential Revision: https://secure.phabricator.com/D16063
Summary:
Ref T10785. Around the time we launched Phacility SAAS we implemented this weird autologin hack. It works fine, so clean it up, get rid of the `instanceof` stuff, and support it for any OAuth2 provider.
(We could conceivably support OAuth1 as well, but no one has expressed an interest in it and I don't think I have any OAuth1 providers configured correctly locally so it would take a little bit to set up and test.)
Test Plan:
- Configured OAuth2 adapters (Facebook) for auto-login.
- Saw no config option on other adapters (LDAP).
- Nuked all options but one, did autologin with Facebook and Phabricator.
- Logged out, got logout screen.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10785
Differential Revision: https://secure.phabricator.com/D16060
Summary: Just adds a little more space to the quick create menu.
Test Plan: Test stock and modded quick create menu.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16057
Summary:
Ref T10856. The rendering logic was already there, but it was expecting the information under `properties`
field, whereas arc puts it under `metadata`. Not sure if that something that changed a long time ago or if
it was always like this.
Test Plan: {F1252657 size=full}
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T10856
Differential Revision: https://secure.phabricator.com/D15828
Summary:
Ref T3353. This hooks the prose engine up to the UI and throws away the hard-wrapping hacks.
These are likely still very rough in many cases, but are hopefully a big step forward from the old version in the vast majority of cases.
Test Plan: {F1677809}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T3353
Differential Revision: https://secure.phabricator.com/D16056
Summary: Ref T10811. This is a companion change for D16053, but affects the Phabricator version of this script.
Test Plan: Started daemons, ^C'd them, saw them handle the signal.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10811
Differential Revision: https://secure.phabricator.com/D16054
Summary:
Fixes T11088. When a task is removed from a project, we don't normally delete its column positions. If you accidentally remove a project and then restore the project, it's nice for the task to stay where you put it.
However, we do need to remove its positions in proxy columns to avoid the issue in T11088.
Test Plan:
- Added a failing unit test, made it pass.
- Added a task to "X > Milestone 1", loaded workboard, used "Edit Projects" to move it to "X" instead, loaded workboard.
- Before, it stayed in the "Milestone 1" column.
- After, it moves to the "Backlog" column.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11088
Differential Revision: https://secure.phabricator.com/D16052
Summary:
Ref T11098.
- Allow "Editor" to be set to the empty string.
- Don't match a validation error to a field unless the actual settings for the field and error match.
Test Plan:
- Tried to set "Editor" to "", success.
- Tried to set "Editor" to "javascript://", only that field got marked "Invalid".
Reviewers: avivey, chad
Reviewed By: chad
Maniphest Tasks: T11098
Differential Revision: https://secure.phabricator.com/D16051
Summary: Ref T11098. Template preferences don't have a user, but this codepath didn't get fully updated to account for that.
Test Plan: Saved mail tags in global prefernces.
Reviewers: avivey, chad
Reviewed By: chad
Maniphest Tasks: T11098
Differential Revision: https://secure.phabricator.com/D16050
Summary:
Ref T11098. We have a fair number of these, including links in email, which we can't turn into explicit `/user/` URIs.
Just redirect them to the modern places.
Test Plan: Clicked "Customize Menu..." on home page.
Reviewers: chad, avivey
Reviewed By: avivey
Subscribers: avivey
Maniphest Tasks: T11098
Differential Revision: https://secure.phabricator.com/D16049
Summary:
Ref T4103. This just adds a single global default setting group, not full profiles.
Primarily, I'm not sure how administrators are supposed to set profiles for users, since most ways user accounts get created don't really support setting roles.. When we figure that out, it should be reasonably easy to extend this. There also isn't much of a need for this now, since pretty much everyone just wants to turn off mail.
Test Plan:
- Edited personal settings.
- Edited global settings.
- Edited a bot's settings.
- Tried to edit some other user's settings.
- Saw defaults change appropriately as I edited global and personal settings.
{F1677266}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16048
Summary:
Ref T4103. If the database has `""` (empty string) for select/option settings, we can let that value be effective in the UI right now.
One consequence is that timestamps can vanish from the UI.
Instead, be stricter and discard it as an invalid value.
Test Plan:
- Forced `time-format` setting to `''`.
- Saw timestamps vanish before change.
- Saw timestamps return to the default value after change.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16047
Summary:
Ref T4103. Two small improvements:
- Don't work as hard to validate translations. We just need to know if a translation exists, we don't need to count how many strings it has and build the entire menu.
- Allow `getUserSetting()` to work on any setting without doing all the application/visibility checks. It's OK for code to look at, say, your "Conpherence Notifications" setting even if that application is not installed for you.
Test Plan: Used XHProf and saw 404 page drop from ~60ms to ~40ms locally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16046
Summary:
Ref T10077. Currently, we issue 6+ queries on every page to build this menu, since the menu is built application-by-application.
Build the menu with dedicated modules instead so a single "EditEngine" module can provide all of them with one query.
I'd like to reduce this to 0 queries but I'm not totally sure what we want to do with this menu.
This change removes these items, because EditEngine can not currently provide them:
- Calendar: Eventually via EditEngine eventually.
- Conpherence: Probably via EditEngine, doesn't seem too important.
- People: Maybe via EditEngine, doesn't seem too important? "Welcome" is likely better?
- Pholio: Eventually via EditEngine.
It adds a bunch of other items as a side effect:
{F1677151}
This reduces the queries issued on every page by ~5.
This also makes quick create actions visible while logged out (see T7073).
Test Plan:
- Viewed menu while logged in.
- Viewed menu while logged out.
- Viewed standalone version of menu.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10077
Differential Revision: https://secure.phabricator.com/D16045
Summary:
Ref T10078. Currently, you toggle DarkConsole and then load a page, but on the load we have to refill your settings cache since toggling DarkConsole dirtied it.
This is fine, except that it makes it harder to understand what's going on with queries on a page. Just force it to reload right away instead.
Test Plan: Toggled DarkConsole, reloaded page, no longer saw settings toggle-related cache fill.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10078
Differential Revision: https://secure.phabricator.com/D16044
Summary: Ref T4103. Ref T10078. We currently have separate "usable" and "raw" values, but can simplify this by making `newValueForUsers()` return the raw value.
Test Plan: Ran unit tests; browsed around; dropped caches and browsed around.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103, T10078
Differential Revision: https://secure.phabricator.com/D16043
Summary:
Ref T4103. Ref T10078. This puts a user cache in front of notification and message counts.
This reduces the number of queries issued on every page by 4 (2x building the menu, 2x building Quicksand data).
Also fixes some minor issues:
- Daemons could choke on sending mail in the user's translation.
- No-op object updates could fail in the daemons.
- Questionable data access pattern in the file query coming out of the profile file cache.
Test Plan:
- Sent myself notifications. Saw count go up.
- Cleared them by visiting objects and clearing all notifications. Saw count go down.
- Sent myself messages. Saw count go up.
- Cleared them by visiting threads. Saw count go down.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103, T10078
Differential Revision: https://secure.phabricator.com/D16041
Summary:
Ref T4103. Ref T10078. This moves profile image caches to new usercache infrastructure.
These dirty automatically based on configuration and User properties, so add some stuff to make that happen.
This reduces the number of queries issued on every page by 1.
Test Plan: Browsed around, changed profile image, viewed as self, viewed as another user, verified no more query to pull this information on every page
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103, T10078
Differential Revision: https://secure.phabricator.com/D16040
Summary:
Ref T4103. Ref T10078. Currently, when a user misses a cache we just build it for them.
This is the behavior we want for the the viewer (so we don't have to build every cache up front if we don't actually need them), but not the right behavior for other users (since it allows performance problems to go undetected).
Make inline cache generation strict by default, then make sure all the things that rely on cache data request the correct data (well, all of the things identified by unit tests, at least: there might be some more stuff I haven't hit yet).
This fixes test failures in D16040, and backports a piece of that change.
Test Plan: Identified and then fixed failures with `arc unit --everything`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103, T10078
Differential Revision: https://secure.phabricator.com/D16042
Summary: Ref T4103. This method has no more callers.
Test Plan: `grep`
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16039
Summary:
Ref T4103. This isn't completely perfect but should let us move forward without also expanding scope into "too much mail".
I split the existing "Mail Preferences" into two panels: a "Mail Delivery" panel for the EditEngine settings, and a "2000000 dropdowns" panel for the two million dropdowns. This one retains the old code more or less unmodified.
Test Plan:
- Ran unit tests, which cover most of this stuff.
- Grepped for all removed constants.
- Ran migrations, inspected database results.
- Changed settings in both modified panels.
- This covers a lot of ground, but anything I missed will hopefully be fairly obvious.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16038
Summary: Ref T4103. Also get rid of the weird cache clear that nothing else uses and which we don't actually need.
Test Plan:
- Resolved timezone conflict by ignoring it.
- Resolved timezone conflict by picking a valid timezone.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16037
Summary: Fixes T8846. Ref T4103. I just took the shortest reasonable path here, this panel could use some attention on the next Conpherence iteration.
Test Plan: Turned on/off desktop notifications. Observed corresponding behavior in test notifications.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103, T8846
Differential Revision: https://secure.phabricator.com/D16036
Summary:
Ref T4103. Some settings (mostly nav collapsed/expanded states) use this endpoint to make adjustments when users press keys (like `\` to toggle the durable column).
All of these settings are now formal, so swap things over to transactions.
Test Plan: Collapsed/expanded various navs, reloaded pages, settings stuck.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16035
Summary: Ref T4103. Provide a CLI mechanism for purging the user cache.
Test Plan:
- Purged with `--purge-user` and `--purge-all`.
- Verified cache table got wiped.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16033
Summary: Ref T4103. Fully modernize the filetree show/hide, durable column show/hide, and profile menu collapse/wide settings.
Test Plan:
- Toggled filetree on/off, reloaded page, setting stuck.
- Same with conpherence column and profile menus.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16034
Summary: Ref T4103. This primarily makes sure the console gets turned on when you enable it so you aren't like "where's the console???"
Test Plan: Enabled console, saw console.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16030
Summary:
Ref T4103. These settings long-predate proper settings and are based on hard-coded user properties. Turn them into real settings.
(I didn't try to migrate the value since they're trivial to restore and only useful to developers.)
Test Plan:
- Toggled console on/off.
- Swapped tabs.
- Reloaded page, everything stayed sticky.
Reviewers: chad
Reviewed By: chad
Subscribers: eadler
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16029
Summary:
Ref T4103. A few bits here:
- We have an ancient "tiles" preference which was just a fallback from 2-3 years ago. Throw that away.
- Modenize the other pinned stuff. We should likely revisit this after the next homepage update but I just left the actual defaults alone for now.
- Lightly prepare for global default editing.
- Add a "reset to defaults" option.
Test Plan:
- Pinned, unpinned, reordered and reset application homepage order.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16028
Summary:
Ref T4103. Convert this into a proper internal setting and use transactions to mutate it.
Also remove some no-longer-used old non-modular settings constants.
Test Plan:
- Used policy dropdown, saw recently-used projects.
- Selected some new projects, saw them appear.
- Grepped for all removed constants.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16027
Summary: Ref T4103. Modernize the blame/color toggles in Diffusion. These have no separate settings UI.
Test Plan: Toggled blame and colors, reloaded pages, settings stuck.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16026
Summary:
Ref T4103. Conpherence is doing some weird stuff and has its own redudnant settings object.
- Get rid of `ConpherenceSettings`.
- Use `getUserSetting()` instead of `loadPreferences()`.
- When applying transactions, add a new mechanism to efficiently prefill caches (this will still work anyway, but it's slower if we don't bulk-fetch).
Test Plan:
- Changed global Conpherence setting.
- Created a new Conpherence, saw setting set to global default.
- Changed local room setting.
- Submitted messages.
- Saw cache prefill for all particpiants in database.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16025
Summary: Ref T4103. This isn't necessary or particularly useful anymore since panels have been converted.
Test Plan: Visited URI, got a 404.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16024
Summary: Ref T4103. This converts other straightforward panels to modern stuff.
Test Plan:
- Edited various settings.
- Tried to set a bogus editor value.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16023
Summary: Ref T4103. Only trick here is hiding the panel if Conpherence is not installed.
Test Plan:
- Edited Conpherence preferences.
- Uninstalled Conpherence, saw panel vanish.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16022
