e-mission / em-public-dashboard

A simple and stupid public dashboard prototype.
BSD 3-Clause "New" or "Revised" License
0 stars 23 forks source link

Broken Dashboard for USAID Laos #172

Open Abby-Wheelis opened 3 weeks ago

Abby-Wheelis commented 3 weeks ago

I broke the dashboard for USAID-Laos last night by updating the list of labels. The partners for that project really wanted to reduce the list to make it easier for user to label, so I would prefer to not simply revert the list, but did not think through the backwards compatibility for the dashboard, so I'll need to work on that ...

Would it fix things to just update the translations to include both new AND old labels?

I think that would help some ... because the error is likely related to trying to translate the underlying values to labels, and I took some of the translations out and changed others ...

The good news is that this only affects Laos, so the impact of this error are limited.

shankari commented 3 weeks ago

@Abby-Wheelis what is the error in the logs? Have you confirmed that it is related to translating the values to labels?

Abby-Wheelis commented 3 weeks ago

I have the data loaded now, charts work when I input the old dynamic labels, but break in the same way I'm seeing on production when I use the new list.

I was hoping that it would work to just add the extra translations, but it does not.

When we compose the translation dictionaries, we match the values in "MODE" to the translations for those values in "en".

Since I took things out of "MODE", it is not enough to just add them back to "en".

Maybe I can come up with some sort of backwards compat hack to pull translations for values present in the dataframe but not the labels?

Abby-Wheelis commented 3 weeks ago

I think we also have to decide what is the expected behavior in this case: do we want the "old" labels to show up with their original translations? To group them in as other?

My best case scenario would be developing a mapping between the old labels and the new ones (ie we combined gas car drove alone and with others into gas car) but I think that is a bit too comprehensive a change to introduce in a quick patch.

@shankari what do you think about original labels vs other?

JGreenlee commented 3 weeks ago

@Abby-Wheelis

I am just seeing this; after thinking about it I've realized there are additional ramifications of removing options from the label options config beyond just missing translations on the public dashboard.

It would break footprint calculations (CHEER would just report footprints of 0 for trips labeled with the "old" labels) and it would break the UI if users go back to view trips that were labeled with one of the old choices (it would show up grey or brown because the UI doesn't know what base mode, what color, what icon to use)

As long as we are using the label options config as the source of truth for footprints, colors, icons modes, and performing runtime lookups with it, I don't think we can safely remove any options. Instead, we should mark them as "inactive" and have the UI filter out "inactive" options when it renders the mode & purpose popups.

Abby-Wheelis commented 3 weeks ago

As long as we are using the label options config as the source of truth for footprints, colors, icons modes, and performing runtime lookups with it, I don't think we can safely remove any options.

Ok, so I need to revert the change until we can reevaluate then sounds like, if the phone UI is broken that is really bad. They are mostly between user pools right now (it has been more than a month since the last onboarding and they are only asking for 1 month of data), so hopefully not too many people were affected.

Abby-Wheelis commented 3 weeks ago

Reverted in #129 until we can come up with a better solution