phetsims / rosetta

PhET's Simulation Translation Utility
MIT License
3 stars 1 forks source link

Common strings don't update on previously submitted translations #43

Closed phet-steele closed 4 years ago

phet-steele commented 9 years ago

After submitting a translation for MAL, I switched to Chains (without changing locales) and changed some translations for common strings. I then submitted, got a correct looking debug info and commit, and went to check if my previous submission of MAL on the website had the new strings in it automatically. It didn't; isn't it a desired function that it should? Going back to MAL and resubmitting fixed the issue.

aaronsamuel137 commented 9 years ago

I don't think we want this functionality. When a sim is translated, it shouldn't be update automatically every time a new common string is translated. If they want to update it and they are the trusted translator, they can republish.

Any thoughts on this @ariel-phet?

ariel-phet commented 9 years ago

@pixelzoom can you comment on the current paradigm for Java translations. I believe we don't republish sims when common code strings are changed. Is that correct?

pixelzoom commented 9 years ago

For Java/Flash, we don't republish sims for any translations. We modify the existing jar files. For sim translations, we do this fairly soon after receiving the translations - @oliver-phet would be the person to ask about when it's typically done. For common-code strings (which are all in one bundle, not spread out across repositories as we do for HTML), those translations are published less frequently, because the process is long, and must be monitored. I believe @jonathanolson handles that, and he could tell you how often it's done.

ariel-phet commented 9 years ago

@oliver-phet could you comment on this issue?

oliver-phet commented 9 years ago

New sim translations come in weekly. I usually try to get them processed within a few days of receiving them from translators. However, Java/Flash translations have been slowing down a lot (we used to get several every day, now we get a few a week). Common strings are all done by @jonathanolson, but once they are published they are reflected in every sim that uses the common string.

phet-steele commented 9 years ago

@ariel-phet @aaronsamuel137 it's sounding like @oliver-phet is saying common strings should change (whether automatically or manually) in every sim after a string is updated. Are we going ahead with this at some point or not? What is going to be the function here so I know what to say in the help document :)

aaronsamuel137 commented 9 years ago

For now, sims are not automatically updated when a common string changes. The translator has the option to manually resubmit that sim if they want to updated it.

That being said, it wouldn't be too difficult to republish all translations of a locale when a common string changes if we want to. I don't think we should worry about this for 1.0.

Any thoughts @ariel-phet?

ariel-phet commented 9 years ago

I agree, not for 1.0, unassigning for the moment

jbphet commented 7 years ago

@ariel-phet - I'm reviving this discussion for the chipper:2.0 effort. In my opinion, we should not update all simulations that use a common string when said string is updated. I'm saying this not so much because I don't like the idea - I'm rather neutral on it - but it would take some effort to implement this, would likely cause a lot more builds and version updates to occur, and wouldn't add a tremendous amount of value. We should decide whether to close (my vote), defer, or tee this up for chipper:2.0.

pixelzoom commented 7 years ago

@jbphet said:

In my opinion, we should not update all simulations that use a common string when said string is updated. ...

Adding to the complexity is that we'd need to figure out which common-code strings a sim actually uses -- e.g.., we wouldn't want to update a sim just because it uses scenery-phet and a scenery-phet string was updated.

So if we don't update, then the latest common strings would only be picked up when we explicitly republish a sim, correct?

ariel-phet commented 7 years ago

@jbphet I think I am in agreement that we should likely close.

In particular, we want to in some sense minimize version updates (especially as our app popularity grows).

And I think you are correct @pixelzoom, although wouldn't the latest common strings get picked up when a translation is updated. So lets say update a Spanish translation in terms of sim specific strings, won't that trigger a build that grabs latest common strings?

ariel-phet commented 7 years ago

@pixelzoom I guess I think of a new translation being built as not us "explicitly republishing"

Or are you saying, those common strings in a sense would not exist because the sim is using old versions of the repos from which they were taken?

pixelzoom commented 7 years ago

@ariel-phet asked:

And I think you are correct @pixelzoom, although wouldn't the latest common strings get picked up when a translation is updated. So lets say update a Spanish translation in terms of sim specific strings, won't that trigger a build that grabs latest common strings?

I don't know anything about what happens when a translation is received. But I would think that when an update for an existing translation is received, that had would trigger building a new version of the sim (with next version id), otherwise Spanish users won't be notified of the update. And in that case, yes, the latest Spanish common-code strings would come along for the ride.

pixelzoom commented 7 years ago

@ariel-phet asked:

Or are you saying, those common strings in a sense would not exist because the sim is using old versions of the repos from which they were taken?

I believe that translated strings are always taken from babel master, not a branch. But should confirm that with @jbphet.

ariel-phet commented 7 years ago

@jbphet if translated strings are always taken from master (including common strings), feel free to close. If not please assign back to me for further discussion.

jbphet commented 7 years ago

Yes, common code strings are always taken from master - no branches are created. In one of the comments above, @pixelzoom said, "[T]he latest common strings would only be picked up when we explicitly republish a sim, correct?" The answer is yes, but to be clear, there are two ways that I'm aware of that would cause an explicit republication:

  1. A translator submits an update to the simulation's strings via the translation utility.
  2. A maintenance update occurs for the simulation.

I should mention that for item 1, the user currently would have to change something and submit it for this to work, i.e., they couldn't just open up the translation page for the sim, pick up the changes to common code string(s), and hit "Submit". This is a known issue and may be fixed during chipper:2.0, see https://github.com/phetsims/rosetta/issues/127.

pixelzoom commented 7 years ago

@jbphet can you clarify? When a translator submits an updated translation, does that result in the sim being published with a new version id? Or is the sim republished with the current version id?

jbphet commented 7 years ago

The sim is re-built with the same version ID an only the strings change. Version IDs remain consistent with the English version.

pixelzoom commented 7 years ago

Yikes, that seems very wrong (and very English centric). If a non-English user has a downloaded version, they will not be notified that their version has been updated with a new translation. And presumably this affects the PhET apps too.

jbphet commented 7 years ago

Assigning to @ariel-phet to decide if he also thinks that the decision that was made while back to not trigger version number changes, and thus not cause update notifications, for changes to non-English translations "seems very wrong". This could potentially impact our version number scheme, so should be decided fairly quickly, since it could impact other work being done for the chipper:2.0 effort.

ariel-phet commented 7 years ago

A question for @mattpen - my understanding is that the apps rely on meta data from the website that involves "published time" not the version id.

mattpen commented 7 years ago

Currently the apps will download a new simulation if the hash of the English file changes, it's not dependent on build time or version id. Presently we do not support localized sims in the app.

We are currently tracking the translate time of each localized sim in the website database and providing this time in the Metadata API.

The plan for including localized sims in the app is that they will track this translate time from the metadata api and update the sims when the user's preferred locale is translated or the sim is updated.

Example of metadata:

$ curl -s 'https://phet.colorado.edu/services/metadata/1.2/simulations?format=json&type=html&simulation=make-a-ten&debug' | grep 'translate\|locale\|name'
    "name": "html/make-a-ten",
      "name": "make-a-ten",
          "translateTime": "2017-02-28 16:57:57.757",
          "locale": "en",
          "translateTime": "2017-03-01 21:01:22.581",
          "locale": "vi",
          "translateTime": "2017-03-02 01:07:08.954",
          "locale": "it",
          "translateTime": "2017-03-02 06:33:23.296",
          "locale": "fr",
          "translateTime": "2017-03-02 07:10:24.301",
          "locale": "el",
          "translateTime": "2017-03-02 09:13:02.46",
          "locale": "da",
          "translateTime": "2017-03-02 12:48:50.112",
          "locale": "pt_BR",
          "translateTime": "2017-03-02 13:35:01.439",
          "locale": "nl",
          "translateTime": "2017-03-03 10:14:07.947",
          "locale": "es",
          "translateTime": "2017-03-03 00:21:46.629",
          "locale": "pl",
          "translateTime": "2017-03-04 08:44:11.255",
          "locale": "es_PE",
          "translateTime": "2017-03-07 05:46:11.052",
          "locale": "eu",
          "translateTime": "2017-03-17 16:54:15.059",
          "locale": "lv",
          "translateTime": "2017-03-30 07:48:34.921",
          "locale": "ko",
          "translateTime": "2017-04-16 01:10:52.184",
          "locale": "zh_TW",
          "translateTime": "2017-04-18 04:02:34.32",
          "locale": "zh_CN",
          "translateTime": "2017-05-04 06:17:58.532",
          "locale": "mr",
          "translateTime": "2017-05-12 20:02:41.514",
          "locale": "es_MX",
          "translateTime": "2017-06-02 01:32:51.012",
          "locale": "de",
          "translateTime": "2017-07-22 06:38:32.376",
          "locale": "hu",
          "translateTime": "2017-08-02 03:50:36.512",
          "locale": "ru",
          "translateTime": "2017-08-06 01:06:14.345",
          "locale": "sr",
          "translateTime": "2017-08-15 01:56:07.821",
          "locale": "kk",
          "translateTime": "2017-09-10 15:35:54.68",
          "locale": "sv",
          "translateTime": "2017-11-03 13:24:09.815",
          "locale": "bs",
ariel-phet commented 7 years ago

@jbphet @pixelzoom since the plan is to use the translate time for the apps to trigger updates, I think the current version id scheme is fine (in my opinion). It seems to me to keep things a bit cleaner...that way the "all.html" version id will always be in lockstep with the published version.

pixelzoom commented 7 years ago

The plan for triggering app dates sounds fine.

But I'm not on board with what this means for sim updates in general. My understanding is that the "Check for Updates" feature within sims is based on version id. (@jonathanolson correct me if I'm wrong.) So if you update a translation by regenerating the html file without changing the version id, then users of that translation will NOT be notified of the update. Imo, this is an English-centric solution, and is antithetical to PhET's commitment to i18n.

jbphet commented 7 years ago

If we're relying purely on the version ID and the file name that is based on it to decide about updates, I think we should consider changing this. It forces us to try to put what is, IMO, too much information in the version ID. I'd be up for having some piece of meta data that is either embedded in the built sim, available from the web site's metadata, or both, that could be used to make additional information available about a sim. If this were available, the build time itself could be used to resolve the problem of making it known that a new translation is available. Or, we could start tracking a "translation number" value if we wanted to be more explicit about it.

pixelzoom commented 7 years ago

Re @jbphet's suggestion in https://github.com/phetsims/rosetta/issues/43#issuecomment-347252364... We'd also need to change the "Check for Update" user interface, otherwise the user will see the nonsensical (for example) "You have version 1.1.0, a new version 1.1.0 is available."

jonathanolson commented 7 years ago

If we're relying purely on the version ID and the file name that is based on it to decide about updates

Our server gets to decide whether the sim is considered "out-of-date", see https://github.com/phetsims/joist/issues/189#issuecomment-112884265 for more on how the update feature works.

pixelzoom commented 7 years ago

To further clarify https://github.com/phetsims/rosetta/issues/43#issuecomment-347260570 ...

UpdateDialog (by calling UpdateNodes.createOutOfDateDialogNode) displays these 2 strings (from joist-strings_en.json) when an update is available:

  "updates.newVersionAvailable": {
    "value": "There is a new version available: {0}."
  },
  "updates.yourCurrentVersion": {
    "value": "Your current version is: {0}."
  },

So the user will see the same version id for both the current and new version, for example:

There is a new version available: 1.1.0. Your current version is: 1.1.0.

This is confusing and nonsensical.

jonathanolson commented 7 years ago

Ahh sorry, I think I misunderstood.

So we want to notify the user of updated translations to their locale, but our current UI for that would show a confusing message?

Presumably the website or some tooling would need to have a complete history of every build timestamp, and which locales were updated.

jbphet commented 5 years ago

This issue has been idle for about a year and a half. In the dialog above, it seems to me that we essentially come to a consensus that the current behavior with respect to common strings is what we want, meaning that if a user submits a translation that changes a shared string, we do not automatically rebuild all sims that use that string.

There is further discussion in the issue about whether string changes should trigger version updates, how we let users know that a new translation version is available, and so forth. The issue is assigned to me, but it's not clear to me what I can or should do about it. I'm going to assign it to @ariel-phet to determine what should be done, if anything. If it is decided that some followup is necessary, I'd suggest logging a separate GitHub issue about versioning and translations, and link it back to this one (which could then be closed).

ariel-phet commented 4 years ago

@jbphet I think we can close this issue.

  1. I agree, the current behavior with respect to common strings is what we want. I don't believe common strings get updated particularly often, and it seems that when they do it would be a "better/more desirable translation" not necessarily a translation that is plain wrong. We haven't really heard any complaints about such things from our translators or international users, so seems like a non-issue currently.

  2. As for the discussion above concerning string changes and updates...the app handles all of this nicely for supported locales, and the "check for updates" in the PhET menu, I am not sure how much this feature is used. Really, its only use would be for someone using an HTML5 sim offline, and then having the ability to be online and see if a new version is available. I don't necessarily see an updated translation as being as high priority as an updated version (which would imply a bug fix of some sort). Again, I know of no issues reported by users or translators to suggest we need to notify regarding updated translations. It is true, that is a fairly English-centric view, but until we hear otherwise, I don't see the value in investing in what would likely be a fairly extensive change to versioning/our app update schema/etc

Closing