🇫🇷France @fmb

At the same time, I notice that since commit d941cd35 from #3395327: Replace xmlrpc for new localize.drupal.org, we are not using that field anymore.

Mmmh, but we will need a way to store credentials for a given user in 📌 Localize client authentication with d.o infrastructure Active . But then, of course, nothing forces us to use a field if we judge this is not an appropriate solution (personally, I do not think it is, though it certainly appears to be the easiest method at first).

If for whatever reason we decide to stick with a field, we would need a way to advise users to manually delete the field first, just like we need to manually delete content when removing an entity type, for instance. Since this task might not be trivial, this is another argument against using a field in this context.

I notice that the submodule does not have a README.md or .txt.
Should we add one, or put everything in the main module README?

I think a README for the submodule would be fine.

fmb → created an issue.

Thanks!

All right, let's move it here then.

The commented part in l10n_packager_cron() creates queues that will later parse releases. This part did not exist in the Drupal 7 version. Besides, we already have a drush command (drush 10n_server:parse) which implements this and could be used directly in a crontab. Implementing this feature also in a queue would either lead to code duplication or require some refactoring. How relevant is this?

fmb → created an issue.

fmb → created an issue.

Validation des traductions et poursuite du développement de Localize D10.

nicoloye → credited fmb → .

fmb → created an issue.

What is the translation you are trying to save?

fmb → created an issue.

fmb → created an issue.

Thanks!

fmb → made their first commit to this issue’s fork.

Thanks!

See 📌 Update groups to 3.x Active .

I think this has long been fixed.

fmb → created an issue.

submit suggestions. moderate suggestions from others, moderate own suggestions are now group permissions instead of global permissions.

In a follow-up issue, we should also review other permissions (see attached capture of Drupal 7 group permissions configuration). As some of them are currently used in routes, I am leaving as they are for the time being.

fmb → created an issue.

Fastly dealt with in 📌 Review integration of l10n_drupal_rest with drupalorg_crosssite Active .

Fixed in 📌 Improve code and performances of the homepage Active .

Make this step optional and improve documentation to explain how to import projects and strings.

For performance reasons, these data will be kept in cache for 3600 seconds before they are updated.

For performance reasons, these data will be kept in cache for 3600 seconds before they are updated.

Will be dealt with in 📌 Finalize l10n_packager route /downloads. Active .

This feature will not be supported by the new 3.0.x branch.

It turns out that docker-compose.chromedriver.yaml is not actually needed by anybody working on the project, so I removed it.

Wow, thanks a lot Randy for passing by here! Much appreciated.

I am sorry I did not give enough details. The main module is here, but the platform is on gitlab.com. We are talking about this file.

I think we are good regarding suggestions, can somebody review this?

Now, It looks like at least some global permissions defined in l10n_community.permissions.yml should be turned into group permissions. Then, we may get rid of global permission checks in L10nAccess, but this could be done in a follow-up issue.

fmb → created an issue.

Even as a maintainer, I cannot use "git push --force" on a branch which is used for a dev release.

Mainly, the culprit is this commit which caused a conflict in l10n_contributor.module which was not correctly dealt with, introducing again the l10n_client_contributor_send_translation() function which had been previously removed in a previous commit (for good reason, as it is now implemented in a service, plus it used xmlrpc). Some things need to be removed (like old xmlrpc-based tests) but this commit cannot be held as responsible.

I think this can now be closed, since the rest of the work (authentication) will be dealt with in 📌 Localize client authentication with d.o infrastructure Active .

I was able to successfully test this in a new installation (the only thing to know is that you need to manually install External Entities first because of some dependency on external_entity_type, but I guess this is a completely different matter). Thanks!

@guignonv I wondered why this MR was so gigantic when I realized you had requested a merge into the 7.x-1.x branch. I took the liberty to edit the MR to set it to 2.0.x.

Thanks Guiu! I successfully tested this on a development Drupal 7 instance. Failing tests are not related to this change. @drumm, could you deploy this onto the live instance (localize.drupal.org)?

@kartagis for now, the fix was applied to the 3.0.x branch of the l10n_server module, which is currently under development. The next step is to backport it to the older 7.x-1.x branch, which is still the live version (i.e. the on on localize.drupal.org).

It is a good thing that JSON:API does not match, it means it will not be mistaken as a replacement token.

Thanks Guiu! This now needs to be backported to 7.x-1.x-dev.

fmb → made their first commit to this issue’s fork.

Thanks @donquixote!

Thanks for spending time on analyzing this bug. We do not really care about data deletion in this case. I would say it is expected to remove the key when the module is uninstalled. Generating a new token would not be that hard anyway.

Thanks for reporting this. Is it also happening with the latest Git version?

String 5052 (Audio File Info) does not have this issue. In string 2718592, the "JSON:API-specification-compliant" is partly interpreted as a replacement token (:API-specification-compliant), apparently because of the colon. If you can replicate this bug, it might be related to the regexp in the TranslateForm::validateForm() method.

I talked to the maintainer of the Group module today. As it turns out, we can define a group permission (in a .group.permission.yml file), and call $group->hasPermission() wherever we need to check access (the group can be retrieved via the language code, maybe not by using the route, see L10nAccess::check()).

We do no actually need relationship entities, as we do not want to restrict view access to translation suggestions.

• @kartagis what module does this string come from?
• I think the whole :API-specification-compliant pattern should be present in the translation.

We still need to:

• Clean up the code. I guess most of the code in l10n_client_contributor.module is obsolete (talking about you, l10n_client_contributor_send_translation()).
• Figure out how to deal with authentication ( 📌 Localize client authentication with d.o infrastructure Active ).

Setting priority as major as this is both a regression and a security issue.

Please note that we should also check that access is correctly dealt with in the REST endpoint (provided by the l10n_remote module). Do we need the L10nServerContributorResource::access() method?

Thanks! Next steps:

• Figuring out why we need the L10nRemoteSubscriber event subscriber for, and either remove debug traces or remove it completely.
• Properly check access in L10nServerContributorResource::access(): user should be a member of the translation team. That being said, we most certainly should implement access checks properly in 🐛 Translation suggestion creation and moderation don't provide group permission Active instead.
• Deal with authentication ( 📌 Localize client authentication with d.o infrastructure Active ), instead of the temporary basic auth mechanism.

Work happens in 📌 New endpoints for l10n_client_contributor Needs review .

Solved in this commit.

+1 for RTBC

Thanks everyone!

You rock!

@drumm we added some instructions in the README. Is it OK to you?

FMB → created an issue.

Do you need the migrations to be running constantly or is it just a one-off?

One-off.

Would uploading the DB that you have locally (where I assume migrations where tested against) work? You wouldn’t be able to run migrations but would be able to use the site, right?

Yes, that would do.

We would like to be able to allow testers to work on a common instance. We do not really need SSO right now. How long until you figure out a way to access the D7 database? Can't you work with a dump in the meantime? It is certainly far from being as huge as on drupal.org ^^

It took longer than expected™, but we have finally finished testing migrations. Drupal core and projects are up to date, and I added some instructions in the README file. We feel the time has come to set up a staging instance. Please tell us if something is missing.

Thanks again!

Thanks!