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

[Bug : Tour Editor] Air quality is not displayed when it's saved with a different language #1193

Closed
wolfgang-ch opened this issue Oct 6, 2023 · 17 comments
Labels
Milestone

Comments

@wolfgang-ch
Copy link
Collaborator

What happened ?

The air quality is saved as string with the selected language. The tour editor do not select the saved air quality when the language has changed.

Version

23.10 and before

System

All

Relevant log output

No response

@wolfgang-ch wolfgang-ch added the Bug label Oct 6, 2023
@wolfgang-ch
Copy link
Collaborator Author

I think we have to change this in further versions, that air quality is saved as a number which is independend of the language

@wolfgang-ch
Copy link
Collaborator Author

wolfgang-ch commented Oct 6, 2023

After thinking more about this issue, we should use the weather_AirQuality field the same way as the weather_Clouds field. In this field, an english tag is saved which can then be translated and displayed according to the selected language.

The database upgrade process has to convert the translated texts into the weather air quality tags.

@Thompson-ongithub
Copy link

I don't know if it's the same issue, but already when updating from 23.5 to 23.8 (which I did just recently on my way to test 23.10) whatever is inserted in "air quality" gets removed (MTB in German)

@wolfgang-ch wolfgang-ch added this to the 23.next milestone Oct 15, 2023
@FJBDev
Copy link
Collaborator

FJBDev commented Oct 15, 2023

I don't know if it's the same issue, but already when updating from 23.5 to 23.8 (which I did just recently on my way to test 23.10) whatever is inserted in "air quality" gets removed (MTB in German)

I can't reproduce the issue on 23.10, can you provide a scenario and/or video capture to show the problem ?

@Thompson-ongithub
Copy link

Thompson-ongithub commented Oct 15, 2023

In version 23.5 I had a few gps-tracks where "air quality" was set when updating the weather information. After updating this to version 23.8 "air quality" was set to "nicht ausgewählt" (not selected).
Only after updating the weather information once more (by clicking on "weather", obviously) and getting updates from openweathermap air quality once again was set.

@FJBDev
Copy link
Collaborator

FJBDev commented Oct 15, 2023

@Thompson-ongithub When you updated from 23.8 to 23.10, did you have the same issue ? Also, did you happen to switch languages or did you keep using MT in german all that time ?
@wolfgang-ch Since you use german too, did you encounter the issue ?

@Thompson-ongithub
Copy link

@Thompson-ongithub When you updated from 23.8 to 23.10, did you have the same issue ? Also, did you happen to switch languages or did you keep using MT in german all that time ? @wolfgang-ch Since you use german too, did you encounter the issue ?

If I remember correctly when upgrading from 23.8 to 23.10 the air quality wasn't removed (I'll double check later this evening)
And no, I never switched langauges. I'm using MTB in German since like forever ;)

@Thompson-ongithub
Copy link

@Thompson-ongithub When you updated from 23.8 to 23.10, did you have the same issue ? Also, did you happen to switch languages or did you keep using MT in german all that time ? @wolfgang-ch Since you use german too, did you encounter the issue ?

If I remember correctly when upgrading from 23.8 to 23.10 the air quality wasn't removed (I'll double check later this evening) And no, I never switched langauges. I'm using MTB in German since like forever ;)

I just double checked: when upgrading from 23.08 to 23.10beta "air quality" is remembered (i.e. is still set where it was set in 23.08)

@wolfgang-ch
Copy link
Collaborator Author

Because the translated text is saved in the database field and the german translation was changed #1124 (comment) there is now a mess.

I was not aware that the tranlated text is saved when I changed the translation but this issue will and can be fixed in the following version after 23.10

@Thompson-ongithub
Copy link

Because the translated text is saved in the database field and the german translation was changed #1124 (comment) there is now a mess.

I was not aware that the tranlated text is saved when I changed the translation but this issue will and can be fixed in the following version after 23.10

Thanks.
(although I don't really use air quality) ;)

@FJBDev
Copy link
Collaborator

FJBDev commented Oct 19, 2023

Because the translated text is saved in the database field and the german translation was changed #1124 (comment) there is now a mess.

I was not aware that the tranlated text is saved when I changed the translation but this issue will and can be fixed in the following version after 23.10

I have done a fix here for the next version.

there is now a mess

Sorry for the "mess" !

@wolfgang-ch
Copy link
Collaborator Author

I also already started a fix for this issue 5e4b82c + 6ba1d75 and used simple SQL which is way faster than EJB, you cannot even see the update progress in the UI.

I was not aware that you already generalized the concurrent update data process in updateDb__3_Data_Concurrent() which is not necessarily needed for this update but I will merge your fix into the main branch when MT 23.10 is released.

@FJBDev
Copy link
Collaborator

FJBDev commented Oct 20, 2023

I was not aware that you already generalized the concurrent update data process in updateDb__3_Data_Concurrent() which is not necessarily needed for this update

I used a concurrent update because, for example, in my PROD MT, ALL my tours from 2020 (27th November 2020) to 2023 have air quality and I thought it would be a lot of tours and hence better with concurrency.

@FJBDev
Copy link
Collaborator

FJBDev commented Oct 20, 2023

but I will merge your fix into the main branch when MT 23.10 is released.

That sounds good. It's probably too late and risky to put it in 23.10

@wolfgang-ch
Copy link
Collaborator Author

I was not aware that you already generalized the concurrent update data process in updateDb__3_Data_Concurrent() which is not necessarily needed for this update

I used a concurrent update because, for example, in my PROD MT, ALL my tours from 2020 (27th November 2020) to 2023 have air quality and I thought it would be a lot of tours and hence better with concurrency.

When only a sql field must be updated, like in this issue, then I think it's faster with native sql commands because EJB is loading the whole TourData with all it's relations and native sql command could also be run concurrently

@wolfgang-ch
Copy link
Collaborator Author

but I will merge your fix into the main branch when MT 23.10 is released.

That sounds good. It's probably too late and risky to put it in 23.10

After having published a test version then I don't change the database version in final fixes

@FJBDev
Copy link
Collaborator

FJBDev commented Oct 23, 2023

After having published a test version then I don't change the database version in final fixes

That makes sense, it's better not take any risks for new releases

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

No branches or pull requests

3 participants