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

Broken Dashboard for USAID Laos #172

Open
Abby-Wheelis opened this issue Oct 24, 2024 · 6 comments
Open

Broken Dashboard for USAID Laos #172

Abby-Wheelis opened this issue Oct 24, 2024 · 6 comments

Comments

@Abby-Wheelis
Copy link
Member

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
Copy link
Contributor

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

@Abby-Wheelis
Copy link
Member Author

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
Copy link
Member Author

Abby-Wheelis commented Oct 25, 2024

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
Copy link
Contributor

JGreenlee commented Oct 25, 2024

@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
Copy link
Member Author

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
Copy link
Member Author

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

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

No branches or pull requests

3 participants