forked from phacility/phabricator
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Phabricator Update #2
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…passphrase" related Summary: Ref T13454. Fixes T13006. When a user provide us with an SSH private key and (possibly) a passphrase: # Try to verify that they're correct by extracting the public key. # If that fails, try to figure out why it didn't work. Our success in step (2) will vary depending on what the problem is, and we may end up falling through to a very generic error, but the outcome should generally be better than the old approach. Previously, we had a very unsophisticated test for the text "ENCRYPTED" in the key body and questionable handling of the results: for example, providing a passphrase when a key did not require one did not raise an error. Test Plan: Created and edited credentials with: - Valid, passphrase-free keys. - Valid, passphrased keys with the right passphrase. - Valid, passphrase-free keys with a passphrase ("surplus passphrase" error). - Valid, passphrased keys with no passphrase ("missing passphrase" error). - Valid, passphrased keys with an invalid passphrase ("invalid passphrase" error). - Invalid keys ("format" error). The precision of these errors will vary depending on how helpful "ssh-keygen" is. Maniphest Tasks: T13454, T13006 Differential Revision: https://secure.phabricator.com/D20905
Summary: Fixes T13456. These edits are remarkup edits and should attach files, trigger mentions, and so on. Test Plan: Created a text panel, dropped a file in. After changes, saw the file attach properly. Maniphest Tasks: T13456 Differential Revision: https://secure.phabricator.com/D20906
Summary: Fixes T13460. This rule vanished from the UI in D20165; update things so it returns to the UI. Test Plan: {F7035134} Maniphest Tasks: T13460 Differential Revision: https://secure.phabricator.com/D20917
Summary: Fixes T13461. Some applications provide hints about policy strength in the header, but these hints are inconsistent and somewhat confusing. They don't make much sense for modern objects with Custom Forms, which don't have a single "default" policy. Remove this feature since it seems to be confusing things more than illuminating them. Test Plan: - Viewed various objects, no longer saw colored policy hints. - Grepped for all removed symbols. Maniphest Tasks: T13461 Differential Revision: https://secure.phabricator.com/D20918
…s so they behave like milestones Summary: Ref T13462. Currently, when testing milestone edit policies during creation, the project object does not behave like a milestone: - it doesn't have a milestone number yet, so it doesn't try to access the parent project; and - the parent project isn't attached yet. Instead: attach the parent project sooner (which "should" be harmless, although it's possible this has weird side effects); and give the adjusted policy object a dummy milestone number if it doesn't have one yet. This forces it to act like a milestone when emitting policies. Test Plan: - Set "Projects" application default edit policy to "No One". - Created a milestone I had permission to create. - Before: failed with a policy error, because the project behaved like a non-milestone and returned "No One" as the effective edit policy. - After: worked properly, correctly evaluting the parent project edit policy as the effective edit policy. - Tried to create a milestone I did not have permission to create (no edit permission on parent project). - Got an appropriate edit policy error. Maniphest Tasks: T13462 Differential Revision: https://secure.phabricator.com/D20919
…ill have parent membership Summary: Depends on D20919. Ref T13462. When editing milestones, we currently predict they will have no members for policy evaluation purposes. This isn't the right rule. Instead, predict that their membership will be the same as the parent project's membership, and pass this hint to the policy layer. See T13462 for additional context and discussion. Test Plan: - Set project A's edit policy to "Project Members". - Joined project A. - Tried to create a milestone of project A. - Before: policy exception that the edit policy excludes me. - After: clean milestone creation. - As a non-member, tried to create a milestone. Received appropriate policy error. Maniphest Tasks: T13462 Differential Revision: https://secure.phabricator.com/D20920
Summary: Ref T13444. Currently, identities for a particular email address are queried with "LIKE" against a binary column, which makes the query case-sensitive. - Extract the email address into a separate "sort255" column. - Add a key for it. - Make the query a standard "IN (%Ls)" query. - Deal with weird cases where an email address is 10000 bytes long or full of binary junk. Test Plan: - Ran migration, inspected database for general sanity. - Ran query script in T13444, saw it return the same hits for "git@" and "GIT@". Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20907
…early and consistently Summary: Ref T13444. You can currently explicitly unassign an identity (useful if the matching algorithm is misfiring). However, this populates the "currentEffectiveUserPHID" with the "unassigned()" token, which mostly makes things more difficult. When an identity is explicitly unassigned, convert that into an explicit `null` in the effective user PHID. Then, realign "assigned" / "effective" language a bit. Previously, `withAssigneePHIDs(...)` actualy queried effective users, which was misleading. Finally, bulk up the list view a little bit to make testing slightly easier. Test Plan: - Unassigned an identity, ran migration, saw `currentEffectiveUserPHID` become `NULL` for the identity. - Unassigned a fresh identity, saw NULL. - Queried for various identities under the modified constraints. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20908
Summary: Ref T13444. This is an ancient event and part of the old event system. It is not likely to be in use anymore, and repository identities should generally replace it nowadays anyway. Test Plan: Grepped for constant and related methods, no longer found any hits. Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20909
Summary: Ref T13444. Send all repository identity/detection through a new "DiffusionRepositoryIdentityEngine" which handles resolution and detection updates in one place. Test Plan: - Ran `bin/repository reparse --message ...`, saw author/committer identity updates. - Added "goose@example.com" to my email addresses, ran daemons, saw the identity relationship get picked up. - Ran `bin/repository rebuild-identities ...`, saw sensible rebuilds. Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20910
…e flexibility Summary: Ref T13444. Repository identities have, at a minimum, some bugs where they do not update relationships properly after many types of email address changes. It is currently very difficult to fix this once the damage is done since there's no good way to inspect or rebuild them. Take some steps toward improving observability and providing repair tools: allow `bin/repository rebuild-identities` to effect more repairs and operate on identities more surgically. Test Plan: Ran `bin/repository rebuild-identities` with all new flags, saw what looked like reasonable rebuilds occur. Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20911
Summary: Ref T13444. Prepare to hook identity updates when user email addreses are destroyed. Test Plan: - Destroyed a user with `bin/remove destroy ... --trace`, saw email deleted. - Destroyed an email from the web UI, saw email deleted. Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20912
Summary: Ref T13444. To interact meaningfully with "DestructionEngine", objects need a PHID. The "UserEmail" object currently does not have one (or a real "Query"). Provide basic PHID support so "DestructionEngine" can interact with the object more powerfully. Test Plan: - Ran migrations, checked data in database, saw sensible PHIDs assigned. - Added a new email address to my account, saw it get a PHID. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20913
…ddresses Summary: Ref T13444. Currently, many mutations to users and email addresses (particularly: user creation; and user and address destruction) do not propagate properly to repository identities. Add hooks to all mutation workflows so repository identities get rebuilt properly when users are created, email addresses are removed, users or email addresses are destroyed, or email addresses are reassigned. Test Plan: - Added random email address to account, removed it. - Added unassociated email address to account, saw identity update (and associate). - Removed it, saw identity update (and disassociate). - Registered an account with an unassociated email address, saw identity update (and associate). - Destroyed the account, saw identity update (and disassociate). - Added address X to account A, unverified. - Invited address X. - Clicked invite link as account B. - Confirmed desire to steal address. - Saw identity update and reassociate. Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20914
… and "bin/repository rebuild-identities" Summary: Fixes T13457. Ref T13444. When we iterate over commits in a particular repository, the default iteration strategy can't effectively use the keys on the table. Tweak the ordering so the "<repositoryID, epoch, [id]>" key can be used. Test Plan: - Ran `bin/audit delete --repository X` and `bin/repository rebuild-identities --repository X` before and after changes. - With just the key changes, performance was slightly better. My local data isn't large enough to really emphasize the key changes. - With the page size changes, performance was a bit better (~30%, but on 1-3 second run durations). - Used `--trace` and ran `EXPLAIN ...` on the new queries, saw them select the "<repositoryID, epoch, [id]>" key and report a bare "Using index condition" in the "Extra" column. Maniphest Tasks: T13457, T13444 Differential Revision: https://secure.phabricator.com/D20921
Summary: Ref T13444. Allow the effects of performing an identity rebuild to be previewed without committing to any changes. Test Plan: Ran "bin/repository rebuild-identities --all-identities" with and without "--dry-run". Maniphest Tasks: T13444 Differential Revision: https://secure.phabricator.com/D20922
…ror: undefined index "display" Summary: Fixes T13464. Fully-realized paths have a "path" (normalized, effective path) and a "display" path (user-facing, un-normalized path). During transaction validation we build ref keys for paths before we normalize the "display" values. A few different approaches could be taken to resolve this, but just default the "display" path to the raw "path" if it isn't present since that seems simplest. Test Plan: Edited paths in an Owners package, no longer saw a warning in the logs. Maniphest Tasks: T13464 Differential Revision: https://secure.phabricator.com/D20923
Summary: Fixes T13465. This "phlog()" made some degree of sense at one time, but is no longer useful or consistent. Get rid of it. See T13465 for discussion. Test Plan: Made a conduit call that hit a policy error, no longer saw error in log. Maniphest Tasks: T13465 Differential Revision: https://secure.phabricator.com/D20924
…hem properly Summary: See PHI1558. Ref T11860. Ref T13444. I partly implemented PHIDs for "UserEmail" objects, but they can't load on their own so you can't directly `bin/remove destroy` them yet. Allow them to actually load by implementing "PolicyInterface". Addresses are viewable and editable by the associated user, unless they are a bot/list address, in which case they are viewable and editable by administrators (in preparation for T11860). This has no real impact on anything today. Test Plan: - Used `bin/remove destroy <phid>` to destroy an individual email address. - Before: error while loading the object by PHID in the query policy layer. - After: clean load and destroy. Maniphest Tasks: T13444, T11860 Differential Revision: https://secure.phabricator.com/D20927
Summary: Ref T13362. Some applications moved to fixed-width a while ago but I was generally unsatisfied with where they ended up and have been pushing them back to full-width. Push Config back to full-width. Some of the subpages end up a little weird, but this provides more space to work with to make some improvements, like makign `maniphest.statuses` more legible in the UI> Test Plan: Grepped for `setFixed(`, updated each page in `/config/`. Browsed each controller, saw workable full-width UIs. Maniphest Tasks: T13362 Differential Revision: https://secure.phabricator.com/D20925
…ditor and PhabricatorApplicationTransactionEditor Summary: Maniphest object has `getURI` method, let's use it Test Plan: Create event in task - URI generated as expected in email notification Reviewers: epriestley, Pawka, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D20935
…nside lower-priority link rules Summary: See <https://hackerone.com/reports/758002>. The link rules don't test that their parameters are flat text before using them in unsafe contexts. Since almost all rules are lower-priority than these link rules, this behavior isn't obvious. However, two rules have broadly higher priority (monospaced text, and one variation of link rules has higher priority than the other), and the latter can be used to perform an XSS attack with input in the general form `()[ [[ ... | ... ]] ]` so that the inner link rule is evaluated first, then the outer link rule uses non-flat text in an unsafe way. Test Plan: Tested examples in HackerOne report. A simple example of broken (but not unsafe) behavior is: ``` [[ `x` | `y` ]] ``` Differential Revision: https://secure.phabricator.com/D20937
Summary: Ref T13472. Ref T13395. These classes are only used by Phabricator and not likely to find much use in Arcanist. Test Plan: Grepped libphutil and Arcanist for removed symbols. Maniphest Tasks: T13472, T13395 Differential Revision: https://secure.phabricator.com/D20939
Summary: Fixes T13472. This library uses `$a{0}`, but this is deprecated in favor of `$a[0]`. Test Plan: Ran `bin/search index Txxx --force` on a task with "filing" in the title (this term reaches the "m" rule of the stemmer). (I'm not on new enough PHP for this to actually raise an error, but I'll follow up with the reporting user.) Maniphest Tasks: T13472 Differential Revision: https://secure.phabricator.com/D20941
…arning Summary: Fixes T13471. Recent versions of PHP raise a warning when this function is called. We're only calling it so we can instantly fatal if it's enabled, so use "@" to silence the warning. Test Plan: Loaded site; see also T13471 for a user reporting that this fix is effective. Maniphest Tasks: T13471 Differential Revision: https://secure.phabricator.com/D20942
Summary: Fixes T13469. Currently, "Mute" applies a generic edge transaction but the editor doesn't whitelist them. Test Plan: Muted a Herald rule. Maniphest Tasks: T13469 Differential Revision: https://secure.phabricator.com/D20943
…each other while wrapping Summary: Fixes T13476. Policy tags in object headers and "Visible To" controls in some dialog contexts may stack and wrap oddly. Improve spacing so they don't overlap visually when wrapping. Test Plan: Viewed affected interfaces in narrow and wide windows. Maniphest Tasks: T13476 Differential Revision: https://secure.phabricator.com/D20944
Summary: Fixes T13479. The behavior of "git rev-parse --show-toplevel" has changed in Git 2.25.0, and it now fails in bare repositories. Instead, use "git rev-parse --git-dir" to sanity-check the working copy. This appears to have more stable behavior across Git versions, although it's a little more complicated for our purposes. Test Plan: - Ran `bin/repository update ...` on an observed, bare repository. - ...on an observed, non-bare ("legacy") repository. - ...on a hosted, bare repository. Maniphest Tasks: T13479 Differential Revision: https://secure.phabricator.com/D20945
Summary: Ref T13480. These fields don't serve a specific strong use case, but are broadly reasonable capabilities after "state" vs "change" actions were relaxed by T13283. Test Plan: Wrote rules using the new fields, added and removed projects (and did neither) to fire them / not fire them. Inspected the transcripts to see the project PHIDs making it to the field values. Maniphest Tasks: T13480 Differential Revision: https://secure.phabricator.com/D20946
Summary: Ref T13480. Add an "Author's packages" field to Herald to support writing rules like "if affected packages include X, and author's packages do not include X, raise the alarm". Test Plan: Wrote and executed rules with the field, saw a sensible field value in the transcript. Maniphest Tasks: T13480 Differential Revision: https://secure.phabricator.com/D20947
tavasja
pushed a commit
that referenced
this pull request
Jan 29, 2020
… when billing a Phortune subscription Summary: Fixes T13327. Currently, when we try to bill an account and all members are disabled, we fail temporarily and the task retries forever. At least for now, just treat this as a permanent failure. Test Plan: - Used `bin/phortune invoice` to generate a normal invoice for a regular subscription. - Disabled all the account members, then tried again. Got a helpful permanent failure: ``` $ ./bin/phortune invoice --subscription PHID-PSUB-kbedwt5cyepoc6tohjq5 --auto-range Set current time to Mon, Jun 24, 2:47 PM. Preparing to invoice subscription "localb.phacility.com" from Fri, May 31, 10:14 AM to Sun, Jun 30, 10:14 AM. WARNING Manually invoicing will double bill payment accounts if the range overlaps an existing or future invoice. This script is intended for testing and development, and should not be part of routine billing operations. If you continue, you may incorrectly overcharge customers. Really invoice this subscription? [y/N] y [2019-06-24 14:47:57] EXCEPTION: (PhabricatorWorkerPermanentFailureException) All members of the account ("PHID-ACNT-qp54y3unedoaxgkkjpj4") for this subscription ("PHID-PSUB-kbedwt5cyepoc6tohjq5") are disabled. at [<phabricator>/src/applications/phortune/worker/PhortuneSubscriptionWorker.php:88] arcanist(head=experimental, ref.master=d92fa96366c0, ref.experimental=db4cd55d4673), corgi(head=master, ref.master=6371578c9d32), instances(head=stable, ref.master=ba9e4a19df1c, ref.stable=37fb1f4917c7), libcore(), phabricator(head=master, ref.master=65bc481c91de, custom=11), phutil(head=master, ref.master=7adfe4e4f4a3), services(head=master, ref.master=5424383159ac) #0 PhortuneSubscriptionWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:124] #1 PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:163] #2 PhabricatorWorker::scheduleTask(string, array, array) called at [<phabricator>/src/applications/phortune/management/PhabricatorPhortuneManagementInvoiceWorkflow.php:169] #3 PhabricatorPhortuneManagementInvoiceWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:457] #4 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:349] #5 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/setup/manage_phortune.php:21] $ ``` Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13327 Differential Revision: https://secure.phabricator.com/D20613
tavasja
pushed a commit
that referenced
this pull request
Jan 29, 2020
…t failure when billing a Phortune subscription Summary: Fixes T13327. Currently, when we try to bill an account and all members are disabled, we fail temporarily and the task retries forever. At least for now, just treat this as a permanent failure. Test Plan: - Used `bin/phortune invoice` to generate a normal invoice for a regular subscription. - Disabled all the account members, then tried again. Got a helpful permanent failure: ``` $ ./bin/phortune invoice --subscription PHID-PSUB-kbedwt5cyepoc6tohjq5 --auto-range Set current time to Mon, Jun 24, 2:47 PM. Preparing to invoice subscription "localb.phacility.com" from Fri, May 31, 10:14 AM to Sun, Jun 30, 10:14 AM. WARNING Manually invoicing will double bill payment accounts if the range overlaps an existing or future invoice. This script is intended for testing and development, and should not be part of routine billing operations. If you continue, you may incorrectly overcharge customers. Really invoice this subscription? [y/N] y [2019-06-24 14:47:57] EXCEPTION: (PhabricatorWorkerPermanentFailureException) All members of the account ("PHID-ACNT-qp54y3unedoaxgkkjpj4") for this subscription ("PHID-PSUB-kbedwt5cyepoc6tohjq5") are disabled. at [<phabricator>/src/applications/phortune/worker/PhortuneSubscriptionWorker.php:88] arcanist(head=experimental, ref.master=d92fa96366c0, ref.experimental=db4cd55d4673), corgi(head=master, ref.master=6371578c9d32), instances(head=stable, ref.master=ba9e4a19df1c, ref.stable=37fb1f4917c7), libcore(), phabricator(head=master, ref.master=65bc481c91de, custom=11), phutil(head=master, ref.master=7adfe4e4f4a3), services(head=master, ref.master=5424383159ac) #0 PhortuneSubscriptionWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:124] #1 PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:163] #2 PhabricatorWorker::scheduleTask(string, array, array) called at [<phabricator>/src/applications/phortune/management/PhabricatorPhortuneManagementInvoiceWorkflow.php:169] #3 PhabricatorPhortuneManagementInvoiceWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:457] #4 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:349] #5 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/setup/manage_phortune.php:21] $ ``` Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13327 Differential Revision: https://secure.phabricator.com/D20613
tavasja
pushed a commit
that referenced
this pull request
Jan 29, 2020
Summary: On fresh installation which doesn't have yet any task closed you will not be able to open charts because of error below: Fixes: ``` [Mon Oct 21 15:42:41 2019] [2019-10-21 15:42:41] EXCEPTION: (TypeError) Argument 1 passed to head_key() must be of the type array, null given, called in ..phabricator/src/applications/fact/chart/PhabricatorFactChartFunction.php on line 86 at [<phutil>/src/utils/utils.php:832] [Mon Oct 21 15:42:41 2019] #0 phlog(TypeError) called at [<phabricator>/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php:27] [Mon Oct 21 15:42:41 2019] #1 PhabricatorAjaxRequestExceptionHandler::handleRequestThrowable(AphrontRequest, TypeError) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:797] [Mon Oct 21 15:42:41 2019] #2 AphrontApplicationConfiguration::handleThrowable(TypeError) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:345] [Mon Oct 21 15:42:41 2019] #3 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:214] [Mon Oct 21 15:42:41 2019] #4 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:35 ``` To fix issue - lets return empty data set instead Test Plan: 1) Create fresh phabricator installation 2) Create fresh project 3) Try viewing charts Reviewers: epriestley, Pawka, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, yelirekim Differential Revision: https://secure.phabricator.com/D20861
tavasja
pushed a commit
that referenced
this pull request
Jan 5, 2021
…custom policy preference Summary: The "Export Data" workflow incorrectly uses the "Policy Favorites" setting to choose a default export format. This is just a copy/paste error; the correct setting exists and is unused. If the setting value is an array (as the "Policy Favorites" value often is), we try to use it as an array index. This generates a runtime exception after D21044. ``` [2020-06-16 06:32:12] EXCEPTION: (RuntimeException) Illegal offset type in isset or empty at [<arcanist>/src/error/PhutilErrorHandler.php:263] #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phabricator>/src/applications/search/controller/PhabricatorApplicationSearchController.php:460] ``` - Use the correct setting. - Make sure the value we read is a string. Test Plan: - Used "Export Data" with a nonempty, array-valued "Policy Favorites" setting. - Before: runtime exception. - After: clean export. - Used "Export Data" again, saw my selection from the first time persisted. Differential Revision: https://secure.phabricator.com/D21361
tavasja
pushed a commit
that referenced
this pull request
Jan 5, 2021
…nonempty custom policy preference Summary: The "Export Data" workflow incorrectly uses the "Policy Favorites" setting to choose a default export format. This is just a copy/paste error; the correct setting exists and is unused. If the setting value is an array (as the "Policy Favorites" value often is), we try to use it as an array index. This generates a runtime exception after D21044. ``` [2020-06-16 06:32:12] EXCEPTION: (RuntimeException) Illegal offset type in isset or empty at [<arcanist>/src/error/PhutilErrorHandler.php:263] #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<phabricator>/src/applications/search/controller/PhabricatorApplicationSearchController.php:460] ``` - Use the correct setting. - Make sure the value we read is a string. Test Plan: - Used "Export Data" with a nonempty, array-valued "Policy Favorites" setting. - Before: runtime exception. - After: clean export. - Used "Export Data" again, saw my selection from the first time persisted. Differential Revision: https://secure.phabricator.com/D21361
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updating local version with latest updates.