Skip to content
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

add an alternate flat locations fixture #12543

Merged
merged 12 commits into from
Jul 21, 2016
Merged

Conversation

czue
Copy link
Member

@czue czue commented Jul 20, 2016

This adds a feature flag that adds a second locations fixture in a flat format (which is often easier to work with in apps).

The format might change a bit, but want to iterate on this with kriti for UATBC.

<fixture id="commtrack:locations_v2" user_id="d7c3c4246b1126d6506522d4feb235df">
    <locations>
      <location id="dae90df587e610adb7baedf1d5038abf" state_id="dae90df587e610adb7baedf1d5039270" type="city">
        <name>Cape Town</name>
        <site_code>cape_town</site_code>
        <external_id/>
        <latitude/>
        <longitude/>
        <location_type>City</location_type>
        <supply_point_id>23b435953e894b8dbfc5ad0e03f0b81d</supply_point_id>
        <location_data/>
      </location>
      <location city_id="dae90df587e610adb7baedf1d5038abf" id="dae90df587e610adb7baedf1d503885a" state_id="dae90df587e610adb7baedf1d5039270" type="neighborhood">
        <name>Vredehoek</name>
        <site_code>vredehoek</site_code>
        <external_id/>
        <latitude>-33.9411000000</latitude>
        <longitude>18.4232000000</longitude>
        <location_type>Neighborhood</location_type>
        <supply_point_id>ad9dbae5bbd24a12b1c352e0b625ef03</supply_point_id>
        <location_data/>
      </location>
      <location id="dae90df587e610adb7baedf1d5039270" type="state">
        <name>Western Cape</name>
        <site_code>western_cape</site_code>
        <external_id/>
        <latitude/>
        <longitude/>
        <location_type>State</location_type>
        <supply_point_id>896373cca62c454f8d9fa41a544e7e6a</supply_point_id>
        <location_data/>
      </location>
    </locations>
  </fixture>

@esoergel @sheelio and @proteusvacuum probably interested. can review either way. tried to break up by 🐟 but took a couple small detours.

@esoergel
Copy link
Contributor

Errors look real:

======================================================================
ERROR: test_fixtures_by_id (casexml.apps.phone.tests.test_ota_fixtures.OtaFixtureTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/commcare-hq-ro/corehq/util/test_utils.py", line 232, in inner
    return helper(*args, **kwargs)
  File "/mnt/commcare-hq-ro/corehq/util/test_utils.py", line 205, in __call__
    call_with_settings(fn_with_pre_and_post, run_config.settings, args, kwargs)
  File "/mnt/commcare-hq-ro/corehq/util/test_utils.py", line 220, in call_with_settings
    fn(*args, **kwargs)
  File "/mnt/commcare-hq-ro/corehq/util/test_utils.py", line 201, in fn_with_pre_and_post
    self.fn(*args, **kwargs)
  File "corehq/ex-submodules/casexml/apps/phone/tests/test_ota_fixtures.py", line 113, in test_fixtures_by_id
    fixture_xml = generator.get_fixture_by_id('user-groups', self.restore_user, version=V2)
  File "corehq/ex-submodules/casexml/apps/phone/fixtures.py", line 82, in get_fixture_by_id
    fixtures = self._get_fixtures(None, fixture_id, user, version, last_sync, None)
  File "corehq/ex-submodules/casexml/apps/phone/fixtures.py", line 74, in _get_fixtures
    providers = self.get_providers(user, version, group, fixture_id)
  File "corehq/ex-submodules/casexml/apps/phone/fixtures.py", line 69, in get_providers
    providers = [provider for provider in providers if provider_matches(provider)]
  File "corehq/ex-submodules/casexml/apps/phone/fixtures.py", line 67, in provider_matches
    return provider.id == full_id or provider.id == prefix
AttributeError: 'LocationFixtureProvider' object has no attribute 'id'

tmp_location = location
while tmp_location.parent:
tmp_location = tmp_location.parent
attrs['{}_id'.format(tmp_location.location_type.code)] = tmp_location.location_id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this looks useful!

@@ -60,6 +60,11 @@ def should_sync_locations(last_sync, location_db, restore_user):
class LocationFixtureProvider(object):
id = 'commtrack:locations'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snopoke I updated this fixture generator to optionally return two fixtures. as per the comment on https://github.com/dimagi/commcare-hq/blob/master/corehq/ex-submodules/casexml/apps/phone/fixtures.py#L33 it seems like I should change the id to just be 'commtrack' based on that change, but was wondering exactly how the ID is used. 'commtrack' is also a prefix for the product fixture and not sure how that would affect it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea that should work. it will just mean that for any commtrack fixture requested by cloudcare it will generate both of these even if they aren't required.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice thanks. ended up just making two providers with different IDs

@sheelio
Copy link
Contributor

sheelio commented Jul 20, 2016

@czue @esoergel

Would it be feasible to remove commtrack from that fixture ID? Not important, but don't really have commtrack as abrand.

@czue
Copy link
Member Author

czue commented Jul 20, 2016

I can definitely remove it form the new one. for the old one we can cover it in the migration conversation

@esoergel
Copy link
Contributor

Looks g2m on build

id = 'locations'

def get_xml_nodes(self, restore_user, all_locations):
if not toggles.FLAT_LOCATION_FIXTURE.enabled(restore_user.project.name):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really that important since project is memoized, but more as an fyi the restore user has a domain property that is exactly what this is

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@czue
Copy link
Member Author

czue commented Jul 21, 2016

updated to use different providers and moved the location queries to the user so they could be cached. this should be g2g when ✅

@czue
Copy link
Member Author

czue commented Jul 21, 2016

@proteusvacuum proteusvacuum merged commit a7e6c10 into master Jul 21, 2016
@proteusvacuum proteusvacuum deleted the cz/flat-locations-fixture branch July 21, 2016 14:00
@proteusvacuum
Copy link
Contributor

🌐

@esoergel
Copy link
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants