OpenUpSA / wazimap-ng-ui

User interface for Wazimap next generation. See also https://github.com/OpenUpSA/wazimap-ng
https://geo.vulekamali.gov.za/
Apache License 2.0
5 stars 16 forks source link

Choropleth for subplaces on YE break #223

Closed adieyal closed 3 years ago

adieyal commented 3 years ago
  1. Navigate to https://beta.youthexplorer.org.za/#geo:165005
  2. Open Data Mapper
  3. Select Demographics -> Language -> Language most spoken at home -> English

This breaks - see the error in the console

helpers.ts:111 Uncaught TypeError: Cannot read property '_layers' of undefined
    at t.value (layerstyler.js:37)
    at t.value (layerstyler.js:62)
    at choropleth.js:56
    at Array.forEach (<anonymous>)
    at c.value (choropleth.js:51)
    at choropleth.js:12
    at utils.js:143
    at Array.forEach (<anonymous>)
    at c.value (utils.js:142)
    at c.value (controller.js:76)
    at c.value (controller.js:245)
    at choropleth.js:5
    at utils.js:143
    at Array.forEach (<anonymous>)
    at a.value (utils.js:142)
    at a.value (mapchip.js:123)
    at choropleth.js:28
    at s (helpers.ts:87)

Refers to https://trello.com/c/RkasByMS/3-ref-6-grey-choropleth-at-the-small-area-level-are-we-missing-data-or-is-it-something-else

https://trello.com/c/QjdKd1aR/635-choropleth-for-subplaces-on-ye-break

emre2038 commented 3 years ago

just a note for the future, this error happens if the user has already selected a subindicator before navigating to #geo:165005. so the correct order to reproduce this issue is :

1 - Open Data Mapper 2 - Select Demographics -> Language -> Language most spoken at home -> English 3 - Click through to the geography #165005 (SA > Western Cape > Cape Winelands > Witzenberg > Prince Alfred Hamlet)

emre2038 commented 3 years ago

@adieyal geo:WC022 has 23 children(11 mainplaces, 12 wards) image

but under profile > profile_data > Demographics > subcategories > indicators > Language most spoken at home > subindicators languages have different number of children for example

is this expected / normal?

this inconsistency causes the issue. should we prevent that error occuring or should the data be corrected?

@milafrerichs FYI

adieyal commented 3 years ago

I can only imagine that perhaps the values are 0 for some of those geographies and they are not sent through.

On Tue, Jan 19, 2021 at 2:45 PM emre2038 notifications@github.com wrote:

@adieyal https://github.com/adieyal geo:WC022 has 23 children(11 mainplaces, 12 wards) [image: image] https://user-images.githubusercontent.com/53019884/105035713-83416180-5a6c-11eb-9551-9a74f70fb590.png

under profile > profile_data > Demographics > subcategories > indicators > Language most spoken at home > subindicators languages have different number of children for example

  • Afrikaans has 23
  • English has 22
  • Isindebele has 14
  • Sepedi has 16....

is this expected / normal?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenUpSA/wazimap-ng-ui/issues/223#issuecomment-762818295, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADGXMUQ2ANC4YSIO77TEQDS2V5HPANCNFSM4WDZ7MQQ .

--

Adi Eyal Director, OpenUp 1 Thicket Street, Newlands, Cape Town www.openup.org.za | +27 21 671 6306

Facebook https://www.facebook.com/OpenUpSA/ | Twitter https://twitter.com/openupsa | GitHub https://github.com/openupsa/

OpenUp is a non-profit organisation registered with the South African Department of Social Development, number 133-850NPO. Licensed under a Creative Commons Attribution 4.0 International License.

emre2038 commented 3 years ago

so should we do anything on FE? or is this a BE job?

adieyal commented 3 years ago

I think frontend unless I misunderstood the issue. The data looks fine to me, it's how the frontend deals with it that is the problem.

On Wed, Jan 20, 2021 at 8:31 AM emre2038 notifications@github.com wrote:

so should we do anything on FE? or is this a BE job?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenUpSA/wazimap-ng-ui/issues/223#issuecomment-763371178, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADGXMV7QD74EJS57ESLDOTS2Z2DLANCNFSM4WDZ7MQQ .

--

Adi Eyal Director, OpenUp 1 Thicket Street, Newlands, Cape Town www.openup.org.za | +27 21 671 6306

Facebook https://www.facebook.com/OpenUpSA/ | Twitter https://twitter.com/openupsa | GitHub https://github.com/openupsa/

OpenUp is a non-profit organisation registered with the South African Department of Social Development, number 133-850NPO. Licensed under a Creative Commons Attribution 4.0 International License.

emre2038 commented 3 years ago

I can assume the data is 0 if it is missing and do the calculations accordingly. this would prevent the error. is this ok?

adieyal commented 3 years ago

Do you even know that it is there if it is missing?

On Wed, Jan 20, 2021 at 9:19 AM emre2038 notifications@github.com wrote:

I can assume the data is 0 if it is missing and do the calculations accordingly. this would prevent the error. is this ok?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenUpSA/wazimap-ng-ui/issues/223#issuecomment-763393938, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADGXMR3LSLTXSOB7UA7UWTS2Z7ZTANCNFSM4WDZ7MQQ .

--

Adi Eyal Director, OpenUp 1 Thicket Street, Newlands, Cape Town www.openup.org.za | +27 21 671 6306

Facebook https://www.facebook.com/OpenUpSA/ | Twitter https://twitter.com/openupsa | GitHub https://github.com/openupsa/

OpenUp is a non-profit organisation registered with the South African Department of Social Development, number 133-850NPO. Licensed under a Creative Commons Attribution 4.0 International License.

milafrerichs commented 3 years ago

not sure this has any to do with the data.

the break happens here:

https://github.com/OpenUpSA/wazimap-ng-ui/blob/staging/src/js/map/choropleth/choropleth.js#L54

this.currentLayers.forEach(code => {
  const layer = self.layers[code];
  self.layerStyler.setLayerToSelected(layer);

self.layers does not have all the codes that are in this.currentLayers that's why it breaks. it cannot reset these layers.

emre2038 commented 3 years ago

@adieyal in SubindicatorCalculator I can find out which children are missing.

@milafrerichs I know but children numbers being inconsistent causes that code to break. I just wanted to make sure that the data is correct. I can go on and fix it if the data is correct the way it is.

adieyal commented 3 years ago

I can't be certain that the data is correct, but it seems reasonable to me that data might only be sent where it exists. I think go-ahead with the fix and we can review the data later if it remains an issue.

On Wed, Jan 20, 2021 at 10:16 AM emre2038 notifications@github.com wrote:

@adieyal https://github.com/adieyal in SubindicatorCalculator I can find out which children are missing.

@milafrerichs https://github.com/milafrerichs I know but children numbers being inconsistent causes that code to break. I just wanted to make sure that the data is correct. I can go on and fix it if the data is correct the way it is.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenUpSA/wazimap-ng-ui/issues/223#issuecomment-763421891, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADGXMWOUQXBUU2TPP5RUDLS22GOJANCNFSM4WDZ7MQQ .

--

Adi Eyal Director, OpenUp 1 Thicket Street, Newlands, Cape Town www.openup.org.za | +27 21 671 6306

Facebook https://www.facebook.com/OpenUpSA/ | Twitter https://twitter.com/openupsa | GitHub https://github.com/openupsa/

OpenUp is a non-profit organisation registered with the South African Department of Social Development, number 133-850NPO. Licensed under a Creative Commons Attribution 4.0 International License.

emre2038 commented 3 years ago

ok Adi, thanks