Provide a views filter for administrative areas

Addressfield still doesn't have one: #2137339: Provide a views filter and contextual filter that allows filtering by administrative area if it exists, or locality if it doesn't..
But maybe someone can use that patch as an example for writing a similar Address patch.

Yes, I'd love this as well. I'm building an international directory view. Currently I have exposed filters for country, administrative area (state), locality (city) and postal code. Right now, filtering by the administrative area works if you happen to know the underlying codes (e.g. in the US, 2-letter state codes). However, it's kind of clunky to expose this to end users, and it would get really confusing if different countries use the same codes for their own areas.

It seems ideal to have an exposed views filter that works much like the field widget: If/when you select a country, the dependent filters are updated to reflect the available choices for that country.

However, I'm not sure the cleanest way to represent this or implement it. Before I dig in and actually start writing something, I'd love to hear from the maintainers about the viability of this approach, and their willingness to commit it upstream if I ultimately get it working. ;)

Thanks!
-Derek

Here's a first draft of a partially working patch that adds this. ;)

I don't agree with the assessment in #2137339 that we want a single filter that toggles itself between administrative areas and localities depending on the country. I think it's reasonable to always be able to filter by locality, using the existing views filter for that. Even for countries that use administrative areas, you probably still want the locality filter, too (e.g. to drill down by city if you're looking at a lot of results in a single state). So, this is an entirely different approach from what's proposed over there.

This is a magic filter for administrative area. When you configure the filter, you point it at the country code filter you want it to reference for its magic. You can also configure if you want a static exposed filter label (like just about everything else in Views) or if you want the label for the administrative area to be dynamic based on the country (like you expect everywhere else in address.module land -- e.g. "Province" if country is Canada, "State" if US, "County" if Ireland, etc.).

The remaining todo items:
* @todo: Hide this element on the exposed filter form if there's no country.
* @todo: Hide this if the current country doesn't use administrative areas.
* @todo: Rebuild this form element via AJAX when the country changes.
* @todo: Let user specify the country via an argument, instead of filter.

In case it's not obvious, the last @todo would handle the case where the country is defined via a View argument (and therefore known in advance -- just like how drupal.org/project/issues/address knows the valid options for version and component based on the view argument at /project/issues/%). I envision a set of radios or something for if the country should be determined via another filter or a view argument, adding another config setting for country_argument_id, and using #states to hide/show the right sub-option based on the radios. getCurrentCountry() would be smarter and know which method to use for finding a country code...

Otherwise, it's basically working on a local test site. Posting here to get feedback both on the general approach and on specific implementation details.

Thanks!
-Derek

This version removes the final @todo mentioned above and adds support for finding the country via either an exposed filter (as before) or the current value of an argument to the view.

The other @todo items are still pending.

This seems like a reasonably clean and simple way to take out another 2 @todo items:

  public function buildExposedForm(&$form, FormStateInterface $form_state) {
    parent::buildExposedForm($form, $form_state);
    // Only render the form element if we have value options
    // (i.e. the country is set and it uses administrative areas).
    if (empty($this->valueOptions)) {
      $identifier = $this->options['expose']['identifier'];
      $form[$identifier]['#access'] = FALSE;
    }
  }

Is that cheating? ;)

So, now the only remaining @todo is some AJAX to automatically rebuild this form element as soon as the country selector changes, not just on submit.

Upon further reflection, there are definitely use-cases where you might want to filter by administrative area programmatically, not via an exposed filter. In that case, the country would have to be fixed in the filter configuration, but then this could work as either exposed or not, and otherwise function more like a normal InOperator filter.

This patch reorganizes the code/settings a lot, and adds a 3rd choice for the "Country source" to define a static country. There are a few wonky edge cases in the UI that aren't totally slick and polished yet, but it's now basically working properly with all 3 options for the country source. Most of the option validation is now solid and prevents you from broken configurations.

I also added a custom adminSummary() method, since it seems the country source is now a fairly important aspect of the filter configuration to advertise to admins.

A few open questions:

1) When creating the list of countries for a static country source, is there an easy way to say: "give me a list of countries that use administrative areas at all"? Or, do I have to loop over the countries and call SubdivisionRepository::getList() for each one to see if there are any options? Seems kinda pointless to let someone configure this filter via a static country that doesn't support administrative areas.

2) Presumably that list should also be filtered by the available countries in the field configuration for the field we're attached to, right? I can more or less replicate the logic from the CountryCode.php filter if we want this behavior. Or is it worth refactoring CountryCode and this new filter to share a base class that provides getAvailableCountries() and friends? Actually, there's very little in CountryCode *beyond* that, perhaps this new filter should be a child of CountryCode instead of InOperator. Then, we'd only have to override __construct(), create() and getValueOptions(). But maybe it's dangerous to let this directly extend CountryCode since future changes to CountryCode.php might not make sense here. So if we're going to share code, I think it'd be better to put the shared code into a base class and let both CountryCode and AdministrativeArea extend that. Thoughts?

Next step, getting the AJAX stuff working properly, at least for the exposed filter case. It'd also be nice to have some AJAX within the options form, but that seems a bit more convoluted with all the form rebuilding already happening in there. :/

This patch is the same as #7, but fixes code style by using [] instead of array(). The @todo and open questions remain, but I noticed this code-style change in other places and wanted to clean up the new AdministrativeArea.php to match.

Good grief. :/ Views makes it really hard to use #ajax in the exposed filter form. Looks like we need a patch to views to even make this possible. See the related issue I'm adding for the gory details.

So, I'm promoting this to "needs review" since I'm not going to block forward progress on the whole upstream Views + AJAX mess. This works pretty well already, although you have to hit 'Apply' to see the changes from the country exposed filter (which I confirmed *does* work if you set the view to 'Use AJAX' without a full page reload).

Can I please get feedback on the approach and existing settings/config UI? Happy to make any suggested/needed changes.

Thanks!
-Derek

p.s. This patch adds a @see next to the AJAX @todo pointing to the upstream Views issue.

Added a proper issue summary, collected all the remaining tasks, etc.

Briefly discussed this with bojanz in IRC. Basically, he's happy with the approach. He doesn't think we need to block this on getting the Views #ajax mess sorted out. He's okay with needing to hit "Apply" to refresh the options in the administrative area filter (for now).

So, I opened #2840717: Use AJAX to update the available options for exposed administrative area filters whenever the country changes as a child issue of this, currently postponed on getting the rest of this issue fixed and committed, and on the upstream Views issue to allow #ajax to work in exposed filter forms at all.

Updated the remaining tasks and related issues accordingly. I hope to make progress on the rest of the remaining tasks in the next few days. Stay tuned.

Thanks!
-Derek

This moves almost all of the code from CountryCode.php into a shared abstract base class, currently called CountryAwareInOperatorBase. CountryCode now only has to define a (very trivial) getValueOptions() method. AdministrativeArea now extends from the same base class, and uses CountryAwareInOperatorBase::getAvailableCountries() to remove one of the todo items: The available options in the country selector for the static ID case now honor the available countries list from the field configuration.

However, setting this back to Needs work since there are some legit @todo items that I want to fix before this is really considered viable for review (although anyone else interested in helping develop/test are welcome to keep using this on a dev site).

Thanks,
-Derek

This resolves remaining task 7 by filtering the list of countries for the static code to those that have a subdivision depth greater than 0 (and therefore have an administrative area at all).

New patch that starts to get AJAX rebuilds working for changing the country source and/or code in the filter options form. Added a getCountrySource() helper function (since we need to know in a few spots). Both it and getCountryCode() check form_state for their active values, and fall back to the filter configuration if the form_state doesn't have a value, etc.

However, even trying to clear the values via an #after_build callback when the country (code|source) changes, this still triggers the 'Illegal choice' validation error in some cases. :/ Also, there's some unholy interaction between the AJAX to rebuild the options form when the filter is exposed vs. what we're trying to do for the specific administrative area value options. I think I see where that's going wrong, but I haven't had a chance to dig into it.

Handles some other UI edge cases and clean-ups:

• The 'reduce' option for InOperator filters ("Limit list to selected items") only makes sense if there's a static country code and pre-defined values. So, we hide that checkbox and clear the values unless we have what we need.
• The static vs. dynamic label setting only makes sense if the filter is exposed (and has a label at all), so we conditionally hide that option.
• Fleshes out the text for the validation errors on the country source.
• Uses 'active country' instead of 'currently selected country' in various form descriptions + messages.
• If we're using a filter for the country source, and that filter allows multiple values, we used to crash. Now we use the first selected country. Left a @todo comment in the code, since I wonder if we want to prevent this kind of configuration at all, or otherwise warn users that this behavior is happening.

Still 'needs work', for sure, but I wanted to post another snapshot in case anyone else wants to get involved or take a look.

Thanks,
-Derek

This fixes most of the problems I mentioned in #14 with AJAX and validation errors. The bug is I was looking for the country code via $form_state->getValue() but inside the validation for the filter itself, the form values aren't yet recorded into the form state. So, getCurrentCountry() now uses $form_state->getUserInput() and the validation errors are now resolved since all stages of form building are now using the same administrative area options.

Also fixed clearValues() to be more selective so it only clobbers the admin area options if the triggering element was the country code or source. This preserves the specific value choices when toggling between exposed and not, for example.

Finally, cleaned up some debugging code from while I was trying to get AJAX working properly.

So, all that remains here before crossing off remaining task #5 is to get the AJAX happy within the exposed filter case. This might be as simple as ensuring we preserve our wrapper div around the value options, or it could require deeper hackery. Stay tuned...

Indeed, the AJAX-within-exposed problem was just that our wrapper div was getting clobbered via FilterPluginBase::buildExposeForm(). So this patch hereby resolves remaining task #5 since all the internal AJAX is now working.

Unfortunately, Views makes some pretty big assumptions about how the filter config UI should be once the filter is exposed. I'm not sure how much effort it's worth trying to fight against that. The ordering of config choices is a bit weird in the case when the filter is exposed, but I'm not sure how much we can improve upon that without doing unholy things ourselves. :/

I added an "Open questions" section to the issue summary, and cleaned up the remaining tasks accordingly.

Other than addressing the open questions, all that remains is an edge-case of trying to support canBuildGroup() (which I'd be very happy to punt to a follow-up issue–I certainly don't need that at this point and would love not to hold this up any further).

Thoughts?

Thanks!
-Derek

This cleans up a few strict notices that were getting thrown from the previous patch. Basically, we need to be more careful about not assuming we have the values we think we do. ;)

1. Unless there's a static country code, we don't want to try to save the filter values (since there won't be any).
2. Inside getCurrentCountry() we can't assume that the form_state user input has 'options' since the same method is called for the exposed filter form, not just the filter configuration forms.

Minor fixes:

- using Drupal\Component\Utility\Html was stale code from a previous iteration. That's no longer needed, so now gone.
- There was a stray newline inside exposedInfo().

Minor updates to open questions and remaining tasks. Re-ordering the config options is probably going to be too hard to be worth it. None of the Views maintainers understand how the current behavior is happening. ;)

Further testing of edge-cases revealed a bug in getCountrySource() that was causing it to be confused in the AJAX case, sometimes leading to the actual administrative area options not getting saved when trying to configure the filter with a static country code. Once again, relying on $form_state->getValue() was biting me, and to be really sure what's happening, we have to use $form_state->getUserInput(). Now the values are always saved, even if you start with this filter configured to use an exposed filter for the country source, then change it to a static country, then toggle the country, then select values.

Phew! ;)

Cleaning up stray whitespace that snuck into the previous patches, otherwise, unchanged.

Re-roll to make the subdivision list properly honor the current locale in both the settings form (when there's a fixed country and you see checkboxes for each administrative area), and in the exposed filter form element on the view itself. Basically fixes the bug from #2751679: The address widget renders subdivision dropdowns in the wrong language that I had replicated the broken behavior of in here.

Fixed summary to point to the right upstream issue in Views (now in core) about fixing AJAX for exposed filters.

Wow! This looks like it was a lot of work. Thank you.

General complaint: we keep talking about "arguments", even though the views ui calls them "contextual filters". That's bound to confuse someone.

+  /**
+   * If we're in the middle of building a form, its current state.
+   */
+  protected $formState;
+
+  /**
+   * The currently selected country (if any).
+   */
+  protected $currentCountryCode;

Missing @var in both cases.

+  protected function defineOptions() {
+    $options = parent::defineOptions();
+
+    $options['country'] = [

Missing docblock.

+    // Find all country_code arguments from address.module for the valid choices.
+    foreach ($this->view->display_handler->getHandlers('argument') as $name => $argument) {

This comment is weird, I don't understand it. Is it copy paste from another code block?

+        '#title' => t('Country argument to determine values'),

"to determine values" sounds odd, I'd go with something shorter ("Contextual filter"? Since I already said above that "Argument" doesn't show up in the UI)

+      // #states doesn't work on markup elements, so we to use a container.

You accidentally a word.

+      '#title' => t('Static country for administrative areas'),

"Static" sounds odd. Maybe something like "Predefined"?

+        if (empty($country_filter)) {
+          $error = $this->t("You must select a country filter for this filter to work using 'filter' for the 'Country source'.");
+          $form_state->setError($form['country']['country_source'], $error);
+        }
+        if (empty($is_exposed)) {
+          $error = $this->t('This filter must be exposed to use a filter to specify the country.');
+          $form_state->setError($form['country']['country_source'], $error);
+        }

One uses active voice, one uses passive voice. Pick one (don't we default to passive?)

+  public function valueForm(&$form, FormStateInterface $form_state) {
+    $this->valueOptions = [];
+    $this->formState = $form_state;

+  protected function getCountrySource() {

+  protected function getCurrentCountry() {

+  public function buildExposedForm(&$form, FormStateInterface $form_state) {

+  public function adminSummary() {

Missing docblock.

Also, it would be great to get a functional test for this. We can use a single view, and modify the filter settings in each test method (to cover predefined country / exposed filter / contextual filter).

Thanks for the detailed review! I'll reroll to fix your concerns ASAP. Indeed, it has been a lot of work to try to get this bullet-proof.

Meanwhile:

General complaint: we keep talking about "arguments", even though the views ui calls them "contextual filters". That's bound to confuse someone.

Hah, good point. I plead "not guilty"! ;) I've been working on this patch as someone who co-maintained Views for a while and thinks about how it all looks in the code. From a developer perspective, these are "arguments" everywhere in Views except the UI. Function names, configuration, class names, etc, etc. So, clearly, someone decided "Contextual filter" was nicer UX, and the UI was changed, but the underlying plumbing was never touched since it'd be a huge task to rename "argument". But you're right, end-users don't see that, so it's not right to expose this term into the UI help text like I've done. I'll fix the user-facing stuff.

However, what about the underlying code? Do you want the variables + settings and comments and such to refer to these as "contextual filter" to match the UI, or as "argument" to match the Views code, plugin classes, views configuration, etc?

Thanks again,
-Derek

We can let the underlying code say "argument", so we match the Views duality.

Re: #27 - cool, that sounds good.

New patch (hopefully) resolves all your concerns about code comments and user-facing text. Passive voice was used for all error messages. ;) (I tend to hate passive voice, but if that's the standard, I'll stick with it. Agree that consistency is good).

Re:
// Find all country_code arguments from address.module for the valid choices.

This comment is weird, I don't understand it. Is it copy paste from another code block?

Whoops. I wrote that comment before the code. My intention was to find all the arguments on the current View display that point to an address.module-provided country code to create the list of valid choices for the country_argument_id setting. However, I couldn't easily find a reasonable way to do this, since the argument is just a generic string argument and there doesn't seem to be a clean way to decide what it's pointing at so we can filter the argument options here. Perhaps by poking around in $argument->definition we could decide if the argument is pointing at the country code, but I fear we could have false positives if we simply rely on finding "country_code" in the field name or something. If we ever add a custom argument handler for country codes (e.g. to allow proper validation, etc), we can revisit this. For now, I left the @todo in the code. Let me know if you think this is worth spending more time on given the current state of things.

I'll try to find you in IRC to discuss a functional test for all of this. There aren't any current tests involving Views support, and if I'm going to spend time on it, I want to make sure it's in line with your vision.

Thanks,
-Derek

patch #28 seems to works very well with latest dev

bojanz wanted some functional tests for this. Here's the proposal from our IRC discussion:

Create a dummy view that contains the new exposed filter, as well as an optional country argument & exposed filter
Then you have 3 test methods, one for each source: 1) predefined country 2) argument 3) exposed filter
In each one you load the view, change the exposed filter settings to point to the right source, then render the view (visit the page), confirm the results
The new dummy view can be added to tests/modules/address_test
Test classes live in src/Functional/AdministrativeAreaFilterTest (probably can't render a view in a kernel test)
The tests only cover the output of the view, not all the Views configuration UI

Open question: do we create entities with address fields and actually validate that the filter is producing the right results, or do we just need to load the empty view and see that the exposed filter for administrative area has the right subdivision options depending on the country source?

If anyone wants to take this on, please do. :)

Actually, briefly looking at core/modules/views/tests, it seems that kernel tests can render views. In fact, maybe we want these test classes to be children of ViewsKernelTestBase?

So perhaps the test classes should live in tests/src/Kernel/Views/AdministrativeAreaFilterTest.php ?

I finally made time to look into writing automated Views tests for D8.

All the Kernel tests I looked at that touch views are operating directly on the View object, executing the query, and inspecting the results. From what I understand about bojanz's wishes here, we really do want functional tests that open the resulting view page and inspect the UI of the exposed filters. So, here's a patch to add these as Functional tests that extend WebTestBase.

The good news is that writing the tests revealed a bug. ;) I had forgotten to update the address config schema to handle the settings on the new administrative area filter. So that's also included with this patch.

Some notes (mostly to myself, but also to anyone else writing views tests) about a few of the gotchas I ran into:

A) The default view can't have filters or contextual filters pointing to an address field if that field doesn't exist and isn't attached to an entity. The address_test module is now defining an 'address_test' node type with the 'field_address' address field.

B) Views exposed filter forms are GET forms, not POST. So you need drupalGet() with an $options argument to define 'query', instead of trying to use drupalPostForm().

Anyway, when I run this test class locally:

php core/scripts/run-tests.sh --non-html --verbose --class 'Drupal\Tests\address\Functional\Views\AdministrativeAreaFilterTest'

I get these results:

Tests to be run:
  - Drupal\Tests\address\Functional\Views\AdministrativeAreaFilterTest

Test summary
------------

Drupal\Tests\address\Functional\Views\AdministrativeAreaFilt 226 passes 
                  19 messages

Test run duration: 1 min 10 sec
...

I believe this is finally RTBC, but it'd be great to get other eyes on the code, user-facing text, and the new test.

Added docblock for a few functions that were missing it.
Added tests that the static vs. dynamic label functionality works.

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\address\Functional\Views\AdministrativeAreaFilterTest

Test run started:
  Monday, February 27, 2017 - 22:14

Test summary
------------

Drupal\Tests\address\Functional\Views\AdministrativeAreaFilt 267 passes                           29 messages

Test run duration: 1 min 40 sec

If I choose the 'all' countries value, from the parent filter I'm getting the following error message if you use the 'select all' option in views. It appears as if an 'all' token is passed, rather than the list of countries. The work around is to just check them off but it's a bit tedious. I'm not familiar with views, I'd love to write a patch, but is the correct work around when 'all is selected' to just build the array out and pass it, or is this more fixable in views?

The website encountered an unexpected error. Please try again later.

InvalidArgumentException: Invalid country code "all" provided. in CommerceGuys\Addressing\AddressFormat\AddressFormatRepository->get() (line 22 of ./vendor/commerceguys/addressing/src/AddressFormat/AddressFormatRepository.php).

Argh, whoops. I forgot about 'all'. ;) I need to add some code here to ignore that and treat it as if no country has yet been selected.

However, this touches upon one of the @todo comments I left in the patch:

  if (is_array($input[$country_filter_identifier])) {
    // @todo Maybe the config validation should prevent multi-valued
    // country filters. For now, we use the first country in the list.
    $this->currentCountryCode = array_shift($input[$country_filter_identifier]);
  }

If the country filter allows multiple values, it's unclear how the administrative area filter should behave. I welcome input on this. Here are a few possible approaches:

A) Allow the filter to handle multiple countries. When multiple countries are selected, gather the administrative areas for all of them. Use optgroups to group the admin areas by country (to make the UI sane), do some magic use 'country_code-admin_area_code' as the keys (to ensure uniqueness), and then re-convert the values back to just the right admin area codes before we actually hand them off to the query (to make it actually work). However, this could lead to confusion. For example, if the country filter included both Brazil and Canada, end-users would probably be confused that if they select 'Pernambuco' as a filter value (a state in Brazil with admin area code 'PE'), and then they see results from both Pernambuco and Prince Edward Island ('PE' in Canada). :( To really make this "work" would probably require automatically re-writing the query so that for each specific admin area choice, we force a constraint on the parent country for that admin area. Sounds very complicated, bloaty, and error-prone.

B) Completely disallow the administrative area filter to work with multi-valued country filters (e.g. config validation).

C) Allow it to work, but only if the country filter has a single value (e.g. run-time validation). Maybe generate a message or other hint to the user that drilling down by admin area only works if they filter to a single country?

D) Silently select the first country in the list (what happens now). Probably not the best choice. :/

E) Other?

Thoughts?

Thanks,
-Derek

Weird, upon closer inspection, the code was already correctly ignoring 'All' as a possible country option. I *thought* I was already handling that case. ;) But the check was case sensitive, and I guess there are possible cases where 'all' is being sent instead of 'All', and that was getting past the checks.

So here's a new patch that is even more defensive about the possible country codes:

1) Inside getCurrentCountry() we now check that the value we're about to return is a key in the country list (as provided by $this->countryRepository->getList()). This way, if someone passes a bogus country code in the GET parameters, we don't get a PHP exception, only the 'An illegal choice has been detected.' error from the FAPI validation.

2) Because of this, we no longer have to try to handle 'All' or 'all' in the places that call getCurrentCountry(), since those are safely ignored.

3) To address my concerns in comment #35 about the proper handling of a multi-valued country filter, instead of silently using the first country value in the list, we now only consider a valid country selection if there's a single value (option C from my list above). I'm still not sure this is the best solution, and I'm open to trying to warn site builders (or end users) that the administrative area filter will only appear once you converge on a single country. But I'm not going to further complicate this (already huge) patch to do so unless bojanz requests it.

Thanks,
-Derek

Argh. Previously while trying to verify the tests in this patch were working, I was only running the new test class. I just tried running the full test suite for address module and got the following error:

    Drupal\Tests\address\FunctionalJavascript\AddressDefaultWidgetTest::testEvents
    Drupal\Core\Config\PreExistingConfigException: Configuration objects
    (field.storage.node.field_address) provided by address_test already exist
    in active configuration

Turns out that Drupal\Tests\address\FunctionalJavascript\AddressDefaultWidgetTest is programmatically defining a 'field_address' field, which was conflicting with the one I'm adding here to the default configuration of the address_test module. :(

This patch renames the new field to 'field_address_test' and updates the AdministrativeAreaFilterTest class accordingly. Now (almost) everything is passing:

php core/scripts/run-tests.sh --non-html --url http://localhost/address-dev --verbose --module address

Tests to be run:
  - Drupal\Tests\address\FunctionalJavascript\AddressDefaultWidgetTest
  - Drupal\Tests\address\FunctionalJavascript\ZoneTest
  - Drupal\Tests\address\Kernel\Formatter\DefaultFormatterTest
  - Drupal\Tests\address\Kernel\Formatter\PlainFormatterTest
  - Drupal\Tests\address\Unit\Plugin\Validation\Constraint\CountryConstraintValidatorTest
  - Drupal\Tests\address\Functional\Views\AdministrativeAreaFilterTest

...

Test summary
------------

Drupal\Tests\address\FunctionalJavascript\AddressDefaultWidg   5 passes
Drupal\Tests\address\FunctionalJavascript\ZoneTest             3 passes
Drupal\Tests\address\Kernel\Formatter\DefaultFormatterTest     0 passes   1 fails
Drupal\Tests\address\Kernel\Formatter\PlainFormatterTest       1 passes
Drupal\Tests\address\Unit\Plugin\Validation\Constraint\Count   6 passes
Drupal\Tests\address\Functional\Views\AdministrativeAreaFilt 267 passes                           29 messages

Test run duration: 6 min 20 sec
...

    There was 1 failure:

  1)
    Drupal\Tests\address\Kernel\Formatter\DefaultFormatterTest::testTaiwanAddress
    The TW address has been properly formatted.
    Failed asserting that false is true.

The Taiwan failure is showing up with a totally clean install of address from the end of the 8.x-1.x branch, so I'm ignoring it.

But at least all the other existing and new tests are passing.

Dear universe, can I please be done with this issue? ;)

Thanks,
-Derek

I opened #2859870: DefaultFormatterTest::testTaiwanAddress() is failing to report (and if anyone is so inclined, fix) the test failure for the Taiwan address formatting, since it obviously has nothing to do with this issue or patch.

Thanks,
-Derek

Rerolled, fixed phpcs warnings (unused use statements, wrong @param docs), fixed the test (wrong base class, wrong @group, usage of deprecated methods). Needs more cleanup, but that can happen while the people are using it.

Thanks, dww!