1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-19 12:00:55 +01:00

Continue modernizing application access to user preferences

Summary:
Ref T4103. This is just incremental cleanup:

  - Add "internal" settings, which aren't editable via the UI. They can still do validation and run through the normal pathway. Move a couple settings to use this.
  - Remove `getPreference()` on `PhabricatorUser`, which was a sort of prototype version of `getUserSetting()`.
  - Make `getUserSetting()` validate setting values before returning them, to improve robustness if we change allowable values later.
  - Add a user setting cache, since reading user settings was getting fairly expensive on Calendar.
  - Improve performance of setting validation for timezone setting (don't require building/computing all timezone offsets).
  - Since we have the cache anyway, make the timezone override a little more general in its approach.
  - Move editor stuff to use `getUserSetting()`.

Test Plan:
  - Changed search scopes.
  - Reconciled local and server timezone settings by ignoring and changing timezones.
  - Changed date/time settings, browsed Calendar, queried date ranges.
  - Verified editor links generate properly in Diffusion.
  - Browsed around with time/date settings looking at timestamps.
  - Grepped for `getPreference()`, nuked all the ones coming off `$user` or `$viewer` that I could find.
  - Changed accessiblity to high-contrast colors.
  - Ran all unit tests.
  - Grepped for removed constants.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4103

Differential Revision: https://secure.phabricator.com/D16015
This commit is contained in:
epriestley 2016-06-02 17:12:15 -07:00
parent 57c2f61b75
commit 67482fd19d
18 changed files with 171 additions and 100 deletions

View file

@ -2594,6 +2594,7 @@ phutil_register_library_map(array(
'PhabricatorInlineCommentPreviewController' => 'infrastructure/diff/PhabricatorInlineCommentPreviewController.php',
'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php',
'PhabricatorInstructionsEditField' => 'applications/transactions/editfield/PhabricatorInstructionsEditField.php',
'PhabricatorInternalSetting' => 'applications/settings/setting/PhabricatorInternalSetting.php',
'PhabricatorInternationalizationManagementExtractWorkflow' => 'infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php',
'PhabricatorInternationalizationManagementWorkflow' => 'infrastructure/internationalization/management/PhabricatorInternationalizationManagementWorkflow.php',
'PhabricatorInvalidConfigSetupCheck' => 'applications/config/check/PhabricatorInvalidConfigSetupCheck.php',
@ -3353,6 +3354,7 @@ phutil_register_library_map(array(
'PhabricatorSearchResultBucketGroup' => 'applications/search/buckets/PhabricatorSearchResultBucketGroup.php',
'PhabricatorSearchResultView' => 'applications/search/view/PhabricatorSearchResultView.php',
'PhabricatorSearchSchemaSpec' => 'applications/search/storage/PhabricatorSearchSchemaSpec.php',
'PhabricatorSearchScopeSetting' => 'applications/settings/setting/PhabricatorSearchScopeSetting.php',
'PhabricatorSearchSelectController' => 'applications/search/controller/PhabricatorSearchSelectController.php',
'PhabricatorSearchSelectField' => 'applications/search/field/PhabricatorSearchSelectField.php',
'PhabricatorSearchStringListField' => 'applications/search/field/PhabricatorSearchStringListField.php',
@ -3538,6 +3540,7 @@ phutil_register_library_map(array(
'PhabricatorTimeFormatSetting' => 'applications/settings/setting/PhabricatorTimeFormatSetting.php',
'PhabricatorTimeGuard' => 'infrastructure/time/PhabricatorTimeGuard.php',
'PhabricatorTimeTestCase' => 'infrastructure/time/__tests__/PhabricatorTimeTestCase.php',
'PhabricatorTimezoneIgnoreOffsetSetting' => 'applications/settings/setting/PhabricatorTimezoneIgnoreOffsetSetting.php',
'PhabricatorTimezoneSetting' => 'applications/settings/setting/PhabricatorTimezoneSetting.php',
'PhabricatorTimezoneSetupCheck' => 'applications/config/check/PhabricatorTimezoneSetupCheck.php',
'PhabricatorTitleGlyphsSetting' => 'applications/settings/setting/PhabricatorTitleGlyphsSetting.php',
@ -7196,6 +7199,7 @@ phutil_register_library_map(array(
'PhabricatorInlineCommentPreviewController' => 'PhabricatorController',
'PhabricatorInlineSummaryView' => 'AphrontView',
'PhabricatorInstructionsEditField' => 'PhabricatorEditField',
'PhabricatorInternalSetting' => 'PhabricatorSetting',
'PhabricatorInternationalizationManagementExtractWorkflow' => 'PhabricatorInternationalizationManagementWorkflow',
'PhabricatorInternationalizationManagementWorkflow' => 'PhabricatorManagementWorkflow',
'PhabricatorInvalidConfigSetupCheck' => 'PhabricatorSetupCheck',
@ -8103,6 +8107,7 @@ phutil_register_library_map(array(
'PhabricatorSearchResultBucketGroup' => 'Phobject',
'PhabricatorSearchResultView' => 'AphrontView',
'PhabricatorSearchSchemaSpec' => 'PhabricatorConfigSchemaSpec',
'PhabricatorSearchScopeSetting' => 'PhabricatorInternalSetting',
'PhabricatorSearchSelectController' => 'PhabricatorSearchBaseController',
'PhabricatorSearchSelectField' => 'PhabricatorSearchField',
'PhabricatorSearchStringListField' => 'PhabricatorSearchField',
@ -8304,6 +8309,7 @@ phutil_register_library_map(array(
'PhabricatorTimeFormatSetting' => 'PhabricatorSelectSetting',
'PhabricatorTimeGuard' => 'Phobject',
'PhabricatorTimeTestCase' => 'PhabricatorTestCase',
'PhabricatorTimezoneIgnoreOffsetSetting' => 'PhabricatorInternalSetting',
'PhabricatorTimezoneSetting' => 'PhabricatorOptionGroupSetting',
'PhabricatorTimezoneSetupCheck' => 'PhabricatorSetupCheck',
'PhabricatorTitleGlyphsSetting' => 'PhabricatorSelectSetting',

View file

@ -64,8 +64,8 @@ final class AphrontAjaxResponse extends AphrontResponse {
if ($request) {
$viewer = $request->getViewer();
if ($viewer) {
$postprocessor_key = $viewer->getPreference(
PhabricatorUserPreferences::PREFERENCE_RESOURCE_POSTPROCESSOR);
$postprocessor_key = $viewer->getUserSetting(
PhabricatorAccessibilitySetting::SETTINGKEY);
if (strlen($postprocessor_key)) {
$response->setPostprocessorKey($postprocessor_key);
}

View file

@ -66,7 +66,9 @@ final class PhabricatorUser
private $authorities = array();
private $handlePool;
private $csrfSalt;
private $timezoneOverride;
private $settingCacheKeys = array();
private $settingCache = array();
protected function readField($field) {
switch ($field) {
@ -481,19 +483,50 @@ final class PhabricatorUser
public function getUserSetting($key) {
// NOTE: We store available keys and cached values separately to make it
// faster to check for `null` in the cache, which is common.
if (isset($this->settingCacheKeys[$key])) {
return $this->settingCache[$key];
}
$settings_key = PhabricatorUserPreferencesCacheType::KEY_PREFERENCES;
$settings = $this->requireCacheData($settings_key);
if (array_key_exists($key, $settings)) {
return $settings[$key];
}
$defaults = PhabricatorSetting::getAllEnabledSettings($this);
if (isset($defaults[$key])) {
return $defaults[$key]->getSettingDefaultValue();
if (array_key_exists($key, $settings)) {
$value = $settings[$key];
// Make sure the value is valid before we return it. This makes things
// more robust when options are changed or removed.
if (isset($defaults[$key])) {
try {
id(clone $defaults[$key])
->setViewer($this)
->assertValidValue($value);
$this->settingCacheKeys[$key] = true;
$this->settingCache[$key] = $value;
return $value;
} catch (Exception $ex) {
// Fall through below and return the default value.
}
}
}
return null;
if (isset($defaults[$key])) {
$value = id(clone $defaults[$key])
->setViewer($this)
->getSettingDefaultValue();
} else {
$value = null;
}
$this->settingCacheKeys[$key] = true;
$this->settingCache[$key] = $value;
return $value;
}
@ -510,15 +543,25 @@ final class PhabricatorUser
return ($actual == $value);
}
/**
* @task settings
*/
public function clearUserSettingCache() {
$this->settingCacheKeys = array();
$this->settingCache = array();
$settings_key = PhabricatorUserPreferencesCacheType::KEY_PREFERENCES;
$this->clearCacheData($settings_key);
return $this;
}
public function getTranslation() {
return $this->getUserSetting(PhabricatorTranslationSetting::SETTINGKEY);
}
public function getTimezoneIdentifier() {
if ($this->timezoneOverride) {
return $this->timezoneOverride;
}
return $this->getUserSetting(PhabricatorTimezoneSetting::SETTINGKEY);
}
@ -533,7 +576,9 @@ final class PhabricatorUser
* @task settings
*/
public function overrideTimezoneIdentifier($identifier) {
$this->timezoneOverride = $identifier;
$timezone_key = PhabricatorTimezoneSetting::SETTINGKEY;
$this->settingCacheKeys[$timezone_key] = true;
$this->settingCache[$timezone_key] = $identifier;
return $this;
}
@ -578,17 +623,17 @@ final class PhabricatorUser
$line,
PhabricatorRepository $repository = null) {
$editor = $this->loadPreferences()->getPreference(
PhabricatorUserPreferences::PREFERENCE_EDITOR);
$editor = $this->getUserSetting(PhabricatorEditorSetting::SETTINGKEY);
if (is_array($path)) {
$multiedit = $this->loadPreferences()->getPreference(
PhabricatorUserPreferences::PREFERENCE_MULTIEDIT);
$multi_key = PhabricatorEditorMultipleSetting::SETTINGKEY;
$multiedit = $this->getUserSetting($multi_key);
switch ($multiedit) {
case '':
case PhabricatorEditorMultipleSetting::VALUE_SPACES:
$path = implode(' ', $path);
break;
case 'disable':
case PhabricatorEditorMultipleSetting::VALUE_SINGLE:
default:
return null;
}
}
@ -848,48 +893,13 @@ final class PhabricatorUser
$format = 'M j';
} else {
// Same year, month and day so show a time of day.
$pref_time = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT;
$format = $this->getPreference($pref_time);
$pref_time = PhabricatorTimeFormatSetting::SETTINGKEY;
$format = $this->getUserSetting($pref_time);
}
return $when->format($format);
}
public function getPreference($key) {
$preferences = $this->loadPreferences();
// TODO: After T4103 and T7707 this should eventually be pushed down the
// stack into modular preference definitions and role profiles. This is
// just fixing T8601 and mildly anticipating those changes.
$value = $preferences->getPreference($key);
$allowed_values = null;
switch ($key) {
case PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT:
$allowed_values = array(
'g:i A',
'H:i',
);
break;
case PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT:
$allowed_values = array(
'Y-m-d',
'n/j/Y',
'd-m-Y',
);
break;
}
if ($allowed_values !== null) {
$allowed_values = array_fuse($allowed_values);
if (empty($allowed_values[$value])) {
$value = head($allowed_values);
}
}
return $value;
}
public function __toString() {
return $this->getUsername();
}

View file

@ -30,7 +30,7 @@ final class PhabricatorSettingsTimezoneController
if ($request->isFormPost()) {
$timezone = $request->getStr('timezone');
$pref_ignore = PhabricatorUserPreferences::PREFERENCE_IGNORE_OFFSET;
$pref_ignore = PhabricatorTimezoneIgnoreOffsetSetting::SETTINGKEY;
$pref_timezone = PhabricatorTimezoneSetting::SETTINGKEY;
$preferences = $viewer->loadPreferences();
@ -56,8 +56,7 @@ final class PhabricatorSettingsTimezoneController
->setPreference($pref_timezone, $timezone)
->save();
$viewer->clearCacheData(
PhabricatorUserPreferencesCacheType::KEY_PREFERENCES);
$viewer->clearUserSettingCache();
}
}

View file

@ -22,7 +22,7 @@ final class PhabricatorDateTimeSettingsPanel extends PhabricatorSettingsPanel {
$pref_time = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT;
$pref_date = PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT;
$pref_week_start = PhabricatorUserPreferences::PREFERENCE_WEEK_START_DAY;
$pref_ignore = PhabricatorUserPreferences::PREFERENCE_IGNORE_OFFSET;
$pref_ignore = PhabricatorTimezoneIgnoreOffsetSetting::SETTINGKEY;
$preferences = $user->loadPreferences();
$errors = array();

View file

@ -0,0 +1,4 @@
<?php
abstract class PhabricatorInternalSetting
extends PhabricatorSetting {}

View file

@ -0,0 +1,16 @@
<?php
final class PhabricatorSearchScopeSetting
extends PhabricatorInternalSetting {
const SETTINGKEY = 'search-scope';
public function getSettingName() {
return pht('Search Scope');
}
public function getSettingDefaultValue() {
return 'all';
}
}

View file

@ -89,6 +89,10 @@ abstract class PhabricatorSetting extends Phobject {
return;
}
public function assertValidValue($value) {
$this->validateTransactionValue($value);
}
public function getTransactionNewValue($value) {
return $value;
}

View file

@ -0,0 +1,12 @@
<?php
final class PhabricatorTimezoneIgnoreOffsetSetting
extends PhabricatorInternalSetting {
const SETTINGKEY = 'time.offset.ignore';
public function getSettingName() {
return pht('Timezone Ignored Offset');
}
}

View file

@ -13,6 +13,31 @@ final class PhabricatorTimezoneSetting
return date_default_timezone_get();
}
public function assertValidValue($value) {
// NOTE: This isn't doing anything fancy, it's just a much faster
// validator than doing all the timezone calculations to build the full
// list of options.
if (!$value) {
return;
}
static $identifiers;
if ($identifiers === null) {
$identifiers = DateTimeZone::listIdentifiers();
$identifiers = array_fuse($identifiers);
}
if (isset($identifiers[$value])) {
return;
}
throw new Exception(
pht(
'Timezone "%s" is not a valid timezone identiifer.',
$value));
}
protected function getSelectOptionGroups() {
$timezones = DateTimeZone::listIdentifiers();
$now = new DateTime('@'.PhabricatorTime::getNow());

View file

@ -24,8 +24,6 @@ final class PhabricatorUserPreferences
const PREFERENCE_VARY_SUBJECT = 'vary-subject';
const PREFERENCE_HTML_EMAILS = 'html-emails';
const PREFERENCE_SEARCH_SCOPE = 'search-scope';
const PREFERENCE_DIFFUSION_BLAME = 'diffusion-blame';
const PREFERENCE_DIFFUSION_COLOR = 'diffusion-color';
@ -46,7 +44,6 @@ final class PhabricatorUserPreferences
const PREFERENCE_PROFILE_MENU_COLLAPSED = 'profile-menu.collapsed';
const PREFERENCE_FAVORITE_POLICIES = 'policy.favorites';
const PREFERENCE_IGNORE_OFFSET = 'time.offset.ignore';
// These are in an unusual order for historic reasons.
const MAILTAG_PREFERENCE_NOTIFY = 0;

View file

@ -113,7 +113,7 @@ final class AphrontFormDateControl extends AphrontFormControl {
return $result;
}
$readable = $this->formatTime($epoch, 'Y!m!d!g:i A');
$readable = $this->formatTime($epoch, 'Y!m!d!'.$this->getTimeFormat());
$readable = explode('!', $readable, 4);
$year = $readable[0];
@ -140,13 +140,15 @@ final class AphrontFormDateControl extends AphrontFormControl {
}
private function getTimeFormat() {
return $this->getViewer()
->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT);
$viewer = $this->getViewer();
$time_key = PhabricatorTimeFormatSetting::SETTINGKEY;
return $viewer->getUserSetting($time_key);
}
private function getDateFormat() {
return $this->getViewer()
->getPreference(PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT);
$viewer = $this->getViewer();
$date_key = PhabricatorDateFormatSetting::SETTINGKEY;
return $viewer->getUserSetting($date_key);
}
private function getTimeInputValue() {
@ -241,7 +243,6 @@ final class AphrontFormDateControl extends AphrontFormControl {
'format' => $this->getTimeFormat(),
));
$time_sel = javelin_tag(
'input',
array(
@ -262,9 +263,9 @@ final class AphrontFormDateControl extends AphrontFormControl {
),
$time_sel);
$preferences = $this->getViewer()->loadPreferences();
$pref_week_start = PhabricatorUserPreferences::PREFERENCE_WEEK_START_DAY;
$week_start = $preferences->getPreference($pref_week_start, 0);
$viewer = $this->getViewer();
$week_key = PhabricatorWeekStartDaySetting::SETTINGKEY;
$week_start = $viewer->getUserSetting($week_key);
Javelin::initBehavior('fancy-datepicker', array(
'format' => $this->getDateFormat(),

View file

@ -207,13 +207,15 @@ final class AphrontFormDateControlValue extends Phobject {
}
private function getTimeFormat() {
return $this->getViewer()
->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT);
$viewer = $this->getViewer();
$time_key = PhabricatorTimeFormatSetting::SETTINGKEY;
return $viewer->getUserSetting($time_key);
}
private function getDateFormat() {
return $this->getViewer()
->getPreference(PhabricatorUserPreferences::PREFERENCE_DATE_FORMAT);
$viewer = $this->getViewer();
$date_key = PhabricatorDateFormatSetting::SETTINGKEY;
return $viewer->getUserSetting($date_key);
}
private function getFormattedDateFromDate($date, $time) {

View file

@ -130,8 +130,8 @@ class PhabricatorBarePageView extends AphrontPageView {
if ($this->getRequest()) {
$viewer = $this->getRequest()->getViewer();
if ($viewer) {
$postprocessor_key = $viewer->getPreference(
PhabricatorUserPreferences::PREFERENCE_RESOURCE_POSTPROCESSOR);
$postprocessor_key = $viewer->getUserSetting(
PhabricatorAccessibilitySetting::SETTINGKEY);
if (strlen($postprocessor_key)) {
$response->setPostProcessorKey($postprocessor_key);
}

View file

@ -225,13 +225,8 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView
if ($user->isLoggedIn()) {
$offset = $user->getTimeZoneOffset();
$preferences = $user->loadPreferences();
$ignore_key = PhabricatorUserPreferences::PREFERENCE_IGNORE_OFFSET;
$ignore = $preferences->getPreference($ignore_key);
if (!strlen($ignore)) {
$ignore = null;
}
$ignore_key = PhabricatorTimezoneIgnoreOffsetSetting::SETTINGKEY;
$ignore = $user->getUserSetting($ignore_key);
Javelin::initBehavior(
'detect-timezone',

View file

@ -50,7 +50,7 @@ final class PhabricatorMainMenuSearchView extends AphrontView {
'');
$search_datasource = new PhabricatorSearchDatasource();
$scope_key = PhabricatorUserPreferences::PREFERENCE_SEARCH_SCOPE;
$scope_key = PhabricatorSearchScopeSetting::SETTINGKEY;
Javelin::initBehavior(
'phabricator-search-typeahead',
@ -177,10 +177,8 @@ final class PhabricatorMainMenuSearchView extends AphrontView {
'href' => PhabricatorEnv::getDoclink('Search User Guide'),
);
$scope_key = PhabricatorUserPreferences::PREFERENCE_SEARCH_SCOPE;
$current_value = $viewer->loadPreferences()->getPreference(
$scope_key,
'all');
$scope_key = PhabricatorSearchScopeSetting::SETTINGKEY;
$current_value = $viewer->getUserSetting($scope_key);
$current_icon = 'fa-globe';
foreach ($items as $item) {

View file

@ -147,16 +147,18 @@ final class PHUICalendarListView extends AphrontTagView {
}
private function getEventTooltip(AphrontCalendarEventView $event) {
$time_pref = $this->getUser()
->getPreference(PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT);
$viewer = $this->getViewer();
$time_key = PhabricatorTimeFormatSetting::SETTINGKEY;
$time_pref = $viewer->getUserSetting($time_key);
Javelin::initBehavior('phabricator-tooltips');
$start = id(AphrontFormDateControlValue::newFromEpoch(
$this->getUser(),
$viewer,
$event->getEpochStart()));
$end = id(AphrontFormDateControlValue::newFromEpoch(
$this->getUser(),
$viewer,
$event->getEpochEnd()));
$start_date = $start->getDateTime()->format('m d Y');

View file

@ -31,21 +31,21 @@ function phabricator_relative_date($epoch, $user, $on = false) {
}
function phabricator_time($epoch, $user) {
$time_key = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT;
$time_key = PhabricatorTimeFormatSetting::SETTINGKEY;
return phabricator_format_local_time(
$epoch,
$user,
$user->getPreference($time_key));
$user->getUserSetting($time_key));
}
function phabricator_datetime($epoch, $user) {
$time_key = PhabricatorUserPreferences::PREFERENCE_TIME_FORMAT;
$time_key = PhabricatorTimeFormatSetting::SETTINGKEY;
return phabricator_format_local_time(
$epoch,
$user,
pht('%s, %s',
phutil_date_format($epoch),
$user->getPreference($time_key)));
$user->getUserSetting($time_key)));
}
/**